Hi Nicolin,
On Wed, 6 Aug 2025 at 01:55, Nicolin Chen <nicol...@nvidia.com> wrote:
>
> 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.
Ok. I think it is better to move that to SMMUv3AccelState and call
address_space_init() in smmuv3_accel_init() instead. Something like
below. Please take a look and let me know.
Thanks,
Shameer
diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index 3b2f45bd88..e7feae931d 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -339,7 +339,6 @@ void smmuv3_accel_batch_cmd(SMMUState *bs, SMMUDevice *sdev,
static SMMUv3AccelDevice *smmuv3_accel_get_dev(SMMUState *bs, SMMUPciBus *sbus,
PCIBus *bus, int devfn)
{
- SMMUv3State *s = ARM_SMMUV3(bs);
SMMUDevice *sdev = sbus->pbdev[devfn];
SMMUv3AccelDevice *accel_dev;
@@ -351,8 +350,6 @@ 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;
@@ -518,6 +515,7 @@ static AddressSpace
*smmuv3_accel_find_msi_as(PCIBus *bus, void *opaque,
int devfn)
{
SMMUState *bs = opaque;
+ SMMUv3State *s = ARM_SMMUV3(bs);
SMMUPciBus *sbus;
SMMUv3AccelDevice *accel_dev;
SMMUDevice *sdev;
@@ -534,7 +532,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 &s->s_accel->as_sysmem;
}
}
@@ -558,6 +556,7 @@ static AddressSpace
*smmuv3_accel_find_add_as(PCIBus *bus, void *opaque,
{
PCIDevice *pdev = pci_find_device(bus, pci_bus_num(bus), devfn);
SMMUState *bs = opaque;
+ SMMUv3State *s = ARM_SMMUV3(bs);
bool vfio_pci = false;
SMMUPciBus *sbus;
SMMUv3AccelDevice *accel_dev;
@@ -575,7 +574,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 &s->s_accel->as_sysmem;
} else {
return &sdev->as;
}
@@ -612,6 +611,8 @@ void smmuv3_accel_init(SMMUv3State *s)
"smmuv3-accel-sysmem", get_system_memory(), 0,
memory_region_size(get_system_memory()));
memory_region_add_subregion(&s_accel->root, 0, &s_accel->sysmem);
+ address_space_init(&s_accel->as_sysmem, &s_accel->root,
+ "smmuv3-accel-sysmem");
}
static void smmuv3_accel_class_init(ObjectClass *oc, const void *data)
diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
index e1e99598b4..8f04f2fa5d 100644
--- a/hw/arm/smmuv3-accel.h
+++ b/hw/arm/smmuv3-accel.h
@@ -37,7 +37,6 @@ typedef struct SMMUS1Hwpt {
typedef struct SMMUv3AccelDevice {
SMMUDevice sdev;
- AddressSpace as_sysmem;
HostIOMMUDeviceIOMMUFD *idev;
SMMUS1Hwpt *s1_hwpt;
SMMUViommu *viommu;
@@ -48,6 +47,7 @@ typedef struct SMMUv3AccelDevice {
typedef struct SMMUv3AccelState {
MemoryRegion root;
MemoryRegion sysmem;
+ AddressSpace as_sysmem;
SMMUViommu *viommu;
struct iommu_hw_info_arm_smmuv3 info;
} SMMUv3AccelState;
