mbox series

[0/5] Qcom smmu-500 TLB invalidation errata for sdm845

Message ID 20180814105528.20592-1-vivek.gautam@codeaurora.org (mailing list archive)
Headers show
Series Qcom smmu-500 TLB invalidation errata for sdm845 | expand

Message

Vivek Gautam Aug. 14, 2018, 10:55 a.m. UTC
Qcom's implementation of arm,mmu-500 on sdm845 has a functional/performance
errata [1] because of which the TCU cache look ups are stalled during
invalidation cycle. This is mitigated by serializing all the invalidation
requests coming to the smmu.

This patch series addresses this errata by adding new tlb_ops for
qcom,sdm845-smmu-500 [2]. These ops take context bank locks for all the
tlb_ops that queue and sync the TLB invalidation requests.

Besides adding locks, there's a way to expadite these TLB invalidations
for display and camera devices by turning off the 'wait-for-safe' logic
in hardware that holds the tlb invalidations until a safe level.
This 'wait-for-safe' logic is controlled by toggling a chicken bit
through a secure register. This secure register is accessed by making an
explicit SCM call into the EL3 firmware.
There are two ways of handling this logic -
 * Firmware, such as tz present on sdm845-mtp devices has a handler to do
   all the register access and bit set/clear. So is the handling in
   downstream arm-smmu driver [3].
 * Other firmwares can have handlers to just read/write this secure
   register. In such cases the kernel make io_read/writel scm calls to
   modify the register.
This patch series adds APIs in qcom-scm driver to handle both of these
cases.

Lastly, since these TLB invalidations can happen in atomic contexts
there's a need to add atomic versions of qcom_scm_io_readl/writel() and
qcom_scm_call() APIs. The traditional scm calls take mutex and we therefore
can't use these calls in atomic contexts.

This patch series is adapted version of how the errata is handled in
downstream [1].

[1] https://source.codeaurora.org/quic/la/kernel/msm-4.9/tree/drivers/iommu/arm-smmu.c?h=msm-4.9#n4842
[2] https://lore.kernel.org/patchwork/patch/974114/
[3] https://source.codeaurora.org/quic/la/kernel/msm-4.9/tree/drivers/iommu/arm-smmu.c?h=msm-4.9#n4864

Vivek Gautam (5):
  firmware: qcom_scm-64: Add atomic version of qcom_scm_call
  firmware/qcom_scm: Add atomic version of io read/write APIs
  firmware/qcom_scm: Add scm call to handle smmu errata
  iommu/arm-smmu: Make way to add Qcom's smmu-500 errata handling
  iommu/arm-smmu: Add support to handle Qcom's TLBI serialization errata

 drivers/firmware/qcom_scm-32.c |  17 ++++
 drivers/firmware/qcom_scm-64.c | 181 +++++++++++++++++++++++++++++++----------
 drivers/firmware/qcom_scm.c    |  18 ++++
 drivers/firmware/qcom_scm.h    |   9 ++
 drivers/iommu/arm-smmu-regs.h  |   2 +
 drivers/iommu/arm-smmu.c       | 168 ++++++++++++++++++++++++++++++++++++--
 include/linux/qcom_scm.h       |   6 ++
 7 files changed, 348 insertions(+), 53 deletions(-)

Comments

Will Deacon Aug. 14, 2018, 11:40 a.m. UTC | #1
Hi Vivek,

On Tue, Aug 14, 2018 at 04:25:23PM +0530, Vivek Gautam wrote:
> Qcom's implementation of arm,mmu-500 on sdm845 has a functional/performance
> errata [1] because of which the TCU cache look ups are stalled during
> invalidation cycle. This is mitigated by serializing all the invalidation
> requests coming to the smmu.

How does this implementation differ from the one supported by qcom_iommu.c?
I notice you're adding firmware hooks here, which we avoided by having the
extra driver. Please help me understand which devices exist, how they
differ, and which drivers are intended to support them!

