On Wed, Sep 13, 2017 at 08:21:11PM +0100, Dave Martin wrote: > On Wed, Sep 13, 2017 at 06:32:06AM -0700, Catalin Marinas wrote: > > On Thu, Aug 31, 2017 at 06:00:42PM +0100, Dave P Martin wrote: > > > diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S > > > index 877d42f..dd22ef2 100644 > > > --- a/arch/arm64/mm/proc.S > > > +++ b/arch/arm64/mm/proc.S > > > @@ -27,6 +27,7 @@ > > > #include <asm/pgtable-hwdef.h> > > > #include <asm/cpufeature.h> > > > #include <asm/alternative.h> > > > +#include <asm/sysreg.h> > > > > > > #ifdef CONFIG_ARM64_64K_PAGES > > > #define TCR_TG_FLAGS TCR_TG0_64K | TCR_TG1_64K > > > @@ -186,8 +187,17 @@ ENTRY(__cpu_setup) > > > tlbi vmalle1 // Invalidate local TLB > > > dsb nsh > > > > > > - mov x0, #3 << 20 > > > - msr cpacr_el1, x0 // Enable FP/ASIMD > > > + mov x0, #3 << 20 // FEN > > > + > > > + /* SVE */ > > > + mrs x5, id_aa64pfr0_el1 > > > + ubfx x5, x5, #ID_AA64PFR0_SVE_SHIFT, #4 > > > + cbz x5, 1f > > > + > > > + bic x0, x0, #CPACR_EL1_ZEN > > > + orr x0, x0, #CPACR_EL1_ZEN_EL1EN // SVE: trap for EL0, not EL1 > > > +1: msr cpacr_el1, x0 // Enable FP/ASIMD > > > > For EL1, I wonder whether we could move this later to cpufeature.c. IIRC > > I tried to do the same with FPSIMD but hit an issue with EFI run-time > > services (I may be wrong though). > > I'll take a look at this -- I believe it should be safe to disable this > trap for EL1 relatively late. This is needed before probing for > available vector lengths, but apart from that the kernel shouldn't touch > SVE until/unless some user task uses SVE. > > This would change if we eventually enable kernel-mode SVE, but I wouldn't > expect that to get used in early boot before the cpufeatures code runs. > > Ard may have a view on this.
I've had a go at this, but there's a lot of splatter. I can add a helper el1_enable_sve() say, and call it before probing ZCR_EL1 in the cpufeatures code. This makes enabling SVE a side-effect of the cpufeatures code, which I'm not that comfortable with -- it feels like something that later refactoring could easily break. I can also add an explicit call to el1_enable_sve() in sve_setup(), but this only works on the boot cpu. For secondaries, I could add something in secondary_start_kernel(), but this looks incongruous since there's no other call to do something similar yet. ** Suzuki, do we have any other case where the trap for a CPU feature is turned off by the cpufeatures code? If there's already a template for this then I'm happy to follow it. Otherwise, maybe it's simpler to keep it in __cpu_setup after all since that's a common path that all CPUs pass through. Cheers ---Dave _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
