Re: [PATCH 2/3] nVMX: Add helper functions to set/unset host RFLAGS.TF on the VMRUN instruction

[Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>]

On 2/3/21 12:15 AM, Paolo Bonzini wrote:
> On 03/02/21 02:28, Krish Sadhukhan wrote:
> > Add helper functions to set host RFLAGS.TF immediately before the VMRUN
> > instruction. These will be used  by the next patch to test Single 
> > Stepping
> > on the VMRUN instruction from the host's perspective.
> >
> > Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
>
> I think you can use prepare_gif_clear to set RFLAGS.TF and the 
> exception handler can:
>
> 1) look for VMRUN at the interrupted EIP.  If it is there store the 
> VMRUN address and set a flag.
>
> 2) on the next #DB (flag set), store the EIP and clear the flag
>
> The finished callback then checks that the EIP was stored and that the 
> two EIPs are 3 bytes apart (the length of a VMRUN).


Thanks for the suggestion. It worked fine and I have sent out v2.

However, I couldn't use the two RIPs (VMRUN and post-VMRUN) to check the 
result because the post-VMRUN RIP was more than the length of the VMRUN 
instruction i.e., when #DB handler got executed following guest exit, 
the RIP had moved forward a few instructions from VMRUN. So, I have used 
the same mechanism I used in v1, to check the results.

> Paolo
>
> > ---
> >   x86/svm.c       | 24 ++++++++++++++++++++++--
> >   x86/svm.h       |  3 +++
> >   x86/svm_tests.c |  1 -
> >   3 files changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/x86/svm.c b/x86/svm.c
> > index a1808c7..547f62a 100644
> > --- a/x86/svm.c
> > +++ b/x86/svm.c
> > @@ -179,6 +179,17 @@ void vmcb_ident(struct vmcb *vmcb)
> >       }
> >   }
> >   +static bool ss_bp_on_vmrun = false;
> > +void set_ss_bp_on_vmrun(void)
> > +{
> > +    ss_bp_on_vmrun = true;
> > +}
> > +
> > +void unset_ss_bp_on_vmrun(void)
> > +{
> > +    ss_bp_on_vmrun = false;
> > +}
> > +
> >   struct regs regs;
> >     struct regs get_regs(void)
> > @@ -215,6 +226,12 @@ struct svm_test *v2_test;
> >                   "mov regs, %%r15\n\t"           \
> >                   "mov %%r15, 0x1f8(%%rax)\n\t"   \
> >                   LOAD_GPR_C                      \
> > +                "cmpb $0, %[ss_bp]\n\t"         \
> > +                "je 1f\n\t"                     \
> > +                "pushf; pop %%r8\n\t"           \
> > +                "or $0x100, %%r8\n\t"           \
> > +                "push %%r8; popf\n\t"           \
> > +                "1: "                           \
> >                   "vmrun %%rax\n\t"               \
> >                   SAVE_GPR_C                      \
> >                   "mov 0x170(%%rax), %%r15\n\t"   \
> > @@ -234,7 +251,8 @@ int svm_vmrun(void)
> >       asm volatile (
> >           ASM_VMRUN_CMD
> >           :
> > -        : "a" (virt_to_phys(vmcb))
> > +        : "a" (virt_to_phys(vmcb)),
> > +        [ss_bp]"m"(ss_bp_on_vmrun)
> >           : "memory", "r15");
> >         return (vmcb->control.exit_code);
> > @@ -253,6 +271,7 @@ static void test_run(struct svm_test *test)
> >       do {
> >           struct svm_test *the_test = test;
> >           u64 the_vmcb = vmcb_phys;
> > +
> >           asm volatile (
> >               "clgi;\n\t" // semi-colon needed for LLVM compatibility
> >               "sti \n\t"
> > @@ -266,7 +285,8 @@ static void test_run(struct svm_test *test)
> >               "=b" (the_vmcb)             // callee save register!
> >               : [test] "0" (the_test),
> >               [vmcb_phys] "1"(the_vmcb),
> > -            [PREPARE_GIF_CLEAR] "i" (offsetof(struct svm_test, 
> > prepare_gif_clear))
> > +            [PREPARE_GIF_CLEAR] "i" (offsetof(struct svm_test, 
> > prepare_gif_clear)),
> > +            [ss_bp]"m"(ss_bp_on_vmrun)
> >               : "rax", "rcx", "rdx", "rsi",
> >               "r8", "r9", "r10", "r11" , "r12", "r13", "r14", "r15",
> >               "memory");
> > diff --git a/x86/svm.h b/x86/svm.h
> > index a0863b8..d521972 100644
> > --- a/x86/svm.h
> > +++ b/x86/svm.h
> > @@ -391,6 +391,9 @@ void vmcb_ident(struct vmcb *vmcb);
> >   struct regs get_regs(void);
> >   void vmmcall(void);
> >   int svm_vmrun(void);
> > +int svm_vmrun1(void);
> > +void set_ss_bp_on_vmrun(void);
> > +void unset_ss_bp_on_vmrun(void);
> >   void test_set_guest(test_guest_func func);
> >     extern struct vmcb *vmcb;
> > diff --git a/x86/svm_tests.c b/x86/svm_tests.c
> > index 29a0b59..7bf3624 100644
> > --- a/x86/svm_tests.c
> > +++ b/x86/svm_tests.c
> > @@ -2046,7 +2046,6 @@ static void basic_guest_main(struct svm_test 
> > *test)
> >   {
> >   }
> >   -
> >   #define SVM_TEST_REG_RESERVED_BITS(start, end, inc, str_name, reg, 
> > val,    \
> >                      resv_mask)                \
> >   {                                    \