| Message ID | 20240730073847.1999420-1-andy.chiu@sifive.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | riscv: fix isa validation for virtual [ms]envcfg extension | expand |
On Tue, Jul 30, 2024 at 03:38:45PM GMT, Andy Chiu wrote: > The exntension RISCV_ISA_EXT_XLINUXENVCFG was accidentally turned off > after introducing ISA validation. The reason is that this extension is > not in the riscv_isa_ext[] list, causing riscv_get_isa_ext_data() fail > to find a corresponding riscv_isa_ext_data pointer. As a result, the > kernel skipped setting resolved_isa bitmask. To fix this, we proceed > with the bit information obtained from source_isa bitmask. This means > that the bit in resolved_isa is set unconditionally, regardless of > whether we find an entry from riscv_isa_ext[] list. This should be safe > as the kernel already parse isa string before writing into source_isa. > > Fixes: 625034abd52a ("riscv: add ISA extensions validation callback") > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > --- > arch/riscv/kernel/cpufeature.c | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > index 8f20607adb40..3f91a1f39a50 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -432,10 +432,7 @@ static void __init riscv_resolve_isa(unsigned long *source_isa, > bitmap_copy(prev_resolved_isa, resolved_isa, RISCV_ISA_EXT_MAX); > for_each_set_bit(bit, source_isa, RISCV_ISA_EXT_MAX) { > ext = riscv_get_isa_ext_data(bit); > - if (!ext) > - continue; > - > - if (ext->validate) { > + if (ext && ext->validate) { > ret = ext->validate(ext, resolved_isa); > if (ret == -EPROBE_DEFER) { > loop = true; > @@ -447,13 +444,13 @@ static void __init riscv_resolve_isa(unsigned long *source_isa, > } > } > > - set_bit(ext->id, resolved_isa); > + set_bit(bit, resolved_isa); > /* No need to keep it in source isa now that it is enabled */ > - clear_bit(ext->id, source_isa); > + clear_bit(bit, source_isa); > > /* Single letter extensions get set in hwcap */ > - if (ext->id < RISCV_ISA_EXT_BASE) > - *this_hwcap |= isa2hwcap[ext->id]; > + if (bit < RISCV_ISA_EXT_BASE) > + *this_hwcap |= isa2hwcap[bit]; > } > } while (loop && memcmp(prev_resolved_isa, resolved_isa, sizeof(prev_resolved_isa))); > } > -- > 2.43.0 > Hi Andy, This looks like the same solution that Samuel sent which is currently under discussion[1]. [1] https://lore.kernel.org/all/20240718213011.2600150-1-samuel.holland@sifive.com/ Thanks, drew
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 8f20607adb40..3f91a1f39a50 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -432,10 +432,7 @@ static void __init riscv_resolve_isa(unsigned long *source_isa, bitmap_copy(prev_resolved_isa, resolved_isa, RISCV_ISA_EXT_MAX); for_each_set_bit(bit, source_isa, RISCV_ISA_EXT_MAX) { ext = riscv_get_isa_ext_data(bit); - if (!ext) - continue; - - if (ext->validate) { + if (ext && ext->validate) { ret = ext->validate(ext, resolved_isa); if (ret == -EPROBE_DEFER) { loop = true; @@ -447,13 +444,13 @@ static void __init riscv_resolve_isa(unsigned long *source_isa, } } - set_bit(ext->id, resolved_isa); + set_bit(bit, resolved_isa); /* No need to keep it in source isa now that it is enabled */ - clear_bit(ext->id, source_isa); + clear_bit(bit, source_isa); /* Single letter extensions get set in hwcap */ - if (ext->id < RISCV_ISA_EXT_BASE) - *this_hwcap |= isa2hwcap[ext->id]; + if (bit < RISCV_ISA_EXT_BASE) + *this_hwcap |= isa2hwcap[bit]; } } while (loop && memcmp(prev_resolved_isa, resolved_isa, sizeof(prev_resolved_isa))); }
The exntension RISCV_ISA_EXT_XLINUXENVCFG was accidentally turned off after introducing ISA validation. The reason is that this extension is not in the riscv_isa_ext[] list, causing riscv_get_isa_ext_data() fail to find a corresponding riscv_isa_ext_data pointer. As a result, the kernel skipped setting resolved_isa bitmask. To fix this, we proceed with the bit information obtained from source_isa bitmask. This means that the bit in resolved_isa is set unconditionally, regardless of whether we find an entry from riscv_isa_ext[] list. This should be safe as the kernel already parse isa string before writing into source_isa. Fixes: 625034abd52a ("riscv: add ISA extensions validation callback") Signed-off-by: Andy Chiu <andy.chiu@sifive.com> --- arch/riscv/kernel/cpufeature.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)