[PATCH v2 2/6] hw/i386/amd_iommu: Remove unused and wrongly set ats_enabled field

Sairaj Kodilkar posted 6 patches 3 months, 3 weeks ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
There is a newer version of this series
[PATCH v2 2/6] hw/i386/amd_iommu: Remove unused and wrongly set ats_enabled field
Posted by Sairaj Kodilkar 3 months, 3 weeks ago
The ats_enabled field is set using HTTUNEN, which is wrong.
Fix this by removing the field as it is never used.

Fixes: d29a09ca68428 ("hw/i386: Introduce AMD IOMMU")
Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
---
 hw/i386/amd_iommu.c | 3 ---
 hw/i386/amd_iommu.h | 1 -
 2 files changed, 4 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 7fb0bb68f008..39bb04fd9b18 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -646,7 +646,6 @@ static void amdvi_handle_control_write(AMDVIState *s)
     unsigned long control = amdvi_readq(s, AMDVI_MMIO_CONTROL);
     s->enabled = !!(control & AMDVI_MMIO_CONTROL_AMDVIEN);
 
-    s->ats_enabled = !!(control & AMDVI_MMIO_CONTROL_HTTUNEN);
     s->evtlog_enabled = s->enabled && !!(control &
                         AMDVI_MMIO_CONTROL_EVENTLOGEN);
 
@@ -1555,7 +1554,6 @@ static void amdvi_init(AMDVIState *s)
     s->excl_allow = false;
     s->mmio_enabled = false;
     s->enabled = false;
-    s->ats_enabled = false;
     s->cmdbuf_enabled = false;
 
     /* reset MMIO */
@@ -1626,7 +1624,6 @@ static const VMStateDescription vmstate_amdvi_sysbus_migratable = {
       /* Updated in  amdvi_handle_control_write() */
       VMSTATE_BOOL(enabled, AMDVIState),
       VMSTATE_BOOL(ga_enabled, AMDVIState),
-      VMSTATE_BOOL(ats_enabled, AMDVIState),
       VMSTATE_BOOL(cmdbuf_enabled, AMDVIState),
       VMSTATE_BOOL(completion_wait_intr, AMDVIState),
       VMSTATE_BOOL(evtlog_enabled, AMDVIState),
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 8b42913ed8da..67078c6f1e22 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -322,7 +322,6 @@ struct AMDVIState {
     uint64_t mmio_addr;
 
     bool enabled;                /* IOMMU enabled                */
-    bool ats_enabled;            /* address translation enabled  */
     bool cmdbuf_enabled;         /* command buffer enabled       */
     bool evtlog_enabled;         /* event log enabled            */
     bool excl_enabled;
-- 
2.34.1
Re: [PATCH v2 2/6] hw/i386/amd_iommu: Remove unused and wrongly set ats_enabled field
Posted by Philippe Mathieu-Daudé 3 months, 3 weeks ago
Hi,

On 24/7/25 08:47, Sairaj Kodilkar wrote:
> The ats_enabled field is set using HTTUNEN, which is wrong.
> Fix this by removing the field as it is never used.
> 
> Fixes: d29a09ca68428 ("hw/i386: Introduce AMD IOMMU")
> Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
>   hw/i386/amd_iommu.c | 3 ---
>   hw/i386/amd_iommu.h | 1 -
>   2 files changed, 4 deletions(-)
> 


> @@ -1626,7 +1624,6 @@ static const VMStateDescription vmstate_amdvi_sysbus_migratable = {
>         /* Updated in  amdvi_handle_control_write() */
>         VMSTATE_BOOL(enabled, AMDVIState),
>         VMSTATE_BOOL(ga_enabled, AMDVIState),
> -      VMSTATE_BOOL(ats_enabled, AMDVIState),

Please have a look at the migration documentation:
https://www.qemu.org/docs/master/devel/migration/main.html#not-sending-existing-elements

>         VMSTATE_BOOL(cmdbuf_enabled, AMDVIState),
>         VMSTATE_BOOL(completion_wait_intr, AMDVIState),
>         VMSTATE_BOOL(evtlog_enabled, AMDVIState),
> diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
> index 8b42913ed8da..67078c6f1e22 100644
> --- a/hw/i386/amd_iommu.h
> +++ b/hw/i386/amd_iommu.h
> @@ -322,7 +322,6 @@ struct AMDVIState {
>       uint64_t mmio_addr;
>   
>       bool enabled;                /* IOMMU enabled                */
> -    bool ats_enabled;            /* address translation enabled  */
>       bool cmdbuf_enabled;         /* command buffer enabled       */
>       bool evtlog_enabled;         /* event log enabled            */
>       bool excl_enabled;
Re: [PATCH v2 2/6] hw/i386/amd_iommu: Remove unused and wrongly set ats_enabled field
Posted by Sairaj Kodilkar 3 months, 2 weeks ago

On 7/24/2025 2:18 PM, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 24/7/25 08:47, Sairaj Kodilkar wrote:
>> The ats_enabled field is set using HTTUNEN, which is wrong.
>> Fix this by removing the field as it is never used.
>>
>> Fixes: d29a09ca68428 ("hw/i386: Introduce AMD IOMMU")
>> Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
>> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
>> ---
>>   hw/i386/amd_iommu.c | 3 ---
>>   hw/i386/amd_iommu.h | 1 -
>>   2 files changed, 4 deletions(-)
>>
> 
> 
>> @@ -1626,7 +1624,6 @@ static const VMStateDescription 
>> vmstate_amdvi_sysbus_migratable = {
>>         /* Updated in  amdvi_handle_control_write() */
>>         VMSTATE_BOOL(enabled, AMDVIState),
>>         VMSTATE_BOOL(ga_enabled, AMDVIState),
>> -      VMSTATE_BOOL(ats_enabled, AMDVIState),
> 
> Please have a look at the migration documentation:
> https://www.qemu.org/docs/master/devel/migration/main.html#not-sending- 
> existing-elements
> 

Thanks for sharing. I'll need to add a VMSTATE_UNUSED() here