All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] KVM: nSVM: Test MBZ bits in nested CR3 (nCR3)
@ 2021-12-07  1:07 Krish Sadhukhan
  2021-12-07  1:08 ` [PATCH 1/2] " Krish Sadhukhan
  2021-12-07  1:08 ` [PATCH kvm-unit-tests 2/2] " Krish Sadhukhan
  0 siblings, 2 replies; 6+ messages in thread
From: Krish Sadhukhan @ 2021-12-07  1:07 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini

According to section "Nested Paging and VMRUN/#VMEXIT" in APM vol 2, the
following guest state is illegal:
    
     "Any MBZ bit of nCR3 is set"

[PATCH 1/2] KVM: nSVM: Test MBZ bits in nested CR3 (nCR3)
[PATCH kvm-unit-tests 2/2] nSVM: Test MBZ bits in nested CR3 (nCR3)

 arch/x86/include/asm/svm.h | 3 +++
 arch/x86/kvm/svm/nested.c  | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

Krish Sadhukhan (1):
      nSVM: Test MBZ bits in nested CR3 (nCR3)

 x86/svm_tests.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

Krish Sadhukhan (1):
      nSVM: Test MBZ bits in nested CR3 (nCR3)



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] KVM: nSVM: Test MBZ bits in nested CR3 (nCR3)
  2021-12-07  1:07 [PATCH 0/2] KVM: nSVM: Test MBZ bits in nested CR3 (nCR3) Krish Sadhukhan
@ 2021-12-07  1:08 ` Krish Sadhukhan
  2021-12-07  4:33   ` Jim Mattson
  2021-12-07  1:08 ` [PATCH kvm-unit-tests 2/2] " Krish Sadhukhan
  1 sibling, 1 reply; 6+ messages in thread
From: Krish Sadhukhan @ 2021-12-07  1:08 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini

According to section "Nested Paging and VMRUN/#VMEXIT" in APM vol 2, the
following guest state is illegal:

	"Any MBZ bit of nCR3 is set"

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
 arch/x86/include/asm/svm.h | 3 +++
 arch/x86/kvm/svm/nested.c  | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index b00dbc5fac2b..a769e3343b07 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -216,9 +216,12 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
 #define SVM_VM_CR_SVM_LOCK_MASK 0x0008ULL
 #define SVM_VM_CR_SVM_DIS_MASK  0x0010ULL
 
+#define SVM_CR3_LONG_MBZ_MASK   0xfff0000000000000U
+
 #define SVM_NESTED_CTL_NP_ENABLE	BIT(0)
 #define SVM_NESTED_CTL_SEV_ENABLE	BIT(1)
 #define SVM_NESTED_CTL_SEV_ES_ENABLE	BIT(2)
+#define SVM_NESTED_CR3_MBZ_MASK        SVM_CR3_LONG_MBZ_MASK
 
 struct vmcb_seg {
 	u16 selector;
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 510b833cbd39..dad479eebae0 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -247,7 +247,8 @@ static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
 	if (CC(control->asid == 0))
 		return false;
 
-	if (CC((control->nested_ctl & SVM_NESTED_CTL_NP_ENABLE) && !npt_enabled))
+	if (CC((control->nested_ctl & SVM_NESTED_CTL_NP_ENABLE) &&
+	    (!npt_enabled || control->nested_cr3 & SVM_CR3_LONG_MBZ_MASK)))
 		return false;
 
 	if (CC(!nested_svm_check_bitmap_pa(vcpu, control->msrpm_base_pa,
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH kvm-unit-tests 2/2] nSVM: Test MBZ bits in nested CR3 (nCR3)
  2021-12-07  1:07 [PATCH 0/2] KVM: nSVM: Test MBZ bits in nested CR3 (nCR3) Krish Sadhukhan
  2021-12-07  1:08 ` [PATCH 1/2] " Krish Sadhukhan
