Re: [PATCH v2 09/20] amd_iommu: Add basic structure to support IOMMU notifier updates

[Alejandro Jimenez <alejandro.j.jimenez@oracle.com>]

On 6/23/25 12:53 PM, Sairaj Kodilkar wrote:
> 
> 
> On 5/2/2025 7:45 AM, Alejandro Jimenez wrote:
>> Add the minimal data structures required to maintain a list of address
>> spaces (i.e. devices) with registered notifiers, and to update the 
>> type of
>> events that require notifications.
>> Note that the ability to register for MAP notifications is not available.
>> It will be unblocked by following changes that enable the 
>> synchronization of
>> guest I/O page tables with host IOMMU state, at which point an amd-iommu
>> device property will be introduced to control this capability.
>>
>> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
>> ---
>>   hw/i386/amd_iommu.c | 26 +++++++++++++++++++++++---
>>   hw/i386/amd_iommu.h |  3 +++
>>   2 files changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
>> index 6a2ba878dfa7..2f69459ab68d 100644
>> --- a/hw/i386/amd_iommu.c
>> +++ b/hw/i386/amd_iommu.c
>> @@ -66,6 +66,11 @@ struct AMDVIAddressSpace {
>>       MemoryRegion iommu_nodma;   /* Alias of shared nodma memory 
>> region  */
>>       MemoryRegion iommu_ir;      /* Device's interrupt remapping 
>> region  */
>>       AddressSpace as;            /* device's corresponding address 
>> space */
>> +
>> +    /* DMA address translation support */
>> +    IOMMUNotifierFlag notifier_flags;
>> +    /* entry in list of Address spaces with registered notifiers */
>> +    QLIST_ENTRY(AMDVIAddressSpace) next;
>>   };
>>   /* AMDVI cache entry */
>> @@ -1711,6 +1716,7 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus 
>> *bus, void *opaque, int devfn)
>>           iommu_as[devfn]->bus_num = (uint8_t)bus_num;
>>           iommu_as[devfn]->devfn = (uint8_t)devfn;
>>           iommu_as[devfn]->iommu_state = s;
>> +        iommu_as[devfn]->notifier_flags = IOMMU_NONE;
> 
> Hey
> Use IOMMU_NOTIFIER_NONE instead of IOMMU_NONE. Though both are same, the 
> clang compiler complains about it and fails to compile (GCC works ok).
> 

Good catch, IOMMU_NOTIFIER_NONE is the correct type to use. I fixed it 
and checked it builds with no errors using:

CC=clang CCX=clang++ ./configure --target-list=x86_64-softmmu --enable-debug

Also fixed the whitespace changes you pointed out on the earlier reply.

Thank you,
Alejandro

> Thanks
> Sairaj Kodilkar
>