Re: [PATCH 1/2] arm64: Relax constraints on ID feature bits

Suzuki K Poulose Wed, 07 Feb 2018 03:41:52 -0800

On 07/02/18 10:40, Dave Martin wrote:

On Thu, Feb 01, 2018 at 10:38:37AM +0000, Suzuki K Poulose wrote:

We treat most of the feature bits in the ID registers as STRICT,
implying that all CPUs should match it the boot CPU state. However,
for most of the features, we can handle if there are any mismatches
by using the safe value. e.g, HWCAPs and other features used by the
kernel. Relax the constraint on the feature bits whose mismatch can
be handled by the kernel.


For VHE, if there is a mismatch we don't care if the kernel is
not using it. If the kernel is indeed running in EL2 mode, then
the mismatches results in a panic. Similarly for ASID bits we
take care of conflicts.

For other features like, PAN, UAO we only enable it only if we
have it on all the CPUs. For IESB, we set the SCTLR bit unconditionally
anyways.


Do the remaining STRICT+LOWER_SAFE / NONSTRICT+EXACT cases still
make sense?


Thats a good point. I did take a look at them. Most of them were not
really used by the kernel and some of them needed some additional checks
to make sure the "STRICT" is enforced via other checks. (e.g IDAA64MMFR1:VMID).

Here is the remaining list :

IDAA64PFR0_EL1
  - GIC - System register GIC itnerface. I think this can be made non-strict,
          since we can now enforce it via capabilities (with the GIC_CPUIF
          being a Boot CPU feature). So, we need to wait until that series
          is merged.

  - EL2 - This is a bit complex. This is STRICT only if all the other CPUs were
          booted in EL2 and KVM is enabled. But then we need to add an extra
          check for hotplugged CPUs.

IDAA64MMFR0_EL1
  - BIGENDEL - Again, the system uses sanitised value, so LOWER_SAFE makes 
sense.
          But there are no checks for hotplugged CPUs, if the SETEND emulation
          is enabled.

ID_AA64MMFR1_EL
  - LOR - Limited Ordering regions, Not supported by the kernel. So we can
          make this non-strict.
  - HPD - Hierarchical permission disables. Currently unused by the kernel.
          So, can be switched to non-strict
  - VMID - VMIDbits width. This is currently STRICT+LOWER_SAFE. The KVM uses
          sanitised value of the feature, so this can be NONSTRICT+LOWER_SAFE.
          However, we need to ensure a hotplugged CPU complies to the sanitised
          width (which may have been used by KVM)
        

To summarise, I can add LOR/HPD changes. But the others requires a bit more
work and can be done as a separate series.

I've wondered in the past whether there is redundancy between the strict
and type fields, but when adding entries I just copy-pasted similar ones
rather than fully understanding what was going on...


I agree. These were defined before we started using the system wide safe
values and enforcing the capabilities on late/secondary CPUs. Now that
we have an infrastructure which makes sure that conflicts are handled,
we could relax the definitions a bit.

Cheers
Suzuki


Cheers
---Dave


Cc: Catalin Marinas <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: James Morse <[email protected]>
Cc: Dave Martin <[email protected]>
Signed-off-by: Suzuki K Poulose <[email protected]>
---
  arch/arm64/kernel/cpufeature.c | 66 ++++++++++++++++++++++--------------------
  1 file changed, 35 insertions(+), 31 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 4b6f9051cf0c..1c39a635a71c 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -123,36 +123,36 @@ cpufeature_pan_not_uao(const struct 
arm64_cpu_capabilities *entry, int __unused)
   * sync with the documentation of the CPU feature register ABI.
   */
  static const struct arm64_ftr_bits ftr_id_aa64isar0[] = {
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_FHM_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_DP_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_SM4_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_SM3_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_SHA3_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_RDM_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_ATOMICS_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_CRC32_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_SHA2_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_SHA1_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_AES_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_FHM_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_DP_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_SM4_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_SM3_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_SHA3_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_RDM_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_ATOMICS_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_CRC32_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_SHA2_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_SHA1_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR0_AES_SHIFT, 4, 0),
        ARM64_FTR_END,
  };

