Re: [PATCH 4/8] acpi_build: Use IOMMU pci device to build IOMMU device ID

On 5/11/26 8:39 AM, Sairaj Kodilkar wrote:
> Currently, build_amd_iommu() uses "addr" property to build the device ID for
> IOMMU device and advertise it throught IVRS. But this property does not encode
> IOMMU bus. This will be a problem if IOMMU is attached to different bus.
> Hence use iommu pci device which provides bus, to build the IOMMU device ID.
> 

Minor nit: I'd like to use "DeviceID" when possible, again with the goal of
being able to quickly find matches to the relevant sections in the
documentation. So making minor changes to the subject and commit message, I
propose:

acpi_build: Use IOMMU pci device to build IOMMU DeviceID

Currently, build_amd_iommu() uses the "addr" property to build a DeviceID
for an IOMMU device and advertise it through IVRS. But this property does
not encode the IOMMU bus number, only the device and function numbers. This
creates an incorrect DeviceID when the IOMMU is attached to a bus other
than the default one with bus number 0.

Build the DeviceID using the complete BDF from the IOMMU PCI device.

> Signed-off-by: Sairaj Kodilkar <[email protected]>
> Reviewed-by: Vasant Hegde <[email protected]>
> ---
>  hw/i386/acpi-build.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index e4ad01eec037..718e3f546b18 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1752,10 +1752,13 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
>                  const char *oem_table_id)
>  {
>      AMDVIState *s = AMD_IOMMU_DEVICE(x86_iommu_get_default());
> +    PCIDevice *iommu_dev = &(s->pci->dev);
>      GArray *ivhd_blob = g_array_new(false, true, 1);
>      AcpiTable table = { .sig = "IVRS", .rev = 1, .oem_id = oem_id,
>                          .oem_table_id = oem_table_id };
>      uint64_t feature_report;
> +    int iommu_bus = pci_bus_num(pci_get_bus(iommu_dev));
> +    uint16_t iommu_devid = PCI_BUILD_BDF(iommu_bus, iommu_dev->devfn);
>  
Instead of the three additions above, the last line can be only:

+    uint16_t iommu_devid = pci_get_bdf(&s->pci->dev);

taking the example from the virtio_iommu call to build_viot() in acpi_build().

Otherwise:
Reviewed-by: Alejandro Jimenez <[email protected]>

>      acpi_table_begin(&table, table_data);
>      /* IVinfo - IO virtualization information common to all
> @@ -1816,9 +1819,7 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
>      /* IVHD length */
>      build_append_int_noprefix(table_data, ivhd_blob->len + 24, 2);
>      /* DeviceID */
> -    build_append_int_noprefix(table_data,
> -                              object_property_get_int(OBJECT(s->pci), "addr",
> -                                                      &error_abort), 2);
> +    build_append_int_noprefix(table_data, iommu_devid, 2);
>      /* Capability offset */
>      build_append_int_noprefix(table_data, s->pci->capab_offset, 2);
>      /* IOMMU base address */
> @@ -1850,10 +1851,9 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
>  
>      /* IVHD length */
>      build_append_int_noprefix(table_data, ivhd_blob->len + 40, 2);
> +
>      /* DeviceID */
> -    build_append_int_noprefix(table_data,
> -                              object_property_get_int(OBJECT(s->pci), "addr",
> -                                                      &error_abort), 2);
> +    build_append_int_noprefix(table_data, iommu_devid, 2);
>      /* Capability offset */
>      build_append_int_noprefix(table_data, s->pci->capab_offset, 2);
>      /* IOMMU base address */