Re: [PATCH v2 1/3] amd_iommu: Use switch case to determine mmio register name

[Sairaj Kodilkar]

On 1/30/2026 3:20 AM, Alejandro Jimenez wrote:

On 1/29/26 5:28 AM, Sairaj Kodilkar wrote:

This makes it easier to add new MMIO registers for tracing and removes
the unnecessary complexity introduced by amdvi_mmio_(low/high) array.

Signed-off-by: Sairaj Kodilkar <sarun...@amd.com>
Reviewed-by: Vasant Hegde <vasant.he...@amd.com>
---
  hw/i386/amd_iommu.c | 65 +++++++++++++++++++--------------------------
  1 file changed, 27 insertions(+), 38 deletions(-)

I'd like to also remove the unused AMDVI_MMIO_REGS_{LOW,HIGH} definitions
and amdvi_mmio_trace_{read,write} helpers as I did on the example diff in:

https://lore.kernel.org/qemu-devel/eaf49cf3-e56b-40f1-974d-207969c73...@oracle.com/

assuming you agree with it, no need to send a new revision, I can add those
changes to the current patch.

Sure go ahead,

Thanks
Sairaj

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

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 789e09d6f2bc..62175cc366ac 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -35,28 +35,7 @@
  #include "kvm/kvm_i386.h"
  #include "qemu/iova-tree.h"

-/* used AMD-Vi MMIO registers */

-const char *amdvi_mmio_low[] = {
-    "AMDVI_MMIO_DEVTAB_BASE",
-    "AMDVI_MMIO_CMDBUF_BASE",
-    "AMDVI_MMIO_EVTLOG_BASE",
-    "AMDVI_MMIO_CONTROL",
-    "AMDVI_MMIO_EXCL_BASE",
-    "AMDVI_MMIO_EXCL_LIMIT",
-    "AMDVI_MMIO_EXT_FEATURES",
-    "AMDVI_MMIO_PPR_BASE",
-    "UNHANDLED"
-};
-const char *amdvi_mmio_high[] = {
-    "AMDVI_MMIO_COMMAND_HEAD",
-    "AMDVI_MMIO_COMMAND_TAIL",
-    "AMDVI_MMIO_EVTLOG_HEAD",
-    "AMDVI_MMIO_EVTLOG_TAIL",
-    "AMDVI_MMIO_STATUS",
-    "AMDVI_MMIO_PPR_HEAD",
-    "AMDVI_MMIO_PPR_TAIL",
-    "UNHANDLED"
-};
+#define MMIO_REG_TO_STRING(mmio_reg) case mmio_reg: return #mmio_reg

struct AMDVIAddressSpace {

      PCIBus *bus;                /* PCIBus (for bus number)              */
@@ -1484,31 +1463,41 @@ static void amdvi_cmdbuf_run(AMDVIState *s)
      }
  }

-static inline uint8_t amdvi_mmio_get_index(hwaddr addr)

-{
-    uint8_t index = (addr & ~0x2000) / 8;
-
-    if ((addr & 0x2000)) {
-        /* high table */
-        index = index >= AMDVI_MMIO_REGS_HIGH ? AMDVI_MMIO_REGS_HIGH : index;
-    } else {
-        index = index >= AMDVI_MMIO_REGS_LOW ? AMDVI_MMIO_REGS_LOW : index;
+static inline
+const char *amdvi_mmio_get_name(hwaddr addr)
+{
+    /* Return MMIO names as string literals */
+    switch (addr) {
+    MMIO_REG_TO_STRING(AMDVI_MMIO_DEVICE_TABLE);
+    MMIO_REG_TO_STRING(AMDVI_MMIO_COMMAND_BASE);
+    MMIO_REG_TO_STRING(AMDVI_MMIO_EVENT_BASE);
+    MMIO_REG_TO_STRING(AMDVI_MMIO_CONTROL);
+    MMIO_REG_TO_STRING(AMDVI_MMIO_EXCL_BASE);
+    MMIO_REG_TO_STRING(AMDVI_MMIO_EXCL_LIMIT);
+    MMIO_REG_TO_STRING(AMDVI_MMIO_EXT_FEATURES);
+    MMIO_REG_TO_STRING(AMDVI_MMIO_COMMAND_HEAD);
+    MMIO_REG_TO_STRING(AMDVI_MMIO_COMMAND_TAIL);
+    MMIO_REG_TO_STRING(AMDVI_MMIO_EVENT_HEAD);
+    MMIO_REG_TO_STRING(AMDVI_MMIO_EVENT_TAIL);
+    MMIO_REG_TO_STRING(AMDVI_MMIO_STATUS);
+    MMIO_REG_TO_STRING(AMDVI_MMIO_PPR_BASE);
+    MMIO_REG_TO_STRING(AMDVI_MMIO_PPR_HEAD);
+    MMIO_REG_TO_STRING(AMDVI_MMIO_PPR_TAIL);
+    default:
+        return "UNHANDLED";
      }
-
-    return index;
  }

static void amdvi_mmio_trace_read(hwaddr addr, unsigned size)

  {
-    uint8_t index = amdvi_mmio_get_index(addr);
-    trace_amdvi_mmio_read(amdvi_mmio_low[index], addr, size, addr & ~0x07);
+    const char *mmio_name = amdvi_mmio_get_name(addr);
+    trace_amdvi_mmio_read(mmio_name, addr, size, addr & ~0x07);
  }

static void amdvi_mmio_trace_write(hwaddr addr, unsigned size, uint64_t val)

  {
-    uint8_t index = amdvi_mmio_get_index(addr);
-    trace_amdvi_mmio_write(amdvi_mmio_low[index], addr, size, val,
-                           addr & ~0x07);
+    const char *mmio_name = amdvi_mmio_get_name(addr);
+    trace_amdvi_mmio_write(mmio_name, addr, size, val, addr & ~0x07);
  }

static uint64_t amdvi_mmio_read(void *opaque, hwaddr addr, unsigned size)