[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v3 21/39] xen/riscv: add guest memory read helper



Introduce read_guest() to allow Xen to safely read guest memory using
HLV/HLVX instructions while reliably capturing trap context.

This is required for instruction fetch emulation and MMIO decoding, where
Xen must inspect guest memory that may not be directly accessible and may
fault.

The implementation is based on kvm_riscv_vcpu_unpriv_read() from Linux,
with one deviation: the hlv/hlvx instructions translate the guest address
through the live vsatp/hgatp CSRs, i.e. through the address space of the
currently running vCPU, so the function can only be called safely for
current. Instead of taking a struct vcpu argument, it always operates on
current directly.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
---
Changes in v3:
 - Rename riscv_read_guest() to read_guest().
 - Document that @trap must be zero-initialised by the caller: it is
   written only when an access faults, so trap->scause == 0 on return is
   what tells the caller that the read succeeded.
 - Drop the ASSERT() on hstatus.SPV: vcpu_guest_cpu_user_regs(current)
   is the guest frame whatever context Xen is in, so SPV is always set in
   it and the check couldn't catch a call made from a nested trap. State
   in the comment above read_guest() that it must not be called from a
   nested trap taken in HS-mode instead.
 - Declare read_guest() in asm/guest_access.h.
---
Changes in v2:
 - Rename riscv_vcpu_unpriv_read() to riscv_read_guest() and make the guest
   address the first parameter. "unprivileged" described how hlv/hlvx perform
   the access rather than what the helper is for, and "unprivileged guest"
   reads as a synonym for DomU although the helper works for any domain.
 - Drop the hstatus save/restore and the local_irq_save() protecting it:
   hstatus already belongs to the vCPU which trapped, as Xen never installs
   a value of its own and does not reschedule before returning to the guest.
   ASSERT() hstatus.SPV in the saved copy instead, which also documents that
   the helper is only usable while handling a trap from a guest.
 - Poison val with ~0UL and make [val] "+&r": the fixup for the first access
   resumes past the loads without writing it, so a caller which checks
   trap->scause is no longer handed an uninitialized value.
 - Describe the trap_info write with a "+m" (*trap) operand instead of a
   "memory" clobber; the exception handler writes that structure and nothing
   else.
 - Combine the two halfwords with slli/or instead of sll/add, and use bnez
   for the instruction length check.
 - Turn the hlv.d/hlv.w selection into #if/#elif with an #error default,
   rather than silently using hlv.w for anything that is not RV64.
 - Document that at most two halfwords are fetched, i.e. that encodings
   wider than 32 bits are unsupported and cannot be completed by calling the
   helper again at guest_addr + 4, since the length check would then be
   applied to a continuation halfword.
---
---
 xen/arch/riscv/guestcopy.c                | 88 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/guest_access.h |  4 ++
 2 files changed, 92 insertions(+)

diff --git a/xen/arch/riscv/guestcopy.c b/xen/arch/riscv/guestcopy.c
index 8a89212e0bea..4aa5a0d967ba 100644
--- a/xen/arch/riscv/guestcopy.c
+++ b/xen/arch/riscv/guestcopy.c
@@ -6,6 +6,7 @@
 #include <xen/string.h>
 
 #include <asm/guest_access.h>
+#include <asm/traps.h>
 
 #define COPY_from_guest     0U
 #define COPY_to_guest       BIT(0, U)
@@ -114,3 +115,90 @@ unsigned long copy_to_guest_phys(struct domain *d, paddr_t 
gpa, void *buf,
     return copy_guest(buf, gpa, len, GPA_INFO(d),
                       COPY_to_guest | COPY_gpa);
 }
