Re: [PATCH] KVM: nSVM: Expose SVM DecodeAssists to guest hypervisors

[Alejandro Vallejo <alejandro.garciavallejo@amd.com>]

Hi,

I've been busy with other matters and didn't have time to push this through,
but I very definitely intend to.

On Thu Jan 29, 2026 at 12:15 AM CET, Jim Mattson wrote:
> On Thu, Jan 15, 2026 at 5:26 AM Alejandro Vallejo
> <alejandro.garciavallejo@amd.com> wrote:
>>
>> Enable exposing DecodeAssists to guests. Performs a copyout of
>> the insn_len and insn_bytes fields of the VMCB when the vCPU has
>> the feature enabled.
>>
>> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
>> ---
>> I wrote a little smoke test for kvm-unit-tests too. I'll send it shortly in
>> reply to this email.
>> ---
>>  arch/x86/kvm/cpuid.c      | 1 +
>>  arch/x86/kvm/svm/nested.c | 6 ++++++
>>  arch/x86/kvm/svm/svm.c    | 3 +++
>>  3 files changed, 10 insertions(+)
>>
>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>> index 88a5426674a10..da9a63c8289e5 100644
>> --- a/arch/x86/kvm/cpuid.c
>> +++ b/arch/x86/kvm/cpuid.c
>> @@ -1181,6 +1181,7 @@ void kvm_set_cpu_caps(void)
>>                 VENDOR_F(FLUSHBYASID),
>>                 VENDOR_F(NRIPS),
>>                 VENDOR_F(TSCRATEMSR),
>> +               VENDOR_F(DECODEASSISTS),
>>                 VENDOR_F(V_VMSAVE_VMLOAD),
>>                 VENDOR_F(LBRV),
>>                 VENDOR_F(PAUSEFILTER),
>> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
>> index ba0f11c68372b..dc8a8e67a22c2 100644
>> --- a/arch/x86/kvm/svm/nested.c
>> +++ b/arch/x86/kvm/svm/nested.c
>> @@ -1128,6 +1128,12 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
>>                 vmcb12->save.ssp        = vmcb02->save.ssp;
>>         }
>>
>> +       if (guest_cpu_cap_has(vcpu, X86_FEATURE_DECODEASSISTS)) {
>> +               memcpy(vmcb12->control.insn_bytes, vmcb02->control.insn_bytes,
>> +                      ARRAY_SIZE(vmcb12->control.insn_bytes));
>> +               vmcb12->control.insn_len = vmcb02->control.insn_len;
>> +       }
>
> This only works if the #VMEXIT is being forwarded from vmcb02. This
> does not work if the #VMEXIT is synthesized by L0 (e.g. via
> nested_svm_inject_npf_exit() or nested_svm_inject_exception_vmexit()
> for #PF).

I very definitely didn't consider that. Subtle. Thanks for bringing it up.

>
>>         vmcb12->control.int_state         = vmcb02->control.int_state;
>>         vmcb12->control.exit_code         = vmcb02->control.exit_code;
>>         vmcb12->control.exit_code_hi      = vmcb02->control.exit_code_hi;
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index 24d59ccfa40d9..8cf6d7904030e 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -5223,6 +5223,9 @@ static __init void svm_set_cpu_caps(void)
>>                 if (nrips)
>>                         kvm_cpu_cap_set(X86_FEATURE_NRIPS);
>>
>> +               if (boot_cpu_has(X86_FEATURE_DECODEASSISTS))
>> +                       kvm_cpu_cap_set(X86_FEATURE_DECODEASSISTS);
>> +
>>                 if (npt_enabled)
>>                         kvm_cpu_cap_set(X86_FEATURE_NPT);
>>
>>
>> base-commit: 0499add8efd72456514c6218c062911ccc922a99
>
> DECODEASSISTS consists of more than instruction bytes and instruction
> length. There is also EXITINFO1 for MOV CRx, MOV DRx, INTn, and
> INVLPG.

Right, I didn't do anything about those because exit_info_1 is unconditionally
copied anyway when forwarding from vmcb02 to vmcb12. Of course that doesn't
account for the emulation paths you mentioned before.

> Since L2 typically gets dibs on a #VMEXIT (in
> nested_svm_intercept()), these typically fall into the "forwarded
> #VMEXIT" category. However, these instructions can also be emulated,
> in which case the vmcb12 intercepts are checked and a #VMEXIT may be
> synthesized. In that case, svm_check_intercept() needs to populate
> EXITINFO1 appropriately.

I'm trying to think of ways to test this.

It's really quite subtle. So testing MOVs to/from CR/DR would be...

  1. Take a page L0 always does trap-and-emulate on (IOAPIC? LAPIC? HPET?)
  2. Map it on L2.
  3. Have L1 intercept MOV to/from CR/DR
  4. Have L2 do a MOV to/from CR/DR from/to that region.
  5. Ensure the VMCB at the L1 has exit_info_1 correctly populated.

INVLPG and INTn presumably would always be in the intercept union, so these are
unaffected.

As for #PF and NPT intercepts... I don't _THINK_ they are affected either? I
don't see which L1/L2 configs might make the L0 emulator execute an instruction
on behalf of the L2 and THEN exit to L1 with an exit code of #PF/NPT.

Even considering FEP, that ought to affect L1, and not L2. And even if L2 had
FEP enabled that would materialise as a forwarded #UD and not something emulated
by L0.

So, unless I'm missing something (which I may very well be, seeing how I missed
this already), synthesising exit_info_1 for the MOV to/from CR/DR ought to
suffice. Which is far less annoying than teaching the emulator to fetch more
data than the current instruction, so I really hope I'm not wrong.

Let me know otherwise, and thanks again for the review.

Cheers,
Alejandro