Split segments reload in vmx_load_host_state()

PATCH Superseded Laurent Vivier 2 participants 6 messages Aug 23, 2007 5 messages not in the archive KVM

lore

Share & tools

lore

Laurent Vivier

author

2007-08-23 12:47 UTC #0 lore raw

PATCH 1/1 Split segments reload in vmx_load_host_state()

It is from the TODO and I hope no one is already working on this.

'vmx_load_host_state() bundles fs, gs, ldt, and tss reloading into one in the hope that it is infrequent. With smp guests, fs reloading is frequent and the separate reloads should be unbundled.'

Signed-off-by Laurent Vivier <Laurent.Vivier-6ktuUTfB/[email protected]>

drivers/kvm/vmx.c +10 -8

Index: kvm/drivers/kvm/vmx.c
===================================================================
@@ -54,7 +54,8 @@ struct vcpu_vmx {
 	struct {
 		int           loaded;
 		u16           fs_sel, gs_sel, ldt_sel;
-		int           fs_gs_ldt_reload_needed;
+		int           gs_ldt_reload_needed;
+		int           fs_reload_needed;
 	}host_state;
 
 };

@@ -353,20 +354,20 @@ static void vmx_save_host_state(struct v
 	 * allow segment selectors with cpl > 0 or ti == 1.
 	 */
 	vmx->host_state.ldt_sel = read_ldt();
-	vmx->host_state.fs_gs_ldt_reload_needed = vmx->host_state.ldt_sel;
+	vmx->host_state.gs_ldt_reload_needed = vmx->host_state.ldt_sel;
 	vmx->host_state.fs_sel = read_fs();
 	if (!(vmx->host_state.fs_sel & 7))
 		vmcs_write16(HOST_FS_SELECTOR, vmx->host_state.fs_sel);
 	else {
 		vmcs_write16(HOST_FS_SELECTOR, 0);
-		vmx->host_state.fs_gs_ldt_reload_needed = 1;
+		vmx->host_state.fs_reload_needed = 1;
 	}
 	vmx->host_state.gs_sel = read_gs();
 	if (!(vmx->host_state.gs_sel & 7))
 		vmcs_write16(HOST_GS_SELECTOR, vmx->host_state.gs_sel);
 	else {
 		vmcs_write16(HOST_GS_SELECTOR, 0);
-		vmx->host_state.fs_gs_ldt_reload_needed = 1;
+		vmx->host_state.gs_ldt_reload_needed = 1;
 	}
 
 #ifdef CONFIG_X86_64

@@ -396,9 +397,10 @@ static void vmx_load_host_state(struct v
 		return;
 
 	vmx->host_state.loaded = 0;
-	if (vmx->host_state.fs_gs_ldt_reload_needed) {
-		load_ldt(vmx->host_state.ldt_sel);
+	if (vmx->host_state.fs_reload_needed)
 		load_fs(vmx->host_state.fs_sel);
+	if (vmx->host_state.gs_ldt_reload_needed) {
+		load_ldt(vmx->host_state.ldt_sel);
 		/*
 		 * If we have to reload gs, we must take care to
 		 * preserve our gs base.

@@ -409,9 +411,9 @@ static void vmx_load_host_state(struct v
 		wrmsrl(MSR_GS_BASE, vmcs_readl(HOST_GS_BASE));
 #endif
 		local_irq_restore(flags);
-
-		reload_tss();
 	}
+	if (vmx->host_state.gs_ldt_reload_needed || vmx->host_state.fs_reload_needed)
+		reload_tss();
 	save_msrs(vmx->guest_msrs, vmx->save_nmsrs);
 	load_msrs(vmx->host_msrs, vmx->save_nmsrs);
 	if (msr_efer_need_save_restore(vmx))

-------------------------------------------------------------------------
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 [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel

A message that is not in the archive (sent off-list or not yet received): 46CD81D5.90707-6ktuUTfB/[email protected]

Avi Kivity

2007-08-23 12:52 UTC #2 lore raw

Quoted 11 lines · Laurent Vivier

It is from the TODO and I hope no one is already working on this.
'vmx_load_host_state() bundles fs, gs, ldt, and tss reloading into one in the hope that it is infrequent. With smp guests, fs reloading is frequent and the separate reloads should be unbundled.'
};

drivers/kvm/vmx.c static void vmx_save_host_state(struct v · line 358

Quoted 6 lines

drivers/kvm/vmx.c +1 -1

@@ -353,20 +354,20 @@ static void vmx_save_host_state(struct v
 	 * allow segment selectors with cpl > 0 or ti == 1.
 	 */
 	vmx->host_state.ldt_sel = read_ldt();
-	vmx->host_state.fs_gs_ldt_reload_needed = vmx->host_state.ldt_sel;
+	vmx->host_state.gs_ldt_reload_needed = vmx->host_state.ldt_sel;

Missing initialization for fs_reload_needed here.

Signature · 8 lines

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/

A message that is not in the archive (sent off-list or not yet received): 46CD8316.2040208-atKUWr5tajBWk0Htik3J/[email protected]

Laurent Vivier

author

2007-08-23 13:02 UTC #4 lore raw

Quoted 10 lines · Avi Kivity

Quoted 9 lines · Laurent Vivier
It is from the TODO and I hope no one is already working on this.
'vmx_load_host_state() bundles fs, gs, ldt, and tss reloading into one in the hope that it is infrequent. With smp guests, fs reloading is frequent and the separate reloads should be unbundled.'

Quoted 11 lines

drivers/kvm/vmx.c static void vmx_save_host_state(struct v · line 358
Quoted 8 lines
};
drivers/kvm/vmx.c +1 -1
@@ -353,20 +354,20 @@ static void vmx_save_host_state(struct v
 	 * allow segment selectors with cpl > 0 or ti == 1.
 	 */
 	vmx->host_state.ldt_sel = read_ldt();
-	vmx->host_state.fs_gs_ldt_reload_needed = vmx->host_state.ldt_sel;
+	vmx->host_state.gs_ldt_reload_needed = vmx->host_state.ldt_sel;
 
Missing initialization for fs_reload_needed here.

It is initialized on fs_sel, should it be initialized on ldt_sel too ?

Laurent

Signature · 2 lines

------------- Laurent.Vivier-6ktuUTfB/[email protected]  --------------
          "Software is hard" - Donald Knuth

_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel

A message that is not in the archive (sent off-list or not yet received): 46CD8562.5000402-6ktuUTfB/[email protected]

Avi Kivity

2007-08-23 13:14 UTC #6 lore raw

Quoted 26 lines · Laurent Vivier

Quoted 11 lines · Avi Kivity
Quoted 9 lines · Laurent Vivier
It is from the TODO and I hope no one is already working on this.
'vmx_load_host_state() bundles fs, gs, ldt, and tss reloading into one in the hope that it is infrequent. With smp guests, fs reloading is frequent and the separate reloads should be unbundled.'
Quoted 12 lines
drivers/kvm/vmx.c static void vmx_save_host_state(struct v · line 359
Quoted 9 lines
};
drivers/kvm/vmx.c +1 -1
@@ -353,20 +354,20 @@ static void vmx_save_host_state(struct v
 	 * allow segment selectors with cpl > 0 or ti == 1.
 	 */
 	vmx->host_state.ldt_sel = read_ldt();
-	vmx->host_state.fs_gs_ldt_reload_needed = vmx->host_state.ldt_sel;
+	vmx->host_state.gs_ldt_reload_needed = vmx->host_state.ldt_sel;
 
 
Missing initialization for fs_reload_needed here.

Quoted 3 lines

It is initialized on fs_sel, should it be initialized on ldt_sel too ?

It's only initialized to 1, but never cleared to zero.

The original code means: if ((ldt_sel != 0) || ((fs_sel & 7) != 0) || ((gs_sel & 7) != 0)) then a reload is needed. In the new code, fs reload should depend only on ((fs_sel & 7) != 0) and ldt/gs reload should depend on ldt_sel and gs_sel only.

Regarding reload_tss(), it should probably be unconditional.

Signature · 8 lines

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/

A message that is not in the archive (sent off-list or not yet received): 46CD881B.6040202-atKUWr5tajBWk0Htik3J/[email protected]

Laurent Vivier

author

2007-08-23 14:33 UTC #8 lore raw

PATCH RESEND 1/1 Split segments reload in vmx_load_host_state()

From KVM/TODO:
vmx_load_host_state() bundles fs, gs, ldt, and tss reloading into one in the hope that it is infrequent. With smp guests, fs reloading is frequent and the separate reloads should be unbundled.

Signed-off-by Laurent Vivier <Laurent.Vivier-6ktuUTfB/[email protected]>

drivers/kvm/vmx.c +12 ~~-10~~

Index: kvm/drivers/kvm/vmx.c
===================================================================
@@ -54,7 +54,8 @@ struct vcpu_vmx {
 	struct {
 		int           loaded;
 		u16           fs_sel, gs_sel, ldt_sel;
-		int           fs_gs_ldt_reload_needed;
+		int           gs_ldt_reload_needed;
+		int           fs_reload_needed;
 	}host_state;
 
 };

@@ -353,20 +354,21 @@ static void vmx_save_host_state(struct v
 	 * allow segment selectors with cpl > 0 or ti == 1.
 	 */
 	vmx->host_state.ldt_sel = read_ldt();
-	vmx->host_state.fs_gs_ldt_reload_needed = vmx->host_state.ldt_sel;
+	vmx->host_state.gs_ldt_reload_needed = vmx->host_state.ldt_sel;
 	vmx->host_state.fs_sel = read_fs();
-	if (!(vmx->host_state.fs_sel & 7))
+	if (!(vmx->host_state.fs_sel & 7)) {
 		vmcs_write16(HOST_FS_SELECTOR, vmx->host_state.fs_sel);
-	else {
+		vmx->host_state.fs_reload_needed = 0;
+	} else {
 		vmcs_write16(HOST_FS_SELECTOR, 0);
-		vmx->host_state.fs_gs_ldt_reload_needed = 1;
+		vmx->host_state.fs_reload_needed = 1;
 	}
 	vmx->host_state.gs_sel = read_gs();
 	if (!(vmx->host_state.gs_sel & 7))
 		vmcs_write16(HOST_GS_SELECTOR, vmx->host_state.gs_sel);
 	else {
 		vmcs_write16(HOST_GS_SELECTOR, 0);
-		vmx->host_state.fs_gs_ldt_reload_needed = 1;
+		vmx->host_state.gs_ldt_reload_needed = 1;
 	}
 
 #ifdef CONFIG_X86_64

@@ -396,9 +398,10 @@ static void vmx_load_host_state(struct v
 		return;
 
 	vmx->host_state.loaded = 0;
-	if (vmx->host_state.fs_gs_ldt_reload_needed) {
-		load_ldt(vmx->host_state.ldt_sel);
+	if (vmx->host_state.fs_reload_needed)
 		load_fs(vmx->host_state.fs_sel);
+	if (vmx->host_state.gs_ldt_reload_needed) {
+		load_ldt(vmx->host_state.ldt_sel);
 		/*
 		 * If we have to reload gs, we must take care to
 		 * preserve our gs base.

@@ -409,9 +412,8 @@ static void vmx_load_host_state(struct v
 		wrmsrl(MSR_GS_BASE, vmcs_readl(HOST_GS_BASE));
 #endif
 		local_irq_restore(flags);
-
-		reload_tss();
 	}
+	reload_tss();
 	save_msrs(vmx->guest_msrs, vmx->save_nmsrs);
 	load_msrs(vmx->host_msrs, vmx->save_nmsrs);
 	if (msr_efer_need_save_restore(vmx))

_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel

A message that is not in the archive (sent off-list or not yet received): 46CD9AA7.9010101-6ktuUTfB/[email protected]

Avi Kivity

Applied

2007-08-23 14:44 UTC #10 lore raw

Quoted 6 lines · Laurent Vivier

From KVM/TODO:
vmx_load_host_state() bundles fs, gs, ldt, and tss reloading into one in the hope that it is infrequent. With smp guests, fs reloading is frequent and the separate reloads should be unbundled.

Applied, thanks.

Signature · 8 lines

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/