Define separate functions to trace MMIO write accesses instead of using
`trace_amdvi_mmio_read()` for both read and write.

Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/amd_iommu.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 5a24c17548d4..7fb0bb68f008 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -592,18 +592,31 @@ static void amdvi_cmdbuf_run(AMDVIState *s)
     }
 }
 
-static void amdvi_mmio_trace(hwaddr addr, unsigned size)
+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;
-        trace_amdvi_mmio_read(amdvi_mmio_high[index], addr, size, addr & ~0x07);
     } else {
         index = index >= AMDVI_MMIO_REGS_LOW ? AMDVI_MMIO_REGS_LOW : index;
-        trace_amdvi_mmio_read(amdvi_mmio_low[index], addr, size, addr & ~0x07);
     }
+
+    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);
+}
+
+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);
 }
 
 static uint64_t amdvi_mmio_read(void *opaque, hwaddr addr, unsigned size)
@@ -623,7 +636,7 @@ static uint64_t amdvi_mmio_read(void *opaque, hwaddr addr, unsigned size)
     } else if (size == 8) {
         val = amdvi_readq(s, addr);
     }
-    amdvi_mmio_trace(addr, size);
+    amdvi_mmio_trace_read(addr, size);
 
     return val;
 }
@@ -770,7 +783,7 @@ static void amdvi_mmio_write(void *opaque, hwaddr addr, uint64_t val,
         return;
     }
 
-    amdvi_mmio_trace(addr, size);
+    amdvi_mmio_trace_write(addr, size, val);
     switch (addr & ~0x07) {
     case AMDVI_MMIO_CONTROL:
         amdvi_mmio_reg_write(s, size, val, addr);
-- 
2.34.1

>-static void amdvi_mmio_trace(hwaddr addr, unsigned size)
>+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;
>-        trace_amdvi_mmio_read(amdvi_mmio_high[index], addr, size, addr & ~0x07);
>     } else {
>         index = index >= AMDVI_MMIO_REGS_LOW ? AMDVI_MMIO_REGS_LOW : index;
>-        trace_amdvi_mmio_read(amdvi_mmio_low[index], addr, size, addr & ~0x07);
>     }
>+
>+    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);
>+}
>+
>+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,

Shouldn't you be picking between amdvi_mmio_low and amdvi_mmio_high in the
above 2 fuctions depending on the addr value?

On 9/30/2025 3:06 PM, vsntk18@gmail.com wrote:
>> -static void amdvi_mmio_trace(hwaddr addr, unsigned size)
>> +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;
>> -        trace_amdvi_mmio_read(amdvi_mmio_high[index], addr, size, addr & ~0x07);
>>      } else {
>>          index = index >= AMDVI_MMIO_REGS_LOW ? AMDVI_MMIO_REGS_LOW : index;
>> -        trace_amdvi_mmio_read(amdvi_mmio_low[index], addr, size, addr & ~0x07);
>>      }
>> +
>> +    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);
>> +}
>> +
>> +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,
> Shouldn't you be picking between amdvi_mmio_low and amdvi_mmio_high in the
> above 2 fuctions depending on the addr value?
Yep I realized that after the patches were merged. I have included the 
fix in the next cleanup series

Thanks
Sairaj