Re: [PATCH v3 2/9] KVM: nSVM: Synthesize DecodeAssists EXITINFO for emulated intercepts
Tina Zhang <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.kvm |
|---|---|
| Message-ID | <[email protected]> |
On 7/30/2026 8:30 PM, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [High] Synthesized EXITINFO1 is incorrectly populated for SVM_EXIT_CR0_SEL_WRITE intercepts triggered by MOV CR0, violating the AMD APM. > -- > > --- Patch [2]: [PATCH v3 2/9] KVM: nSVM: Synthesize DecodeAssists EXITINFO for emulated intercepts --- > commit 60593c4facd0e8bdc8b0aa501c98449feeced81a > Author: Tina Zhang <[email protected]> > > KVM: nSVM: Synthesize DecodeAssists EXITINFO for emulated intercepts > > This commit populates the architectural EXITINFO fields when DecodeAssists > is exposed to L1 and KVM synthesizes a nested VM-Exit for emulated > instructions. It targets specific intercepts like MOV CR/DR, INTn, and > INVLPG. > >> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c >> index d68cba12c772b..b63234a2feb8d 100644 >> --- a/arch/x86/kvm/svm/svm.c >> +++ b/arch/x86/kvm/svm/svm.c >> @@ -4822,6 +4822,52 @@ static const struct __x86_intercept { > [ ... ] >> +static void svm_prepare_decode_assist_exit_info(struct kvm_vcpu *vcpu, >> + const struct x86_instruction_info *info) >> +{ >> + struct vmcb *vmcb = to_svm(vcpu)->vmcb; >> + u64 exit_info_1; >> + >> + if (!guest_cpu_cap_has(vcpu, X86_FEATURE_DECODEASSISTS)) >> + return; >> + >> + switch (info->intercept) { >> + case x86_intercept_cr_read: >> + case x86_intercept_cr_write: >> + /* MOV CRx: bit 63 set, GPR number in bits 3:0. */ >> + exit_info_1 = BIT_ULL(63) | (info->modrm_rm & 0xf); > > [Severity: High] > Will this incorrectly populate exit_info_1 for selective CR0 writes? > > In svm_check_intercept(), the exit code can be dynamically upgraded to > SVM_EXIT_CR0_SEL_WRITE for MOV CR0 instructions: > > svm_check_intercept() { > ... > cr0 = vcpu->arch.cr0 & ~SVM_CR0_SELECTIVE_MASK; > val = info->src_val & ~SVM_CR0_SELECTIVE_MASK; > if (cr0 ^ val) > icpt_info.exit_code = SVM_EXIT_CR0_SEL_WRITE; > ... > } > > Since svm_prepare_decode_assist_exit_info() is subsequently called and > only checks the original instruction intercept (x86_intercept_cr_write), > could this wrongly populate exit_info_1 with decode assist information > instead of leaving it zero as required by the AMD architecture manual > for SVM_EXIT_CR0_SEL_WRITE? SVM_EXIT_CR0_SEL_WRITE does not imply that EXITINFO1 must be zero. The exit code identifies the selective CR0 intercept, whereas the DecodeAssist contents describe the instruction that triggered it. For MOV-to-CR0, the APM specifies that EXITINFO1 provides the GPR number and identifies the instruction as MOV CRx. For LMSW and CLTS, no additional information is provided and bit 63 is clear. Checking info->intercept is therefore intentional, as it distinguishes MOV-to-CR0 from LMSW when both can result in SVM_EXIT_CR0_SEL_WRITE. Thanks, Tina > >> + break; >> + case x86_intercept_clts: > [ ... ] >