static const struct arm64_ftr_bits ftr_id_aa64isar1[] = {

-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR1_LRCPC_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR1_FCMA_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR1_JSCVT_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR1_DPB_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR1_LRCPC_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR1_FCMA_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR1_JSCVT_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64ISAR1_DPB_SHIFT, 4, 0),
        ARM64_FTR_END,
  };

static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = {

        ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64PFR0_CSV3_SHIFT, 4, 0),
        ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64PFR0_CSV2_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64PFR0_SVE_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64PFR0_RAS_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64PFR0_SVE_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64PFR0_RAS_SHIFT, 4, 0),
        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64PFR0_GIC_SHIFT, 4, 0),
-       S_ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64PFR0_ASIMD_SHIFT, 4, ID_AA64PFR0_ASIMD_NI),
-       S_ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64PFR0_FP_SHIFT, 4, ID_AA64PFR0_FP_NI),
+       S_ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64PFR0_ASIMD_SHIFT, 4, ID_AA64PFR0_ASIMD_NI),
+       S_ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64PFR0_FP_SHIFT, 4, ID_AA64PFR0_FP_NI),
        /* Linux doesn't care about the EL3 */
        ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64PFR0_EL3_SHIFT, 4, 0),
        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64PFR0_EL2_SHIFT, 4, 0),
@@ -169,7 +169,8 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr0[] = {
        /* Linux shouldn't care about secure memory */
        ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR0_SNSMEM_SHIFT, 4, 0),
        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR0_BIGENDEL_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR0_ASID_SHIFT, 4, 0),
+       /* We handle differing ASID widths by explicit checks to make sure the 
system is safe */
+       ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR0_ASID_SHIFT, 4, 0),
        /*
         * Differing PARange is fine as long as all peripherals and memory are 
mapped
         * within the minimum PARange of all CPUs
@@ -179,20 +180,23 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr0[] = {
  };

static const struct arm64_ftr_bits ftr_id_aa64mmfr1[] = {

-       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR1_PAN_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR1_PAN_SHIFT, 4, 0),
        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR1_LOR_SHIFT, 4, 0),
        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR1_HPD_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR1_VHE_SHIFT, 4, 0),
+       /* When CONFIG_ARM64_VHE is enabled, we ensure that there is no 
conflict */
+       ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR1_VHE_SHIFT, 4, 0),
        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR1_VMIDBITS_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR1_HADBS_SHIFT, 4, 0),
+       /* We can run a mix of CPUs with and without the support for HW 
management of AF/DBM bits */
+       ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR1_HADBS_SHIFT, 4, 0),
        ARM64_FTR_END,
  };

static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = {

        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR2_LVA_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR2_IESB_SHIFT, 4, 0),
+       /* While IESB is good to have, it is not fatal if we miss this on some 
CPUs */
+       ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR2_IESB_SHIFT, 4, 0),
        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR2_LSM_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR2_UAO_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR2_UAO_SHIFT, 4, 0),
        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_AA64MMFR2_CNP_SHIFT, 4, 0),
        ARM64_FTR_END,
  };
@@ -259,12 +263,12 @@ static const struct arm64_ftr_bits ftr_dczid[] = {

static const struct arm64_ftr_bits ftr_id_isar5[] = {

-       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_ISAR5_RDM_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_ISAR5_CRC32_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_ISAR5_SHA2_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_ISAR5_SHA1_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_ISAR5_AES_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 
ID_ISAR5_SEVL_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_ISAR5_RDM_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_ISAR5_CRC32_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_ISAR5_SHA2_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_ISAR5_SHA1_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_ISAR5_AES_SHIFT, 4, 0),
+       ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, 
ID_ISAR5_SEVL_SHIFT, 4, 0),
        ARM64_FTR_END,
  };

--

2.14.3


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] arm64: Relax constraints on ID feature bits

Reply via email to