| Message ID | 20211021104259.57754-7-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | pci/iommu: Fail early if vfio-pci detected before vIOMMU | expand |
On 21.10.21 12:42, Peter Xu wrote: > There're three places that can be rewritten with the pci_for_each_root_bus() > helper that we just introduced. De-dup the code. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > hw/arm/virt-acpi-build.c | 31 +++++++++++-------------------- > hw/i386/acpi-build.c | 38 ++++++++++---------------------------- > 2 files changed, 21 insertions(+), 48 deletions(-) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 6cec97352b..54b82aa863 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -263,28 +263,20 @@ struct AcpiIortIdMapping { > typedef struct AcpiIortIdMapping AcpiIortIdMapping; > > /* Build the iort ID mapping to SMMUv3 for a given PCI host bridge */ > -static int > -iort_host_bridges(Object *obj, void *opaque) > +static void > +iort_host_bridges(PCIBus *bus, void *opaque) ^ I never understood the use of the line break within a function declaration+definition, especially if everything just fits into a single line. If it's already there, it's most probably in-line with the QEMU coding style. LGTM Reviewed-by: David Hildenbrand <david@redhat.com>
Hi Peter, On 10/21/21 12:42 PM, Peter Xu wrote: > There're three places that can be rewritten with the pci_for_each_root_bus() > helper that we just introduced. De-dup the code. > > Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > --- > hw/arm/virt-acpi-build.c | 31 +++++++++++-------------------- > hw/i386/acpi-build.c | 38 ++++++++++---------------------------- > 2 files changed, 21 insertions(+), 48 deletions(-) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 6cec97352b..54b82aa863 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -263,28 +263,20 @@ struct AcpiIortIdMapping { > typedef struct AcpiIortIdMapping AcpiIortIdMapping; > > /* Build the iort ID mapping to SMMUv3 for a given PCI host bridge */ > -static int > -iort_host_bridges(Object *obj, void *opaque) > +static void > +iort_host_bridges(PCIBus *bus, void *opaque) > { > - GArray *idmap_blob = opaque; > - > - if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) { > - PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus; > - > - if (bus && !pci_bus_bypass_iommu(bus)) { > - int min_bus, max_bus; > + if (!pci_bus_bypass_iommu(bus)) { > + int min_bus, max_bus; > > - pci_bus_range(bus, &min_bus, &max_bus); > + pci_bus_range(bus, &min_bus, &max_bus); > > - AcpiIortIdMapping idmap = { > - .input_base = min_bus << 8, > - .id_count = (max_bus - min_bus + 1) << 8, > - }; > - g_array_append_val(idmap_blob, idmap); > - } > + AcpiIortIdMapping idmap = { > + .input_base = min_bus << 8, > + .id_count = (max_bus - min_bus + 1) << 8, > + }; > + g_array_append_val((GArray *)opaque, idmap); > } > - > - return 0; > } > > static int iort_idmap_compare(gconstpointer a, gconstpointer b) > @@ -318,8 +310,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > if (vms->iommu == VIRT_IOMMU_SMMUV3) { > AcpiIortIdMapping next_range = {0}; > > - object_child_foreach_recursive(object_get_root(), > - iort_host_bridges, smmu_idmaps); > + pci_for_each_root_bus(iort_host_bridges, smmu_idmaps); > > /* Sort the smmu idmap by input_base */ > g_array_sort(smmu_idmaps, iort_idmap_compare); > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index a76b17ed92..3e50acfe35 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -2123,20 +2123,12 @@ insert_scope(PCIBus *bus, PCIDevice *dev, void *opaque) > } > > /* For a given PCI host bridge, walk and insert DMAR scope */ > -static int > -dmar_host_bridges(Object *obj, void *opaque) > +static void > +dmar_host_bridges(PCIBus *bus, void *opaque) > { > - GArray *scope_blob = opaque; > - > - if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) { > - PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus; > - > - if (bus && !pci_bus_bypass_iommu(bus)) { > - pci_for_each_device_under_bus(bus, insert_scope, scope_blob); > - } > + if (!pci_bus_bypass_iommu(bus)) { > + pci_for_each_device_under_bus(bus, insert_scope, opaque); > } > - > - return 0; > } > > /* > @@ -2165,8 +2157,7 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker, const char *oem_id, > * Insert scope for each PCI bridge and endpoint device which > * is attached to a bus with iommu enabled. > */ > - object_child_foreach_recursive(object_get_root(), > - dmar_host_bridges, scope_blob); > + pci_for_each_root_bus(dmar_host_bridges, scope_blob); > > assert(iommu); > if (x86_iommu_ir_supported(iommu)) { > @@ -2329,20 +2320,12 @@ insert_ivhd(PCIBus *bus, PCIDevice *dev, void *opaque) > } > > /* For all PCI host bridges, walk and insert IVHD entries */ > -static int > -ivrs_host_bridges(Object *obj, void *opaque) > +static void > +ivrs_host_bridges(PCIBus *bus, void *opaque) > { > - GArray *ivhd_blob = opaque; > - > - if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) { > - PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus; > - > - if (bus && !pci_bus_bypass_iommu(bus)) { > - pci_for_each_device_under_bus(bus, insert_ivhd, ivhd_blob); > - } > + if (!pci_bus_bypass_iommu(bus)) { > + pci_for_each_device_under_bus(bus, insert_ivhd, opaque); > } > - > - return 0; > } > > static void > @@ -2380,8 +2363,7 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id, > * blob further below. Fall back to an entry covering all devices, which > * is sufficient when no aliases are present. > */ > - object_child_foreach_recursive(object_get_root(), > - ivrs_host_bridges, ivhd_blob); > + pci_for_each_root_bus(ivrs_host_bridges, ivhd_blob); > > if (!ivhd_blob->len) { > /*
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 6cec97352b..54b82aa863 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -263,28 +263,20 @@ struct AcpiIortIdMapping { typedef struct AcpiIortIdMapping AcpiIortIdMapping; /* Build the iort ID mapping to SMMUv3 for a given PCI host bridge */ -static int -iort_host_bridges(Object *obj, void *opaque) +static void +iort_host_bridges(PCIBus *bus, void *opaque) { - GArray *idmap_blob = opaque; - - if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) { - PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus; - - if (bus && !pci_bus_bypass_iommu(bus)) { - int min_bus, max_bus; + if (!pci_bus_bypass_iommu(bus)) { + int min_bus, max_bus; - pci_bus_range(bus, &min_bus, &max_bus); + pci_bus_range(bus, &min_bus, &max_bus); - AcpiIortIdMapping idmap = { - .input_base = min_bus << 8, - .id_count = (max_bus - min_bus + 1) << 8, - }; - g_array_append_val(idmap_blob, idmap); - } + AcpiIortIdMapping idmap = { + .input_base = min_bus << 8, + .id_count = (max_bus - min_bus + 1) << 8, + }; + g_array_append_val((GArray *)opaque, idmap); } - - return 0; } static int iort_idmap_compare(gconstpointer a, gconstpointer b) @@ -318,8 +310,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) if (vms->iommu == VIRT_IOMMU_SMMUV3) { AcpiIortIdMapping next_range = {0}; - object_child_foreach_recursive(object_get_root(), - iort_host_bridges, smmu_idmaps); + pci_for_each_root_bus(iort_host_bridges, smmu_idmaps); /* Sort the smmu idmap by input_base */ g_array_sort(smmu_idmaps, iort_idmap_compare); diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index a76b17ed92..3e50acfe35 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2123,20 +2123,12 @@ insert_scope(PCIBus *bus, PCIDevice *dev, void *opaque) } /* For a given PCI host bridge, walk and insert DMAR scope */ -static int -dmar_host_bridges(Object *obj, void *opaque) +static void +dmar_host_bridges(PCIBus *bus, void *opaque) { - GArray *scope_blob = opaque; - - if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) { - PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus; - - if (bus && !pci_bus_bypass_iommu(bus)) { - pci_for_each_device_under_bus(bus, insert_scope, scope_blob); - } + if (!pci_bus_bypass_iommu(bus)) { + pci_for_each_device_under_bus(bus, insert_scope, opaque); } - - return 0; } /* @@ -2165,8 +2157,7 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker, const char *oem_id, * Insert scope for each PCI bridge and endpoint device which * is attached to a bus with iommu enabled. */ - object_child_foreach_recursive(object_get_root(), - dmar_host_bridges, scope_blob); + pci_for_each_root_bus(dmar_host_bridges, scope_blob); assert(iommu); if (x86_iommu_ir_supported(iommu)) { @@ -2329,20 +2320,12 @@ insert_ivhd(PCIBus *bus, PCIDevice *dev, void *opaque) } /* For all PCI host bridges, walk and insert IVHD entries */ -static int -ivrs_host_bridges(Object *obj, void *opaque) +static void +ivrs_host_bridges(PCIBus *bus, void *opaque) { - GArray *ivhd_blob = opaque; - - if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) { - PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus; - - if (bus && !pci_bus_bypass_iommu(bus)) { - pci_for_each_device_under_bus(bus, insert_ivhd, ivhd_blob); - } + if (!pci_bus_bypass_iommu(bus)) { + pci_for_each_device_under_bus(bus, insert_ivhd, opaque); } - - return 0; } static void @@ -2380,8 +2363,7 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id, * blob further below. Fall back to an entry covering all devices, which * is sufficient when no aliases are present. */ - object_child_foreach_recursive(object_get_root(), - ivrs_host_bridges, ivhd_blob); + pci_for_each_root_bus(ivrs_host_bridges, ivhd_blob); if (!ivhd_blob->len) { /*
There're three places that can be rewritten with the pci_for_each_root_bus() helper that we just introduced. De-dup the code. Signed-off-by: Peter Xu <peterx@redhat.com> --- hw/arm/virt-acpi-build.c | 31 +++++++++++-------------------- hw/i386/acpi-build.c | 38 ++++++++++---------------------------- 2 files changed, 21 insertions(+), 48 deletions(-)