From: "Christoph Müllner" <christoph.muellner@vrull.eu>
To: Alistair Francis <alistair23@gmail.com>
Cc: qemu-riscv@nongnu.org, qemu-devel@nongnu.org,
Alistair Francis <alistair.francis@wdc.com>,
Bin Meng <bin.meng@windriver.com>,
Philipp Tomsich <philipp.tomsich@vrull.eu>,
Palmer Dabbelt <palmer@dabbelt.com>,
Richard Henderson <richard.henderson@linaro.org>,
Cooper Qu <cooper.qu@linux.alibaba.com>,
Zhiwei Liu <zhiwei_liu@linux.alibaba.com>,
Huang Tao <eric.huang@linux.alibaba.com>,
Conor Dooley <conor@kernel.org>,
Andrew Jones <ajones@ventanamicro.com>,
Daniel Henrique Barboza <dbarboza@ventanamicro.com>,
Vivian Wang <uwu@dram.page>, Qingfang Deng <dqfext@gmail.com>,
Alexandre Ghiti <alex@ghiti.fr>,
LIU Zhiwei <zhiwe_liu@linux.alibaba.com>,
Weiwei Li <liwei1518@gmail.com>
Subject: Re: [PATCH v3] riscv: thead: Add th.sxstatus CSR emulation
Date: Mon, 22 Apr 2024 08:42:43 +0200 [thread overview]
Message-ID: <CAEg0e7g2STB3U55EBWw9_busuCf8QhWbt0wW2gFpE0doLVgQ2Q@mail.gmail.com> (raw)
In-Reply-To: <CAKmqyKOPEZQoQqs+W5A68HK1W+9wbLrbKrG1d1Pq4ABnxcZWdg@mail.gmail.com>
On Mon, Apr 22, 2024 at 5:29 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Thu, Apr 18, 2024 at 8:55 AM Christoph Müllner
> <christoph.muellner@vrull.eu> 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 <zhiwe_liu@linux.alibaba.com>
> > Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> > ---
> > 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 0000000000..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_UCME BIT(16)
> > +#define TH_SXSTATUS_MAEE BIT(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,
> > + .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 = &th_csr_list[i].csr_ops;
> > + if (!th_csr_list[i].insertion_test(cpu))
> > + riscv_set_csr_ops(csrno, csr_ops);
> > + }
> > +}
> > --
> > 2.44.0
> >
> >
prev parent reply other threads:[~2024-04-22 6:43 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-17 22:54 [PATCH v3] riscv: thead: Add th.sxstatus CSR emulation Christoph Müllner
2024-04-22 3:29 ` Alistair Francis
2024-04-22 6:42 ` Christoph Müllner [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAEg0e7g2STB3U55EBWw9_busuCf8QhWbt0wW2gFpE0doLVgQ2Q@mail.gmail.com \
--to=christoph.muellner@vrull.eu \
--cc=ajones@ventanamicro.com \
--cc=alex@ghiti.fr \
--cc=alistair.francis@wdc.com \
--cc=alistair23@gmail.com \
--cc=bin.meng@windriver.com \
--cc=conor@kernel.org \
--cc=cooper.qu@linux.alibaba.com \
--cc=dbarboza@ventanamicro.com \
--cc=dqfext@gmail.com \
--cc=eric.huang@linux.alibaba.com \
--cc=liwei1518@gmail.com \
--cc=palmer@dabbelt.com \
--cc=philipp.tomsich@vrull.eu \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=uwu@dram.page \
--cc=zhiwe_liu@linux.alibaba.com \
--cc=zhiwei_liu@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).