> -----Original Message-----
> From: Nicolin Chen <nicol...@nvidia.com>
> Sent: 18 September 2025 23:00
> To: Shameer Kolothum <skolothum...@nvidia.com>
> Cc: Shameer Kolothum <shameerkolot...@gmail.com>; qemu-
> a...@nongnu.org; qemu-devel@nongnu.org; eric.au...@redhat.com;
> peter.mayd...@linaro.org; Jason Gunthorpe <j...@nvidia.com>;
> ddut...@redhat.com; berra...@redhat.com; Nathan Chen
> <nath...@nvidia.com>; Matt Ochs <mo...@nvidia.com>;
> smost...@google.com; linux...@huawei.com; wangzh...@hisilicon.com;
> jiangkun...@huawei.com; jonathan.came...@huawei.com;
> zhangfei....@linaro.org; zhenzhong.d...@intel.com
> Subject: Re: [RFC PATCH v3 06/15] hw/arm/smmuv3-accel: Restrict
> accelerated SMMUv3 to vfio-pci endpoints with iommufd
>
> On Thu, Sep 18, 2025 at 06:31:43AM -0700, Shameer Kolothum wrote:
> > > > > @@ -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;
> > > >
> > > > That's changing from an ioas_id per VFIO device to an ioas_id per
> > > > vSMMU instance, right? I think it's still not enough.
> > > >
> > > > All vSMMU instances could share the same ioas_id. That is why I put in
> > > > the SMMUBaseClass as it's shared structure across vSMMUs.
> > >
> > > Ah..you mean it is basically per VM then. Got it.
> >
> > Regarding using SMMUBaseClass for this, it looks like ObjectClass normally
> holds
> > function pointers. Eric has made a similar observation elsewhere in this
> series.
> >
> > @Eric, any suggestions?
> >
> > Is use of &address_space_memory directly in smmuv3_accel_find_add_as()
> a complete
> > no-go, given we are talking about having the system address space for all
> > the
> SMMUv3
> > accelerated devices here?
>
> smmuv3_accel_find_add_as() is per instance. So, it wouldn't share.
My suggestion was...
static AddressSpace *smmuv3_accel_find_add_as(..)
{
...
if (vfio_pci) {
return &address_space_memory;
} else {
return &sdev->as;
}
}
ie, use the global to system memory address space instead of creating an
alias to the system memory and a different address space. This will provide
the same pointer to VFIO/iommufd and it can then reuse the ioas_id.
I can see that QEMU uses "&address_space_memory" directly in many places
(pci_device_iommu_address_space(), etc). I think the idea behind a separate
address space is to have private ownership and lifetime management probably.
Not sure there are any other concerns here. Please let me know if there are
any.
> Even smmuv3_accel_init() wouldn't work.
>
> If SMMUBaseClass isn't a good place, (I haven't tested but..) can
> we do address_space_init() in the VMS? I see virt.c calling the
> memory_region_init(), so perhaps we could init the as_sysmem there
> and pass it in to each vSMMU instance.
That will still require you to store the address space pointer somewhere and
each time a SMMUv3 accel instance is created set a property to the instance
via the hotplug handler. Doable but still...
Thanks,
Shameer
