2024-04-10 09:13:40

by Clément Léger

[permalink] [raw]
Subject: [PATCH 00/10] Add support for a few Zc* extensions as well as Zcmop

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/[email protected]/ [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



2024-04-10 09:16:38

by Clément Léger

[permalink] [raw]
Subject: [PATCH 01/10] dt-bindings: riscv: add Zca, Zcf, Zcd and Zcb ISA extension description

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 <[email protected]>
---
.../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
--
2.43.0


2024-04-10 09:18:18

by Clément Léger

[permalink] [raw]
Subject: [PATCH 04/10] RISC-V: KVM: Allow Zca, Zcf, Zcd and Zcb extensions for Guest/VM

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 <[email protected]>
---
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:
--
2.43.0


2024-04-10 09:18:49

by Clément Léger

[permalink] [raw]
Subject: [PATCH 05/10] KVM: riscv: selftests: Add some Zc* extensions to get-reg-list test

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 <[email protected]>
---
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,
--
2.43.0


2024-04-10 09:22:32

by Clément Léger

[permalink] [raw]
Subject: [PATCH 09/10] RISC-V: KVM: Allow Zcmop extension for Guest/VM

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 <[email protected]>
---
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:
--
2.43.0


2024-04-10 09:22:37

by Clément Léger

[permalink] [raw]
Subject: [PATCH 08/10] riscv: hwprobe: export Zcmop ISA extension

Export Zcmop ISA extension through hwprobe.

Signed-off-by: Clément Léger <[email protected]>
---
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);
--
2.43.0


2024-04-10 09:23:21

by Clément Léger

[permalink] [raw]
Subject: [PATCH 02/10] riscv: add ISA parsing for Zca, Zcf, Zcd and Zcb

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 <[email protected]>
---
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),
--
2.43.0


2024-04-10 09:23:38

by Clément Léger

[permalink] [raw]
Subject: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop

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 <[email protected]>
---
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),
--
2.43.0


2024-04-10 09:24:54

by Clément Léger

[permalink] [raw]
Subject: [PATCH 06/10] dt-bindings: riscv: add Zcmop ISA extension description

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 <[email protected]>
---
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
--
2.43.0


2024-04-10 09:25:06

by Clément Léger

[permalink] [raw]
Subject: [PATCH 03/10] riscv: hwprobe: export Zca, Zcf, Zcd and Zcb ISA extensions

Export Zca, Zcf, Zcd and Zcb ISA extension through hwprobe.

Signed-off-by: Clément Léger <[email protected]>
---
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
}
--
2.43.0


2024-04-10 09:28:57

by Clément Léger

[permalink] [raw]
Subject: [PATCH 10/10] KVM: riscv: selftests: Add Zcmop extension to get-reg-list test

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 <[email protected]>
---
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,
--
2.43.0


2024-04-10 21:33:19

by Deepak Gupta

[permalink] [raw]
Subject: Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop

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 <[email protected]>
>---
> 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
>
>

2024-04-10 21:35:26

by Deepak Gupta

[permalink] [raw]
Subject: Re: [PATCH 00/10] Add support for a few Zc* extensions as well as Zcmop

For the series:

Reviewed-by: Deepak Gupta <[email protected]>

On Wed, Apr 10, 2024 at 2:13 AM Clément Léger <[email protected]> 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/[email protected]/ [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
>
>

2024-04-10 22:16:39

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop

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 <[email protected]>
> > ---
> > 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
> >
> >


Attachments:
(No filename) (1.85 kB)
signature.asc (235.00 B)
Download all attachments

2024-04-10 22:29:58

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop

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 <[email protected]>
> > > ---
> > > 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..


Attachments:
(No filename) (2.01 kB)
signature.asc (235.00 B)
Download all attachments

2024-04-10 22:34:05

by Deepak Gupta

[permalink] [raw]
Subject: Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop

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 <[email protected]>
>> > > ---
>> > > 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.



2024-04-11 07:44:04

by Clément Léger

[permalink] [raw]
Subject: Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop



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 <[email protected]>
>>> > > ---
>>> > > 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.
>
>

2024-04-11 09:04:31

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop

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 <[email protected]>
> >>> > > ---
> >>> > > 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.


Attachments:
(No filename) (3.32 kB)
signature.asc (235.00 B)
Download all attachments

2024-04-11 09:28:53

by Clément Léger

[permalink] [raw]
Subject: Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop



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 <[email protected]>
>>>>>>> ---
>>>>>>> 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


2024-04-11 11:55:33

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop

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.pdf
|
| +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.


Attachments:
(No filename) (2.06 kB)
signature.asc (235.00 B)
Download all attachments

2024-04-15 09:21:32

by Clément Léger

[permalink] [raw]
Subject: Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop



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.pdf
> |
> | +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.

2024-04-16 14:54:52

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop

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.pdf
> > |
> > | +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...

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!


Attachments:
(No filename) (3.97 kB)
signature.asc (235.00 B)
Download all attachments

2024-04-16 15:24:02

by Clément Léger

[permalink] [raw]
Subject: Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop



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.pdf
>>> |
>>> | +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...
>
> 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

2024-04-17 13:15:09

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 07/10] riscv: add ISA extension parsing for Zcmop

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.pdf
> >>> |
> >>> | +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...
> >
> > 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.


Attachments:
(No filename) (7.30 kB)
signature.asc (235.00 B)
Download all attachments