Re: [PATCH 21/22] x86/traps: Introduce FRED entrypoints

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 08/08/2025 9:23 pm, Andrew Cooper wrote:
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index 270b93ed623e..e67a428e4362 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -2181,6 +2240,94 @@ void asmlinkage check_ist_exit(const struct 
> cpu_user_regs *regs, bool ist_exit)
>  }
>  #endif
>  
> +void asmlinkage entry_from_pv(struct cpu_user_regs *regs)
> +{
> +    /* Copy fred_ss.vector into entry_vector as IDT delivery would have 
> done. */
> +    regs->entry_vector = regs->fred_ss.vector;
> +
> +    switch ( regs->fred_ss.type )
> +    {
> +    case X86_ET_EXT_INTR:
> +        do_IRQ(regs);
> +        break;
> +
> +    case X86_ET_NMI:
> +        do_nmi(regs);
> +        break;
> +
> +    case X86_ET_HW_EXC:
> +    case X86_ET_SW_INT:
> +    case X86_ET_PRIV_SW_EXC:
> +    case X86_ET_SW_EXC:
> +        goto fatal;
> +
> +    default:
> +        goto fatal;
> +    }
> +
> +    return;
> +
> + fatal:
> +    fatal_trap(regs, false);
> +}
> +
> +void asmlinkage entry_from_xen(struct cpu_user_regs *regs)
> +{
> +    /* Copy fred_ss.vector into entry_vector as IDT delivery would have 
> done. */
> +    regs->entry_vector = regs->fred_ss.vector;
> +
> +    switch ( regs->fred_ss.type )
> +    {
> +    case X86_ET_EXT_INTR:
> +        do_IRQ(regs);
> +        break;
> +
> +    case X86_ET_NMI:
> +        do_nmi(regs);
> +        break;
> +
> +    case X86_ET_HW_EXC:
> +    case X86_ET_SW_INT:
> +    case X86_ET_PRIV_SW_EXC:
> +    case X86_ET_SW_EXC:
> +        switch ( regs->fred_ss.vector )
> +        {
> +        case X86_EXC_PF:  do_page_fault(regs); break;
> +        case X86_EXC_GP:  do_general_protection(regs); break;
> +        case X86_EXC_UD:  do_invalid_op(regs); break;
> +        case X86_EXC_NM:  do_device_not_available(regs); break;
> +        case X86_EXC_BP:  do_int3(regs); break;
> +        case X86_EXC_DB:  do_debug(regs); break;
> +        case X86_EXC_DF:  do_double_fault(regs); break;
> +
> +        case X86_EXC_DE:
> +        case X86_EXC_OF:
> +        case X86_EXC_BR:
> +        case X86_EXC_NP:
> +        case X86_EXC_SS:
> +        case X86_EXC_MF:
> +        case X86_EXC_AC:
> +        case X86_EXC_XM:
> +            do_trap(regs);
> +            break;
> +
> +        case X86_EXC_CP:  do_entry_CP(regs); break;
> +
> +        default:
> +            goto fatal;
> +        }
> +        break;
> +
> +    default:
> +        goto fatal;
> +    }
> +
> +    return;
> +
> + fatal:
> +    fatal_trap(regs, false);
> +}

Having started work on the PV support, I think this patch needs to
change somewhat.

I've split #DB and #PF to have separate IDT prologues, which in turns
gives us uniform handling of IRQ re-enabling for synchronous actions. 
But, async still needs special handling.

I think we want something that looks more like:

    switch ( regs->fred_ss.type )
    {
    case X86_ET_EXT_INTR: return do_IRQ(regs);
    case X86_ET_NMI:      return do_nmi(regs);
    case X86_ET_HW_EXC:
        if ( regs->fred_ss.vector == X86_EXC_MC )
            return do_machine_check(regs);
        break;
    }

    if ( regs->eflags & X86_EFLAGS_IF ) // From Xen only
        local_irq_enable();             // From both

    switch ( regs->fred_ss.type )


Either way, it's probably not worth focusing too much on how the C in
this patch looks for now.

~Andrew