linux-kernel - Re: [PATCH 3/5] drm/msm/iommu: introduce msm_iommu_disp_new() for msm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5f80ca29-994c-4a74-a929-0480b0f2c157@quicinc.com>
Date: Tue, 16 Jul 2024 14:09:53 -0700
From: Abhinav Kumar <quic_abhinavk@...cinc.com>
To: Rob Clark <robdclark@...il.com>
CC: <freedreno@...ts.freedesktop.org>,
        Dmitry Baryshkov
	<dmitry.baryshkov@...aro.org>,
        Sean Paul <sean@...rly.run>,
        Marijn Suijten
	<marijn.suijten@...ainline.org>,
        David Airlie <airlied@...il.com>, "Daniel
 Vetter" <daniel@...ll.ch>,
        <dri-devel@...ts.freedesktop.org>, <quic_jesszhan@...cinc.com>,
        <swboyd@...omium.org>, <dianders@...omium.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/5] drm/msm/iommu: introduce msm_iommu_disp_new() for
 msm_kms



On 7/1/2024 1:41 PM, Rob Clark wrote:
> On Fri, Jun 28, 2024 at 2:49 PM Abhinav Kumar <quic_abhinavk@...cinc.com> wrote:
>>
>> Introduce a new API msm_iommu_disp_new() for display use-cases.
>>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
>> ---
>>   drivers/gpu/drm/msm/msm_iommu.c | 26 ++++++++++++++++++++++++++
>>   drivers/gpu/drm/msm/msm_mmu.h   |  1 +
>>   2 files changed, 27 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
>> index a79cd18bc4c9..0420bdc4a224 100644
>> --- a/drivers/gpu/drm/msm/msm_iommu.c
>> +++ b/drivers/gpu/drm/msm/msm_iommu.c
>> @@ -343,6 +343,17 @@ static int msm_gpu_fault_handler(struct iommu_domain *domain, struct device *dev
>>          return 0;
>>   }
>>
>> +static int msm_disp_fault_handler(struct iommu_domain *domain, struct device *dev,
>> +                                 unsigned long iova, int flags, void *arg)
>> +{
>> +       struct msm_iommu *iommu = arg;
>> +
>> +       if (iommu->base.handler)
>> +               return iommu->base.handler(iommu->base.arg, iova, flags, NULL);
>> +
>> +       return -ENOSYS;
>> +}
>> +
>>   static void msm_iommu_resume_translation(struct msm_mmu *mmu)
>>   {
>>          struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(mmu->dev);
>> @@ -434,6 +445,21 @@ struct msm_mmu *msm_iommu_new(struct device *dev, unsigned long quirks)
>>          return &iommu->base;
>>   }
>>
>> +struct msm_mmu *msm_iommu_disp_new(struct device *dev, unsigned long quirks)
>> +{
>> +       struct msm_iommu *iommu;
>> +       struct msm_mmu *mmu;
>> +
>> +       mmu = msm_iommu_new(dev, quirks);
>> +       if (IS_ERR_OR_NULL(mmu))
>> +               return mmu;
>> +
>> +       iommu = to_msm_iommu(mmu);
>> +       iommu_set_fault_handler(iommu->domain, msm_disp_fault_handler, iommu);
>> +
>> +       return mmu;
>> +}
> 
> Hmm, are we using dev drvdata for the display pdev?  If
> dev_get_drvdata() returns NULL for display pdev, we could get away
> without having a different fault handler.
> 
> BR,
> -R

It is being set to struct msm_drm_private* currently. So it shouldnt 
return NULL.

I also thought of re-using the same API as GPU but the drvdata along 
with its own fault handler and having below code in the gpu handler all 
made me conclude that its cleaner to let display have its own handler.

         if (adreno_smmu->set_stall)
                 adreno_smmu->set_stall(adreno_smmu->cookie, true);

> 
>> +
>>   struct msm_mmu *msm_iommu_gpu_new(struct device *dev, struct msm_gpu *gpu, unsigned long quirks)
>>   {
>>          struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(dev);
>> diff --git a/drivers/gpu/drm/msm/msm_mmu.h b/drivers/gpu/drm/msm/msm_mmu.h
>> index 88af4f490881..730458d08d6b 100644
>> --- a/drivers/gpu/drm/msm/msm_mmu.h
>> +++ b/drivers/gpu/drm/msm/msm_mmu.h
>> @@ -42,6 +42,7 @@ static inline void msm_mmu_init(struct msm_mmu *mmu, struct device *dev,
>>
>>   struct msm_mmu *msm_iommu_new(struct device *dev, unsigned long quirks);
>>   struct msm_mmu *msm_iommu_gpu_new(struct device *dev, struct msm_gpu *gpu, unsigned long quirks);
>> +struct msm_mmu *msm_iommu_disp_new(struct device *dev, unsigned long quirks);
>>
>>   static inline void msm_mmu_set_fault_handler(struct msm_mmu *mmu, void *arg,
>>                  int (*handler)(void *arg, unsigned long iova, int flags, void *data))
>> --
>> 2.44.0
>>