| Message ID | 20231114135654.30475-4-quic_bibekkum@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs | expand |
On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro <quic_bibekkum@quicinc.com> wrote: > > Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs > through SoC specific reset ops, which is disabled in the default MMU-500 > reset ops, but is expected for context banks using ACTLR register to > retain the prefetch value during reset and runtime suspend. Please refer to Documentation/process/submitting-patches.rst and rephrase this following the rules there. > > Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > --- > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 ++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > index 0eaf6f2a2e49..fa867b1d9d16 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct device *dev) > return match ? IOMMU_DOMAIN_IDENTITY : 0; > } > > +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) > +{ > + int i; > + u32 reg; > + > + arm_mmu500_reset(smmu); > + > + /* Re-enable context caching after reset */ > + for (i = 0; i < smmu->num_context_banks; ++i) { > + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); > + reg |= CPRE; > + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg); > + } > + > + return 0; > +} > + > static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) > { > int ret; > > - arm_mmu500_reset(smmu); > + qcom_smmu500_reset(smmu); Is this applicable for sdm845? For all other platforms supported by qcom_smmu_500 implementation? > > /* > * To address performance degradation in non-real time clients, > @@ -509,7 +526,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = { > .init_context = qcom_smmu_init_context, > .cfg_probe = qcom_smmu_cfg_probe, > .def_domain_type = qcom_smmu_def_domain_type, > - .reset = arm_mmu500_reset, > + .reset = qcom_smmu500_reset, > .write_s2cr = qcom_smmu_write_s2cr, > .tlb_sync = qcom_smmu_tlb_sync, > }; > @@ -528,7 +545,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = { > .init_context = qcom_smmu_init_context, > .cfg_probe = qcom_smmu_cfg_probe, > .def_domain_type = qcom_smmu_def_domain_type, > - .reset = arm_mmu500_reset, > + .reset = qcom_smmu500_reset, > .write_s2cr = qcom_smmu_write_s2cr, > .tlb_sync = qcom_smmu_tlb_sync, > }; > @@ -544,7 +561,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = { > static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = { > .init_context = qcom_adreno_smmu_init_context, > .def_domain_type = qcom_smmu_def_domain_type, > - .reset = arm_mmu500_reset, > + .reset = qcom_smmu500_reset, > .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank, > .write_sctlr = qcom_adreno_smmu_write_sctlr, > .tlb_sync = qcom_smmu_tlb_sync, > -- > 2.17.1 >
On 11/14/2023 7:45 PM, Dmitry Baryshkov wrote: > On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro > <quic_bibekkum@quicinc.com> wrote: >> >> Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs >> through SoC specific reset ops, which is disabled in the default MMU-500 >> reset ops, but is expected for context banks using ACTLR register to >> retain the prefetch value during reset and runtime suspend. > > Please refer to Documentation/process/submitting-patches.rst and > rephrase this following the rules there. > Noted, will go through the description once and rephrase it in next version complying with rules. >> >> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >> --- >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 ++++++++++++++++++---- >> 1 file changed, 21 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> index 0eaf6f2a2e49..fa867b1d9d16 100644 >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct device *dev) >> return match ? IOMMU_DOMAIN_IDENTITY : 0; >> } >> >> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) >> +{ >> + int i; >> + u32 reg; >> + >> + arm_mmu500_reset(smmu); >> + >> + /* Re-enable context caching after reset */ >> + for (i = 0; i < smmu->num_context_banks; ++i) { >> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); >> + reg |= CPRE; >> + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg); >> + } >> + >> + return 0; >> +} >> + >> static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) >> { >> int ret; >> >> - arm_mmu500_reset(smmu); >> + qcom_smmu500_reset(smmu); > > Is this applicable for sdm845? For all other platforms supported by > qcom_smmu_500 implementation? > In arm_mmu500_reset operation drivers/iommu/arm/arm-smmu/arm-smmu-impl.c CPRE bit is reset for all SoC based on mmu500 platform, hence for all Qualcomm SoCs including sm845 we are setting back the CPRE bit. >> >> /* >> * To address performance degradation in non-real time clients, >> @@ -509,7 +526,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = { >> .init_context = qcom_smmu_init_context, >> .cfg_probe = qcom_smmu_cfg_probe, >> .def_domain_type = qcom_smmu_def_domain_type, >> - .reset = arm_mmu500_reset, >> + .reset = qcom_smmu500_reset, >> .write_s2cr = qcom_smmu_write_s2cr, >> .tlb_sync = qcom_smmu_tlb_sync, >> }; >> @@ -528,7 +545,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = { >> .init_context = qcom_smmu_init_context, >> .cfg_probe = qcom_smmu_cfg_probe, >> .def_domain_type = qcom_smmu_def_domain_type, >> - .reset = arm_mmu500_reset, >> + .reset = qcom_smmu500_reset, >> .write_s2cr = qcom_smmu_write_s2cr, >> .tlb_sync = qcom_smmu_tlb_sync, >> }; >> @@ -544,7 +561,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = { >> static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = { >> .init_context = qcom_adreno_smmu_init_context, >> .def_domain_type = qcom_smmu_def_domain_type, >> - .reset = arm_mmu500_reset, >> + .reset = qcom_smmu500_reset, >> .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank, >> .write_sctlr = qcom_adreno_smmu_write_sctlr, >> .tlb_sync = qcom_smmu_tlb_sync, >> -- >> 2.17.1 >> > >
On Wed, 15 Nov 2023 at 11:45, Bibek Kumar Patro <quic_bibekkum@quicinc.com> wrote: > > On 11/14/2023 7:45 PM, Dmitry Baryshkov wrote: > > On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro > > <quic_bibekkum@quicinc.com> wrote: > >> > >> Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs > >> through SoC specific reset ops, which is disabled in the default MMU-500 > >> reset ops, but is expected for context banks using ACTLR register to > >> retain the prefetch value during reset and runtime suspend. > > > > Please refer to Documentation/process/submitting-patches.rst and > > rephrase this following the rules there. > > > > Noted, will go through the description once and rephrase it > in next version complying with rules. > > >> > >> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > >> --- > >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 ++++++++++++++++++---- > >> 1 file changed, 21 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >> index 0eaf6f2a2e49..fa867b1d9d16 100644 > >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >> @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct device *dev) > >> return match ? IOMMU_DOMAIN_IDENTITY : 0; > >> } > >> > >> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) > >> +{ > >> + int i; > >> + u32 reg; > >> + > >> + arm_mmu500_reset(smmu); > >> + > >> + /* Re-enable context caching after reset */ > >> + for (i = 0; i < smmu->num_context_banks; ++i) { > >> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); > >> + reg |= CPRE; > >> + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg); > >> + } > >> + > >> + return 0; > >> +} > >> + > >> static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) > >> { > >> int ret; > >> > >> - arm_mmu500_reset(smmu); > >> + qcom_smmu500_reset(smmu); > > > > Is this applicable for sdm845? For all other platforms supported by > > qcom_smmu_500 implementation? > > > > In arm_mmu500_reset operation drivers/iommu/arm/arm-smmu/arm-smmu-impl.c > CPRE bit is reset for all SoC based on mmu500 platform, hence for all > Qualcomm SoCs including sm845 we are setting back the CPRE bit. The errata for the CoreLink MMU-500 requires CPRE to be disabled for all revisions before r2p2. Do we know whether these SoC used CoreLink MMU-500 and which version of it? > > >> > >> /* > >> * To address performance degradation in non-real time clients, > >> @@ -509,7 +526,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = { > >> .init_context = qcom_smmu_init_context, > >> .cfg_probe = qcom_smmu_cfg_probe, > >> .def_domain_type = qcom_smmu_def_domain_type, > >> - .reset = arm_mmu500_reset, > >> + .reset = qcom_smmu500_reset, > >> .write_s2cr = qcom_smmu_write_s2cr, > >> .tlb_sync = qcom_smmu_tlb_sync, > >> }; > >> @@ -528,7 +545,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = { > >> .init_context = qcom_smmu_init_context, > >> .cfg_probe = qcom_smmu_cfg_probe, > >> .def_domain_type = qcom_smmu_def_domain_type, > >> - .reset = arm_mmu500_reset, > >> + .reset = qcom_smmu500_reset, > >> .write_s2cr = qcom_smmu_write_s2cr, > >> .tlb_sync = qcom_smmu_tlb_sync, > >> }; > >> @@ -544,7 +561,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = { > >> static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = { > >> .init_context = qcom_adreno_smmu_init_context, > >> .def_domain_type = qcom_smmu_def_domain_type, > >> - .reset = arm_mmu500_reset, > >> + .reset = qcom_smmu500_reset, > >> .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank, > >> .write_sctlr = qcom_adreno_smmu_write_sctlr, > >> .tlb_sync = qcom_smmu_tlb_sync, > >> -- > >> 2.17.1 > >> > > > > -- With best wishes Dmitry
On 11/14/23 14:56, Bibek Kumar Patro wrote: > Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs > through SoC specific reset ops, which is disabled in the default MMU-500 > reset ops, but is expected for context banks using ACTLR register to > retain the prefetch value during reset and runtime suspend. > > Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > --- And I assume that goes for all SMMU500 implementations? Looking at the 8550 ACTRL array from patch 2, CPRE is not enabled at all times.. Is that because of performance, or some other technical reason? Will this regress platforms without ACTRL tables? Konrad
On 11/15/2023 4:33 PM, Dmitry Baryshkov wrote: > On Wed, 15 Nov 2023 at 11:45, Bibek Kumar Patro > <quic_bibekkum@quicinc.com> wrote: >> >> On 11/14/2023 7:45 PM, Dmitry Baryshkov wrote: >>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro >>> <quic_bibekkum@quicinc.com> wrote: >>>> >>>> Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs >>>> through SoC specific reset ops, which is disabled in the default MMU-500 >>>> reset ops, but is expected for context banks using ACTLR register to >>>> retain the prefetch value during reset and runtime suspend. >>> >>> Please refer to Documentation/process/submitting-patches.rst and >>> rephrase this following the rules there. >>> >> >> Noted, will go through the description once and rephrase it >> in next version complying with rules. >> >>>> >>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >>>> --- >>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 ++++++++++++++++++---- >>>> 1 file changed, 21 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>> index 0eaf6f2a2e49..fa867b1d9d16 100644 >>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>> @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct device *dev) >>>> return match ? IOMMU_DOMAIN_IDENTITY : 0; >>>> } >>>> >>>> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) >>>> +{ >>>> + int i; >>>> + u32 reg; >>>> + >>>> + arm_mmu500_reset(smmu); >>>> + >>>> + /* Re-enable context caching after reset */ >>>> + for (i = 0; i < smmu->num_context_banks; ++i) { >>>> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); >>>> + reg |= CPRE; >>>> + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg); >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) >>>> { >>>> int ret; >>>> >>>> - arm_mmu500_reset(smmu); >>>> + qcom_smmu500_reset(smmu); >>> >>> Is this applicable for sdm845? For all other platforms supported by >>> qcom_smmu_500 implementation? >>> >> >> In arm_mmu500_reset operation drivers/iommu/arm/arm-smmu/arm-smmu-impl.c >> CPRE bit is reset for all SoC based on mmu500 platform, hence for all >> Qualcomm SoCs including sm845 we are setting back the CPRE bit. > > The errata for the CoreLink MMU-500 requires CPRE to be disabled for > all revisions before r2p2. Do we know whether these SoC used CoreLink > MMU-500 and which version of it? > Just checked all these SoCs are using r2p4 revision. So CPRE needs to be enabled back here then? >> >>>> >>>> /* >>>> * To address performance degradation in non-real time clients, >>>> @@ -509,7 +526,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = { >>>> .init_context = qcom_smmu_init_context, >>>> .cfg_probe = qcom_smmu_cfg_probe, >>>> .def_domain_type = qcom_smmu_def_domain_type, >>>> - .reset = arm_mmu500_reset, >>>> + .reset = qcom_smmu500_reset, >>>> .write_s2cr = qcom_smmu_write_s2cr, >>>> .tlb_sync = qcom_smmu_tlb_sync, >>>> }; >>>> @@ -528,7 +545,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = { >>>> .init_context = qcom_smmu_init_context, >>>> .cfg_probe = qcom_smmu_cfg_probe, >>>> .def_domain_type = qcom_smmu_def_domain_type, >>>> - .reset = arm_mmu500_reset, >>>> + .reset = qcom_smmu500_reset, >>>> .write_s2cr = qcom_smmu_write_s2cr, >>>> .tlb_sync = qcom_smmu_tlb_sync, >>>> }; >>>> @@ -544,7 +561,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = { >>>> static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = { >>>> .init_context = qcom_adreno_smmu_init_context, >>>> .def_domain_type = qcom_smmu_def_domain_type, >>>> - .reset = arm_mmu500_reset, >>>> + .reset = qcom_smmu500_reset, >>>> .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank, >>>> .write_sctlr = qcom_adreno_smmu_write_sctlr, >>>> .tlb_sync = qcom_smmu_tlb_sync, >>>> -- >>>> 2.17.1 >>>> >>> >>> > > > > -- > With best wishes > Dmitry
On Thu, 16 Nov 2023 at 14:45, Bibek Kumar Patro <quic_bibekkum@quicinc.com> wrote: > > > > On 11/15/2023 4:33 PM, Dmitry Baryshkov wrote: > > On Wed, 15 Nov 2023 at 11:45, Bibek Kumar Patro > > <quic_bibekkum@quicinc.com> wrote: > >> > >> On 11/14/2023 7:45 PM, Dmitry Baryshkov wrote: > >>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro > >>> <quic_bibekkum@quicinc.com> wrote: > >>>> > >>>> Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs > >>>> through SoC specific reset ops, which is disabled in the default MMU-500 > >>>> reset ops, but is expected for context banks using ACTLR register to > >>>> retain the prefetch value during reset and runtime suspend. > >>> > >>> Please refer to Documentation/process/submitting-patches.rst and > >>> rephrase this following the rules there. > >>> > >> > >> Noted, will go through the description once and rephrase it > >> in next version complying with rules. > >> > >>>> > >>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > >>>> --- > >>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 ++++++++++++++++++---- > >>>> 1 file changed, 21 insertions(+), 4 deletions(-) > >>>> > >>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >>>> index 0eaf6f2a2e49..fa867b1d9d16 100644 > >>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >>>> @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct device *dev) > >>>> return match ? IOMMU_DOMAIN_IDENTITY : 0; > >>>> } > >>>> > >>>> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) > >>>> +{ > >>>> + int i; > >>>> + u32 reg; > >>>> + > >>>> + arm_mmu500_reset(smmu); > >>>> + > >>>> + /* Re-enable context caching after reset */ > >>>> + for (i = 0; i < smmu->num_context_banks; ++i) { > >>>> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); > >>>> + reg |= CPRE; > >>>> + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg); > >>>> + } > >>>> + > >>>> + return 0; > >>>> +} > >>>> + > >>>> static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) > >>>> { > >>>> int ret; > >>>> > >>>> - arm_mmu500_reset(smmu); > >>>> + qcom_smmu500_reset(smmu); > >>> > >>> Is this applicable for sdm845? For all other platforms supported by > >>> qcom_smmu_500 implementation? > >>> > >> > >> In arm_mmu500_reset operation drivers/iommu/arm/arm-smmu/arm-smmu-impl.c > >> CPRE bit is reset for all SoC based on mmu500 platform, hence for all > >> Qualcomm SoCs including sm845 we are setting back the CPRE bit. > > > > The errata for the CoreLink MMU-500 requires CPRE to be disabled for > > all revisions before r2p2. Do we know whether these SoC used CoreLink > > MMU-500 and which version of it? > > > > Just checked all these SoCs are using r2p4 revision. > So CPRE needs to be enabled back here then? can be enabled, yes. > > >> > >>>> > >>>> /* > >>>> * To address performance degradation in non-real time clients, > >>>> @@ -509,7 +526,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = { > >>>> .init_context = qcom_smmu_init_context, > >>>> .cfg_probe = qcom_smmu_cfg_probe, > >>>> .def_domain_type = qcom_smmu_def_domain_type, > >>>> - .reset = arm_mmu500_reset, > >>>> + .reset = qcom_smmu500_reset, > >>>> .write_s2cr = qcom_smmu_write_s2cr, > >>>> .tlb_sync = qcom_smmu_tlb_sync, > >>>> }; > >>>> @@ -528,7 +545,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = { > >>>> .init_context = qcom_smmu_init_context, > >>>> .cfg_probe = qcom_smmu_cfg_probe, > >>>> .def_domain_type = qcom_smmu_def_domain_type, > >>>> - .reset = arm_mmu500_reset, > >>>> + .reset = qcom_smmu500_reset, > >>>> .write_s2cr = qcom_smmu_write_s2cr, > >>>> .tlb_sync = qcom_smmu_tlb_sync, > >>>> }; > >>>> @@ -544,7 +561,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = { > >>>> static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = { > >>>> .init_context = qcom_adreno_smmu_init_context, > >>>> .def_domain_type = qcom_smmu_def_domain_type, > >>>> - .reset = arm_mmu500_reset, > >>>> + .reset = qcom_smmu500_reset, > >>>> .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank, > >>>> .write_sctlr = qcom_adreno_smmu_write_sctlr, > >>>> .tlb_sync = qcom_smmu_tlb_sync, > >>>> -- > >>>> 2.17.1 > >>>> > >>> > >>> > > > > > > > > -- > > With best wishes > > Dmitry
On 16/11/2023 3:24 pm, Dmitry Baryshkov wrote: > On Thu, 16 Nov 2023 at 14:45, Bibek Kumar Patro > <quic_bibekkum@quicinc.com> wrote: >> >> >> >> On 11/15/2023 4:33 PM, Dmitry Baryshkov wrote: >>> On Wed, 15 Nov 2023 at 11:45, Bibek Kumar Patro >>> <quic_bibekkum@quicinc.com> wrote: >>>> >>>> On 11/14/2023 7:45 PM, Dmitry Baryshkov wrote: >>>>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro >>>>> <quic_bibekkum@quicinc.com> wrote: >>>>>> >>>>>> Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs >>>>>> through SoC specific reset ops, which is disabled in the default MMU-500 >>>>>> reset ops, but is expected for context banks using ACTLR register to >>>>>> retain the prefetch value during reset and runtime suspend. >>>>> >>>>> Please refer to Documentation/process/submitting-patches.rst and >>>>> rephrase this following the rules there. >>>>> >>>> >>>> Noted, will go through the description once and rephrase it >>>> in next version complying with rules. >>>> >>>>>> >>>>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >>>>>> --- >>>>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 ++++++++++++++++++---- >>>>>> 1 file changed, 21 insertions(+), 4 deletions(-) >>>>>> >>>>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>> index 0eaf6f2a2e49..fa867b1d9d16 100644 >>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>> @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct device *dev) >>>>>> return match ? IOMMU_DOMAIN_IDENTITY : 0; >>>>>> } >>>>>> >>>>>> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) >>>>>> +{ >>>>>> + int i; >>>>>> + u32 reg; >>>>>> + >>>>>> + arm_mmu500_reset(smmu); >>>>>> + >>>>>> + /* Re-enable context caching after reset */ >>>>>> + for (i = 0; i < smmu->num_context_banks; ++i) { >>>>>> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); >>>>>> + reg |= CPRE; >>>>>> + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg); >>>>>> + } >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) >>>>>> { >>>>>> int ret; >>>>>> >>>>>> - arm_mmu500_reset(smmu); >>>>>> + qcom_smmu500_reset(smmu); >>>>> >>>>> Is this applicable for sdm845? For all other platforms supported by >>>>> qcom_smmu_500 implementation? >>>>> >>>> >>>> In arm_mmu500_reset operation drivers/iommu/arm/arm-smmu/arm-smmu-impl.c >>>> CPRE bit is reset for all SoC based on mmu500 platform, hence for all >>>> Qualcomm SoCs including sm845 we are setting back the CPRE bit. >>> >>> The errata for the CoreLink MMU-500 requires CPRE to be disabled for >>> all revisions before r2p2. Do we know whether these SoC used CoreLink >>> MMU-500 and which version of it? >>> >> >> Just checked all these SoCs are using r2p4 revision. >> So CPRE needs to be enabled back here then? > > can be enabled, yes. There are still open errata #562869 and #1047329 which might need this workaround. I guess one could argue that we're not (knowingly) using nested translation at the moment, and also probably not running this in situations which would end up using short-descriptor format, however stuff like pKVM and IOMMUFD could potentially change those assumptions in future, so they still feel a bit sketchy to me. Thanks, Robin. > >> >>>> >>>>>> >>>>>> /* >>>>>> * To address performance degradation in non-real time clients, >>>>>> @@ -509,7 +526,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = { >>>>>> .init_context = qcom_smmu_init_context, >>>>>> .cfg_probe = qcom_smmu_cfg_probe, >>>>>> .def_domain_type = qcom_smmu_def_domain_type, >>>>>> - .reset = arm_mmu500_reset, >>>>>> + .reset = qcom_smmu500_reset, >>>>>> .write_s2cr = qcom_smmu_write_s2cr, >>>>>> .tlb_sync = qcom_smmu_tlb_sync, >>>>>> }; >>>>>> @@ -528,7 +545,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = { >>>>>> .init_context = qcom_smmu_init_context, >>>>>> .cfg_probe = qcom_smmu_cfg_probe, >>>>>> .def_domain_type = qcom_smmu_def_domain_type, >>>>>> - .reset = arm_mmu500_reset, >>>>>> + .reset = qcom_smmu500_reset, >>>>>> .write_s2cr = qcom_smmu_write_s2cr, >>>>>> .tlb_sync = qcom_smmu_tlb_sync, >>>>>> }; >>>>>> @@ -544,7 +561,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = { >>>>>> static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = { >>>>>> .init_context = qcom_adreno_smmu_init_context, >>>>>> .def_domain_type = qcom_smmu_def_domain_type, >>>>>> - .reset = arm_mmu500_reset, >>>>>> + .reset = qcom_smmu500_reset, >>>>>> .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank, >>>>>> .write_sctlr = qcom_adreno_smmu_write_sctlr, >>>>>> .tlb_sync = qcom_smmu_tlb_sync, >>>>>> -- >>>>>> 2.17.1 >>>>>> >>>>> >>>>> >>> >>> >>> >>> -- >>> With best wishes >>> Dmitry > > >
On 11/15/2023 10:13 PM, Konrad Dybcio wrote: > > > On 11/14/23 14:56, Bibek Kumar Patro wrote: >> Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs >> through SoC specific reset ops, which is disabled in the default MMU-500 >> reset ops, but is expected for context banks using ACTLR register to >> retain the prefetch value during reset and runtime suspend. >> >> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >> --- > And I assume that goes for all SMMU500 implementations? > Right, for all SMMU500 implementation for Qualcomm SoCs. Hence implemented this enablement with Qualcomm specific reset operation. > Looking at the 8550 ACTRL array from patch 2, CPRE is not enabled > at all times.. Is that because of performance, or some other > technical reason? > > Will this regress platforms without ACTRL tables? > It should not regress, If you check my recent reply on Dimitry's response, the Corelink revision is r2p4 and it can be enabled. On the Robin's mentioned errata workarounds, let me check once. Thanks & regards, Bibek > Konrad
On 11/16/2023 10:34 PM, Robin Murphy wrote: > On 16/11/2023 3:24 pm, Dmitry Baryshkov wrote: >> On Thu, 16 Nov 2023 at 14:45, Bibek Kumar Patro >> <quic_bibekkum@quicinc.com> wrote: >>> >>> >>> >>> On 11/15/2023 4:33 PM, Dmitry Baryshkov wrote: >>>> On Wed, 15 Nov 2023 at 11:45, Bibek Kumar Patro >>>> <quic_bibekkum@quicinc.com> wrote: >>>>> >>>>> On 11/14/2023 7:45 PM, Dmitry Baryshkov wrote: >>>>>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro >>>>>> <quic_bibekkum@quicinc.com> wrote: >>>>>>> >>>>>>> Context caching is re-enabled in the prefetch buffer for Qualcomm >>>>>>> SoCs >>>>>>> through SoC specific reset ops, which is disabled in the default >>>>>>> MMU-500 >>>>>>> reset ops, but is expected for context banks using ACTLR register to >>>>>>> retain the prefetch value during reset and runtime suspend. >>>>>> >>>>>> Please refer to Documentation/process/submitting-patches.rst and >>>>>> rephrase this following the rules there. >>>>>> >>>>> >>>>> Noted, will go through the description once and rephrase it >>>>> in next version complying with rules. >>>>> >>>>>>> >>>>>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >>>>>>> --- >>>>>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 >>>>>>> ++++++++++++++++++---- >>>>>>> 1 file changed, 21 insertions(+), 4 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>>> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>>> index 0eaf6f2a2e49..fa867b1d9d16 100644 >>>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>>> @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct >>>>>>> device *dev) >>>>>>> return match ? IOMMU_DOMAIN_IDENTITY : 0; >>>>>>> } >>>>>>> >>>>>>> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) >>>>>>> +{ >>>>>>> + int i; >>>>>>> + u32 reg; >>>>>>> + >>>>>>> + arm_mmu500_reset(smmu); >>>>>>> + >>>>>>> + /* Re-enable context caching after reset */ >>>>>>> + for (i = 0; i < smmu->num_context_banks; ++i) { >>>>>>> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); >>>>>>> + reg |= CPRE; >>>>>>> + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg); >>>>>>> + } >>>>>>> + >>>>>>> + return 0; >>>>>>> +} >>>>>>> + >>>>>>> static int qcom_sdm845_smmu500_reset(struct arm_smmu_device >>>>>>> *smmu) >>>>>>> { >>>>>>> int ret; >>>>>>> >>>>>>> - arm_mmu500_reset(smmu); >>>>>>> + qcom_smmu500_reset(smmu); >>>>>> >>>>>> Is this applicable for sdm845? For all other platforms supported by >>>>>> qcom_smmu_500 implementation? >>>>>> >>>>> >>>>> In arm_mmu500_reset operation >>>>> drivers/iommu/arm/arm-smmu/arm-smmu-impl.c >>>>> CPRE bit is reset for all SoC based on mmu500 platform, hence for all >>>>> Qualcomm SoCs including sm845 we are setting back the CPRE bit. >>>> >>>> The errata for the CoreLink MMU-500 requires CPRE to be disabled for >>>> all revisions before r2p2. Do we know whether these SoC used CoreLink >>>> MMU-500 and which version of it? >>>> >>> >>> Just checked all these SoCs are using r2p4 revision. >>> So CPRE needs to be enabled back here then? >> >> can be enabled, yes. > > There are still open errata #562869 and #1047329 which might need this > workaround. I guess one could argue that we're not (knowingly) using > nested translation at the moment, and also probably not running this in > situations which would end up using short-descriptor format, however > stuff like pKVM and IOMMUFD could potentially change those assumptions > in future, so they still feel a bit sketchy to me. > Could you help provide some details on these two errata (#562869 and #1047329).Both of these erratum are there for r2p4 revisions as well? Thanks & regards, Bibek > Thanks, > Robin. > >> >>> >>>>> >>>>>>> >>>>>>> /* >>>>>>> * To address performance degradation in non-real time >>>>>>> clients, >>>>>>> @@ -509,7 +526,7 @@ static const struct arm_smmu_impl >>>>>>> qcom_smmu_500_impl = { >>>>>>> .init_context = qcom_smmu_init_context, >>>>>>> .cfg_probe = qcom_smmu_cfg_probe, >>>>>>> .def_domain_type = qcom_smmu_def_domain_type, >>>>>>> - .reset = arm_mmu500_reset, >>>>>>> + .reset = qcom_smmu500_reset, >>>>>>> .write_s2cr = qcom_smmu_write_s2cr, >>>>>>> .tlb_sync = qcom_smmu_tlb_sync, >>>>>>> }; >>>>>>> @@ -528,7 +545,7 @@ static const struct arm_smmu_impl >>>>>>> sm8550_smmu_500_impl = { >>>>>>> .init_context = qcom_smmu_init_context, >>>>>>> .cfg_probe = qcom_smmu_cfg_probe, >>>>>>> .def_domain_type = qcom_smmu_def_domain_type, >>>>>>> - .reset = arm_mmu500_reset, >>>>>>> + .reset = qcom_smmu500_reset, >>>>>>> .write_s2cr = qcom_smmu_write_s2cr, >>>>>>> .tlb_sync = qcom_smmu_tlb_sync, >>>>>>> }; >>>>>>> @@ -544,7 +561,7 @@ static const struct arm_smmu_impl >>>>>>> qcom_adreno_smmu_v2_impl = { >>>>>>> static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = { >>>>>>> .init_context = qcom_adreno_smmu_init_context, >>>>>>> .def_domain_type = qcom_smmu_def_domain_type, >>>>>>> - .reset = arm_mmu500_reset, >>>>>>> + .reset = qcom_smmu500_reset, >>>>>>> .alloc_context_bank = >>>>>>> qcom_adreno_smmu_alloc_context_bank, >>>>>>> .write_sctlr = qcom_adreno_smmu_write_sctlr, >>>>>>> .tlb_sync = qcom_smmu_tlb_sync, >>>>>>> -- >>>>>>> 2.17.1 >>>>>>> >>>>>> >>>>>> >>>> >>>> >>>> >>>> -- >>>> With best wishes >>>> Dmitry >> >> >>
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c index 0eaf6f2a2e49..fa867b1d9d16 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct device *dev) return match ? IOMMU_DOMAIN_IDENTITY : 0; } +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) +{ + int i; + u32 reg; + + arm_mmu500_reset(smmu); + + /* Re-enable context caching after reset */ + for (i = 0; i < smmu->num_context_banks; ++i) { + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); + reg |= CPRE; + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg); + } + + return 0; +} + static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) { int ret; - arm_mmu500_reset(smmu); + qcom_smmu500_reset(smmu); /* * To address performance degradation in non-real time clients, @@ -509,7 +526,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = { .init_context = qcom_smmu_init_context, .cfg_probe = qcom_smmu_cfg_probe, .def_domain_type = qcom_smmu_def_domain_type, - .reset = arm_mmu500_reset, + .reset = qcom_smmu500_reset, .write_s2cr = qcom_smmu_write_s2cr, .tlb_sync = qcom_smmu_tlb_sync, }; @@ -528,7 +545,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = { .init_context = qcom_smmu_init_context, .cfg_probe = qcom_smmu_cfg_probe, .def_domain_type = qcom_smmu_def_domain_type, - .reset = arm_mmu500_reset, + .reset = qcom_smmu500_reset, .write_s2cr = qcom_smmu_write_s2cr, .tlb_sync = qcom_smmu_tlb_sync, }; @@ -544,7 +561,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = { static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = { .init_context = qcom_adreno_smmu_init_context, .def_domain_type = qcom_smmu_def_domain_type, - .reset = arm_mmu500_reset, + .reset = qcom_smmu500_reset, .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank, .write_sctlr = qcom_adreno_smmu_write_sctlr, .tlb_sync = qcom_smmu_tlb_sync,
Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs through SoC specific reset ops, which is disabled in the default MMU-500 reset ops, but is expected for context banks using ACTLR register to retain the prefetch value during reset and runtime suspend. Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> --- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 ++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) -- 2.17.1