[v2] hw/i386/amd_iommu: Cleanups and fixes

[PATCH v2 5/6] hw/i386/amd_iommu: Support MMIO writes to the status register

Posted by Sairaj Kodilkar 6 months, 2 weeks ago

Support the writes to the status register so that guest can reset the
EventOverflow, EventLogInt, ComWaitIntr, etc bits after servicing the
respective interrupt.

Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
---
 hw/i386/amd_iommu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index bbffd07b4e48..7c2fa80d14ff 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -850,6 +850,9 @@ static void amdvi_mmio_write(void *opaque, hwaddr addr, uint64_t val,
         amdvi_mmio_reg_write(s, size, val, addr);
         amdvi_handle_pprtail_write(s);
         break;
+    case AMDVI_MMIO_STATUS:
+        amdvi_mmio_reg_write(s, size, val, addr);
+        break;
     }
 }
 
-- 
2.34.1

Re: [PATCH v2 5/6] hw/i386/amd_iommu: Support MMIO writes to the status register

Posted by Philippe Mathieu-Daudé 6 months, 2 weeks ago

On 24/7/25 08:47, Sairaj Kodilkar wrote:
> Support the writes to the status register so that guest can reset the
> EventOverflow, EventLogInt, ComWaitIntr, etc bits after servicing the
> respective interrupt.
> 
> Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
>   hw/i386/amd_iommu.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index bbffd07b4e48..7c2fa80d14ff 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -850,6 +850,9 @@ static void amdvi_mmio_write(void *opaque, hwaddr addr, uint64_t val,
>           amdvi_mmio_reg_write(s, size, val, addr);
>           amdvi_handle_pprtail_write(s);
>           break;
> +    case AMDVI_MMIO_STATUS:
> +        amdvi_mmio_reg_write(s, size, val, addr);
> +        break;

Should we add:

         default:
             g_assert_not_reached();

to catch unimplemented cases?

>       }
>   }
>

Re: [PATCH v2 5/6] hw/i386/amd_iommu: Support MMIO writes to the status register

Posted by Sairaj Kodilkar 6 months, 1 week ago


On 7/24/2025 2:22 PM, Philippe Mathieu-Daudé wrote:
> On 24/7/25 08:47, Sairaj Kodilkar wrote:
>> Support the writes to the status register so that guest can reset the
>> EventOverflow, EventLogInt, ComWaitIntr, etc bits after servicing the
>> respective interrupt.
>>
>> Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
>> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
>> ---
>>   hw/i386/amd_iommu.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
>> index bbffd07b4e48..7c2fa80d14ff 100644
>> --- a/hw/i386/amd_iommu.c
>> +++ b/hw/i386/amd_iommu.c
>> @@ -850,6 +850,9 @@ static void amdvi_mmio_write(void *opaque, hwaddr 
>> addr, uint64_t val,
>>           amdvi_mmio_reg_write(s, size, val, addr);
>>           amdvi_handle_pprtail_write(s);
>>           break;
>> +    case AMDVI_MMIO_STATUS:
>> +        amdvi_mmio_reg_write(s, size, val, addr);
>> +        break;
> 
> Should we add:
> 
>          default:
>              g_assert_not_reached();

I am against adding a failure here. Because when xtsup=on, linux tries
to write all the XT registers ( which we dont support yet, because of
which event logging does not work in this mode). Although future cleanup
patches will add this support, I dont want to break things now.

I think amdvi_mmio_trace_write() is sufficient for debugging purpose,
which prints "UNHANDLED" for unsupported registers.

Thanks
Sairaj

> 
> to catch unimplemented cases?
> 
>>       }
>>   }
>

[PATCH v2 1/6] hw/i386/amd_iommu: Fix MMIO register write tracing
[PATCH v2 2/6] hw/i386/amd_iommu: Remove unused and wrongly set ats_enabled field
[PATCH v2 3/6] hw/i386/amd_iommu: Move IOAPIC memory region initialization to the end
[PATCH v2 4/6] hw/i386/amd_iommu: Fix amdvi_write*()
[PATCH v2 5/6] hw/i386/amd_iommu: Support MMIO writes to the status register
[PATCH v2 6/6] hw/i386/amd_iommu: Fix event log generation