- add SEED CSR which must be accessed with a read-write instruction:
A read-only instruction such as CSRRS/CSRRC with rs1=x0 or CSRRSI/CSRRCI
with uimm=0 will raise an illegal instruction exception.
- add USEED, SSEED fields for MSECCFG CSR
Co-authored-by: Ruibo Lu <luruibo2000@163.com>
Co-authored-by: Zewen Ye <lustrew@foxmail.com>
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
---
target/riscv/cpu_bits.h | 9 ++++++
target/riscv/csr.c | 68 ++++++++++++++++++++++++++++++++++++++++
target/riscv/op_helper.c | 9 ++++++
target/riscv/pmp.h | 8 +++--
4 files changed, 91 insertions(+), 3 deletions(-)
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index bb47cf7e77..d401100f47 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -458,6 +458,9 @@
#define CSR_VSPMMASK 0x2c1
#define CSR_VSPMBASE 0x2c2
+/* Crypto Extension */
+#define CSR_SEED 0x015
+
/* mstatus CSR bits */
#define MSTATUS_UIE 0x00000001
#define MSTATUS_SIE 0x00000002
@@ -800,4 +803,10 @@ typedef enum RISCVException {
#define HVICTL_VALID_MASK \
(HVICTL_VTI | HVICTL_IID | HVICTL_IPRIOM | HVICTL_IPRIO)
+/* seed CSR bits */
+#define SEED_OPST (0b11 << 30)
+#define SEED_OPST_BIST (0b00 << 30)
+#define SEED_OPST_WAIT (0b01 << 30)
+#define SEED_OPST_ES16 (0b10 << 30)
+#define SEED_OPST_DEAD (0b11 << 30)
#endif
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 3c61dd69af..5717a51f56 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -24,6 +24,8 @@
#include "qemu/main-loop.h"
#include "exec/exec-all.h"
#include "sysemu/cpu-timers.h"
+#include "qemu/guest-random.h"
+#include "qapi/error.h"
/* CSR function table public API */
void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops)
@@ -292,6 +294,40 @@ static RISCVException epmp(CPURISCVState *env, int csrno)
}
#endif
+static RISCVException seed(CPURISCVState *env, int csrno)
+{
+ RISCVCPU *cpu = env_archcpu(env);
+
+ if (!cpu->cfg.ext_zkr) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+#if !defined(CONFIG_USER_ONLY)
+ if (riscv_has_ext(env, RVS) && riscv_has_ext(env, RVH)) {
+ /* Hypervisor extension is supported */
+ if (riscv_cpu_virt_enabled(env) && (env->priv != PRV_M)) {
+ if (env->mseccfg & MSECCFG_SSEED) {
+ return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+ } else {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+ }
+ }
+
+ if (env->priv == PRV_M) {
+ return RISCV_EXCP_NONE;
+ } else if (env->priv == PRV_S && (env->mseccfg & MSECCFG_SSEED)) {
+ return RISCV_EXCP_NONE;
+ } else if (env->priv == PRV_U && (env->mseccfg & MSECCFG_USEED)) {
+ return RISCV_EXCP_NONE;
+ } else {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+#else
+ return RISCV_EXCP_NONE;
+#endif
+}
+
/* User Floating-Point CSRs */
static RISCVException read_fflags(CPURISCVState *env, int csrno,
target_ulong *val)
@@ -2961,6 +2997,35 @@ static RISCVException write_upmbase(CPURISCVState *env, int csrno,
#endif
+/* Crypto Extension */
+static RISCVException rmw_seed(CPURISCVState *env, int csrno,
+ target_ulong *ret_value,
+ target_ulong new_value, target_ulong write_mask)
+{
+ uint16_t random_v;
+ Error *random_e = NULL;
+ int random_r;
+
+ random_r = qemu_guest_getrandom(&random_v, 2, &random_e);
+ if (unlikely(random_r < 0)) {
+ /*
+ * Failed, for unknown reasons in the crypto subsystem.
+ * The best we can do is log the reason and return a
+ * failure indication to the guest. There is no reason
+ * we know to expect the failure to be transitory, so
+ * indicate DEAD to avoid having the guest spin on WAIT.
+ */
+ qemu_log_mask(LOG_UNIMP, "%s: Crypto failure: %s",
+ __func__, error_get_pretty(random_e));
+ error_free(random_e);
+ *ret_value = SEED_OPST_DEAD;
+ } else {
+ *ret_value = random_v | SEED_OPST_ES16;
+ }
+
+ return RISCV_EXCP_NONE;
+}
+
/*
* riscv_csrrw - read and/or update control and status register
*
@@ -3205,6 +3270,9 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_TIME] = { "time", ctr, read_time },
[CSR_TIMEH] = { "timeh", ctr32, read_timeh },
+ /* Crypto Extension */
+ [CSR_SEED] = { "seed", seed, NULL, NULL, rmw_seed },
+
#if !defined(CONFIG_USER_ONLY)
/* Machine Timers and Counters */
[CSR_MCYCLE] = { "mcycle", any, read_instret },
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index e0d708091e..3d8443416d 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -39,6 +39,15 @@ void helper_raise_exception(CPURISCVState *env, uint32_t exception)
target_ulong helper_csrr(CPURISCVState *env, int csr)
{
+ /*
+ * The seed CSR must be accessed with a read-write instruction. A
+ * read-only instruction such as CSRRS/CSRRC with rs1=x0 or CSRRSI/
+ * CSRRCI with uimm=0 will raise an illegal instruction exception.
+ */
+ if (csr == CSR_SEED) {
+ riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
+ }
+
target_ulong val = 0;
RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0, false);
diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
index fcb6b7c467..a8dd797476 100644
--- a/target/riscv/pmp.h
+++ b/target/riscv/pmp.h
@@ -39,9 +39,11 @@ typedef enum {
} pmp_am_t;
typedef enum {
- MSECCFG_MML = 1 << 0,
- MSECCFG_MMWP = 1 << 1,
- MSECCFG_RLB = 1 << 2
+ MSECCFG_MML = 1 << 0,
+ MSECCFG_MMWP = 1 << 1,
+ MSECCFG_RLB = 1 << 2,
+ MSECCFG_USEED = 1 << 8,
+ MSECCFG_SSEED = 1 << 9
} mseccfg_field_t;
typedef struct {
--
2.17.1Hi, any comments on this patch or patchset?
Currently, read-only instruction to access Seed CSR is checked as a
special case in helper_csrr as suggested in
https://lists.nongnu.org/archive/html/qemu-riscv/2022-03/msg00146.html.
(The new version for that patch is in
https://lists.nongnu.org/archive/html/qemu-riscv/2022-03/msg00156.html)
Regards,
Weiwei Li
在 2022/3/18 下午12:19, Weiwei Li 写道:
> - add SEED CSR which must be accessed with a read-write instruction:
> A read-only instruction such as CSRRS/CSRRC with rs1=x0 or CSRRSI/CSRRCI
> with uimm=0 will raise an illegal instruction exception.
> - add USEED, SSEED fields for MSECCFG CSR
>
> Co-authored-by: Ruibo Lu <luruibo2000@163.com>
> Co-authored-by: Zewen Ye <lustrew@foxmail.com>
> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> ---
> target/riscv/cpu_bits.h | 9 ++++++
> target/riscv/csr.c | 68 ++++++++++++++++++++++++++++++++++++++++
> target/riscv/op_helper.c | 9 ++++++
> target/riscv/pmp.h | 8 +++--
> 4 files changed, 91 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index bb47cf7e77..d401100f47 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -458,6 +458,9 @@
> #define CSR_VSPMMASK 0x2c1
> #define CSR_VSPMBASE 0x2c2
>
> +/* Crypto Extension */
> +#define CSR_SEED 0x015
> +
> /* mstatus CSR bits */
> #define MSTATUS_UIE 0x00000001
> #define MSTATUS_SIE 0x00000002
> @@ -800,4 +803,10 @@ typedef enum RISCVException {
> #define HVICTL_VALID_MASK \
> (HVICTL_VTI | HVICTL_IID | HVICTL_IPRIOM | HVICTL_IPRIO)
>
> +/* seed CSR bits */
> +#define SEED_OPST (0b11 << 30)
> +#define SEED_OPST_BIST (0b00 << 30)
> +#define SEED_OPST_WAIT (0b01 << 30)
> +#define SEED_OPST_ES16 (0b10 << 30)
> +#define SEED_OPST_DEAD (0b11 << 30)
> #endif
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 3c61dd69af..5717a51f56 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -24,6 +24,8 @@
> #include "qemu/main-loop.h"
> #include "exec/exec-all.h"
> #include "sysemu/cpu-timers.h"
> +#include "qemu/guest-random.h"
> +#include "qapi/error.h"
>
> /* CSR function table public API */
> void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops)
> @@ -292,6 +294,40 @@ static RISCVException epmp(CPURISCVState *env, int csrno)
> }
> #endif
>
> +static RISCVException seed(CPURISCVState *env, int csrno)
> +{
> + RISCVCPU *cpu = env_archcpu(env);
> +
> + if (!cpu->cfg.ext_zkr) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> +#if !defined(CONFIG_USER_ONLY)
> + if (riscv_has_ext(env, RVS) && riscv_has_ext(env, RVH)) {
> + /* Hypervisor extension is supported */
> + if (riscv_cpu_virt_enabled(env) && (env->priv != PRV_M)) {
> + if (env->mseccfg & MSECCFG_SSEED) {
> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> + } else {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + }
> + }
> +
> + if (env->priv == PRV_M) {
> + return RISCV_EXCP_NONE;
> + } else if (env->priv == PRV_S && (env->mseccfg & MSECCFG_SSEED)) {
> + return RISCV_EXCP_NONE;
> + } else if (env->priv == PRV_U && (env->mseccfg & MSECCFG_USEED)) {
> + return RISCV_EXCP_NONE;
> + } else {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +#else
> + return RISCV_EXCP_NONE;
> +#endif
> +}
> +
> /* User Floating-Point CSRs */
> static RISCVException read_fflags(CPURISCVState *env, int csrno,
> target_ulong *val)
> @@ -2961,6 +2997,35 @@ static RISCVException write_upmbase(CPURISCVState *env, int csrno,
>
> #endif
>
> +/* Crypto Extension */
> +static RISCVException rmw_seed(CPURISCVState *env, int csrno,
> + target_ulong *ret_value,
> + target_ulong new_value, target_ulong write_mask)
> +{
> + uint16_t random_v;
> + Error *random_e = NULL;
> + int random_r;
> +
> + random_r = qemu_guest_getrandom(&random_v, 2, &random_e);
> + if (unlikely(random_r < 0)) {
> + /*
> + * Failed, for unknown reasons in the crypto subsystem.
> + * The best we can do is log the reason and return a
> + * failure indication to the guest. There is no reason
> + * we know to expect the failure to be transitory, so
> + * indicate DEAD to avoid having the guest spin on WAIT.
> + */
> + qemu_log_mask(LOG_UNIMP, "%s: Crypto failure: %s",
> + __func__, error_get_pretty(random_e));
> + error_free(random_e);
> + *ret_value = SEED_OPST_DEAD;
> + } else {
> + *ret_value = random_v | SEED_OPST_ES16;
> + }
> +
> + return RISCV_EXCP_NONE;
> +}
> +
> /*
> * riscv_csrrw - read and/or update control and status register
> *
> @@ -3205,6 +3270,9 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_TIME] = { "time", ctr, read_time },
> [CSR_TIMEH] = { "timeh", ctr32, read_timeh },
>
> + /* Crypto Extension */
> + [CSR_SEED] = { "seed", seed, NULL, NULL, rmw_seed },
> +
> #if !defined(CONFIG_USER_ONLY)
> /* Machine Timers and Counters */
> [CSR_MCYCLE] = { "mcycle", any, read_instret },
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index e0d708091e..3d8443416d 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -39,6 +39,15 @@ void helper_raise_exception(CPURISCVState *env, uint32_t exception)
>
> target_ulong helper_csrr(CPURISCVState *env, int csr)
> {
> + /*
> + * The seed CSR must be accessed with a read-write instruction. A
> + * read-only instruction such as CSRRS/CSRRC with rs1=x0 or CSRRSI/
> + * CSRRCI with uimm=0 will raise an illegal instruction exception.
> + */
> + if (csr == CSR_SEED) {
> + riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> + }
> +
> target_ulong val = 0;
> RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0, false);
>
> diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
> index fcb6b7c467..a8dd797476 100644
> --- a/target/riscv/pmp.h
> +++ b/target/riscv/pmp.h
> @@ -39,9 +39,11 @@ typedef enum {
> } pmp_am_t;
>
> typedef enum {
> - MSECCFG_MML = 1 << 0,
> - MSECCFG_MMWP = 1 << 1,
> - MSECCFG_RLB = 1 << 2
> + MSECCFG_MML = 1 << 0,
> + MSECCFG_MMWP = 1 << 1,
> + MSECCFG_RLB = 1 << 2,
> + MSECCFG_USEED = 1 << 8,
> + MSECCFG_SSEED = 1 << 9
> } mseccfg_field_t;
>
> typedef struct {
On Mon, Apr 11, 2022 at 2:46 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
> Hi, any comments on this patch or patchset?
>
> Currently, read-only instruction to access Seed CSR is checked as a
> special case in helper_csrr as suggested in
>
> https://lists.nongnu.org/archive/html/qemu-riscv/2022-03/msg00146.html.
Ah sorry, I didn't realise you had updated this.
>
> (The new version for that patch is in
> https://lists.nongnu.org/archive/html/qemu-riscv/2022-03/msg00156.html)
>
> Regards,
>
> Weiwei Li
>
> 在 2022/3/18 下午12:19, Weiwei Li 写道:
> > - add SEED CSR which must be accessed with a read-write instruction:
> > A read-only instruction such as CSRRS/CSRRC with rs1=x0 or CSRRSI/CSRRCI
> > with uimm=0 will raise an illegal instruction exception.
> > - add USEED, SSEED fields for MSECCFG CSR
> >
> > Co-authored-by: Ruibo Lu <luruibo2000@163.com>
> > Co-authored-by: Zewen Ye <lustrew@foxmail.com>
> > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> > ---
> > target/riscv/cpu_bits.h | 9 ++++++
> > target/riscv/csr.c | 68 ++++++++++++++++++++++++++++++++++++++++
> > target/riscv/op_helper.c | 9 ++++++
> > target/riscv/pmp.h | 8 +++--
> > 4 files changed, 91 insertions(+), 3 deletions(-)
> >
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index bb47cf7e77..d401100f47 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -458,6 +458,9 @@
> > #define CSR_VSPMMASK 0x2c1
> > #define CSR_VSPMBASE 0x2c2
> >
> > +/* Crypto Extension */
> > +#define CSR_SEED 0x015
> > +
> > /* mstatus CSR bits */
> > #define MSTATUS_UIE 0x00000001
> > #define MSTATUS_SIE 0x00000002
> > @@ -800,4 +803,10 @@ typedef enum RISCVException {
> > #define HVICTL_VALID_MASK \
> > (HVICTL_VTI | HVICTL_IID | HVICTL_IPRIOM | HVICTL_IPRIO)
> >
> > +/* seed CSR bits */
> > +#define SEED_OPST (0b11 << 30)
> > +#define SEED_OPST_BIST (0b00 << 30)
> > +#define SEED_OPST_WAIT (0b01 << 30)
> > +#define SEED_OPST_ES16 (0b10 << 30)
> > +#define SEED_OPST_DEAD (0b11 << 30)
> > #endif
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 3c61dd69af..5717a51f56 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -24,6 +24,8 @@
> > #include "qemu/main-loop.h"
> > #include "exec/exec-all.h"
> > #include "sysemu/cpu-timers.h"
> > +#include "qemu/guest-random.h"
> > +#include "qapi/error.h"
> >
> > /* CSR function table public API */
> > void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops)
> > @@ -292,6 +294,40 @@ static RISCVException epmp(CPURISCVState *env, int csrno)
> > }
> > #endif
> >
> > +static RISCVException seed(CPURISCVState *env, int csrno)
> > +{
> > + RISCVCPU *cpu = env_archcpu(env);
> > +
> > + if (!cpu->cfg.ext_zkr) {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > +
> > +#if !defined(CONFIG_USER_ONLY)
> > + if (riscv_has_ext(env, RVS) && riscv_has_ext(env, RVH)) {
> > + /* Hypervisor extension is supported */
> > + if (riscv_cpu_virt_enabled(env) && (env->priv != PRV_M)) {
You can simplify this to just riscv_cpu_virt_enabled(). You don't need
to check if we have the extension as well.
> > + if (env->mseccfg & MSECCFG_SSEED) {
> > + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> > + } else {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > + }
> > + }
> > +
> > + if (env->priv == PRV_M) {
> > + return RISCV_EXCP_NONE;
> > + } else if (env->priv == PRV_S && (env->mseccfg & MSECCFG_SSEED)) {
> > + return RISCV_EXCP_NONE;
> > + } else if (env->priv == PRV_U && (env->mseccfg & MSECCFG_USEED)) {
> > + return RISCV_EXCP_NONE;
> > + } else {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > +#else
> > + return RISCV_EXCP_NONE;
> > +#endif
> > +}
> > +
> > /* User Floating-Point CSRs */
> > static RISCVException read_fflags(CPURISCVState *env, int csrno,
> > target_ulong *val)
> > @@ -2961,6 +2997,35 @@ static RISCVException write_upmbase(CPURISCVState *env, int csrno,
> >
> > #endif
> >
> > +/* Crypto Extension */
> > +static RISCVException rmw_seed(CPURISCVState *env, int csrno,
> > + target_ulong *ret_value,
> > + target_ulong new_value, target_ulong write_mask)
> > +{
> > + uint16_t random_v;
> > + Error *random_e = NULL;
> > + int random_r;
> > +
> > + random_r = qemu_guest_getrandom(&random_v, 2, &random_e);
> > + if (unlikely(random_r < 0)) {
> > + /*
> > + * Failed, for unknown reasons in the crypto subsystem.
> > + * The best we can do is log the reason and return a
> > + * failure indication to the guest. There is no reason
> > + * we know to expect the failure to be transitory, so
> > + * indicate DEAD to avoid having the guest spin on WAIT.
> > + */
> > + qemu_log_mask(LOG_UNIMP, "%s: Crypto failure: %s",
> > + __func__, error_get_pretty(random_e));
> > + error_free(random_e);
> > + *ret_value = SEED_OPST_DEAD;
> > + } else {
> > + *ret_value = random_v | SEED_OPST_ES16;
> > + }
Won't this seg fault if a guest does a CSR write?
> > +
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > /*
> > * riscv_csrrw - read and/or update control and status register
> > *
> > @@ -3205,6 +3270,9 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> > [CSR_TIME] = { "time", ctr, read_time },
> > [CSR_TIMEH] = { "timeh", ctr32, read_timeh },
> >
> > + /* Crypto Extension */
> > + [CSR_SEED] = { "seed", seed, NULL, NULL, rmw_seed },
> > +
> > #if !defined(CONFIG_USER_ONLY)
> > /* Machine Timers and Counters */
> > [CSR_MCYCLE] = { "mcycle", any, read_instret },
> > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> > index e0d708091e..3d8443416d 100644
> > --- a/target/riscv/op_helper.c
> > +++ b/target/riscv/op_helper.c
> > @@ -39,6 +39,15 @@ void helper_raise_exception(CPURISCVState *env, uint32_t exception)
> >
> > target_ulong helper_csrr(CPURISCVState *env, int csr)
> > {
> > + /*
> > + * The seed CSR must be accessed with a read-write instruction. A
> > + * read-only instruction such as CSRRS/CSRRC with rs1=x0 or CSRRSI/
> > + * CSRRCI with uimm=0 will raise an illegal instruction exception.
> > + */
> > + if (csr == CSR_SEED) {
> > + riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> > + }
> > +
> > target_ulong val = 0;
> > RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0, false);
> >
> > diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
> > index fcb6b7c467..a8dd797476 100644
> > --- a/target/riscv/pmp.h
> > +++ b/target/riscv/pmp.h
> > @@ -39,9 +39,11 @@ typedef enum {
> > } pmp_am_t;
> >
> > typedef enum {
> > - MSECCFG_MML = 1 << 0,
> > - MSECCFG_MMWP = 1 << 1,
> > - MSECCFG_RLB = 1 << 2
> > + MSECCFG_MML = 1 << 0,
> > + MSECCFG_MMWP = 1 << 1,
> > + MSECCFG_RLB = 1 << 2,
Why are these ones being changed?
Alistair
> > + MSECCFG_USEED = 1 << 8,
> > + MSECCFG_SSEED = 1 << 9
> > } mseccfg_field_t;
> >
> > typedef struct {
>
>
Thanks for your comments.
在 2022/4/14 上午7:57, Alistair Francis 写道:
> On Mon, Apr 11, 2022 at 2:46 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>> Hi, any comments on this patch or patchset?
>>
>> Currently, read-only instruction to access Seed CSR is checked as a
>> special case in helper_csrr as suggested in
>>
>> https://lists.nongnu.org/archive/html/qemu-riscv/2022-03/msg00146.html.
> Ah sorry, I didn't realise you had updated this.
>
>> (The new version for that patch is in
>> https://lists.nongnu.org/archive/html/qemu-riscv/2022-03/msg00156.html)
>>
>> Regards,
>>
>> Weiwei Li
>>
>> 在 2022/3/18 下午12:19, Weiwei Li 写道:
>>> - add SEED CSR which must be accessed with a read-write instruction:
>>> A read-only instruction such as CSRRS/CSRRC with rs1=x0 or CSRRSI/CSRRCI
>>> with uimm=0 will raise an illegal instruction exception.
>>> - add USEED, SSEED fields for MSECCFG CSR
>>>
>>> Co-authored-by: Ruibo Lu <luruibo2000@163.com>
>>> Co-authored-by: Zewen Ye <lustrew@foxmail.com>
>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>>> ---
>>> target/riscv/cpu_bits.h | 9 ++++++
>>> target/riscv/csr.c | 68 ++++++++++++++++++++++++++++++++++++++++
>>> target/riscv/op_helper.c | 9 ++++++
>>> target/riscv/pmp.h | 8 +++--
>>> 4 files changed, 91 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>>> index bb47cf7e77..d401100f47 100644
>>> --- a/target/riscv/cpu_bits.h
>>> +++ b/target/riscv/cpu_bits.h
>>> @@ -458,6 +458,9 @@
>>> #define CSR_VSPMMASK 0x2c1
>>> #define CSR_VSPMBASE 0x2c2
>>>
>>> +/* Crypto Extension */
>>> +#define CSR_SEED 0x015
>>> +
>>> /* mstatus CSR bits */
>>> #define MSTATUS_UIE 0x00000001
>>> #define MSTATUS_SIE 0x00000002
>>> @@ -800,4 +803,10 @@ typedef enum RISCVException {
>>> #define HVICTL_VALID_MASK \
>>> (HVICTL_VTI | HVICTL_IID | HVICTL_IPRIOM | HVICTL_IPRIO)
>>>
>>> +/* seed CSR bits */
>>> +#define SEED_OPST (0b11 << 30)
>>> +#define SEED_OPST_BIST (0b00 << 30)
>>> +#define SEED_OPST_WAIT (0b01 << 30)
>>> +#define SEED_OPST_ES16 (0b10 << 30)
>>> +#define SEED_OPST_DEAD (0b11 << 30)
>>> #endif
>>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>>> index 3c61dd69af..5717a51f56 100644
>>> --- a/target/riscv/csr.c
>>> +++ b/target/riscv/csr.c
>>> @@ -24,6 +24,8 @@
>>> #include "qemu/main-loop.h"
>>> #include "exec/exec-all.h"
>>> #include "sysemu/cpu-timers.h"
>>> +#include "qemu/guest-random.h"
>>> +#include "qapi/error.h"
>>>
>>> /* CSR function table public API */
>>> void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops)
>>> @@ -292,6 +294,40 @@ static RISCVException epmp(CPURISCVState *env, int csrno)
>>> }
>>> #endif
>>>
>>> +static RISCVException seed(CPURISCVState *env, int csrno)
>>> +{
>>> + RISCVCPU *cpu = env_archcpu(env);
>>> +
>>> + if (!cpu->cfg.ext_zkr) {
>>> + return RISCV_EXCP_ILLEGAL_INST;
>>> + }
>>> +
>>> +#if !defined(CONFIG_USER_ONLY)
>>> + if (riscv_has_ext(env, RVS) && riscv_has_ext(env, RVH)) {
>>> + /* Hypervisor extension is supported */
>>> + if (riscv_cpu_virt_enabled(env) && (env->priv != PRV_M)) {
> You can simplify this to just riscv_cpu_virt_enabled(). You don't need
> to check if we have the extension as well.
Yeah, Maybe It can merge into following logic, like:
if (env->priv == PRV_M) { //M
return RISCV_EXCP_NONE;
} else if (riscv_cpu_virt_enabled(env)) { //VS/VU
if (env->mseccfg & MSECCFG_SSEED) {
return RISCV_EXCP_NONE;
} else {
return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
}
} else { //S/U
if (env->priv == PRV_S && (env->mseccfg & MSECCFG_SSEED)) {
return RISCV_EXCP_NONE;
} else if (env->priv == PRV_U && (env->mseccfg & MSECCFG_USEED)) {
return RISCV_EXCP_NONE;
} else {
return RISCV_EXCP_ILLEGAL_INST;
}
}
>
>>> + if (env->mseccfg & MSECCFG_SSEED) {
>>> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>>> + } else {
>>> + return RISCV_EXCP_ILLEGAL_INST;
>>> + }
>>> + }
>>> + }
>>> +
>>> + if (env->priv == PRV_M) {
>>> + return RISCV_EXCP_NONE;
>>> + } else if (env->priv == PRV_S && (env->mseccfg & MSECCFG_SSEED)) {
>>> + return RISCV_EXCP_NONE;
>>> + } else if (env->priv == PRV_U && (env->mseccfg & MSECCFG_USEED)) {
>>> + return RISCV_EXCP_NONE;
>>> + } else {
>>> + return RISCV_EXCP_ILLEGAL_INST;
>>> + }
>>> +#else
>>> + return RISCV_EXCP_NONE;
>>> +#endif
>>> +}
>>> +
>>> /* User Floating-Point CSRs */
>>> static RISCVException read_fflags(CPURISCVState *env, int csrno,
>>> target_ulong *val)
>>> @@ -2961,6 +2997,35 @@ static RISCVException write_upmbase(CPURISCVState *env, int csrno,
>>>
>>> #endif
>>>
>>> +/* Crypto Extension */
>>> +static RISCVException rmw_seed(CPURISCVState *env, int csrno,
>>> + target_ulong *ret_value,
>>> + target_ulong new_value, target_ulong write_mask)
>>> +{
>>> + uint16_t random_v;
>>> + Error *random_e = NULL;
>>> + int random_r;
>>> +
>>> + random_r = qemu_guest_getrandom(&random_v, 2, &random_e);
>>> + if (unlikely(random_r < 0)) {
>>> + /*
>>> + * Failed, for unknown reasons in the crypto subsystem.
>>> + * The best we can do is log the reason and return a
>>> + * failure indication to the guest. There is no reason
>>> + * we know to expect the failure to be transitory, so
>>> + * indicate DEAD to avoid having the guest spin on WAIT.
>>> + */
>>> + qemu_log_mask(LOG_UNIMP, "%s: Crypto failure: %s",
>>> + __func__, error_get_pretty(random_e));
>>> + error_free(random_e);
>>> + *ret_value = SEED_OPST_DEAD;
>>> + } else {
>>> + *ret_value = random_v | SEED_OPST_ES16;
>>> + }
> Won't this seg fault if a guest does a CSR write?
Yeah. Only CSR write seems have no function. However, it's not limited
in the spec, so it's possible.
I'll add NULL judgement here.
>
>>> +
>>> + return RISCV_EXCP_NONE;
>>> +}
>>> +
>>> /*
>>> * riscv_csrrw - read and/or update control and status register
>>> *
>>> @@ -3205,6 +3270,9 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>>> [CSR_TIME] = { "time", ctr, read_time },
>>> [CSR_TIMEH] = { "timeh", ctr32, read_timeh },
>>>
>>> + /* Crypto Extension */
>>> + [CSR_SEED] = { "seed", seed, NULL, NULL, rmw_seed },
>>> +
>>> #if !defined(CONFIG_USER_ONLY)
>>> /* Machine Timers and Counters */
>>> [CSR_MCYCLE] = { "mcycle", any, read_instret },
>>> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
>>> index e0d708091e..3d8443416d 100644
>>> --- a/target/riscv/op_helper.c
>>> +++ b/target/riscv/op_helper.c
>>> @@ -39,6 +39,15 @@ void helper_raise_exception(CPURISCVState *env, uint32_t exception)
>>>
>>> target_ulong helper_csrr(CPURISCVState *env, int csr)
>>> {
>>> + /*
>>> + * The seed CSR must be accessed with a read-write instruction. A
>>> + * read-only instruction such as CSRRS/CSRRC with rs1=x0 or CSRRSI/
>>> + * CSRRCI with uimm=0 will raise an illegal instruction exception.
>>> + */
>>> + if (csr == CSR_SEED) {
>>> + riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
>>> + }
>>> +
>>> target_ulong val = 0;
>>> RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0, false);
>>>
>>> diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
>>> index fcb6b7c467..a8dd797476 100644
>>> --- a/target/riscv/pmp.h
>>> +++ b/target/riscv/pmp.h
>>> @@ -39,9 +39,11 @@ typedef enum {
>>> } pmp_am_t;
>>>
>>> typedef enum {
>>> - MSECCFG_MML = 1 << 0,
>>> - MSECCFG_MMWP = 1 << 1,
>>> - MSECCFG_RLB = 1 << 2
>>> + MSECCFG_MML = 1 << 0,
>>> + MSECCFG_MMWP = 1 << 1,
>>> + MSECCFG_RLB = 1 << 2,
> Why are these ones being changed?
They are changed to align with MSECCFG_USEED. Is it OK or just let them
unchanged?
Regards,
Weiwei Li
>
> Alistair
>
>>> + MSECCFG_USEED = 1 << 8,
>>> + MSECCFG_SSEED = 1 << 9
>>> } mseccfg_field_t;
>>>
>>> typedef struct {
>>
© 2016 - 2026 Red Hat, Inc.