[PATCH 0/8] acpi_build: Refactor and cleanup AMD IVRS build

[PATCH 0/8] acpi_build: Refactor and cleanup AMD IVRS build

[Sairaj Kodilkar]

This series aims to improve redability and maintainibility of the the AMD IVRS
building by introducing macros and packed structs which represents IVRS, IVHDs
headers and IVHD device entries.

The goal is to land a maintainable baseline before adding AMD
hardware-accelerated vIOMMU support in QEMU, where we will require multiple
vIOMMUs attached to different pcie-pxb buses in order to serve set of
passthrough devices.

Please refer to AMD IOMMU spec [1] for more details about IVRS.
[1] https://docs.amd.com/api/khub/documents/GD6kOXjzWsek8QUbn_qMvg/content

Sairaj Kodilkar (8):
  tests/acpi: x86: Allow IVRS acpi table changes
  amd_iommu: update PA, GVA and VA size macros
  amd_iommu: Return empty efr for stub call
  acpi_build: Use IOMMU pci device to build IOMMU device ID
  acpi_build: Introduce necessary macros and structs for AMD IOMMU IVRS
  acpi_build: Build IVRS feature report using extended feature register
  acpi_build: Cleanup AMD IOMMU IVRS building
  tests/acpi: x86: update golden masters for IVRS

 hw/i386/acpi-build.c     | 191 +++++++++++++++++++--------------------
 hw/i386/acpi-build.h     |  93 ++++++++++++++++++-
 hw/i386/amd_iommu-stub.c |   2 +-
 hw/i386/amd_iommu.c      |   2 +-
 hw/i386/amd_iommu.h      |  12 ++-
 5 files changed, 195 insertions(+), 105 deletions(-)

-- 
2.34.1

[PATCH 1/8] tests/acpi: x86: Allow IVRS acpi table changes

[Sairaj Kodilkar]

Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8bf4..33e998165591 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,2 @@
 /* List of comma-separated changed AML files to ignore */
+tests/data/acpi/x86/q35/IVRS.ivrs
-- 
2.34.1

[PATCH 2/8] amd_iommu: update PA, GVA and VA size macros

[Sairaj Kodilkar]

Current implemenation supports upto 64 bit IOVA and upto 52 bit PA.
Modify related macros so that guest can see right size.

Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
---
 hw/i386/acpi-build.c | 2 +-
 hw/i386/amd_iommu.c  | 2 +-
 hw/i386/amd_iommu.h  | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 9446a9f862ca..e4ad01eec037 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1763,7 +1763,7 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
      */
     build_append_int_noprefix(table_data,
                              (1UL << 0) | /* EFRSup */
-                             (40UL << 8), /* PASize */
+                             AMDVI_PA_SIZE_52,
                              4);
     /* reserved */
     build_append_int_noprefix(table_data, 0, 8);
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 789e09d6f2bc..7c52f9ade6e6 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -2450,7 +2450,7 @@ static void amdvi_pci_realize(PCIDevice *pdev, Error **errp)
                  0xff000000);
     pci_set_long(pdev->config + s->capab_offset + AMDVI_CAPAB_MISC, 0);
     pci_set_long(pdev->config + s->capab_offset + AMDVI_CAPAB_MISC,
-            AMDVI_MAX_PH_ADDR | AMDVI_MAX_GVA_ADDR | AMDVI_MAX_VA_ADDR);
+            AMDVI_PA_SIZE_52 | AMDVI_GVA_SIZE_48 | AMDVI_VA_SIZE_64);
 }
 
 static void amdvi_sysbus_reset(DeviceState *dev)
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 302ccca5121f..fe8f4a6cdc74 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -245,9 +245,9 @@
 #define AMDVI_PAGE_SHIFT_4K 12
 #define AMDVI_PAGE_MASK_4K      GENMASK64(63, 12)
 
-#define AMDVI_MAX_GVA_ADDR      (2UL << 5)
-#define AMDVI_MAX_PH_ADDR       (40UL << 8)
-#define AMDVI_MAX_VA_ADDR       (48UL << 15)
+#define AMDVI_GVA_SIZE_48       (2UL << 5)
+#define AMDVI_PA_SIZE_52        (52UL << 8)
+#define AMDVI_VA_SIZE_64        (64UL << 15)
 
 /* Completion Wait data size */
 #define AMDVI_COMPLETION_DATA_SIZE    8
-- 
2.34.1

[PATCH 3/8] amd_iommu: Return empty efr for stub call

[Sairaj Kodilkar]

The stub call should not return normal EFR as it is used as place holder
when actual AMD IOMMU calls are not available. Hence return 0.

Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>

---
 hw/i386/amd_iommu-stub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/amd_iommu-stub.c b/hw/i386/amd_iommu-stub.c
