RE: [RFC PATCH v3 09/15] hw/arm/smmuv3-accel: Support nested STE install/uninstall support

[Shameerali Kolothum Thodi via]

> -----Original Message-----
> From: Nicolin Chen <nicol...@nvidia.com>
> Sent: Wednesday, July 16, 2025 12:13 AM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.th...@huawei.com>
> Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org;
> eric.au...@redhat.com; peter.mayd...@linaro.org; j...@nvidia.com;
> ddut...@redhat.com; berra...@redhat.com; nath...@nvidia.com;
> mo...@nvidia.com; smost...@google.com; Linuxarm
> <linux...@huawei.com>; Wangzhou (B) <wangzh...@hisilicon.com>;
> jiangkunkun <jiangkun...@huawei.com>; Jonathan Cameron
> <jonathan.came...@huawei.com>; zhangfei....@linaro.org;
> zhenzhong.d...@intel.com; shameerkolot...@gmail.com
> Subject: Re: [RFC PATCH v3 09/15] hw/arm/smmuv3-accel: Support nested
> STE install/uninstall support
> 
> On Mon, Jul 14, 2025 at 04:59:35PM +0100, Shameer Kolothum wrote:
> > +static void
> > +smmuv3_accel_ste_range(gpointer key, gpointer value, gpointer
> user_data)
> > +{
> > +    SMMUDevice *sdev = (SMMUDevice *)key;
> > +    uint32_t sid = smmu_get_sid(sdev);
> > +    SMMUSIDRange *sid_range = (SMMUSIDRange *)user_data;
> > +
> > +    if (sid >= sid_range->start && sid <= sid_range->end) {
> > +        SMMUv3State *s = sdev->smmu;
> > +        SMMUState *bs = &s->smmu_state;
> > +
> > +        smmuv3_accel_install_nested_ste(bs, sdev, sid);
> > +    }
> > +}
> > +
> > +void
> > +smmuv3_accel_install_nested_ste_range(SMMUState *bs,
> SMMUSIDRange *range)
> > +{
> > +    if (!bs->accel) {
> > +        return;
> > +    }
> > +
> > +    g_hash_table_foreach(bs->configs, smmuv3_accel_ste_range, range);
> 
> This will not work correctly?
> 
> The bs->configs is a cache that gets an entry inserted to when a
> config is fetched via smmuv3_get_config(), which gets invoked by
> smmuv3_notify_iova() and smmuv3_translate() only.
> 
> But CMDQ_OP_CFGI_ALL can actually happen very early, e.g. Linux
> driver does that in the probe() right after SMMU CMDQ is enabled,
> at which point neither smmuv3_notify_iova nor smmuv3_translate
> could ever get invoked, meaning that the g_hash_table is empty.
> 
> Without the acceleration, this foreach works because vSMMU does
> not need to do anything since the cache is indeed empty.
> 
> But, with accel, it must call smmuv3_accel_install_nested_ste().

Ok. The only place I can see CMDQ_OP_CFGI_ALL get invoked by Linux
kernel is during arm_smmu_device_reset() and that is to clear all.
But I am not sure we will have any valid STEs at that time. Just curious,
are you seeing any issues with this at the moment?
 
> So, I think this should foreach the viommu->device_list instead.

But agree. Using device_list is more appropriate unless we cache the 
configs during each install_netsed_ste() path.

Thanks,
Shameer