Hi Shameer,
On Mon, Jul 14, 2025 at 04:59:32PM +0100, Shameer Kolothum wrote:
> @@ -25,30 +31,72 @@ static SMMUv3AccelDevice *smmuv3_accel_get_dev(SMMUState
> *bs, SMMUPciBus *sbus,
>
> sbus->pbdev[devfn] = sdev;
> smmu_init_sdev(bs, sdev, bus, devfn);
> + address_space_init(&accel_dev->as_sysmem, &s->s_accel->root,
> + "smmuv3-accel-sysmem");
> }
>
> return accel_dev;
> }
[..]
> static AddressSpace *smmuv3_accel_find_add_as(PCIBus *bus, void *opaque,
> int devfn)
> {
> + PCIDevice *pdev = pci_find_device(bus, pci_bus_num(bus), devfn);
> SMMUState *bs = opaque;
> + bool vfio_pci = false;
> SMMUPciBus *sbus;
> SMMUv3AccelDevice *accel_dev;
> SMMUDevice *sdev;
>
> + if (pdev && !smmuv3_accel_pdev_allowed(pdev, &vfio_pci)) {
> + error_report("Device(%s) not allowed. Only PCIe root complex devices
> "
> + "or PCI bridge devices or vfio-pci endpoint devices
> with "
> + "iommufd as backend is allowed with
> arm-smmuv3,accel=on",
> + pdev->name);
> + exit(1);
> + }
> sbus = smmu_get_sbus(bs, bus);
> accel_dev = smmuv3_accel_get_dev(bs, sbus, bus, devfn);
> sdev = &accel_dev->sdev;
>
> - return &sdev->as;
> + if (vfio_pci) {
> + return &accel_dev->as_sysmem;
I found a new problem here that we initialize new as_sysmem per
VFIO device. So, sdevs would return own individual AS pointers
here at this get_address_space function, although they point to
the same system address space.
Since address space pointers are returned differently for VFIO
devices, this fails to reuse ioas_id in iommufd_cdev_attach(),
and ends up with allocating a new ioas for each device.
I think we can try the following change to make sure all accel
linked VFIO devices would share the same ioas_id, though I am
not sure if SMMUBaseClass is the right place to go. Please take
a look.
Then, once kernel is patched to share S2 hwpt across vSMMUs,
iommufd_cdev_attach() will go further to reuse the S2 HWPT in
the same container.
Thanks
Nicolin
---
hw/arm/smmuv3-accel.c | 14 ++++++++++----
hw/arm/smmuv3-accel.h | 2 +-
include/hw/arm/smmu-common.h | 2 ++
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index c6dc50cf45..405b72095f 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -370,13 +370,19 @@ static SMMUv3AccelDevice *smmuv3_accel_get_dev(SMMUState
*bs, SMMUPciBus *sbus,
if (sdev) {
accel_dev = container_of(sdev, SMMUv3AccelDevice, sdev);
} else {
+ SMMUBaseClass *sbc = ARM_SMMU_GET_CLASS(s);
+
accel_dev = g_new0(SMMUv3AccelDevice, 1);
sdev = &accel_dev->sdev;
sbus->pbdev[devfn] = sdev;
smmu_init_sdev(bs, sdev, bus, devfn);
- address_space_init(&accel_dev->as_sysmem, &s->s_accel->root,
- "smmuv3-accel-sysmem");
+ if (!sbc->as_sysmem) {
+ sbc->as_sysmem = g_new0(AddressSpace, 1);
+ address_space_init(sbc->as_sysmem, &s->s_accel->root,
+ "smmuv3-accel-sysmem");
+ }
+ accel_dev->as_sysmem = sbc->as_sysmem;
}
return accel_dev;
@@ -558,7 +564,7 @@ static AddressSpace *smmuv3_accel_find_msi_as(PCIBus *bus,
void *opaque,
if (accel_dev->s1_hwpt) {
return &sdev->as;
} else {
- return &accel_dev->as_sysmem;
+ return accel_dev->as_sysmem;
}
}
@@ -599,7 +605,7 @@ static AddressSpace *smmuv3_accel_find_add_as(PCIBus *bus,
void *opaque,
sdev = &accel_dev->sdev;
if (vfio_pci) {
- return &accel_dev->as_sysmem;
+ return accel_dev->as_sysmem;
} else {
return &sdev->as;
}
diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
index e1e99598b4..9faa0818d7 100644
--- a/hw/arm/smmuv3-accel.h
+++ b/hw/arm/smmuv3-accel.h
@@ -37,7 +37,7 @@ typedef struct SMMUS1Hwpt {
typedef struct SMMUv3AccelDevice {
SMMUDevice sdev;
- AddressSpace as_sysmem;
+ AddressSpace *as_sysmem;
HostIOMMUDeviceIOMMUFD *idev;
SMMUS1Hwpt *s1_hwpt;
SMMUViommu *viommu;
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index c459d24427..9bb9554435 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -168,6 +168,8 @@ struct SMMUState {
struct SMMUBaseClass {
/* <private> */
SysBusDeviceClass parent_class;
+ /* FIXME is there any better shared place for different vSMMU instances? */
+ AddressSpace *as_sysmem;
/*< public >*/
--
