Re: [PATCH v4 04/11] disas/riscv: Support zcmop disassemble
On Tue, Jul 9, 2024 at 7:41 PM LIU Zhiwei wrote: > > Although in QEMU disassemble, we usually lift compressed instruction > to an normal format when display the instruction name. For C.MOP.n, > it is more reasonable to directly display its compressed name, because > its behavior can be redefined by later extension. > > Signed-off-by: LIU Zhiwei > Acked-by: Alistair Francis > Reviewed-by: Deepak Gupta > --- > disas/riscv.c | 23 +++ > 1 file changed, 23 insertions(+) Reviewed-by: Jim Shu
Re: [PATCH v4 03/11] target/riscv: Add zcmop extension
On Tue, Jul 9, 2024 at 7:40 PM LIU Zhiwei wrote: > > Zcmop defines eight 16-bit MOP instructions named C.MOP.n, where n is > an odd integer between 1 and 15, inclusive. C.MOP.n is encoded in > the reserved encoding space corresponding to C.LUI xn, 0. > > Unlike the MOPs defined in the Zimop extension, the C.MOP.n instructions > are defined to not write any register. > > In current implementation, C.MOP.n only has an check function, without any > other more behavior. > > Signed-off-by: LIU Zhiwei > Reviewed-by: Alistair Francis > Reviewed-by: Deepak Gupta > --- > target/riscv/cpu.c | 2 ++ > target/riscv/cpu_cfg.h | 1 + > target/riscv/insn16.decode | 1 + > target/riscv/insn_trans/trans_rvzcmop.c.inc | 29 + > target/riscv/tcg/tcg-cpu.c | 5 > target/riscv/translate.c| 1 + > 6 files changed, 39 insertions(+) > create mode 100644 target/riscv/insn_trans/trans_rvzcmop.c.inc > Reviewed-by: Jim Shu
Re: [PATCH v4 02/11] disas/riscv: Support zimop disassemble
On Tue, Jul 9, 2024 at 7:41 PM LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > Acked-by: Alistair Francis > Reviewed-by: Deepak Gupta > Reviewed-by: Jim Shu
Re: [PATCH v4 01/11] target/riscv: Add zimop extension
Reviewed-by: Jim Shu
On Tue, Jul 9, 2024 at 7:39 PM LIU Zhiwei
wrote:
> Zimop extension defines an encoding space for 40 MOPs.The Zimop
> extension defines 32 MOP instructions named MOP.R.n, where n is
> an integer between 0 and 31, inclusive. The Zimop extension
> additionally defines 8 MOP instructions named MOP.RR.n, where n
> is an integer between 0 and 7.
>
> These 40 MOPs initially are defined to simply write zero to x[rd],
> but are designed to be redefined by later extensions to perform some
> other action.
>
> Signed-off-by: LIU Zhiwei
> Reviewed-by: Alistair Francis
> Reviewed-by: Deepak Gupta
> ---
> target/riscv/cpu.c | 2 ++
> target/riscv/cpu_cfg.h | 1 +
> target/riscv/insn32.decode | 11 ++
> target/riscv/insn_trans/trans_rvzimop.c.inc | 37 +
> target/riscv/translate.c| 1 +
> 5 files changed, 52 insertions(+)
> create mode 100644 target/riscv/insn_trans/trans_rvzimop.c.inc
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index a2640cf259..d3853a5804 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -113,6 +113,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
> ISA_EXT_DATA_ENTRY(zihintntl, PRIV_VERSION_1_10_0, ext_zihintntl),
> ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause),
> ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_zihpm),
> +ISA_EXT_DATA_ENTRY(zimop, PRIV_VERSION_1_13_0, ext_zimop),
> ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul),
> ISA_EXT_DATA_ENTRY(za64rs, PRIV_VERSION_1_12_0, has_priv_1_11),
> ISA_EXT_DATA_ENTRY(zaamo, PRIV_VERSION_1_12_0, ext_zaamo),
> @@ -1471,6 +1472,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[]
> = {
> MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true),
> MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
> MULTI_EXT_CFG_BOOL("zihintpause", ext_zihintpause, true),
> +MULTI_EXT_CFG_BOOL("zimop", ext_zimop, false),
> MULTI_EXT_CFG_BOOL("zacas", ext_zacas, false),
> MULTI_EXT_CFG_BOOL("zaamo", ext_zaamo, false),
> MULTI_EXT_CFG_BOOL("zalrsc", ext_zalrsc, false),
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index fb7eebde52..9f53512053 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -71,6 +71,7 @@ struct RISCVCPUConfig {
> bool ext_zihintntl;
> bool ext_zihintpause;
> bool ext_zihpm;
> +bool ext_zimop;
> bool ext_ztso;
> bool ext_smstateen;
> bool ext_sstc;
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index f22df04cfd..60da673153 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -38,6 +38,8 @@
> %imm_bs 30:2 !function=ex_shift_3
> %imm_rnum 20:4
> %imm_z6 26:1 15:5
> +%imm_mop5 30:1 26:2 20:2
> +%imm_mop3 30:1 26:2
>
> # Argument sets:
>
> @@ -56,6 +58,8 @@
> vm rd rs1 nf
> vm rd rs1 rs2 nf
> _aes shamt rs2 rs1 rd
> + imm rd rs1
> + imm rd rs1 rs2
>
> # Formats 32:
> @r ... . . ... . ... %rs2
> %rs1 %rd
> @@ -98,6 +102,9 @@
> @k_aes .. . . . ... . ... _aes shamt=%imm_bs
> %rs2 %rs1 %rd
> @i_aes .. . . . ... . ... imm=%imm_rnum
> %rs1 %rd
>
> +@mop5 . . .. .. .. . ... . ... imm=%imm_mop5 %rd
> %rs1
> +@mop3 . . .. .. . . . ... . ... imm=%imm_mop3 %rd
> %rs1 %rs2
> +
> # Formats 64:
> @sh5 ... . . ... . ... shamt=%sh5
> %rs1 %rd
>
> @@ -1010,3 +1017,7 @@ amocas_w00101 . . . . 010 . 010
> @atom_st
> amocas_d00101 . . . . 011 . 010 @atom_st
> # *** RV64 Zacas Standard Extension ***
> amocas_q00101 . . . . 100 . 010 @atom_st
> +
> +# *** Zimop may-be-operation extension ***
> +mop_r_n 1 . 00 .. 0111 .. . 100 . 1110011 @mop5
> +mop_rr_n1 . 00 .. 1 . . 100 . 1110011 @mop3
> diff --git a/target/riscv/insn_trans/trans_rvzimop.c.inc
> b/target/riscv/insn_trans/trans_rvzimop.c.inc
> new file mode 100644
> index 00..165aacd2b6
> --- /dev/null
> +++ b/target/riscv/insn_trans/trans_rvzimop.c.inc
> @@ -0,0 +1,37 @@
> +/*
> + * RISC-V translation routines for May-Be-Operation(zimop).
> + *
> + * Copyright (c) 2024 Alibaba Group.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU G
Re: [RFC PATCH 01/16] accel/tcg: Store section pointer in CPUTLBEntryFull
Hi Zhiwei,
Common IOMMU devices will not have IOMMUMemoryRegion in the path of
CPU access since It only affects DMA access.
In QEMU, it usually places this IOMMU MR as the parent of
"system_memory", and changes the target_mr of DMA from "system_memory"
to IOMMU MR.
For the wgChecker, it is in front of memory or device MMIO and
protects both CPU/DMA access to memory or device MMIO.
In QEMU, wgChecker re-use IOMMUMemoryRegion to implement the memory
protection inside the translate() function of IOMMU MR.
In the machine code, wgChecker replaces the MemoryRegion of protected
resources with the checker's IOMMU MR in the MemoryRegion tree of
"system_memory".
Both CPU/DMA access will go through the "system_memory". They will go
through the checker's IOMMU MR when accessing the protected resources.
This mechanism is used by Cortex-M MPC devices (hw/misc/tz-mpc.c)
originally. I have leveraged it and extended it little (in patch 2) as
MPC doesn't support RO/WO permission.
If we'd like to have a device to do the memory protection of both CPU
& DMA access, we could implement it in this mechanism.
(p.s. Cortex-A TZASC is not supported in QEMU, which is similar to MPC
or wgChecker device.)
Thanks,
Jim Shu
On Thu, Jun 13, 2024 at 2:23 PM LIU Zhiwei wrote:
>
> On 2024/6/12 16:14, Jim Shu wrote:
> > 'CPUTLBEntryFull.xlat_section' stores section_index in last 12 bits to
> > find the correct section when CPU access the IO region over the IOTLB
> > (iotlb_to_section()).
> >
> > However, section_index is only unique inside single AddressSpace. If
> > address space translation is over IOMMUMemoryRegion, it could return
> > section from other AddressSpace. 'iotlb_to_section()' API only finds the
> > sections from CPU's AddressSpace so that it couldn't find section in
> > other AddressSpace. Thus, using 'iotlb_to_section()' API will find the
> > wrong section and QEMU will have wrong load/store access.
> >
> > To fix this bug, store complete MemoryRegionSection pointer in
> > CPUTLBEntryFull instead of section_index.
> >
> > This bug occurs only when
> > (1) IOMMUMemoryRegion is in the path of CPU access.
>
> Hi Jim,
>
> Can you explain a little more on when IOMMUMemoryRegion is in the path
> of CPU access?
>
> Thanks,
> Zhiwei
>
> > (2) IOMMUMemoryRegion returns different target_as and the section is in
> > the IO region.
> >
> > Common IOMMU devices don't have this issue since they are only in the
> > path of DMA access. Currently, the bug only occurs when ARM MPC device
> > (hw/misc/tz-mpc.c) returns 'blocked_io_as' to emulate blocked access
> > handling. Upcoming RISC-V wgChecker device is also affected by this bug.
> >
> > Signed-off-by: Jim Shu
> > ---
> > accel/tcg/cputlb.c| 19 +--
> > include/hw/core/cpu.h | 3 +++
> > 2 files changed, 12 insertions(+), 10 deletions(-)
> >
> > diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> > index 117b516739..8cf124b760 100644
> > --- a/accel/tcg/cputlb.c
> > +++ b/accel/tcg/cputlb.c
> > @@ -1169,6 +1169,7 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
> > desc->fulltlb[index] = *full;
> > full = >fulltlb[index];
> > full->xlat_section = iotlb - addr_page;
> > +full->section = section;
> > full->phys_addr = paddr_page;
> >
> > /* Now calculate the new entry */
> > @@ -1248,14 +1249,14 @@ static inline void cpu_unaligned_access(CPUState
> > *cpu, vaddr addr,
> > }
> >
> > static MemoryRegionSection *
> > -io_prepare(hwaddr *out_offset, CPUState *cpu, hwaddr xlat,
> > +io_prepare(hwaddr *out_offset, CPUState *cpu, CPUTLBEntryFull *full,
> > MemTxAttrs attrs, vaddr addr, uintptr_t retaddr)
> > {
> > MemoryRegionSection *section;
> > hwaddr mr_offset;
> >
> > -section = iotlb_to_section(cpu, xlat, attrs);
> > -mr_offset = (xlat & TARGET_PAGE_MASK) + addr;
> > +section = full->section;
> > +mr_offset = (full->xlat_section & TARGET_PAGE_MASK) + addr;
> > cpu->mem_io_pc = retaddr;
> > if (!cpu->neg.can_do_io) {
> > cpu_io_recompile(cpu, retaddr);
> > @@ -1571,9 +1572,7 @@ bool tlb_plugin_lookup(CPUState *cpu, vaddr addr, int
> > mmu_idx,
> >
> > /* We must have an iotlb entry for MMIO */
> > if (tlb_addr & TLB_MMIO) {
> > -MemoryRegionSection *section =
> > -iotlb_to_section(cpu, full->xlat_section & ~TARGET_PAGE_MASK,
> > - full->attrs);
> &g
Re: [RFC PATCH 02/16] accel/tcg: memory access from CPU will pass access_type to IOMMU
Hi Ethan, Yes, this mechanism could handle such situations. The important part is that the translate() function of IOMMUMemoryRegion only returns the correct permission of the section related to CPU access type. For example, if wgChecker only permits RO permission, it will return "downstream_as" with RO perm or "blocked_io_as" with WO perm. (depending on CPU access type). When the CPU access type is different from the previous one, it will get TLB miss due to mismatched permissions. Then, it will try to translate again, find the section of another access type, and fill into iotlb. (also kick out the previous iotlb entry). This mechanism has poor performance if alternatively doing read/write access from CPU to IOMMUMemoryRegion with RO/WO perm due to TLB miss. I think it is the limitation that CPUTLBEntry doesn't support having different sections of each permission. At least this mechanism has the correct behavior. Thanks, Jim On Thu, Jun 13, 2024 at 1:34 PM Ethan Chen wrote: > > On Wed, Jun 12, 2024 at 04:14:02PM +0800, Jim Shu wrote: > > [EXTERNAL MAIL] > > > > It is the preparation patch for upcoming RISC-V wgChecker device. > > > > Since RISC-V wgChecker could permit access in RO/WO permission, the > > IOMMUMemoryRegion could return different section for read & write > > access. The memory access from CPU should also pass the access_type to > > IOMMU translate function so that IOMMU could return the correct section > > of specified access_type. > > > > Hi Jim, > > Does this method take into account the situation where the CPU access type is > different from the access type when creating iotlb? I think the section > might be wrong in this situation. > > Thanks, > Ethan > > > >
[RFC PATCH 06/16] target/riscv: Add hard-coded CPU state of WG extension
Add hard-coded state of WG extension. 'mwid' is the M-mode WID of CPU.
'mwidlist' is the list of allowed WID value of 'mlwid' CSR.
These CPU states can be set by CPU option, or can be set by machine code
via newly added APIs. If we want different WG configs of CPUs, we should
set it by machine code.
Signed-off-by: Jim Shu
---
target/riscv/cpu.c| 2 ++
target/riscv/cpu.h| 2 ++
target/riscv/cpu_cfg.h| 2 ++
target/riscv/cpu_helper.c | 18 ++
4 files changed, 24 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d70eedf957..4e87fa4d5b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2291,6 +2291,8 @@ static Property riscv_cpu_properties[] = {
* it with -x and default to 'false'.
*/
DEFINE_PROP_BOOL("x-misa-w", RISCVCPU, cfg.misa_w, false),
+DEFINE_PROP_UINT32("mwid", RISCVCPU, cfg.mwid, UINT32_MAX),
+DEFINE_PROP_UINT32("mwidlist", RISCVCPU, cfg.mwidlist, UINT32_MAX),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 6fe0d712b4..2d3bfedbba 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -540,6 +540,8 @@ void riscv_cpu_set_aia_ireg_rmw_fn(CPURISCVState *env,
uint32_t priv,
void *rmw_fn_arg);
RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit);
+void riscv_cpu_set_wg_mwid(CPURISCVState *env, uint32_t mwid);
+void riscv_cpu_set_wg_mwidlist(CPURISCVState *env, uint32_t mwidlist);
#endif /* !CONFIG_USER_ONLY */
void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv);
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 23e779ae08..de9c134b15 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -166,6 +166,8 @@ struct RISCVCPUConfig {
bool pmp;
bool debug;
bool misa_w;
+uint32_t mwid;
+uint32_t mwidlist;
bool short_isa_string;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 161df34626..ff20ab6ab8 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -149,6 +149,24 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
*pflags = flags;
}
+#ifndef CONFIG_USER_ONLY
+void riscv_cpu_set_wg_mwid(CPURISCVState *env, uint32_t mwid)
+{
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
+
+cpu->cfg.mwid = mwid;
+}
+
+void riscv_cpu_set_wg_mwidlist(CPURISCVState *env, uint32_t mwidlist)
+{
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
+
+cpu->cfg.mwidlist = mwidlist;
+}
+#endif /* CONFIG_USER_ONLY */
+
void riscv_cpu_update_mask(CPURISCVState *env)
{
target_ulong mask = 0, base = 0;
--
2.17.1
[RFC PATCH 04/16] hw/misc: riscv_worldguard: Add RISC-V WorldGuard global config
Add a device for RISCV WG global config, which contains the number of
worlds, reset value, and trusted WID ... etc.
This global config is used by both CPU WG extension and wgChecker devices.
Signed-off-by: Jim Shu
---
hw/misc/Kconfig| 3 +
hw/misc/meson.build| 1 +
hw/misc/riscv_worldguard.c | 183 +
include/hw/misc/riscv_worldguard.h | 55 +
4 files changed, 242 insertions(+)
create mode 100644 hw/misc/riscv_worldguard.c
create mode 100644 include/hw/misc/riscv_worldguard.h
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index 1e08785b83..08fc0f2b8c 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -213,4 +213,7 @@ config IOSB
config XLNX_VERSAL_TRNG
bool
+config RISCV_WORLDGUARD
+bool
+
source macio/Kconfig
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 86596a3888..a75668ff86 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -34,6 +34,7 @@ system_ss.add(when: 'CONFIG_SIFIVE_E_PRCI', if_true:
files('sifive_e_prci.c'))
system_ss.add(when: 'CONFIG_SIFIVE_E_AON', if_true: files('sifive_e_aon.c'))
system_ss.add(when: 'CONFIG_SIFIVE_U_OTP', if_true: files('sifive_u_otp.c'))
system_ss.add(when: 'CONFIG_SIFIVE_U_PRCI', if_true: files('sifive_u_prci.c'))
+specific_ss.add(when: 'CONFIG_RISCV_WORLDGUARD', if_true:
files('riscv_worldguard.c'))
subdir('macio')
diff --git a/hw/misc/riscv_worldguard.c b/hw/misc/riscv_worldguard.c
new file mode 100644
index 00..c839cc4e87
--- /dev/null
+++ b/hw/misc/riscv_worldguard.c
@@ -0,0 +1,183 @@
+/*
+ * RISC-V WorldGuard Device
+ *
+ * Copyright (c) 2022 SiFive, Inc.
+ *
+ * This provides WorldGuard global config.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "exec/hwaddr.h"
+#include "hw/registerfields.h"
+#include "hw/sysbus.h"
+#include "hw/hw.h"
+#include "hw/qdev-properties.h"
+#include "hw/misc/riscv_worldguard.h"
+#include "hw/core/cpu.h"
+#include "target/riscv/cpu.h"
+#include "trace.h"
+
+/*
+ * WorldGuard global config:
+ * List the global setting of WG, like num-of-worlds. It is unique in the
machine.
+ * All CPUs with WG extension and wgChecker devices will use it.
+ */
+struct RISCVWorldGuardState *worldguard_config = NULL;
+
+static Property riscv_worldguard_properties[] = {
+DEFINE_PROP_UINT32("nworlds", RISCVWorldGuardState, nworlds, 0),
+
+/* Only Trusted WID could access wgCheckers if it is enabled. */
+DEFINE_PROP_UINT32("trustedwid", RISCVWorldGuardState, trustedwid,
NO_TRUSTEDWID),
+
+/*
+ * WG reset value is bypass mode in HW. All WG permission checkings are
+ * pass by default, so SW could correctly run on the machine w/o any WG
+ * programming.
+ */
+DEFINE_PROP_BOOL("hw-bypass", RISCVWorldGuardState, hw_bypass, false),
+
+/*
+ * TrustZone compatible mode:
+ * This mode is only supported in 2 worlds system. It converts WorldGuard
+ * WID to TZ NS signal on the bus so WG could be cooperated with
+ * TZ components. In QEMU, it converts WID to 'MemTxAttrs.secure' bit used
+ * by TZ.
+ */
+DEFINE_PROP_BOOL("tz-compat", RISCVWorldGuardState, tz_compat, false),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+/* WID to MemTxAttrs converter */
+static void wid_to_mem_attrs(MemTxAttrs *attrs, uint32_t wid)
+{
+g_assert(wid < worldguard_config->nworlds);
+
+attrs->unspecified = 0;
+if (worldguard_config->tz_compat) {
+attrs->secure = wid;
+} else {
+attrs->world_id = wid;
+}
+}
+
+/* MemTxAttrs to WID converter */
+uint32_t mem_attrs_to_wid(MemTxAttrs attrs)
+{
+if (attrs.unspecified) {
+if (worldguard_config->trustedwid != NO_TRUSTEDWID) {
+return worldguard_config->trustedwid;
+} else {
+return worldguard_config->nworlds - 1;
+}
+}
+
+if (worldguard_config->tz_compat) {
+return attrs.secure;
+} else {
+return attrs.world_id;
+}
+}
+
+bool could_access_wgblocks(MemTxAttrs attrs, const char *wgblock)
+{
+uint32_t wid = mem_attrs_to_wid(attrs);
+uint32_t trustedwid = worldguard_
[RFC PATCH 05/16] target/riscv: Add CPU options of WorldGuard CPU extension
We define CPU options for WG CSR support in RISC-V CPUs which
can be set by machine/device emulation. The RISC-V CSR emulation
will also check this feature for emulating WG CSRs.
Signed-off-by: Jim Shu
---
target/riscv/cpu.c | 8
target/riscv/cpu_cfg.h | 3 +++
target/riscv/tcg/tcg-cpu.c | 11 +++
3 files changed, 22 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 69a08e8c2c..d70eedf957 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -204,6 +204,9 @@ const RISCVIsaExtData isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(xtheadmempair, PRIV_VERSION_1_11_0, ext_xtheadmempair),
ISA_EXT_DATA_ENTRY(xtheadsync, PRIV_VERSION_1_11_0, ext_xtheadsync),
ISA_EXT_DATA_ENTRY(xventanacondops, PRIV_VERSION_1_12_0,
ext_XVentanaCondOps),
+ISA_EXT_DATA_ENTRY(smwg, PRIV_VERSION_1_12_0, ext_smwg),
+ISA_EXT_DATA_ENTRY(smwgd, PRIV_VERSION_1_12_0, ext_smwgd),
+ISA_EXT_DATA_ENTRY(sswg, PRIV_VERSION_1_12_0, ext_sswg),
DEFINE_PROP_END_OF_LIST(),
};
@@ -1595,6 +1598,11 @@ const RISCVCPUMultiExtConfig
riscv_cpu_experimental_exts[] = {
const RISCVCPUMultiExtConfig riscv_cpu_named_features[] = {
MULTI_EXT_CFG_BOOL("zic64b", ext_zic64b, true),
+/* RISC-V WorldGuard v0.4 */
+MULTI_EXT_CFG_BOOL("x-smwg", ext_smwg, false),
+MULTI_EXT_CFG_BOOL("x-smwgd", ext_smwgd, false),
+MULTI_EXT_CFG_BOOL("x-sswg", ext_sswg, false),
+
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index e1e4f32698..23e779ae08 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -120,6 +120,9 @@ struct RISCVCPUConfig {
bool ext_ssaia;
bool ext_sscofpmf;
bool ext_smepmp;
+bool ext_smwg;
+bool ext_smwgd;
+bool ext_sswg;
bool rvv_ta_all_1s;
bool rvv_ma_all_1s;
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 683f604d9f..dc86e6e1d5 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -726,6 +726,17 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu,
Error **errp)
cpu->pmu_avail_ctrs = 0;
}
+/* RISC-V WorldGuard */
+if (cpu->cfg.ext_sswg && !cpu->cfg.ext_smwg) {
+error_setg(errp, "Sswg extension requires Smwg extension");
+return;
+}
+
+if (cpu->cfg.ext_smwgd != cpu->cfg.ext_sswg) {
+error_setg(errp, "Smwgd/Sswg extensions should be enabled together");
+return;
+}
+
/*
* Disable isa extensions based on priv spec after we
* validated and set everything we need.
--
2.17.1
[RFC PATCH 11/16] hw/misc: riscv_worldguard: Add API to enable WG extension of CPU
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
---
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 c839cc4e87..836ba43239 100644
--- a/hw/misc/riscv_worldguard.c
+++ b/hw/misc/riscv_worldguard.c
@@ -93,6 +93,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);
+
+CPUState *cpu = qemu_get_cpu(hartid);
+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;
+}
+
+/* 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);
--
2.17.1
[RFC PATCH 10/16] target/riscv: Add WID to MemTxAttrs of CPU memory transactions
When a RISC-V HART has WG extension, their memory transactions will
contain WID. Support MemTxAttrs in RISC-V target and add WID inside if
a HART has WG extension.
Signed-off-by: Jim Shu
---
target/riscv/cpu.c| 2 +-
target/riscv/cpu.h| 1 +
target/riscv/cpu_helper.c | 51 ---
3 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ff1c22c71c..55d980ff4b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2343,7 +2343,7 @@ static int64_t riscv_get_arch_id(CPUState *cs)
#include "hw/core/sysemu-cpu-ops.h"
static const struct SysemuCPUOps riscv_sysemu_ops = {
-.get_phys_page_debug = riscv_cpu_get_phys_page_debug,
+.get_phys_page_attrs_debug = riscv_cpu_get_phys_page_attrs_debug,
.write_elf64_note = riscv_cpu_write_elf64_note,
.write_elf32_note = riscv_cpu_write_elf32_note,
.legacy_vmsd = _riscv_cpu,
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 43ab558111..588f5de7f7 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -512,6 +512,7 @@ void riscv_cpu_set_geilen(CPURISCVState *env, target_ulong
geilen);
bool riscv_cpu_vector_enabled(CPURISCVState *env);
void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
int riscv_env_mmu_index(CPURISCVState *env, bool ifetch);
+hwaddr riscv_cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
MemTxAttrs *attrs);
G_NORETURN void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
MMUAccessType access_type,
int mmu_idx, uintptr_t retaddr);
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index ff20ab6ab8..afdccdd672 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -150,6 +150,34 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
}
#ifndef CONFIG_USER_ONLY
+static uint32_t riscv_cpu_wg_get_wid(CPURISCVState *env, int mode)
+{
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
+bool virt = env->virt_enabled;
+
+if (mode == PRV_M) {
+return cpu->cfg.mwid;
+} else if (mode == PRV_S) {
+if (!virt || !env->mwiddeleg) {
+/* HS-mode, S-mode w/o RVH, or VS-mode but mwiddeleg = 0 */
+return env->mlwid;
+} else {
+/* VS-mode */
+return env->slwid;
+}
+} else if (mode == PRV_U) {
+if (!riscv_has_ext(env, RVS) || !env->mwiddeleg) {
+/* M/U mode CPU or mwiddeleg = 0 */
+return env->mlwid;
+} else {
+return env->slwid;
+}
+}
+
+return cpu->cfg.mwid;
+}
+
void riscv_cpu_set_wg_mwid(CPURISCVState *env, uint32_t mwid)
{
CPUState *cs = env_cpu(env);
@@ -1229,13 +1257,22 @@ static void raise_mmu_exception(CPURISCVState *env,
target_ulong address,
env->two_stage_indirect_lookup = two_stage_indirect;
}
-hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
+hwaddr riscv_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
MemTxAttrs *attrs)
{
RISCVCPU *cpu = RISCV_CPU(cs);
CPURISCVState *env = >env;
hwaddr phys_addr;
int prot;
int mmu_idx = riscv_env_mmu_index(>env, false);
+int mode = mmuidx_priv(mmu_idx);
+uint32_t wid;
+
+if (riscv_cpu_cfg(env)->ext_smwg && env->wid_to_mem_attrs) {
+wid = riscv_cpu_wg_get_wid(env, mode);
+env->wid_to_mem_attrs(attrs, wid);
+} else {
+*attrs = MEMTXATTRS_UNSPECIFIED;
+}
if (get_physical_address(env, _addr, , addr, NULL, 0, mmu_idx,
true, env->virt_enabled, true)) {
@@ -1339,12 +1376,20 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address,
int size,
int mode = mmuidx_priv(mmu_idx);
/* default TLB page size */
target_ulong tlb_size = TARGET_PAGE_SIZE;
+uint32_t wid;
+MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
env->guest_phys_fault_addr = 0;
qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
__func__, address, access_type, mmu_idx);
+if (riscv_cpu_cfg(env)->ext_smwg && env->wid_to_mem_attrs) {
+mode = mmuidx_priv(mmu_idx);
+wid = riscv_cpu_wg_get_wid(env, mode);
+env->wid_to_mem_attrs(, wid);
+}
+
pmu_tlb_fill_incr_ctr(cpu, access_type);
if (two_stage_lookup) {
/* Two stage lookup */
@@ -1436,8 +1481,8 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
}
if (ret == TRANSLATE_SUCCESS) {
-tlb_set_page(cs, address & ~(tlb_size - 1), pa & ~(tlb_size - 1),
- prot, access_type, mmu_idx, tlb_size);
+tlb_set_page_with_attrs(cs, address & ~(tlb_size - 1), pa & ~(tlb_size
- 1),
+
[RFC PATCH 13/16] hw/misc: riscv_wgchecker: Implement wgchecker slot registers
wgChecker slot is similar to PMP region. SW could program each slot to
configure the permission of address range.
Signed-off-by: Jim Shu
---
hw/misc/riscv_wgchecker.c | 330 +
hw/misc/riscv_worldguard.c | 3 +
include/hw/misc/riscv_worldguard.h | 4 +
3 files changed, 337 insertions(+)
diff --git a/hw/misc/riscv_wgchecker.c b/hw/misc/riscv_wgchecker.c
index 2421b1438c..ab03fd671f 100644
--- a/hw/misc/riscv_wgchecker.c
+++ b/hw/misc/riscv_wgchecker.c
@@ -53,6 +53,52 @@ REG64(ERRCAUSE, 0x010)
R_ERRCAUSE_IP_MASK)
REG64(ERRADDR, 0x018)
+REG64(WGC_SLOT, 0x020)
+
+/* wgChecker slots */
+REG64(SLOT_ADDR,0x000)
+REG64(SLOT_PERM,0x008)
+REG32(SLOT_CFG, 0x010)
+FIELD(SLOT_CFG, A, 0, 2)
+FIELD(SLOT_CFG, ER, 8, 1)
+FIELD(SLOT_CFG, EW, 9, 1)
+FIELD(SLOT_CFG, IR, 10, 1)
+FIELD(SLOT_CFG, IW, 11, 1)
+FIELD(SLOT_CFG, LOCK, 31, 1)
+
+#define SLOT_SIZE 0x020
+
+#define SLOT0_CFG_MASK \
+(R_SLOT_CFG_ER_MASK | \
+ R_SLOT_CFG_EW_MASK | \
+ R_SLOT_CFG_IR_MASK | \
+ R_SLOT_CFG_IW_MASK | \
+ R_SLOT_CFG_LOCK_MASK)
+
+#define SLOT_CFG_MASK \
+(R_SLOT_CFG_A_MASK | (SLOT0_CFG_MASK))
+
+#define WGC_SLOT_END(nslots) \
+(A_WGC_SLOT + SLOT_SIZE * (nslots + 1))
+
+/* wgChecker slot is 4K alignment */
+#define WG_ALIGNED_SIZE (1 << 12)
+#define WG_ALIGNED_MASK MAKE_64BIT_MASK(0, 12)
+
+/* wgChecker slot address is (addr / 4). */
+#define TO_SLOT_ADDR(addr) ((addr) >> 2)
+#define FROM_SLOT_ADDR(addr)((addr) << 2)
+
+/* wgChecker slot cfg.A[1:0] */
+#define A_OFF 0
+#define A_TOR 1
+#define A_NA4 2
+#define A_NAPOT 3
+
+/* wgChecker slot perm */
+#define WGC_PERM(wid, perm) ((uint64_t)(perm) << (2 * (wid)))
+#define P_READ (1 << 0)
+#define P_WRITE (1 << 1)
/*
* Accesses only reach these read and write functions if the wgChecker
@@ -146,6 +192,28 @@ static uint64_t riscv_wgchecker_readq(void *opaque, hwaddr
addr)
RISCVWgCheckerState *s = RISCV_WGCHECKER(opaque);
uint64_t val = 0;
+if ((addr >= A_WGC_SLOT) && (addr < WGC_SLOT_END(s->slot_count))) {
+/* Read from WGC slot */
+int slot_id = (addr - A_WGC_SLOT) / SLOT_SIZE;
+int slot_offset = (addr - A_WGC_SLOT) % SLOT_SIZE;
+
+switch (slot_offset) {
+case A_SLOT_ADDR:
+val = s->slots[slot_id].addr;
+break;
+case A_SLOT_PERM:
+val = s->slots[slot_id].perm;
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Unexpected memory access to (0x%" HWADDR_PRIX
", %u) \n",
+ __func__, addr, 8);
+break;
+}
+
+return val;
+}
+
switch (addr) {
case A_ERRCAUSE:
val = s->errcause & ERRCAUSE_MASK;
@@ -171,6 +239,37 @@ static uint64_t riscv_wgchecker_readl(void *opaque, hwaddr
addr)
RISCVWgCheckerState *s = RISCV_WGCHECKER(opaque);
uint64_t val = 0;
+if ((addr >= A_WGC_SLOT) && (addr < WGC_SLOT_END(s->slot_count))) {
+/* Read from WGC slot */
+int slot_id = (addr - A_WGC_SLOT) / SLOT_SIZE;
+int slot_offset = (addr - A_WGC_SLOT) % SLOT_SIZE;
+
+switch (slot_offset) {
+case A_SLOT_ADDR:
+val = extract64(s->slots[slot_id].addr, 0, 32);
+break;
+case A_SLOT_ADDR + 4:
+val = extract64(s->slots[slot_id].addr, 32, 32);
+break;
+case A_SLOT_PERM:
+val = extract64(s->slots[slot_id].perm, 0, 32);
+break;
+case A_SLOT_PERM + 4:
+val = extract64(s->slots[slot_id].perm, 32, 32);
+break;
+case A_SLOT_CFG:
+val = s->slots[slot_id].cfg & SLOT_CFG_MASK;
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Unexpected memory access to (0x%" HWADDR_PRIX
", %u) \n",
+ __func__, addr, 4);
+break;
+}
+
+return val;
+}
+
switch (addr) {
case A_VENDOR:
val = 0;
@@ -228,11 +327,121 @@ static uint64_t riscv_wgchecker_read(void *opaque,
hwaddr addr, unsigned size)
return val;
}
+/*
+ * Validate the WGC slot address is between address range.
+ *
+ * Fix the slot address to the start address if it's not within the address
range.
+ * We need validation when changing "slot address" or "TOR/NAPOT mode (cfg.A)"
+ */
+static void validate_slot_address(void *opaque, int slot_id)
+{
+RISCVWgCheckerState *s = RISCV_WGCHECKER
[RFC PATCH 16/16] hw/riscv: virt: Add WorldGuard support
* Add 'wg=on' option to enable RISC-V WorldGuard
* Add wgChecker to protect several resources:
DRAM, FLASH, UART.
Signed-off-by: Jim Shu
---
docs/system/riscv/virt.rst | 10 +++
hw/riscv/Kconfig | 1 +
hw/riscv/virt.c| 163 -
include/hw/riscv/virt.h| 17 +++-
4 files changed, 186 insertions(+), 5 deletions(-)
diff --git a/docs/system/riscv/virt.rst b/docs/system/riscv/virt.rst
index 9a06f95a34..2d2992dc34 100644
--- a/docs/system/riscv/virt.rst
+++ b/docs/system/riscv/virt.rst
@@ -116,6 +116,16 @@ The following machine-specific options are supported:
having AIA IMSIC (i.e. "aia=aplic-imsic" selected). When not specified,
the default number of per-HART VS-level AIA IMSIC pages is 0.
+- wg=[on|off]
+
+ When this option is "on", RISC-V WorldGuard will be enabled in the system
+ to provide the isolation of multiple worlds. RISC-V HARTS will enable WG
+ extensions to have WID in memory transaction. wgCheckers in front of RAMs
+ and device MMIO will be enabled to provide the access control of resources
+ if the transaction contains WID. When not specified, this option is assumed
+ to be "off".
+ This option is restricted to the TCG accelerator.
+
Running Linux kernel
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index a2030e3a6f..7804fdbb7a 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -56,6 +56,7 @@ config RISCV_VIRT
select PLATFORM_BUS
select ACPI
select ACPI_PCI
+select RISCV_WORLDGUARD
config SHAKTI_C
bool
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 4fdb660525..eed49ebd02 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -55,6 +55,7 @@
#include "hw/acpi/aml-build.h"
#include "qapi/qapi-visit-common.h"
#include "hw/virtio/virtio-iommu.h"
+#include "hw/misc/riscv_worldguard.h"
/* KVM AIA only supports APLIC MSI. APLIC Wired is always emulated by QEMU. */
static bool virt_use_kvm_aia(RISCVVirtState *s)
@@ -76,6 +77,9 @@ static const MemMapEntry virt_memmap[] = {
[VIRT_ACLINT_SSWI] = { 0x2F0,0x4000 },
[VIRT_PCIE_PIO] = { 0x300, 0x1 },
[VIRT_PLATFORM_BUS] = { 0x400, 0x200 },
+[VIRT_WGC_DRAM] = { 0x600,0x1000 },
+[VIRT_WGC_FLASH] ={ 0x6001000,0x1000 },
+[VIRT_WGC_UART] = { 0x6002000,0x1000 },
[VIRT_PLIC] = { 0xc00, VIRT_PLIC_SIZE(VIRT_CPUS_MAX * 2) },
[VIRT_APLIC_M] = { 0xc00, APLIC_SIZE(VIRT_CPUS_MAX) },
[VIRT_APLIC_S] = { 0xd00, APLIC_SIZE(VIRT_CPUS_MAX) },
@@ -101,6 +105,38 @@ static MemMapEntry virt_high_pcie_memmap;
#define VIRT_FLASH_SECTOR_SIZE (256 * KiB)
+/* wgChecker helpers */
+typedef struct WGCInfo {
+int memmap_idx;
+uint32_t irq_num;
+uint32_t slot_count;
+
+int num_of_child;
+MemoryRegion *c_region[WGC_NUM_REGIONS];
+uint64_t c_offset[WGC_NUM_REGIONS];
+} WGCInfo;
+
+enum {
+WGC_DRAM,
+WGC_FLASH,
+WGC_UART,
+WGC_NUM,
+};
+
+static WGCInfo virt_wgcinfo[] = {
+[WGC_DRAM] = { VIRT_WGC_DRAM, WGC_DRAM_IRQ, 16 },
+[WGC_FLASH] = { VIRT_WGC_FLASH, WGC_FLASH_IRQ, 16 },
+[WGC_UART] = { VIRT_WGC_UART, WGC_UART_IRQ, 1 },
+};
+
+static void wgc_append_child(WGCInfo *info, MemoryRegion *region,
+ uint64_t offset)
+{
+info->c_region[info->num_of_child] = region;
+info->c_offset[info->num_of_child] = offset;
+info->num_of_child += 1;
+}
+
static PFlashCFI01 *virt_flash_create1(RISCVVirtState *s,
const char *name,
const char *alias_prop_name)
@@ -151,7 +187,8 @@ static void virt_flash_map1(PFlashCFI01 *flash,
}
static void virt_flash_map(RISCVVirtState *s,
- MemoryRegion *sysmem)
+ MemoryRegion *sysmem,
+ WGCInfo *info)
{
hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
@@ -160,6 +197,15 @@ static void virt_flash_map(RISCVVirtState *s,
sysmem);
virt_flash_map1(s->flash[1], flashbase + flashsize, flashsize,
sysmem);
+
+if (info) {
+wgc_append_child(info,
+ sysbus_mmio_get_region(SYS_BUS_DEVICE(s->flash[0]),
0),
+ flashbase);
+wgc_append_child(info,
+ sysbus_mmio_get_region(SYS_BUS_DEVICE(s->flash[1]),
0),
+ flashbase + flashsize);
+}
}
static void create_pcie_irq_map(RISCVVirtState *s, void *fdt, char *nodename,
@@ -1303,6 +1349,71 @@ static void virt_build_smbios(RISCVVirtState *s)
}
}
+static DeviceState *create_wgc(WGCInfo *info, DeviceState *irqchip)
+{
+MemoryRegion *system_memor
[RFC PATCH 14/16] hw/misc: riscv_wgchecker: Implement correct block-access behavior
The wgChecker is configurable for whether blocked accesses:
* should cause a bus error or just read return zero and write ignore
* should generate the interrupt or not
Signed-off-by: Jim Shu
---
hw/misc/riscv_wgchecker.c | 169 +-
1 file changed, 167 insertions(+), 2 deletions(-)
diff --git a/hw/misc/riscv_wgchecker.c b/hw/misc/riscv_wgchecker.c
index ab03fd671f..55e5e8127f 100644
--- a/hw/misc/riscv_wgchecker.c
+++ b/hw/misc/riscv_wgchecker.c
@@ -100,6 +100,169 @@ REG32(SLOT_CFG, 0x010)
#define P_READ (1 << 0)
#define P_WRITE (1 << 1)
+static void decode_napot(hwaddr a, hwaddr *sa, hwaddr *ea)
+{
+/*
+ * ...aaa0 8-byte NAPOT range
+ * ...aa01 16-byte NAPOT range
+ * ...a011 32-byte NAPOT range
+ * ...
+ * aa01... 2^XLEN-byte NAPOT range
+ * a011... 2^(XLEN+1)-byte NAPOT range
+ * 0111... 2^(XLEN+2)-byte NAPOT range
+ * ... Reserved
+ */
+
+a = FROM_SLOT_ADDR(a) | 0x3;
+
+if (sa) {
+*sa = a & (a + 1);
+}
+if (ea) {
+*ea = a | (a + 1);
+}
+}
+
+typedef struct WgAccessResult WgAccessResult;
+struct WgAccessResult {
+bool is_success;
+bool has_bus_error;
+bool has_interrupt;
+uint8_t perm:2;
+};
+
+static WgAccessResult wgc_check_access(
+RISCVWgCheckerState *s, hwaddr phys_addr, uint32_t wid, bool is_write)
+{
+WgCheckerSlot *slot, *prev_slot;
+uint32_t cfg_a, prev_cfg_a;
+uint64_t start, end;
+int slot_id, wgc_perm = 0;
+WgAccessResult result = { 0 };
+
+bool is_matching = false;
+bool slot0_be, slot0_ip;
+bool matched_slot_be = false, matched_slot_ip = false;
+
+for (slot_id = 0; slot_id < s->slot_count; slot_id++) {
+slot = >slots[slot_id + 1];
+cfg_a = FIELD_EX32(slot->cfg, SLOT_CFG, A);
+
+if (cfg_a == A_TOR) {
+prev_slot = >slots[slot_id];
+
+prev_cfg_a = FIELD_EX32(prev_slot->cfg, SLOT_CFG, A);
+if (prev_cfg_a == A_NA4) {
+start = FROM_SLOT_ADDR(prev_slot->addr) + 4;
+} else if (prev_cfg_a == A_NAPOT) {
+decode_napot(prev_slot->addr, NULL, );
+start += 1;
+} else { /* A_TOR or A_OFF */
+start = FROM_SLOT_ADDR(prev_slot->addr);
+}
+end = FROM_SLOT_ADDR(slot->addr);
+} else if (cfg_a == A_NA4) {
+start = FROM_SLOT_ADDR(slot->addr);
+end = start + 4;
+} else if (cfg_a == A_NAPOT) {
+decode_napot(slot->addr, , );
+end += 1;
+} else {
+/* A_OFF: not in slot range. */
+continue;
+}
+
+/* wgChecker slot range is between start to (end - 1). */
+if ((start <= phys_addr) && (phys_addr < end)) {
+/* Match the wgC slot */
+int perm = ((slot->perm >> (wid * 2)) & 0x3);
+
+/* If any matching rule permits access, the access is permitted. */
+wgc_perm |= perm;
+
+/*
+ * If any matching rule wants to report error (IRQ or Bus Error),
+ * the denied access should report error.
+ */
+is_matching = true;
+if (is_write) {
+matched_slot_be |= FIELD_EX64(slot->cfg, SLOT_CFG, EW);
+matched_slot_ip |= FIELD_EX64(slot->cfg, SLOT_CFG, IW);
+} else {
+matched_slot_be |= FIELD_EX64(slot->cfg, SLOT_CFG, ER);
+matched_slot_ip |= FIELD_EX64(slot->cfg, SLOT_CFG, IR);
+}
+}
+}
+
+/* If no matching rule, error reporting depends on the slot0's config. */
+if (is_write) {
+slot0_be = FIELD_EX64(s->slots[0].cfg, SLOT_CFG, EW);
+slot0_ip = FIELD_EX64(s->slots[0].cfg, SLOT_CFG, IW);
+} else {
+slot0_be = FIELD_EX64(s->slots[0].cfg, SLOT_CFG, ER);
+slot0_ip = FIELD_EX64(s->slots[0].cfg, SLOT_CFG, IR);
+}
+
+result.is_success = is_write ? (wgc_perm & P_WRITE) : (wgc_perm & P_READ);
+result.perm = wgc_perm;
+if (!result.is_success) {
+if (is_matching) {
+result.has_bus_error = matched_slot_be;
+result.has_interrupt = matched_slot_ip;
+} else {
+result.has_bus_error = slot0_be;
+result.has_interrupt = slot0_ip;
+}
+}
+
+return result;
+}
+
+static MemTxResult riscv_wgc_handle_blocked_access(
+WgCheckerRegion *region, hwaddr addr, uint32_t wid, bool is_write)
+{
+RISCVWgCheckerState *s = RISCV_WGCHECKER(region->wgchecker);
+bool be, ip;
+WgAccessResult result;
+hwaddr phys_addr;
+
+be = FIELD_EX64(s->errcause, ERRCAUSE, BE);
+ip = FIELD_EX64(s->errcause, ERRCAUSE, IP);
+phys_addr = addr +
[RFC PATCH 07/16] target/riscv: Add defines for WorldGuard CSRs
Add CSRs for 3 WG extensions: Smwg, Smwgd, and Sswg. Signed-off-by: Jim Shu --- target/riscv/cpu_bits.h | 5 + 1 file changed, 5 insertions(+) diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h index 74318a925c..3ea8a8e9a0 100644 --- a/target/riscv/cpu_bits.h +++ b/target/riscv/cpu_bits.h @@ -360,6 +360,11 @@ #define CSR_DPC 0x7b1 #define CSR_DSCRATCH0x7b2 +/* RISC-V WorldGuard */ +#define CSR_MLWID 0x390 +#define CSR_SLWID 0x190 +#define CSR_MWIDDELEG 0x748 + /* Performance Counters */ #define CSR_MHPMCOUNTER30xb03 #define CSR_MHPMCOUNTER40xb04 -- 2.17.1
[RFC PATCH 09/16] target/riscv: Implement WorldGuard CSRs
The WG v0.4 specification adds 3 CSRs to configure S/U/HS/VS-mode WIDs
of CPUs in the higher privileged modes.
The Smwg extension at least requires a RISC-V HART to have M/U-mode, and
the Sswg/Smwgd extension at least requires a RISC-V HART to have
M/S/U-mode.
Signed-off-by: Jim Shu
---
target/riscv/cpu.c | 4 ++
target/riscv/cpu.h | 5 +++
target/riscv/csr.c | 107 +
3 files changed, 116 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4e87fa4d5b..ff1c22c71c 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1022,6 +1022,10 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType
type)
riscv_trigger_reset_hold(env);
}
+if (riscv_cpu_cfg(env)->ext_smwg && env->wg_reset) {
+env->wg_reset(env);
+}
+
if (kvm_enabled()) {
kvm_riscv_reset_vcpu(cpu);
}
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 50a0fba127..43ab558111 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -432,6 +432,11 @@ struct CPUArchState {
uint64_t kvm_timer_frequency;
#endif /* CONFIG_KVM */
+/* RISC-V WorldGuard */
+target_ulong mlwid;
+target_ulong slwid;
+target_ulong mwiddeleg;
+
/* machine specific WorldGuard callback */
void (*wg_reset)(CPURISCVState *env);
void (*wid_to_mem_attrs)(MemTxAttrs *attrs, uint32_t wid);
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 58ef7079dc..f3536e9e5d 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -4264,6 +4264,109 @@ static RISCVException write_upmbase(CPURISCVState *env,
int csrno,
return RISCV_EXCP_NONE;
}
+/* RISC-V Worldguard */
+static RISCVException worldguard_umode(CPURISCVState *env, int csrno)
+{
+if (!riscv_cpu_cfg(env)->ext_smwg) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+return umode(env, csrno);
+}
+
+static RISCVException worldguard_sumode(CPURISCVState *env, int csrno)
+{
+RISCVException ret;
+
+if (!riscv_cpu_cfg(env)->ext_sswg) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+ret = smode(env, csrno);
+
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+
+return umode(env, csrno);
+}
+
+static RISCVException rmw_mlwid(CPURISCVState *env, int csrno,
+target_ulong *ret_val,
+target_ulong new_val, target_ulong wr_mask)
+{
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
+target_ulong new_mlwid = (env->mlwid & ~wr_mask) | (new_val & wr_mask);
+
+if (ret_val) {
+*ret_val = env->mlwid;
+}
+
+g_assert(cpu->cfg.mwidlist);
+if (!(BIT(new_mlwid) & cpu->cfg.mwidlist)) {
+/* Set WID to lowest legal value if writing illegal value (WARL) */
+new_mlwid = find_first_bit((unsigned long *)>cfg.mwidlist, 32);
+}
+
+if (env->mlwid != new_mlwid) {
+env->mlwid = new_mlwid;
+tlb_flush(cs);
+}
+
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException rmw_slwid(CPURISCVState *env, int csrno,
+target_ulong *ret_val,
+target_ulong new_val, target_ulong wr_mask)
+{
+target_ulong new_slwid = (env->slwid & ~wr_mask) | (new_val & wr_mask);
+
+if (!env->mwiddeleg) {
+/*
+ * When mwiddeleg CSR is zero, access to slwid raises an illegal
+ * instruction exception.
+ */
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+if (ret_val) {
+*ret_val = env->slwid;
+}
+
+if (!(BIT(new_slwid) & env->mwiddeleg)) {
+/* Set WID to lowest legal value if writing illegal value (WARL) */
+new_slwid = find_first_bit(
+(unsigned long *)>mwiddeleg, TARGET_LONG_BITS);
+}
+
+if (env->slwid != new_slwid) {
+env->slwid = new_slwid;
+tlb_flush(env_cpu(env));
+}
+
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException rmw_mwiddeleg(CPURISCVState *env, int csrno,
+target_ulong *ret_val,
+target_ulong new_val, target_ulong wr_mask)
+{
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
+
+if (ret_val) {
+*ret_val = env->mwiddeleg;
+}
+
+env->mwiddeleg = (env->mwiddeleg & ~wr_mask) | (new_val & wr_mask);
+
+/* Core wgMarker can only have WID value in mwidlist. */
+env->mwiddeleg &= cpu->cfg.mwidlist;
+
+return RISCV_EXCP_NONE;
+}
#endif
/* Crypto Extension */
@@ -5230,5 +5333,9 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_SCOUNTOVF] = { "scountovf", sscofpmf, read_scountovf,
.min_priv_ver = PRIV_VERSION_1_12_0 },
+/* RISC-V WorldGuard */
+[CSR_MLWID] = { "mlwid", worldguard_umode, NULL, NU
[RFC PATCH 15/16] hw/misc: riscv_wgchecker: Check the slot settings in translate
The final part of wgChecker we need to implement is actually using the
wgChecker slots programmed by guest to determine whether to block the
transaction or not.
Since this means we now change transaction mappings when
the guest writes to wgChecker slots, we must also call the IOMMU
notifiers at that point.
One tricky part here is that the perm of 'blocked_io_as' is the
condition of deny access. For example, if wgChecker only permits RO
access, the perm of 'downstream_as' will be IOMMU_RO and the perm of
'blocked_io_as' will be IOMMU_WO.
Signed-off-by: Jim Shu
---
hw/misc/riscv_wgchecker.c | 70 ---
hw/misc/trace-events | 1 +
2 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/hw/misc/riscv_wgchecker.c b/hw/misc/riscv_wgchecker.c
index 55e5e8127f..cab4e40921 100644
--- a/hw/misc/riscv_wgchecker.c
+++ b/hw/misc/riscv_wgchecker.c
@@ -100,6 +100,52 @@ REG32(SLOT_CFG, 0x010)
#define P_READ (1 << 0)
#define P_WRITE (1 << 1)
+static IOMMUAccessFlags wgc_perm_to_iommu_flags(int wgc_perm)
+{
+if (wgc_perm == (P_READ | P_WRITE)) {
+return IOMMU_RW;
+} else if (wgc_perm & P_WRITE) {
+return IOMMU_WO;
+} else if (wgc_perm & P_READ) {
+return IOMMU_RO;
+} else {
+return IOMMU_NONE;
+}
+}
+
+static void wgchecker_iommu_notify_all(RISCVWgCheckerState *s)
+{
+/*
+ * Do tlb_flush() to whole address space via memory_region_notify_iommu()
+ * when wgChecker changes it's config.
+ */
+
+IOMMUTLBEvent event = {
+.entry = {
+.addr_mask = -1ULL,
+}
+};
+
+trace_riscv_wgc_iommu_notify_all();
+
+for (int i=0; imem_regions[i];
+uint32_t nworlds = worldguard_config->nworlds;
+
+if (!region->downstream) {
+continue;
+}
+event.entry.iova = 0;
+event.entry.translated_addr = 0;
+event.type = IOMMU_NOTIFIER_UNMAP;
+event.entry.perm = IOMMU_NONE;
+
+for (int wid=0; widupstream, wid, event);
+}
+}
+}
+
static void decode_napot(hwaddr a, hwaddr *sa, hwaddr *ea)
{
/*
@@ -309,6 +355,9 @@ static IOMMUTLBEntry riscv_wgc_translate(IOMMUMemoryRegion
*iommu,
{
WgCheckerRegion *region = container_of(iommu, WgCheckerRegion, upstream);
RISCVWgCheckerState *s = RISCV_WGCHECKER(region->wgchecker);
+bool is_write;
+WgAccessResult result;
+int wgc_perm;
hwaddr phys_addr;
uint64_t region_size;
@@ -327,18 +376,25 @@ static IOMMUTLBEntry
riscv_wgc_translate(IOMMUMemoryRegion *iommu,
* Look at the wgChecker configuration for this address, and
* return a TLB entry directing the transaction at either
* downstream_as or blocked_io_as, as appropriate.
- * For the moment, always permit accesses.
*/
/* Use physical address instead of offset */
phys_addr = addr + region->region_offset;
+is_write = (flags == IOMMU_WO);
-is_success = true;
+result = wgc_check_access(s, phys_addr, iommu_idx, is_write);
trace_riscv_wgc_translate(phys_addr, flags,
-iommu_idx, is_success ? "pass" : "block");
+iommu_idx, result.is_success ? "pass" : "block");
-ret.target_as = is_success ? >downstream_as :
>blocked_io_as;
+wgc_perm = result.perm;
+if (!result.is_success) {
+/* if target_as is blocked_io_as, the perm is the condition of deny
access. */
+wgc_perm ^= (P_READ | P_WRITE);
+}
+ret.perm = wgc_perm_to_iommu_flags(wgc_perm);
+
+ret.target_as = result.is_success ? >downstream_as :
>blocked_io_as;
return ret;
}
@@ -604,6 +660,9 @@ static void riscv_wgchecker_writeq(void *opaque, hwaddr
addr,
break;
}
+/* Flush softmmu TLB when wgChecker changes config. */
+wgchecker_iommu_notify_all(s);
+
return;
}
@@ -699,6 +758,9 @@ static void riscv_wgchecker_writel(void *opaque, hwaddr
addr,
break;
}
+/* Flush softmmu TLB when wgChecker changes config. */
+wgchecker_iommu_notify_all(s);
+
return;
}
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index a64c7f0f9f..80907c45d2 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -359,3 +359,4 @@ riscv_wgchecker_mmio_write(uint64_t base, uint64_t offset,
unsigned int size, ui
riscv_wgc_mem_blocked_read(uint64_t addr, unsigned size, uint32_t wid)
"wgChecker blocked read: offset 0x%" PRIx64 " size %u wid %" PRIu32
riscv_wgc_mem_blocked_write(uint64_t addr, uint64_t data, unsigned size,
uint32_t wid) "wgChecker blocked write: offset 0x%" PRIx64 " data 0x%" PRIx64 "
size %u wid %" PRIu32
riscv_wgc_translate(uint64_t addr, int flags, int wid, const char *res)
"wgChecker translate: addr 0x%016" PRIx64 " flags 0x%x wid %d: %s"
+riscv_wgc_iommu_notify_all(void) "wgChecker iommu: notifying UNMAP for all"
--
2.17.1
[RFC PATCH 12/16] hw/misc: riscv_wgchecker: Implement RISC-V WorldGuard Checker
Implement the RISC-V WorldGuard Checker, which sits in front of RAM or
device MMIO and allow software to configure it to either pass through or
reject transactions.
We implement the wgChecker as a QEMU IOMMU, which will direct transactions
either through to the devices and memory behind it or to a special
"never works" AddressSpace if they are blocked.
This initial commit implements the skeleton of the device:
* it always permits accesses
* it doesn't implement wgChecker's slot registers
* it doesn't implement the interrupt or other behaviour
for blocked transactions
Signed-off-by: Jim Shu
---
hw/misc/meson.build| 2 +-
hw/misc/riscv_wgchecker.c | 604 +
hw/misc/trace-events | 8 +
include/hw/misc/riscv_worldguard.h | 63 +++
4 files changed, 676 insertions(+), 1 deletion(-)
create mode 100644 hw/misc/riscv_wgchecker.c
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index a75668ff86..c5cb3bf4ee 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -34,7 +34,7 @@ system_ss.add(when: 'CONFIG_SIFIVE_E_PRCI', if_true:
files('sifive_e_prci.c'))
system_ss.add(when: 'CONFIG_SIFIVE_E_AON', if_true: files('sifive_e_aon.c'))
system_ss.add(when: 'CONFIG_SIFIVE_U_OTP', if_true: files('sifive_u_otp.c'))
system_ss.add(when: 'CONFIG_SIFIVE_U_PRCI', if_true: files('sifive_u_prci.c'))
-specific_ss.add(when: 'CONFIG_RISCV_WORLDGUARD', if_true:
files('riscv_worldguard.c'))
+specific_ss.add(when: 'CONFIG_RISCV_WORLDGUARD', if_true:
files('riscv_worldguard.c', 'riscv_wgchecker.c'))
subdir('macio')
diff --git a/hw/misc/riscv_wgchecker.c b/hw/misc/riscv_wgchecker.c
new file mode 100644
index 00..2421b1438c
--- /dev/null
+++ b/hw/misc/riscv_wgchecker.c
@@ -0,0 +1,604 @@
+/*
+ * RISC-V WorldGuard Checker Device
+ *
+ * Copyright (c) 2022 SiFive, Inc.
+ *
+ * This provides WorldGuard Checker model.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "exec/hwaddr.h"
+#include "exec/exec-all.h"
+#include "hw/irq.h"
+#include "hw/registerfields.h"
+#include "hw/sysbus.h"
+#include "hw/hw.h"
+#include "hw/qdev-properties.h"
+#include "hw/misc/riscv_worldguard.h"
+#include "target/riscv/cpu.h"
+#include "trace.h"
+
+/* Common */
+REG32(VENDOR, 0x000)
+REG32(IMPID,0x004)
+
+/* wgChecker */
+REG32(NSLOTS, 0x008)
+REG64(ERRCAUSE, 0x010)
+FIELD(ERRCAUSE, WID,0, 8)
+FIELD(ERRCAUSE, R, 8, 1)
+FIELD(ERRCAUSE, W, 9, 1)
+FIELD(ERRCAUSE, BE, 62, 1)
+FIELD(ERRCAUSE, IP, 63, 1)
+
+#define ERRCAUSE_MASK \
+(R_ERRCAUSE_WID_MASK | \
+ R_ERRCAUSE_R_MASK | \
+ R_ERRCAUSE_W_MASK | \
+ R_ERRCAUSE_BE_MASK | \
+ R_ERRCAUSE_IP_MASK)
+
+REG64(ERRADDR, 0x018)
+
+/*
+ * Accesses only reach these read and write functions if the wgChecker
+ * is blocking them; non-blocked accesses go directly to the downstream
+ * memory region without passing through this code.
+ */
+static MemTxResult riscv_wgc_mem_blocked_read(void *opaque, hwaddr addr,
+ uint64_t *pdata,
+ unsigned size, MemTxAttrs attrs)
+{
+uint32_t wid = mem_attrs_to_wid(attrs);
+
+trace_riscv_wgc_mem_blocked_read(addr, size, wid);
+
+*pdata = 0;
+return MEMTX_OK;
+}
+
+static MemTxResult riscv_wgc_mem_blocked_write(void *opaque, hwaddr addr,
+ uint64_t value,
+ unsigned size, MemTxAttrs attrs)
+{
+uint32_t wid = mem_attrs_to_wid(attrs);
+
+trace_riscv_wgc_mem_blocked_write(addr, value, size, wid);
+
+return MEMTX_OK;
+}
+
+static const MemoryRegionOps riscv_wgc_mem_blocked_ops = {
+.read_with_attrs = riscv_wgc_mem_blocked_read,
+.write_with_attrs = riscv_wgc_mem_blocked_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+.valid.min_access_size = 1,
+.valid.max_access_size = 8,
+.impl.min_access_size = 1,
+.impl.max_access_size = 8,
+};
+
+static IOMMUTLBEntry riscv_wgc_translate(IOMMUMemoryRegio
[RFC PATCH 08/16] target/riscv: Allow global WG config to set WG CPU callbacks
Some WG CPU functions depend on global WG config (like num-of-world), so
we let the global WG config device to set callbacks of a RISC-V HART.
Signed-off-by: Jim Shu
---
target/riscv/cpu.h | 4
1 file changed, 4 insertions(+)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 2d3bfedbba..50a0fba127 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -431,6 +431,10 @@ struct CPUArchState {
uint64_t kvm_timer_state;
uint64_t kvm_timer_frequency;
#endif /* CONFIG_KVM */
+
+/* machine specific WorldGuard callback */
+void (*wg_reset)(CPURISCVState *env);
+void (*wid_to_mem_attrs)(MemTxAttrs *attrs, uint32_t wid);
};
/*
--
2.17.1
[RFC PATCH 01/16] accel/tcg: Store section pointer in CPUTLBEntryFull
'CPUTLBEntryFull.xlat_section' stores section_index in last 12 bits to
find the correct section when CPU access the IO region over the IOTLB
(iotlb_to_section()).
However, section_index is only unique inside single AddressSpace. If
address space translation is over IOMMUMemoryRegion, it could return
section from other AddressSpace. 'iotlb_to_section()' API only finds the
sections from CPU's AddressSpace so that it couldn't find section in
other AddressSpace. Thus, using 'iotlb_to_section()' API will find the
wrong section and QEMU will have wrong load/store access.
To fix this bug, store complete MemoryRegionSection pointer in
CPUTLBEntryFull instead of section_index.
This bug occurs only when
(1) IOMMUMemoryRegion is in the path of CPU access.
(2) IOMMUMemoryRegion returns different target_as and the section is in
the IO region.
Common IOMMU devices don't have this issue since they are only in the
path of DMA access. Currently, the bug only occurs when ARM MPC device
(hw/misc/tz-mpc.c) returns 'blocked_io_as' to emulate blocked access
handling. Upcoming RISC-V wgChecker device is also affected by this bug.
Signed-off-by: Jim Shu
---
accel/tcg/cputlb.c| 19 +--
include/hw/core/cpu.h | 3 +++
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 117b516739..8cf124b760 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1169,6 +1169,7 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
desc->fulltlb[index] = *full;
full = >fulltlb[index];
full->xlat_section = iotlb - addr_page;
+full->section = section;
full->phys_addr = paddr_page;
/* Now calculate the new entry */
@@ -1248,14 +1249,14 @@ static inline void cpu_unaligned_access(CPUState *cpu,
vaddr addr,
}
static MemoryRegionSection *
-io_prepare(hwaddr *out_offset, CPUState *cpu, hwaddr xlat,
+io_prepare(hwaddr *out_offset, CPUState *cpu, CPUTLBEntryFull *full,
MemTxAttrs attrs, vaddr addr, uintptr_t retaddr)
{
MemoryRegionSection *section;
hwaddr mr_offset;
-section = iotlb_to_section(cpu, xlat, attrs);
-mr_offset = (xlat & TARGET_PAGE_MASK) + addr;
+section = full->section;
+mr_offset = (full->xlat_section & TARGET_PAGE_MASK) + addr;
cpu->mem_io_pc = retaddr;
if (!cpu->neg.can_do_io) {
cpu_io_recompile(cpu, retaddr);
@@ -1571,9 +1572,7 @@ bool tlb_plugin_lookup(CPUState *cpu, vaddr addr, int
mmu_idx,
/* We must have an iotlb entry for MMIO */
if (tlb_addr & TLB_MMIO) {
-MemoryRegionSection *section =
-iotlb_to_section(cpu, full->xlat_section & ~TARGET_PAGE_MASK,
- full->attrs);
+MemoryRegionSection *section = full->section;
data->is_io = true;
data->mr = section->mr;
} else {
@@ -1972,7 +1971,7 @@ static uint64_t do_ld_mmio_beN(CPUState *cpu,
CPUTLBEntryFull *full,
tcg_debug_assert(size > 0 && size <= 8);
attrs = full->attrs;
-section = io_prepare(_offset, cpu, full->xlat_section, attrs, addr, ra);
+section = io_prepare(_offset, cpu, full, attrs, addr, ra);
mr = section->mr;
BQL_LOCK_GUARD();
@@ -1993,7 +1992,7 @@ static Int128 do_ld16_mmio_beN(CPUState *cpu,
CPUTLBEntryFull *full,
tcg_debug_assert(size > 8 && size <= 16);
attrs = full->attrs;
-section = io_prepare(_offset, cpu, full->xlat_section, attrs, addr, ra);
+section = io_prepare(_offset, cpu, full, attrs, addr, ra);
mr = section->mr;
BQL_LOCK_GUARD();
@@ -2513,7 +2512,7 @@ static uint64_t do_st_mmio_leN(CPUState *cpu,
CPUTLBEntryFull *full,
tcg_debug_assert(size > 0 && size <= 8);
attrs = full->attrs;
-section = io_prepare(_offset, cpu, full->xlat_section, attrs, addr, ra);
+section = io_prepare(_offset, cpu, full, attrs, addr, ra);
mr = section->mr;
BQL_LOCK_GUARD();
@@ -2533,7 +2532,7 @@ static uint64_t do_st16_mmio_leN(CPUState *cpu,
CPUTLBEntryFull *full,
tcg_debug_assert(size > 8 && size <= 16);
attrs = full->attrs;
-section = io_prepare(_offset, cpu, full->xlat_section, attrs, addr, ra);
+section = io_prepare(_offset, cpu, full, attrs, addr, ra);
mr = section->mr;
BQL_LOCK_GUARD();
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index a2c8536943..3f6c10897b 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -217,6 +217,9 @@ typedef struct CPUTLBEntryFull {
*/
hwaddr xlat_section;
+/* @section contains physical section. */
+MemoryRegionSection *section;
+
/*
* @phys_addr contains the physical address in the address space
* given by cpu_asidx_from_attrs(cpu, @attrs).
--
2.17.1
[RFC PATCH 02/16] accel/tcg: memory access from CPU will pass access_type to IOMMU
It is the preparation patch for upcoming RISC-V wgChecker device.
Since RISC-V wgChecker could permit access in RO/WO permission, the
IOMMUMemoryRegion could return different section for read & write
access. The memory access from CPU should also pass the access_type to
IOMMU translate function so that IOMMU could return the correct section
of specified access_type.
Signed-off-by: Jim Shu
---
accel/tcg/cputlb.c | 15 +--
include/exec/exec-all.h | 11 +++
system/physmem.c | 16 +++-
target/alpha/helper.c| 2 +-
target/arm/tcg/tlb_helper.c | 2 +-
target/avr/helper.c | 2 +-
target/cris/helper.c | 2 +-
target/hppa/mem_helper.c | 2 +-
target/i386/tcg/sysemu/excp_helper.c | 3 ++-
target/loongarch/tcg/tlb_helper.c| 2 +-
target/m68k/helper.c | 10 +++---
target/microblaze/helper.c | 8
target/mips/tcg/sysemu/tlb_helper.c | 4 ++--
target/openrisc/mmu.c| 2 +-
target/ppc/mmu_helper.c | 2 +-
target/riscv/cpu_helper.c| 2 +-
target/rx/cpu.c | 3 ++-
target/s390x/tcg/excp_helper.c | 2 +-
target/sh4/helper.c | 2 +-
target/sparc/mmu_helper.c| 6 +++---
target/tricore/helper.c | 2 +-
target/xtensa/helper.c | 3 ++-
22 files changed, 61 insertions(+), 42 deletions(-)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 8cf124b760..f1b07f6926 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1036,7 +1036,8 @@ static inline void tlb_set_compare(CPUTLBEntryFull *full,
CPUTLBEntry *ent,
* critical section.
*/
void tlb_set_page_full(CPUState *cpu, int mmu_idx,
- vaddr addr, CPUTLBEntryFull *full)
+ vaddr addr, MMUAccessType access_type,
+ CPUTLBEntryFull *full)
{
CPUTLB *tlb = >neg.tlb;
CPUTLBDesc *desc = >d[mmu_idx];
@@ -1063,7 +1064,8 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
prot = full->prot;
asidx = cpu_asidx_from_attrs(cpu, full->attrs);
section = address_space_translate_for_iotlb(cpu, asidx, paddr_page,
-, , full->attrs,
);
+, , full->attrs, ,
+access_type);
assert(sz >= TARGET_PAGE_SIZE);
tlb_debug("vaddr=%016" VADDR_PRIx " paddr=0x" HWADDR_FMT_plx
@@ -1200,7 +1202,8 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
void tlb_set_page_with_attrs(CPUState *cpu, vaddr addr,
hwaddr paddr, MemTxAttrs attrs, int prot,
- int mmu_idx, uint64_t size)
+ MMUAccessType access_type, int mmu_idx,
+ uint64_t size)
{
CPUTLBEntryFull full = {
.phys_addr = paddr,
@@ -1210,15 +1213,15 @@ void tlb_set_page_with_attrs(CPUState *cpu, vaddr addr,
};
assert(is_power_of_2(size));
-tlb_set_page_full(cpu, mmu_idx, addr, );
+tlb_set_page_full(cpu, mmu_idx, addr, access_type, );
}
void tlb_set_page(CPUState *cpu, vaddr addr,
- hwaddr paddr, int prot,
+ hwaddr paddr, int prot, MMUAccessType access_type,
int mmu_idx, uint64_t size)
{
tlb_set_page_with_attrs(cpu, addr, paddr, MEMTXATTRS_UNSPECIFIED,
-prot, mmu_idx, size);
+prot, access_type, mmu_idx, size);
}
/*
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index b6b46ad13c..0d5363ac02 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -205,7 +205,7 @@ void tlb_flush_range_by_mmuidx_all_cpus_synced(CPUState
*cpu,
* used by tlb_flush_page.
*/
void tlb_set_page_full(CPUState *cpu, int mmu_idx, vaddr addr,
- CPUTLBEntryFull *full);
+ MMUAccessType access_type, CPUTLBEntryFull *full);
/**
* tlb_set_page_with_attrs:
@@ -231,7 +231,8 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx, vaddr
addr,
*/
void tlb_set_page_with_attrs(CPUState *cpu, vaddr addr,
hwaddr paddr, MemTxAttrs attrs,
- int prot, int mmu_idx, vaddr size);
+ int prot, MMUAccessType access_type, int mmu_idx,
+ vaddr size);
/* tlb_set_page:
*
* This function is equivalent to calling tlb_set_page_with_attrs()
@@ -240,7 +241,8 @@ void tlb_set_page_with_attrs(CPUState *cpu, vaddr addr,
*/
void tlb_set_page(CPUState *cpu, vaddr addr,
hwaddr paddr, int prot,
- int mmu_idx, vaddr size);
+ MMUAccessType access_type, int mmu_idx,
+
[RFC PATCH 03/16] exec: Add RISC-V WorldGuard WID to MemTxAttrs
RISC-V WorldGuard will add 5-bit world_id (WID) to the each memory
transaction on the bus. The wgChecker in front of RAM or peripherals
MMIO could do the access control based on the WID. It is similar to ARM
TrustZone NS bit, but the WID is 5-bit.
The common implementation of WID is AXI4 AxUSER signal.
Signed-off-by: Jim Shu
---
include/exec/memattrs.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index 14cdd8d582..d00f3c5500 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -52,6 +52,11 @@ typedef struct MemTxAttrs {
unsigned int memory:1;
/* Requester ID (for MSI for example) */
unsigned int requester_id:16;
+
+/*
+ * RISC-V WorldGuard: the 5-bit WID of memory access.
+ */
+uint8_t world_id;
} MemTxAttrs;
/* Bus masters which don't specify any attributes will get this,
--
2.17.1
[RFC PATCH 00/16] Implements RISC-V WorldGuard extension v0.4
This patchset implements Smwg/Smwgd/Sswg CPU extension and wgChecker device defined in WorldGuard spec v0.4. The WG v0.4 spec could be found here: https://lists.riscv.org/g/security/attachment/711/0/worldguard_rvia_spec-v0.4.pdf To enable WG in QEMU, pass "wg=on" as machine parameter to virt machine. It enables both WG CPU CSRs to apply WID of CPU and wgCheckers on the DRAM, FLASH, and UART to protect these resources. This patchset contains 5 parts: 1. Commit 1: Bugfix of IOMMUMemoryRegion 2. Commit 2 ~ 3: Extend IOMMUMemoryRegion and MemTxAttr for WG support 3. Commit 4 ~ 11: Add WG global device and CPU extensions 4. Commit 12 ~ 15: Add WG checker device 5. Commit 16: Add WG support to the virt machine Jim Shu (16): accel/tcg: Store section pointer in CPUTLBEntryFull accel/tcg: memory access from CPU will pass access_type to IOMMU exec: Add RISC-V WorldGuard WID to MemTxAttrs hw/misc: riscv_worldguard: Add RISC-V WorldGuard global config target/riscv: Add CPU options of WorldGuard CPU extension target/riscv: Add hard-coded CPU state of WG extension target/riscv: Add defines for WorldGuard CSRs target/riscv: Allow global WG config to set WG CPU callbacks target/riscv: Implement WorldGuard CSRs target/riscv: Add WID to MemTxAttrs of CPU memory transactions hw/misc: riscv_worldguard: Add API to enable WG extension of CPU hw/misc: riscv_wgchecker: Implement RISC-V WorldGuard Checker hw/misc: riscv_wgchecker: Implement wgchecker slot registers hw/misc: riscv_wgchecker: Implement correct block-access behavior hw/misc: riscv_wgchecker: Check the slot settings in translate hw/riscv: virt: Add WorldGuard support accel/tcg/cputlb.c | 34 +- docs/system/riscv/virt.rst | 10 + hw/misc/Kconfig |3 + hw/misc/meson.build |1 + hw/misc/riscv_wgchecker.c| 1161 ++ hw/misc/riscv_worldguard.c | 273 ++ hw/misc/trace-events |9 + hw/riscv/Kconfig |1 + hw/riscv/virt.c | 163 +++- include/exec/exec-all.h | 11 +- include/exec/memattrs.h |5 + include/hw/core/cpu.h|3 + include/hw/misc/riscv_worldguard.h | 123 +++ include/hw/riscv/virt.h | 17 +- system/physmem.c | 16 +- target/alpha/helper.c|2 +- target/arm/tcg/tlb_helper.c |2 +- target/avr/helper.c |2 +- target/cris/helper.c |2 +- target/hppa/mem_helper.c |2 +- target/i386/tcg/sysemu/excp_helper.c |3 +- target/loongarch/tcg/tlb_helper.c|2 +- target/m68k/helper.c | 10 +- target/microblaze/helper.c |8 +- target/mips/tcg/sysemu/tlb_helper.c |4 +- target/openrisc/mmu.c|2 +- target/ppc/mmu_helper.c |2 +- target/riscv/cpu.c | 16 +- target/riscv/cpu.h | 12 + target/riscv/cpu_bits.h |5 + target/riscv/cpu_cfg.h |5 + target/riscv/cpu_helper.c| 69 +- target/riscv/csr.c | 107 +++ target/riscv/tcg/tcg-cpu.c | 11 + target/rx/cpu.c |3 +- target/s390x/tcg/excp_helper.c |2 +- target/sh4/helper.c |2 +- target/sparc/mmu_helper.c|6 +- target/tricore/helper.c |2 +- target/xtensa/helper.c |3 +- 40 files changed, 2054 insertions(+), 60 deletions(-) create mode 100644 hw/misc/riscv_wgchecker.c create mode 100644 hw/misc/riscv_worldguard.c create mode 100644 include/hw/misc/riscv_worldguard.h -- 2.17.1
[PATCH] target/riscv: support atomic instruction fetch (Ziccif)
Support 4-byte atomic instruction fetch when instruction is natural
aligned.
Current implementation is not atomic because it loads instruction twice
for first and last 2 bytes. We load 4 bytes at once to keep the
atomicity. This instruction preload method only applys when instruction
is 4-byte aligned. If instruction is unaligned, it could be across pages
so that preload will trigger additional page fault.
We encounter this issue when doing pressure test of enabling & disabling
Linux kernel ftrace. Ftrace with kernel preemption requires concurrent
modification and execution of instruction, so non-atomic instruction
fetch will cause the race condition. We may fetch the wrong instruction
which is the mixing of 2 instructions.
Also, RISC-V Profile wants to provide this feature by HW. RVA20U64
Ziccif protects the atomicity of instruction fetch when it is
natural aligned.
Signed-off-by: Jim Shu
Reviewed-by: Frank Chang
---
target/riscv/translate.c | 45 ++--
1 file changed, 34 insertions(+), 11 deletions(-)
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 0569224e53..2be8ef63e6 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1133,13 +1133,37 @@ const RISCVDecoder decoder_table[] = {
const size_t decoder_table_size = ARRAY_SIZE(decoder_table);
-static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
+static void decode_opc(CPURISCVState *env, DisasContext *ctx)
{
ctx->virt_inst_excp = false;
+
+uint32_t opcode;
+bool is_4byte_align = false;
+
+if ((ctx->base.pc_next % 4) == 0) {
+/*
+ * Load 4 bytes at once to make instruction fetch atomically.
+ *
+ * Note: When pc is 4-byte aligned, 4-byte instruction wouldn't be
+ * across pages. We could preload 4 bytes instruction no matter
+ * real one is 2 or 4 bytes. Instruction preload wouldn't trigger
+ * additional page fault.
+ */
+opcode = translator_ldl(env, >base, ctx->base.pc_next);
+is_4byte_align = true;
+} else {
+/*
+ * For unaligned pc, instruction preload may trigger additional
+ * page fault so we only load 2 bytes here.
+ */
+opcode = (uint32_t) translator_lduw(env, >base,
ctx->base.pc_next);
+}
+ctx->ol = ctx->xl;
+
ctx->cur_insn_len = insn_len(opcode);
/* Check for compressed insn */
if (ctx->cur_insn_len == 2) {
-ctx->opcode = opcode;
+ctx->opcode = (uint16_t)opcode;
/*
* The Zca extension is added as way to refer to instructions in the C
* extension that do not include the floating-point loads and stores
@@ -1149,15 +1173,16 @@ static void decode_opc(CPURISCVState *env, DisasContext
*ctx, uint16_t opcode)
return;
}
} else {
-uint32_t opcode32 = opcode;
-opcode32 = deposit32(opcode32, 16, 16,
- translator_lduw(env, >base,
- ctx->base.pc_next + 2));
-ctx->opcode = opcode32;
+if (!is_4byte_align) {
+/* Load last 2 bytes of instruction here */
+opcode = deposit32(opcode, 16, 16,
+ translator_lduw(env, >base,
+ ctx->base.pc_next + 2));
+}
for (guint i = 0; i < ctx->decoders->len; ++i) {
riscv_cpu_decode_fn func = g_ptr_array_index(ctx->decoders, i);
-if (func(ctx, opcode32)) {
+if (func(ctx, opcode)) {
return;
}
}
@@ -1226,10 +1251,8 @@ static void riscv_tr_translate_insn(DisasContextBase
*dcbase, CPUState *cpu)
{
DisasContext *ctx = container_of(dcbase, DisasContext, base);
CPURISCVState *env = cpu_env(cpu);
-uint16_t opcode16 = translator_lduw(env, >base, ctx->base.pc_next);
-ctx->ol = ctx->xl;
-decode_opc(env, ctx, opcode16);
+decode_opc(env, ctx);
ctx->base.pc_next += ctx->cur_insn_len;
/* Only the first insn within a TB is allowed to cross a page boundary. */
--
2.17.1
Re: [PATCH 2/2] target/riscv: Make the "virt" register writable by GDB
On Mon, Mar 6, 2023 at 7:26 PM LIU Zhiwei wrote:
>
>
> On 2023/3/5 17:42, Jim Shu wrote:
> > This patch also enables debugger to set current privilege mode to
> > VU/VS-mode.
> >
> > Extend previous commit 81d2929c41d32af138f3562f5a7b309f6eac7ca7 to
> > support H-extension.
> >
> > Signed-off-by: Jim Shu
> > Reviewed-by: Frank Chang
> > ---
> > target/riscv/gdbstub.c | 18 --
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> > index 1755fd9d51..a7f234beaf 100644
> > --- a/target/riscv/gdbstub.c
> > +++ b/target/riscv/gdbstub.c
> > @@ -203,15 +203,29 @@ static int riscv_gdb_get_virtual(CPURISCVState *cs,
> > GByteArray *buf, int n)
> >
> > static int riscv_gdb_set_virtual(CPURISCVState *cs, uint8_t *mem_buf, int
> > n)
> > {
> > +#ifdef CONFIG_USER_ONLY
> > +if (n >= 0 && n <= 1) {
> > +return sizeof(target_ulong);
> > +}
> > +#else
> > +bool virt;
> > +
> > if (n == 0) {
> > -#ifndef CONFIG_USER_ONLY
> > cs->priv = ldtul_p(mem_buf) & 0x3;
> > if (cs->priv == PRV_H) {
> > cs->priv = PRV_S;
> > }
> > -#endif
> > +return sizeof(target_ulong);
> We should return according to the misa_mxl_max. And this is a bug before
> your commit.
Hi Zhiwei,
After reading other gdbstub.c code, I think it is OK to use
'sizeof(target_ulong)' as virtual register length.
Its length is 32-bit in RV32 and is 64-bit in RV64/RV128. We don't
need to handle RV128 specially.
Virtual register length is same as CSR length and
'riscv_gdb_set_csr()' also use 'sizeof(target_ulong)'.
Jim Shu
> > +} else if (n == 1) {
> > +virt = ldtul_p(mem_buf) & 0x1;
> > +if ((cs->priv == PRV_M) && (virt == true)) {
> > +/* M-mode only supports V=0. */
> > +virt = false;
> > +}
> > +riscv_cpu_set_virt_enabled(cs, virt);
> > return sizeof(target_ulong);
> Same error here. Otherwise,
>
> Reviewed-by: LIU Zhiwei
>
> Zhiwei
>
> > }
> > +#endif
> > return 0;
> > }
> >
Re: [PATCH 2/2] target/riscv: Make the "virt" register writable by GDB
Thanks for reviewing.
I'll fix this issue.
On Mon, Mar 6, 2023 at 7:26 PM LIU Zhiwei wrote:
>
>
> On 2023/3/5 17:42, Jim Shu wrote:
> > This patch also enables debugger to set current privilege mode to
> > VU/VS-mode.
> >
> > Extend previous commit 81d2929c41d32af138f3562f5a7b309f6eac7ca7 to
> > support H-extension.
> >
> > Signed-off-by: Jim Shu
> > Reviewed-by: Frank Chang
> > ---
> > target/riscv/gdbstub.c | 18 --
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> > index 1755fd9d51..a7f234beaf 100644
> > --- a/target/riscv/gdbstub.c
> > +++ b/target/riscv/gdbstub.c
> > @@ -203,15 +203,29 @@ static int riscv_gdb_get_virtual(CPURISCVState *cs,
> > GByteArray *buf, int n)
> >
> > static int riscv_gdb_set_virtual(CPURISCVState *cs, uint8_t *mem_buf, int
> > n)
> > {
> > +#ifdef CONFIG_USER_ONLY
> > +if (n >= 0 && n <= 1) {
> > +return sizeof(target_ulong);
> > +}
> > +#else
> > +bool virt;
> > +
> > if (n == 0) {
> > -#ifndef CONFIG_USER_ONLY
> > cs->priv = ldtul_p(mem_buf) & 0x3;
> > if (cs->priv == PRV_H) {
> > cs->priv = PRV_S;
> > }
> > -#endif
> > +return sizeof(target_ulong);
> We should return according to the misa_mxl_max. And this is a bug before
> your commit.
> > +} else if (n == 1) {
> > +virt = ldtul_p(mem_buf) & 0x1;
> > +if ((cs->priv == PRV_M) && (virt == true)) {
> > +/* M-mode only supports V=0. */
> > +virt = false;
> > +}
> > +riscv_cpu_set_virt_enabled(cs, virt);
> > return sizeof(target_ulong);
> Same error here. Otherwise,
>
> Reviewed-by: LIU Zhiwei
>
> Zhiwei
>
> > }
> > +#endif
> > return 0;
> > }
> >
[PATCH 1/2] target/riscv: Expose "virt" register for GDB for reads
This patch enables a debugger to read current virtualization mode via
virtual "virt" register. After it, we could get full current privilege
mode via both "priv" and "virt" register.
Extend previous commit ab9056ff9bdb3f95db6e7a666d10522d289f14ec to
support H-extension.
Signed-off-by: Jim Shu
Reviewed-by: Frank Chang
---
gdb-xml/riscv-32bit-virtual.xml | 1 +
gdb-xml/riscv-64bit-virtual.xml | 1 +
target/riscv/gdbstub.c | 12
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/gdb-xml/riscv-32bit-virtual.xml b/gdb-xml/riscv-32bit-virtual.xml
index 905f1c555d..d44b6ca2dc 100644
--- a/gdb-xml/riscv-32bit-virtual.xml
+++ b/gdb-xml/riscv-32bit-virtual.xml
@@ -8,4 +8,5 @@
+
diff --git a/gdb-xml/riscv-64bit-virtual.xml b/gdb-xml/riscv-64bit-virtual.xml
index 62d86c237b..7c9b63d5b6 100644
--- a/gdb-xml/riscv-64bit-virtual.xml
+++ b/gdb-xml/riscv-64bit-virtual.xml
@@ -8,4 +8,5 @@
+
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 6048541606..1755fd9d51 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -187,13 +187,17 @@ static int riscv_gdb_set_csr(CPURISCVState *env, uint8_t
*mem_buf, int n)
static int riscv_gdb_get_virtual(CPURISCVState *cs, GByteArray *buf, int n)
{
-if (n == 0) {
#ifdef CONFIG_USER_ONLY
+if (n >= 0 && n <= 1) {
return gdb_get_regl(buf, 0);
+}
#else
+if (n == 0) {
return gdb_get_regl(buf, cs->priv);
-#endif
+} else if (n == 1) {
+return gdb_get_regl(buf, riscv_cpu_virt_enabled(cs));
}
+#endif
return 0;
}
@@ -328,13 +332,13 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState
*cs)
case MXL_RV32:
gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
riscv_gdb_set_virtual,
- 1, "riscv-32bit-virtual.xml", 0);
+ 2, "riscv-32bit-virtual.xml", 0);
break;
case MXL_RV64:
case MXL_RV128:
gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
riscv_gdb_set_virtual,
- 1, "riscv-64bit-virtual.xml", 0);
+ 2, "riscv-64bit-virtual.xml", 0);
break;
default:
g_assert_not_reached();
--
2.17.1
[PATCH 2/2] target/riscv: Make the "virt" register writable by GDB
This patch also enables debugger to set current privilege mode to
VU/VS-mode.
Extend previous commit 81d2929c41d32af138f3562f5a7b309f6eac7ca7 to
support H-extension.
Signed-off-by: Jim Shu
Reviewed-by: Frank Chang
---
target/riscv/gdbstub.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 1755fd9d51..a7f234beaf 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -203,15 +203,29 @@ static int riscv_gdb_get_virtual(CPURISCVState *cs,
GByteArray *buf, int n)
static int riscv_gdb_set_virtual(CPURISCVState *cs, uint8_t *mem_buf, int n)
{
+#ifdef CONFIG_USER_ONLY
+if (n >= 0 && n <= 1) {
+return sizeof(target_ulong);
+}
+#else
+bool virt;
+
if (n == 0) {
-#ifndef CONFIG_USER_ONLY
cs->priv = ldtul_p(mem_buf) & 0x3;
if (cs->priv == PRV_H) {
cs->priv = PRV_S;
}
-#endif
+return sizeof(target_ulong);
+} else if (n == 1) {
+virt = ldtul_p(mem_buf) & 0x1;
+if ((cs->priv == PRV_M) && (virt == true)) {
+/* M-mode only supports V=0. */
+virt = false;
+}
+riscv_cpu_set_virt_enabled(cs, virt);
return sizeof(target_ulong);
}
+#endif
return 0;
}
--
2.17.1
[PATCH] hw/intc: sifive_plic: fix out-of-bound access of source_priority array
If the number of interrupt is not multiple of 32, PLIC will have
out-of-bound access to source_priority array. Compute the number of
interrupt in the last word to avoid this out-of-bound access of array.
Signed-off-by: Jim Shu
---
hw/intc/sifive_plic.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
index c2dfacf028..1cf156cf85 100644
--- a/hw/intc/sifive_plic.c
+++ b/hw/intc/sifive_plic.c
@@ -78,6 +78,7 @@ static uint32_t sifive_plic_claimed(SiFivePLICState *plic,
uint32_t addrid)
uint32_t max_irq = 0;
uint32_t max_prio = plic->target_priority[addrid];
int i, j;
+int num_irq_in_word = 32;
for (i = 0; i < plic->bitfield_words; i++) {
uint32_t pending_enabled_not_claimed =
@@ -88,7 +89,16 @@ static uint32_t sifive_plic_claimed(SiFivePLICState *plic,
uint32_t addrid)
continue;
}
-for (j = 0; j < 32; j++) {
+if (i == (plic->bitfield_words - 1)) {
+/*
+ * If plic->num_sources is not multiple of 32, num-of-irq in last
+ * word is not 32. Compute the num-of-irq of last word to avoid
+ * out-of-bound access of source_priority array.
+ */
+num_irq_in_word = plic->num_sources - ((plic->bitfield_words - 1)
<< 5);
+}
+
+for (j = 0; j < num_irq_in_word; j++) {
int irq = (i << 5) + j;
uint32_t prio = plic->source_priority[irq];
int enabled = pending_enabled_not_claimed & (1 << j);
--
2.17.1
[PATCH] target/riscv: support cache-related PMU events in virtual mode
let tlb_fill() function also increments PMU counter when it is from
two-stage translation, so QEMU could also monitor these PMU events when
CPU runs in VS/VU mode (like running guest OS).
Signed-off-by: Jim Shu
---
target/riscv/cpu_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 278d163803..a52a9b14d7 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1248,6 +1248,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
}
}
+pmu_tlb_fill_incr_ctr(cpu, access_type);
if (riscv_cpu_virt_enabled(env) ||
((riscv_cpu_two_stage_lookup(mmu_idx) || two_stage_lookup) &&
access_type != MMU_INST_FETCH)) {
@@ -1311,7 +1312,6 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
}
}
} else {
-pmu_tlb_fill_incr_ctr(cpu, access_type);
/* Single stage lookup */
ret = get_physical_address(env, , , address, NULL,
access_type, mmu_idx, true, false, false);
--
2.17.1
Re: [PATCH v3 0/2] Enhance maximum priority support of PLIC
Gentle ping. It's a patch for fix and spec alignment of PLIC. On Mon, Oct 3, 2022 at 12:14 PM Jim Shu wrote: > > This patchset fixes hard-coded maximum priority of interrupt priority > register and also changes this register to WARL field to align the PLIC > spec. > > Changelog: > > v3: > * fix opposite of power-of-2 max priority checking expression. > > v2: > * change interrupt priority register to WARL field. > > Jim Shu (2): > hw/intc: sifive_plic: fix hard-coded max priority level > hw/intc: sifive_plic: change interrupt priority register to WARL field > > hw/intc/sifive_plic.c | 25 ++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > -- > 2.17.1 >
[PATCH v3 0/2] Enhance maximum priority support of PLIC
This patchset fixes hard-coded maximum priority of interrupt priority register and also changes this register to WARL field to align the PLIC spec. Changelog: v3: * fix opposite of power-of-2 max priority checking expression. v2: * change interrupt priority register to WARL field. Jim Shu (2): hw/intc: sifive_plic: fix hard-coded max priority level hw/intc: sifive_plic: change interrupt priority register to WARL field hw/intc/sifive_plic.c | 25 ++--- 1 file changed, 22 insertions(+), 3 deletions(-) -- 2.17.1
[PATCH v3 2/2] hw/intc: sifive_plic: change interrupt priority register to WARL field
PLIC spec [1] requires interrupt source priority registers are WARL
field and the number of supported priority is power-of-2 to simplify SW
discovery.
Existing QEMU RISC-V machine (e.g. shakti_c) don't strictly follow PLIC
spec, whose number of supported priority is not power-of-2. Just change
each bit of interrupt priority register to WARL field when the number of
supported priority is power-of-2.
[1]
https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc#interrupt-priorities
Signed-off-by: Jim Shu
---
hw/intc/sifive_plic.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
index f864efa761..c2dfacf028 100644
--- a/hw/intc/sifive_plic.c
+++ b/hw/intc/sifive_plic.c
@@ -180,7 +180,15 @@ static void sifive_plic_write(void *opaque, hwaddr addr,
uint64_t value,
if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) {
uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
-if (value <= plic->num_priorities) {
+if (((plic->num_priorities + 1) & plic->num_priorities) == 0) {
+/*
+ * if "num_priorities + 1" is power-of-2, make each register bit of
+ * interrupt priority WARL (Write-Any-Read-Legal). Just filter
+ * out the access to unsupported priority bits.
+ */
+plic->source_priority[irq] = value % (plic->num_priorities + 1);
+sifive_plic_update(plic);
+} else if (value <= plic->num_priorities) {
plic->source_priority[irq] = value;
sifive_plic_update(plic);
}
@@ -207,7 +215,16 @@ static void sifive_plic_write(void *opaque, hwaddr addr,
uint64_t value,
uint32_t contextid = (addr & (plic->context_stride - 1));
if (contextid == 0) {
-if (value <= plic->num_priorities) {
+if (((plic->num_priorities + 1) & plic->num_priorities) == 0) {
+/*
+ * if "num_priorities + 1" is power-of-2, each register bit of
+ * interrupt priority is WARL (Write-Any-Read-Legal). Just
+ * filter out the access to unsupported priority bits.
+ */
+plic->target_priority[addrid] = value %
+(plic->num_priorities + 1);
+sifive_plic_update(plic);
+} else if (value <= plic->num_priorities) {
plic->target_priority[addrid] = value;
sifive_plic_update(plic);
}
--
2.17.1
[PATCH v3 1/2] hw/intc: sifive_plic: fix hard-coded max priority level
The maximum priority level is hard-coded when writing to interrupt
priority register. However, when writing to priority threshold register,
the maximum priority level is from num_priorities Property which is
configured by platform.
Also change interrupt priority register to use num_priorities Property
in maximum priority level.
Signed-off-by: Emmanuel Blot
Signed-off-by: Jim Shu
Reviewed-by: Frank Chang
---
hw/intc/sifive_plic.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
index af4ae3630e..f864efa761 100644
--- a/hw/intc/sifive_plic.c
+++ b/hw/intc/sifive_plic.c
@@ -180,8 +180,10 @@ static void sifive_plic_write(void *opaque, hwaddr addr,
uint64_t value,
if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) {
uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
-plic->source_priority[irq] = value & 7;
-sifive_plic_update(plic);
+if (value <= plic->num_priorities) {
+plic->source_priority[irq] = value;
+sifive_plic_update(plic);
+}
} else if (addr_between(addr, plic->pending_base,
plic->num_sources >> 3)) {
qemu_log_mask(LOG_GUEST_ERROR,
--
2.17.1
Re: [PATCH v2 2/2] hw/intc: sifive_plic: change interrupt priority register to WARL field
Hi Clément,
> > > @@ -180,7 +180,15 @@ static void sifive_plic_write(void *opaque, hwaddr
> > > addr, uint64_t value,
> > > if (addr_between(addr, plic->priority_base, plic->num_sources << 2))
> > > {
> > > uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
> > >
> > > -if (value <= plic->num_priorities) {
> > > +if ((plic->num_priorities + 1) & (plic->num_priorities)) {
> >
> > That's the opposite. If n is a power of 2, n & (n-1) == 0 (eg 8 & 7 ==
> > 0, 9 & 8 == 8).
> > Note that n must be positive too. But I'm not sure it matters here.
> > I'll let you decide.
> >
num_priorities is a uint32_t variable so that n is always positive.
Re: [PATCH v2 2/2] hw/intc: sifive_plic: change interrupt priority register to WARL field
hi Clément,
Thank you very much.
I'll fix it in the next version patch.
Thanks,
Jim Shu
On Fri, Sep 30, 2022 at 8:58 PM Clément Chigot wrote:
>
> Hi Jim,
>
> On Fri, Sep 30, 2022 at 2:32 PM Jim Shu wrote:
> >
> > PLIC spec [1] requires interrupt source priority registers are WARL
> > field and the number of supported priority is power-of-2 to simplify SW
> > discovery.
> >
> > Existing QEMU RISC-V machine (e.g. shakti_c) don't strictly follow PLIC
> > spec, whose number of supported priority is not power-of-2. Just change
> > each bit of interrupt priority register to WARL field when the number of
> > supported priority is power-of-2.
> >
> > [1]
> > https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc#interrupt-priorities
> >
> > Signed-off-by: Jim Shu
> > ---
> > hw/intc/sifive_plic.c | 21 +++--
> > 1 file changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> > index f864efa761..218ccff8bd 100644
> > --- a/hw/intc/sifive_plic.c
> > +++ b/hw/intc/sifive_plic.c
> > @@ -180,7 +180,15 @@ static void sifive_plic_write(void *opaque, hwaddr
> > addr, uint64_t value,
> > if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) {
> > uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
> >
> > -if (value <= plic->num_priorities) {
> > +if ((plic->num_priorities + 1) & (plic->num_priorities)) {
>
> That's the opposite. If n is a power of 2, n & (n-1) == 0 (eg 8 & 7 ==
> 0, 9 & 8 == 8).
> Note that n must be positive too. But I'm not sure it matters here.
> I'll let you decide.
>
> > +/*
> > + * if "num_priorities + 1" is power-of-2, make each register
> > bit of
> > + * interrupt priority WARL (Write-Any-Read-Legal). Just filter
> > + * out the access to unsupported priority bits.
> > + */
> > +plic->source_priority[irq] = value % (plic->num_priorities +
> > 1);
> > +sifive_plic_update(plic);
> > +} else if (value <= plic->num_priorities) {
> > plic->source_priority[irq] = value;
> > sifive_plic_update(plic);
> > }
> > @@ -207,7 +215,16 @@ static void sifive_plic_write(void *opaque, hwaddr
> > addr, uint64_t value,
> > uint32_t contextid = (addr & (plic->context_stride - 1));
> >
> > if (contextid == 0) {
> > -if (value <= plic->num_priorities) {
> > +if ((plic->num_priorities + 1) & (plic->num_priorities)) {
>
> Same.
>
> > +/*
> > + * if "num_priorities + 1" is power-of-2, each register
> > bit of
> > + * interrupt priority is WARL (Write-Any-Read-Legal). Just
> > + * filter out the access to unsupported priority bits.
> > + */
> > +plic->target_priority[addrid] = value %
> > +(plic->num_priorities + 1);
> > +sifive_plic_update(plic);
> > +} else if (value <= plic->num_priorities) {
> > plic->target_priority[addrid] = value;
> > sifive_plic_update(plic);
> > }
> > --
> > 2.17.1
>
> Clément
[PATCH v2 2/2] hw/intc: sifive_plic: change interrupt priority register to WARL field
PLIC spec [1] requires interrupt source priority registers are WARL
field and the number of supported priority is power-of-2 to simplify SW
discovery.
Existing QEMU RISC-V machine (e.g. shakti_c) don't strictly follow PLIC
spec, whose number of supported priority is not power-of-2. Just change
each bit of interrupt priority register to WARL field when the number of
supported priority is power-of-2.
[1]
https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc#interrupt-priorities
Signed-off-by: Jim Shu
---
hw/intc/sifive_plic.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
index f864efa761..218ccff8bd 100644
--- a/hw/intc/sifive_plic.c
+++ b/hw/intc/sifive_plic.c
@@ -180,7 +180,15 @@ static void sifive_plic_write(void *opaque, hwaddr addr,
uint64_t value,
if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) {
uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
-if (value <= plic->num_priorities) {
+if ((plic->num_priorities + 1) & (plic->num_priorities)) {
+/*
+ * if "num_priorities + 1" is power-of-2, make each register bit of
+ * interrupt priority WARL (Write-Any-Read-Legal). Just filter
+ * out the access to unsupported priority bits.
+ */
+plic->source_priority[irq] = value % (plic->num_priorities + 1);
+sifive_plic_update(plic);
+} else if (value <= plic->num_priorities) {
plic->source_priority[irq] = value;
sifive_plic_update(plic);
}
@@ -207,7 +215,16 @@ static void sifive_plic_write(void *opaque, hwaddr addr,
uint64_t value,
uint32_t contextid = (addr & (plic->context_stride - 1));
if (contextid == 0) {
-if (value <= plic->num_priorities) {
+if ((plic->num_priorities + 1) & (plic->num_priorities)) {
+/*
+ * if "num_priorities + 1" is power-of-2, each register bit of
+ * interrupt priority is WARL (Write-Any-Read-Legal). Just
+ * filter out the access to unsupported priority bits.
+ */
+plic->target_priority[addrid] = value %
+(plic->num_priorities + 1);
+sifive_plic_update(plic);
+} else if (value <= plic->num_priorities) {
plic->target_priority[addrid] = value;
sifive_plic_update(plic);
}
--
2.17.1
[PATCH v2 0/2] Enhance maximum priority support of PLIC
This patchset fixes hard-coded maximum priority of interrupt priority register and also changes this register to WARL field to align the PLIC spec. Changelog: v2: * change interrupt priority register to WARL field. Jim Shu (2): hw/intc: sifive_plic: fix hard-coded max priority level hw/intc: sifive_plic: change interrupt priority register to WARL field hw/intc/sifive_plic.c | 24 +--- 1 file changed, 21 insertions(+), 3 deletions(-) -- 2.17.1
[PATCH v2 1/2] hw/intc: sifive_plic: fix hard-coded max priority level
The maximum priority level is hard-coded when writing to interrupt
priority register. However, when writing to priority threshold register,
the maximum priority level is from num_priorities Property which is
configured by platform.
Also change interrupt priority register to use num_priorities Property
in maximum priority level.
Signed-off-by: Emmanuel Blot
Signed-off-by: Jim Shu
Reviewed-by: Frank Chang
---
hw/intc/sifive_plic.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
index af4ae3630e..f864efa761 100644
--- a/hw/intc/sifive_plic.c
+++ b/hw/intc/sifive_plic.c
@@ -180,8 +180,10 @@ static void sifive_plic_write(void *opaque, hwaddr addr,
uint64_t value,
if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) {
uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
-plic->source_priority[irq] = value & 7;
-sifive_plic_update(plic);
+if (value <= plic->num_priorities) {
+plic->source_priority[irq] = value;
+sifive_plic_update(plic);
+}
} else if (addr_between(addr, plic->pending_base,
plic->num_sources >> 3)) {
qemu_log_mask(LOG_GUEST_ERROR,
--
2.17.1
Re: [PATCH] hw/intc: sifive_plic: fix hard-coded max priority level
Hi Clément,
Thanks for your opinion. I think it's a good enhancement.
PLIC spec [1] says that interrupt source priority registers should be
WARL fields to allow software to determine "the number and position of
read-write bits" in each priority specification, if any. To simplify
discovery of supported priority values, each priority register must
support any combination of values in the bits that are variable within
the register, i.e., if there are two variable bits in the register,
all four combinations of values in those bits must operate as valid
priority levels.
I think "num_priorities + 1" should be power-of-2 and SW could
discover available bits of interrupt source priority.
I'll do this enhancement in the next version patch.
[1]
https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc#interrupt-priorities
Thanks,
Jim Shu
On Mon, Sep 26, 2022 at 3:52 PM Clément Chigot wrote:
>
> Hi Jim,
>
> On Sun, Sep 25, 2022 at 3:26 PM Jim Shu wrote:
> >
> > The maximum priority level is hard-coded when writing to interrupt
> > priority register. However, when writing to priority threshold register,
> > the maximum priority level is from num_priorities Property which is
> > configured by platform.
> >
> > Also change interrupt priority register to use num_priorities Property
> > in maximum priority level.
> >
> > Signed-off-by: Emmanuel Blot
> > Signed-off-by: Jim Shu
> > Reviewed-by: Frank Chang
> > ---
> > hw/intc/sifive_plic.c | 6 --
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> > index af4ae3630e..f864efa761 100644
> > --- a/hw/intc/sifive_plic.c
> > +++ b/hw/intc/sifive_plic.c
> > @@ -180,8 +180,10 @@ static void sifive_plic_write(void *opaque, hwaddr
> > addr, uint64_t value,
> > if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) {
> > uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
> >
> > -plic->source_priority[irq] = value & 7;
> > -sifive_plic_update(plic);
> > +if (value <= plic->num_priorities) {
> > +plic->source_priority[irq] = value;
> > +sifive_plic_update(plic);
> > +}
>
> If I'm not mistaking the documentation [1], these registers are WARL
> (Write-Any-Read-Legal). However, in your case, any value above
> "num_priorities" will be ignored.
>
> We had an issue related to that and ended up making a local patch.
> However, we are assuming that "num_priorities+1" is a power of 2
> (which is currently the case)
>
> -plic->source_priority[irq] = value & 7;
> +/* Interrupt Priority registers are Write-Any Read-Legal. Cleanup
> + * incoming values before storing them.
> + */
> +plic->source_priority[irq] = value % (plic->num_priorities + 1);
>
> Note that it should also be done for target_priority a bit below.
> -if (value <= plic->num_priorities) {
> -plic->target_priority[addrid] = value;
> -sifive_plic_update(plic);
> -}
> +/* Priority Thresholds registers are Write-Any Read-Legal.
> Cleanup
> + * incoming values before storing them.
> + */
> +plic->target_priority[addrid] = value % (plic->num_priorities +
> 1);
> +sifive_plic_update(plic);
>
> [1] https://static.dev.sifive.com/FE310-G000.pdf
>
> Thanks,
> Clément
Re: [PATCH 2/4] hw/intc: sifive_plic.c: Fix interrupt priority index.
Hi Tyler,
Thanks for the explanation. I understand the issue here.
I think we should align the priority base in each RISC-V platform to
the same value (no matter 0x0 or 0x4) if they use PLIC in the same
way.
Thanks,
Jim Shu
On Tue, Sep 27, 2022 at 4:04 AM Tyler Ng wrote:
>
> Hi Jim,
>
> Thanks for raising this comment. I think I understand where the confusion
> happens and it's because in the OpenTitan machine (which uses the sifive
> plic), it uses 0x00 as the priority base by default, which was the source of
> the problems. I'll drop this commit in the next version.
>
> -Tyler
>
> On Sun, Sep 25, 2022 at 6:47 AM Jim Shu wrote:
>>
>> Hi Tyler,
>>
>> This fix is incorrect.
>>
>> In PLIC spec, Interrupt Source Priority Memory Map is
>> 0x00: Reserved (interrupt source 0 does not exist)
>> 0x04: Interrupt source 1 priority
>> 0x08: Interrupt source 2 priority
>>
>> Current RISC-V machines (virt, sifive_u) use 0x4 as priority_base, so
>> current formula "irq = ((addr - plic->priority_base) >> 2) + 1" will
>> take offset 0x4 as IRQ source 1, which is correct.
>> Your fix will cause the bug in existing machines.
>>
>> Thanks,
>> Jim Shu
>>
>>
>>
>>
>> On Tue, Sep 6, 2022 at 11:21 PM Tyler Ng wrote:
>> >
>> > Here's the patch SHA that introduced the offset:
>> > 0feb4a7129eb4f120c75849ddc9e50495c50cb63
>> >
>> > -Tyler
>> >
>> > On Mon, Sep 5, 2022 at 6:15 AM Andrew Jones
>> > wrote:
>> >>
>> >> On Thu, Sep 01, 2022 at 03:50:06PM -0700, Tyler Ng wrote:
>> >> > Fixes a bug in which the index of the interrupt priority is off by 1.
>> >> > For example, using an IRQ number of 3 with a priority of 1 is supposed
>> >> > to set
>> >> > plic->source_priority[2] = 1, but instead it sets
>> >> > plic->source_priority[3] = 1. When an interrupt is claimed to be
>> >> > serviced, it checks the index 2 instead of 3.
>> >> >
>> >> > Signed-off-by: Tyler Ng
>> >>
>> >> Fixes tag?
>> >>
>> >> Thanks,
>> >> drew
>> >>
>> >> > ---
>> >> > hw/intc/sifive_plic.c | 2 +-
>> >> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
>> >> > index af4ae3630e..e75c47300a 100644
>> >> > --- a/hw/intc/sifive_plic.c
>> >> > +++ b/hw/intc/sifive_plic.c
>> >> > @@ -178,7 +178,7 @@ static void sifive_plic_write(void *opaque, hwaddr
>> >> > addr, uint64_t value,
>> >> > SiFivePLICState *plic = opaque;
>> >> >
>> >> > if (addr_between(addr, plic->priority_base, plic->num_sources <<
>> >> > 2)) {
>> >> > -uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
>> >> > +uint32_t irq = ((addr - plic->priority_base) >> 2) + 0;
>> >> >
>> >> > plic->source_priority[irq] = value & 7;
>> >> > sifive_plic_update(plic);
>> >> > --
>> >> > 2.30.2
>> >> >
Re: [PATCH 2/4] hw/intc: sifive_plic.c: Fix interrupt priority index.
Hi Tyler,
This fix is incorrect.
In PLIC spec, Interrupt Source Priority Memory Map is
0x00: Reserved (interrupt source 0 does not exist)
0x04: Interrupt source 1 priority
0x08: Interrupt source 2 priority
Current RISC-V machines (virt, sifive_u) use 0x4 as priority_base, so
current formula "irq = ((addr - plic->priority_base) >> 2) + 1" will
take offset 0x4 as IRQ source 1, which is correct.
Your fix will cause the bug in existing machines.
Thanks,
Jim Shu
On Tue, Sep 6, 2022 at 11:21 PM Tyler Ng wrote:
>
> Here's the patch SHA that introduced the offset:
> 0feb4a7129eb4f120c75849ddc9e50495c50cb63
>
> -Tyler
>
> On Mon, Sep 5, 2022 at 6:15 AM Andrew Jones wrote:
>>
>> On Thu, Sep 01, 2022 at 03:50:06PM -0700, Tyler Ng wrote:
>> > Fixes a bug in which the index of the interrupt priority is off by 1.
>> > For example, using an IRQ number of 3 with a priority of 1 is supposed to
>> > set
>> > plic->source_priority[2] = 1, but instead it sets
>> > plic->source_priority[3] = 1. When an interrupt is claimed to be
>> > serviced, it checks the index 2 instead of 3.
>> >
>> > Signed-off-by: Tyler Ng
>>
>> Fixes tag?
>>
>> Thanks,
>> drew
>>
>> > ---
>> > hw/intc/sifive_plic.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
>> > index af4ae3630e..e75c47300a 100644
>> > --- a/hw/intc/sifive_plic.c
>> > +++ b/hw/intc/sifive_plic.c
>> > @@ -178,7 +178,7 @@ static void sifive_plic_write(void *opaque, hwaddr
>> > addr, uint64_t value,
>> > SiFivePLICState *plic = opaque;
>> >
>> > if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) {
>> > -uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
>> > +uint32_t irq = ((addr - plic->priority_base) >> 2) + 0;
>> >
>> > plic->source_priority[irq] = value & 7;
>> > sifive_plic_update(plic);
>> > --
>> > 2.30.2
>> >
[PATCH] hw/intc: sifive_plic: fix hard-coded max priority level
The maximum priority level is hard-coded when writing to interrupt
priority register. However, when writing to priority threshold register,
the maximum priority level is from num_priorities Property which is
configured by platform.
Also change interrupt priority register to use num_priorities Property
in maximum priority level.
Signed-off-by: Emmanuel Blot
Signed-off-by: Jim Shu
Reviewed-by: Frank Chang
---
hw/intc/sifive_plic.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
index af4ae3630e..f864efa761 100644
--- a/hw/intc/sifive_plic.c
+++ b/hw/intc/sifive_plic.c
@@ -180,8 +180,10 @@ static void sifive_plic_write(void *opaque, hwaddr addr,
uint64_t value,
if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) {
uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
-plic->source_priority[irq] = value & 7;
-sifive_plic_update(plic);
+if (value <= plic->num_priorities) {
+plic->source_priority[irq] = value;
+sifive_plic_update(plic);
+}
} else if (addr_between(addr, plic->pending_base,
plic->num_sources >> 3)) {
qemu_log_mask(LOG_GUEST_ERROR,
--
2.17.1
Re: [PATCH] include/hw/riscv/sifive_e.h: Fix the type of parent_obj of SiFiveEState.
Reviewed-by: Jim Shu
On Fri, Aug 19, 2022 at 3:11 PM Tommy Wu wrote:
>
> Fix the type of parent_obj of SiFiveEState from 'SysBusDevice'
> to 'MachineState'. Because the parent of SiFiveEState is 'MachineState'.
>
> Signed-off-by: Tommy Wu
> ---
> include/hw/riscv/sifive_e.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h
> index 83604da805..24359f9fe5 100644
> --- a/include/hw/riscv/sifive_e.h
> +++ b/include/hw/riscv/sifive_e.h
> @@ -41,7 +41,7 @@ typedef struct SiFiveESoCState {
>
> typedef struct SiFiveEState {
> /*< private >*/
> -SysBusDevice parent_obj;
> +MachineState parent_obj;
>
> /*< public >*/
> SiFiveESoCState soc;
> --
> 2.27.0
>
>
Re: [PATCH] target/riscv: Support SW update of PTE A/D bits and Ssptwad extension
Hi Alistair, Why do we want to support that? We can do either and we are > implementing the much more usual scheme. I don't see a reason to > bother implementing the other one. Is anyone ever going to use it? > Thanks for your response. I got it. Regards, Jim Shu
Re: [PATCH] target/riscv: Support SW update of PTE A/D bits and Ssptwad extension
Hi Anup,
Do you think it is OK if we only use ssptwad as a CPU option name
but not a RISC-V extension? just like MMU and PMP options of RISC-V.
(And we could change it to RISC-V extension in the future
if Ssptwad becomes the formal RISC-V extension)
Both HW/SW update schemes are already defined in RISC-V priv spec,
so I think it's reasonable to implement them in QEMU. The only issue here is
to choose a proper CPU option name to turn on/off HW update of A/D bits.
Regards,
Jim Shu
On Mon, Jul 18, 2022 at 12:02 PM Anup Patel wrote:
> +Atish
>
> On Mon, Jul 18, 2022 at 9:23 AM Jim Shu wrote:
> >
> > RISC-V priv spec v1.12 permits 2 PTE-update schemes of A/D-bit
> > (Access/Dirty bit): HW update or SW update. RISC-V profile defines the
> > extension name 'Ssptwad' for HW update to PTE A/D bits.
> > https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc
>
> The Ssptwad (even though used by profiles) is not a well defined RISC-V
> ISA extension. Rather, Ssptwad is just a name used in profiles to represent
> an optional feature.
>
> In fact, since it is not a well-defined ISA extension, QEMU cannot include
> Ssptwad in the ISA string as well.
>
> I think for such optionalities which are not well-defined ISA extensions,
> QEMU should treat it differently.
>
> Regards,
> Anup
>
> >
> > Current QEMU RISC-V implements HW update scheme, so this commit
> > introduces SW update scheme to QEMU and uses the 'Ssptwad' extension
> > as the CPU option to select 2 PTE-update schemes. QEMU RISC-V CPU still
> > uses HW update scheme (ext_ssptwad=true) by default to keep the backward
> > compatibility.
> >
> > SW update scheme implemention is based on priv spec v1.12:
> > "When a virtual page is accessed and the A bit is clear, or is written
> > and the D bit is clear, a page-fault exception (corresponding to the
> > original access type) is raised."
> >
> > Signed-off-by: Jim Shu
> > Reviewed-by: Frank Chang
> > ---
> > target/riscv/cpu.c| 2 ++
> > target/riscv/cpu.h| 1 +
> > target/riscv/cpu_helper.c | 9 +
> > 3 files changed, 12 insertions(+)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 1bb3973806..1d38c1c1d2 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -857,6 +857,7 @@ static void riscv_cpu_init(Object *obj)
> >
> > cpu->cfg.ext_ifencei = true;
> > cpu->cfg.ext_icsr = true;
> > +cpu->cfg.ext_ssptwad = true;
> > cpu->cfg.mmu = true;
> > cpu->cfg.pmp = true;
> >
> > @@ -900,6 +901,7 @@ static Property riscv_cpu_extensions[] = {
> > DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
> > DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
> > DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
> > +DEFINE_PROP_BOOL("ssptwad", RISCVCPU, cfg.ext_ssptwad, true),
> >
> > DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
> > DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 5c7acc055a..2eee59af98 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -433,6 +433,7 @@ struct RISCVCPUConfig {
> > bool ext_zve32f;
> > bool ext_zve64f;
> > bool ext_zmmul;
> > +bool ext_ssptwad;
> > bool rvv_ta_all_1s;
> >
> > uint32_t mvendorid;
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 59b3680b1b..a8607c0d7b 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -981,6 +981,15 @@ restart:
> >
> > /* Page table updates need to be atomic with MTTCG enabled
> */
> > if (updated_pte != pte) {
> > +if (!cpu->cfg.ext_ssptwad) {
> > +/*
> > + * If A/D bits are managed by SW, HW just raises the
> > + * page fault exception corresponding to the
> original
> > + * access type when A/D bits need to be updated.
> > + */
> > +return TRANSLATE_FAIL;
> > +}
> > +
> > /*
> > * - if accessed or dirty bits need updating, and the
> PTE is
> > * in RAM, then we do so atomically with a compare
> and swap.
> > --
> > 2.17.1
> >
> >
>
[PATCH] target/riscv: Support SW update of PTE A/D bits and Ssptwad extension
RISC-V priv spec v1.12 permits 2 PTE-update schemes of A/D-bit
(Access/Dirty bit): HW update or SW update. RISC-V profile defines the
extension name 'Ssptwad' for HW update to PTE A/D bits.
https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc
Current QEMU RISC-V implements HW update scheme, so this commit
introduces SW update scheme to QEMU and uses the 'Ssptwad' extension
as the CPU option to select 2 PTE-update schemes. QEMU RISC-V CPU still
uses HW update scheme (ext_ssptwad=true) by default to keep the backward
compatibility.
SW update scheme implemention is based on priv spec v1.12:
"When a virtual page is accessed and the A bit is clear, or is written
and the D bit is clear, a page-fault exception (corresponding to the
original access type) is raised."
Signed-off-by: Jim Shu
Reviewed-by: Frank Chang
---
target/riscv/cpu.c| 2 ++
target/riscv/cpu.h| 1 +
target/riscv/cpu_helper.c | 9 +
3 files changed, 12 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1bb3973806..1d38c1c1d2 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -857,6 +857,7 @@ static void riscv_cpu_init(Object *obj)
cpu->cfg.ext_ifencei = true;
cpu->cfg.ext_icsr = true;
+cpu->cfg.ext_ssptwad = true;
cpu->cfg.mmu = true;
cpu->cfg.pmp = true;
@@ -900,6 +901,7 @@ static Property riscv_cpu_extensions[] = {
DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
+DEFINE_PROP_BOOL("ssptwad", RISCVCPU, cfg.ext_ssptwad, true),
DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 5c7acc055a..2eee59af98 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -433,6 +433,7 @@ struct RISCVCPUConfig {
bool ext_zve32f;
bool ext_zve64f;
bool ext_zmmul;
+bool ext_ssptwad;
bool rvv_ta_all_1s;
uint32_t mvendorid;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 59b3680b1b..a8607c0d7b 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -981,6 +981,15 @@ restart:
/* Page table updates need to be atomic with MTTCG enabled */
if (updated_pte != pte) {
+if (!cpu->cfg.ext_ssptwad) {
+/*
+ * If A/D bits are managed by SW, HW just raises the
+ * page fault exception corresponding to the original
+ * access type when A/D bits need to be updated.
+ */
+return TRANSLATE_FAIL;
+}
+
/*
* - if accessed or dirty bits need updating, and the PTE is
* in RAM, then we do so atomically with a compare and swap.
--
2.17.1
Re: [PATCH v4 11/14] softmmu/memory: add memory_region_try_add_subregion function
Tested-by: Jim Shu On Fri, Mar 4, 2022 at 7:00 PM Damien Hedde wrote: > > > > On 3/3/22 14:32, Philippe Mathieu-Daudé wrote: > > On 23/2/22 10:12, Damien Hedde wrote: > >> Hi Philippe, > >> > >> I suppose it is ok if I change your mail in the reviewed by ? > > > > No, the email is fine (git tools should take care of using the > > correct email via the .mailmap entry, see commit 90f285fd83). > > > >> Thanks, > >> Damien > >> > > ok. > > Looks like git keeps as-is the "*-by:" entries untouched when cc-ing them. > > -- > Damien > >
Re: [PATCH v4 09/14] none-machine: allow cold plugging sysbus devices
Tested-by: Jim Shu On Thu, Mar 3, 2022 at 10:46 PM Philippe Mathieu-Daudé < philippe.mathieu.da...@gmail.com> wrote: > On 23/2/22 10:07, Damien Hedde wrote: > > Allow plugging any sysbus device on this machine (the sysbus > > devices still need to be 'user-creatable'). > > > > This commit is needed to use the 'none' machine as a base, and > > subsequently to dynamically populate it with sysbus devices using > > qapi commands. > > > > Note that this only concern cold-plug: sysbus devices cann't be hot > > "can not" is easier to understand for non-native / not good level of > English speakers IMHO. > > > plugged because the sysbus bus does not support it. > > > > Signed-off-by: Damien Hedde > > --- > > hw/core/null-machine.c | 4 > > 1 file changed, 4 insertions(+) > > Reviewed-by: Philippe Mathieu-Daudé > >
Re: [PATCH v4 13/14] hw/mem/system-memory: add a memory sysbus device
Tested-by: Jim Shu
On Wed, Feb 23, 2022 at 5:14 PM Damien Hedde
wrote:
> This device can be used to create a memory wrapped into a
> sysbus device.
> This device has one property 'readonly' which allows
> to choose between a ram or a rom.
>
> The purpose for this device is to be used with qapi command
> device_add.
>
> Signed-off-by: Damien Hedde
> ---
> include/hw/mem/sysbus-memory.h | 28
> hw/mem/sysbus-memory.c | 80 ++
> hw/mem/meson.build | 2 +
> 3 files changed, 110 insertions(+)
> create mode 100644 include/hw/mem/sysbus-memory.h
> create mode 100644 hw/mem/sysbus-memory.c
>
> diff --git a/include/hw/mem/sysbus-memory.h
> b/include/hw/mem/sysbus-memory.h
> new file mode 100644
> index 00..5c596f8b4f
> --- /dev/null
> +++ b/include/hw/mem/sysbus-memory.h
> @@ -0,0 +1,28 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * SysBusDevice Memory
> + *
> + * Copyright (c) 2021 Greensocs
> + */
> +
> +#ifndef HW_SYSBUS_MEMORY_H
> +#define HW_SYSBUS_MEMORY_H
> +
> +#include "hw/sysbus.h"
> +#include "qom/object.h"
> +
> +#define TYPE_SYSBUS_MEMORY "sysbus-memory"
> +OBJECT_DECLARE_SIMPLE_TYPE(SysBusMemoryState, SYSBUS_MEMORY)
> +
> +struct SysBusMemoryState {
> +/* */
> +SysBusDevice parent_obj;
> +uint64_t size;
> +bool readonly;
> +
> +/* */
> +MemoryRegion mem;
> +};
> +
> +#endif /* HW_SYSBUS_MEMORY_H */
> diff --git a/hw/mem/sysbus-memory.c b/hw/mem/sysbus-memory.c
> new file mode 100644
> index 00..f1ad7ba7ec
> --- /dev/null
> +++ b/hw/mem/sysbus-memory.c
> @@ -0,0 +1,80 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * SysBusDevice Memory
> + *
> + * Copyright (c) 2021 Greensocs
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/mem/sysbus-memory.h"
> +#include "hw/qdev-properties.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +#include "qapi/error.h"
> +
> +static Property sysbus_memory_properties[] = {
> +DEFINE_PROP_UINT64("size", SysBusMemoryState, size, 0),
> +DEFINE_PROP_BOOL("readonly", SysBusMemoryState, readonly, false),
> +DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void sysbus_memory_realize(DeviceState *dev, Error **errp)
> +{
> +SysBusMemoryState *s = SYSBUS_MEMORY(dev);
> +gchar *name;
> +
> +if (!s->size) {
> +error_setg(errp, "'size' must be non-zero.");
> +return;
> +}
> +
> +/*
> + * We impose having an id (which is unique) because we need to
> generate
> + * a unique name for the memory region.
> + * memory_region_init_ram/rom() will abort() (in qemu_ram_set_idstr()
> + * function if 2 system-memory devices are created with the same name
> + * for the memory region).
> + */
> +if (!dev->id) {
> +error_setg(errp, "system-memory device must have an id.");
> +return;
> +}
> +name = g_strdup_printf("%s.region", dev->id);
> +
> +if (s->readonly) {
> +memory_region_init_rom(>mem, OBJECT(dev), name, s->size, errp);
> +} else {
> +memory_region_init_ram(>mem, OBJECT(dev), name, s->size, errp);
> +}
> +
> +g_free(name);
> +if (*errp) {
> +return;
> +}
> +
> +sysbus_init_mmio(SYS_BUS_DEVICE(s), >mem);
> +}
> +
> +static void sysbus_memory_class_init(ObjectClass *klass, void *data)
> +{
> +DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +dc->user_creatable = true;
> +dc->realize = sysbus_memory_realize;
> +device_class_set_props(dc, sysbus_memory_properties);
> +}
> +
> +static const TypeInfo sysbus_memory_info = {
> +.name = TYPE_SYSBUS_MEMORY,
> +.parent= TYPE_SYS_BUS_DEVICE,
> +.instance_size = sizeof(SysBusMemoryState),
> +.class_init= sysbus_memory_class_init,
> +};
> +
> +static void sysbus_memory_register_types(void)
> +{
> +type_register_static(_memory_info);
> +}
> +
> +type_init(sysbus_memory_register_types)
> diff --git a/hw/mem/meson.build b/hw/mem/meson.build
> index 82f86d117e..04c74e12f2 100644
> --- a/hw/mem/meson.build
> +++ b/hw/mem/meson.build
> @@ -7,3 +7,5 @@ mem_ss.add(when: 'CONFIG_NVDIMM', if_true:
> files('nvdimm.c'))
> softmmu_ss.add_all(when: 'CONFIG_MEM_DEVICE', if_true: mem_ss)
>
> softmmu_ss.add(when: 'CONFIG_SPARSE_MEM', if_true: files('sparse-mem.c'))
> +
> +softmmu_ss.add(files('sysbus-memory.c'))
> --
> 2.35.1
>
>
>
Re: [PATCH v4 14/14] hw: set user_creatable on opentitan/sifive_e devices
Tested-by: Jim Shu On Fri, Mar 4, 2022 at 11:23 PM Philippe Mathieu-Daudé < philippe.mathieu.da...@gmail.com> wrote: > On 23/2/22 10:07, Damien Hedde wrote: > > The devices are: > > + ibex-timer > > + ibex-uart > > + riscv.aclint.swi > > + riscv.aclint.mtimer > > + riscv.hart_array > > + riscv.sifive.e.prci > > + riscv.sifive.plic > > + riscv.sifive.uart > > + sifive_soc.gpio > > + unimplemented-device > > > > These devices are clean regarding error handling in realize. > > > > They are all sysbus devices, so setting user-creatable will only > > enable cold-plugging them on machine having explicitely allowed them > > (only _none_ machine does that). > > > > Note that this commit include the ricv_array which embeds cpus. There > > Typo "includes" I guess. > > Reviewed-by: Philippe Mathieu-Daudé > > > are some deep internal constraints about them: you cannot create more > > cpus than the machine's maxcpus. TCG accelerator's code will for example > > assert if a user try to create too many cpus. > > > > Signed-off-by: Damien Hedde > > --- > > > > I can also split this patch if you think it's better. > > But it is mostly a one-line fix per file. > > > > This patch requires first some cleanups in order to fix error errors > > and some more memory leaks that could happend in legit user-related > > life cycles: a miss-configuration should not be a fatal error anymore. > > > https://lore.kernel.org/qemu-devel/20220218164646.132112-1-damien.he...@greensocs.com > > --- > > hw/char/ibex_uart.c | 1 + > > hw/char/sifive_uart.c | 1 + > > hw/gpio/sifive_gpio.c | 1 + > > hw/intc/riscv_aclint.c | 2 ++ > > hw/intc/sifive_plic.c | 1 + > > hw/misc/sifive_e_prci.c | 8 > > hw/misc/unimp.c | 1 + > > hw/riscv/riscv_hart.c | 1 + > > hw/timer/ibex_timer.c | 1 + > > 9 files changed, 17 insertions(+) > >
Re: [PATCH v4 08/14] none-machine: add 'ram-addr' property
Tested-by: Jim Shu
On Fri, Mar 4, 2022 at 12:36 AM Damien Hedde
wrote:
>
>
> On 3/3/22 15:41, Philippe Mathieu-Daudé wrote:
> > On 23/2/22 10:07, Damien Hedde wrote:
> >> Add the property to configure a the base address of the ram.
> >> The default value remains zero.
> >>
> >> This commit is needed to use the 'none' machine as a base, and
> >> subsequently to dynamically populate it using qapi commands. Having
> >> a non null 'ram' is really hard to workaround because of the actual
> >> constraints on the generic loader: it prevents loading binaries
> >> bigger than ram_size (with a null ram, we cannot load anything).
> >> For now we need to be able to use the existing ram creation
> >> feature of the none machine with a configurable base address.
> >>
> >> Signed-off-by: Damien Hedde
> >> ---
> >> hw/core/null-machine.c | 34 --
> >> 1 file changed, 32 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c
> >> index 7eb258af07..5fd1cc0218 100644
> >> --- a/hw/core/null-machine.c
> >> +++ b/hw/core/null-machine.c
> >> @@ -16,9 +16,11 @@
> >> #include "hw/boards.h"
> >> #include "exec/address-spaces.h"
> >> #include "hw/core/cpu.h"
> >> +#include "qapi/visitor.h"
> >> struct NoneMachineState {
> >> MachineState parent;
> >> +uint64_t ram_addr;
> >> };
> >> #define TYPE_NONE_MACHINE MACHINE_TYPE_NAME("none")
> >> @@ -26,6 +28,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(NoneMachineState,
> >> NONE_MACHINE)
> >> static void machine_none_init(MachineState *mch)
> >> {
> >> +NoneMachineState *nms = NONE_MACHINE(mch);
> >> CPUState *cpu = NULL;
> >> /* Initialize CPU (if user asked for it) */
> >> @@ -37,9 +40,13 @@ static void machine_none_init(MachineState *mch)
> >> }
> >> }
> >> -/* RAM at address zero */
> >> +/* RAM at configured address (default: 0) */
> >> if (mch->ram) {
> >> -memory_region_add_subregion(get_system_memory(), 0, mch->ram);
> >> +memory_region_add_subregion(get_system_memory(), nms->ram_addr,
> >> +mch->ram);
> >> +} else if (nms->ram_addr) {
> >> +error_report("'ram-addr' has been specified but the size is
> >> zero");
> >
> > I'm not sure about this error message, IIUC we can get here if no ram
> > backend is provided, not if we have one zero-sized. Otherwise LGTM.
>
> You're most probably right. Keeping the ram_size to 0 is just one way of
> getting here. I can replace the message by a more generic formulation
> "'ram-addr' has been specified but the machine has no ram"
>
>
>
Re: [PATCH v4 12/14] add sysbus-mmio-map qapi command
Tested-by: Jim Shu
On Wed, Feb 23, 2022 at 5:37 PM Damien Hedde
wrote:
> This command allows to map an mmio region of sysbus device onto
> the system memory. Its behavior mimics the sysbus_mmio_map()
> function apart from the automatic unmap (the C function unmaps
> the region if it is already mapped).
> For the qapi function we consider it is an error to try to map
> an already mapped function. If unmapping is required, it is
> probably better to add a sysbus-mmip-unmap command.
>
> This command is still experimental (hence the 'unstable' feature),
> as it is related to the sysbus device creation through qapi commands.
>
> This command is required to be able to dynamically build a machine
> from scratch as there is no qapi-way of doing a memory mapping.
>
> Signed-off-by: Damien Hedde
> ---
> Cc: Alistair Francis
>
> v4:
> + integrate priority parameter
> + use 'unstable' feature flag instead of 'x-' prefix
> + bump version to 7.0
> + dropped Alistair's reviewed-by as a consequence
> ---
> qapi/qdev.json | 31 ++
> hw/core/sysbus.c | 49
> 2 files changed, 80 insertions(+)
>
> diff --git a/qapi/qdev.json b/qapi/qdev.json
> index 2e2de41499..4830e87a90 100644
> --- a/qapi/qdev.json
> +++ b/qapi/qdev.json
> @@ -160,3 +160,34 @@
> ##
> { 'event': 'DEVICE_UNPLUG_GUEST_ERROR',
>'data': { '*device': 'str', 'path': 'str' } }
> +
> +##
> +# @sysbus-mmio-map:
> +#
> +# Map a sysbus device mmio onto the main system bus.
> +#
> +# @device: the device's QOM path
> +#
> +# @mmio: The mmio number to be mapped (defaults to 0).
> +#
> +# @addr: The base address for the mapping.
> +#
> +# @priority: The priority of the mapping (defaults to 0).
> +#
> +# Features:
> +# @unstable: Command is meant to map sysbus devices
> +#while in preconfig mode.
> +#
> +# Since: 7.0
> +#
> +# Returns: Nothing on success
> +#
> +##
> +
> +{ 'command': 'sysbus-mmio-map',
> + 'data': { 'device': 'str',
> +'*mmio': 'uint8',
> +'addr': 'uint64',
> +'*priority': 'int32' },
> + 'features': ['unstable'],
> + 'allow-preconfig' : true }
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index 05c1da3d31..df1f1f43a5 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -23,6 +23,7 @@
> #include "hw/sysbus.h"
> #include "monitor/monitor.h"
> #include "exec/address-spaces.h"
> +#include "qapi/qapi-commands-qdev.h"
>
> static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent);
> static char *sysbus_get_fw_dev_path(DeviceState *dev);
> @@ -154,6 +155,54 @@ static void sysbus_mmio_map_common(SysBusDevice *dev,
> int n, hwaddr addr,
> }
> }
>
> +void qmp_sysbus_mmio_map(const char *device,
> + bool has_mmio, uint8_t mmio,
> + uint64_t addr,
> + bool has_priority, int32_t priority,
> + Error **errp)
> +{
> +Object *obj = object_resolve_path_type(device, TYPE_SYS_BUS_DEVICE,
> NULL);
> +SysBusDevice *dev;
> +
> +if (phase_get() != PHASE_MACHINE_INITIALIZED) {
> +error_setg(errp, "The command is permitted only when "
> + "the machine is in initialized phase");
> +return;
> +}
> +
> +if (obj == NULL) {
> +error_setg(errp, "Device '%s' not found", device);
> +return;
> +}
> +dev = SYS_BUS_DEVICE(obj);
> +
> +if (!has_mmio) {
> +mmio = 0;
> +}
> +if (!has_priority) {
> +priority = 0;
> +}
> +
> +if (mmio >= dev->num_mmio) {
> +error_setg(errp, "MMIO index '%u' does not exist in '%s'",
> + mmio, device);
> +return;
> +}
> +
> +if (dev->mmio[mmio].addr != (hwaddr)-1) {
> +error_setg(errp, "MMIO index '%u' is already mapped", mmio);
> +return;
> +}
> +
> +if (!memory_region_try_add_subregion(get_system_memory(), addr,
> + dev->mmio[mmio].memory, priority,
> + errp)) {
> +return;
> +}
> +
> +dev->mmio[mmio].addr = addr;
> +}
> +
> void sysbus_mmio_unmap(SysBusDevice *dev, int n)
> {
> assert(n >= 0 && n < dev->num_mmio);
> --
> 2.35.1
>
>
>
Re: [PATCH v4 07/14] none-machine: add the NoneMachineState structure
Tested-by: Jim Shu
On Wed, Feb 23, 2022 at 5:59 PM Damien Hedde
wrote:
> The none machine was using the parent state structure.
> We'll need a custom state to add a field in the following commit.
>
> Signed-off-by: Damien Hedde
> ---
> hw/core/null-machine.c | 24 ++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c
> index f586a4bef5..7eb258af07 100644
> --- a/hw/core/null-machine.c
> +++ b/hw/core/null-machine.c
> @@ -17,6 +17,13 @@
> #include "exec/address-spaces.h"
> #include "hw/core/cpu.h"
>
> +struct NoneMachineState {
> +MachineState parent;
> +};
> +
> +#define TYPE_NONE_MACHINE MACHINE_TYPE_NAME("none")
> +OBJECT_DECLARE_SIMPLE_TYPE(NoneMachineState, NONE_MACHINE)
> +
> static void machine_none_init(MachineState *mch)
> {
> CPUState *cpu = NULL;
> @@ -42,8 +49,10 @@ static void machine_none_init(MachineState *mch)
> }
> }
>
> -static void machine_none_machine_init(MachineClass *mc)
> +static void machine_none_class_init(ObjectClass *oc, void *data)
> {
> +MachineClass *mc = MACHINE_CLASS(oc);
> +
> mc->desc = "empty machine";
> mc->init = machine_none_init;
> mc->max_cpus = 1;
> @@ -56,4 +65,15 @@ static void machine_none_machine_init(MachineClass *mc)
> mc->no_sdcard = 1;
> }
>
> -DEFINE_MACHINE("none", machine_none_machine_init)
> +static const TypeInfo none_machine_info = {
> +.name = TYPE_NONE_MACHINE,
> +.parent= TYPE_MACHINE,
> +.instance_size = sizeof(NoneMachineState),
> +.class_init= machine_none_class_init,
> +};
> +
> +static void none_machine_register_types(void)
> +{
> +type_register_static(_machine_info);
> +}
> +type_init(none_machine_register_types);
> --
> 2.35.1
>
>
>
Re: [PATCH v4 05/14] qapi/device_add: handle the rom_order_override when cold-plugging
Tested-by: Jim Shu
On Wed, Feb 23, 2022 at 5:18 PM Damien Hedde
wrote:
> rom_set_order_override() and rom_reset_order_override() were called
> in qemu_create_cli_devices() to set the rom_order_override value
> once and for all when creating the devices added on CLI.
>
> Unfortunately this won't work with qapi commands.
>
> Move the calls inside device_add so that it will be done in every
> case:
> + CLI option: -device
> + QAPI command: device_add
>
> rom_[set|reset]_order_override() are implemented in hw/core/loader.c
> They either do nothing or call fw_cfg_[set|reset]_order_override().
> The later functions are implemented in hw/nvram/fw_cfg.c and only
> change an integer value of a "global" variable.
> In consequence, there are no complex side effects involved and we can
> safely move them from outside the -device option loop to the inner
> function.
>
> Signed-off-by: Damien Hedde
> ---
> softmmu/qdev-monitor.c | 11 +++
> softmmu/vl.c | 2 --
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 47a89aee20..9ec3e0ebff 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -43,6 +43,7 @@
> #include "hw/qdev-properties.h"
> #include "hw/clock.h"
> #include "hw/boards.h"
> +#include "hw/loader.h"
>
> /*
> * Aliases were a bad idea from the start. Let's keep them
> @@ -671,6 +672,10 @@ DeviceState *qdev_device_add_from_qdict(const QDict
> *opts,
> return NULL;
> }
>
> +if (!is_hotplug) {
> +rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE);
> +}
> +
> /* create device */
> dev = qdev_new(driver);
>
> @@ -712,6 +717,9 @@ DeviceState *qdev_device_add_from_qdict(const QDict
> *opts,
> if (!qdev_realize(DEVICE(dev), bus, errp)) {
> goto err_del_dev;
> }
> +if (!is_hotplug) {
> +rom_reset_order_override();
> +}
> return dev;
>
> err_del_dev:
> @@ -719,6 +727,9 @@ err_del_dev:
> object_unparent(OBJECT(dev));
> object_unref(OBJECT(dev));
> }
> +if (!is_hotplug) {
> +rom_reset_order_override();
> +}
> return NULL;
> }
>
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 50337d68b9..b91ae1b8ae 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2680,7 +2680,6 @@ static void qemu_create_cli_devices(void)
> }
>
> /* init generic devices */
> -rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE);
> qemu_opts_foreach(qemu_find_opts("device"),
>device_init_func, NULL, _fatal);
> QTAILQ_FOREACH(opt, _opts, next) {
> @@ -2697,7 +2696,6 @@ static void qemu_create_cli_devices(void)
> object_unref(OBJECT(dev));
> loc_pop(>loc);
> }
> -rom_reset_order_override();
> }
>
> static void qemu_machine_creation_done(void)
> --
> 2.35.1
>
>
>
Re: [PATCH v5 3/6] vl: support machine-initialized target in phase_until()
Tested-by: Jim Shu
On Thu, May 19, 2022 at 11:36 PM Damien Hedde
wrote:
> phase_until() now supports the following transitions:
> + accel-created -> machine-initialized
> + machine-initialized -> machine-ready
>
> As a consequence we can now support the use of qmp_exit_preconfig()
> from phases _accel-created_ and _machine-initialized_.
>
> This commit is a preparation to support cold plugging a device
> using qapi (which will be introduced in a following commit). For this
> we need fine grain control of the phase.
>
> Signed-off-by: Damien Hedde
> Reviewed-by: Philippe Mathieu-Daudé
> ---
>
> v5: update due to refactor of previous commit
> ---
> softmmu/vl.c | 26 +-
> 1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 7f8d15b5b8..ea15e37973 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2698,8 +2698,9 @@ static void qemu_machine_creation_done(void)
>
> void qmp_x_exit_preconfig(Error **errp)
> {
> -if (phase_check(PHASE_MACHINE_INITIALIZED)) {
> -error_setg(errp, "The command is permitted only before machine
> initialization");
> +if (phase_check(PHASE_MACHINE_READY)) {
> +error_setg(errp, "The command is permitted only before"
> + " machine is ready");
> return;
> }
> phase_until(PHASE_MACHINE_READY, errp);
> @@ -2707,9 +2708,6 @@ void qmp_x_exit_preconfig(Error **errp)
>
> static void qemu_phase_ready(Error **errp)
> {
> -qemu_init_board();
> -/* phase is now PHASE_MACHINE_INITIALIZED. */
> -qemu_create_cli_devices();
> cxl_fixed_memory_window_link_targets(errp);
> qemu_machine_creation_done();
> /* Phase is now PHASE_MACHINE_READY. */
> @@ -2749,6 +2747,24 @@ bool phase_until(MachineInitPhase phase, Error
> **errp)
>
> switch (cur_phase) {
> case PHASE_ACCEL_CREATED:
> +qemu_init_board();
> +/* Phase is now PHASE_MACHINE_INITIALIZED. */
> +/*
> + * Handle CLI devices now in order leave this case in a state
> + * where we can cold plug devices with QMP. The following call
> + * handles the CLI options:
> + * + -fw_cfg (has side effects on device cold plug)
> + * + -device
> + */
> +qemu_create_cli_devices();
> +/*
> + * At this point all CLI options are handled apart:
> + * + -S (autostart)
> + * + -incoming
> + */
> +break;
> +
> +case PHASE_MACHINE_INITIALIZED:
> qemu_phase_ready(errp);
> break;
>
> --
> 2.36.1
>
>
>
Re: [PATCH v5 2/6] machine: introduce phase_until() to handle phase transitions
Tested-by: Jim Shu
On Thu, May 19, 2022 at 11:41 PM Damien Hedde
wrote:
> phase_until() is implemented in vl.c and is meant to be used
> to make startup progress up to a specified phase being reached().
> At this point, no behavior change is introduced: phase_until()
> only supports a single double transition corresponding
> to the functionality of qmp_exit_preconfig():
> + accel-created -> machine-initialized -> machine-ready
>
> As a result qmp_exit_preconfig() now uses phase_until().
>
> This commit is a preparation to support cold plugging a device
> using qapi command (which will be introduced in a following commit).
> For this we need fine grain control of the phase.
>
> Signed-off-by: Damien Hedde
> ---
>
> v5:
> + refactor to avoid indentation change
> ---
> include/hw/qdev-core.h | 14 +
> softmmu/vl.c | 46 ++
> 2 files changed, 60 insertions(+)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index e29c705b74..5f73d06408 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -909,4 +909,18 @@ extern bool phase_check(MachineInitPhase phase);
> */
> extern void phase_advance(MachineInitPhase phase);
>
> +/**
> + * @phase_until:
> + * @phase: the target phase
> + * @errp: error report
> + *
> + * Make the machine init progress until the target phase is reached.
> + *
> + * Its is a no-op is the target phase is the current or an earlier
> + * phase.
> + *
> + * Returns true in case of success.
> + */
> +extern bool phase_until(MachineInitPhase phase, Error **errp);
> +
> #endif
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 84a31eba76..7f8d15b5b8 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2702,11 +2702,17 @@ void qmp_x_exit_preconfig(Error **errp)
> error_setg(errp, "The command is permitted only before machine
> initialization");
> return;
> }
> +phase_until(PHASE_MACHINE_READY, errp);
> +}
>
> +static void qemu_phase_ready(Error **errp)
> +{
> qemu_init_board();
> +/* phase is now PHASE_MACHINE_INITIALIZED. */
> qemu_create_cli_devices();
> cxl_fixed_memory_window_link_targets(errp);
> qemu_machine_creation_done();
> +/* Phase is now PHASE_MACHINE_READY. */
>
> if (loadvm) {
> load_snapshot(loadvm, NULL, false, NULL, _fatal);
> @@ -2729,6 +2735,46 @@ void qmp_x_exit_preconfig(Error **errp)
> }
> }
>
> +bool phase_until(MachineInitPhase phase, Error **errp)
> +{
> +ERRP_GUARD();
> +if (!phase_check(PHASE_ACCEL_CREATED)) {
> +error_setg(errp, "Phase transition is not supported until
> accelerator"
> + " is created");
> +return false;
> +}
> +
> +while (!phase_check(phase)) {
> +MachineInitPhase cur_phase = phase_get();
> +
> +switch (cur_phase) {
> +case PHASE_ACCEL_CREATED:
> +qemu_phase_ready(errp);
> +break;
> +
> +default:
> +/*
> + * If we end up here, it is because we miss a case above.
> + */
> +error_setg(_abort, "Requested phase transition is not"
> + " implemented");
> +return false;
> +}
> +
> +if (*errp) {
> +return false;
> +}
> +
> +/*
> + * Ensure we made some progress.
> + * With the default case above, it should be enough to prevent
> + * any infinite loop.
> + */
> +assert(cur_phase < phase_get());
> +}
> +return true;
> +}
> +
> void qemu_init(int argc, char **argv, char **envp)
> {
> QemuOpts *opts;
> --
> 2.36.1
>
>
>
Re: [PATCH v5 4/6] qapi/device_add: compute is_hotplug flag
Tested-by: Jim Shu
On Thu, May 19, 2022 at 11:37 PM Damien Hedde
wrote:
> Instead of checking the phase everytime, just store the result
> in a flag. We will use more of it in the following commit.
>
> Signed-off-by: Damien Hedde
> Reviewed-by: Philippe Mathieu-Daudé
> ---
> softmmu/qdev-monitor.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 12fe60c467..d68ef883b5 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -619,6 +619,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict
> *opts,
> char *id;
> DeviceState *dev = NULL;
> BusState *bus = NULL;
> +bool is_hotplug = phase_check(PHASE_MACHINE_READY);
>
> driver = qdict_get_try_str(opts, "driver");
> if (!driver) {
> @@ -662,7 +663,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict
> *opts,
> return NULL;
> }
>
> -if (phase_check(PHASE_MACHINE_READY) && bus &&
> !qbus_is_hotpluggable(bus)) {
> +if (is_hotplug && bus && !qbus_is_hotpluggable(bus)) {
> error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
> return NULL;
> }
> @@ -676,7 +677,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict
> *opts,
> dev = qdev_new(driver);
>
> /* Check whether the hotplug is allowed by the machine */
> -if (phase_check(PHASE_MACHINE_READY)) {
> +if (is_hotplug) {
> if (!qdev_hotplug_allowed(dev, errp)) {
> goto err_del_dev;
> }
> --
> 2.36.1
>
>
>
Re: [PATCH v5 6/6] qapi/device_add: Allow execution in machine initialized phase
Tested-by: Jim Shu
On Thu, May 19, 2022 at 11:37 PM Damien Hedde
wrote:
> From: Mirela Grujic
>
> This commit allows to use the QMP command to add a cold-plugged
> device like we can do with the CLI option -device.
>
> Note: for device_add command in qdev.json adding the 'allow-preconfig'
> option has no effect because the command appears to bypass QAPI (see
> TODO at qapi/qdev.json:61). The option is added there solely to
> document the intent.
> For the same reason, the flags have to be explicitly set in
> monitor_init_qmp_commands() when the device_add command is registered.
>
> Signed-off-by: Mirela Grujic
> Signed-off-by: Damien Hedde
> ---
>
> v4:
> + use phase_until()
> + add missing flag in hmp-commands.hx
> ---
> qapi/qdev.json | 3 ++-
> monitor/misc.c | 2 +-
> softmmu/qdev-monitor.c | 4
> hmp-commands.hx| 1 +
> 4 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/qdev.json b/qapi/qdev.json
> index 26cd10106b..2e2de41499 100644
> --- a/qapi/qdev.json
> +++ b/qapi/qdev.json
> @@ -77,7 +77,8 @@
> { 'command': 'device_add',
>'data': {'driver': 'str', '*bus': 'str', '*id': 'str'},
>'gen': false, # so we can get the additional arguments
> - 'features': ['json-cli', 'json-cli-hotplug'] }
> + 'features': ['json-cli', 'json-cli-hotplug'],
> + 'allow-preconfig': true }
>
> ##
> # @device_del:
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 6c5bb82d3b..d3d413d70c 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -233,7 +233,7 @@ static void monitor_init_qmp_commands(void)
> qmp_init_marshal(_commands);
>
> qmp_register_command(_commands, "device_add",
> - qmp_device_add, 0, 0);
> + qmp_device_add, QCO_ALLOW_PRECONFIG, 0);
>
> QTAILQ_INIT(_cap_negotiation_commands);
> qmp_register_command(_cap_negotiation_commands,
> "qmp_capabilities",
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 7cbee2b0d8..c53f62be51 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -855,6 +855,10 @@ void qmp_device_add(QDict *qdict, QObject **ret_data,
> Error **errp)
> QemuOpts *opts;
> DeviceState *dev;
>
> +if (!phase_until(PHASE_MACHINE_INITIALIZED, errp)) {
> +return;
> +}
> +
> opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, errp);
> if (!opts) {
> return;
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 03e6a73d1f..0091b8e2dd 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -672,6 +672,7 @@ ERST
> .help = "add device, like -device on the command line",
> .cmd= hmp_device_add,
> .command_completion = device_add_completion,
> +.flags = "p",
> },
>
> SRST
> --
> 2.36.1
>
>
>
Re: [PATCH v5 1/6] machine: add phase_get() and document phase_check()/advance()
Tested-by: Jim Shu
On Thu, May 19, 2022 at 11:41 PM Damien Hedde
wrote:
> phase_get() returns the current phase, we'll use it in next
> commit.
>
> Signed-off-by: Damien Hedde
> Reviewed-by: Philippe Mathieu-Daudé
> ---
> include/hw/qdev-core.h | 19 +++
> hw/core/qdev.c | 5 +
> 2 files changed, 24 insertions(+)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 92c3d65208..e29c705b74 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -887,7 +887,26 @@ typedef enum MachineInitPhase {
> PHASE_MACHINE_READY,
> } MachineInitPhase;
>
> +/*
> + * phase_get:
> + * Returns the current phase
> + */
> +MachineInitPhase phase_get(void);
> +
> +/**
> + * phase_check:
> + * Test if current phase is at least @phase.
> + *
> + * Returns true if this is the case.
> + */
> extern bool phase_check(MachineInitPhase phase);
> +
> +/**
> + * @phase_advance:
> + * Update the current phase to @phase.
> + *
> + * Must only be used to make a single phase step.
> + */
> extern void phase_advance(MachineInitPhase phase);
>
> #endif
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 84f3019440..632dc0a4be 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -910,6 +910,11 @@ Object *qdev_get_machine(void)
>
> static MachineInitPhase machine_phase;
>
> +MachineInitPhase phase_get(void)
> +{
> +return machine_phase;
> +}
> +
> bool phase_check(MachineInitPhase phase)
> {
> return machine_phase >= phase;
> --
> 2.36.1
>
>
>
Re: [PATCH v4 00/14] Initial support for machine creation via QMP
Hi all, Thanks for the work! I'm from SiFive and we are very interested in this feature. QMP/QAPI configurable QEMU machine is a useful feature in our use case. With this feature, we can both model our versatile FPGA-based platforms more easily and model a new platform without modification of source code. It is helpful for early software development of SoC prototyping. We think this feature is also helpful to the QEMU community. Also, I have tested this patchset (v4) and newer v5 patchset [1] with Damien's firmware [2] and it works correctly. p.s. QMP option "-qmp socket,path=./qmpsocket,server" in v5 patchset instruction may not work? I use the option "-qmp unix:./qmpsocket,server" instead. [1] [PATCH v5 0/6] QAPI support for device cold-plug https://lore.kernel.org/qemu-devel/20220519153402.41540-1-damien.he...@greensocs.com/ [2] Test firmware for patchset v5: https://github.com/GreenSocs/qemu-qmp-machines/tree/master/arm-virt v4: https://github.com/GreenSocs/qemu-qmp-machines/tree/eba16dab8b587e624d65c5c302aeef424bece3a0 On Thu, Mar 3, 2022 at 7:02 PM Damien Hedde wrote: > Ping ! > > It would be good to have some feedback on 1st and 2nd part. > > Thanks, > Damien > > On 2/23/22 10:06, Damien Hedde wrote: > > Hi, > > > > This series adds initial support to build a machine using QMP/QAPI > > commands. With this series, one can start from the 'none' machine, > > create cpus, sysbus devices, memory map them and wire interrupts. > > > > Sorry for the huge cc list on this cover-letter. Apart from people > > who attended the kvm call about this topic, I've cc'ed you only > > according to MAINTAINERS file. > > > > The series is divided in 4 parts which are independent of each other, > > but we need the 4 parts to be able to use this mechanism: > > + Patches 1 to 6 allow to use the qapi command device_add to cold > >plug devices (like CLI -device do) > > + Patches 7 to 10 modify the 'none' machine which serves as base > >machine. > > + Patches 11 to 13 handle memory mapping and memory creation > > + Patches 14 allows dynamic cold plug of opentitan/sifive_e machine > >to build some example. This last patch is based on a cleanup > >series: it probably works without it, but some config errors are > >not handled (see based-on below). > > > > Only patch 11 is reviewed-by. > > > > v4: > > + cold plugging approach changed in order not to conflict with > >startup. I do not add additional command to handle this so that > >we can change everything easily. > > + device_add in cold plug context is also now equivalent to -device > >CLI regarding -fw_cfg. I also added patches to modify the 'none' > >machine. > > + reworked most of the none machine part > > + updated the sybus-mmio-map command patch > > > > Note that there are still lot of limitations (for example if you try > > to create more cpus than the _max_cpus_, tcg will abort()). > > Basically all tasks done by machine init reading some parameters are > > really tricky: for example, loading complex firmware. But we have to > > start by something and all this is not accessible unless the user > > asked for none machine and -preconfig. > > > > I can maintain the code introduced here. I'm not sure what's the > > process. Is there something else to do than propose a patch to > > MAINTAINERS ? > > If there is a global agreement on moving on with these feature, it > > would be great to have a login on qemu wiki so I can document > > limitations and the work being done to solve them. > > > > A simple test can be done with the following scenario which build > > a machine subset of the opentitan. > > > > $ cat commands.qmp > > // RAM 0x1000 > > device_add driver=sysbus-memory id=ram size=0x4000 readonly=false > > sysbus-mmio-map device=ram addr=268435456 > > // CPUS > > device_add driver=riscv.hart_array id=cpus > cpu-type=lowrisc-ibex-riscv-cpu num-harts=1 resetvec=0x8080 > > // ROM 0x8000 > > device_add driver=sysbus-memory id=rom size=0x4000 readonly=true > > sysbus-mmio-map device=rom addr=32768 > > // PLIC 0x4800 > > device_add driver=riscv.sifive.plic id=plic hart-config=M hartid-base=0 > num-sources=180 num-priorities=3 priority-base=0x0 pending-base=0x1000 > enable-base=0x2000 enable-stride=32 context-base=0x20 context-stride=8 > aperture-size=0x4005000 > > sysbus-mmio-map device=plic addr=1207959552 > > qom-set path=plic property=unnamed-gpio-out[1] > value=cpus/harts[0]/unnamed-gpio-in[11] > > // UART 0x4000 > > device_add driver=ibex-uart id=uart chardev=serial0 > > sysbus-mmio-map device=uart addr=1073741824 > > qom-set path=uart property=sysbus-irq[1] value=plic/unnamed-gpio-in[2] > > // FIRMWARE > > device_add driver=loader cpu-num=0 file=/path/to/firmware.elf > > x-exit-preconfig > > > > $ qemu-system-riscv32 -display none -M none -preconfig -serial stdio > -qmp unix:/tmp/qmp-sock,server > > > > In another terminal, you'll need to send the commands with, for example: > > $ grep -v '^//'
[PATCH v2 0/2] Align SiFive PDMA behavior to real hardware
HiFive Unmatched PDMA supports high/low 32-bit access of 64-bit register, but QEMU emulation supports low part access now. Enhance QEMU emulation to support high 32-bit access. Also, permit 4/8-byte valid access in PDMA as we have verified 32/64-bit accesses of PDMA registers are supported. Changelog: v2: * Fix high 32-bit write access of 64-bit RO registers * Fix commit log Jim Shu (2): hw/dma: sifive_pdma: support high 32-bit access of 64-bit register hw/dma: sifive_pdma: permit 4/8-byte access size of PDMA registers hw/dma/sifive_pdma.c | 181 +-- 1 file changed, 159 insertions(+), 22 deletions(-) -- 2.25.1
[PATCH v2 2/2] hw/dma: sifive_pdma: permit 4/8-byte access size of PDMA registers
It's obvious that PDMA supports 64-bit access of 64-bit registers, and
in previous commit, we confirm that PDMA supports 32-bit access of
both 32/64-bit registers. Thus, we configure 32/64-bit memory access
of PDMA registers as valid in general.
Signed-off-by: Jim Shu
Reviewed-by: Frank Chang
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Reviewed-by: Bin Meng
Tested-by: Bin Meng
---
hw/dma/sifive_pdma.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
index f4df16449b..1dd88f3479 100644
--- a/hw/dma/sifive_pdma.c
+++ b/hw/dma/sifive_pdma.c
@@ -444,6 +444,10 @@ static const MemoryRegionOps sifive_pdma_ops = {
.impl = {
.min_access_size = 4,
.max_access_size = 8,
+},
+.valid = {
+.min_access_size = 4,
+.max_access_size = 8,
}
};
--
2.25.1
[PATCH v2 1/2] hw/dma: sifive_pdma: support high 32-bit access of 64-bit register
Real PDMA supports high 32-bit read/write memory access of 64-bit
register.
The following result is PDMA tested in U-Boot on Unmatched board:
1. Real PDMA allows high 32-bit read/write to 64-bit register.
=> mw.l 0x300 0x0 <= Disclaim channel 0
=> mw.l 0x300 0x1 <= Claim channel 0
=> mw.l 0x310 0x8000 <= Write low 32-bit NextDest
(NextDest = 0x28000)
=> mw.l 0x314 0x2 <= Write high 32-bit NextDest
=> md.l 0x310 1<= Dump low 32-bit NextDest
0310: 8000
=> md.l 0x314 1<= Dump high 32-bit NextDest
0314: 0002
=> mw.l 0x318 0x80001000 <= Write low 32-bit NextSrc (NextSrc
= 0x280001000)
=> mw.l 0x31c 0x2 <= Write high 32-bit NextSrc
=> md.l 0x318 1<= Dump low 32-bit NextSrc
0310: 80001000
=> md.l 0x31c 1<= Dump high 32-bit NextSrc
0314: 0002
2. PDMA transfer from 0x280001000 to 0x28000 is OK.
=> mw.q 0x308 0x4 <= NextBytes = 4
=> mw.l 0x304 0x2200 <= wsize = rsize = 2 (2^2 = 4 bytes)
=> mw.l 0x28000 0x87654321 <= Fill test data to dst
=> mw.l 0x280001000 0x12345678 <= Fill test data to src
=> md.l 0x28000 1; md.l 0x280001000 1 <= Dump src/dst memory contents
28000: 87654321 !Ce.
280001000: 12345678 xV4.
=> md.l 0x300 8<= Dump PDMA status
0300: 0001 2200 0004 ..."
0310: 8000 0002 80001000 0002
=> mw.l 0x300 0x3 <= Set channel 0 run and claim bits
=> md.l 0x300 8<= Dump PDMA status
0300: 4001 2200 0004 ...@..."
0310: 8000 0002 80001000 0002
=> md.l 0x28000 1; md.l 0x280001000 1 <= Dump src/dst memory contents
280000000: 12345678 xV4.
280001000: 12345678 xV4.
Signed-off-by: Jim Shu
Reviewed-by: Frank Chang
Reviewed-by: Alistair Francis
Reviewed-by: Bin Meng
Tested-by: Bin Meng
---
hw/dma/sifive_pdma.c | 177 +--
1 file changed, 155 insertions(+), 22 deletions(-)
diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
index 85fe34f5f3..f4df16449b 100644
--- a/hw/dma/sifive_pdma.c
+++ b/hw/dma/sifive_pdma.c
@@ -177,18 +177,44 @@ static inline void sifive_pdma_update_irq(SiFivePDMAState
*s, int ch)
s->chan[ch].state = DMA_CHAN_STATE_IDLE;
}
-static uint64_t sifive_pdma_read(void *opaque, hwaddr offset, unsigned size)
+static uint64_t sifive_pdma_readq(SiFivePDMAState *s, int ch, hwaddr offset)
{
-SiFivePDMAState *s = opaque;
-int ch = SIFIVE_PDMA_CHAN_NO(offset);
uint64_t val = 0;
-if (ch >= SIFIVE_PDMA_CHANS) {
-qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid channel no %d\n",
- __func__, ch);
-return 0;
+offset &= 0xfff;
+switch (offset) {
+case DMA_NEXT_BYTES:
+val = s->chan[ch].next_bytes;
+break;
+case DMA_NEXT_DST:
+val = s->chan[ch].next_dst;
+break;
+case DMA_NEXT_SRC:
+val = s->chan[ch].next_src;
+break;
+case DMA_EXEC_BYTES:
+val = s->chan[ch].exec_bytes;
+break;
+case DMA_EXEC_DST:
+val = s->chan[ch].exec_dst;
+break;
+case DMA_EXEC_SRC:
+val = s->chan[ch].exec_src;
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Unexpected 64-bit access to 0x%" HWADDR_PRIX "\n",
+ __func__, offset);
+break;
}
+return val;
+}
+
+static uint32_t sifive_pdma_readl(SiFivePDMAState *s, int ch, hwaddr offset)
+{
+uint32_t val = 0;
+
offset &= 0xfff;
switch (offset) {
case DMA_CONTROL:
@@ -198,28 +224,47 @@ static uint64_t sifive_pdma_read(void *opaque, hwaddr
offset, unsigned size)
val = s->chan[ch].next_config;
break;
case DMA_NEXT_BYTES:
-val = s->chan[ch].next_bytes;
+val = extract64(s->chan[ch].next_bytes, 0, 32);
+break;
+case DMA_NEXT_BYTES + 4:
+val = extract64(s->chan[ch].next_bytes, 32, 32);
break;
case DMA_NEXT_DST:
-val = s->chan[ch].next_dst;
+val = extract64(s->chan[ch].next_dst, 0, 32);
+break;
+case DMA_NEXT_DST + 4:
+val = extract64(s->chan[ch].next_dst, 32, 32);
break;
case DMA_NEXT_SRC:
-val = s->chan[ch].next_src;
+v
Re: [PATCH 1/2] hw/dma: sifive_pdma: support high 32-bit access of 64-bit register
Hi Bin,
Thanks for the review.
I will fix the commit log and the behavior of writing high 32-bit of RO
registers in v2 patch.
Thanks,
Jim Shu
On Tue, Jan 4, 2022 at 10:55 AM Bin Meng wrote:
> Hi Jim,
>
> On Tue, Dec 28, 2021 at 8:53 AM Jim Shu wrote:
> >
> > Real PDMA support high 32-bit read/write memory access of 64-bit
>
> %s/support/supports
>
> > register.
> >
> > The following result is PDMA tested in U-Boot on Unmatched board:
> >
> > 1. Real PDMA is allowed high 32-bit read/write to 64-bit register.
>
> %s/is allowed/allows
>
> > => mw.l 0x300 0x0 <= Disclaim channel 0
> > => mw.l 0x300 0x1 <= Claim channel 0
> > => mw.l 0x310 0x8000 <= Write low 32-bit NextDest
> (NextDest = 0x28000)
> > => mw.l 0x314 0x2 <= Write high 32-bit NextDest
> > => md.l 0x310 1<= Dump low 32-bit NextDest
> > 0310: 8000
> > => md.l 0x314 1<= Dump high 32-bit NextDest
> > 0314: 0002
> > => mw.l 0x318 0x80001000 <= Write low 32-bit NextSrc
> (NextSrc = 0x280001000)
> > => mw.l 0x31c 0x2 <= Write high 32-bit NextSrc
> > => md.l 0x318 1<= Dump low 32-bit NextSrc
> > 0310: 80001000
> > => md.l 0x31c 1<= Dump high 32-bit NextSrc
> > 0314: 0002
> >
> > 2. PDMA transfer from 0x280001000 to 0x28000 is OK.
> > => mw.q 0x308 0x4 <= NextBytes = 4
> > => mw.l 0x304 0x2200 <= wsize = rsize = 2 (2^2 = 4
> bytes)
> > => mw.l 0x28000 0x87654321 <= Fill test data to dst
> > => mw.l 0x280001000 0x12345678 <= Fill test data to src
> > => md.l 0x28000 1; md.l 0x280001000 1 <= Dump src/dst memory
> contents
> > 28000: 87654321 !Ce.
> > 280001000: 12345678 xV4.
> > => md.l 0x300 8<= Dump PDMA status
> > 0300: 0001 2200 0004 ..."
> > 0310: 8000 0002 80001000 0002
> > => mw.l 0x300 0x3 <= Set channel 0 run and
> claim bits
> > => md.l 0x300 8<= Dump PDMA status
> > 0300: 4001 2200 0004 ...@..."
> > 0310: 8000 0002 80001000 0002
> > => md.l 0x28000 1; md.l 0x280001000 1 <= Dump src/dst memory
> contents
> > 28000: 12345678 xV4.
> > 280001000: 12345678 xV4.
> >
> > Signed-off-by: Jim Shu
> > Reviewed-by: Frank Chang
> > ---
> > hw/dma/sifive_pdma.c | 174 +--
> > 1 file changed, 152 insertions(+), 22 deletions(-)
> >
> > diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> > index 85fe34f5f3..b8b198ab4e 100644
> > --- a/hw/dma/sifive_pdma.c
> > +++ b/hw/dma/sifive_pdma.c
> > @@ -177,18 +177,44 @@ static inline void
> sifive_pdma_update_irq(SiFivePDMAState *s, int ch)
> > s->chan[ch].state = DMA_CHAN_STATE_IDLE;
> > }
> >
> > -static uint64_t sifive_pdma_read(void *opaque, hwaddr offset, unsigned
> size)
> > +static uint64_t sifive_pdma_readq(SiFivePDMAState *s, int ch, hwaddr
> offset)
> > {
> > -SiFivePDMAState *s = opaque;
> > -int ch = SIFIVE_PDMA_CHAN_NO(offset);
> > uint64_t val = 0;
> >
> > -if (ch >= SIFIVE_PDMA_CHANS) {
> > -qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid channel no %d\n",
> > - __func__, ch);
> > -return 0;
> > +offset &= 0xfff;
> > +switch (offset) {
> > +case DMA_NEXT_BYTES:
> > +val = s->chan[ch].next_bytes;
> > +break;
> > +case DMA_NEXT_DST:
> > +val = s->chan[ch].next_dst;
> > +break;
> > +case DMA_NEXT_SRC:
> > +val = s->chan[ch].next_src;
> > +break;
> > +case DMA_EXEC_BYTES:
> > +val = s->chan[ch].exec_bytes;
> > +break;
> > +case DMA_EXEC_DST:
> > +val = s->chan[ch].exec_dst;
> > +break;
> > +case DMA_EXEC_SRC:
> > +val = s->chan[ch].exec_src;
[PATCH 2/2] hw/dma: sifive_pdma: permit 4/8-byte access size of PDMA registers
It's obvious that PDMA support 64-bit access of 64-bit registers, and
in previous commit, we confirm that PDMA support 32-bit access of both
32/64-bit registers. Thus, we configure 32/64-bit memory access of
PDMA registers as valid in general.
Signed-off-by: Jim Shu
Reviewed-by: Frank Chang
---
hw/dma/sifive_pdma.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
index b8b198ab4e..731fcdcf89 100644
--- a/hw/dma/sifive_pdma.c
+++ b/hw/dma/sifive_pdma.c
@@ -441,6 +441,10 @@ static const MemoryRegionOps sifive_pdma_ops = {
.impl = {
.min_access_size = 4,
.max_access_size = 8,
+},
+.valid = {
+.min_access_size = 4,
+.max_access_size = 8,
}
};
--
2.25.1
[PATCH 1/2] hw/dma: sifive_pdma: support high 32-bit access of 64-bit register
Real PDMA support high 32-bit read/write memory access of 64-bit
register.
The following result is PDMA tested in U-Boot on Unmatched board:
1. Real PDMA is allowed high 32-bit read/write to 64-bit register.
=> mw.l 0x300 0x0 <= Disclaim channel 0
=> mw.l 0x300 0x1 <= Claim channel 0
=> mw.l 0x310 0x8000 <= Write low 32-bit NextDest
(NextDest = 0x28000)
=> mw.l 0x314 0x2 <= Write high 32-bit NextDest
=> md.l 0x310 1<= Dump low 32-bit NextDest
0310: 8000
=> md.l 0x314 1<= Dump high 32-bit NextDest
0314: 0002
=> mw.l 0x318 0x80001000 <= Write low 32-bit NextSrc (NextSrc
= 0x280001000)
=> mw.l 0x31c 0x2 <= Write high 32-bit NextSrc
=> md.l 0x318 1<= Dump low 32-bit NextSrc
0310: 80001000
=> md.l 0x31c 1<= Dump high 32-bit NextSrc
0314: 0002
2. PDMA transfer from 0x280001000 to 0x28000 is OK.
=> mw.q 0x308 0x4 <= NextBytes = 4
=> mw.l 0x304 0x2200 <= wsize = rsize = 2 (2^2 = 4 bytes)
=> mw.l 0x28000 0x87654321 <= Fill test data to dst
=> mw.l 0x280001000 0x12345678 <= Fill test data to src
=> md.l 0x28000 1; md.l 0x280001000 1 <= Dump src/dst memory contents
28000: 87654321 !Ce.
280001000: 12345678 xV4.
=> md.l 0x300 8<= Dump PDMA status
0300: 0001 2200 0004 ..."
0310: 8000 0002 80001000 0002
=> mw.l 0x300 0x3 <= Set channel 0 run and claim bits
=> md.l 0x300 8<= Dump PDMA status
0300: 4001 2200 0004 ...@..."
0310: 8000 0002 80001000 0002
=> md.l 0x28000 1; md.l 0x280001000 1 <= Dump src/dst memory contents
280000000: 12345678 xV4.
280001000: 12345678 xV4.
Signed-off-by: Jim Shu
Reviewed-by: Frank Chang
---
hw/dma/sifive_pdma.c | 174 +--
1 file changed, 152 insertions(+), 22 deletions(-)
diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
index 85fe34f5f3..b8b198ab4e 100644
--- a/hw/dma/sifive_pdma.c
+++ b/hw/dma/sifive_pdma.c
@@ -177,18 +177,44 @@ static inline void sifive_pdma_update_irq(SiFivePDMAState
*s, int ch)
s->chan[ch].state = DMA_CHAN_STATE_IDLE;
}
-static uint64_t sifive_pdma_read(void *opaque, hwaddr offset, unsigned size)
+static uint64_t sifive_pdma_readq(SiFivePDMAState *s, int ch, hwaddr offset)
{
-SiFivePDMAState *s = opaque;
-int ch = SIFIVE_PDMA_CHAN_NO(offset);
uint64_t val = 0;
-if (ch >= SIFIVE_PDMA_CHANS) {
-qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid channel no %d\n",
- __func__, ch);
-return 0;
+offset &= 0xfff;
+switch (offset) {
+case DMA_NEXT_BYTES:
+val = s->chan[ch].next_bytes;
+break;
+case DMA_NEXT_DST:
+val = s->chan[ch].next_dst;
+break;
+case DMA_NEXT_SRC:
+val = s->chan[ch].next_src;
+break;
+case DMA_EXEC_BYTES:
+val = s->chan[ch].exec_bytes;
+break;
+case DMA_EXEC_DST:
+val = s->chan[ch].exec_dst;
+break;
+case DMA_EXEC_SRC:
+val = s->chan[ch].exec_src;
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Unexpected 64-bit access to 0x%" HWADDR_PRIX "\n",
+ __func__, offset);
+break;
}
+return val;
+}
+
+static uint32_t sifive_pdma_readl(SiFivePDMAState *s, int ch, hwaddr offset)
+{
+uint32_t val = 0;
+
offset &= 0xfff;
switch (offset) {
case DMA_CONTROL:
@@ -198,28 +224,47 @@ static uint64_t sifive_pdma_read(void *opaque, hwaddr
offset, unsigned size)
val = s->chan[ch].next_config;
break;
case DMA_NEXT_BYTES:
-val = s->chan[ch].next_bytes;
+val = extract64(s->chan[ch].next_bytes, 0, 32);
+break;
+case DMA_NEXT_BYTES + 4:
+val = extract64(s->chan[ch].next_bytes, 32, 32);
break;
case DMA_NEXT_DST:
-val = s->chan[ch].next_dst;
+val = extract64(s->chan[ch].next_dst, 0, 32);
+break;
+case DMA_NEXT_DST + 4:
+val = extract64(s->chan[ch].next_dst, 32, 32);
break;
case DMA_NEXT_SRC:
-val = s->chan[ch].next_src;
+val = extract64(s->chan[ch].next_src, 0, 32);
+break;
+cas
[PATCH 0/2] Align SiFive PDMA behavior to real hardware
HiFive Unmatched PDMA supports high/low 32-bit access of 64-bit register, but QEMU emulation support low part access now. Enhance QEMU emulation to support high 32-bit access. Also, permit 4/8-byte valid access in PDMA as we have verified 32/64-bit accesses of PDMA registers are supported. Jim Shu (2): hw/dma: sifive_pdma: support high 32-bit access of 64-bit register hw/dma: sifive_pdma: permit 4/8-byte access size of PDMA registers hw/dma/sifive_pdma.c | 178 +-- 1 file changed, 156 insertions(+), 22 deletions(-) -- 2.25.1
[PATCH 1/3] target/riscv: propagate PMP permission to TLB page
Currently, PMP permission checking of TLB page is bypassed if TLB hits
Fix it by propagating PMP permission to TLB page permission.
PMP permission checking also use MMU-style API to change TLB permission
and size.
Signed-off-by: Jim Shu
---
target/riscv/cpu_helper.c | 84 +--
target/riscv/pmp.c| 80 +++--
target/riscv/pmp.h| 4 +-
3 files changed, 125 insertions(+), 43 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 2f43939fb6..f6ac63bf0e 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -280,6 +280,49 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong
newpriv)
env->load_res = -1;
}
+/*
+ * get_physical_address_pmp - check PMP permission for this physical address
+ *
+ * Match the PMP region and check permission for this physical address and it's
+ * TLB page. Returns 0 if the permission checking was successful
+ *
+ * @env: CPURISCVState
+ * @prot: The returned protection attributes
+ * @tlb_size: TLB page size containing addr. It could be modified after PMP
+ *permission checking. NULL if not set TLB page for addr.
+ * @addr: The physical address to be checked permission
+ * @access_type: The type of MMU access
+ * @mode: Indicates current privilege level.
+ */
+static int get_physical_address_pmp(CPURISCVState *env, int *prot,
+target_ulong *tlb_size, hwaddr addr,
+int size, MMUAccessType access_type,
+int mode)
+{
+pmp_priv_t pmp_priv;
+target_ulong tlb_size_pmp = 0;
+
+if (!riscv_feature(env, RISCV_FEATURE_PMP)) {
+*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+return TRANSLATE_SUCCESS;
+}
+
+if (!pmp_hart_has_privs(env, addr, size, 1 << access_type, _priv,
+mode)) {
+*prot = 0;
+return TRANSLATE_PMP_FAIL;
+}
+
+*prot = pmp_priv_to_page_prot(pmp_priv);
+if (tlb_size != NULL) {
+if (pmp_is_range_in_tlb(env, addr & ~(*tlb_size - 1), _size_pmp)) {
+*tlb_size = tlb_size_pmp;
+}
+}
+
+return TRANSLATE_SUCCESS;
+}
+
/* get_physical_address - get the physical address for this virtual address
*
* Do a page table walk to obtain the physical address corresponding to a
@@ -442,9 +485,11 @@ restart:
pte_addr = base + idx * ptesize;
}
-if (riscv_feature(env, RISCV_FEATURE_PMP) &&
-!pmp_hart_has_privs(env, pte_addr, sizeof(target_ulong),
-1 << MMU_DATA_LOAD, PRV_S)) {
+int pmp_prot;
+int pmp_ret = get_physical_address_pmp(env, _prot, NULL, pte_addr,
+ sizeof(target_ulong),
+ MMU_DATA_LOAD, PRV_S);
+if (pmp_ret != TRANSLATE_SUCCESS) {
return TRANSLATE_PMP_FAIL;
}
@@ -682,13 +727,14 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
#ifndef CONFIG_USER_ONLY
vaddr im_address;
hwaddr pa = 0;
-int prot, prot2;
+int prot, prot2, prot_pmp;
bool pmp_violation = false;
bool first_stage_error = true;
bool two_stage_lookup = false;
int ret = TRANSLATE_FAIL;
int mode = mmu_idx;
-target_ulong tlb_size = 0;
+/* default TLB page size */
+target_ulong tlb_size = TARGET_PAGE_SIZE;
env->guest_phys_fault_addr = 0;
@@ -745,10 +791,10 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
prot &= prot2;
-if (riscv_feature(env, RISCV_FEATURE_PMP) &&
-(ret == TRANSLATE_SUCCESS) &&
-!pmp_hart_has_privs(env, pa, size, 1 << access_type, mode)) {
-ret = TRANSLATE_PMP_FAIL;
+if (ret == TRANSLATE_SUCCESS) {
+ret = get_physical_address_pmp(env, _pmp, _size, pa,
+ size, access_type, mode);
+prot &= prot_pmp;
}
if (ret != TRANSLATE_SUCCESS) {
@@ -771,25 +817,21 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
"%s address=%" VADDR_PRIx " ret %d physical "
TARGET_FMT_plx " prot %d\n",
__func__, address, ret, pa, prot);
-}
-if (riscv_feature(env, RISCV_FEATURE_PMP) &&
-(ret == TRANSLATE_SUCCESS) &&
-!pmp_hart_has_privs(env, pa, size, 1 << access_type, mode)) {
-ret = TRANSLATE_PMP_FAIL;
+if (ret == TRANSLATE_SUCCESS) {
+ret = get_physical_address_pmp(env, _pmp, _size, pa,
+ size, access_type, mode);
+prot &= prot_pmp;
+}
[PATCH 1/3] target/riscv: propagate PMP permission to TLB page
Currently, PMP permission checking of TLB page is bypassed if TLB hits
Fix it by propagating PMP permission to TLB page permission.
PMP permission checking also use MMU-style API to change TLB permission
and size.
Signed-off-by: Jim Shu
---
target/riscv/cpu_helper.c | 84 +--
target/riscv/pmp.c| 80 +++--
target/riscv/pmp.h| 4 +-
3 files changed, 125 insertions(+), 43 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 2f43939fb6..f6ac63bf0e 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -280,6 +280,49 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong
newpriv)
env->load_res = -1;
}
+/*
+ * get_physical_address_pmp - check PMP permission for this physical address
+ *
+ * Match the PMP region and check permission for this physical address and it's
+ * TLB page. Returns 0 if the permission checking was successful
+ *
+ * @env: CPURISCVState
+ * @prot: The returned protection attributes
+ * @tlb_size: TLB page size containing addr. It could be modified after PMP
+ *permission checking. NULL if not set TLB page for addr.
+ * @addr: The physical address to be checked permission
+ * @access_type: The type of MMU access
+ * @mode: Indicates current privilege level.
+ */
+static int get_physical_address_pmp(CPURISCVState *env, int *prot,
+target_ulong *tlb_size, hwaddr addr,
+int size, MMUAccessType access_type,
+int mode)
+{
+pmp_priv_t pmp_priv;
+target_ulong tlb_size_pmp = 0;
+
+if (!riscv_feature(env, RISCV_FEATURE_PMP)) {
+*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+return TRANSLATE_SUCCESS;
+}
+
+if (!pmp_hart_has_privs(env, addr, size, 1 << access_type, _priv,
+mode)) {
+*prot = 0;
+return TRANSLATE_PMP_FAIL;
+}
+
+*prot = pmp_priv_to_page_prot(pmp_priv);
+if (tlb_size != NULL) {
+if (pmp_is_range_in_tlb(env, addr & ~(*tlb_size - 1), _size_pmp)) {
+*tlb_size = tlb_size_pmp;
+}
+}
+
+return TRANSLATE_SUCCESS;
+}
+
/* get_physical_address - get the physical address for this virtual address
*
* Do a page table walk to obtain the physical address corresponding to a
@@ -442,9 +485,11 @@ restart:
pte_addr = base + idx * ptesize;
}
-if (riscv_feature(env, RISCV_FEATURE_PMP) &&
-!pmp_hart_has_privs(env, pte_addr, sizeof(target_ulong),
-1 << MMU_DATA_LOAD, PRV_S)) {
+int pmp_prot;
+int pmp_ret = get_physical_address_pmp(env, _prot, NULL, pte_addr,
+ sizeof(target_ulong),
+ MMU_DATA_LOAD, PRV_S);
+if (pmp_ret != TRANSLATE_SUCCESS) {
return TRANSLATE_PMP_FAIL;
}
@@ -682,13 +727,14 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
#ifndef CONFIG_USER_ONLY
vaddr im_address;
hwaddr pa = 0;
-int prot, prot2;
+int prot, prot2, prot_pmp;
bool pmp_violation = false;
bool first_stage_error = true;
bool two_stage_lookup = false;
int ret = TRANSLATE_FAIL;
int mode = mmu_idx;
-target_ulong tlb_size = 0;
+/* default TLB page size */
+target_ulong tlb_size = TARGET_PAGE_SIZE;
env->guest_phys_fault_addr = 0;
@@ -745,10 +791,10 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
prot &= prot2;
-if (riscv_feature(env, RISCV_FEATURE_PMP) &&
-(ret == TRANSLATE_SUCCESS) &&
-!pmp_hart_has_privs(env, pa, size, 1 << access_type, mode)) {
-ret = TRANSLATE_PMP_FAIL;
+if (ret == TRANSLATE_SUCCESS) {
+ret = get_physical_address_pmp(env, _pmp, _size, pa,
+ size, access_type, mode);
+prot &= prot_pmp;
}
if (ret != TRANSLATE_SUCCESS) {
@@ -771,25 +817,21 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
"%s address=%" VADDR_PRIx " ret %d physical "
TARGET_FMT_plx " prot %d\n",
__func__, address, ret, pa, prot);
-}
-if (riscv_feature(env, RISCV_FEATURE_PMP) &&
-(ret == TRANSLATE_SUCCESS) &&
-!pmp_hart_has_privs(env, pa, size, 1 << access_type, mode)) {
-ret = TRANSLATE_PMP_FAIL;
+if (ret == TRANSLATE_SUCCESS) {
+ret = get_physical_address_pmp(env, _pmp, _size, pa,
+ size, access_type, mode);
+prot &= prot_pmp;
+}
[PATCH 0/3] target/riscv: fix PMP permission checking when softmmu's TLB hits
Sorry for sending this patch set again. The cover letter of my previous mail doesn't add cc list. --- Current implementation of PMP permission checking only has effect when softmmu's TLB miss. PMP checking is bypassed when TLB hits because TLB page permission isn't affected by PMP permission. To fix this issue, this patch set addes the feature to propagate PMP permission to the TLB page and flush TLB pages if PMP permission has been changed. The patch set is tested on Zephyr RTOS userspace testsuite on QEMU riscv32 virt machine. Jim Shu (3): target/riscv: propagate PMP permission to TLB page target/riscv: add log of PMP permission checking target/riscv: flush TLB pages if PMP permission has been changed target/riscv/cpu_helper.c | 96 ++- target/riscv/pmp.c| 84 +- target/riscv/pmp.h| 4 +- 3 files changed, 141 insertions(+), 43 deletions(-) -- 2.30.1
[PATCH 2/3] target/riscv: add log of PMP permission checking
Like MMU translation, add qemu log of PMP permission checking for
debugging.
Signed-off-by: Jim Shu
---
target/riscv/cpu_helper.c | 12
1 file changed, 12 insertions(+)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index f6ac63bf0e..c1ecb8a710 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -794,6 +794,12 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
if (ret == TRANSLATE_SUCCESS) {
ret = get_physical_address_pmp(env, _pmp, _size, pa,
size, access_type, mode);
+
+qemu_log_mask(CPU_LOG_MMU,
+ "%s PMP address=" TARGET_FMT_plx " ret %d prot"
+ " %d tlb_size " TARGET_FMT_lu "\n",
+ __func__, pa, ret, prot_pmp, tlb_size);
+
prot &= prot_pmp;
}
@@ -821,6 +827,12 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
if (ret == TRANSLATE_SUCCESS) {
ret = get_physical_address_pmp(env, _pmp, _size, pa,
size, access_type, mode);
+
+qemu_log_mask(CPU_LOG_MMU,
+ "%s PMP address=" TARGET_FMT_plx " ret %d prot"
+ " %d tlb_size " TARGET_FMT_lu "\n",
+ __func__, pa, ret, prot_pmp, tlb_size);
+
prot &= prot_pmp;
}
}
--
2.30.1
[PATCH 2/3] target/riscv: add log of PMP permission checking
Like MMU translation, add qemu log of PMP permission checking for
debugging.
Signed-off-by: Jim Shu
---
target/riscv/cpu_helper.c | 12
1 file changed, 12 insertions(+)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index f6ac63bf0e..c1ecb8a710 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -794,6 +794,12 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
if (ret == TRANSLATE_SUCCESS) {
ret = get_physical_address_pmp(env, _pmp, _size, pa,
size, access_type, mode);
+
+qemu_log_mask(CPU_LOG_MMU,
+ "%s PMP address=" TARGET_FMT_plx " ret %d prot"
+ " %d tlb_size " TARGET_FMT_lu "\n",
+ __func__, pa, ret, prot_pmp, tlb_size);
+
prot &= prot_pmp;
}
@@ -821,6 +827,12 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
if (ret == TRANSLATE_SUCCESS) {
ret = get_physical_address_pmp(env, _pmp, _size, pa,
size, access_type, mode);
+
+qemu_log_mask(CPU_LOG_MMU,
+ "%s PMP address=" TARGET_FMT_plx " ret %d prot"
+ " %d tlb_size " TARGET_FMT_lu "\n",
+ __func__, pa, ret, prot_pmp, tlb_size);
+
prot &= prot_pmp;
}
}
--
2.30.1
[PATCH 3/3] target/riscv: flush TLB pages if PMP permission has been changed
If PMP permission of any address has been changed by updating PMP entry, flush all TLB pages to prevent from getting old permission. Signed-off-by: Jim Shu --- target/riscv/pmp.c | 4 1 file changed, 4 insertions(+) diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c index ebd874cde3..cff020122a 100644 --- a/target/riscv/pmp.c +++ b/target/riscv/pmp.c @@ -28,6 +28,7 @@ #include "qapi/error.h" #include "cpu.h" #include "trace.h" +#include "exec/exec-all.h" static void pmp_write_cfg(CPURISCVState *env, uint32_t addr_index, uint8_t val); @@ -347,6 +348,9 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index, cfg_val = (val >> 8 * i) & 0xff; pmp_write_cfg(env, (reg_index * 4) + i, cfg_val); } + +/* If PMP permission of any addr has been changed, flush TLB pages. */ +tlb_flush(env_cpu(env)); } -- 2.30.1
[PATCH 3/3] target/riscv: flush TLB pages if PMP permission has been changed
If PMP permission of any address has been changed by updating PMP entry, flush all TLB pages to prevent from getting old permission. Signed-off-by: Jim Shu --- target/riscv/pmp.c | 4 1 file changed, 4 insertions(+) diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c index ebd874cde3..cff020122a 100644 --- a/target/riscv/pmp.c +++ b/target/riscv/pmp.c @@ -28,6 +28,7 @@ #include "qapi/error.h" #include "cpu.h" #include "trace.h" +#include "exec/exec-all.h" static void pmp_write_cfg(CPURISCVState *env, uint32_t addr_index, uint8_t val); @@ -347,6 +348,9 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index, cfg_val = (val >> 8 * i) & 0xff; pmp_write_cfg(env, (reg_index * 4) + i, cfg_val); } + +/* If PMP permission of any addr has been changed, flush TLB pages. */ +tlb_flush(env_cpu(env)); } -- 2.30.1
