[PATCH 2/2] Add MSR Bitmap support in VMX

[PATCH 2/2] Add MSR Bitmap support in VMX

[Yang, Sheng]

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

Add MSR Bitmap support in VMX so that some execution of RDMSR or WRMSR
won't cause a VM exit.

Signed-off-by: Sheng Yang <sheng.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Qing He <qing.he-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/kvm/vmx.c |   44 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/kvm/vmx.h |    3 +++
 2 files changed, 46 insertions(+), 1 deletions(-)

diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 3b901fe..bebe0d4 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -71,6 +71,7 @@ static DEFINE_PER_CPU(struct vmcs *, current_vmcs);
 
 static struct page *vmx_io_bitmap_a;
 static struct page *vmx_io_bitmap_b;
+static struct page *vmx_msr_bitmap;
 
 #define EFER_SAVE_RESTORE_BITS ((u64)EFER_SCE)
 
@@ -867,7 +868,7 @@ static __init int setup_vmcs_config(struct
vmcs_config *vmcs_conf)
 	      CPU_BASED_USE_IO_BITMAPS |
 	      CPU_BASED_MOV_DR_EXITING |
 	      CPU_BASED_USE_TSC_OFFSETING;
-	opt = 0;
+	opt = CPU_BASED_USE_MSR_BITMAPS;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_PROCBASED_CTLS,
 				&_cpu_based_exec_control) < 0)
 		return -EIO;
@@ -1426,6 +1427,10 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
 	vmcs_write64(IO_BITMAP_A, page_to_phys(vmx_io_bitmap_a));
 	vmcs_write64(IO_BITMAP_B, page_to_phys(vmx_io_bitmap_b));
 
+	/* MSR bitmap */
+	if (cpu_has_vmx_msr_bitmap)
+		vmcs_write64(MSR_BITMAP, page_to_phys(vmx_msr_bitmap));
+
 	guest_write_tsc(0);
 
 	vmcs_write64(VMCS_LINK_POINTER, -1ull); /* 22.3.1.5 */
@@ -2424,6 +2429,19 @@ static void __init
vmx_check_processor_compat(void *rtn)
 	}
 }
 
