[PATCH 0/3] KVM paravirt_ops implementation

[PATCH 0/3] KVM paravirt_ops implementation

[Anthony Liguori]

This is the start of a paravirt_ops implementation for KVM.  Most of it 
was done by Ingo Molnar, I just moved things around a bit.  I don't 
think there's a measurable performance benefit just yet but there are a 
few more optimizations that I think we can get in time for 2.6.23 that 
will be measurable.

Regards,

Anthony Liguori

[PATCH 1/3] KVM paravirt_ops infrastructure

[Anthony Liguori]

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

Regards,

Anthony Liguori

[-- Attachment #2: kvm-paravirt-guest.diff --]
[-- Type: text/x-patch, Size: 4616 bytes --]

Subject: [PATCH] Add KVM paravirt_ops implementation
From: Anthony Liguori <aliguori@us.ibm.com>

This patch adds the basic infrastructure for paravirtualizing a KVM guest.
Discovery of running under KVM is done by sharing a page of memory between
the guest and host (initially through an MSR write).

This is based on a patch written by Ingo Molnar.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Index: kvm/arch/i386/Kconfig
===================================================================
--- kvm.orig/arch/i386/Kconfig	2007-05-30 08:42:31.000000000 -0500
+++ kvm/arch/i386/Kconfig	2007-05-30 08:42:38.000000000 -0500
@@ -231,6 +231,13 @@
 	  at the moment), by linking the kernel to a GPL-ed ROM module
 	  provided by the hypervisor.
 
+config KVM_GUEST
+	bool "KVM paravirt-ops support"
+	depends on PARAVIRT
+	help
+	  This option enables various optimizations for running under the KVM
+          hypervisor.
+
 config ACPI_SRAT
 	bool
 	default y
Index: kvm/arch/i386/kernel/Makefile
===================================================================
--- kvm.orig/arch/i386/kernel/Makefile	2007-05-30 08:42:26.000000000 -0500
+++ kvm/arch/i386/kernel/Makefile	2007-05-30 08:42:38.000000000 -0500
@@ -41,6 +41,7 @@
 obj-$(CONFIG_K8_NB)		+= k8.o
 
 obj-$(CONFIG_VMI)		+= vmi.o vmiclock.o
+obj-$(CONFIG_KVM_GUEST)		+= kvm.o
 obj-$(CONFIG_PARAVIRT)		+= paravirt.o
 obj-y				+= pcspeaker.o
 
Index: kvm/arch/i386/kernel/kvm.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ kvm/arch/i386/kernel/kvm.c	2007-05-30 09:29:37.000000000 -0500
@@ -0,0 +1,100 @@
+/*
+ * KVM paravirt_ops implementation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ * Copyright (C) 2007, Red Hat, Inc., Ingo Molnar <mingo@redhat.com>
+ * Copyright IBM Corporation, 2007
+ *   Authors: Anthony Liguori <aliguori@us.ibm.com>
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/kvm_para.h>
+#include <linux/cpu.h>
+
+static DEFINE_PER_CPU(struct kvm_vcpu_para_state, para_state);
+extern unsigned char hypercall_addr[4];
+
+static void kvm_guest_setup(void)
+{
+	paravirt_ops.name = "KVM";
+	paravirt_ops.paravirt_enabled = 1;
+}
+
+/*
+ * This is the vm-syscall address - to be patched by the host to
+ * VMCALL (Intel) or VMMCALL (AMD), depending on the CPU model:
+ */
+asm (
+	".globl hypercall_addr\n"
+	".align 4\n"
+	"hypercall_addr:\n"
+	"movl $-38, %eax\n"
+	"ret\n"
+);
+
+static int kvm_guest_register_para(int cpu)
+{
+	struct kvm_vcpu_para_state *para_state = &per_cpu(para_state, cpu);
+
+	printk(KERN_DEBUG "kvm guest on VCPU#%d: trying to register para_state %p\n",
+	       cpu, para_state);
+
+	/*
+	 * Try to write to a magic MSR (which is invalid on any real CPU),
+	 * and thus signal to KVM that we wish to entering paravirtualized
+	 * mode:
+	 */
+	para_state->guest_version = KVM_PARA_API_VERSION;
+	para_state->host_version = -1;
+	para_state->size = sizeof(*para_state);
+	para_state->ret = 0;
+	para_state->hypercall_gpa = __pa(hypercall_addr);
+
+	if (wrmsr_safe(MSR_KVM_API_MAGIC, __pa(para_state), 0)) {
+		printk(KERN_INFO "KVM guest: WRMSR probe failed.\n");
+		return -ENOENT;
+	}
+
+	printk(KERN_DEBUG "kvm guest: host returned %d\n", para_state->ret);
+	printk(KERN_DEBUG "kvm guest: host version: %d\n", para_state->host_version);
+	printk(KERN_DEBUG "kvm guest: syscall entry: %02x %02x %02x %02x\n",
+	       hypercall_addr[0], hypercall_addr[1],
+	       hypercall_addr[2], hypercall_addr[3]);
+
+	if (para_state->ret) {
+		printk(KERN_ERR "kvm guest: host refused registration.\n");
+		return para_state->ret;
+	}
+
+	return 0;
+}
+
+static int __init kvm_guest_init(void)
+{
+	int rc;
+
+	rc = kvm_guest_register_para(smp_processor_id());
+	if (rc) {
+		printk(KERN_INFO "paravirt KVM unavailable\n");
+		goto out;
+	}
+
+	kvm_guest_setup();
+ out:
+	return 0;
+}
+core_initcall(kvm_guest_init);

