refactor(cm): make SVE and SME build dependencies logical

Currently, enabling SME forces SVE off. However, the SME enablement
requires SVE to be enabled, which is reflected in code. This is the
opposite of what the build flags require.

Further, the few platforms that enable SME also explicitly enable SVE.
Their platform.mk runs after the defaults.mk file so this override never
materializes. As a result, the override is only present on the
commandline.

Change it to something sensible where if SME is on then code can rely on
SVE being on too. Do this with a check in the Makefile as it is the more
widely used pattern. This maintains all valid use cases but subtly
changes corner cases no one uses at the moment to require a slightly
different combination of flags.

Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com>
Change-Id: If7ca3972ebc3c321e554533d7bc81af49c2472be

diff --git a/Makefile b/Makefile
index 7368ca59b2ea03f16069343a8c3fcde97395bb5d..2c9cdd9c1176ad308ed2b586d1d23334f0a9c351 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -912,11 +912,20 @@ ifeq (${ENABLE_RME},1)
     endif
 endif
 
+ifneq (${ENABLE_SME_FOR_NS},0)
+    ifeq (${ENABLE_SVE_FOR_NS},0)
+        $(error "ENABLE_SME_FOR_NS requires ENABLE_SVE_FOR_NS")
+    endif
+endif
+
 # Secure SME/SVE requires the non-secure component as well
 ifeq (${ENABLE_SME_FOR_SWD},1)
     ifeq (${ENABLE_SME_FOR_NS},0)
         $(error "ENABLE_SME_FOR_SWD requires ENABLE_SME_FOR_NS")
     endif
+    ifeq (${ENABLE_SVE_FOR_SWD},0)
+        $(error "ENABLE_SME_FOR_SWD requires ENABLE_SVE_FOR_SWD")
+    endif
 endif
 ifeq (${ENABLE_SVE_FOR_SWD},1)
     ifeq (${ENABLE_SVE_FOR_NS},0)

diff --git a/bl31/bl31.mk b/bl31/bl31.mk
index 9b5cf55bcea85cd1ac5729aee0cf5df56eba56b6..b16b4e79b52eb6e2ed24e120636875c246ad4331 100644 (file)
--- a/bl31/bl31.mk
+++ b/bl31/bl31.mk
@@ -101,12 +101,10 @@ endif
 
 ifneq (${ENABLE_SME_FOR_NS},0)
 BL31_SOURCES           +=      lib/extensions/sme/sme.c
-BL31_SOURCES           +=      lib/extensions/sve/sve.c
-else
+endif
 ifneq (${ENABLE_SVE_FOR_NS},0)
 BL31_SOURCES           +=      lib/extensions/sve/sve.c
 endif
-endif
 
 ifneq (${ENABLE_MPAM_FOR_LOWER_ELS},0)
 BL31_SOURCES           +=      lib/extensions/mpam/mpam.c

diff --git a/docs/getting_started/build-options.rst b/docs/getting_started/build-options.rst
index 2735f173f22138bd56fe9cb1ada36203a3c08c01..ffde0a409b65a009a227ecee2bf709e45646f5cd 100644 (file)
--- a/docs/getting_started/build-options.rst
+++ b/docs/getting_started/build-options.rst
@@ -436,7 +436,8 @@ Common build options
    (SME), SVE, and FPU/SIMD for the non-secure world only. These features share
    registers so are enabled together. Using this option without
    ENABLE_SME_FOR_SWD=1 will cause SME, SVE, and FPU/SIMD instructions in secure
-   world to trap to EL3. SME is an optional architectural feature for AArch64
+   world to trap to EL3. Requires ``ENABLE_SVE_FOR_NS`` to be set as SME is a
+   superset of SVE. SME is an optional architectural feature for AArch64
    and TF-A support is experimental. At this time, this build option cannot be
    used on systems that have SPD=spmd/SPM_MM or ENABLE_RME, and attempting to
    build with these options will fail. This flag can take the values 0 to 2, to
@@ -450,10 +451,9 @@ Common build options
    align with the ``FEATURE_DETECTION`` mechanism. Default is 0.
 
 -  ``ENABLE_SME_FOR_SWD``: Boolean option to enable the Scalable Matrix
-   Extension for secure world use along with SVE and FPU/SIMD, ENABLE_SME_FOR_NS
-   must also be set to use this. If enabling this, the secure world MUST
-   handle context switching for SME, SVE, and FPU/SIMD registers to ensure that
-   no data is leaked to non-secure world. This is experimental. Default is 0.
+   Extension for secure world. Used along with SVE and FPU/SIMD.
+   ENABLE_SME_FOR_NS and ENABLE_SVE_FOR_SWD must also be set to use this.
+   This is experimental. Default is 0.
 
 -  ``ENABLE_SPE_FOR_NS`` : Numeric value to enable Statistical Profiling
    extensions. This is an optional architectural feature for AArch64.
@@ -469,17 +469,15 @@ Common build options
    This is to avoid corruption of the Non-secure world data in the Z-registers
    which are aliased by the SIMD and FP registers. The build option is not
    compatible with the ``CTX_INCLUDE_FPREGS`` build option, and will raise an
