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

[Eric Auger <eric.auger@redhat.com>]

On 7/16/25 10:36 AM, Shameerali Kolothum Thodi wrote:
>
>> -----Original Message-----
>> From: Nicolin Chen <nicolinc@nvidia.com>
>> Sent: Wednesday, July 16, 2025 12:13 AM
>> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
>> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
>> eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
>> ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org;
>> zhenzhong.duan@intel.com; shameerkolothum@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
Easy to say but I think we shall rather look at what the spec mandates
and not what the Linux driver does ;-)

Thanks

Eric
> 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 
>