[PATCH v2 25/39] xen/riscv: add guest load emulation for trapped MMIO accesses
Oleksii Kurochko <[email protected]>
| Newsgroups | gmane.comp.emulators.xen.devel |
|---|---|
| Message-ID | <7cf3203f8f826eefa1b24bb2dedbd76c0b611dc1.1787838835.git.oleksii.kurochko@gmail.com> |
Implement emulate_load() on top of the decoding interface introduced by the previous patch: fetch the trapped instruction, decode it, dispatch the access to a registered MMIO handler via do_mmio(), write the result back into the destination register and step over the instruction. Xen dispatches MMIO synchronously to an in-hypervisor handler, so unlike KVM RISC-V there is no userspace exit/return step and no equivalent of the kvm_io_bus_read() / KVM_EXIT_MMIO / kvm_riscv_vcpu_mmio_return() split; the result is consumed in place. Sign extension is done here rather than in the handlers: a signed load is normalized by a shift pair, so a handler need only report the value it read. At the moment vINTC is the only backend registered with the MMIO dispatch, so in practice this only covers vINTC traps. An access which no handler claims currently crashes the domain; injecting an access fault into the guest instead is left for later. Signed-off-by: Oleksii Kurochko <[email protected]> --- Changes in v2: - Move the emulation code to the new arch/riscv/emulate.c, leaving traps.c with trap dispatch only. - Split the patch up: struct decoded_insn and the decoding helpers are introduced by "xen/riscv: introduce the interface for trapped instruction decoding" and filled in by "xen/riscv: implement trapped instruction decoding", so only emulate_load() itself is left here. - do_mmio() is no longer defined alongside emulate_load(); it now lives in mmio.c, next to the dispatch it drives. - Don't write the result of a load into x0. SET_RD() wrote rd unconditionally, so a load into x0 clobbered regs->zero and broke the invariant that it reads as zero when x0 is a source operand elsewhere. - Recognize the XLEN=64-only encodings by the guest's effective XLEN (guest_xlen()) rather than by Xen's own (CONFIG_RISCV_32). Besides those encodings simply being reserved on RV32, the compressed ones are ambiguous there: C.LD and C.FLW share the encoding 0x6000 (mask 0xe003), and likewise C.SD/C.FSW, C.LDSP/C.FLWSP and C.SDSP/C.FSWSP. - Take the faulting address from struct guest_fault, filled in by resolve_faulting_gpa(), rather than from get_faulting_gpa(). - Drop the description of a fault taken while re-reading the trapped instruction: that code is now in the patch implementing fetch_trapped_insn(), where a G-stage fault is reported to the guest as CAUSE_FETCH_ACCESS instead of hitting a BUG_ON(). - Update the commit message accordingly. --- --- xen/arch/riscv/emulate.c | 53 +++++++++++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/xen/arch/riscv/emulate.c b/xen/arch/riscv/emulate.c index 81a50643a5ec..e52f2851800c 100644 --- a/xen/arch/riscv/emulate.c +++ b/xen/arch/riscv/emulate.c @@ -5,7 +5,6 @@ */ #include <xen/bug.h> -#include <xen/compiler.h> #include <xen/errno.h> #include <xen/sched.h> #include <xen/types.h> @@ -15,6 +14,7 @@ #include <asm/current.h> #include <asm/emulate.h> #include <asm/guest_access.h> +#include <asm/mmio.h> #include <asm/processor.h> #include <asm/riscv_encoding.h> #include <asm/traps.h> @@ -65,8 +65,7 @@ static bool is_load_guest_page_fault(unsigned long scause) return scause == CAUSE_LOAD_GUEST_PAGE_FAULT; } -static __maybe_unused void advance_pc(struct cpu_user_regs *regs, - unsigned int step) +static void advance_pc(struct cpu_user_regs *regs, unsigned int step) { regs->sepc += step; } @@ -88,7 +87,7 @@ static __maybe_unused void advance_pc(struct cpu_user_regs *regs, * IS_ENABLED() can't be used here as HSTATUS_VSXL is defined for * __riscv_xlen == 64 only, the field not existing on RV32 in the first place. */ -static __maybe_unused unsigned int guest_xlen(const struct cpu_user_regs *regs) +static unsigned int guest_xlen(const struct cpu_user_regs *regs) { #ifdef CONFIG_RISCV_32 return 32; @@ -179,8 +178,7 @@ static void resolve_faulting_gpa(struct guest_fault *gf) * Relies on x0..x31 being laid out at the start of struct cpu_user_regs in * architectural register-number order; see the comment there. */ -static __maybe_unused unsigned long *guest_gpr(struct cpu_user_regs *regs, - unsigned int reg) +static unsigned long *guest_gpr(struct cpu_user_regs *regs, unsigned int reg) { ASSERT(reg < 32); @@ -197,8 +195,8 @@ static __maybe_unused unsigned long *guest_gpr(struct cpu_user_regs *regs, * caller to do. Where it returns false, @di has been filled in and emulation * is to continue. */ -static bool __maybe_unused insn_fetch_faulted(const struct guest_fault *gf, - struct decoded_insn *di) +static bool insn_fetch_faulted(const struct guest_fault *gf, + struct decoded_insn *di) { unsigned long htinst = gf->htinst; @@ -306,8 +304,7 @@ static bool __maybe_unused insn_fetch_faulted(const struct guest_fault *gf, * Returns false if the instruction is not a load or store which can be * emulated here. */ -static __maybe_unused bool decode_ldst_insn(struct decoded_insn *di, - unsigned int xlen) +static bool decode_ldst_insn(struct decoded_insn *di, unsigned int xlen) { unsigned long insn = di->insn; /* Register fields of the uncompressed forms ... */ @@ -419,7 +416,41 @@ static __maybe_unused bool decode_ldst_insn(struct decoded_insn *di, static int emulate_load(const struct guest_fault *gf) { - return -EOPNOTSUPP; + struct cpu_user_regs *regs = gf->regs; + mmio_info_t info = { .is_write = false }; + struct decoded_insn di; + unsigned int shift = 0; + int rc; + + /* A fault taken re-reading the instruction is redirected to the guest. */ + if ( insn_fetch_faulted(gf, &di) ) + return 0; + + if ( !decode_ldst_insn(&di, guest_xlen(regs)) || di.is_write ) + return -EOPNOTSUPP; + + if ( !di.is_unsigned ) + shift = BITS_PER_BYTE * (sizeof(unsigned long) - di.len); + +#ifdef EMULATE_LOAD_DEBUG + gdprintk(XENLOG_DEBUG, "pc=%#lx, addr=%#"PRIpaddr", len=%u, shift=%u\n", + regs->sepc, gf->gpa, di.len, shift); +#endif + + rc = do_mmio(&info, gf->gpa, di.len); + if ( rc ) + return rc; + + /* + * A load into x0 discards its result: writing regs->zero would break the + * invariant that it reads as zero when x0 is a source operand elsewhere. + */ + if ( di.reg ) + *guest_gpr(regs, di.reg) = (long)(info.data << shift) >> shift; + + advance_pc(regs, di.insn_len); + + return 0; } static int emulate_store(struct guest_fault *gf) -- 2.55.0