KVM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] KVM: nSVM: Check reserved values for 'Type' and invalid vectors in EVENTINJ
@ 2020-10-17  0:02 Krish Sadhukhan
  2020-10-17  0:02 ` [PATCH 1/2] " Krish Sadhukhan
  2020-10-17  0:02 ` [PATCH 2/2] nSVM: Test " Krish Sadhukhan
  0 siblings, 2 replies; 5+ messages in thread
From: Krish Sadhukhan @ 2020-10-17  0:02 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, jmattson, sean.j.christopherson

According to sections "Canonicalization and Consistency Checks" and "Event
Injection" in APM vol 2,

    VMRUN exits with VMEXIT_INVALID error code if either:
      - Reserved values of TYPE have been specified, or
      - TYPE = 3 (exception) has been specified with a vector that does not
	correspond to an exception (this includes vector 2, which is an NMI,
	not an exception).

Patch# 1 adds these checks to KVM.
Patch# 2 adds tests for these checks.


[PATCH 1/2] KVM: nSVM: Check reserved values for 'Type' and invalid
[PATCH 2/2] nSVM: Test reserved values for 'Type' and invalid vectors in

 arch/x86/include/asm/svm.h |  4 ++++
 arch/x86/kvm/svm/nested.c  | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

Krish Sadhukhan (1):
      KVM: nSVM: Check reserved values for 'Type' and invalid vectors in EVENTINJ

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

Krish Sadhukhan (1):
      nSVM: Test reserved values for 'Type' and invalid vectors in EVENTINJ


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

* [PATCH 1/2] KVM: nSVM: Check reserved values for 'Type' and invalid vectors in EVENTINJ
  2020-10-17  0:02 [PATCH 0/2] KVM: nSVM: Check reserved values for 'Type' and invalid vectors in EVENTINJ Krish Sadhukhan
@ 2020-10-17  0:02 ` Krish Sadhukhan
  2020-10-17  0:02 ` [PATCH 2/2] nSVM: Test " Krish Sadhukhan
  1 sibling, 0 replies; 5+ messages in thread
From: Krish Sadhukhan @ 2020-10-17  0:02 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, jmattson, sean.j.christopherson

According to sections "Canonicalization and Consistency Checks" and "Event
Injection" in APM vol 2

    VMRUN exits with VMEXIT_INVALID error code if either:
      - Reserved values of TYPE have been specified, or
      - TYPE = 3 (exception) has been specified with a vector that does not
	correspond to an exception (this includes vector 2, which is an NMI,
	not an exception).

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

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 8a1f5382a4ea..261240acc7e9 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -272,9 +272,13 @@ struct __attribute__ ((__packed__)) vmcb {
 #define SVM_EVTINJ_TYPE_MASK (7 << SVM_EVTINJ_TYPE_SHIFT)
 
 #define SVM_EVTINJ_TYPE_INTR (0 << SVM_EVTINJ_TYPE_SHIFT)
+#define SVM_EVTINJ_TYPE_RESV1 (1 << SVM_EVTINJ_TYPE_SHIFT)
 #define SVM_EVTINJ_TYPE_NMI (2 << SVM_EVTINJ_TYPE_SHIFT)
 #define SVM_EVTINJ_TYPE_EXEPT (3 << SVM_EVTINJ_TYPE_SHIFT)
 #define SVM_EVTINJ_TYPE_SOFT (4 << SVM_EVTINJ_TYPE_SHIFT)
+#define SVM_EVTINJ_TYPE_RESV5 (5 << SVM_EVTINJ_TYPE_SHIFT)
+#define SVM_EVTINJ_TYPE_RESV6 (6 << SVM_EVTINJ_TYPE_SHIFT)
+#define SVM_EVTINJ_TYPE_RESV7 (7 << SVM_EVTINJ_TYPE_SHIFT)
 
 #define SVM_EVTINJ_VALID (1 << 31)
 #define SVM_EVTINJ_VALID_ERR (1 << 11)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index e90bc436f584..840fbf0582bb 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -202,6 +202,9 @@ static bool nested_svm_vmrun_msrpm(struct vcpu_svm *svm)
 
 static bool nested_vmcb_check_controls(struct vmcb_control_area *control)
 {
+	u32 type, vector;
+	bool valid;
+
 	if ((control->intercept & (1ULL << INTERCEPT_VMRUN)) == 0)
 		return false;
 
@@ -212,6 +215,17 @@ static bool nested_vmcb_check_controls(struct vmcb_control_area *control)
 	    !npt_enabled)
 		return false;
 
+	valid = control->event_inj & SVM_EVTINJ_VALID;
+	type = control->event_inj & SVM_EVTINJ_TYPE_MASK;
+	if (valid && ((type == SVM_EVTINJ_TYPE_RESV1) ||
+	    (type >= SVM_EVTINJ_TYPE_RESV5)))
+		return false;
+
+	vector = control->event_inj & SVM_EVTINJ_VEC_MASK;
+	if (valid && (type == SVM_EVTINJ_TYPE_EXEPT) &&
+	    (vector == NMI_VECTOR || (vector > 31 && vector < 256)))
+		return false;
+
 	return true;
 }
 
