All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dong, Eddie" <eddie.dong@intel.com>
To: Gleb Natapov <gleb@redhat.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Avi Kivity <avi@redhat.com>, "Dong, Eddie" <eddie.dong@intel.com>
Subject: RE: Implement generic double fault generation mechanism
Date: Fri, 8 May 2009 16:27:28 +0800	[thread overview]
Message-ID: <9832F13BD22FB94A829F798DA4A8280501A81A8E83@pdsmsx503.ccr.corp.intel.com> (raw)
In-Reply-To: <20090503105330.GL9795@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 5012 bytes --]

Gleb Natapov wrote:
>> +
>> +static int exception_class(int vector)
>> +{
>> +	if (vector == 14)
>> +		return EXCPT_PF;
>> +	else if (vector == 0 || (vector >= 10 && vector <= 13)) +		return
>> EXCPT_CONTRIBUTORY; +	else
>> +		return EXCPT_BENIGN;
>> +}
>> +
> This makes double fault (8) benign exception. Surely not what you
> want. 

double fault fall into neither of above class per SDM. But it should be 
checked earlier than generating DB fault. See new updated.
>> +	/* to check exception */
>> +	prev_nr = vcpu->arch.exception.nr;
>> +	class2 = exception_class(nr);
>> +	class1 = exception_class(prev_nr);
>> +	if ((class1 == EXCPT_CONTRIBUTORY && class2 == EXCPT_CONTRIBUTORY)
>> +		|| (class1 == EXCPT_PF && class2 != EXCPT_BENIGN)) {
>> +		/* generate double fault per SDM Table 5-5 */
>> +		printk(KERN_DEBUG "kvm: double fault 0x%x on 0x%x\n",
>> +			prev_nr, nr); +		vcpu->arch.exception.pending = true;
>> +		vcpu->arch.exception.has_error_code = 1;
>> +		vcpu->arch.exception.nr = DF_VECTOR;
>> +		vcpu->arch.exception.error_code = 0;
>> +		if (prev_nr == DF_VECTOR) {
>> +			/* triple fault -> shutdown */
>> +			set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); +		}
>> +	} else
>> +		printk(KERN_ERR "Exception 0x%x on 0x%x happens serially\n",
>> +			prev_nr, nr); +}
> When two exceptions happens serially is is better to replace pending
> exception with a new one. This way the first exception (that is lost)
> will be regenerated when instruction will be re-executed.

Do you want it to be covered for now? For exception, it is easy but for IRQ, it needs to be pushed back.

Thx, eddie



    Move Double-Fault generation logic out of page fault
    exception generating function to cover more generic case.
    
    Signed-off-by: Eddie Dong <eddie.dong@intel.com>

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ab1fdac..d0e75a2 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -162,12 +162,60 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data)
 }
 EXPORT_SYMBOL_GPL(kvm_set_apic_base);
 
+#define EXCPT_BENIGN		0
+#define EXCPT_CONTRIBUTORY	1
+#define EXCPT_PF		2
+
+static int exception_class(int vector)
+{
+	if (vector == 14)
+		return EXCPT_PF;
+	else if (vector == 0 || (vector >= 10 && vector <= 13))
+		return EXCPT_CONTRIBUTORY;
+	else
+		return EXCPT_BENIGN;
+}
+
+static void kvm_multiple_exception(struct kvm_vcpu *vcpu,
+		unsigned nr, bool has_error, u32 error_code)
+{
+	u32 prev_nr;
+	int class1, class2;
+
+	if (!vcpu->arch.exception.pending) {
+		vcpu->arch.exception.pending = true;
+		vcpu->arch.exception.has_error_code = has_error;
+		vcpu->arch.exception.nr = nr;
+		vcpu->arch.exception.error_code = error_code;
+		return;
+	}
+
+	/* to check exception */
+	prev_nr = vcpu->arch.exception.nr;
+	if (prev_nr == DF_VECTOR) {
+		/* triple fault -> shutdown */
+		set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests);
+		return;
+	}
+	class1 = exception_class(prev_nr);
+	class2 = exception_class(nr);
+	if ((class1 == EXCPT_CONTRIBUTORY && class2 == EXCPT_CONTRIBUTORY)
+		|| (class1 == EXCPT_PF && class2 != EXCPT_BENIGN)) {
+		/* generate double fault per SDM Table 5-5 */
+		printk(KERN_DEBUG "kvm: double fault 0x%x on 0x%x\n",
+			prev_nr, nr);
+		vcpu->arch.exception.pending = true;
+		vcpu->arch.exception.has_error_code = true;
+		vcpu->arch.exception.nr = DF_VECTOR;
+		vcpu->arch.exception.error_code = 0;
+	} else
+		printk(KERN_ERR "Exception 0x%x on 0x%x happens serially\n",
+			prev_nr, nr);
+}
+
 void kvm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr)
 {
-	WARN_ON(vcpu->arch.exception.pending);
-	vcpu->arch.exception.pending = true;
-	vcpu->arch.exception.has_error_code = false;
-	vcpu->arch.exception.nr = nr;
+	kvm_multiple_exception(vcpu, nr, false, 0);
 }
 EXPORT_SYMBOL_GPL(kvm_queue_exception);
 
