> -----Original Message----- > From: Peter Maydell <[email protected]> > Sent: 20 September 2025 14:04 > To: Shameer Kolothum <[email protected]> > Cc: Nicolin Chen <[email protected]>; Shameer Kolothum > <[email protected]>; [email protected]; qemu- > [email protected]; [email protected]; Jason Gunthorpe > <[email protected]>; [email protected]; [email protected]; Nathan > Chen <[email protected]>; Matt Ochs <[email protected]>; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected] > Subject: Re: [RFC PATCH v3 06/15] hw/arm/smmuv3-accel: Restrict accelerated > SMMUv3 to vfio-pci endpoints with iommufd > > External email: Use caution opening links or attachments > > > On Fri, 19 Sept 2025 at 08:38, Shameer Kolothum > <[email protected]> wrote: > > > > > > > > > -----Original Message----- > > > From: Nicolin Chen <[email protected]> > > > Sent: 18 September 2025 23:00 > > > To: Shameer Kolothum <[email protected]> > > > Cc: Shameer Kolothum <[email protected]>; qemu- > > > [email protected]; [email protected]; [email protected]; > > > [email protected]; Jason Gunthorpe <[email protected]>; > > > [email protected]; [email protected]; Nathan Chen > > > <[email protected]>; Matt Ochs <[email protected]>; > > > [email protected]; [email protected]; > [email protected]; > > > [email protected]; [email protected]; > > > [email protected]; [email protected] > > > 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. > > I don't know about vfio, but why is it special here? Generally > directly doing stuff with address_space_memory is not a good > idea : you should be dealing with whatever address space the > board model handed you as what you do transactions to. We > have a lot of legacy code that assumes it can directly work > with address_space_memory, but we should usually try to avoid > adding new code that does that.
The goal is to return the same system address space pointer for all devices behind the accelerated SMMUv3s in a VM. That way VFIO/iommufd can reuse a single IOAS ID in iommufd_cdev_attach(), allowing the Stage-2 page tables to be shared within the VM instead of duplicating them for every SMMUv3 instance. If directly using &address_space_memory in SMMUv3 is discouraged, the other two options considered so far are, -Make use of SMMUv3 ObjectClass to create a separate Address Space so that all instances can return the same pointer. But it looks like #ObjectClass is mainly to hold function pointers and, AFAICS, AddressSpace is not an object type making it difficult to associate the ptr using object_class_property_add/object_class_property_link etc. -Same problem if we do it within virt code. How can we link it in virt with SMMUv3 dev state? Please let me know if I am missing something and there is a way around this. Any pointers appreciated. Thanks, Shameer
