The AMU feature can be enabled on a subset of the cores in a system.
Because of that, it prints a message for each core as it is detected.
This becomes tedious when there are hundreds of cores. Instead, for
CPU features which can be enabled on a subset of the present cores,
lets wait until update_cpu_capabilities() and print the subset of cores
the feature was enabled on.
Since the infrastructure now exists to display the subset of cores
where a feature is enabled, lets also enable it for the Dirty Bit Management
and 32-bit EL0. The 32-bit EL0 patch is only lightly tested as the AEM/TFA
model firmware doesn't like mismatched AARCH32 support. Its also the least
ugly version of the three or so options I considered.
v1->v2:
Add DBM and 32-bit EL0 code so they now display the core subset
Add Ionela Voinescu review/testing tags
Small comment tweak in the first patch
Jeremy Linton (3):
arm64: cpufeature: Display the set of cores with a feature
arm64: cpufeature: Change DBM to display enabled cores
arm64: cpufeature: Change 32-bit EL0 to display enabled cores
arch/arm64/include/asm/cpufeature.h | 2 +
arch/arm64/kernel/cpufeature.c | 70 ++++++++++++++---------------
2 files changed, 36 insertions(+), 36 deletions(-)
--
2.41.0
Now that we have the ability to display the list of cores
with a feature when its selectivly enabled, lets convert
DBM to use that as well.
Signed-off-by: Jeremy Linton <[email protected]>
---
arch/arm64/kernel/cpufeature.c | 33 ++++++++-------------------------
1 file changed, 8 insertions(+), 25 deletions(-)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 2dd695fc3472..b7b67bac0e60 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1848,6 +1848,8 @@ static int __init parse_kpti(char *str)
early_param("kpti", parse_kpti);
#ifdef CONFIG_ARM64_HW_AFDBM
+static struct cpumask dbm_cpus __read_mostly;
+
static inline void __cpu_enable_hw_dbm(void)
{
u64 tcr = read_sysreg(tcr_el1) | TCR_HD;
@@ -1883,35 +1885,22 @@ static bool cpu_can_use_dbm(const struct arm64_cpu_capabilities *cap)
static void cpu_enable_hw_dbm(struct arm64_cpu_capabilities const *cap)
{
- if (cpu_can_use_dbm(cap))
+ if (cpu_can_use_dbm(cap)) {
__cpu_enable_hw_dbm();
+ cpumask_set_cpu(smp_processor_id(), &dbm_cpus);
+ }
}
static bool has_hw_dbm(const struct arm64_cpu_capabilities *cap,
int __unused)
{
- static bool detected = false;
/*
* DBM is a non-conflicting feature. i.e, the kernel can safely
* run a mix of CPUs with and without the feature. So, we
* unconditionally enable the capability to allow any late CPU
* to use the feature. We only enable the control bits on the
- * CPU, if it actually supports.
- *
- * We have to make sure we print the "feature" detection only
- * when at least one CPU actually uses it. So check if this CPU
- * can actually use it and print the message exactly once.
- *
- * This is safe as all CPUs (including secondary CPUs - due to the
- * LOCAL_CPU scope - and the hotplugged CPUs - via verification)
- * goes through the "matches" check exactly once. Also if a CPU
- * matches the criteria, it is guaranteed that the CPU will turn
- * the DBM on, as the capability is unconditionally enabled.
+ * CPU, if it is supported.
*/
- if (!detected && cpu_can_use_dbm(cap)) {
- detected = true;
- pr_info("detected: Hardware dirty bit management\n");
- }
return true;
}
@@ -2448,18 +2437,12 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
},
#ifdef CONFIG_ARM64_HW_AFDBM
{
- /*
- * Since we turn this on always, we don't want the user to
- * think that the feature is available when it may not be.
- * So hide the description.
- *
- * .desc = "Hardware pagetable Dirty Bit Management",
- *
- */
+ .desc = "Hardware dirty bit management",
.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
.capability = ARM64_HW_DBM,
.matches = has_hw_dbm,
.cpu_enable = cpu_enable_hw_dbm,
+ .cpus = &dbm_cpus,
ARM64_CPUID_FIELDS(ID_AA64MMFR1_EL1, HAFDBS, DBM)
},
#endif
--
2.41.0
Now that we have the ability to display the list of cores
with a feature when it is selectivly enabled, lets display the
cores enabled for 32-bit use at EL0.
Signed-off-by: Jeremy Linton <[email protected]>
---
arch/arm64/kernel/cpufeature.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index b7b67bac0e60..512cbe446b41 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1533,8 +1533,17 @@ static bool has_32bit_el0(const struct arm64_cpu_capabilities *entry, int scope)
if (!has_cpuid_feature(entry, scope))
return allow_mismatched_32bit_el0;
- if (scope == SCOPE_SYSTEM)
- pr_info("detected: 32-bit EL0 Support\n");
+ if (scope == SCOPE_SYSTEM) {
+ struct arm64_cpu_capabilities *has_32bit;
+
+ has_32bit = (struct arm64_cpu_capabilities *)entry;
+
+ has_32bit->cpus = system_32bit_el0_cpumask();
+ if (has_32bit->cpus == cpu_possible_mask)
+ has_32bit->cpus = cpu_online_mask;
+ if (has_32bit->cpus == cpu_none_mask)
+ has_32bit->cpus = NULL;
+ }
return true;
}
@@ -2307,9 +2316,11 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
ARM64_CPUID_FIELDS(ID_AA64MMFR2_EL1, NV, IMP)
},
{
+ .desc = "32-bit EL0 Support",
.capability = ARM64_HAS_32BIT_EL0_DO_NOT_USE,
.type = ARM64_CPUCAP_SYSTEM_FEATURE,
.matches = has_32bit_el0,
+ .cpus = cpu_none_mask,
ARM64_CPUID_FIELDS(ID_AA64PFR0_EL1, EL0, AARCH32)
},
#ifdef CONFIG_KVM
--
2.41.0
On Tue, 17 Oct 2023 06:23:22 +0100,
Jeremy Linton <[email protected]> wrote:
>
> Now that we have the ability to display the list of cores
> with a feature when it is selectivly enabled, lets display the
> cores enabled for 32-bit use at EL0.
>
> Signed-off-by: Jeremy Linton <[email protected]>
> ---
> arch/arm64/kernel/cpufeature.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index b7b67bac0e60..512cbe446b41 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1533,8 +1533,17 @@ static bool has_32bit_el0(const struct arm64_cpu_capabilities *entry, int scope)
> if (!has_cpuid_feature(entry, scope))
> return allow_mismatched_32bit_el0;
>
> - if (scope == SCOPE_SYSTEM)
> - pr_info("detected: 32-bit EL0 Support\n");
> + if (scope == SCOPE_SYSTEM) {
> + struct arm64_cpu_capabilities *has_32bit;
> +
> + has_32bit = (struct arm64_cpu_capabilities *)entry;
> +
> + has_32bit->cpus = system_32bit_el0_cpumask();
This seems rather dodgy. 'entry' comes from a static const array which
will, in all likelihood be mapped R/O pretty soon after the initial
CPU bringup. Try offlining/onlining a CPU and you should see a
firework similar to what I have below (I hacked the CnP property, but
that's no different from what you are doing):
bash-5.1# echo 0 >/sys/devices/system/cpu/cpu2/online
[ 51.367137] psci: CPU2 killed (polled 0 ms)
bash-5.1# echo 1 >/sys/devices/system/cpu/cpu2/online
[ 56.703092] Unable to handle kernel write to read-only memory at virtual address ffff800080d63db0
[ 56.703494] Mem abort info:
[ 56.703627] ESR = 0x000000009600004f
[ 56.703799] EC = 0x25: DABT (current EL), IL = 32 bits
[ 56.704052] SET = 0, FnV = 0
[ 56.704196] EA = 0, S1PTW = 0
[ 56.704340] FSC = 0x0f: level 3 permission fault
[ 56.704575] Data abort info:
[ 56.704715] ISV = 0, ISS = 0x0000004f, ISS2 = 0x00000000
[ 56.704961] CM = 0, WnR = 1, TnD = 0, TagAccess = 0
[ 56.705195] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ 56.705435] swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000008142e000
[ 56.705742] [ffff800080d63db0] pgd=10000000dbfff003, p4d=10000000dbfff003, pud=10000000dbffe003, pmd=10000000dbffc003, pte=0060000080d63783
[ 56.706310] Internal error: Oops: 000000009600004f [#1] PREEMPT SMP
[ 56.706591] Modules linked in:
[ 56.706741] CPU: 2 PID: 0 Comm: swapper/2 Not tainted 6.6.0-rc1-00007-g805cc7d30885-dirty #1702
[ 56.707145] Hardware name: linux,dummy-virt (DT)
[ 56.707359] pstate: 800001c5 (Nzcv dAIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 56.707676] pc : cpu_enable_cnp+0x24/0x60
[ 56.707873] lr : verify_local_cpu_caps+0x6c/0x140
[ 56.708046] sp : ffff800082103e10
[ 56.708201] x29: ffff800082103e10 x28: 0000000000000000 x27: 0000000000000000
[ 56.708517] x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
[ 56.708837] x23: ffff800081eec540 x22: 0000000000000003 x21: ffff8000813945f0
[ 56.709166] x20: ffff800081394358 x19: ffff800080d63db0 x18: 0000000000000000
[ 56.709488] x17: 000000040044ffff x16: 00500073b5503510 x15: 0000000000000000
[ 56.709805] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
[ 56.710100] x11: 0000000000000000 x10: 0000000000000000 x9 : ffff8000800273ac
[ 56.710416] x8 : ffff80008003b640 x7 : 0000000000000000 x6 : 0000000000000000
[ 56.710738] x5 : 0000000000000000 x4 : ffff800081394000 x3 : ffff800081394000
[ 56.711070] x2 : ffff800081241ef8 x1 : ffff800080027218 x0 : ffff800080d63db0
[ 56.711396] Call trace:
[ 56.711512] cpu_enable_cnp+0x24/0x60
[ 56.711684] verify_local_cpu_caps+0x6c/0x140
[ 56.711885] verify_local_cpu_capabilities+0x1c/0x198
[ 56.712112] check_local_cpu_capabilities+0x28/0x50
[ 56.712337] secondary_start_kernel+0xb4/0x1a0
[ 56.712546] __secondary_switched+0xb8/0xc0
[ 56.712739] Code: 910003fd b0009b63 d00090c2 913be042 (f9000002)
[ 56.713013] ---[ end trace 0000000000000000 ]---
[ 56.713224] Kernel panic - not syncing: Attempted to kill the idle task!
[ 56.713518] SMP: stopping secondary CPUs
[ 56.713752] Kernel Offset: disabled
[ 56.713908] CPU features: 0x00000000,50024d43,1947f723
[ 56.714144] Memory Limit: none
[ 56.714292] ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Hi,
On 10/17/23 13:01, Marc Zyngier wrote:
> On Tue, 17 Oct 2023 06:23:22 +0100,
> Jeremy Linton <[email protected]> wrote:
>>
>> Now that we have the ability to display the list of cores
>> with a feature when it is selectivly enabled, lets display the
>> cores enabled for 32-bit use at EL0.
>>
>> Signed-off-by: Jeremy Linton <[email protected]>
>> ---
>> arch/arm64/kernel/cpufeature.c | 15 +++++++++++++--
>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index b7b67bac0e60..512cbe446b41 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -1533,8 +1533,17 @@ static bool has_32bit_el0(const struct arm64_cpu_capabilities *entry, int scope)
>> if (!has_cpuid_feature(entry, scope))
>> return allow_mismatched_32bit_el0;
>>
>> - if (scope == SCOPE_SYSTEM)
>> - pr_info("detected: 32-bit EL0 Support\n");
>> + if (scope == SCOPE_SYSTEM) {
>> + struct arm64_cpu_capabilities *has_32bit;
>> +
>> + has_32bit = (struct arm64_cpu_capabilities *)entry;
>> +
>> + has_32bit->cpus = system_32bit_el0_cpumask();
>
> This seems rather dodgy. 'entry' comes from a static const array which
> will, in all likelihood be mapped R/O pretty soon after the initial
> CPU bringup. Try offlining/onlining a CPU and you should see a
> firework similar to what I have below (I hacked the CnP property, but
> that's no different from what you are doing):
Yes, dodgy is a good word. The other choices, maintain a mask just for
the print or dump the static key and always use the cpu_32bit_el0_mask
or some combination, weren't much better in the "ick" category. If
anyone sees a better way I'm open to suggestion, although simply
dropping this last patch is fine too.
>
> bash-5.1# echo 0 >/sys/devices/system/cpu/cpu2/online
> [ 51.367137] psci: CPU2 killed (polled 0 ms)
> bash-5.1# echo 1 >/sys/devices/system/cpu/cpu2/online
> [ 56.703092] Unable to handle kernel write to read-only memory at virtual address ffff800080d63db0
> [ 56.703494] Mem abort info:
> [ 56.703627] ESR = 0x000000009600004f
> [ 56.703799] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 56.704052] SET = 0, FnV = 0
> [ 56.704196] EA = 0, S1PTW = 0
> [ 56.704340] FSC = 0x0f: level 3 permission fault
> [ 56.704575] Data abort info:
> [ 56.704715] ISV = 0, ISS = 0x0000004f, ISS2 = 0x00000000
> [ 56.704961] CM = 0, WnR = 1, TnD = 0, TagAccess = 0
> [ 56.705195] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [ 56.705435] swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000008142e000
> [ 56.705742] [ffff800080d63db0] pgd=10000000dbfff003, p4d=10000000dbfff003, pud=10000000dbffe003, pmd=10000000dbffc003, pte=0060000080d63783
> [ 56.706310] Internal error: Oops: 000000009600004f [#1] PREEMPT SMP
> [ 56.706591] Modules linked in:
> [ 56.706741] CPU: 2 PID: 0 Comm: swapper/2 Not tainted 6.6.0-rc1-00007-g805cc7d30885-dirty #1702
> [ 56.707145] Hardware name: linux,dummy-virt (DT)
> [ 56.707359] pstate: 800001c5 (Nzcv dAIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 56.707676] pc : cpu_enable_cnp+0x24/0x60
> [ 56.707873] lr : verify_local_cpu_caps+0x6c/0x140
> [ 56.708046] sp : ffff800082103e10
> [ 56.708201] x29: ffff800082103e10 x28: 0000000000000000 x27: 0000000000000000
> [ 56.708517] x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
> [ 56.708837] x23: ffff800081eec540 x22: 0000000000000003 x21: ffff8000813945f0
> [ 56.709166] x20: ffff800081394358 x19: ffff800080d63db0 x18: 0000000000000000
> [ 56.709488] x17: 000000040044ffff x16: 00500073b5503510 x15: 0000000000000000
> [ 56.709805] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
> [ 56.710100] x11: 0000000000000000 x10: 0000000000000000 x9 : ffff8000800273ac
> [ 56.710416] x8 : ffff80008003b640 x7 : 0000000000000000 x6 : 0000000000000000
> [ 56.710738] x5 : 0000000000000000 x4 : ffff800081394000 x3 : ffff800081394000
> [ 56.711070] x2 : ffff800081241ef8 x1 : ffff800080027218 x0 : ffff800080d63db0
> [ 56.711396] Call trace:
> [ 56.711512] cpu_enable_cnp+0x24/0x60
> [ 56.711684] verify_local_cpu_caps+0x6c/0x140
> [ 56.711885] verify_local_cpu_capabilities+0x1c/0x198
> [ 56.712112] check_local_cpu_capabilities+0x28/0x50
> [ 56.712337] secondary_start_kernel+0xb4/0x1a0
> [ 56.712546] __secondary_switched+0xb8/0xc0
> [ 56.712739] Code: 910003fd b0009b63 d00090c2 913be042 (f9000002)
> [ 56.713013] ---[ end trace 0000000000000000 ]---
> [ 56.713224] Kernel panic - not syncing: Attempted to kill the idle task!
> [ 56.713518] SMP: stopping secondary CPUs
> [ 56.713752] Kernel Offset: disabled
> [ 56.713908] CPU features: 0x00000000,50024d43,1947f723
> [ 56.714144] Memory Limit: none
> [ 56.714292] ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---
>
> Thanks,
>
> M.
>
On Tue, 17 Oct 2023 20:15:43 +0100,
Jeremy Linton <[email protected]> wrote:
>
> Hi,
>
> On 10/17/23 13:01, Marc Zyngier wrote:
> > On Tue, 17 Oct 2023 06:23:22 +0100,
> > Jeremy Linton <[email protected]> wrote:
> >>
> >> Now that we have the ability to display the list of cores
> >> with a feature when it is selectivly enabled, lets display the
> >> cores enabled for 32-bit use at EL0.
> >>
> >> Signed-off-by: Jeremy Linton <[email protected]>
> >> ---
> >> arch/arm64/kernel/cpufeature.c | 15 +++++++++++++--
> >> 1 file changed, 13 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> >> index b7b67bac0e60..512cbe446b41 100644
> >> --- a/arch/arm64/kernel/cpufeature.c
> >> +++ b/arch/arm64/kernel/cpufeature.c
> >> @@ -1533,8 +1533,17 @@ static bool has_32bit_el0(const struct arm64_cpu_capabilities *entry, int scope)
> >> if (!has_cpuid_feature(entry, scope))
> >> return allow_mismatched_32bit_el0;
> >> - if (scope == SCOPE_SYSTEM)
> >> - pr_info("detected: 32-bit EL0 Support\n");
> >> + if (scope == SCOPE_SYSTEM) {
> >> + struct arm64_cpu_capabilities *has_32bit;
> >> +
> >> + has_32bit = (struct arm64_cpu_capabilities *)entry;
> >> +
> >> + has_32bit->cpus = system_32bit_el0_cpumask();
> >
> > This seems rather dodgy. 'entry' comes from a static const array which
> > will, in all likelihood be mapped R/O pretty soon after the initial
> > CPU bringup. Try offlining/onlining a CPU and you should see a
> > firework similar to what I have below (I hacked the CnP property, but
> > that's no different from what you are doing):
>
> Yes, dodgy is a good word. The other choices, maintain a mask just for
> the print or dump the static key and always use the cpu_32bit_el0_mask
> or some combination, weren't much better in the "ick" category. If
> anyone sees a better way I'm open to suggestion, although simply
> dropping this last patch is fine too.
An obvious choice would be to replace the 'cpus' cpumask with a
function that evaluates a cpumask stored separately.
M.
--
Without deviation from the norm, progress is not possible.
On 10/18/23 07:43, Marc Zyngier wrote:
> On Tue, 17 Oct 2023 20:15:43 +0100,
> Jeremy Linton <[email protected]> wrote:
>>
>> Hi,
>>
>> On 10/17/23 13:01, Marc Zyngier wrote:
>>> On Tue, 17 Oct 2023 06:23:22 +0100,
>>> Jeremy Linton <[email protected]> wrote:
>>>>
>>>> Now that we have the ability to display the list of cores
>>>> with a feature when it is selectivly enabled, lets display the
>>>> cores enabled for 32-bit use at EL0.
>>>>
>>>> Signed-off-by: Jeremy Linton <[email protected]>
>>>> ---
>>>> arch/arm64/kernel/cpufeature.c | 15 +++++++++++++--
>>>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>>>> index b7b67bac0e60..512cbe446b41 100644
>>>> --- a/arch/arm64/kernel/cpufeature.c
>>>> +++ b/arch/arm64/kernel/cpufeature.c
>>>> @@ -1533,8 +1533,17 @@ static bool has_32bit_el0(const struct arm64_cpu_capabilities *entry, int scope)
>>>> if (!has_cpuid_feature(entry, scope))
>>>> return allow_mismatched_32bit_el0;
>>>> - if (scope == SCOPE_SYSTEM)
>>>> - pr_info("detected: 32-bit EL0 Support\n");
>>>> + if (scope == SCOPE_SYSTEM) {
>>>> + struct arm64_cpu_capabilities *has_32bit;
>>>> +
>>>> + has_32bit = (struct arm64_cpu_capabilities *)entry;
>>>> +
>>>> + has_32bit->cpus = system_32bit_el0_cpumask();
>>>
>>> This seems rather dodgy. 'entry' comes from a static const array which
>>> will, in all likelihood be mapped R/O pretty soon after the initial
>>> CPU bringup. Try offlining/onlining a CPU and you should see a
>>> firework similar to what I have below (I hacked the CnP property, but
>>> that's no different from what you are doing):
>>
>> Yes, dodgy is a good word. The other choices, maintain a mask just for
>> the print or dump the static key and always use the cpu_32bit_el0_mask
>> or some combination, weren't much better in the "ick" category. If
>> anyone sees a better way I'm open to suggestion, although simply
>> dropping this last patch is fine too.
>
> An obvious choice would be to replace the 'cpus' cpumask with a
> function that evaluates a cpumask stored separately.
Right, I was too busy trying to cleanup the 32-bit mask, that works too.
On 2023-10-18 16:42, Jeremy Linton wrote:
> On 10/18/23 07:43, Marc Zyngier wrote:
>> On Tue, 17 Oct 2023 20:15:43 +0100,
>> Jeremy Linton <[email protected]> wrote:
>>>
>>> Hi,
>>>
>>> On 10/17/23 13:01, Marc Zyngier wrote:
>>>> On Tue, 17 Oct 2023 06:23:22 +0100,
>>>> Jeremy Linton <[email protected]> wrote:
>>>>>
>>>>> Now that we have the ability to display the list of cores
>>>>> with a feature when it is selectivly enabled, lets display the
>>>>> cores enabled for 32-bit use at EL0.
>>>>>
>>>>> Signed-off-by: Jeremy Linton <[email protected]>
>>>>> ---
>>>>> arch/arm64/kernel/cpufeature.c | 15 +++++++++++++--
>>>>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/kernel/cpufeature.c
>>>>> b/arch/arm64/kernel/cpufeature.c
>>>>> index b7b67bac0e60..512cbe446b41 100644
>>>>> --- a/arch/arm64/kernel/cpufeature.c
>>>>> +++ b/arch/arm64/kernel/cpufeature.c
>>>>> @@ -1533,8 +1533,17 @@ static bool has_32bit_el0(const struct
>>>>> arm64_cpu_capabilities *entry, int scope)
>>>>> if (!has_cpuid_feature(entry, scope))
>>>>> return allow_mismatched_32bit_el0;
>>>>> - if (scope == SCOPE_SYSTEM)
>>>>> - pr_info("detected: 32-bit EL0 Support\n");
>>>>> + if (scope == SCOPE_SYSTEM) {
>>>>> + struct arm64_cpu_capabilities *has_32bit;
>>>>> +
>>>>> + has_32bit = (struct arm64_cpu_capabilities *)entry;
>>>>> +
>>>>> + has_32bit->cpus = system_32bit_el0_cpumask();
>>>>
>>>> This seems rather dodgy. 'entry' comes from a static const array which
>>>> will, in all likelihood be mapped R/O pretty soon after the initial
>>>> CPU bringup. Try offlining/onlining a CPU and you should see a
>>>> firework similar to what I have below (I hacked the CnP property, but
>>>> that's no different from what you are doing):
>>>
>>> Yes, dodgy is a good word. The other choices, maintain a mask just for
>>> the print or dump the static key and always use the cpu_32bit_el0_mask
>>> or some combination, weren't much better in the "ick" category. If
>>> anyone sees a better way I'm open to suggestion, although simply
>>> dropping this last patch is fine too.
>>
>> An obvious choice would be to replace the 'cpus' cpumask with a
>> function that evaluates a cpumask stored separately.
>
> Right, I was too busy trying to cleanup the 32-bit mask, that works too.
But if a cpumask does already exist, wouldn't the *most* logical thing
be to use it? If it is somehow harmful to the existing machinery to set
all of the CPUs in cpu_32bit_el0_mask if all of the CPUs support 32-bit
EL0, that seems like a mind-bogglingly dumb design which deserves fixing
in its own right.
To me the obvious thing would be to allocate and populate
cpu_32bit_el0_mask unconditionally so the initial feature detection can
work as normal, and then if anyone's really really bothered, free it
again later if we determine there's no mismatch.
Thanks,
Robin.
On Tue, 17 Oct 2023 00:23:19 -0500, Jeremy Linton wrote:
> The AMU feature can be enabled on a subset of the cores in a system.
> Because of that, it prints a message for each core as it is detected.
> This becomes tedious when there are hundreds of cores. Instead, for
> CPU features which can be enabled on a subset of the present cores,
> lets wait until update_cpu_capabilities() and print the subset of cores
> the feature was enabled on.
>
> [...]
Applied to arm64 (for-next/cpufeat-display-cores), thanks!
[1/3] arm64: cpufeature: Display the set of cores with a feature
https://git.kernel.org/arm64/c/23b727dc2092
[2/3] arm64: cpufeature: Change DBM to display enabled cores
https://git.kernel.org/arm64/c/04d402a453c3
I dropped the third patch from the series.
--
Catalin