Re: [PATCH v2 05/17] arm64: Helper for parange to PASize

[Julien Grall]

On 27/04/18 16:18, Suzuki K Poulose wrote:
> On 26/04/18 11:58, Julien Grall wrote:
> > Hi Suzuki,
> >
> > On 27/03/18 14:15, Suzuki K Poulose wrote:
> > > Add a helper to convert ID_AA64MMFR0_EL1:PARange to they physical
> > > size shift. Limit the size to the maximum supported by the kernel.
> > > We are about to move the user of this code and this helps to
> > > keep the changes cleaner.
> >
> > It is probably worth to mention that you are also adding 52-bit 
> > support in the patch.
>
> Sure, will do. Can I take that as a Reviewed-by with the fixed
> commit description ?

Yes. Here we go:

Reviewed-by: Julien Grall <julien.grall@xxxxxxx>

Cheers,

> Cheers
> Suzuki
>
> > Cheers,
> >
> > > Cc: Mark Rutland <mark.rutland@xxxxxxx>
> > > Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> > > Cc: Will Deacon <will.deacon@xxxxxxx>
> > > Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
> > > Cc: Christoffer Dall <cdall@xxxxxxxxxx>
> > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> > > ---
> > > Â arch/arm64/include/asm/cpufeature.h | 16 ++++++++++++++++
> > > Â arch/arm64/kvm/hyp/s2-setup.cÂÂÂÂÂÂ | 28 +++++-----------------------
> > > Â 2 files changed, 21 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/cpufeature.h 
> > > b/arch/arm64/include/asm/cpufeature.h
> > > index fbf0aab..1f2a5dd 100644
> > > --- a/arch/arm64/include/asm/cpufeature.h
> > > +++ b/arch/arm64/include/asm/cpufeature.h
> > > @@ -311,6 +311,22 @@ static inline u64 read_zcr_features(void)
> > > ÂÂÂÂÂ return zcr;
> > > Â }
> > > +static inline u32 id_aa64mmfr0_parange_to_phys_shift(int parange)
> > > +{
> > > +ÂÂÂ switch (parange) {
> > > +ÂÂÂ case 0: return 32;
> > > +ÂÂÂ case 1: return 36;
> > > +ÂÂÂ case 2: return 40;
> > > +ÂÂÂ case 3: return 42;
> > > +ÂÂÂ case 4: return 44;
> > > +ÂÂÂ /* Report 48 bit if the kernel doesn't support 52bit */
> > > +ÂÂÂ default:
> > > +ÂÂÂ case 5: return 48;
> > > +#ifdef CONFIG_ARM64_PA_BITS_52
> > > +ÂÂÂ case 6: return 52;
> > > +#endif
> > > +ÂÂÂ }
> > > +}
> > > Â #endif /* __ASSEMBLY__ */
> > > Â #endif
> > > diff --git a/arch/arm64/kvm/hyp/s2-setup.c 
> > > b/arch/arm64/kvm/hyp/s2-setup.c
> > > index 603e1ee..b1129c8 100644
> > > --- a/arch/arm64/kvm/hyp/s2-setup.c
> > > +++ b/arch/arm64/kvm/hyp/s2-setup.c
> > > @@ -19,11 +19,13 @@
> > > Â #include <asm/kvm_arm.h>
> > > Â #include <asm/kvm_asm.h>
> > > Â #include <asm/kvm_hyp.h>
> > > +#include <asm/cpufeature.h>
> > > Â u32 __hyp_text __init_stage2_translation(void)
> > > Â {
> > > ÂÂÂÂÂ u64 val = VTCR_EL2_FLAGS;
> > > ÂÂÂÂÂ u64 parange;
> > > +ÂÂÂ u32 phys_shift;
> > > ÂÂÂÂÂ u64 tmp;
> > > ÂÂÂÂÂ /*
> > > @@ -37,27 +39,7 @@ u32 __hyp_text __init_stage2_translation(void)
> > > ÂÂÂÂÂ val |= parange << 16;
> > > ÂÂÂÂÂ /* Compute the actual PARange... */
> > > -ÂÂÂ switch (parange) {
> > > -ÂÂÂ case 0:
> > > -ÂÂÂÂÂÂÂ parange = 32;
> > > -ÂÂÂÂÂÂÂ break;
> > > -ÂÂÂ case 1:
> > > -ÂÂÂÂÂÂÂ parange = 36;
> > > -ÂÂÂÂÂÂÂ break;
> > > -ÂÂÂ case 2:
> > > -ÂÂÂÂÂÂÂ parange = 40;
> > > -ÂÂÂÂÂÂÂ break;
> > > -ÂÂÂ case 3:
> > > -ÂÂÂÂÂÂÂ parange = 42;
> > > -ÂÂÂÂÂÂÂ break;
> > > -ÂÂÂ case 4:
> > > -ÂÂÂÂÂÂÂ parange = 44;
> > > -ÂÂÂÂÂÂÂ break;
> > > -ÂÂÂ case 5:
> > > -ÂÂÂ default:
> > > -ÂÂÂÂÂÂÂ parange = 48;
> > > -ÂÂÂÂÂÂÂ break;
> > > -ÂÂÂ }
> > > +ÂÂÂ phys_shift = id_aa64mmfr0_parange_to_phys_shift(parange);
> > > ÂÂÂÂÂ /*
> > > ÂÂÂÂÂÂ * ... and clamp it to 40 bits, unless we have some braindead
> > > @@ -65,7 +47,7 @@ u32 __hyp_text __init_stage2_translation(void)
> > > ÂÂÂÂÂÂ * return that value for the rest of the kernel to decide what
> > > ÂÂÂÂÂÂ * to do.
> > > ÂÂÂÂÂÂ */
> > > -ÂÂÂ val |= 64 - (parange > 40 ? 40 : parange);
> > > +ÂÂÂ val |= 64 - (phys_shift > 40 ? 40 : phys_shift);
> > > ÂÂÂÂÂ /*
> > > ÂÂÂÂÂÂ * Check the availability of Hardware Access Flag / Dirty Bit
> > > @@ -86,5 +68,5 @@ u32 __hyp_text __init_stage2_translation(void)
> > > ÂÂÂÂÂ write_sysreg(val, vtcr_el2);
> > > -ÂÂÂ return parange;
> > > +ÂÂÂ return phys_shift;
> > > Â }

--
Julien Grall