Also -- you didn't CC all the maintainers for the firmware bits, so adding
Andy here for that, and Rob for the previous question.

Thanks,

Will
Vivek Gautam Aug. 14, 2018, 12:24 p.m. UTC | #2
Hi Will,


On 8/14/2018 5:10 PM, Will Deacon wrote:
> Hi Vivek,
>
> On Tue, Aug 14, 2018 at 04:25:23PM +0530, Vivek Gautam wrote:
>> Qcom's implementation of arm,mmu-500 on sdm845 has a functional/performance
>> errata [1] because of which the TCU cache look ups are stalled during
>> invalidation cycle. This is mitigated by serializing all the invalidation
>> requests coming to the smmu.
> How does this implementation differ from the one supported by qcom_iommu.c?
> I notice you're adding firmware hooks here, which we avoided by having the
> extra driver. Please help me understand which devices exist, how they
> differ, and which drivers are intended to support them!

IIRC, the qcom_iommu driver was intended to support the static context 
bank - SID
mapping, and is very specific to the smmu-v2 version present on msm8916 soc.
However, this is the qcom's mmu-500 implementation specific errata. 
qcom_iommu
will not be able to support mmu-500 configurations.
Rob Clark can add more.
Let you know what you suggest.

>
> Also -- you didn't CC all the maintainers for the firmware bits, so adding
> Andy here for that, and Rob for the previous question.

I added Andy to the series, would you want me to add Rob H also?

Best regards
Vivek

>
> Thanks,
>
> Will
Vivek Gautam Sept. 5, 2018, 9:22 a.m. UTC | #3
On 8/14/2018 5:54 PM, Vivek Gautam wrote:
> Hi Will,
>
>
> On 8/14/2018 5:10 PM, Will Deacon wrote:
>> Hi Vivek,
>>
>> On Tue, Aug 14, 2018 at 04:25:23PM +0530, Vivek Gautam wrote:
>>> Qcom's implementation of arm,mmu-500 on sdm845 has a 
>>> functional/performance
>>> errata [1] because of which the TCU cache look ups are stalled during
>>> invalidation cycle. This is mitigated by serializing all the 
>>> invalidation
>>> requests coming to the smmu.
>> How does this implementation differ from the one supported by 
>> qcom_iommu.c?
>> I notice you're adding firmware hooks here, which we avoided by 
>> having the
>> extra driver. Please help me understand which devices exist, how they
>> differ, and which drivers are intended to support them!
>
> IIRC, the qcom_iommu driver was intended to support the static context 
> bank - SID
> mapping, and is very specific to the smmu-v2 version present on 
> msm8916 soc.
> However, this is the qcom's mmu-500 implementation specific errata. 
> qcom_iommu
> will not be able to support mmu-500 configurations.
> Rob Clark can add more.
> Let you know what you suggest.

Rob, can you please comment about how qcom-smmu driver has different 
implementation
from arm-smmu driver?
Will, in case we would want to use arm-smmu driver, what would you 
suggest for
having the firmware hooks?
Thanks.