@@ -176,18 +224,6 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr,
 {
 	++vcpu->stat.pf_guest;
 
-	if (vcpu->arch.exception.pending) {
-		if (vcpu->arch.exception.nr == PF_VECTOR) {
-			printk(KERN_DEBUG "kvm: inject_page_fault:"
-					" double fault 0x%lx\n", addr);
-			vcpu->arch.exception.nr = DF_VECTOR;
-			vcpu->arch.exception.error_code = 0;
-		} else if (vcpu->arch.exception.nr == DF_VECTOR) {
-			/* triple fault -> shutdown */
-			set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests);
-		}
-		return;
-	}
 	vcpu->arch.cr2 = addr;
 	kvm_queue_exception_e(vcpu, PF_VECTOR, error_code);
 }
@@ -200,11 +236,7 @@ EXPORT_SYMBOL_GPL(kvm_inject_nmi);
 
 void kvm_queue_exception_e(struct kvm_vcpu *vcpu, unsigned nr, u32 error_code)
 {
-	WARN_ON(vcpu->arch.exception.pending);
-	vcpu->arch.exception.pending = true;
-	vcpu->arch.exception.has_error_code = true;
-	vcpu->arch.exception.nr = nr;
-	vcpu->arch.exception.error_code = error_code;
+	kvm_multiple_exception(vcpu, nr, true, error_code);
 }
 EXPORT_SYMBOL_GPL(kvm_queue_exception_e);
 

[-- Attachment #2: irq2.patch --]
[-- Type: application/octet-stream, Size: 3255 bytes --]


    Move Double-Fault generation logic out of page fault
    exception generating function to cover more generic case.
    
    Signed-off-by: Eddie Dong <eddie.dong@intel.com>

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ab1fdac..d0e75a2 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -162,12 +162,60 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data)
 }
 EXPORT_SYMBOL_GPL(kvm_set_apic_base);
 
+#define EXCPT_BENIGN		0
+#define EXCPT_CONTRIBUTORY	1
+#define EXCPT_PF		2
+
+static int exception_class(int vector)
+{
+	if (vector == 14)
+		return EXCPT_PF;
+	else if (vector == 0 || (vector >= 10 && vector <= 13))
+		return EXCPT_CONTRIBUTORY;
+	else
+		return EXCPT_BENIGN;
+}
+
+static void kvm_multiple_exception(struct kvm_vcpu *vcpu,
+		unsigned nr, bool has_error, u32 error_code)
+{
+	u32 prev_nr;
+	int class1, class2;
+
+	if (!vcpu->arch.exception.pending) {
+		vcpu->arch.exception.pending = true;
+		vcpu->arch.exception.has_error_code = has_error;
+		vcpu->arch.exception.nr = nr;
+		vcpu->arch.exception.error_code = error_code;
+		return;
+	}
+
+	/* to check exception */
+	prev_nr = vcpu->arch.exception.nr;
+	if (prev_nr == DF_VECTOR) {
+		/* triple fault -> shutdown */
+		set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests);
+		return;
+	}
+	class1 = exception_class(prev_nr);
+	class2 = exception_class(nr);
+	if ((class1 == EXCPT_CONTRIBUTORY && class2 == EXCPT_CONTRIBUTORY)
+		|| (class1 == EXCPT_PF && class2 != EXCPT_BENIGN)) {
+		/* generate double fault per SDM Table 5-5 */
+		printk(KERN_DEBUG "kvm: double fault 0x%x on 0x%x\n",
+			prev_nr, nr);
+		vcpu->arch.exception.pending = true;
+		vcpu->arch.exception.has_error_code = true;
+		vcpu->arch.exception.nr = DF_VECTOR;
+		vcpu->arch.exception.error_code = 0;
+	} else
+		printk(KERN_ERR "Exception 0x%x on 0x%x happens serially\n",
+			prev_nr, nr);
+}
+
 void kvm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr)
 {
-	WARN_ON(vcpu->arch.exception.pending);
-	vcpu->arch.exception.pending = true;
-	vcpu->arch.exception.has_error_code = false;
-	vcpu->arch.exception.nr = nr;
+	kvm_multiple_exception(vcpu, nr, false, 0);
 }
 EXPORT_SYMBOL_GPL(kvm_queue_exception);
 
