Re: [PATCH 4/10] Trivial: Make decode_register() static

Re: [PATCH 4/10] Trivial: Make decode_register() static

[Gregory Haskins]

On Tue, 2007-07-17 at 23:16 +1000, Rusty Russell wrote:
> I have shied away from touching x86_emulate.c (it could definitely use
> some love, but it is forked from the Xen code, and it would be more
> productive to cross-merge fixes).

On this topic, here's an idea I have been kicking around for a while:

If the x86_emulate code is so buggy/incomplete, and the QEMU one seems
to be able to generally handle most situations...could we simply exit to
userspace and use the qemu emulator somehow?  I realize the overhead is
greater, but slow+working is > fast+broken in my book ;)

Perhaps a hybrid solution would work?  E.g. exit to qemu emulator when
the in-kernel stuff hits a mis-emulation point (do we realize this
consciously in the code, or only after the guest crashes?) 

I'm not really sure if this is plausible.  Its just something I was
thinking about.

Regards,
-Greg


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH 4/10] Trivial: Make decode_register() static

[Dor Laor]

>> I have shied away from touching x86_emulate.c (it could definitely
use
>> some love, but it is forked from the Xen code, and it would be more
>> productive to cross-merge fixes).
>
>On this topic, here's an idea I have been kicking around for a while:
>
>If the x86_emulate code is so buggy/incomplete, and the QEMU one seems
>to be able to generally handle most situations...could we simply exit
to
>userspace and use the qemu emulator somehow?  I realize the overhead is
>greater, but slow+working is > fast+broken in my book ;)
>
>Perhaps a hybrid solution would work?  E.g. exit to qemu emulator when
>the in-kernel stuff hits a mis-emulation point (do we realize this
>consciously in the code, or only after the guest crashes?)
>
>I'm not really sure if this is plausible.  Its just something I was
>thinking about.

Actually this is how the first generation of KVM worked.
Since Avi & Yaniv had bad experience with serializing the cpu state with
qemu 
they droped this direction. 
Basically it could have been nice, Xen though of doing V2E migrations
(virtual to emulate).
I don't know if they implemented that.

Anyway Nitin A Kamble works for finishing the in-kernel emulator.


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH 4/10] Trivial: Make decode_register() static

[Anthony Liguori]

Gregory Haskins wrote:
> On Tue, 2007-07-17 at 23:16 +1000, Rusty Russell wrote:
>   
>> I have shied away from touching x86_emulate.c (it could definitely use
>> some love, but it is forked from the Xen code, and it would be more
>> productive to cross-merge fixes).
>>     
>
> On this topic, here's an idea I have been kicking around for a while:
>
> If the x86_emulate code is so buggy/incomplete, and the QEMU one seems
> to be able to generally handle most situations...could we simply exit to
> userspace and use the qemu emulator somehow?  I realize the overhead is
> greater, but slow+working is > fast+broken in my book ;)
>
> Perhaps a hybrid solution would work?  E.g. exit to qemu emulator when
> the in-kernel stuff hits a mis-emulation point (do we realize this
> consciously in the code, or only after the guest crashes?) 
>   

SMP is really tricky in this environment.  The code that QEMU generates 
doesn't guarantee atomicity of instructions so if you have one CPU 
running in QEMU and another running on bare metal and they both were 
attempting to access a spin lock things would break down pretty quickly.

Regards,

Anthony Liguori

> I'm not really sure if this is plausible.  Its just something I was
> thinking about.
>
> Regards,
> -Greg
>
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by DB2 Express
> Download DB2 Express C - the FREE version of DB2 express and take
> control of your XML. No limits. Just data. Click to get it now.
> http://sourceforge.net/powerbar/db2/
> _______________________________________________
> kvm-devel mailing list
> kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: Trivial: Make decode_register() static

[Mike Day]

On 17/07/07 08:41 -0500, Anthony Liguori wrote:
>Gregory Haskins wrote:
>> On Tue, 2007-07-17 at 23:16 +1000, Rusty Russell wrote:
>>   
>>> I have shied away from touching x86_emulate.c (it could definitely use
>>> some love, but it is forked from the Xen code, and it would be more
>>> productive to cross-merge fixes).
>>>     
>>
>> On this topic, here's an idea I have been kicking around for a while:
>>
>> If the x86_emulate code is so buggy/incomplete, and the QEMU one seems
>> to be able to generally handle most situations...could we simply exit to
>> userspace and use the qemu emulator somehow?  I realize the overhead is
>> greater, but slow+working is > fast+broken in my book ;)
>>
>> Perhaps a hybrid solution would work?  E.g. exit to qemu emulator when
>> the in-kernel stuff hits a mis-emulation point (do we realize this
>> consciously in the code, or only after the guest crashes?) 
>>   
>
>SMP is really tricky in this environment.  The code that QEMU generates 
>doesn't guarantee atomicity of instructions so if you have one CPU 
>running in QEMU and another running on bare metal and they both were 
>attempting to access a spin lock things would break down pretty quickly.