-   assert on platforms where SVE is implemented and ``ENABLE_SVE_FOR_NS`` enabled.
-   This flag can take the values 0 to 2, to align with the ``FEATURE_DETECTION``
-   mechanism. The default is 2 but is automatically disabled when
-   ENABLE_SME_FOR_NS is enabled ( set to 1 or 2) since SME encompasses SVE.
-   At this time, this build option cannot be used on systems that have SPM_MM
-   enabled.
+   assert on platforms where SVE is implemented and ``ENABLE_SVE_FOR_NS``
+   enabled.  This flag can take the values 0 to 2, to align with the
+   ``FEATURE_DETECTION`` mechanism. At this time, this build option cannot be
+   used on systems that have SPM_MM enabled. The default is 1.
 
 -  ``ENABLE_SVE_FOR_SWD``: Boolean option to enable SVE for the Secure world.
    SVE is an optional architectural feature for AArch64. Note that this option
-   requires ENABLE_SVE_FOR_NS to be enabled. The default is 0 and it
-   is automatically disabled when the target architecture is AArch32.
+   requires ENABLE_SVE_FOR_NS to be enabled. The default is 0 and it is
+   automatically disabled when the target architecture is AArch32.
 
 -  ``ENABLE_STACK_PROTECTOR``: String option to enable the stack protection
    checks in GCC. Allowed values are "all", "strong", "default" and "none". The

diff --git a/lib/el3_runtime/aarch64/context_mgmt.c b/lib/el3_runtime/aarch64/context_mgmt.c
index e38b34dcd12ae8a566c619b9006f7f8ca3f050be..744e4f910120c4794f5f6fe8fd8287bb8a67d238 100644 (file)
--- a/lib/el3_runtime/aarch64/context_mgmt.c
+++ b/lib/el3_runtime/aarch64/context_mgmt.c
@@ -517,12 +517,13 @@ static void manage_extensions_nonsecure(bool el2_unused, cpu_context_t *ctx)
                amu_enable(el2_unused, ctx);
        }
 
-       /* Enable SME, SVE, and FPU/SIMD for non-secure world. */
+       /* Enable SVE and FPU/SIMD */
+       if (is_feat_sve_supported()) {
+               sve_enable(ctx);
+       }
+
        if (is_feat_sme_supported()) {
                sme_enable(ctx);
-       } else if (is_feat_sve_supported()) {
-               /* Enable SVE and FPU/SIMD for non-secure world. */
-               sve_enable(ctx);
        }
 
        if (is_feat_mpam_supported()) {
@@ -553,22 +554,7 @@ static void manage_extensions_nonsecure(bool el2_unused, cpu_context_t *ctx)
 static void manage_extensions_secure(cpu_context_t *ctx)
 {
 #if IMAGE_BL31
-
-       if (is_feat_sme_supported()) {
-               if (ENABLE_SME_FOR_SWD) {
-               /*
-                * Enable SME, SVE, FPU/SIMD in secure context, secure manager
-                * must ensure SME, SVE, and FPU/SIMD context properly managed.
-                */
-                       sme_enable(ctx);
-               } else {
-               /*
-                * Disable SME, SVE, FPU/SIMD in secure context so non-secure
-                * world can safely use the associated registers.
-                */
-                       sme_disable(ctx);
-               }
-       } else if (is_feat_sve_supported()) {
+       if (is_feat_sve_supported()) {
                if (ENABLE_SVE_FOR_SWD) {
                /*
                 * Enable SVE and FPU in secure context, secure manager must
@@ -585,6 +571,21 @@ static void manage_extensions_secure(cpu_context_t *ctx)
                }
        }
 
+       if (is_feat_sme_supported()) {
+               if (ENABLE_SME_FOR_SWD) {
+               /*
+                * Enable SME, SVE, FPU/SIMD in secure context, secure manager
+                * must ensure SME, SVE, and FPU/SIMD context properly managed.
+                */
+                       sme_enable(ctx);
+               } else {
+               /*
+                * Disable SME, SVE, FPU/SIMD in secure context so non-secure
+                * world can safely use the associated registers.
+                */
+                       sme_disable(ctx);
+               }
+       }
 #endif /* IMAGE_BL31 */
 }
 

diff --git a/lib/extensions/sme/sme.c b/lib/extensions/sme/sme.c
index f888d12c46a7699a6d91f675e2acf4b3004f4b84..3423dbaf59879634f546a1992da765ba5f4e9cc0 100644 (file)
--- a/lib/extensions/sme/sme.c
+++ b/lib/extensions/sme/sme.c
@@ -65,9 +65,6 @@ void sme_enable(cpu_context_t *context)
        /* Reset CPTR_EL3 value. */
        write_cptr_el3(cptr_el3);
        isb();
-
-       /* Enable SVE/FPU in addition to SME. */
-       sve_enable(context);
 }
 
 void sme_disable(cpu_context_t *context)

diff --git a/make_helpers/defaults.mk b/make_helpers/defaults.mk
index 8ec16fa608c97f88e2326784819b7ca4ba4cccf8..e2a67f7acf44b71e78e83e718908511b9fa6f1f0 100644 (file)
--- a/make_helpers/defaults.mk
+++ b/make_helpers/defaults.mk
@@ -401,14 +401,6 @@ SVE_VECTOR_LEN                     := 2048
 # SME defaults to disabled
 ENABLE_SME_FOR_NS              := 0
 ENABLE_SME_FOR_SWD             := 0
-
-# If SME is enabled then force SVE off
-ifneq (${ENABLE_SME_FOR_NS},0)
-       override ENABLE_SVE_FOR_NS      := 0
-       override ENABLE_SVE_FOR_SWD     := 0
-endif
-
-# SME2 defaults to disabled
 ENABLE_SME2_FOR_NS             := 0
 
 SANITIZE_UB := off