Best regards
Vivek
>
>>
>> Also -- you didn't CC all the maintainers for the firmware bits, so 
>> adding
>> Andy here for that, and Rob for the previous question.
>
> I added Andy to the series, would you want me to add Rob H also?
>
> Best regards
> Vivek
>
>>
>> Thanks,
>>
>> Will
>
Rob Clark Sept. 5, 2018, 10:04 a.m. UTC | #4
On Wed, Sep 5, 2018 at 5:22 AM Vivek Gautam <vivek.gautam@codeaurora.org> wrote:
>
>
> On 8/14/2018 5:54 PM, Vivek Gautam wrote:
> > Hi Will,
> >
> >
> > On 8/14/2018 5:10 PM, Will Deacon wrote:
> >> Hi Vivek,
> >>
> >> On Tue, Aug 14, 2018 at 04:25:23PM +0530, Vivek Gautam wrote:
> >>> Qcom's implementation of arm,mmu-500 on sdm845 has a
> >>> functional/performance
> >>> errata [1] because of which the TCU cache look ups are stalled during
> >>> invalidation cycle. This is mitigated by serializing all the
> >>> invalidation
> >>> requests coming to the smmu.
> >> How does this implementation differ from the one supported by
> >> qcom_iommu.c?
> >> I notice you're adding firmware hooks here, which we avoided by
> >> having the
> >> extra driver. Please help me understand which devices exist, how they
> >> differ, and which drivers are intended to support them!
> >
> > IIRC, the qcom_iommu driver was intended to support the static context
> > bank - SID
> > mapping, and is very specific to the smmu-v2 version present on
> > msm8916 soc.
> > However, this is the qcom's mmu-500 implementation specific errata.
> > qcom_iommu
> > will not be able to support mmu-500 configurations.
> > Rob Clark can add more.
> > Let you know what you suggest.
>
> Rob, can you please comment about how qcom-smmu driver has different
> implementation
> from arm-smmu driver?

sorry, I missed this thread earlier.  But yeah, as you mentioned, the
purpose for qcom_iommu.c was to deal with the static context/SID
mapping.

(I guess it is all just software, and we could make qcom_iommu.c
support dynamic mapping as well, but I think then it starts to
duplicate most of arm_smmu.c, so that doesn't seem like the right
direction)

BR,
-R

> Will, in case we would want to use arm-smmu driver, what would you
> suggest for
> having the firmware hooks?
> Thanks.
>
> Best regards
> Vivek
Vivek Gautam Sept. 5, 2018, 11:25 a.m. UTC | #5
On 9/5/2018 3:34 PM, Rob Clark wrote:
> On Wed, Sep 5, 2018 at 5:22 AM Vivek Gautam <vivek.gautam@codeaurora.org> wrote:
>>
>> On 8/14/2018 5:54 PM, Vivek Gautam wrote:
>>> Hi Will,
>>>
>>>
>>> On 8/14/2018 5:10 PM, Will Deacon wrote:
>>>> Hi Vivek,
>>>>
>>>> On Tue, Aug 14, 2018 at 04:25:23PM +0530, Vivek Gautam wrote:
>>>>> Qcom's implementation of arm,mmu-500 on sdm845 has a
>>>>> functional/performance
>>>>> errata [1] because of which the TCU cache look ups are stalled during
>>>>> invalidation cycle. This is mitigated by serializing all the
>>>>> invalidation
>>>>> requests coming to the smmu.
>>>> How does this implementation differ from the one supported by
>>>> qcom_iommu.c?
>>>> I notice you're adding firmware hooks here, which we avoided by
>>>> having the
>>>> extra driver. Please help me understand which devices exist, how they
>>>> differ, and which drivers are intended to support them!
>>> IIRC, the qcom_iommu driver was intended to support the static context
>>> bank - SID
>>> mapping, and is very specific to the smmu-v2 version present on
>>> msm8916 soc.
>>> However, this is the qcom's mmu-500 implementation specific errata.
>>> qcom_iommu
>>> will not be able to support mmu-500 configurations.
>>> Rob Clark can add more.
>>> Let you know what you suggest.
>> Rob, can you please comment about how qcom-smmu driver has different
>> implementation
>> from arm-smmu driver?
> sorry, I missed this thread earlier.  But yeah, as you mentioned, the
> purpose for qcom_iommu.c was to deal with the static context/SID
> mapping.
>
> (I guess it is all just software, and we could make qcom_iommu.c
> support dynamic mapping as well, but I think then it starts to
> duplicate most of arm_smmu.c, so that doesn't seem like the right
> direction)

Thanks Rob for the response. I will wait for Will's response on how would he
like this support be implemented.

Best regards
Vivek
>
> BR,
> -R
>
>> Will, in case we would want to use arm-smmu driver, what would you
>> suggest for
>> having the firmware hooks?
>> Thanks.
>>
>> Best regards
>> Vivek