* [PATCH 0/1 v2] nSVM: Test host RFLAGS.TF on VMRUN
@ 2021-02-04 23:29 Krish Sadhukhan
2021-02-04 23:29 ` [PATCH 1/1 v2] nSVM: Test effect of " Krish Sadhukhan
0 siblings, 1 reply; 5+ messages in thread
From: Krish Sadhukhan @ 2021-02-04 23:29 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, jmattson, seanjc
v1 -> v2:
i) Patch# 1 from v1 has been removed since Paolo has already applied it.
ii) Patch# 2 from v1 has been removed since we don't need the helpers.
The test now uses the 'prepare_gif_clear' callback in order to set
host RFLAGS.TF. The test also triggers a #UD on the VMRUN
instruction in order to find out its RIP which is then used by the
#DB handler as the begginning of the sub-test that tests Single-
Stepping right on VMRUN.
[PATCH 1/1 v2] nSVM: Test effect of host RFLAGS.TF on VMRUN
x86/svm_tests.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 129 insertions(+)
Krish Sadhukhan (1):
nSVM: Test effect of host RFLAGS.TF on VMRUN
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1 v2] nSVM: Test effect of host RFLAGS.TF on VMRUN
2021-02-04 23:29 [PATCH 0/1 v2] nSVM: Test host RFLAGS.TF on VMRUN Krish Sadhukhan
@ 2021-02-04 23:29 ` Krish Sadhukhan
2021-02-05 8:14 ` Paolo Bonzini
0 siblings, 1 reply; 5+ messages in thread
From: Krish Sadhukhan @ 2021-02-04 23:29 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, jmattson, seanjc
According to section "VMRUN and TF/RF Bits in EFLAGS" in AMD APM vol 2,
"From the host point of view, VMRUN acts like a single instruction,
even though an arbitrary number of guest instructions may execute
before a #VMEXIT effectively completes the VMRUN. As a single
host instruction, VMRUN interacts with EFLAGS.TF like ordinary
instructions. EFLAGS.TF causes a #DB trap after the VMRUN completes
on the host side (i.e., after the #VMEXIT from the guest).
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
x86/svm_tests.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 129 insertions(+)
diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 29a0b59..def5880 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -2002,6 +2002,132 @@ static bool init_intercept_check(struct svm_test *test)
return init_intercept;
}
+/*
+ * Setting host EFLAGS.TF causes a #DB trap after the VMRUN completes on the
+ * host side (i.e., after the #VMEXIT from the guest).
+ *
+ * [AMD APM]
+ */
+static volatile u8 host_rflags_guest_main_flag = 0;
+static volatile u8 host_rflags_db_handler_flag = 0;
+static volatile bool host_rflags_ss_on_vmrun = false;
+static u64 vmrun_rip;
+static volatile bool host_rflags_vmrun_reached = false;
+static volatile bool host_rflags_set_tf = false;
+
+static void host_rflags_ud_handler(struct ex_regs *r)
+{
+ vmrun_rip = r->rip;
+ wrmsr(MSR_EFER, rdmsr(MSR_EFER) | EFER_SVME);
+}
+
+static void host_rflags_db_handler(struct ex_regs *r)
+{
+ if (host_rflags_ss_on_vmrun) {
+ if (host_rflags_vmrun_reached) {
+ host_rflags_db_handler_flag =
+ (host_rflags_guest_main_flag == 1) ? 2 : 1;
+ r->rflags &= ~X86_EFLAGS_TF;
+ } else {
+ if (r->rip == vmrun_rip) {
+ host_rflags_guest_main_flag =
+ host_rflags_db_handler_flag = 0;
+ host_rflags_vmrun_reached = true;
+ }
+ }
+ } else {
+ host_rflags_db_handler_flag =
+ (host_rflags_guest_main_flag == 1) ? 2 : 1;
+ r->rflags &= ~X86_EFLAGS_TF;
+ }
+}
+
+static void host_rflags_prepare(struct svm_test *test)
+{
+ default_prepare(test);
+ handle_exception(DB_VECTOR, host_rflags_db_handler);
+ set_test_stage(test, 0);
+ /*
+ * We trigger a #UD in order to find out the RIP of VMRUN instruction
+ */
+ wrmsr(MSR_EFER, rdmsr(MSR_EFER) & ~EFER_SVME);
+ handle_exception(UD_VECTOR, host_rflags_ud_handler);
+}
+
+static void host_rflags_prepare_gif_clear(struct svm_test *test)
+{
+ if (host_rflags_set_tf) {
+ write_rflags(read_rflags() | X86_EFLAGS_TF);
+ }
+}
+
+static void host_rflags_test(struct svm_test *test)
+{
+ while (1) {
+ if (get_test_stage(test) > 0)
+ host_rflags_guest_main_flag =
+ (host_rflags_db_handler_flag == 1) ? 2 : 1;
+ vmmcall();
+ if (get_test_stage(test) == 3)
+ break;
+ }
+}
+
+static bool host_rflags_finished(struct svm_test *test)
+{
+ switch (get_test_stage(test)) {
+ case 0:
+ if (vmcb->control.exit_code != SVM_EXIT_VMMCALL &&
+ host_rflags_db_handler_flag != 0 && host_rflags_guest_main_flag != 0) {
+ report(false, "Unexpected VMEXIT. Exit reason 0x%x",
+ vmcb->control.exit_code);
+ return true;
+ }
+ vmcb->save.rip += 3;
+ /*
+ * Setting host EFLAGS.TF not immediately before VMRUN, causes
+ * #DB trap before first guest instruction is executed
+ */
+ host_rflags_set_tf = true;
+ break;
+ case 1:
+ if (vmcb->control.exit_code != SVM_EXIT_VMMCALL &&
+ host_rflags_db_handler_flag != 1 && host_rflags_guest_main_flag != 2) {
+ report(false, "Unexpected VMEXIT. Exit reason 0x%x",
+ vmcb->control.exit_code);
+ return true;
+ }
+ host_rflags_guest_main_flag = host_rflags_db_handler_flag = 0;
+ vmcb->save.rip += 3;
+ /*
+ * Setting host EFLAGS.TF immediately before VMRUN, causes #DB
+ * trap after VMRUN completes on the host side (i.e., after
+ * VMEXIT from guest).
+ */
+ host_rflags_ss_on_vmrun = true;
+ break;
+ case 2:
+ if (vmcb->control.exit_code != SVM_EXIT_VMMCALL &&
+ host_rflags_db_handler_flag != 2 && host_rflags_guest_main_flag != 1) {
+ report(false, "Unexpected VMEXIT. Exit reason 0x%x",
+ vmcb->control.exit_code);
+ return true;
+ }
+ host_rflags_set_tf = false;
+ vmcb->save.rip += 3;
+ break;
+ default:
+ return true;
+ }
+ inc_test_stage(test);
+ return get_test_stage(test) == 4;
+}
+
+static bool host_rflags_check(struct svm_test *test)
+{
+ return get_test_stage(test) == 3;
+}
+
#define TEST(name) { #name, .v2 = name }
/*
@@ -2492,6 +2618,9 @@ struct svm_test svm_tests[] = {
{ "svm_init_intercept_test", smp_supported, init_intercept_prepare,
default_prepare_gif_clear, init_intercept_test,
init_intercept_finished, init_intercept_check, .on_vcpu = 2 },
+ { "host_rflags", default_supported, host_rflags_prepare,
+ host_rflags_prepare_gif_clear, host_rflags_test,
+ host_rflags_finished, host_rflags_check },
TEST(svm_cr4_osxsave_test),
TEST(svm_guest_state_test),
TEST(svm_vmrun_errata_test),
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1 v2] nSVM: Test effect of host RFLAGS.TF on VMRUN
2021-02-04 23:29 ` [PATCH 1/1 v2] nSVM: Test effect of " Krish Sadhukhan
@ 2021-02-05 8:14 ` Paolo Bonzini
2021-02-23 20:28 ` Krish Sadhukhan
0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2021-02-05 8:14 UTC (permalink / raw)
To: Krish Sadhukhan, kvm; +Cc: jmattson, seanjc
On 05/02/21 00:29, Krish Sadhukhan wrote:
>
> +static void host_rflags_prepare(struct svm_test *test)
> +{
> + default_prepare(test);
> + handle_exception(DB_VECTOR, host_rflags_db_handler);
> + set_test_stage(test, 0);
> + /*
> + * We trigger a #UD in order to find out the RIP of VMRUN instruction
> + */
> + wrmsr(MSR_EFER, rdmsr(MSR_EFER) & ~EFER_SVME);
> + handle_exception(UD_VECTOR, host_rflags_ud_handler);
> +}
> +
I think you'd get the RIP of VMLOAD, not VMRUN.
Maybe something like:
diff --git a/x86/svm.c b/x86/svm.c
index a1808c7..88d8452 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -208,14 +208,14 @@ struct regs get_regs(void)
struct svm_test *v2_test;
-#define ASM_VMRUN_CMD \
+#define ASM_PRE_VMRUN \
"vmload %%rax\n\t" \
"mov regs+0x80, %%r15\n\t" \
"mov %%r15, 0x170(%%rax)\n\t" \
"mov regs, %%r15\n\t" \
"mov %%r15, 0x1f8(%%rax)\n\t" \
- LOAD_GPR_C \
- "vmrun %%rax\n\t" \
+ LOAD_GPR_C
+#define ASM_POST_VMRUN \
SAVE_GPR_C \
"mov 0x170(%%rax), %%r15\n\t" \
"mov %%r15, regs+0x80\n\t" \
@@ -232,7 +232,9 @@ int svm_vmrun(void)
regs.rdi = (ulong)v2_test;
asm volatile (
- ASM_VMRUN_CMD
+ ASM_PRE_VMRUN
+ "vmrun %%rax\n\t"
+ ASM_POST_VMRUN
:
: "a" (virt_to_phys(vmcb))
: "memory", "r15");
@@ -240,6 +242,7 @@ int svm_vmrun(void)
return (vmcb->control.exit_code);
}
+extern unsigned char vmrun_rip;
static void test_run(struct svm_test *test)
{
u64 vmcb_phys = virt_to_phys(vmcb);
@@ -258,7 +261,9 @@ static void test_run(struct svm_test *test)
"sti \n\t"
"call *%c[PREPARE_GIF_CLEAR](%[test]) \n \t"
"mov %[vmcb_phys], %%rax \n\t"
- ASM_VMRUN_CMD
+ ASM_PRE_VMRUN
+ "vmrun_rip: vmrun %%rax\n\t"
+ ASM_POST_VMRUN
"cli \n\t"
"stgi"
: // inputs clobbered by the guest:
(untested)
Paolo
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1 v2] nSVM: Test effect of host RFLAGS.TF on VMRUN
2021-02-05 8:14 ` Paolo Bonzini
@ 2021-02-23 20:28 ` Krish Sadhukhan
2021-02-24 7:46 ` Paolo Bonzini
0 siblings, 1 reply; 5+ messages in thread
From: Krish Sadhukhan @ 2021-02-23 20:28 UTC (permalink / raw)
To: Paolo Bonzini, kvm; +Cc: jmattson, seanjc
On 2/5/21 12:14 AM, Paolo Bonzini wrote:
> On 05/02/21 00:29, Krish Sadhukhan wrote:
>>
>> +static void host_rflags_prepare(struct svm_test *test)
>> +{
>> + default_prepare(test);
>> + handle_exception(DB_VECTOR, host_rflags_db_handler);
>> + set_test_stage(test, 0);
>> + /*
>> + * We trigger a #UD in order to find out the RIP of VMRUN
>> instruction
>> + */
>> + wrmsr(MSR_EFER, rdmsr(MSR_EFER) & ~EFER_SVME);
>> + handle_exception(UD_VECTOR, host_rflags_ud_handler);
>> +}
>> +
>
> I think you'd get the RIP of VMLOAD, not VMRUN.
>
> Maybe something like:
>
> diff --git a/x86/svm.c b/x86/svm.c
> index a1808c7..88d8452 100644
> --- a/x86/svm.c
> +++ b/x86/svm.c
> @@ -208,14 +208,14 @@ struct regs get_regs(void)
>
> struct svm_test *v2_test;
>
> -#define ASM_VMRUN_CMD \
> +#define ASM_PRE_VMRUN \
> "vmload %%rax\n\t" \
> "mov regs+0x80, %%r15\n\t" \
> "mov %%r15, 0x170(%%rax)\n\t" \
> "mov regs, %%r15\n\t" \
> "mov %%r15, 0x1f8(%%rax)\n\t" \
> - LOAD_GPR_C \
> - "vmrun %%rax\n\t" \
> + LOAD_GPR_C
> +#define ASM_POST_VMRUN \
> SAVE_GPR_C \
> "mov 0x170(%%rax), %%r15\n\t" \
> "mov %%r15, regs+0x80\n\t" \
> @@ -232,7 +232,9 @@ int svm_vmrun(void)
> regs.rdi = (ulong)v2_test;
>
> asm volatile (
> - ASM_VMRUN_CMD
> + ASM_PRE_VMRUN
> + "vmrun %%rax\n\t"
> + ASM_POST_VMRUN
> :
> : "a" (virt_to_phys(vmcb))
> : "memory", "r15");
> @@ -240,6 +242,7 @@ int svm_vmrun(void)
> return (vmcb->control.exit_code);
> }
>
> +extern unsigned char vmrun_rip;
> static void test_run(struct svm_test *test)
> {
> u64 vmcb_phys = virt_to_phys(vmcb);
> @@ -258,7 +261,9 @@ static void test_run(struct svm_test *test)
> "sti \n\t"
> "call *%c[PREPARE_GIF_CLEAR](%[test]) \n \t"
> "mov %[vmcb_phys], %%rax \n\t"
> - ASM_VMRUN_CMD
> + ASM_PRE_VMRUN
> + "vmrun_rip: vmrun %%rax\n\t"
> + ASM_POST_VMRUN
> "cli \n\t"
> "stgi"
> : // inputs clobbered by the guest:
>
> (untested)
>
> Paolo
>
Thanks for the suggestion. I have implemented it in v3 that I have sent out.
The reason why my test (v2) had passed, was because virtual
VMLOAD/VMSAVE is enabled by default and no #VMEXIT happens due to
intercepts being disabled (via init_vmcb()). So my #UD handler didn't
get invoked on VMLOAD but got invoked on VMRUN.
This brings out an interesting point. When intercept for VMLOAD/VMSAVE
is disabled, there is no effect of unsetting EFER.SVME on VMLOAD/VMLOAD,
even thought APM vol 2 says it should generate a #UD. I couldn't find
any text in the APM that describes the effect of unsetting EFER.SVME on
the virtual VMLOAD/VMSAVE. Is this the expected behavior of the SVM
hardware or is this a bug in KVM and KVM should handle this ?
I plan to add some more tests based on the correct behavior of virtual
VMLOAD/VMSAVE.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1 v2] nSVM: Test effect of host RFLAGS.TF on VMRUN
2021-02-23 20:28 ` Krish Sadhukhan
@ 2021-02-24 7:46 ` Paolo Bonzini
0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2021-02-24 7:46 UTC (permalink / raw)
To: Krish Sadhukhan, kvm; +Cc: jmattson, seanjc
On 23/02/21 21:28, Krish Sadhukhan wrote:
> I couldn't find any text in the APM that describes the effect of
> unsetting EFER.SVME on the virtual VMLOAD/VMSAVE. Is this the expected
> behavior of the SVM hardware or is this a bug in KVM and KVM should
> handle this ?
The guest always runs with EFER.SVME=1 (it's part of the VMRUN
requirements), so yeah, vVMLOAD/VMSAVE must be enabled only if the guest
has EFER.SVME=1. Nice!
Paolo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-02-24 7:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-04 23:29 [PATCH 0/1 v2] nSVM: Test host RFLAGS.TF on VMRUN Krish Sadhukhan
2021-02-04 23:29 ` [PATCH 1/1 v2] nSVM: Test effect of " Krish Sadhukhan
2021-02-05 8:14 ` Paolo Bonzini
2021-02-23 20:28 ` Krish Sadhukhan
2021-02-24 7:46 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.