@ 2021-12-07  1:08 ` Krish Sadhukhan
  1 sibling, 0 replies; 6+ messages in thread
From: Krish Sadhukhan @ 2021-12-07  1:08 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini

According to section "Nested Paging and VMRUN/#VMEXIT" in APM vol 2, the
following guest state is illegal:

            "Any MBZ bit of nCR3 is set"

Signed-off-by: Krish Sadhukhan <krish.sadhkhan@oracle.com>
---
 x86/svm_tests.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 8ad6122..ecfb5cb 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -2183,7 +2183,10 @@ static void basic_guest_main(struct svm_test *test)
 			vmcb->save.cr0 = tmp;				\
 			break;						\
 		case 3:							\
-			vmcb->save.cr3 = tmp;				\
+			if (strcmp(test_name, "nested ") == 0)		\
+				vmcb->control.nested_cr3 = tmp;		\
+			else						\
+				vmcb->save.cr3 = tmp;			\
 			break;						\
 		case 4:							\
 			vmcb->save.cr4 = tmp;				\
@@ -2547,6 +2550,41 @@ static void guest_rflags_test_db_handler(struct ex_regs *r)
 	r->rflags &= ~X86_EFLAGS_TF;
 }
 
+static void test_ncr3(void)
+{
+	u64 ncr3_saved = vmcb->control.nested_cr3;
+	u64 nested_ctl_saved = vmcb->control.nested_ctl;
+	u32 ret;
+
+	if (!npt_supported()) {
+		report_skip("NPT not supported");
+		return;
+	}
+
+	vmcb->control.nested_ctl = 0;
+	SVM_TEST_CR_RESERVED_BITS(0, 63, 1, 3, ncr3_saved,
+	    SVM_CR3_LONG_MBZ_MASK, SVM_EXIT_VMMCALL, "nested ");
+
+	vmcb->control.nested_cr3 = ncr3_saved & ~SVM_CR3_LONG_MBZ_MASK;
+	ret = svm_vmrun();
+	report (ret == SVM_EXIT_VMMCALL, "Test CR3 nested 63:0: %lx, wanted "
+	    "exit 0x%x, got 0x%x", ncr3_saved & ~SVM_CR3_LONG_MBZ_MASK,
+	    SVM_EXIT_VMMCALL, ret);
+
+	vmcb->control.nested_ctl = 1;
+	SVM_TEST_CR_RESERVED_BITS(0, 63, 1, 3, ncr3_saved,
+	    SVM_CR3_LONG_MBZ_MASK, SVM_EXIT_ERR, "nested ");
+
+	vmcb->control.nested_cr3 = ncr3_saved & ~SVM_CR3_LONG_MBZ_MASK;
+	ret = svm_vmrun();
+	report (ret == SVM_EXIT_VMMCALL, "Test CR3 nested 63:0: %lx, wanted "
+	    "exit 0x%x, got 0x%x", ncr3_saved & ~SVM_CR3_LONG_MBZ_MASK,
+	    SVM_EXIT_VMMCALL, ret);
+
+	vmcb->control.nested_cr3 = ncr3_saved;
+	vmcb->control.nested_ctl = nested_ctl_saved;
+}
+
 static void svm_guest_state_test(void)
 {
 	test_set_guest(basic_guest_main);
@@ -2557,6 +2595,7 @@ static void svm_guest_state_test(void)
 	test_dr();
 	test_msrpm_iopm_bitmap_addrs();
 	test_canonicalization();
+	test_ncr3();
 }
 
 extern void guest_rflags_test_guest(struct svm_test *test);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] KVM: nSVM: Test MBZ bits in nested CR3 (nCR3)
  2021-12-07  1:08 ` [PATCH 1/2] " Krish Sadhukhan
@ 2021-12-07  4:33   ` Jim Mattson
  2021-12-07 20:08     ` Krish Sadhukhan
  0 siblings, 1 reply; 6+ messages in thread
From: Jim Mattson @ 2021-12-07  4:33 UTC (permalink / raw)
  To: Krish Sadhukhan; +Cc: kvm, pbonzini

On Mon, Dec 6, 2021 at 6:03 PM Krish Sadhukhan
<krish.sadhukhan@oracle.com> wrote:
>
> According to section "Nested Paging and VMRUN/#VMEXIT" in APM vol 2, the
> following guest state is illegal:
>
>         "Any MBZ bit of nCR3 is set"
>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> ---
>  arch/x86/include/asm/svm.h | 3 +++
>  arch/x86/kvm/svm/nested.c  | 3 ++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index b00dbc5fac2b..a769e3343b07 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -216,9 +216,12 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
>  #define SVM_VM_CR_SVM_LOCK_MASK 0x0008ULL
>  #define SVM_VM_CR_SVM_DIS_MASK  0x0010ULL
>
> +#define SVM_CR3_LONG_MBZ_MASK   0xfff0000000000000U
> +
>  #define SVM_NESTED_CTL_NP_ENABLE       BIT(0)
>  #define SVM_NESTED_CTL_SEV_ENABLE      BIT(1)
>  #define SVM_NESTED_CTL_SEV_ES_ENABLE   BIT(2)
> +#define SVM_NESTED_CR3_MBZ_MASK        SVM_CR3_LONG_MBZ_MASK

