Add support for (yet again) more RVA23U64 missing extensions. Add support for Zcmop, Zca, Zcf, Zcd and Zcb extensions isa string parsing, hwprobe and kvm support. Zce, Zcmt and Zcmp extensions have been left out since they target microcontrollers/embedded CPUs and are not needed by RVA23U64
This series is based on the Zimop one [1].
Link: https://lore.kernel.org/linux-riscv/20240404103254.1752834-1-cleger@rivosinc... [1]
Clément Léger (10): dt-bindings: riscv: add Zca, Zcf, Zcd and Zcb ISA extension description riscv: add ISA parsing for Zca, Zcf, Zcd and Zcb riscv: hwprobe: export Zca, Zcf, Zcd and Zcb ISA extensions RISC-V: KVM: Allow Zca, Zcf, Zcd and Zcb extensions for Guest/VM KVM: riscv: selftests: Add some Zc* extensions to get-reg-list test dt-bindings: riscv: add Zcmop ISA extension description riscv: add ISA extension parsing for Zcmop riscv: hwprobe: export Zcmop ISA extension RISC-V: KVM: Allow Zcmop extension for Guest/VM KVM: riscv: selftests: Add Zcmop extension to get-reg-list test
Documentation/arch/riscv/hwprobe.rst | 24 ++++++++++++ .../devicetree/bindings/riscv/extensions.yaml | 37 +++++++++++++++++++ arch/riscv/include/asm/hwcap.h | 5 +++ arch/riscv/include/uapi/asm/hwprobe.h | 5 +++ arch/riscv/include/uapi/asm/kvm.h | 5 +++ arch/riscv/kernel/cpufeature.c | 5 +++ arch/riscv/kernel/sys_hwprobe.c | 5 +++ arch/riscv/kvm/vcpu_onereg.c | 10 +++++ .../selftests/kvm/riscv/get-reg-list.c | 20 ++++++++++ 9 files changed, 116 insertions(+)
Add description for Zca, Zcf, Zcd and Zcb extensions which are part the Zc* standard extensions for code size reduction.
Signed-off-by: Clément Léger cleger@rivosinc.com --- .../devicetree/bindings/riscv/extensions.yaml | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+)
diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml index 616370318a66..516f57bdfeeb 100644 --- a/Documentation/devicetree/bindings/riscv/extensions.yaml +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml @@ -220,6 +220,38 @@ properties: instructions as ratified at commit 6d33919 ("Merge pull request #158 from hirooih/clmul-fix-loop-end-condition") of riscv-bitmanip.
+ - const: zca + description: | + The Zca extension part of Zc* standard extensions for code size + reduction, as ratified in commit 8be3419c1c0 ("Zcf doesn't exist on + RV64 as it contains no instructions") of riscv-code-size-reduction, + merged in the riscv-isa-manual by commit dbc79cf28a2 ("Initial seed + of zc.adoc to src tree."). + + - const: zcb + description: | + The Zcb extension part of Zc* standard extensions for code size + reduction, as ratified in commit 8be3419c1c0 ("Zcf doesn't exist on + RV64 as it contains no instructions") of riscv-code-size-reduction, + merged in the riscv-isa-manual by commit dbc79cf28a2 ("Initial seed + of zc.adoc to src tree."). + + - const: zcd + description: | + The Zcd extension part of Zc* standard extensions for code size + reduction, as ratified in commit 8be3419c1c0 ("Zcf doesn't exist on + RV64 as it contains no instructions") of riscv-code-size-reduction, + merged in the riscv-isa-manual by commit dbc79cf28a2 ("Initial seed + of zc.adoc to src tree."). + + - const: zcf + description: | + The Zcf extension part of Zc* standard extensions for code size + reduction, as ratified in commit 8be3419c1c0 ("Zcf doesn't exist on + RV64 as it contains no instructions") of riscv-code-size-reduction, + merged in the riscv-isa-manual by commit dbc79cf28a2 ("Initial seed + of zc.adoc to src tree."). + - const: zfa description: The standard Zfa extension for additional floating point
The Zc* standard extension for code reduction introduces new extensions. This patch adds support for Zca, Zcf, Zcd and Zcb. Zce, Zcmt and Zcmp are left out of this patch since they are targeting microcontrollers/ embedded CPUs instead of application processors.
Signed-off-by: Clément Léger cleger@rivosinc.com --- arch/riscv/include/asm/hwcap.h | 4 ++++ arch/riscv/kernel/cpufeature.c | 4 ++++ 2 files changed, 8 insertions(+)
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index 543e3ea2da0e..b7551bad341b 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -82,6 +82,10 @@ #define RISCV_ISA_EXT_ZACAS 73 #define RISCV_ISA_EXT_XANDESPMU 74 #define RISCV_ISA_EXT_ZIMOP 75 +#define RISCV_ISA_EXT_ZCA 76 +#define RISCV_ISA_EXT_ZCB 77 +#define RISCV_ISA_EXT_ZCD 78 +#define RISCV_ISA_EXT_ZCF 79
#define RISCV_ISA_EXT_XLINUXENVCFG 127
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 115ba001f1bc..09dee071274d 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -261,6 +261,10 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = { __RISCV_ISA_EXT_DATA(zfa, RISCV_ISA_EXT_ZFA), __RISCV_ISA_EXT_DATA(zfh, RISCV_ISA_EXT_ZFH), __RISCV_ISA_EXT_DATA(zfhmin, RISCV_ISA_EXT_ZFHMIN), + __RISCV_ISA_EXT_DATA(zca, RISCV_ISA_EXT_ZCA), + __RISCV_ISA_EXT_DATA(zcb, RISCV_ISA_EXT_ZCB), + __RISCV_ISA_EXT_DATA(zcd, RISCV_ISA_EXT_ZCD), + __RISCV_ISA_EXT_DATA(zcf, RISCV_ISA_EXT_ZCF), __RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA), __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB), __RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC),
Export Zca, Zcf, Zcd and Zcb ISA extension through hwprobe.
Signed-off-by: Clément Léger cleger@rivosinc.com --- Documentation/arch/riscv/hwprobe.rst | 20 ++++++++++++++++++++ arch/riscv/include/uapi/asm/hwprobe.h | 4 ++++ arch/riscv/kernel/sys_hwprobe.c | 4 ++++ 3 files changed, 28 insertions(+)
diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst index 9ca5b093b6d5..bf96b4e8ba3b 100644 --- a/Documentation/arch/riscv/hwprobe.rst +++ b/Documentation/arch/riscv/hwprobe.rst @@ -192,6 +192,26 @@ The following keys are defined: supported as defined in the RISC-V ISA manual starting from commit 58220614a5f ("Zimop is ratified/1.0").
+ * :c:macro:`RISCV_HWPROBE_EXT_ZCA`: The Zca extension part of Zc* standard + extensions for code size reduction, as ratified in commit 8be3419c1c0 + ("Zcf doesn't exist on RV64 as it contains no instructions") of + riscv-code-size-reduction. + + * :c:macro:`RISCV_HWPROBE_EXT_ZCB`: The Zcb extension part of Zc* standard + extensions for code size reduction, as ratified in commit 8be3419c1c0 + ("Zcf doesn't exist on RV64 as it contains no instructions") of + riscv-code-size-reduction. + + * :c:macro:`RISCV_HWPROBE_EXT_ZCD`: The Zcd extension part of Zc* standard + extensions for code size reduction, as ratified in commit 8be3419c1c0 + ("Zcf doesn't exist on RV64 as it contains no instructions") of + riscv-code-size-reduction. + + * :c:macro:`RISCV_HWPROBE_EXT_ZCF`: The Zcf extension part of Zc* standard + extensions for code size reduction, as ratified in commit 8be3419c1c0 + ("Zcf doesn't exist on RV64 as it contains no instructions") of + riscv-code-size-reduction. + * :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains performance information about the selected set of processors.
diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h index ac6874ab743a..dd4ad77faf49 100644 --- a/arch/riscv/include/uapi/asm/hwprobe.h +++ b/arch/riscv/include/uapi/asm/hwprobe.h @@ -60,6 +60,10 @@ struct riscv_hwprobe { #define RISCV_HWPROBE_EXT_ZACAS (1ULL << 34) #define RISCV_HWPROBE_EXT_ZICOND (1ULL << 35) #define RISCV_HWPROBE_EXT_ZIMOP (1ULL << 36) +#define RISCV_HWPROBE_EXT_ZCA (1ULL << 37) +#define RISCV_HWPROBE_EXT_ZCB (1ULL << 38) +#define RISCV_HWPROBE_EXT_ZCD (1ULL << 39) +#define RISCV_HWPROBE_EXT_ZCF (1ULL << 40) #define RISCV_HWPROBE_KEY_CPUPERF_0 5 #define RISCV_HWPROBE_MISALIGNED_UNKNOWN (0 << 0) #define RISCV_HWPROBE_MISALIGNED_EMULATED (1 << 0) diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c index c99a4cf231c5..2ffa0fe5101e 100644 --- a/arch/riscv/kernel/sys_hwprobe.c +++ b/arch/riscv/kernel/sys_hwprobe.c @@ -112,6 +112,8 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair, EXT_KEY(ZACAS); EXT_KEY(ZICOND); EXT_KEY(ZIMOP); + EXT_KEY(ZCA); + EXT_KEY(ZCB);
if (has_vector()) { EXT_KEY(ZVBB); @@ -132,6 +134,8 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair, EXT_KEY(ZFH); EXT_KEY(ZFHMIN); EXT_KEY(ZFA); + EXT_KEY(ZCD); + EXT_KEY(ZCF); } #undef EXT_KEY }
Extend the KVM ISA extension ONE_REG interface to allow KVM user space to detect and enable Zca, Zcf, Zcd and Zcb extensions for Guest/VM.
Signed-off-by: Clément Léger cleger@rivosinc.com --- arch/riscv/include/uapi/asm/kvm.h | 4 ++++ arch/riscv/kvm/vcpu_onereg.c | 8 ++++++++ 2 files changed, 12 insertions(+)
diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h index 35a12aa1953e..57db3fea679f 100644 --- a/arch/riscv/include/uapi/asm/kvm.h +++ b/arch/riscv/include/uapi/asm/kvm.h @@ -168,6 +168,10 @@ enum KVM_RISCV_ISA_EXT_ID { KVM_RISCV_ISA_EXT_ZTSO, KVM_RISCV_ISA_EXT_ZACAS, KVM_RISCV_ISA_EXT_ZIMOP, + KVM_RISCV_ISA_EXT_ZCA, + KVM_RISCV_ISA_EXT_ZCB, + KVM_RISCV_ISA_EXT_ZCD, + KVM_RISCV_ISA_EXT_ZCF, KVM_RISCV_ISA_EXT_MAX, };
diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c index c6ee763422f2..7d47fc910bd9 100644 --- a/arch/riscv/kvm/vcpu_onereg.c +++ b/arch/riscv/kvm/vcpu_onereg.c @@ -48,6 +48,10 @@ static const unsigned long kvm_isa_ext_arr[] = { KVM_ISA_EXT_ARR(ZBKC), KVM_ISA_EXT_ARR(ZBKX), KVM_ISA_EXT_ARR(ZBS), + KVM_ISA_EXT_ARR(ZCA), + KVM_ISA_EXT_ARR(ZCB), + KVM_ISA_EXT_ARR(ZCD), + KVM_ISA_EXT_ARR(ZCF), KVM_ISA_EXT_ARR(ZFA), KVM_ISA_EXT_ARR(ZFH), KVM_ISA_EXT_ARR(ZFHMIN), @@ -128,6 +132,10 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext) case KVM_RISCV_ISA_EXT_ZBKC: case KVM_RISCV_ISA_EXT_ZBKX: case KVM_RISCV_ISA_EXT_ZBS: + case KVM_RISCV_ISA_EXT_ZCA: + case KVM_RISCV_ISA_EXT_ZCB: + case KVM_RISCV_ISA_EXT_ZCD: + case KVM_RISCV_ISA_EXT_ZCF: case KVM_RISCV_ISA_EXT_ZFA: case KVM_RISCV_ISA_EXT_ZFH: case KVM_RISCV_ISA_EXT_ZFHMIN:
The KVM RISC-V allows Zca, Zcf, Zcd and Zcb extensions for Guest/VM so add these extensions to get-reg-list test.
Signed-off-by: Clément Léger cleger@rivosinc.com --- tools/testing/selftests/kvm/riscv/get-reg-list.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c index 40107bb61975..61cad4514197 100644 --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c @@ -55,6 +55,10 @@ bool filter_reg(__u64 reg) case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBKC: case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBKX: case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBS: + case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZCA: + case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZCB: + case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZCD: + case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZCF: case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZFA: case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZFH: case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZFHMIN: @@ -421,6 +425,10 @@ static const char *isa_ext_single_id_to_str(__u64 reg_off) KVM_ISA_EXT_ARR(ZBKC), KVM_ISA_EXT_ARR(ZBKX), KVM_ISA_EXT_ARR(ZBS), + KVM_ISA_EXT_ARR(ZCA), + KVM_ISA_EXT_ARR(ZCB), + KVM_ISA_EXT_ARR(ZCD), + KVM_ISA_EXT_ARR(ZCF), KVM_ISA_EXT_ARR(ZFA), KVM_ISA_EXT_ARR(ZFH), KVM_ISA_EXT_ARR(ZFHMIN), @@ -945,6 +953,10 @@ KVM_ISA_EXT_SIMPLE_CONFIG(zbkb, ZBKB); KVM_ISA_EXT_SIMPLE_CONFIG(zbkc, ZBKC); KVM_ISA_EXT_SIMPLE_CONFIG(zbkx, ZBKX); KVM_ISA_EXT_SIMPLE_CONFIG(zbs, ZBS); +KVM_ISA_EXT_SIMPLE_CONFIG(zca, ZCA), +KVM_ISA_EXT_SIMPLE_CONFIG(zcb, ZCB), +KVM_ISA_EXT_SIMPLE_CONFIG(zcd, ZCD), +KVM_ISA_EXT_SIMPLE_CONFIG(zcf, ZCF), KVM_ISA_EXT_SIMPLE_CONFIG(zfa, ZFA); KVM_ISA_EXT_SIMPLE_CONFIG(zfh, ZFH); KVM_ISA_EXT_SIMPLE_CONFIG(zfhmin, ZFHMIN); @@ -1001,6 +1013,10 @@ struct vcpu_reg_list *vcpu_configs[] = { &config_zbkc, &config_zbkx, &config_zbs, + &config_zca, + &config_zcb, + &config_zcd, + &config_zcf, &config_zfa, &config_zfh, &config_zfhmin,
Add description for the Zcmop (Compressed May-Be-Operations) ISA extension which was ratified in commit c732a4f39a4 ("Zcmop is ratified/1.0") of the riscv-isa-manual.
Signed-off-by: Clément Léger cleger@rivosinc.com --- Documentation/devicetree/bindings/riscv/extensions.yaml | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml index 516f57bdfeeb..902800b6dfe1 100644 --- a/Documentation/devicetree/bindings/riscv/extensions.yaml +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml @@ -252,6 +252,11 @@ properties: merged in the riscv-isa-manual by commit dbc79cf28a2 ("Initial seed of zc.adoc to src tree.").
+ - const: zcmop + description: + The standard Zcmop extension version 1.0, as ratified in commit + c732a4f39a4 ("Zcmop is ratified/1.0") of the riscv-isa-manual. + - const: zfa description: The standard Zfa extension for additional floating point
Add parsing for Zcmop ISA extension which was ratified in commit b854a709c00 ("Zcmop is ratified/1.0") of the riscv-isa-manual.
Signed-off-by: Clément Léger cleger@rivosinc.com --- arch/riscv/include/asm/hwcap.h | 1 + arch/riscv/kernel/cpufeature.c | 1 + 2 files changed, 2 insertions(+)
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index b7551bad341b..cff7660de268 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -86,6 +86,7 @@ #define RISCV_ISA_EXT_ZCB 77 #define RISCV_ISA_EXT_ZCD 78 #define RISCV_ISA_EXT_ZCF 79 +#define RISCV_ISA_EXT_ZCMOP 80
#define RISCV_ISA_EXT_XLINUXENVCFG 127
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 09dee071274d..f1450cd7231e 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -265,6 +265,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = { __RISCV_ISA_EXT_DATA(zcb, RISCV_ISA_EXT_ZCB), __RISCV_ISA_EXT_DATA(zcd, RISCV_ISA_EXT_ZCD), __RISCV_ISA_EXT_DATA(zcf, RISCV_ISA_EXT_ZCF), + __RISCV_ISA_EXT_DATA(zcmop, RISCV_ISA_EXT_ZCMOP), __RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA), __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB), __RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC),
On Wed, Apr 10, 2024 at 11:11:00AM +0200, Clément Léger wrote:
Add parsing for Zcmop ISA extension which was ratified in commit b854a709c00 ("Zcmop is ratified/1.0") of the riscv-isa-manual.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/include/asm/hwcap.h | 1 + arch/riscv/kernel/cpufeature.c | 1 + 2 files changed, 2 insertions(+)
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index b7551bad341b..cff7660de268 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -86,6 +86,7 @@ #define RISCV_ISA_EXT_ZCB 77 #define RISCV_ISA_EXT_ZCD 78 #define RISCV_ISA_EXT_ZCF 79 +#define RISCV_ISA_EXT_ZCMOP 80
#define RISCV_ISA_EXT_XLINUXENVCFG 127
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 09dee071274d..f1450cd7231e 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -265,6 +265,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = { __RISCV_ISA_EXT_DATA(zcb, RISCV_ISA_EXT_ZCB), __RISCV_ISA_EXT_DATA(zcd, RISCV_ISA_EXT_ZCD), __RISCV_ISA_EXT_DATA(zcf, RISCV_ISA_EXT_ZCF),
- __RISCV_ISA_EXT_DATA(zcmop, RISCV_ISA_EXT_ZCMOP),
As per spec zcmop is dependent on zca. So perhaps below ?
__RISCV_ISA_EXT_SUPERSET(zicboz, RISCV_ISA_EXT_ZCMOP, RISCV_ISA_EXT_ZCA)
__RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA), __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB), __RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC), -- 2.43.0
On Wed, Apr 10, 2024 at 02:32:41PM -0700, Deepak Gupta wrote:
On Wed, Apr 10, 2024 at 11:11:00AM +0200, Clément Léger wrote:
Add parsing for Zcmop ISA extension which was ratified in commit b854a709c00 ("Zcmop is ratified/1.0") of the riscv-isa-manual.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/include/asm/hwcap.h | 1 + arch/riscv/kernel/cpufeature.c | 1 + 2 files changed, 2 insertions(+)
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index b7551bad341b..cff7660de268 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -86,6 +86,7 @@ #define RISCV_ISA_EXT_ZCB 77 #define RISCV_ISA_EXT_ZCD 78 #define RISCV_ISA_EXT_ZCF 79 +#define RISCV_ISA_EXT_ZCMOP 80
#define RISCV_ISA_EXT_XLINUXENVCFG 127
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 09dee071274d..f1450cd7231e 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -265,6 +265,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = { __RISCV_ISA_EXT_DATA(zcb, RISCV_ISA_EXT_ZCB), __RISCV_ISA_EXT_DATA(zcd, RISCV_ISA_EXT_ZCD), __RISCV_ISA_EXT_DATA(zcf, RISCV_ISA_EXT_ZCF),
- __RISCV_ISA_EXT_DATA(zcmop, RISCV_ISA_EXT_ZCMOP),
As per spec zcmop is dependent on zca. So perhaps below ?
__RISCV_ISA_EXT_SUPERSET(zicboz, RISCV_ISA_EXT_ZCMOP, RISCV_ISA_EXT_ZCA)
What's zicboz got to do with it, copy-pasto I guess? If we're gonna imply stuff like this though I think we need some comments explaining why it's okay.
__RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA), __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB), __RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC), -- 2.43.0
On Wed, Apr 10, 2024 at 11:16:11PM +0100, Conor Dooley wrote:
On Wed, Apr 10, 2024 at 02:32:41PM -0700, Deepak Gupta wrote:
On Wed, Apr 10, 2024 at 11:11:00AM +0200, Clément Léger wrote:
Add parsing for Zcmop ISA extension which was ratified in commit b854a709c00 ("Zcmop is ratified/1.0") of the riscv-isa-manual.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/include/asm/hwcap.h | 1 + arch/riscv/kernel/cpufeature.c | 1 + 2 files changed, 2 insertions(+)
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index b7551bad341b..cff7660de268 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -86,6 +86,7 @@ #define RISCV_ISA_EXT_ZCB 77 #define RISCV_ISA_EXT_ZCD 78 #define RISCV_ISA_EXT_ZCF 79 +#define RISCV_ISA_EXT_ZCMOP 80
#define RISCV_ISA_EXT_XLINUXENVCFG 127
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 09dee071274d..f1450cd7231e 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -265,6 +265,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = { __RISCV_ISA_EXT_DATA(zcb, RISCV_ISA_EXT_ZCB), __RISCV_ISA_EXT_DATA(zcd, RISCV_ISA_EXT_ZCD), __RISCV_ISA_EXT_DATA(zcf, RISCV_ISA_EXT_ZCF),
- __RISCV_ISA_EXT_DATA(zcmop, RISCV_ISA_EXT_ZCMOP),
As per spec zcmop is dependent on zca. So perhaps below ?
__RISCV_ISA_EXT_SUPERSET(zicboz, RISCV_ISA_EXT_ZCMOP, RISCV_ISA_EXT_ZCA)
What's zicboz got to do with it, copy-pasto I guess? If we're gonna imply stuff like this though I think we need some comments explaining why it's okay.
Also, I'm inclined to call that out specifically in the binding, I've not yet checked if dependencies actually work for elements of a string array like the do for individual properties. I'll todo list that..
On Wed, Apr 10, 2024 at 11:27:16PM +0100, Conor Dooley wrote:
On Wed, Apr 10, 2024 at 11:16:11PM +0100, Conor Dooley wrote:
On Wed, Apr 10, 2024 at 02:32:41PM -0700, Deepak Gupta wrote:
On Wed, Apr 10, 2024 at 11:11:00AM +0200, Clément Léger wrote:
Add parsing for Zcmop ISA extension which was ratified in commit b854a709c00 ("Zcmop is ratified/1.0") of the riscv-isa-manual.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/include/asm/hwcap.h | 1 + arch/riscv/kernel/cpufeature.c | 1 + 2 files changed, 2 insertions(+)
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index b7551bad341b..cff7660de268 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -86,6 +86,7 @@ #define RISCV_ISA_EXT_ZCB 77 #define RISCV_ISA_EXT_ZCD 78 #define RISCV_ISA_EXT_ZCF 79 +#define RISCV_ISA_EXT_ZCMOP 80
#define RISCV_ISA_EXT_XLINUXENVCFG 127
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 09dee071274d..f1450cd7231e 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -265,6 +265,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = { __RISCV_ISA_EXT_DATA(zcb, RISCV_ISA_EXT_ZCB), __RISCV_ISA_EXT_DATA(zcd, RISCV_ISA_EXT_ZCD), __RISCV_ISA_EXT_DATA(zcf, RISCV_ISA_EXT_ZCF),
- __RISCV_ISA_EXT_DATA(zcmop, RISCV_ISA_EXT_ZCMOP),
As per spec zcmop is dependent on zca. So perhaps below ?
__RISCV_ISA_EXT_SUPERSET(zicboz, RISCV_ISA_EXT_ZCMOP, RISCV_ISA_EXT_ZCA)
What's zicboz got to do with it, copy-pasto I guess?
Yes, copy-pasta :-)
If we're gonna imply stuff like this though I think we need some comments explaining why it's okay.
Also, I'm inclined to call that out specifically in the binding, I've not yet checked if dependencies actually work for elements of a string array like the do for individual properties. I'll todo list that..
Earlier examples of specifying dependency on envcfg actually had functional use case. So you are right, I am not sure if its actually needed in this particular case.
And yes definitley, dependency should be mentioned in binding.
On 11/04/2024 00:32, Deepak Gupta wrote:
On Wed, Apr 10, 2024 at 11:27:16PM +0100, Conor Dooley wrote:
On Wed, Apr 10, 2024 at 11:16:11PM +0100, Conor Dooley wrote:
On Wed, Apr 10, 2024 at 02:32:41PM -0700, Deepak Gupta wrote:
On Wed, Apr 10, 2024 at 11:11:00AM +0200, Clément Léger wrote:
Add parsing for Zcmop ISA extension which was ratified in commit b854a709c00 ("Zcmop is ratified/1.0") of the riscv-isa-manual.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/include/asm/hwcap.h | 1 + arch/riscv/kernel/cpufeature.c | 1 + 2 files changed, 2 insertions(+)
diff --git a/arch/riscv/include/asm/hwcap.h
b/arch/riscv/include/asm/hwcap.h
index b7551bad341b..cff7660de268 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -86,6 +86,7 @@ #define RISCV_ISA_EXT_ZCB 77 #define RISCV_ISA_EXT_ZCD 78 #define RISCV_ISA_EXT_ZCF 79 +#define RISCV_ISA_EXT_ZCMOP 80
#define RISCV_ISA_EXT_XLINUXENVCFG 127
diff --git a/arch/riscv/kernel/cpufeature.c
b/arch/riscv/kernel/cpufeature.c
index 09dee071274d..f1450cd7231e 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -265,6 +265,7 @@ const struct riscv_isa_ext_data
riscv_isa_ext[] = {
__RISCV_ISA_EXT_DATA(zcb, RISCV_ISA_EXT_ZCB), __RISCV_ISA_EXT_DATA(zcd, RISCV_ISA_EXT_ZCD), __RISCV_ISA_EXT_DATA(zcf, RISCV_ISA_EXT_ZCF), + __RISCV_ISA_EXT_DATA(zcmop, RISCV_ISA_EXT_ZCMOP),
As per spec zcmop is dependent on zca. So perhaps below ?
__RISCV_ISA_EXT_SUPERSET(zicboz, RISCV_ISA_EXT_ZCMOP,
RISCV_ISA_EXT_ZCA)
What's zicboz got to do with it, copy-pasto I guess?
Yes, copy-pasta :-)
If we're gonna imply stuff like this though I think we need some comments explaining why it's okay.
Also, I'm inclined to call that out specifically in the binding, I've not yet checked if dependencies actually work for elements of a string array like the do for individual properties. I'll todo list that..
Earlier examples of specifying dependency on envcfg actually had functional use case. So you are right, I am not sure if its actually needed in this particular case.
I actually saw that and think about addressing it but AFAICT, this should be handled by the machine firmware passing the isa string to the kernel (ie, it should be valid). In the case of QEMU, it takes care of setting the extension that are required by this extension itself.
If we consider to have potentially broken isa string (ie extensions dependencies not correctly handled), then we'll need some way to validate this within the kernel.
Clément
And yes definitley, dependency should be mentioned in binding.
On Thu, Apr 11, 2024 at 09:25:06AM +0200, Clément Léger wrote:
On 11/04/2024 00:32, Deepak Gupta wrote:
On Wed, Apr 10, 2024 at 11:27:16PM +0100, Conor Dooley wrote:
On Wed, Apr 10, 2024 at 11:16:11PM +0100, Conor Dooley wrote:
On Wed, Apr 10, 2024 at 02:32:41PM -0700, Deepak Gupta wrote:
On Wed, Apr 10, 2024 at 11:11:00AM +0200, Clément Léger wrote:
Add parsing for Zcmop ISA extension which was ratified in commit b854a709c00 ("Zcmop is ratified/1.0") of the riscv-isa-manual.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/include/asm/hwcap.h | 1 + arch/riscv/kernel/cpufeature.c | 1 + 2 files changed, 2 insertions(+)
diff --git a/arch/riscv/include/asm/hwcap.h
b/arch/riscv/include/asm/hwcap.h
index b7551bad341b..cff7660de268 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -86,6 +86,7 @@ #define RISCV_ISA_EXT_ZCB 77 #define RISCV_ISA_EXT_ZCD 78 #define RISCV_ISA_EXT_ZCF 79 +#define RISCV_ISA_EXT_ZCMOP 80
#define RISCV_ISA_EXT_XLINUXENVCFG 127
diff --git a/arch/riscv/kernel/cpufeature.c
b/arch/riscv/kernel/cpufeature.c
index 09dee071274d..f1450cd7231e 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -265,6 +265,7 @@ const struct riscv_isa_ext_data
riscv_isa_ext[] = {
__RISCV_ISA_EXT_DATA(zcb, RISCV_ISA_EXT_ZCB), __RISCV_ISA_EXT_DATA(zcd, RISCV_ISA_EXT_ZCD), __RISCV_ISA_EXT_DATA(zcf, RISCV_ISA_EXT_ZCF), + __RISCV_ISA_EXT_DATA(zcmop, RISCV_ISA_EXT_ZCMOP),
As per spec zcmop is dependent on zca. So perhaps below ?
__RISCV_ISA_EXT_SUPERSET(zicboz, RISCV_ISA_EXT_ZCMOP,
RISCV_ISA_EXT_ZCA)
What's zicboz got to do with it, copy-pasto I guess?
Yes, copy-pasta :-)
If we're gonna imply stuff like this though I think we need some comments explaining why it's okay.
Also, I'm inclined to call that out specifically in the binding, I've not yet checked if dependencies actually work for elements of a string array like the do for individual properties. I'll todo list that..
Earlier examples of specifying dependency on envcfg actually had functional use case. So you are right, I am not sure if its actually needed in this particular case.
I actually saw that and think about addressing it but AFAICT, this should be handled by the machine firmware passing the isa string to the kernel (ie, it should be valid). In the case of QEMU, it takes care of setting the extension that are required by this extension itself.
If we consider to have potentially broken isa string (ie extensions dependencies not correctly handled), then we'll need some way to validate this within the kernel.
No, the DT passed to the kernel should be correct and we by and large we should not have to do validation of it. What I meant above was writing the binding so that something invalid will not pass dtbs_check.
On 11/04/2024 11:03, Conor Dooley wrote:
On Thu, Apr 11, 2024 at 09:25:06AM +0200, Clément Léger wrote:
On 11/04/2024 00:32, Deepak Gupta wrote:
On Wed, Apr 10, 2024 at 11:27:16PM +0100, Conor Dooley wrote:
On Wed, Apr 10, 2024 at 11:16:11PM +0100, Conor Dooley wrote:
On Wed, Apr 10, 2024 at 02:32:41PM -0700, Deepak Gupta wrote:
On Wed, Apr 10, 2024 at 11:11:00AM +0200, Clément Léger wrote: > Add parsing for Zcmop ISA extension which was ratified in commit > b854a709c00 ("Zcmop is ratified/1.0") of the riscv-isa-manual. > > Signed-off-by: Clément Léger cleger@rivosinc.com > --- > arch/riscv/include/asm/hwcap.h | 1 + > arch/riscv/kernel/cpufeature.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/arch/riscv/include/asm/hwcap.h
b/arch/riscv/include/asm/hwcap.h
> index b7551bad341b..cff7660de268 100644 > --- a/arch/riscv/include/asm/hwcap.h > +++ b/arch/riscv/include/asm/hwcap.h > @@ -86,6 +86,7 @@ > #define RISCV_ISA_EXT_ZCB 77 > #define RISCV_ISA_EXT_ZCD 78 > #define RISCV_ISA_EXT_ZCF 79 > +#define RISCV_ISA_EXT_ZCMOP 80 > > #define RISCV_ISA_EXT_XLINUXENVCFG 127 > > diff --git a/arch/riscv/kernel/cpufeature.c
b/arch/riscv/kernel/cpufeature.c
> index 09dee071274d..f1450cd7231e 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -265,6 +265,7 @@ const struct riscv_isa_ext_data
riscv_isa_ext[] = {
> __RISCV_ISA_EXT_DATA(zcb, RISCV_ISA_EXT_ZCB), > __RISCV_ISA_EXT_DATA(zcd, RISCV_ISA_EXT_ZCD), > __RISCV_ISA_EXT_DATA(zcf, RISCV_ISA_EXT_ZCF), > + __RISCV_ISA_EXT_DATA(zcmop, RISCV_ISA_EXT_ZCMOP),
As per spec zcmop is dependent on zca. So perhaps below ?
__RISCV_ISA_EXT_SUPERSET(zicboz, RISCV_ISA_EXT_ZCMOP,
RISCV_ISA_EXT_ZCA)
What's zicboz got to do with it, copy-pasto I guess?
Yes, copy-pasta :-)
If we're gonna imply stuff like this though I think we need some comments explaining why it's okay.
Also, I'm inclined to call that out specifically in the binding, I've not yet checked if dependencies actually work for elements of a string array like the do for individual properties. I'll todo list that..
Earlier examples of specifying dependency on envcfg actually had functional use case. So you are right, I am not sure if its actually needed in this particular case.
I actually saw that and think about addressing it but AFAICT, this should be handled by the machine firmware passing the isa string to the kernel (ie, it should be valid). In the case of QEMU, it takes care of setting the extension that are required by this extension itself.
If we consider to have potentially broken isa string (ie extensions dependencies not correctly handled), then we'll need some way to validate this within the kernel.
No, the DT passed to the kernel should be correct and we by and large we should not have to do validation of it. What I meant above was writing the binding so that something invalid will not pass dtbs_check.
Acked, I was mainly answering Deepak question about dependencies wrt to using __RISCV_ISA_EXT_SUPERSET() which does not seems to be relevant since we expect a correct isa string to be passed. But as you stated, DT validation clearly make sense. I think a lot of extensions strings would benefit such support (All the Zv* depends on V, etc).
Clément
On Thu, Apr 11, 2024 at 11:08:21AM +0200, Clément Léger wrote:
If we consider to have potentially broken isa string (ie extensions dependencies not correctly handled), then we'll need some way to validate this within the kernel.
No, the DT passed to the kernel should be correct and we by and large we should not have to do validation of it. What I meant above was writing the binding so that something invalid will not pass dtbs_check.
Acked, I was mainly answering Deepak question about dependencies wrt to using __RISCV_ISA_EXT_SUPERSET() which does not seems to be relevant since we expect a correct isa string to be passed.
Ahh, okay.
But as you stated, DT validation clearly make sense. I think a lot of extensions strings would benefit such support (All the Zv* depends on V, etc).
I think it is actually as simple something like this, which makes it invalid to have "d" without "f":
| diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml | index 468c646247aa..594828700cbe 100644 | --- a/Documentation/devicetree/bindings/riscv/extensions.yaml | +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml | @@ -484,5 +484,20 @@ properties: | Registers in the AX45MP datasheet. | https://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.... | | +allOf: | + - if: | + properties: | + riscv,isa-extensions: | + contains: | + const: "d" | + not: | + contains: | + const: "f" | + then: | + properties: | + riscv,isa-extensions: | + false | + | + | additionalProperties: true | ...
If you do have d without f, the checker will say: cpu@2: riscv,isa-extensions: False schema does not allow ['i', 'm', 'a', 'd', 'c']
At least that's readable, even though not clear about what to do. I wish the former could be said about the wall of text you get for /each/ undocumented entry in the string.
On 11/04/2024 13:53, Conor Dooley wrote:
On Thu, Apr 11, 2024 at 11:08:21AM +0200, Clément Léger wrote:
If we consider to have potentially broken isa string (ie extensions dependencies not correctly handled), then we'll need some way to validate this within the kernel.
No, the DT passed to the kernel should be correct and we by and large we should not have to do validation of it. What I meant above was writing the binding so that something invalid will not pass dtbs_check.
Acked, I was mainly answering Deepak question about dependencies wrt to using __RISCV_ISA_EXT_SUPERSET() which does not seems to be relevant since we expect a correct isa string to be passed.
Ahh, okay.
But as you stated, DT validation clearly make sense. I think a lot of extensions strings would benefit such support (All the Zv* depends on V, etc).
I think it is actually as simple something like this, which makes it invalid to have "d" without "f":
| diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml | index 468c646247aa..594828700cbe 100644 | --- a/Documentation/devicetree/bindings/riscv/extensions.yaml | +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml | @@ -484,5 +484,20 @@ properties: | Registers in the AX45MP datasheet. | https://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.... | | +allOf: | + - if: | + properties: | + riscv,isa-extensions: | + contains: | + const: "d" | + not: | + contains: | + const: "f" | + then: | + properties: | + riscv,isa-extensions: | + false | + | + | additionalProperties: true | ...
If you do have d without f, the checker will say: cpu@2: riscv,isa-extensions: False schema does not allow ['i', 'm', 'a', 'd', 'c']
At least that's readable, even though not clear about what to do. I wish
That looks really readable indeed but the messages that result from errors are not so informative.
It tried playing with various constructs and found this one to yield a comprehensive message:
+allOf: + - if: + properties: + riscv,isa-extensions: + contains: + const: zcf + not: + contains: + const: zca + then: + properties: + riscv,isa-extensions: + items: + anyOf: + - const: zca
arch/riscv/boot/dts/allwinner/sun20i-d1-dongshan-nezha-stu.dtb: cpu@0: riscv,isa-extensions:10: 'anyOf' conditional failed, one must be fixed: 'zca' was expected from schema $id: http://devicetree.org/schemas/riscv/extensions.yaml
Even though dt-bindings-check passed, not sure if this is totally a valid construct though...
Thanks,
Clément
the former could be said about the wall of text you get for /each/ undocumented entry in the string.
On Mon, Apr 15, 2024 at 11:10:24AM +0200, Clément Léger wrote:
On 11/04/2024 13:53, Conor Dooley wrote:
On Thu, Apr 11, 2024 at 11:08:21AM +0200, Clément Léger wrote:
If we consider to have potentially broken isa string (ie extensions dependencies not correctly handled), then we'll need some way to validate this within the kernel.
No, the DT passed to the kernel should be correct and we by and large we should not have to do validation of it. What I meant above was writing the binding so that something invalid will not pass dtbs_check.
Acked, I was mainly answering Deepak question about dependencies wrt to using __RISCV_ISA_EXT_SUPERSET() which does not seems to be relevant since we expect a correct isa string to be passed.
Ahh, okay.
But as you stated, DT validation clearly make sense. I think a lot of extensions strings would benefit such support (All the Zv* depends on V, etc).
I think it is actually as simple something like this, which makes it invalid to have "d" without "f":
| diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml | index 468c646247aa..594828700cbe 100644 | --- a/Documentation/devicetree/bindings/riscv/extensions.yaml | +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml | @@ -484,5 +484,20 @@ properties: | Registers in the AX45MP datasheet. | https://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.... | | +allOf: | + - if: | + properties: | + riscv,isa-extensions: | + contains: | + const: "d" | + not: | + contains: | + const: "f" | + then: | + properties: | + riscv,isa-extensions: | + false | + | + | additionalProperties: true | ...
If you do have d without f, the checker will say: cpu@2: riscv,isa-extensions: False schema does not allow ['i', 'm', 'a', 'd', 'c']
At least that's readable, even though not clear about what to do. I wish
That looks really readable indeed but the messages that result from errors are not so informative.
It tried playing with various constructs and found this one to yield a comprehensive message:
+allOf:
- if:
properties:riscv,isa-extensions:contains:const: zcfnot:contains:const: zca- then:
properties:riscv,isa-extensions:items:anyOf:- const: zcaarch/riscv/boot/dts/allwinner/sun20i-d1-dongshan-nezha-stu.dtb: cpu@0: riscv,isa-extensions:10: 'anyOf' conditional failed, one must be fixed: 'zca' was expected from schema $id: http://devicetree.org/schemas/riscv/extensions.yaml
Even though dt-bindings-check passed, not sure if this is totally a valid construct though...
I asked Rob about this yesterday, he suggested adding: riscv,isa-extensions: if: contains: const: zcf then: contains: const: zca to the existing property, not in an allOf. I think that is by far the most readable version in terms of what goes into the binding. The output would look like: cpu@0: riscv,isa-extensions: ['i', 'm', 'a', 'd', 'c'] does not contain items matching the given schema (for d requiring f cos I am lazy)
Also, his comment about your one that gives the nice message was that it would wrong as the anyOf was pointless and it says all items must be "zca". I didn't try it, but I have a feeling your nice output will be rather less nice if several different deps are unmet - but hey, probably will still be better than having an undocumented extension!
On 16/04/2024 16:54, Conor Dooley wrote:
On Mon, Apr 15, 2024 at 11:10:24AM +0200, Clément Léger wrote:
On 11/04/2024 13:53, Conor Dooley wrote:
On Thu, Apr 11, 2024 at 11:08:21AM +0200, Clément Léger wrote:
If we consider to have potentially broken isa string (ie extensions dependencies not correctly handled), then we'll need some way to validate this within the kernel.
No, the DT passed to the kernel should be correct and we by and large we should not have to do validation of it. What I meant above was writing the binding so that something invalid will not pass dtbs_check.
Acked, I was mainly answering Deepak question about dependencies wrt to using __RISCV_ISA_EXT_SUPERSET() which does not seems to be relevant since we expect a correct isa string to be passed.
Ahh, okay.
But as you stated, DT validation clearly make sense. I think a lot of extensions strings would benefit such support (All the Zv* depends on V, etc).
I think it is actually as simple something like this, which makes it invalid to have "d" without "f":
| diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml | index 468c646247aa..594828700cbe 100644 | --- a/Documentation/devicetree/bindings/riscv/extensions.yaml | +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml | @@ -484,5 +484,20 @@ properties: | Registers in the AX45MP datasheet. | https://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.... | | +allOf: | + - if: | + properties: | + riscv,isa-extensions: | + contains: | + const: "d" | + not: | + contains: | + const: "f" | + then: | + properties: | + riscv,isa-extensions: | + false | + | + | additionalProperties: true | ...
If you do have d without f, the checker will say: cpu@2: riscv,isa-extensions: False schema does not allow ['i', 'm', 'a', 'd', 'c']
At least that's readable, even though not clear about what to do. I wish
That looks really readable indeed but the messages that result from errors are not so informative.
It tried playing with various constructs and found this one to yield a comprehensive message:
+allOf:
- if:
properties:riscv,isa-extensions:contains:const: zcfnot:contains:const: zca- then:
properties:riscv,isa-extensions:items:anyOf:- const: zcaarch/riscv/boot/dts/allwinner/sun20i-d1-dongshan-nezha-stu.dtb: cpu@0: riscv,isa-extensions:10: 'anyOf' conditional failed, one must be fixed: 'zca' was expected from schema $id: http://devicetree.org/schemas/riscv/extensions.yaml
Even though dt-bindings-check passed, not sure if this is totally a valid construct though...
I asked Rob about this yesterday, he suggested adding: riscv,isa-extensions: if: contains: const: zcf then: contains: const: zca
That is way more readable and concise !
to the existing property, not in an allOf. I think that is by far the most readable version in terms of what goes into the binding. The output would look like: cpu@0: riscv,isa-extensions: ['i', 'm', 'a', 'd', 'c'] does not contain items matching the given schema (for d requiring f cos I am lazy)
Than fine by me. The error is at least a bit more understandable than the one with the false schema ;)
Also, his comment about your one that gives the nice message was that it would wrong as the anyOf was pointless and it says all items must be "zca".
That's what I understood also.
I didn't try it, but I have a feeling your nice output will be rather less nice if several different deps are unmet - but hey, probably will still be better than having an undocumented extension!
If you are ok with that, let's go with Rob suggestion. I'll resubmit a V2 with validation for these extensions and probably a followup for the other ones lacking dependency checking.
Thanks,
Clément
On Tue, Apr 16, 2024 at 05:23:51PM +0200, Clément Léger wrote:
On 16/04/2024 16:54, Conor Dooley wrote:
On Mon, Apr 15, 2024 at 11:10:24AM +0200, Clément Léger wrote:
On 11/04/2024 13:53, Conor Dooley wrote:
On Thu, Apr 11, 2024 at 11:08:21AM +0200, Clément Léger wrote:
> If we consider to have potentially broken isa string (ie extensions > dependencies not correctly handled), then we'll need some way to > validate this within the kernel.
No, the DT passed to the kernel should be correct and we by and large we should not have to do validation of it. What I meant above was writing the binding so that something invalid will not pass dtbs_check.
Acked, I was mainly answering Deepak question about dependencies wrt to using __RISCV_ISA_EXT_SUPERSET() which does not seems to be relevant since we expect a correct isa string to be passed.
Ahh, okay.
But as you stated, DT validation clearly make sense. I think a lot of extensions strings would benefit such support (All the Zv* depends on V, etc).
I think it is actually as simple something like this, which makes it invalid to have "d" without "f":
| diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml | index 468c646247aa..594828700cbe 100644 | --- a/Documentation/devicetree/bindings/riscv/extensions.yaml | +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml | @@ -484,5 +484,20 @@ properties: | Registers in the AX45MP datasheet. | https://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.... | | +allOf: | + - if: | + properties: | + riscv,isa-extensions: | + contains: | + const: "d" | + not: | + contains: | + const: "f" | + then: | + properties: | + riscv,isa-extensions: | + false | + | + | additionalProperties: true | ...
If you do have d without f, the checker will say: cpu@2: riscv,isa-extensions: False schema does not allow ['i', 'm', 'a', 'd', 'c']
At least that's readable, even though not clear about what to do. I wish
That looks really readable indeed but the messages that result from errors are not so informative.
It tried playing with various constructs and found this one to yield a comprehensive message:
+allOf:
- if:
properties:riscv,isa-extensions:contains:const: zcfnot:contains:const: zca- then:
properties:riscv,isa-extensions:items:anyOf:- const: zcaarch/riscv/boot/dts/allwinner/sun20i-d1-dongshan-nezha-stu.dtb: cpu@0: riscv,isa-extensions:10: 'anyOf' conditional failed, one must be fixed: 'zca' was expected from schema $id: http://devicetree.org/schemas/riscv/extensions.yaml
Even though dt-bindings-check passed, not sure if this is totally a valid construct though...
I asked Rob about this yesterday, he suggested adding: riscv,isa-extensions: if: contains: const: zcf then: contains: const: zca
That is way more readable and concise !
to the existing property, not in an allOf. I think that is by far the most readable version in terms of what goes into the binding. The output would look like: cpu@0: riscv,isa-extensions: ['i', 'm', 'a', 'd', 'c'] does not contain items matching the given schema (for d requiring f cos I am lazy)
Than fine by me. The error is at least a bit more understandable than the one with the false schema ;)
Also, his comment about your one that gives the nice message was that it would wrong as the anyOf was pointless and it says all items must be "zca".
That's what I understood also.
I didn't try it, but I have a feeling your nice output will be rather less nice if several different deps are unmet - but hey, probably will still be better than having an undocumented extension!
If you are ok with that, let's go with Rob suggestion. I'll resubmit a V2 with validation for these extensions and probably a followup for the other ones lacking dependency checking.
Also, Rob made some modifications to dt-schema yesterday, so now the report about an undocumented extension looks like: cpu@1: riscv,isa-extensions:3: '0' is not one of ['i', 'm', 'a', 'f', 'd', 'q', 'c', 'v', 'h', 'smaia', 'smstateen', 'ssaia', 'sscofpmf', 'sstc', 'svinval', 'svnapot', 'svpbmt', 'zacas', 'zba', 'zbb', 'zbc', 'zbkb', 'zbkc', 'zbkx', 'zbs', 'zfa', 'zfh', 'zfhmin', 'zk', 'zkn', 'zknd', 'zkne', 'zknh', 'zkr', 'zks', 'zksed', 'zksh', 'zkt', 'zicbom', 'zicbop', 'zicboz', 'zicntr', 'zicond', 'zicsr', 'zifencei', 'zihintpause', 'zihintntl', 'zihpm', 'ztso', 'zvbb', 'zvbc', 'zvfh', 'zvfhmin', 'zvkb', 'zvkg', 'zvkn', 'zvknc', 'zvkned', 'zvkng', 'zvknha', 'zvknhb', 'zvks', 'zvksc', 'zvksed', 'zvksh', 'zvksg', 'zvkt', 'xandespmu'] instead of cpu@0: riscv,isa-extensions:4: 'anyOf' conditional failed, one must be fixed: 'i' was expected 'm' was expected 'a' was expected 'f' was expected 'd' was expected 'q' was expected 'c' was expected 'v' was expected 'h' was expected 'smaia' was expected 'smstateen' was expected 'ssaia' was expected 'sscofpmf' was expected 'sstc' was expected 'svinval' was expected 'svnapot' was expected 'svpbmt' was expected 'zacas' was expected 'zba' was expected 'zbb' was expected 'zbc' was expected 'zbkb' was expected 'zbkc' was expected 'zbkx' was expected 'zbs' was expected 'zfa' was expected 'zfh' was expected 'zfhmin' was expected 'zk' was expected 'zkn' was expected 'zknd' was expected 'zkne' was expected 'zknh' was expected 'zkr' was expected 'zks' was expected 'zksed' was expected 'zksh' was expected 'zkt' was expected 'zicbom' was expected 'zicbop' was expected 'zicboz' was expected 'zicntr' was expected 'zicond' was expected 'zicsr' was expected 'zifencei' was expected 'zihintpause' was expected 'zihintntl' was expected 'zihpm' was expected 'ztso' was expected 'zvbb' was expected 'zvbc' was expected 'zvfh' was expected 'zvfhmin' was expected 'zvkb' was expected 'zvkg' was expected 'zvkn' was expected 'zvknc' was expected 'zvkned' was expected 'zvkng' was expected 'zvknha' was expected 'zvknhb' was expected 'zvks' was expected 'zvksc' was expected 'zvksed' was expected 'zvksh' was expected 'zvksg' was expected 'zvkt' was expected 'xandespmu' was expected from schema $id: http://devicetree.org/schemas/riscv/cpus.yaml#
Which is really great from a readability pov. Not only is it compressed to a single line, it actually points out which extension is the offender.
Thanks, Conor.
Export Zcmop ISA extension through hwprobe.
Signed-off-by: Clément Léger cleger@rivosinc.com --- Documentation/arch/riscv/hwprobe.rst | 4 ++++ arch/riscv/include/uapi/asm/hwprobe.h | 1 + arch/riscv/kernel/sys_hwprobe.c | 1 + 3 files changed, 6 insertions(+)
diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst index bf96b4e8ba3b..e3187659a077 100644 --- a/Documentation/arch/riscv/hwprobe.rst +++ b/Documentation/arch/riscv/hwprobe.rst @@ -212,6 +212,10 @@ The following keys are defined: ("Zcf doesn't exist on RV64 as it contains no instructions") of riscv-code-size-reduction.
+ * :c:macro:`RISCV_HWPROBE_EXT_ZCMOP`: The Zcmop May-Be-Operations extension is + supported as defined in the RISC-V ISA manual starting from commit + c732a4f39a4 ("Zcmop is ratified/1.0"). + * :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains performance information about the selected set of processors.
diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h index dd4ad77faf49..d97ac5436447 100644 --- a/arch/riscv/include/uapi/asm/hwprobe.h +++ b/arch/riscv/include/uapi/asm/hwprobe.h @@ -64,6 +64,7 @@ struct riscv_hwprobe { #define RISCV_HWPROBE_EXT_ZCB (1ULL << 38) #define RISCV_HWPROBE_EXT_ZCD (1ULL << 39) #define RISCV_HWPROBE_EXT_ZCF (1ULL << 40) +#define RISCV_HWPROBE_EXT_ZCMOP (1ULL << 41) #define RISCV_HWPROBE_KEY_CPUPERF_0 5 #define RISCV_HWPROBE_MISALIGNED_UNKNOWN (0 << 0) #define RISCV_HWPROBE_MISALIGNED_EMULATED (1 << 0) diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c index 2ffa0fe5101e..9457231bd1c0 100644 --- a/arch/riscv/kernel/sys_hwprobe.c +++ b/arch/riscv/kernel/sys_hwprobe.c @@ -114,6 +114,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair, EXT_KEY(ZIMOP); EXT_KEY(ZCA); EXT_KEY(ZCB); + EXT_KEY(ZCMOP);
if (has_vector()) { EXT_KEY(ZVBB);
Extend the KVM ISA extension ONE_REG interface to allow KVM user space to detect and enable Zcmop extension for Guest/VM.
Signed-off-by: Clément Léger cleger@rivosinc.com --- arch/riscv/include/uapi/asm/kvm.h | 1 + arch/riscv/kvm/vcpu_onereg.c | 2 ++ 2 files changed, 3 insertions(+)
diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h index 57db3fea679f..0366389a0bae 100644 --- a/arch/riscv/include/uapi/asm/kvm.h +++ b/arch/riscv/include/uapi/asm/kvm.h @@ -172,6 +172,7 @@ enum KVM_RISCV_ISA_EXT_ID { KVM_RISCV_ISA_EXT_ZCB, KVM_RISCV_ISA_EXT_ZCD, KVM_RISCV_ISA_EXT_ZCF, + KVM_RISCV_ISA_EXT_ZCMOP, KVM_RISCV_ISA_EXT_MAX, };
diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c index 7d47fc910bd9..af4fefa189af 100644 --- a/arch/riscv/kvm/vcpu_onereg.c +++ b/arch/riscv/kvm/vcpu_onereg.c @@ -52,6 +52,7 @@ static const unsigned long kvm_isa_ext_arr[] = { KVM_ISA_EXT_ARR(ZCB), KVM_ISA_EXT_ARR(ZCD), KVM_ISA_EXT_ARR(ZCF), + KVM_ISA_EXT_ARR(ZCMOP), KVM_ISA_EXT_ARR(ZFA), KVM_ISA_EXT_ARR(ZFH), KVM_ISA_EXT_ARR(ZFHMIN), @@ -136,6 +137,7 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext) case KVM_RISCV_ISA_EXT_ZCB: case KVM_RISCV_ISA_EXT_ZCD: case KVM_RISCV_ISA_EXT_ZCF: + case KVM_RISCV_ISA_EXT_ZCMOP: case KVM_RISCV_ISA_EXT_ZFA: case KVM_RISCV_ISA_EXT_ZFH: case KVM_RISCV_ISA_EXT_ZFHMIN:
The KVM RISC-V allows Zcmop extension for Guest/VM so add this extension to get-reg-list test.
Signed-off-by: Clément Léger cleger@rivosinc.com --- tools/testing/selftests/kvm/riscv/get-reg-list.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c index 61cad4514197..9604c8ece787 100644 --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c @@ -59,6 +59,7 @@ bool filter_reg(__u64 reg) case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZCB: case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZCD: case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZCF: + case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZCMOP: case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZFA: case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZFH: case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZFHMIN: @@ -429,6 +430,7 @@ static const char *isa_ext_single_id_to_str(__u64 reg_off) KVM_ISA_EXT_ARR(ZCB), KVM_ISA_EXT_ARR(ZCD), KVM_ISA_EXT_ARR(ZCF), + KVM_ISA_EXT_ARR(ZCMOP), KVM_ISA_EXT_ARR(ZFA), KVM_ISA_EXT_ARR(ZFH), KVM_ISA_EXT_ARR(ZFHMIN), @@ -957,6 +959,7 @@ KVM_ISA_EXT_SIMPLE_CONFIG(zca, ZCA), KVM_ISA_EXT_SIMPLE_CONFIG(zcb, ZCB), KVM_ISA_EXT_SIMPLE_CONFIG(zcd, ZCD), KVM_ISA_EXT_SIMPLE_CONFIG(zcf, ZCF), +KVM_ISA_EXT_SIMPLE_CONFIG(zcmop, ZCMOP); KVM_ISA_EXT_SIMPLE_CONFIG(zfa, ZFA); KVM_ISA_EXT_SIMPLE_CONFIG(zfh, ZFH); KVM_ISA_EXT_SIMPLE_CONFIG(zfhmin, ZFHMIN); @@ -1017,6 +1020,7 @@ struct vcpu_reg_list *vcpu_configs[] = { &config_zcb, &config_zcd, &config_zcf, + &config_zcmop, &config_zfa, &config_zfh, &config_zfhmin,
For the series:
Reviewed-by: Deepak Gupta debug@rivosinc.com
On Wed, Apr 10, 2024 at 2:13 AM Clément Léger cleger@rivosinc.com wrote:
Add support for (yet again) more RVA23U64 missing extensions. Add support for Zcmop, Zca, Zcf, Zcd and Zcb extensions isa string parsing, hwprobe and kvm support. Zce, Zcmt and Zcmp extensions have been left out since they target microcontrollers/embedded CPUs and are not needed by RVA23U64
This series is based on the Zimop one [1].
Link: https://lore.kernel.org/linux-riscv/20240404103254.1752834-1-cleger@rivosinc... [1]
Clément Léger (10): dt-bindings: riscv: add Zca, Zcf, Zcd and Zcb ISA extension description riscv: add ISA parsing for Zca, Zcf, Zcd and Zcb riscv: hwprobe: export Zca, Zcf, Zcd and Zcb ISA extensions RISC-V: KVM: Allow Zca, Zcf, Zcd and Zcb extensions for Guest/VM KVM: riscv: selftests: Add some Zc* extensions to get-reg-list test dt-bindings: riscv: add Zcmop ISA extension description riscv: add ISA extension parsing for Zcmop riscv: hwprobe: export Zcmop ISA extension RISC-V: KVM: Allow Zcmop extension for Guest/VM KVM: riscv: selftests: Add Zcmop extension to get-reg-list test
Documentation/arch/riscv/hwprobe.rst | 24 ++++++++++++ .../devicetree/bindings/riscv/extensions.yaml | 37 +++++++++++++++++++ arch/riscv/include/asm/hwcap.h | 5 +++ arch/riscv/include/uapi/asm/hwprobe.h | 5 +++ arch/riscv/include/uapi/asm/kvm.h | 5 +++ arch/riscv/kernel/cpufeature.c | 5 +++ arch/riscv/kernel/sys_hwprobe.c | 5 +++ arch/riscv/kvm/vcpu_onereg.c | 10 +++++ .../selftests/kvm/riscv/get-reg-list.c | 20 ++++++++++ 9 files changed, 116 insertions(+)
-- 2.43.0
linux-kselftest-mirror@lists.linaro.org
-
Clément Léger -
Conor Dooley -
Conor Dooley -
Deepak Gupta