+
+/*
+ * Read machine word from guest memory
+ *
+ * @guest_addr: Guest address to read
+ * @read_insn: Flag representing whether we are reading instruction
+ * @trap: Output pointer to trap details if something went wrong during read.
+ *        It must be zero-initialised by the caller: it is written only when
+ *        an access faults, so trap->scause == 0 on return is what tells the
+ *        caller that the read succeeded.
+ *
+ * The hlv/hlvx instructions translate guest_addr through the live
+ * vsatp/hgatp CSRs, so the read is only meaningful for the address
+ * space of the currently running vCPU. It must therefore only be called
+ * while handling a trap taken from that vCPU, never from a nested trap
+ * taken in HS-mode, which may have interrupted Xen at a point where
+ * hstatus, vsatp or hgatp don't belong to the vCPU.
+ *
+ * At most two halfwords are fetched when @read_insn is true, i.e. encodings
+ * wider than 32 bits are not supported. Such an encoding cannot be completed
+ * by calling this function again at @guest_addr + 4: the length check is
+ * applied to the first halfword read, which would then be a continuation of
+ * the instruction rather than its opcode. It is up to the caller to reject
+ * anything that is neither a 16- nor a 32-bit encoding.
+ */
+unsigned long read_guest(unsigned long guest_addr, bool read_insn,
+                         struct trap_info *trap)
+{
+    /*
+     * Poison the result: if the very first access faults, the fixup skips
+     * over the loads without writing it. Callers must check trap->scause.
+     */
+    unsigned long val = ~0UL, tmp;
+
+    /*
+     * hlv/hlvx use hstatus.SPVP for the privilege of the access, and the
+     * live vsatp/hgatp for the translation. Xen never installs a value of
+     * its own in hstatus (it is only saved on trap entry and restored
+     * before sret) and it doesn't reschedule before returning to the
+     * guest, so all three still belong to the vCPU which trapped.
+     */
+    if ( read_insn )
+    {
+        asm volatile ( "\n"
+            "1: hlvx.hu %[val], (%[addr])\n"
+            ASM_EXTABLE_TRAP_INFO(1b, 3f, %[ti])
+            "   andi %[tmp], %[val], 3\n"
+            "   addi %[tmp], %[tmp], -3\n"
+            "   bnez %[tmp], 3f\n"
+            "   addi %[addr], %[addr], 2\n"
+            "\n"
+            "2: hlvx.hu %[tmp], (%[addr])\n"
+            ASM_EXTABLE_TRAP_INFO(2b, 3f, %[ti])
+            "   slli %[tmp], %[tmp], 16\n"
+            "   or %[val], %[val], %[tmp]\n"
+            "3:\n"
+        : [val] "+&r" (val), [tmp] "=&r" (tmp), [addr] "+&r" (guest_addr),
+          "+m" (*trap)
+        : [ti] "r" (trap) );
+
+        /*
+         * Although HLVX instructions' explicit memory accesses require execute
+         * permissions, they still raise the same exceptions as other load
+         * instructions, rather than raising fetch exceptions instead.
+         */
+        if ( trap->scause == CAUSE_LOAD_PAGE_FAULT )
+            trap->scause = CAUSE_FETCH_PAGE_FAULT;
+    }
+    else
+    {
+        asm volatile ( "\n"
+            "1: "
+#if defined(CONFIG_RISCV_64)
+            "hlv.d %[val], (%[addr])\n"
+#elif defined(CONFIG_RISCV_32)
+            "hlv.w %[val], (%[addr])\n"
+#else
+# error "unsupported RISC-V variant: no hlv for a machine word"
+#endif
+            "2:\n"
+            ASM_EXTABLE_TRAP_INFO(1b, 2b, %[ti])
+        : [val] "+&r" (val), "+m" (*trap)
+        : [addr] "r" (guest_addr), [ti] "r" (trap) );
+    }
+
+    return val;
+}
diff --git a/xen/arch/riscv/include/asm/guest_access.h 
b/xen/arch/riscv/include/asm/guest_access.h
index 8d679319ded0..41c6ede4baae 100644
--- a/xen/arch/riscv/include/asm/guest_access.h
+++ b/xen/arch/riscv/include/asm/guest_access.h
@@ -5,6 +5,7 @@
 #include <xen/types.h>
 
 struct domain;
+struct trap_info;
 
 unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len);
 unsigned long raw_copy_from_guest(void *to, const void *from, unsigned len);
@@ -25,6 +26,9 @@ unsigned long raw_clear_guest(void *to, unsigned int len);
 unsigned long copy_to_guest_phys(struct domain *d, paddr_t gpa, void *buf,
                                  unsigned long len);
 
+unsigned long read_guest(unsigned long guest_addr, bool read_insn,
+                         struct trap_info *trap);
+
 #endif /* ASM__RISCV__GUEST_ACCESS_H */
 /*
  * Local variables:
-- 
2.55.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.