On 13.08.2026 10:12, Jan Beulich wrote:
>On 06.08.2026 19:23, 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
>
>Isn't this 4.10, just like you have it further up?
Yes, that's correct. I'll fix this to 4.10 in v4.
>> 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/[email protected]/
>>
>> Signed-off-by: Abdelkareem Abdelsaamad <[email protected]>
>> ---
>> 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.
>
>When reading this, I was puzzled, but the code below is correct: It's not
>the host you check, but the guest's CR4.
Right, this should refer to the guest's CR4. I will update this in v4.
>> @@ -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(
>
>Is in particular "svm" but perhaps also "vmcb" really relevant in the name
>here (which is a static helper)?
I will change to a shorter function name is_valid_injected_exception_vector in
v4.
>
>> + 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:
>
>Doesn't #XM (AMD: #XF) require CR4.OSXMMEXCPT to be set?
This logic was ported and adopted from the KVM. The intent is to avoid
injecting invalid events on the platforms where the vector is entirely
reserved, rather than blocking valid events where a capability is not
opted-in.The X86_EXC_XM vector is valid (not reserved) on SVM-supported
platforms. I performed some testing with Naples and Genoa platforms, injecting
this vector resulted in a guest triple fault rather than a VMEXIT_INVALID. This
shows that event delivery succeeded and did not trigger VMEXIT_INVALID
>
>> + case X86_EXC_HV:
>> + case X86_EXC_SX:
>
>Are #HV and #SX really permitted without any constraints?
The same reasoning for X86_EXC_SX applies here.
For X86_EXC_HV, this vector is associated with SEV-SNP guests on AMD platforms.
I performed testing on both Genoa and Naples guests, injecting this event
always triggered a VMEXIT_INVALID. Based on these results, I will drop this
specific vector from the permitted events in v4.
>
>> + return true;
>> + case X86_EXC_OF:
>> + case X86_EXC_BR:
>> + return !(vmcb_get_efer(vmcb) & EFER_LMA) || !(vmcb->cs.l);
>
>Nit: No need for parentheses on the rhs of the ||.
I will drop in v4.
>
>> + case X86_EXC_VC:
>> + return vmcb_get_sev_es(vmcb);
>> + case X86_EXC_CP:
>> + return !!(vmcb_get_cr4(vmcb) & X86_CR4_CET);
>
>No need for !! when converting to bool.
I will drop in v4.
>
>> + default:
>> + return false;
>> + }
>
>Throughout: Blank lines please between non-fall-through case blocks.
I will change in v4.
>
>> @@ -392,6 +433,16 @@ bool svm_vmcb_isvalid(
>> PRINTF("eventinj: MBZ bits are set (%#"PRIx64")\n",
>> vmcb->event_inj.raw);
>>
>> + if ( !((1 << vmcb_injected_type) & vmcb_valid_event_inj_types_mask) )
>> + PRINTF("eventinj: Invalid Injected Event Type: (%#"PRIx8")\n",
>> + vmcb_injected_type);
>> +
>> + if ( (vmcb_injected_type == X86_ET_HW_EXC) &&
>> + !is_valid_svm_vmcb_injected_exception_vector(
>> + vmcb, vmcb_injected_vector) )
>
>Nit: Indentation is off by one here. The anchor on the earlier line isn't the
>'!' but the 'i'.
I will change in v4.
>
>> + PRINTF("eventinj: Invalid Injected Event. Exception type:
>> (%#"PRIx8"),"
>> + " with a vector: (%#"PRIx8") does not belong to an exception
>> on"
>> + " the platform \n", vmcb_injected_type,
>> vmcb_injected_vector);
>
>This message is quite a bit too long. There's also a stray blank ahead of the
>\n. And further arguments after one that was already wrapped across lines want
>to start on a separate line.
I will properly wrap the subsequent arguments in v4. I will change the message
to be more concise to something like
"eventinj: Invalid exception type: (%#"PRIx8") vector: (%#"PRIx8") for the "
"platform\n"
>
>Jan