-- 
2.18.4


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

* [PATCH 2/2] nSVM: Test reserved values for 'Type' and invalid vectors in EVENTINJ
  2020-10-17  0:02 [PATCH 0/2] KVM: nSVM: Check reserved values for 'Type' and invalid vectors in EVENTINJ Krish Sadhukhan
  2020-10-17  0:02 ` [PATCH 1/2] " Krish Sadhukhan
@ 2020-10-17  0:02 ` Krish Sadhukhan
  2020-10-17  6:12   ` Nadav Amit
  1 sibling, 1 reply; 5+ messages in thread
From: Krish Sadhukhan @ 2020-10-17  0:02 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, jmattson, sean.j.christopherson

According to sections "Canonicalization and Consistency Checks" and "Event
Injection" in APM vol 2

    VMRUN exits with VMEXIT_INVALID error code if either:
      - Reserved values of TYPE have been specified, or
      - TYPE = 3 (exception) has been specified with a vector that does not
	correspond to an exception (this includes vector 2, which is an NMI,
	not an exception).

Existing tests already cover part of the second rule. This patch covers the
the first rule and the missing pieces of the second rule.

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

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index f78c9e4..e6554e4 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -2132,6 +2132,45 @@ static void test_dr(void)
 	vmcb->save.dr7 = dr_saved;
 }
 
+static void test_event_inject(void)
+{
+	u32 i;
+	u32 event_inj_saved = vmcb->control.event_inj;
+
+	handle_exception(DE_VECTOR, my_isr);
+
+	report (svm_vmrun() == SVM_EXIT_VMMCALL && count_exc == 0, "Test "
+	    "No EVENTINJ");
+
+	/*
+	 * Reserved values for 'Type' in EVENTINJ causes VMEXIT_INVALID.
+	 */
+	for (i = 1; i < 8; i++) {
+		if (i != 1 && i < 5)
+			continue;
+		vmcb->control.event_inj = DE_VECTOR |
+		    i << SVM_EVTINJ_TYPE_SHIFT | SVM_EVTINJ_VALID;
+		report(svm_vmrun() == SVM_EXIT_ERR && count_exc == 0,
+		    "Test invalid TYPE (%x) in EVENTINJ", i);
+	}
+
+	/*
+	 * Invalid vector number for event type 'exception' in EVENTINJ
+	 * causes VMEXIT_INVALID.
+	 */
+	i = 32;
+	while (i < 256) {
+		vmcb->control.event_inj = i | SVM_EVTINJ_TYPE_EXEPT |
+		    SVM_EVTINJ_VALID;
+		report(svm_vmrun() == SVM_EXIT_ERR && count_exc == 0,
+		    "Test invalid vector (%u) in EVENTINJ for event type "
+		    "\'exception\'", i);
+		i += 4;
+	}
+
+	vmcb->control.event_inj = event_inj_saved;
+}
+
 static void svm_guest_state_test(void)
 {
 	test_set_guest(basic_guest_main);
@@ -2141,6 +2180,7 @@ static void svm_guest_state_test(void)
 	test_cr3();
 	test_cr4();
 	test_dr();
+	test_event_inject();
 }
 
 struct svm_test svm_tests[] = {
-- 
2.18.4


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

* Re: [PATCH 2/2] nSVM: Test reserved values for 'Type' and invalid vectors in EVENTINJ
  2020-10-17  0:02 ` [PATCH 2/2] nSVM: Test " Krish Sadhukhan
