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

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

[Alejandro Vallejo]

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;
+	}
+
 	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
-- 
2.43.0

[kvm-unit-tests] x86: Add #PF test case for the SVM DecodeAssists feature

[Alejandro Vallejo]

Tests an intercepted #PF accesing the last (unmapped) qword of the
virtual address space. The assist ought provides a prefetched
code stream starting at the offending instruction.

This is little more than a smoke test. There's more cases not covered.
Namely, CR/DR MOVs, INTn, INVLPG, nested PFs, and fault-on-fetch.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
I'm not a big fan of using a literal -8ULL as "unbacked va", but I'm not
sure how to instruct the harness to give me a hole. Likewise, some cases remain
untested, with the interesting one (fault-on-fetch) requiring some cumbersome
setup (put the codestream in the 14 bytes leading to a non-present NPT page.
Happy to add such a case, but I'm not sure how.

As for all other cases, KVM copies ext_info_1 unconditionally, which is where
the decoded info goes. It's highly unlikely they would ever provide much value.
Happy to add them too if they're deemed useful.
---
 lib/x86/processor.h |  1 +
 x86/svm_tests.c     | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 42dd2d2a..32ffd015 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -374,6 +374,7 @@ struct x86_cpu_feature {
 #define X86_FEATURE_LBRV		X86_CPU_FEATURE(0x8000000A, 0, EDX, 1)
 #define X86_FEATURE_NRIPS		X86_CPU_FEATURE(0x8000000A, 0, EDX, 3)
 #define X86_FEATURE_TSCRATEMSR		X86_CPU_FEATURE(0x8000000A, 0, EDX, 4)
+#define X86_FEATURE_DECODE_ASSISTS		X86_CPU_FEATURE(0x8000000A, 0, EDX, 7)
 #define X86_FEATURE_PAUSEFILTER		X86_CPU_FEATURE(0x8000000A, 0, EDX, 10)
 #define X86_FEATURE_PFTHRESHOLD		X86_CPU_FEATURE(0x8000000A, 0, EDX, 12)
 #define X86_FEATURE_VGIF		X86_CPU_FEATURE(0x8000000A, 0, EDX, 16)
diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 37616476..5c93d738 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -409,6 +409,36 @@ static bool check_next_rip(struct svm_test *test)
 	return address == vmcb->control.next_rip;
 }
 
+static bool decode_assists_supported(void)
+{
+	return this_cpu_has(X86_FEATURE_DECODE_ASSISTS);
+}
+
+static void prepare_decode_assists(struct svm_test *test)
+{
+	vmcb->control.intercept_exceptions |= (1ULL << PF_VECTOR);
+}
+
+static void test_decode_assists(struct svm_test *test)
+{
+	asm volatile (".globl opcode_pre\n\t"
+		      "opcode_pre:\n\t"
+		      "mov %0, (%0)\n\t" /* #PF */
+		      ".globl opcode_post\n\t"
+		      "opcode_post:\n\t" :: "r"(-8ULL));
+}
+
+static bool check_decode_assists(struct svm_test *test)
+{
+	extern unsigned char opcode_pre[], opcode_post[];
+	unsigned len = (unsigned)(opcode_post - opcode_pre);
+
+	return vmcb->control.exit_code == (SVM_EXIT_EXCP_BASE + PF_VECTOR)) &&
+		!memcmp(vmcb->control.insn_bytes, opcode_pre, len)          &&
+		vmcb->control.insn_len >= len                               &&
+		vmcb->control.insn_len <= ARRAY_SIZE(vmcb->control.insn_bytes);
+}
+
 extern u8 *msr_bitmap;
 
 static bool is_x2apic;
