Patch# 1: Moves the check for upper 32 reserved bits of DR6 to a new function. Patch# 2: Adds the KVM checks for DR6[63:32] and DR7[64:32] reserved bits Patch# 3: Adds kvm-unit-tests for DR6[63:32] and DR7[64:32] reserved bits and reserved bits in EFER Patch# 4: Removes the duplicate definition of 'vmcb' that sneaked via one of my previous patches. [PATCH 1/4] KVM: x86: Move the check for upper 32 reserved bits of [PATCH 2/4] KVM: nSVM: Check that DR6[63:32] and DR7[64:32] are not [PATCH 3/4] kvm-unit-tests: nSVM: Test that DR6[63:32], DR7[63:32] [PATCH 4/4] kvm-unit-tests: x86: Remove duplicate instance of 'vmcb' arch/x86/kvm/svm/nested.c | 3 +++ arch/x86/kvm/x86.c | 2 +- arch/x86/kvm/x86.h | 5 +++++ 3 files changed, 9 insertions(+), 1 deletion(-) Krish Sadhukhan (2): KVM: x86: Move the check for upper 32 reserved bits of DR6 to separate fun KVM: nVMX: Check that DR6[63:32] and DR7[64:32] are not set on vmrun of ne x86/svm.c | 1 - x86/svm.h | 3 +++ x86/svm_tests.c | 59 ++++++++++++++++++++++++++++++++++++++------------------- 3 files changed, 42 insertions(+), 21 deletions(-) Krish Sadhukhan (2): kvm-unit-tests: nSVM: Test that DR6[63:32], DR7[63:32] and EFER reserved b kvm-unit-tests: x86: Remove duplicate instance of 'vmcb'
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> --- arch/x86/kvm/x86.c | 2 +- arch/x86/kvm/x86.h | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c17e6eb..4746ec1 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -1096,7 +1096,7 @@ static int __kvm_set_dr(struct kvm_vcpu *vcpu, int dr, unsigned long val) case 4: /* fall through */ case 6: - if (val & 0xffffffff00000000ULL) + if (!kvm_dr6_valid(val)) return -1; /* #GP */ vcpu->arch.dr6 = (val & DR6_VOLATILE) | kvm_dr6_fixed(vcpu); break; diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index b968acc..5043108 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -354,6 +354,11 @@ static inline bool kvm_dr7_valid(u64 data) /* Bits [63:32] are reserved */ return !(data >> 32); } +static inline bool kvm_dr6_valid(u64 data) +{ + /* Bits [63:32] are reserved */ + return !(data >> 32); +} void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu); void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu); -- 1.8.3.1
According to section "Canonicalization and Consistency Checks" in APM vol. 2 the following guest state is illegal: "DR6[63:32] are not zero." "DR7[63:32] are not zero." "Any MBZ bit of EFER is set." Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> --- arch/x86/kvm/svm/nested.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c index 9a2a62e..2fec51d 100644 --- a/arch/x86/kvm/svm/nested.c +++ b/arch/x86/kvm/svm/nested.c @@ -208,6 +208,9 @@ static bool nested_vmcb_checks(struct vmcb *vmcb) if ((vmcb->save.efer & EFER_SVME) == 0) return false; + if (!kvm_dr6_valid(vmcb->save.dr6) || !kvm_dr7_valid(vmcb->save.dr7)) + return false; + if ((vmcb->control.intercept & (1ULL << INTERCEPT_VMRUN)) == 0) return false; -- 1.8.3.1
According to section "Canonicalization and Consistency Checks" in APM vol. 2 the following guest state is illegal: "DR6[63:32] are not zero." "DR7[63:32] are not zero." "Any MBZ bit of EFER is set." Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> --- x86/svm.h | 3 +++ x86/svm_tests.c | 59 ++++++++++++++++++++++++++++++++++++++------------------- 2 files changed, 42 insertions(+), 20 deletions(-) diff --git a/x86/svm.h b/x86/svm.h index 3d5b79c..14418ef 100644 --- a/x86/svm.h +++ b/x86/svm.h @@ -329,6 +329,9 @@ struct __attribute__ ((__packed__)) vmcb { #define SVM_CR3_LONG_RESERVED_MASK 0xfff0000000000fe7U #define SVM_CR4_LEGACY_RESERVED_MASK 0xffbaf000U #define SVM_CR4_RESERVED_MASK 0xffffffffffbaf000U +#define SVM_DR6_RESERVED_MASK 0xffffffffffff1ff0U +#define SVM_DR7_RESERVED_MASK 0xffffffff0000cc00U +#define SVM_EFER_RESERVED_MASK 0xffffffffffff0200U #define MSR_BITMAP_SIZE 8192 diff --git a/x86/svm_tests.c b/x86/svm_tests.c index 99cfe4a..d96f3ee 100644 --- a/x86/svm_tests.c +++ b/x86/svm_tests.c @@ -1799,7 +1799,8 @@ static void basic_guest_main(struct svm_test *test) { } -#define SVM_TEST_CR_RESERVED_BITS(start, end, inc, cr, val, resv_mask) \ +#define SVM_TEST_REG_RESERVED_BITS(start, end, inc, str_name, reg, val, \ + resv_mask) \ { \ u64 tmp, mask; \ int i; \ @@ -1809,18 +1810,9 @@ static void basic_guest_main(struct svm_test *test) if (!(mask & resv_mask)) \ continue; \ tmp = val | mask; \ - switch (cr) { \ - case 0: \ - vmcb->save.cr0 = tmp; \ - break; \ - case 3: \ - vmcb->save.cr3 = tmp; \ - break; \ - case 4: \ - vmcb->save.cr4 = tmp; \ - } \ - report(svm_vmrun() == SVM_EXIT_ERR, "Test CR%d %d:%d: %lx",\ - cr, end, start, tmp); \ + reg = tmp; \ + report(svm_vmrun() == SVM_EXIT_ERR, "Test %s %d:%d: %lx",\ + str_name, end, start, tmp); \ } \ } @@ -1871,7 +1863,7 @@ static void svm_guest_state_test(void) */ cr0 = cr0_saved; - SVM_TEST_CR_RESERVED_BITS(32, 63, 4, 0, cr0_saved, + SVM_TEST_REG_RESERVED_BITS(32, 63, 4, "CR0", vmcb->save.cr0, cr0_saved, SVM_CR0_RESERVED_MASK); vmcb->save.cr0 = cr0_saved; @@ -1891,19 +1883,19 @@ static void svm_guest_state_test(void) vmcb->save.efer = efer; cr4 |= X86_CR4_PAE; vmcb->save.cr4 = cr4; - SVM_TEST_CR_RESERVED_BITS(0, 2, 1, 3, cr3_saved, + SVM_TEST_REG_RESERVED_BITS(0, 2, 1, "CR3", vmcb->save.cr3, cr3_saved, SVM_CR3_LEGACY_PAE_RESERVED_MASK); cr4 = cr4_saved & ~X86_CR4_PAE; vmcb->save.cr4 = cr4; - SVM_TEST_CR_RESERVED_BITS(0, 11, 2, 3, cr3_saved, + SVM_TEST_REG_RESERVED_BITS(0, 11, 2, "CR3", vmcb->save.cr3, cr3_saved, SVM_CR3_LEGACY_RESERVED_MASK); cr4 |= X86_CR4_PAE; vmcb->save.cr4 = cr4; efer |= EFER_LMA; vmcb->save.efer = efer; - SVM_TEST_CR_RESERVED_BITS(0, 63, 2, 3, cr3_saved, + SVM_TEST_REG_RESERVED_BITS(0, 63, 2, "CR3", vmcb->save.cr3, cr3_saved, SVM_CR3_LONG_RESERVED_MASK); vmcb->save.cr4 = cr4_saved; @@ -1919,18 +1911,45 @@ static void svm_guest_state_test(void) efer_saved = vmcb->save.efer; efer &= ~EFER_LMA; vmcb->save.efer = efer; - SVM_TEST_CR_RESERVED_BITS(12, 31, 2, 4, cr4_saved, + SVM_TEST_REG_RESERVED_BITS(12, 31, 2, "CR4", vmcb->save.cr4, cr4_saved, SVM_CR4_LEGACY_RESERVED_MASK); efer |= EFER_LMA; vmcb->save.efer = efer; - SVM_TEST_CR_RESERVED_BITS(12, 31, 2, 4, cr4_saved, + SVM_TEST_REG_RESERVED_BITS(12, 31, 2, "CR4", vmcb->save.cr4, cr4_saved, SVM_CR4_RESERVED_MASK); - SVM_TEST_CR_RESERVED_BITS(32, 63, 4, 4, cr4_saved, + SVM_TEST_REG_RESERVED_BITS(32, 63, 4, "CR4", vmcb->save.cr4, cr4_saved, SVM_CR4_RESERVED_MASK); vmcb->save.cr4 = cr4_saved; vmcb->save.efer = efer_saved; + + /* + * DR6[63:32] and DR7[63:32] are MBZ + */ + u64 dr_saved = vmcb->save.dr6; + + SVM_TEST_REG_RESERVED_BITS(32, 63, 4, "DR6", vmcb->save.dr6, dr_saved, + SVM_DR6_RESERVED_MASK); + vmcb->save.dr6 = dr_saved; + + dr_saved = vmcb->save.dr7; + SVM_TEST_REG_RESERVED_BITS(32, 63, 4, "DR7", vmcb->save.dr7, dr_saved, + SVM_DR7_RESERVED_MASK); + + vmcb->save.dr7 = dr_saved; + + /* + * EFER MBZ bits: 63:16, 9 + */ + efer_saved = vmcb->save.efer; + + SVM_TEST_REG_RESERVED_BITS(8, 9, 1, "EFER", vmcb->save.efer, + efer_saved, SVM_EFER_RESERVED_MASK); + SVM_TEST_REG_RESERVED_BITS(16, 63, 4, "EFER", vmcb->save.efer, + efer_saved, SVM_EFER_RESERVED_MASK); + + vmcb->save.efer = efer_saved; } struct svm_test svm_tests[] = { -- 1.8.3.1
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> --- x86/svm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/x86/svm.c b/x86/svm.c index 41685bf..f984a60 100644 --- a/x86/svm.c +++ b/x86/svm.c @@ -201,7 +201,6 @@ struct regs get_regs(void) #define LOAD_GPR_C SAVE_GPR_C struct svm_test *v2_test; -struct vmcb *vmcb; #define ASM_VMRUN_CMD \ "vmload %%rax\n\t" \ -- 1.8.3.1
On 23/05/20 00:19, Krish Sadhukhan wrote:
> Patch# 1: Moves the check for upper 32 reserved bits of DR6 to a new function.
> Patch# 2: Adds the KVM checks for DR6[63:32] and DR7[64:32] reserved bits
> Patch# 3: Adds kvm-unit-tests for DR6[63:32] and DR7[64:32] reserved bits and
> reserved bits in EFER
> Patch# 4: Removes the duplicate definition of 'vmcb' that sneaked via one of
> my previous patches.
>
>
> [PATCH 1/4] KVM: x86: Move the check for upper 32 reserved bits of
> [PATCH 2/4] KVM: nSVM: Check that DR6[63:32] and DR7[64:32] are not
> [PATCH 3/4] kvm-unit-tests: nSVM: Test that DR6[63:32], DR7[63:32]
> [PATCH 4/4] kvm-unit-tests: x86: Remove duplicate instance of 'vmcb'
>
> arch/x86/kvm/svm/nested.c | 3 +++
> arch/x86/kvm/x86.c | 2 +-
> arch/x86/kvm/x86.h | 5 +++++
> 3 files changed, 9 insertions(+), 1 deletion(-)
>
> Krish Sadhukhan (2):
> KVM: x86: Move the check for upper 32 reserved bits of DR6 to separate fun
> KVM: nVMX: Check that DR6[63:32] and DR7[64:32] are not set on vmrun of ne
> x86/svm.c | 1 -
> x86/svm.h | 3 +++
> x86/svm_tests.c | 59 ++++++++++++++++++++++++++++++++++++++-------------------
> 3 files changed, 42 insertions(+), 21 deletions(-)
>
> Krish Sadhukhan (2):
> kvm-unit-tests: nSVM: Test that DR6[63:32], DR7[63:32] and EFER reserved b
> kvm-unit-tests: x86: Remove duplicate instance of 'vmcb'
>
Queued, thanks.
Paolo
Ping.
On 5/22/20 3:19 PM, Krish Sadhukhan wrote:
> Patch# 1: Moves the check for upper 32 reserved bits of DR6 to a new function.
> Patch# 2: Adds the KVM checks for DR6[63:32] and DR7[64:32] reserved bits
> Patch# 3: Adds kvm-unit-tests for DR6[63:32] and DR7[64:32] reserved bits and
> reserved bits in EFER
> Patch# 4: Removes the duplicate definition of 'vmcb' that sneaked via one of
> my previous patches.
>
>
> [PATCH 1/4] KVM: x86: Move the check for upper 32 reserved bits of
> [PATCH 2/4] KVM: nSVM: Check that DR6[63:32] and DR7[64:32] are not
> [PATCH 3/4] kvm-unit-tests: nSVM: Test that DR6[63:32], DR7[63:32]
> [PATCH 4/4] kvm-unit-tests: x86: Remove duplicate instance of 'vmcb'
>
> arch/x86/kvm/svm/nested.c | 3 +++
> arch/x86/kvm/x86.c | 2 +-
> arch/x86/kvm/x86.h | 5 +++++
> 3 files changed, 9 insertions(+), 1 deletion(-)
>
> Krish Sadhukhan (2):
> KVM: x86: Move the check for upper 32 reserved bits of DR6 to separate fun
> KVM: nVMX: Check that DR6[63:32] and DR7[64:32] are not set on vmrun of ne
> x86/svm.c | 1 -
> x86/svm.h | 3 +++
> x86/svm_tests.c | 59 ++++++++++++++++++++++++++++++++++++++-------------------
> 3 files changed, 42 insertions(+), 21 deletions(-)
>
> Krish Sadhukhan (2):
> kvm-unit-tests: nSVM: Test that DR6[63:32], DR7[63:32] and EFER reserved b
> kvm-unit-tests: x86: Remove duplicate instance of 'vmcb'
>
On 03/07/20 00:33, Krish Sadhukhan wrote:
> Ping.
>
> On 5/22/20 3:19 PM, Krish Sadhukhan wrote:
>> Patch# 1: Moves the check for upper 32 reserved bits of DR6 to a new
>> function.
>> Patch# 2: Adds the KVM checks for DR6[63:32] and DR7[64:32] reserved bits
>> Patch# 3: Adds kvm-unit-tests for DR6[63:32] and DR7[64:32] reserved
>> bits and
>> reserved bits in EFER
>> Patch# 4: Removes the duplicate definition of 'vmcb' that sneaked via
>> one of
>> my previous patches.
>>
>>
>> [PATCH 1/4] KVM: x86: Move the check for upper 32 reserved bits of
>> [PATCH 2/4] KVM: nSVM: Check that DR6[63:32] and DR7[64:32] are not
>> [PATCH 3/4] kvm-unit-tests: nSVM: Test that DR6[63:32], DR7[63:32]
>> [PATCH 4/4] kvm-unit-tests: x86: Remove duplicate instance of 'vmcb'
>>
>> arch/x86/kvm/svm/nested.c | 3 +++
>> arch/x86/kvm/x86.c | 2 +-
>> arch/x86/kvm/x86.h | 5 +++++
>> 3 files changed, 9 insertions(+), 1 deletion(-)
>>
>> Krish Sadhukhan (2):
>> KVM: x86: Move the check for upper 32 reserved bits of DR6 to
>> separate fun
>> KVM: nVMX: Check that DR6[63:32] and DR7[64:32] are not set on
>> vmrun of ne
>> x86/svm.c | 1 -
>> x86/svm.h | 3 +++
>> x86/svm_tests.c | 59
>> ++++++++++++++++++++++++++++++++++++++-------------------
>> 3 files changed, 42 insertions(+), 21 deletions(-)
>>
>> Krish Sadhukhan (2):
>> kvm-unit-tests: nSVM: Test that DR6[63:32], DR7[63:32] and EFER
>> reserved b
>> kvm-unit-tests: x86: Remove duplicate instance of 'vmcb'
>>
>
Queued, thanks.
Paolo