On Thu, Oct 05, 2017 at 12:04:12PM +0100, Suzuki K Poulose wrote: > On 05/10/17 11:47, Dave Martin 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 [..] > >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. > > The closest thing you have is an "enable" callback for each "available" > capability, which gets invoked on all CPUs (boot time active CPUs and the > ones which are brought up later). This is used by things like, PAN to > do some CPU specific setups. > > See : > enable_cpu_capabilities() // For all boot time active CPUs > and > verify_local_cpu_features() // For CPUs brought up later That may allow me to do something tidier, provided the enable method is called early enough. I'll take a look. Cheers ---Dave