index d62a3732e60f..fb36dee903ee 100644
--- a/hw/i386/amd_iommu-stub.c
+++ b/hw/i386/amd_iommu-stub.c
@@ -22,5 +22,5 @@
 
 uint64_t amdvi_extended_feature_register(AMDVIState *s)
 {
-    return AMDVI_DEFAULT_EXT_FEATURES;
+    return 0;
 }
-- 
2.34.1

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

[Sairaj Kodilkar]

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.

Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
---
 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);
 
     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 */
-- 
2.34.1

[PATCH 5/8] acpi_build: Introduce necessary macros and structs for AMD IOMMU IVRS

[Sairaj Kodilkar]

Introduce necessary macros, along with packed structs which represents
the AMD IVRS data structure, to hold the necessary information. This
will improve readability of the current code.

Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
---
 hw/i386/acpi-build.h | 92 ++++++++++++++++++++++++++++++++++++++++++++
 hw/i386/amd_iommu.h  |  6 +++
 2 files changed, 98 insertions(+)

diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
index 8ba3c33e4831..9fd60a186db1 100644
--- a/hw/i386/acpi-build.h
+++ b/hw/i386/acpi-build.h
@@ -2,10 +2,102 @@
 #ifndef HW_I386_ACPI_BUILD_H
 #define HW_I386_ACPI_BUILD_H
 #include "hw/acpi/acpi-defs.h"
+#include "qemu/bitops.h"
 
 extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio;
 
 void acpi_setup(void);
 Object *acpi_get_i386_pci_host(void);
 
+#define AMD_IVINFO_EFR_SUP       BIT(0)
+
+#define AMD_IVHD_FLAG_HT_TUN_EN    BIT(0)
+#define AMD_IVHD_FLAG_IOTLB_SUP    BIT(4)
+#define AMD_IVHD_FLAG_PREF_SUP     BIT(6)
+#define AMD_IVHD_FLAG_PPR_SUP      BIT(7)
+
+#define AMD_IVHD_FEATURE_REPORT_XT_SUP_SHIFT      (0)
+#define AMD_IVHD_FEATURE_REPORT_GT_SUP_SHIFT      (2)
+#define AMD_IVHD_FEATURE_REPORT_GLX_SUP_SHIFT     (3)
+#define AMD_IVHD_FEATURE_REPORT_GA_SUP_SHIFT      (6)
+#define AMD_IVHD_FEATURE_REPORT_GATS_SHIFT        (28)
+#define AMD_IVHD_FEATURE_REPORT_HATS_SHIFT        (30)
+
+#define AMD_IVHD_ATTRIBUTES_HATDIS_SHIFT        (0)
+
+#define AMD_IVHD_DEVICE_ENTRY_TYPE_RESERVED             (0)
+#define AMD_IVHD_DEVICE_ENTRY_TYPE_ALL                  (1)
+#define AMD_IVHD_DEVICE_ENTRY_TYPE_SELECT               (2)
+#define AMD_IVHD_DEVICE_ENTRY_TYPE_START_RANGE          (3)
+#define AMD_IVHD_DEVICE_ENTRY_TYPE_END_RANGE            (4)
+#define AMD_IVHD_DEVICE_ENTRY_TYPE_ALIAS_START_RANGE    (0x43)
+#define AMD_IVHD_DEVICE_ENTRY_TYPE_SPECIAL_DEVICE       (0x48)
+
+#define IVHD_VARIETY_IOAPIC (1)
+#define IVHD_VARIETY_HPET   (2)
+
+/*
+ * Vendor(AMD) specific fields in the IVRS header
+ * Excludes fields in ACPI table header
+ */
+typedef
+struct AmdIvrsVendorHdr {
+    uint32_t ivinfo;
+    uint64_t reserved;
+} __attribute__((packed)) AmdIvrsVendorHdr;
+
+/* IVHD type 10h */
+typedef
+struct AmdIvhdHdr10 {
+    uint8_t type;
+    uint8_t flags;
+    uint16_t length;
+    uint16_t devid;
+    uint16_t capab_offset;
+    uint64_t base_addr;
+    uint16_t pci_seg;
+    uint16_t iommu_info;
+    uint32_t iommu_feature_report;
+} __attribute__((packed)) AmdIvhdHdr10;
+
+/* IVHD type 11h */
+typedef
+struct AmdIvhdHdr11 {
+    uint8_t type;
+    uint8_t flags;
+    uint16_t length;
+    uint16_t devid;
+    uint16_t capab_offset;
+    uint64_t base_addr;
+    uint16_t pci_seg;
+    uint16_t iommu_info;
+    uint32_t iommu_attributes;
+    uint64_t efr;
+    uint64_t efr2;
+} __attribute__((packed)) AmdIvhdHdr11;
+
+typedef
+struct AmdIvhdDeviceEntry {
+    uint8_t type;
+    uint16_t devid;
+    uint8_t dte_setting;
+} __attribute__((packed)) AmdIvhdDeviceEntry;
+
+typedef
+struct AmdIvhdDeviceEntryExt {
+    uint8_t type;
+    uint16_t devid_a;
+    uint8_t dte_setting;
+    union {
+        struct {
+            uint8_t handle;
+            uint16_t devid_b;
+            uint8_t variety;
+        } __attribute__((packed));
+        struct {
+            uint32_t ext_dte_setting;
+        } __attribute__((packed));
+    };
+} __attribute__((packed)) AmdIvhdDeviceEntryExt;
+
 #endif
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index fe8f4a6cdc74..d97ddbd612dc 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -175,9 +175,15 @@
 #define AMDVI_DTE_QUAD3_RESERVED        (GENMASK64(14, 0) | GENMASK64(53, 48))
 
 /* AMDVI paging mode */