[-- Attachment #3: Type: text/plain, Size: 184 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

[PATCH 2/3][PARAVIRT] Make IO delay a NOP

[Anthony Liguori]

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

Regards,

Anthony Liguori

[-- Attachment #2: paravirt-no-iodelay.diff --]
[-- Type: text/x-patch, Size: 874 bytes --]

Subject: [PATCH][PARAVIRT] Make IO delay a NOP for paravirt guests

No delay is required in between PIO operations under KVM guests so make IO
delay a NOP.  This was originally part of Ingo Molnar's paravirt series.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Index: kvm/arch/i386/kernel/kvm.c
===================================================================
--- kvm.orig/arch/i386/kernel/kvm.c	2007-05-30 09:30:42.000000000 -0500
+++ kvm/arch/i386/kernel/kvm.c	2007-05-30 09:31:46.000000000 -0500
@@ -28,9 +28,17 @@
 static DEFINE_PER_CPU(struct kvm_vcpu_para_state, para_state);
 extern unsigned char hypercall_addr[4];
 
+/*
+ * No need for any "IO delay" on KVM
+ */
+static void kvm_io_delay(void)
+{
+}
+
 static void kvm_guest_setup(void)
 {
 	paravirt_ops.name = "KVM";
+	paravirt_ops.io_delay = kvm_io_delay;
 	paravirt_ops.paravirt_enabled = 1;
 }
 

[-- Attachment #3: Type: text/plain, Size: 184 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

[PATCH 3/3] Eliminate read_cr3 on TLB flush

[Anthony Liguori]

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

Regards,

Anthony Liguori

[-- Attachment #2: paravirt-no-read-cr3-flush.diff --]
[-- Type: text/x-patch, Size: 1619 bytes --]

Subject: [PATCH][PARAVIRT] Eliminate unnecessary CR3 read in TLB flush

This patch eliminates the CR3 read (which would cause a VM exit) in the TLB
flush path.  The patch is based on Ingo Molnar's paravirt series.

Signed-off-by: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Index: kvm/arch/i386/kernel/kvm.c
===================================================================
--- kvm.orig/arch/i386/kernel/kvm.c	2007-05-30 09:13:48.000000000 -0500
+++ kvm/arch/i386/kernel/kvm.c	2007-05-30 09:14:24.000000000 -0500
@@ -24,11 +24,35 @@
 #include <linux/kernel.h>
 #include <linux/kvm_para.h>
 #include <linux/cpu.h>
+#include <linux/mm.h>
+#include <asm/processor.h>
+#include <asm/tlbflush.h>
 
 static DEFINE_PER_CPU(struct kvm_vcpu_para_state, para_state);
 extern unsigned char hypercall_addr[4];
 
 /*
+ * Avoid the VM exit upon cr3 load by using the cached
+ * ->active_mm->pgd value:
+ */
+static void kvm_flush_tlb_user(void)
+{
+	write_cr3(__pa(current->active_mm->pgd));
+}
+
+/*
+ * Avoid VM exit for cr3 read by calling into kvm_flush_tlb_user
+ */
+static fastcall void kvm_flush_tlb_kernel(void)
+{
+	unsigned long orig_cr4 = read_cr4();
+
+	write_cr4(orig_cr4 & ~X86_CR4_PGE);
+	kvm_flush_tlb_user();
+	write_cr4(orig_cr4);
+}
+
+/*
  * No need for any "IO delay" on KVM
  */
 static void kvm_io_delay(void)
@@ -38,6 +62,8 @@
 static void kvm_guest_setup(void)
 {
 	paravirt_ops.name = "KVM";
+	paravirt_ops.flush_tlb_user = kvm_flush_tlb_user;
+	paravirt_ops.flush_tlb_kernel = kvm_flush_tlb_kernel;
 	paravirt_ops.io_delay = kvm_io_delay;
 	paravirt_ops.paravirt_enabled = 1;
 }

[-- Attachment #3: Type: text/plain, Size: 286 bytes --]

-------------------------------------------------------------------------
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/

[-- Attachment #4: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Andi Kleen]

On Wednesday 30 May 2007 16:53:41 Anthony Liguori wrote:
> Subject: [PATCH][PARAVIRT] Eliminate unnecessary CR3 read in TLB flush
> 
> This patch eliminates the CR3 read (which would cause a VM exit) in the TLB
> flush path.  The patch is based on Ingo Molnar's paravirt series.
> 

This change could be just done generically for the native architecture, couldn't 
it?

-Andi

-------------------------------------------------------------------------
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 3/3] Eliminate read_cr3 on TLB flush

[Anthony Liguori]

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

Andi Kleen wrote:
> On Wednesday 30 May 2007 16:53:41 Anthony Liguori wrote:
>   
>> Subject: [PATCH][PARAVIRT] Eliminate unnecessary CR3 read in TLB flush
>>
>> This patch eliminates the CR3 read (which would cause a VM exit) in the TLB
>> flush path.  The patch is based on Ingo Molnar's paravirt series.
>>
>>     
>
> This change could be just done generically for the native architecture, couldn't 
> it?
>   

Sure.  It adds a few more cycles onto native though (two memory reads, 
and some math).  How does the following look?

Regards,

Anthony Liguori

> -Andi
>   


[-- Attachment #2: no-read-cr3-on-tlbflush.diff --]
[-- Type: text/x-patch, Size: 2122 bytes --]

Subject: [PATCH] Avoid reading CR3 on TLB flush
From: Anthony Liguori <aliguori@us.ibm.com>

In a virtualized environment, there is significant overhead in reading and
writing control registers.  Since we already have the value of CR3 in
current->active_mm->pgd, we can avoid taking the exit on CR3 read when
doing a TLB flush.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Index: kvm/include/asm-i386/tlbflush.h
===================================================================
--- kvm.orig/include/asm-i386/tlbflush.h	2007-05-30 10:22:48.000000000 -0500
+++ kvm/include/asm-i386/tlbflush.h	2007-05-30 10:22:54.000000000 -0500
@@ -14,13 +14,12 @@
 
 #define __native_flush_tlb()						\
 	do {								\
-		unsigned int tmpreg;					\
+		unsigned int cr3 = __pa(current->active_mm->pgd);	\
 									\
 		__asm__ __volatile__(					\
-			"movl %%cr3, %0;              \n"		\
 			"movl %0, %%cr3;  # flush TLB \n"		\
-			: "=r" (tmpreg)					\
-			:: "memory");					\
+			:: "r" (cr3)				       	\
+			: "memory");					\
 	} while (0)
 
 /*
@@ -29,18 +28,18 @@
  */
 #define __native_flush_tlb_global()					\
 	do {								\
-		unsigned int tmpreg, cr4, cr4_orig;			\
+		unsigned int cr3 = __pa(current->active_mm->pgd);	\
+		unsigned int cr4, cr4_orig;				\
 									\
 		__asm__ __volatile__(					\
-			"movl %%cr4, %2;  # turn off PGE     \n"	\
-			"movl %2, %1;                        \n"	\
-			"andl %3, %1;                        \n"	\
-			"movl %1, %%cr4;                     \n"	\
-			"movl %%cr3, %0;                     \n"	\
-			"movl %0, %%cr3;  # flush TLB        \n"	\
-			"movl %2, %%cr4;  # turn PGE back on \n"	\
-			: "=&r" (tmpreg), "=&r" (cr4), "=&r" (cr4_orig)	\
-			: "i" (~X86_CR4_PGE)				\
+			"movl %%cr4, %1;  # turn off PGE     \n"	\
+			"movl %1, %0;                        \n"	\
+			"andl %2, %0;                        \n"	\
+			"movl %0, %%cr4;                     \n"	\
+			"movl %3, %%cr3;  # flush TLB        \n"	\
+			"movl %1, %%cr4;  # turn PGE back on \n"	\
+			: "=&r" (cr4), "=&r" (cr4_orig)			\
+			: "i" (~X86_CR4_PGE), "r" (cr3)			\
 			: "memory");					\
 	} while (0)
 

[-- Attachment #3: Type: text/plain, Size: 184 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Jeremy Fitzhardinge]

Anthony Liguori wrote:
> Sure.  It adds a few more cycles onto native though (two memory reads,
> and some math).

As opposed to a serializing control-register read?  I think that's
probably a win.

    J

RE: [kvm-devel] [PATCH 1/3] KVM paravirt_ops infrastructure

[Nakajima, Jun]

Anthony Liguori wrote:
> Regards,
> 
> Anthony Liguori

I think we should use the CPUID instruction (leaf 0x40000000) to detect
the hypervosor as we are doing in Xen. 

Jun
---
Intel Open Source Technology Center

Re: [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Nakajima, Jun]

Jeremy Fitzhardinge wrote:
> Anthony Liguori wrote:
> > Sure.  It adds a few more cycles onto native though (two memory
reads,
> > and some math).
> 
> As opposed to a serializing control-register read?  I think that's
> probably a win.
> 
>     J
> 

And actually you don't need the write to CR3 to flush TLB because the
one to CR4 does it. Or does kvm_flush_tlb_kernel assume that CR3 is
updated at the same time?

Jun
---
Intel Open Source Technology Center

-------------------------------------------------------------------------
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: [kvm-devel] [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Nakajima, Jun]

Jeremy Fitzhardinge wrote:
> Anthony Liguori wrote:
> > Sure.  It adds a few more cycles onto native though (two memory
reads,
> > and some math).
> 
> As opposed to a serializing control-register read?  I think that's
> probably a win.
> 
>     J
> 

And actually you don't need the write to CR3 to flush TLB because the
one to CR4 does it. Or does kvm_flush_tlb_kernel assume that CR3 is
updated at the same time?

Jun
---
Intel Open Source Technology Center

Re: [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Zachary Amsden]

Nakajima, Jun wrote:
> And actually you don't need the write to CR3 to flush TLB because the
> one to CR4 does it. Or does kvm_flush_tlb_kernel assume that CR3 is
> updated at the same time?
>
> Jun

It should not be necessary, but I believe this was added as a workaround 
to a PII erratum.  I can't find the erratum, however, and the history of 
using G bits in Linux is complicated (several bugs introduced and many 
intermediate versions of this code).  Since this is not performance 
critical, I think it is probably best to leave the CR3 reload.

However, being unnecessary on modern processors, I already submitted a 
patch to eliminate it on 64-bit (or maybe just told Andi about it, I 
can't recall).

Zach

-------------------------------------------------------------------------
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: [kvm-devel] [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Zachary Amsden]

Nakajima, Jun wrote:
> And actually you don't need the write to CR3 to flush TLB because the
> one to CR4 does it. Or does kvm_flush_tlb_kernel assume that CR3 is
> updated at the same time?
>
> Jun

It should not be necessary, but I believe this was added as a workaround 
to a PII erratum.  I can't find the erratum, however, and the history of 
using G bits in Linux is complicated (several bugs introduced and many 
intermediate versions of this code).  Since this is not performance 
critical, I think it is probably best to leave the CR3 reload.

However, being unnecessary on modern processors, I already submitted a 
patch to eliminate it on 64-bit (or maybe just told Andi about it, I 
can't recall).

Zach

Re: [PATCH 1/3] KVM paravirt_ops infrastructure

[Anthony Liguori]

Nakajima, Jun wrote:
> Anthony Liguori wrote:
>   
>> Regards,
>>
>> Anthony Liguori
>>     
>
> I think we should use the CPUID instruction (leaf 0x40000000) to detect
> the hypervosor as we are doing in Xen. 
>   

Is that leaf reserved for such use by Intel?

Regards,

Anthony Liguori

> Jun
> ---
> Intel Open Source Technology Center
>   


-------------------------------------------------------------------------
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 1/3] KVM paravirt_ops infrastructure

[Nakajima, Jun]

Anthony Liguori wrote:
> Nakajima, Jun wrote:
> > Anthony Liguori wrote:
> > 
> > > Regards,
> > > 
> > > Anthony Liguori
> > > 
> > 
> > I think we should use the CPUID instruction (leaf 0x40000000) to
detect
> > the hypervosor as we are doing in Xen.
> > 
> 
> Is that leaf reserved for such use by Intel?
> 

What I can say is that we (including the H/W teams) reviewed it
internally.

> Regards,
> 
> Anthony Liguori
> 


Jun
---
Intel Open Source Technology Center

-------------------------------------------------------------------------
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 3/3] Eliminate read_cr3 on TLB flush

[Nakajima, Jun]

Zachary Amsden wrote:
> Nakajima, Jun wrote:
> > And actually you don't need the write to CR3 to flush TLB because
the
> > one to CR4 does it. Or does kvm_flush_tlb_kernel assume that CR3 is
updated
> > at the same time? 
> > 
> > Jun
> 
> It should not be necessary, but I believe this was added as a
workaround
> to a PII erratum.  I can't find the erratum, however, and the history
of
> using G bits in Linux is complicated (several bugs introduced and many
> intermediate versions of this code).  Since this is not performance
> critical, I think it is probably best to leave the CR3 reload.

I don't recommend this for old processors.

> 
> However, being unnecessary on modern processors, I already submitted a
> patch to eliminate it on 64-bit (or maybe just told Andi about it, I
> can't recall).
> 
> Zach

For KVM, it should be okay as well. But we can replace two CR4 accesses
with just one hypercall.

Jun
---
Intel Open Source Technology Center

-------------------------------------------------------------------------
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: [kvm-devel] [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Nakajima, Jun]

Zachary Amsden wrote:
> Nakajima, Jun wrote:
> > And actually you don't need the write to CR3 to flush TLB because
the
> > one to CR4 does it. Or does kvm_flush_tlb_kernel assume that CR3 is
updated
> > at the same time? 
> > 
> > Jun
> 
> It should not be necessary, but I believe this was added as a
workaround
> to a PII erratum.  I can't find the erratum, however, and the history
of
> using G bits in Linux is complicated (several bugs introduced and many
> intermediate versions of this code).  Since this is not performance
> critical, I think it is probably best to leave the CR3 reload.

I don't recommend this for old processors.

> 
> However, being unnecessary on modern processors, I already submitted a
> patch to eliminate it on 64-bit (or maybe just told Andi about it, I
> can't recall).
> 
> Zach

For KVM, it should be okay as well. But we can replace two CR4 accesses
with just one hypercall.

Jun
---
Intel Open Source Technology Center

Re: [kvm-devel] [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Anthony Liguori]

Nakajima, Jun wrote:
> Zachary Amsden wrote:
>   
>> Nakajima, Jun wrote:
>>     
>>> And actually you don't need the write to CR3 to flush TLB because
>>>       
> the
>   
>>> one to CR4 does it. Or does kvm_flush_tlb_kernel assume that CR3 is
>>>       
> updated
>   
>>> at the same time? 
>>>
>>> Jun
>>>       
>> It should not be necessary, but I believe this was added as a
>>     
> workaround
>   
>> to a PII erratum.  I can't find the erratum, however, and the history
>>     
> of
>   
>> using G bits in Linux is complicated (several bugs introduced and many
>> intermediate versions of this code).  Since this is not performance
>> critical, I think it is probably best to leave the CR3 reload.
>>     
>
> I don't recommend this for old processors.
>
>   
>> However, being unnecessary on modern processors, I already submitted a
>> patch to eliminate it on 64-bit (or maybe just told Andi about it, I
>> can't recall).
>>
>> Zach
>>     
>
> For KVM, it should be okay as well. But we can replace two CR4 accesses
> with just one hypercall.
>   

I was thinking the same thing :-)

I was actually thinking about adding a hypercall to set/clear a bit in a 
control register.  The thought here is that it would be useful not just 
for the global bit but also for CR0.TS although we would need another 
paravirt_op hook for stts.

Regards,

Anthony Liguori

> Jun
> ---
> Intel Open Source Technology Center
>   

Re: [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Nakajima, Jun]

Anthony Liguori wrote:
> Nakajima, Jun wrote:

<snip>

> > For KVM, it should be okay as well. But we can replace two CR4
accesses
> > with just one hypercall. 
> > 
> 
> I was thinking the same thing :-)
> 
> I was actually thinking about adding a hypercall to set/clear a bit in
a
> control register.  The thought here is that it would be useful not
just
> for the global bit but also for CR0.TS although we would need another
> paravirt_op hook for stts.

Given the optimizations for CPU virtualization in the current H/W, I'm
not sure if such hooks are useful. Do you have any performance data that
justify such hooks? 

> 
> Regards,
> 
> Anthony Liguori
> 


Jun
---
Intel Open Source Technology Center

-------------------------------------------------------------------------
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: [kvm-devel] [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Nakajima, Jun]

Anthony Liguori wrote:
> Nakajima, Jun wrote:

<snip>

> > For KVM, it should be okay as well. But we can replace two CR4
accesses
> > with just one hypercall. 
> > 
> 
> I was thinking the same thing :-)
> 
> I was actually thinking about adding a hypercall to set/clear a bit in
a
> control register.  The thought here is that it would be useful not
just
> for the global bit but also for CR0.TS although we would need another
> paravirt_op hook for stts.

Given the optimizations for CPU virtualization in the current H/W, I'm
not sure if such hooks are useful. Do you have any performance data that
justify such hooks? 

> 
> Regards,
> 
> Anthony Liguori
> 


Jun
---
Intel Open Source Technology Center

Re: [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Zachary Amsden]

Anthony Liguori wrote:
>
> I was thinking the same thing :-)
>
> I was actually thinking about adding a hypercall to set/clear a bit in 
> a control register.  The thought here is that it would be useful not 
> just for the global bit but also for CR0.TS although we would need 
> another paravirt_op hook for stts.

You don't need STTS: just cache CR0 value on writes and replace 
read_cr0.  More paravirt_op hooks would likely be frowned upon, we've 
already got too many.

Zach

-------------------------------------------------------------------------
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: [kvm-devel] [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Zachary Amsden]

Anthony Liguori wrote:
>
> I was thinking the same thing :-)
>
> I was actually thinking about adding a hypercall to set/clear a bit in 
> a control register.  The thought here is that it would be useful not 
> just for the global bit but also for CR0.TS although we would need 
> another paravirt_op hook for stts.

You don't need STTS: just cache CR0 value on writes and replace 
read_cr0.  More paravirt_op hooks would likely be frowned upon, we've 
already got too many.

Zach

Re: [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Anthony Liguori]

Nakajima, Jun wrote:
> Anthony Liguori wrote:
>   
>
> Given the optimizations for CPU virtualization in the current H/W, I'm
> not sure if such hooks are useful. Do you have any performance data that
> justify such hooks? 
>   

No, I don't.  It was just a thought that was yet to be confirmed.

Regards,

Anthony Liguori

>> Regards,
>>
>> Anthony Liguori
>>
>>     
>
>
> Jun
> ---
> Intel Open Source Technology Center
>
> -------------------------------------------------------------------------
> 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: [kvm-devel] [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Anthony Liguori]

Nakajima, Jun wrote:
> Anthony Liguori wrote:
>   
>
> Given the optimizations for CPU virtualization in the current H/W, I'm
> not sure if such hooks are useful. Do you have any performance data that
> justify such hooks? 
>   

No, I don't.  It was just a thought that was yet to be confirmed.

Regards,

Anthony Liguori

>> Regards,
>>
>> Anthony Liguori
>>
>>     
>
>
> Jun
> ---
> Intel Open Source Technology Center
>
> -------------------------------------------------------------------------
> 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@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>
>   

Re: [PATCH 1/3] KVM paravirt_ops infrastructure

[Rusty Russell]

On Wed, 2007-05-30 at 09:52 -0500, Anthony Liguori wrote:
> This patch adds the basic infrastructure for paravirtualizing a KVM
> guest.

Hi Anthony!

	Nice patch, comments below.

> Discovery of running under KVM is done by sharing a page of memory
> between
> the guest and host (initially through an MSR write).

I missed the shared page in this patch?  If you are going to do that,
perhaps putting the hypercall magic in that page is a good idea?

> +extern unsigned char hypercall_addr[4];

Perhaps in a header?

> +asm (
> +       ".globl hypercall_addr\n"
> +       ".align 4\n"
> +       "hypercall_addr:\n"
> +       "movl $-38, %eax\n"
> +       "ret\n"
> +);

I don't think we want the hypercall returning Linux error numbers, and
magic numbers are bad too.  ud2 here I think.

> +       para_state->guest_version = KVM_PARA_API_VERSION;
> +       para_state->host_version = -1;
> +       para_state->size = sizeof(*para_state);
> +       para_state->ret = 0;
> +       para_state->hypercall_gpa = __pa(hypercall_addr);

Two versions, size *and* ret?  This seems like overkill...

> +       if (wrmsr_safe(MSR_KVM_API_MAGIC, __pa(para_state), 0)) {
> +               printk(KERN_INFO "KVM guest: WRMSR probe failed.\n");
> +               return -ENOENT;
> +       }

How about printk(KERN_INFO "I am not a KVM guest\n");?

> +static int __init kvm_guest_init(void)
> +{
> +       int rc;
> +
> +       rc = kvm_guest_register_para(smp_processor_id());
> +       if (rc) {
> +               printk(KERN_INFO "paravirt KVM unavailable\n");

Double-printk when KVM isn't detected seems overkill.  Perhaps you could
just fold this all into one function...

(Personal gripe: I consider a variable named "rc" to be an admission of
semantic defeat... "err" would be better here...)

Thanks!
Rusty.

Re: [kvm-devel] [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Rusty Russell]

On Wed, 2007-05-30 at 14:22 -0500, Anthony Liguori wrote:
> I was actually thinking about adding a hypercall to set/clear a bit in a 
> control register.  The thought here is that it would be useful not just 
> for the global bit but also for CR0.TS although we would need another 
> paravirt_op hook for stts.

We don't really need one, because Linux (i386) only cares about the TS
bit of cr0.  From lguest (you'd want this per-cpu of course):

        static unsigned long current_cr0, current_cr3;
        static void lguest_write_cr0(unsigned long val)
        {
        	lazy_hcall(LHCALL_TS, val & 8, 0, 0);
        	current_cr0 = val;
        }
        
        static unsigned long lguest_read_cr0(void)
        {
        	return current_cr0;
        }

Cheers,
Rusty.

Re: [kvm-devel] [PATCH 1/3] KVM paravirt_ops infrastructure

[Anthony Liguori]

Rusty Russell wrote:
> On Wed, 2007-05-30 at 09:52 -0500, Anthony Liguori wrote:
>   
>> This patch adds the basic infrastructure for paravirtualizing a KVM
>> guest.
>>     
>
> Hi Anthony!
>
> 	Nice patch, comments below.
>
>   
>> Discovery of running under KVM is done by sharing a page of memory
>> between
>> the guest and host (initially through an MSR write).
>>     
>
> I missed the shared page in this patch?  If you are going to do that,
> perhaps putting the hypercall magic in that page is a good idea?
>   

para_state is the shared page.  The address is passed to the KVM via the 
MSR (so it's a shared page owned by the guest).

>> +extern unsigned char hypercall_addr[4];
>>     
>
> Perhaps in a header?
>
>   
>> +asm (
>> +       ".globl hypercall_addr\n"
>> +       ".align 4\n"
>> +       "hypercall_addr:\n"
>> +       "movl $-38, %eax\n"
>> +       "ret\n"
>> +);
>>     
>
> I don't think we want the hypercall returning Linux error numbers, and
> magic numbers are bad too.  ud2 here I think.
>   

Yeah, you're not the first one to suggest this.  The thing is, KVM 
already has host-side support for a hypercall API.  I didn't want to 
change that unless I had to.  However, based on the prior feedback re: 
using CPUID, I will be changing it so I'll update this too.

>> +       para_state->guest_version = KVM_PARA_API_VERSION;
>> +       para_state->host_version = -1;
>> +       para_state->size = sizeof(*para_state);
>> +       para_state->ret = 0;
>> +       para_state->hypercall_gpa = __pa(hypercall_addr);
>>     
>
> Two versions, size *and* ret?  This seems like overkill...
>   

Yeah, I agree :-)  I actually am not a huge fan of using version 
numbers.  I think I'm going to try the next patch using a single version 
number and a feature bitmap.  Some of the optimizations (like MMU 
batching) don't make sense in a NPT/EPT environment but the guest 
shouldn't have to be aware of that.

>> +       if (wrmsr_safe(MSR_KVM_API_MAGIC, __pa(para_state), 0)) {
>> +               printk(KERN_INFO "KVM guest: WRMSR probe failed.\n");
>> +               return -ENOENT;
>> +       }
>>     
>
> How about printk(KERN_INFO "I am not a KVM guest\n");?
>
>   
>> +static int __init kvm_guest_init(void)
>> +{
>> +       int rc;
>> +
>> +       rc = kvm_guest_register_para(smp_processor_id());
>> +       if (rc) {
>> +               printk(KERN_INFO "paravirt KVM unavailable\n");
>>     
>
> Double-printk when KVM isn't detected seems overkill.  Perhaps you could
> just fold this all into one function...
>   

Already have.

> (Personal gripe: I consider a variable named "rc" to be an admission of
> semantic defeat... "err" would be better here...)
>   

I'm not sure I agree that's one's better than the other.  Although I 
guess if (err) { reads a little better...

Regards,

Anthony Liguori

> Thanks!
> Rusty.
>
>
> -------------------------------------------------------------------------
> 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@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>
>   

Re: [PATCH 1/3] KVM paravirt_ops infrastructure

[Avi Kivity]

Rusty Russell wrote:
> Two versions, size *and* ret?  This seems like overkill...
>
>   

I think we ought to move away from version numbers and use feature 
availability flags instead.

>> +       if (rc) {
>> +               printk(KERN_INFO "paravirt KVM unavailable\n");
>>     
>
> Double-printk when KVM isn't detected seems overkill.  Perhaps you could
> just fold this all into one function...
>
> (Personal gripe: I consider a variable named "rc" to be an admission of
> semantic defeat... "err" would be better here...)
>   

I just use 'r' (short for 'argh').


-- 
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/

Re: [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Avi Kivity]

Anthony Liguori wrote:
>>>     
>>>       
>> For KVM, it should be okay as well. But we can replace two CR4 accesses
>> with just one hypercall.
>>   
>>     
>
> I was thinking the same thing :-)
>
> I was actually thinking about adding a hypercall to set/clear a bit in a 
> control register.  The thought here is that it would be useful not just 
> for the global bit but also for CR0.TS although we would need another 
> paravirt_op hook for stts.
>   

Are global tlb flushes frequent enough to warrant optimization?

-- 
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/

Re: [kvm-devel] [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Avi Kivity]

Anthony Liguori wrote:
>>>     
>>>       
>> For KVM, it should be okay as well. But we can replace two CR4 accesses
>> with just one hypercall.
>>   
>>     
>
> I was thinking the same thing :-)
>
> I was actually thinking about adding a hypercall to set/clear a bit in a 
> control register.  The thought here is that it would be useful not just 
> for the global bit but also for CR0.TS although we would need another 
> paravirt_op hook for stts.
>   

Are global tlb flushes frequent enough to warrant optimization?

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

Re: [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Zachary Amsden]

Avi Kivity wrote:
> Anthony Liguori wrote:
>>>>           
>>> For KVM, it should be okay as well. But we can replace two CR4 accesses
>>> with just one hypercall.
>>>       
>>
>> I was thinking the same thing :-)
>>
>> I was actually thinking about adding a hypercall to set/clear a bit 
>> in a control register.  The thought here is that it would be useful 
>> not just for the global bit but also for CR0.TS although we would 
>> need another paravirt_op hook for stts.
>>   
>
> Are global tlb flushes frequent enough to warrant optimization?

Not very, unless you have a most unusual workload on 32-bit with highmem 
and suffer from kmap exhaustion.

Zach

-------------------------------------------------------------------------
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 1/3] KVM paravirt_ops infrastructure

[Andi Kleen]

> > +       if (wrmsr_safe(MSR_KVM_API_MAGIC, __pa(para_state), 0)) {
> > +               printk(KERN_INFO "KVM guest: WRMSR probe failed.\n");
> > +               return -ENOENT;
> > +       }
> 
> How about printk(KERN_INFO "I am not a KVM guest\n");?

Actually paravirt probes that fail should be silent; similar like
drivers that don't find their hardware should do the same. Otherwise
if there are later distro kernels with various of those compiled
in the boot log would become quite noisy

-andi

-------------------------------------------------------------------------
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 1/3] KVM paravirt_ops infrastructure

[Ingo Molnar]

* Andi Kleen <ak-l3A5Bk7waGM@public.gmane.org> wrote:

> 
> > > +       if (wrmsr_safe(MSR_KVM_API_MAGIC, __pa(para_state), 0)) {
> > > +               printk(KERN_INFO "KVM guest: WRMSR probe failed.\n");
> > > +               return -ENOENT;
> > > +       }
> > 
> > How about printk(KERN_INFO "I am not a KVM guest\n");?
> 
> Actually paravirt probes that fail should be silent; similar like 
> drivers that don't find their hardware should do the same. Otherwise 
> if there are later distro kernels with various of those compiled in 
> the boot log would become quite noisy

yeah. I suspect printing that it's executing in native mode is OK.

	Ingo

-------------------------------------------------------------------------
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 1/3] KVM paravirt_ops infrastructure

[Andi Kleen]

On Thursday 31 May 2007 12:11:16 Ingo Molnar wrote:
> 
> * Andi Kleen <ak-l3A5Bk7waGM@public.gmane.org> wrote:
> 
> > 
> > > > +       if (wrmsr_safe(MSR_KVM_API_MAGIC, __pa(para_state), 0)) {
> > > > +               printk(KERN_INFO "KVM guest: WRMSR probe failed.\n");
> > > > +               return -ENOENT;
> > > > +       }
> > > 
> > > How about printk(KERN_INFO "I am not a KVM guest\n");?
> > 
> > Actually paravirt probes that fail should be silent; similar like 
> > drivers that don't find their hardware should do the same. Otherwise 
> > if there are later distro kernels with various of those compiled in 
> > the boot log would become quite noisy
> 
> yeah. I suspect printing that it's executing in native mode is OK.

But only a single printk for that please

-Andi

-------------------------------------------------------------------------
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 1/3] KVM paravirt_ops infrastructure

[Rusty Russell]

On Thu, 2007-05-31 at 12:40 +0200, Andi Kleen wrote:
> On Thursday 31 May 2007 12:11:16 Ingo Molnar wrote:
> > 
> > * Andi Kleen <ak-l3A5Bk7waGM@public.gmane.org> wrote:
> > 
> > > 
> > > > > +       if (wrmsr_safe(MSR_KVM_API_MAGIC, __pa(para_state), 0)) {
> > > > > +               printk(KERN_INFO "KVM guest: WRMSR probe failed.\n");
> > > > > +               return -ENOENT;
> > > > > +       }
> > > > 
> > > > How about printk(KERN_INFO "I am not a KVM guest\n");?
> > > 
> > > Actually paravirt probes that fail should be silent; similar like 
> > > drivers that don't find their hardware should do the same. Otherwise 
> > > if there are later distro kernels with various of those compiled in 
> > > the boot log would become quite noisy
> > 
> > yeah. I suspect printing that it's executing in native mode is OK.
> 
> But only a single printk for that please

Which already exists in paravirt.c:

static void __init default_banner(void)
{
	printk(KERN_INFO "Booting paravirtualized kernel on %s\n",
	       paravirt_ops.name);
}
...
static int __init print_banner(void)
{
	paravirt_ops.banner();
	return 0;
}
core_initcall(print_banner);

Hmm, this will predate your kvm initcall tho, and making that a
pure_initcall is pretty gross...

Cheers,
Rusty.


-------------------------------------------------------------------------
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: [kvm-devel] [PATCH 1/3] KVM paravirt_ops infrastructure

[Anthony Liguori]

Rusty Russell wrote:
> On Thu, 2007-05-31 at 12:40 +0200, Andi Kleen wrote:
>   
>> On Thursday 31 May 2007 12:11:16 Ingo Molnar wrote:
>>     
>>> * Andi Kleen <ak@suse.de> wrote:
>>>
>>>       
>>>>>> +       if (wrmsr_safe(MSR_KVM_API_MAGIC, __pa(para_state), 0)) {
>>>>>> +               printk(KERN_INFO "KVM guest: WRMSR probe failed.\n");
>>>>>> +               return -ENOENT;
>>>>>> +       }
>>>>>>             
>>>>> How about printk(KERN_INFO "I am not a KVM guest\n");?
>>>>>           
>>>> Actually paravirt probes that fail should be silent; similar like 
>>>> drivers that don't find their hardware should do the same. Otherwise 
>>>> if there are later distro kernels with various of those compiled in 
>>>> the boot log would become quite noisy
>>>>         
>>> yeah. I suspect printing that it's executing in native mode is OK.
>>>       
>> But only a single printk for that please
>>     
>
> Which already exists in paravirt.c:
>
> static void __init default_banner(void)
> {
> 	printk(KERN_INFO "Booting paravirtualized kernel on %s\n",
> 	       paravirt_ops.name);
> }
> ...
> static int __init print_banner(void)
> {
> 	paravirt_ops.banner();
> 	return 0;
> }
> core_initcall(print_banner);
>
> Hmm, this will predate your kvm initcall tho, and making that a
> pure_initcall is pretty gross...
>   

My current patch uses core_initcall() and presumably works by magic of 
linking order.

Any other suggestions over core_initcall()?

Regards,

Anthony Liguori

> Cheers,
> Rusty.
>
>
> -------------------------------------------------------------------------
> 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@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>
>   

Re: [PATCH 1/3] KVM paravirt_ops infrastructure

[Anthony Liguori]

Andi Kleen wrote:
> On Thursday 31 May 2007 12:11:16 Ingo Molnar wrote:
>   
>> yeah. I suspect printing that it's executing in native mode is OK.
>>     
>
> But only a single printk for that please
>   

This will be taken care of in the next round of patches.

Regards,

Anthony Liguori

> -Andi
>
> -------------------------------------------------------------------------
> 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: [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>   
>>>>     
>>>>       
>>>>         
>>> For KVM, it should be okay as well. But we can replace two CR4 accesses
>>> with just one hypercall.
>>>   
>>>     
>>>       
>> I was thinking the same thing :-)
>>
>> I was actually thinking about adding a hypercall to set/clear a bit in a 
>> control register.  The thought here is that it would be useful not just 
>> for the global bit but also for CR0.TS although we would need another 
>> paravirt_op hook for stts.
>>   
>>     
>
> Are global tlb flushes frequent enough to warrant optimization?
>   

I don't think so, so I'm going to remove this from the series.  I'm 
adding another an MMU batching patch which should be enough measurable 
performance advantage to warrant the series.

Regards,

Anthony Liguori


-------------------------------------------------------------------------
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: [kvm-devel] [PATCH 3/3] Eliminate read_cr3 on TLB flush

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>   
>>>>     
>>>>       
>>>>         
>>> For KVM, it should be okay as well. But we can replace two CR4 accesses
>>> with just one hypercall.
>>>   
>>>     
>>>       
>> I was thinking the same thing :-)
>>
>> I was actually thinking about adding a hypercall to set/clear a bit in a 
>> control register.  The thought here is that it would be useful not just 
>> for the global bit but also for CR0.TS although we would need another 
>> paravirt_op hook for stts.
>>   
>>     
>
> Are global tlb flushes frequent enough to warrant optimization?
>   

I don't think so, so I'm going to remove this from the series.  I'm 
adding another an MMU batching patch which should be enough measurable 
performance advantage to warrant the series.

Regards,

Anthony Liguori

Re: [PATCH 1/3] KVM paravirt_ops infrastructure

[Anthony Liguori]

Nakajima, Jun wrote:
> Anthony Liguori wrote:
>   
>> Nakajima, Jun wrote:
>>     
>>> Anthony Liguori wrote:
>>>
>>>       
>>>> Regards,
>>>>
>>>> Anthony Liguori
>>>>
>>>>         
>>> I think we should use the CPUID instruction (leaf 0x40000000) to
>>>       
> detect
>   
>>> the hypervosor as we are doing in Xen.
>>>
>>>       
>> Is that leaf reserved for such use by Intel?
>>
>>     
>
> What I can say is that we (including the H/W teams) reviewed it
> internally.
>   

I've switched to using CPUID.  I noticed that Xen also uses 0x400000001 
so I'm presuming that that is okay too?

Regards,

Anthony Liguori

>> Regards,
>>
>> Anthony Liguori
>>
>>     
>
>
> Jun
> ---
> Intel Open Source Technology Center
>
> -------------------------------------------------------------------------
> 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: [PATCH 1/3] KVM paravirt_ops infrastructure

[Nakajima, Jun]

Anthony Liguori wrote:
> Nakajima, Jun wrote:
> > What I can say is that we (including the H/W teams) reviewed it
internally.
> > 
> 
> I've switched to using CPUID.  I noticed that Xen also uses
0x400000001
> so I'm presuming that that is okay too?

Yes.

> 
> Regards,
> 
> Anthony Liguori

Jun
---
Intel Open Source Technology Center

-------------------------------------------------------------------------
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/