* [PATCH RFC 1/8] target/riscv: Add properties for Indirect CSR Access extension
2024-02-17 0:01 [PATCH RFC 0/8] Add Counter delegation ISA extension support Atish Patra
@ 2024-02-17 0:01 ` Atish Patra
2024-02-17 0:01 ` [PATCH RFC 2/8] target/riscv: Decouple AIA processing from xiselect and xireg Atish Patra
` (8 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Atish Patra @ 2024-02-17 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Atish Patra, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1
From: Kaiwen Xue <kaiwenx@rivosinc.com>
This adds the properties for sxcsrind. Definitions of new registers and
implementations will come with future patches.
Signed-off-by: Atish Patra <atishp@rivosinc.com>
Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
---
target/riscv/cpu.c | 4 ++++
target/riscv/cpu_cfg.h | 2 ++
2 files changed, 6 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 8af99ed2f6de..ff7c6c7c380e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -152,10 +152,12 @@ const RISCVIsaExtData isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
ISA_EXT_DATA_ENTRY(smcntrpmf, PRIV_VERSION_1_12_0, ext_smcntrpmf),
+ ISA_EXT_DATA_ENTRY(smcsrind, PRIV_VERSION_1_12_0, ext_smcsrind),
ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
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(sscsrind, PRIV_VERSION_1_12_0, ext_sscsrind),
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),
@@ -1348,6 +1350,8 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
/* Defaults for standard extensions */
MULTI_EXT_CFG_BOOL("sscofpmf", ext_sscofpmf, false),
MULTI_EXT_CFG_BOOL("smcntrpmf", ext_smcntrpmf, false),
+ MULTI_EXT_CFG_BOOL("smcsrind", ext_smcsrind, false),
+ MULTI_EXT_CFG_BOOL("sscsrind", ext_sscsrind, false),
MULTI_EXT_CFG_BOOL("zifencei", ext_zifencei, true),
MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true),
MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index eabbecb8f962..b9086464752e 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -74,6 +74,8 @@ struct RISCVCPUConfig {
bool ext_smstateen;
bool ext_sstc;
bool ext_smcntrpmf;
+ bool ext_smcsrind;
+ bool ext_sscsrind;
bool ext_svadu;
bool ext_svinval;
bool ext_svnapot;
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH RFC 2/8] target/riscv: Decouple AIA processing from xiselect and xireg
2024-02-17 0:01 [PATCH RFC 0/8] Add Counter delegation ISA extension support Atish Patra
2024-02-17 0:01 ` [PATCH RFC 1/8] target/riscv: Add properties for Indirect CSR Access extension Atish Patra
@ 2024-02-17 0:01 ` Atish Patra
2024-06-05 8:17 ` Jason Chien
2024-02-17 0:01 ` [PATCH RFC 3/8] target/riscv: Enable S*stateen bits for AIA Atish Patra
` (7 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Atish Patra @ 2024-02-17 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Atish Patra, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1
From: Kaiwen Xue <kaiwenx@rivosinc.com>
Since xiselect and xireg also will be of use in sxcsrind, AIA should
have its own separated interface when those CSRs are accessed.
Signed-off-by: Atish Patra <atishp@rivosinc.com>
Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
---
target/riscv/csr.c | 147 +++++++++++++++++++++++++++++++++++++--------
1 file changed, 122 insertions(+), 25 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 8829dee7bc75..1af0c8890a2b 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -287,6 +287,15 @@ static int aia_any32(CPURISCVState *env, int csrno)
return any32(env, csrno);
}
+static int sxcsrind_or_aia_any(CPURISCVState *env, int csrno)
+{
+ if (!riscv_cpu_cfg(env)->ext_smaia && !riscv_cpu_cfg(env)->ext_smcsrind) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ return any(env, csrno);
+}
+
static RISCVException smode(CPURISCVState *env, int csrno)
{
if (riscv_has_ext(env, RVS)) {
@@ -323,6 +332,15 @@ static int aia_smode32(CPURISCVState *env, int csrno)
return smode32(env, csrno);
}
+static int sxcsrind_or_aia_smode(CPURISCVState *env, int csrno)
+{
+ if (!riscv_cpu_cfg(env)->ext_ssaia && !riscv_cpu_cfg(env)->ext_sscsrind) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ return smode(env, csrno);
+}
+
static RISCVException hmode(CPURISCVState *env, int csrno)
{
if (riscv_has_ext(env, RVH)) {
@@ -342,6 +360,15 @@ static RISCVException hmode32(CPURISCVState *env, int csrno)
}
+static int sxcsrind_or_aia_hmode(CPURISCVState *env, int csrno)
+{
+ if (!riscv_cpu_cfg(env)->ext_ssaia && !riscv_cpu_cfg(env)->ext_sscsrind) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ return hmode(env, csrno);
+}
+
static RISCVException umode(CPURISCVState *env, int csrno)
{
if (riscv_has_ext(env, RVU)) {
@@ -1804,13 +1831,29 @@ static int aia_xlate_vs_csrno(CPURISCVState *env, int csrno)
};
}
+static int sxcsrind_xlate_vs_csrno(CPURISCVState *env, int csrno)
+{
+ if (!env->virt_enabled) {
+ return csrno;
+ }
+
+ switch (csrno) {
+ case CSR_SISELECT:
+ return CSR_VSISELECT;
+ case CSR_SIREG:
+ return CSR_VSIREG;
+ default:
+ return csrno;
+ };
+}
+
static int rmw_xiselect(CPURISCVState *env, int csrno, target_ulong *val,
target_ulong new_val, target_ulong wr_mask)
{
target_ulong *iselect;
/* Translate CSR number for VS-mode */
- csrno = aia_xlate_vs_csrno(env, csrno);
+ csrno = sxcsrind_xlate_vs_csrno(env, csrno);
/* Find the iselect CSR based on CSR number */
switch (csrno) {
@@ -1839,6 +1882,12 @@ static int rmw_xiselect(CPURISCVState *env, int csrno, target_ulong *val,
return RISCV_EXCP_NONE;
}
+static bool xiselect_aia_range(target_ulong isel)
+{
+ return (ISELECT_IPRIO0 <= isel && isel <= ISELECT_IPRIO15) ||
+ (ISELECT_IMSIC_FIRST <= isel && isel <= ISELECT_IMSIC_LAST);
+}
+
static int rmw_iprio(target_ulong xlen,
target_ulong iselect, uint8_t *iprio,
target_ulong *val, target_ulong new_val,
@@ -1884,44 +1933,44 @@ static int rmw_iprio(target_ulong xlen,
return 0;
}
-static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
- target_ulong new_val, target_ulong wr_mask)
+static int rmw_xireg_aia(CPURISCVState *env, int csrno,
+ target_ulong isel, target_ulong *val,
+ target_ulong new_val, target_ulong wr_mask)
{
- bool virt, isel_reserved;
- uint8_t *iprio;
+ bool virt = false, isel_reserved = false;
int ret = -EINVAL;
- target_ulong priv, isel, vgein;
-
- /* Translate CSR number for VS-mode */
- csrno = aia_xlate_vs_csrno(env, csrno);
+ uint8_t *iprio;
+ target_ulong priv, vgein;
- /* Decode register details from CSR number */
- virt = false;
- isel_reserved = false;
+ /* VS-mode CSR number passed in has already been translated */
switch (csrno) {
case CSR_MIREG:
+ if (!riscv_cpu_cfg(env)->ext_smaia) {
+ goto done;
+ }
iprio = env->miprio;
- isel = env->miselect;
priv = PRV_M;
break;
case CSR_SIREG:
- if (env->priv == PRV_S && env->mvien & MIP_SEIP &&
+ if (!riscv_cpu_cfg(env)->ext_ssaia ||
+ (env->priv == PRV_S && env->mvien & MIP_SEIP &&
env->siselect >= ISELECT_IMSIC_EIDELIVERY &&
- env->siselect <= ISELECT_IMSIC_EIE63) {
+ env->siselect <= ISELECT_IMSIC_EIE63)) {
goto done;
}
iprio = env->siprio;
- isel = env->siselect;
priv = PRV_S;
break;
case CSR_VSIREG:
+ if (!riscv_cpu_cfg(env)->ext_ssaia) {
+ goto done;
+ }
iprio = env->hviprio;
- isel = env->vsiselect;
priv = PRV_S;
virt = true;
break;
default:
- goto done;
+ goto done;
};
/* Find the selected guest interrupt file */
@@ -1952,10 +2001,53 @@ static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
}
done:
+ /*
+ * If AIA is not enabled, illegal instruction exception is always
+ * returned regardless of whether we are in VS-mode or not
+ */
if (ret) {
return (env->virt_enabled && virt && !isel_reserved) ?
RISCV_EXCP_VIRT_INSTRUCTION_FAULT : RISCV_EXCP_ILLEGAL_INST;
}
+
+ return RISCV_EXCP_NONE;
+}
+
+static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ bool virt = false;
+ int ret = -EINVAL;
+ target_ulong isel;
+
+ /* Translate CSR number for VS-mode */
+ csrno = sxcsrind_xlate_vs_csrno(env, csrno);
+
+ /* Decode register details from CSR number */
+ switch (csrno) {
+ case CSR_MIREG:
+ isel = env->miselect;
+ break;
+ case CSR_SIREG:
+ isel = env->siselect;
+ break;
+ case CSR_VSIREG:
+ isel = env->vsiselect;
+ virt = true;
+ break;
+ default:
+ goto done;
+ };
+
+ if (xiselect_aia_range(isel)) {
+ return rmw_xireg_aia(env, csrno, isel, val, new_val, wr_mask);
+ }
+
+done:
+ if (ret) {
+ return (env->virt_enabled && virt) ?
+ RISCV_EXCP_VIRT_INSTRUCTION_FAULT : RISCV_EXCP_ILLEGAL_INST;
+ }
return RISCV_EXCP_NONE;
}
@@ -4682,8 +4774,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_MIP] = { "mip", any, NULL, NULL, rmw_mip },
/* Machine-Level Window to Indirectly Accessed Registers (AIA) */
- [CSR_MISELECT] = { "miselect", aia_any, NULL, NULL, rmw_xiselect },
- [CSR_MIREG] = { "mireg", aia_any, NULL, NULL, rmw_xireg },
+ [CSR_MISELECT] = { "miselect", sxcsrind_or_aia_any, NULL, NULL,
+ rmw_xiselect },
+ [CSR_MIREG] = { "mireg", sxcsrind_or_aia_any, NULL, NULL,
+ rmw_xireg },
/* Machine-Level Interrupts (AIA) */
[CSR_MTOPEI] = { "mtopei", aia_any, NULL, NULL, rmw_xtopei },
@@ -4801,8 +4895,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_SATP] = { "satp", satp, read_satp, write_satp },
/* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
- [CSR_SISELECT] = { "siselect", aia_smode, NULL, NULL, rmw_xiselect },
- [CSR_SIREG] = { "sireg", aia_smode, NULL, NULL, rmw_xireg },
+ [CSR_SISELECT] = { "siselect", sxcsrind_or_aia_smode, NULL, NULL,
+ rmw_xiselect },
+ [CSR_SIREG] = { "sireg", sxcsrind_or_aia_smode, NULL, NULL,
+ rmw_xireg },
/* Supervisor-Level Interrupts (AIA) */
[CSR_STOPEI] = { "stopei", aia_smode, NULL, NULL, rmw_xtopei },
@@ -4881,9 +4977,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
/*
* VS-Level Window to Indirectly Accessed Registers (H-extension with AIA)
*/
- [CSR_VSISELECT] = { "vsiselect", aia_hmode, NULL, NULL,
- rmw_xiselect },
- [CSR_VSIREG] = { "vsireg", aia_hmode, NULL, NULL, rmw_xireg },
+ [CSR_VSISELECT] = { "vsiselect", sxcsrind_or_aia_hmode, NULL, NULL,
+ rmw_xiselect },
+ [CSR_VSIREG] = { "vsireg", sxcsrind_or_aia_hmode, NULL, NULL,
+ rmw_xireg },
/* VS-Level Interrupts (H-extension with AIA) */
[CSR_VSTOPEI] = { "vstopei", aia_hmode, NULL, NULL, rmw_xtopei },
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH RFC 2/8] target/riscv: Decouple AIA processing from xiselect and xireg
2024-02-17 0:01 ` [PATCH RFC 2/8] target/riscv: Decouple AIA processing from xiselect and xireg Atish Patra
@ 2024-06-05 8:17 ` Jason Chien
0 siblings, 0 replies; 19+ messages in thread
From: Jason Chien @ 2024-06-05 8:17 UTC (permalink / raw)
To: Atish Patra, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1
Atish Patra 於 2024/2/17 上午 08:01 寫道:
> From: Kaiwen Xue <kaiwenx@rivosinc.com>
>
> Since xiselect and xireg also will be of use in sxcsrind, AIA should
> have its own separated interface when those CSRs are accessed.
>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
> ---
> target/riscv/csr.c | 147 +++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 122 insertions(+), 25 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 8829dee7bc75..1af0c8890a2b 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -287,6 +287,15 @@ static int aia_any32(CPURISCVState *env, int csrno)
> return any32(env, csrno);
> }
>
> +static int sxcsrind_or_aia_any(CPURISCVState *env, int csrno)
Could you rename the function as csrind_or_aia_any() to keep the naming
consistency with aia?
> +{
> + if (!riscv_cpu_cfg(env)->ext_smaia && !riscv_cpu_cfg(env)->ext_smcsrind) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> + return any(env, csrno);
> +}
> +
> static RISCVException smode(CPURISCVState *env, int csrno)
> {
> if (riscv_has_ext(env, RVS)) {
> @@ -323,6 +332,15 @@ static int aia_smode32(CPURISCVState *env, int csrno)
> return smode32(env, csrno);
> }
>
> +static int sxcsrind_or_aia_smode(CPURISCVState *env, int csrno)
Could you rename the function as csrind_or_aia_smode() to keep the
naming consistency with aia?
> +{
> + if (!riscv_cpu_cfg(env)->ext_ssaia && !riscv_cpu_cfg(env)->ext_sscsrind) {
S-mode CSRs are defined in Smaia and Smcsrind as well. We should ensure
at least one of Smaia, Ssaia, Smcsrind, Sscsrind is enabled.
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> + return smode(env, csrno);
> +}
> +
> static RISCVException hmode(CPURISCVState *env, int csrno)
> {
> if (riscv_has_ext(env, RVH)) {
> @@ -342,6 +360,15 @@ static RISCVException hmode32(CPURISCVState *env, int csrno)
>
> }
>
> +static int sxcsrind_or_aia_hmode(CPURISCVState *env, int csrno)
Could you rename the function as csrind_or_aia_hmode() to keep the
naming consistency with aia?
> +{
> + if (!riscv_cpu_cfg(env)->ext_ssaia && !riscv_cpu_cfg(env)->ext_sscsrind) {
Hypervisor and VS CSRs are defined in Smaia and Smcsrind as well. We
should ensure at least one of Smaia, Ssaia, Smcsrind, Sscsrind is enabled.
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> + return hmode(env, csrno);
> +}
> +
> static RISCVException umode(CPURISCVState *env, int csrno)
> {
> if (riscv_has_ext(env, RVU)) {
> @@ -1804,13 +1831,29 @@ static int aia_xlate_vs_csrno(CPURISCVState *env, int csrno)
> };
> }
>
> +static int sxcsrind_xlate_vs_csrno(CPURISCVState *env, int csrno)
Could you rename the function as csrind_xlate_vs_csrno() to keep the
naming consistency with aia_xlate_vs_csrno()?
> +{
> + if (!env->virt_enabled) {
> + return csrno;
> + }
> +
> + switch (csrno) {
> + case CSR_SISELECT:
> + return CSR_VSISELECT;
> + case CSR_SIREG:
> + return CSR_VSIREG;
> + default:
> + return csrno;
> + };
> +}
> +
> static int rmw_xiselect(CPURISCVState *env, int csrno, target_ulong *val,
> target_ulong new_val, target_ulong wr_mask)
> {
> target_ulong *iselect;
>
> /* Translate CSR number for VS-mode */
> - csrno = aia_xlate_vs_csrno(env, csrno);
> + csrno = sxcsrind_xlate_vs_csrno(env, csrno);
>
> /* Find the iselect CSR based on CSR number */
> switch (csrno) {
> @@ -1839,6 +1882,12 @@ static int rmw_xiselect(CPURISCVState *env, int csrno, target_ulong *val,
> return RISCV_EXCP_NONE;
> }
>
> +static bool xiselect_aia_range(target_ulong isel)
> +{
> + return (ISELECT_IPRIO0 <= isel && isel <= ISELECT_IPRIO15) ||
> + (ISELECT_IMSIC_FIRST <= isel && isel <= ISELECT_IMSIC_LAST);
> +}
> +
> static int rmw_iprio(target_ulong xlen,
> target_ulong iselect, uint8_t *iprio,
> target_ulong *val, target_ulong new_val,
> @@ -1884,44 +1933,44 @@ static int rmw_iprio(target_ulong xlen,
> return 0;
> }
>
> -static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
> - target_ulong new_val, target_ulong wr_mask)
> +static int rmw_xireg_aia(CPURISCVState *env, int csrno,
> + target_ulong isel, target_ulong *val,
> + target_ulong new_val, target_ulong wr_mask)
> {
> - bool virt, isel_reserved;
> - uint8_t *iprio;
> + bool virt = false, isel_reserved = false;
> int ret = -EINVAL;
> - target_ulong priv, isel, vgein;
> -
> - /* Translate CSR number for VS-mode */
> - csrno = aia_xlate_vs_csrno(env, csrno);
> + uint8_t *iprio;
> + target_ulong priv, vgein;
>
> - /* Decode register details from CSR number */
> - virt = false;
> - isel_reserved = false;
> + /* VS-mode CSR number passed in has already been translated */
> switch (csrno) {
> case CSR_MIREG:
> + if (!riscv_cpu_cfg(env)->ext_smaia) {
> + goto done;
> + }
> iprio = env->miprio;
> - isel = env->miselect;
> priv = PRV_M;
> break;
> case CSR_SIREG:
> - if (env->priv == PRV_S && env->mvien & MIP_SEIP &&
> + if (!riscv_cpu_cfg(env)->ext_ssaia ||
> + (env->priv == PRV_S && env->mvien & MIP_SEIP &&
> env->siselect >= ISELECT_IMSIC_EIDELIVERY &&
> - env->siselect <= ISELECT_IMSIC_EIE63) {
> + env->siselect <= ISELECT_IMSIC_EIE63)) {
> goto done;
> }
> iprio = env->siprio;
> - isel = env->siselect;
> priv = PRV_S;
> break;
> case CSR_VSIREG:
> + if (!riscv_cpu_cfg(env)->ext_ssaia) {
> + goto done;
> + }
> iprio = env->hviprio;
> - isel = env->vsiselect;
> priv = PRV_S;
> virt = true;
> break;
> default:
> - goto done;
> + goto done;
> };
>
> /* Find the selected guest interrupt file */
> @@ -1952,10 +2001,53 @@ static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
> }
>
> done:
> + /*
> + * If AIA is not enabled, illegal instruction exception is always
> + * returned regardless of whether we are in VS-mode or not
> + */
> if (ret) {
> return (env->virt_enabled && virt && !isel_reserved) ?
> RISCV_EXCP_VIRT_INSTRUCTION_FAULT : RISCV_EXCP_ILLEGAL_INST;
> }
> +
> + return RISCV_EXCP_NONE;
> +}
> +
> +static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
> + target_ulong new_val, target_ulong wr_mask)
> +{
> + bool virt = false;
> + int ret = -EINVAL;
> + target_ulong isel;
> +
> + /* Translate CSR number for VS-mode */
> + csrno = sxcsrind_xlate_vs_csrno(env, csrno);
> +
> + /* Decode register details from CSR number */
> + switch (csrno) {
> + case CSR_MIREG:
> + isel = env->miselect;
> + break;
> + case CSR_SIREG:
> + isel = env->siselect;
> + break;
> + case CSR_VSIREG:
> + isel = env->vsiselect;
> + virt = true;
> + break;
> + default:
> + goto done;
> + };
> +
> + if (xiselect_aia_range(isel)) {
> + return rmw_xireg_aia(env, csrno, isel, val, new_val, wr_mask);
> + }
> +
> +done:
> + if (ret) {
> + return (env->virt_enabled && virt) ?
> + RISCV_EXCP_VIRT_INSTRUCTION_FAULT : RISCV_EXCP_ILLEGAL_INST;
A virtual instruction exception is raised for attempts from VS-mode or
VU-mode to directly access vsiselect or vsireg* in riscv_csrrw_check(),
we don't need to check again here.
> + }
> return RISCV_EXCP_NONE;
> }
>
> @@ -4682,8 +4774,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_MIP] = { "mip", any, NULL, NULL, rmw_mip },
>
> /* Machine-Level Window to Indirectly Accessed Registers (AIA) */
> - [CSR_MISELECT] = { "miselect", aia_any, NULL, NULL, rmw_xiselect },
> - [CSR_MIREG] = { "mireg", aia_any, NULL, NULL, rmw_xireg },
> + [CSR_MISELECT] = { "miselect", sxcsrind_or_aia_any, NULL, NULL,
> + rmw_xiselect },
> + [CSR_MIREG] = { "mireg", sxcsrind_or_aia_any, NULL, NULL,
> + rmw_xireg },
>
> /* Machine-Level Interrupts (AIA) */
> [CSR_MTOPEI] = { "mtopei", aia_any, NULL, NULL, rmw_xtopei },
> @@ -4801,8 +4895,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_SATP] = { "satp", satp, read_satp, write_satp },
>
> /* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
> - [CSR_SISELECT] = { "siselect", aia_smode, NULL, NULL, rmw_xiselect },
> - [CSR_SIREG] = { "sireg", aia_smode, NULL, NULL, rmw_xireg },
> + [CSR_SISELECT] = { "siselect", sxcsrind_or_aia_smode, NULL, NULL,
> + rmw_xiselect },
> + [CSR_SIREG] = { "sireg", sxcsrind_or_aia_smode, NULL, NULL,
> + rmw_xireg },
>
> /* Supervisor-Level Interrupts (AIA) */
> [CSR_STOPEI] = { "stopei", aia_smode, NULL, NULL, rmw_xtopei },
> @@ -4881,9 +4977,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> /*
> * VS-Level Window to Indirectly Accessed Registers (H-extension with AIA)
> */
> - [CSR_VSISELECT] = { "vsiselect", aia_hmode, NULL, NULL,
> - rmw_xiselect },
> - [CSR_VSIREG] = { "vsireg", aia_hmode, NULL, NULL, rmw_xireg },
> + [CSR_VSISELECT] = { "vsiselect", sxcsrind_or_aia_hmode, NULL, NULL,
> + rmw_xiselect },
> + [CSR_VSIREG] = { "vsireg", sxcsrind_or_aia_hmode, NULL, NULL,
> + rmw_xireg },
>
> /* VS-Level Interrupts (H-extension with AIA) */
> [CSR_VSTOPEI] = { "vstopei", aia_hmode, NULL, NULL, rmw_xtopei },
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH RFC 3/8] target/riscv: Enable S*stateen bits for AIA
2024-02-17 0:01 [PATCH RFC 0/8] Add Counter delegation ISA extension support Atish Patra
2024-02-17 0:01 ` [PATCH RFC 1/8] target/riscv: Add properties for Indirect CSR Access extension Atish Patra
2024-02-17 0:01 ` [PATCH RFC 2/8] target/riscv: Decouple AIA processing from xiselect and xireg Atish Patra
@ 2024-02-17 0:01 ` Atish Patra
2024-02-17 0:01 ` [PATCH RFC 4/8] target/riscv: Support generic CSR indirect access Atish Patra
` (6 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Atish Patra @ 2024-02-17 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Atish Patra, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1
As per the ratified AIA spec v1.0, three stateen bits control AIA CSR
access.
Bit 60 controls the indirect CSRs
Bit 59 controls the most AIA CSR state
Bit 58 controls the IMSIC state such as stopei and vstopei
Enable the corresponding bits in [m|h]stateen and enable corresponding
checks in the CSR accessor functions.
Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
target/riscv/csr.c | 89 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 88 insertions(+), 1 deletion(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 1af0c8890a2b..89a1325a02a5 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -316,19 +316,42 @@ static int smode32(CPURISCVState *env, int csrno)
static int aia_smode(CPURISCVState *env, int csrno)
{
+ int ret;
+
if (!riscv_cpu_cfg(env)->ext_ssaia) {
return RISCV_EXCP_ILLEGAL_INST;
}
+ if (csrno == CSR_STOPEI) {
+ ret = smstateen_acc_ok(env, 0, SMSTATEEN0_IMSIC);
+ } else {
+ ret = smstateen_acc_ok(env, 0, SMSTATEEN0_AIA);
+ }
+
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
return smode(env, csrno);
}
static int aia_smode32(CPURISCVState *env, int csrno)
{
+ int ret;
+
if (!riscv_cpu_cfg(env)->ext_ssaia) {
return RISCV_EXCP_ILLEGAL_INST;
}
+ ret = smstateen_acc_ok(env, 0, SMSTATEEN0_AIA);
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
return smode32(env, csrno);
}
@@ -552,15 +575,38 @@ static RISCVException pointer_masking(CPURISCVState *env, int csrno)
static int aia_hmode(CPURISCVState *env, int csrno)
{
+ int ret;
+
if (!riscv_cpu_cfg(env)->ext_ssaia) {
return RISCV_EXCP_ILLEGAL_INST;
}
- return hmode(env, csrno);
+ if (csrno == CSR_VSTOPEI) {
+ ret = smstateen_acc_ok(env, 0, SMSTATEEN0_IMSIC);
+ } else {
+ ret = smstateen_acc_ok(env, 0, SMSTATEEN0_AIA);
+ }
+
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
+ return hmode(env, csrno);
}
static int aia_hmode32(CPURISCVState *env, int csrno)
{
+ int ret;
+
+ if (!riscv_cpu_cfg(env)->ext_ssaia) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ ret = smstateen_acc_ok(env, 0, SMSTATEEN0_AIA);
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
if (!riscv_cpu_cfg(env)->ext_ssaia) {
return RISCV_EXCP_ILLEGAL_INST;
}
@@ -1851,6 +1897,12 @@ static int rmw_xiselect(CPURISCVState *env, int csrno, target_ulong *val,
target_ulong new_val, target_ulong wr_mask)
{
target_ulong *iselect;
+ int ret;
+
+ ret = smstateen_acc_ok(env, 0, SMSTATEEN0_SVSLCT);
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
/* Translate CSR number for VS-mode */
csrno = sxcsrind_xlate_vs_csrno(env, csrno);
@@ -2020,6 +2072,11 @@ static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
int ret = -EINVAL;
target_ulong isel;
+ ret = smstateen_acc_ok(env, 0, SMSTATEEN0_SVSLCT);
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
/* Translate CSR number for VS-mode */
csrno = sxcsrind_xlate_vs_csrno(env, csrno);
@@ -2419,6 +2476,23 @@ static RISCVException write_mstateen0(CPURISCVState *env, int csrno,
wr_mask |= SMSTATEEN0_FCSR;
}
+ /*
+ * TODO: Do we need to check ssaia as well ? Can we enable ssaia without
+ * smaia ?
+ */
+ if (riscv_cpu_cfg(env)->ext_smaia) {
+ wr_mask |= SMSTATEEN0_SVSLCT;
+ }
+
+ /*
+ * As per the AIA specification, SMSTATEEN0_IMSIC is valid only if IMSIC is
+ * implemented. However, that information is with MachineState and we can't
+ * figure that out in csr.c. Just enable if Smaia is available.
+ */
+ if (riscv_cpu_cfg(env)->ext_smaia) {
+ wr_mask |= (SMSTATEEN0_AIA | SMSTATEEN0_IMSIC);
+ }
+
return write_mstateen(env, csrno, wr_mask, new_val);
}
@@ -2495,6 +2569,19 @@ static RISCVException write_hstateen0(CPURISCVState *env, int csrno,
wr_mask |= SMSTATEEN0_FCSR;
}
+ if (riscv_cpu_cfg(env)->ext_ssaia) {
+ wr_mask |= SMSTATEEN0_SVSLCT;
+ }
+
+ /*
+ * As per the AIA specification, SMSTATEEN0_IMSIC is valid only if IMSIC is
+ * implemented. However, that information is with MachineState and we can't
+ * figure that out in csr.c. Just enable if Ssaia is available.
+ */
+ if (riscv_cpu_cfg(env)->ext_ssaia) {
+ wr_mask |= (SMSTATEEN0_AIA | SMSTATEEN0_IMSIC);
+ }
+
return write_hstateen(env, csrno, wr_mask, new_val);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH RFC 4/8] target/riscv: Support generic CSR indirect access
2024-02-17 0:01 [PATCH RFC 0/8] Add Counter delegation ISA extension support Atish Patra
` (2 preceding siblings ...)
2024-02-17 0:01 ` [PATCH RFC 3/8] target/riscv: Enable S*stateen bits for AIA Atish Patra
@ 2024-02-17 0:01 ` Atish Patra
2024-06-05 11:49 ` Jason Chien
2024-02-17 0:01 ` [PATCH RFC 5/8] target/riscv: Add smcdeleg/ssccfg properties Atish Patra
` (5 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Atish Patra @ 2024-02-17 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Atish Patra, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1
From: Kaiwen Xue <kaiwenx@rivosinc.com>
This adds the indirect access registers required by sscsrind/smcsrind
and the operations on them. Note that xiselect and xireg are used for
both AIA and sxcsrind, and the behavior of accessing them depends on
whether each extension is enabled and the value stored in xiselect.
Co-developed-by: Atish Patra <atishp@rivosinc.com>
Signed-off-by: Atish Patra <atishp@rivosinc.com>
Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
---
target/riscv/cpu_bits.h | 28 +++++++-
target/riscv/csr.c | 146 +++++++++++++++++++++++++++++++++++++++-
2 files changed, 169 insertions(+), 5 deletions(-)
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 0ee91e502e8f..3a66f83009b5 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -176,6 +176,13 @@
#define CSR_MISELECT 0x350
#define CSR_MIREG 0x351
+/* Machine Indirect Register Alias */
+#define CSR_MIREG2 0x352
+#define CSR_MIREG3 0x353
+#define CSR_MIREG4 0x355
+#define CSR_MIREG5 0x356
+#define CSR_MIREG6 0x357
+
/* Machine-Level Interrupts (AIA) */
#define CSR_MTOPEI 0x35c
#define CSR_MTOPI 0xfb0
@@ -225,6 +232,13 @@
#define CSR_SISELECT 0x150
#define CSR_SIREG 0x151
+/* Supervisor Indirect Register Alias */
+#define CSR_SIREG2 0x152
+#define CSR_SIREG3 0x153
+#define CSR_SIREG4 0x155
+#define CSR_SIREG5 0x156
+#define CSR_SIREG6 0x157
+
/* Supervisor-Level Interrupts (AIA) */
#define CSR_STOPEI 0x15c
#define CSR_STOPI 0xdb0
@@ -291,6 +305,13 @@
#define CSR_VSISELECT 0x250
#define CSR_VSIREG 0x251
+/* Virtual Supervisor Indirect Alias */
+#define CSR_VSIREG2 0x252
+#define CSR_VSIREG3 0x253
+#define CSR_VSIREG4 0x255
+#define CSR_VSIREG5 0x256
+#define CSR_VSIREG6 0x257
+
/* VS-Level Interrupts (H-extension with AIA) */
#define CSR_VSTOPEI 0x25c
#define CSR_VSTOPI 0xeb0
@@ -847,10 +868,13 @@ typedef enum RISCVException {
#define ISELECT_IMSIC_EIE63 0xff
#define ISELECT_IMSIC_FIRST ISELECT_IMSIC_EIDELIVERY
#define ISELECT_IMSIC_LAST ISELECT_IMSIC_EIE63
-#define ISELECT_MASK 0x1ff
+#define ISELECT_MASK_AIA 0x1ff
+
+/* MISELECT, SISELECT, and VSISELECT bits (AIA) */
+#define ISELECT_MASK_SXCSRIND 0xfff
/* Dummy [M|S|VS]ISELECT value for emulating [M|S|VS]TOPEI CSRs */
-#define ISELECT_IMSIC_TOPEI (ISELECT_MASK + 1)
+#define ISELECT_IMSIC_TOPEI (ISELECT_MASK_AIA + 1)
/* IMSIC bits (AIA) */
#define IMSIC_TOPEI_IID_SHIFT 16
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 89a1325a02a5..a1c10f1d010a 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -287,6 +287,17 @@ static int aia_any32(CPURISCVState *env, int csrno)
return any32(env, csrno);
}
+static RISCVException sxcsrind_any(CPURISCVState *env, int csrno)
+{
+ RISCVCPU *cpu = env_archcpu(env);
+
+ if (!cpu->cfg.ext_smcsrind) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ return RISCV_EXCP_NONE;
+}
+
static int sxcsrind_or_aia_any(CPURISCVState *env, int csrno)
{
if (!riscv_cpu_cfg(env)->ext_smaia && !riscv_cpu_cfg(env)->ext_smcsrind) {
@@ -355,6 +366,17 @@ static int aia_smode32(CPURISCVState *env, int csrno)
return smode32(env, csrno);
}
+static RISCVException sxcsrind_smode(CPURISCVState *env, int csrno)
+{
+ RISCVCPU *cpu = env_archcpu(env);
+
+ if (!cpu->cfg.ext_sscsrind) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ return smode(env, csrno);
+}
+
static int sxcsrind_or_aia_smode(CPURISCVState *env, int csrno)
{
if (!riscv_cpu_cfg(env)->ext_ssaia && !riscv_cpu_cfg(env)->ext_sscsrind) {
@@ -383,6 +405,17 @@ static RISCVException hmode32(CPURISCVState *env, int csrno)
}
+static RISCVException sxcsrind_hmode(CPURISCVState *env, int csrno)
+{
+ RISCVCPU *cpu = env_archcpu(env);
+
+ if (!cpu->cfg.ext_sscsrind) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ return hmode(env, csrno);
+}
+
static int sxcsrind_or_aia_hmode(CPURISCVState *env, int csrno)
{
if (!riscv_cpu_cfg(env)->ext_ssaia && !riscv_cpu_cfg(env)->ext_sscsrind) {
@@ -1926,7 +1959,12 @@ static int rmw_xiselect(CPURISCVState *env, int csrno, target_ulong *val,
*val = *iselect;
}
- wr_mask &= ISELECT_MASK;
+ if (riscv_cpu_cfg(env)->ext_smcsrind || riscv_cpu_cfg(env)->ext_sscsrind) {
+ wr_mask &= ISELECT_MASK_SXCSRIND;
+ } else {
+ wr_mask &= ISELECT_MASK_AIA;
+ }
+
if (wr_mask) {
*iselect = (*iselect & ~wr_mask) | (new_val & wr_mask);
}
@@ -2065,6 +2103,59 @@ done:
return RISCV_EXCP_NONE;
}
+/*
+ * rmw_xireg_sxcsrind: Perform indirect access to xireg and xireg2-xireg6
+ *
+ * Perform indirect access to xireg and xireg2-xireg6.
+ * This is a generic interface for all xireg CSRs. Apart from AIA, all other
+ * extension using sxcsrind should be implemented here.
+ */
+static int rmw_xireg_sxcsrind(CPURISCVState *env, int csrno,
+ target_ulong isel, target_ulong *val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ return -EINVAL;
+}
+
+static int rmw_xiregi(CPURISCVState *env, int csrno, target_ulong *val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ bool virt = false;
+ int ret = -EINVAL;
+ target_ulong isel;
+
+ ret = smstateen_acc_ok(env, 0, SMSTATEEN0_SVSLCT);
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
+ /* Translate CSR number for VS-mode */
+ csrno = sxcsrind_xlate_vs_csrno(env, csrno);
+
+ if (CSR_MIREG <= csrno && csrno <= CSR_MIREG6 &&
+ csrno != CSR_MIREG4 - 1) {
+ isel = env->miselect;
+ } else if (CSR_SIREG <= csrno && csrno <= CSR_SIREG6 &&
+ csrno != CSR_SIREG4 - 1) {
+ isel = env->siselect;
+ } else if (CSR_VSIREG <= csrno && csrno <= CSR_VSIREG6 &&
+ csrno != CSR_VSIREG4 - 1) {
+ isel = env->vsiselect;
+ virt = true;
+ } else {
+ goto done;
+ }
+
+ return rmw_xireg_sxcsrind(env, csrno, isel, val, new_val, wr_mask);
+
+done:
+ if (ret) {
+ return (env->virt_enabled && virt) ?
+ RISCV_EXCP_VIRT_INSTRUCTION_FAULT : RISCV_EXCP_ILLEGAL_INST;
+ }
+ return RISCV_EXCP_NONE;
+}
+
static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
target_ulong new_val, target_ulong wr_mask)
{
@@ -2096,8 +2187,21 @@ static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
goto done;
};
+ /*
+ * Use the xiselect range to determine actual op on xireg.
+ *
+ * Since we only checked the existence of AIA or Indirect Access in the
+ * predicate, we should check the existence of the exact extension when
+ * we get to a specific range and return illegal instruction exception even
+ * in VS-mode.
+ */
if (xiselect_aia_range(isel)) {
return rmw_xireg_aia(env, csrno, isel, val, new_val, wr_mask);
+ } else if (riscv_cpu_cfg(env)->ext_smcsrind ||
+ riscv_cpu_cfg(env)->ext_sscsrind) {
+ return rmw_xireg_sxcsrind(env, csrno, isel, val, new_val, wr_mask);
+ } else {
+ return RISCV_EXCP_ILLEGAL_INST;
}
done:
@@ -2480,7 +2584,7 @@ static RISCVException write_mstateen0(CPURISCVState *env, int csrno,
* TODO: Do we need to check ssaia as well ? Can we enable ssaia without
* smaia ?
*/
- if (riscv_cpu_cfg(env)->ext_smaia) {
+ if (riscv_cpu_cfg(env)->ext_smaia || riscv_cpu_cfg(env)->ext_smcsrind) {
wr_mask |= SMSTATEEN0_SVSLCT;
}
@@ -2569,7 +2673,7 @@ static RISCVException write_hstateen0(CPURISCVState *env, int csrno,
wr_mask |= SMSTATEEN0_FCSR;
}
- if (riscv_cpu_cfg(env)->ext_ssaia) {
+ if (riscv_cpu_cfg(env)->ext_ssaia || riscv_cpu_cfg(env)->ext_sscsrind) {
wr_mask |= SMSTATEEN0_SVSLCT;
}
@@ -4866,6 +4970,18 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_MIREG] = { "mireg", sxcsrind_or_aia_any, NULL, NULL,
rmw_xireg },
+ /* Machine Indirect Register Alias */
+ [CSR_MIREG2] = { "mireg2", sxcsrind_any, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+ [CSR_MIREG3] = { "mireg3", sxcsrind_any, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+ [CSR_MIREG4] = { "mireg4", sxcsrind_any, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+ [CSR_MIREG5] = { "mireg5", sxcsrind_any, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+ [CSR_MIREG6] = { "mireg6", sxcsrind_any, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+
/* Machine-Level Interrupts (AIA) */
[CSR_MTOPEI] = { "mtopei", aia_any, NULL, NULL, rmw_xtopei },
[CSR_MTOPI] = { "mtopi", aia_any, read_mtopi },
@@ -4987,6 +5103,18 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_SIREG] = { "sireg", sxcsrind_or_aia_smode, NULL, NULL,
rmw_xireg },
+ /* Supervisor Indirect Register Alias */
+ [CSR_SIREG2] = { "sireg2", sxcsrind_smode, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+ [CSR_SIREG3] = { "sireg3", sxcsrind_smode, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+ [CSR_SIREG4] = { "sireg4", sxcsrind_smode, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+ [CSR_SIREG5] = { "sireg5", sxcsrind_smode, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+ [CSR_SIREG6] = { "sireg6", sxcsrind_smode, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+
/* Supervisor-Level Interrupts (AIA) */
[CSR_STOPEI] = { "stopei", aia_smode, NULL, NULL, rmw_xtopei },
[CSR_STOPI] = { "stopi", aia_smode, read_stopi },
@@ -5069,6 +5197,18 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_VSIREG] = { "vsireg", sxcsrind_or_aia_hmode, NULL, NULL,
rmw_xireg },
+ /* Virtual Supervisor Indirect Alias */
+ [CSR_VSIREG2] = { "vsireg2", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+ [CSR_VSIREG3] = { "vsireg3", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+ [CSR_VSIREG4] = { "vsireg4", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+ [CSR_VSIREG5] = { "vsireg5", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+ [CSR_VSIREG6] = { "vsireg6", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+
/* VS-Level Interrupts (H-extension with AIA) */
[CSR_VSTOPEI] = { "vstopei", aia_hmode, NULL, NULL, rmw_xtopei },
[CSR_VSTOPI] = { "vstopi", aia_hmode, read_vstopi },
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH RFC 4/8] target/riscv: Support generic CSR indirect access
2024-02-17 0:01 ` [PATCH RFC 4/8] target/riscv: Support generic CSR indirect access Atish Patra
@ 2024-06-05 11:49 ` Jason Chien
2024-07-23 23:31 ` Atish Kumar Patra
0 siblings, 1 reply; 19+ messages in thread
From: Jason Chien @ 2024-06-05 11:49 UTC (permalink / raw)
To: Atish Patra, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1
The predicate functions should contain the access control by the
state-enable CSRs, which is not presented in this patch. Do you mind
that I take over the indirect CSR access control part? The Signed-off-by
will be kept.
Atish Patra 於 2024/2/17 上午 08:01 寫道:
> From: Kaiwen Xue <kaiwenx@rivosinc.com>
>
> This adds the indirect access registers required by sscsrind/smcsrind
> and the operations on them. Note that xiselect and xireg are used for
> both AIA and sxcsrind, and the behavior of accessing them depends on
> whether each extension is enabled and the value stored in xiselect.
>
> Co-developed-by: Atish Patra <atishp@rivosinc.com>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
> ---
> target/riscv/cpu_bits.h | 28 +++++++-
> target/riscv/csr.c | 146 +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 169 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 0ee91e502e8f..3a66f83009b5 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -176,6 +176,13 @@
> #define CSR_MISELECT 0x350
> #define CSR_MIREG 0x351
>
> +/* Machine Indirect Register Alias */
> +#define CSR_MIREG2 0x352
> +#define CSR_MIREG3 0x353
> +#define CSR_MIREG4 0x355
> +#define CSR_MIREG5 0x356
> +#define CSR_MIREG6 0x357
> +
> /* Machine-Level Interrupts (AIA) */
> #define CSR_MTOPEI 0x35c
> #define CSR_MTOPI 0xfb0
> @@ -225,6 +232,13 @@
> #define CSR_SISELECT 0x150
> #define CSR_SIREG 0x151
>
> +/* Supervisor Indirect Register Alias */
> +#define CSR_SIREG2 0x152
> +#define CSR_SIREG3 0x153
> +#define CSR_SIREG4 0x155
> +#define CSR_SIREG5 0x156
> +#define CSR_SIREG6 0x157
> +
> /* Supervisor-Level Interrupts (AIA) */
> #define CSR_STOPEI 0x15c
> #define CSR_STOPI 0xdb0
> @@ -291,6 +305,13 @@
> #define CSR_VSISELECT 0x250
> #define CSR_VSIREG 0x251
>
> +/* Virtual Supervisor Indirect Alias */
> +#define CSR_VSIREG2 0x252
> +#define CSR_VSIREG3 0x253
> +#define CSR_VSIREG4 0x255
> +#define CSR_VSIREG5 0x256
> +#define CSR_VSIREG6 0x257
> +
> /* VS-Level Interrupts (H-extension with AIA) */
> #define CSR_VSTOPEI 0x25c
> #define CSR_VSTOPI 0xeb0
> @@ -847,10 +868,13 @@ typedef enum RISCVException {
> #define ISELECT_IMSIC_EIE63 0xff
> #define ISELECT_IMSIC_FIRST ISELECT_IMSIC_EIDELIVERY
> #define ISELECT_IMSIC_LAST ISELECT_IMSIC_EIE63
> -#define ISELECT_MASK 0x1ff
> +#define ISELECT_MASK_AIA 0x1ff
> +
> +/* MISELECT, SISELECT, and VSISELECT bits (AIA) */
> +#define ISELECT_MASK_SXCSRIND 0xfff
Could you rename ISELECT_MASK_SXCSRIND to ISELECT_MASK_CSRIND to keep
the naming consistency with ISELECT_MASK_AIA?
>
> /* Dummy [M|S|VS]ISELECT value for emulating [M|S|VS]TOPEI CSRs */
> -#define ISELECT_IMSIC_TOPEI (ISELECT_MASK + 1)
> +#define ISELECT_IMSIC_TOPEI (ISELECT_MASK_AIA + 1)
>
> /* IMSIC bits (AIA) */
> #define IMSIC_TOPEI_IID_SHIFT 16
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 89a1325a02a5..a1c10f1d010a 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -287,6 +287,17 @@ static int aia_any32(CPURISCVState *env, int csrno)
> return any32(env, csrno);
> }
>
> +static RISCVException sxcsrind_any(CPURISCVState *env, int csrno)
Could you rename sxcsrind_any() to csrind_any() to keep naming
consistency with aia_any()?
> +{
> + RISCVCPU *cpu = env_archcpu(env);
> +
> + if (!cpu->cfg.ext_smcsrind) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> + return RISCV_EXCP_NONE;
> +}
> +
> static int sxcsrind_or_aia_any(CPURISCVState *env, int csrno)
> {
> if (!riscv_cpu_cfg(env)->ext_smaia && !riscv_cpu_cfg(env)->ext_smcsrind) {
> @@ -355,6 +366,17 @@ static int aia_smode32(CPURISCVState *env, int csrno)
> return smode32(env, csrno);
> }
>
> +static RISCVException sxcsrind_smode(CPURISCVState *env, int csrno)
Could you rename sxcsrind_smode() to csrind_smode() to keep naming
consistency with aia_smode()?
> +{
> + RISCVCPU *cpu = env_archcpu(env);
> +
> + if (!cpu->cfg.ext_sscsrind) {
S-mode CSRs are defined in Smcsrind as well. If both Smcsrind and
Sscsrind are disabled, return RISCV_EXCP_ILLEGAL_INST.
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
A virtual instruction exception should be raised here for attempts from
VU-mode to access siselect or sireg*.
> + return smode(env, csrno);
> +}
> +
> static int sxcsrind_or_aia_smode(CPURISCVState *env, int csrno)
> {
> if (!riscv_cpu_cfg(env)->ext_ssaia && !riscv_cpu_cfg(env)->ext_sscsrind) {
> @@ -383,6 +405,17 @@ static RISCVException hmode32(CPURISCVState *env, int csrno)
>
> }
>
> +static RISCVException sxcsrind_hmode(CPURISCVState *env, int csrno)
Could you rename sxcsrind_hmode() to csrind_hmode() to keep naming
consistency with aia_hmode()?
> +{
> + RISCVCPU *cpu = env_archcpu(env);
> +
> + if (!cpu->cfg.ext_sscsrind) {
VS-mode CSRs are defined in Smcsrind as well. If both Smcsrind and
Sscsrind are disabled, return RISCV_EXCP_ILLEGAL_INST.
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> + return hmode(env, csrno);
> +}
> +
> static int sxcsrind_or_aia_hmode(CPURISCVState *env, int csrno)
> {
> if (!riscv_cpu_cfg(env)->ext_ssaia && !riscv_cpu_cfg(env)->ext_sscsrind) {
> @@ -1926,7 +1959,12 @@ static int rmw_xiselect(CPURISCVState *env, int csrno, target_ulong *val,
> *val = *iselect;
> }
>
> - wr_mask &= ISELECT_MASK;
> + if (riscv_cpu_cfg(env)->ext_smcsrind || riscv_cpu_cfg(env)->ext_sscsrind) {
> + wr_mask &= ISELECT_MASK_SXCSRIND;
> + } else {
> + wr_mask &= ISELECT_MASK_AIA;
> + }
> +
> if (wr_mask) {
> *iselect = (*iselect & ~wr_mask) | (new_val & wr_mask);
> }
> @@ -2065,6 +2103,59 @@ done:
> return RISCV_EXCP_NONE;
> }
>
> +/*
> + * rmw_xireg_sxcsrind: Perform indirect access to xireg and xireg2-xireg6
> + *
> + * Perform indirect access to xireg and xireg2-xireg6.
> + * This is a generic interface for all xireg CSRs. Apart from AIA, all other
> + * extension using sxcsrind should be implemented here.
> + */
> +static int rmw_xireg_sxcsrind(CPURISCVState *env, int csrno,
> + target_ulong isel, target_ulong *val,
> + target_ulong new_val, target_ulong wr_mask)
Could you rename rmw_xireg_sxcsrind() to rmw_xireg_csrind() to keep the
naming consistency with rmw_xireg_aia()?
> +{
> + return -EINVAL;
> +}
> +
> +static int rmw_xiregi(CPURISCVState *env, int csrno, target_ulong *val,
> + target_ulong new_val, target_ulong wr_mask)
> +{
> + bool virt = false;
> + int ret = -EINVAL;
> + target_ulong isel;
> +
> + ret = smstateen_acc_ok(env, 0, SMSTATEEN0_SVSLCT);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
> +
> + /* Translate CSR number for VS-mode */
> + csrno = sxcsrind_xlate_vs_csrno(env, csrno);
> +
> + if (CSR_MIREG <= csrno && csrno <= CSR_MIREG6 &&
> + csrno != CSR_MIREG4 - 1) {
> + isel = env->miselect;
> + } else if (CSR_SIREG <= csrno && csrno <= CSR_SIREG6 &&
> + csrno != CSR_SIREG4 - 1) {
> + isel = env->siselect;
> + } else if (CSR_VSIREG <= csrno && csrno <= CSR_VSIREG6 &&
> + csrno != CSR_VSIREG4 - 1) {
> + isel = env->vsiselect;
> + virt = true;
> + } else {
> + goto done;
> + }
> +
> + return rmw_xireg_sxcsrind(env, csrno, isel, val, new_val, wr_mask);
> +
> +done:
> + if (ret) {
> + return (env->virt_enabled && virt) ?
A virtual instruction exception is raised for attempts from VS-mode or
VU-mode to directly access vsiselect or vsireg* in riscv_csrrw_check(),
we don't need to check again here.
> + RISCV_EXCP_VIRT_INSTRUCTION_FAULT : RISCV_EXCP_ILLEGAL_INST;
> + }
> + return RISCV_EXCP_NONE;
> +}
> +
> static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
> target_ulong new_val, target_ulong wr_mask)
> {
> @@ -2096,8 +2187,21 @@ static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
> goto done;
> };
>
> + /*
> + * Use the xiselect range to determine actual op on xireg.
> + *
> + * Since we only checked the existence of AIA or Indirect Access in the
> + * predicate, we should check the existence of the exact extension when
> + * we get to a specific range and return illegal instruction exception even
> + * in VS-mode.
> + */
> if (xiselect_aia_range(isel)) {
> return rmw_xireg_aia(env, csrno, isel, val, new_val, wr_mask);
> + } else if (riscv_cpu_cfg(env)->ext_smcsrind ||
> + riscv_cpu_cfg(env)->ext_sscsrind) {
> + return rmw_xireg_sxcsrind(env, csrno, isel, val, new_val, wr_mask);
> + } else {
> + return RISCV_EXCP_ILLEGAL_INST;
> }
>
> done:
> @@ -2480,7 +2584,7 @@ static RISCVException write_mstateen0(CPURISCVState *env, int csrno,
> * TODO: Do we need to check ssaia as well ? Can we enable ssaia without
> * smaia ?
> */
> - if (riscv_cpu_cfg(env)->ext_smaia) {
> + if (riscv_cpu_cfg(env)->ext_smaia || riscv_cpu_cfg(env)->ext_smcsrind) {
> wr_mask |= SMSTATEEN0_SVSLCT;
> }
>
> @@ -2569,7 +2673,7 @@ static RISCVException write_hstateen0(CPURISCVState *env, int csrno,
> wr_mask |= SMSTATEEN0_FCSR;
> }
>
> - if (riscv_cpu_cfg(env)->ext_ssaia) {
> + if (riscv_cpu_cfg(env)->ext_ssaia || riscv_cpu_cfg(env)->ext_sscsrind) {
> wr_mask |= SMSTATEEN0_SVSLCT;
> }
>
> @@ -4866,6 +4970,18 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_MIREG] = { "mireg", sxcsrind_or_aia_any, NULL, NULL,
> rmw_xireg },
>
> + /* Machine Indirect Register Alias */
> + [CSR_MIREG2] = { "mireg2", sxcsrind_any, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> + [CSR_MIREG3] = { "mireg3", sxcsrind_any, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> + [CSR_MIREG4] = { "mireg4", sxcsrind_any, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> + [CSR_MIREG5] = { "mireg5", sxcsrind_any, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> + [CSR_MIREG6] = { "mireg6", sxcsrind_any, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> +
> /* Machine-Level Interrupts (AIA) */
> [CSR_MTOPEI] = { "mtopei", aia_any, NULL, NULL, rmw_xtopei },
> [CSR_MTOPI] = { "mtopi", aia_any, read_mtopi },
> @@ -4987,6 +5103,18 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_SIREG] = { "sireg", sxcsrind_or_aia_smode, NULL, NULL,
> rmw_xireg },
>
> + /* Supervisor Indirect Register Alias */
> + [CSR_SIREG2] = { "sireg2", sxcsrind_smode, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> + [CSR_SIREG3] = { "sireg3", sxcsrind_smode, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> + [CSR_SIREG4] = { "sireg4", sxcsrind_smode, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> + [CSR_SIREG5] = { "sireg5", sxcsrind_smode, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> + [CSR_SIREG6] = { "sireg6", sxcsrind_smode, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> +
> /* Supervisor-Level Interrupts (AIA) */
> [CSR_STOPEI] = { "stopei", aia_smode, NULL, NULL, rmw_xtopei },
> [CSR_STOPI] = { "stopi", aia_smode, read_stopi },
> @@ -5069,6 +5197,18 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_VSIREG] = { "vsireg", sxcsrind_or_aia_hmode, NULL, NULL,
> rmw_xireg },
>
> + /* Virtual Supervisor Indirect Alias */
> + [CSR_VSIREG2] = { "vsireg2", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> + [CSR_VSIREG3] = { "vsireg3", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> + [CSR_VSIREG4] = { "vsireg4", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> + [CSR_VSIREG5] = { "vsireg5", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> + [CSR_VSIREG6] = { "vsireg6", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
> + .min_priv_ver = PRIV_VERSION_1_12_0 },
> +
> /* VS-Level Interrupts (H-extension with AIA) */
> [CSR_VSTOPEI] = { "vstopei", aia_hmode, NULL, NULL, rmw_xtopei },
> [CSR_VSTOPI] = { "vstopi", aia_hmode, read_vstopi },
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH RFC 4/8] target/riscv: Support generic CSR indirect access
2024-06-05 11:49 ` Jason Chien
@ 2024-07-23 23:31 ` Atish Kumar Patra
0 siblings, 0 replies; 19+ messages in thread
From: Atish Kumar Patra @ 2024-07-23 23:31 UTC (permalink / raw)
To: Jason Chien
Cc: qemu-devel, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1
On Wed, Jun 5, 2024 at 4:49 AM Jason Chien <jason.chien@sifive.com> wrote:
>
> The predicate functions should contain the access control by the
> state-enable CSRs, which is not presented in this patch. Do you mind
> that I take over the indirect CSR access control part? The Signed-off-by
> will be kept.
>
> Atish Patra 於 2024/2/17 上午 08:01 寫道:
> > From: Kaiwen Xue <kaiwenx@rivosinc.com>
> >
> > This adds the indirect access registers required by sscsrind/smcsrind
> > and the operations on them. Note that xiselect and xireg are used for
> > both AIA and sxcsrind, and the behavior of accessing them depends on
> > whether each extension is enabled and the value stored in xiselect.
> >
> > Co-developed-by: Atish Patra <atishp@rivosinc.com>
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
> > ---
> > target/riscv/cpu_bits.h | 28 +++++++-
> > target/riscv/csr.c | 146 +++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 169 insertions(+), 5 deletions(-)
> >
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index 0ee91e502e8f..3a66f83009b5 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -176,6 +176,13 @@
> > #define CSR_MISELECT 0x350
> > #define CSR_MIREG 0x351
> >
> > +/* Machine Indirect Register Alias */
> > +#define CSR_MIREG2 0x352
> > +#define CSR_MIREG3 0x353
> > +#define CSR_MIREG4 0x355
> > +#define CSR_MIREG5 0x356
> > +#define CSR_MIREG6 0x357
> > +
> > /* Machine-Level Interrupts (AIA) */
> > #define CSR_MTOPEI 0x35c
> > #define CSR_MTOPI 0xfb0
> > @@ -225,6 +232,13 @@
> > #define CSR_SISELECT 0x150
> > #define CSR_SIREG 0x151
> >
> > +/* Supervisor Indirect Register Alias */
> > +#define CSR_SIREG2 0x152
> > +#define CSR_SIREG3 0x153
> > +#define CSR_SIREG4 0x155
> > +#define CSR_SIREG5 0x156
> > +#define CSR_SIREG6 0x157
> > +
> > /* Supervisor-Level Interrupts (AIA) */
> > #define CSR_STOPEI 0x15c
> > #define CSR_STOPI 0xdb0
> > @@ -291,6 +305,13 @@
> > #define CSR_VSISELECT 0x250
> > #define CSR_VSIREG 0x251
> >
> > +/* Virtual Supervisor Indirect Alias */
> > +#define CSR_VSIREG2 0x252
> > +#define CSR_VSIREG3 0x253
> > +#define CSR_VSIREG4 0x255
> > +#define CSR_VSIREG5 0x256
> > +#define CSR_VSIREG6 0x257
> > +
> > /* VS-Level Interrupts (H-extension with AIA) */
> > #define CSR_VSTOPEI 0x25c
> > #define CSR_VSTOPI 0xeb0
> > @@ -847,10 +868,13 @@ typedef enum RISCVException {
> > #define ISELECT_IMSIC_EIE63 0xff
> > #define ISELECT_IMSIC_FIRST ISELECT_IMSIC_EIDELIVERY
> > #define ISELECT_IMSIC_LAST ISELECT_IMSIC_EIE63
> > -#define ISELECT_MASK 0x1ff
> > +#define ISELECT_MASK_AIA 0x1ff
> > +
> > +/* MISELECT, SISELECT, and VSISELECT bits (AIA) */
> > +#define ISELECT_MASK_SXCSRIND 0xfff
> Could you rename ISELECT_MASK_SXCSRIND to ISELECT_MASK_CSRIND to keep
> the naming consistency with ISELECT_MASK_AIA?
> >
> > /* Dummy [M|S|VS]ISELECT value for emulating [M|S|VS]TOPEI CSRs */
> > -#define ISELECT_IMSIC_TOPEI (ISELECT_MASK + 1)
> > +#define ISELECT_IMSIC_TOPEI (ISELECT_MASK_AIA + 1)
> >
> > /* IMSIC bits (AIA) */
> > #define IMSIC_TOPEI_IID_SHIFT 16
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 89a1325a02a5..a1c10f1d010a 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -287,6 +287,17 @@ static int aia_any32(CPURISCVState *env, int csrno)
> > return any32(env, csrno);
> > }
> >
> > +static RISCVException sxcsrind_any(CPURISCVState *env, int csrno)
> Could you rename sxcsrind_any() to csrind_any() to keep naming
> consistency with aia_any()?
> > +{
> > + RISCVCPU *cpu = env_archcpu(env);
> > +
> > + if (!cpu->cfg.ext_smcsrind) {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > +
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > static int sxcsrind_or_aia_any(CPURISCVState *env, int csrno)
> > {
> > if (!riscv_cpu_cfg(env)->ext_smaia && !riscv_cpu_cfg(env)->ext_smcsrind) {
> > @@ -355,6 +366,17 @@ static int aia_smode32(CPURISCVState *env, int csrno)
> > return smode32(env, csrno);
> > }
> >
> > +static RISCVException sxcsrind_smode(CPURISCVState *env, int csrno)
> Could you rename sxcsrind_smode() to csrind_smode() to keep naming
> consistency with aia_smode()?
> > +{
> > + RISCVCPU *cpu = env_archcpu(env);
> > +
> > + if (!cpu->cfg.ext_sscsrind) {
> S-mode CSRs are defined in Smcsrind as well. If both Smcsrind and
> Sscsrind are disabled, return RISCV_EXCP_ILLEGAL_INST.
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > +
> A virtual instruction exception should be raised here for attempts from
> VU-mode to access siselect or sireg*.
That should be covered in riscv_csrrw_check.
> > + return smode(env, csrno);
> > +}
> > +
> > static int sxcsrind_or_aia_smode(CPURISCVState *env, int csrno)
> > {
> > if (!riscv_cpu_cfg(env)->ext_ssaia && !riscv_cpu_cfg(env)->ext_sscsrind) {
> > @@ -383,6 +405,17 @@ static RISCVException hmode32(CPURISCVState *env, int csrno)
> >
> > }
> >
> > +static RISCVException sxcsrind_hmode(CPURISCVState *env, int csrno)
> Could you rename sxcsrind_hmode() to csrind_hmode() to keep naming
> consistency with aia_hmode()?
> > +{
> > + RISCVCPU *cpu = env_archcpu(env);
> > +
> > + if (!cpu->cfg.ext_sscsrind) {
> VS-mode CSRs are defined in Smcsrind as well. If both Smcsrind and
> Sscsrind are disabled, return RISCV_EXCP_ILLEGAL_INST.
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > +
> > + return hmode(env, csrno);
> > +}
> > +
> > static int sxcsrind_or_aia_hmode(CPURISCVState *env, int csrno)
> > {
> > if (!riscv_cpu_cfg(env)->ext_ssaia && !riscv_cpu_cfg(env)->ext_sscsrind) {
> > @@ -1926,7 +1959,12 @@ static int rmw_xiselect(CPURISCVState *env, int csrno, target_ulong *val,
> > *val = *iselect;
> > }
> >
> > - wr_mask &= ISELECT_MASK;
> > + if (riscv_cpu_cfg(env)->ext_smcsrind || riscv_cpu_cfg(env)->ext_sscsrind) {
> > + wr_mask &= ISELECT_MASK_SXCSRIND;
> > + } else {
> > + wr_mask &= ISELECT_MASK_AIA;
> > + }
> > +
> > if (wr_mask) {
> > *iselect = (*iselect & ~wr_mask) | (new_val & wr_mask);
> > }
> > @@ -2065,6 +2103,59 @@ done:
> > return RISCV_EXCP_NONE;
> > }
> >
> > +/*
> > + * rmw_xireg_sxcsrind: Perform indirect access to xireg and xireg2-xireg6
> > + *
> > + * Perform indirect access to xireg and xireg2-xireg6.
> > + * This is a generic interface for all xireg CSRs. Apart from AIA, all other
> > + * extension using sxcsrind should be implemented here.
> > + */
> > +static int rmw_xireg_sxcsrind(CPURISCVState *env, int csrno,
> > + target_ulong isel, target_ulong *val,
> > + target_ulong new_val, target_ulong wr_mask)
> Could you rename rmw_xireg_sxcsrind() to rmw_xireg_csrind() to keep the
> naming consistency with rmw_xireg_aia()?
> > +{
> > + return -EINVAL;
> > +}
> > +
> > +static int rmw_xiregi(CPURISCVState *env, int csrno, target_ulong *val,
> > + target_ulong new_val, target_ulong wr_mask)
> > +{
> > + bool virt = false;
> > + int ret = -EINVAL;
> > + target_ulong isel;
> > +
> > + ret = smstateen_acc_ok(env, 0, SMSTATEEN0_SVSLCT);
> > + if (ret != RISCV_EXCP_NONE) {
> > + return ret;
> > + }
> > +
> > + /* Translate CSR number for VS-mode */
> > + csrno = sxcsrind_xlate_vs_csrno(env, csrno);
> > +
> > + if (CSR_MIREG <= csrno && csrno <= CSR_MIREG6 &&
> > + csrno != CSR_MIREG4 - 1) {
> > + isel = env->miselect;
> > + } else if (CSR_SIREG <= csrno && csrno <= CSR_SIREG6 &&
> > + csrno != CSR_SIREG4 - 1) {
> > + isel = env->siselect;
> > + } else if (CSR_VSIREG <= csrno && csrno <= CSR_VSIREG6 &&
> > + csrno != CSR_VSIREG4 - 1) {
> > + isel = env->vsiselect;
> > + virt = true;
> > + } else {
> > + goto done;
> > + }
> > +
> > + return rmw_xireg_sxcsrind(env, csrno, isel, val, new_val, wr_mask);
> > +
> > +done:
> > + if (ret) {
> > + return (env->virt_enabled && virt) ?
> A virtual instruction exception is raised for attempts from VS-mode or
> VU-mode to directly access vsiselect or vsireg* in riscv_csrrw_check(),
> we don't need to check again here.
The check is for invalid value while accessing siregi variants.
Thanks for the review. Addressed all other suggestions in v2.
> > + RISCV_EXCP_VIRT_INSTRUCTION_FAULT : RISCV_EXCP_ILLEGAL_INST;
> > + }
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
> > target_ulong new_val, target_ulong wr_mask)
> > {
> > @@ -2096,8 +2187,21 @@ static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
> > goto done;
> > };
> >
> > + /*
> > + * Use the xiselect range to determine actual op on xireg.
> > + *
> > + * Since we only checked the existence of AIA or Indirect Access in the
> > + * predicate, we should check the existence of the exact extension when
> > + * we get to a specific range and return illegal instruction exception even
> > + * in VS-mode.
> > + */
> > if (xiselect_aia_range(isel)) {
> > return rmw_xireg_aia(env, csrno, isel, val, new_val, wr_mask);
> > + } else if (riscv_cpu_cfg(env)->ext_smcsrind ||
> > + riscv_cpu_cfg(env)->ext_sscsrind) {
> > + return rmw_xireg_sxcsrind(env, csrno, isel, val, new_val, wr_mask);
> > + } else {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > }
> >
> > done:
> > @@ -2480,7 +2584,7 @@ static RISCVException write_mstateen0(CPURISCVState *env, int csrno,
> > * TODO: Do we need to check ssaia as well ? Can we enable ssaia without
> > * smaia ?
> > */
> > - if (riscv_cpu_cfg(env)->ext_smaia) {
> > + if (riscv_cpu_cfg(env)->ext_smaia || riscv_cpu_cfg(env)->ext_smcsrind) {
> > wr_mask |= SMSTATEEN0_SVSLCT;
> > }
> >
> > @@ -2569,7 +2673,7 @@ static RISCVException write_hstateen0(CPURISCVState *env, int csrno,
> > wr_mask |= SMSTATEEN0_FCSR;
> > }
> >
> > - if (riscv_cpu_cfg(env)->ext_ssaia) {
> > + if (riscv_cpu_cfg(env)->ext_ssaia || riscv_cpu_cfg(env)->ext_sscsrind) {
> > wr_mask |= SMSTATEEN0_SVSLCT;
> > }
> >
> > @@ -4866,6 +4970,18 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> > [CSR_MIREG] = { "mireg", sxcsrind_or_aia_any, NULL, NULL,
> > rmw_xireg },
> >
> > + /* Machine Indirect Register Alias */
> > + [CSR_MIREG2] = { "mireg2", sxcsrind_any, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > + [CSR_MIREG3] = { "mireg3", sxcsrind_any, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > + [CSR_MIREG4] = { "mireg4", sxcsrind_any, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > + [CSR_MIREG5] = { "mireg5", sxcsrind_any, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > + [CSR_MIREG6] = { "mireg6", sxcsrind_any, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > +
> > /* Machine-Level Interrupts (AIA) */
> > [CSR_MTOPEI] = { "mtopei", aia_any, NULL, NULL, rmw_xtopei },
> > [CSR_MTOPI] = { "mtopi", aia_any, read_mtopi },
> > @@ -4987,6 +5103,18 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> > [CSR_SIREG] = { "sireg", sxcsrind_or_aia_smode, NULL, NULL,
> > rmw_xireg },
> >
> > + /* Supervisor Indirect Register Alias */
> > + [CSR_SIREG2] = { "sireg2", sxcsrind_smode, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > + [CSR_SIREG3] = { "sireg3", sxcsrind_smode, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > + [CSR_SIREG4] = { "sireg4", sxcsrind_smode, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > + [CSR_SIREG5] = { "sireg5", sxcsrind_smode, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > + [CSR_SIREG6] = { "sireg6", sxcsrind_smode, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > +
> > /* Supervisor-Level Interrupts (AIA) */
> > [CSR_STOPEI] = { "stopei", aia_smode, NULL, NULL, rmw_xtopei },
> > [CSR_STOPI] = { "stopi", aia_smode, read_stopi },
> > @@ -5069,6 +5197,18 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> > [CSR_VSIREG] = { "vsireg", sxcsrind_or_aia_hmode, NULL, NULL,
> > rmw_xireg },
> >
> > + /* Virtual Supervisor Indirect Alias */
> > + [CSR_VSIREG2] = { "vsireg2", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > + [CSR_VSIREG3] = { "vsireg3", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > + [CSR_VSIREG4] = { "vsireg4", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > + [CSR_VSIREG5] = { "vsireg5", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > + [CSR_VSIREG6] = { "vsireg6", sxcsrind_hmode, NULL, NULL, rmw_xiregi,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > +
> > /* VS-Level Interrupts (H-extension with AIA) */
> > [CSR_VSTOPEI] = { "vstopei", aia_hmode, NULL, NULL, rmw_xtopei },
> > [CSR_VSTOPI] = { "vstopi", aia_hmode, read_vstopi },
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH RFC 5/8] target/riscv: Add smcdeleg/ssccfg properties
2024-02-17 0:01 [PATCH RFC 0/8] Add Counter delegation ISA extension support Atish Patra
` (3 preceding siblings ...)
2024-02-17 0:01 ` [PATCH RFC 4/8] target/riscv: Support generic CSR indirect access Atish Patra
@ 2024-02-17 0:01 ` Atish Patra
2024-02-17 0:01 ` [PATCH RFC 6/8] target/riscv: Add counter delegation definitions Atish Patra
` (4 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Atish Patra @ 2024-02-17 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Atish Patra, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1
From: Kaiwen Xue <kaiwenx@rivosinc.com>
This adds the properties of smcdeleg/ssccfg. Implementation will be in
future patches.
Signed-off-by: Atish Patra <atishp@rivosinc.com>
Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
---
target/riscv/cpu.c | 4 ++++
target/riscv/cpu_cfg.h | 2 ++
2 files changed, 6 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ff7c6c7c380e..c5ec203fb8fd 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -151,11 +151,13 @@ const RISCVIsaExtData isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx),
ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
+ ISA_EXT_DATA_ENTRY(smcdeleg, PRIV_VERSION_1_12_0, ext_smcdeleg),
ISA_EXT_DATA_ENTRY(smcntrpmf, PRIV_VERSION_1_12_0, ext_smcntrpmf),
ISA_EXT_DATA_ENTRY(smcsrind, PRIV_VERSION_1_12_0, ext_smcsrind),
ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
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(ssccfg, PRIV_VERSION_1_12_0, ext_ssccfg),
ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
ISA_EXT_DATA_ENTRY(sscsrind, PRIV_VERSION_1_12_0, ext_sscsrind),
ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
@@ -1352,6 +1354,8 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
MULTI_EXT_CFG_BOOL("smcntrpmf", ext_smcntrpmf, false),
MULTI_EXT_CFG_BOOL("smcsrind", ext_smcsrind, false),
MULTI_EXT_CFG_BOOL("sscsrind", ext_sscsrind, false),
+ MULTI_EXT_CFG_BOOL("smcdeleg", ext_smcdeleg, false),
+ MULTI_EXT_CFG_BOOL("ssccfg", ext_ssccfg, false),
MULTI_EXT_CFG_BOOL("zifencei", ext_zifencei, true),
MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true),
MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index b9086464752e..a3be34c88ef0 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -76,6 +76,8 @@ struct RISCVCPUConfig {
bool ext_smcntrpmf;
bool ext_smcsrind;
bool ext_sscsrind;
+ bool ext_smcdeleg;
+ bool ext_ssccfg;
bool ext_svadu;
bool ext_svinval;
bool ext_svnapot;
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH RFC 6/8] target/riscv: Add counter delegation definitions
2024-02-17 0:01 [PATCH RFC 0/8] Add Counter delegation ISA extension support Atish Patra
` (4 preceding siblings ...)
2024-02-17 0:01 ` [PATCH RFC 5/8] target/riscv: Add smcdeleg/ssccfg properties Atish Patra
@ 2024-02-17 0:01 ` Atish Patra
2024-02-17 0:01 ` [PATCH RFC 7/8] target/riscv: Add select value range check for counter delegation Atish Patra
` (3 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Atish Patra @ 2024-02-17 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Atish Patra, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1
From: Kaiwen Xue <kaiwenx@rivosinc.com>
This adds definitions for counter delegation, including the new
scountinhibit register and the mstateen.CD bit.
Signed-off-by: Atish Patra <atishp@rivosinc.com>
Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
---
target/riscv/cpu.h | 1 +
target/riscv/cpu_bits.h | 8 +++++++-
target/riscv/machine.c | 1 +
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 85afde48fade..d7dcfdb0e5e0 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -342,6 +342,7 @@ struct CPUArchState {
target_ulong scounteren;
target_ulong mcounteren;
+ target_ulong scountinhibit;
target_ulong mcountinhibit;
/* PMU cycle & instret privilege mode filtering */
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 3a66f83009b5..0bffec3476ab 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -213,6 +213,9 @@
#define CSR_SSTATEEN2 0x10E
#define CSR_SSTATEEN3 0x10F
+/* Supervisor Counter Delegation */
+#define CSR_SCOUNTINHIBIT 0x120
+
/* Supervisor Trap Handling */
#define CSR_SSCRATCH 0x140
#define CSR_SEPC 0x141
@@ -779,6 +782,7 @@ typedef enum RISCVException {
#define MENVCFG_CBIE (3UL << 4)
#define MENVCFG_CBCFE BIT(6)
#define MENVCFG_CBZE BIT(7)
+#define MENVCFG_CDE (1ULL << 60)
#define MENVCFG_ADUE (1ULL << 61)
#define MENVCFG_PBMTE (1ULL << 62)
#define MENVCFG_STCE (1ULL << 63)
@@ -870,7 +874,9 @@ typedef enum RISCVException {
#define ISELECT_IMSIC_LAST ISELECT_IMSIC_EIE63
#define ISELECT_MASK_AIA 0x1ff
-/* MISELECT, SISELECT, and VSISELECT bits (AIA) */
+/* [M|S|VS]SELCT value for Indirect CSR Access Extension */
+#define ISELECT_CD_FIRST 0x40
+#define ISELECT_CD_LAST 0x5f
#define ISELECT_MASK_SXCSRIND 0xfff
/* Dummy [M|S|VS]ISELECT value for emulating [M|S|VS]TOPEI CSRs */
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 72fe2374dc2a..d26742f99ed7 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -400,6 +400,7 @@ const VMStateDescription vmstate_riscv_cpu = {
VMSTATE_UINTTL(env.siselect, RISCVCPU),
VMSTATE_UINTTL(env.scounteren, RISCVCPU),
VMSTATE_UINTTL(env.mcounteren, RISCVCPU),
+ VMSTATE_UINTTL(env.scountinhibit, RISCVCPU),
VMSTATE_UINTTL(env.mcountinhibit, RISCVCPU),
VMSTATE_STRUCT_ARRAY(env.pmu_ctrs, RISCVCPU, RV_MAX_MHPMCOUNTERS, 0,
vmstate_pmu_ctr_state, PMUCTRState),
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH RFC 7/8] target/riscv: Add select value range check for counter delegation
2024-02-17 0:01 [PATCH RFC 0/8] Add Counter delegation ISA extension support Atish Patra
` (5 preceding siblings ...)
2024-02-17 0:01 ` [PATCH RFC 6/8] target/riscv: Add counter delegation definitions Atish Patra
@ 2024-02-17 0:01 ` Atish Patra
2024-02-17 0:01 ` [PATCH RFC 8/8] target/riscv: Add counter delegation/configuration support Atish Patra
` (2 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Atish Patra @ 2024-02-17 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Atish Patra, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1
From: Kaiwen Xue <kaiwenx@rivosinc.com>
This adds checks in ops performed on xireg and xireg2-xireg6 so that the
counter delegation function will receive a valid xiselect value with the
proper extensions enabled.
Co-developed-by: Atish Patra <atishp@rivosinc.com>
Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
target/riscv/csr.c | 36 +++++++++++++++++++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index a1c10f1d010a..d5218a47ffbf 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1978,6 +1978,11 @@ static bool xiselect_aia_range(target_ulong isel)
(ISELECT_IMSIC_FIRST <= isel && isel <= ISELECT_IMSIC_LAST);
}
+static bool xiselect_cd_range(target_ulong isel)
+{
+ return (ISELECT_CD_FIRST <= isel && isel <= ISELECT_CD_LAST);
+}
+
static int rmw_iprio(target_ulong xlen,
target_ulong iselect, uint8_t *iprio,
target_ulong *val, target_ulong new_val,
@@ -2103,6 +2108,17 @@ done:
return RISCV_EXCP_NONE;
}
+static int rmw_xireg_cd(CPURISCVState *env, int csrno,
+ target_ulong isel, target_ulong *val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ if (!riscv_cpu_cfg(env)->ext_smcdeleg) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+ /* TODO: Implement the functionality later */
+ return RISCV_EXCP_NONE;
+}
+
/*
* rmw_xireg_sxcsrind: Perform indirect access to xireg and xireg2-xireg6
*
@@ -2114,7 +2130,25 @@ static int rmw_xireg_sxcsrind(CPURISCVState *env, int csrno,
target_ulong isel, target_ulong *val,
target_ulong new_val, target_ulong wr_mask)
{
- return -EINVAL;
+ int ret = -EINVAL;
+ bool virt = csrno == CSR_VSIREG ? true : false;
+
+ if (xiselect_cd_range(isel)) {
+ ret = rmw_xireg_cd(env, csrno, isel, val, new_val, wr_mask);
+ } else {
+ /*
+ * As per the specification, access to unimplented region is undefined
+ * but recommendation is to raise illegal instruction exception.
+ */
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ if (ret) {
+ return (env->virt_enabled && virt) ?
+ RISCV_EXCP_VIRT_INSTRUCTION_FAULT : RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ return RISCV_EXCP_NONE;
}
static int rmw_xiregi(CPURISCVState *env, int csrno, target_ulong *val,
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH RFC 8/8] target/riscv: Add counter delegation/configuration support
2024-02-17 0:01 [PATCH RFC 0/8] Add Counter delegation ISA extension support Atish Patra
` (6 preceding siblings ...)
2024-02-17 0:01 ` [PATCH RFC 7/8] target/riscv: Add select value range check for counter delegation Atish Patra
@ 2024-02-17 0:01 ` Atish Patra
2024-02-21 14:58 ` [PATCH RFC 0/8] Add Counter delegation ISA extension support Daniel Henrique Barboza
2024-06-01 9:52 ` Daniel Henrique Barboza
9 siblings, 0 replies; 19+ messages in thread
From: Atish Patra @ 2024-02-17 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Atish Patra, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1
From: Kaiwen Xue <kaiwenx@rivosinc.com>
The Smcdeleg/Ssccfg adds the support for counter delegation via
S*indcsr and Ssccfg.
It also adds a new shadow CSR scountinhibit and menvcfg enable bit (CDE)
to enable this extension and scountovf virtualization.
Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
Co-developed-by: Atish Patra <atishp@rivosinc.com>
Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
target/riscv/csr.c | 307 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 294 insertions(+), 13 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d5218a47ffbf..3542c522ba07 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -366,6 +366,21 @@ static int aia_smode32(CPURISCVState *env, int csrno)
return smode32(env, csrno);
}
+static RISCVException scountinhibit_pred(CPURISCVState *env, int csrno)
+{
+ RISCVCPU *cpu = env_archcpu(env);
+
+ if (!cpu->cfg.ext_ssccfg || !cpu->cfg.ext_smcdeleg) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ if (env->virt_enabled) {
+ return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+ }
+
+ return smode(env, csrno);
+}
+
static RISCVException sxcsrind_smode(CPURISCVState *env, int csrno)
{
RISCVCPU *cpu = env_archcpu(env);
@@ -1089,9 +1104,9 @@ done:
return result;
}
-static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val)
+static RISCVException riscv_pmu_write_ctr(CPURISCVState *env, target_ulong val,
+ uint32_t ctr_idx)
{
- int ctr_idx = csrno - CSR_MCYCLE;
PMUCTRState *counter = &env->pmu_ctrs[ctr_idx];
uint64_t mhpmctr_val = val;
@@ -1115,9 +1130,9 @@ static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val)
return RISCV_EXCP_NONE;
}
-static int write_mhpmcounterh(CPURISCVState *env, int csrno, target_ulong val)
+static RISCVException riscv_pmu_write_ctrh(CPURISCVState *env, target_ulong val,
+ uint32_t ctr_idx)
{
- int ctr_idx = csrno - CSR_MCYCLEH;
PMUCTRState *counter = &env->pmu_ctrs[ctr_idx];
uint64_t mhpmctr_val = counter->mhpmcounter_val;
uint64_t mhpmctrh_val = val;
@@ -1138,6 +1153,20 @@ static int write_mhpmcounterh(CPURISCVState *env, int csrno, target_ulong val)
return RISCV_EXCP_NONE;
}
+static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val)
+{
+ int ctr_idx = csrno - CSR_MCYCLE;
+
+ return riscv_pmu_write_ctr(env, val, ctr_idx);
+}
+
+static int write_mhpmcounterh(CPURISCVState *env, int csrno, target_ulong val)
+{
+ int ctr_idx = csrno - CSR_MCYCLEH;
+
+ return riscv_pmu_write_ctrh(env, val, ctr_idx);
+}
+
static RISCVException riscv_pmu_read_ctr(CPURISCVState *env, target_ulong *val,
bool upper_half, uint32_t ctr_idx)
{
@@ -1207,6 +1236,167 @@ static int read_hpmcounterh(CPURISCVState *env, int csrno, target_ulong *val)
return riscv_pmu_read_ctr(env, val, true, ctr_index);
}
+static int rmw_cd_mhpmcounter(CPURISCVState *env, int ctr_idx,
+ target_ulong *val, target_ulong new_val,
+ target_ulong wr_mask)
+{
+ if (wr_mask != 0 && wr_mask != -1) {
+ return -EINVAL;
+ }
+
+ if (!wr_mask && val) {
+ riscv_pmu_read_ctr(env, val, false, ctr_idx);
+ } else if (wr_mask) {
+ riscv_pmu_write_ctr(env, new_val, ctr_idx);
+ } else {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int rmw_cd_mhpmcounterh(CPURISCVState *env, int ctr_idx,
+ target_ulong *val, target_ulong new_val,
+ target_ulong wr_mask)
+{
+ if (wr_mask != 0 && wr_mask != -1) {
+ return -EINVAL;
+ }
+
+ if (!wr_mask && val) {
+ riscv_pmu_read_ctr(env, val, true, ctr_idx);
+ } else if (wr_mask) {
+ riscv_pmu_write_ctrh(env, new_val, ctr_idx);
+ } else {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int rmw_cd_mhpmevent(CPURISCVState *env, int evt_index,
+ target_ulong *val, target_ulong new_val,
+ target_ulong wr_mask)
+{
+ uint64_t mhpmevt_val = new_val;
+
+ if (wr_mask != 0 && wr_mask != -1) {
+ return -EINVAL;
+ }
+
+ if (!wr_mask && val) {
+ *val = env->mhpmevent_val[evt_index];
+ if (riscv_cpu_cfg(env)->ext_sscofpmf) {
+ *val &= ~MHPMEVENT_BIT_MINH;
+ }
+ } else if (wr_mask) {
+ wr_mask &= ~MHPMEVENT_BIT_MINH;
+ mhpmevt_val = (new_val & wr_mask) |
+ (env->mhpmevent_val[evt_index] & ~wr_mask);
+ if (riscv_cpu_mxl(env) == MXL_RV32) {
+ mhpmevt_val = mhpmevt_val |
+ ((uint64_t)env->mhpmeventh_val[evt_index] << 32);
+ }
+ env->mhpmevent_val[evt_index] = mhpmevt_val;
+ riscv_pmu_update_event_map(env, mhpmevt_val, evt_index);
+ } else {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int rmw_cd_mhpmeventh(CPURISCVState *env, int evt_index,
+ target_ulong *val, target_ulong new_val,
+ target_ulong wr_mask)
+{
+ uint64_t mhpmevth_val;
+ uint64_t mhpmevt_val = env->mhpmevent_val[evt_index];
+
+ if (wr_mask != 0 && wr_mask != -1) {
+ return -EINVAL;
+ }
+
+ if (!wr_mask && val) {
+ *val = env->mhpmeventh_val[evt_index];
+ if (riscv_cpu_cfg(env)->ext_sscofpmf) {
+ *val &= ~MHPMEVENTH_BIT_MINH;
+ }
+ } else if (wr_mask) {
+ wr_mask &= ~MHPMEVENTH_BIT_MINH;
+ env->mhpmeventh_val[evt_index] =
+ (new_val & wr_mask) | (env->mhpmeventh_val[evt_index] & ~wr_mask);
+ mhpmevth_val = env->mhpmeventh_val[evt_index];
+ mhpmevt_val = mhpmevt_val | (mhpmevth_val << 32);
+ riscv_pmu_update_event_map(env, mhpmevt_val, evt_index);
+ } else {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int rmw_cd_ctr_cfg(CPURISCVState *env, int cfg_index, target_ulong *val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ switch (cfg_index) {
+ case 0: /* CYCLECFG */
+ if (wr_mask) {
+ wr_mask &= ~MCYCLECFG_BIT_MINH;
+ env->mcyclecfg = (new_val & wr_mask) | (env->mcyclecfg & ~wr_mask);
+ } else {
+ *val = env->mcyclecfg &= ~MHPMEVENTH_BIT_MINH;
+ }
+ break;
+ case 2: /* INSTRETCFG */
+ if (wr_mask) {
+ wr_mask &= ~MINSTRETCFG_BIT_MINH;
+ env->minstretcfg = (new_val & wr_mask) |
+ (env->minstretcfg & ~wr_mask);
+ } else {
+ *val = env->minstretcfg &= ~MHPMEVENTH_BIT_MINH;
+ }
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int rmw_cd_ctr_cfgh(CPURISCVState *env, int cfg_index, target_ulong *val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+
+ if (riscv_cpu_mxl(env) != MXL_RV32) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ switch (cfg_index) {
+ case 0: /* CYCLECFGH */
+ if (wr_mask) {
+ wr_mask &= ~MCYCLECFGH_BIT_MINH;
+ env->mcyclecfgh = (new_val & wr_mask) |
+ (env->mcyclecfgh & ~wr_mask);
+ } else {
+ *val = env->mcyclecfgh;
+ }
+ break;
+ case 2: /* INSTRETCFGH */
+ if (wr_mask) {
+ wr_mask &= ~MINSTRETCFGH_BIT_MINH;
+ env->minstretcfgh = (new_val & wr_mask) |
+ (env->minstretcfgh & ~wr_mask);
+ } else {
+ *val = env->minstretcfgh;
+ }
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+
static int read_scountovf(CPURISCVState *env, int csrno, target_ulong *val)
{
int mhpmevt_start = CSR_MHPMEVENT3 - CSR_MCOUNTINHIBIT;
@@ -1215,6 +1405,14 @@ static int read_scountovf(CPURISCVState *env, int csrno, target_ulong *val)
target_ulong *mhpm_evt_val;
uint64_t of_bit_mask;
+ /* Virtualize scountovf for counter delegation */
+ if (riscv_cpu_cfg(env)->ext_sscofpmf &&
+ riscv_cpu_cfg(env)->ext_ssccfg &&
+ get_field(env->menvcfg, MENVCFG_CDE) &&
+ env->virt_enabled) {
+ return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+ }
+
if (riscv_cpu_mxl(env) == MXL_RV32) {
mhpm_evt_val = env->mhpmeventh_val;
of_bit_mask = MHPMEVENTH_BIT_OF;
@@ -2112,11 +2310,70 @@ static int rmw_xireg_cd(CPURISCVState *env, int csrno,
target_ulong isel, target_ulong *val,
target_ulong new_val, target_ulong wr_mask)
{
- if (!riscv_cpu_cfg(env)->ext_smcdeleg) {
+ int ret = -EINVAL;
+ int ctr_index = isel - ISELECT_CD_FIRST;
+ int isel_hpm_start = ISELECT_CD_FIRST + 3;
+
+ if (!riscv_cpu_cfg(env)->ext_smcdeleg || !riscv_cpu_cfg(env)->ext_ssccfg) {
return RISCV_EXCP_ILLEGAL_INST;
}
- /* TODO: Implement the functionality later */
- return RISCV_EXCP_NONE;
+
+ /* Invalid siselect value for reserved */
+ if (ctr_index == 1) {
+ goto done;
+ }
+
+ /* sireg4 and sireg5 provides access RV32 only CSRs */
+ if (((csrno == CSR_SIREG5) || (csrno == CSR_SIREG4)) &&
+ (riscv_cpu_mxl(env) != MXL_RV32)) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ /* Check Sscofpmf dependancy */
+ if (!riscv_cpu_cfg(env)->ext_sscofpmf && csrno == CSR_SIREG5 &&
+ (isel_hpm_start <= isel && isel <= ISELECT_CD_LAST)) {
+ goto done;
+ }
+
+ /* Check smcntrpmf dependancy */
+ if (!riscv_cpu_cfg(env)->ext_smcntrpmf &&
+ (csrno == CSR_SIREG2 || csrno == CSR_SIREG5) &&
+ (ISELECT_CD_FIRST <= isel && isel < isel_hpm_start)) {
+ goto done;
+ }
+
+ if (!get_field(env->mcounteren, BIT(ctr_index)) ||
+ !get_field(env->menvcfg, MENVCFG_CDE)) {
+ goto done;
+ }
+
+ switch (csrno) {
+ case CSR_SIREG:
+ ret = rmw_cd_mhpmcounter(env, ctr_index, val, new_val, wr_mask);
+ break;
+ case CSR_SIREG4:
+ ret = rmw_cd_mhpmcounterh(env, ctr_index, val, new_val, wr_mask);
+ break;
+ case CSR_SIREG2:
+ if (ctr_index <= 2) {
+ ret = rmw_cd_ctr_cfg(env, ctr_index, val, new_val, wr_mask);
+ } else {
+ ret = rmw_cd_mhpmevent(env, ctr_index, val, new_val, wr_mask);
+ }
+ break;
+ case CSR_SIREG5:
+ if (ctr_index <= 2) {
+ ret = rmw_cd_ctr_cfgh(env, ctr_index, val, new_val, wr_mask);
+ } else {
+ ret = rmw_cd_mhpmeventh(env, ctr_index, val, new_val, wr_mask);
+ }
+ break;
+ default:
+ goto done;
+ }
+
+done:
+ return ret;
}
/*
@@ -2335,14 +2592,15 @@ static RISCVException write_mcountinhibit(CPURISCVState *env, int csrno,
int cidx;
PMUCTRState *counter;
RISCVCPU *cpu = env_archcpu(env);
+ uint32_t present_ctrs = cpu->pmu_avail_ctrs | COUNTEREN_CY | COUNTEREN_IR;
/* WARL register - disable unavailable counters; TM bit is always 0 */
- env->mcountinhibit =
- val & (cpu->pmu_avail_ctrs | COUNTEREN_CY | COUNTEREN_IR);
+ env->mcountinhibit = val & present_ctrs;
/* Check if any other counter is also monitoring cycles/instructions */
for (cidx = 0; cidx < RV_MAX_MHPMCOUNTERS; cidx++) {
- if (!get_field(env->mcountinhibit, BIT(cidx))) {
+ if ((BIT(cidx) & present_ctrs) &&
+ (!get_field(env->mcountinhibit, BIT(cidx)))) {
counter = &env->pmu_ctrs[cidx];
counter->started = true;
}
@@ -2351,6 +2609,21 @@ static RISCVException write_mcountinhibit(CPURISCVState *env, int csrno,
return RISCV_EXCP_NONE;
}
+static RISCVException read_scountinhibit(CPURISCVState *env, int csrno,
+ target_ulong *val)
+{
+ /* S-mode can only access the bits delegated by M-mode */
+ *val = env->mcountinhibit & env->mcounteren;
+ return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_scountinhibit(CPURISCVState *env, int csrno,
+ target_ulong val)
+{
+ write_mcountinhibit(env, csrno, val & env->mcounteren);
+ return RISCV_EXCP_NONE;
+}
+
static RISCVException read_mcounteren(CPURISCVState *env, int csrno,
target_ulong *val)
{
@@ -2453,12 +2726,14 @@ 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_FIOM | MENVCFG_CBIE | MENVCFG_CBCFE |
+ MENVCFG_CBZE | MENVCFG_CDE;
if (riscv_cpu_mxl(env) == MXL_RV64) {
mask |= (cfg->ext_svpbmt ? MENVCFG_PBMTE : 0) |
(cfg->ext_sstc ? MENVCFG_STCE : 0) |
- (cfg->ext_svadu ? MENVCFG_ADUE : 0);
+ (cfg->ext_svadu ? MENVCFG_ADUE : 0) |
+ (cfg->ext_smcdeleg ? MENVCFG_CDE : 0);
}
env->menvcfg = (env->menvcfg & ~mask) | (val & mask);
@@ -2478,7 +2753,8 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
uint64_t mask = (cfg->ext_svpbmt ? MENVCFG_PBMTE : 0) |
(cfg->ext_sstc ? MENVCFG_STCE : 0) |
- (cfg->ext_svadu ? MENVCFG_ADUE : 0);
+ (cfg->ext_svadu ? MENVCFG_ADUE : 0) |
+ (cfg->ext_smcdeleg ? MENVCFG_CDE : 0);
uint64_t valh = (uint64_t)val << 32;
env->menvcfg = (env->menvcfg & ~mask) | (valh & mask);
@@ -5102,6 +5378,11 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
write_sstateen_1_3,
.min_priv_ver = PRIV_VERSION_1_12_0 },
+ /* Supervisor Counter Delegation */
+ [CSR_SCOUNTINHIBIT] = {"scountinhibit", scountinhibit_pred,
+ read_scountinhibit, write_scountinhibit,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+
/* Supervisor Trap Setup */
[CSR_SSTATUS] = { "sstatus", smode, read_sstatus, write_sstatus,
NULL, read_sstatus_i128 },
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH RFC 0/8] Add Counter delegation ISA extension support
2024-02-17 0:01 [PATCH RFC 0/8] Add Counter delegation ISA extension support Atish Patra
` (7 preceding siblings ...)
2024-02-17 0:01 ` [PATCH RFC 8/8] target/riscv: Add counter delegation/configuration support Atish Patra
@ 2024-02-21 14:58 ` Daniel Henrique Barboza
2024-02-21 17:06 ` Atish Kumar Patra
2024-06-01 9:52 ` Daniel Henrique Barboza
9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-02-21 14:58 UTC (permalink / raw)
To: Atish Patra, qemu-devel
Cc: Alistair Francis, Bin Meng, Liu Zhiwei, Palmer Dabbelt,
qemu-riscv, Weiwei Li, kaiwenxue1
Hi Atish,
This series and its dependency, which I assume it's
"[PATCH v4 0/5] Add ISA extension smcntrpmf support"
Doesn't apply in neither master nor riscv-to-apply.next because of this patch:
"target/riscv: Use RISCVException as return type for all csr ops"
That changed some functions from 'int' to "RISCVException" type. The conflicts
from the v4 series are rather trivial but the conflicts for this RFC are annoying
to deal with. It would be better if you could re-send both series rebased with
the latest changes.
One more thing:
On 2/16/24 21:01, Atish Patra wrote:
> This series adds the counter delegation extension support. The counter
> delegation ISA extension(Smcdeleg/Ssccfg) actually depends on multiple ISA
> extensions.
>
> 1. S[m|s]csrind : The indirect CSR extension[1] which defines additional
> 5 ([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size limitation of
> RISC-V CSR address space.
> 2. Smstateen: The stateen bit[60] controls the access to the registers
> indirectly via the above indirect registers.
> 3. Smcdeleg/Ssccfg: The counter delegation extensions[2]
>
> The counter delegation extension allows Supervisor mode to program the
> hpmevent and hpmcounters directly without needing the assistance from the
> M-mode via SBI calls. This results in a faster perf profiling and very
> few traps. This extension also introduces a scountinhibit CSR which allows
> to stop/start any counter directly from the S-mode. As the counter
> delegation extension potentially can have more than 100 CSRs, the specificaiton
> leverages the indirect CSR extension to save the precious CSR address range.
>
> Due to the dependancy of these extensions, the following extensions must be
> enabled to use the counter delegation feature in S-mode.
>
> "smstateen=true,sscofpmf=true,ssccfg=true,smcdeleg=true,smcsrind=true,sscsrind=true"
>
> This makes the qemu command line quite tedious. In stead of that, I think we
> can enable these features by default if there is no objection.
It wasn't need so far but, if needed, we can add specialized setters for extensions
that has multiple dependencies. Instead of the usual setter we would do something
like:
cpu_set_ssccfg() {
if (enabled) {
smstateen=true
sscofpmf=true
smcdeleg=true
smcsrind=true
sscsrind=true
}
}
The advantage is that this setter would also work for CPUs that doesn't inherit defaults,
like bare-cps and profile CPUs.
That doesn't mean we can't add defaults for rv64, but for this particular case I wonder if
the 'max' CPU wouldn't be better.
Thanks,
Daniel
>
> The first 2 patches decouple the indirect CSR usage from AIA implementation
> while patch3 adds stateen bits validation for AIA.
> The PATCH4 implements indirect CSR extensions while remaining patches
> implement the counter delegation extensions.
>
> The Qemu patches can be found here:
> https://github.com/atishp04/qemu/tree/counter_delegation_rfc
>
> The opensbi patch can be found here:
> https://github.com/atishp04/opensbi/tree/counter_delegation_v1
>
> The Linux kernel patches can be found here:
> https://github.com/atishp04/linux/tree/counter_delegation_rfc
>
> [1] https://github.com/riscv/riscv-indirect-csr-access
> [2] https://github.com/riscv/riscv-smcdeleg-ssccfg
>
> Atish Patra (1):
> target/riscv: Enable S*stateen bits for AIA
>
> Kaiwen Xue (7):
> target/riscv: Add properties for Indirect CSR Access extension
> target/riscv: Decouple AIA processing from xiselect and xireg
> target/riscv: Support generic CSR indirect access
> target/riscv: Add smcdeleg/ssccfg properties
> target/riscv: Add counter delegation definitions
> target/riscv: Add select value range check for counter delegation
> target/riscv: Add counter delegation/configuration support
>
> target/riscv/cpu.c | 8 +
> target/riscv/cpu.h | 1 +
> target/riscv/cpu_bits.h | 34 +-
> target/riscv/cpu_cfg.h | 4 +
> target/riscv/csr.c | 713 +++++++++++++++++++++++++++++++++++++---
> target/riscv/machine.c | 1 +
> 6 files changed, 722 insertions(+), 39 deletions(-)
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH RFC 0/8] Add Counter delegation ISA extension support
2024-02-21 14:58 ` [PATCH RFC 0/8] Add Counter delegation ISA extension support Daniel Henrique Barboza
@ 2024-02-21 17:06 ` Atish Kumar Patra
2024-02-21 18:26 ` Daniel Henrique Barboza
0 siblings, 1 reply; 19+ messages in thread
From: Atish Kumar Patra @ 2024-02-21 17:06 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, Alistair Francis, Bin Meng, Liu Zhiwei,
Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1
[-- Attachment #1: Type: text/plain, Size: 5268 bytes --]
On Wed, Feb 21, 2024 at 6:58 AM Daniel Henrique Barboza <
dbarboza@ventanamicro.com> wrote:
> Hi Atish,
>
> This series and its dependency, which I assume it's
>
> "[PATCH v4 0/5] Add ISA extension smcntrpmf support"
>
> Doesn't apply in neither master nor riscv-to-apply.next because of this
> patch:
>
"target/riscv: Use RISCVException as return type for all csr ops"
>
> That changed some functions from 'int' to "RISCVException" type. The
> conflicts
> from the v4 series are rather trivial but the conflicts for this RFC are
> annoying
> to deal with. It would be better if you could re-send both series rebased
> with
> the latest changes.
>
>
I was waiting for Alistair's ACK on the smcntrpmf series as he had some
comments. It looks like he is okay
with the series now (no further questions). Let me respin both the series.
> One more thing:
>
> On 2/16/24 21:01, Atish Patra wrote:
> > This series adds the counter delegation extension support. The counter
> > delegation ISA extension(Smcdeleg/Ssccfg) actually depends on multiple
> ISA
> > extensions.
> >
> > 1. S[m|s]csrind : The indirect CSR extension[1] which defines additional
> > 5 ([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size limitation
> of
> > RISC-V CSR address space.
> > 2. Smstateen: The stateen bit[60] controls the access to the registers
> > indirectly via the above indirect registers.
> > 3. Smcdeleg/Ssccfg: The counter delegation extensions[2]
> >
> > The counter delegation extension allows Supervisor mode to program the
> > hpmevent and hpmcounters directly without needing the assistance from the
> > M-mode via SBI calls. This results in a faster perf profiling and very
> > few traps. This extension also introduces a scountinhibit CSR which
> allows
> > to stop/start any counter directly from the S-mode. As the counter
> > delegation extension potentially can have more than 100 CSRs, the
> specificaiton
> > leverages the indirect CSR extension to save the precious CSR address
> range.
> >
> > Due to the dependancy of these extensions, the following extensions must
> be
> > enabled to use the counter delegation feature in S-mode.
> >
> >
> "smstateen=true,sscofpmf=true,ssccfg=true,smcdeleg=true,smcsrind=true,sscsrind=true"
> >
> > This makes the qemu command line quite tedious. In stead of that, I
> think we
> > can enable these features by default if there is no objection.
>
> It wasn't need so far but, if needed, we can add specialized setters for
> extensions
> that has multiple dependencies. Instead of the usual setter we would do
> something
> like:
>
> cpu_set_ssccfg() {
>
> if (enabled) {
> smstateen=true
> sscofpmf=true
> smcdeleg=true
> smcsrind=true
> sscsrind=true
> }
> }
>
>
> The advantage is that this setter would also work for CPUs that doesn't
> inherit defaults,
> like bare-cps and profile CPUs.
>
>
Your suggested approach looks good to me. But I was asking about concerns
about enabling these extensions
by default rather than the actual mechanism to implement it. Few of the
extensions listed here such as smstateen,smcsrind
sscsrind are independent ISA extensions which are used for other ISA
extensions as well.
It looks like you are okay with the use case also ?
> That doesn't mean we can't add defaults for rv64, but for this particular
> case I wonder if
> the 'max' CPU wouldn't be better.
>
>
Not sure what you mean here. What does 'max' cpu have to do with pmu
extensions ?
>
> Thanks,
>
>
> Daniel
>
> >
> > The first 2 patches decouple the indirect CSR usage from AIA
> implementation
> > while patch3 adds stateen bits validation for AIA.
> > The PATCH4 implements indirect CSR extensions while remaining patches
> > implement the counter delegation extensions.
> >
> > The Qemu patches can be found here:
> > https://github.com/atishp04/qemu/tree/counter_delegation_rfc
> >
> > The opensbi patch can be found here:
> > https://github.com/atishp04/opensbi/tree/counter_delegation_v1
> >
> > The Linux kernel patches can be found here:
> > https://github.com/atishp04/linux/tree/counter_delegation_rfc
> >
> > [1] https://github.com/riscv/riscv-indirect-csr-access
> > [2] https://github.com/riscv/riscv-smcdeleg-ssccfg
> >
> > Atish Patra (1):
> > target/riscv: Enable S*stateen bits for AIA
> >
> > Kaiwen Xue (7):
> > target/riscv: Add properties for Indirect CSR Access extension
> > target/riscv: Decouple AIA processing from xiselect and xireg
> > target/riscv: Support generic CSR indirect access
> > target/riscv: Add smcdeleg/ssccfg properties
> > target/riscv: Add counter delegation definitions
> > target/riscv: Add select value range check for counter delegation
> > target/riscv: Add counter delegation/configuration support
> >
> > target/riscv/cpu.c | 8 +
> > target/riscv/cpu.h | 1 +
> > target/riscv/cpu_bits.h | 34 +-
> > target/riscv/cpu_cfg.h | 4 +
> > target/riscv/csr.c | 713 +++++++++++++++++++++++++++++++++++++---
> > target/riscv/machine.c | 1 +
> > 6 files changed, 722 insertions(+), 39 deletions(-)
> >
> > --
> > 2.34.1
> >
>
[-- Attachment #2: Type: text/html, Size: 7332 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH RFC 0/8] Add Counter delegation ISA extension support
2024-02-21 17:06 ` Atish Kumar Patra
@ 2024-02-21 18:26 ` Daniel Henrique Barboza
2024-02-21 20:17 ` Atish Patra
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-02-21 18:26 UTC (permalink / raw)
To: Atish Kumar Patra
Cc: qemu-devel, Alistair Francis, Bin Meng, Liu Zhiwei,
Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1, Andrew Jones
On 2/21/24 14:06, Atish Kumar Patra wrote:
>
>
> On Wed, Feb 21, 2024 at 6:58 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com <mailto:dbarboza@ventanamicro.com>> wrote:
>
> Hi Atish,
>
> This series and its dependency, which I assume it's
>
> "[PATCH v4 0/5] Add ISA extension smcntrpmf support"
>
> Doesn't apply in neither master nor riscv-to-apply.next because of this patch:
>
> "target/riscv: Use RISCVException as return type for all csr ops"
>
> That changed some functions from 'int' to "RISCVException" type. The conflicts
> from the v4 series are rather trivial but the conflicts for this RFC are annoying
> to deal with. It would be better if you could re-send both series rebased with
> the latest changes.
>
>
> I was waiting for Alistair's ACK on the smcntrpmf series as he had some comments. It looks like he is okay
> with the series now (no further questions). Let me respin both the series.
>
> One more thing:
>
> On 2/16/24 21:01, Atish Patra wrote:
> > This series adds the counter delegation extension support. The counter
> > delegation ISA extension(Smcdeleg/Ssccfg) actually depends on multiple ISA
> > extensions.
> >
> > 1. S[m|s]csrind : The indirect CSR extension[1] which defines additional
> > 5 ([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size limitation of
> > RISC-V CSR address space.
> > 2. Smstateen: The stateen bit[60] controls the access to the registers
> > indirectly via the above indirect registers.
> > 3. Smcdeleg/Ssccfg: The counter delegation extensions[2]
> >
> > The counter delegation extension allows Supervisor mode to program the
> > hpmevent and hpmcounters directly without needing the assistance from the
> > M-mode via SBI calls. This results in a faster perf profiling and very
> > few traps. This extension also introduces a scountinhibit CSR which allows
> > to stop/start any counter directly from the S-mode. As the counter
> > delegation extension potentially can have more than 100 CSRs, the specificaiton
> > leverages the indirect CSR extension to save the precious CSR address range.
> >
> > Due to the dependancy of these extensions, the following extensions must be
> > enabled to use the counter delegation feature in S-mode.
> >
> > "smstateen=true,sscofpmf=true,ssccfg=true,smcdeleg=true,smcsrind=true,sscsrind=true"
> >
> > This makes the qemu command line quite tedious. In stead of that, I think we
> > can enable these features by default if there is no objection.
>
> It wasn't need so far but, if needed, we can add specialized setters for extensions
> that has multiple dependencies. Instead of the usual setter we would do something
> like:
>
> cpu_set_ssccfg() {
>
> if (enabled) {
> smstateen=true
> sscofpmf=true
> smcdeleg=true
> smcsrind=true
> sscsrind=true
> }
> }
>
>
> The advantage is that this setter would also work for CPUs that doesn't inherit defaults,
> like bare-cps and profile CPUs.
>
>
> Your suggested approach looks good to me. But I was asking about concerns about enabling these extensions
> by default rather than the actual mechanism to implement it. Few of the extensions listed here such as smstateen,smcsrind
> sscsrind are independent ISA extensions which are used for other ISA extensions as well.
>
> It looks like you are okay with the use case also ?
I don't mind setting new defaults in rv64.
>
> That doesn't mean we can't add defaults for rv64, but for this particular case I wonder if
> the 'max' CPU wouldn't be better.
>
>
> Not sure what you mean here. What does 'max' cpu have to do with pmu extensions ?
Save a few exceptions, all the extensions declared in riscv_cpu_extensions[]
will be enabled in the 'max' CPU, regardless of their default value for the
rv64 CPU (see riscv_init_max_cpu_extensions() in tcg-cpu.c).
If we count both the v4 and this RFC, the following extensions were added in
riscv_cpu_extensions[]:
+ MULTI_EXT_CFG_BOOL("smcntrpmf", ext_smcntrpmf, false),
+ MULTI_EXT_CFG_BOOL("smcsrind", ext_smcsrind, false),
+ MULTI_EXT_CFG_BOOL("sscsrind", ext_sscsrind, false),
+ MULTI_EXT_CFG_BOOL("smcdeleg", ext_smcdeleg, false),
+ MULTI_EXT_CFG_BOOL("ssccfg", ext_ssccfg, false),
All of them will be enabled by default in the 'max' CPU.
This is what I was referring to. We can just use the 'max' CPU and don't worry about
enabling defaults in rv64.
Thanks,
Daniel
>
>
> Thanks,
>
>
> Daniel
>
> >
> > The first 2 patches decouple the indirect CSR usage from AIA implementation
> > while patch3 adds stateen bits validation for AIA.
> > The PATCH4 implements indirect CSR extensions while remaining patches
> > implement the counter delegation extensions.
> >
> > The Qemu patches can be found here:
> > https://github.com/atishp04/qemu/tree/counter_delegation_rfc <https://github.com/atishp04/qemu/tree/counter_delegation_rfc>
> >
> > The opensbi patch can be found here:
> > https://github.com/atishp04/opensbi/tree/counter_delegation_v1 <https://github.com/atishp04/opensbi/tree/counter_delegation_v1>
> >
> > The Linux kernel patches can be found here:
> > https://github.com/atishp04/linux/tree/counter_delegation_rfc <https://github.com/atishp04/linux/tree/counter_delegation_rfc>
> >
> > [1] https://github.com/riscv/riscv-indirect-csr-access <https://github.com/riscv/riscv-indirect-csr-access>
> > [2] https://github.com/riscv/riscv-smcdeleg-ssccfg <https://github.com/riscv/riscv-smcdeleg-ssccfg>
> >
> > Atish Patra (1):
> > target/riscv: Enable S*stateen bits for AIA
> >
> > Kaiwen Xue (7):
> > target/riscv: Add properties for Indirect CSR Access extension
> > target/riscv: Decouple AIA processing from xiselect and xireg
> > target/riscv: Support generic CSR indirect access
> > target/riscv: Add smcdeleg/ssccfg properties
> > target/riscv: Add counter delegation definitions
> > target/riscv: Add select value range check for counter delegation
> > target/riscv: Add counter delegation/configuration support
> >
> > target/riscv/cpu.c | 8 +
> > target/riscv/cpu.h | 1 +
> > target/riscv/cpu_bits.h | 34 +-
> > target/riscv/cpu_cfg.h | 4 +
> > target/riscv/csr.c | 713 +++++++++++++++++++++++++++++++++++++---
> > target/riscv/machine.c | 1 +
> > 6 files changed, 722 insertions(+), 39 deletions(-)
> >
> > --
> > 2.34.1
> >
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH RFC 0/8] Add Counter delegation ISA extension support
2024-02-21 18:26 ` Daniel Henrique Barboza
@ 2024-02-21 20:17 ` Atish Patra
2024-02-21 21:10 ` Daniel Henrique Barboza
0 siblings, 1 reply; 19+ messages in thread
From: Atish Patra @ 2024-02-21 20:17 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, Alistair Francis, Bin Meng, Liu Zhiwei,
Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1, Andrew Jones
On 2/21/24 10:26, Daniel Henrique Barboza wrote:
>
>
> On 2/21/24 14:06, Atish Kumar Patra wrote:
>>
>>
>> On Wed, Feb 21, 2024 at 6:58 AM Daniel Henrique Barboza
>> <dbarboza@ventanamicro.com <mailto:dbarboza@ventanamicro.com>> wrote:
>>
>> Hi Atish,
>>
>> This series and its dependency, which I assume it's
>>
>> "[PATCH v4 0/5] Add ISA extension smcntrpmf support"
>>
>> Doesn't apply in neither master nor riscv-to-apply.next because
>> of this patch:
>>
>> "target/riscv: Use RISCVException as return type for all csr ops"
>>
>> That changed some functions from 'int' to "RISCVException" type.
>> The conflicts
>> from the v4 series are rather trivial but the conflicts for this
>> RFC are annoying
>> to deal with. It would be better if you could re-send both series
>> rebased with
>> the latest changes.
>>
>>
>> I was waiting for Alistair's ACK on the smcntrpmf series as he had
>> some comments. It looks like he is okay
>> with the series now (no further questions). Let me respin both the
>> series.
>>
>> One more thing:
>>
>> On 2/16/24 21:01, Atish Patra wrote:
>> > This series adds the counter delegation extension support. The
>> counter
>> > delegation ISA extension(Smcdeleg/Ssccfg) actually depends on
>> multiple ISA
>> > extensions.
>> >
>> > 1. S[m|s]csrind : The indirect CSR extension[1] which defines
>> additional
>> > 5 ([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size
>> limitation of
>> > RISC-V CSR address space.
>> > 2. Smstateen: The stateen bit[60] controls the access to the
>> registers
>> > indirectly via the above indirect registers.
>> > 3. Smcdeleg/Ssccfg: The counter delegation extensions[2]
>> >
>> > The counter delegation extension allows Supervisor mode to
>> program the
>> > hpmevent and hpmcounters directly without needing the
>> assistance from the
>> > M-mode via SBI calls. This results in a faster perf profiling
>> and very
>> > few traps. This extension also introduces a scountinhibit CSR
>> which allows
>> > to stop/start any counter directly from the S-mode. As the
>> counter
>> > delegation extension potentially can have more than 100 CSRs,
>> the specificaiton
>> > leverages the indirect CSR extension to save the precious CSR
>> address range.
>> >
>> > Due to the dependancy of these extensions, the following
>> extensions must be
>> > enabled to use the counter delegation feature in S-mode.
>> >
>> >
>> "smstateen=true,sscofpmf=true,ssccfg=true,smcdeleg=true,smcsrind=true,sscsrind=true"
>> >
>> > This makes the qemu command line quite tedious. In stead of
>> that, I think we
>> > can enable these features by default if there is no objection.
>>
>> It wasn't need so far but, if needed, we can add specialized
>> setters for extensions
>> that has multiple dependencies. Instead of the usual setter we
>> would do something
>> like:
>>
>> cpu_set_ssccfg() {
>>
>> if (enabled) {
>> smstateen=true
>> sscofpmf=true
>> smcdeleg=true
>> smcsrind=true
>> sscsrind=true
>> }
>> }
>>
>>
>> The advantage is that this setter would also work for CPUs that
>> doesn't inherit defaults,
>> like bare-cps and profile CPUs.
>>
>>
>> Your suggested approach looks good to me. But I was asking about
>> concerns about enabling these extensions
>> by default rather than the actual mechanism to implement it. Few of
>> the extensions listed here such as smstateen,smcsrind
>> sscsrind are independent ISA extensions which are used for other ISA
>> extensions as well.
>>
>> It looks like you are okay with the use case also ?
>
> I don't mind setting new defaults in rv64.
>
>>
>> That doesn't mean we can't add defaults for rv64, but for this
>> particular case I wonder if
>> the 'max' CPU wouldn't be better.
>>
>>
>> Not sure what you mean here. What does 'max' cpu have to do with pmu
>> extensions ?
>
>
> Save a few exceptions, all the extensions declared in
> riscv_cpu_extensions[]
> will be enabled in the 'max' CPU, regardless of their default value
> for the
> rv64 CPU (see riscv_init_max_cpu_extensions() in tcg-cpu.c).
>
Ahh okay. That makes sense. I got confused with maxcpus option.
> If we count both the v4 and this RFC, the following extensions were
> added in
> riscv_cpu_extensions[]:
>
> + MULTI_EXT_CFG_BOOL("smcntrpmf", ext_smcntrpmf, false),
>
> + MULTI_EXT_CFG_BOOL("smcsrind", ext_smcsrind, false),
> + MULTI_EXT_CFG_BOOL("sscsrind", ext_sscsrind, false),
>
> + MULTI_EXT_CFG_BOOL("smcdeleg", ext_smcdeleg, false),
> + MULTI_EXT_CFG_BOOL("ssccfg", ext_ssccfg, false),
>
>
> All of them will be enabled by default in the 'max' CPU.
>
> This is what I was referring to. We can just use the 'max' CPU and
> don't worry about
> enabling defaults in rv64.
>
We should definitely enable them in 'max' cpu as these extensions are
ratified.
The comment in the code says it should enable all ratified extensions.
Is that a guiding policy ?
Qemu allows merging non frozen extensions.
But rv64 still is the default one everyone running. So I feel we should
enable there as for user convenience.
>
> Thanks,
>
> Daniel
>
>>
>>
>> Thanks,
>>
>>
>> Daniel
>>
>> >
>> > The first 2 patches decouple the indirect CSR usage from AIA
>> implementation
>> > while patch3 adds stateen bits validation for AIA.
>> > The PATCH4 implements indirect CSR extensions while remaining
>> patches
>> > implement the counter delegation extensions.
>> >
>> > The Qemu patches can be found here:
>> > https://github.com/atishp04/qemu/tree/counter_delegation_rfc
>> <https://github.com/atishp04/qemu/tree/counter_delegation_rfc>
>> >
>> > The opensbi patch can be found here:
>> > https://github.com/atishp04/opensbi/tree/counter_delegation_v1
>> <https://github.com/atishp04/opensbi/tree/counter_delegation_v1>
>> >
>> > The Linux kernel patches can be found here:
>> > https://github.com/atishp04/linux/tree/counter_delegation_rfc
>> <https://github.com/atishp04/linux/tree/counter_delegation_rfc>
>> >
>> > [1] https://github.com/riscv/riscv-indirect-csr-access
>> <https://github.com/riscv/riscv-indirect-csr-access>
>> > [2] https://github.com/riscv/riscv-smcdeleg-ssccfg
>> <https://github.com/riscv/riscv-smcdeleg-ssccfg>
>> >
>> > Atish Patra (1):
>> > target/riscv: Enable S*stateen bits for AIA
>> >
>> > Kaiwen Xue (7):
>> > target/riscv: Add properties for Indirect CSR Access extension
>> > target/riscv: Decouple AIA processing from xiselect and xireg
>> > target/riscv: Support generic CSR indirect access
>> > target/riscv: Add smcdeleg/ssccfg properties
>> > target/riscv: Add counter delegation definitions
>> > target/riscv: Add select value range check for counter delegation
>> > target/riscv: Add counter delegation/configuration support
>> >
>> > target/riscv/cpu.c | 8 +
>> > target/riscv/cpu.h | 1 +
>> > target/riscv/cpu_bits.h | 34 +-
>> > target/riscv/cpu_cfg.h | 4 +
>> > target/riscv/csr.c | 713
>> +++++++++++++++++++++++++++++++++++++---
>> > target/riscv/machine.c | 1 +
>> > 6 files changed, 722 insertions(+), 39 deletions(-)
>> >
>> > --
>> > 2.34.1
>> >
>>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH RFC 0/8] Add Counter delegation ISA extension support
2024-02-21 20:17 ` Atish Patra
@ 2024-02-21 21:10 ` Daniel Henrique Barboza
0 siblings, 0 replies; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-02-21 21:10 UTC (permalink / raw)
To: Atish Patra
Cc: qemu-devel, Alistair Francis, Bin Meng, Liu Zhiwei,
Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1, Andrew Jones
On 2/21/24 17:17, Atish Patra wrote:
>
> On 2/21/24 10:26, Daniel Henrique Barboza wrote:
>>
>>
>> On 2/21/24 14:06, Atish Kumar Patra wrote:
>>>
>>>
>>> On Wed, Feb 21, 2024 at 6:58 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com <mailto:dbarboza@ventanamicro.com>> wrote:
>>>
>>> Hi Atish,
>>>
>>> This series and its dependency, which I assume it's
>>>
>>> "[PATCH v4 0/5] Add ISA extension smcntrpmf support"
>>>
>>> Doesn't apply in neither master nor riscv-to-apply.next because of this patch:
>>>
>>> "target/riscv: Use RISCVException as return type for all csr ops"
>>>
>>> That changed some functions from 'int' to "RISCVException" type. The conflicts
>>> from the v4 series are rather trivial but the conflicts for this RFC are annoying
>>> to deal with. It would be better if you could re-send both series rebased with
>>> the latest changes.
>>>
>>>
>>> I was waiting for Alistair's ACK on the smcntrpmf series as he had some comments. It looks like he is okay
>>> with the series now (no further questions). Let me respin both the series.
>>>
>>> One more thing:
>>>
>>> On 2/16/24 21:01, Atish Patra wrote:
>>> > This series adds the counter delegation extension support. The counter
>>> > delegation ISA extension(Smcdeleg/Ssccfg) actually depends on multiple ISA
>>> > extensions.
>>> >
>>> > 1. S[m|s]csrind : The indirect CSR extension[1] which defines additional
>>> > 5 ([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size limitation of
>>> > RISC-V CSR address space.
>>> > 2. Smstateen: The stateen bit[60] controls the access to the registers
>>> > indirectly via the above indirect registers.
>>> > 3. Smcdeleg/Ssccfg: The counter delegation extensions[2]
>>> >
>>> > The counter delegation extension allows Supervisor mode to program the
>>> > hpmevent and hpmcounters directly without needing the assistance from the
>>> > M-mode via SBI calls. This results in a faster perf profiling and very
>>> > few traps. This extension also introduces a scountinhibit CSR which allows
>>> > to stop/start any counter directly from the S-mode. As the counter
>>> > delegation extension potentially can have more than 100 CSRs, the specificaiton
>>> > leverages the indirect CSR extension to save the precious CSR address range.
>>> >
>>> > Due to the dependancy of these extensions, the following extensions must be
>>> > enabled to use the counter delegation feature in S-mode.
>>> >
>>> > "smstateen=true,sscofpmf=true,ssccfg=true,smcdeleg=true,smcsrind=true,sscsrind=true"
>>> >
>>> > This makes the qemu command line quite tedious. In stead of that, I think we
>>> > can enable these features by default if there is no objection.
>>>
>>> It wasn't need so far but, if needed, we can add specialized setters for extensions
>>> that has multiple dependencies. Instead of the usual setter we would do something
>>> like:
>>>
>>> cpu_set_ssccfg() {
>>>
>>> if (enabled) {
>>> smstateen=true
>>> sscofpmf=true
>>> smcdeleg=true
>>> smcsrind=true
>>> sscsrind=true
>>> }
>>> }
>>>
>>>
>>> The advantage is that this setter would also work for CPUs that doesn't inherit defaults,
>>> like bare-cps and profile CPUs.
>>>
>>>
>>> Your suggested approach looks good to me. But I was asking about concerns about enabling these extensions
>>> by default rather than the actual mechanism to implement it. Few of the extensions listed here such as smstateen,smcsrind
>>> sscsrind are independent ISA extensions which are used for other ISA extensions as well.
>>>
>>> It looks like you are okay with the use case also ?
>>
>> I don't mind setting new defaults in rv64.
>>
>>>
>>> That doesn't mean we can't add defaults for rv64, but for this particular case I wonder if
>>> the 'max' CPU wouldn't be better.
>>>
>>>
>>> Not sure what you mean here. What does 'max' cpu have to do with pmu extensions ?
>>
>>
>> Save a few exceptions, all the extensions declared in riscv_cpu_extensions[]
>> will be enabled in the 'max' CPU, regardless of their default value for the
>> rv64 CPU (see riscv_init_max_cpu_extensions() in tcg-cpu.c).
>>
>
> Ahh okay. That makes sense. I got confused with maxcpus option.
>
>
>> If we count both the v4 and this RFC, the following extensions were added in
>> riscv_cpu_extensions[]:
>>
>> + MULTI_EXT_CFG_BOOL("smcntrpmf", ext_smcntrpmf, false),
>>
>> + MULTI_EXT_CFG_BOOL("smcsrind", ext_smcsrind, false),
>> + MULTI_EXT_CFG_BOOL("sscsrind", ext_sscsrind, false),
>>
>> + MULTI_EXT_CFG_BOOL("smcdeleg", ext_smcdeleg, false),
>> + MULTI_EXT_CFG_BOOL("ssccfg", ext_ssccfg, false),
>>
>>
>> All of them will be enabled by default in the 'max' CPU.
>>
>> This is what I was referring to. We can just use the 'max' CPU and don't worry about
>> enabling defaults in rv64.
>>
> We should definitely enable them in 'max' cpu as these extensions are ratified.
> The comment in the code says it should enable all ratified extensions. Is that a guiding policy ?
> Qemu allows merging non frozen extensions.
I guess we need to update that comment ... we're been accepting "frozen" extensions as
stable (i.e. no need for the leading "x-") recently. It doesn't have to be ratified.
>
> But rv64 still is the default one everyone running. So I feel we should enable there as for user convenience.
I've been talking with Drew about making 'max' the default CPU for riscv64. Users
that don't bother with any particular CPU can then use the most feature-rich CPU we
can provide by default. rv64 will still be around for people that wants more control,
although for real control of what the CPU is enabling or not I advise using rv64i or
even a profile CPU.
But anyway, as I said in an earlier reply, I don't mind enabling more defaults in rv64,
especially in a case where one extension have 3+ dependencies.
Thanks,
Daniel
>
>>
>> Thanks,
>>
>> Daniel
>>
>>>
>>>
>>> Thanks,
>>>
>>>
>>> Daniel
>>>
>>> >
>>> > The first 2 patches decouple the indirect CSR usage from AIA implementation
>>> > while patch3 adds stateen bits validation for AIA.
>>> > The PATCH4 implements indirect CSR extensions while remaining patches
>>> > implement the counter delegation extensions.
>>> >
>>> > The Qemu patches can be found here:
>>> > https://github.com/atishp04/qemu/tree/counter_delegation_rfc <https://github.com/atishp04/qemu/tree/counter_delegation_rfc>
>>> >
>>> > The opensbi patch can be found here:
>>> > https://github.com/atishp04/opensbi/tree/counter_delegation_v1 <https://github.com/atishp04/opensbi/tree/counter_delegation_v1>
>>> >
>>> > The Linux kernel patches can be found here:
>>> > https://github.com/atishp04/linux/tree/counter_delegation_rfc <https://github.com/atishp04/linux/tree/counter_delegation_rfc>
>>> >
>>> > [1] https://github.com/riscv/riscv-indirect-csr-access <https://github.com/riscv/riscv-indirect-csr-access>
>>> > [2] https://github.com/riscv/riscv-smcdeleg-ssccfg <https://github.com/riscv/riscv-smcdeleg-ssccfg>
>>> >
>>> > Atish Patra (1):
>>> > target/riscv: Enable S*stateen bits for AIA
>>> >
>>> > Kaiwen Xue (7):
>>> > target/riscv: Add properties for Indirect CSR Access extension
>>> > target/riscv: Decouple AIA processing from xiselect and xireg
>>> > target/riscv: Support generic CSR indirect access
>>> > target/riscv: Add smcdeleg/ssccfg properties
>>> > target/riscv: Add counter delegation definitions
>>> > target/riscv: Add select value range check for counter delegation
>>> > target/riscv: Add counter delegation/configuration support
>>> >
>>> > target/riscv/cpu.c | 8 +
>>> > target/riscv/cpu.h | 1 +
>>> > target/riscv/cpu_bits.h | 34 +-
>>> > target/riscv/cpu_cfg.h | 4 +
>>> > target/riscv/csr.c | 713 +++++++++++++++++++++++++++++++++++++---
>>> > target/riscv/machine.c | 1 +
>>> > 6 files changed, 722 insertions(+), 39 deletions(-)
>>> >
>>> > --
>>> > 2.34.1
>>> >
>>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RFC 0/8] Add Counter delegation ISA extension support
2024-02-17 0:01 [PATCH RFC 0/8] Add Counter delegation ISA extension support Atish Patra
` (8 preceding siblings ...)
2024-02-21 14:58 ` [PATCH RFC 0/8] Add Counter delegation ISA extension support Daniel Henrique Barboza
@ 2024-06-01 9:52 ` Daniel Henrique Barboza
2024-06-02 6:39 ` Atish Kumar Patra
9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-06-01 9:52 UTC (permalink / raw)
To: Atish Patra, qemu-devel
Cc: Alistair Francis, Bin Meng, Liu Zhiwei, Palmer Dabbelt,
qemu-riscv, Weiwei Li, kaiwenxue1
Hi Atish,
I see that Rajnesh sent some patches that were built on top of this
work [1], and this series no longer applies neither to alistair's
risc-to-apply.next nor to master.
If you could send a rebased version of this series that would be great.
Thanks,
Daniel
[1] https://lore.kernel.org/qemu-riscv/20240529160950.132754-1-rkanwal@rivosinc.com/
On 2/16/24 21:01, Atish Patra wrote:
> This series adds the counter delegation extension support. The counter
> delegation ISA extension(Smcdeleg/Ssccfg) actually depends on multiple ISA
> extensions.
>
> 1. S[m|s]csrind : The indirect CSR extension[1] which defines additional
> 5 ([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size limitation of
> RISC-V CSR address space.
> 2. Smstateen: The stateen bit[60] controls the access to the registers
> indirectly via the above indirect registers.
> 3. Smcdeleg/Ssccfg: The counter delegation extensions[2]
>
> The counter delegation extension allows Supervisor mode to program the
> hpmevent and hpmcounters directly without needing the assistance from the
> M-mode via SBI calls. This results in a faster perf profiling and very
> few traps. This extension also introduces a scountinhibit CSR which allows
> to stop/start any counter directly from the S-mode. As the counter
> delegation extension potentially can have more than 100 CSRs, the specificaiton
> leverages the indirect CSR extension to save the precious CSR address range.
>
> Due to the dependancy of these extensions, the following extensions must be
> enabled to use the counter delegation feature in S-mode.
>
> "smstateen=true,sscofpmf=true,ssccfg=true,smcdeleg=true,smcsrind=true,sscsrind=true"
>
> This makes the qemu command line quite tedious. In stead of that, I think we
> can enable these features by default if there is no objection.
>
> The first 2 patches decouple the indirect CSR usage from AIA implementation
> while patch3 adds stateen bits validation for AIA.
> The PATCH4 implements indirect CSR extensions while remaining patches
> implement the counter delegation extensions.
>
> The Qemu patches can be found here:
> https://github.com/atishp04/qemu/tree/counter_delegation_rfc
>
> The opensbi patch can be found here:
> https://github.com/atishp04/opensbi/tree/counter_delegation_v1
>
> The Linux kernel patches can be found here:
> https://github.com/atishp04/linux/tree/counter_delegation_rfc
>
> [1] https://github.com/riscv/riscv-indirect-csr-access
> [2] https://github.com/riscv/riscv-smcdeleg-ssccfg
>
> Atish Patra (1):
> target/riscv: Enable S*stateen bits for AIA
>
> Kaiwen Xue (7):
> target/riscv: Add properties for Indirect CSR Access extension
> target/riscv: Decouple AIA processing from xiselect and xireg
> target/riscv: Support generic CSR indirect access
> target/riscv: Add smcdeleg/ssccfg properties
> target/riscv: Add counter delegation definitions
> target/riscv: Add select value range check for counter delegation
> target/riscv: Add counter delegation/configuration support
>
> target/riscv/cpu.c | 8 +
> target/riscv/cpu.h | 1 +
> target/riscv/cpu_bits.h | 34 +-
> target/riscv/cpu_cfg.h | 4 +
> target/riscv/csr.c | 713 +++++++++++++++++++++++++++++++++++++---
> target/riscv/machine.c | 1 +
> 6 files changed, 722 insertions(+), 39 deletions(-)
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH RFC 0/8] Add Counter delegation ISA extension support
2024-06-01 9:52 ` Daniel Henrique Barboza
@ 2024-06-02 6:39 ` Atish Kumar Patra
0 siblings, 0 replies; 19+ messages in thread
From: Atish Kumar Patra @ 2024-06-02 6:39 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, Alistair Francis, Bin Meng, Liu Zhiwei,
Palmer Dabbelt, qemu-riscv, Weiwei Li, kaiwenxue1
Hi Daniel,
On Sat, Jun 1, 2024 at 2:52 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Hi Atish,
>
>
> I see that Rajnesh sent some patches that were built on top of this
> work [1], and this series no longer applies neither to alistair's
> risc-to-apply.next nor to master.
>
> If you could send a rebased version of this series that would be great.
>
>
Yes. I am planning to send revised series for counter delegation,
Smcntrpmf and a few other miscellaneous fixes
soon.
> Thanks,
>
>
> Daniel
>
>
>
> [1] https://lore.kernel.org/qemu-riscv/20240529160950.132754-1-rkanwal@rivosinc.com/
>
>
> On 2/16/24 21:01, Atish Patra wrote:
> > This series adds the counter delegation extension support. The counter
> > delegation ISA extension(Smcdeleg/Ssccfg) actually depends on multiple ISA
> > extensions.
> >
> > 1. S[m|s]csrind : The indirect CSR extension[1] which defines additional
> > 5 ([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size limitation of
> > RISC-V CSR address space.
> > 2. Smstateen: The stateen bit[60] controls the access to the registers
> > indirectly via the above indirect registers.
> > 3. Smcdeleg/Ssccfg: The counter delegation extensions[2]
> >
> > The counter delegation extension allows Supervisor mode to program the
> > hpmevent and hpmcounters directly without needing the assistance from the
> > M-mode via SBI calls. This results in a faster perf profiling and very
> > few traps. This extension also introduces a scountinhibit CSR which allows
> > to stop/start any counter directly from the S-mode. As the counter
> > delegation extension potentially can have more than 100 CSRs, the specificaiton
> > leverages the indirect CSR extension to save the precious CSR address range.
> >
> > Due to the dependancy of these extensions, the following extensions must be
> > enabled to use the counter delegation feature in S-mode.
> >
> > "smstateen=true,sscofpmf=true,ssccfg=true,smcdeleg=true,smcsrind=true,sscsrind=true"
> >
> > This makes the qemu command line quite tedious. In stead of that, I think we
> > can enable these features by default if there is no objection.
> >
> > The first 2 patches decouple the indirect CSR usage from AIA implementation
> > while patch3 adds stateen bits validation for AIA.
> > The PATCH4 implements indirect CSR extensions while remaining patches
> > implement the counter delegation extensions.
> >
> > The Qemu patches can be found here:
> > https://github.com/atishp04/qemu/tree/counter_delegation_rfc
> >
> > The opensbi patch can be found here:
> > https://github.com/atishp04/opensbi/tree/counter_delegation_v1
> >
> > The Linux kernel patches can be found here:
> > https://github.com/atishp04/linux/tree/counter_delegation_rfc
> >
> > [1] https://github.com/riscv/riscv-indirect-csr-access
> > [2] https://github.com/riscv/riscv-smcdeleg-ssccfg
> >
> > Atish Patra (1):
> > target/riscv: Enable S*stateen bits for AIA
> >
> > Kaiwen Xue (7):
> > target/riscv: Add properties for Indirect CSR Access extension
> > target/riscv: Decouple AIA processing from xiselect and xireg
> > target/riscv: Support generic CSR indirect access
> > target/riscv: Add smcdeleg/ssccfg properties
> > target/riscv: Add counter delegation definitions
> > target/riscv: Add select value range check for counter delegation
> > target/riscv: Add counter delegation/configuration support
> >
> > target/riscv/cpu.c | 8 +
> > target/riscv/cpu.h | 1 +
> > target/riscv/cpu_bits.h | 34 +-
> > target/riscv/cpu_cfg.h | 4 +
> > target/riscv/csr.c | 713 +++++++++++++++++++++++++++++++++++++---
> > target/riscv/machine.c | 1 +
> > 6 files changed, 722 insertions(+), 39 deletions(-)
> >
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 19+ messages in thread