Re: linux-next: manual merge of the kvm-arm tree with the arm64 tree

[Ard Biesheuvel]

On Mon, 19 Feb 2024 at 16:22, Marc Zyngier <maz@xxxxxxxxxx> wrote:
>
> On Mon, 19 Feb 2024 12:14:18 +0000,
> Catalin Marinas <catalin.marinas@xxxxxxx> wrote:
> >
> > On Mon, Feb 19, 2024 at 01:58:05PM +1100, Stephen Rothwell wrote:
> > > diff --cc arch/arm64/kernel/cpufeature.c
> > > index 0be9296e9253,f309fd542c20..000000000000
> > > --- a/arch/arm64/kernel/cpufeature.c
> > > +++ b/arch/arm64/kernel/cpufeature.c
> > > @@@ -721,13 -754,12 +756,14 @@@ static const struct __ftr_reg_entry
> > >                            &id_aa64isar2_override),
> > >
> > >     /* Op1 = 0, CRn = 0, CRm = 7 */
> > >  -  ARM64_FTR_REG(SYS_ID_AA64MMFR0_EL1, ftr_id_aa64mmfr0),
> > >  +  ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64MMFR0_EL1, ftr_id_aa64mmfr0,
> > >  +                         &id_aa64mmfr0_override),
> > >     ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64MMFR1_EL1, ftr_id_aa64mmfr1,
> > >                            &id_aa64mmfr1_override),
> > >  -  ARM64_FTR_REG(SYS_ID_AA64MMFR2_EL1, ftr_id_aa64mmfr2),
> > >  +  ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64MMFR2_EL1, ftr_id_aa64mmfr2,
> > >  +                         &id_aa64mmfr2_override),
> > >     ARM64_FTR_REG(SYS_ID_AA64MMFR3_EL1, ftr_id_aa64mmfr3),
> > > +   ARM64_FTR_REG(SYS_ID_AA64MMFR4_EL1, ftr_id_aa64mmfr4),
> > >
> > >     /* Op1 = 1, CRn = 0, CRm = 0 */
> > >     ARM64_FTR_REG(SYS_GMID_EL1, ftr_gmid),
> > > @@@ -2701,33 -2817,13 +2779,40 @@@ static const struct arm64_cpu_capabilit
> > >             .type = ARM64_CPUCAP_SYSTEM_FEATURE,
> > >             .matches = has_lpa2,
> > >     },
> > >  +#ifdef CONFIG_ARM64_VA_BITS_52
> > >  +  {
> > >  +          .capability = ARM64_HAS_VA52,
> > >  +          .type = ARM64_CPUCAP_BOOT_CPU_FEATURE,
> > >  +          .matches = has_cpuid_feature,
> > >  +          .field_width = 4,
> > >  +#ifdef CONFIG_ARM64_64K_PAGES
> > >  +          .desc = "52-bit Virtual Addressing (LVA)",
> > >  +          .sign = FTR_SIGNED,
> > >  +          .sys_reg = SYS_ID_AA64MMFR2_EL1,
> > >  +          .field_pos = ID_AA64MMFR2_EL1_VARange_SHIFT,
> > >  +          .min_field_value = ID_AA64MMFR2_EL1_VARange_52,
> > >  +#else
> > >  +          .desc = "52-bit Virtual Addressing (LPA2)",
> > >  +          .sys_reg = SYS_ID_AA64MMFR0_EL1,
> > >  +#ifdef CONFIG_ARM64_4K_PAGES
> > >  +          .sign = FTR_SIGNED,
> > >  +          .field_pos = ID_AA64MMFR0_EL1_TGRAN4_SHIFT,
> > >  +          .min_field_value = ID_AA64MMFR0_EL1_TGRAN4_52_BIT,
> > >  +#else
> > >  +          .sign = FTR_UNSIGNED,
> > >  +          .field_pos = ID_AA64MMFR0_EL1_TGRAN16_SHIFT,
> > >  +          .min_field_value = ID_AA64MMFR0_EL1_TGRAN16_52_BIT,
> > >  +#endif
> > >  +#endif
> > >  +  },
> > >  +#endif
> > > +   {
> > > +           .desc = "NV1",
> > > +           .capability = ARM64_HAS_HCR_NV1,
> > > +           .type = ARM64_CPUCAP_SYSTEM_FEATURE,
> > > +           .matches = has_nv1,
> > > +           ARM64_CPUID_FIELDS_NEG(ID_AA64MMFR4_EL1, E2H0, NI_NV1)
> > > +   },
> > >     {},
> > >   };
> >
> > Thanks Stephen. It looks fine.
>
> Actually, it breaks 52bit support in a "subtle" way (multiple reports
> on the list and IRC, all pointing to failures on QEMU). The KVM tree
> adds support for feature ranges, which this code is totally unaware
> of, and only provides the min value and not the max. Things go wrong
> from there.
>
> I propose to fix it like below, which makes it robust against the KVM
> changes (patch applies to arm64/for-next/core). I have tested it in
> combination with kvmarm/next, with 4kB and 16kB (LVA2), as well as
> 64kB (LVA).
>
> Thanks,
>
>         M.
>
> From f24638a5f41424faf47f3d9035e6dcbd3800fcb6 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@xxxxxxxxxx>
> Date: Mon, 19 Feb 2024 15:13:22 +0000
> Subject: [PATCH] arm64: Use Signed/Unsigned enums for TGRAN{4,16,64} and
>  VARange
>
> Open-coding the feature matching parameters for LVA/LVA2 leads to
> issues with upcoming changes to the cpufeature code.
>
> By making TGRAN{4,16,64} and VARange signed/unsigned as per the
> architecture, we can use the existing macros, making the feature
> match robust against those changes.
>
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>

Thanks for the fix.

Acked-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
Tested-by: Ard Biesheuvel <ardb@xxxxxxxxxx>