Re: [PATCH v6] x86/nSVM: Check injected event consistency

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.09.2026 17:50, Abdelkareem Abdelsaamad wrote:
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2619,6 +2619,7 @@ void asmlinkage svm_vmexit_handler(void)
>  
>      if ( unlikely(exit_reason == VMEXIT_INVALID) )
>      {
> +        svm_vmcb_isvalid(__func__, vmcb, v, true);
>          gdprintk(XENLOG_ERR, "invalid VMCB state:\n");
>          svm_vmcb_dump(__func__, vmcb);
>          domain_crash(v->domain);

This is the kind of use that we're seeing for a function doing full
checking. The two pre-existing uses, otoh, are getting too expensive with
more checking added. Therefore what we (the x86 maintainers) think is
there needs to be a split: Only checks that need doing ahead of VMRUN
should be done there. All other checks should be done only for diagnostic
purposes after a failed VMRUN.

> --- a/xen/arch/x86/hvm/svm/vmcb.c
> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> @@ -327,6 +327,70 @@ void svm_vmcb_dump(const char *from, const struct 
> vmcb_struct *vmcb)
>      svm_dump_sel("  TR", &vmcb->tr);
>  }
>  
> +static bool is_valid_injected_exception_vector(const struct vmcb_struct 
> *vmcb,
> +                                               uint8_t vmcb_injected_vector,
> +                                               const struct vcpu *v)
> +{
> +    switch ( vmcb_injected_vector )
> +    {
> +    case X86_EXC_DE:
> +    case X86_EXC_DB:
> +    case X86_EXC_UD:
> +    case X86_EXC_NM:
> +    case X86_EXC_DF:
> +    case X86_EXC_TS:
> +    case X86_EXC_NP:
> +    case X86_EXC_SS:
> +    case X86_EXC_GP:
> +    case X86_EXC_PF:
> +    case X86_EXC_MF:
> +    case X86_EXC_AC:
> +    case X86_EXC_MC:
> +    case X86_EXC_XM:
> +    case X86_EXC_SX:
> +        return true;
> +
> +    /*
> +     * Exception vectors 3 and 4 may not be injected into SEV-ES guests.
> +     * If this is attempted, the VMRUN will fail with a VMEXIT_INVALID
> +     * error code.
> +     * See the APM Volume #2 15.35 Encrypted State (SEV-ES).
> +     */
> +    case X86_EXC_BP:
> +        return !vmcb_get_sev_es(vmcb);

For this it would, btw, be helpful if the testing table also had an
entry for #BP.

> +    /*
> +     * If the VMM attempts to inject an event that is impossible for the
> +     * guest mode (e.g., a #BR exception when the guest is in 64-bit mode),
> +     * the event injection will fail... VMRUN will immediately exit with
> +     * VMEXIT_INVALID.
> +     * See the APM Volume #2 15.20 Event Injection.
> +     */
> +    case X86_EXC_OF:
> +        if ( vmcb_get_sev_es(vmcb) )
> +            return false;
> +        fallthrough;
> +    case X86_EXC_BR:
> +        return !(vmcb_get_efer(vmcb) & EFER_LMA) || !vmcb->cs.l;
> +
> +    case X86_EXC_CP:
> +    {
> +        unsigned long guest_valid_cr4 = hvm_cr4_guest_valid_bits(v->domain);
> +
> +        return guest_valid_cr4 & X86_CR4_CET;
> +    }

As said before - this wants to check the live CR4 value of the guest, not
what it may in principle enable there. While it looks like Andrew doesn't
fully agree with me here (because that's not how hardware behaves), I can
only point out that the way he wants it (deriving from the raw policy), it
would result in behavior changing for a guest migrating from a Naples host
to a Genoa one.

Jan