@@ -176,18 +224,6 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr,
 {
 	++vcpu->stat.pf_guest;
 
-	if (vcpu->arch.exception.pending) {
-		if (vcpu->arch.exception.nr == PF_VECTOR) {
-			printk(KERN_DEBUG "kvm: inject_page_fault:"
-					" double fault 0x%lx\n", addr);
-			vcpu->arch.exception.nr = DF_VECTOR;
-			vcpu->arch.exception.error_code = 0;
-		} else if (vcpu->arch.exception.nr == DF_VECTOR) {
-			/* triple fault -> shutdown */
-			set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests);
-		}
-		return;
-	}
 	vcpu->arch.cr2 = addr;
 	kvm_queue_exception_e(vcpu, PF_VECTOR, error_code);
 }
@@ -200,11 +236,7 @@ EXPORT_SYMBOL_GPL(kvm_inject_nmi);
 
 void kvm_queue_exception_e(struct kvm_vcpu *vcpu, unsigned nr, u32 error_code)
 {
-	WARN_ON(vcpu->arch.exception.pending);
-	vcpu->arch.exception.pending = true;
-	vcpu->arch.exception.has_error_code = true;
-	vcpu->arch.exception.nr = nr;
-	vcpu->arch.exception.error_code = error_code;
+	kvm_multiple_exception(vcpu, nr, true, error_code);
 }
 EXPORT_SYMBOL_GPL(kvm_queue_exception_e);
 

  reply	other threads:[~2009-05-08  8:29 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-30  7:24 Implement generic double fault generation mechanism Dong, Eddie
2009-05-03 10:53 ` Gleb Natapov
2009-05-08  8:27   ` Dong, Eddie [this message]
2009-05-08  9:53     ` Gleb Natapov
2009-05-08 10:39       ` Dong, Eddie
2009-05-08 10:46         ` Dong, Eddie
2009-05-08 12:23           ` Gleb Natapov
2009-05-08 15:00             ` Dong, Eddie
2009-05-08 18:44               ` Gleb Natapov
2009-05-11  1:04                 ` Dong, Eddie
2009-05-11  6:02                   ` Gleb Natapov
2009-05-12  5:35                     ` Dong, Eddie
2009-05-12  7:01                       ` Gleb Natapov
2009-05-12 15:06                         ` Enable IRQ windows after exception injection if there are pending virq Dong, Eddie
2009-05-12 15:27                           ` Gleb Natapov
2009-05-13  7:45                             ` Dong, Eddie
2009-05-13 10:29                               ` Gleb Natapov
2009-05-13 14:05                         ` Implement generic double fault generation mechanism Dong, Eddie
2009-05-11  6:17                   ` Avi Kivity
2009-05-12  7:38                     ` event injection MACROs Dong, Eddie
2009-05-12  8:49                       ` Gleb Natapov
2009-05-13  9:49                       ` Avi Kivity
2009-05-13 14:20                         ` Dong, Eddie
2009-05-14  9:27                           ` Avi Kivity
2009-05-14 13:43                             ` Dong, Eddie
2009-05-14 14:16                               ` Gleb Natapov
2009-05-14 14:34                                 ` Dong, Eddie
2009-05-14 15:44                                   ` Gleb Natapov
2009-05-15  7:57                                     ` Dong, Eddie
2009-05-17  9:44                                       ` Gleb Natapov
2009-05-08 12:16         ` Implement generic double fault generation mechanism Gleb Natapov
2009-05-08  8:19 ` Dong, Eddie
2009-05-08  8:28   ` Avi Kivity

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9832F13BD22FB94A829F798DA4A8280501A81A8E83@pdsmsx503.ccr.corp.intel.com \
    --to=eddie.dong@intel.com \
    --cc=avi@redhat.com \
    --cc=gleb@redhat.com \
    --cc=kvm@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.