+static void __init disable_intercept_for_msr(u32 msr, void
*msr_bitmap_va)
+{
+	/* See Intel PRM Vol. 3, 20.6.9 (MSR-Bitmap Address). */
+	if (msr <= 0x1fff) { /* read and write bitmaps for low MSRs */
+		__clear_bit(msr, msr_bitmap_va + 0x000);
+		__clear_bit(msr, msr_bitmap_va + 0x800);
+	} else if ((msr >= 0xc0000000) && (msr <= 0xc0001fff)) {
+		msr &= 0x1fff; /* read and write bitmaps for high MSRs
*/
+		__clear_bit(msr, msr_bitmap_va + 0x400);
+		__clear_bit(msr, msr_bitmap_va + 0xc00);
+	}
+}
+
 static struct kvm_arch_ops vmx_arch_ops = {
 	.cpu_has_kvm_support = cpu_has_kvm_support,
 	.disabled_by_bios = vmx_disabled_by_bios,
@@ -2505,6 +2523,28 @@ static int __init vmx_init(void)
 	if (r)
 		goto out1;
 
+	if (cpu_has_vmx_msr_bitmap) {
+		vmx_msr_bitmap = alloc_page(GFP_KERNEL | __GFP_HIGHMEM);
+		if (!vmx_msr_bitmap) {
+			r = -ENOMEM;
+			goto out1;
+		}
+
+		printk(KERN_INFO "kvm: Enable MSR bitmap support.\n");
+
+		va = kmap(vmx_msr_bitmap);
+		memset(va, 0xff, PAGE_SIZE);
+
+		disable_intercept_for_msr(MSR_FS_BASE, va);
+		disable_intercept_for_msr(MSR_GS_BASE, va);
+
+		disable_intercept_for_msr(MSR_IA32_SYSENTER_CS, va);
+		disable_intercept_for_msr(MSR_IA32_SYSENTER_ESP, va);
+		disable_intercept_for_msr(MSR_IA32_SYSENTER_EIP, va);
+
+		kunmap(vmx_msr_bitmap);
+	}
+
 	return 0;
 
 out1:
@@ -2516,6 +2556,8 @@ out:
 
 static void __exit vmx_exit(void)
 {
+	if (cpu_has_vmx_msr_bitmap)
+		__free_page(vmx_msr_bitmap);
 	__free_page(vmx_io_bitmap_b);
 	__free_page(vmx_io_bitmap_a);
 
diff --git a/drivers/kvm/vmx.h b/drivers/kvm/vmx.h
index 7e4dc12..b61c7f3 100644
--- a/drivers/kvm/vmx.h
+++ b/drivers/kvm/vmx.h
@@ -309,4 +309,7 @@ enum vmcs_field {
 #define MSR_IA32_FEATURE_CONTROL_LOCKED         0x1
 #define MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED  0x4
 
+#define cpu_has_vmx_msr_bitmap \
+	(vmcs_config.cpu_based_exec_ctrl & CPU_BASED_USE_MSR_BITMAPS)
+
 #endif
-- 
1.5.2


[-- Attachment #2: Add-MSR-Bitmap-support-for-VMX.patch --]
[-- Type: application/octet-stream, Size: 3658 bytes --]

From ec1c753c90e22777a035dcf55aaf0e2571c35046 Mon Sep 17 00:00:00 2001
From: Sheng Yang <sheng.yang@intel.com>
Date: Wed, 1 Aug 2007 16:11:54 +0800
Subject: [PATCH 2/2] Add MSR Bitmap support for VMX

Add MSR Bitmap support for VMX so that some execution of RDMSR or WRMSR
won't cause a VM exit.

Signed-off-by: Sheng Yang <sheng.yang@intel.com>
Signed-off-by: Qing He <qing.he@intel.com>
---
 drivers/kvm/vmx.c |   44 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/kvm/vmx.h |    3 +++
 2 files changed, 46 insertions(+), 1 deletions(-)

diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 3b901fe..bebe0d4 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -71,6 +71,7 @@ static DEFINE_PER_CPU(struct vmcs *, current_vmcs);
 
 static struct page *vmx_io_bitmap_a;
 static struct page *vmx_io_bitmap_b;
+static struct page *vmx_msr_bitmap;
 
 #define EFER_SAVE_RESTORE_BITS ((u64)EFER_SCE)
 
@@ -867,7 +868,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 	      CPU_BASED_USE_IO_BITMAPS |
 	      CPU_BASED_MOV_DR_EXITING |
 	      CPU_BASED_USE_TSC_OFFSETING;
-	opt = 0;
+	opt = CPU_BASED_USE_MSR_BITMAPS;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_PROCBASED_CTLS,
 				&_cpu_based_exec_control) < 0)
 		return -EIO;
@@ -1426,6 +1427,10 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
 	vmcs_write64(IO_BITMAP_A, page_to_phys(vmx_io_bitmap_a));
 	vmcs_write64(IO_BITMAP_B, page_to_phys(vmx_io_bitmap_b));
 
+	/* MSR bitmap */
+	if (cpu_has_vmx_msr_bitmap)
+		vmcs_write64(MSR_BITMAP, page_to_phys(vmx_msr_bitmap));
+
 	guest_write_tsc(0);
 
 	vmcs_write64(VMCS_LINK_POINTER, -1ull); /* 22.3.1.5 */
@@ -2424,6 +2429,19 @@ static void __init vmx_check_processor_compat(void *rtn)
 	}
 }
 
