| Message ID | 20190212230402.26851-2-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: nVMX: Fix unrestricted guest toggling | expand |
On 02/12/2019 03:04 PM, Sean Christopherson wrote: > setup_ept() explicitly checks that it can enable EPT and the starting > values for the controls are pulled from the VMCS. The only way the > masking has any effect is if hardware (or a lower VMM) reads out a > value that conflicts with its allowed settings, i.e. hardware is > seriously borked. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > x86/vmx_tests.c | 11 ++--------- > 1 file changed, 2 insertions(+), 9 deletions(-) > > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index 28bab81..a990081 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -1009,7 +1009,6 @@ static int insn_intercept_exit_handler(void) > static int setup_ept(bool enable_ad) > { > unsigned long end_of_memory; > - u32 ctrl_cpu[2]; > > if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) || > !(ctrl_cpu_rev[1].clr & CPU_EPT)) { > @@ -1032,14 +1031,8 @@ static int setup_ept(bool enable_ad) > printf("\tPWL4 is not supported\n"); > return 1; > } > - ctrl_cpu[0] = vmcs_read(CPU_EXEC_CTRL0); > - ctrl_cpu[1] = vmcs_read(CPU_EXEC_CTRL1); > - ctrl_cpu[0] = (ctrl_cpu[0] | CPU_SECONDARY) > - & ctrl_cpu_rev[0].clr; > - ctrl_cpu[1] = (ctrl_cpu[1] | CPU_EPT) > - & ctrl_cpu_rev[1].clr; > - vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu[0]); > - vmcs_write(CPU_EXEC_CTRL1, ctrl_cpu[1]); > + vmcs_write(CPU_EXEC_CTRL0, vmcs_read(CPU_EXEC_CTRL0)| CPU_SECONDARY); > + vmcs_write(CPU_EXEC_CTRL1, vmcs_read(CPU_EXEC_CTRL1)| CPU_EPT); > eptp |= (3 << EPTP_PG_WALK_LEN_SHIFT); > pml4 = alloc_page(); > memset(pml4, 0, PAGE_SIZE); Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
On Tue, Feb 12, 2019 at 3:04 PM Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > setup_ept() explicitly checks that it can enable EPT and the starting > values for the controls are pulled from the VMCS. The only way the > masking has any effect is if hardware (or a lower VMM) reads out a > value that conflicts with its allowed settings, i.e. hardware is > seriously borked. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > x86/vmx_tests.c | 11 ++--------- > 1 file changed, 2 insertions(+), 9 deletions(-) > > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index 28bab81..a990081 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -1009,7 +1009,6 @@ static int insn_intercept_exit_handler(void) > static int setup_ept(bool enable_ad) > { > unsigned long end_of_memory; > - u32 ctrl_cpu[2]; > > if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) || > !(ctrl_cpu_rev[1].clr & CPU_EPT)) { > @@ -1032,14 +1031,8 @@ static int setup_ept(bool enable_ad) > printf("\tPWL4 is not supported\n"); > return 1; > } > - ctrl_cpu[0] = vmcs_read(CPU_EXEC_CTRL0); > - ctrl_cpu[1] = vmcs_read(CPU_EXEC_CTRL1); > - ctrl_cpu[0] = (ctrl_cpu[0] | CPU_SECONDARY) > - & ctrl_cpu_rev[0].clr; > - ctrl_cpu[1] = (ctrl_cpu[1] | CPU_EPT) > - & ctrl_cpu_rev[1].clr; > - vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu[0]); > - vmcs_write(CPU_EXEC_CTRL1, ctrl_cpu[1]); > + vmcs_write(CPU_EXEC_CTRL0, vmcs_read(CPU_EXEC_CTRL0)| CPU_SECONDARY); > + vmcs_write(CPU_EXEC_CTRL1, vmcs_read(CPU_EXEC_CTRL1)| CPU_EPT); > eptp |= (3 << EPTP_PG_WALK_LEN_SHIFT); > pml4 = alloc_page(); > memset(pml4, 0, PAGE_SIZE); > -- > 2.20.1 > Reviewed-by: Marc Orr <marcorr@google.com>
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c index 28bab81..a990081 100644 --- a/x86/vmx_tests.c +++ b/x86/vmx_tests.c @@ -1009,7 +1009,6 @@ static int insn_intercept_exit_handler(void) static int setup_ept(bool enable_ad) { unsigned long end_of_memory; - u32 ctrl_cpu[2]; if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) || !(ctrl_cpu_rev[1].clr & CPU_EPT)) { @@ -1032,14 +1031,8 @@ static int setup_ept(bool enable_ad) printf("\tPWL4 is not supported\n"); return 1; } - ctrl_cpu[0] = vmcs_read(CPU_EXEC_CTRL0); - ctrl_cpu[1] = vmcs_read(CPU_EXEC_CTRL1); - ctrl_cpu[0] = (ctrl_cpu[0] | CPU_SECONDARY) - & ctrl_cpu_rev[0].clr; - ctrl_cpu[1] = (ctrl_cpu[1] | CPU_EPT) - & ctrl_cpu_rev[1].clr; - vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu[0]); - vmcs_write(CPU_EXEC_CTRL1, ctrl_cpu[1]); + vmcs_write(CPU_EXEC_CTRL0, vmcs_read(CPU_EXEC_CTRL0)| CPU_SECONDARY); + vmcs_write(CPU_EXEC_CTRL1, vmcs_read(CPU_EXEC_CTRL1)| CPU_EPT); eptp |= (3 << EPTP_PG_WALK_LEN_SHIFT); pml4 = alloc_page(); memset(pml4, 0, PAGE_SIZE);
setup_ept() explicitly checks that it can enable EPT and the starting values for the controls are pulled from the VMCS. The only way the masking has any effect is if hardware (or a lower VMM) reads out a value that conflicts with its allowed settings, i.e. hardware is seriously borked. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- x86/vmx_tests.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)