| Message ID | 20230428175543.11902-5-ayan.kumar.halder@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for 32-bit physical address | expand |
On 28/04/2023 19:55, Ayan Kumar Halder wrote: > > > Refer ARM IHI 0062D.c ID070116 (SMMU 2.0 spec), 17-360, 17.3.9, > SMMU_CBn_TTBR0 is a 64 bit register. Thus, one can use > writeq_relaxed_non_atomic() to write to it instead of invoking > writel_relaxed() twice for lower half and upper half of the register. > > This also helps us as p2maddr is 'paddr_t' (which may be u32 in future). > Thus, one can assign p2maddr to a 64 bit register and do the bit > manipulations on it, to generate the value for SMMU_CBn_TTBR0. > > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > Changes from - > > v1 - 1. Extracted the patch from "[XEN v1 8/9] xen/arm: Other adaptations required to support 32bit paddr". > Use writeq_relaxed_non_atomic() to write u64 register in a non-atomic > fashion. > > v2 - 1. Added R-b. > > v3 - 1. No changes. > > v4 - 1. Reordered the R-b. No further changes. > (This patch can be committed independent of the series). > > v5 - Used 'uint64_t' instead of u64. As the change looked trivial to me, I > retained the R-b. > > xen/drivers/passthrough/arm/smmu.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c > index 79281075ba..fb8bef5f69 100644 > --- a/xen/drivers/passthrough/arm/smmu.c > +++ b/xen/drivers/passthrough/arm/smmu.c > @@ -499,8 +499,7 @@ enum arm_smmu_s2cr_privcfg { > #define ARM_SMMU_CB_SCTLR 0x0 > #define ARM_SMMU_CB_RESUME 0x8 > #define ARM_SMMU_CB_TTBCR2 0x10 > -#define ARM_SMMU_CB_TTBR0_LO 0x20 > -#define ARM_SMMU_CB_TTBR0_HI 0x24 > +#define ARM_SMMU_CB_TTBR0 0x20 > #define ARM_SMMU_CB_TTBCR 0x30 > #define ARM_SMMU_CB_S1_MAIR0 0x38 > #define ARM_SMMU_CB_FSR 0x58 > @@ -1083,6 +1082,7 @@ static void arm_smmu_flush_pgtable(struct arm_smmu_device *smmu, void *addr, > static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain) > { > u32 reg; > + uint64_t reg64; > bool stage1; > struct arm_smmu_cfg *cfg = &smmu_domain->cfg; > struct arm_smmu_device *smmu = smmu_domain->smmu; > @@ -1177,12 +1177,13 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain) > dev_notice(smmu->dev, "d%u: p2maddr 0x%"PRIpaddr"\n", > smmu_domain->cfg.domain->domain_id, p2maddr); > > - reg = (p2maddr & ((1ULL << 32) - 1)); > - writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_LO); > - reg = (p2maddr >> 32); > + reg64 = p2maddr; > + > if (stage1) > - reg |= ARM_SMMU_CB_ASID(cfg) << TTBRn_HI_ASID_SHIFT; > - writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_HI); > + reg64 |= (((uint64_t) (ARM_SMMU_CB_ASID(cfg) << TTBRn_HI_ASID_SHIFT)) > + << 32); I think << should be aligned to the second '(' above. Reviewed-by: Michal Orzel <michal.orzel@amd.com> ~Michal
Hi Ayan, On 4 May 2023, at 8:37 am, Michal Orzel <michal.orzel@amd.com> wrote: On 28/04/2023 19:55, Ayan Kumar Halder wrote: Refer ARM IHI 0062D.c ID070116 (SMMU 2.0 spec), 17-360, 17.3.9, SMMU_CBn_TTBR0 is a 64 bit register. Thus, one can use writeq_relaxed_non_atomic() to write to it instead of invoking writel_relaxed() twice for lower half and upper half of the register. This also helps us as p2maddr is 'paddr_t' (which may be u32 in future). Thus, one can assign p2maddr to a 64 bit register and do the bit manipulations on it, to generate the value for SMMU_CBn_TTBR0. Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> --- Changes from - v1 - 1. Extracted the patch from "[XEN v1 8/9] xen/arm: Other adaptations required to support 32bit paddr". Use writeq_relaxed_non_atomic() to write u64 register in a non-atomic fashion. v2 - 1. Added R-b. v3 - 1. No changes. v4 - 1. Reordered the R-b. No further changes. (This patch can be committed independent of the series). v5 - Used 'uint64_t' instead of u64. As the change looked trivial to me, I retained the R-b. xen/drivers/passthrough/arm/smmu.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c index 79281075ba..fb8bef5f69 100644 --- a/xen/drivers/passthrough/arm/smmu.c +++ b/xen/drivers/passthrough/arm/smmu.c @@ -499,8 +499,7 @@ enum arm_smmu_s2cr_privcfg { #define ARM_SMMU_CB_SCTLR 0x0 #define ARM_SMMU_CB_RESUME 0x8 #define ARM_SMMU_CB_TTBCR2 0x10 -#define ARM_SMMU_CB_TTBR0_LO 0x20 -#define ARM_SMMU_CB_TTBR0_HI 0x24 +#define ARM_SMMU_CB_TTBR0 0x20 #define ARM_SMMU_CB_TTBCR 0x30 #define ARM_SMMU_CB_S1_MAIR0 0x38 #define ARM_SMMU_CB_FSR 0x58 @@ -1083,6 +1082,7 @@ static void arm_smmu_flush_pgtable(struct arm_smmu_device *smmu, void *addr, static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain) { u32 reg; + uint64_t reg64; bool stage1; struct arm_smmu_cfg *cfg = &smmu_domain->cfg; struct arm_smmu_device *smmu = smmu_domain->smmu; @@ -1177,12 +1177,13 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain) dev_notice(smmu->dev, "d%u: p2maddr 0x%"PRIpaddr"\n", smmu_domain->cfg.domain->domain_id, p2maddr); - reg = (p2maddr & ((1ULL << 32) - 1)); - writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_LO); - reg = (p2maddr >> 32); + reg64 = p2maddr; + if (stage1) - reg |= ARM_SMMU_CB_ASID(cfg) << TTBRn_HI_ASID_SHIFT; - writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_HI); + reg64 |= (((uint64_t) (ARM_SMMU_CB_ASID(cfg) << TTBRn_HI_ASID_SHIFT)) + << 32); I think << should be aligned to the second '(' above. Reviewed-by: Michal Orzel <michal.orzel@amd.com<mailto:michal.orzel@amd.com>> With the Michal comment fixed. Reviewed-by: Rahul Singh <rahul.singh@arm.com<mailto:rahul.singh@arm.com>> Regards, Rahul
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c index 79281075ba..fb8bef5f69 100644 --- a/xen/drivers/passthrough/arm/smmu.c +++ b/xen/drivers/passthrough/arm/smmu.c @@ -499,8 +499,7 @@ enum arm_smmu_s2cr_privcfg { #define ARM_SMMU_CB_SCTLR 0x0 #define ARM_SMMU_CB_RESUME 0x8 #define ARM_SMMU_CB_TTBCR2 0x10 -#define ARM_SMMU_CB_TTBR0_LO 0x20 -#define ARM_SMMU_CB_TTBR0_HI 0x24 +#define ARM_SMMU_CB_TTBR0 0x20 #define ARM_SMMU_CB_TTBCR 0x30 #define ARM_SMMU_CB_S1_MAIR0 0x38 #define ARM_SMMU_CB_FSR 0x58 @@ -1083,6 +1082,7 @@ static void arm_smmu_flush_pgtable(struct arm_smmu_device *smmu, void *addr, static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain) { u32 reg; + uint64_t reg64; bool stage1; struct arm_smmu_cfg *cfg = &smmu_domain->cfg; struct arm_smmu_device *smmu = smmu_domain->smmu; @@ -1177,12 +1177,13 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain) dev_notice(smmu->dev, "d%u: p2maddr 0x%"PRIpaddr"\n", smmu_domain->cfg.domain->domain_id, p2maddr); - reg = (p2maddr & ((1ULL << 32) - 1)); - writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_LO); - reg = (p2maddr >> 32); + reg64 = p2maddr; + if (stage1) - reg |= ARM_SMMU_CB_ASID(cfg) << TTBRn_HI_ASID_SHIFT; - writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_HI); + reg64 |= (((uint64_t) (ARM_SMMU_CB_ASID(cfg) << TTBRn_HI_ASID_SHIFT)) + << 32); + + writeq_relaxed_non_atomic(reg64, cb_base + ARM_SMMU_CB_TTBR0); /* * TTBCR