+#define AMDVI_GATS_MODE_SHIFT           (12)
+#define AMDVI_GATS_MODE_MASK            (3ULL <<  12)
 #define AMDVI_GATS_MODE                 (2ULL <<  12)
+#define AMDVI_HATS_MODE_SHIFT           (10)
+#define AMDVI_HATS_MODE_MASK            (3ULL <<  10)
 #define AMDVI_HATS_MODE                 (2ULL <<  10)
 #define AMDVI_HATS_MODE_RESERVED        (3ULL <<  10)
+#define AMDVI_GLX_SUP_SHIFT             (14)
+#define AMDVI_GLX_SUP_MASK              (3ULL << 14)
 
 /* Page Table format */
 
-- 
2.34.1

[PATCH 6/8] acpi_build: Build IVRS feature report using extended feature register

[Sairaj Kodilkar]

Currently IVRS feature report values are hardcoded, this is difficult to
maintain as any updates to extended feature must be synced. Along with
it current feature report does not have GATS and HATS set. Hence use
the extended feature registers to build the IVRS feature report.

Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
---
 hw/i386/acpi-build.c | 38 +++++++++++++++++++++++++++++++-------
 hw/i386/acpi-build.h |  1 -
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 718e3f546b18..82208e06e155 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1747,6 +1747,36 @@ ivrs_host_bridges(Object *obj, void *opaque)
     return 0;
 }
 
+/*
+ * IVHD type 0x10 reports features using Feature Reporting field, which has
+ * different format than extended feature register (EFR) in the IOMMU MMIO
+ * space.
+ *
+ * Convert the EFR format to feature reporting format.
+ */
+static uint32_t
+get_amd_ivhd_feature_report(AMDVIState *s)
+{
+    uint64_t feature = amdvi_extended_feature_register(s);
+    uint32_t is_gt = !!(feature & AMDVI_FEATURE_GT);
+    uint32_t is_ga = !!(feature & AMDVI_FEATURE_GA);
+    uint64_t glx_sup = (feature & AMDVI_GLX_SUP_MASK) >> AMDVI_GLX_SUP_SHIFT;
+    uint64_t hats_mode = (feature & AMDVI_HATS_MODE_MASK) >>
+                         AMDVI_HATS_MODE_SHIFT;
+    uint64_t gats_mode = (feature & AMDVI_GATS_MODE_MASK) >>
+                         AMDVI_GATS_MODE_SHIFT;
+    uint32_t feature_report;
+
+    feature_report = s->xtsup << AMD_IVHD_FEATURE_REPORT_XT_SUP_SHIFT |
+                     is_gt << AMD_IVHD_FEATURE_REPORT_GT_SUP_SHIFT |
+                     glx_sup << AMD_IVHD_FEATURE_REPORT_GLX_SUP_SHIFT |
+                     is_ga << AMD_IVHD_FEATURE_REPORT_GA_SUP_SHIFT |
+                     hats_mode << AMD_IVHD_FEATURE_REPORT_HATS_SHIFT |
+                     gats_mode << AMD_IVHD_FEATURE_REPORT_GATS_SHIFT;
+
+    return feature_report;
+}
+
 static void
 build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
                 const char *oem_table_id)
@@ -1829,13 +1859,7 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
     /* IOMMU info */
     build_append_int_noprefix(table_data, 0, 2);
     /* IOMMU Feature Reporting */
-    feature_report = (48UL << 30) | /* HATS   */
-                     (48UL << 28) | /* GATS   */
-                     (1UL << 2)   | /* GTSup  */
-                     (1UL << 6);    /* GASup  */
-    if (s->xtsup) {
-        feature_report |= (1UL << 0); /* XTSup */
-    }
+    feature_report = get_amd_ivhd_feature_report(s);
     build_append_int_noprefix(table_data, feature_report, 4);
 
     /* IVHD entries as found above */
diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
index 9fd60a186db1..d81b7890e6e2 100644
--- a/hw/i386/acpi-build.h
+++ b/hw/i386/acpi-build.h
@@ -1,4 +1,3 @@
-
 #ifndef HW_I386_ACPI_BUILD_H
 #define HW_I386_ACPI_BUILD_H
 #include "hw/acpi/acpi-defs.h"
-- 
2.34.1

[PATCH 7/8] acpi_build: Cleanup AMD IOMMU IVRS building

[Sairaj Kodilkar]

Use structs and macros to improve the readability and maintainability of
the the code. 

Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
---
 hw/i386/acpi-build.c | 149 +++++++++++++++++--------------------------
 1 file changed, 59 insertions(+), 90 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 82208e06e155..e18c9be801a2 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1663,11 +1663,13 @@ static void
 insert_ivhd(PCIBus *bus, PCIDevice *dev, void *opaque)
 {
     GArray *table_data = opaque;
-    uint32_t entry;
+    AmdIvhdDeviceEntry entry = {};
 
     /* "Select" IVHD entry, type 0x2 */
-    entry = PCI_BUILD_BDF(pci_bus_num(bus), dev->devfn) << 8 | 0x2;
-    build_append_int_noprefix(table_data, entry, 4);
+    entry.type = AMD_IVHD_DEVICE_ENTRY_TYPE_SELECT;
+    entry.devid = PCI_BUILD_BDF(pci_bus_num(bus), dev->devfn);
+
+    g_array_append_vals(table_data, &entry, sizeof(entry));
 
     if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) {
         PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(dev));
@@ -1691,11 +1693,14 @@ insert_ivhd(PCIBus *bus, PCIDevice *dev, void *opaque)
              */
             if (sec == sub) { /* leaf bus */
                 /* "Start of Range" IVHD entry, type 0x3 */
-                entry = PCI_BUILD_BDF(sec, PCI_DEVFN(0, 0)) << 8 | 0x3;
-                build_append_int_noprefix(table_data, entry, 4);
+                entry.type = AMD_IVHD_DEVICE_ENTRY_TYPE_START_RANGE;
+                entry.devid = PCI_BUILD_BDF(sec, PCI_DEVFN(0, 0));
+                g_array_append_vals(table_data, &entry, sizeof(entry));
+
                 /* "End of Range" IVHD entry, type 0x4 */
-                entry = PCI_BUILD_BDF(sub, PCI_DEVFN(31, 7)) << 8 | 0x4;
-                build_append_int_noprefix(table_data, entry, 4);
+                entry.type = AMD_IVHD_DEVICE_ENTRY_TYPE_END_RANGE;
+                entry.devid = PCI_BUILD_BDF(sub, PCI_DEVFN_MAX - 1);
+                g_array_append_vals(table_data, &entry, sizeof(entry));
             } else {
                 pci_for_each_device(sec_bus, sec, insert_ivhd, table_data);
             }
@@ -1708,24 +1713,26 @@ insert_ivhd(PCIBus *bus, PCIDevice *dev, void *opaque)
              * express bridges, just as in pci_device_iommu_address_space().
              * DeviceIDa vs DeviceIDb as per the AMD IOMMU spec.
              */
-            uint16_t dev_id_a, dev_id_b;
+            AmdIvhdDeviceEntryExt entry_ext = {};
 
-            dev_id_a = PCI_BUILD_BDF(sec, PCI_DEVFN(0, 0));
+            entry_ext.type = AMD_IVHD_DEVICE_ENTRY_TYPE_ALIAS_START_RANGE;
+            entry_ext.devid_a = PCI_BUILD_BDF(sec, PCI_DEVFN(0, 0));
 
             if (pci_is_express(dev) &&
                 pcie_cap_get_type(dev) == PCI_EXP_TYPE_PCI_BRIDGE) {
-                dev_id_b = dev_id_a;
+                entry_ext.devid_b = entry_ext.devid_a;
             } else {
-                dev_id_b = PCI_BUILD_BDF(pci_bus_num(bus), dev->devfn);
+                entry_ext.devid_b = PCI_BUILD_BDF(pci_bus_num(bus),
+                                                  dev->devfn);
             }
 
             /* "Alias Start of Range" IVHD entry, type 0x43, 8 bytes */
-            build_append_int_noprefix(table_data, dev_id_a << 8 | 0x43, 4);
-            build_append_int_noprefix(table_data, dev_id_b << 8 | 0x0, 4);
+            g_array_append_vals(table_data, &entry_ext, sizeof(entry_ext));
 
             /* "End of Range" IVHD entry, type 0x4 */
-            entry = PCI_BUILD_BDF(sub, PCI_DEVFN(31, 7)) << 8 | 0x4;
-            build_append_int_noprefix(table_data, entry, 4);
+            entry.type = AMD_IVHD_DEVICE_ENTRY_TYPE_END_RANGE;
+            entry.devid = PCI_BUILD_BDF(sub, PCI_DEVFN_MAX - 1);
+            g_array_append_vals(table_data, &entry, sizeof(entry));
         }
     }
 }
