On Tue, Jan 10, 2012 at 07:21:01PM +0100, Jan Kiszka wrote: > > ATM writes to msi/msix mask bit have no effect for assigned > > devices. For virtio, they are implemented by deassigning irqfd > > which is a very slow operation (rcu write side). > > > > Instead, When guest writes to mask, qemu can set/clear by calling > > this ioctl. > > Isn't that effort better invested in proper in-kernel mask emulation for > MSI-X? This gives us a working implementation fo free. Whether MSIX mask writes are worth accelerating in kernel I'm not 100% sure. But IMO this shows it is a more generic interface. > > > >>> > >>>> As long as the > >>>> +guest masks the legacy INTx, the kernel will refrain from unmasking it at > >>>> +hardware level and will not assert the guest's IRQ line. User space is still > >>>> +responsible for applying this state to the assigned device's real config space. > >>> > >>> Can this be made more explicit? You mean writing into 1st > >>> byte of PCI control, right? > >> > >> For sure, I can state this. > >> > >>> > >>>> +To avoid that the kernel overwrites the state user space wants to set, > >>>> +KVM_ASSIGN_SET_INTX_MASK has to be called prior to updating the config space. > >>> > >>> This looks like a strange requirement, could you explain how > >>> this helps avoid races? > >> > >> By declaring the target state of the INTx bit first to the kernel, > >> concurrent changes of the kernel while user space performs a > >> read-modify-write will not lead to an old mask state being written. > > > > I note you don't require KVM_ASSIGN_SET_INTX_MASK before read though. > > Further, userspace might cache the control byte. If we require > > it not to do it, we probably need to be explicit? > > User space can do with the control byte what it wants - kernel can't > help this anyway. I should just tell the kernel ahead of time what the > next INTx mask state will be. That particularly avoids that the kernel > sets the mask when user space wants it cleared. The other way around is > actually unproblematic as we check KVM_ASSIGN_SET_INTX_MASK before > delivering the IRQ to the guest. > > > > >>> This also raises questions about > >>> what should be done to write a bit unrelated to masking. > >> > >> Just write it, using the INTx state user space maintains. In the worst > >> case, some masking done by the kernel in the meantime will be > >> overwritten, leading to a single spurious but harmless IRQ. That event > >> won't be delivered to the guest unless it is ready to receive it - as we > >> updated the mask state prior to writing to the config space. The point > >> is that the kernel mechanism has to deal with crazy user space clearing > >> the mask for whatever reason again. > > > > I guess the point is that we need to avoid is this: > > > > kernel masks bit > > read > > kernel unmasks bit > > write > > > > I'm not sure I understand how the text above suggests > > doing this in a race free manner. > > User space must not write INTx as read from the hardware but according > to its own view. Then the above is harmless. > > > > > > > A simple way would be to ask userspace to always clear > > this bit on writes. What do you think? > > That or - sounds more consistent - writing the state that user space > exposes to the guest anyway. That (in addition to the ordering > requirement) should be clearly stated in the doc, I agree. > > Jan Yes, I agree it all works, just needs clear documentation. In summary, userspace must ignore the value of the bit it reads from device. > -- > Siemens AG, Corporate Technology, CT T DE IT 1 > Corporate Competence Center Embedded Linux -- 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