+static void __init disable_intercept_for_msr(u32 msr, void *msr_bitmap_va)
+{
+	/* See Intel PRM Vol. 3, 20.6.9 (MSR-Bitmap Address). */
+	if (msr <= 0x1fff) { /* read and write bitmaps for low MSRs */
+		__clear_bit(msr, msr_bitmap_va + 0x000);
+		__clear_bit(msr, msr_bitmap_va + 0x800);
+	} else if ((msr >= 0xc0000000) && (msr <= 0xc0001fff)) {
+		msr &= 0x1fff; /* read and write bitmaps for high MSRs */
+		__clear_bit(msr, msr_bitmap_va + 0x400);
+		__clear_bit(msr, msr_bitmap_va + 0xc00);
+	}
+}
+
 static struct kvm_arch_ops vmx_arch_ops = {
 	.cpu_has_kvm_support = cpu_has_kvm_support,
 	.disabled_by_bios = vmx_disabled_by_bios,
@@ -2505,6 +2523,28 @@ static int __init vmx_init(void)
 	if (r)
 		goto out1;
 
+	if (cpu_has_vmx_msr_bitmap) {
+		vmx_msr_bitmap = alloc_page(GFP_KERNEL | __GFP_HIGHMEM);
+		if (!vmx_msr_bitmap) {
+			r = -ENOMEM;
+			goto out1;
+		}
+
+		printk(KERN_INFO "kvm: Enable MSR bitmap support.\n");
+
+		va = kmap(vmx_msr_bitmap);
+		memset(va, 0xff, PAGE_SIZE);
+
+		disable_intercept_for_msr(MSR_FS_BASE, va);
+		disable_intercept_for_msr(MSR_GS_BASE, va);
+
+		disable_intercept_for_msr(MSR_IA32_SYSENTER_CS, va);
+		disable_intercept_for_msr(MSR_IA32_SYSENTER_ESP, va);
+		disable_intercept_for_msr(MSR_IA32_SYSENTER_EIP, va);
+
+		kunmap(vmx_msr_bitmap);
+	}
+
 	return 0;
 
 out1:
@@ -2516,6 +2556,8 @@ out:
 
 static void __exit vmx_exit(void)
 {
+	if (cpu_has_vmx_msr_bitmap)
+		__free_page(vmx_msr_bitmap);
 	__free_page(vmx_io_bitmap_b);
 	__free_page(vmx_io_bitmap_a);
 
diff --git a/drivers/kvm/vmx.h b/drivers/kvm/vmx.h
index 7e4dc12..b61c7f3 100644
--- a/drivers/kvm/vmx.h
+++ b/drivers/kvm/vmx.h
@@ -309,4 +309,7 @@ enum vmcs_field {
 #define MSR_IA32_FEATURE_CONTROL_LOCKED         0x1
 #define MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED  0x4
 
+#define cpu_has_vmx_msr_bitmap \
+	(vmcs_config.cpu_based_exec_ctrl & CPU_BASED_USE_MSR_BITMAPS)
+
 #endif
-- 
1.5.2


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

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- 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 2/2] Add MSR Bitmap support in VMX

[Avi Kivity]

Yang, Sheng wrote:
> Add MSR Bitmap support in VMX so that some execution of RDMSR or WRMSR
> won't cause a VM exit.
>
> Signed-off-by: Sheng Yang <sheng.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Qing He <qing.he-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/kvm/vmx.c |   44 +++++++++++++++++++++++++++++++++++++++++++-
>  drivers/kvm/vmx.h |    3 +++
>  2 files changed, 46 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
> index 3b901fe..bebe0d4 100644
> --- a/drivers/kvm/vmx.c
> +++ b/drivers/kvm/vmx.c
> @@ -71,6 +71,7 @@ static DEFINE_PER_CPU(struct vmcs *, current_vmcs);
>  
>  static struct page *vmx_io_bitmap_a;
>  static struct page *vmx_io_bitmap_b;
> +static struct page *vmx_msr_bitmap;
>  
>   

hmm.  While there's nothing wrong with the patch, there is a simpler way 
to do this:

    static unsigned long vmx_msr_bitmap[PAGE_SIZE / sizeof(unsigned 
long)] __aligned(PAGE_SIZE);

now there's no need to allocate, error-check, free, or kmap the memory.  
The io bitmaps can receive similar treatment.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH 2/2] Add MSR Bitmap support in VMX

[He, Qing]

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



>-----Original Message-----
>From: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>[mailto:kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org] On Behalf Of Avi Kivity
>Sent: 2007年8月2日 19:00
>To: Yang, Sheng
>Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>Subject: Re: [kvm-devel] [PATCH 2/2] Add MSR Bitmap support in VMX
>
>Yang, Sheng wrote:
>> Add MSR Bitmap support in VMX so that some execution of RDMSR or WRMSR
>> won't cause a VM exit.
>>
>> Signed-off-by: Sheng Yang <sheng.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> Signed-off-by: Qing He <qing.he-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> ---
>>  drivers/kvm/vmx.c |   44
>+++++++++++++++++++++++++++++++++++++++++++-
>>  drivers/kvm/vmx.h |    3 +++
>>  2 files changed, 46 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
>> index 3b901fe..bebe0d4 100644
>> --- a/drivers/kvm/vmx.c
>> +++ b/drivers/kvm/vmx.c
>> @@ -71,6 +71,7 @@ static DEFINE_PER_CPU(struct vmcs *, current_vmcs);
>>
>>  static struct page *vmx_io_bitmap_a;
>>  static struct page *vmx_io_bitmap_b;
>> +static struct page *vmx_msr_bitmap;
>>
>>
>
>hmm.  While there's nothing wrong with the patch, there is a simpler way
>to do this:
>
>    static unsigned long vmx_msr_bitmap[PAGE_SIZE / sizeof(unsigned
>long)] __aligned(PAGE_SIZE);
>
>now there's no need to allocate, error-check, free, or kmap the memory.
>The io bitmaps can receive similar treatment.

Well, though not so important, kmapping do have a tiny advantage, it uses less virtual space if HIGHMEM is used. This makes sense when 1G limited kernel space is used, although it's highly unlikely to be a real problem.

Either way, I'm OK.

>
>--
>error compiling committee.c: too many arguments to function
>
>
>-------------------------------------------------------------------------
>This SF.net email is sponsored by: Splunk Inc.
>Still grepping through log files to find problems?  Stop.
>Now Search log events and configuration files using AJAX and a browser.
>Download your FREE copy of Splunk now >>  http://get.splunk.com/
>_______________________________________________
>kvm-devel mailing list
>kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>https://lists.sourceforge.net/lists/listinfo/kvm-devel


[-- Attachment #2: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #3: 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 2/2] Add MSR Bitmap support in VMX

[Avi Kivity]

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

He, Qing wrote:
>>
>> hmm.  While there's nothing wrong with the patch, there is a simpler way
>> to do this:
>>
>>    static unsigned long vmx_msr_bitmap[PAGE_SIZE / sizeof(unsigned
>> long)] __aligned(PAGE_SIZE);
>>
>> now there's no need to allocate, error-check, free, or kmap the memory.
>> The io bitmaps can receive similar treatment.
>>     
>
> Well, though not so important, kmapping do have a tiny advantage, it uses less virtual space if HIGHMEM is used. This makes sense when 1G limited kernel space is used, although it's highly unlikely to be a real problem.
>
>   

Starting a virtual machine consumes about 4MB of low memory for the
shadow mmu (more for the memory map and for various slabs), so 4K is not
an issue.

However, I do see an issue with my proposal. To get the physical address
of the page, we need to use vmalloc_to_page(). But that won't work if
kvm is built into the kernel (and thus uses large pages for data).

Rusty, what say you to a 'struct page *module_to_page(void *kaddr)'
which does the right thing? Attached an implementation.


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


[-- Attachment #2: module_to_page.patch --]
[-- Type: text/x-patch, Size: 1590 bytes --]

diff --git a/include/linux/module.h b/include/linux/module.h
index b6a646c..c520a3a 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -16,6 +16,7 @@
 #include <linux/kobject.h>
 #include <linux/moduleparam.h>
 #include <asm/local.h>
+#include <asm/page.h>
 
 #include <asm/module.h>
 
@@ -371,6 +372,7 @@ static inline int module_is_live(struct module *mod)
 struct module *module_text_address(unsigned long addr);
 struct module *__module_text_address(unsigned long addr);
 int is_module_address(unsigned long addr);
+struct page *module_to_page(void *kaddr);
 
 /* Returns 0 and fills in value, defined and namebuf, or -ERANGE if
    symnum out of range. */
@@ -498,6 +500,11 @@ static inline int is_module_address(unsigned long addr)
 	return 0;
 }
 
+static inline struct page *module_to_page(void *kaddr)
+{
+	return virt_to_page(kaddr);
+}
+
 /* Get/put a kernel symbol (calls should be symmetric) */
 #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak)); &(x); })
 #define symbol_put(x) do { } while(0)
diff --git a/kernel/module.c b/kernel/module.c
index 33c04ad..4618792 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2347,6 +2347,16 @@ int is_module_address(unsigned long addr)
 	return 0;
 }
 
+/*
+ * Convert a virtual address, possibly in a module, to a struct page.
+ */
+struct page *module_to_page(void *kaddr)
+{
+	if (is_module_address((unsigned long)kaddr))
+		return vmalloc_to_page(kaddr);
+	else
+		return virt_to_page(kaddr);
+}
 
 /* Is this a valid kernel address? */
 struct module *__module_text_address(unsigned long addr)

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

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- 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 2/2] Add MSR Bitmap support in VMX

