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

[Abdelkareem Abdelsaamad <abdelkareem.abdelsaamad@xxxxxxxxxx>]

On 24.08.2026 12:12, Teddy Astie wrote:
>On 06.08.2026 19:26, Abdelkareem Abdelsaamad wrote:
>> On the AMD platforms, allowing a VMRUN instruction with a malformed VMCB has
>> debugging complications, security and performance implications. The APM
>> volume #2 15.20 (40332-Rev. 4.10-July 2026) states the two possibilities that
>> result in a VMRUN exit with VMEXIT_INVALID due to the injected event. These 
>> are
>> • Reserved values of TYPE have been specified.
>> • TYPE = 3 (exception) has been specified with a vector that does not
>>    correspond to an exception (this includes vector 2, which is an NMI, not
>>    an exception).
>> 
>> Extend the VMCB validation to check for such inconsistency.
>> 
>> The collection of the invalid exception vectors are ported from the upstream
>> KVM commit
>> ("7e79f71bca5c" KVM: nSVM: Add missing consistency check for EVENTINJ). 
>> Adjust
>> the checks from the commit to align with the APM Volume #2 and Volume #3
>> (40332—Rev. 4.40—July 2026) for the X86_EXC_OF and X86_EXC_BR vectors which
>> should not be valid on the x86 64-bit (long mode) platforms. The adjustment 
>> is
>> posted to the KVM mailing commit patch thread
>> https://lore.kernel.org/all/20260803225402.2324595-1-abdelkareem.abdelsaamad@xxxxxxxxxx/
>> 
>> Signed-off-by: Abdelkareem Abdelsaamad <abdelkareem.abdelsaamad@xxxxxxxxxx>
>> ---
>> Changes in v3:
>> - Restricted X86_EXC_OF (4) and X86_EXC_BR (5) vector injections to
>>    non-64-bit guests to prevent impossible guest-mode state injections
>>    per AMD APM Volumes 2 & 3.
>> - Refactored exception vector validation from if-conditions to a switch
>>    statement to improve readability and extensibility.
>> - Restricted X86_EXC_CP (21) vector injection to hosts with enabled CET
>>    to prevent VMRUN failures on hardware without CET support.
>> 
>> Changes in v2:
>> - Remove the redundant SVM_EVENT_INJ_TYPE_MASK and SVM_EVENT_INJ_VEC_MASK
>>    constants.
>> - Correct the Injected Event Type consistency check to disallow the injection
>>    of reserved type 1 events.
>> ---
>
>(...)
>
>> https://gitlab.com/xen-project/people/aabdelsa/xen/-/pipelines/2734283788
>> ---
>>   xen/arch/x86/hvm/svm/vmcb.c | 51 +++++++++++++++++++++++++++++++++++++
>>   1 file changed, 51 insertions(+)
>> 
>
>I would add this newly introduced function in svm_vmexit_handler(), when 
>encountering VMEXIT_INVALID to attempt giving more information of the 
>problem (nobody likes to debug VMEXIT_INVALID).
By this, do you mean calling the svm_vmcb_isvalid from svm_vmexit_handler or
calling only is_valid_svm_vmcb_injected_exception_vector? I see that
VMEXIT_INVALID already dumps the VMCB.
>
>> diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
>> index 975a1eaef8..4379bbef09 100644
>> --- a/xen/arch/x86/hvm/svm/vmcb.c
>> +++ b/xen/arch/x86/hvm/svm/vmcb.c
>> @@ -320,6 +320,41 @@ 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)
>> +{
>> +    switch ( vmcb_injected_vector )
>> +    {
>> +    case X86_EXC_DE:
>> +    case X86_EXC_DB:
>> +    case X86_EXC_BP:
>> +    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_HV:
>
>As you plan to drop #HV (due to being SEV-SNP specific), could it be at 
>least commented out; which would hint the need for a appropriate check 
>when implementing SEV-SNP restricted injections. 
I will address this in v5, since v4 has already been posted.
>
>> +    case X86_EXC_SX:
>> +        return true;
>> +    case X86_EXC_OF:
>> +    case X86_EXC_BR:
>> +        return !(vmcb_get_efer(vmcb) & EFER_LMA) || !(vmcb->cs.l);
>> +    case X86_EXC_VC:
>> +        return vmcb_get_sev_es(vmcb);
>> +    case X86_EXC_CP:
>> +        return !!(vmcb_get_cr4(vmcb) & X86_CR4_CET);
>> +    default:
>> +        return false;
>> +    }
>> +}
>> +
>
>
>Teddy