@@ -1786,20 +1793,20 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
     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);
+    AmdIvrsVendorHdr ivrs_hdr = {};
+    AmdIvhdHdr10 ivhd10 = {};
+    AmdIvhdHdr11 ivhd11 = {};
 
     acpi_table_begin(&table, table_data);
     /* IVinfo - IO virtualization information common to all
      * IOMMU units in a system
      */
-    build_append_int_noprefix(table_data,
-                             (1UL << 0) | /* EFRSup */
-                             AMDVI_PA_SIZE_52,
-                             4);
-    /* reserved */
-    build_append_int_noprefix(table_data, 0, 8);
+    ivrs_hdr.ivinfo = AMD_IVINFO_EFR_SUP | AMDVI_GVA_SIZE_48 |
+                      AMDVI_PA_SIZE_52 | AMDVI_VA_SIZE_64;
+
+    g_array_append_vals(table_data, &ivrs_hdr, sizeof(ivrs_hdr));
 
     /*
      * A PCI bus walk, for each PCI host bridge, is necessary to create a
@@ -1817,7 +1824,8 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
          *   These are 4-byte device entries currently reporting the range of
          *   Refer to Spec - Table 95:IVHD Device Entry Type Codes(4-byte)
          */
-        build_append_int_noprefix(ivhd_blob, 0x0000001, 4);
+        AmdIvhdDeviceEntry entry = { .type = AMD_IVHD_DEVICE_ENTRY_TYPE_ALL };
+        g_array_append_vals(ivhd_blob, &entry, sizeof(entry));
     }
 
     /*
@@ -1829,76 +1837,37 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
      * See Linux kernel commit 'c2ff5cf5294bcbd7fa50f7d860e90a66db7e5059'
      */
     if (x86_iommu_ir_supported(x86_iommu_get_default())) {
-        build_append_int_noprefix(ivhd_blob,
-                                 (0x1ull << 56) |           /* type IOAPIC */
-                                 (IOAPIC_SB_DEVID << 40) |  /* IOAPIC devid */
-                                 0x48,                      /* special device */
-                                 8);
-    }
-
-    /* IVHD definition - type 10h */
-    build_append_int_noprefix(table_data, 0x10, 1);
-    /* virtualization flags */
-    build_append_int_noprefix(table_data,
-                             (1UL << 0) | /* HtTunEn      */
-                             (1UL << 4) | /* iotblSup     */
-                             (1UL << 6) | /* PrefSup      */
-                             (1UL << 7),  /* PPRSup       */
-                             1);
-
-    /* IVHD length */
-    build_append_int_noprefix(table_data, ivhd_blob->len + 24, 2);
-    /* DeviceID */
-    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 */
-    build_append_int_noprefix(table_data, s->mr_mmio.addr, 8);
-    /* PCI Segment Group */
-    build_append_int_noprefix(table_data, 0, 2);
-    /* IOMMU info */
-    build_append_int_noprefix(table_data, 0, 2);
-    /* IOMMU Feature Reporting */
-    feature_report = get_amd_ivhd_feature_report(s);
-    build_append_int_noprefix(table_data, feature_report, 4);
-
+        AmdIvhdDeviceEntryExt entry_ext = {
+                    .type = AMD_IVHD_DEVICE_ENTRY_TYPE_SPECIAL_DEVICE,
+                    .devid_b = IOAPIC_SB_DEVID,
+                    .variety = IVHD_VARIETY_IOAPIC
+                };
+
+        g_array_append_vals(ivhd_blob, &entry_ext, sizeof(entry_ext));
+    }
+
+    ivhd10.type = 0x10;
+    ivhd10.flags = AMD_IVHD_FLAG_HT_TUN_EN | AMD_IVHD_FLAG_IOTLB_SUP |
+                   AMD_IVHD_FLAG_PREF_SUP  | AMD_IVHD_FLAG_PPR_SUP;
+    ivhd10.length = ivhd_blob->len + sizeof(ivhd10);
+    ivhd10.devid = iommu_devid;
+    ivhd10.capab_offset = s->pci->capab_offset;
+    ivhd10.base_addr = s->mr_mmio.addr;
+    ivhd10.iommu_feature_report = get_amd_ivhd_feature_report(s);
+    g_array_append_vals(table_data, &ivhd10, sizeof(ivhd10));
     /* IVHD entries as found above */
     g_array_append_vals(table_data, ivhd_blob->data, ivhd_blob->len);
 
