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.hal...@amd.com>
> Reviewed-by: Stefano Stabellini <sstabell...@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.or...@amd.com>
~Michal