Re: [PATCH v2 19/39] xen/riscv: implement trap redirection to a guest

[Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>]

> Some traps taken by Xen on behalf of a guest can't or shouldn't be handled
> by the hypervisor and have to be reflected to the guest's own S-mode trap
> handler instead: the access faults which handle_guest_page_fault() injects
> for a fault that can never become an emulated access, and, later on, a
> fault taken by the hlv/hlvx sequences of riscv_read_guest() while
> accessing guest memory on a vCPU's behalf.
>
Access faults from handle_guest_page_fault() aren't "taken by Xen on
behalf of a guest" as those are guest-page faults taken directly from the
guest's own execution. Xen just decides they can't be emulated and
reflects them back as access faults. Only the hlv/hlvx case is Xen
trapping on the guest's behalf (Xen itself executes the faulting access).
Conflating the two under one description makes the paragraph confusing.

Suggest splitting into two:

    Two kinds of traps can't or shouldn't be handled by the hypervisor and
    have to be reflected to the guest's own S-mode trap handler instead:

    - Traps Xen takes on the guest's behalf: the hlv/hlvx sequences
    riscv_read_guest() uses to access guest memory.
    - Access faults handle_guest_page_fault() injects for a guest-page
    fault that can never become an emulated access.

> 
> Implement trap_redirect(), until now a BUG_ON() placeholder, for that
> purpose. It makes the trap appear to the guest as if it had been taken
> directly in VS-mode: the trap information is transferred to the guest's
> virtual supervisor CSRs and the vCPU is resumed at its exception vector in
> supervisor mode, following the trap entry rules of the RISC-V privileged
> specification.
> 
> Add the STVEC_* definitions needed to tell the BASE and MODE fields of
> vstvec apart.
> 
> The implementation is based on kvm_riscv_vcpu_trap_redirect() from Linux,
> with a few deviations:
>  - The function reads and writes physical VS-mode CSRs, so it is only
>    meaningful for the currently running vCPU. Instead of taking a
>    struct vcpu argument, it always operates on current.
>  - The MODE field of vstvec is masked off explicitly when computing the
>    exception target PC (exceptions always vector to BASE), rather than
>    relying on the hardwired zero bit of sepc to drop it on VM entry.
>  - Assertions document the preconditions: the trap must have been taken
>    from virtualized mode (hstatus.SPV set), and only synchronous
>    exceptions may be redirected - interrupts must be injected via hvip
>    instead, so that the hardware performs VS-mode trap entry itself,
>    respecting vsstatus.SIE and vectored vstvec dispatch.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
>
> diff --git a/xen/arch/riscv/include/asm/riscv_encoding.h 
> b/xen/arch/riscv/include/asm/riscv_encoding.h
> index 2d2e7e11b3..b2071f4758 100644
> --- a/xen/arch/riscv/include/asm/riscv_encoding.h
> +++ b/xen/arch/riscv/include/asm/riscv_encoding.h
> @@ -109,6 +109,12 @@
>  #define SIP_SSIP                     MIP_SSIP
>  #define SIP_STIP                     MIP_STIP
>  
> +/* stvec/vstvec: MODE is bits [1:0], BASE is bits [XLEN-1:2] */
> +#define STVEC_MODE_MASK                      _UL(0x3)
> +#define STVEC_MODE_DIRECT            _UL(0x0)
> +#define STVEC_MODE_VECTORED          _UL(0x1)
> +#define STVEC_BASE_MASK                      (~STVEC_MODE_MASK)
Nit: STVEC_MODE_DIRECT and STVEC_MODE_VECTORED aren't used anywhere in
this patch (only STVEC_BASE_MASK is). Either use them where you decide
exceptions always target BASE regardless of MODE, or drop them until a
patch that needs them.

-- 
Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>