* [PATCH 0/2] nVMX: Check Host Segment Registers and Descriptor Tables on vmentry of nested guests
@ 2019-06-28 22:14 Krish Sadhukhan
2019-06-28 22:14 ` [PATCH 1/2] KVM " Krish Sadhukhan
2019-06-28 22:14 ` [PATCH 2/2] kvm-unit-test nVMX: Test " Krish Sadhukhan
0 siblings, 2 replies; 7+ messages in thread
From: Krish Sadhukhan @ 2019-06-28 22:14 UTC (permalink / raw)
To: kvm; +Cc: rkrcmar, pbonzini, jmattson
Patch# 1 implements the following checks, from Intel SDM vol 3C, on
vmentry of nested guests:
- In the selector field for each of CS, SS, DS, ES, FS, GS and TR, the
RPL (bits 1:0) and the TI flag (bit 2) must be 0.
- The selector fields for CS and TR cannot be 0000H.
- The selector field for SS cannot be 0000H if the "host address-space
size" VM-exit control is 0.
- On processors that support Intel 64 architecture, the base-address
fields for FS, GS and TR must contain canonical addresses.
Patch# 2 adds kvm-unit-tests for the above checks.
[PATCH 1/2] KVM nVMX: Check Host Segment Registers and Descriptor Tables on
[PATCH 2/2] kvm-unit-test nVMX: Test Host Segment Registers and Descriptor Tables on
arch/x86/kvm/vmx/nested.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
Krish Sadhukhan (1):
nVMX: Check Host Segment Registers and Descriptor Tables on vmentry of nested guests
x86/vmx_tests.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 164 insertions(+)
Krish Sadhukhan (1):
nVMX: Test Host Segment Registers and Descriptor Tables on vmentry of nested guests
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] KVM nVMX: Check Host Segment Registers and Descriptor Tables on vmentry of nested guests
2019-06-28 22:14 [PATCH 0/2] nVMX: Check Host Segment Registers and Descriptor Tables on vmentry of nested guests Krish Sadhukhan
@ 2019-06-28 22:14 ` Krish Sadhukhan
2019-07-02 16:25 ` Paolo Bonzini
2019-06-28 22:14 ` [PATCH 2/2] kvm-unit-test nVMX: Test " Krish Sadhukhan
1 sibling, 1 reply; 7+ messages in thread
From: Krish Sadhukhan @ 2019-06-28 22:14 UTC (permalink / raw)
To: kvm; +Cc: rkrcmar, pbonzini, jmattson
According to section "Checks on Host Segment and Descriptor-Table
Registers" in Intel SDM vol 3C, the following checks are performed on
vmentry of nested guests:
- In the selector field for each of CS, SS, DS, ES, FS, GS and TR, the
RPL (bits 1:0) and the TI flag (bit 2) must be 0.
- The selector fields for CS and TR cannot be 0000H.
- The selector field for SS cannot be 0000H if the "host address-space
size" VM-exit control is 0.
- On processors that support Intel 64 architecture, the base-address
fields for FS, GS and TR must contain canonical addresses.
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
---
arch/x86/kvm/vmx/nested.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index f1a69117ac0f..856a83aa42f5 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -2609,6 +2609,30 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu,
!kvm_pat_valid(vmcs12->host_ia32_pat))
return -EINVAL;
+ ia32e = (vmcs12->vm_exit_controls &
+ VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0;
+
+ if (vmcs12->host_cs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
+ vmcs12->host_ss_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
+ vmcs12->host_ds_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
+ vmcs12->host_es_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
+ vmcs12->host_fs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
+ vmcs12->host_gs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
+ vmcs12->host_tr_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
+ vmcs12->host_cs_selector == 0 ||
+ vmcs12->host_tr_selector == 0 ||
+ (vmcs12->host_ss_selector == 0 && !ia32e))
+ return -EINVAL;
+
+#ifdef CONFIG_X86_64
+ if (is_noncanonical_address(vmcs12->host_fs_base, vcpu) ||
+ is_noncanonical_address(vmcs12->host_gs_base, vcpu) ||
+ is_noncanonical_address(vmcs12->host_gdtr_base, vcpu) ||
+ is_noncanonical_address(vmcs12->host_idtr_base, vcpu) ||
+ is_noncanonical_address(vmcs12->host_tr_base, vcpu))
+ return -EINVAL;
+#endif
+
/*
* If the load IA32_EFER VM-exit control is 1, bits reserved in the
* IA32_EFER MSR must be 0 in the field for that register. In addition,
@@ -2616,8 +2640,6 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu,
* the host address-space size VM-exit control.
*/
if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_EFER) {
- ia32e = (vmcs12->vm_exit_controls &
- VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0;
if (!kvm_valid_efer(vcpu, vmcs12->host_ia32_efer) ||
ia32e != !!(vmcs12->host_ia32_efer & EFER_LMA) ||
ia32e != !!(vmcs12->host_ia32_efer & EFER_LME))
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] kvm-unit-test nVMX: Test Host Segment Registers and Descriptor Tables on vmentry of nested guests
2019-06-28 22:14 [PATCH 0/2] nVMX: Check Host Segment Registers and Descriptor Tables on vmentry of nested guests Krish Sadhukhan
2019-06-28 22:14 ` [PATCH 1/2] KVM " Krish Sadhukhan
@ 2019-06-28 22:14 ` Krish Sadhukhan
2019-06-29 2:00 ` Nadav Amit
1 sibling, 1 reply; 7+ messages in thread
From: Krish Sadhukhan @ 2019-06-28 22:14 UTC (permalink / raw)
To: kvm; +Cc: rkrcmar, pbonzini, jmattson
According to section "Checks on Host Segment and Descriptor-Table
Registers" in Intel SDM vol 3C, the following checks are performed on
vmentry of nested guests:
- In the selector field for each of CS, SS, DS, ES, FS, GS and TR, the
RPL (bits 1:0) and the TI flag (bit 2) must be 0.
- The selector fields for CS and TR cannot be 0000H.
- The selector field for SS cannot be 0000H if the "host address-space
size" VM-exit control is 0.
- On processors that support Intel 64 architecture, the base-address
fields for FS, GS, GDTR, IDTR, and TR must contain canonical
addresses.
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
---
lib/x86/processor.h | 5 ++
x86/vmx_tests.c | 159 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 164 insertions(+)
diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 4fef0bc..c6edc26 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -461,6 +461,11 @@ static inline void write_pkru(u32 pkru)
: : "a" (eax), "c" (ecx), "d" (edx));
}
+static u64 make_non_canonical(u64 addr)
+{
+ return (addr | 1ull << 48);
+}
+
static inline bool is_canonical(u64 addr)
{
return (s64)(addr << 16) >> 16 == addr;
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index b50d858..5911a60 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -6938,6 +6938,163 @@ static void test_load_host_pat(void)
test_pat(HOST_PAT, "HOST_PAT", EXI_CONTROLS, EXI_LOAD_PAT);
}
+/*
+ * Test a value for the given VMCS field.
+ *
+ * "field" - VMCS field
+ * "field_name" - string name of VMCS field
+ * "bit_start" - starting bit
+ * "bit_end" - ending bit
+ * "val" - value that the bit range must or must not contain
+ * "valid_val" - whether value given in 'val' must be valid or not
+ * "error" - expected VMCS error when vmentry fails for an invalid value
+ */
+static void test_vmcs_field(u64 field, const char *field_name, u32 bit_start,
+ u32 bit_end, u64 val, bool valid_val, u32 error)
+{
+ u64 field_saved = vmcs_read(field);
+ u32 i;
+ u64 tmp;
+ u32 bit_on;
+ u64 mask = ~0ull;
+
+ mask = (mask >> bit_end) << bit_end;
+ mask = mask | ((1 << bit_start) - 1);
+ tmp = (field_saved & mask) | (val << bit_start);
+
+ vmcs_write(field, tmp);
+ report_prefix_pushf("%s %lx", field_name, tmp);
+ if (valid_val)
+ test_vmx_vmlaunch(0, false);
+ else
+ test_vmx_vmlaunch(error, false);
+ report_prefix_pop();
+
+ for (i = bit_start; i <= bit_end; i = i + 2) {
+ bit_on = ((1ull < i) & (val << bit_start)) ? 0 : 1;
+ if (bit_on)
+ tmp = field_saved | (1ull << i);
+ else
+ tmp = field_saved & ~(1ull << i);
+ vmcs_write(field, tmp);
+ report_prefix_pushf("%s %lx", field_name, tmp);
+ if (valid_val)
+ test_vmx_vmlaunch(error, false);
+ else
+ test_vmx_vmlaunch(0, false);
+ report_prefix_pop();
+ }
+
+ vmcs_write(field, field_saved);
+}
+
+static void test_canonical(u64 field, const char * field_name)
+{
+ u64 addr_saved = vmcs_read(field);
+ u64 addr = addr_saved;
+
+ report_prefix_pushf("%s %lx", field_name, addr);
+ if (is_canonical(addr)) {
+ test_vmx_vmlaunch(0, false);
+ report_prefix_pop();
+
+ addr = make_non_canonical(addr);
+ vmcs_write(field, addr);
+ report_prefix_pushf("%s %lx", field_name, addr);
+ test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD,
+ false);
+
+ vmcs_write(field, addr_saved);
+ } else {
+ test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD,
+ false);
+ }
+ report_prefix_pop();
+}
+
+/*
+ * 1. In the selector field for each of CS, SS, DS, ES, FS, GS and TR, the
+ * RPL (bits 1:0) and the TI flag (bit 2) must be 0.
+ * 2. The selector fields for CS and TR cannot be 0000H.
+ * 3. The selector field for SS cannot be 0000H if the "host address-space
+ * size" VM-exit control is 0.
+ * 4. On processors that support Intel 64 architecture, the base-address
+ * fields for FS, GS and TR must contain canonical addresses.
+ */
+static void test_host_segment_regs(void)
+{
+ u32 exit_ctrl_saved = vmcs_read(EXI_CONTROLS);
+ u16 selector_saved;
+
+ /*
+ * Test RPL and TI flags
+ */
+ test_vmcs_field(HOST_SEL_CS, "HOST_SEL_CS", 0, 2, 0x0, true,
+ VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
+ test_vmcs_field(HOST_SEL_SS, "HOST_SEL_SS", 0, 2, 0x0, true,
+ VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
+ test_vmcs_field(HOST_SEL_DS, "HOST_SEL_DS", 0, 2, 0x0, true,
+ VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
+ test_vmcs_field(HOST_SEL_ES, "HOST_SEL_ES", 0, 2, 0x0, true,
+ VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
+ test_vmcs_field(HOST_SEL_FS, "HOST_SEL_FS", 0, 2, 0x0, true,
+ VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
+ test_vmcs_field(HOST_SEL_GS, "HOST_SEL_GS", 0, 2, 0x0, true,
+ VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
+ test_vmcs_field(HOST_SEL_TR, "HOST_SEL_TR", 0, 2, 0x0, true,
+ VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
+
+ /*
+ * Test that CS and TR fields can not be 0x0000
+ */
+ test_vmcs_field(HOST_SEL_CS, "HOST_SEL_CS", 3, 15, 0x0000, false,
+ VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
+ test_vmcs_field(HOST_SEL_TR, "HOST_SEL_TR", 3, 15, 0x0000, false,
+ VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
+
+ /*
+ * SS field can not be 0x0000 if "host address-space size" VM-exit
+ * control is 0
+ */
+ selector_saved = vmcs_read(HOST_SEL_SS);
+ vmcs_write(HOST_SEL_SS, 0);
+ if (exit_ctrl_saved & EXI_HOST_64) {
+ report_prefix_pushf("HOST_SEL_SS 0");
+ test_vmx_vmlaunch(0, false);
+ report_prefix_pop();
+
+ vmcs_write(EXI_CONTROLS, exit_ctrl_saved & ~EXI_HOST_64);
+ }
+
+ report_prefix_pushf("HOST_SEL_SS 0");
+ test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD, false);
+ report_prefix_pop();
+
+ vmcs_write(HOST_SEL_SS, selector_saved);
+ vmcs_write(EXI_CONTROLS, exit_ctrl_saved);
+
+#ifdef __x86_64__
+ /*
+ * Base address for FS, GS and TR must be canonical
+ */
+ test_canonical(HOST_BASE_FS, "HOST_BASE_FS");
+ test_canonical(HOST_BASE_GS, "HOST_BASE_GS");
+ test_canonical(HOST_BASE_TR, "HOST_BASE_TR");
+#endif
+}
+
+/*
+ * On processors that support Intel 64 architecture, the base-address
+ * fields for GDTR and IDTR must contain canonical addresses.
+ */
+static void test_host_desc_tables(void)
+{
+#ifdef __x86_64__
+ test_canonical(HOST_BASE_GDTR, "HOST_BASE_GDTR");
+ test_canonical(HOST_BASE_IDTR, "HOST_BASE_IDTR");
+#endif
+}
+
/*
* Check that the virtual CPU checks the VMX Host State Area as
* documented in the Intel SDM.
@@ -6958,6 +7115,8 @@ static void vmx_host_state_area_test(void)
test_host_efer();
test_load_host_pat();
+ test_host_segment_regs();
+ test_host_desc_tables();
}
/*
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] kvm-unit-test nVMX: Test Host Segment Registers and Descriptor Tables on vmentry of nested guests
2019-06-28 22:14 ` [PATCH 2/2] kvm-unit-test nVMX: Test " Krish Sadhukhan
@ 2019-06-29 2:00 ` Nadav Amit
2019-06-29 2:30 ` Nadav Amit
0 siblings, 1 reply; 7+ messages in thread
From: Nadav Amit @ 2019-06-29 2:00 UTC (permalink / raw)
To: Krish Sadhukhan; +Cc: kvm, rkrcmar, pbonzini, jmattson
> On Jun 28, 2019, at 3:14 PM, Krish Sadhukhan <krish.sadhukhan@oracle.com> wrote:
>
> According to section "Checks on Host Segment and Descriptor-Table
> Registers" in Intel SDM vol 3C, the following checks are performed on
> vmentry of nested guests:
>
> - In the selector field for each of CS, SS, DS, ES, FS, GS and TR, the
> RPL (bits 1:0) and the TI flag (bit 2) must be 0.
> - The selector fields for CS and TR cannot be 0000H.
> - The selector field for SS cannot be 0000H if the "host address-space
> size" VM-exit control is 0.
> - On processors that support Intel 64 architecture, the base-address
> fields for FS, GS, GDTR, IDTR, and TR must contain canonical
> addresses.
>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
> ---
> lib/x86/processor.h | 5 ++
> x86/vmx_tests.c | 159 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 164 insertions(+)
>
> diff --git a/lib/x86/processor.h b/lib/x86/processor.h
> index 4fef0bc..c6edc26 100644
> --- a/lib/x86/processor.h
> +++ b/lib/x86/processor.h
> @@ -461,6 +461,11 @@ static inline void write_pkru(u32 pkru)
> : : "a" (eax), "c" (ecx), "d" (edx));
> }
>
> +static u64 make_non_canonical(u64 addr)
> +{
> + return (addr | 1ull << 48);
> +}
You may wan to make it “static inline”. On my system I get:
processor.h:464:12: error: ‘make_non_canonical’ defined but not used [-Werror=unused-function]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] kvm-unit-test nVMX: Test Host Segment Registers and Descriptor Tables on vmentry of nested guests
2019-06-29 2:00 ` Nadav Amit
@ 2019-06-29 2:30 ` Nadav Amit
0 siblings, 0 replies; 7+ messages in thread
From: Nadav Amit @ 2019-06-29 2:30 UTC (permalink / raw)
To: Krish Sadhukhan; +Cc: kvm, rkrcmar, pbonzini, jmattson
> On Jun 28, 2019, at 7:00 PM, Nadav Amit <nadav.amit@gmail.com> wrote:
>
>> On Jun 28, 2019, at 3:14 PM, Krish Sadhukhan <krish.sadhukhan@oracle.com> wrote:
>>
>> According to section "Checks on Host Segment and Descriptor-Table
>> Registers" in Intel SDM vol 3C, the following checks are performed on
>> vmentry of nested guests:
>>
>> - In the selector field for each of CS, SS, DS, ES, FS, GS and TR, the
>> RPL (bits 1:0) and the TI flag (bit 2) must be 0.
>> - The selector fields for CS and TR cannot be 0000H.
>> - The selector field for SS cannot be 0000H if the "host address-space
>> size" VM-exit control is 0.
>> - On processors that support Intel 64 architecture, the base-address
>> fields for FS, GS, GDTR, IDTR, and TR must contain canonical
>> addresses.
>>
>> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
>> Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
>> ---
>> lib/x86/processor.h | 5 ++
>> x86/vmx_tests.c | 159 ++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 164 insertions(+)
>>
>> diff --git a/lib/x86/processor.h b/lib/x86/processor.h
>> index 4fef0bc..c6edc26 100644
>> --- a/lib/x86/processor.h
>> +++ b/lib/x86/processor.h
>> @@ -461,6 +461,11 @@ static inline void write_pkru(u32 pkru)
>> : : "a" (eax), "c" (ecx), "d" (edx));
>> }
>>
>> +static u64 make_non_canonical(u64 addr)
>> +{
>> + return (addr | 1ull << 48);
>> +}
>
> You may wan to make it “static inline”. On my system I get:
>
> processor.h:464:12: error: ‘make_non_canonical’ defined but not used [-Werror=unused-function]
And I also get on bare-metal:
FAIL: HOST_SEL_SS 0: VMX inst error is 8 (actual 7)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] KVM nVMX: Check Host Segment Registers and Descriptor Tables on vmentry of nested guests
2019-06-28 22:14 ` [PATCH 1/2] KVM " Krish Sadhukhan
@ 2019-07-02 16:25 ` Paolo Bonzini
2019-07-03 23:57 ` Krish Sadhukhan
0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2019-07-02 16:25 UTC (permalink / raw)
To: Krish Sadhukhan, kvm; +Cc: rkrcmar, jmattson
On 29/06/19 00:14, Krish Sadhukhan wrote:
> According to section "Checks on Host Segment and Descriptor-Table
> Registers" in Intel SDM vol 3C, the following checks are performed on
> vmentry of nested guests:
>
> - In the selector field for each of CS, SS, DS, ES, FS, GS and TR, the
> RPL (bits 1:0) and the TI flag (bit 2) must be 0.
> - The selector fields for CS and TR cannot be 0000H.
> - The selector field for SS cannot be 0000H if the "host address-space
> size" VM-exit control is 0.
> - On processors that support Intel 64 architecture, the base-address
> fields for FS, GS and TR must contain canonical addresses.
>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
All these tests are getting expensive. Can you look into skipping them
whenever dirty_vmcs12 is clear?
Thanks,
Paolo
> ---
> arch/x86/kvm/vmx/nested.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index f1a69117ac0f..856a83aa42f5 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -2609,6 +2609,30 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu,
> !kvm_pat_valid(vmcs12->host_ia32_pat))
> return -EINVAL;
>
> + ia32e = (vmcs12->vm_exit_controls &
> + VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0;
> +
> + if (vmcs12->host_cs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
> + vmcs12->host_ss_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
> + vmcs12->host_ds_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
> + vmcs12->host_es_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
> + vmcs12->host_fs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
> + vmcs12->host_gs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
> + vmcs12->host_tr_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
> + vmcs12->host_cs_selector == 0 ||
> + vmcs12->host_tr_selector == 0 ||
> + (vmcs12->host_ss_selector == 0 && !ia32e))
> + return -EINVAL;
> +
> +#ifdef CONFIG_X86_64
> + if (is_noncanonical_address(vmcs12->host_fs_base, vcpu) ||
> + is_noncanonical_address(vmcs12->host_gs_base, vcpu) ||
> + is_noncanonical_address(vmcs12->host_gdtr_base, vcpu) ||
> + is_noncanonical_address(vmcs12->host_idtr_base, vcpu) ||
> + is_noncanonical_address(vmcs12->host_tr_base, vcpu))
> + return -EINVAL;
> +#endif
> +
> /*
> * If the load IA32_EFER VM-exit control is 1, bits reserved in the
> * IA32_EFER MSR must be 0 in the field for that register. In addition,
> @@ -2616,8 +2640,6 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu,
> * the host address-space size VM-exit control.
> */
> if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_EFER) {
> - ia32e = (vmcs12->vm_exit_controls &
> - VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0;
> if (!kvm_valid_efer(vcpu, vmcs12->host_ia32_efer) ||
> ia32e != !!(vmcs12->host_ia32_efer & EFER_LMA) ||
> ia32e != !!(vmcs12->host_ia32_efer & EFER_LME))
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] KVM nVMX: Check Host Segment Registers and Descriptor Tables on vmentry of nested guests
2019-07-02 16:25 ` Paolo Bonzini
@ 2019-07-03 23:57 ` Krish Sadhukhan
0 siblings, 0 replies; 7+ messages in thread
From: Krish Sadhukhan @ 2019-07-03 23:57 UTC (permalink / raw)
To: Paolo Bonzini, kvm; +Cc: rkrcmar, jmattson
On 07/02/2019 09:25 AM, Paolo Bonzini wrote:
> On 29/06/19 00:14, Krish Sadhukhan wrote:
>> According to section "Checks on Host Segment and Descriptor-Table
>> Registers" in Intel SDM vol 3C, the following checks are performed on
>> vmentry of nested guests:
>>
>> - In the selector field for each of CS, SS, DS, ES, FS, GS and TR, the
>> RPL (bits 1:0) and the TI flag (bit 2) must be 0.
>> - The selector fields for CS and TR cannot be 0000H.
>> - The selector field for SS cannot be 0000H if the "host address-space
>> size" VM-exit control is 0.
>> - On processors that support Intel 64 architecture, the base-address
>> fields for FS, GS and TR must contain canonical addresses.
>>
>> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
>> Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
> All these tests are getting expensive. Can you look into skipping them
> whenever dirty_vmcs12 is clear?
OK. I am working on it and will send a separate patch-set for that. In
the meantime, I will send v2 of this patchset containing a fix for a
compilation error.
Thanks.
> Thanks,
>
> Paolo
>
>> ---
>> arch/x86/kvm/vmx/nested.c | 26 ++++++++++++++++++++++++--
>> 1 file changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
>> index f1a69117ac0f..856a83aa42f5 100644
>> --- a/arch/x86/kvm/vmx/nested.c
>> +++ b/arch/x86/kvm/vmx/nested.c
>> @@ -2609,6 +2609,30 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu,
>> !kvm_pat_valid(vmcs12->host_ia32_pat))
>> return -EINVAL;
>>
>> + ia32e = (vmcs12->vm_exit_controls &
>> + VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0;
>> +
>> + if (vmcs12->host_cs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
>> + vmcs12->host_ss_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
>> + vmcs12->host_ds_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
>> + vmcs12->host_es_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
>> + vmcs12->host_fs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
>> + vmcs12->host_gs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
>> + vmcs12->host_tr_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) ||
>> + vmcs12->host_cs_selector == 0 ||
>> + vmcs12->host_tr_selector == 0 ||
>> + (vmcs12->host_ss_selector == 0 && !ia32e))
>> + return -EINVAL;
>> +
>> +#ifdef CONFIG_X86_64
>> + if (is_noncanonical_address(vmcs12->host_fs_base, vcpu) ||
>> + is_noncanonical_address(vmcs12->host_gs_base, vcpu) ||
>> + is_noncanonical_address(vmcs12->host_gdtr_base, vcpu) ||
>> + is_noncanonical_address(vmcs12->host_idtr_base, vcpu) ||
>> + is_noncanonical_address(vmcs12->host_tr_base, vcpu))
>> + return -EINVAL;
>> +#endif
>> +
>> /*
>> * If the load IA32_EFER VM-exit control is 1, bits reserved in the
>> * IA32_EFER MSR must be 0 in the field for that register. In addition,
>> @@ -2616,8 +2640,6 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu,
>> * the host address-space size VM-exit control.
>> */
>> if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_EFER) {
>> - ia32e = (vmcs12->vm_exit_controls &
>> - VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0;
>> if (!kvm_valid_efer(vcpu, vmcs12->host_ia32_efer) ||
>> ia32e != !!(vmcs12->host_ia32_efer & EFER_LMA) ||
>> ia32e != !!(vmcs12->host_ia32_efer & EFER_LME))
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-07-03 23:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-28 22:14 [PATCH 0/2] nVMX: Check Host Segment Registers and Descriptor Tables on vmentry of nested guests Krish Sadhukhan
2019-06-28 22:14 ` [PATCH 1/2] KVM " Krish Sadhukhan
2019-07-02 16:25 ` Paolo Bonzini
2019-07-03 23:57 ` Krish Sadhukhan
2019-06-28 22:14 ` [PATCH 2/2] kvm-unit-test nVMX: Test " Krish Sadhukhan
2019-06-29 2:00 ` Nadav Amit
2019-06-29 2:30 ` Nadav Amit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).