On Thu, Mar 03, 2016 at 01:19:09PM +0200, Marcel Apfelbaum wrote:
> On 03/03/2016 12:45 PM, Michael S. Tsirkin wrote:
> >On Thu, Mar 03, 2016 at 12:12:27PM +0200, Marcel Apfelbaum wrote:
> >>>>+int msi_init(struct PCIDevice *dev, uint8_t offset, unsigned int
> >>>>nr_vectors,
> >>>>+ bool msi64bit, bool msi_per_vector_mask, Error **errp)
> >>>> {
> >>>> unsigned int vectors_order;
> >>>>- uint16_t flags;
> >>>>+ uint16_t flags; /* Message Control register value */
> >>>> uint8_t cap_size;
> >>>> int config_offset;
> >>>>
> >>>> if (!msi_supported) {
> >>>>+ error_setg(errp, "MSI is not supported by interrupt controller");
> >>>> return -ENOTSUP;
> >>>
> >>>First failure mode: board does not support MSI (-ENOTSUP).
> >>>
> >>>Question to the PCI guys: why is this even an error? A device with
> >>>capability MSI should work just fine in such a board.
> >>
> >>Hi Markus,
> >>
> >>Adding Jan Kiszka, maybe he can help.
> >>
> >>That's a fair question. Is there any history for this decision?
> >>The board not supporting MSI has nothing to do with the capability being
> >>there.
> >>The HW should not change because the board doe not support it.
> >>
> >>The capability should be present but not active.
> >
> >Digging in git log will tell you why we have the msi_supported flag:
> >
> >commit 02eb84d0ec97f183ac23ee939403a139e8849b1d ("qemu/pci: MSI-X support
> >functions")
> >
> > This is a safety measure to avoid breaking platforms which should
> > support
> > MSI-X but currently lack this in the interrupt controller emulation.
> >
> >in other words, on some platforms we must hide msi support from devices
> >because otherwise guests will try to use it, and our emulation is
> >incomplete.
>
>
> OK, thanks. So the flag should be "msi_broken" or
> "msi_present_but_not_implemented" and not
> "msi_supported" that leads (at least me) to the assumption that some platform
> *does not support msi*
> rather than it supports it, but we don't emulate it.
>
>
> >
> >And the conclusion from that is that for msi_init to fail silently is
> >at the moment the right thing to do.
>
> But this is not the only thing we do, we are modifying the PCI devices. We
> could fail starting the VM
> if a device supporting MSI is added on a platform with broken msi, but this
> will prevent us to use
> assigned devices. Emulated devices should be created with a specific
> "msi=off" flag.
>
> Thanks,
> Marcel
That will just break a bunch of valid configurations, for no real
benefit to users.
> >
> >The only other reason for it to fail is pci config space corruption,
> >this probably never happens in practice.
> >
