Re: Re: [PATCH v2] nSVM: Check injected event consistency

[Abdelkareem Abdelsaamad <abdelkareem.abdelsaamad@xxxxxxxxxx>]

On 28.07.2026 14:04, Teddy Astie wrote:
>On 16.07.2026 17:41, Abdelkareem Abdelsaamad wrote:
>> --- a/xen/arch/x86/hvm/svm/vmcb.c
>> +++ b/xen/arch/x86/hvm/svm/vmcb.c
>> @@ -320,6 +320,31 @@ void svm_vmcb_dump(const char *from, const struct 
>> vmcb_struct *vmcb)
>>       svm_dump_sel("  TR", &vmcb->tr);
>>   }
>>   
>> +static bool is_valid_svm_vmcb_injected_exception_vector(
>> +    const struct vmcb_struct *vmcb, uint8_t vmcb_injected_vector)
>> +{
>> +    return ( (vmcb_injected_vector == X86_EXC_DE) ||
>> +             (vmcb_injected_vector == X86_EXC_DB) ||
>> +             (vmcb_injected_vector == X86_EXC_BP) ||
>> +             (vmcb_injected_vector == X86_EXC_OF) ||
>> +             (vmcb_injected_vector == X86_EXC_BR) ||
>
>This particular exception is special. AMD APM states that this event is 
>"impossible" if the guest is in 64-bit mode and will cause 
>VMEXIT_INVALID in such case.
>
> > 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 and no guest state instructions will be 
>executed; VMRUN will immediately exit with an error code of VMEXIT_INVALID.
>
>So this one likely want a additional check for hvm_guest_x86_mode() != 
>X86_MODE_64BIT.
>
>It looks like #OF has the same quirk (invalid in 64-bits mode).
>
I agree your point is valid. I will address in V3.
>Though I don't know if any other exception has a similar behavior though.
I have double-checked the APM vOL3(24594—Rev. 3.37—jULY 2025) regarding the
other exception vectors and instructions. Vector 4 (#OF) and vector 5 (#BR) are
unique because their triggering instructions BOUND and INTO are invalid and
disabled in 64-bit mode, making them structurally invalid. Other vectors remain
legal across the other modes. In V3, I will make use of hvm_guest_x86_mode to
correctly isolate these two cases.
>
>> +             (vmcb_injected_vector == X86_EXC_UD) ||
>> +             (vmcb_injected_vector == X86_EXC_NM) ||
>> +             (vmcb_injected_vector == X86_EXC_DF) ||
>> +             (vmcb_injected_vector == X86_EXC_TS) ||
>> +             (vmcb_injected_vector == X86_EXC_NP) ||
>> +             (vmcb_injected_vector == X86_EXC_SS) ||
>> +             (vmcb_injected_vector == X86_EXC_GP) ||
>> +             (vmcb_injected_vector == X86_EXC_PF) ||
>> +             (vmcb_injected_vector == X86_EXC_MF) ||
>> +             (vmcb_injected_vector == X86_EXC_AC) ||
>> +             (vmcb_injected_vector == X86_EXC_MC) ||
>> +             (vmcb_injected_vector == X86_EXC_XM) ||
>> +             (vmcb_injected_vector == X86_EXC_HV) ||
>> +             (vmcb_injected_vector == X86_EXC_SX) ||
>> +             (vmcb_get_sev_es(vmcb) && vmcb_injected_vector == X86_EXC_VC) 
>> );
>> +}
>
>I think using a switch here would help making things more readable, 
>especially if we need to add additional comparisons in specific cases 
>(SEV-ES for #VC, !64-bits for #BR and #OF, ...).
>
>I have in mind something like
>
>   switch (vmcb_injected_vector)
>   {
>   case X86_EXC_OF:
>   case X86_EXC_BR:
>       return hvm_guest_x86_mode(v) != X86_MODE_64BIT;
>
>   (all other special cases, ...)
>
>   case X86_EXC_UD:
>   (all other simple cases ...)
>       return true;
>
>   default:
>       return false;
>   }
Yes, I agree writing it with switch-case makes sense with this structure. I
will change to switch structure in V3.
>
>Teddy
--Abdelkareem