arm64: cpufeatures: Correctly handle signed values

Although we've had signed values for some features such as PMUv3
and FP, the code that handles the comparaison with some limit
has a couple of annoying issues:

- the min_field_value is always unsigned, meaning that we cannot
  easily compare it with a negative value

- it is not possible to have a range of values, let alone a range
  of negative values

Fix this by:

- adding an upper limit to the comparison, defaulting to all bits
  being set to the maximum positive value

- ensuring that the signess of the min and max values are taken into
  account

A ARM64_CPUID_FIELDS_NEG() macro is provided for signed features, but
nothing is using it yet.

Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Link: https://lore.kernel.org/r/20240122181344.258974-3-maz@kernel.org
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 21c824edf8ce4a6fa1c208b7a882df4b4e31eeb7..a98d95f3492bb45f505673af72681f1811b430fe 100644 (file)
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -363,6 +363,7 @@ struct arm64_cpu_capabilities {
                        u8 field_pos;
                        u8 field_width;
                        u8 min_field_value;
+                       u8 max_field_value;
                        u8 hwcap_type;
                        bool sign;
                        unsigned long hwcap;

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 8d1a634a403eed6e13a210331a8f25133354ca28..92b1546f26223b4750fdfc2fc38dd2a353e35076 100644 (file)
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -140,12 +140,42 @@ void dump_cpu_features(void)
        pr_emerg("0x%*pb\n", ARM64_NCAPS, &system_cpucaps);
 }
 
+#define __ARM64_MAX_POSITIVE(reg, field)                               \
+               ((reg##_##field##_SIGNED ?                              \
+                 BIT(reg##_##field##_WIDTH - 1) :                      \
+                 BIT(reg##_##field##_WIDTH)) - 1)
+
+#define __ARM64_MIN_NEGATIVE(reg, field)  BIT(reg##_##field##_WIDTH - 1)
+
+#define __ARM64_CPUID_FIELDS(reg, field, min_value, max_value)         \
+               .sys_reg = SYS_##reg,                                   \
+               .field_pos = reg##_##field##_SHIFT,                     \
+               .field_width = reg##_##field##_WIDTH,                   \
+               .sign = reg##_##field##_SIGNED,                         \
+               .min_field_value = min_value,                           \
+               .max_field_value = max_value,
+
+/*
+ * ARM64_CPUID_FIELDS() encodes a field with a range from min_value to
+ * an implicit maximum that depends on the sign-ess of the field.
+ *
+ * An unsigned field will be capped at all ones, while a signed field
+ * will be limited to the positive half only.
+ */
 #define ARM64_CPUID_FIELDS(reg, field, min_value)                      \
-               .sys_reg = SYS_##reg,                                                   \
-               .field_pos = reg##_##field##_SHIFT,                                             \
-               .field_width = reg##_##field##_WIDTH,                                           \
-               .sign = reg##_##field##_SIGNED,                                                 \
-               .min_field_value = reg##_##field##_##min_value,
+       __ARM64_CPUID_FIELDS(reg, field,                                \
+                            SYS_FIELD_VALUE(reg, field, min_value),    \
+                            __ARM64_MAX_POSITIVE(reg, field))
+
+/*
+ * ARM64_CPUID_FIELDS_NEG() encodes a field with a range from an
+ * implicit minimal value to max_value. This should be used when
+ * matching a non-implemented property.
+ */
+#define ARM64_CPUID_FIELDS_NEG(reg, field, max_value)                  \
+       __ARM64_CPUID_FIELDS(reg, field,                                \
+                            __ARM64_MIN_NEGATIVE(reg, field),          \
+                            SYS_FIELD_VALUE(reg, field, max_value))
 
 #define __ARM64_FTR_BITS(SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \
        {                                               \
@@ -1451,11 +1481,28 @@ has_always(const struct arm64_cpu_capabilities *entry, int scope)
 static bool
 feature_matches(u64 reg, const struct arm64_cpu_capabilities *entry)
 {
-       int val = cpuid_feature_extract_field_width(reg, entry->field_pos,
-                                                   entry->field_width,
-                                                   entry->sign);
+       int val, min, max;
+       u64 tmp;
+
+       val = cpuid_feature_extract_field_width(reg, entry->field_pos,
+                                               entry->field_width,
+                                               entry->sign);
+
+       tmp = entry->min_field_value;
+       tmp <<= entry->field_pos;
+
+       min = cpuid_feature_extract_field_width(tmp, entry->field_pos,
+                                               entry->field_width,
+                                               entry->sign);
+
+       tmp = entry->max_field_value;
+       tmp <<= entry->field_pos;
+
+       max = cpuid_feature_extract_field_width(tmp, entry->field_pos,
+                                               entry->field_width,
+                                               entry->sign);
 
-       return val >= entry->min_field_value;
+       return val >= min && val <= max;
 }
 
 static u64