[Avi Kivity]

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

Avi Kivity wrote:
> However, I do see an issue with my proposal. To get the physical address
> of the page, we need to use vmalloc_to_page(). But that won't work if
> kvm is built into the kernel (and thus uses large pages for data).
>   

Because of this issue, I'd like to apply the patch now and remove the
allocations if/when the module_to_page() thing hits mainline. However:

>  static struct kvm_arch_ops vmx_arch_ops = {
>  	.cpu_has_kvm_support = cpu_has_kvm_support,
>  	.disabled_by_bios = vmx_disabled_by_bios,
> @@ -2505,6 +2523,28 @@ static int __init vmx_init(void)
>  	if (r)
>  		goto out1;
>  
> +	if (cpu_has_vmx_msr_bitmap) {
> +		vmx_msr_bitmap = alloc_page(GFP_KERNEL | __GFP_HIGHMEM);
> +		if (!vmx_msr_bitmap) {
> +			r = -ENOMEM;
> +			goto out1;
>   

We need to undo kvm_init_arch() here.

> +		}
> +
> +		printk(KERN_INFO "kvm: Enable MSR bitmap support.\n");
> +
> +		va = kmap(vmx_msr_bitmap);
>   

kmap_atomic() is preferable.

>  
> +#define cpu_has_vmx_msr_bitmap \
> +	(vmcs_config.cpu_based_exec_ctrl & CPU_BASED_USE_MSR_BITMAPS)
> +
>  #endif

This needs to be an inline local to vmx.c.


Did you see any performance improvements out of this?

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



[-- Attachment #2: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #3: 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 2/2] Add MSR Bitmap support in VMX

[Li, Xin B]

>Did you see any performance improvements out of this?

Acturally we don't expect any obviously performance because MSR accesses
are not frequent.
-Xin

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH 2/2] Add MSR Bitmap support in VMX

[Avi Kivity]

Li, Xin B wrote:
>> Did you see any performance improvements out of this?
>>     
>
> Acturally we don't expect any obviously performance because MSR accesses
> are not frequent.
>   

Well, why do this then?

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH 2/2] Add MSR Bitmap support in VMX

[Li, Xin B]

>-----Original Message-----
>From: Avi Kivity [mailto:avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org] 
>Sent: Thursday, August 02, 2007 10:50 PM
>To: Li, Xin B
>Cc: He, Qing; Rusty Russell; kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>Subject: Re: [kvm-devel] [PATCH 2/2] Add MSR Bitmap support in VMX
>
>Li, Xin B wrote:
>>> Did you see any performance improvements out of this?
>>>     
>>
>> Acturally we don't expect any obviously performance because 
>MSR accesses
>> are not frequent.
>>   
>
>Well, why do this then?

Anyway it does save CPU cycles, and benefit guest processes context
switch, although not so obviously.
-Xin 

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH 2/2] Add MSR Bitmap support in VMX

