From: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
To: tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org,
will.deacon-5wv7dgnIgG8@public.gmane.org
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH V2] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Date: Tue, 23 Feb 2016 12:19:20 +0000 [thread overview]
Message-ID: <56CC4E48.1060103@arm.com> (raw)
In-Reply-To: <1455820158-8529-1-git-send-email-tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
On 18/02/16 18:29, tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org wrote:
> From: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
>
> Due to Errata#27704 CN88xx SMMUv2,supports only shared ASID and VMID
> namespaces; specifically within a given node SMMU0 and SMMU1 share,
> as does SMMU2 and SMMU3.
>
> This patch tries to address these issuee by supplying asid and vmid
> base from devicetree.
>
> changes from V1:
> - rebased on top of 16 bit VMID patch
> - removed redundent options from DT
> - insted of transform, DT now supplies starting ASID/VMID
>
> Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++
> drivers/iommu/arm-smmu.c | 37 +++++++++++++++-------
> 2 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index bb7e569..80b8484 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -57,6 +57,14 @@ conditions.
>
> - smmu-enable-vmid16 : Enable 16 bit VMID, if allowed.
>
> +- asid-base : Buggy SMMUv2 implementations which doesn't satisfy the
> + ASID namespace needs, use this field to specify starting
> + ASID for the SMMU.
> +
> +- vmid-base : Buggy SMMUv2 implementations which doesn't satisfy the VMID
> + namespace needs, use this field to specify starting VMID
> + for the SMMU.
> +
From how you've described the situation, this is still just some
arbitrary number for the sake of a driver workaround and not an actual
property of the hardware. Either way there should be a proper compatible
string for this implementation, e.g. something like "cavium,cn88xx-smmu".
Given that, you then shouldn't actually need anything else, because you
can simply keep track of how many Cavium SMMUs have been probed and
automatically generate a suitable offset, e.g. (smmu->index *
ARM_SMMU_MAX_CBS) to ensure global ASID/VMID uniqueness.
Robin.
> Example:
>
> smmu {
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 003c442..dc46b9a 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -320,6 +320,9 @@ struct arm_smmu_device {
> unsigned long ipa_size;
> unsigned long pa_size;
>
> + u32 asid_base;
> + u32 vmid_base;
> +
> u32 num_global_irqs;
> u32 num_context_irqs;
> unsigned int *irqs;
> @@ -335,8 +338,8 @@ struct arm_smmu_cfg {
> };
> #define INVALID_IRPTNDX 0xff
>
> -#define ARM_SMMU_CB_ASID(cfg) ((cfg)->cbndx)
> -#define ARM_SMMU_CB_VMID(cfg) ((cfg)->cbndx + 1)
> +#define ARM_SMMU_CB_ASID(smmu, cfg) ((u16)((smmu)->asid_base + (cfg)->cbndx))
> +#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)((smmu)->vmid_base + (cfg)->cbndx))
>
> enum arm_smmu_domain_stage {
> ARM_SMMU_DOMAIN_S1 = 0,
> @@ -576,11 +579,11 @@ static void arm_smmu_tlb_inv_context(void *cookie)
>
> if (stage1) {
> base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
> - writel_relaxed(ARM_SMMU_CB_ASID(cfg),
> + writel_relaxed(ARM_SMMU_CB_ASID(smmu, cfg),
> base + ARM_SMMU_CB_S1_TLBIASID);
> } else {
> base = ARM_SMMU_GR0(smmu);
> - writel_relaxed(ARM_SMMU_CB_VMID(cfg),
> + writel_relaxed(ARM_SMMU_CB_VMID(smmu, cfg),
> base + ARM_SMMU_GR0_TLBIVMID);
> }
>
> @@ -602,7 +605,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,
>
> if (!IS_ENABLED(CONFIG_64BIT) || smmu->version == ARM_SMMU_V1) {
> iova &= ~12UL;
> - iova |= ARM_SMMU_CB_ASID(cfg);
> + iova |= ARM_SMMU_CB_ASID(smmu, cfg);
> do {
> writel_relaxed(iova, reg);
> iova += granule;
> @@ -610,7 +613,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,
> #ifdef CONFIG_64BIT
> } else {
> iova >>= 12;
> - iova |= (u64)ARM_SMMU_CB_ASID(cfg) << 48;
> + iova |= (u64)ARM_SMMU_CB_ASID(smmu, cfg) << 48;
> do {
> writeq_relaxed(iova, reg);
> iova += granule >> 12;
> @@ -630,7 +633,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,
> #endif
> } else {
> reg = ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_TLBIVMID;
> - writel_relaxed(ARM_SMMU_CB_VMID(cfg), reg);
> + writel_relaxed(ARM_SMMU_CB_VMID(smmu, cfg), reg);
> }
> }
>
> @@ -744,7 +747,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
> #endif
> /* if 16bit VMID required set VMID in CBA2R */
> if (smmu->options & ARM_SMMU_OPT_ENABLE_VMID16)
> - reg |= ARM_SMMU_CB_VMID(cfg) << CBA2R_VMID_SHIFT;
> + reg |= ARM_SMMU_CB_VMID(smmu, cfg) << CBA2R_VMID_SHIFT;
>
> writel_relaxed(reg, gr1_base + ARM_SMMU_GR1_CBA2R(cfg->cbndx));
> }
> @@ -763,7 +766,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
> (CBAR_S1_MEMATTR_WB << CBAR_S1_MEMATTR_SHIFT);
> } else if (!(smmu->options & ARM_SMMU_OPT_ENABLE_VMID16)) {
> /*16 bit VMID is not enabled set 8 bit VMID here */
> - reg |= ARM_SMMU_CB_VMID(cfg) << CBAR_VMID_SHIFT;
> + reg |= ARM_SMMU_CB_VMID(smmu, cfg) << CBAR_VMID_SHIFT;
> }
> writel_relaxed(reg, gr1_base + ARM_SMMU_GR1_CBAR(cfg->cbndx));
>
> @@ -771,11 +774,11 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
> if (stage1) {
> reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
>
> - reg64 |= ((u64)ARM_SMMU_CB_ASID(cfg)) << TTBRn_ASID_SHIFT;
> + reg64 |= ((u64)ARM_SMMU_CB_ASID(smmu, cfg)) << TTBRn_ASID_SHIFT;
> smmu_writeq(reg64, cb_base + ARM_SMMU_CB_TTBR0);
>
> reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1];
> - reg64 |= ((u64)ARM_SMMU_CB_ASID(cfg)) << TTBRn_ASID_SHIFT;
> + reg64 |= ((u64)ARM_SMMU_CB_ASID(smmu, cfg)) << TTBRn_ASID_SHIFT;
> smmu_writeq(reg64, cb_base + ARM_SMMU_CB_TTBR1);
> } else {
> reg64 = pgtbl_cfg->arm_lpae_s2_cfg.vttbr;
> @@ -1812,6 +1815,18 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
>
> parse_driver_options(smmu);
>
> + if (of_property_read_u32(dev->of_node, "#asid-base",
> + &smmu->asid_base)) {
> + smmu->asid_base = 0;
> + }
> +
> + if (of_property_read_u32(dev->of_node, "#vmid-base",
> + &smmu->vmid_base)) {
> + smmu->vmid_base = 1;
> + }
> + if (smmu->vmid_base == 0)
> + smmu->vmid_base = 1;
> +
> if (smmu->version > ARM_SMMU_V1 &&
> smmu->num_context_banks != smmu->num_context_irqs) {
> dev_err(dev,
>
WARNING: multiple messages have this Message-ID (diff)
From: robin.murphy@arm.com (Robin Murphy)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V2] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Date: Tue, 23 Feb 2016 12:19:20 +0000 [thread overview]
Message-ID: <56CC4E48.1060103@arm.com> (raw)
In-Reply-To: <1455820158-8529-1-git-send-email-tchalamarla@caviumnetworks.com>
On 18/02/16 18:29, tchalamarla at caviumnetworks.com wrote:
> From: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
>
> Due to Errata#27704 CN88xx SMMUv2,supports only shared ASID and VMID
> namespaces; specifically within a given node SMMU0 and SMMU1 share,
> as does SMMU2 and SMMU3.
>
> This patch tries to address these issuee by supplying asid and vmid
> base from devicetree.
>
> changes from V1:
> - rebased on top of 16 bit VMID patch
> - removed redundent options from DT
> - insted of transform, DT now supplies starting ASID/VMID
>
> Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula@caviumnetworks.com>
> Signed-off-by: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++
> drivers/iommu/arm-smmu.c | 37 +++++++++++++++-------
> 2 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index bb7e569..80b8484 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -57,6 +57,14 @@ conditions.
>
> - smmu-enable-vmid16 : Enable 16 bit VMID, if allowed.
>
> +- asid-base : Buggy SMMUv2 implementations which doesn't satisfy the
> + ASID namespace needs, use this field to specify starting
> + ASID for the SMMU.
> +
> +- vmid-base : Buggy SMMUv2 implementations which doesn't satisfy the VMID
> + namespace needs, use this field to specify starting VMID
> + for the SMMU.
> +
From how you've described the situation, this is still just some
arbitrary number for the sake of a driver workaround and not an actual
property of the hardware. Either way there should be a proper compatible
string for this implementation, e.g. something like "cavium,cn88xx-smmu".
Given that, you then shouldn't actually need anything else, because you
can simply keep track of how many Cavium SMMUs have been probed and
automatically generate a suitable offset, e.g. (smmu->index *
ARM_SMMU_MAX_CBS) to ensure global ASID/VMID uniqueness.
Robin.
> Example:
>
> smmu {
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 003c442..dc46b9a 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -320,6 +320,9 @@ struct arm_smmu_device {
> unsigned long ipa_size;
> unsigned long pa_size;
>
> + u32 asid_base;
> + u32 vmid_base;
> +
> u32 num_global_irqs;
> u32 num_context_irqs;
> unsigned int *irqs;
> @@ -335,8 +338,8 @@ struct arm_smmu_cfg {
> };
> #define INVALID_IRPTNDX 0xff
>
> -#define ARM_SMMU_CB_ASID(cfg) ((cfg)->cbndx)
> -#define ARM_SMMU_CB_VMID(cfg) ((cfg)->cbndx + 1)
> +#define ARM_SMMU_CB_ASID(smmu, cfg) ((u16)((smmu)->asid_base + (cfg)->cbndx))
> +#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)((smmu)->vmid_base + (cfg)->cbndx))
>
> enum arm_smmu_domain_stage {
> ARM_SMMU_DOMAIN_S1 = 0,
> @@ -576,11 +579,11 @@ static void arm_smmu_tlb_inv_context(void *cookie)
>
> if (stage1) {
> base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
> - writel_relaxed(ARM_SMMU_CB_ASID(cfg),
> + writel_relaxed(ARM_SMMU_CB_ASID(smmu, cfg),
> base + ARM_SMMU_CB_S1_TLBIASID);
> } else {
> base = ARM_SMMU_GR0(smmu);
> - writel_relaxed(ARM_SMMU_CB_VMID(cfg),
> + writel_relaxed(ARM_SMMU_CB_VMID(smmu, cfg),
> base + ARM_SMMU_GR0_TLBIVMID);
> }
>
> @@ -602,7 +605,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,
>
> if (!IS_ENABLED(CONFIG_64BIT) || smmu->version == ARM_SMMU_V1) {
> iova &= ~12UL;
> - iova |= ARM_SMMU_CB_ASID(cfg);
> + iova |= ARM_SMMU_CB_ASID(smmu, cfg);
> do {
> writel_relaxed(iova, reg);
> iova += granule;
> @@ -610,7 +613,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,
> #ifdef CONFIG_64BIT
> } else {
> iova >>= 12;
> - iova |= (u64)ARM_SMMU_CB_ASID(cfg) << 48;
> + iova |= (u64)ARM_SMMU_CB_ASID(smmu, cfg) << 48;
> do {
> writeq_relaxed(iova, reg);
> iova += granule >> 12;
> @@ -630,7 +633,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,
> #endif
> } else {
> reg = ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_TLBIVMID;
> - writel_relaxed(ARM_SMMU_CB_VMID(cfg), reg);
> + writel_relaxed(ARM_SMMU_CB_VMID(smmu, cfg), reg);
> }
> }
>
> @@ -744,7 +747,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
> #endif
> /* if 16bit VMID required set VMID in CBA2R */
> if (smmu->options & ARM_SMMU_OPT_ENABLE_VMID16)
> - reg |= ARM_SMMU_CB_VMID(cfg) << CBA2R_VMID_SHIFT;
> + reg |= ARM_SMMU_CB_VMID(smmu, cfg) << CBA2R_VMID_SHIFT;
>
> writel_relaxed(reg, gr1_base + ARM_SMMU_GR1_CBA2R(cfg->cbndx));
> }
> @@ -763,7 +766,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
> (CBAR_S1_MEMATTR_WB << CBAR_S1_MEMATTR_SHIFT);
> } else if (!(smmu->options & ARM_SMMU_OPT_ENABLE_VMID16)) {
> /*16 bit VMID is not enabled set 8 bit VMID here */
> - reg |= ARM_SMMU_CB_VMID(cfg) << CBAR_VMID_SHIFT;
> + reg |= ARM_SMMU_CB_VMID(smmu, cfg) << CBAR_VMID_SHIFT;
> }
> writel_relaxed(reg, gr1_base + ARM_SMMU_GR1_CBAR(cfg->cbndx));
>
> @@ -771,11 +774,11 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
> if (stage1) {
> reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
>
> - reg64 |= ((u64)ARM_SMMU_CB_ASID(cfg)) << TTBRn_ASID_SHIFT;
> + reg64 |= ((u64)ARM_SMMU_CB_ASID(smmu, cfg)) << TTBRn_ASID_SHIFT;
> smmu_writeq(reg64, cb_base + ARM_SMMU_CB_TTBR0);
>
> reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1];
> - reg64 |= ((u64)ARM_SMMU_CB_ASID(cfg)) << TTBRn_ASID_SHIFT;
> + reg64 |= ((u64)ARM_SMMU_CB_ASID(smmu, cfg)) << TTBRn_ASID_SHIFT;
> smmu_writeq(reg64, cb_base + ARM_SMMU_CB_TTBR1);
> } else {
> reg64 = pgtbl_cfg->arm_lpae_s2_cfg.vttbr;
> @@ -1812,6 +1815,18 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
>
> parse_driver_options(smmu);
>
> + if (of_property_read_u32(dev->of_node, "#asid-base",
> + &smmu->asid_base)) {
> + smmu->asid_base = 0;
> + }
> +
> + if (of_property_read_u32(dev->of_node, "#vmid-base",
> + &smmu->vmid_base)) {
> + smmu->vmid_base = 1;
> + }
> + if (smmu->vmid_base == 0)
> + smmu->vmid_base = 1;
> +
> if (smmu->version > ARM_SMMU_V1 &&
> smmu->num_context_banks != smmu->num_context_irqs) {
> dev_err(dev,
>
next prev parent reply other threads:[~2016-02-23 12:19 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-18 18:29 [PATCH V2] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704 tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8
2016-02-18 18:29 ` tchalamarla at caviumnetworks.com
[not found] ` <1455820158-8529-1-git-send-email-tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
2016-02-23 11:49 ` Will Deacon
2016-02-23 11:49 ` Will Deacon
2016-02-23 12:19 ` Robin Murphy [this message]
2016-02-23 12:19 ` Robin Murphy
[not found] ` <56CC4E48.1060103-5wv7dgnIgG8@public.gmane.org>
2016-02-23 23:56 ` Tirumalesh Chalamarla
2016-02-23 23:56 ` Tirumalesh Chalamarla
[not found] ` <56CCF198.3090409-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
2016-02-24 13:38 ` Robin Murphy
2016-02-24 13:38 ` Robin Murphy
[not found] ` <56CDB270.7040706-5wv7dgnIgG8@public.gmane.org>
2016-02-24 18:53 ` Tirumalesh Chalamarla
2016-02-24 18:53 ` Tirumalesh Chalamarla
2016-02-23 12:26 ` Mark Rutland
2016-02-23 12:26 ` Mark Rutland
2016-02-23 23:50 ` Tirumalesh Chalamarla
2016-02-23 23:50 ` Tirumalesh Chalamarla
[not found] ` <56CCF03D.3000809-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
2016-02-24 11:32 ` Mark Rutland
2016-02-24 11:32 ` Mark Rutland
2016-02-24 15:54 ` Chalamarla, Tirumalesh
2016-02-24 15:54 ` Chalamarla, Tirumalesh
[not found] ` <1452D0BB-4AF3-4A19-983A-3A54E94A4F2F-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
2016-02-24 19:10 ` Mark Rutland
2016-02-24 19:10 ` Mark Rutland
2016-02-24 19:15 ` Tirumalesh Chalamarla
2016-02-24 19:15 ` Tirumalesh Chalamarla
2016-02-23 23:54 ` Tirumalesh Chalamarla
2016-02-23 23:54 ` Tirumalesh Chalamarla
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56CC4E48.1060103@arm.com \
--to=robin.murphy-5wv7dgnigg8@public.gmane.org \
--cc=Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org \
--cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
--cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
--cc=tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org \
--cc=will.deacon-5wv7dgnIgG8@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.