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

[PATCH v3 22/39] xen/riscv: look up the exception table for any trap taken in Xen context



do_trap() consulted the exception table only for CAUSE_ILLEGAL_INSTRUCTION,
which covers csr_read_safe() but not the hlv/hlvx sequences reading guest
memory: those fault with load/store (guest) page fault causes and would
reach do_unexpected_trap() instead of their fixup.

Move the lookup ahead of the cause switch, and gate it on the trap having
been taken in Xen context and not being an interrupt:

- sepc of a trap taken from the guest is a guest VA/PA, which the
  guest can point at an address listed in the exception table; Xen would
  then act on that entry and, for EX_TYPE_TRAP_INFO, write through a
  pointer fully under guest control. Entries are matched by exact address,
  so this needs no more than a numerical collision.

- an interrupt taken at an address listed in the table would otherwise be
  "fixed up" as if the access itself had faulted, silently skipping it and
  handing the caller the interrupt's scause as a fault cause.

Returning early skips check_for_pcpu_work(), which is correct: that only
runs for traps taken from the guest.

With that in place a G-stage fault reaching the switch can no longer have
been caused by an hlv/hlvx covered by an entry, so anything left must have
come from the guest. Say so in the comment ahead of the BUG_ON(!from_guest)
in the guest page fault case.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
---
Changes in v3:
 - Update the comment ahead of BUG_ON(!from_guest) in the guest page fault
   case to refer to the fixup_exception() call added here.
 - Drop "assert as much" and the paragraph about caching the "trap came from
   the guest" test from the commit message: the BUG_ON() and the local are
   introduced by "xen/riscv: add guest page fault handling stub".
 - Refer to read_guest() instead of riscv_read_guest() in the comment in
   do_trap(), following the rename of the helper.
---
Changes in v2:
 - New patch.
---
---
 xen/arch/riscv/traps.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index 08aae2e5280b..dc938bae5b00 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -196,11 +196,34 @@ void do_trap(struct cpu_user_regs *cpu_regs)
     unsigned long cause = csr_read(CSR_SCAUSE);
     bool from_guest = cpu_regs->hstatus & HSTATUS_SPV;
 
+    /*
+     * A synchronous trap taken in Xen context may come from an access done on
+     * a vCPU's behalf, e.g. the hlv/hlvx sequences in read_guest(),
+     * or from a probing access like csr_read_safe(). Both are covered by
+     * exception table entries which record the fault details for the caller
+     * and resume execution past the faulting instruction.
+     *
+     * Traps taken from the guest must never be fixed up: sepc is then a guest
+     * address, which the guest could point at an address listed in the
+     * exception table, making Xen act on an entry (and, for EX_TYPE_TRAP_INFO,
+     * write through a pointer) fully under guest control.
+     *
+     * Interrupts must be excluded too: one taken at an address which happens
+     * to be listed in the exception table would otherwise be "fixed up" as if
+     * the access itself had faulted, silently skipping it.
+     *
+     * Returning early skips check_for_pcpu_work() below, which is correct:
+     * that only runs for traps taken from the guest.
+     */
+    if ( !from_guest && !(cause & CAUSE_IRQ_FLAG) &&
+         fixup_exception(cpu_regs, cause) )
+        return;
+
     switch ( cause )
     {
     case CAUSE_VIRTUAL_SUPERVISOR_ECALL:
         /* CAUSE_VIRTUAL_SUPERVISOR_ECALL should come from VS-mode */
-        BUG_ON(!(cpu_regs->hstatus & HSTATUS_SPV));
+        BUG_ON(!from_guest);
 
         vsbi_handle_ecall(cpu_regs);
         break;
@@ -209,8 +232,9 @@ void do_trap(struct cpu_user_regs *cpu_regs)
     case CAUSE_LOAD_GUEST_PAGE_FAULT:
     case CAUSE_STORE_GUEST_PAGE_FAULT:
         /*
-         * Xen doesn't access guest memory, so it can't take a guest page
-         * fault itself: only a guest can get here.
+         * A guest page fault taken in Xen context comes from an hlv/hlvx
+         * access made on a vCPU's behalf and is dealt with by the
+         * fixup_exception() above, so only a guest can get here.
          */
         BUG_ON(!from_guest);
 
@@ -231,9 +255,6 @@ void do_trap(struct cpu_user_regs *cpu_regs)
             break;
         }
 
-        if ( fixup_exception(cpu_regs, cause) )
-            break;
-
         fallthrough;
     default:
         if ( cause & CAUSE_IRQ_FLAG )
-- 
2.55.0




 


Rackspace

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