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

[PATCH v3 0/6] hw/i386/amd_iommu: Cleanups and fixes

Posted by Sairaj Kodilkar 3 months, 2 weeks ago

This series provides few cleanups and fixes for the amd iommu

Changes since v2:
- Used VMSTATE_UNUSED() to maintain migration compatibility when ats_enabled
  flag is removed [Phil].
- Simplified the amdvi_writew [Phil].
v2: https://lore.kernel.org/qemu-devel/2e8f2b72-8fb5-474f-9844-61f306efeb2b@amd.com/

Changes since v1:
- Dropped top two patches which depend on the Alejandro's changes and rebased
  remaining patches on top of v10.1.0-rc0 [Phil].
- Added a patch to fix amdvi_write*() [Ethon].
- Reset event log head and tail when guest writes to event log base register
  [Ethon].
- Considered "evtlog_intr" flag while generating event log interrupt [Ethon].
- Fixed comment [Ethon].
v1: https://lore.kernel.org/qemu-devel/20250716073145.915-1-sarunkod@amd.com/

Base commit: 9e601684dc24a521bb1d23215a63e5c6e79ea0bb (v10.1.0-rc0)

Sairaj Kodilkar (6):
  hw/i386/amd_iommu: Fix MMIO register write tracing
  hw/i386/amd_iommu: Remove unused and wrongly set ats_enabled field
  hw/i386/amd_iommu: Move IOAPIC memory region initialization to the end
  hw/i386/amd_iommu: Fix amdvi_write*()
  hw/i386/amd_iommu: Support MMIO writes to the status register
  hw/i386/amd_iommu: Fix event log generation

 hw/i386/amd_iommu.c | 102 ++++++++++++++++++++++++++++++++++----------
 hw/i386/amd_iommu.h |   2 +-
 2 files changed, 80 insertions(+), 24 deletions(-)

-- 
2.34.1

Re: [PATCH v3 0/6] hw/i386/amd_iommu: Cleanups and fixes

Posted by Michael Tokarev 3 months, 2 weeks ago

On 01.08.2025 09:05, Sairaj Kodilkar wrote:
> This series provides few cleanups and fixes for the amd iommu
> 
> Changes since v2:
> - Used VMSTATE_UNUSED() to maintain migration compatibility when ats_enabled
>    flag is removed [Phil].
> - Simplified the amdvi_writew [Phil].
> v2: https://lore.kernel.org/qemu-devel/2e8f2b72-8fb5-474f-9844-61f306efeb2b@amd.com/
> 
> Changes since v1:
> - Dropped top two patches which depend on the Alejandro's changes and rebased
>    remaining patches on top of v10.1.0-rc0 [Phil].
> - Added a patch to fix amdvi_write*() [Ethon].
> - Reset event log head and tail when guest writes to event log base register
>    [Ethon].
> - Considered "evtlog_intr" flag while generating event log interrupt [Ethon].
> - Fixed comment [Ethon].
> v1: https://lore.kernel.org/qemu-devel/20250716073145.915-1-sarunkod@amd.com/
> 
> Base commit: 9e601684dc24a521bb1d23215a63e5c6e79ea0bb (v10.1.0-rc0)
> 
> Sairaj Kodilkar (6):
>    hw/i386/amd_iommu: Fix MMIO register write tracing
>    hw/i386/amd_iommu: Remove unused and wrongly set ats_enabled field
>    hw/i386/amd_iommu: Move IOAPIC memory region initialization to the end
>    hw/i386/amd_iommu: Fix amdvi_write*()
>    hw/i386/amd_iommu: Support MMIO writes to the status register
>    hw/i386/amd_iommu: Fix event log generation
> 
>   hw/i386/amd_iommu.c | 102 ++++++++++++++++++++++++++++++++++----------
>   hw/i386/amd_iommu.h |   2 +-
>   2 files changed, 80 insertions(+), 24 deletions(-)

Hi!

Is there anything there worth to pick up for qemu 10.0.x stable series?
(the "Move IOAPIC memory init" does not apply to 10.0 already).

