Re: [PATCH v3 20/22] x86/pv: ERETU error handling

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.10.2025 00:53, Andrew Cooper wrote:
> ERETU can fault for guest reasons, and like IRET needs special handling to
> forward the error into the guest.
> 
> As this is largely written in C, take the opportunity to better classify the
> sources of error, and in particilar, not forward errors that are actually
> Xen's fault into the guest, opting for a domain crash instead.
> 
> Because ERETU does not enable NMIs if it faults, a corner case exists if an
> NMI was taken while in guest context, and the ERETU back out faults.  Recovery
> must involve an ERETS with the interrupted context's NMI flag.
> 
> See the comments for full details.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
albeit ...

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -2345,6 +2345,113 @@ void asmlinkage entry_from_pv(struct cpu_user_regs 
> *regs)
>      fatal_trap(regs, false);
>  }
>  
> +void nocall eretu_error_dom_crash(void);
> +
> +/*
> + * Classify an event at the ERETU instruction, and handle if possible.
> + * Returns @true if handled, @false if the event should continue down the
> + * normal handlers.
> + */
> +static bool handle_eretu_event(struct cpu_user_regs *regs)
> +{
> +    unsigned long recover;
> +
> +    /*
> +     * WARNING: The GPRs in gregs overlaps with regs.  Only gregs->error_code
> +     *          and later are legitimate to access.
> +     */
> +    struct cpu_user_regs *gregs =
> +        _p(regs->rsp - offsetof(struct cpu_user_regs, error_code));
> +
> +    /*
> +     * The asynchronous or fatal events (INTR, NMI, #MC, #DF) have been dealt
> +     * with, meaning we only have syncrhonous ones to consider.  Anything
> +     * which isn't a hardware exception wants handling normally.
> +     */
> +    if ( regs->fred_ss.type != X86_ET_HW_EXC )
> +        return false;
> +
> +    /*
> +     * Guests are permitted to write non-present GDT/LDT entries.  Therefore
> +     * #NP[sel] (%cs) and #SS[sel] (%ss) must be handled as guest errors.  
> The
> +     * only other source of #SS is for a bad %ss-relative memory access in
> +     * Xen, and if the stack is that bad, we'll have escalated to #DF.
> +     *
> +     * #PF can happen from ERETU accessing the GDT/LDT.  Xen may translate
> +     * these into #GP for the guest, so must be handled as guest errors.  In
> +     * theory we can get #PF for a bad instruction fetch or bad stack access,
> +     * but either of these will be fatal and not end up here.
> +     */
> +    switch ( regs->fred_ss.vector )
> +    {
> +    case X86_EXC_GP:
> +        /*
> +         * #GP[0] can occur because of a NULL %cs or %ss (which are a guest
> +         * error), but some #GP[0]'s are errors in Xen (ERETU at SL != 0), or
> +         * errors of Xen handling guest state (bad metadata).  These magic
> +         * numbers came from the FRED Spec; they check that ERETU is trying 
> to
> +         * return to Ring 3, and that reserved or inapplicable bits are 0.
> +         */
> +        if ( regs->error_code == 0 && (gregs->cs & ~3) && (gregs->ss & ~3) &&
> +             (regs->fred_cs.sl != 0 ||
> +              (gregs->csx    & 0xffffffffffff0003UL) != 3 ||
> +              (gregs->rflags & 0xffffffffffc2b02aUL) != 2 ||
> +              (gregs->ssx    &         0xfff80003UL) != 3) )

... I continue to dislike these uses of literal numbers.

Jan