diff mbox series

[kvm-unit-tests] X86: Set up EPT before running vmx_pf_exception_test

Message ID 20220715113334.52491-1-yu.c.zhang@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [kvm-unit-tests] X86: Set up EPT before running vmx_pf_exception_test | expand

Commit Message

Yu Zhang July 15, 2022, 11:33 a.m. UTC
Although currently vmx_pf_exception_test can succeed, its
success is actually because we are using identical mappings
in the page tables and EB.PF is not set by L1. In practice,
the #PFs shall be expected by L1, if it is using shadowing
for L2.

So just set up the EPT, and clear the EB.PT, then L1 has the
right to claim a failure if a #PF is encountered.

Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
---
 x86/vmx_tests.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Sean Christopherson Aug. 2, 2022, 8:41 p.m. UTC | #1
On Fri, Jul 15, 2022, Yu Zhang wrote:
> Although currently vmx_pf_exception_test can succeed, its
> success is actually because we are using identical mappings
> in the page tables and EB.PF is not set by L1. In practice,
> the #PFs shall be expected by L1, if it is using shadowing
> for L2.

I'm a bit lost.  Is there an actual failure somewhere?  AFAICT, this passes when
run as L1 or L2, with or without EPT enabled.

> So just set up the EPT, and clear the EB.PT, then L1 has the
> right to claim a failure if a #PF is encountered.
> 
> Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
> ---
>  x86/vmx_tests.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 4d581e7..cc90611 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -10639,6 +10639,17 @@ static void __vmx_pf_exception_test(invalidate_tlb_t inv_fn, void *data)
>  
>  static void vmx_pf_exception_test(void)
>  {
> +	u32 eb;
> +
> +	if (setup_ept(false)) {
> +		printf("EPT not supported.\n");
> +		return;
> +	}
> +
> +	eb = vmcs_read(EXC_BITMAP);
> +	eb &= ~(1 << PF_VECTOR);
> +	vmcs_write(EXC_BITMAP, eb);
> +
>  	__vmx_pf_exception_test(NULL, NULL);
>  }
>  
> -- 
> 2.25.1
>
Yu Zhang Aug. 3, 2022, 1:57 a.m. UTC | #2
On Tue, Aug 02, 2022 at 08:41:47PM +0000, Sean Christopherson wrote:
> On Fri, Jul 15, 2022, Yu Zhang wrote:
> > Although currently vmx_pf_exception_test can succeed, its
> > success is actually because we are using identical mappings
> > in the page tables and EB.PF is not set by L1. In practice,
> > the #PFs shall be expected by L1, if it is using shadowing
> > for L2.
> 
> I'm a bit lost.  Is there an actual failure somewhere?  AFAICT, this passes when
> run as L1 or L2, with or without EPT enabled.

Thanks for your reply, Sean.

There's no failure. But IMHO, there should have been(for the
vmx_pf_exception_test, not the access test) -  L1 shall expect
#PF induced VM exits, when it is using shadow for L2.

B.R.
Yu


> > So just set up the EPT, and clear the EB.PT, then L1 has the
> > right to claim a failure if a #PF is encountered.
> > 
> > Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
> > ---
> >  x86/vmx_tests.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > > index
4d581e7..cc90611 100644
> > --- a/x86/vmx_tests.c
> > +++ b/x86/vmx_tests.c
> > @@ -10639,6 +10639,17 @@ static void __vmx_pf_exception_test(invalidate_tlb_t inv_fn, void *data)
> >  
> >  static void vmx_pf_exception_test(void)
> >  {
> > +	u32 eb;
> > +
> > +	if (setup_ept(false)) {
> > +		printf("EPT not supported.\n");
> > +		return;
> > +	}
> > +
> > +	eb = vmcs_read(EXC_BITMAP);
> > +	eb &= ~(1 << PF_VECTOR);
> > +	vmcs_write(EXC_BITMAP, eb);
> > +
> >  	__vmx_pf_exception_test(NULL, NULL);
> >  }
> >  
> > -- 
> > 2.25.1
> >
Sean Christopherson Aug. 3, 2022, 2:25 p.m. UTC | #3
On Wed, Aug 03, 2022, Yu Zhang wrote:
> On Tue, Aug 02, 2022 at 08:41:47PM +0000, Sean Christopherson wrote:
> > On Fri, Jul 15, 2022, Yu Zhang wrote:
> > > Although currently vmx_pf_exception_test can succeed, its
> > > success is actually because we are using identical mappings
> > > in the page tables and EB.PF is not set by L1. In practice,
> > > the #PFs shall be expected by L1, if it is using shadowing
> > > for L2.
> > 
> > I'm a bit lost.  Is there an actual failure somewhere?  AFAICT, this passes when
> > run as L1 or L2, with or without EPT enabled.
> 
> Thanks for your reply, Sean.
> 
> There's no failure. But IMHO, there should have been(for the
> vmx_pf_exception_test, not the access test) -  L1 shall expect
> #PF induced VM exits, when it is using shadow for L2.