A fixed mask isn't sufficient. According to the APM, "All CR3 bits are
writable, except for unimplemented physical address bits, which must
be cleared to 0." In this context, that means that the MBZ bits for L1
are all bits above L1's physical address width, given by
CPUID.80000008H:EAX[7:0] (or 36, if this CPUID leaf doesn't exist).

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] KVM: nSVM: Test MBZ bits in nested CR3 (nCR3)
  2021-12-07  4:33   ` Jim Mattson
@ 2021-12-07 20:08     ` Krish Sadhukhan
  2021-12-07 23:38       ` Jim Mattson
  0 siblings, 1 reply; 6+ messages in thread
From: Krish Sadhukhan @ 2021-12-07 20:08 UTC (permalink / raw)
  To: Jim Mattson; +Cc: kvm, pbonzini


On 12/6/21 8:33 PM, Jim Mattson wrote:
> On Mon, Dec 6, 2021 at 6:03 PM Krish Sadhukhan
> <krish.sadhukhan@oracle.com> wrote:
>> According to section "Nested Paging and VMRUN/#VMEXIT" in APM vol 2, the
>> following guest state is illegal:
>>
>>          "Any MBZ bit of nCR3 is set"
>>
>> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
>> ---
>>   arch/x86/include/asm/svm.h | 3 +++
>>   arch/x86/kvm/svm/nested.c  | 3 ++-
>>   2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
>> index b00dbc5fac2b..a769e3343b07 100644
>> --- a/arch/x86/include/asm/svm.h
>> +++ b/arch/x86/include/asm/svm.h
>> @@ -216,9 +216,12 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
>>   #define SVM_VM_CR_SVM_LOCK_MASK 0x0008ULL
>>   #define SVM_VM_CR_SVM_DIS_MASK  0x0010ULL
>>
>> +#define SVM_CR3_LONG_MBZ_MASK   0xfff0000000000000U
>> +
>>   #define SVM_NESTED_CTL_NP_ENABLE       BIT(0)
>>   #define SVM_NESTED_CTL_SEV_ENABLE      BIT(1)
>>   #define SVM_NESTED_CTL_SEV_ES_ENABLE   BIT(2)
>> +#define SVM_NESTED_CR3_MBZ_MASK        SVM_CR3_LONG_MBZ_MASK
> A fixed mask isn't sufficient. According to the APM, "All CR3 bits are
> writable, except for unimplemented physical address bits, which must
> be cleared to 0." In this context, that means that the MBZ bits for L1
> are all bits above L1's physical address width, given by
> CPUID.80000008H:EAX[7:0] (or 36, if this CPUID leaf doesn't exist).
OK. If the processor's physical address width determines the MBZ mask, 
should we also fix the existing test_cr3() in kvm-unit-tests ? That one 
also uses the same fixed mask.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] KVM: nSVM: Test MBZ bits in nested CR3 (nCR3)
  2021-12-07 20:08     ` Krish Sadhukhan
@ 2021-12-07 23:38       ` Jim Mattson
  0 siblings, 0 replies; 6+ messages in thread
From: Jim Mattson @ 2021-12-07 23:38 UTC (permalink / raw)
  To: Krish Sadhukhan; +Cc: kvm, pbonzini

On Tue, Dec 7, 2021 at 12:09 PM Krish Sadhukhan
<krish.sadhukhan@oracle.com> wrote:

> OK. If the processor's physical address width determines the MBZ mask,
> should we also fix the existing test_cr3() in kvm-unit-tests ? That one
> also uses the same fixed mask.

Yep.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-12-07 23:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-07  1:07 [PATCH 0/2] KVM: nSVM: Test MBZ bits in nested CR3 (nCR3) Krish Sadhukhan
2021-12-07  1:08 ` [PATCH 1/2] " Krish Sadhukhan
2021-12-07  4:33   ` Jim Mattson
2021-12-07 20:08     ` Krish Sadhukhan
2021-12-07 23:38       ` Jim Mattson
2021-12-07  1:08 ` [PATCH kvm-unit-tests 2/2] " Krish Sadhukhan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.