Another possibility is pulling x86_emulate into its own library that
is shared by xen, kvm, and available for use by any other project. making
it into an emulation library.

Mike

-- 
Mike Day
http://www.ncultra.org
AIM: ncmikeday |  Yahoo IM: ultra.runner
PGP key: http://www.ncultra.org/ncmike/pubkey.asc

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH 4/10] Trivial: Make decode_register() static

[Avi Kivity]

Gregory Haskins wrote:
> On Tue, 2007-07-17 at 23:16 +1000, Rusty Russell wrote:
>   
>> I have shied away from touching x86_emulate.c (it could definitely use
>> some love, but it is forked from the Xen code, and it would be more
>> productive to cross-merge fixes).
>>     
>
> On this topic, here's an idea I have been kicking around for a while:
>
> If the x86_emulate code is so buggy/incomplete, and the QEMU one seems
> to be able to generally handle most situations...could we simply exit to
> userspace and use the qemu emulator somehow?  I realize the overhead is
> greater, but slow+working is > fast+broken in my book ;)
>
> Perhaps a hybrid solution would work?  E.g. exit to qemu emulator when
> the in-kernel stuff hits a mis-emulation point (do we realize this
> consciously in the code, or only after the guest crashes?) 
>
> I'm not really sure if this is plausible.  Its just something I was
> thinking about.
>
>   
It was once done that way (we started by having qemu emulate 32- and 16- 
bit code, then just 16-, then nothing).  However, I'd like kvm to be 
independent of an external emulator, so that all cpu emulation is done 
in kernel.

Of course we have to have an in-kernel emulator, for page tables and for 
in-kernel mmio, so having a partial emulator in kernel and a full 
emulator in userspace seems messy.


-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[PATCH 1/10] Trivial: /dev/kvm interface is no longer experimental.

[Rusty Russell]

KVM interface is no longer experimental.

Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>

diff -r 4e57f5c6d4a9 include/linux/kvm.h
--- a/include/linux/kvm.h	Tue Jul 17 13:04:58 2007 +1000
+++ b/include/linux/kvm.h	Tue Jul 17 13:09:54 2007 +1000
@@ -4,8 +4,7 @@
 /*
  * Userspace interface for /dev/kvm - kernel based virtual machine
  *
- * Note: this interface is considered experimental and may change without
- *       notice.
+ * Note: you must update KVM_API_VERSION if you change this interface.
  */
 
 #include <asm/types.h>



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[PATCH 2/10] Trivial: Remove KVM_IRQ_BITMAP macro

[Rusty Russell]

Creating one's own BITMAP macro seems suboptimal: if we use manual
arithmetic in the one place exposed to userspace, we can use standard
macros elsewhere.

The - 7 + 8 calc is overkill: can NR_IRQ_WORDS ever really change?

Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>