Note, I'm assuming L1 == KVM-Unit-Tests, let me know if we're not using the same
terminology.

Not using EPT / TDP doesn't strictly imply page table shadowing.  E.g. if a hypervisor
provides a paravirt interface to install mappings, and the contract is that the VM
must use the paravirt API, then the hypervisor doesn't need to intercept page faults
because there are effectively no guest PTEs to write-protect / shadow.  

That's more or less what's happening here, L1 and L2 are collaborating to create
page tables for L2, and so L1 doesn't need to intercept #PF.
Yu Zhang Aug. 4, 2022, 2:14 a.m. UTC | #4
On Wed, Aug 03, 2022 at 02:25:15PM +0000, Sean Christopherson wrote:
> On Wed, Aug 03, 2022, Yu Zhang wrote:
> > On Tue, Aug 02, 2022 at 08:41:47PM +0000, Sean Christopherson wrote:
> > > On Fri, Jul 15, 2022, Yu Zhang wrote:
> > > > Although currently vmx_pf_exception_test can succeed, its
> > > > success is actually because we are using identical mappings
> > > > in the page tables and EB.PF is not set by L1. In practice,
> > > > the #PFs shall be expected by L1, if it is using shadowing
> > > > for L2.
> > > 
> > > I'm a bit lost.  Is there an actual failure somewhere?  AFAICT, this passes when
> > > run as L1 or L2, with or without EPT enabled.
> > 
> > Thanks for your reply, Sean.
> > 
> > There's no failure. But IMHO, there should have been(for the
> > vmx_pf_exception_test, not the access test) -  L1 shall expect
> > #PF induced VM exits, when it is using shadow for L2.
> 
> Note, I'm assuming L1 == KVM-Unit-Tests, let me know if we're not using the same
> terminology.
> 
> Not using EPT / TDP doesn't strictly imply page table shadowing.  E.g. if a hypervisor
> provides a paravirt interface to install mappings, and the contract is that the VM
> must use the paravirt API, then the hypervisor doesn't need to intercept page faults
> because there are effectively no guest PTEs to write-protect / shadow.  
> 
> That's more or less what's happening here, L1 and L2 are collaborating to create
> page tables for L2, and so L1 doesn't need to intercept #PF.

Oh... So it is intentionally designed to let L1 and L2 use the same address space.
Then we can just drop this patch. Thanks a lot for the explanation!

B.R.
Yu
diff mbox series

Patch

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 4d581e7..cc90611 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -10639,6 +10639,17 @@  static void __vmx_pf_exception_test(invalidate_tlb_t inv_fn, void *data)
 
 static void vmx_pf_exception_test(void)
 {
+	u32 eb;
+
+	if (setup_ept(false)) {
+		printf("EPT not supported.\n");
+		return;
+	}
+
+	eb = vmcs_read(EXC_BITMAP);
+	eb &= ~(1 << PF_VECTOR);
+	vmcs_write(EXC_BITMAP, eb);
+
 	__vmx_pf_exception_test(NULL, NULL);
 }