[ron minnich]

On 8/2/07, Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote:
> Li, Xin B wrote:
> >> Did you see any performance improvements out of this?
> >>
> >
> > Acturally we don't expect any obviously performance because MSR accesses
> > are not frequent.
> >
>
> Well, why do this then?

ah, see, you asked the question I was going to ask. OK, this can be
faster; but, are you sure you really care?

ron

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH 2/2] Add MSR Bitmap support in VMX

[Avi Kivity]

ron minnich wrote:
> On 8/2/07, Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote:
>   
>> Li, Xin B wrote:
>>     
>>>> Did you see any performance improvements out of this?
>>>>
>>>>         
>>> Acturally we don't expect any obviously performance because MSR accesses
>>> are not frequent.
>>>
>>>       
>> Well, why do this then?
>>     
>
> ah, see, you asked the question I was going to ask. OK, this can be
> faster; but, are you sure you really care?
>
>   

Looking at the Linux context switch code, it can bang on MSR_FS_BASE and 
MSR_KERNEL_GS_BASE. A high context switch rate between threads (which 
use %gs or %fs) can show an improvement with this.  Things like kbuild 
probably won't as they use single threaded processes.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH 2/2] Add MSR Bitmap support in VMX

[Nakajima, Jun]

Avi Kivity wrote:
> ron minnich wrote:
> > On 8/2/07, Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote:
> > 
> > > Li, Xin B wrote:
> > > 
> > > > > Did you see any performance improvements out of this?
> > > > > 
> > > > > 
> > > > Acturally we don't expect any obviously performance because MSR
> > > > accesses are not frequent. 
> > > > 
> > > > 
> > > Well, why do this then?
> > > 
> > 
> > ah, see, you asked the question I was going to ask. OK, this can be
> > faster; but, are you sure you really care?
> > 
> > 
> 
> Looking at the Linux context switch code, it can bang on MSR_FS_BASE
and
> MSR_KERNEL_GS_BASE. A high context switch rate between threads (which
> use %gs or %fs) can show an improvement with this.  Things like kbuild
> probably won't as they use single threaded processes.

