Re: [PATCH v4] riscv: thead: Add th.sxstatus CSR emulation
On Mon, Apr 29, 2024 at 5:29 AM Alistair Francis wrote:
>
> On Mon, Apr 22, 2024 at 4:53 PM Christoph Müllner
> wrote:
> >
> > The th.sxstatus CSR can be used to identify available custom extension
> > on T-Head CPUs. The CSR is documented here:
> >
> > https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadsxstatus.adoc
> >
> > An important property of this patch is, that the th.sxstatus MAEE field
> > is not set (indicating that XTheadMae is not available).
> > XTheadMae is a memory attribute extension (similar to Svpbmt) which is
> > implemented in many T-Head CPUs (C906, C910, etc.) and utilizes bits
> > in PTEs that are marked as reserved. QEMU maintainers prefer to not
> > implement XTheadMae, so we need give kernels a mechanism to identify
> > if XTheadMae is available in a system or not. And this patch introduces
> > this mechanism in QEMU in a way that's compatible with real HW
> > (i.e., probing the th.sxstatus.MAEE bit).
> >
> > Further context can be found on the list:
> > https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg00775.html
> >
> > Reviewed-by: LIU Zhiwei
> > Signed-off-by: Christoph Müllner
> > ---
> > target/riscv/cpu.c | 1 +
> > target/riscv/cpu.h | 3 ++
> > target/riscv/meson.build | 1 +
> > target/riscv/th_csr.c| 77
> > 4 files changed, 82 insertions(+)
> > create mode 100644 target/riscv/th_csr.c
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 36e3e5fdaf..b82ba95ae6 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -545,6 +545,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
> > cpu->cfg.mvendorid = THEAD_VENDOR_ID;
> > #ifndef CONFIG_USER_ONLY
> > set_satp_mode_max_supported(cpu, VM_1_10_SV39);
> > +th_register_custom_csrs(cpu);
> > #endif
> >
> > /* inherited from parent obj via riscv_cpu_init() */
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 3b1a02b944..c9f8f06751 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -824,4 +824,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState
> > *cs);
> > uint8_t satp_mode_max_from_map(uint32_t map);
> > const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
> >
> > +/* Implemented in th_csr.c */
> > +void th_register_custom_csrs(RISCVCPU *cpu);
> > +
> > #endif /* RISCV_CPU_H */
> > diff --git a/target/riscv/meson.build b/target/riscv/meson.build
> > index a5e0734e7f..a4bd61e52a 100644
> > --- a/target/riscv/meson.build
> > +++ b/target/riscv/meson.build
> > @@ -33,6 +33,7 @@ riscv_system_ss.add(files(
> >'monitor.c',
> >'machine.c',
> >'pmu.c',
> > + 'th_csr.c',
> >'time_helper.c',
> >'riscv-qmp-cmds.c',
> > ))
> > diff --git a/target/riscv/th_csr.c b/target/riscv/th_csr.c
> > new file mode 100644
> > index 00..0eb3ad64f1
> > --- /dev/null
> > +++ b/target/riscv/th_csr.c
> > @@ -0,0 +1,77 @@
> > +/*
> > + * T-Head-specific CSRs.
> > + *
> > + * Copyright (c) 2024 VRULL GmbH
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> > for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along
> > with
> > + * this program. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "cpu.h"
> > +#include "cpu_vendorid.h"
> > +
> > +#define CSR_TH_SXSTATUS 0x5c0
> > +
> > +/* TH_SXSTATUS bits */
> > +#define TH_SXSTATUS_UCMEBIT(16)
> > +#define TH_SXSTATUS_MAEEBIT(21)
> > +#define TH_SXSTATUS_THEADISAEE BIT(22)
> > +
> > +typedef struct {
> > +int csrno;
> > +int (*insertion_test)(RISCVCPU *cpu);
> > +riscv_csr_operations csr_ops;
> > +} riscv_csr;
> > +
> > +static RISCVException smode(CPURISCVState *env, int csrno)
> > +{
> > +if (ris
[PATCH v5] riscv: thead: Add th.sxstatus CSR emulation
The th.sxstatus CSR can be used to identify available custom extension
on T-Head CPUs. The CSR is documented here:
https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadsxstatus.adoc
An important property of this patch is, that the th.sxstatus MAEE field
is not set (indicating that XTheadMae is not available).
XTheadMae is a memory attribute extension (similar to Svpbmt) which is
implemented in many T-Head CPUs (C906, C910, etc.) and utilizes bits
in PTEs that are marked as reserved. QEMU maintainers prefer to not
implement XTheadMae, so we need give kernels a mechanism to identify
if XTheadMae is available in a system or not. And this patch introduces
this mechanism in QEMU in a way that's compatible with real HW
(i.e., probing the th.sxstatus.MAEE bit).
Further context can be found on the list:
https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg00775.html
Reviewed-by: LIU Zhiwei
Reviewed-by: Alistair Francis
Signed-off-by: Christoph Müllner
---
MAINTAINERS | 1 +
target/riscv/cpu.c | 1 +
target/riscv/cpu.h | 3 ++
target/riscv/meson.build | 1 +
target/riscv/th_csr.c| 79
5 files changed, 85 insertions(+)
create mode 100644 target/riscv/th_csr.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 302b6fd00c..628e2b3141 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -342,6 +342,7 @@ L: qemu-ri...@nongnu.org
S: Supported
F: target/riscv/insn_trans/trans_xthead.c.inc
F: target/riscv/xthead*.decode
+F: target/riscv/th_*
F: disas/riscv-xthead*
RISC-V XVentanaCondOps extension
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index eb1a2e7d6d..70d30a2c00 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -545,6 +545,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
cpu->cfg.mvendorid = THEAD_VENDOR_ID;
#ifndef CONFIG_USER_ONLY
set_satp_mode_max_supported(cpu, VM_1_10_SV39);
+th_register_custom_csrs(cpu);
#endif
/* inherited from parent obj via riscv_cpu_init() */
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 2d0c02c35b..8dd6175e20 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -822,4 +822,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
uint8_t satp_mode_max_from_map(uint32_t map);
const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
+/* Implemented in th_csr.c */
+void th_register_custom_csrs(RISCVCPU *cpu);
+
#endif /* RISCV_CPU_H */
diff --git a/target/riscv/meson.build b/target/riscv/meson.build
index a5e0734e7f..a4bd61e52a 100644
--- a/target/riscv/meson.build
+++ b/target/riscv/meson.build
@@ -33,6 +33,7 @@ riscv_system_ss.add(files(
'monitor.c',
'machine.c',
'pmu.c',
+ 'th_csr.c',
'time_helper.c',
'riscv-qmp-cmds.c',
))
diff --git a/target/riscv/th_csr.c b/target/riscv/th_csr.c
new file mode 100644
index 00..6c970d4e81
--- /dev/null
+++ b/target/riscv/th_csr.c
@@ -0,0 +1,79 @@
+/*
+ * T-Head-specific CSRs.
+ *
+ * Copyright (c) 2024 VRULL GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "cpu_vendorid.h"
+
+#define CSR_TH_SXSTATUS 0x5c0
+
+/* TH_SXSTATUS bits */
+#define TH_SXSTATUS_UCMEBIT(16)
+#define TH_SXSTATUS_MAEEBIT(21)
+#define TH_SXSTATUS_THEADISAEE BIT(22)
+
+typedef struct {
+int csrno;
+int (*insertion_test)(RISCVCPU *cpu);
+riscv_csr_operations csr_ops;
+} riscv_csr;
+
+static RISCVException smode(CPURISCVState *env, int csrno)
+{
+if (riscv_has_ext(env, RVS)) {
+return RISCV_EXCP_NONE;
+}
+
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+static int test_thead_mvendorid(RISCVCPU *cpu)
+{
+if (cpu->cfg.mvendorid != THEAD_VENDOR_ID) {
+return -1;
+}
+
+return 0;
+}
+
+static RISCVException read_th_sxstatus(CPURISCVState *env, int csrno,
+ target_ulong *val)
+{
+/* We don't set MAEE here, because QEMU does not implement MAEE. */
+*val = TH_SXSTATUS_UCME | TH_SXSTATUS_THEADISAEE;
+return RISCV_EXCP_NONE;
+}
+
+static riscv_csr th_csr_list[] = {
+{
+.csrno = CSR_TH_SXSTATUS,
+.insertion_test = test_thead_mvendorid,
+.csr_ops = { "th.sxstatus", smode, read_th_sxstatus }
+}
+};
+
+void th_register_custom_csrs(RISCVCPU *cpu)
+{
+for (size_t i = 0; i
[PATCH v4] riscv: thead: Add th.sxstatus CSR emulation
The th.sxstatus CSR can be used to identify available custom extension
on T-Head CPUs. The CSR is documented here:
https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadsxstatus.adoc
An important property of this patch is, that the th.sxstatus MAEE field
is not set (indicating that XTheadMae is not available).
XTheadMae is a memory attribute extension (similar to Svpbmt) which is
implemented in many T-Head CPUs (C906, C910, etc.) and utilizes bits
in PTEs that are marked as reserved. QEMU maintainers prefer to not
implement XTheadMae, so we need give kernels a mechanism to identify
if XTheadMae is available in a system or not. And this patch introduces
this mechanism in QEMU in a way that's compatible with real HW
(i.e., probing the th.sxstatus.MAEE bit).
Further context can be found on the list:
https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg00775.html
Reviewed-by: LIU Zhiwei
Signed-off-by: Christoph Müllner
---
target/riscv/cpu.c | 1 +
target/riscv/cpu.h | 3 ++
target/riscv/meson.build | 1 +
target/riscv/th_csr.c| 77
4 files changed, 82 insertions(+)
create mode 100644 target/riscv/th_csr.c
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 36e3e5fdaf..b82ba95ae6 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -545,6 +545,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
cpu->cfg.mvendorid = THEAD_VENDOR_ID;
#ifndef CONFIG_USER_ONLY
set_satp_mode_max_supported(cpu, VM_1_10_SV39);
+th_register_custom_csrs(cpu);
#endif
/* inherited from parent obj via riscv_cpu_init() */
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 3b1a02b944..c9f8f06751 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -824,4 +824,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
uint8_t satp_mode_max_from_map(uint32_t map);
const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
+/* Implemented in th_csr.c */
+void th_register_custom_csrs(RISCVCPU *cpu);
+
#endif /* RISCV_CPU_H */
diff --git a/target/riscv/meson.build b/target/riscv/meson.build
index a5e0734e7f..a4bd61e52a 100644
--- a/target/riscv/meson.build
+++ b/target/riscv/meson.build
@@ -33,6 +33,7 @@ riscv_system_ss.add(files(
'monitor.c',
'machine.c',
'pmu.c',
+ 'th_csr.c',
'time_helper.c',
'riscv-qmp-cmds.c',
))
diff --git a/target/riscv/th_csr.c b/target/riscv/th_csr.c
new file mode 100644
index 00..0eb3ad64f1
--- /dev/null
+++ b/target/riscv/th_csr.c
@@ -0,0 +1,77 @@
+/*
+ * T-Head-specific CSRs.
+ *
+ * Copyright (c) 2024 VRULL GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "cpu_vendorid.h"
+
+#define CSR_TH_SXSTATUS 0x5c0
+
+/* TH_SXSTATUS bits */
+#define TH_SXSTATUS_UCMEBIT(16)
+#define TH_SXSTATUS_MAEEBIT(21)
+#define TH_SXSTATUS_THEADISAEE BIT(22)
+
+typedef struct {
+int csrno;
+int (*insertion_test)(RISCVCPU *cpu);
+riscv_csr_operations csr_ops;
+} riscv_csr;
+
+static RISCVException smode(CPURISCVState *env, int csrno)
+{
+if (riscv_has_ext(env, RVS)) {
+return RISCV_EXCP_NONE;
+}
+
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+static int test_thead_mvendorid(RISCVCPU *cpu)
+{
+if (cpu->cfg.mvendorid != THEAD_VENDOR_ID)
+return -1;
+
+return 0;
+}
+
+static RISCVException read_th_sxstatus(CPURISCVState *env, int csrno,
+ target_ulong *val)
+{
+/* We don't set MAEE here, because QEMU does not implement MAEE. */
+*val = TH_SXSTATUS_UCME | TH_SXSTATUS_THEADISAEE;
+return RISCV_EXCP_NONE;
+}
+
+static riscv_csr th_csr_list[] = {
+{
+.csrno = CSR_TH_SXSTATUS,
+.insertion_test = test_thead_mvendorid,
+.csr_ops = { "th.sxstatus", smode, read_th_sxstatus }
+}
+};
+
+void th_register_custom_csrs(RISCVCPU *cpu)
+{
+for (size_t i = 0; i < ARRAY_SIZE(th_csr_list); i++) {
+int csrno = th_csr_list[i].csrno;
+riscv_csr_operations *csr_ops = _csr_list[i].csr_ops;
+if (!th_csr_list[i].insertion_test(cpu))
+riscv_set_csr_ops(csrno, csr_ops);
+}
+}
--
2.44.0
Re: [PATCH v3] riscv: thead: Add th.sxstatus CSR emulation
On Mon, Apr 22, 2024 at 5:29 AM Alistair Francis wrote:
>
> On Thu, Apr 18, 2024 at 8:55 AM Christoph Müllner
> wrote:
> >
> > The th.sxstatus CSR can be used to identify available custom extension
> > on T-Head CPUs. The CSR is documented here:
> >
> > https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadsxstatus.adoc
> >
> > An important property of this patch is, that the th.sxstatus MAEE field
> > is not set (indicating that XTheadMae is not available).
> > XTheadMae is a memory attribute extension (similar to Svpbmt) which is
> > implemented in many T-Head CPUs (C906, C910, etc.) and utilizes bits
> > in PTEs that are marked as reserved. QEMU maintainers prefer to not
> > implement XTheadMae, so we need give kernels a mechanism to identify
> > if XTheadMae is available in a system or not. And this patch introduces
> > this mechanism in QEMU in a way that's compatible with real HW
> > (i.e., probing the th.sxstatus.MAEE bit).
> >
> > Further context can be found on the list:
> > https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg00775.html
> >
> > Reviewed-by: LIU Zhiwei
> > Signed-off-by: Christoph Müllner
> > ---
> > target/riscv/cpu.c | 1 +
> > target/riscv/cpu.h | 4 +++
> > target/riscv/csr.c | 2 +-
> > target/riscv/meson.build | 1 +
> > target/riscv/th_csr.c| 68
> > 5 files changed, 75 insertions(+), 1 deletion(-)
> > create mode 100644 target/riscv/th_csr.c
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 36e3e5fdaf..b82ba95ae6 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -545,6 +545,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
> > cpu->cfg.mvendorid = THEAD_VENDOR_ID;
> > #ifndef CONFIG_USER_ONLY
> > set_satp_mode_max_supported(cpu, VM_1_10_SV39);
> > +th_register_custom_csrs(cpu);
> > #endif
> >
> > /* inherited from parent obj via riscv_cpu_init() */
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 3b1a02b944..fd9424c8e9 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -818,10 +818,14 @@ extern const bool valid_vm_1_10_32[],
> > valid_vm_1_10_64[];
> >
> > void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
> > void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
> > +RISCVException smode(CPURISCVState *env, int csrno);
>
> I don't think we should make this public. Especially not the name `smode()`.
>
> One option is to rename this, something like `riscv_csr_check_smode()` maybe?
>
> The better option is probably just to copy it to the vendor
> implementation as it's a pretty simple function.
Ok, I will copy it and create a v3.
Thanks!
>
> Alistair
>
> >
> > void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
> >
> > uint8_t satp_mode_max_from_map(uint32_t map);
> > const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
> >
> > +/* Implemented in th_csr.c */
> > +void th_register_custom_csrs(RISCVCPU *cpu);
> > +
> > #endif /* RISCV_CPU_H */
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 726096444f..503eeb5f24 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -260,7 +260,7 @@ static RISCVException aia_any32(CPURISCVState *env, int
> > csrno)
> > return any32(env, csrno);
> > }
> >
> > -static RISCVException smode(CPURISCVState *env, int csrno)
> > +RISCVException smode(CPURISCVState *env, int csrno)
> > {
> > if (riscv_has_ext(env, RVS)) {
> > return RISCV_EXCP_NONE;
> > diff --git a/target/riscv/meson.build b/target/riscv/meson.build
> > index a5e0734e7f..a4bd61e52a 100644
> > --- a/target/riscv/meson.build
> > +++ b/target/riscv/meson.build
> > @@ -33,6 +33,7 @@ riscv_system_ss.add(files(
> >'monitor.c',
> >'machine.c',
> >'pmu.c',
> > + 'th_csr.c',
> >'time_helper.c',
> >'riscv-qmp-cmds.c',
> > ))
> > diff --git a/target/riscv/th_csr.c b/target/riscv/th_csr.c
> > new file mode 100644
> > index 00..44e28a9298
> > --- /dev/null
> > +++ b/target/riscv/th_csr.c
> > @@ -0,0 +1,68 @@
> > +/*
> > + * T-Head-specific CSRs.
> > + *
> > + * Copyright (c) 2024 VRULL GmbH
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and condit
Re: [PATCH v2] riscv: thead: Add th.sxstatus CSR emulation
On Fri, Apr 5, 2024 at 3:36 AM LIU Zhiwei wrote:
>
>
> On 2024/3/29 20:04, Christoph Müllner wrote:
> > The th.sxstatus CSR can be used to identify available custom extension
> > on T-Head CPUs. The CSR is documented here:
> >https://github.com/T-head-Semi/thead-extension-spec/pull/46
> >
> > An important property of this patch is, that the th.sxstatus MAEE field
> > is not set (indicating that XTheadMaee is not available).
> > XTheadMaee is a memory attribute extension (similar to Svpbmt) which is
> > implemented in many T-Head CPUs (C906, C910, etc.) and utilizes bits
> > in PTEs that are marked as reserved. QEMU maintainers prefer to not
> > implement XTheadMaee, so we need give kernels a mechanism to identify
> > if XTheadMaee is available in a system or not. And this patch introduces
> > this mechanism in QEMU in a way that's compatible with real HW
> > (i.e., probing the th.sxstatus.MAEE bit).
> >
> > Further context can be found on the list:
> > https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg00775.html
> >
> > Signed-off-by: Christoph Müllner
> > ---
> > target/riscv/cpu.c | 1 +
> > target/riscv/cpu.h | 3 ++
> > target/riscv/meson.build | 1 +
> > target/riscv/th_csr.c| 78
> > 4 files changed, 83 insertions(+)
> > create mode 100644 target/riscv/th_csr.c
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 36e3e5fdaf..b82ba95ae6 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -545,6 +545,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
> > cpu->cfg.mvendorid = THEAD_VENDOR_ID;
> > #ifndef CONFIG_USER_ONLY
> > set_satp_mode_max_supported(cpu, VM_1_10_SV39);
> > +th_register_custom_csrs(cpu);
> > #endif
> >
> > /* inherited from parent obj via riscv_cpu_init() */
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 3b1a02b944..c9f8f06751 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -824,4 +824,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState
> > *cs);
> > uint8_t satp_mode_max_from_map(uint32_t map);
> > const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
> >
> > +/* Implemented in th_csr.c */
> > +void th_register_custom_csrs(RISCVCPU *cpu);
> > +
> > #endif /* RISCV_CPU_H */
> > diff --git a/target/riscv/meson.build b/target/riscv/meson.build
> > index a5e0734e7f..a4bd61e52a 100644
> > --- a/target/riscv/meson.build
> > +++ b/target/riscv/meson.build
> > @@ -33,6 +33,7 @@ riscv_system_ss.add(files(
> > 'monitor.c',
> > 'machine.c',
> > 'pmu.c',
> > + 'th_csr.c',
> > 'time_helper.c',
> > 'riscv-qmp-cmds.c',
> > ))
> > diff --git a/target/riscv/th_csr.c b/target/riscv/th_csr.c
> > new file mode 100644
> > index 00..66d260cabd
> > --- /dev/null
> > +++ b/target/riscv/th_csr.c
> > @@ -0,0 +1,78 @@
> > +/*
> > + * T-Head-specific CSRs.
> > + *
> > + * Copyright (c) 2024 VRULL GmbH
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> > for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along
> > with
> > + * this program. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "cpu.h"
> > +#include "cpu_vendorid.h"
> > +
> > +#define CSR_TH_SXSTATUS 0x5c0
> > +
> > +/* TH_SXSTATUS bits */
> > +#define TH_SXSTATUS_UCMEBIT(16)
> > +#define TH_SXSTATUS_MAEEBIT(21)
> > +#define TH_SXSTATUS_THEADISAEE BIT(22)
> > +
> > +typedef struct {
> > +int csrno;
> > +int (*insertion_test)(RISCVCPU *cpu);
> > +riscv_csr_operations csr_ops;
> > +} riscv_csr;
> > +
> > +static RISCVException s_mode_csr(CPURISCVState *env, int csrno)
> > +{
> > +if (env->debugger)
> > +
[PATCH v3] riscv: thead: Add th.sxstatus CSR emulation
The th.sxstatus CSR can be used to identify available custom extension
on T-Head CPUs. The CSR is documented here:
https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadsxstatus.adoc
An important property of this patch is, that the th.sxstatus MAEE field
is not set (indicating that XTheadMae is not available).
XTheadMae is a memory attribute extension (similar to Svpbmt) which is
implemented in many T-Head CPUs (C906, C910, etc.) and utilizes bits
in PTEs that are marked as reserved. QEMU maintainers prefer to not
implement XTheadMae, so we need give kernels a mechanism to identify
if XTheadMae is available in a system or not. And this patch introduces
this mechanism in QEMU in a way that's compatible with real HW
(i.e., probing the th.sxstatus.MAEE bit).
Further context can be found on the list:
https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg00775.html
Reviewed-by: LIU Zhiwei
Signed-off-by: Christoph Müllner
---
target/riscv/cpu.c | 1 +
target/riscv/cpu.h | 4 +++
target/riscv/csr.c | 2 +-
target/riscv/meson.build | 1 +
target/riscv/th_csr.c| 68
5 files changed, 75 insertions(+), 1 deletion(-)
create mode 100644 target/riscv/th_csr.c
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 36e3e5fdaf..b82ba95ae6 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -545,6 +545,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
cpu->cfg.mvendorid = THEAD_VENDOR_ID;
#ifndef CONFIG_USER_ONLY
set_satp_mode_max_supported(cpu, VM_1_10_SV39);
+th_register_custom_csrs(cpu);
#endif
/* inherited from parent obj via riscv_cpu_init() */
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 3b1a02b944..fd9424c8e9 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -818,10 +818,14 @@ extern const bool valid_vm_1_10_32[], valid_vm_1_10_64[];
void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
+RISCVException smode(CPURISCVState *env, int csrno);
void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
uint8_t satp_mode_max_from_map(uint32_t map);
const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
+/* Implemented in th_csr.c */
+void th_register_custom_csrs(RISCVCPU *cpu);
+
#endif /* RISCV_CPU_H */
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 726096444f..503eeb5f24 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -260,7 +260,7 @@ static RISCVException aia_any32(CPURISCVState *env, int
csrno)
return any32(env, csrno);
}
-static RISCVException smode(CPURISCVState *env, int csrno)
+RISCVException smode(CPURISCVState *env, int csrno)
{
if (riscv_has_ext(env, RVS)) {
return RISCV_EXCP_NONE;
diff --git a/target/riscv/meson.build b/target/riscv/meson.build
index a5e0734e7f..a4bd61e52a 100644
--- a/target/riscv/meson.build
+++ b/target/riscv/meson.build
@@ -33,6 +33,7 @@ riscv_system_ss.add(files(
'monitor.c',
'machine.c',
'pmu.c',
+ 'th_csr.c',
'time_helper.c',
'riscv-qmp-cmds.c',
))
diff --git a/target/riscv/th_csr.c b/target/riscv/th_csr.c
new file mode 100644
index 00..44e28a9298
--- /dev/null
+++ b/target/riscv/th_csr.c
@@ -0,0 +1,68 @@
+/*
+ * T-Head-specific CSRs.
+ *
+ * Copyright (c) 2024 VRULL GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "cpu_vendorid.h"
+
+#define CSR_TH_SXSTATUS 0x5c0
+
+/* TH_SXSTATUS bits */
+#define TH_SXSTATUS_UCMEBIT(16)
+#define TH_SXSTATUS_MAEEBIT(21)
+#define TH_SXSTATUS_THEADISAEE BIT(22)
+
+typedef struct {
+int csrno;
+int (*insertion_test)(RISCVCPU *cpu);
+riscv_csr_operations csr_ops;
+} riscv_csr;
+
+static int test_thead_mvendorid(RISCVCPU *cpu)
+{
+if (cpu->cfg.mvendorid != THEAD_VENDOR_ID)
+return -1;
+
+return 0;
+}
+
+static RISCVException read_th_sxstatus(CPURISCVState *env, int csrno,
+ target_ulong *val)
+{
+/* We don't set MAEE here, because QEMU does not implement MAEE. */
+*val = TH_SXSTATUS_UCME | TH_SXSTATUS_THEADISAEE;
+return RISCV_EXCP_NONE;
+}
+
+static riscv_csr th_csr_list[] = {
+{
+.csrno = CSR_TH_SXSTATUS,
+.insertion_test = test_thead_mvendorid,
[PATCH v2] riscv: thead: Add th.sxstatus CSR emulation
The th.sxstatus CSR can be used to identify available custom extension
on T-Head CPUs. The CSR is documented here:
https://github.com/T-head-Semi/thead-extension-spec/pull/46
An important property of this patch is, that the th.sxstatus MAEE field
is not set (indicating that XTheadMaee is not available).
XTheadMaee is a memory attribute extension (similar to Svpbmt) which is
implemented in many T-Head CPUs (C906, C910, etc.) and utilizes bits
in PTEs that are marked as reserved. QEMU maintainers prefer to not
implement XTheadMaee, so we need give kernels a mechanism to identify
if XTheadMaee is available in a system or not. And this patch introduces
this mechanism in QEMU in a way that's compatible with real HW
(i.e., probing the th.sxstatus.MAEE bit).
Further context can be found on the list:
https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg00775.html
Signed-off-by: Christoph Müllner
---
target/riscv/cpu.c | 1 +
target/riscv/cpu.h | 3 ++
target/riscv/meson.build | 1 +
target/riscv/th_csr.c| 78
4 files changed, 83 insertions(+)
create mode 100644 target/riscv/th_csr.c
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 36e3e5fdaf..b82ba95ae6 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -545,6 +545,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
cpu->cfg.mvendorid = THEAD_VENDOR_ID;
#ifndef CONFIG_USER_ONLY
set_satp_mode_max_supported(cpu, VM_1_10_SV39);
+th_register_custom_csrs(cpu);
#endif
/* inherited from parent obj via riscv_cpu_init() */
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 3b1a02b944..c9f8f06751 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -824,4 +824,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
uint8_t satp_mode_max_from_map(uint32_t map);
const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
+/* Implemented in th_csr.c */
+void th_register_custom_csrs(RISCVCPU *cpu);
+
#endif /* RISCV_CPU_H */
diff --git a/target/riscv/meson.build b/target/riscv/meson.build
index a5e0734e7f..a4bd61e52a 100644
--- a/target/riscv/meson.build
+++ b/target/riscv/meson.build
@@ -33,6 +33,7 @@ riscv_system_ss.add(files(
'monitor.c',
'machine.c',
'pmu.c',
+ 'th_csr.c',
'time_helper.c',
'riscv-qmp-cmds.c',
))
diff --git a/target/riscv/th_csr.c b/target/riscv/th_csr.c
new file mode 100644
index 00..66d260cabd
--- /dev/null
+++ b/target/riscv/th_csr.c
@@ -0,0 +1,78 @@
+/*
+ * T-Head-specific CSRs.
+ *
+ * Copyright (c) 2024 VRULL GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "cpu_vendorid.h"
+
+#define CSR_TH_SXSTATUS 0x5c0
+
+/* TH_SXSTATUS bits */
+#define TH_SXSTATUS_UCMEBIT(16)
+#define TH_SXSTATUS_MAEEBIT(21)
+#define TH_SXSTATUS_THEADISAEE BIT(22)
+
+typedef struct {
+int csrno;
+int (*insertion_test)(RISCVCPU *cpu);
+riscv_csr_operations csr_ops;
+} riscv_csr;
+
+static RISCVException s_mode_csr(CPURISCVState *env, int csrno)
+{
+if (env->debugger)
+return RISCV_EXCP_NONE;
+
+if (env->priv >= PRV_S)
+return RISCV_EXCP_NONE;
+
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+static int test_thead_mvendorid(RISCVCPU *cpu)
+{
+if (cpu->cfg.mvendorid != THEAD_VENDOR_ID)
+return -1;
+return 0;
+}
+
+static RISCVException read_th_sxstatus(CPURISCVState *env, int csrno,
+ target_ulong *val)
+{
+/* We don't set MAEE here, because QEMU does not implement MAEE. */
+*val = TH_SXSTATUS_UCME | TH_SXSTATUS_THEADISAEE;
+return RISCV_EXCP_NONE;
+}
+
+static riscv_csr th_csr_list[] = {
+{
+.csrno = CSR_TH_SXSTATUS,
+.insertion_test = test_thead_mvendorid,
+.csr_ops = { "th.sxstatus", s_mode_csr, read_th_sxstatus }
+}
+};
+
+void th_register_custom_csrs(RISCVCPU *cpu)
+{
+for (size_t i = 0; i < ARRAY_SIZE(th_csr_list); i++) {
+int csrno = th_csr_list[i].csrno;
+riscv_csr_operations *csr_ops = _csr_list[i].csr_ops;
+if (!th_csr_list[i].insertion_test(cpu))
+riscv_set_csr_ops(csrno, csr_ops);
+}
+}
--
2.44.0
Re: [PATCH v2 2/2] target/riscv: Support xtheadmaee for thead-c906
On Thu, Mar 28, 2024 at 2:19 AM Alistair Francis wrote: > > On Wed, Mar 27, 2024 at 9:19 PM Conor Dooley wrote: > > > > Christoph linked here on his submission to Linux of a fix for this, so I > > am reviving this to leave a couple comments :) > > > > On Thu, Feb 15, 2024 at 02:24:02PM +1000, Alistair Francis wrote: > > > On Mon, Feb 5, 2024 at 6:37 PM Christoph Müllner > > > wrote: > > > > On Mon, Feb 5, 2024 at 3:42 AM Alistair Francis > > > > wrote: > > > > > On Sun, Feb 4, 2024 at 3:44 PM LIU Zhiwei > > > > > wrote: > > > > > > > > ppn = (pte & (target_ulong)PTE_PPN_MASK) >> > > > > > > PTE_PPN_SHIFT; > > > > > > > > > > Unfortunately we won't be able to take this upstream. This is core > > > > > QEMU RISC-V code that is now being changed against the spec. I think > > > > > adding the CSR is fine, but we can't take this core change. > > > > > > > > > > A fix that works for everyone should be supporting the th_mxstatus > > > > > CSR, but don't support setting the TH_MXSTATUS_MAEE bit. That way > > > > > guests can detect that the bit isn't set and not use the reserved bits > > > > > in the PTE. From my understanding the extra PTE bits are related to > > > > > cache control in the hardware, which we don't need here > > > > > > > > Sounds good! Let me recap the overall plan: > > > > * QEMU does not emulate MAEE, but signals that MAEE is not available > > > > by setting TH_MXSTATUS_MAEE to 0. > > > > > > Yep! > > > > > > > * Consequence: The c906 emulation does not enable any page-base memory > > > > attribute mechanism. > > > > > > Exactly > > > > > > > * OpenSBI tests the TH_MXSTATUS_MAEE bit (M-mode only) and provides > > > > that information to user-space (e.g. DTB). > > > > > > To the kernel, but yep! > > > > > > > * The current Linux errata code will be enhanced to not assume MAEE > > > > for each core with T-Head vendor ID, but also query the MAEE bit and > > > > ensure it is set. > > > > > > I feel like it should already do that :) > > > > It doesn't quite do this right now. It only makes the assumption for > > CPUs where marchid and mvendorid are zero. The c908, and I think Guo Ren > > confirmed it will be the case going forward, sets these to non-zero > > values. We should have always required a dt property be set, rather than > > using m*id, but we can't go back on that for these devices. Going > > forward, if there are more CPUs that want to use this e.g. C908 in MAEE > > mode (it can do svpbmt too) I'm gonna require it is explicitly set in > > A DT node that we don't set also works fine for us I would really like to do that, but given the page table is set up so early in the boot process probing via CSR is much easier to realize in the kernel than parsing the DT. Therefore, I think th.sxstatus emulation + probing is the best way to move forward. Thanks, Christoph
[PATCH] riscv: thead: Add th.mxstatus CSR emulation
The th.mxstatus CSR can be used to identify available custom extension
on T-Head CPUs. The CSR is documented here:
https://github.com/T-head-Semi/thead-extension-spec/pull/45
An important property of this patch is, that the th.mxstatus MAEE field
is not set (indicating that XTheadMaee is not available).
XTheadMaee is a memory attribute extension (similar to Svpbmt) which is
implemented in many T-Head CPUs (C906, C910, etc.) and utilizes bits
in PTEs that are marked as reserved. QEMU maintainers prefer to not
implement XTheadMaee, so we need give kernels a mechanism to identify
if XTheadMaee is available in a system or not. And this patch introduces
this mechanism in QEMU in a way that's compatible with real HW
(i.e., probing the th.mxstatus.MAEE bit).
Further context can be found on the list:
https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg00775.html
Signed-off-by: Christoph Müllner
---
target/riscv/cpu.c | 1 +
target/riscv/cpu.h | 3 ++
target/riscv/meson.build | 1 +
target/riscv/th_csr.c| 76
4 files changed, 81 insertions(+)
create mode 100644 target/riscv/th_csr.c
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 36e3e5fdaf..b82ba95ae6 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -545,6 +545,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
cpu->cfg.mvendorid = THEAD_VENDOR_ID;
#ifndef CONFIG_USER_ONLY
set_satp_mode_max_supported(cpu, VM_1_10_SV39);
+th_register_custom_csrs(cpu);
#endif
/* inherited from parent obj via riscv_cpu_init() */
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 3b1a02b944..c9f8f06751 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -824,4 +824,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
uint8_t satp_mode_max_from_map(uint32_t map);
const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
+/* Implemented in th_csr.c */
+void th_register_custom_csrs(RISCVCPU *cpu);
+
#endif /* RISCV_CPU_H */
diff --git a/target/riscv/meson.build b/target/riscv/meson.build
index a5e0734e7f..a4bd61e52a 100644
--- a/target/riscv/meson.build
+++ b/target/riscv/meson.build
@@ -33,6 +33,7 @@ riscv_system_ss.add(files(
'monitor.c',
'machine.c',
'pmu.c',
+ 'th_csr.c',
'time_helper.c',
'riscv-qmp-cmds.c',
))
diff --git a/target/riscv/th_csr.c b/target/riscv/th_csr.c
new file mode 100644
index 00..7fbcf88b5b
--- /dev/null
+++ b/target/riscv/th_csr.c
@@ -0,0 +1,76 @@
+/*
+ * T-Head-specific CSRs.
+ *
+ * Copyright (c) 2024 VRULL GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "cpu_vendorid.h"
+
+#define CSR_TH_MXSTATUS 0x7c0
+
+/* TH_MXSTATUS bits */
+#define TH_MXSTATUS_UCMEBIT(16)
+#define TH_MXSTATUS_MAEEBIT(21)
+#define TH_MXSTATUS_THEADISAEE BIT(22)
+
+typedef struct {
+int csrno;
+int (*insertion_test)(RISCVCPU *cpu);
+riscv_csr_operations csr_ops;
+} riscv_csr;
+
+static RISCVException m_mode(CPURISCVState *env, int csrno)
+{
+if (env->debugger)
+return RISCV_EXCP_NONE;
+if (env->priv != PRV_M)
+return RISCV_EXCP_ILLEGAL_INST;
+return RISCV_EXCP_NONE;
+}
+
+static int test_thead_mvendorid(RISCVCPU *cpu)
+{
+if (cpu->cfg.mvendorid != THEAD_VENDOR_ID)
+return -1;
+return 0;
+}
+
+static RISCVException read_th_mxstatus(CPURISCVState *env, int csrno,
+ target_ulong *val)
+{
+/* We don't set MAEE here, because QEMU does not implement MAEE. */
+*val = TH_MXSTATUS_UCME | TH_MXSTATUS_THEADISAEE;
+return RISCV_EXCP_NONE;
+}
+
+static riscv_csr th_csr_list[] = {
+{
+.csrno = CSR_TH_MXSTATUS,
+.insertion_test = test_thead_mvendorid,
+.csr_ops = { "th.mxstatus", m_mode, read_th_mxstatus }
+}
+};
+
+void th_register_custom_csrs(RISCVCPU *cpu)
+{
+for (size_t i = 0; i < ARRAY_SIZE(th_csr_list); i++) {
+int csrno = th_csr_list[i].csrno;
+riscv_csr_operations *csr_ops = _csr_list[i].csr_ops;
+if (!th_csr_list[i].insertion_test(cpu))
+riscv_set_csr_ops(csrno, csr_ops);
+}
+}
--
2.44.0
Re: [PATCH 2/2] linux-user/riscv: Sync hwprobe keys with Linux
On Fri, Mar 8, 2024 at 5:23 AM Alistair Francis wrote: > > On Wed, Feb 7, 2024 at 10:00 PM Christoph Müllner > wrote: > > > > Upstream Linux recently added many additional keys to the hwprobe API. > > This patch adds support for all of them with the exception of Ztso, > > which is currently not supported in QEMU. > > > > Signed-off-by: Christoph Müllner > > --- > > linux-user/syscall.c | 98 > > 1 file changed, 91 insertions(+), 7 deletions(-) > > > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > > index 43467c9707..3ba20f99ad 100644 > > --- a/linux-user/syscall.c > > +++ b/linux-user/syscall.c > > @@ -8793,13 +8793,41 @@ static int do_getdents64(abi_long dirfd, abi_long > > arg2, abi_long count) > > #define RISCV_HWPROBE_BASE_BEHAVIOR_IMA (1 << 0) > > > > #define RISCV_HWPROBE_KEY_IMA_EXT_0 4 > > -#define RISCV_HWPROBE_IMA_FD (1 << 0) > > -#define RISCV_HWPROBE_IMA_C(1 << 1) > > -#define RISCV_HWPROBE_IMA_V(1 << 2) > > -#define RISCV_HWPROBE_EXT_ZBA (1 << 3) > > -#define RISCV_HWPROBE_EXT_ZBB (1 << 4) > > -#define RISCV_HWPROBE_EXT_ZBS (1 << 5) > > -#define RISCV_HWPROBE_EXT_ZICBOZ (1 << 6) > > +#defineRISCV_HWPROBE_IMA_FD(1 << 0) > > +#defineRISCV_HWPROBE_IMA_C (1 << 1) > > +#defineRISCV_HWPROBE_IMA_V (1 << 2) > > +#defineRISCV_HWPROBE_EXT_ZBA (1 << 3) > > +#defineRISCV_HWPROBE_EXT_ZBB (1 << 4) > > +#defineRISCV_HWPROBE_EXT_ZBS (1 << 5) > > +#defineRISCV_HWPROBE_EXT_ZICBOZ(1 << 6) > > +#defineRISCV_HWPROBE_EXT_ZBC (1 << 7) > > +#defineRISCV_HWPROBE_EXT_ZBKB (1 << 8) > > +#defineRISCV_HWPROBE_EXT_ZBKC (1 << 9) > > +#defineRISCV_HWPROBE_EXT_ZBKX (1 << 10) > > +#defineRISCV_HWPROBE_EXT_ZKND (1 << 11) > > +#defineRISCV_HWPROBE_EXT_ZKNE (1 << 12) > > +#defineRISCV_HWPROBE_EXT_ZKNH (1 << 13) > > +#defineRISCV_HWPROBE_EXT_ZKSED (1 << 14) > > +#defineRISCV_HWPROBE_EXT_ZKSH (1 << 15) > > +#defineRISCV_HWPROBE_EXT_ZKT (1 << 16) > > +#defineRISCV_HWPROBE_EXT_ZVBB (1 << 17) > > +#defineRISCV_HWPROBE_EXT_ZVBC (1 << 18) > > +#defineRISCV_HWPROBE_EXT_ZVKB (1 << 19) > > +#defineRISCV_HWPROBE_EXT_ZVKG (1 << 20) > > +#defineRISCV_HWPROBE_EXT_ZVKNED(1 << 21) > > +#defineRISCV_HWPROBE_EXT_ZVKNHA(1 << 22) > > +#defineRISCV_HWPROBE_EXT_ZVKNHB(1 << 23) > > +#defineRISCV_HWPROBE_EXT_ZVKSED(1 << 24) > > +#defineRISCV_HWPROBE_EXT_ZVKSH (1 << 25) > > +#defineRISCV_HWPROBE_EXT_ZVKT (1 << 26) > > +#defineRISCV_HWPROBE_EXT_ZFH (1 << 27) > > +#defineRISCV_HWPROBE_EXT_ZFHMIN(1 << 28) > > +#defineRISCV_HWPROBE_EXT_ZIHINTNTL (1 << 29) > > +#defineRISCV_HWPROBE_EXT_ZVFH (1 << 30) > > +#defineRISCV_HWPROBE_EXT_ZVFHMIN (1 << 31) > > +#defineRISCV_HWPROBE_EXT_ZFA (1ULL << 32) > > +#defineRISCV_HWPROBE_EXT_ZACAS (1ULL << 34) > > +#defineRISCV_HWPROBE_EXT_ZICOND(1ULL << 35) > > This fails to pass checkpatch I copied 1:1 from the kernel, so I guess it is the tabs. Sorry for this! And as you have already fixed that: thanks! > > Alistair > > > > > #define RISCV_HWPROBE_KEY_CPUPERF_0 5 > > #define RISCV_HWPROBE_MISALIGNED_UNKNOWN (0 << 0) > > @@ -8860,6 +,62 @@ static void risc_hwprobe_fill_pairs(CPURISCVState > > *env, > > RISCV_HWPROBE_EXT_ZBS : 0; > > value |= cfg->ext_zicboz ? > > RISCV_HWPROBE_EXT_ZICBOZ : 0; > > +value |= cfg->ext_zbc ? > > +
Re: [PATCH] target/riscv: Implement dynamic establishment of custom decoder
On Thu, Mar 7, 2024 at 9:35 PM Richard Henderson
wrote:
>
> >>> -for (size_t i = 0; i < ARRAY_SIZE(decoders); ++i) {
> >>> -if (decoders[i].guard_func(ctx->cfg_ptr) &&
> >>> -decoders[i].decode_func(ctx, opcode32)) {
> >>> +for (size_t i = 0; i < decoder_table_size; ++i) {
> >>> +if (ctx->decoder[i](ctx, opcode32)) {
> >>> return;
>
> By the way, you're adding layers of pointer chasing, so I suspect you'll find
> all of this
> is a wash or worse, performance-wise.
>
>
> Indeed, since some of the predicates are trivial, going the other way might
> help: allow
> everything to be inlined:
>
> if (decode_insn32(...)) {
> return;
> }
> if (has_xthead_p(...) && decode_xthead(...)) {
> return;
> }
> ...
>
>
> Even if there are 10 entries here, so what? All of the code has to be
> compiled into QEMU.
> You're not going to somehow add truly dynamic code that you've loaded from
> a module.
I just tested this with GCC -O2/-O3. The generated code from the
existing decoder loop will
result in exactly what you have listed here (loop unrolling,
transforming the indirect calls
to direct calls, inlining, and evaluation of statically known conditions).
has_xthead_p() can get inlined as well, if the inlining costs are
considered low enough
(thank you, Richard, for giving some hints about that below).
What the commit message is not mentioning (and what this patch was
actually addressing and
therefore should have been mentioned):
Having dynamic control of the decoder order was meant to allow adding
a vendor_decoder
before decode_insn32() with minimal overhead (no guard_func) for users
that don't need
this particular vendor_decoder.
Being more explicit: there is interest in supporting the
non-conforming (conflicting) instruction
extension XTheadVector:
https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadvector.adoc
XTheadVector uses the RVV 0.7.1 draft encoding, which conflicts with
the ratified RVV spec.
The idea is to avoid these conflicts with a call to
decode_xtheadvector() before decode_insn32().
This implies that everyone has to call has_xtheadvector_p() before
calling decode_insn32().
And the intent of this patch is to provide a mechanism to reduce this overhead.
When suggesting the dynamic decoder list, I was not aware that
always_true_p() will be
eliminated by the compiler. Since this is the case, I agree with the
"wash or worse" for
decode_insn32(). The elimination of following guard functions for
vendor decoders is likely
less performance relevant.
I don't think we should discuss efficiency any further unless we have
some data to
justify any changes. E.g. emulating a RISC-V SPEC CPU 2017 run on x86_64 and
looking at the runtimes could give the relevant insights for the
following scenarios:
* existing code on upstream/master
* existing code + adding a new extension that comes before
decode_insn32 in decoders[]
* existing code + this patch (dynamic decoders)
Related overheads that could be measured: adding 20 new instructions
to decode_insn32(),
which are not executed (to put the costs into perspective).
> You could perhaps streamline predicates such as has_xthead_p to not test 11
> variables by
> adding an artificial "ext_xthead_any" configuration entry that is the sum of
> all of those.
>
>
> r~
[PATCH] tests: riscv64: Use 'zfa' instead of 'Zfa'
Running test-fcvtmod triggers the following deprecation warning: warning: CPU property 'Zfa' is deprecated. Please use 'zfa' instead Let's fix that. Signed-off-by: Christoph Müllner --- tests/tcg/riscv64/Makefile.target | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target index a7e390c384..4da5b9a3b3 100644 --- a/tests/tcg/riscv64/Makefile.target +++ b/tests/tcg/riscv64/Makefile.target @@ -17,4 +17,4 @@ run-test-aes: QEMU_OPTS += -cpu rv64,zk=on TESTS += test-fcvtmod test-fcvtmod: CFLAGS += -march=rv64imafdc test-fcvtmod: LDFLAGS += -static -run-test-fcvtmod: QEMU_OPTS += -cpu rv64,d=true,Zfa=true +run-test-fcvtmod: QEMU_OPTS += -cpu rv64,d=true,zfa=true -- 2.43.2
Re: [PATCH 0/2] RISC-V: Add Ztso extension
On Wed, Feb 14, 2024 at 5:25 PM Andrew Jones wrote: > > On Wed, Feb 14, 2024 at 02:38:34PM +0100, Christoph Müllner wrote: > > On Wed, Feb 14, 2024 at 2:35 PM Daniel Henrique Barboza > > wrote: > > > > > > > > > > > > On 2/7/24 09:22, Christoph Müllner wrote: > > > > The first patch of this series picks up an earlier v2 Ztso patch from > > > > Palmer, > > > > which can be found here: > > > > > > > > https://patchwork.kernel.org/project/qemu-devel/patch/20220917072635.11616-1-pal...@rivosinc.com/ > > > > This patch did not apply cleanly but the necessary changes were trivial. > > > > There was a request to extend the commit message, which is part of the > > > > posted patch of this series. As this patch was reviewed a year ago, > > > > I believe it could be merged. > > > > > > > > The second patch simply exposes Ztso via hwprobe. > > > > > > It's also worth mentioning that the second patch relies on: > > > > > > "[PATCH 0/2] linux-user/riscv: Sync hwprobe keys with kernel" > > > > > > To be applied beforehand. > > > > Indeed! Therefore, the end of the cover letter contains the following > > paragraph: > > """ > > This series is based on today's riscv-to-apply.next with my other series > > that adds the new hwprobe keys > > (https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg01293.html). > > """ > > I think a line like > > Based-on: 20240207115926.887816-1-christoph.muell...@vrull.eu > > in the cover letter would allow the automated tools to green-light this > series too. Should I resend? > > Thanks, > drew > > > > > > To ease reviewing and testing for others, I've also created a remote > > branch on GitHub. > > > > Thanks for reviewing! > > > > > > > > > > > > > > Thanks, > > > > > > Daniel > > > > > > > > > > > > > > Relevant in this context might be also, that Richard's patch to improve > > > > TCG's memory barrier selection depending on host and guest memory > > > > ordering > > > > landed in June 2023: > > > > > > > > https://lore.kernel.org/all/a313b36b-dcc1-f812-ccbd-afed1cbd5...@linaro.org/T/ > > > > > > > > The first patch was already sent as part of an RFC series for Ssdtso: > > > > > > > > https://lists.nongnu.org/archive/html/qemu-devel/2023-11/msg02962.html > > > > Since I don't want to keep this patch until the ratification of Ssdtso, > > > > I would like to get this merged independent of Ssdtso. > > > > > > > > This series is based on today's riscv-to-apply.next with my other series > > > > that adds the new hwprobe keys > > > > (https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg01293.html). > > > > > > > > This series can also be found here: > > > >https://github.com/cmuellner/qemu/tree/ztso > > > > > > > > Christoph Müllner (1): > > > >linux-user/riscv: Add Ztso extension to hwprobe > > > > > > > > Palmer Dabbelt (1): > > > >RISC-V: Add support for Ztso > > > > > > > > linux-user/syscall.c| 3 +++ > > > > target/riscv/cpu.c | 2 ++ > > > > target/riscv/cpu_cfg.h | 1 + > > > > target/riscv/insn_trans/trans_rva.c.inc | 11 --- > > > > target/riscv/insn_trans/trans_rvi.c.inc | 16 ++-- > > > > target/riscv/insn_trans/trans_rvv.c.inc | 20 > > > > target/riscv/translate.c| 3 +++ > > > > 7 files changed, 51 insertions(+), 5 deletions(-) > > > >
Re: [PATCH 0/2] RISC-V: Add Ztso extension
On Wed, Feb 14, 2024 at 2:35 PM Daniel Henrique Barboza wrote: > > > > On 2/7/24 09:22, Christoph Müllner wrote: > > The first patch of this series picks up an earlier v2 Ztso patch from > > Palmer, > > which can be found here: > > > > https://patchwork.kernel.org/project/qemu-devel/patch/20220917072635.11616-1-pal...@rivosinc.com/ > > This patch did not apply cleanly but the necessary changes were trivial. > > There was a request to extend the commit message, which is part of the > > posted patch of this series. As this patch was reviewed a year ago, > > I believe it could be merged. > > > > The second patch simply exposes Ztso via hwprobe. > > It's also worth mentioning that the second patch relies on: > > "[PATCH 0/2] linux-user/riscv: Sync hwprobe keys with kernel" > > To be applied beforehand. Indeed! Therefore, the end of the cover letter contains the following paragraph: """ This series is based on today's riscv-to-apply.next with my other series that adds the new hwprobe keys (https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg01293.html). """ To ease reviewing and testing for others, I've also created a remote branch on GitHub. Thanks for reviewing! > > > > Thanks, > > Daniel > > > > > > Relevant in this context might be also, that Richard's patch to improve > > TCG's memory barrier selection depending on host and guest memory ordering > > landed in June 2023: > > > > https://lore.kernel.org/all/a313b36b-dcc1-f812-ccbd-afed1cbd5...@linaro.org/T/ > > > > The first patch was already sent as part of an RFC series for Ssdtso: > >https://lists.nongnu.org/archive/html/qemu-devel/2023-11/msg02962.html > > Since I don't want to keep this patch until the ratification of Ssdtso, > > I would like to get this merged independent of Ssdtso. > > > > This series is based on today's riscv-to-apply.next with my other series > > that adds the new hwprobe keys > > (https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg01293.html). > > > > This series can also be found here: > >https://github.com/cmuellner/qemu/tree/ztso > > > > Christoph Müllner (1): > >linux-user/riscv: Add Ztso extension to hwprobe > > > > Palmer Dabbelt (1): > >RISC-V: Add support for Ztso > > > > linux-user/syscall.c| 3 +++ > > target/riscv/cpu.c | 2 ++ > > target/riscv/cpu_cfg.h | 1 + > > target/riscv/insn_trans/trans_rva.c.inc | 11 --- > > target/riscv/insn_trans/trans_rvi.c.inc | 16 ++-- > > target/riscv/insn_trans/trans_rvv.c.inc | 20 > > target/riscv/translate.c| 3 +++ > > 7 files changed, 51 insertions(+), 5 deletions(-) > >
[RFC PATCH v2 2/4] linux-user/riscv: Add Ssdtso extension to hwprobe
This patch exposes Ssdtso via hwprobe in QEMU's user space emulator. Signed-off-by: Christoph Müllner --- linux-user/syscall.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 24fa11d946..bf0d66b8a8 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -8829,6 +8829,7 @@ static int do_getdents64(abi_long dirfd, abi_long arg2, abi_long count) #defineRISCV_HWPROBE_EXT_ZTSO (1ULL << 33) #defineRISCV_HWPROBE_EXT_ZACAS (1ULL << 34) #defineRISCV_HWPROBE_EXT_ZICOND(1ULL << 35) +#defineRISCV_HWPROBE_EXT_SSDTSO(1ULL << 36) #define RISCV_HWPROBE_KEY_CPUPERF_0 5 #define RISCV_HWPROBE_MISALIGNED_UNKNOWN (0 << 0) @@ -8947,6 +8948,8 @@ static void risc_hwprobe_fill_pairs(CPURISCVState *env, RISCV_HWPROBE_EXT_ZACAS : 0; value |= cfg->ext_zicond ? RISCV_HWPROBE_EXT_ZICOND : 0; +value |= cfg->ext_ssdtso ? + RISCV_HWPROBE_EXT_SSDTSO : 0; __put_user(value, >value); break; case RISCV_HWPROBE_KEY_CPUPERF_0: -- 2.43.0
[RFC PATCH v2 4/4] linux-user/riscv: Implement dynamic memory consistency model support
This patch implements the dynamic memory consistency model prctl calls
for RISC-V. The implementation introduces a single boolean variable to
keep the DTSO state.
Signed-off-by: Christoph Müllner
---
linux-user/riscv/target_prctl.h | 76 -
target/riscv/cpu.c | 5 +++
target/riscv/cpu.h | 1 +
3 files changed, 81 insertions(+), 1 deletion(-)
diff --git a/linux-user/riscv/target_prctl.h b/linux-user/riscv/target_prctl.h
index eb53b31ad5..e54321d872 100644
--- a/linux-user/riscv/target_prctl.h
+++ b/linux-user/riscv/target_prctl.h
@@ -1 +1,75 @@
-/* No special prctl support required. */
+/*
+ * RISC-V specific prctl functions for linux-user
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef RISCV_TARGET_PRCTL_H
+#define RISCV_TARGET_PRCTL_H
+
+static inline void riscv_dtso_set_enable(CPURISCVState *env, bool enable)
+{
+env->dtso_ena = enable;
+}
+
+static inline bool riscv_dtso_is_enabled(CPURISCVState *env)
+{
+return env->dtso_ena;
+}
+
+static abi_long do_prctl_set_memory_consistency_model(CPUArchState *cpu_env,
+ abi_long arg2)
+{
+RISCVCPU *cpu = env_archcpu(cpu_env);
+bool dtso_ena_old = riscv_dtso_is_enabled(cpu_env);
+bool dtso_ena_new;
+bool has_dtso = cpu->cfg.ext_ssdtso;
+
+switch (arg2) {
+case PR_MEMORY_CONSISTENCY_MODEL_RISCV_WMO:
+dtso_ena_new = false;
+break;
+case PR_MEMORY_CONSISTENCY_MODEL_RISCV_TSO:
+ dtso_ena_new = true;
+break;
+default:
+return -TARGET_EINVAL;
+}
+
+/* No change requested. */
+if (dtso_ena_old == dtso_ena_new)
+ return 0;
+
+/* Enabling TSO only works if DTSO is available. */
+if (dtso_ena_new && !has_dtso)
+ return -TARGET_EINVAL;
+
+/* Switchin TSO->WMO is not allowed. */
+if (!dtso_ena_new)
+ return -TARGET_EINVAL;
+
+riscv_dtso_set_enable(cpu_env, dtso_ena_new);
+
+/*
+ * No need to reschedule other threads, because the emulation
+ * of DTSO is fine (from a memory model view) if they are out
+ * of sync until they will eventually reschedule.
+ */
+
+return 0;
+}
+
+#define do_prctl_set_memory_consistency_model \
+do_prctl_set_memory_consistency_model
+
+static abi_long do_prctl_get_memory_consistency_model(CPUArchState *cpu_env)
+{
+if (riscv_dtso_is_enabled(cpu_env))
+ return PR_MEMORY_CONSISTENCY_MODEL_RISCV_TSO;
+
+return PR_MEMORY_CONSISTENCY_MODEL_RISCV_WMO;
+}
+
+#define do_prctl_get_memory_consistency_model \
+do_prctl_get_memory_consistency_model
+
+#endif /* RISCV_TARGET_PRCTL_H */
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ee90c09600..2e2aac73dc 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -922,6 +922,11 @@ static void riscv_cpu_reset_hold(Object *obj)
if (mcc->parent_phases.hold) {
mcc->parent_phases.hold(obj);
}
+#ifdef CONFIG_USER_ONLY
+/* Default is true if Ztso is enabled, false otherwise. */
+env->dtso_ena = cpu->cfg.ext_ztso;
+#endif
+
#ifndef CONFIG_USER_ONLY
env->misa_mxl = mcc->misa_mxl_max;
env->priv = PRV_M;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f52dce78ba..69420b2ae3 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -200,6 +200,7 @@ struct CPUArchState {
#ifdef CONFIG_USER_ONLY
uint32_t elf_flags;
+bool dtso_ena; /* Dynamic TSO enable */
#endif
#ifndef CONFIG_USER_ONLY
--
2.43.0
[RFC PATCH v2 1/4] RISC-V: Add support for Ssdtso
The Ssdtso extension introduces a DTSO field to the {m,s,h}envcfg
register to enable TSO at run-time. Building on top of Ztso support,
this patch treats Ssdtso just like Ztso (always execute in TSO mode),
which should be fine from a correctness perspective.
Similar like Ztso, this is expected to have little overhead on
host machines that operate in TSO mode (e.g. x86).
However, executing the TSO fences on guests without TSO, will
have a negative performance impact, regardless if TSO is enabled
in the guest or not (e.g. running a RV guest with Ssdtso and disabled
DTSO bit on an aarch64 host).
Signed-off-by: Christoph Müllner
---
target/riscv/cpu.c | 8 ++--
target/riscv/cpu_bits.h | 3 +++
target/riscv/cpu_cfg.h | 1 +
target/riscv/csr.c | 14 +++---
target/riscv/translate.c | 2 +-
5 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index b679ecd8c7..ee90c09600 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -174,6 +174,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia),
ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
+ISA_EXT_DATA_ENTRY(ssdtso, PRIV_VERSION_1_12_0, ext_ssdtso),
ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu),
ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
@@ -950,9 +951,11 @@ static void riscv_cpu_reset_hold(Object *obj)
env->two_stage_lookup = false;
env->menvcfg = (cpu->cfg.ext_svpbmt ? MENVCFG_PBMTE : 0) |
- (cpu->cfg.ext_svadu ? MENVCFG_ADUE : 0);
+ (cpu->cfg.ext_svadu ? MENVCFG_ADUE : 0) |
+ (cpu->cfg.ext_ztso && cpu->cfg.ext_ssdtso ? MENVCFG_DTSO :
0);
env->henvcfg = (cpu->cfg.ext_svpbmt ? HENVCFG_PBMTE : 0) |
- (cpu->cfg.ext_svadu ? HENVCFG_ADUE : 0);
+ (cpu->cfg.ext_svadu ? HENVCFG_ADUE : 0) |
+ (cpu->cfg.ext_ztso && cpu->cfg.ext_ssdtso ? HENVCFG_DTSO :
0);
/* Initialized default priorities of local interrupts. */
for (i = 0; i < ARRAY_SIZE(env->miprio); i++) {
@@ -1460,6 +1463,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
MULTI_EXT_CFG_BOOL("zve32f", ext_zve32f, false),
MULTI_EXT_CFG_BOOL("zve64f", ext_zve64f, false),
MULTI_EXT_CFG_BOOL("zve64d", ext_zve64d, false),
+MULTI_EXT_CFG_BOOL("ssdtso", ext_ssdtso, false),
MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
MULTI_EXT_CFG_BOOL("smepmp", ext_smepmp, false),
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index fc2068ee4d..e11191977d 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -744,6 +744,7 @@ typedef enum RISCVException {
#define MENVCFG_CBIE (3UL << 4)
#define MENVCFG_CBCFE BIT(6)
#define MENVCFG_CBZE BIT(7)
+#define MENVCFG_DTSO BIT(8)
#define MENVCFG_ADUE (1ULL << 61)
#define MENVCFG_PBMTE (1ULL << 62)
#define MENVCFG_STCE (1ULL << 63)
@@ -757,11 +758,13 @@ typedef enum RISCVException {
#define SENVCFG_CBIE MENVCFG_CBIE
#define SENVCFG_CBCFE MENVCFG_CBCFE
#define SENVCFG_CBZE MENVCFG_CBZE
+#define SENVCFG_DTSO MENVCFG_DTSO
#define HENVCFG_FIOM MENVCFG_FIOM
#define HENVCFG_CBIE MENVCFG_CBIE
#define HENVCFG_CBCFE MENVCFG_CBCFE
#define HENVCFG_CBZE MENVCFG_CBZE
+#define HENVCFG_DTSO MENVCFG_DTSO
#define HENVCFG_ADUE MENVCFG_ADUE
#define HENVCFG_PBMTE MENVCFG_PBMTE
#define HENVCFG_STCE MENVCFG_STCE
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index afba8ed0b2..606ae5a120 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -73,6 +73,7 @@ struct RISCVCPUConfig {
bool ext_zihpm;
bool ext_ztso;
bool ext_smstateen;
+bool ext_ssdtso;
bool ext_sstc;
bool ext_svadu;
bool ext_svinval;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d4e8ac13b9..c06b674994 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -2058,7 +2058,9 @@ static RISCVException write_menvcfg(CPURISCVState *env,
int csrno,
target_ulong val)
{
const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
-uint64_t mask = MENVCFG_FIOM | MENVCFG_CBIE | MENVCFG_CBCFE | MENVCFG_CBZE;
+uint64_t mask = MENVCFG_FI
[RFC PATCH v2 0/4] RISC-V: Add dynamic TSO support
The upcoming RISC-V Ssdtso specification introduces a bit in the senvcfg CSR to switch the memory consistency model of user mode at run-time from RVWMO to TSO. The active consistency model can therefore be switched on a per-hart base and managed by the kernel on a per-process base. This patch treats Ssdtso similar to TSO, i.e., the guest is always being executed in TSO mode, which simplifies the implementation a lot while still being correct. The downside is, that we have a performance penalty on hosts that don't run with TSO. This patch implements basic Ssdtso support and adds a prctl API on top so that user-space processes can switch to a stronger memory consistency model (than the kernel was written for) at run-time. This series is based on the third draft of the Ssdtso specification which can be found here: https://github.com/riscv/riscv-ssdtso/releases/tag/v1.0-draft3 Note, that the Ssdtso specification is in development state (i.e., not frozen or even ratified) which is also the reason why this series is marked as RFC. This series saw the following changes since v1: * Adding compatibility with Ztso (spec change in draft 3) * Use PR_MEMORY_CONSISTENCY_MODEL* instead of numeric constants This series is based on riscv-to-apply.next with two series on top: * Sync hwprobe keys with kernel https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg01293.html * Add Ztso extension https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg01296.html This patchset can also be found in this GitHub branch: https://github.com/cmuellner/qemu/tree/ssdtso-v2 A Linux implementation of DTSO can be found in this GitHub branch: https://github.com/cmuellner/linux/tree/ssdtso-v2 Christoph Müllner (4): RISC-V: Add support for Ssdtso linux-user/riscv: Add Ssdtso extension to hwprobe linux-user/prctl: Add dynamic memory consistency model prctl API linux-user/riscv: Implement dynamic memory consistency model support linux-user/riscv/target_prctl.h | 76 - linux-user/syscall.c| 20 + target/riscv/cpu.c | 13 +- target/riscv/cpu.h | 1 + target/riscv/cpu_bits.h | 3 ++ target/riscv/cpu_cfg.h | 1 + target/riscv/csr.c | 14 -- target/riscv/translate.c| 2 +- 8 files changed, 123 insertions(+), 7 deletions(-) -- 2.43.0
[RFC PATCH v2 3/4] linux-user/prctl: Add dynamic memory consistency model prctl API
This patch implements the prctl calls to set and get the current memory consistency model. This patch does not implement any real functionality but just defines the relevant hooks, where target code take over. Signed-off-by: Christoph Müllner --- linux-user/syscall.c | 17 + 1 file changed, 17 insertions(+) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index bf0d66b8a8..cf0845a074 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -6309,6 +6309,12 @@ abi_long do_arch_prctl(CPUX86State *env, int code, abi_ulong addr) # define PR_SME_VL_LEN_MASK 0x # define PR_SME_VL_INHERIT (1 << 17) #endif +#ifndef PR_SET_MEMORY_CONSISTENCY_MODEL +# define PR_SET_MEMORY_CONSISTENCY_MODEL71 +# define PR_GET_MEMORY_CONSISTENCY_MODEL72 +# define PR_MEMORY_CONSISTENCY_MODEL_RISCV_WMO 1 +# define PR_MEMORY_CONSISTENCY_MODEL_RISCV_TSO 2 +#endif #include "target_prctl.h" @@ -6355,6 +6361,12 @@ static abi_long do_prctl_inval1(CPUArchState *env, abi_long arg2) #ifndef do_prctl_sme_set_vl #define do_prctl_sme_set_vl do_prctl_inval1 #endif +#ifndef do_prctl_set_memory_consistency_model +#define do_prctl_set_memory_consistency_model do_prctl_inval1 +#endif +#ifndef do_prctl_get_memory_consistency_model +#define do_prctl_get_memory_consistency_model do_prctl_inval0 +#endif static abi_long do_prctl(CPUArchState *env, abi_long option, abi_long arg2, abi_long arg3, abi_long arg4, abi_long arg5) @@ -6481,6 +6493,11 @@ static abi_long do_prctl(CPUArchState *env, abi_long option, abi_long arg2, /* Disable to prevent the target disabling stuff we need. */ return -TARGET_EINVAL; +case PR_SET_MEMORY_CONSISTENCY_MODEL: +return do_prctl_set_memory_consistency_model(env, arg2); +case PR_GET_MEMORY_CONSISTENCY_MODEL: + return do_prctl_get_memory_consistency_model(env); + default: qemu_log_mask(LOG_UNIMP, "Unsupported prctl: " TARGET_ABI_FMT_ld "\n", option); -- 2.43.0
[PATCH 2/2] linux-user/riscv: Add Ztso extension to hwprobe
This patch exposes Ztso via hwprobe in QEMU's user space emulator. Signed-off-by: Christoph Müllner --- linux-user/syscall.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 3ba20f99ad..24fa11d946 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -8826,6 +8826,7 @@ static int do_getdents64(abi_long dirfd, abi_long arg2, abi_long count) #defineRISCV_HWPROBE_EXT_ZVFH (1 << 30) #defineRISCV_HWPROBE_EXT_ZVFHMIN (1 << 31) #defineRISCV_HWPROBE_EXT_ZFA (1ULL << 32) +#defineRISCV_HWPROBE_EXT_ZTSO (1ULL << 33) #defineRISCV_HWPROBE_EXT_ZACAS (1ULL << 34) #defineRISCV_HWPROBE_EXT_ZICOND(1ULL << 35) @@ -8940,6 +8941,8 @@ static void risc_hwprobe_fill_pairs(CPURISCVState *env, RISCV_HWPROBE_EXT_ZVFHMIN : 0; value |= cfg->ext_zfa ? RISCV_HWPROBE_EXT_ZFA : 0; +value |= cfg->ext_ztso ? + RISCV_HWPROBE_EXT_ZTSO : 0; value |= cfg->ext_zacas ? RISCV_HWPROBE_EXT_ZACAS : 0; value |= cfg->ext_zicond ? -- 2.43.0
[PATCH 0/2] RISC-V: Add Ztso extension
The first patch of this series picks up an earlier v2 Ztso patch from Palmer, which can be found here: https://patchwork.kernel.org/project/qemu-devel/patch/20220917072635.11616-1-pal...@rivosinc.com/ This patch did not apply cleanly but the necessary changes were trivial. There was a request to extend the commit message, which is part of the posted patch of this series. As this patch was reviewed a year ago, I believe it could be merged. The second patch simply exposes Ztso via hwprobe. Relevant in this context might be also, that Richard's patch to improve TCG's memory barrier selection depending on host and guest memory ordering landed in June 2023: https://lore.kernel.org/all/a313b36b-dcc1-f812-ccbd-afed1cbd5...@linaro.org/T/ The first patch was already sent as part of an RFC series for Ssdtso: https://lists.nongnu.org/archive/html/qemu-devel/2023-11/msg02962.html Since I don't want to keep this patch until the ratification of Ssdtso, I would like to get this merged independent of Ssdtso. This series is based on today's riscv-to-apply.next with my other series that adds the new hwprobe keys (https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg01293.html). This series can also be found here: https://github.com/cmuellner/qemu/tree/ztso Christoph Müllner (1): linux-user/riscv: Add Ztso extension to hwprobe Palmer Dabbelt (1): RISC-V: Add support for Ztso linux-user/syscall.c| 3 +++ target/riscv/cpu.c | 2 ++ target/riscv/cpu_cfg.h | 1 + target/riscv/insn_trans/trans_rva.c.inc | 11 --- target/riscv/insn_trans/trans_rvi.c.inc | 16 ++-- target/riscv/insn_trans/trans_rvv.c.inc | 20 target/riscv/translate.c| 3 +++ 7 files changed, 51 insertions(+), 5 deletions(-) -- 2.43.0
[PATCH 1/2] RISC-V: Add support for Ztso
From: Palmer Dabbelt
The Ztso extension is already ratified, this adds it as a CPU property
and adds various fences throughout the port in order to allow TSO
targets to function on weaker hosts. We need no fences for AMOs as
they're already SC, the places we need barriers are described.
These fences are placed in the RISC-V backend rather than TCG as is
planned for x86-on-arm64 because RISC-V allows heterogeneous (and
likely soon dynamic) hart memory models.
Reviewed-by: Daniel Henrique Barboza
Signed-off-by: Palmer Dabbelt
Signed-off-by: Christoph Müllner
---
target/riscv/cpu.c | 2 ++
target/riscv/cpu_cfg.h | 1 +
target/riscv/insn_trans/trans_rva.c.inc | 11 ---
target/riscv/insn_trans/trans_rvi.c.inc | 16 ++--
target/riscv/insn_trans/trans_rvv.c.inc | 20
target/riscv/translate.c| 3 +++
6 files changed, 48 insertions(+), 5 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1b8d001d23..b679ecd8c7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -143,6 +143,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zksed, PRIV_VERSION_1_12_0, ext_zksed),
ISA_EXT_DATA_ENTRY(zksh, PRIV_VERSION_1_12_0, ext_zksh),
ISA_EXT_DATA_ENTRY(zkt, PRIV_VERSION_1_12_0, ext_zkt),
+ISA_EXT_DATA_ENTRY(ztso, PRIV_VERSION_1_12_0, ext_ztso),
ISA_EXT_DATA_ENTRY(zvbb, PRIV_VERSION_1_12_0, ext_zvbb),
ISA_EXT_DATA_ENTRY(zvbc, PRIV_VERSION_1_12_0, ext_zvbc),
ISA_EXT_DATA_ENTRY(zve32f, PRIV_VERSION_1_10_0, ext_zve32f),
@@ -1488,6 +1489,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
MULTI_EXT_CFG_BOOL("zksed", ext_zksed, false),
MULTI_EXT_CFG_BOOL("zksh", ext_zksh, false),
MULTI_EXT_CFG_BOOL("zkt", ext_zkt, false),
+MULTI_EXT_CFG_BOOL("ztso", ext_ztso, false),
MULTI_EXT_CFG_BOOL("zdinx", ext_zdinx, false),
MULTI_EXT_CFG_BOOL("zfinx", ext_zfinx, false),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 833bf58217..afba8ed0b2 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -71,6 +71,7 @@ struct RISCVCPUConfig {
bool ext_zihintntl;
bool ext_zihintpause;
bool ext_zihpm;
+bool ext_ztso;
bool ext_smstateen;
bool ext_sstc;
bool ext_svadu;
diff --git a/target/riscv/insn_trans/trans_rva.c.inc
b/target/riscv/insn_trans/trans_rva.c.inc
index 267930e5bc..4a9e4591d1 100644
--- a/target/riscv/insn_trans/trans_rva.c.inc
+++ b/target/riscv/insn_trans/trans_rva.c.inc
@@ -40,7 +40,11 @@ static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp
mop)
tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL);
}
tcg_gen_qemu_ld_tl(load_val, src1, ctx->mem_idx, mop);
-if (a->aq) {
+/*
+ * TSO defines AMOs as acquire+release-RCsc, but does not define LR/SC as
+ * AMOs. Instead treat them like loads.
+ */
+if (a->aq || ctx->ztso) {
tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ);
}
@@ -76,9 +80,10 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp
mop)
gen_set_label(l1);
/*
* Address comparison failure. However, we still need to
- * provide the memory barrier implied by AQ/RL.
+ * provide the memory barrier implied by AQ/RL/TSO.
*/
-tcg_gen_mb(TCG_MO_ALL + a->aq * TCG_BAR_LDAQ + a->rl * TCG_BAR_STRL);
+TCGBar bar_strl = (ctx->ztso || a->rl) ? TCG_BAR_STRL : 0;
+tcg_gen_mb(TCG_MO_ALL + a->aq * TCG_BAR_LDAQ + bar_strl);
gen_set_gpr(ctx, a->rd, tcg_constant_tl(1));
gen_set_label(l2);
diff --git a/target/riscv/insn_trans/trans_rvi.c.inc
b/target/riscv/insn_trans/trans_rvi.c.inc
index faf6d65064..ad40d3e87f 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -266,12 +266,20 @@ static bool gen_load_i128(DisasContext *ctx, arg_lb *a,
MemOp memop)
static bool gen_load(DisasContext *ctx, arg_lb *a, MemOp memop)
{
+bool out;
+
decode_save_opc(ctx);
if (get_xl(ctx) == MXL_RV128) {
-return gen_load_i128(ctx, a, memop);
+out = gen_load_i128(ctx, a, memop);
} else {
-return gen_load_tl(ctx, a, memop);
+out = gen_load_tl(ctx, a, memop);
+}
+
+if (ctx->ztso) {
+tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ);
}
+
+return out;
}
static bool trans_lb(DisasContext *ctx, arg_lb *a)
@@ -328,6 +336,10 @@ static bool gen_store_tl(DisasContext *ctx, arg_sb *a,
MemOp memop)
TCGv addr = get_address(ctx, a->rs1, a->imm);
TCGv data = get_gpr(ctx, a->rs2, EXT_NONE);
+if (ctx->ztso) {
+tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL);
+}
+
tcg_gen_qemu_st_tl(data, addr, ctx->mem_idx, memop);
return true;
}
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 9e101ab434..742008f58b
[PATCH 1/2] linux-user/riscv: Add Zicboz extensions to hwprobe
Upstream Linux recently added RISC-V Zicboz support to the hwprobe API. This patch introduces this for QEMU's user space emulator. Signed-off-by: Christoph Müllner --- linux-user/syscall.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index e384e14248..43467c9707 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -8799,6 +8799,7 @@ static int do_getdents64(abi_long dirfd, abi_long arg2, abi_long count) #define RISCV_HWPROBE_EXT_ZBA (1 << 3) #define RISCV_HWPROBE_EXT_ZBB (1 << 4) #define RISCV_HWPROBE_EXT_ZBS (1 << 5) +#define RISCV_HWPROBE_EXT_ZICBOZ (1 << 6) #define RISCV_HWPROBE_KEY_CPUPERF_0 5 #define RISCV_HWPROBE_MISALIGNED_UNKNOWN (0 << 0) @@ -8857,6 +8858,8 @@ static void risc_hwprobe_fill_pairs(CPURISCVState *env, RISCV_HWPROBE_EXT_ZBB : 0; value |= cfg->ext_zbs ? RISCV_HWPROBE_EXT_ZBS : 0; +value |= cfg->ext_zicboz ? + RISCV_HWPROBE_EXT_ZICBOZ : 0; __put_user(value, >value); break; case RISCV_HWPROBE_KEY_CPUPERF_0: -- 2.43.0
[PATCH 2/2] linux-user/riscv: Sync hwprobe keys with Linux
Upstream Linux recently added many additional keys to the hwprobe API. This patch adds support for all of them with the exception of Ztso, which is currently not supported in QEMU. Signed-off-by: Christoph Müllner --- linux-user/syscall.c | 98 1 file changed, 91 insertions(+), 7 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 43467c9707..3ba20f99ad 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -8793,13 +8793,41 @@ static int do_getdents64(abi_long dirfd, abi_long arg2, abi_long count) #define RISCV_HWPROBE_BASE_BEHAVIOR_IMA (1 << 0) #define RISCV_HWPROBE_KEY_IMA_EXT_0 4 -#define RISCV_HWPROBE_IMA_FD (1 << 0) -#define RISCV_HWPROBE_IMA_C(1 << 1) -#define RISCV_HWPROBE_IMA_V(1 << 2) -#define RISCV_HWPROBE_EXT_ZBA (1 << 3) -#define RISCV_HWPROBE_EXT_ZBB (1 << 4) -#define RISCV_HWPROBE_EXT_ZBS (1 << 5) -#define RISCV_HWPROBE_EXT_ZICBOZ (1 << 6) +#defineRISCV_HWPROBE_IMA_FD(1 << 0) +#defineRISCV_HWPROBE_IMA_C (1 << 1) +#defineRISCV_HWPROBE_IMA_V (1 << 2) +#defineRISCV_HWPROBE_EXT_ZBA (1 << 3) +#defineRISCV_HWPROBE_EXT_ZBB (1 << 4) +#defineRISCV_HWPROBE_EXT_ZBS (1 << 5) +#defineRISCV_HWPROBE_EXT_ZICBOZ(1 << 6) +#defineRISCV_HWPROBE_EXT_ZBC (1 << 7) +#defineRISCV_HWPROBE_EXT_ZBKB (1 << 8) +#defineRISCV_HWPROBE_EXT_ZBKC (1 << 9) +#defineRISCV_HWPROBE_EXT_ZBKX (1 << 10) +#defineRISCV_HWPROBE_EXT_ZKND (1 << 11) +#defineRISCV_HWPROBE_EXT_ZKNE (1 << 12) +#defineRISCV_HWPROBE_EXT_ZKNH (1 << 13) +#defineRISCV_HWPROBE_EXT_ZKSED (1 << 14) +#defineRISCV_HWPROBE_EXT_ZKSH (1 << 15) +#defineRISCV_HWPROBE_EXT_ZKT (1 << 16) +#defineRISCV_HWPROBE_EXT_ZVBB (1 << 17) +#defineRISCV_HWPROBE_EXT_ZVBC (1 << 18) +#defineRISCV_HWPROBE_EXT_ZVKB (1 << 19) +#defineRISCV_HWPROBE_EXT_ZVKG (1 << 20) +#defineRISCV_HWPROBE_EXT_ZVKNED(1 << 21) +#defineRISCV_HWPROBE_EXT_ZVKNHA(1 << 22) +#defineRISCV_HWPROBE_EXT_ZVKNHB(1 << 23) +#defineRISCV_HWPROBE_EXT_ZVKSED(1 << 24) +#defineRISCV_HWPROBE_EXT_ZVKSH (1 << 25) +#defineRISCV_HWPROBE_EXT_ZVKT (1 << 26) +#defineRISCV_HWPROBE_EXT_ZFH (1 << 27) +#defineRISCV_HWPROBE_EXT_ZFHMIN(1 << 28) +#defineRISCV_HWPROBE_EXT_ZIHINTNTL (1 << 29) +#defineRISCV_HWPROBE_EXT_ZVFH (1 << 30) +#defineRISCV_HWPROBE_EXT_ZVFHMIN (1 << 31) +#defineRISCV_HWPROBE_EXT_ZFA (1ULL << 32) +#defineRISCV_HWPROBE_EXT_ZACAS (1ULL << 34) +#defineRISCV_HWPROBE_EXT_ZICOND(1ULL << 35) #define RISCV_HWPROBE_KEY_CPUPERF_0 5 #define RISCV_HWPROBE_MISALIGNED_UNKNOWN (0 << 0) @@ -8860,6 +,62 @@ static void risc_hwprobe_fill_pairs(CPURISCVState *env, RISCV_HWPROBE_EXT_ZBS : 0; value |= cfg->ext_zicboz ? RISCV_HWPROBE_EXT_ZICBOZ : 0; +value |= cfg->ext_zbc ? + RISCV_HWPROBE_EXT_ZBC : 0; +value |= cfg->ext_zbkb ? + RISCV_HWPROBE_EXT_ZBKB : 0; +value |= cfg->ext_zbkc ? + RISCV_HWPROBE_EXT_ZBKC : 0; +value |= cfg->ext_zbkx ? + RISCV_HWPROBE_EXT_ZBKX : 0; +value |= cfg->ext_zknd ? + RISCV_HWPROBE_EXT_ZKND : 0; +value |= cfg->ext_zkne ? + RISCV_HWPROBE_EXT_ZKNE : 0; +value |= cfg->ext_zknh ? + RISCV_HWPROBE_EXT_ZKNH : 0; +value |= cfg->ext_zksed ? + RISCV_HWPROBE_EXT_ZKSED : 0; +value |= cfg->ext_zksh ? + RISCV_HWPROBE_EXT_ZKSH : 0; +value |= cfg->ext_zkt ? + RISCV_HWPROBE_EXT_ZKT : 0; +value |= cfg->ext_zvbb ? + RISCV_HWPROBE_EXT_ZVBB : 0; +value |= cfg->ext_zvbc ? +
[PATCH 0/2] linux-user/riscv: Sync hwprobe keys with kernel
This series syncs the hwprobe keys with those available in the upstream kernel repository with the exception of Ztso, which is not supported in QEMU as of now. The first patch is a resend (sent on Nov 27), as it should have been picked up on Dec 6, but seems to got lost. Christoph Müllner (2): linux-user/riscv: Add Zicboz extensions to hwprobe linux-user/riscv: Sync hwprobe keys with Linux linux-user/syscall.c | 99 +--- 1 file changed, 93 insertions(+), 6 deletions(-) -- 2.43.0
Re: [PATCH v2] linux-user/riscv: Add Zicboz extensions to hwprobe
On Wed, Dec 6, 2023 at 1:57 AM Alistair Francis wrote: > > On Mon, Nov 27, 2023 at 12:37 AM Christoph Muellner > wrote: > > > > From: Christoph Müllner > > > > Upstream Linux recently added RISC-V Zicboz support to the hwprobe API. > > This patch introduces this for QEMU's user space emulator. > > > > Signed-off-by: Christoph Müllner > > Thanks! > > Applied to riscv-to-apply.next I just saw that this did not land yet on master. I also noticed that this patch is not in https://github.com/alistair23/qemu/tree/riscv-to-apply.next or https://github.com/alistair23/qemu/commits/riscv-to-apply.for-upstream. Was there some issue with the patch? Meanwhile a lot of additional extensions got defined in the hwprobe interface (patches are already merged in the kernel). I'll send out a patch for these in a few minutes and include this patch here as well. BR Christoph > > Alistair > > > --- > > linux-user/syscall.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > > index 65ac3ac796..2f9a1c5279 100644 > > --- a/linux-user/syscall.c > > +++ b/linux-user/syscall.c > > @@ -8799,6 +8799,7 @@ static int do_getdents64(abi_long dirfd, abi_long > > arg2, abi_long count) > > #define RISCV_HWPROBE_EXT_ZBA (1 << 3) > > #define RISCV_HWPROBE_EXT_ZBB (1 << 4) > > #define RISCV_HWPROBE_EXT_ZBS (1 << 5) > > +#define RISCV_HWPROBE_EXT_ZICBOZ (1 << 6) > > > > #define RISCV_HWPROBE_KEY_CPUPERF_0 5 > > #define RISCV_HWPROBE_MISALIGNED_UNKNOWN (0 << 0) > > @@ -8855,6 +8856,8 @@ static void risc_hwprobe_fill_pairs(CPURISCVState > > *env, > > RISCV_HWPROBE_EXT_ZBB : 0; > > value |= cfg->ext_zbs ? > > RISCV_HWPROBE_EXT_ZBS : 0; > > +value |= cfg->ext_zicboz ? > > + RISCV_HWPROBE_EXT_ZICBOZ : 0; > > __put_user(value, >value); > > break; > > case RISCV_HWPROBE_KEY_CPUPERF_0: > > -- > > 2.41.0 > > > >
Re: [PATCH v2 2/2] target/riscv: Support xtheadmaee for thead-c906
On Mon, Feb 5, 2024 at 3:42 AM Alistair Francis wrote:
>
> On Sun, Feb 4, 2024 at 3:44 PM LIU Zhiwei
> wrote:
> >
> > This patch set fix the regression on kernel pointed by Björn Töpel in
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg1018232.html.
> >
> > thead-c906 uses some flags in pte [60-63] bits. It has history reasons that
> > SVPBMT didn't exist when thead-c906 came to wotrld. We named this feature as
> > xtheadmaee[1]. this feature is controlled by an custom CSR named mxstatus,
> > whose maee field encodes whether enable the pte [60-63] bits.
> >
> > [1]:https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadmaee.adoc
> >
> > Signed-off-by: LIU Zhiwei
> > ---
> > v1->v2:
> > 1) Remove mxstatus user mode access
> > 2) Add reference documentation to the commit log
> > ---
> > target/riscv/cpu.c | 6
> > target/riscv/cpu.h | 9 ++
> > target/riscv/cpu_bits.h| 6
> > target/riscv/cpu_cfg.h | 4 ++-
> > target/riscv/cpu_helper.c | 25 ---
> > target/riscv/meson.build | 1 +
> > target/riscv/tcg/tcg-cpu.c | 7 +++-
> > target/riscv/xthead_csr.c | 65 ++
> > 8 files changed, 110 insertions(+), 13 deletions(-)
> > create mode 100644 target/riscv/xthead_csr.c
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 2dcbc9ff32..bfdbb0539a 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -171,6 +171,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
> > ISA_EXT_DATA_ENTRY(xtheadmemidx, PRIV_VERSION_1_11_0,
> > ext_xtheadmemidx),
> > ISA_EXT_DATA_ENTRY(xtheadmempair, PRIV_VERSION_1_11_0,
> > ext_xtheadmempair),
> > ISA_EXT_DATA_ENTRY(xtheadsync, PRIV_VERSION_1_11_0, ext_xtheadsync),
> > +ISA_EXT_DATA_ENTRY(xtheadmaee, PRIV_VERSION_1_11_0, ext_xtheadmaee),
> > ISA_EXT_DATA_ENTRY(xventanacondops, PRIV_VERSION_1_12_0,
> > ext_XVentanaCondOps),
> >
> > DEFINE_PROP_END_OF_LIST(),
> > @@ -506,6 +507,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
> >
> > cpu->cfg.mvendorid = THEAD_VENDOR_ID;
> > #ifndef CONFIG_USER_ONLY
> > +cpu->cfg.ext_xtheadmaee = true;
> > set_satp_mode_max_supported(cpu, VM_1_10_SV39);
> > #endif
> >
> > @@ -949,6 +951,9 @@ static void riscv_cpu_reset_hold(Object *obj)
> > }
> >
> > pmp_unlock_entries(env);
> > +if (riscv_cpu_cfg(env)->ext_xtheadmaee) {
> > +env->th_mxstatus |= TH_MXSTATUS_MAEE;
> > +}
> > #endif
> > env->xl = riscv_cpu_mxl(env);
> > riscv_cpu_update_mask(env);
> > @@ -1439,6 +1444,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_vendor_exts[]
> > = {
> > MULTI_EXT_CFG_BOOL("xtheadmemidx", ext_xtheadmemidx, false),
> > MULTI_EXT_CFG_BOOL("xtheadmempair", ext_xtheadmempair, false),
> > MULTI_EXT_CFG_BOOL("xtheadsync", ext_xtheadsync, false),
> > +MULTI_EXT_CFG_BOOL("xtheadmaee", ext_xtheadmaee, false),
> > MULTI_EXT_CFG_BOOL("xventanacondops", ext_XVentanaCondOps, false),
> >
> > DEFINE_PROP_END_OF_LIST(),
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 5f3955c38d..1bacf40355 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -412,6 +412,14 @@ struct CPUArchState {
> > target_ulong cur_pmmask;
> > target_ulong cur_pmbase;
> >
> > +union {
> > +/* Custom CSR for Xuantie CPU */
> > +struct {
> > +#ifndef CONFIG_USER_ONLY
> > +target_ulong th_mxstatus;
> > +#endif
> > +};
> > +};
> > /* Fields from here on are preserved across CPU reset. */
> > QEMUTimer *stimer; /* Internal timer for S-mode interrupt */
> > QEMUTimer *vstimer; /* Internal timer for VS-mode interrupt */
> > @@ -799,6 +807,7 @@ void riscv_add_satp_mode_properties(Object *obj);
> > bool riscv_cpu_accelerator_compatible(RISCVCPU *cpu);
> >
> > /* CSR function table */
> > +extern riscv_csr_operations th_csr_ops[CSR_TABLE_SIZE];
> > extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
> >
> > extern const bool valid_vm_1_10_32[], valid_vm_1_10_64[];
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index e116f6c252..67ebb1cefe 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -897,4 +897,10 @@ typedef enum RISCVException {
> > /* JVT CSR bits */
> > #define JVT_MODE 0x3F
> > #define JVT_BASE (~0x3F)
> > +
> > +/* Xuantie custom CSRs */
> > +#define CSR_TH_MXSTATUS 0x7c0
> > +
> > +#define TH_MXSTATUS_MAEE_SHIFT 21
> > +#define TH_MXSTATUS_MAEE(0x1 << TH_MXSTATUS_MAEE_SHIFT)
> > #endif
> > diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> > index 780ae6ef17..3735c69fd6 100644
> > --- a/target/riscv/cpu_cfg.h
> > +++ b/target/riscv/cpu_cfg.h
> > @@ -136,6 +136,7 @@ struct RISCVCPUConfig {
> > bool ext_xtheadmemidx;
> > bool ext_xtheadmempair;
> > bool ext_xtheadsync;
> > +bool
Re: [PATCH 2/2] target/riscv: Support xtheadmaee for thead-c906
On Tue, Jan 30, 2024 at 12:12 PM LIU Zhiwei
wrote:
>
> thead-c906 uses some flags in pte [60-63] bits. It has history reasons that
> SVPBMT didn't exist when thead-c906 came to world.
>
> We named this feature as xtheadmaee. this feature is controlled by an custom
> CSR named mxstatus, whose maee field encodes whether enable the pte [60-63]
> bits.
>
> The sections "5.2.2.1 Page table structure" and "15.1.7.1 M-mode extension
> status register (MXSTATUS)" in document[1] give the detailed information
> about its design.
I would prefer if we would not define an extension like XTheadMaee
without a specification.
The linked document defines the bit MAEE in a custom CSR, but the
scope of XTheadMaee
is not clearly defined (the term XTheadMaee is not even part of the PDF).
We have all the XThead* extensions well described here:
https://github.com/T-head-Semi/thead-extension-spec/tree/master
And it would not be much effort to add XTheadMaee there as well.
For those who don't know the context of this patch, here is the c906
boot regression report from Björn:
https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg04766.html
BR
Christoph
Christoph
>
> [1]:https://occ-intl-prod.oss-ap-southeast-1.aliyuncs.com/resource//1699265191641/XuanTie-Openc906-UserManual.pdf
> Signed-off-by: LIU Zhiwei
> ---
> target/riscv/cpu.c | 6
> target/riscv/cpu.h | 9 ++
> target/riscv/cpu_bits.h| 6
> target/riscv/cpu_cfg.h | 4 ++-
> target/riscv/cpu_helper.c | 25 ---
> target/riscv/meson.build | 1 +
> target/riscv/tcg/tcg-cpu.c | 9 ++
> target/riscv/xthead_csr.c | 63 ++
> 8 files changed, 105 insertions(+), 18 deletions(-)
> create mode 100644 target/riscv/xthead_csr.c
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 2dcbc9ff32..bfdbb0539a 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -171,6 +171,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
> ISA_EXT_DATA_ENTRY(xtheadmemidx, PRIV_VERSION_1_11_0, ext_xtheadmemidx),
> ISA_EXT_DATA_ENTRY(xtheadmempair, PRIV_VERSION_1_11_0,
> ext_xtheadmempair),
> ISA_EXT_DATA_ENTRY(xtheadsync, PRIV_VERSION_1_11_0, ext_xtheadsync),
> +ISA_EXT_DATA_ENTRY(xtheadmaee, PRIV_VERSION_1_11_0, ext_xtheadmaee),
> ISA_EXT_DATA_ENTRY(xventanacondops, PRIV_VERSION_1_12_0,
> ext_XVentanaCondOps),
>
> DEFINE_PROP_END_OF_LIST(),
> @@ -506,6 +507,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
>
> cpu->cfg.mvendorid = THEAD_VENDOR_ID;
> #ifndef CONFIG_USER_ONLY
> +cpu->cfg.ext_xtheadmaee = true;
> set_satp_mode_max_supported(cpu, VM_1_10_SV39);
> #endif
>
> @@ -949,6 +951,9 @@ static void riscv_cpu_reset_hold(Object *obj)
> }
>
> pmp_unlock_entries(env);
> +if (riscv_cpu_cfg(env)->ext_xtheadmaee) {
> +env->th_mxstatus |= TH_MXSTATUS_MAEE;
> +}
> #endif
> env->xl = riscv_cpu_mxl(env);
> riscv_cpu_update_mask(env);
> @@ -1439,6 +1444,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_vendor_exts[] = {
> MULTI_EXT_CFG_BOOL("xtheadmemidx", ext_xtheadmemidx, false),
> MULTI_EXT_CFG_BOOL("xtheadmempair", ext_xtheadmempair, false),
> MULTI_EXT_CFG_BOOL("xtheadsync", ext_xtheadsync, false),
> +MULTI_EXT_CFG_BOOL("xtheadmaee", ext_xtheadmaee, false),
> MULTI_EXT_CFG_BOOL("xventanacondops", ext_XVentanaCondOps, false),
>
> DEFINE_PROP_END_OF_LIST(),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 5f3955c38d..1bacf40355 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -412,6 +412,14 @@ struct CPUArchState {
> target_ulong cur_pmmask;
> target_ulong cur_pmbase;
>
> +union {
> +/* Custom CSR for Xuantie CPU */
> +struct {
> +#ifndef CONFIG_USER_ONLY
> +target_ulong th_mxstatus;
> +#endif
> +};
> +};
> /* Fields from here on are preserved across CPU reset. */
> QEMUTimer *stimer; /* Internal timer for S-mode interrupt */
> QEMUTimer *vstimer; /* Internal timer for VS-mode interrupt */
> @@ -799,6 +807,7 @@ void riscv_add_satp_mode_properties(Object *obj);
> bool riscv_cpu_accelerator_compatible(RISCVCPU *cpu);
>
> /* CSR function table */
> +extern riscv_csr_operations th_csr_ops[CSR_TABLE_SIZE];
> extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
>
> extern const bool valid_vm_1_10_32[], valid_vm_1_10_64[];
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index e116f6c252..67ebb1cefe 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -897,4 +897,10 @@ typedef enum RISCVException {
> /* JVT CSR bits */
> #define JVT_MODE 0x3F
> #define JVT_BASE (~0x3F)
> +
> +/* Xuantie custom CSRs */
> +#define CSR_TH_MXSTATUS 0x7c0
> +
> +#define TH_MXSTATUS_MAEE_SHIFT 21
> +#define TH_MXSTATUS_MAEE(0x1 << TH_MXSTATUS_MAEE_SHIFT)
> #endif
> diff --git
Re: [PATCH for 8.2] target/riscv: Fix th.dcache.cval1 priviledge check
On Fri, Dec 8, 2023 at 10:44 AM LIU Zhiwei wrote: > > According to the specification, the th.dcache.cvall1 can be executed > under all priviledges. > The specification about xtheadcmo located in, > https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadcmo/dcache_cval1.adoc > > Signed-off-by: LIU Zhiwei Reviewed-by: Christoph Muellner > --- > target/riscv/insn_trans/trans_xthead.c.inc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/riscv/insn_trans/trans_xthead.c.inc > b/target/riscv/insn_trans/trans_xthead.c.inc > index 810d76665a..dbb6411239 100644 > --- a/target/riscv/insn_trans/trans_xthead.c.inc > +++ b/target/riscv/insn_trans/trans_xthead.c.inc > @@ -296,7 +296,7 @@ NOP_PRIVCHECK(th_dcache_csw, REQUIRE_XTHEADCMO, > REQUIRE_PRIV_MS) > NOP_PRIVCHECK(th_dcache_cisw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS) > NOP_PRIVCHECK(th_dcache_isw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS) > NOP_PRIVCHECK(th_dcache_cpal1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS) > -NOP_PRIVCHECK(th_dcache_cval1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS) > +NOP_PRIVCHECK(th_dcache_cval1, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MSU) > > NOP_PRIVCHECK(th_icache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS) > NOP_PRIVCHECK(th_icache_ialls, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS) > -- > 2.17.1 >
Re: [PATCH v2] linux-user/riscv: Add Zicboz extensions to hwprobe
On Fri, Nov 24, 2023 at 5:59 PM Andrew Jones wrote: > > On Thu, Nov 23, 2023 at 07:12:59PM +0100, Christoph Muellner wrote: > > From: Christoph Müllner > > > > Upstream Linux recently added RISC-V Zicboz support to the hwprobe API. > > This patch introduces this for QEMU's user space emulator. > > > > Signed-off-by: Christoph Müllner > > --- > > linux-user/syscall.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > > index 65ac3ac796..2f9a1c5279 100644 > > --- a/linux-user/syscall.c > > +++ b/linux-user/syscall.c > > @@ -8799,6 +8799,7 @@ static int do_getdents64(abi_long dirfd, abi_long > > arg2, abi_long count) > > #define RISCV_HWPROBE_EXT_ZBA (1 << 3) > > #define RISCV_HWPROBE_EXT_ZBB (1 << 4) > > #define RISCV_HWPROBE_EXT_ZBS (1 << 5) > > +#define RISCV_HWPROBE_EXT_ZICBOZ (1 << 6) > > > > #define RISCV_HWPROBE_KEY_CPUPERF_0 5 > > #define RISCV_HWPROBE_MISALIGNED_UNKNOWN (0 << 0) > > @@ -8855,6 +8856,8 @@ static void risc_hwprobe_fill_pairs(CPURISCVState > > *env, > > RISCV_HWPROBE_EXT_ZBB : 0; > > value |= cfg->ext_zbs ? > > RISCV_HWPROBE_EXT_ZBS : 0; > > +value |= cfg->ext_zicboz ? > > + RISCV_HWPROBE_EXT_ZICBOZ : 0; > > __put_user(value, >value); > > break; > > case RISCV_HWPROBE_KEY_CPUPERF_0: > > -- > > 2.41.0 > > > > > > We should also add support for getting the block size with > RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE. Hi Andrew, this is already upstream, just the EXT_ZICBOZ is missing: commit 301c65f49f9602f39b9f3ce0ad9ff70d4bda7226 Author: Palmer Dabbelt Date: Fri Nov 10 09:37:16 2023 -0800 linux-user/riscv: Add Zicboz block size to hwprobe Support for probing the Zicboz block size landed in Linux 6.6, which was released a few weeks ago. This provides the user-configured block size when Zicboz is enabled. > > Thanks, > drew
Re: [PATCH] linux-user/riscv: Add Zicboz extensions to hwprobe
On Thu, Nov 23, 2023 at 7:01 PM Christoph Muellner wrote: > > From: Christoph Müllner > > Upstream Linux recently added RISC-V Zicboz support to the hwprobe API. > This patch introduces this for QEMU's user space emulator. > > Signed-off-by: Christoph Müllner > --- > linux-user/syscall.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 65ac3ac796..22e947d752 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -8799,6 +8799,7 @@ static int do_getdents64(abi_long dirfd, abi_long arg2, > abi_long count) > #define RISCV_HWPROBE_EXT_ZBA (1 << 3) > #define RISCV_HWPROBE_EXT_ZBB (1 << 4) > #define RISCV_HWPROBE_EXT_ZBS (1 << 5) > +#define RISCV_HWPROBE_EXT_ZICBOZ (1 << 6) > > #define RISCV_HWPROBE_KEY_CPUPERF_0 5 > #define RISCV_HWPROBE_MISALIGNED_UNKNOWN (0 << 0) > @@ -8855,6 +8856,8 @@ static void risc_hwprobe_fill_pairs(CPURISCVState *env, > RISCV_HWPROBE_EXT_ZBB : 0; > value |= cfg->ext_zbs ? > RISCV_HWPROBE_EXT_ZBS : 0; > +value |= cfg->ext_zicboz ? > + RISCV_HWPROBE_EXT_ZBS : 0; I accidently sent out the wrong patch file. A v2 will be sent to set the right bit. Sorry for sending this! > __put_user(value, >value); > break; > case RISCV_HWPROBE_KEY_CPUPERF_0: > -- > 2.41.0 >
Re: [PATCH] MAINTAINERS: Add unowned RISC-V related files to the right sections
On Fri, Sep 29, 2023 at 2:37 PM Thomas Huth wrote: > > There are a bunch of RISC-V files that are currently not covered > by the "get_maintainers.pl" script. Add them to the right sections > in MAINTAINERS to fix this problem. > > Signed-off-by: Thomas Huth Acked-by: Christoph Müllner Thanks! > > --- > MAINTAINERS | 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 355b1960ce..1313257180 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -317,8 +317,11 @@ R: Daniel Henrique Barboza > R: Liu Zhiwei > L: qemu-ri...@nongnu.org > S: Supported > +F: configs/targets/riscv* > +F: docs/system/target-riscv.rst > F: target/riscv/ > F: hw/riscv/ > +F: hw/intc/riscv* > F: include/hw/riscv/ > F: linux-user/host/riscv32/ > F: linux-user/host/riscv64/ > @@ -330,6 +333,7 @@ L: qemu-ri...@nongnu.org > S: Supported > F: target/riscv/insn_trans/trans_xthead.c.inc > F: target/riscv/xthead*.decode > +F: disas/riscv-xthead* > > RISC-V XVentanaCondOps extension > M: Philipp Tomsich > @@ -337,6 +341,7 @@ L: qemu-ri...@nongnu.org > S: Maintained > F: target/riscv/XVentanaCondOps.decode > F: target/riscv/insn_trans/trans_xventanacondops.c.inc > +F: disas/riscv-xventana* > > RENESAS RX CPUs > R: Yoshinori Sato > @@ -1518,6 +1523,7 @@ Microchip PolarFire SoC Icicle Kit > M: Bin Meng > L: qemu-ri...@nongnu.org > S: Supported > +F: docs/system/riscv/microchip-icicle-kit.rst > F: hw/riscv/microchip_pfsoc.c > F: hw/char/mchp_pfsoc_mmuart.c > F: hw/misc/mchp_pfsoc_dmc.c > @@ -1533,6 +1539,7 @@ Shakti C class SoC > M: Vijai Kumar K > L: qemu-ri...@nongnu.org > S: Supported > +F: docs/system/riscv/shakti-c.rst > F: hw/riscv/shakti_c.c > F: hw/char/shakti_uart.c > F: include/hw/riscv/shakti_c.h > @@ -1544,6 +1551,7 @@ M: Bin Meng > M: Palmer Dabbelt > L: qemu-ri...@nongnu.org > S: Supported > +F: docs/system/riscv/sifive_u.rst > F: hw/*/*sifive*.c > F: include/hw/*/*sifive*.h > > @@ -3543,7 +3551,7 @@ M: Alistair Francis > L: qemu-ri...@nongnu.org > S: Maintained > F: tcg/riscv/ > -F: disas/riscv.c > +F: disas/riscv.[ch] > > S390 TCG target > M: Richard Henderson > -- > 2.41.0 >
Re: [PATCH v7] riscv: Add support for the Zfa extension
On Mon, Jul 10, 2023 at 2:58 AM Alistair Francis wrote:
>
> On Mon, Jul 3, 2023 at 4:27 PM Christoph Muellner
> wrote:
> >
> > From: Christoph Müllner
> >
> > This patch introduces the RISC-V Zfa extension, which introduces
> > additional floating-point instructions:
> > * fli (load-immediate) with pre-defined immediates
> > * fminm/fmaxm (like fmin/fmax but with different NaN behaviour)
> > * fround/froundmx (round to integer)
> > * fcvtmod.w.d (Modular Convert-to-Integer)
> > * fmv* to access high bits of float register bigger than XLEN
> > * Quiet comparison instructions (fleq/fltq)
> >
> > Zfa defines its instructions in combination with the following extensions:
> > * single-precision floating-point (F)
> > * double-precision floating-point (D)
> > * quad-precision floating-point (Q)
> > * half-precision floating-point (Zfh)
> >
> > Since QEMU does not support the RISC-V quad-precision floating-point
> > ISA extension (Q), this patch does not include the instructions that
> > depend on this extension. All other instructions are included in this
> > patch.
> >
> > The Zfa specification can be found here:
> > https://github.com/riscv/riscv-isa-manual/blob/master/src/zfa.tex
> > The Zfa specifciation is frozen and is in public review since May 3, 2023:
> > https://groups.google.com/a/groups.riscv.org/g/isa-dev/c/SED4ntBkabg
> >
> > The patch also includes a TCG test for the fcvtmod.w.d instruction.
> > The test cases test for correct results and flag behaviour.
> > Note, that the Zfa specification requires fcvtmod's flag behaviour
> > to be identical to a fcvt with the same operands (which is also
> > tested).
> >
> > Reviewed-by: Richard Henderson
> > Signed-off-by: Christoph Müllner
>
> Urgh, sorry but disas/riscv.c has changed again. Do you mind sending a v8?
No worries.
I've just rebased, addressed the conflicts and sent the v8.
Thanks,
Christoph
> >
> > ---
> >
> > This patch depends on float64_to_int64_modulo(), which is provided
> > by a patchset from Richard Henderson:
> > https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg07022.html
> >
> > Changes in v7:
> > * Rebase on alistair23/riscv-to-apply.next and resolve conflicts
> >
> > Changes in v6:
> > * Address issues in trans_fmvp_d_x() and trans_fmvh_x_d()
> >
> > Changes in v5:
> > * Merge all three commits
> > * Address issues reported by Richard
> >
> > Changes in v4:
> > * Rebase and resolve conflicts
> > * Fix whitespace issue (thanks Rob)
> > * Add patch to implemnt fcvtmod.w.d using float64_to_int64_modulo()
> > * Add (demo) test for fcvtmod.w.d
> >
> > Changes in v3:
> > * Add disassembler support
> > * Enable Zfa by default
> > * Remove forgotten comments in the decoder
> > * Fix fli translation code (use movi instead of ld)
> > * Tested against SPEC CPU2017 fprate
> > * Use floatN_[min|max] for f[min|max]m.* instructions
> >
> > Changes in v2:
> > * Remove calls to mark_fs_dirty() in comparison trans functions
> > * Rewrite fround(nx) using float*_round_to_int()
> > * Move fli* to translation unit and fix NaN-boxing of NaN values
> > * Reimplement FCVTMOD.W.D
> > * Add use of second register in trans_fmvp_d_x()
> >
> > disas/riscv.c | 147 ++
> > disas/riscv.h | 3 +
> > target/riscv/cpu.c| 8 +
> > target/riscv/cpu_cfg.h| 1 +
> > target/riscv/fpu_helper.c | 154 +++
> > target/riscv/helper.h | 19 +
> > target/riscv/insn32.decode| 26 ++
> > target/riscv/insn_trans/trans_rvzfa.c.inc | 521 ++
> > target/riscv/translate.c | 1 +
> > tests/tcg/riscv64/Makefile.target | 6 +
> > tests/tcg/riscv64/test-fcvtmod.c | 345 ++
> > 11 files changed, 1231 insertions(+)
> > create mode 100644 target/riscv/insn_trans/trans_rvzfa.c.inc
> > create mode 100644 tests/tcg/riscv64/test-fcvtmod.c
> >
> > diff --git a/disas/riscv.c b/disas/riscv.c
> > index 94e568a7e9..b5b3c7a868 100644
> > --- a/disas/riscv.c
> > +++ b/disas/riscv.c
> > @@ -819,6 +819,39 @@ typedef enum {
> > rv_op_cm_jalt = 788,
> > rv_op_czero_eqz = 789,
> > rv_op_czero_nez = 790,
> > +rv_op_fli_s = 791,
> > +rv_op_fli_d = 792,
> > +rv_op_fli_q = 793,
> > +rv_op_fli_h = 794,
Re: [PATCH v4 02/37] tests/multiarch: Add test-aes
On Mon, Jul 3, 2023 at 12:07 PM Richard Henderson
wrote:
>
> Use a shared driver and backends for i386, aarch64, ppc64, riscv64.
>
> Acked-by: Alex Bennée
> Signed-off-by: Richard Henderson
> ---
> tests/tcg/aarch64/test-aes.c| 58
> tests/tcg/i386/test-aes.c | 68 +
> tests/tcg/ppc64/test-aes.c | 116 +++
> tests/tcg/riscv64/test-aes.c| 76 ++
> tests/tcg/multiarch/test-aes-main.c.inc | 183
> tests/tcg/aarch64/Makefile.target | 4 +
> tests/tcg/i386/Makefile.target | 4 +
> tests/tcg/ppc64/Makefile.target | 1 +
> tests/tcg/riscv64/Makefile.target | 13 ++
> 9 files changed, 523 insertions(+)
> create mode 100644 tests/tcg/aarch64/test-aes.c
> create mode 100644 tests/tcg/i386/test-aes.c
> create mode 100644 tests/tcg/ppc64/test-aes.c
> create mode 100644 tests/tcg/riscv64/test-aes.c
> create mode 100644 tests/tcg/multiarch/test-aes-main.c.inc
>
> diff --git a/tests/tcg/aarch64/test-aes.c b/tests/tcg/aarch64/test-aes.c
> new file mode 100644
> index 00..2cd324f09b
> --- /dev/null
> +++ b/tests/tcg/aarch64/test-aes.c
> @@ -0,0 +1,58 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#include "../multiarch/test-aes-main.c.inc"
> +
> +bool test_SB_SR(uint8_t *o, const uint8_t *i)
> +{
> +/* aese also adds round key, so supply zero. */
> +asm("ld1 { v0.16b }, [%1]\n\t"
> +"movi v1.16b, #0\n\t"
> +"aese v0.16b, v1.16b\n\t"
> +"st1 { v0.16b }, [%0]"
> +: : "r"(o), "r"(i) : "v0", "v1", "memory");
> +return true;
> +}
> +
> +bool test_MC(uint8_t *o, const uint8_t *i)
> +{
> +asm("ld1 { v0.16b }, [%1]\n\t"
> +"aesmc v0.16b, v0.16b\n\t"
> +"st1 { v0.16b }, [%0]"
> +: : "r"(o), "r"(i) : "v0", "memory");
> +return true;
> +}
> +
> +bool test_SB_SR_MC_AK(uint8_t *o, const uint8_t *i, const uint8_t *k)
> +{
> +return false;
> +}
> +
> +bool test_ISB_ISR(uint8_t *o, const uint8_t *i)
> +{
> +/* aesd also adds round key, so supply zero. */
> +asm("ld1 { v0.16b }, [%1]\n\t"
> +"movi v1.16b, #0\n\t"
> +"aesd v0.16b, v1.16b\n\t"
> +"st1 { v0.16b }, [%0]"
> +: : "r"(o), "r"(i) : "v0", "v1", "memory");
> +return true;
> +}
> +
> +bool test_IMC(uint8_t *o, const uint8_t *i)
> +{
> +asm("ld1 { v0.16b }, [%1]\n\t"
> +"aesimc v0.16b, v0.16b\n\t"
> +"st1 { v0.16b }, [%0]"
> +: : "r"(o), "r"(i) : "v0", "memory");
> +return true;
> +}
> +
> +bool test_ISB_ISR_AK_IMC(uint8_t *o, const uint8_t *i, const uint8_t *k)
> +{
> +return false;
> +}
> +
> +bool test_ISB_ISR_IMC_AK(uint8_t *o, const uint8_t *i, const uint8_t *k)
> +{
> +return false;
> +}
> diff --git a/tests/tcg/i386/test-aes.c b/tests/tcg/i386/test-aes.c
> new file mode 100644
> index 00..199395e6cc
> --- /dev/null
> +++ b/tests/tcg/i386/test-aes.c
> @@ -0,0 +1,68 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#include "../multiarch/test-aes-main.c.inc"
> +#include
> +
> +static bool test_SB_SR(uint8_t *o, const uint8_t *i)
> +{
> +__m128i vi = _mm_loadu_si128((const __m128i_u *)i);
> +
> +/* aesenclast also adds round key, so supply zero. */
> +vi = _mm_aesenclast_si128(vi, _mm_setzero_si128());
> +
> +_mm_storeu_si128((__m128i_u *)o, vi);
> +return true;
> +}
> +
> +static bool test_MC(uint8_t *o, const uint8_t *i)
> +{
> +return false;
> +}
> +
> +static bool test_SB_SR_MC_AK(uint8_t *o, const uint8_t *i, const uint8_t *k)
> +{
> +__m128i vi = _mm_loadu_si128((const __m128i_u *)i);
> +__m128i vk = _mm_loadu_si128((const __m128i_u *)k);
> +
> +vi = _mm_aesenc_si128(vi, vk);
> +
> +_mm_storeu_si128((__m128i_u *)o, vi);
> +return true;
> +}
> +
> +static bool test_ISB_ISR(uint8_t *o, const uint8_t *i)
> +{
> +__m128i vi = _mm_loadu_si128((const __m128i_u *)i);
> +
> +/* aesdeclast also adds round key, so supply zero. */
> +vi = _mm_aesdeclast_si128(vi, _mm_setzero_si128());
> +
> +_mm_storeu_si128((__m128i_u *)o, vi);
> +return true;
> +}
> +
> +static bool test_IMC(uint8_t *o, const uint8_t *i)
> +{
> +__m128i vi = _mm_loadu_si128((const __m128i_u *)i);
> +
> +vi = _mm_aesimc_si128(vi);
> +
> +_mm_storeu_si128((__m128i_u *)o, vi);
> +return true;
> +}
> +
> +static bool test_ISB_ISR_AK_IMC(uint8_t *o, const uint8_t *i, const uint8_t
> *k)
> +{
> +return false;
> +}
> +
> +static bool test_ISB_ISR_IMC_AK(uint8_t *o, const uint8_t *i, const uint8_t
> *k)
> +{
> +__m128i vi = _mm_loadu_si128((const __m128i_u *)i);
> +__m128i vk = _mm_loadu_si128((const __m128i_u *)k);
> +
> +vi = _mm_aesdec_si128(vi, vk);
> +
> +_mm_storeu_si128((__m128i_u *)o, vi);
> +return true;
> +}
> diff --git a/tests/tcg/ppc64/test-aes.c b/tests/tcg/ppc64/test-aes.c
> new file mode 100644
> index 00..1d2be488e9
> --- /dev/null
>
Re: [PATCH v6] riscv: Add support for the Zfa extension
On Mon, Jul 3, 2023 at 5:58 AM Alistair Francis wrote:
>
> On Sat, Jul 1, 2023 at 3:04 AM Christoph Muellner
> wrote:
> >
> > From: Christoph Müllner
> >
> > This patch introduces the RISC-V Zfa extension, which introduces
> > additional floating-point instructions:
> > * fli (load-immediate) with pre-defined immediates
> > * fminm/fmaxm (like fmin/fmax but with different NaN behaviour)
> > * fround/froundmx (round to integer)
> > * fcvtmod.w.d (Modular Convert-to-Integer)
> > * fmv* to access high bits of float register bigger than XLEN
> > * Quiet comparison instructions (fleq/fltq)
> >
> > Zfa defines its instructions in combination with the following extensions:
> > * single-precision floating-point (F)
> > * double-precision floating-point (D)
> > * quad-precision floating-point (Q)
> > * half-precision floating-point (Zfh)
> >
> > Since QEMU does not support the RISC-V quad-precision floating-point
> > ISA extension (Q), this patch does not include the instructions that
> > depend on this extension. All other instructions are included in this
> > patch.
> >
> > The Zfa specification can be found here:
> > https://github.com/riscv/riscv-isa-manual/blob/master/src/zfa.tex
> > The Zfa specifciation is frozen and is in public review since May 3, 2023:
> > https://groups.google.com/a/groups.riscv.org/g/isa-dev/c/SED4ntBkabg
> >
> > The patch also includes a TCG test for the fcvtmod.w.d instruction.
> > The test cases test for correct results and flag behaviour.
> > Note, that the Zfa specification requires fcvtmod's flag behaviour
> > to be identical to a fcvt with the same operands (which is also
> > tested).
> >
> > Signed-off-by: Christoph Müllner
>
> Thanks for the patch. Do you mind rebasing it on
> https://github.com/alistair23/qemu/tree/riscv-to-apply.next and
> sending a new version
Sure!
A v7 will be out in a few minutes.
BR
Christoph
>
> >
> > ---
> >
> > This patch depends on float64_to_int64_modulo(), which is provided
> > by a patchset from Richard Henderson:
> > https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg07022.html
> >
> > Changes in v6:
> > * Address issues in trans_fmvp_d_x() and trans_fmvh_x_d()
> >
> > Changes in v5:
> > * Merge all three commits
> > * Address issues reported by Richard
> >
> > Changes in v4:
> > * Rebase and resolve conflicts
> > * Fix whitespace issue (thanks Rob)
> > * Add patch to implemnt fcvtmod.w.d using float64_to_int64_modulo()
> > * Add (demo) test for fcvtmod.w.d
> >
> > Changes in v3:
> > * Add disassembler support
> > * Enable Zfa by default
> > * Remove forgotten comments in the decoder
> > * Fix fli translation code (use movi instead of ld)
> > * Tested against SPEC CPU2017 fprate
> > * Use floatN_[min|max] for f[min|max]m.* instructions
> >
> > Changes in v2:
> > * Remove calls to mark_fs_dirty() in comparison trans functions
> > * Rewrite fround(nx) using float*_round_to_int()
> > * Move fli* to translation unit and fix NaN-boxing of NaN values
> > * Reimplement FCVTMOD.W.D
> > * Add use of second register in trans_fmvp_d_x()
> >
> > disas/riscv.c | 151 +++
> > target/riscv/cpu.c| 8 +
> > target/riscv/cpu_cfg.h| 1 +
> > target/riscv/fpu_helper.c | 154 +++
> > target/riscv/helper.h | 19 +
> > target/riscv/insn32.decode| 26 ++
> > target/riscv/insn_trans/trans_rvzfa.c.inc | 521 ++
> > target/riscv/translate.c | 1 +
> > tests/tcg/riscv64/Makefile.target | 6 +
> > tests/tcg/riscv64/test-fcvtmod.c | 345 ++
> > 10 files changed, 1232 insertions(+)
> > create mode 100644 target/riscv/insn_trans/trans_rvzfa.c.inc
> > create mode 100644 tests/tcg/riscv64/test-fcvtmod.c
> >
> > diff --git a/disas/riscv.c b/disas/riscv.c
> > index 5005364aba..d8ae1e53a3 100644
> > --- a/disas/riscv.c
> > +++ b/disas/riscv.c
> > @@ -170,6 +170,7 @@ typedef enum {
> > rv_codec_zcmp_cm_pushpop,
> > rv_codec_zcmp_cm_mv,
> > rv_codec_zcmt_jt,
> > +rv_codec_fli,
> > } rv_codec;
> >
> > typedef enum {
> > @@ -964,6 +965,39 @@ typedef enum {
> > rv_op_cm_jalt = 788,
> > rv_op_czero_eqz = 789,
> > rv_op_czero_nez = 790,
> > +rv_op_fli_s = 791,
> > +rv_op_fli_d = 792,
>
Re: [PATCH v5] riscv: Add support for the Zfa extension
On Fri, Jun 30, 2023 at 4:03 PM Richard Henderson
wrote:
>
> On 6/30/23 13:52, Christoph Muellner wrote:
> > +bool trans_fmvh_x_d(DisasContext *ctx, arg_fmvh_x_d *a)
> > +{
> > +REQUIRE_FPU;
> > +REQUIRE_ZFA(ctx);
> > +REQUIRE_EXT(ctx, RVD);
> > +REQUIRE_32BIT(ctx);
> > +
> > +TCGv dst = dest_gpr(ctx, a->rd);
> > +TCGv_i64 t1 = tcg_temp_new_i64();
> > +
> > +tcg_gen_extract_i64(t1, cpu_fpr[a->rs1], 32, 32);
> > +tcg_gen_trunc_i64_tl(dst, t1);
> > +gen_set_gpr(ctx, a->rd, dst);
>
> I think you would prefer
>
>tcg_gen_srai_tl(t1, cpu_fpr[rs1], 32);
sari_tl() will not work, because cpu_fpr[a->rs1] is a TCGv_i64.
So I need to use sari_i64() and keep the trunc_i64_tl():
TCGv dst = dest_gpr(ctx, a->rd);
TCGv_i64 t1 = tcg_temp_new_i64();
tcg_gen_sari_i64(dst, cpu_fpr[a->rs1], 32);
tcg_gen_trunc_i64_tl(dst, t1);
gen_set_gpr(ctx, a->rd, dst);
Thanks,
Christoph
>
> so that dst is sign-extended to begin, instead of zero-extended. You don't
> see an error
> because gen_set_gpr, for MXL_RV32, sign-extends the stored value.
>
> However, the tcg optimizer would elide the second sign-extend if it can see
> that the value
> is already sign-extended. So this could reduce to 1 operation instead of 2.
>
>
> r~
Re: [PATCH v4 3/3] DO NOT MERGE: tests/tcg/riscv64: Add test for fcvtmod.w.d
On Fri, Jun 30, 2023 at 11:46 AM Richard Henderson
wrote:
>
> On 6/30/23 11:13, Christoph Muellner wrote:
> > From: Christoph Müllner
> >
> > This patch introduces a test for Zfa's fcvtmod.w.d instruction.
> > The test cases test for correct results and flag behaviour.
> > Note, that the Zfa specification requires fcvtmod's flag behaviour
> > to be identical to a fcvt with the same operands (which is also
> > tested).
> >
> > DO NOT MERGE!!!
> > Although this test works just fine, it requires a toolchain
> > that supports the Zfa extension. Unless this is available
> > this patch cannot be merged.
> > DO NOT MERGE!!!
>
> That's what
>
>/* fcvtmod.w.d rd, rs1, rtz = 111 01000 rs1 001 rd 1010011 */
>asm(".insn r, 0x53, 0x1, 0x61, %0, %1, f8" : "=r"(ret) : "f"(fpr));
>
> is for.
>
> > +__asm__ __volatile__("fmv.d.x %0, %1" : "=fp"(fpr) : "r"(inp));
>
> Always "f" not "fp". "fp" is *two* constraints, "f" and "p", and the second
> is incorrect.
Fixed, tested and merged into the other patch.
Thanks!
>
>
> r~
Re: [PATCH v4 1/3] riscv: Add support for the Zfa extension
On Fri, Jun 30, 2023 at 11:53 AM Richard Henderson
wrote:
>
> On 6/30/23 11:13, Christoph Muellner wrote:
> > +static bool trans_fli_h(DisasContext *ctx, arg_fli_h *a)
> > +{
> > +REQUIRE_FPU;
> > +REQUIRE_ZFA(ctx);
> > +REQUIRE_ZFH(ctx);
> > +
> > +/* Values below are NaN-boxed to avoid a gen_nanbox_h(). */
> > +const uint64_t fli_h_table[] = {
>
> static const.
Sorry for missing that.
Thanks,
Christoph
>
>
> r~
Re: [PATCH v4 2/3] target/riscv: Use float64_to_int64_modulo for fcvtmod.w.d
On Fri, Jun 30, 2023 at 11:22 AM Richard Henderson
wrote:
>
> On 6/30/23 11:13, Christoph Muellner wrote:
> > From: Christoph Müllner
> >
> > For the most part we can use the new generic routine,
> > though exceptions need some post-processing.
> >
> > Signed-off-by: Christoph Müllner
> > ---
> > target/riscv/fpu_helper.c | 78 ++-
> > 1 file changed, 27 insertions(+), 51 deletions(-)
> >
> > diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
> > index 289b3bbea5..0f897cf072 100644
> > --- a/target/riscv/fpu_helper.c
> > +++ b/target/riscv/fpu_helper.c
> > @@ -482,70 +482,46 @@ target_ulong helper_fcvt_w_d(CPURISCVState *env,
> > uint64_t frs1)
> > return float64_to_int32(frs1, >fp_status);
> > }
> >
> > +/* T floating (double) */
> > +static inline float64 t_to_float64(uint64_t a)
> > +{
> > +/* Memory format is the same as float64 */
> > +CPU_DoubleU r;
> > +r.ll = a;
> > +return r.d;
> > +}
>
> You don't need this. Nor does Alpha anymore, come to that.
> float64 is uint64_t now, always.
Ok.
>
> > +int64_t ret;
> > +int32_t ret32;
> > +uint32_t e_old, e_new;
> > +float64 fvalue;
> > +
> > +e_old = get_float_exception_flags(status);
> > +set_float_exception_flags(0, status);
> > +fvalue = t_to_float64(value);
> > +ret = float64_to_int32_modulo(fvalue, float_round_to_zero, status);
> > +e_new = get_float_exception_flags(status);
> > +
> > +/* Map the flags to the specified ones. */
> > +if (e_new & float_flag_inexact) {
> > +e_new = float_flag_inexact;
> > +} else if (e_new) {
> > +e_new = float_flag_invalid;
> > }
>
> Why? Generic code will not set both inexact and invalid.
> So this is a nop.
>
> Removing that, all of your fp flags handling can go away.
I added that because float64_to_int32_modulo() might also set
float_flag_invalid_cvti. I just realized that it is not needed,
because riscv_cpu_get_fflags() takes care to not expose
flags that are not defined for RISC-V.
>
>
> > /* Truncate to 32-bits. */
> > -int32_t ret32 = (int32_t)ret;
> > +ret32 = (int32_t)ret;
> >
> > /* If the truncation drops bits then raise NV. */
> > if ((uint64_t)ret32 != ret)
>
> This will never fail, because you used float64_to_int32_modulo, which already
> returns int32_t.
>
> But we have already raised invalid for overflow, so this can go away as well.
Understood.
>
> Finally, this patch must be merged with the previous, which introduced this
> function.
Ok.
Thanks!
>
>
> r~
Re: [PATCH 0/4] fpu: Add float64_to_int{32,64}_modulo
On Sat, May 27, 2023 at 4:19 PM Richard Henderson
wrote:
>
> Extract some common code from Alpha and Arm, and which will
> shortly also be required by the RISC-V Zfa extension.
> Added a new test for Alpha; I already had a RISU test for Arm.
Thank you for providing a generic implementation of this conversion!
I've rebased the RISC-V Zfa patch onto this series and could
implement the fcvtmod.w.d instruction (with a little bit of flag manipulation
after calling float64_to_int32()).
Reviewed-by: Christoph Muellner
Tested-by: Christoph Muellner
>
>
> r~
>
>
> Richard Henderson (4):
> fpu: Add float64_to_int{32,64}_modulo
> tests/tcg/alpha: Add test for cvttq
> target/alpha: Use float64_to_int64_modulo for CVTTQ
> target/arm: Use float64_to_int32_modulo for FJCVTZS
>
> include/fpu/softfloat.h | 3 ++
> fpu/softfloat.c | 31
> target/alpha/fpu_helper.c | 85 +++--
> target/arm/vfp_helper.c | 71 +--
> tests/tcg/alpha/test-cvttq.c| 78 ++
> fpu/softfloat-parts.c.inc | 78 ++
> tests/tcg/alpha/Makefile.target | 2 +-
> 7 files changed, 221 insertions(+), 127 deletions(-)
> create mode 100644 tests/tcg/alpha/test-cvttq.c
>
> --
> 2.34.1
>
Re: [PATCH v2 8/8] disas/riscv: Add support for XThead* instructions
On Thu, Jun 15, 2023 at 8:53 AM Weiwei Li wrote:
>
>
> On 2023/6/12 19:10, Christoph Muellner wrote:
> > From: Christoph Müllner
> >
> > Support for emulating XThead* instruction has been added recently.
> > This patch adds support for these instructions to the RISC-V disassembler.
> >
> > Co-developed-by: LIU Zhiwei
> > Acked-by: Alistair Francis
> > Signed-off-by: Christoph Müllner
> > ---
> > disas/meson.build | 1 +
> > disas/riscv-xthead.c | 707 +
> > disas/riscv-xthead.h | 28 ++
> > disas/riscv.c | 69
> > disas/riscv.h | 12 +
> > target/riscv/cpu_cfg.h | 11 +
> > 6 files changed, 828 insertions(+)
> > create mode 100644 disas/riscv-xthead.c
> > create mode 100644 disas/riscv-xthead.h
> >
> > diff --git a/disas/meson.build b/disas/meson.build
> > index e0ee326411..8f64e378f9 100644
> > --- a/disas/meson.build
> > +++ b/disas/meson.build
> > @@ -8,6 +8,7 @@ common_ss.add(when: 'CONFIG_MIPS_DIS', if_true:
> > files('mips.c', 'nanomips.c'))
> > common_ss.add(when: 'CONFIG_NIOS2_DIS', if_true: files('nios2.c'))
> > common_ss.add(when: 'CONFIG_RISCV_DIS', if_true: files(
> > 'riscv.c',
> > +'riscv-xthead.c',
> > 'riscv-xventana.c'
> > ))
> > common_ss.add(when: 'CONFIG_SH4_DIS', if_true: files('sh4.c'))
> > diff --git a/disas/riscv-xthead.c b/disas/riscv-xthead.c
> > new file mode 100644
> > index 00..99da679d16
> > --- /dev/null
> > +++ b/disas/riscv-xthead.c
> > @@ -0,0 +1,707 @@
> > +/*
> > + * QEMU RISC-V Disassembler for xthead.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "disas/riscv.h"
> > +#include "disas/riscv-xthead.h"
> > +
> > +typedef enum {
> > +/* 0 is reserved for rv_op_illegal. */
> > +/* XTheadBa */
> > +rv_op_th_addsl = 1,
> > +/* XTheadBb */
> > +rv_op_th_srri,
> > +rv_op_th_srriw,
> > +rv_op_th_ext,
> > +rv_op_th_extu,
> > +rv_op_th_ff0,
> > +rv_op_th_ff1,
> > +rv_op_th_rev,
> > +rv_op_th_revw,
> > +rv_op_th_tstnbz,
> > +/* XTheadBs */
> > +rv_op_th_tst,
> > +/* XTheadCmo */
> > +rv_op_th_dcache_call,
> > +rv_op_th_dcache_ciall,
> > +rv_op_th_dcache_iall,
> > +rv_op_th_dcache_cpa,
> > +rv_op_th_dcache_cipa,
> > +rv_op_th_dcache_ipa,
> > +rv_op_th_dcache_cva,
> > +rv_op_th_dcache_civa,
> > +rv_op_th_dcache_iva,
> > +rv_op_th_dcache_csw,
> > +rv_op_th_dcache_cisw,
> > +rv_op_th_dcache_isw,
> > +rv_op_th_dcache_cpal1,
> > +rv_op_th_dcache_cval1,
> > +rv_op_th_icache_iall,
> > +rv_op_th_icache_ialls,
> > +rv_op_th_icache_ipa,
> > +rv_op_th_icache_iva,
> > +rv_op_th_l2cache_call,
> > +rv_op_th_l2cache_ciall,
> > +rv_op_th_l2cache_iall,
> > +/* XTheadCondMov */
> > +rv_op_th_mveqz,
> > +rv_op_th_mvnez,
> > +/* XTheadFMemIdx */
> > +rv_op_th_flrd,
> > +rv_op_th_flrw,
> > +rv_op_th_flurd,
> > +rv_op_th_flurw,
> > +rv_op_th_fsrd,
> > +rv_op_th_fsrw,
> > +rv_op_th_fsurd,
> > +rv_op_th_fsurw,
> > +/* XTheadFmv */
> > +rv_op_th_fmv_hw_x,
> > +rv_op_th_fmv_x_hw,
> > +/* XTheadMac */
> > +rv_op_th_mula,
> > +rv_op_th_mulah,
> > +rv_op_th_mulaw,
> > +rv_op_th_muls,
> > +rv_op_th_mulsw,
> > +rv_op_th_mulsh,
> > +/* XTheadMemIdx */
> > +rv_op_th_lbia,
> > +rv_op_th_lbib,
> > +rv_op_th_lbuia,
> > +rv_op_th_lbuib,
> > +rv_op_th_lhia,
> > +rv_op_th_lhib,
> > +rv_op_th_lhuia,
> > +rv_op_th_lhuib,
> > +rv_op_th_lwia,
> > +rv_op_th_lwib,
> > +rv_op_th_lwuia,
> > +rv_op_th_lwuib,
> > +rv_op_th_ldia,
> > +rv_op_th_ldib,
> > +rv_op_th_sbia,
> > +rv_op_th_sbib,
> > +rv_op_th_shia,
> > +rv_op_th_shib,
> > +rv_op_th_swia,
> > +rv_op_th_swib,
> > +rv_op_th_sdia,
> > +rv_op_th_sdib,
> > +rv_op_th_lrb,
> > +rv_op_th_lrbu,
> > +rv_op_th_lrh,
> > +rv_op_th_lrhu,
> > +rv_op_th_lrw,
> > +rv_op_th_lrwu,
> > +rv_op_th_lrd,
> > +rv_op_th_sr
Re: [PATCH 0/9] disas/riscv: Add vendor extension support
On Tue, Jun 6, 2023 at 7:38 PM Daniel Henrique Barboza
wrote:
>
> Hi,
>
> Can you please rebase on top of Alistair's riscv-to-apply.next and re-send?
Done.
Thanks,
Christoph
>
> Some patches can't be applied cleanly, in particular patch 2, which conflicts
> with Weiwei's "target/riscv: Split RISCVCPUConfig declarations from cpu.h
> into cpu_cfg.h" that landed into riscv-to-apply.next a few weeks ago. In
> this particular case patch 2 of this series would need to move just the
> bits of target/ppc/translate.c to the already existing cpu_cfg.h file.
>
>
> Put me in the CC when you re-send and I'll review it asap. Thanks,
>
>
> Daniel
>
> On 5/30/23 10:18, Christoph Muellner wrote:
> > From: Christoph Müllner
> >
> > This series adds vendor extension support to the QEMU disassembler
> > for RISC-V. The following vendor extensions are covered:
> > * XThead{Ba,Bb,Bs,Cmo,CondMov,FMemIdx,Fmv,Mac,MemIdx,MemPair,Sync}
> > * XVentanaCondOps
> >
> > So far, there have been two attempts to add vendor extension support
> > to the QEMU disassembler. The first one [1] was posted in August 2022
> > by LIU Zhiwei and attempts to separate vendor extension specifics
> > from standard extension code in combination with a patch that introduced
> > support for XVentanaCondOps. The second one [2] was posted in March 2023
> > by me and added XThead* support without separating the vendor extensions
> > from the standard code.
> >
> > This patchset represents the third attempt to add vendor extension
> > support to the QEMU disassembler. It adds all features of the previous
> > attempts and integrates them into a patchset that uses the same
> > mechanism for testing the extension availability like translate.c
> > (using the booleans RISCVCPUConfig::ext_*).
> > To achieve that, a couple of patches were needed to restructure
> > the existing code.
> >
> > Note, that this patchset allows an instruction encoder function for each
> > vendor extension, but operand decoding and instruction printing remains
> > common code. This is irrelevant for XVentanaCondOps, but the patch for
> > the XThead* extensions includes changes in riscv.c and riscv.h.
> > This could be changed to force more separation with the cost of
> > duplication.
> >
> > The first patch of this series is cherry-picked from LIU Zhiwei's series.
> > It was reviewed by Alistair Francis and Richard Henderson, but never
> > made it on master. I've added "Reviewed-by" tags to the commit.
> >
> > I've added "Co-developed-by" tags to those commits that are derived
> > from the series of LIU Zhiwei.
> >
> > [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-08/msg03662.html
> > [2] https://lists.nongnu.org/archive/html/qemu-devel/2023-03/msg04566.html
> >
> > Christoph Müllner (8):
> >target/riscv: Factor out RISCVCPUConfig from cpu.h
> >disas/riscv: Move types/constants to new header file
> >disas/riscv: Make rv_op_illegal a shared enum value
> >disas/riscv: Encapsulate opcode_data into decode
> >target/riscv/cpu: Share RISCVCPUConfig with disassembler
> >disas/riscv: Provide infrastructure for vendor extensions
> >disas/riscv: Add support for XVentanaCondOps
> >disas/riscv: Add support for XThead* instructions
> >
> > LIU Zhiwei (1):
> >target/riscv: Use xl instead of mxl for disassemble
> >
> > disas/meson.build | 6 +-
> > disas/riscv-xthead.c | 707 ++
> > disas/riscv-xthead.h | 28 ++
> > disas/riscv-xventana.c| 41 +++
> > disas/riscv-xventana.h| 18 +
> > disas/riscv.c | 384 ++---
> > disas/riscv.h | 297
> > target/riscv/cpu-config.h | 159 +
> > target/riscv/cpu.c| 6 +-
> > target/riscv/cpu.h| 114 +-
> > target/riscv/translate.c | 27 +-
> > 11 files changed, 1374 insertions(+), 413 deletions(-)
> > create mode 100644 disas/riscv-xthead.c
> > create mode 100644 disas/riscv-xthead.h
> > create mode 100644 disas/riscv-xventana.c
> > create mode 100644 disas/riscv-xventana.h
> > create mode 100644 disas/riscv.h
> > create mode 100644 target/riscv/cpu-config.h
> >
Re: [PATCH 7/9] disas/riscv: Provide infrastructure for vendor extensions
On Thu, Jun 8, 2023 at 3:05 PM LIU Zhiwei wrote:
>
>
> On 2023/5/30 21:18, Christoph Muellner wrote:
> > From: Christoph Müllner
> >
> > A previous patch provides a pointer to the RISCVCPUConfig data.
> > Let's use this to add the necessary code for vendor extensions.
> > This patch does not change the current behaviour, but clearly
> > defines how vendor extension support can be added to the disassembler.
> >
> > Signed-off-by: Christoph Müllner
> > ---
> > disas/riscv.c | 34 ++
> > 1 file changed, 30 insertions(+), 4 deletions(-)
> >
> > diff --git a/disas/riscv.c b/disas/riscv.c
> > index 086edee6a2..db98e3ea6a 100644
> > --- a/disas/riscv.c
> > +++ b/disas/riscv.c
> > @@ -20,6 +20,7 @@
> > #include "qemu/osdep.h"
> > #include "disas/dis-asm.h"
> > #include "disas/riscv.h"
> > +#include "target/riscv/cpu-config.h"
> >
> > typedef enum {
> > /* 0 is reserved for rv_op_illegal. */
> > @@ -4599,13 +4600,38 @@ static void decode_inst_decompress(rv_decode *dec,
> > rv_isa isa)
> > /* disassemble instruction */
> >
> > static void
> > -disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst
> > inst)
> > +disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst
> > inst,
> > +struct disassemble_info *info)
> > {
> > +RISCVCPUConfig *cfg = info->private_data;
> > rv_decode dec = { 0 };
> > dec.pc = pc;
> > dec.inst = inst;
> > -dec.opcode_data = rvi_opcode_data;
> > -decode_inst_opcode(, isa);
> > +
> > +static const struct {
> > +bool (*guard_func)(const RISCVCPUConfig *);
> > +const rv_opcode_data *opcode_data;
> > +void (*decode_func)(rv_decode *, rv_isa);
> > +} decoders[] = {
> > +{ always_true_p, rvi_opcode_data, decode_inst_opcode },
> > +};
> > +
> > +for (size_t i = 0; i < ARRAY_SIZE(decoders); i++) {
> > +bool (*guard_func)(const RISCVCPUConfig *) =
> > decoders[i].guard_func;
> > +const rv_opcode_data *opcode_data = decoders[i].opcode_data;
> > +void (*decode_func)(rv_decode *, rv_isa) = decoders[i].decode_func;
> > +
> > +if (guard_func(cfg)) {
> > +dec.opcode_data = opcode_data;
> > +decode_func(, isa);
> > +if (dec.op != rv_op_illegal)
> > +break;
> > +}
> > +}
> > +
> > +if (dec.op == rv_op_illegal)
> > +dec.opcode_data = rvi_opcode_data;
>
> Always enclose the if sentence.
Done.
Thanks,
Christoph
>
> Otherwise,
>
> Reviewed-by: LIU Zhiwei
>
> > +
> > decode_inst_operands(, isa);
> > decode_inst_decompress(, isa);
> > decode_inst_lift_pseudo();
> > @@ -4659,7 +4685,7 @@ print_insn_riscv(bfd_vma memaddr, struct
> > disassemble_info *info, rv_isa isa)
> > break;
> > }
> >
> > -disasm_inst(buf, sizeof(buf), isa, memaddr, inst);
> > +disasm_inst(buf, sizeof(buf), isa, memaddr, inst, info);
> > (*info->fprintf_func)(info->stream, "%s", buf);
> >
> > return len;
Re: [PATCH 6/9] target/riscv/cpu: Share RISCVCPUConfig with disassembler
On Mon, Jun 12, 2023 at 12:01 PM LIU Zhiwei wrote:
>
>
> On 2023/6/12 17:47, Christoph Müllner wrote:
> > On Mon, Jun 12, 2023 at 8:25 AM LIU Zhiwei wrote:
> >>
> >> On 2023/5/30 21:18, Christoph Muellner wrote:
> >>> From: Christoph Müllner
> >>>
> >>> The disassembler needs the available extensions in order
> >>> to properly decode instructions in case of overlapping
> >>> encodings (e.g. for vendor extensions).
> >>>
> >>> Let's use the field 'disassemble_info::private_data' to store
> >>> our RISCVCPUConfig pointer.
> >>>
> >>> Signed-off-by: Christoph Müllner
> >>> ---
> >>>target/riscv/cpu.c | 3 +++
> >>>1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> >>> index 5b7818dbd1..6f0cd9a0bb 100644
> >>> --- a/target/riscv/cpu.c
> >>> +++ b/target/riscv/cpu.c
> >>> @@ -819,6 +819,9 @@ static void riscv_cpu_disas_set_info(CPUState *s,
> >>> disassemble_info *info)
> >>>{
> >>>RISCVCPU *cpu = RISCV_CPU(s);
> >>>CPURISCVState *env = >env;
> >>> +RISCVCPUConfig *cfg = >cfg;
> >>> +
> >>> +info->private_data = cfg;
> >> I don't know if this field will be overridden by the binutils. Can we
> >> extend the struct disassemble_info, and add some fields like supporting
> >> for Capstone?
> >
> > Initially I wanted to add a new field, but then I noticed that the field
> > 'disassemble_info::private_data' is used for a similar purpose by
> > disas/cris.c, disas/m68k.c, and dias/xtensa.c.
> > So I decided to not add yet another field, which only serves one
> > architecture.
> I think you can CC these arch maintainers to see if it need some
> specially process before using the private_data.
> >
> > But if that's the preferred way, then I can change.
>
> I prefer this way, but not insist on if it really works using the
> private_data.
This topic is already resolved by using the field 'info->target_info'.
So I dropped this patch anyway.
BR
Christoph
>
> Zhiwei
>
> >
> > Thanks
> > Christoph
> >
> >> Zhiwei
> >>
> >>>switch (env->xl) {
> >>>case MXL_RV32:
Re: [PATCH 6/9] target/riscv/cpu: Share RISCVCPUConfig with disassembler
On Mon, Jun 12, 2023 at 8:25 AM LIU Zhiwei wrote:
>
>
> On 2023/5/30 21:18, Christoph Muellner wrote:
> > From: Christoph Müllner
> >
> > The disassembler needs the available extensions in order
> > to properly decode instructions in case of overlapping
> > encodings (e.g. for vendor extensions).
> >
> > Let's use the field 'disassemble_info::private_data' to store
> > our RISCVCPUConfig pointer.
> >
> > Signed-off-by: Christoph Müllner
> > ---
> > target/riscv/cpu.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 5b7818dbd1..6f0cd9a0bb 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -819,6 +819,9 @@ static void riscv_cpu_disas_set_info(CPUState *s,
> > disassemble_info *info)
> > {
> > RISCVCPU *cpu = RISCV_CPU(s);
> > CPURISCVState *env = >env;
> > +RISCVCPUConfig *cfg = >cfg;
> > +
> > +info->private_data = cfg;
>
> I don't know if this field will be overridden by the binutils. Can we
> extend the struct disassemble_info, and add some fields like supporting
> for Capstone?
Initially I wanted to add a new field, but then I noticed that the field
'disassemble_info::private_data' is used for a similar purpose by
disas/cris.c, disas/m68k.c, and dias/xtensa.c.
So I decided to not add yet another field, which only serves one architecture.
But if that's the preferred way, then I can change.
Thanks
Christoph
>
> Zhiwei
>
> >
> > switch (env->xl) {
> > case MXL_RV32:
Re: [PATCH] riscv: Add support for the Zfa extension
On Fri, Mar 31, 2023 at 11:39 PM Richard Henderson
wrote:
>
> On 3/31/23 11:22, Christoph Müllner wrote:
> > On Mon, Mar 27, 2023 at 7:18 PM Richard Henderson
> > wrote:
> >>
> >> On 3/27/23 01:00, Christoph Muellner wrote:
> >>> +uint64_t helper_fminm_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
> >>> +{
> >>> +float32 frs1 = check_nanbox_s(env, rs1);
> >>> +float32 frs2 = check_nanbox_s(env, rs2);
> >>> +
> >>> +if (float32_is_any_nan(frs1) || float32_is_any_nan(frs2)) {
> >>> +return float32_default_nan(>fp_status);
> >>> +}
> >>> +
> >>> +return nanbox_s(env, float32_minimum_number(frs1, frs2,
> >>> >fp_status));
> >>> +}
> >>
> >> Better to set and clear fp_status->default_nan_mode around the operation.
> >
> > I don't see how this can help:
> > * default_nan_mode defines if the default_nan is generated or if the
> > operand's NaN should be used
> > * RISC-V has default_nan_mode always set to true (operations should
> > return the a canonical NaN and not propagate NaN values)
> > * That also does not help to eliminate the is_any_nan() tests, because
> > float*_minimum_number() and float*_minnum() return the non-NaN number
> > if (only) one operand is NaN
> >
> > Am I missing something?
>
> Oh goodness, I did mis-read this.
>
> But if you need a nan when an input is a nan, then float32_min instead of
> float32_minimum_number (which goes out of its way to select the non-nan
> result) is the
> correct function to use.
Understood and fixed.
Thanks!
>
>
> r~
Re: [RFC PATCH v2] riscv: Add support for the Zfa extension
".
On Mon, Apr 10, 2023 at 3:23 PM LIU Zhiwei wrote:
>
>
> On 2023/4/1 2:28, Christoph Muellner wrote:
> > From: Christoph Müllner
> >
> > This patch introduces the RISC-V Zfa extension, which introduces
> > additional floating-point extensions:
> > * fli (load-immediate) with pre-defined immediates
> > * fminm/fmaxm (like fmin/fmax but with different NaN behaviour)
> > * fround/froundmx (round to integer)
> > * fcvtmod.w.d (Modular Convert-to-Integer)
> > * fmv* to access high bits of float register bigger than XLEN
> > * Quiet comparison instructions (fleq/fltq)
> >
> > Zfa defines its instructions in combination with the following extensions:
> > * single-precision floating-point (F)
> > * double-precision floating-point (D)
> > * quad-precision floating-point (Q)
> > * half-precision floating-point (Zfh)
> >
> > Since QEMU does not support the RISC-V quad-precision floating-point
> > ISA extension (Q), this patch does not include the instructions that
> > depend on this extension. All other instructions are included in this
> > patch.
> >
> > The Zfa specification is not frozen at the moment (which is why this
> > patch is RFC) and can be found here:
> >https://github.com/riscv/riscv-isa-manual/blob/master/src/zfa.tex
> >
> > Signed-off-by: Christoph Müllner
> > ---
> > Changes in v2:
> > * Remove calls to mark_fs_dirty() in comparison trans functions
> > * Rewrite fround(nx) using float*_round_to_int()
> > * Move fli* to translation unit and fix NaN-boxing of NaN values
> > * Reimplement FCVTMOD.W.D
> > * Add use of second register in trans_fmvp_d_x()
> >
> > target/riscv/cpu.c| 8 +
> > target/riscv/cpu.h| 1 +
> > target/riscv/fpu_helper.c | 258 +++
> > target/riscv/helper.h | 19 +
> > target/riscv/insn32.decode| 67 +++
> > target/riscv/insn_trans/trans_rvzfa.c.inc | 529 ++
> > target/riscv/translate.c | 1 +
> > 7 files changed, 883 insertions(+)
> > create mode 100644 target/riscv/insn_trans/trans_rvzfa.c.inc
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 1e97473af2..bac9ced4a2 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -83,6 +83,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
> > ISA_EXT_DATA_ENTRY(zifencei, true, PRIV_VERSION_1_10_0, ext_ifencei),
> > ISA_EXT_DATA_ENTRY(zihintpause, true, PRIV_VERSION_1_10_0,
> > ext_zihintpause),
> > ISA_EXT_DATA_ENTRY(zawrs, true, PRIV_VERSION_1_12_0, ext_zawrs),
> > +ISA_EXT_DATA_ENTRY(zfa, true, PRIV_VERSION_1_12_0, ext_zfa),
> > ISA_EXT_DATA_ENTRY(zfh, true, PRIV_VERSION_1_11_0, ext_zfh),
> > ISA_EXT_DATA_ENTRY(zfhmin, true, PRIV_VERSION_1_12_0, ext_zfhmin),
> > ISA_EXT_DATA_ENTRY(zfinx, true, PRIV_VERSION_1_12_0, ext_zfinx),
> > @@ -404,6 +405,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
> > cpu->cfg.ext_u = true;
> > cpu->cfg.ext_s = true;
> > cpu->cfg.ext_icsr = true;
> > +cpu->cfg.ext_zfa = true;
> > cpu->cfg.ext_zfh = true;
> > cpu->cfg.mmu = true;
> > cpu->cfg.ext_xtheadba = true;
> > @@ -865,6 +867,11 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU
> > *cpu, Error **errp)
> > return;
> > }
> >
> > +if (cpu->cfg.ext_zfa && !cpu->cfg.ext_f) {
> > +error_setg(errp, "Zfa extension requires F extension");
> > +return;
> > +}
> > +
> > if (cpu->cfg.ext_zfh) {
> > cpu->cfg.ext_zfhmin = true;
> > }
> > @@ -1381,6 +1388,7 @@ static Property riscv_cpu_extensions[] = {
> > DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> > DEFINE_PROP_BOOL("Zihintpause", RISCVCPU, cfg.ext_zihintpause, true),
> > DEFINE_PROP_BOOL("Zawrs", RISCVCPU, cfg.ext_zawrs, true),
> > +DEFINE_PROP_BOOL("Zfa", RISCVCPU, cfg.ext_zfa, false),
> > DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
> > DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
> > DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 638e47c75a..deae410fc2 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/risc
Re: [PATCH] riscv: Add support for the Zfa extension
On Mon, Mar 27, 2023 at 10:42 AM liweiwei wrote:
>
>
> On 2023/3/27 16:00, Christoph Muellner wrote:
> > From: Christoph Müllner
> >
> > This patch introduces the RISC-V Zfa extension, which introduces
> > additional floating-point extensions:
> > * fli (load-immediate) with pre-defined immediates
> > * fminm/fmaxm (like fmin/fmax but with different NaN behaviour)
> > * fround/froundmx (round to integer)
> > * fcvtmod.w.d (Modular Convert-to-Integer)
> > * fmv* to access high bits of float register bigger than XLEN
> > * Quiet comparison instructions (fleq/fltq)
> >
> > Zfa defines its instructions in combination with the following extensions:
> > * single-precision floating-point (F)
> > * double-precision floating-point (D)
> > * quad-precision floating-point (Q)
> > * half-precision floating-point (Zfh)
> >
> > Since QEMU does not support the RISC-V quad-precision floating-point
> > ISA extension (Q), this patch does not include the instructions that
> > depend on this extension. All other instructions are included in this
> > patch.
> >
> > The Zfa specification is not frozen at the moment (which is why this
> > patch is RFC) and can be found here:
> >https://github.com/riscv/riscv-isa-manual/blob/master/src/zfa.tex
> >
> > Signed-off-by: Christoph Müllner
> > ---
> > target/riscv/cpu.c| 8 +
> > target/riscv/cpu.h| 1 +
> > target/riscv/fpu_helper.c | 324 +
> > target/riscv/helper.h | 22 ++
> > target/riscv/insn32.decode| 67
> > target/riscv/insn_trans/trans_rvzfa.c.inc | 410 ++
> > target/riscv/translate.c | 1 +
> > 7 files changed, 833 insertions(+)
> > create mode 100644 target/riscv/insn_trans/trans_rvzfa.c.inc
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 1e97473af2..bac9ced4a2 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -83,6 +83,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
> > ISA_EXT_DATA_ENTRY(zifencei, true, PRIV_VERSION_1_10_0, ext_ifencei),
> > ISA_EXT_DATA_ENTRY(zihintpause, true, PRIV_VERSION_1_10_0,
> > ext_zihintpause),
> > ISA_EXT_DATA_ENTRY(zawrs, true, PRIV_VERSION_1_12_0, ext_zawrs),
> > +ISA_EXT_DATA_ENTRY(zfa, true, PRIV_VERSION_1_12_0, ext_zfa),
> > ISA_EXT_DATA_ENTRY(zfh, true, PRIV_VERSION_1_11_0, ext_zfh),
> > ISA_EXT_DATA_ENTRY(zfhmin, true, PRIV_VERSION_1_12_0, ext_zfhmin),
> > ISA_EXT_DATA_ENTRY(zfinx, true, PRIV_VERSION_1_12_0, ext_zfinx),
> > @@ -404,6 +405,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
> > cpu->cfg.ext_u = true;
> > cpu->cfg.ext_s = true;
> > cpu->cfg.ext_icsr = true;
> > +cpu->cfg.ext_zfa = true;
> > cpu->cfg.ext_zfh = true;
> > cpu->cfg.mmu = true;
> > cpu->cfg.ext_xtheadba = true;
> > @@ -865,6 +867,11 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU
> > *cpu, Error **errp)
> > return;
> > }
> >
> > +if (cpu->cfg.ext_zfa && !cpu->cfg.ext_f) {
> > +error_setg(errp, "Zfa extension requires F extension");
> > +return;
> > +}
> > +
> > if (cpu->cfg.ext_zfh) {
> > cpu->cfg.ext_zfhmin = true;
> > }
> > @@ -1381,6 +1388,7 @@ static Property riscv_cpu_extensions[] = {
> > DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> > DEFINE_PROP_BOOL("Zihintpause", RISCVCPU, cfg.ext_zihintpause, true),
> > DEFINE_PROP_BOOL("Zawrs", RISCVCPU, cfg.ext_zawrs, true),
> > +DEFINE_PROP_BOOL("Zfa", RISCVCPU, cfg.ext_zfa, false),
> > DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
> > DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
> > DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 638e47c75a..deae410fc2 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -462,6 +462,7 @@ struct RISCVCPUConfig {
> > bool ext_svpbmt;
> > bool ext_zdinx;
> > bool ext_zawrs;
> > +bool ext_zfa;
> > bool ext_zfh;
> > bool ext_zfhmin;
> > bool ext_zfinx;
> > diff --git a/target/riscv/fpu_helper.c b/t
Re: [PATCH] riscv: Add support for the Zfa extension
On Mon, Mar 27, 2023 at 7:18 PM Richard Henderson
wrote:
>
> On 3/27/23 01:00, Christoph Muellner wrote:
> > +uint64_t helper_fminm_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
> > +{
> > +float32 frs1 = check_nanbox_s(env, rs1);
> > +float32 frs2 = check_nanbox_s(env, rs2);
> > +
> > +if (float32_is_any_nan(frs1) || float32_is_any_nan(frs2)) {
> > +return float32_default_nan(>fp_status);
> > +}
> > +
> > +return nanbox_s(env, float32_minimum_number(frs1, frs2,
> > >fp_status));
> > +}
>
> Better to set and clear fp_status->default_nan_mode around the operation.
I don't see how this can help:
* default_nan_mode defines if the default_nan is generated or if the
operand's NaN should be used
* RISC-V has default_nan_mode always set to true (operations should
return the a canonical NaN and not propagate NaN values)
* That also does not help to eliminate the is_any_nan() tests, because
float*_minimum_number() and float*_minnum() return the non-NaN number
if (only) one operand is NaN
Am I missing something?
>
> > +uint64_t helper_fround_s(CPURISCVState *env, uint64_t frs1)
> > +{
> > +if (float32_is_zero(frs1) ||
> > +float32_is_infinity(frs1)) {
> > +return frs1;
> > +}
> > +
> > +if (float32_is_any_nan(frs1)) {
> > +riscv_cpu_set_fflags(env, FPEXC_NV);
> > +return frs1;
> > +}
> > +
> > +int32_t tmp = float32_to_int32(frs1, >fp_status);
> > +return nanbox_s(env, int32_to_float32(tmp, >fp_status));
> > +}
>
> Very much incorrect, since int32_t does not have the range for the
> intermediate result.
> In any case, the function you want is float32_round_to_int, which eliminates
> the
> zero/inf/nan special cases. It will raise inexact, so perfect for froundnx,
> but you'll
> need to save/restore float_flag_inexact around the function.
Understood the issue and changed to the proposed API.
>
> > +uint64_t helper_fli_s(CPURISCVState *env, uint32_t rs1)
> > +{
> > +const uint32_t fli_s_table[] = {
>
> static const. You don't need to use float32_default_nan, use the correct
> architected
> constant. This entire operation should be done at translation time.
Done.
>
> > +target_ulong helper_fcvtmod_w_d(CPURISCVState *env, uint64_t frs1)
> > +{
> > +if (float64_is_any_nan(frs1) ||
> > +float64_is_infinity(frs1)) {
> > +return 0;
> > +}
> > +
> > +return float64_to_int32(frs1, >fp_status);
> > +}
>
> Incorrect, as float64_to_int32 will saturate the result, whereas you need the
> modular result.
>
> There is code to do the conversion mod 2**64 in target/alpha/ (do_cvttq). We
> should move
> this to generic code if it is to be used by more than one target.
Understood the issue.
ARM has something similar in HELPER(fjcvtzs).
Given the different flag behaviour of the ARM and the Alpha
instructions, I created a RISC-V specific routine.
For RISC-V the flags have to be identical to fcvt.w.d with the same value.
>
> > +bool trans_fmvp_d_x(DisasContext *ctx, arg_fmvp_d_x *a)
> > +{
> > +REQUIRE_FPU;
> > +REQUIRE_ZFA(ctx);
> > +REQUIRE_EXT(ctx, RVD);
> > +REQUIRE_32BIT(ctx);
> > +
> > +TCGv src1 = get_gpr(ctx, a->rs1, EXT_ZERO);
> > +TCGv_i64 t1 = tcg_temp_new_i64();
> > +
> > +tcg_gen_extu_tl_i64(t1, src1);
> > +tcg_gen_deposit_i64(cpu_fpr[a->rd], cpu_fpr[a->rd], t1, 32, 32);
> > +mark_fs_dirty(ctx);
> > +return true;
> > +}
>
> This does not match the linked document, which says that this insn has two
> inputs and sets
> the complete fpr.
Fixed.
Thanks!
Re: [PATCH 00/45] Add RISC-V vector cryptographic instruction set support
On Thu, Mar 23, 2023 at 12:34 PM Lawrence Hunter
wrote:
>
> On 21/03/2023 12:02, Christoph Müllner wrote:
> > On Fri, Mar 10, 2023 at 10:16 AM Lawrence Hunter
> > wrote:
> >>
> >> This patchset provides an implementation for Zvkb, Zvkned, Zvknh,
> >> Zvksh, Zvkg, and Zvksed of the draft RISC-V vector cryptography
> >> extensions as per the 20230303 version of the specification(1)
> >> (1fcbb30). Please note that the Zvkt data-independent execution
> >> latency extension has not been implemented, and we would recommend not
> >> using these patches in an environment where timing attacks are an
> >> issue.
> >>
> >> Work performed by Dickon, Lawrence, Nazar, Kiran, and William from
> >> Codethink sponsored by SiFive, as well as Max Chou and Frank Chang
> >> from SiFive.
> >>
> >> For convenience we have created a git repo with our patches on top of
> >> a recent master. https://github.com/CodethinkLabs/qemu-ct
> >
> > I did test and review this patchset.
> > Since most of my comments affect multiple patches I have summarized
> > them here in one email.
> > Observations that only affect a single patch will be sent in response
> > to the corresponding email.
> >
> > I have tested this series with the OpenSSL PR for Zvk that can be found
> > here:
> >https://github.com/openssl/openssl/pull/20149
> > I ran with all Zvk* extensions enabled (using Zvkg for GCM) and with
> > Zvkb only (using Zvkb for GCM).
> > All tests succeed. Note, however, that the test coverage is limited
> > (e.g. no .vv instructions, vstart is always zero).
> >
> > When sending out a follow-up version (even if it just introduces a
> > minimal fix),
> > then consider using patchset versioning (e.g. git format-patch -v2
> > ...).
>
> Ok, will do
>
> > It might be a matter of taste, but I would prefer a series that groups
> > and orders the commits differently:
> >a) independent changes to the existing code (refactoring only, but
> > no new features) - one commit per topic
> >b) introduction of new functionality - one commit per extension
> > A series using such a commit granularity and order would be easier to
> > maintain and review (and not result in 45 patches).
> > Also, the refactoring changes could land before Zvk freezes if
> > maintainers decide to do so.
>
> Makes sense, will do
>
> > So far all translation files in target/riscv/insn_trans/* contain
> > multiple extensions if they are related.
> > I think we should follow this pattern and use a common trans_zvk.c.inc
> > file.
>
> Agree, will do
>
> > All patches to insn32.decode have comments of the form "RV64 Zvk*
> > vector crypto extension".
> > What is the point of the "RV64"? I would simply remove that.
>
> Ok, will remove it
>
> > All instructions set "env->vstart = 0;" at the end.
> > I don't think that this is correct (the specification does not require
> > this).
>
> That's from vector spec: "All vector instructions are defined to begin
> execution with the element number given in the vstart CSR, leaving
> earlier elements in the destination vector undisturbed, and to reset the
> vstart CSR to zero at the end of execution." - from "3.7. Vector Start
> Index CSR vstart"
Yes, you are right.
I just created a PR for the Zvk* spec to clarify this:
https://github.com/riscv/riscv-crypto/pull/308
>
> > The tests of the reserved encodings are not consistent:
> > * Zvknh does a dynamic test (query tcg_gen_*())
> > * Zvkned does a dynamic test (tcg_gen_*())
> > * Zvkg does not test for (vl%EGS == 0)
>
> Zvkg also does dynamic test, by calling macros GEN_V_UNMASKED_TRANS and
> GEN_VV_UNMASKED_TRANS
>
> > The vl CSR can only be updated by the vset{i}vl{i} instructions.
> > The same applies to the vstart CSR and the vtype CSR that holds vsew,
> > vlmul and other fields.
> > The current code tests the VSTART/SEW value using "s->vstart % 4 ==
> > 0"/"s->sew == MO_32".
> > Why is it not possible to do the same with VL, i.e. "s->vl % 4 == 0"
> > (after adding it to DisasContext)?
>
> vl can also be updated by another instruction - from vector spec "3.5.
> Vector Length Register vl" - "The XLEN-bit-wide read-only vl CSR can
> only be updated by the vset{i}vl{i} instructions, and the
> fault-only-first vector load instruction variants." So just because of
> that fault-only-first instruction we need dynamic checks.
>
> vstart is just another CSR -- software can write to it, but probably
> shouldn't. Whether that's ever going to be useful outside testing ISA
> conformance tests or not I don't know, but it's clearly read-write (also
> section 3.7).
>
> > Also, I would introduce named constants or macros for the EGS values
> > to avoid magic constants in the code
> > (some extensions do that - e.g. ZVKSED_EGS).
>
> Makes sense, will do
>
> Best,
> Lawrence
Re: [PATCH 00/45] Add RISC-V vector cryptographic instruction set support
On Fri, Mar 10, 2023 at 10:16 AM Lawrence Hunter
wrote:
>
> This patchset provides an implementation for Zvkb, Zvkned, Zvknh, Zvksh,
> Zvkg, and Zvksed of the draft RISC-V vector cryptography extensions as per
> the 20230303 version of the specification(1) (1fcbb30). Please note that the
> Zvkt data-independent execution latency extension has not been implemented,
> and we would recommend not using these patches in an environment where timing
> attacks are an issue.
>
> Work performed by Dickon, Lawrence, Nazar, Kiran, and William from Codethink
> sponsored by SiFive, as well as Max Chou and Frank Chang from SiFive.
>
> For convenience we have created a git repo with our patches on top of a
> recent master. https://github.com/CodethinkLabs/qemu-ct
I did test and review this patchset.
Since most of my comments affect multiple patches I have summarized
them here in one email.
Observations that only affect a single patch will be sent in response
to the corresponding email.
I have tested this series with the OpenSSL PR for Zvk that can be found here:
https://github.com/openssl/openssl/pull/20149
I ran with all Zvk* extensions enabled (using Zvkg for GCM) and with
Zvkb only (using Zvkb for GCM).
All tests succeed. Note, however, that the test coverage is limited
(e.g. no .vv instructions, vstart is always zero).
When sending out a follow-up version (even if it just introduces a minimal fix),
then consider using patchset versioning (e.g. git format-patch -v2 ...).
It might be a matter of taste, but I would prefer a series that groups
and orders the commits differently:
a) independent changes to the existing code (refactoring only, but
no new features) - one commit per topic
b) introduction of new functionality - one commit per extension
A series using such a commit granularity and order would be easier to
maintain and review (and not result in 45 patches).
Also, the refactoring changes could land before Zvk freezes if
maintainers decide to do so.
So far all translation files in target/riscv/insn_trans/* contain
multiple extensions if they are related.
I think we should follow this pattern and use a common trans_zvk.c.inc file.
All patches to insn32.decode have comments of the form "RV64 Zvk*
vector crypto extension".
What is the point of the "RV64"? I would simply remove that.
All instructions set "env->vstart = 0;" at the end.
I don't think that this is correct (the specification does not require this).
The tests of the reserved encodings are not consistent:
* Zvknh does a dynamic test (query tcg_gen_*())
* Zvkned does a dynamic test (tcg_gen_*())
* Zvkg does not test for (vl%EGS == 0)
The vl CSR can only be updated by the vset{i}vl{i} instructions.
The same applies to the vstart CSR and the vtype CSR that holds vsew,
vlmul and other fields.
The current code tests the VSTART/SEW value using "s->vstart % 4 ==
0"/"s->sew == MO_32".
Why is it not possible to do the same with VL, i.e. "s->vl % 4 == 0"
(after adding it to DisasContext)?
Also, I would introduce named constants or macros for the EGS values
to avoid magic constants in the code
(some extensions do that - e.g. ZVKSED_EGS).
BR
Christoph
>
> 1. https://github.com/riscv/riscv-crypto/releases
>
>
> Dickon Hood (2):
> qemu/bitops.h: Limit rotate amounts
> target/riscv: Add vrol.[vv,vx] and vror.[vv,vx,vi] decoding,
> translation and execution support
>
> Kiran Ostrolenk (8):
> target/riscv: Refactor some of the generic vector functionality
> target/riscv: Refactor some of the generic vector functionality
> target/riscv: Refactor some of the generic vector functionality
> target/riscv: Refactor some of the generic vector functionality
> target/riscv: Add vsha2ms.vv decoding, translation and execution
> support
> target/riscv: Add zvksh cpu property
> target/riscv: Add vsm3c.vi decoding, translation and execution support
> target/riscv: Expose zvksh cpu property
>
> Lawrence Hunter (17):
> target/riscv: Add vclmul.vv decoding, translation and execution
> support
> target/riscv: Add vclmul.vx decoding, translation and execution
> support
> target/riscv: Add vclmulh.vv decoding, translation and execution
> support
> target/riscv: Add vclmulh.vx decoding, translation and execution
> support
> target/riscv: Add vaesef.vv decoding, translation and execution
> support
> target/riscv: Add vaesef.vs decoding, translation and execution
> support
> target/riscv: Add vaesdf.vv decoding, translation and execution
> support
> target/riscv: Add vaesdf.vs decoding, translation and execution
> support
> target/riscv: Add vaesdm.vv decoding, translation and execution
> support
> target/riscv: Add vaesdm.vs decoding, translation and execution
> support
> target/riscv: Add vaesz.vs decoding, translation and execution support
> target/riscv: Add vsha2c[hl].vv decoding, translation and execution
> support
> target/riscv: Add vsm3me.vv decoding, translation and
Re: [PATCH 02/45] target/riscv: Refactor some of the generic vector functionality
On Fri, Mar 10, 2023 at 5:06 PM Lawrence Hunter
wrote:
>
> From: Kiran Ostrolenk
>
> Summary of refactoring:
>
> * take some functions/macros out of `vector_helper` and put them in a
> new module called `vector_internals`
>
> * factor the non SEW-specific stuff out of `GEN_OPIVV_TRANS` into
> function `opivv_trans` (similar to `opivi_trans`)
I think splitting this commit into two changes would be better.
Besides that the two changes look reasonable and correct.
BR
Christoph
>
> All this refactoring ensures more functions/macros can be used by both
> vector and vector-crypto helpers (latter implemented in proceeding
> commit).
>
> Signed-off-by: Kiran Ostrolenk
> ---
> target/riscv/insn_trans/trans_rvv.c.inc | 62 +-
> target/riscv/meson.build| 1 +
> target/riscv/vector_helper.c| 155 +---
> target/riscv/vector_internals.c | 57 +
> target/riscv/vector_internals.h | 155
> 5 files changed, 246 insertions(+), 184 deletions(-)
> create mode 100644 target/riscv/vector_internals.c
> create mode 100644 target/riscv/vector_internals.h
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
> b/target/riscv/insn_trans/trans_rvv.c.inc
> index f2e3d38515..4106bd6994 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -1643,38 +1643,40 @@ GEN_OPIWX_WIDEN_TRANS(vwadd_wx)
> GEN_OPIWX_WIDEN_TRANS(vwsubu_wx)
> GEN_OPIWX_WIDEN_TRANS(vwsub_wx)
>
> +static bool opivv_trans(uint32_t vd, uint32_t vs1, uint32_t vs2, uint32_t vm,
> +gen_helper_gvec_4_ptr *fn, DisasContext *s)
> +{
> +uint32_t data = 0;
> +TCGLabel *over = gen_new_label();
> +tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
> +tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
> +
> +data = FIELD_DP32(data, VDATA, VM, vm);
> +data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
> +data = FIELD_DP32(data, VDATA, VTA, s->vta);
> +data = FIELD_DP32(data, VDATA, VTA_ALL_1S, s->cfg_vta_all_1s);
> +data = FIELD_DP32(data, VDATA, VMA, s->vma);
> +tcg_gen_gvec_4_ptr(vreg_ofs(s, vd), vreg_ofs(s, 0), vreg_ofs(s, vs1),
> + vreg_ofs(s, vs2), cpu_env, s->cfg_ptr->vlen / 8,
> + s->cfg_ptr->vlen / 8, data, fn);
> +mark_vs_dirty(s);
> +gen_set_label(over);
> +return true;
> +}
> +
> /* Vector Integer Add-with-Carry / Subtract-with-Borrow Instructions */
> /* OPIVV without GVEC IR */
> -#define GEN_OPIVV_TRANS(NAME, CHECK) \
> -static bool trans_##NAME(DisasContext *s, arg_rmrr *a) \
> -{ \
> -if (CHECK(s, a)) { \
> -uint32_t data = 0; \
> -static gen_helper_gvec_4_ptr * const fns[4] = {\
> -gen_helper_##NAME##_b, gen_helper_##NAME##_h, \
> -gen_helper_##NAME##_w, gen_helper_##NAME##_d, \
> -}; \
> -TCGLabel *over = gen_new_label(); \
> -tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over); \
> -tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); \
> - \
> -data = FIELD_DP32(data, VDATA, VM, a->vm); \
> -data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \
> -data = FIELD_DP32(data, VDATA, VTA, s->vta); \
> -data = \
> -FIELD_DP32(data, VDATA, VTA_ALL_1S, s->cfg_vta_all_1s);\
> -data = FIELD_DP32(data, VDATA, VMA, s->vma); \
> -tcg_gen_gvec_4_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0), \
> - vreg_ofs(s, a->rs1),\
> - vreg_ofs(s, a->rs2), cpu_env, \
> - s->cfg_ptr->vlen / 8, \
> - s->cfg_ptr->vlen / 8, data, \
> - fns[s->sew]); \
> -mark_vs_dirty(s); \
> -gen_set_label(over); \
> -return true; \
> -} \
> -return false; \
> +#define GEN_OPIVV_TRANS(NAME, CHECK) \
> +static bool trans_##NAME(DisasContext *s, arg_rmrr *a) \
> +{\
> +if (CHECK(s, a)) {
Re: [PATCH v4 08/14] RISC-V: Adding T-Head MemPair extension
On Tue, Jan 31, 2023 at 7:23 PM Richard Henderson
wrote:
>
> On 1/31/23 08:01, Christoph Muellner wrote:
> > +if ((memop & MO_SIZE) == MO_64) {
> > +addr2 = get_address(ctx, a->rs, 8 + (a->sh2 << shamt));
> > +} else {
> > +addr2 = get_address(ctx, a->rs, 4 + (a->sh2 << shamt));
> > +}
>
> Use memop_size(memop) instead.
Will be part of v5 (will be sent in a couple of minutes).
I have also added a "int imm = a->sh2 << shamt;".
>
> Otherwise,
> Reviewed-by: Richard Henderson
Thanks for the review!
>
>
> r~
Re: [PATCH v3 09/14] RISC-V: Adding T-Head MemIdx extension
On Tue, Jan 24, 2023 at 10:21 PM Richard Henderson
wrote:
>
> On 1/24/23 09:59, Christoph Muellner wrote:
> > +/* XTheadMemIdx */
> > +
> > +/*
> > + * Load with memop from indexed address and add (imm5 << imm2) to rs1.
> > + * If !preinc, then the load address is rs1.
> > + * If preinc, then the load address is rs1 + (imm5) << imm2).
> > + */
> > +static bool gen_load_inc(DisasContext *ctx, arg_th_meminc *a, MemOp memop,
> > + bool preinc)
> > +{
> > +TCGv rd = dest_gpr(ctx, a->rd);
> > +TCGv addr = get_address(ctx, a->rs1, preinc ? a->imm5 << a->imm2 : 0);
> > +
> > +tcg_gen_qemu_ld_tl(rd, addr, ctx->mem_idx, memop);
> > +addr = get_address(ctx, a->rs1, !preinc ? a->imm5 << a->imm2 : 0);
>
> First, you're leaking the previous 'addr' temporary.
Indeed!
The real question is of course, why we call get_address() twice...
Fixed in v4.
> Second, get_address may make modifications to 'addr' which you don't want to
> write back.
Fixed in v4.
> Third, you are not checking for rd != rs1.
Fixed in v4.
>
> I think what you want is
>
> int imm = a->imm5 << a->imm2;
> TCGv addr = get_address(ctx, a->rs1, preinc ? imm : 0);
> TCGv rd = dest_gpr(ctx, a->rd);
> TCGv rs1 = get_gpr(ctx, a->rs1, EXT_NONE);
>
> tcg_gen_qemu_ld_tl(rd, addr, ctx->mem_idx, memop);
> tcg_gen_addi_tl(rs1, rs1, imm);
> gen_set_gpr(ctx, a->rd, rd);
> gen_set_gpr(ctx, a->rs1, rs1);
Yes (plus the check for rd != rs1).
Fixed in v4.
Thank you!
>
>
> r~
Re: [PATCH v3 12/14] RISC-V: Add initial support for T-Head C906
On Tue, Jan 24, 2023 at 10:26 PM Richard Henderson wrote: > > On 1/24/23 09:59, Christoph Muellner wrote: > > +++ b/target/riscv/cpu.h > > @@ -27,6 +27,7 @@ > > #include "qom/object.h" > > #include "qemu/int128.h" > > #include "cpu_bits.h" > > +#include "cpu_vendorid.h" > > I don't see that this ID is required for all users of riscv/cpu.h. > This include should be limited to cpu.c. Fixed in v4. Thank you! > > > r~
Re: [PATCH v3 08/14] RISC-V: Adding T-Head MemPair extension
On Tue, Jan 24, 2023 at 9:44 PM Richard Henderson
wrote:
>
> On 1/24/23 09:59, Christoph Muellner wrote:
> > +static bool gen_loadpair_tl(DisasContext *ctx, arg_th_pair *a, MemOp memop,
> > +int shamt)
> > +{
> > +TCGv rd1 = dest_gpr(ctx, a->rd1);
> > +TCGv rd2 = dest_gpr(ctx, a->rd2);
> > +TCGv addr1 = tcg_temp_new();
> > +TCGv addr2 = tcg_temp_new();
> > +
> > +addr1 = get_address(ctx, a->rs, a->sh2 << shamt);
> > +if ((memop & MO_SIZE) == MO_64) {
> > +addr2 = get_address(ctx, a->rs, 8 + (a->sh2 << shamt));
> > +} else {
> > +addr2 = get_address(ctx, a->rs, 4 + (a->sh2 << shamt));
> > +}
> > +
> > +tcg_gen_qemu_ld_tl(rd1, addr1, ctx->mem_idx, memop);
> > +tcg_gen_qemu_ld_tl(rd2, addr2, ctx->mem_idx, memop);
> > +gen_set_gpr(ctx, a->rd1, rd1);
> > +gen_set_gpr(ctx, a->rd2, rd2);
>
> Since dest_gpr may return cpu_gpr[n], this may update the rd1 before
> recognizing the
> exception that the second load may generate. Is that correct?
Solved in v4 by using temporaries.
>
> The manual says that rd1, rd2, and rs1 must not be the same, but you do not
> check this.
Fixed in v4.
Thank you!
>
>
> r~
Re: [PATCH v2 01/15] RISC-V: Adding XTheadCmo ISA extension
On Sun, Jan 29, 2023 at 11:40 PM Alistair Francis wrote:
>
> On Wed, Jan 25, 2023 at 5:51 AM Christoph Müllner
> wrote:
> >
> >
> >
> > On Tue, Jan 24, 2023 at 6:31 PM Christoph Müllner
> > wrote:
> >>
> >>
> >>
> >> On Mon, Jan 23, 2023 at 11:50 PM Alistair Francis
> >> wrote:
> >>>
> >>> On Sat, Dec 24, 2022 at 4:09 AM Christoph Muellner
> >>> wrote:
> >>> >
> >>> > From: Christoph Müllner
> >>> >
> >>> > This patch adds support for the XTheadCmo ISA extension.
> >>> > To avoid interfering with standard extensions, decoder and translation
> >>> > are in its own xthead* specific files.
> >>> > Future patches should be able to easily add additional T-Head extension.
> >>> >
> >>> > The implementation does not have much functionality (besides accepting
> >>> > the instructions and not qualifying them as illegal instructions if
> >>> > the hart executes in the required privilege level for the instruction),
> >>> > as QEMU does not model CPU caches and instructions are documented
> >>> > to not raise any exceptions.
> >>> >
> >>> > Changes in v2:
> >>> > - Add ISA_EXT_DATA_ENTRY()
> >>> > - Explicit test for PRV_U
> >>> > - Encapsule access to env-priv in inline function
> >>> > - Use single decoder for XThead extensions
> >>> >
> >>> > Co-developed-by: LIU Zhiwei
> >>> > Signed-off-by: Christoph Müllner
> >>> > ---
> >>> > target/riscv/cpu.c | 2 +
> >>> > target/riscv/cpu.h | 1 +
> >>> > target/riscv/insn_trans/trans_xthead.c.inc | 89 ++
> >>> > target/riscv/meson.build | 1 +
> >>> > target/riscv/translate.c | 15 +++-
> >>> > target/riscv/xthead.decode | 38 +
> >>> > 6 files changed, 143 insertions(+), 3 deletions(-)
> >>> > create mode 100644 target/riscv/insn_trans/trans_xthead.c.inc
> >>> > create mode 100644 target/riscv/xthead.decode
> >>> >
> >>> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> >>> > index 6fe176e483..a90b82c5c5 100644
> >>> > --- a/target/riscv/cpu.c
> >>> > +++ b/target/riscv/cpu.c
> >>> > @@ -108,6 +108,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
> >>> > ISA_EXT_DATA_ENTRY(svinval, true, PRIV_VERSION_1_12_0,
> >>> > ext_svinval),
> >>> > ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0,
> >>> > ext_svnapot),
> >>> > ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
> >>> > +ISA_EXT_DATA_ENTRY(xtheadcmo, true, PRIV_VERSION_1_11_0,
> >>> > ext_xtheadcmo),
> >>> > ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0,
> >>> > ext_XVentanaCondOps),
> >>> > };
> >>> >
> >>> > @@ -1060,6 +1061,7 @@ static Property riscv_cpu_extensions[] = {
> >>> > DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
> >>> >
> >>> > /* Vendor-specific custom extensions */
> >>> > +DEFINE_PROP_BOOL("xtheadcmo", RISCVCPU, cfg.ext_xtheadcmo, false),
> >>> > DEFINE_PROP_BOOL("xventanacondops", RISCVCPU,
> >>> > cfg.ext_XVentanaCondOps, false),
> >>> >
> >>> > /* These are experimental so mark with 'x-' */
> >>> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> >>> > index 443d15a47c..ad1c19f870 100644
> >>> > --- a/target/riscv/cpu.h
> >>> > +++ b/target/riscv/cpu.h
> >>> > @@ -465,6 +465,7 @@ struct RISCVCPUConfig {
> >>> > uint64_t mimpid;
> >>> >
> >>> > /* Vendor-specific custom extensions */
> >>> > +bool ext_xtheadcmo;
> >>> > bool ext_XVentanaCondOps;
> >>> >
> >>> > uint8_t pmu_num;
> >>> > diff --git a/target/riscv/insn_trans/trans_xthead.c.inc
> >>> > b/target/riscv/insn_trans/trans_xthead.c.inc
> >>> > new fil
Re: [PATCH v3 02/14] RISC-V: Adding XTheadSync ISA extension
On Tue, Jan 24, 2023 at 9:21 PM Richard Henderson <
richard.hender...@linaro.org> wrote:
> On 1/24/23 09:59, Christoph Muellner wrote:
> > +static bool trans_th_sfence_vmas(DisasContext *ctx, arg_th_sfence_vmas
> *a)
> > +{
> > +(void) a;
> > +REQUIRE_XTHEADSYNC(ctx);
> > +
> > +#ifndef CONFIG_USER_ONLY
> > +REQUIRE_PRIV_MS(ctx);
> > +decode_save_opc(ctx);
> > +gen_helper_tlb_flush_all(cpu_env);
>
> Why are you using decode_save_opc() when helper_tlb_flush_all() cannot
> raise an exception?
>
No particular reason.
Will be dropped.
Thanks,
Christoph
>
>
> r~
>
Re: [PATCH v2 01/15] RISC-V: Adding XTheadCmo ISA extension
On Tue, Jan 24, 2023 at 6:31 PM Christoph Müllner <
christoph.muell...@vrull.eu> wrote:
>
>
> On Mon, Jan 23, 2023 at 11:50 PM Alistair Francis
> wrote:
>
>> On Sat, Dec 24, 2022 at 4:09 AM Christoph Muellner
>> wrote:
>> >
>> > From: Christoph Müllner
>> >
>> > This patch adds support for the XTheadCmo ISA extension.
>> > To avoid interfering with standard extensions, decoder and translation
>> > are in its own xthead* specific files.
>> > Future patches should be able to easily add additional T-Head extension.
>> >
>> > The implementation does not have much functionality (besides accepting
>> > the instructions and not qualifying them as illegal instructions if
>> > the hart executes in the required privilege level for the instruction),
>> > as QEMU does not model CPU caches and instructions are documented
>> > to not raise any exceptions.
>> >
>> > Changes in v2:
>> > - Add ISA_EXT_DATA_ENTRY()
>> > - Explicit test for PRV_U
>> > - Encapsule access to env-priv in inline function
>> > - Use single decoder for XThead extensions
>> >
>> > Co-developed-by: LIU Zhiwei
>> > Signed-off-by: Christoph Müllner
>> > ---
>> > target/riscv/cpu.c | 2 +
>> > target/riscv/cpu.h | 1 +
>> > target/riscv/insn_trans/trans_xthead.c.inc | 89 ++
>> > target/riscv/meson.build | 1 +
>> > target/riscv/translate.c | 15 +++-
>> > target/riscv/xthead.decode | 38 +
>> > 6 files changed, 143 insertions(+), 3 deletions(-)
>> > create mode 100644 target/riscv/insn_trans/trans_xthead.c.inc
>> > create mode 100644 target/riscv/xthead.decode
>> >
>> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> > index 6fe176e483..a90b82c5c5 100644
>> > --- a/target/riscv/cpu.c
>> > +++ b/target/riscv/cpu.c
>> > @@ -108,6 +108,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
>> > ISA_EXT_DATA_ENTRY(svinval, true, PRIV_VERSION_1_12_0,
>> ext_svinval),
>> > ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0,
>> ext_svnapot),
>> > ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
>> > +ISA_EXT_DATA_ENTRY(xtheadcmo, true, PRIV_VERSION_1_11_0,
>> ext_xtheadcmo),
>> > ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0,
>> ext_XVentanaCondOps),
>> > };
>> >
>> > @@ -1060,6 +1061,7 @@ static Property riscv_cpu_extensions[] = {
>> > DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
>> >
>> > /* Vendor-specific custom extensions */
>> > +DEFINE_PROP_BOOL("xtheadcmo", RISCVCPU, cfg.ext_xtheadcmo, false),
>> > DEFINE_PROP_BOOL("xventanacondops", RISCVCPU,
>> cfg.ext_XVentanaCondOps, false),
>> >
>> > /* These are experimental so mark with 'x-' */
>> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> > index 443d15a47c..ad1c19f870 100644
>> > --- a/target/riscv/cpu.h
>> > +++ b/target/riscv/cpu.h
>> > @@ -465,6 +465,7 @@ struct RISCVCPUConfig {
>> > uint64_t mimpid;
>> >
>> > /* Vendor-specific custom extensions */
>> > +bool ext_xtheadcmo;
>> > bool ext_XVentanaCondOps;
>> >
>> > uint8_t pmu_num;
>> > diff --git a/target/riscv/insn_trans/trans_xthead.c.inc
>> b/target/riscv/insn_trans/trans_xthead.c.inc
>> > new file mode 100644
>> > index 00..00e75c7dca
>> > --- /dev/null
>> > +++ b/target/riscv/insn_trans/trans_xthead.c.inc
>> > @@ -0,0 +1,89 @@
>> > +/*
>> > + * RISC-V translation routines for the T-Head vendor extensions
>> (xthead*).
>> > + *
>> > + * Copyright (c) 2022 VRULL GmbH.
>> > + *
>> > + * This program is free software; you can redistribute it and/or
>> modify it
>> > + * under the terms and conditions of the GNU General Public License,
>> > + * version 2 or later, as published by the Free Software Foundation.
>> > + *
>> > + * This program is distributed in the hope it will be useful, but
>> WITHOUT
>> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
>> or
>> > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
>> License for
>> > + * more details.
>
Re: [PATCH v2 01/15] RISC-V: Adding XTheadCmo ISA extension
On Mon, Jan 23, 2023 at 11:50 PM Alistair Francis
wrote:
> On Sat, Dec 24, 2022 at 4:09 AM Christoph Muellner
> wrote:
> >
> > From: Christoph Müllner
> >
> > This patch adds support for the XTheadCmo ISA extension.
> > To avoid interfering with standard extensions, decoder and translation
> > are in its own xthead* specific files.
> > Future patches should be able to easily add additional T-Head extension.
> >
> > The implementation does not have much functionality (besides accepting
> > the instructions and not qualifying them as illegal instructions if
> > the hart executes in the required privilege level for the instruction),
> > as QEMU does not model CPU caches and instructions are documented
> > to not raise any exceptions.
> >
> > Changes in v2:
> > - Add ISA_EXT_DATA_ENTRY()
> > - Explicit test for PRV_U
> > - Encapsule access to env-priv in inline function
> > - Use single decoder for XThead extensions
> >
> > Co-developed-by: LIU Zhiwei
> > Signed-off-by: Christoph Müllner
> > ---
> > target/riscv/cpu.c | 2 +
> > target/riscv/cpu.h | 1 +
> > target/riscv/insn_trans/trans_xthead.c.inc | 89 ++
> > target/riscv/meson.build | 1 +
> > target/riscv/translate.c | 15 +++-
> > target/riscv/xthead.decode | 38 +
> > 6 files changed, 143 insertions(+), 3 deletions(-)
> > create mode 100644 target/riscv/insn_trans/trans_xthead.c.inc
> > create mode 100644 target/riscv/xthead.decode
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 6fe176e483..a90b82c5c5 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -108,6 +108,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
> > ISA_EXT_DATA_ENTRY(svinval, true, PRIV_VERSION_1_12_0, ext_svinval),
> > ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
> > ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
> > +ISA_EXT_DATA_ENTRY(xtheadcmo, true, PRIV_VERSION_1_11_0,
> ext_xtheadcmo),
> > ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0,
> ext_XVentanaCondOps),
> > };
> >
> > @@ -1060,6 +1061,7 @@ static Property riscv_cpu_extensions[] = {
> > DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
> >
> > /* Vendor-specific custom extensions */
> > +DEFINE_PROP_BOOL("xtheadcmo", RISCVCPU, cfg.ext_xtheadcmo, false),
> > DEFINE_PROP_BOOL("xventanacondops", RISCVCPU,
> cfg.ext_XVentanaCondOps, false),
> >
> > /* These are experimental so mark with 'x-' */
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 443d15a47c..ad1c19f870 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -465,6 +465,7 @@ struct RISCVCPUConfig {
> > uint64_t mimpid;
> >
> > /* Vendor-specific custom extensions */
> > +bool ext_xtheadcmo;
> > bool ext_XVentanaCondOps;
> >
> > uint8_t pmu_num;
> > diff --git a/target/riscv/insn_trans/trans_xthead.c.inc
> b/target/riscv/insn_trans/trans_xthead.c.inc
> > new file mode 100644
> > index 00..00e75c7dca
> > --- /dev/null
> > +++ b/target/riscv/insn_trans/trans_xthead.c.inc
> > @@ -0,0 +1,89 @@
> > +/*
> > + * RISC-V translation routines for the T-Head vendor extensions
> (xthead*).
> > + *
> > + * Copyright (c) 2022 VRULL GmbH.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but
> WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
> License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> along with
> > + * this program. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#define REQUIRE_XTHEADCMO(ctx) do { \
> > +if (!ctx->cfg_ptr->ext_xtheadcmo) { \
> > +return false;\
> > +}\
> > +} while (0)
> >
Re: [PATCH v2 11/15] RISC-V: Adding T-Head XMAE support
On Tue, Jan 24, 2023 at 12:49 AM Alistair Francis
wrote:
> On Sat, Dec 24, 2022 at 4:04 AM Christoph Muellner
> wrote:
> >
> > From: Christoph Müllner
> >
> > This patch adds support for the T-Head specific extended memory
> > attributes. Similar like Svpbmt, this support does not have much effect
> > as most behaviour is not modelled in QEMU.
> >
> > We also don't set any EDATA information, because XMAE discovery is done
> > using the vendor ID in the Linux kernel.
> >
> > Changes in v2:
> > - Add ISA_EXT_DATA_ENTRY()
> >
> > Co-developed-by: LIU Zhiwei
> > Signed-off-by: Christoph Müllner
> > ---
> > target/riscv/cpu.c| 2 ++
> > target/riscv/cpu.h| 1 +
> > target/riscv/cpu_helper.c | 6 --
> > 3 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 9c31a50e90..bb310755b1 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -118,6 +118,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
> > ISA_EXT_DATA_ENTRY(xtheadmemidx, true, PRIV_VERSION_1_11_0,
> ext_xtheadmemidx),
> > ISA_EXT_DATA_ENTRY(xtheadmempair, true, PRIV_VERSION_1_11_0,
> ext_xtheadmempair),
> > ISA_EXT_DATA_ENTRY(xtheadsync, true, PRIV_VERSION_1_11_0,
> ext_xtheadsync),
> > +ISA_EXT_DATA_ENTRY(xtheadxmae, true, PRIV_VERSION_1_11_0,
> ext_xtheadxmae),
> > ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0,
> ext_XVentanaCondOps),
> > };
> >
> > @@ -1080,6 +1081,7 @@ static Property riscv_cpu_extensions[] = {
> > DEFINE_PROP_BOOL("xtheadmemidx", RISCVCPU, cfg.ext_xtheadmemidx,
> false),
> > DEFINE_PROP_BOOL("xtheadmempair", RISCVCPU, cfg.ext_xtheadmempair,
> false),
> > DEFINE_PROP_BOOL("xtheadsync", RISCVCPU, cfg.ext_xtheadsync, false),
> > +DEFINE_PROP_BOOL("xtheadxmae", RISCVCPU, cfg.ext_xtheadxmae, false),
> > DEFINE_PROP_BOOL("xventanacondops", RISCVCPU,
> cfg.ext_XVentanaCondOps, false),
> >
> > /* These are experimental so mark with 'x-' */
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index c97c1c0af0..897962f107 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -475,6 +475,7 @@ struct RISCVCPUConfig {
> > bool ext_xtheadmemidx;
> > bool ext_xtheadmempair;
> > bool ext_xtheadsync;
> > +bool ext_xtheadxmae;
> > bool ext_XVentanaCondOps;
> >
> > uint8_t pmu_num;
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 278d163803..345bb69b79 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -938,7 +938,8 @@ restart:
> >
> > if (riscv_cpu_sxl(env) == MXL_RV32) {
> > ppn = pte >> PTE_PPN_SHIFT;
> > -} else if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot) {
> > +} else if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot ||
> > + cpu->cfg.ext_xtheadxmae) {
>
> I don't like this. This is some pretty core code that is now getting
> vendor extensions. I know this is very simple, but I'm worried we are
> opening the doors to other vendors adding their MMU changes.
>
> Can we just set ext_svpbmt instead?
>
Ok.
I will drop this patch.
>
> Alistair
>
> > ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> > } else {
> > ppn = pte >> PTE_PPN_SHIFT;
> > @@ -950,7 +951,8 @@ restart:
> > if (!(pte & PTE_V)) {
> > /* Invalid PTE */
> > return TRANSLATE_FAIL;
> > -} else if (!cpu->cfg.ext_svpbmt && (pte & PTE_PBMT)) {
> > +} else if (!cpu->cfg.ext_svpbmt && (pte & PTE_PBMT) &&
> > + !cpu->cfg.ext_xtheadxmae) {
> > return TRANSLATE_FAIL;
> > } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
> > /* Inner PTE, continue walking */
> > --
> > 2.38.1
> >
> >
>
Re: [PATCH 10/11] RISC-V: Adding T-Head FMemIdx extension
On Thu, Sep 8, 2022 at 9:45 AM Richard Henderson <
richard.hender...@linaro.org> wrote:
> On 9/6/22 13:22, Christoph Muellner wrote:
> > @@ -732,6 +733,7 @@ static int ex_rvc_shifti(DisasContext *ctx, int imm)
> > #include "decode-xtheadbs.c.inc"
> > #include "decode-xtheadcmo.c.inc"
> > #include "decode-xtheadcondmov.c.inc"
> > +#include "decode-xtheadfmemidx.c.inc"
> > #include "decode-xtheadmac.c.inc"
> > #include "decode-xtheadmemidx.c.inc"
> > #include "decode-xtheadmempair.c.inc"
> > @@ -1061,6 +1063,7 @@ static void decode_opc(CPURISCVState *env,
> DisasContext *ctx, uint16_t opcode)
> > { has_xtheadbs_p, decode_xtheadbs },
> > { has_xtheadcmo_p, decode_xtheadcmo },
> > { has_xtheadcondmov_p, decode_xtheadcondmov },
> > +{ has_xtheadfmemidx_p, decode_xtheadfmemidx },
> > { has_xtheadmac_p, decode_xtheadmac },
> > { has_xtheadmemidx_p, decode_xtheadmemidx },
> > { has_xtheadmempair_p, decode_xtheadmempair },
>
> I think you should have a single decoder for all of the xthread
> extensions, and each
> translate function should test for the individual extension. You know
> up-front that these
> extensions do not conflict.
>
>
Ok, I will restructure the series and use a single decoder.
Thanks!
>
> r~
>
Re: [PATCH 03/11] RISC-V: Adding T-Head SYNC instructions
On Thu, Sep 8, 2022 at 9:30 AM Richard Henderson <
richard.hender...@linaro.org> wrote:
> On 9/6/22 13:22, Christoph Muellner wrote:
> > +NOP_PRIVCHECK(th_sfence_vmas, REQUIRE_PRIV_MHS)
> > +NOP_PRIVCHECK(th_sync, REQUIRE_PRIV_MHSU)
> > +NOP_PRIVCHECK(th_sync_i, REQUIRE_PRIV_MHSU)
> > +NOP_PRIVCHECK(th_sync_is, REQUIRE_PRIV_MHSU)
> > +NOP_PRIVCHECK(th_sync_s, REQUIRE_PRIV_MHSU)
>
> These should not be nops: th_sfence_vmas requires a tlb flush;
> th.sync{,.i} needs to end
> the current translation block; th.sync.{s,is} needs multiprocessor sync,
> which involves a
> call to async_safe_run_on_cpu.
>
Understood.
For a new revision, I'll do the following:
* th_sfence_vmas: async_safe_run_on_cpu() with run_on_cpu_func which
flushes TLB on all CPUs (similar like trans_sfence_vma())
* th_sync/th_sync_i: end the TB (similar like trans_fence_i())
* th_sync_s/th_sync_is: async_safe_run_on_cpu() with run_on_cpu_func which
ends the TB (similar like trans_fence_i())
Thanks!
>
>
> r~
>
Re: [PATCH 11/11] RISC-V: Add initial support for T-Head C906 and C910 CPUs
On Thu, Sep 8, 2022 at 10:56 AM Richard Henderson < richard.hender...@linaro.org> wrote: > On 9/8/22 09:23, Christoph Müllner wrote: > > > > > > On Thu, Sep 8, 2022 at 9:46 AM Richard Henderson < > richard.hender...@linaro.org > > <mailto:richard.hender...@linaro.org>> wrote: > > > > On 9/6/22 13:22, Christoph Muellner wrote: > > > +DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C906, > rv64_thead_c906_cpu_init), > > > +DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C910, > rv64_thead_c906_cpu_init), > > > > Why model both if they're identical? > > > > > > I figured that users might expect that (existence of "thead-c906" and > "thead-c910"). > > And using "thead-c9xx" feels like it would be regretted in the future. > > > > Should I drop "thead-c910"? > > Quite possibly. For Arm, we don't try to supply every cpu model, only > those that differ > in some substantial way. > Ok, will do. Thanks! > > > r~ >
Re: [PATCH 11/11] RISC-V: Add initial support for T-Head C906 and C910 CPUs
On Thu, Sep 8, 2022 at 9:46 AM Richard Henderson < richard.hender...@linaro.org> wrote: > On 9/6/22 13:22, Christoph Muellner wrote: > > +DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C906, > rv64_thead_c906_cpu_init), > > +DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C910, > rv64_thead_c906_cpu_init), > > Why model both if they're identical? > I figured that users might expect that (existence of "thead-c906" and "thead-c910"). And using "thead-c9xx" feels like it would be regretted in the future. Should I drop "thead-c910"? > > > r~ >
Re: [RFC PATCH v3] RISC-V: Add Zawrs ISA extension support
On Mon, Jun 27, 2022 at 7:20 AM Alistair Francis
wrote:
> On Fri, Jun 24, 2022 at 1:31 AM Christoph Muellner
> wrote:
> >
> > This patch adds support for the Zawrs ISA extension.
> > Given the current (incomplete) implementation of reservation sets
> > there seems to be no way to provide a full emulation of the WRS
> > instruction (wake on reservation set invalidation or timeout or
> > interrupt). Therefore, we just pretend that an interrupt occured,
> > exit the execution loop and finally continue execution.
> >
> > The specification can be found here:
> > https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> >
> > Note, that the Zawrs extension is not frozen or ratified yet.
> > Therefore this patch is an RFC and not intended to get merged.
> >
> > Changes since v2:
> > * Adjustments according to a specification change
> > * Inline REQUIRE_ZAWRS() since it has only one user
> >
> > Changes since v1:
> > * Adding zawrs to the ISA string that is passed to the kernel
> >
> > Signed-off-by: Christoph Müllner
> > ---
> > target/riscv/cpu.c | 2 +
> > target/riscv/cpu.h | 1 +
> > target/riscv/insn32.decode | 4 ++
> > target/riscv/insn_trans/trans_rvzawrs.c.inc | 54 +
> > target/riscv/translate.c| 1 +
> > 5 files changed, 62 insertions(+)
> > create mode 100644 target/riscv/insn_trans/trans_rvzawrs.c.inc
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 05e6521351..6cb00fadff 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -882,6 +882,7 @@ static Property riscv_cpu_extensions[] = {
> > DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
> > DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
> > DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> > +DEFINE_PROP_BOOL("zawrs", RISCVCPU, cfg.ext_zawrs, true),
>
> Would this be enabled by default?
>
The "true" was a personal preference (I prefer to keep the argument list
for QEMU short)
and I did not see any conflicts with existing behavior (no code should
break).
If you prefer otherwise or if I missed a policy I will change it.
>
> > DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
> > DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
> > DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
> > @@ -1075,6 +1076,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu,
> char **isa_str, int max_str_len)
> > ISA_EDATA_ENTRY(zicsr, ext_icsr),
> > ISA_EDATA_ENTRY(zifencei, ext_ifencei),
> > ISA_EDATA_ENTRY(zmmul, ext_zmmul),
> > +ISA_EDATA_ENTRY(zawrs, ext_zawrs),
> > ISA_EDATA_ENTRY(zfh, ext_zfh),
> > ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
> > ISA_EDATA_ENTRY(zfinx, ext_zfinx),
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 7d6397acdf..a22bc0fa09 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -380,6 +380,7 @@ struct RISCVCPUConfig {
> > bool ext_h;
> > bool ext_j;
> > bool ext_v;
> > +bool ext_zawrs;
> > bool ext_zba;
> > bool ext_zbb;
> > bool ext_zbc;
> > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> > index 4033565393..513ea227fe 100644
> > --- a/target/riscv/insn32.decode
> > +++ b/target/riscv/insn32.decode
> > @@ -711,6 +711,10 @@ vsetvli 0 ... . 111 .
> 1010111 @r2_zimm11
> > vsetivli11 .. . 111 . 1010111 @r2_zimm10
> > vsetvl 100 . . 111 . 1010111 @r
> >
> > +# *** Zawrs Standard Extension ***
> > +wrs_nto1101 0 000 0 1110011
> > +wrs_sto00011101 0 000 0 1110011
> > +
> > # *** RV32 Zba Standard Extension ***
> > sh1add 001 .. 010 . 0110011 @r
> > sh2add 001 .. 100 . 0110011 @r
> > diff --git a/target/riscv/insn_trans/trans_rvzawrs.c.inc
> b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> > new file mode 100644
> > index 00..d0df56378e
> > --- /dev/null
> > +++ b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> > @@ -0,0 +1,54 @@
> > +/*
> > + * RISC-V translation routines for the RISC-V Zawrs Extension.
> > + *
> > + * Copyright (c) 2022 Christoph Mue
Re: [RFC PATCH v2] RISC-V: Add Zawrs ISA extension support
On Thu, Jun 2, 2022 at 5:07 PM Richard Henderson
wrote:
>
> On 6/2/22 06:40, Christoph Muellner wrote:
> > diff --git a/target/riscv/insn_trans/trans_rvzawrs.c.inc
> > b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> > new file mode 100644
> > index 00..38b71d0085
> > --- /dev/null
> > +++ b/target/riscv/insn_trans/trans_rvzawrs.c.inc
>
> Typo in the filename -- s/rvz/rz/.
This was actually intended as other files are named using the same pattern
(ext={a,b,d,f,h,i,k,m,v,zfh} -> trans_rv${ext}.c.inc).
I had the impression that Zawrs would fit into the pattern as well.
But if you prefer trans_rzawrs.c.inc I can change accordingly.
>
> > +#define REQUIRE_ZAWRS(ctx) do { \
> > +if (!ctx->cfg_ptr->ext_zawrs) { \
> > +return false; \
> > +} \
> > +} while (0)
> > +
> > +static bool trans_wrs(DisasContext *ctx, arg_sfence_vm *a)
> > +{
> > +REQUIRE_ZAWRS(ctx);
>
> No point in the macro for what will only ever be a single user.
Ok, will change.
Thanks!
>
> Otherwise, the implementation looks correct.
> Reviewed-by: Richard Henderson
>
>
> r~
Re: [PATCH] docs/tcg-plugins: document QEMU_PLUGIN behaviour
On Wed, Mar 16, 2022 at 4:01 PM Alex Bennée wrote: > > Christoph Muellner writes: > > > QEMU plugins can be loaded via command line arguments or via > > the QEMU_PLUGIN environment variable. Currently, only the first method > > is documented. Let's document QEMU_PLUGIN. > > > > Signed-off-by: Christoph Muellner > > --- > > docs/devel/tcg-plugins.rst | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst > > index f93ef4fe52..ba48be18d0 100644 > > --- a/docs/devel/tcg-plugins.rst > > +++ b/docs/devel/tcg-plugins.rst > > @@ -34,6 +34,10 @@ Arguments are plugin specific and can be used to > modify their > > behaviour. In this case the howvec plugin is being asked to use inline > > ops to count and break down the hint instructions by type. > > > > +QEMU also evaluates the environment variable ``QEMU_PLUGIN``:: > > You should also make it clear this only works for *-user builds of QEMU. > For system emulation you still need to use the CLI interface. Looks like this is even more restrictive as I can see support only in linux-user/main.c. I'll reword this to only be available for Linux user-mode emulation. Thanks! > > > > + > > + QEMU_PLUGIN="file=tests/plugin/libhowec.so,inline=on,count=hint" $QEMU > > + > > Writing plugins > > --- > > > -- > Alex Bennée >
Re: [PATCH] docs/tcg-plugins: document QEMU_PLUGIN behaviour
On Wed, Mar 16, 2022 at 2:44 PM Mahmoud Abumandour wrote: > > > On Wed, Mar 16, 2022 at 2:40 PM Christoph Muellner > wrote: > >> QEMU plugins can be loaded via command line arguments or via >> the QEMU_PLUGIN environment variable. Currently, only the first method >> is documented. Let's document QEMU_PLUGIN. >> >> Signed-off-by: Christoph Muellner >> --- >> docs/devel/tcg-plugins.rst | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst >> index f93ef4fe52..ba48be18d0 100644 >> --- a/docs/devel/tcg-plugins.rst >> +++ b/docs/devel/tcg-plugins.rst >> @@ -34,6 +34,10 @@ Arguments are plugin specific and can be used to >> modify their >> behaviour. In this case the howvec plugin is being asked to use inline >> ops to count and break down the hint instructions by type. >> >> +QEMU also evaluates the environment variable ``QEMU_PLUGIN``:: >> + >> + QEMU_PLUGIN="file=tests/plugin/libhowec.so,inline=on,count=hint" $QEMU >> > > The plugin howvec is in contrib/plugins, so I think giving the correct > path would be > better. Note also that there's a typo in "libhowec.so". > Oh yes, I was just copying the contents of the example above but did not notice that this is also wrong. I'll respin a v2 which fixes the paths. Thanks! > > If you want, you could also fix the next example that has the same path or > leave it > for another patch. > > + >> Writing plugins >> --- >> >> -- >> 2.35.1 >> >> > Other than that, > Reviewed-By: Mahmoud Mandour > > Thanks, > Mahmoud >
Re: [PATCH v4 2/2] target/riscv: Enable Zicbo[m,z,p] instructions
On Thu, Feb 17, 2022 at 3:15 AM Weiwei Li wrote:
>
> 在 2022/2/16 下午11:48, Christoph Muellner 写道:
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 39ffb883fc..04500fe352 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -764,6 +764,10 @@ static Property riscv_cpu_properties[] = {
> > DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
> > DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
> > DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> > +DEFINE_PROP_BOOL("zicbom", RISCVCPU, cfg.ext_icbom, true),
> > +DEFINE_PROP_BOOL("zicboz", RISCVCPU, cfg.ext_icboz, true),
> > +DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize,
> 64),
> > +DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize,
> 64),
> Why use two different cache block size here? Is there any new spec
> update for this?
>
No, we are talking about the same specification.
Section 2.7 states the following:
"""
The initial set of CMO extensions requires the following information to be
discovered by software:
* The size of the cache block for management and prefetch instructions
* The size of the cache block for zero instructions
* CBIE support at each privilege level
"""
So at least the spec authors did differentiate between the two block sizes
as well.
> > DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
> > DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
> > DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
> > +
> > +/* helper_zicbom_access
> > + *
> > + * Check access permissions (LOAD, STORE or FETCH as specified in
> section
> > + * 2.5.2 of the CMO specification) for Zicbom, raising either store
> > + * page-fault (non-virtualised) or store guest-page fault (virtualised).
> > + */
> > +static void helper_zicbom_access(CPURISCVState *env, target_ulong
> address,
> > + uintptr_t ra)
> > +{
> > +int ret;
> > +void* phost;
> > +int mmu_idx = cpu_mmu_index(env, false);
> > +
> > +/* Get the size of the cache block for management instructions. */
> > +RISCVCPU *cpu = env_archcpu(env);
> > +uint16_t cbomlen = cpu->cfg.cbom_blocksize;
> > +
> > +/* Mask off low-bits to align-down to the cache-block. */
> > +address &= ~(cbomlen - 1);
> > +
> > +/* A cache-block management instruction is permitted to access
> > + * the specified cache block whenever a load instruction, store
> > + * instruction, or instruction fetch is permitted to access the
> > + * corresponding physical addresses.
> > + */
> > +ret = probe_access_range_flags(env, address, cbomlen, MMU_DATA_LOAD,
> > + mmu_idx, true, , ra);
> > +if (ret == TLB_INVALID_MASK)
> > +ret = probe_access_range_flags(env, address, cbomlen,
> MMU_INST_FETCH,
> > + mmu_idx, true, , ra);
> > +if (ret == TLB_INVALID_MASK)
> > +probe_access_range_flags(env, address, cbomlen, MMU_DATA_STORE,
> > + mmu_idx, false, , ra);
> > +}
> > +
>
>
> I think it's a little different here. Probe_access_range_flags may
> trigger different execptions for different access_type. For example:
>
> If the page for the address is executable and readable but not
> writable, and the access cannot pass the pmp check for all access_type,
>
> it may trigger access fault for load/fetch access, and trigger page
> fault for store access.
>
Just to be clear:
The patch does not trigger any fault for LOAD or FETCH because nonfault is
set
to true (6th argument of probe_access_range_flags()).
Only the last call to probe_access_range_flags() raises an exception.
Section 2.5.2 states the following:
"""
If access to the cache block is not permitted, a cache-block management
instruction raises a store page fault or store guest-page fault exception
if address translation does not permit any
access or raises a store access fault exception otherwise.
"""
In your scenario we have (1...allowed; 0...not allowed):
* read: perm:1, pmp:0
* fetch: perm:1: pmp:0
* write: perm:0, pmp:0
Address translation would allow read and fetch access, but PMP blocks that.
So the "does not permit any"-part is wrong, therefore we should raise a
store page fault.
In fact, I can't predict what will happen, because the code in
target/riscv/cpu_helper.c does
not really prioritize page faults or PMP faults. it returns one of them,
once they are encountered.
In order to model this properly, we would have to refactor cpu_helper.c to
separate page permissions
from PMP. However, that seems a bit out of scope for a Zicbo* support
patchset.
>
> I think the final exception should be access fault instead of the page
> fault caused by probe_access_range_flags with MMU_DATA_STORE.
>
> Regards,
>
> Weiwei Li
>
>
Re: [PATCH v3] target/riscv: Enable Zicbo[m,z,p] instructions
On Fri, Feb 11, 2022 at 3:41 AM Weiwei Li wrote:
>
> 在 2022/2/11 上午12:34, Christoph Muellner 写道:
> > The RISC-V base cache management operation ISA extension has been
> > ratified [1]. This patch adds support for the defined instructions.
> >
> > The cmo.prefetch instructions are nops for QEMU (no emulation of the
> memory
> > hierarchy, no illegal instructions, no permission faults, no traps),
> > therefore there's only a comment where they would be decoded.
> >
> > The other cbo* instructions are moved into an overlap group to
> > resolve the overlapping pattern with the LQ instruction.
> > The cbo.zero zeros a configurable amount of bytes.
> > Similar to other extensions (e.g. atomic instructions),
> > the trap behavior is limited such, that only the page permissions
> > are checked (ignoring other optional protection mechanisms like
> > PMA or PMP).
> >
> > [1] https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions
> >
> > v3:
> > - Enable by default (like zb*)
> > - Rename flags Zicbo* -> zicbo* (like zb*)
> > - Rename ext_zicbo* -> ext_icbo* (like ext_icsr)
> > - Rename trans_zicbo.c.inc -> trans_rvzicbo.c.inc (like all others)
> > - Simplify prefetch instruction support to a single comment
> > - Rebase on top of github-alistair23/riscv-to-apply.next plus the
> >Priv v1.12 series from github-atishp04/priv_1_12_support_v3
> >
> > v2:
> > - Fix overlapping instruction encoding with LQ instructions
> > - Drop CSR related changes and rebase on Priv 1.12 patchset
> >
> > Co-developed-by: Philipp Tomsich
> > Signed-off-by: Christoph Muellner
> > ---
> > target/riscv/cpu.c | 3 +
> > target/riscv/cpu.h | 3 +
> > target/riscv/helper.h | 5 ++
> > target/riscv/insn32.decode | 16 +++-
> > target/riscv/insn_trans/trans_rvzicbo.c.inc | 57 +
> > target/riscv/op_helper.c| 94 +
> > target/riscv/translate.c| 1 +
> > 7 files changed, 178 insertions(+), 1 deletion(-)
> > create mode 100644 target/riscv/insn_trans/trans_rvzicbo.c.inc
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 39ffb883fc..cbd0a34318 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -764,6 +764,9 @@ static Property riscv_cpu_properties[] = {
> > DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
> > DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
> > DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> > +DEFINE_PROP_BOOL("zicbom", RISCVCPU, cfg.ext_icbom, true),
> > +DEFINE_PROP_BOOL("zicboz", RISCVCPU, cfg.ext_icboz, true),
> > +DEFINE_PROP_UINT16("cbozlen", RISCVCPU, cfg.cbozlen, 64),
> > DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
> > DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
> > DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index fe80caeec0..7bd2fd26d6 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -368,6 +368,8 @@ struct RISCVCPUConfig {
> > bool ext_counters;
> > bool ext_ifencei;
> > bool ext_icsr;
> > +bool ext_icbom;
> > +bool ext_icboz;
> > bool ext_zfh;
> > bool ext_zfhmin;
> > bool ext_zve32f;
> > @@ -382,6 +384,7 @@ struct RISCVCPUConfig {
> > char *vext_spec;
> > uint16_t vlen;
> > uint16_t elen;
> > +uint16_t cbozlen;
> > bool mmu;
> > bool pmp;
> > bool epmp;
> > diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> > index 72cc2582f4..ef1944da8f 100644
> > --- a/target/riscv/helper.h
> > +++ b/target/riscv/helper.h
> > @@ -92,6 +92,11 @@ DEF_HELPER_FLAGS_2(fcvt_h_l, TCG_CALL_NO_RWG, i64,
> env, tl)
> > DEF_HELPER_FLAGS_2(fcvt_h_lu, TCG_CALL_NO_RWG, i64, env, tl)
> > DEF_HELPER_FLAGS_1(fclass_h, TCG_CALL_NO_RWG_SE, tl, i64)
> >
> > +/* Cache-block operations */
> > +DEF_HELPER_2(cbo_clean_flush, void, env, tl)
> > +DEF_HELPER_2(cbo_inval, void, env, tl)
> > +DEF_HELPER_2(cbo_zero, void, env, tl)
> > +
> > /* Special functions */
> > DEF_HELPER_2(csrr, tl, env, int)
> > DEF_HELPER_3(csrw, void, env, int, tl)
> > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> > index 5bbedc254c..d5f8329970 100644
> > --- a/target/riscv/insn32.decode
> > +++ b/target/riscv/insn32.decode
> > @@ -128,6 +128,7 @@ addi . 000 . 0010011 @i
> > slti . 010 . 0010011 @i
> > sltiu . 011 . 0010011 @i
> > xori . 100 . 0010011 @i
> > +# cbo.prefetch_{i,r,m} instructions are ori with rd=x0 and not decoded.
> > ori . 110 . 0010011 @i
> > andi . 111 . 0010011 @i
> > slli 0. ... 001 .
Re: [RFC 5/5] target/riscv: Enable privileged spec version 1.12
On Fri, Jan 21, 2022 at 12:16 AM Atish Patra wrote:
> Virt machine uses privileged specification version 1.12 now.
> All other machine continue to use the default one defined for that
> machine unless changed to 1.12 by the user explicitly.
>
> Signed-off-by: Atish Patra
> ---
> target/riscv/cpu.c | 8 +---
> target/riscv/csr.c | 10 ++
> 2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 9bc25d3055d4..cec5791151e7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -153,7 +153,7 @@ static void riscv_any_cpu_init(Object *obj)
> #elif defined(TARGET_RISCV64)
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> #endif
> -set_priv_version(env, PRIV_VERSION_1_11_0);
> +set_priv_version(env, PRIV_VERSION_1_12_0);
> }
>
> #if defined(TARGET_RISCV64)
> @@ -439,7 +439,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
> }
>
> if (cpu->cfg.priv_spec) {
> -if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
> +if (!g_strcmp0(cpu->cfg.priv_spec, "v1.12.0")) {
> +priv_version = PRIV_VERSION_1_12_0;
> +} else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
> priv_version = PRIV_VERSION_1_11_0;
> } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
> priv_version = PRIV_VERSION_1_10_0;
> @@ -454,7 +456,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
> if (priv_version) {
> set_priv_version(env, priv_version);
> } else if (!env->priv_ver) {
> -set_priv_version(env, PRIV_VERSION_1_11_0);
> +set_priv_version(env, PRIV_VERSION_1_12_0);
> }
>
> if (cpu->cfg.mmu) {
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index a4bbae7a1bbd..62d429cc3f17 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1874,6 +1874,12 @@ static inline RISCVException
> riscv_csrrw_check(CPURISCVState *env,
> int read_only = get_field(csrno, 0xC00) == 3;
> #if !defined(CONFIG_USER_ONLY)
> int effective_priv = env->priv;
> +int csr_min_priv = csr_ops[csrno].min_priv_ver;
> +
> +/* The default privilege specification version supported is 1.10 */
> +if (!csr_min_priv) {
> +csr_min_priv = PRIV_VERSION_1_10_0;
> +}
>
> if (riscv_has_ext(env, RVH) &&
> env->priv == PRV_S &&
> @@ -1904,6 +1910,10 @@ static inline RISCVException
> riscv_csrrw_check(CPURISCVState *env,
> return RISCV_EXCP_ILLEGAL_INST;
> }
>
> +if (env->priv_ver < csr_min_priv) {
> +return RISCV_EXCP_ILLEGAL_INST;
> +}
>
This does not compile if CONFIG_USER_ONLY is defined.
> +
> return csr_ops[csrno].predicate(env, csrno);
> }
>
> --
> 2.30.2
>
>
Re: [RESEND] target/riscv: Enable bitmanip Zicbo[m,z,p] instructions
On Tue, Jan 18, 2022 at 9:31 PM Atish Patra wrote:
>
> On Tue, Jan 18, 2022 at 8:48 AM Christoph Muellner
> wrote:
> >
> > The RISC-V base cache management operation ISA extension has been
> > ratified [1]. This patch adds support for the defined instructions
> > and CSRs.
> >
> > [1] https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions
> >
> > Co-developed-by: Philipp Tomsich
> > Signed-off-by: Philipp Tomsich
> > Signed-off-by: Christoph Muellner
> > ---
> > target/riscv/cpu.c| 4 +
> > target/riscv/cpu.h| 9 +++
> > target/riscv/cpu_bits.h | 10 +++
> > target/riscv/csr.c| 47
> > target/riscv/helper.h | 5 ++
> > target/riscv/insn32.decode| 20 -
> > target/riscv/insn_trans/trans_zicbo.c.inc | 72 ++
> > target/riscv/op_helper.c | 89 +++
> > target/riscv/translate.c | 1 +
> > 9 files changed, 256 insertions(+), 1 deletion(-)
> > create mode 100644 target/riscv/insn_trans/trans_zicbo.c.inc
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 9bc25d3055..b4a87cfcdc 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -660,6 +660,10 @@ static Property riscv_cpu_properties[] = {
> > DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> > DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
> > DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
> > +DEFINE_PROP_BOOL("Zicbom", RISCVCPU, cfg.ext_zicbom, false),
> > +DEFINE_PROP_BOOL("Zicbop", RISCVCPU, cfg.ext_zicbop, false),
> > +DEFINE_PROP_BOOL("Zicboz", RISCVCPU, cfg.ext_zicboz, false),
> > +DEFINE_PROP_UINT16("cbolen", RISCVCPU, cfg.cbolen, 64),
> > DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
> > DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 4d63086765..acfe21cb75 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -263,6 +263,11 @@ struct CPURISCVState {
> > target_ulong spmbase;
> > target_ulong upmmask;
> > target_ulong upmbase;
> > +
> > +/* [mhs]envcfg CSRs */
> > +target_ulong menvcfg;
> > +target_ulong henvcfg;
> > +target_ulong senvcfg;
> > #endif
> >
> > float_status fp_status;
> > @@ -329,6 +334,9 @@ struct RISCVCPU {
> > bool ext_icsr;
> > bool ext_zfh;
> > bool ext_zfhmin;
> > +bool ext_zicbom;
> > +bool ext_zicbop;
> > +bool ext_zicboz;
> >
> > char *priv_spec;
> > char *user_spec;
> > @@ -336,6 +344,7 @@ struct RISCVCPU {
> > char *vext_spec;
> > uint16_t vlen;
> > uint16_t elen;
> > +uint16_t cbolen;
> > bool mmu;
> > bool pmp;
> > bool epmp;
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index 5a6d49aa64..38c529b493 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -217,6 +217,11 @@
> > #define CSR_MTINST 0x34a
> > #define CSR_MTVAL2 0x34b
> >
> > +/* Environment configuration */
> > +#define CSR_SENVCFG 0x10a
> > +#define CSR_MENVCFG 0x30a
> > +#define CSR_HENVCFG 0x60a
> > +
> > /* Enhanced Physical Memory Protection (ePMP) */
> > #define CSR_MSECCFG 0x747
> > #define CSR_MSECCFGH0x757
> > @@ -449,6 +454,11 @@ typedef enum {
> > #define COUNTEREN_IR (1 << 2)
> > #define COUNTEREN_HPM3 (1 << 3)
> >
> > +/* [msh]envcfg CSR bits */
> > +#define ENVCFG_CBIE (0b11 << 4)
> > +#define ENVCFG_CBCFE (1 << 6)
> > +#define ENVCFG_CBZE (1 << 7)
> > +
> > /* Privilege modes */
> > #define PRV_U 0
> > #define PRV_S 1
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index adb3d4381d..6693f695e4 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -1478,6 +1478,48 @@ static RISCVException write_mtinst(CPURISCVState
> > *env, int csrno,
> > return RISCV_EXCP_NONE;
> > }
> >
> > +static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
> > + target_ulong *val)
> > +{
> > +*val = env->menvcfg;
> > +return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
> > +target_ulong val)
> > +{
> > +env->menvcfg = val;
> > +return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> > + target_ulong *val)
> > +{
> > +*val = env->henvcfg;
> > +return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> > +target_ulong val)
> > +{
> > +env->henvcfg =
Re: [RESEND] target/riscv: fix RV128 lq encoding
Hi Frédéric, you are right, I misunderstood the "LQ is added to the MISC-MEM major opcode" part of the spec. I saw the encoding conflict with the CBO instructions and thought of a bug in qemu's LQ encoding. Philipp already highlighted that cbo.* instructions are actually LQ with rd=0. Thanks, Christoph On Wed, Jan 19, 2022 at 8:45 PM Frédéric Pétrot wrote: > > Le 18/01/2022 à 17:32, Christoph Muellner a écrit : > > If LQ has func3==010 and is located in the MISC-MEM opcodes, > > then it conflicts with the CBO opcode space. > > However, since LQ is specified as: "LQ is added to the MISC-MEM major > > opcode", we have an implementation bug, because 'major opcode' > > refers to func3, which must be 111. > > > > This results in the following instruction encodings: > > > > lq .111 .000 > > cbo_clean 0001 .010 > > cbo_flush 0010 .010 > > cbo_inval .010 > > cbo_zero 0100 .010 > > ^^^-func3 > >^^^-opcode > >Hello Christoph, >I see page table 26.1 of the last riscv-isa-manual.pdf what is called major >opcodes in my understanding, and MISC-MEM is one of them with value > 00_111_11. >The value for func3 that I chose comes from >https://github.com/michaeljclark/riscv-meta/blob/master/opcodes >which admittedly is out-dated, but I don't see any particular value for >LQ/SQ in the new spec either (I mean, riscv-isa-manual.pdf, any pointer we >could refer to ?). >I have nothing against changing the opcode, but then we need to change >disas/riscv.c which also uses the previous opcode to dump instructions when >running with -d in_asm. > >Frédéric > > > > Signed-off-by: Christoph Muellner > > --- > > target/riscv/insn32.decode | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode > > index 5bbedc254c..d3f798ca10 100644 > > --- a/target/riscv/insn32.decode > > +++ b/target/riscv/insn32.decode > > @@ -168,7 +168,7 @@ sraw 010 . . 101 . 0111011 @r > > > > # *** RV128I Base Instruction Set (in addition to RV64I) *** > > ldu . 111 . 011 @i > > -lq . 010 . 000 @i > > +lq . 111 . 000 @i > > sq . 100 . 0100011 @s > > addid . 000 . 1011011 @i > > sllid00 .. . 001 . 1011011 @sh6 > > -- > +---+ > | Frédéric Pétrot, Pr. Grenoble INP-Ensimag/TIMA, Ensimag deputy director | > | Mob/Pho: +33 6 74 57 99 65/+33 4 76 57 48 70 Ad augusta per angusta | > | http://tima.univ-grenoble-alpes.fr frederic.pet...@univ-grenoble-alpes.fr | > +---+
Re: [PATCH] target/riscv: Enable bitmanip Zicbo[m,z,p] instructions
Resend from the correct email address to get accepted by Mailman.
On Tue, Jan 18, 2022 at 4:14 PM Christoph Muellner wrote:
>
> The RISC-V base cache management operation ISA extension has been
> ratified [1]. This patch adds support for the defined instructions
> and CSRs.
>
> [1] https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions
>
> Co-developed-by: Philipp Tomsich
> Signed-off-by: Philipp Tomsich
> Signed-off-by: Christoph Muellner
> ---
> target/riscv/cpu.c| 4 +
> target/riscv/cpu.h| 9 +++
> target/riscv/cpu_bits.h | 10 +++
> target/riscv/csr.c| 47
> target/riscv/helper.h | 5 ++
> target/riscv/insn32.decode| 20 -
> target/riscv/insn_trans/trans_zicbo.c.inc | 72 ++
> target/riscv/op_helper.c | 89 +++
> target/riscv/translate.c | 1 +
> 9 files changed, 256 insertions(+), 1 deletion(-)
> create mode 100644 target/riscv/insn_trans/trans_zicbo.c.inc
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 9bc25d3055..b4a87cfcdc 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -660,6 +660,10 @@ static Property riscv_cpu_properties[] = {
> DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
> DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
> +DEFINE_PROP_BOOL("Zicbom", RISCVCPU, cfg.ext_zicbom, false),
> +DEFINE_PROP_BOOL("Zicbop", RISCVCPU, cfg.ext_zicbop, false),
> +DEFINE_PROP_BOOL("Zicboz", RISCVCPU, cfg.ext_zicboz, false),
> +DEFINE_PROP_UINT16("cbolen", RISCVCPU, cfg.cbolen, 64),
> DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
> DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 4d63086765..acfe21cb75 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -263,6 +263,11 @@ struct CPURISCVState {
> target_ulong spmbase;
> target_ulong upmmask;
> target_ulong upmbase;
> +
> +/* [mhs]envcfg CSRs */
> +target_ulong menvcfg;
> +target_ulong henvcfg;
> +target_ulong senvcfg;
> #endif
>
> float_status fp_status;
> @@ -329,6 +334,9 @@ struct RISCVCPU {
> bool ext_icsr;
> bool ext_zfh;
> bool ext_zfhmin;
> +bool ext_zicbom;
> +bool ext_zicbop;
> +bool ext_zicboz;
>
> char *priv_spec;
> char *user_spec;
> @@ -336,6 +344,7 @@ struct RISCVCPU {
> char *vext_spec;
> uint16_t vlen;
> uint16_t elen;
> +uint16_t cbolen;
> bool mmu;
> bool pmp;
> bool epmp;
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 5a6d49aa64..38c529b493 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -217,6 +217,11 @@
> #define CSR_MTINST 0x34a
> #define CSR_MTVAL2 0x34b
>
> +/* Environment configuration */
> +#define CSR_SENVCFG 0x10a
> +#define CSR_MENVCFG 0x30a
> +#define CSR_HENVCFG 0x60a
> +
> /* Enhanced Physical Memory Protection (ePMP) */
> #define CSR_MSECCFG 0x747
> #define CSR_MSECCFGH0x757
> @@ -449,6 +454,11 @@ typedef enum {
> #define COUNTEREN_IR (1 << 2)
> #define COUNTEREN_HPM3 (1 << 3)
>
> +/* [msh]envcfg CSR bits */
> +#define ENVCFG_CBIE (0b11 << 4)
> +#define ENVCFG_CBCFE (1 << 6)
> +#define ENVCFG_CBZE (1 << 7)
> +
> /* Privilege modes */
> #define PRV_U 0
> #define PRV_S 1
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index adb3d4381d..6693f695e4 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1478,6 +1478,48 @@ static RISCVException write_mtinst(CPURISCVState *env,
> int csrno,
> return RISCV_EXCP_NONE;
> }
>
> +static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
> + target_ulong *val)
> +{
> +*val = env->menvcfg;
> +return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
> +target_ulong val)
> +{
> +env->menvcfg = val;
> +return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> + target_ulong *val)
> +{
> +*val = env->henvcfg;
> +return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> +target_ulong val)
> +{
> +env->henvcfg = val;
> +return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
> + target_ulong *val)
> +{
> +*val = env->senvcfg;
> +return RISCV_EXCP_NONE;
> +}
> +
> +static
Re: [PATCH] target/riscv: fix RV128 lq encoding
Resend from the correct email address to get accepted by Mailman. On Tue, Jan 18, 2022 at 4:12 PM Christoph Muellner wrote: > > If LQ has func3==010 and is located in the MISC-MEM opcodes, > then it conflicts with the CBO opcode space. > However, since LQ is specified as: "LQ is added to the MISC-MEM major > opcode", we have an implementation bug, because 'major opcode' > refers to func3, which must be 111. > > This results in the following instruction encodings: > > lq .111 .000 > cbo_clean 0001 .010 > cbo_flush 0010 .010 > cbo_inval .010 > cbo_zero 0100 .010 > ^^^-func3 > ^^^-opcode > > Signed-off-by: Christoph Muellner > --- > target/riscv/insn32.decode | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode > index 5bbedc254c..d3f798ca10 100644 > --- a/target/riscv/insn32.decode > +++ b/target/riscv/insn32.decode > @@ -168,7 +168,7 @@ sraw 010 . . 101 . 0111011 @r > > # *** RV128I Base Instruction Set (in addition to RV64I) *** > ldu . 111 . 011 @i > -lq . 010 . 000 @i > +lq . 111 . 000 @i > sq . 100 . 0100011 @s > addid . 000 . 1011011 @i > sllid00 .. . 001 . 1011011 @sh6 > -- > 2.34.1 >
