On Thu, Oct 28, 2021 at 08:52:42AM -0600, Alex Williamson wrote:
> > +void x86_iommu_pre_plug(X86IOMMUState *iommu, Error **errp)
> > +{
> > + bool ambiguous = false;
> > + Object *object;
> > +
> > + object = object_resolve_path_type("", TYPE_VFIO_PCI, &ambiguous);
> > + if (object || ambiguous) {
> > + /* There're one or more vfio-pci devices detected */
> > + error_setg(errp, "Please specify all the vfio-pci devices to be
> > after "
> > + "the vIOMMU device");
> > + }
>
> I still really don't buy the argument that vfio-pci is the only driver
> that does "this thing", therefore we can just look for vfio-pci devices
> by name rather than try to generically detect devices that have this
> dependency. That seems short sighted.
>
> I've already suggested that pci-core could record on the PCIDevice
> structure if the device address space has been accessed. We could also
> do something like create a TYPE_PCI_AS_DEVICE class derived from
> TYPE_PCI_DEVICE and any PCI drivers that make use of the device address
> space before machine-init-done would be of this class. That could even
> be enforced by pci_device_iommu_address_space() and would allow the
> same sort of object resolution as used here. Thanks,
Sorry Alex, I didn't receive any follow up so I thought you were fine with it.
I was always fine with either way, though I think another parent class would be
an overkill just for this. Would you think below acceptable?
---8<---
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 5cdf1d4298..2156b5d3ed 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3266,6 +3266,7 @@ static void vfio_pci_dev_class_init(ObjectClass *klass,
void *data)
pdc->exit = vfio_exitfn;
pdc->config_read = vfio_pci_read_config;
pdc->config_write = vfio_pci_write_config;
+ pdc->require_consolidated_iommu_as = true;
}
static const TypeInfo vfio_pci_dev_info = {
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 6813f128e0..ffddc766ba 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -239,6 +239,14 @@ struct PCIDeviceClass {
*/
bool is_bridge;
+ /*
+ * Set this to true when a pci device needs consolidated result from the
+ * pci_device_iommu_address_space() in its realize() fn. This also means
+ * when specified in cmdline, this "-device" parameter needs to be put
+ * before the vIOMMU devices so as to make it work.
+ */
+ bool require_consolidated_iommu_as;
+
/* rom bar */
const char *romfile;
};
---8<---
Then I'll need to pick the dropped patch back on pci scanning, since then I
won't be able to use object_resolve_path_type() anymore, and I'll need to check
up PCIDeviceClass instead.
Michael, Igor, others - any objections?
--
Peter Xu
