On Tue, Jul 14, 2009 at 04:25:07PM -0700, Chris Wright wrote: > * Michael S. Tsirkin (mst@xxxxxxxxxx) wrote: > > On Tue, Jul 14, 2009 at 11:03:38AM -0700, Chris Wright wrote: > > > * Michael S. Tsirkin (mst@xxxxxxxxxx) wrote: > > > > - remove irqcontrol: user can enable interrupts by > > > > writing command register directly > > > > > > Sorry if I gave you the impression that removing was needed. > > > I actually think the irqcontrol was useful since it's atomic. > > > > What I'm saying, it is not strictly needed (user can safely write 0 in > > that register and worst case you just get an extra interrupt). So let's > > make the decision on what does irqcontrol do when we have a pressing > > need for an extra kernel/user interface. > > OK. My concern is theoretical (as in the current design wouldn't > trigger an issue): > > cmd = pread() > ------+ > \ > cmd &= ~INTX_DISABLE +----+ > / | > -------+ | > pwrite(cmd) | > - > During this window Command Reg can change[1] and cmd is stale > > [1] due to irqhandler (doesn't matter in this case since it touches > same bit). or due to some other thread updating Command Reg (also > doesn't matter since this does not happen right now). Right. Note that the limitation that only a single thread should touch config space through sysfs applies to any sub-byte field. If we wanted to make such ops atomic, pci devices would need some kind of compare and swap ioctl. -- MST -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html