-   /* IVHD definition - type 11h */
-    build_append_int_noprefix(table_data, 0x11, 1);
-    /* virtualization flags */
-    build_append_int_noprefix(table_data,
-                             (1UL << 0) | /* HtTunEn      */
-                             (1UL << 4),  /* iotblSup     */
-                             1);
-
-    /* IVHD length */
-    build_append_int_noprefix(table_data, ivhd_blob->len + 40, 2);
-
-    /* DeviceID */
-    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 */
-    build_append_int_noprefix(table_data, s->mr_mmio.addr, 8);
-    /* PCI Segment Group */
-    build_append_int_noprefix(table_data, 0, 2);
-    /* IOMMU info */
-    build_append_int_noprefix(table_data, 0, 2);
-    /* IOMMU Attributes */
-    if (!s->iommu.dma_translation) {
-        build_append_int_noprefix(table_data, (1UL << 0) /* HATDis */, 4);
-    } else {
-        build_append_int_noprefix(table_data, 0, 4);
-    }
-    /* EFR Register Image */
-    build_append_int_noprefix(table_data,
-                              amdvi_extended_feature_register(s),
-                              8);
-    /* EFR Register Image 2 */
-    build_append_int_noprefix(table_data, 0, 8);
-
+    ivhd11.type = 0x11;
+    ivhd11.flags = AMD_IVHD_FLAG_HT_TUN_EN | AMD_IVHD_FLAG_IOTLB_SUP;
+    ivhd11.length = ivhd_blob->len + sizeof(ivhd11);
+    ivhd11.devid = iommu_devid;
+    ivhd11.capab_offset = s->pci->capab_offset;
+    ivhd11.base_addr = s->mr_mmio.addr;
+    ivhd11.iommu_attributes = !s->iommu.dma_translation <<
+                              AMD_IVHD_ATTRIBUTES_HATDIS_SHIFT;
+    ivhd11.efr = amdvi_extended_feature_register(s);
+    g_array_append_vals(table_data, &ivhd11, sizeof(ivhd11));
     /* IVHD entries as found above */
     g_array_append_vals(table_data, ivhd_blob->data, ivhd_blob->len);
 
-- 
2.34.1

[PATCH 8/8] tests/acpi: x86: update golden masters for IVRS

[Sairaj Kodilkar]

Differences between disassembled ASL files for IVRS:

 /*
  * Intel ACPI Component Architecture
  * AML/ASL+ Disassembler version 20200925 (64-bit version)
  * Copyright (c) 2000 - 2020 Intel Corporation
  *
- * Disassembly of tests/data/acpi/x86/q35/IVRS.ivrs, Mon May 11 11:21:47 2026
+ * Disassembly of /tmp/aml-NGQ9O3, Mon May 11 11:21:47 2026
  *
  * ACPI Data Table [IVRS]
  *
  * Format: [HexOffset DecimalOffset ByteLength]  FieldName : FieldValue
  */

 [000h 0000   4]                    Signature : "IVRS"    [I/O Virtualization Reporting Structure]
 [004h 0004   4]                 Table Length : 000000B0
 [008h 0008   1]                     Revision : 01
-[009h 0009   1]                     Checksum : 74
+[009h 0009   1]                     Checksum : 68
 [00Ah 0010   6]                       Oem ID : "BOCHS "
 [010h 0016   8]                 Oem Table ID : "BXPC    "
 [018h 0024   4]                 Oem Revision : 00000001
 [01Ch 0028   4]              Asl Compiler ID : "BXPC"
 [020h 0032   4]        Asl Compiler Revision : 00000001

-[024h 0036   4]          Virtualization Info : 00002801
+[024h 0036   4]          Virtualization Info : 00203441
 [028h 0040   8]                     Reserved : 0000000000000000

 [030h 0048   1]                Subtable Type : 10 [Hardware Definition Block]
 [031h 0049   1]                        Flags : D1
 [032h 0050   2]                       Length : 0038
 [034h 0052   2]                     DeviceId : 0010

 [036h 0054   2]            Capability Offset : 0040
 [038h 0056   8]                 Base Address : 00000000FED80000
 [040h 0064   2]            PCI Segment Group : 0000
 [042h 0066   2]          Virtualization Info : 0000
-[044h 0068   4]            Feature Reporting : 00000044
+[044h 0068   4]            Feature Reporting : A0000044

 [048h 0072   1]                   Entry Type : 02
 [049h 0073   2]                    Device ID : 0000
 [04Bh 0075   1]                 Data Setting : 00

 [04Ch 0076   1]                   Entry Type : 02
 [04Dh 0077   2]                    Device ID : 0008
 [04Fh 0079   1]                 Data Setting : 00

 [050h 0080   1]                   Entry Type : 02
 [051h 0081   2]                    Device ID : 0010
 [053h 0083   1]                 Data Setting : 00

 [054h 0084   1]                   Entry Type : 02
 [055h 0085   2]                    Device ID : 00F8
 [057h 0087   1]                 Data Setting : 00