diff -r 85a8c51d3df8 drivers/kvm/kvm.h
--- a/drivers/kvm/kvm.h	Tue Jul 17 13:10:00 2007 +1000
+++ b/drivers/kvm/kvm.h	Tue Jul 17 13:15:22 2007 +1000
@@ -342,8 +342,7 @@ struct kvm_vcpu {
 	int guest_mode;
 	unsigned long requests;
 	unsigned long irq_summary; /* bit vector: 1 per word in irq_pending */
-#define NR_IRQ_WORDS KVM_IRQ_BITMAP_SIZE(unsigned long)
-	unsigned long irq_pending[NR_IRQ_WORDS];
+	DECLARE_BITMAP(irq_pending, KVM_NR_INTERRUPTS);
 	unsigned long regs[NR_VCPU_REGS]; /* for rsp: vcpu_load_rsp_rip() */
 	unsigned long rip;      /* needs vcpu_load_rsp_rip() */
 
diff -r 85a8c51d3df8 drivers/kvm/kvm_main.c
--- a/drivers/kvm/kvm_main.c	Tue Jul 17 13:10:00 2007 +1000
+++ b/drivers/kvm/kvm_main.c	Tue Jul 17 16:26:10 2007 +1000
@@ -2122,7 +2122,7 @@ static int kvm_vcpu_ioctl_set_sregs(stru
 	memcpy(vcpu->irq_pending, sregs->interrupt_bitmap,
 	       sizeof vcpu->irq_pending);
 	vcpu->irq_summary = 0;
-	for (i = 0; i < NR_IRQ_WORDS; ++i)
+	for (i = 0; i < ARRAY_SIZE(vcpu->irq_pending); ++i)
 		if (vcpu->irq_pending[i])
 			__set_bit(i, &vcpu->irq_summary);
 
diff -r 85a8c51d3df8 include/linux/kvm.h
--- a/include/linux/kvm.h	Tue Jul 17 13:10:00 2007 +1000
+++ b/include/linux/kvm.h	Tue Jul 17 13:25:48 2007 +1000
@@ -12,14 +12,8 @@
 
 #define KVM_API_VERSION 12
 
-/*
- * Architectural interrupt line count, and the size of the bitmap needed
- * to hold them.
- */
+/* Architectural interrupt line count. */
 #define KVM_NR_INTERRUPTS 256
-#define KVM_IRQ_BITMAP_SIZE_BYTES    ((KVM_NR_INTERRUPTS + 7) / 8)
-#define KVM_IRQ_BITMAP_SIZE(type)    (KVM_IRQ_BITMAP_SIZE_BYTES / sizeof(type))
-
 
 /* for KVM_CREATE_MEMORY_REGION */
 struct kvm_memory_region {
@@ -163,7 +157,7 @@ struct kvm_sregs {
 	__u64 cr0, cr2, cr3, cr4, cr8;
 	__u64 efer;
 	__u64 apic_base;
-	__u64 interrupt_bitmap[KVM_IRQ_BITMAP_SIZE(__u64)];
+	__u64 interrupt_bitmap[(KVM_NR_INTERRUPTS + 7) / 8];
 };
 
 struct kvm_msr_entry {



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[PATCH 3/10] Trivial: Remove unused struct cpu_user_regs declaration

[Rusty Russell]

Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>

diff -r 725f52d66cc0 drivers/kvm/x86_emulate.h
--- a/drivers/kvm/x86_emulate.h	Tue Jul 17 13:25:52 2007 +1000
+++ b/drivers/kvm/x86_emulate.h	Tue Jul 17 13:33:34 2007 +1000
@@ -112,8 +112,6 @@ struct x86_emulate_ops {
 
 };
 
-struct cpu_user_regs;
-
 struct x86_emulate_ctxt {
 	/* Register state before/after emulation. */
 	struct kvm_vcpu *vcpu;



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[PATCH 4/10] Trivial: Make decode_register() static

[Rusty Russell]

I have shied away from touching x86_emulate.c (it could definitely use
some love, but it is forked from the Xen code, and it would be more
productive to cross-merge fixes).

Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>

diff -r c7348ac9677f drivers/kvm/x86_emulate.c
--- a/drivers/kvm/x86_emulate.c	Tue Jul 17 13:33:39 2007 +1000
+++ b/drivers/kvm/x86_emulate.c	Tue Jul 17 13:35:19 2007 +1000
@@ -443,8 +443,13 @@ struct operand {
 			   (((reg) + _inc) & ((1UL << (ad_bytes << 3)) - 1)); \
 	} while (0)
 
-void *decode_register(u8 modrm_reg, unsigned long *regs,
-		      int highbyte_regs)
+/*
+ * Given the 'reg' portion of a ModRM byte, and a register block, return a
+ * pointer into the block that addresses the relevant register.
+ * @highbyte_regs specifies whether to decode AH,CH,DH,BH.
+ */
+static void *decode_register(u8 modrm_reg, unsigned long *regs,
+			     int highbyte_regs)
 {
 	void *p;
 
diff -r c7348ac9677f drivers/kvm/x86_emulate.h
--- a/drivers/kvm/x86_emulate.h	Tue Jul 17 13:33:39 2007 +1000
+++ b/drivers/kvm/x86_emulate.h	Tue Jul 17 13:35:06 2007 +1000
@@ -152,12 +152,4 @@ int x86_emulate_memop(struct x86_emulate
 int x86_emulate_memop(struct x86_emulate_ctxt *ctxt,
 		      struct x86_emulate_ops *ops);
 
-/*
- * Given the 'reg' portion of a ModRM byte, and a register block, return a
- * pointer into the block that addresses the relevant register.
- * @highbyte_regs specifies whether to decode AH,CH,DH,BH.
- */
-void *decode_register(u8 modrm_reg, unsigned long *regs,
-		      int highbyte_regs);
-
 #endif				/* __X86_EMULATE_H__ */



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH 4/10] Trivial: Make decode_register() static

[Avi Kivity]

Rusty Russell wrote:
> I have shied away from touching x86_emulate.c (it could definitely use
> some love, but it is forked from the Xen code, and it would be more
> productive to cross-merge fixes).
>
>   

Applied, thanks.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/