Right. 

> +		disable_intercept_for_msr(MSR_FS_BASE, va);
> +		disable_intercept_for_msr(MSR_GS_BASE, va);
> +
> +		disable_intercept_for_msr(MSR_IA32_SYSENTER_CS, va);
> +		disable_intercept_for_msr(MSR_IA32_SYSENTER_ESP, va);
> +		disable_intercept_for_msr(MSR_IA32_SYSENTER_EIP, va);

I think we should focus on MSR_FS_BASE and MSR_GS_BASE. The rest are
typically set up once for the life of OS. Some other MSRs like
MSR_IA32_PERF_CTL can be accessed frequently in some configration, but
we don't want such a pass-through in KVM.

Jun
---
Intel Open Source Technology Center

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH 2/2] Add MSR Bitmap support in VMX

[Ulrich Drepper]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Nakajima, Jun wrote:
>> Looking at the Linux context switch code, it can bang on MSR_FS_BASE
> and
>> MSR_KERNEL_GS_BASE. A high context switch rate between threads (which
>> use %gs or %fs) can show an improvement with this.  Things like kbuild
>> probably won't as they use single threaded processes.
> 
> Right. 

In Linux there is no difference between single- and multi-threaded
processes when it comes to setting up segment registers.  Every x86-64
process uses %fs for TLS and every x86 process uses %gs.  There are
always variables accessed this way (errno, for instance).

- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)

iD8DBQFGsiE02ijCOnn/RHQRAusOAJoDB29M1JXWybmgdONnIZP+O7aYoACgmDth
7EXTDup10Didr2vAKVpFjJc=
=TruR
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH 2/2] Add MSR Bitmap support in VMX

[Li, Xin B]

>-----Original Message-----
>From: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org 
>[mailto:kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org] On Behalf Of 
>Ulrich Drepper
>Sent: Friday, August 03, 2007 2:24 AM
>To: Nakajima, Jun
>Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>Subject: Re: [kvm-devel] [PATCH 2/2] Add MSR Bitmap support in VMX
>
>-----BEGIN PGP SIGNED MESSAGE-----
>Hash: SHA1
>
>Nakajima, Jun wrote:
>>> Looking at the Linux context switch code, it can bang on MSR_FS_BASE
>> and
>>> MSR_KERNEL_GS_BASE. A high context switch rate between 
>threads (which
>>> use %gs or %fs) can show an improvement with this.  Things 
>like kbuild
>>> probably won't as they use single threaded processes.
>> 
>> Right. 
>
>In Linux there is no difference between single- and multi-threaded
>processes when it comes to setting up segment registers.  Every x86-64
>process uses %fs for TLS and every x86 process uses %gs.  There are
>always variables accessed this way (errno, for instance).
>

I have to say, the benefit form _no_ VMExits on these VMExits are not so
obviously, because each context switch invloves a guest CR3 update, then
a page fault vmexit is triggered and a shadow handling is involved,
which is the major cost of guest context switch.
-Xin 

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH 2/2] Add MSR Bitmap support in VMX

[Avi Kivity]

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