@@ -98,27 +98,27 @@
 [0A1h 0161   2]                    Device ID : 00FA
 [0A3h 0163   1]                 Data Setting : 00

 [0A4h 0164   1]                   Entry Type : 02
 [0A5h 0165   2]                    Device ID : 00FB
 [0A7h 0167   1]                 Data Setting : 00

 [0A8h 0168   1]                   Entry Type : 48
 [0A9h 0169   2]                    Device ID : 0000
 [0ABh 0171   1]                 Data Setting : 00
 [0ACh 0172   1]                       Handle : 00
 [0ADh 0173   2]        Source Used Device ID : 00A0
 [0AFh 0175   1]                      Variety : 01

Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 33e998165591..dfb8523c8bf4 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,2 +1 @@
 /* List of comma-separated changed AML files to ignore */
-tests/data/acpi/x86/q35/IVRS.ivrs
-- 
2.34.1

Re: [PATCH 2/8] amd_iommu: update PA, GVA and VA size macros

[Vasant Hegde]

On 5/11/2026 6:09 PM, Sairaj Kodilkar wrote:
> Current implemenation supports upto 64 bit IOVA and upto 52 bit PA.
> Modify related macros so that guest can see right size.
> 
> Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>

Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>

-Vasant

Re: [PATCH 6/8] acpi_build: Build IVRS feature report using extended feature register

[Vasant Hegde]

On 5/11/2026 6:09 PM, Sairaj Kodilkar wrote:
> Currently IVRS feature report values are hardcoded, this is difficult to
> maintain as any updates to extended feature must be synced. Along with
> it current feature report does not have GATS and HATS set. Hence use
> the extended feature registers to build the IVRS feature report.
> > Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
> ---
>  hw/i386/acpi-build.c | 38 +++++++++++++++++++++++++++++++-------
>  hw/i386/acpi-build.h |  1 -
>  2 files changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 718e3f546b18..82208e06e155 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1747,6 +1747,36 @@ ivrs_host_bridges(Object *obj, void *opaque)
>      return 0;
>  }
>  
> +/*
> + * IVHD type 0x10 reports features using Feature Reporting field, which has
> + * different format than extended feature register (EFR) in the IOMMU MMIO
> + * space.
> + *
> + * Convert the EFR format to feature reporting format.
> + */
> +static uint32_t
> +get_amd_ivhd_feature_report(AMDVIState *s)
> +{
> +    uint64_t feature = amdvi_extended_feature_register(s);
> +    uint32_t is_gt = !!(feature & AMDVI_FEATURE_GT);
> +    uint32_t is_ga = !!(feature & AMDVI_FEATURE_GA);
> +    uint64_t glx_sup = (feature & AMDVI_GLX_SUP_MASK) >> AMDVI_GLX_SUP_SHIFT;
> +    uint64_t hats_mode = (feature & AMDVI_HATS_MODE_MASK) >>
> +                         AMDVI_HATS_MODE_SHIFT;
> +    uint64_t gats_mode = (feature & AMDVI_GATS_MODE_MASK) >>
> +                         AMDVI_GATS_MODE_SHIFT;
> +    uint32_t feature_report;
> +
> +    feature_report = s->xtsup << AMD_IVHD_FEATURE_REPORT_XT_SUP_SHIFT |
> +                     is_gt << AMD_IVHD_FEATURE_REPORT_GT_SUP_SHIFT |
> +                     glx_sup << AMD_IVHD_FEATURE_REPORT_GLX_SUP_SHIFT |

Looks like glx is new? Please drop the glx changes as we are not supporting
guest page table yet.


> +                     is_ga << AMD_IVHD_FEATURE_REPORT_GA_SUP_SHIFT |
> +                     hats_mode << AMD_IVHD_FEATURE_REPORT_HATS_SHIFT |
> +                     gats_mode << AMD_IVHD_FEATURE_REPORT_GATS_SHIFT;
> +
> +    return feature_report;
> +}
> +
>  static void
>  build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
>                  const char *oem_table_id)
> @@ -1829,13 +1859,7 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
>      /* IOMMU info */
>      build_append_int_noprefix(table_data, 0, 2);
>      /* IOMMU Feature Reporting */
> -    feature_report = (48UL << 30) | /* HATS   */
> -                     (48UL << 28) | /* GATS   */
> -                     (1UL << 2)   | /* GTSup  */
> -                     (1UL << 6);    /* GASup  */
> -    if (s->xtsup) {
> -        feature_report |= (1UL << 0); /* XTSup */
> -    }
> +    feature_report = get_amd_ivhd_feature_report(s);
>      build_append_int_noprefix(table_data, feature_report, 4);
>  
>      /* IVHD entries as found above */
> diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
> index 9fd60a186db1..d81b7890e6e2 100644
> --- a/hw/i386/acpi-build.h
> +++ b/hw/i386/acpi-build.h
> @@ -1,4 +1,3 @@
> -

