On Thu, Oct 28, 2021 at 09:17:35AM +0200, David Hildenbrand wrote: > On 28.10.21 06:31, Peter Xu wrote: > > Add a pre-plug hook for x86-iommu, so that we can detect vfio-pci devices > > before realizing the vIOMMU device. > > > > When the guest contains both the x86 vIOMMU and vfio-pci devices, the user > > needs to specify the x86 vIOMMU before the vfio-pci devices. The reason is, > > vfio_realize() calls pci_device_iommu_address_space() to fetch the correct > > dma > > address space for the device, while that API can only work right after the > > vIOMMU device initialized first. > > > > For example, the iommu_fn() that is used in > > pci_device_iommu_address_space() is > > only setup in realize() of the vIOMMU devices. > > > > For a long time we have had libvirt making sure that the ordering is > > correct, > > however from qemu side we never fail a guest from booting even if the > > ordering > > is specified wrongly. When the order is wrong, the guest will encounter > > misterious error when operating on the vfio-pci device because in QEMU we'll > > still assume the vfio-pci devices are put into the default DMA domain > > (which is > > normally the direct GPA mapping), so e.g. the DMAs will never go right. > > > > This patch fails the guest from booting when we detected such errornous > > cmdline > > specified, then the guest at least won't encounter weird device behavior > > after > > booted. The error message will also help the user to know how to fix the > > issue. > > > > Cc: Alex Williamson <alex.william...@redhat.com> > > Suggested-by: Igor Mammedov <imamm...@redhat.com> > > Signed-off-by: Peter Xu <pet...@redhat.com> > > I think that's a big improvement. I ran into this issue myself and found > the documentation at https://wiki.qemu.org/Features/VT-d at one time > ("Meanwhile, the intel-iommu device must be specified as the first > device in the parameter list (before all the rest of the devices). "). > > So feel free to add my > > Acked-by: David Hildenbrand <da...@redhat.com>
Thanks, will do. > > @@ -172,4 +172,12 @@ void x86_iommu_iec_notify_all(X86IOMMUState *iommu, > > bool global, > > * @out: Output MSI message > > */ > > void x86_iommu_irq_to_msi_message(X86IOMMUIrq *irq, MSIMessage *out); > > + > > +/** > > + * x86_iommu_pre_plug: called before plugging the iommu device > > + * @X86IOMMUState: the pointer to x86 iommu state > > + * @errp: the double pointer to Error, set if we want to fail the plug > > + */ > > I'd drop that documentation because it's essentially just how any other > pre_plug handlers works. But maybe it's just me that knows how the whole > hotplug machinery works, so ... Yes the documentation is not very helpful because it shouldn't be called randomly but only in the machine pre-plug hook of x86. It's just trying to not be the 1st one exported function in the header that does not have a comment. Thanks, -- Peter Xu
