diff mbox series

[2/5] iommu/arm-smmu: Emulate bypass by using context banks

Message ID 20200709050145.3520931-3-bjorn.andersson@linaro.org (mailing list archive)
State New, archived
Headers show
Series iommu/arm-smmu: Support maintaining bootloader mappings | expand

Commit Message

Bjorn Andersson July 9, 2020, 5:01 a.m. UTC
Some firmware found on various Qualcomm platforms traps writes to S2CR
of type BYPASS and writes FAULT into the register. This prevents us from
marking the streams for the display controller as BYPASS to allow
continued scanout of the screen through the initialization of the ARM
SMMU.

This adds a Qualcomm specific cfg_probe function, which probes the
behavior of the S2CR registers and if found faulty enables the related
quirk. Based on this quirk context banks are allocated for IDENTITY
domains as well, but with ARM_SMMU_SCTLR_M omitted.

The result is valid stream mappings, without translation.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/iommu/arm-smmu-qcom.c | 21 +++++++++++++++++++++
 drivers/iommu/arm-smmu.c      | 14 ++++++++++++--
 drivers/iommu/arm-smmu.h      |  3 +++
 3 files changed, 36 insertions(+), 2 deletions(-)

Comments

Rob Clark July 9, 2020, 4:17 p.m. UTC | #1
On Wed, Jul 8, 2020 at 10:01 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> Some firmware found on various Qualcomm platforms traps writes to S2CR
> of type BYPASS and writes FAULT into the register. This prevents us from
> marking the streams for the display controller as BYPASS to allow
> continued scanout of the screen through the initialization of the ARM
> SMMU.
>
> This adds a Qualcomm specific cfg_probe function, which probes the
> behavior of the S2CR registers and if found faulty enables the related
> quirk. Based on this quirk context banks are allocated for IDENTITY
> domains as well, but with ARM_SMMU_SCTLR_M omitted.
>
> The result is valid stream mappings, without translation.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  drivers/iommu/arm-smmu-qcom.c | 21 +++++++++++++++++++++
>  drivers/iommu/arm-smmu.c      | 14 ++++++++++++--
>  drivers/iommu/arm-smmu.h      |  3 +++
>  3 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
> index cf01d0215a39..e8a36054e912 100644
> --- a/drivers/iommu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm-smmu-qcom.c
> @@ -23,6 +23,26 @@ static const struct of_device_id qcom_smmu_client_of_match[] = {
>         { }
>  };
>
> +static int qcom_smmu_cfg_probe(struct arm_smmu_device *smmu)
> +{
> +       unsigned int last_s2cr = ARM_SMMU_GR0_S2CR(smmu->num_mapping_groups - 1);
> +       u32 reg;
> +
> +       /*
> +        * With some firmware writes to S2CR of type FAULT are ignored, and
> +        * writing BYPASS will end up as FAULT in the register. Perform a write
> +        * to S2CR to detect if this is the case with the current firmware.
> +        */
> +       arm_smmu_gr0_write(smmu, last_s2cr, FIELD_PREP(ARM_SMMU_S2CR_TYPE, S2CR_TYPE_BYPASS) |
> +                                           FIELD_PREP(ARM_SMMU_S2CR_CBNDX, 0xff) |
> +                                           FIELD_PREP(ARM_SMMU_S2CR_PRIVCFG, S2CR_PRIVCFG_DEFAULT));
> +       reg = arm_smmu_gr0_read(smmu, last_s2cr);
> +       if (FIELD_GET(ARM_SMMU_S2CR_TYPE, reg) != S2CR_TYPE_BYPASS)
> +               smmu->qcom_bypass_quirk = true;
> +
> +       return 0;
> +}
> +
>  static int qcom_smmu_def_domain_type(struct device *dev)
>  {
>         const struct of_device_id *match =
> @@ -61,6 +81,7 @@ static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
>  }
>
>  static const struct arm_smmu_impl qcom_smmu_impl = {
> +       .cfg_probe = qcom_smmu_cfg_probe,
>         .def_domain_type = qcom_smmu_def_domain_type,
>         .reset = qcom_smmu500_reset,
>  };
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 2e27cf9815ab..f33eda3117fa 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -654,7 +654,9 @@ static void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx)
>
>         /* SCTLR */
>         reg = ARM_SMMU_SCTLR_CFIE | ARM_SMMU_SCTLR_CFRE | ARM_SMMU_SCTLR_AFE |
> -             ARM_SMMU_SCTLR_TRE | ARM_SMMU_SCTLR_M;
> +             ARM_SMMU_SCTLR_TRE;
> +       if (cfg->m)
> +               reg |= ARM_SMMU_SCTLR_M;
>         if (stage1)
>                 reg |= ARM_SMMU_SCTLR_S1_ASIDPNE;
>         if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
> @@ -678,7 +680,11 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
>         if (smmu_domain->smmu)
>                 goto out_unlock;
>
> -       if (domain->type == IOMMU_DOMAIN_IDENTITY) {
> +       /*
> +        * Nothing to do for IDENTITY domains,unless disabled context banks are
> +        * used to emulate bypass mappings on Qualcomm platforms.
> +        */
> +       if (domain->type == IOMMU_DOMAIN_IDENTITY && !smmu->qcom_bypass_quirk) {

maybe I'm overlooking something, but I think this would put us back to
allocating pgtables (and making iommu->map/unmap() no longer no-ops),
which I don't think we want

BR,
-R

>                 smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
>                 smmu_domain->smmu = smmu;
>                 goto out_unlock;
> @@ -826,6 +832,10 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
>         domain->geometry.aperture_end = (1UL << ias) - 1;
>         domain->geometry.force_aperture = true;
>
> +       /* Enable translation for non-identity context banks */
> +       if (domain->type != IOMMU_DOMAIN_IDENTITY)
> +               cfg->m = true;
> +
>         /* Initialise the context bank with our page table cfg */
>         arm_smmu_init_context_bank(smmu_domain, &pgtbl_cfg);
>         arm_smmu_write_context_bank(smmu, cfg->cbndx);
> diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
> index d172c024be61..a71d193073e4 100644
> --- a/drivers/iommu/arm-smmu.h
> +++ b/drivers/iommu/arm-smmu.h
> @@ -305,6 +305,8 @@ struct arm_smmu_device {
>
>         /* IOMMU core code handle */
>         struct iommu_device             iommu;
> +
> +       bool                            qcom_bypass_quirk;
>  };
>
>  enum arm_smmu_context_fmt {
> @@ -323,6 +325,7 @@ struct arm_smmu_cfg {
>         };
>         enum arm_smmu_cbar_type         cbar;
>         enum arm_smmu_context_fmt       fmt;
> +       bool                            m;
>  };
>  #define ARM_SMMU_INVALID_IRPTNDX       0xff
>
> --
> 2.26.2
>
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
Bjorn Andersson July 9, 2020, 4:48 p.m. UTC | #2
On Thu 09 Jul 09:17 PDT 2020, Rob Clark wrote:

> On Wed, Jul 8, 2020 at 10:01 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
[..]
> > @@ -678,7 +680,11 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> >         if (smmu_domain->smmu)
> >                 goto out_unlock;
> >
> > -       if (domain->type == IOMMU_DOMAIN_IDENTITY) {
> > +       /*
> > +        * Nothing to do for IDENTITY domains,unless disabled context banks are
> > +        * used to emulate bypass mappings on Qualcomm platforms.
> > +        */
> > +       if (domain->type == IOMMU_DOMAIN_IDENTITY && !smmu->qcom_bypass_quirk) {
> 
> maybe I'm overlooking something, but I think this would put us back to
> allocating pgtables (and making iommu->map/unmap() no longer no-ops),
> which I don't think we want
> 

You're right, we are allocating page tables for these contexts and
map/unmap would modify the page tables. But afaict traversal is never
performed, given that the banks are never enabled.

But as drivers probe properly, or the direct mapped drivers sets up
their iommu domains explicitly with translation this would not be used.

So afaict we're just wasting some memory - for the gain of not
overcomplicating this function.

Regards,
Bjorn

> BR,
> -R
> 
> >                 smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
> >                 smmu_domain->smmu = smmu;
> >                 goto out_unlock;
> > @@ -826,6 +832,10 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> >         domain->geometry.aperture_end = (1UL << ias) - 1;
> >         domain->geometry.force_aperture = true;
> >
> > +       /* Enable translation for non-identity context banks */
> > +       if (domain->type != IOMMU_DOMAIN_IDENTITY)
> > +               cfg->m = true;
> > +
> >         /* Initialise the context bank with our page table cfg */
> >         arm_smmu_init_context_bank(smmu_domain, &pgtbl_cfg);
> >         arm_smmu_write_context_bank(smmu, cfg->cbndx);
> > diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
> > index d172c024be61..a71d193073e4 100644
> > --- a/drivers/iommu/arm-smmu.h
> > +++ b/drivers/iommu/arm-smmu.h
> > @@ -305,6 +305,8 @@ struct arm_smmu_device {
> >
> >         /* IOMMU core code handle */
> >         struct iommu_device             iommu;
> > +
> > +       bool                            qcom_bypass_quirk;
> >  };
> >
> >  enum arm_smmu_context_fmt {
> > @@ -323,6 +325,7 @@ struct arm_smmu_cfg {
> >         };
> >         enum arm_smmu_cbar_type         cbar;
> >         enum arm_smmu_context_fmt       fmt;
> > +       bool                            m;
> >  };
> >  #define ARM_SMMU_INVALID_IRPTNDX       0xff
> >
> > --
> > 2.26.2
> >
> > _______________________________________________
> > iommu mailing list
> > iommu@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/iommu
Rob Clark July 9, 2020, 4:56 p.m. UTC | #3
On Thu, Jul 9, 2020 at 9:48 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Thu 09 Jul 09:17 PDT 2020, Rob Clark wrote:
>
> > On Wed, Jul 8, 2020 at 10:01 PM Bjorn Andersson
> > <bjorn.andersson@linaro.org> wrote:
> [..]
> > > @@ -678,7 +680,11 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> > >         if (smmu_domain->smmu)
> > >                 goto out_unlock;
> > >
> > > -       if (domain->type == IOMMU_DOMAIN_IDENTITY) {
> > > +       /*
> > > +        * Nothing to do for IDENTITY domains,unless disabled context banks are
> > > +        * used to emulate bypass mappings on Qualcomm platforms.
> > > +        */
> > > +       if (domain->type == IOMMU_DOMAIN_IDENTITY && !smmu->qcom_bypass_quirk) {
> >
> > maybe I'm overlooking something, but I think this would put us back to
> > allocating pgtables (and making iommu->map/unmap() no longer no-ops),
> > which I don't think we want
> >
>
> You're right, we are allocating page tables for these contexts and
> map/unmap would modify the page tables. But afaict traversal is never
> performed, given that the banks are never enabled.
>
> But as drivers probe properly, or the direct mapped drivers sets up
> their iommu domains explicitly with translation this would not be used.
>
> So afaict we're just wasting some memory - for the gain of not
> overcomplicating this function.

the problem is that it makes dma_map/unmap less of a no-op than it
should be (for the case where the driver is explicitly managing it's
own domain)..  I was hoping to get rid of the hacks to use dma_sync go
back to dma_map/unmap for cache cleaning

BR,
-R


>
> Regards,
> Bjorn
>
> > BR,
> > -R
> >
> > >                 smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
> > >                 smmu_domain->smmu = smmu;
> > >                 goto out_unlock;
> > > @@ -826,6 +832,10 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> > >         domain->geometry.aperture_end = (1UL << ias) - 1;
> > >         domain->geometry.force_aperture = true;
> > >
> > > +       /* Enable translation for non-identity context banks */
> > > +       if (domain->type != IOMMU_DOMAIN_IDENTITY)
> > > +               cfg->m = true;
> > > +
> > >         /* Initialise the context bank with our page table cfg */
> > >         arm_smmu_init_context_bank(smmu_domain, &pgtbl_cfg);
> > >         arm_smmu_write_context_bank(smmu, cfg->cbndx);
> > > diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
> > > index d172c024be61..a71d193073e4 100644
> > > --- a/drivers/iommu/arm-smmu.h
> > > +++ b/drivers/iommu/arm-smmu.h
> > > @@ -305,6 +305,8 @@ struct arm_smmu_device {
> > >
> > >         /* IOMMU core code handle */
> > >         struct iommu_device             iommu;
> > > +
> > > +       bool                            qcom_bypass_quirk;
> > >  };
> > >
> > >  enum arm_smmu_context_fmt {
> > > @@ -323,6 +325,7 @@ struct arm_smmu_cfg {
> > >         };
> > >         enum arm_smmu_cbar_type         cbar;
> > >         enum arm_smmu_context_fmt       fmt;
> > > +       bool                            m;
> > >  };
> > >  #define ARM_SMMU_INVALID_IRPTNDX       0xff
> > >
> > > --
> > > 2.26.2
> > >
> > > _______________________________________________
> > > iommu mailing list
> > > iommu@lists.linux-foundation.org
> > > https://lists.linuxfoundation.org/mailman/listinfo/iommu
Rob Clark July 9, 2020, 6:55 p.m. UTC | #4
On Thu, Jul 9, 2020 at 9:56 AM Rob Clark <robdclark@gmail.com> wrote:
>
> On Thu, Jul 9, 2020 at 9:48 AM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > On Thu 09 Jul 09:17 PDT 2020, Rob Clark wrote:
> >
> > > On Wed, Jul 8, 2020 at 10:01 PM Bjorn Andersson
> > > <bjorn.andersson@linaro.org> wrote:
> > [..]
> > > > @@ -678,7 +680,11 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> > > >         if (smmu_domain->smmu)
> > > >                 goto out_unlock;
> > > >
> > > > -       if (domain->type == IOMMU_DOMAIN_IDENTITY) {
> > > > +       /*
> > > > +        * Nothing to do for IDENTITY domains,unless disabled context banks are
> > > > +        * used to emulate bypass mappings on Qualcomm platforms.
> > > > +        */
> > > > +       if (domain->type == IOMMU_DOMAIN_IDENTITY && !smmu->qcom_bypass_quirk) {
> > >
> > > maybe I'm overlooking something, but I think this would put us back to
> > > allocating pgtables (and making iommu->map/unmap() no longer no-ops),
> > > which I don't think we want
> > >
> >
> > You're right, we are allocating page tables for these contexts and
> > map/unmap would modify the page tables. But afaict traversal is never
> > performed, given that the banks are never enabled.
> >
> > But as drivers probe properly, or the direct mapped drivers sets up
> > their iommu domains explicitly with translation this would not be used.
> >
> > So afaict we're just wasting some memory - for the gain of not
> > overcomplicating this function.
>
> the problem is that it makes dma_map/unmap less of a no-op than it
> should be (for the case where the driver is explicitly managing it's
> own domain)..  I was hoping to get rid of the hacks to use dma_sync go
> back to dma_map/unmap for cache cleaning

That said, it seems to cause less explosions than before.. either that
or I'm not trying hard enough.  Still, I think it would probably
result in unnecessary tlb inv's.

Previously, *somehow* we seemed to end up with dma_map/unmap trying to
modify the domain that we had attached.

BR,
-R

> BR,
> -R
>
>
> >
> > Regards,
> > Bjorn
> >
> > > BR,
> > > -R
> > >
> > > >                 smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
> > > >                 smmu_domain->smmu = smmu;
> > > >                 goto out_unlock;
> > > > @@ -826,6 +832,10 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> > > >         domain->geometry.aperture_end = (1UL << ias) - 1;
> > > >         domain->geometry.force_aperture = true;
> > > >
> > > > +       /* Enable translation for non-identity context banks */
> > > > +       if (domain->type != IOMMU_DOMAIN_IDENTITY)
> > > > +               cfg->m = true;
> > > > +
> > > >         /* Initialise the context bank with our page table cfg */
> > > >         arm_smmu_init_context_bank(smmu_domain, &pgtbl_cfg);
> > > >         arm_smmu_write_context_bank(smmu, cfg->cbndx);
> > > > diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
> > > > index d172c024be61..a71d193073e4 100644
> > > > --- a/drivers/iommu/arm-smmu.h
> > > > +++ b/drivers/iommu/arm-smmu.h
> > > > @@ -305,6 +305,8 @@ struct arm_smmu_device {
> > > >
> > > >         /* IOMMU core code handle */
> > > >         struct iommu_device             iommu;
> > > > +
> > > > +       bool                            qcom_bypass_quirk;
> > > >  };
> > > >
> > > >  enum arm_smmu_context_fmt {
> > > > @@ -323,6 +325,7 @@ struct arm_smmu_cfg {
> > > >         };
> > > >         enum arm_smmu_cbar_type         cbar;
> > > >         enum arm_smmu_context_fmt       fmt;
> > > > +       bool                            m;
> > > >  };
> > > >  #define ARM_SMMU_INVALID_IRPTNDX       0xff
> > > >
> > > > --
> > > > 2.26.2
> > > >
> > > > _______________________________________________
> > > > iommu mailing list
> > > > iommu@lists.linux-foundation.org
> > > > https://lists.linuxfoundation.org/mailman/listinfo/iommu
Bjorn Andersson July 9, 2020, 7:40 p.m. UTC | #5
On Thu 09 Jul 11:55 PDT 2020, Rob Clark wrote:

> On Thu, Jul 9, 2020 at 9:56 AM Rob Clark <robdclark@gmail.com> wrote:
> >
> > On Thu, Jul 9, 2020 at 9:48 AM Bjorn Andersson
> > <bjorn.andersson@linaro.org> wrote:
> > >
> > > On Thu 09 Jul 09:17 PDT 2020, Rob Clark wrote:
> > >
> > > > On Wed, Jul 8, 2020 at 10:01 PM Bjorn Andersson
> > > > <bjorn.andersson@linaro.org> wrote:
> > > [..]
> > > > > @@ -678,7 +680,11 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> > > > >         if (smmu_domain->smmu)
> > > > >                 goto out_unlock;
> > > > >
> > > > > -       if (domain->type == IOMMU_DOMAIN_IDENTITY) {
> > > > > +       /*
> > > > > +        * Nothing to do for IDENTITY domains,unless disabled context banks are
> > > > > +        * used to emulate bypass mappings on Qualcomm platforms.
> > > > > +        */
> > > > > +       if (domain->type == IOMMU_DOMAIN_IDENTITY && !smmu->qcom_bypass_quirk) {
> > > >
> > > > maybe I'm overlooking something, but I think this would put us back to
> > > > allocating pgtables (and making iommu->map/unmap() no longer no-ops),
> > > > which I don't think we want
> > > >
> > >
> > > You're right, we are allocating page tables for these contexts and
> > > map/unmap would modify the page tables. But afaict traversal is never
> > > performed, given that the banks are never enabled.
> > >
> > > But as drivers probe properly, or the direct mapped drivers sets up
> > > their iommu domains explicitly with translation this would not be used.
> > >
> > > So afaict we're just wasting some memory - for the gain of not
> > > overcomplicating this function.
> >
> > the problem is that it makes dma_map/unmap less of a no-op than it
> > should be (for the case where the driver is explicitly managing it's
> > own domain)..  I was hoping to get rid of the hacks to use dma_sync go
> > back to dma_map/unmap for cache cleaning
> 
> That said, it seems to cause less explosions than before.. either that
> or I'm not trying hard enough.  Still, I think it would probably
> result in unnecessary tlb inv's.
> 
> Previously, *somehow* we seemed to end up with dma_map/unmap trying to
> modify the domain that we had attached.
> 

I might need some help to really understand the plumbing here, but this
is what I read from the code and can see in my debugging...


The display device will upon creation be allocated a default_domain, of
type IOMMU_DOMAIN_IDENTITY - per the qcom-impl. Then you then allocate a
new iommu domain on the platform bus in the display driver and attach
this to the device. This will result in the group's domain being of type
IOMMU_DOMAIN_UNMANAGED.

But when you then invoke dma_map_single() on the display device, the map
operation will acquire the iommu_group's default_domain (not domain) and
as such attempt to map stuff on the IDENTITY domain.

__iommu_map() will however reject this, given that the type does not
have __IOMMU_DOMAIN_PAGING set. Afaict, this would happen regardless of
this patch actually setting up a page table for the default_domain or
not.


So, afaict, you can't use dma_map_single()/dma_unmap() to operate your
page table on a lately attached iommu domain.

This would also imply that the display device consumes two context
banks, which worries me more than the waste of page tables etc.

Regards,
Bjorn

> BR,
> -R
> 
> > BR,
> > -R
> >
> >
> > >
> > > Regards,
> > > Bjorn
> > >
> > > > BR,
> > > > -R
> > > >
> > > > >                 smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
> > > > >                 smmu_domain->smmu = smmu;
> > > > >                 goto out_unlock;
> > > > > @@ -826,6 +832,10 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> > > > >         domain->geometry.aperture_end = (1UL << ias) - 1;
> > > > >         domain->geometry.force_aperture = true;
> > > > >
> > > > > +       /* Enable translation for non-identity context banks */
> > > > > +       if (domain->type != IOMMU_DOMAIN_IDENTITY)
> > > > > +               cfg->m = true;
> > > > > +
> > > > >         /* Initialise the context bank with our page table cfg */
> > > > >         arm_smmu_init_context_bank(smmu_domain, &pgtbl_cfg);
> > > > >         arm_smmu_write_context_bank(smmu, cfg->cbndx);
> > > > > diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
> > > > > index d172c024be61..a71d193073e4 100644
> > > > > --- a/drivers/iommu/arm-smmu.h
> > > > > +++ b/drivers/iommu/arm-smmu.h
> > > > > @@ -305,6 +305,8 @@ struct arm_smmu_device {
> > > > >
> > > > >         /* IOMMU core code handle */
> > > > >         struct iommu_device             iommu;
> > > > > +
> > > > > +       bool                            qcom_bypass_quirk;
> > > > >  };
> > > > >
> > > > >  enum arm_smmu_context_fmt {
> > > > > @@ -323,6 +325,7 @@ struct arm_smmu_cfg {
> > > > >         };
> > > > >         enum arm_smmu_cbar_type         cbar;
> > > > >         enum arm_smmu_context_fmt       fmt;
> > > > > +       bool                            m;
> > > > >  };
> > > > >  #define ARM_SMMU_INVALID_IRPTNDX       0xff
> > > > >
> > > > > --
> > > > > 2.26.2
> > > > >
> > > > > _______________________________________________
> > > > > iommu mailing list
> > > > > iommu@lists.linux-foundation.org
> > > > > https://lists.linuxfoundation.org/mailman/listinfo/iommu
diff mbox series

Patch

diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
index cf01d0215a39..e8a36054e912 100644
--- a/drivers/iommu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm-smmu-qcom.c
@@ -23,6 +23,26 @@  static const struct of_device_id qcom_smmu_client_of_match[] = {
 	{ }
 };
 
+static int qcom_smmu_cfg_probe(struct arm_smmu_device *smmu)
+{
+	unsigned int last_s2cr = ARM_SMMU_GR0_S2CR(smmu->num_mapping_groups - 1);
+	u32 reg;
+
+	/*
+	 * With some firmware writes to S2CR of type FAULT are ignored, and
+	 * writing BYPASS will end up as FAULT in the register. Perform a write
+	 * to S2CR to detect if this is the case with the current firmware.
+	 */
+	arm_smmu_gr0_write(smmu, last_s2cr, FIELD_PREP(ARM_SMMU_S2CR_TYPE, S2CR_TYPE_BYPASS) |
+					    FIELD_PREP(ARM_SMMU_S2CR_CBNDX, 0xff) |
+					    FIELD_PREP(ARM_SMMU_S2CR_PRIVCFG, S2CR_PRIVCFG_DEFAULT));
+	reg = arm_smmu_gr0_read(smmu, last_s2cr);
+	if (FIELD_GET(ARM_SMMU_S2CR_TYPE, reg) != S2CR_TYPE_BYPASS)
+		smmu->qcom_bypass_quirk = true;
+
+	return 0;
+}
+
 static int qcom_smmu_def_domain_type(struct device *dev)
 {
 	const struct of_device_id *match =
@@ -61,6 +81,7 @@  static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
 }
 
 static const struct arm_smmu_impl qcom_smmu_impl = {
+	.cfg_probe = qcom_smmu_cfg_probe,
 	.def_domain_type = qcom_smmu_def_domain_type,
 	.reset = qcom_smmu500_reset,
 };
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 2e27cf9815ab..f33eda3117fa 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -654,7 +654,9 @@  static void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx)
 
 	/* SCTLR */
 	reg = ARM_SMMU_SCTLR_CFIE | ARM_SMMU_SCTLR_CFRE | ARM_SMMU_SCTLR_AFE |
-	      ARM_SMMU_SCTLR_TRE | ARM_SMMU_SCTLR_M;
+	      ARM_SMMU_SCTLR_TRE;
+	if (cfg->m)
+		reg |= ARM_SMMU_SCTLR_M;
 	if (stage1)
 		reg |= ARM_SMMU_SCTLR_S1_ASIDPNE;
 	if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
@@ -678,7 +680,11 @@  static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 	if (smmu_domain->smmu)
 		goto out_unlock;
 
-	if (domain->type == IOMMU_DOMAIN_IDENTITY) {
+	/*
+	 * Nothing to do for IDENTITY domains,unless disabled context banks are
+	 * used to emulate bypass mappings on Qualcomm platforms.
+	 */
+	if (domain->type == IOMMU_DOMAIN_IDENTITY && !smmu->qcom_bypass_quirk) {
 		smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
 		smmu_domain->smmu = smmu;
 		goto out_unlock;
@@ -826,6 +832,10 @@  static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 	domain->geometry.aperture_end = (1UL << ias) - 1;
 	domain->geometry.force_aperture = true;
 
+	/* Enable translation for non-identity context banks */
+	if (domain->type != IOMMU_DOMAIN_IDENTITY)
+		cfg->m = true;
+
 	/* Initialise the context bank with our page table cfg */
 	arm_smmu_init_context_bank(smmu_domain, &pgtbl_cfg);
 	arm_smmu_write_context_bank(smmu, cfg->cbndx);
diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
index d172c024be61..a71d193073e4 100644
--- a/drivers/iommu/arm-smmu.h
+++ b/drivers/iommu/arm-smmu.h
@@ -305,6 +305,8 @@  struct arm_smmu_device {
 
 	/* IOMMU core code handle */
 	struct iommu_device		iommu;
+
+	bool				qcom_bypass_quirk;
 };
 
 enum arm_smmu_context_fmt {
@@ -323,6 +325,7 @@  struct arm_smmu_cfg {
 	};
 	enum arm_smmu_cbar_type		cbar;
 	enum arm_smmu_context_fmt	fmt;
+	bool				m;
 };
 #define ARM_SMMU_INVALID_IRPTNDX	0xff