On 4/17/25 7:52 AM, Jim Shu wrote:
> riscv_worldguard_apply_cpu() could enable WG CPU extension and set WG
> callback to CPUs. It is used by machine code after realizing global WG
> device.
>
> Signed-off-by: Jim Shu <jim....@sifive.com>
> ---
> hw/misc/riscv_worldguard.c | 87 ++++++++++++++++++++++++++++++
> include/hw/misc/riscv_worldguard.h | 1 +
> 2 files changed, 88 insertions(+)
>
> diff --git a/hw/misc/riscv_worldguard.c b/hw/misc/riscv_worldguard.c
> index b02bd28d02..1a910f4cf3 100644
> --- a/hw/misc/riscv_worldguard.c
> +++ b/hw/misc/riscv_worldguard.c
> @@ -92,6 +92,93 @@ uint32_t mem_attrs_to_wid(MemTxAttrs attrs)
> }
> }
>
> +static void riscv_cpu_wg_reset(CPURISCVState *env)
> +{
> + CPUState *cs = env_cpu(env);
> + RISCVCPU *cpu = RISCV_CPU(cs);
> + uint32_t mlwid, slwid, mwiddeleg;
> + uint32_t trustedwid;
> +
> + if (!riscv_cpu_cfg(env)->ext_smwg) {
> + return;
> + }
> +
> + if (worldguard_config == NULL) {
> + /*
> + * Note: This reset is dummy now and WG CSRs will be reset again
> + * after worldguard_config is realized.
> + */
> + return;
> + }
> +
> + trustedwid = worldguard_config->trustedwid;
> + if (trustedwid == NO_TRUSTEDWID) {
> + trustedwid = worldguard_config->nworlds - 1;
> + }
> +
> + /* Reset mlwid, slwid, mwiddeleg CSRs */
> + if (worldguard_config->hw_bypass) {
> + /* HW bypass mode */
> + mlwid = trustedwid;
> + } else {
> + mlwid = 0;
> + }
> + slwid = 0;
> + mwiddeleg = 0;
> +
> + env->mlwid = mlwid;
> + if (riscv_cpu_cfg(env)->ext_sswg) {
> + env->slwid = slwid;
> + env->mwiddeleg = mwiddeleg;
> + }
> +
> + /* Check mwid, mwidlist config */
> + if (worldguard_config != NULL) {
> + uint32_t valid_widlist = MAKE_64BIT_MASK(0,
worldguard_config->nworlds);
> +
> + /* CPU use default mwid / mwidlist config if not set */
> + if (cpu->cfg.mwidlist == UINT32_MAX) {
> + /* mwidlist contains all WIDs */
> + cpu->cfg.mwidlist = valid_widlist;
> + }
> + if (cpu->cfg.mwid == UINT32_MAX) {
> + cpu->cfg.mwid = trustedwid;
> + }
> +
> + /* Check if mwid/mwidlist HW config is valid in NWorld. */
> + g_assert((cpu->cfg.mwidlist & ~valid_widlist) == 0);
> + g_assert(cpu->cfg.mwid < worldguard_config->nworlds);
> + }
> +}
> +
> +/*
> + * riscv_worldguard_apply_cpu - Enable WG extension of CPU
> + *
> + * Note: This API should be used after global WG device is created
> + * (riscv_worldguard_realize()).
> + */
> +void riscv_worldguard_apply_cpu(uint32_t hartid)
> +{
> + /* WG global config should exist */
> + g_assert(worldguard_config);
We usually add g_asserts() after the variable declarations.
> +
> + CPUState *cpu = qemu_get_cpu(hartid);
arm_get_cpu() uses CPUState::cpu_index to obtain the corresponding CPUState
pointer.
However, CPUState::cpu_index and the RISC-V HART index are not necessarily
strictly
one-to-one (for instance, when the hartid base is non-zero or when hartids are
discontinuous).
Typically, we use arm_get_cpu() at the accelerators, rather than in hw/code.
A better approach is to use cpu_by_arch_id() instead of qemu_get_cpu(),
in RISC-V cpu_by_arch_id() uses the hartid.
e.g.
CPUState *cpu = cpu_by_arch_id(hartid);
Thanks,
Chao
> + RISCVCPU *rcpu = RISCV_CPU(cpu);
> + CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
> +
> + rcpu->cfg.ext_smwg = true;
> + if (riscv_has_ext(env, RVS) && riscv_has_ext(env, RVU)) {
> + rcpu->cfg.ext_sswg = true;
> + }
riscv_has_ext() will segfault if env == NULL, and you're creating a code
path where this might happen:
> + CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
In fact, cpu == NULL will explode on you earlier via this macro:
> + RISCVCPU *rcpu = RISCV_CPU(cpu);
You can either handle cpu == NULL with a clean exit before using 'cpu' to assign
stuff or g_assert(cpu != NULL) for a more rude exit. But with this code as is
you're gambling with segfaults.
Thanks,
Daniel
> +
> + /* Set machine specific WorldGuard callback */
> + env->wg_reset = riscv_cpu_wg_reset;
> + env->wid_to_mem_attrs = wid_to_mem_attrs;
> +
> + /* Reset WG CSRs in CPU */
> + env->wg_reset(env);
> +}
> +
> bool could_access_wgblocks(MemTxAttrs attrs, const char *wgblock)
> {
> uint32_t wid = mem_attrs_to_wid(attrs);
> diff --git a/include/hw/misc/riscv_worldguard.h
b/include/hw/misc/riscv_worldguard.h
> index 8a533a0517..211a72e438 100644
> --- a/include/hw/misc/riscv_worldguard.h
> +++ b/include/hw/misc/riscv_worldguard.h
> @@ -48,6 +48,7 @@ extern struct RISCVWorldGuardState *worldguard_config;
>
> DeviceState *riscv_worldguard_create(uint32_t nworlds, uint32_t trustedwid,
> bool hw_bypass, bool tz_compat);
> +void riscv_worldguard_apply_cpu(uint32_t hartid);
>
> uint32_t mem_attrs_to_wid(MemTxAttrs attrs);
> bool could_access_wgblocks(MemTxAttrs attrs, const char *wgblock);