Thanks,

/mjt

Re: [PATCH v3 0/6] hw/i386/amd_iommu: Cleanups and fixes

Posted by Sairaj Kodilkar 3 months, 1 week ago


On 8/2/2025 11:11 AM, Michael Tokarev wrote:
> On 01.08.2025 09:05, Sairaj Kodilkar wrote:
>> This series provides few cleanups and fixes for the amd iommu
>>
>> Changes since v2:
>> - Used VMSTATE_UNUSED() to maintain migration compatibility when 
>> ats_enabled
>>    flag is removed [Phil].
>> - Simplified the amdvi_writew [Phil].
>> v2: https://lore.kernel.org/qemu- 
>> devel/2e8f2b72-8fb5-474f-9844-61f306efeb2b@amd.com/
>>
>> Changes since v1:
>> - Dropped top two patches which depend on the Alejandro's changes and 
>> rebased
>>    remaining patches on top of v10.1.0-rc0 [Phil].
>> - Added a patch to fix amdvi_write*() [Ethon].
>> - Reset event log head and tail when guest writes to event log base 
>> register
>>    [Ethon].
>> - Considered "evtlog_intr" flag while generating event log interrupt 
>> [Ethon].
>> - Fixed comment [Ethon].
>> v1: https://lore.kernel.org/qemu-devel/20250716073145.915-1- 
>> sarunkod@amd.com/
>>
>> Base commit: 9e601684dc24a521bb1d23215a63e5c6e79ea0bb (v10.1.0-rc0)
>>
>> Sairaj Kodilkar (6):
>>    hw/i386/amd_iommu: Fix MMIO register write tracing
>>    hw/i386/amd_iommu: Remove unused and wrongly set ats_enabled field
>>    hw/i386/amd_iommu: Move IOAPIC memory region initialization to the end
>>    hw/i386/amd_iommu: Fix amdvi_write*()
>>    hw/i386/amd_iommu: Support MMIO writes to the status register
>>    hw/i386/amd_iommu: Fix event log generation
>>
>>   hw/i386/amd_iommu.c | 102 ++++++++++++++++++++++++++++++++++----------
>>   hw/i386/amd_iommu.h |   2 +-
>>   2 files changed, 80 insertions(+), 24 deletions(-)
> 
> Hi!
> 
> Is there anything there worth to pick up for qemu 10.0.x stable series?
> (the "Move IOAPIC memory init" does not apply to 10.0 already).

Hi MJT,

I will backport the patch manually and send it to qemu stable mailing list

Thanks
Sairaj

Re: [PATCH v3 0/6] hw/i386/amd_iommu: Cleanups and fixes

Posted by Michael Tokarev 3 months, 1 week ago

On 04.08.2025 14:06, Sairaj Kodilkar wrote:
...
>>> Sairaj Kodilkar (6):
>>>    hw/i386/amd_iommu: Fix MMIO register write tracing
>>>    hw/i386/amd_iommu: Remove unused and wrongly set ats_enabled field
>>>    hw/i386/amd_iommu: Move IOAPIC memory region initialization to the 
>>> end
>>>    hw/i386/amd_iommu: Fix amdvi_write*()
>>>    hw/i386/amd_iommu: Support MMIO writes to the status register
>>>    hw/i386/amd_iommu: Fix event log generation
>>>
>>>   hw/i386/amd_iommu.c | 102 ++++++++++++++++++++++++++++++++++----------
>>>   hw/i386/amd_iommu.h |   2 +-
>>>   2 files changed, 80 insertions(+), 24 deletions(-)
>>
>> Hi!
>>
>> Is there anything there worth to pick up for qemu 10.0.x stable series?
>> (the "Move IOAPIC memory init" does not apply to 10.0 already).
> 
> Hi MJT,
> 
> I will backport the patch manually and send it to qemu stable mailing list

Hi!

There's no need to back-port it, -- there's just minor context fix
required, here:

