Re: [PATCH] iommu/arm-smmu-qcom: NULL pointer check for driver data

[Aravind Vijayakumar <quic_aprasann@quicinc.com>]

On 8/16/2023 6:01 PM, Rob Clark wrote:
> On Wed, Aug 16, 2023 at 3:55 PM Aravind Vijayakumar
> <quic_aprasann@quicinc.com> wrote:
>> The driver_data is NULL when qcom_adreno_smmu_init_context()
>> is called before the dev_set_drvdata() from the client driver
>> and is resulting in kernel crash.
>>
>> So add a null pointer check to handle the scenario
>> where the client driver for the GPU SMMU device would
>> be setting the driver data after the smmu client device
>> probe is done and not necessarily before that. The function
>> qcom_adreno_smmu_init_context() assumes that the client
>> driver always set the driver data using dev_set_drvdata()
>> before the smmu client device probe, but this assumption
>> is not always true.
>>
>> Signed-off-by: Aravind Vijayakumar <quic_aprasann@quicinc.com>
>> ---
>>   drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index c71afda79d64..5323f82264ca 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -231,6 +231,9 @@ static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>>           */
>>
>>          priv = dev_get_drvdata(dev);
>> +       if (!priv)
>> +               return 0;
> could this -EPROBE_DEFER instead, or something like that?  I think you
> patch as proposed would result in per-process gpu pgtables silently
> failing
>
> BR,
> -R

Thanks for the review comments. Returning -EPROBE_DEFER wont work 
because the probe of the client driver (which sets the driver data) will 
never get triggered. However, the probe of the client driver succeeds if 
we return -ENODATA. would that be acceptable?

Regards,

Aravind

>> +
>>          priv->cookie = smmu_domain;
>>          priv->get_ttbr1_cfg = qcom_adreno_smmu_get_ttbr1_cfg;
>>          priv->set_ttbr0_cfg = qcom_adreno_smmu_set_ttbr0_cfg;
>> --
>> 2.40.1
>>