Unnecessary change.

-Vasant

Re: [PATCH 2/8] amd_iommu: update PA, GVA and VA size macros

[Alejandro Jimenez]

On 5/11/26 8:39 AM, Sairaj Kodilkar wrote:
> Current implemenation 
s/implemenation/implementation

supports upto 64 bit IOVA and upto 52 bit PA.

s/upto/up to

> Modify related macros so that guest can see right size.
> 
> Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
> ---
>  hw/i386/acpi-build.c | 2 +-
>  hw/i386/amd_iommu.c  | 2 +-
>  hw/i386/amd_iommu.h  | 6 +++---
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 9446a9f862ca..e4ad01eec037 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1763,7 +1763,7 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
>       */
>      build_append_int_noprefix(table_data,
>                               (1UL << 0) | /* EFRSup */
> -                             (40UL << 8), /* PASize */
> +                             AMDVI_PA_SIZE_52,
>                               4);
>      /* reserved */
>      build_append_int_noprefix(table_data, 0, 8);
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 789e09d6f2bc..7c52f9ade6e6 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -2450,7 +2450,7 @@ static void amdvi_pci_realize(PCIDevice *pdev, Error **errp)
>                   0xff000000);
>      pci_set_long(pdev->config + s->capab_offset + AMDVI_CAPAB_MISC, 0);
>      pci_set_long(pdev->config + s->capab_offset + AMDVI_CAPAB_MISC,
> -            AMDVI_MAX_PH_ADDR | AMDVI_MAX_GVA_ADDR | AMDVI_MAX_VA_ADDR);
> +            AMDVI_PA_SIZE_52 | AMDVI_GVA_SIZE_48 | AMDVI_VA_SIZE_64);
>  }
>  
>  static void amdvi_sysbus_reset(DeviceState *dev)
> diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
> index 302ccca5121f..fe8f4a6cdc74 100644
> --- a/hw/i386/amd_iommu.h
> +++ b/hw/i386/amd_iommu.h
> @@ -245,9 +245,9 @@
>  #define AMDVI_PAGE_SHIFT_4K 12
>  #define AMDVI_PAGE_MASK_4K      GENMASK64(63, 12)
>  
> -#define AMDVI_MAX_GVA_ADDR      (2UL << 5)
> -#define AMDVI_MAX_PH_ADDR       (40UL << 8)
> -#define AMDVI_MAX_VA_ADDR       (48UL << 15)

Could you add a comment explaining that these constants encode fields from
the AMD IOMMU “Miscellaneous Information Register 0” at IOMMU Capability
Offset 10h. I prefer listing the field names and bit ranges exactly how
they appear in the spec, which helps when searching through the doc, and
also help explain the shifts. i.e. something like:

+/*
+ * IOMMU Capability Offset 10h: Miscellaneous Information Register 0.
+ * VAsize[21:15], PAsize[14:8], and GVAsize[7:5].
+ * These fields use the same layout in the IVRS IVinfo Field.
+ */
> +#define AMDVI_GVA_SIZE_48       (2UL << 5)
> +#define AMDVI_PA_SIZE_52        (52UL << 8)
> +#define AMDVI_VA_SIZE_64        (64UL << 15)
>  
>  /* Completion Wait data size */
>  #define AMDVI_COMPLETION_DATA_SIZE    8

With the above changes:

Reviewed-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>

Re: [PATCH 3/8] amd_iommu: Return empty efr for stub call

[Alejandro Jimenez]

On 5/11/26 8:39 AM, Sairaj Kodilkar wrote:
> The stub call should not return normal EFR as it is used as place holder
> when actual AMD IOMMU calls are not available. Hence return 0.
> 
> Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
> 

Reviewed-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>

> ---
>  hw/i386/amd_iommu-stub.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/i386/amd_iommu-stub.c b/hw/i386/amd_iommu-stub.c
> index d62a3732e60f..fb36dee903ee 100644
> --- a/hw/i386/amd_iommu-stub.c
> +++ b/hw/i386/amd_iommu-stub.c
> @@ -22,5 +22,5 @@
>  
>  uint64_t amdvi_extended_feature_register(AMDVIState *s)
>  {
> -    return AMDVI_DEFAULT_EXT_FEATURES;
> +    return 0;
>  }

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

[Alejandro Jimenez]

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 <sarunkod@amd.com>
> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
>  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 <alejandro.j.jimenez@oracle.com>

>      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 */

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

[Michael S. Tsirkin]

On Mon, May 11, 2026 at 06:09:33PM +0530, 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.
> 
> Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>

But is this called after firmware has enumerated the pci bus?
And I guess OS better not change that bus number eh?

> ---
>  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);
>  
>      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 */
> -- 
> 2.34.1