+++ b/hw/i386/amd_iommu.c
@@ -1693,19 +1693,16 @@ static void amdvi_sysbus_realize(DeviceState 
*dev, Error **errp)
...

      s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
                                       amdvi_uint64_equal, g_free, g_free);

-    /* Pseudo address space under root PCI bus. */
-    x86ms->ioapic_as = amdvi_host_dma_iommu(bus, s, AMDVI_IOAPIC_SB_DEVID);
-
      /* set up MMIO */
      memory_region_init_io(&s->mr_mmio, OBJECT(s), &mmio_mem_ops, s,
                            "amdvi-mmio", AMDVI_MMIO_SIZE);

the "s->iotlb = g_hash_table_new_full" part were added by commit
f864a3235ea1d1 "hw/i386/amd_iommu: Isolate AMDVI-PCI from amd-iommu
device to allow full control over the PCI device creation".  It should
be okay to just remove the 3 marked lines from here (to be moved to
the right place).

My question was not about back-porting this commit, but more about
the set of commits which needs to be picked up for the stable series.

I picked up this commit.  Please let me know if there are other changes
needed to be picked up.

Thanks,

/mjt

Re: [PATCH v3 0/6] hw/i386/amd_iommu: Cleanups and fixes

Posted by Sairaj Kodilkar 3 months, 1 week ago


On 8/6/2025 12:28 PM, Michael Tokarev wrote:
> On 04.08.2025 14:06, Sairaj Kodilkar wrote:
> ...
>>>> Sairaj Kodilkar (6):
>>>>    hw/i386/amd_iommu: Fix MMIO register write tracing
>>>>    hw/i386/amd_iommu: Remove unused and wrongly set ats_enabled field
>>>>    hw/i386/amd_iommu: Move IOAPIC memory region initialization to 
>>>> the end
>>>>    hw/i386/amd_iommu: Fix amdvi_write*()
>>>>    hw/i386/amd_iommu: Support MMIO writes to the status register
>>>>    hw/i386/amd_iommu: Fix event log generation
>>>>
>>>>   hw/i386/amd_iommu.c | 102 +++++++++++++++++++++++++++++++++ 
>>>> +----------
>>>>   hw/i386/amd_iommu.h |   2 +-
>>>>   2 files changed, 80 insertions(+), 24 deletions(-)
>>>
>>> Hi!
>>>
>>> Is there anything there worth to pick up for qemu 10.0.x stable series?
>>> (the "Move IOAPIC memory init" does not apply to 10.0 already).
>>
>> Hi MJT,
>>
>> I will backport the patch manually and send it to qemu stable mailing 
>> list
> 
> Hi!
> 
> There's no need to back-port it, -- there's just minor context fix
> required, here:
> 
> +++ b/hw/i386/amd_iommu.c
> @@ -1693,19 +1693,16 @@ static void amdvi_sysbus_realize(DeviceState 
> *dev, Error **errp)
> ...
> 
>       s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
>                                        amdvi_uint64_equal, g_free, g_free);
> 
> -    /* Pseudo address space under root PCI bus. */
> -    x86ms->ioapic_as = amdvi_host_dma_iommu(bus, s, 
> AMDVI_IOAPIC_SB_DEVID);
> -
>       /* set up MMIO */
>       memory_region_init_io(&s->mr_mmio, OBJECT(s), &mmio_mem_ops, s,
>                             "amdvi-mmio", AMDVI_MMIO_SIZE);
> 
> the "s->iotlb = g_hash_table_new_full" part were added by commit
> f864a3235ea1d1 "hw/i386/amd_iommu: Isolate AMDVI-PCI from amd-iommu
> device to allow full control over the PCI device creation".  It should
> be okay to just remove the 3 marked lines from here (to be moved to
> the right place).
> 
> My question was not about back-porting this commit, but more about
> the set of commits which needs to be picked up for the stable series.
> 
> I picked up this commit.  Please let me know if there are other changes
> needed to be picked up.
> 

Hi MJT

I think this one is sufficient.

Thanks
Sairaj