@ 2020-10-17  6:12   ` Nadav Amit
  2020-10-19 17:30     ` Krish Sadhukhan
  0 siblings, 1 reply; 5+ messages in thread
From: Nadav Amit @ 2020-10-17  6:12 UTC (permalink / raw)
  To: Krish Sadhukhan; +Cc: kvm, pbonzini, jmattson, sean.j.christopherson

> On Oct 16, 2020, at 5:02 PM, Krish Sadhukhan <krish.sadhukhan@oracle.com> wrote:
> 
> According to sections "Canonicalization and Consistency Checks" and "Event
> Injection" in APM vol 2
> 
>    VMRUN exits with VMEXIT_INVALID error code if either:
>      - Reserved values of TYPE have been specified, or
>      - TYPE = 3 (exception) has been specified with a vector that does not
> 	correspond to an exception (this includes vector 2, which is an NMI,
> 	not an exception).
> 
> Existing tests already cover part of the second rule. This patch covers the
> the first rule and the missing pieces of the second rule.
> 
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> ---
> x86/svm_tests.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
> 
> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
> index f78c9e4..e6554e4 100644
> --- a/x86/svm_tests.c
> +++ b/x86/svm_tests.c
> @@ -2132,6 +2132,45 @@ static void test_dr(void)
> 	vmcb->save.dr7 = dr_saved;
> }
> 
> +static void test_event_inject(void)
> +{
> +	u32 i;
> +	u32 event_inj_saved = vmcb->control.event_inj;
> +
> +	handle_exception(DE_VECTOR, my_isr);
> +
> +	report (svm_vmrun() == SVM_EXIT_VMMCALL && count_exc == 0, "Test "
> +	    "No EVENTINJ");
> +
> +	/*
> +	 * Reserved values for 'Type' in EVENTINJ causes VMEXIT_INVALID.
> +	 */
> +	for (i = 1; i < 8; i++) {
> +		if (i != 1 && i < 5)
> +			continue;
> +		vmcb->control.event_inj = DE_VECTOR |
> +		    i << SVM_EVTINJ_TYPE_SHIFT | SVM_EVTINJ_VALID;
> +		report(svm_vmrun() == SVM_EXIT_ERR && count_exc == 0,
> +		    "Test invalid TYPE (%x) in EVENTINJ", i);
> +	}
> +
> +	/*
> +	 * Invalid vector number for event type 'exception' in EVENTINJ
> +	 * causes VMEXIT_INVALID.
> +	 */
> +	i = 32;
> +	while (i < 256) {
> +		vmcb->control.event_inj = i | SVM_EVTINJ_TYPE_EXEPT |
> +		    SVM_EVTINJ_VALID;
> +		report(svm_vmrun() == SVM_EXIT_ERR && count_exc == 0,
> +		    "Test invalid vector (%u) in EVENTINJ for event type "
> +		    "\'exception\'", i);
> +		i += 4;
> +	}

I know that kvm-unit-tests has nothing to do with style, but can’t this loop
be turned into a for-loop for readability?

And why "i += 4" ?


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

* Re: [PATCH 2/2] nSVM: Test reserved values for 'Type' and invalid vectors in EVENTINJ
  2020-10-17  6:12   ` Nadav Amit
@ 2020-10-19 17:30     ` Krish Sadhukhan
  0 siblings, 0 replies; 5+ messages in thread
From: Krish Sadhukhan @ 2020-10-19 17:30 UTC (permalink / raw)
  To: Nadav Amit; +Cc: kvm, pbonzini, jmattson, sean.j.christopherson


On 10/16/20 11:12 PM, Nadav Amit wrote:
>> On Oct 16, 2020, at 5:02 PM, Krish Sadhukhan <krish.sadhukhan@oracle.com> wrote:
>>
>> According to sections "Canonicalization and Consistency Checks" and "Event
>> Injection" in APM vol 2
>>
>>     VMRUN exits with VMEXIT_INVALID error code if either:
>>       - Reserved values of TYPE have been specified, or
>>       - TYPE = 3 (exception) has been specified with a vector that does not
>> 	correspond to an exception (this includes vector 2, which is an NMI,
>> 	not an exception).
>>
>> Existing tests already cover part of the second rule. This patch covers the
>> the first rule and the missing pieces of the second rule.
>>
>> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
>> ---
>> x86/svm_tests.c | 40 ++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 40 insertions(+)
>>
>> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
>> index f78c9e4..e6554e4 100644
>> --- a/x86/svm_tests.c
>> +++ b/x86/svm_tests.c
>> @@ -2132,6 +2132,45 @@ static void test_dr(void)
>> 	vmcb->save.dr7 = dr_saved;
>> }
>>
>> +static void test_event_inject(void)
>> +{
>> +	u32 i;
>> +	u32 event_inj_saved = vmcb->control.event_inj;
>> +
>> +	handle_exception(DE_VECTOR, my_isr);
>> +
>> +	report (svm_vmrun() == SVM_EXIT_VMMCALL && count_exc == 0, "Test "
>> +	    "No EVENTINJ");
>> +
>> +	/*
>> +	 * Reserved values for 'Type' in EVENTINJ causes VMEXIT_INVALID.
>> +	 */
>> +	for (i = 1; i < 8; i++) {
>> +		if (i != 1 && i < 5)
>> +			continue;
>> +		vmcb->control.event_inj = DE_VECTOR |
>> +		    i << SVM_EVTINJ_TYPE_SHIFT | SVM_EVTINJ_VALID;
>> +		report(svm_vmrun() == SVM_EXIT_ERR && count_exc == 0,
>> +		    "Test invalid TYPE (%x) in EVENTINJ", i);
>> +	}
>> +
>> +	/*
>> +	 * Invalid vector number for event type 'exception' in EVENTINJ
>> +	 * causes VMEXIT_INVALID.
>> +	 */
>> +	i = 32;
>> +	while (i < 256) {
>> +		vmcb->control.event_inj = i | SVM_EVTINJ_TYPE_EXEPT |
>> +		    SVM_EVTINJ_VALID;
>> +		report(svm_vmrun() == SVM_EXIT_ERR && count_exc == 0,
>> +		    "Test invalid vector (%u) in EVENTINJ for event type "
>> +		    "\'exception\'", i);
>> +		i += 4;
>> +	}
> I know that kvm-unit-tests has nothing to do with style, but can’t this loop
> be turned into a for-loop for readability?


Yes, it's possible in this case.

>
> And why "i += 4" ?
>
Just wanted to limit the number of tests :-)

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-17  0:02 [PATCH 0/2] KVM: nSVM: Check reserved values for 'Type' and invalid vectors in EVENTINJ Krish Sadhukhan
2020-10-17  0:02 ` [PATCH 1/2] " Krish Sadhukhan
2020-10-17  0:02 ` [PATCH 2/2] nSVM: Test " Krish Sadhukhan
2020-10-17  6:12   ` Nadav Amit
2020-10-19 17:30     ` Krish Sadhukhan

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git