Li, Xin B wrote:
>> -----Original Message-----
>> From: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org 
>> [mailto:kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org] On Behalf Of 
>> Ulrich Drepper
>> Sent: Friday, August 03, 2007 2:24 AM
>> To: Nakajima, Jun
>> Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>> Subject: Re: [kvm-devel] [PATCH 2/2] Add MSR Bitmap support in VMX
>>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Nakajima, Jun wrote:
>>     
>>>> Looking at the Linux context switch code, it can bang on MSR_FS_BASE
>>>>         
>>> and
>>>       
>>>> MSR_KERNEL_GS_BASE. A high context switch rate between 
>>>>         
>> threads (which
>>     
>>>> use %gs or %fs) can show an improvement with this.  Things 
>>>>         
>> like kbuild
>>     
>>>> probably won't as they use single threaded processes.
>>>>         
>>> Right. 
>>>       
>> In Linux there is no difference between single- and multi-threaded
>> processes when it comes to setting up segment registers.  Every x86-64
>> process uses %fs for TLS and every x86 process uses %gs.  There are
>> always variables accessed this way (errno, for instance).
>>
>>     
>
> I have to say, the benefit form _no_ VMExits on these VMExits are not so
> obviously, because each context switch invloves a guest CR3 update, then
> a page fault vmexit is triggered and a shadow handling is involved,
> which is the major cost of guest context switch.
>   

A small test program demonstrates the benefit.  In thread mode, it takes 
5 seconds without the patch and 3 seconds with the patch applied (200K 
and ~0 exits/sec).  In fork mode, it takes 10 seconds without the patch 
and 7.2 seconds with the patch (420K and 280K exits/sec).  Note that 
with this test there are no page fault exits, only cr and msr exits 
(with the threaded mode avoiding the cr exit).

So the patch is quite measurable on microbenchmarks -- in threaded mode 
we're at native performance.

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


[-- Attachment #2: cswitch.c --]
[-- Type: text/x-csrc, Size: 573 bytes --]


#include <pthread.h>
#include <unistd.h>
#include <stdlib.h>

static int p1[2], p2[2];

#define N 1000000

void *server(void *_)
{
	char buf;

	while (read(p1[0], &buf, 1) == 1)
		write(p2[1], &buf, 1);
}

void client()
{
	char buf;
	int i;

	for (i = 0; i < N; ++i) {
		write(p1[1], &buf, 1);
		read(p2[0], &buf, 1);
	}
}

int main(int ac, char **av)
{
	pthread_t thread;

	pipe(p1);
	pipe(p2);

	if (ac < 2 || strcmp(av[1], "fork") != 0)
		pthread_create(&thread, NULL, server, NULL);
	else
		if (fork() == 0) {
			server(NULL);
			return 0;
		}
	client();
	return 0;
}

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

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- 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 2/2] Add MSR Bitmap support in VMX

[Rusty Russell]

On Thu, 2007-08-02 at 15:39 +0300, Avi Kivity wrote:
> He, Qing wrote:
> >>
> >> hmm.  While there's nothing wrong with the patch, there is a simpler way
> >> to do this:
> >>
> >>    static unsigned long vmx_msr_bitmap[PAGE_SIZE / sizeof(unsigned
> >> long)] __aligned(PAGE_SIZE);
> >>
> >> now there's no need to allocate, error-check, free, or kmap the memory.
> >> The io bitmaps can receive similar treatment.
> >>     
> >
> > Well, though not so important, kmapping do have a tiny advantage, it uses less virtual space if HIGHMEM is used. This makes sense when 1G limited kernel space is used, although it's highly unlikely to be a real problem.
> >
> >   
> 
> Starting a virtual machine consumes about 4MB of low memory for the
> shadow mmu (more for the memory map and for various slabs), so 4K is not
> an issue.
> 
> However, I do see an issue with my proposal. To get the physical address
> of the page, we need to use vmalloc_to_page(). But that won't work if
> kvm is built into the kernel (and thus uses large pages for data).
> 
> Rusty, what say you to a 'struct page *module_to_page(void *kaddr)'
> which does the right thing? Attached an implementation.

Hi Avi,

	Arch-specific, unfortunately: modules do not need to be allocated with
vmalloc (and for small modules probably shouldn't anyway).  Luckily
vmalloc always page-aligns AFAICT, so you get that.

So I think dynamic alloc is easier.
Rusty.





> 
> 


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH 2/2] Add MSR Bitmap support in VMX

[Avi Kivity]

Rusty Russell wrote:
>> Rusty, what say you to a 'struct page *module_to_page(void *kaddr)'
>> which does the right thing? Attached an implementation.
>>     
>
> Hi Avi,
>
> 	Arch-specific, unfortunately: modules do not need to be allocated with
> vmalloc (and for small modules probably shouldn't anyway).  Luckily
> vmalloc always page-aligns AFAICT, so you get that.
>
> So I think dynamic alloc is easier.
> Rusty.
>
>   

Okay, we'll go with that.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/