@@ -3628,6 +3658,9 @@ struct svm_test svm_tests[] = {
 	{ "next_rip", next_rip_supported, prepare_next_rip,
 	  default_prepare_gif_clear, test_next_rip,
 	  default_finished, check_next_rip },
+	{ "decode_assists", decode_assists_supported, prepare_decode_assists,
+	  default_prepare_gif_clear, test_decode_assists,
+	  default_finished, check_decode_assists },
 	{ "msr intercept check", default_supported, prepare_msr_intercept,
 	  default_prepare_gif_clear, test_msr_intercept,
 	  msr_intercept_finished, check_msr_intercept },

base-commit: 31d91f5c9b7546471b729491664b05c933d64a7a
-- 
2.43.0

Re: [kvm-unit-tests] x86: Add #PF test case for the SVM DecodeAssists feature

[Sean Christopherson]

+Kevin and Yosry, who are working on similar tests

On Thu, Jan 15, 2026, Alejandro Vallejo wrote:
> Tests an intercepted #PF accesing the last (unmapped) qword of the
> virtual address space. The assist ought provides a prefetched
> code stream starting at the offending instruction.
> 
> This is little more than a smoke test. There's more cases not covered.
> Namely, CR/DR MOVs, INTn, INVLPG, nested PFs, and fault-on-fetch.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> ---
> I'm not a big fan of using a literal -8ULL as "unbacked va", but I'm not
> sure how to instruct the harness to give me a hole.

Allocate a page, then use install_pte() or install_page_prot() to create a mapping
that will #PF.

> Likewise, some cases remain
> untested, with the interesting one (fault-on-fetch) requiring some cumbersome
> setup (put the codestream in the 14 bytes leading to a non-present NPT page.

Not _that_ cumbersome though.  Allocate a page, install_page_prot() with NX,
copy/write an instruction to the page, jump/call into the page.

run_in_user_ex() makes it even easier to do that at CPL3.

All the above said, I would rather piggyback the access test and not reinvent the
wheel.  E.g. wrap ac_test_run() a la vmx_pf_exception_test(), then intercept #PF
to verify the instruction information is filled as expected.  We'd need to modify
ac_test_do_access() to record what instruction it expected to fault, but that
shouldn't be _too_ hard, and it might even force us to improve the inscrutable
asm blob.

Re: [kvm-unit-tests] x86: Add #PF test case for the SVM DecodeAssists feature

[Alejandro Vallejo]

Hi,

On Thu Jan 15, 2026 at 7:32 PM CET, Sean Christopherson wrote:
> +Kevin and Yosry, who are working on similar tests
>
> On Thu, Jan 15, 2026, Alejandro Vallejo wrote:
>> Tests an intercepted #PF accesing the last (unmapped) qword of the
>> virtual address space. The assist ought provides a prefetched
>> code stream starting at the offending instruction.
>> 
>> This is little more than a smoke test. There's more cases not covered.
>> Namely, CR/DR MOVs, INTn, INVLPG, nested PFs, and fault-on-fetch.
>> 
>> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
>> ---
>> I'm not a big fan of using a literal -8ULL as "unbacked va", but I'm not
>> sure how to instruct the harness to give me a hole.
>
> Allocate a page, then use install_pte() or install_page_prot() to create a mapping
> that will #PF.

Right. It hadn't clicked for me exactly how L1 and L2 are related in the
harness. So this test group runs with the L2 reusing the mappings from the L1?

>
>> Likewise, some cases remain
>> untested, with the interesting one (fault-on-fetch) requiring some cumbersome
>> setup (put the codestream in the 14 bytes leading to a non-present NPT page.
>
> Not _that_ cumbersome though.  Allocate a page, install_page_prot() with NX,
> copy/write an instruction to the page, jump/call into the page.
>
> run_in_user_ex() makes it even easier to do that at CPL3.

I didn't mean fetch as in faulted instruction, but rather the CPU hitting a
missing PT as it fetches the 15 bytes of the codestream including and following
the faulting instruction. That'd be...

  opcode_pre:
  mov ($not_present), %al /* #PF! */
  not_present: /* page boundary */
  int3; /* unmapped in the NPT */

... where the CPU would try to fetch int3, and a bunch more bytes on VMEXIT but
fail, thus leaving insn_len as "not_present - opcode_pre" rather than 15. In the
absence of faults the CPU always fetches as much as possible on #PF/NPT-fault
intercepts.

A complete testcase would show the expected non-15 number as length.

>
> All the above said, I would rather piggyback the access test and not reinvent the
> wheel.  E.g. wrap ac_test_run() a la vmx_pf_exception_test(), then intercept #PF
> to verify the instruction information is filled as expected.  We'd need to modify
> ac_test_do_access() to record what instruction it expected to fault, but that
> shouldn't be _too_ hard, and it might even force us to improve the inscrutable
> asm blob.

I can see how this would be better for the simple test I have in this patch, but
it would probably overcomplicate ac_test_do_access() in the more complete
version with the instruction being at the boundary of a missing page.

I'll try to integrate it anyway on the off chance any other caller happens to
need it. I'm skeptical the result would look pretty, but we'll see; maybe it
turns out alright. I'll get back to you after I find some time to play with it.

On another note, would you prefer individual tests for every single assist
or does it sound ok to have this single test for the #PF case? There's no
meaningful difference between #PF and NPT faults as far as the assists are
concerned, and all others are implicitly covered by virtue of exit_info_1
being unconditionally copied irrespective of exit code.

They are trivial to write, so it's simply a matter of the test volume you're
willing to have.

Cheers for the feedback,
Alejandro

Re: [kvm-unit-tests] x86: Add #PF test case for the SVM DecodeAssists feature

[Alejandro Vallejo]

On Fri Jan 16, 2026 at 1:59 PM CET, Alejandro Vallejo wrote:
> I didn't mean fetch as in faulted instruction, but rather the CPU hitting a
> missing PT as it fetches the 15 bytes of the codestream including and following
> the faulting instruction. That'd be...
>
>   opcode_pre:
>   mov ($not_present), %al /* #PF! */
>   not_present: /* page boundary */
>   int3; /* unmapped in the NPT */

self-correction: NPT or PT.

Cheers,
Alejandro

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

[Jim Mattson]

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).

>         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. 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.

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

[Alejandro Vallejo]

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

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

[Jim Mattson]

On Thu, Jan 29, 2026 at 8:54 AM Alejandro Vallejo
<alejandro.garciavallejo@amd.com> wrote:
>
> 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.

Any of these instructions can be emulated. I think the easiest way to
test is FEP (force emulation prefix), which will result in a #UD
#VMEXIT to L0. Then, during emulation, if L0 finds the intercept bit
set in vmcb12, it will synthesize the appropriate #VMEXIT to L1.

> 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.

MOVBE on Bulldozer? (I don't actually have a Bulldozer to check.) KVM
is happy to emulate MOVBE on hardware that doesn't support it. See
KVM_GET_EMULATED_CPUID.

> 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.

The #UD is handled by the host first. See nested_svm_exit_special().
Hence, FEP should result in emulation by L0. The main reason for this
is that if L0 advertises an instruction to L0 that hardware doesn't
support (e.g. MOVBE), then L0 is responsible for emulating that
instruction for both L1 and L2.

> 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