Skip to content

Commit 1771c8c

Browse files
mmindpalmer-dabbelt
authored andcommitted
riscv: remove usage of function-pointers from cpufeatures and t-head errata
Having a list of alternatives to check with a per-entry function pointer to a check function is nice style-wise. But in case of early-alternatives it can clash with the non-relocated kernel and the function pointer in the list pointing to a completely wrong location. This isn't an issue with one or two list entries, as in that case the compiler seems to unroll the loop and even usage of the list structure and then only does relative jumps into the check functions based on this. When adding a third entry to either list though, the issue that was hiding there from the beginning is triggered resulting a jump to a memory address that isn't part of the kernel at all. The list of features/erratas only contained an unused name and the pointer to the check function, so an easy solution for the problem is to just unroll the loop in code, dismantle the whole list structure and just call the relevant check functions one by one ourself. For the T-Head errata this includes moving the stage-check inside the check functions. The issue is only relevant for things that might be called for early- alternatives (T-Head and possible future main extensions), so the SiFive erratas were not affected from the beginning, as they got an early return for early-alternatives in the original patchset. Signed-off-by: Heiko Stuebner <[email protected]> Tested-by: Samuel Holland <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Palmer Dabbelt <[email protected]>
1 parent b684001 commit 1771c8c

File tree

2 files changed

+22
-48
lines changed

2 files changed

+22
-48
lines changed

arch/riscv/errata/thead/errata.c

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -14,40 +14,26 @@
1414
#include <asm/patch.h>
1515
#include <asm/vendorid_list.h>
1616

17-
struct errata_info {
18-
char name[ERRATA_STRING_LENGTH_MAX];
19-
bool (*check_func)(unsigned long arch_id, unsigned long impid);
20-
unsigned int stage;
21-
};
22-
23-
static bool errata_mt_check_func(unsigned long arch_id, unsigned long impid)
17+
static bool errata_probe_pbmt(unsigned int stage,
18+
unsigned long arch_id, unsigned long impid)
2419
{
2520
if (arch_id != 0 || impid != 0)
2621
return false;
27-
return true;
28-
}
2922

30-
static const struct errata_info errata_list[ERRATA_THEAD_NUMBER] = {
31-
{
32-
.name = "memory-types",
33-
.stage = RISCV_ALTERNATIVES_EARLY_BOOT,
34-
.check_func = errata_mt_check_func
35-
},
36-
};
23+
if (stage == RISCV_ALTERNATIVES_EARLY_BOOT ||
24+
stage == RISCV_ALTERNATIVES_MODULE)
25+
return true;
26+
27+
return false;
28+
}
3729

38-
static u32 thead_errata_probe(unsigned int stage, unsigned long archid, unsigned long impid)
30+
static u32 thead_errata_probe(unsigned int stage,
31+
unsigned long archid, unsigned long impid)
3932
{
40-
const struct errata_info *info;
4133
u32 cpu_req_errata = 0;
42-
int idx;
43-
44-
for (idx = 0; idx < ERRATA_THEAD_NUMBER; idx++) {
45-
info = &errata_list[idx];
4634

47-
if ((stage == RISCV_ALTERNATIVES_MODULE ||
48-
info->stage == stage) && info->check_func(archid, impid))
49-
cpu_req_errata |= (1U << idx);
50-
}
35+
if (errata_probe_pbmt(stage, archid, impid))
36+
cpu_req_errata |= (1U << ERRATA_THEAD_PBMT);
5137

5238
return cpu_req_errata;
5339
}

arch/riscv/kernel/cpufeature.c

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -245,12 +245,7 @@ void __init riscv_fill_hwcap(void)
245245
}
246246

247247
#ifdef CONFIG_RISCV_ALTERNATIVE
248-
struct cpufeature_info {
249-
char name[ERRATA_STRING_LENGTH_MAX];
250-
bool (*check_func)(unsigned int stage);
251-
};
252-
253-
static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
248+
static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
254249
{
255250
#ifdef CONFIG_RISCV_ISA_SVPBMT
256251
switch (stage) {
@@ -264,26 +259,19 @@ static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
264259
return false;
265260
}
266261

267-
static const struct cpufeature_info __initdata_or_module
268-
cpufeature_list[CPUFEATURE_NUMBER] = {
269-
{
270-
.name = "svpbmt",
271-
.check_func = cpufeature_svpbmt_check_func
272-
},
273-
};
274-
262+
/*
263+
* Probe presence of individual extensions.
264+
*
265+
* This code may also be executed before kernel relocation, so we cannot use
266+
* addresses generated by the address-of operator as they won't be valid in
267+
* this context.
268+
*/
275269
static u32 __init_or_module cpufeature_probe(unsigned int stage)
276270
{
277-
const struct cpufeature_info *info;
278271
u32 cpu_req_feature = 0;
279-
int idx;
280-
281-
for (idx = 0; idx < CPUFEATURE_NUMBER; idx++) {
282-
info = &cpufeature_list[idx];
283272

284-
if (info->check_func(stage))
285-
cpu_req_feature |= (1U << idx);
286-
}
273+
if (cpufeature_probe_svpbmt(stage))
274+
cpu_req_feature |= (1U << CPUFEATURE_SVPBMT);
287275

288276
return cpu_req_feature;
289277
}

0 commit comments

Comments
 (0)