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

2025-09-17 Thread Alejandro Jimenez




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 
---
  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






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

2025-06-23 Thread Sairaj Kodilkar




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 
---
  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).


Thanks
Sairaj Kodilkar




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

2025-05-11 Thread Sairaj Kodilkar




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 
---
  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;
  
  amdvi_dev_as = iommu_as[devfn];
  
@@ -1791,14 +1797,28 @@ static int amdvi_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,

 Error **errp)
  {
  AMDVIAddressSpace *as = container_of(iommu, AMDVIAddressSpace, iommu);
+AMDVIState *s = as->iommu_state;
  
  if (new & IOMMU_NOTIFIER_MAP) {

  error_setg(errp,
-   "device %02x.%02x.%x requires iommu notifier which is not "
-   "currently supported", as->bus_num, PCI_SLOT(as->devfn),
-   PCI_FUNC(as->devfn));
+"device %02x.%02x.%x requires iommu notifier which is not "
+"currently supported", as->bus_num, PCI_SLOT(as->devfn),
+PCI_FUNC(as->devfn));


Redundant whitespace changes, please revert.

Regards
Sairaj Kodilkar