On Fri, Oct 31, 2025 at 10:49:45AM +0000, Shameer Kolothum wrote:
> +static bool
> +smmuv3_accel_dev_alloc_viommu(SMMUv3AccelDevice *accel_dev,
> + HostIOMMUDeviceIOMMUFD *idev, Error **errp)
Let's make it simply do alloc() on s_accel:
static bool smmuv3_accel_alloc_viommu(SMMUv3AccelState *s_accel,
HostIOMMUDeviceIOMMUFD *idev,
Error **errp)
Then..
> + SMMUDevice *sdev = &accel_dev->sdev;
> + SMMUState *bs = sdev->smmu;
> + SMMUv3State *s = ARM_SMMUV3(bs);
> + SMMUv3AccelState *s_accel = s->s_accel;
Drop these.
> + if (s_accel->vsmmu) {
> + accel_dev->vsmmu = s_accel->vsmmu;
> + return true;
> + }
And this.
> +static bool smmuv3_accel_set_iommu_device(PCIBus *bus, void *opaque, int
> devfn,
> + HostIOMMUDevice *hiod, Error
> **errp)
[...]
> + if (!smmuv3_accel_dev_alloc_viommu(accel_dev, idev, errp)) {
> + error_append_hint(errp, "Device 0x%x: Unable to alloc viommu", sid);
> + return false;
> + }
And here:
if (!s_accel->vsmmu && !smmuv3_accel_alloc_viommu(s_accel, idev, errp)) {
error_append_hint(errp, "Device 0x%x: Unable to alloc viommu", sid);
return false;
}
accel_dev->idev = idev;
accel_dev->vsmmu = s_accel->vsmmu;
Feels slightly cleaner.
> +/*
> + * Represents a virtual SMMU instance backed by an iommufd vIOMMU object.
> + * Holds references to the core iommufd vIOMMU object and to proxy HWPTs
I read "reference" as a pointer, yet...
> + * (bypass and abort) used for device attachment.
> + */
> +typedef struct SMMUViommu {
> + IOMMUFDBackend *iommufd;
> + IOMMUFDViommu viommu;
> + uint32_t bypass_hwpt_id;
> + uint32_t abort_hwpt_id;
...viommu is a containment and two HWPTs are IDs.
So, it'd sound more accurate, being:
/*
* Represents a virtual SMMU instance backed by an iommufd vIOMMU object.
* Holds bypass and abort proxy HWPT ids used for device attachment.
*/
> +typedef struct SMMUv3AccelState {
> + SMMUViommu *vsmmu;
> +} SMMUv3AccelState;
Hmm, maybe we don't need another layer of structure. Every access
or validation to s_accel is for s_accel->vsmmu.
e.g.
if (!s_accel || !s_accel->vsmmu) {
could be
if (!s_accel) {
So, let's try merging them into one. And feel free to leave one
of your favorite.
Nic
