* [PATCH 0/2] target/riscv: Support mxstatus CSR for thead-c906 @ 2024-01-30 11:11 LIU Zhiwei 2024-01-30 11:11 ` [PATCH 1/2] target/riscv: Register vendors CSR LIU Zhiwei 2024-01-30 11:11 ` [PATCH 2/2] target/riscv: Support xtheadmaee for thead-c906 LIU Zhiwei 0 siblings, 2 replies; 10+ messages in thread From: LIU Zhiwei @ 2024-01-30 11:11 UTC (permalink / raw) To: qemu-devel Cc: Alistair.Francis, palmer, bin.meng, liwei1518, dbarboza, qemu-riscv, christoph.muellner, bjorn, LIU Zhiwei We first add a framework for vendor CSRs in patch 1. After that we add one thead-c906 CSR mxstatus, which is used for mmu extension xtheadmaee. 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. LIU Zhiwei (2): target/riscv: Register vendors CSR target/riscv: Support xtheadmaee for thead-c906 target/riscv/cpu.c | 9 ++++++ 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 | 25 ++++++++++++++- target/riscv/tcg/tcg-cpu.h | 1 + target/riscv/xthead_csr.c | 63 ++++++++++++++++++++++++++++++++++++++ 9 files changed, 130 insertions(+), 13 deletions(-) create mode 100644 target/riscv/xthead_csr.c -- 2.25.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] target/riscv: Register vendors CSR 2024-01-30 11:11 [PATCH 0/2] target/riscv: Support mxstatus CSR for thead-c906 LIU Zhiwei @ 2024-01-30 11:11 ` LIU Zhiwei 2024-01-31 5:06 ` Richard Henderson 2024-01-30 11:11 ` [PATCH 2/2] target/riscv: Support xtheadmaee for thead-c906 LIU Zhiwei 1 sibling, 1 reply; 10+ messages in thread From: LIU Zhiwei @ 2024-01-30 11:11 UTC (permalink / raw) To: qemu-devel Cc: Alistair.Francis, palmer, bin.meng, liwei1518, dbarboza, qemu-riscv, christoph.muellner, bjorn, LIU Zhiwei riscv specification allows custom CSRs in decode area. So we should register all vendor CSRs in cpu realize stage. Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> --- target/riscv/cpu.c | 3 +++ target/riscv/tcg/tcg-cpu.c | 26 ++++++++++++++++++++++++++ target/riscv/tcg/tcg-cpu.h | 1 + 3 files changed, 30 insertions(+) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 8cbfc7e781..2dcbc9ff32 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -1113,6 +1113,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) return; } + if (tcg_enabled()) { + riscv_tcg_cpu_register_vendor_csr(cpu); + } riscv_cpu_register_gdb_regs_for_features(cs); #ifndef CONFIG_USER_ONLY diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c index 994ca1cdf9..408b2ebffa 100644 --- a/target/riscv/tcg/tcg-cpu.c +++ b/target/riscv/tcg/tcg-cpu.c @@ -871,6 +871,32 @@ static void riscv_cpu_validate_profiles(RISCVCPU *cpu) } } +/* This stub just works for making vendors array not empty */ +riscv_csr_operations stub_csr_ops[CSR_TABLE_SIZE]; +static inline bool never_p(const RISCVCPUConfig *cfg) +{ + return false; +} + +void riscv_tcg_cpu_register_vendor_csr(RISCVCPU *cpu) +{ + static const struct { + bool (*guard_func)(const RISCVCPUConfig *); + riscv_csr_operations *csr_ops; + } vendors[] = { + { never_p, stub_csr_ops }, + }; + for (size_t i = 0; i < ARRAY_SIZE(vendors); ++i) { + if (!vendors[i].guard_func(&cpu->cfg)) { + continue; + } + for (size_t j = 0; j < CSR_TABLE_SIZE && + vendors[i].csr_ops[j].name; j++) { + csr_ops[j] = vendors[i].csr_ops[j]; + } + } +} + void riscv_tcg_cpu_finalize_features(RISCVCPU *cpu, Error **errp) { CPURISCVState *env = &cpu->env; diff --git a/target/riscv/tcg/tcg-cpu.h b/target/riscv/tcg/tcg-cpu.h index f7b32417f8..3daaebf4fb 100644 --- a/target/riscv/tcg/tcg-cpu.h +++ b/target/riscv/tcg/tcg-cpu.h @@ -25,5 +25,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp); void riscv_tcg_cpu_finalize_features(RISCVCPU *cpu, Error **errp); bool riscv_cpu_tcg_compatible(RISCVCPU *cpu); +void riscv_tcg_cpu_register_vendor_csr(RISCVCPU *cpu); #endif -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] target/riscv: Register vendors CSR 2024-01-30 11:11 ` [PATCH 1/2] target/riscv: Register vendors CSR LIU Zhiwei @ 2024-01-31 5:06 ` Richard Henderson 2024-01-31 6:14 ` LIU Zhiwei 0 siblings, 1 reply; 10+ messages in thread From: Richard Henderson @ 2024-01-31 5:06 UTC (permalink / raw) To: LIU Zhiwei, qemu-devel Cc: Alistair.Francis, palmer, bin.meng, liwei1518, dbarboza, qemu-riscv, christoph.muellner, bjorn On 1/30/24 21:11, LIU Zhiwei wrote: > +/* This stub just works for making vendors array not empty */ > +riscv_csr_operations stub_csr_ops[CSR_TABLE_SIZE]; > +static inline bool never_p(const RISCVCPUConfig *cfg) > +{ > + return false; > +} > + > +void riscv_tcg_cpu_register_vendor_csr(RISCVCPU *cpu) > +{ > + static const struct { > + bool (*guard_func)(const RISCVCPUConfig *); > + riscv_csr_operations *csr_ops; > + } vendors[] = { > + { never_p, stub_csr_ops }, > + }; > + for (size_t i = 0; i < ARRAY_SIZE(vendors); ++i) { Presumably you did this to avoid a Werror for "i < 0", since i is unsigned. It would be better to either use "int i", or for (size_t i = 0, n = ARRAY_SIZE(vendors); i < n; ++i) either of which will not Werror. Especially considering the size of stub_csr_ops. r~ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] target/riscv: Register vendors CSR 2024-01-31 5:06 ` Richard Henderson @ 2024-01-31 6:14 ` LIU Zhiwei 0 siblings, 0 replies; 10+ messages in thread From: LIU Zhiwei @ 2024-01-31 6:14 UTC (permalink / raw) To: Richard Henderson, LIU Zhiwei, qemu-devel Cc: Alistair.Francis, palmer, bin.meng, liwei1518, dbarboza, qemu-riscv, christoph.muellner, bjorn On 2024/1/31 13:06, Richard Henderson wrote: > On 1/30/24 21:11, LIU Zhiwei wrote: >> +/* This stub just works for making vendors array not empty */ >> +riscv_csr_operations stub_csr_ops[CSR_TABLE_SIZE]; >> +static inline bool never_p(const RISCVCPUConfig *cfg) >> +{ >> + return false; >> +} >> + >> +void riscv_tcg_cpu_register_vendor_csr(RISCVCPU *cpu) >> +{ >> + static const struct { >> + bool (*guard_func)(const RISCVCPUConfig *); >> + riscv_csr_operations *csr_ops; >> + } vendors[] = { >> + { never_p, stub_csr_ops }, >> + }; >> + for (size_t i = 0; i < ARRAY_SIZE(vendors); ++i) { > > Presumably you did this to avoid a Werror for "i < 0", since i is > unsigned. Yes. That's the gcc complains. > > It would be better to either use "int i" OK > , or > > for (size_t i = 0, n = ARRAY_SIZE(vendors); i < n; ++i) > > either of which will not Werror. This works. I don't know why it works, because n is 0 and never changes. > > Especially considering the size of stub_csr_ops. Do you mean we should remove the stub_csr_ops? I don't know how to relate your two solving ways to stub_csr_ops. Thanks, Zhiwei > > r~ > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] target/riscv: Support xtheadmaee for thead-c906 2024-01-30 11:11 [PATCH 0/2] target/riscv: Support mxstatus CSR for thead-c906 LIU Zhiwei 2024-01-30 11:11 ` [PATCH 1/2] target/riscv: Register vendors CSR LIU Zhiwei @ 2024-01-30 11:11 ` LIU Zhiwei 2024-01-30 11:43 ` Christoph Müllner 2024-01-31 5:07 ` Richard Henderson 1 sibling, 2 replies; 10+ messages in thread From: LIU Zhiwei @ 2024-01-30 11:11 UTC (permalink / raw) To: qemu-devel Cc: Alistair.Francis, palmer, bin.meng, liwei1518, dbarboza, qemu-riscv, christoph.muellner, bjorn, LIU Zhiwei 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. [1]:https://occ-intl-prod.oss-ap-southeast-1.aliyuncs.com/resource//1699265191641/XuanTie-Openc906-UserManual.pdf Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> --- 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 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 ext_xtheadmaee; bool ext_XVentanaCondOps; uint32_t pmu_mask; @@ -176,7 +177,8 @@ static inline bool has_xthead_p(const RISCVCPUConfig *cfg) cfg->ext_xtheadcondmov || cfg->ext_xtheadfmemidx || cfg->ext_xtheadfmv || cfg->ext_xtheadmac || cfg->ext_xtheadmemidx || - cfg->ext_xtheadmempair || cfg->ext_xtheadsync; + cfg->ext_xtheadmempair || cfg->ext_xtheadsync || + cfg->ext_xtheadmaee; } #define MATERIALISE_EXT_PREDICATE(ext) \ diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index c7cc7eb423..5c1f380276 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -812,6 +812,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical, int napot_bits = 0; target_ulong napot_mask; + bool skip_pte_check = env->th_mxstatus & TH_MXSTATUS_MAEE; /* * Check if we should use the background registers for the two * stage translation. We don't need to check if we actually need @@ -974,18 +975,19 @@ restart: if (riscv_cpu_sxl(env) == MXL_RV32) { ppn = pte >> PTE_PPN_SHIFT; } else { - if (pte & PTE_RESERVED) { - return TRANSLATE_FAIL; - } + if (!skip_pte_check) { + if (pte & PTE_RESERVED) { + return TRANSLATE_FAIL; + } - if (!pbmte && (pte & PTE_PBMT)) { - return TRANSLATE_FAIL; - } + if (!pbmte && (pte & PTE_PBMT)) { + return TRANSLATE_FAIL; + } - if (!riscv_cpu_cfg(env)->ext_svnapot && (pte & PTE_N)) { - return TRANSLATE_FAIL; + if (!riscv_cpu_cfg(env)->ext_svnapot && (pte & PTE_N)) { + return TRANSLATE_FAIL; + } } - ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT; } @@ -998,7 +1000,8 @@ restart: } /* Inner PTE, continue walking */ - if (pte & (PTE_D | PTE_A | PTE_U | PTE_ATTR)) { + if ((pte & (PTE_D | PTE_A | PTE_U)) || + (!skip_pte_check && (pte & PTE_ATTR))) { return TRANSLATE_FAIL; } base = ppn << PGSHIFT; @@ -1012,7 +1015,7 @@ restart: /* Misaligned PPN */ return TRANSLATE_FAIL; } - if (!pbmte && (pte & PTE_PBMT)) { + if (!skip_pte_check && !pbmte && (pte & PTE_PBMT)) { /* Reserved without Svpbmt. */ return TRANSLATE_FAIL; } diff --git a/target/riscv/meson.build b/target/riscv/meson.build index a5e0734e7f..d7f675881d 100644 --- a/target/riscv/meson.build +++ b/target/riscv/meson.build @@ -12,6 +12,7 @@ riscv_ss.add(files( 'cpu.c', 'cpu_helper.c', 'csr.c', + 'xthead_csr.c', 'fpu_helper.c', 'gdbstub.c', 'op_helper.c', diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c index 408b2ebffa..18a22dfb7a 100644 --- a/target/riscv/tcg/tcg-cpu.c +++ b/target/riscv/tcg/tcg-cpu.c @@ -763,7 +763,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) cpu->cfg.pmu_mask = 0; cpu->pmu_avail_ctrs = 0; } - /* * Disable isa extensions based on priv spec after we * validated and set everything we need. @@ -871,11 +870,9 @@ static void riscv_cpu_validate_profiles(RISCVCPU *cpu) } } -/* This stub just works for making vendors array not empty */ -riscv_csr_operations stub_csr_ops[CSR_TABLE_SIZE]; -static inline bool never_p(const RISCVCPUConfig *cfg) +static inline bool th_csr_p(const RISCVCPUConfig *cfg) { - return false; + return cfg->ext_xtheadmaee; } void riscv_tcg_cpu_register_vendor_csr(RISCVCPU *cpu) @@ -884,7 +881,7 @@ void riscv_tcg_cpu_register_vendor_csr(RISCVCPU *cpu) bool (*guard_func)(const RISCVCPUConfig *); riscv_csr_operations *csr_ops; } vendors[] = { - { never_p, stub_csr_ops }, + { th_csr_p, th_csr_ops }, }; for (size_t i = 0; i < ARRAY_SIZE(vendors); ++i) { if (!vendors[i].guard_func(&cpu->cfg)) { diff --git a/target/riscv/xthead_csr.c b/target/riscv/xthead_csr.c new file mode 100644 index 0000000000..c119a18789 --- /dev/null +++ b/target/riscv/xthead_csr.c @@ -0,0 +1,63 @@ +/* + * Xuantie implementation for RISC-V Control and Status Registers. + * + * Copyright (c) 2024 Alibaba Group. All rights reserved. + * + * 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 "qemu/log.h" +#include "cpu.h" +#include "tcg/tcg-cpu.h" +#include "exec/exec-all.h" +#include "exec/tb-flush.h" +#include "qapi/error.h" + +static RISCVException th_maee_check(CPURISCVState *env, int csrno) +{ + if (riscv_cpu_cfg(env)->ext_xtheadmaee) { + return RISCV_EXCP_ILLEGAL_INST; + } + return RISCV_EXCP_NONE; +} + +static RISCVException +read_th_mxstatus(CPURISCVState *env, int csrno, target_ulong *val) +{ + *val = env->th_mxstatus; + return RISCV_EXCP_NONE; +} + +static RISCVException +write_th_mxstatus(CPURISCVState *env, int csrno, target_ulong val) +{ + uint64_t mxstatus = env->th_mxstatus; + uint64_t mask = TH_MXSTATUS_MAEE; + + if ((val ^ mxstatus) & TH_MXSTATUS_MAEE) { + tlb_flush(env_cpu(env)); + } + + mxstatus = (mxstatus & ~mask) | (val & mask); + env->th_mxstatus = mxstatus; + return RISCV_EXCP_NONE; +} + +riscv_csr_operations th_csr_ops[CSR_TABLE_SIZE] = { +#if !defined(CONFIG_USER_ONLY) + [CSR_TH_MXSTATUS] = { "th_mxstatus", th_maee_check, read_th_mxstatus, + write_th_mxstatus}, +#endif /* !CONFIG_USER_ONLY */ +}; -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] target/riscv: Support xtheadmaee for thead-c906 2024-01-30 11:11 ` [PATCH 2/2] target/riscv: Support xtheadmaee for thead-c906 LIU Zhiwei @ 2024-01-30 11:43 ` Christoph Müllner 2024-01-31 18:52 ` Conor Dooley 2024-02-04 5:47 ` LIU Zhiwei 2024-01-31 5:07 ` Richard Henderson 1 sibling, 2 replies; 10+ messages in thread From: Christoph Müllner @ 2024-01-30 11:43 UTC (permalink / raw) To: LIU Zhiwei Cc: qemu-devel, Alistair.Francis, palmer, bin.meng, liwei1518, dbarboza, qemu-riscv, bjorn, Philipp Tomsich On Tue, Jan 30, 2024 at 12:12 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> 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 <zhiwei_liu@linux.alibaba.com> > --- > 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 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 ext_xtheadmaee; > bool ext_XVentanaCondOps; > > uint32_t pmu_mask; > @@ -176,7 +177,8 @@ static inline bool has_xthead_p(const RISCVCPUConfig *cfg) > cfg->ext_xtheadcondmov || > cfg->ext_xtheadfmemidx || cfg->ext_xtheadfmv || > cfg->ext_xtheadmac || cfg->ext_xtheadmemidx || > - cfg->ext_xtheadmempair || cfg->ext_xtheadsync; > + cfg->ext_xtheadmempair || cfg->ext_xtheadsync || > + cfg->ext_xtheadmaee; > } > > #define MATERIALISE_EXT_PREDICATE(ext) \ > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index c7cc7eb423..5c1f380276 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -812,6 +812,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical, > int napot_bits = 0; > target_ulong napot_mask; > > + bool skip_pte_check = env->th_mxstatus & TH_MXSTATUS_MAEE; > /* > * Check if we should use the background registers for the two > * stage translation. We don't need to check if we actually need > @@ -974,18 +975,19 @@ restart: > if (riscv_cpu_sxl(env) == MXL_RV32) { > ppn = pte >> PTE_PPN_SHIFT; > } else { > - if (pte & PTE_RESERVED) { > - return TRANSLATE_FAIL; > - } > + if (!skip_pte_check) { > + if (pte & PTE_RESERVED) { > + return TRANSLATE_FAIL; > + } > > - if (!pbmte && (pte & PTE_PBMT)) { > - return TRANSLATE_FAIL; > - } > + if (!pbmte && (pte & PTE_PBMT)) { > + return TRANSLATE_FAIL; > + } > > - if (!riscv_cpu_cfg(env)->ext_svnapot && (pte & PTE_N)) { > - return TRANSLATE_FAIL; > + if (!riscv_cpu_cfg(env)->ext_svnapot && (pte & PTE_N)) { > + return TRANSLATE_FAIL; > + } > } > - > ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT; > } > > @@ -998,7 +1000,8 @@ restart: > } > > /* Inner PTE, continue walking */ > - if (pte & (PTE_D | PTE_A | PTE_U | PTE_ATTR)) { > + if ((pte & (PTE_D | PTE_A | PTE_U)) || > + (!skip_pte_check && (pte & PTE_ATTR))) { > return TRANSLATE_FAIL; > } > base = ppn << PGSHIFT; > @@ -1012,7 +1015,7 @@ restart: > /* Misaligned PPN */ > return TRANSLATE_FAIL; > } > - if (!pbmte && (pte & PTE_PBMT)) { > + if (!skip_pte_check && !pbmte && (pte & PTE_PBMT)) { > /* Reserved without Svpbmt. */ > return TRANSLATE_FAIL; > } > diff --git a/target/riscv/meson.build b/target/riscv/meson.build > index a5e0734e7f..d7f675881d 100644 > --- a/target/riscv/meson.build > +++ b/target/riscv/meson.build > @@ -12,6 +12,7 @@ riscv_ss.add(files( > 'cpu.c', > 'cpu_helper.c', > 'csr.c', > + 'xthead_csr.c', > 'fpu_helper.c', > 'gdbstub.c', > 'op_helper.c', > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c > index 408b2ebffa..18a22dfb7a 100644 > --- a/target/riscv/tcg/tcg-cpu.c > +++ b/target/riscv/tcg/tcg-cpu.c > @@ -763,7 +763,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > cpu->cfg.pmu_mask = 0; > cpu->pmu_avail_ctrs = 0; > } > - > /* > * Disable isa extensions based on priv spec after we > * validated and set everything we need. > @@ -871,11 +870,9 @@ static void riscv_cpu_validate_profiles(RISCVCPU *cpu) > } > } > > -/* This stub just works for making vendors array not empty */ > -riscv_csr_operations stub_csr_ops[CSR_TABLE_SIZE]; > -static inline bool never_p(const RISCVCPUConfig *cfg) > +static inline bool th_csr_p(const RISCVCPUConfig *cfg) > { > - return false; > + return cfg->ext_xtheadmaee; > } > > void riscv_tcg_cpu_register_vendor_csr(RISCVCPU *cpu) > @@ -884,7 +881,7 @@ void riscv_tcg_cpu_register_vendor_csr(RISCVCPU *cpu) > bool (*guard_func)(const RISCVCPUConfig *); > riscv_csr_operations *csr_ops; > } vendors[] = { > - { never_p, stub_csr_ops }, > + { th_csr_p, th_csr_ops }, > }; > for (size_t i = 0; i < ARRAY_SIZE(vendors); ++i) { > if (!vendors[i].guard_func(&cpu->cfg)) { > diff --git a/target/riscv/xthead_csr.c b/target/riscv/xthead_csr.c > new file mode 100644 > index 0000000000..c119a18789 > --- /dev/null > +++ b/target/riscv/xthead_csr.c > @@ -0,0 +1,63 @@ > +/* > + * Xuantie implementation for RISC-V Control and Status Registers. > + * > + * Copyright (c) 2024 Alibaba Group. All rights reserved. > + * > + * 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 "qemu/log.h" > +#include "cpu.h" > +#include "tcg/tcg-cpu.h" > +#include "exec/exec-all.h" > +#include "exec/tb-flush.h" > +#include "qapi/error.h" > + > +static RISCVException th_maee_check(CPURISCVState *env, int csrno) > +{ > + if (riscv_cpu_cfg(env)->ext_xtheadmaee) { > + return RISCV_EXCP_ILLEGAL_INST; > + } > + return RISCV_EXCP_NONE; > +} > + > +static RISCVException > +read_th_mxstatus(CPURISCVState *env, int csrno, target_ulong *val) > +{ > + *val = env->th_mxstatus; > + return RISCV_EXCP_NONE; > +} > + > +static RISCVException > +write_th_mxstatus(CPURISCVState *env, int csrno, target_ulong val) > +{ > + uint64_t mxstatus = env->th_mxstatus; > + uint64_t mask = TH_MXSTATUS_MAEE; > + > + if ((val ^ mxstatus) & TH_MXSTATUS_MAEE) { > + tlb_flush(env_cpu(env)); > + } > + > + mxstatus = (mxstatus & ~mask) | (val & mask); > + env->th_mxstatus = mxstatus; > + return RISCV_EXCP_NONE; > +} > + > +riscv_csr_operations th_csr_ops[CSR_TABLE_SIZE] = { > +#if !defined(CONFIG_USER_ONLY) > + [CSR_TH_MXSTATUS] = { "th_mxstatus", th_maee_check, read_th_mxstatus, > + write_th_mxstatus}, > +#endif /* !CONFIG_USER_ONLY */ > +}; > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] target/riscv: Support xtheadmaee for thead-c906 2024-01-30 11:43 ` Christoph Müllner @ 2024-01-31 18:52 ` Conor Dooley 2024-02-04 5:47 ` LIU Zhiwei 1 sibling, 0 replies; 10+ messages in thread From: Conor Dooley @ 2024-01-31 18:52 UTC (permalink / raw) To: Christoph Müllner Cc: LIU Zhiwei, qemu-devel, Alistair.Francis, palmer, bin.meng, liwei1518, dbarboza, qemu-riscv, bjorn, Philipp Tomsich [-- Attachment #1: Type: text/plain, Size: 1593 bytes --] On Tue, Jan 30, 2024 at 12:43:25PM +0100, Christoph Müllner wrote: > On Tue, Jan 30, 2024 at 12:12 PM LIU Zhiwei > <zhiwei_liu@linux.alibaba.com> 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. Yeah, I was gonna request exactly this, so glad to see you beat me to the punch. It would be really great if this was done, particularly if this xmaee is going to appear in devicetrees and sooner or later is gonna want to be documented in a binding. Cheers, Conor. > 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] target/riscv: Support xtheadmaee for thead-c906 2024-01-30 11:43 ` Christoph Müllner 2024-01-31 18:52 ` Conor Dooley @ 2024-02-04 5:47 ` LIU Zhiwei 1 sibling, 0 replies; 10+ messages in thread From: LIU Zhiwei @ 2024-02-04 5:47 UTC (permalink / raw) To: Christoph Müllner, LIU Zhiwei Cc: qemu-devel, Alistair.Francis, palmer, bin.meng, liwei1518, dbarboza, qemu-riscv, bjorn, Philipp Tomsich On 2024/1/30 19:43, Christoph Müllner wrote: > On Tue, Jan 30, 2024 at 12:12 PM LIU Zhiwei > <zhiwei_liu@linux.alibaba.com> 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. Done. Thanks for the comment. The XTheadMaee specification is here: https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadmaee.adoc Thanks, Zhiwei > > 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 <zhiwei_liu@linux.alibaba.com> >> --- >> 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 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 ext_xtheadmaee; >> bool ext_XVentanaCondOps; >> >> uint32_t pmu_mask; >> @@ -176,7 +177,8 @@ static inline bool has_xthead_p(const RISCVCPUConfig *cfg) >> cfg->ext_xtheadcondmov || >> cfg->ext_xtheadfmemidx || cfg->ext_xtheadfmv || >> cfg->ext_xtheadmac || cfg->ext_xtheadmemidx || >> - cfg->ext_xtheadmempair || cfg->ext_xtheadsync; >> + cfg->ext_xtheadmempair || cfg->ext_xtheadsync || >> + cfg->ext_xtheadmaee; >> } >> >> #define MATERIALISE_EXT_PREDICATE(ext) \ >> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c >> index c7cc7eb423..5c1f380276 100644 >> --- a/target/riscv/cpu_helper.c >> +++ b/target/riscv/cpu_helper.c >> @@ -812,6 +812,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical, >> int napot_bits = 0; >> target_ulong napot_mask; >> >> + bool skip_pte_check = env->th_mxstatus & TH_MXSTATUS_MAEE; >> /* >> * Check if we should use the background registers for the two >> * stage translation. We don't need to check if we actually need >> @@ -974,18 +975,19 @@ restart: >> if (riscv_cpu_sxl(env) == MXL_RV32) { >> ppn = pte >> PTE_PPN_SHIFT; >> } else { >> - if (pte & PTE_RESERVED) { >> - return TRANSLATE_FAIL; >> - } >> + if (!skip_pte_check) { >> + if (pte & PTE_RESERVED) { >> + return TRANSLATE_FAIL; >> + } >> >> - if (!pbmte && (pte & PTE_PBMT)) { >> - return TRANSLATE_FAIL; >> - } >> + if (!pbmte && (pte & PTE_PBMT)) { >> + return TRANSLATE_FAIL; >> + } >> >> - if (!riscv_cpu_cfg(env)->ext_svnapot && (pte & PTE_N)) { >> - return TRANSLATE_FAIL; >> + if (!riscv_cpu_cfg(env)->ext_svnapot && (pte & PTE_N)) { >> + return TRANSLATE_FAIL; >> + } >> } >> - >> ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT; >> } >> >> @@ -998,7 +1000,8 @@ restart: >> } >> >> /* Inner PTE, continue walking */ >> - if (pte & (PTE_D | PTE_A | PTE_U | PTE_ATTR)) { >> + if ((pte & (PTE_D | PTE_A | PTE_U)) || >> + (!skip_pte_check && (pte & PTE_ATTR))) { >> return TRANSLATE_FAIL; >> } >> base = ppn << PGSHIFT; >> @@ -1012,7 +1015,7 @@ restart: >> /* Misaligned PPN */ >> return TRANSLATE_FAIL; >> } >> - if (!pbmte && (pte & PTE_PBMT)) { >> + if (!skip_pte_check && !pbmte && (pte & PTE_PBMT)) { >> /* Reserved without Svpbmt. */ >> return TRANSLATE_FAIL; >> } >> diff --git a/target/riscv/meson.build b/target/riscv/meson.build >> index a5e0734e7f..d7f675881d 100644 >> --- a/target/riscv/meson.build >> +++ b/target/riscv/meson.build >> @@ -12,6 +12,7 @@ riscv_ss.add(files( >> 'cpu.c', >> 'cpu_helper.c', >> 'csr.c', >> + 'xthead_csr.c', >> 'fpu_helper.c', >> 'gdbstub.c', >> 'op_helper.c', >> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c >> index 408b2ebffa..18a22dfb7a 100644 >> --- a/target/riscv/tcg/tcg-cpu.c >> +++ b/target/riscv/tcg/tcg-cpu.c >> @@ -763,7 +763,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) >> cpu->cfg.pmu_mask = 0; >> cpu->pmu_avail_ctrs = 0; >> } >> - >> /* >> * Disable isa extensions based on priv spec after we >> * validated and set everything we need. >> @@ -871,11 +870,9 @@ static void riscv_cpu_validate_profiles(RISCVCPU *cpu) >> } >> } >> >> -/* This stub just works for making vendors array not empty */ >> -riscv_csr_operations stub_csr_ops[CSR_TABLE_SIZE]; >> -static inline bool never_p(const RISCVCPUConfig *cfg) >> +static inline bool th_csr_p(const RISCVCPUConfig *cfg) >> { >> - return false; >> + return cfg->ext_xtheadmaee; >> } >> >> void riscv_tcg_cpu_register_vendor_csr(RISCVCPU *cpu) >> @@ -884,7 +881,7 @@ void riscv_tcg_cpu_register_vendor_csr(RISCVCPU *cpu) >> bool (*guard_func)(const RISCVCPUConfig *); >> riscv_csr_operations *csr_ops; >> } vendors[] = { >> - { never_p, stub_csr_ops }, >> + { th_csr_p, th_csr_ops }, >> }; >> for (size_t i = 0; i < ARRAY_SIZE(vendors); ++i) { >> if (!vendors[i].guard_func(&cpu->cfg)) { >> diff --git a/target/riscv/xthead_csr.c b/target/riscv/xthead_csr.c >> new file mode 100644 >> index 0000000000..c119a18789 >> --- /dev/null >> +++ b/target/riscv/xthead_csr.c >> @@ -0,0 +1,63 @@ >> +/* >> + * Xuantie implementation for RISC-V Control and Status Registers. >> + * >> + * Copyright (c) 2024 Alibaba Group. All rights reserved. >> + * >> + * 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 "qemu/log.h" >> +#include "cpu.h" >> +#include "tcg/tcg-cpu.h" >> +#include "exec/exec-all.h" >> +#include "exec/tb-flush.h" >> +#include "qapi/error.h" >> + >> +static RISCVException th_maee_check(CPURISCVState *env, int csrno) >> +{ >> + if (riscv_cpu_cfg(env)->ext_xtheadmaee) { >> + return RISCV_EXCP_ILLEGAL_INST; >> + } >> + return RISCV_EXCP_NONE; >> +} >> + >> +static RISCVException >> +read_th_mxstatus(CPURISCVState *env, int csrno, target_ulong *val) >> +{ >> + *val = env->th_mxstatus; >> + return RISCV_EXCP_NONE; >> +} >> + >> +static RISCVException >> +write_th_mxstatus(CPURISCVState *env, int csrno, target_ulong val) >> +{ >> + uint64_t mxstatus = env->th_mxstatus; >> + uint64_t mask = TH_MXSTATUS_MAEE; >> + >> + if ((val ^ mxstatus) & TH_MXSTATUS_MAEE) { >> + tlb_flush(env_cpu(env)); >> + } >> + >> + mxstatus = (mxstatus & ~mask) | (val & mask); >> + env->th_mxstatus = mxstatus; >> + return RISCV_EXCP_NONE; >> +} >> + >> +riscv_csr_operations th_csr_ops[CSR_TABLE_SIZE] = { >> +#if !defined(CONFIG_USER_ONLY) >> + [CSR_TH_MXSTATUS] = { "th_mxstatus", th_maee_check, read_th_mxstatus, >> + write_th_mxstatus}, >> +#endif /* !CONFIG_USER_ONLY */ >> +}; >> -- >> 2.25.1 >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] target/riscv: Support xtheadmaee for thead-c906 2024-01-30 11:11 ` [PATCH 2/2] target/riscv: Support xtheadmaee for thead-c906 LIU Zhiwei 2024-01-30 11:43 ` Christoph Müllner @ 2024-01-31 5:07 ` Richard Henderson 2024-01-31 6:21 ` LIU Zhiwei 1 sibling, 1 reply; 10+ messages in thread From: Richard Henderson @ 2024-01-31 5:07 UTC (permalink / raw) To: LIU Zhiwei, qemu-devel Cc: Alistair.Francis, palmer, bin.meng, liwei1518, dbarboza, qemu-riscv, christoph.muellner, bjorn On 1/30/24 21:11, LIU Zhiwei wrote: > +riscv_csr_operations th_csr_ops[CSR_TABLE_SIZE] = { > +#if !defined(CONFIG_USER_ONLY) > + [CSR_TH_MXSTATUS] = { "th_mxstatus", th_maee_check, read_th_mxstatus, > + write_th_mxstatus}, > +#endif /* !CONFIG_USER_ONLY */ > +}; This is clearly the wrong data structure for a single entry in the array. r~ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] target/riscv: Support xtheadmaee for thead-c906 2024-01-31 5:07 ` Richard Henderson @ 2024-01-31 6:21 ` LIU Zhiwei 0 siblings, 0 replies; 10+ messages in thread From: LIU Zhiwei @ 2024-01-31 6:21 UTC (permalink / raw) To: Richard Henderson, LIU Zhiwei, qemu-devel Cc: Alistair.Francis, palmer, bin.meng, liwei1518, dbarboza, qemu-riscv, christoph.muellner, bjorn On 2024/1/31 13:07, Richard Henderson wrote: > On 1/30/24 21:11, LIU Zhiwei wrote: >> +riscv_csr_operations th_csr_ops[CSR_TABLE_SIZE] = { >> +#if !defined(CONFIG_USER_ONLY) >> + [CSR_TH_MXSTATUS] = { "th_mxstatus", th_maee_check, >> read_th_mxstatus, >> + write_th_mxstatus}, >> +#endif /* !CONFIG_USER_ONLY */ >> +}; > > This is clearly the wrong data structure for a single entry in the array. This array should have the same size with csr_ops so that we can override custom CSR behavior directly. Besides, It will have other entries in the near future. I see that I missed surround the th_maee_check, read_th_mxstatus, write_mxstatus with !CONFIG_USER_ONLY. But I don't understand why it is wrong for a single entry in the array, at least the compiler think it has no error. Thanks, Zhiwei > > > r~ > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-02-04 5:47 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-30 11:11 [PATCH 0/2] target/riscv: Support mxstatus CSR for thead-c906 LIU Zhiwei 2024-01-30 11:11 ` [PATCH 1/2] target/riscv: Register vendors CSR LIU Zhiwei 2024-01-31 5:06 ` Richard Henderson 2024-01-31 6:14 ` LIU Zhiwei 2024-01-30 11:11 ` [PATCH 2/2] target/riscv: Support xtheadmaee for thead-c906 LIU Zhiwei 2024-01-30 11:43 ` Christoph Müllner 2024-01-31 18:52 ` Conor Dooley 2024-02-04 5:47 ` LIU Zhiwei 2024-01-31 5:07 ` Richard Henderson 2024-01-31 6:21 ` LIU Zhiwei
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).