Re: [PATCH 7/8] KVM: assigned dev: Introduce io_device for MSI-X MMIO accessing

Michael S. Tsirkin
2010-10-21T09:31:30Z

On Thu, Oct 21, 2010 at 11:27:30AM +0200, Avi Kivity wrote:
> On 10/21/2010 08:46 AM, Sheng Yang wrote:
> >> > + r = -EOPNOTSUPP;
> >>
> >> If the guest assigned the device to another guest, it allows the nested
> >> guest to kill the non-nested guest. Need to exit in a graceful fashion.
> >
> >Don't understand... It wouldn't result in kill but return to QEmu/userspace.
>
> What would qemu do on EOPNOTSUPP? It has no way of knowing that
> this was triggered by an unsupported msix access. What can it do?
>
> Best to just ignore the write.
>
> If you're worried about debugging, we can have a trace_kvm_discard()
> tracepoint that logs the address and a type enum field that explains
> why an access was discarded.

The issue is that the same page is used for mask and entry programming.

> >> > + if (addr % PCI_MSIX_ENTRY_SIZE != PCI_MSIX_ENTRY_VECTOR_CTRL) {
> >> > + /* Only allow entry modification when entry was masked */
> >> > + if (!entry_masked) {
> >> > + printk(KERN_WARNING
> >> > + "KVM: guest try to write unmasked MSI-X entry. "
> >> > + "addr 0x%llx, len %d, val 0x%lx\n",
> >> > + addr, len, new_val);
> >> > + r = 0;
> >>
> >> What does the spec says about this situation?
> >
> >As Michael pointed out. The spec said the result is "undefined" indeed.
>
> Ok. Then we should silently discard the write instead of allowing
> the guest to flood host dmesg.
>
> >>
> >> > + goto out;
> >> > + }
> >> > + if (new_val& ~1ul) {
> >>
> >> Is there a #define for this bit?
> >
> >Sorry I didn't find it. mask_msi_irq() also use the number 1... Maybe we can add
> >one.
>
> Yes please.
>
>
> --
> error compiling committee.c: too many arguments to function