x86: Initialize segment selectors
PATCH 1/1 x86: Initialize segment selectors
Currently, the BSP's segment selectors are not initialized in 32-bit (cstart.S). As a result the tests implicitly rely on the segment selector values that are set by the BIOS. If this assumption is not kept, the task-switch test fails.
Fix it by initializing them.
--- x86/cstart.S | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
x86/cstart.S +11 -6
@@ -94,6 +94,15 @@ MSR_GS_BASE = 0xc0000101 wrmsr .endm +.macro setup_segments + mov $0x10, %ax + mov %ax, %ds + mov %ax, %es + mov %ax, %fs + mov %ax, %gs + mov %ax, %ss +.endm + .globl start start: mov $stacktop, %esp
@@ -109,6 +118,7 @@ start: prepare_32: lgdtl gdt32_descr + setup_segments mov %cr4, %eax bts $4, %eax // pse
@@ -133,12 +143,7 @@ save_id: retl ap_start32: - mov $0x10, %ax - mov %ax, %ds - mov %ax, %es - mov %ax, %fs - mov %ax, %gs - mov %ax, %ss + setup_segments mov $-4096, %esp lock/xaddl %esp, smp_stacktop setup_percpu_area
Signature · 1 line
2.20.1
Quoted 9 lines · Nadav Amit
Currently, the BSP's segment selectors are not initialized in 32-bit (
cstart.S). As a result the tests implicitly rely on the segment selector values that are set by the BIOS. If this assumption is not kept, the task-switch test fails.Fix it by initializing them.
Signed-off-by Nadav Amit <[email protected]>
Quoted 45 lines · Nadav Amit
Currently, the BSP's segment selectors are not initialized in 32-bit (
cstart.S). As a result the tests implicitly rely on the segment selector values that are set by the BIOS. If this assumption is not kept, the task-switch test fails.Fix it by initializing them.
Signed-off-by Nadav Amit <[email protected]>--- x86/cstart.S | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)x86/cstart.S +10
-0@@ -94,6 +94,15 @@ MSR_GS_BASE = 0xc0000101 wrmsr .endm +.macro setup_segments + mov $0x10, %ax + mov %ax, %ds + mov %ax, %es + mov %ax, %fs + mov %ax, %gs + mov %ax, %ss +.endm + .globl start start: mov $stacktop, %esp@@ -109,6 +118,7 @@ start: prepare_32: lgdtl gdt32_descr + setup_segments mov %cr4, %eax bts $4, %eax // pse@@ -133,12 +143,7 @@ save_id: retl ap_start32:
x86/cstart.S save_id: · line 149
Quoted 10 lines
x86/cstart.S +1
-6- mov $0x10, %ax - mov %ax, %ds - mov %ax, %es - mov %ax, %fs - mov %ax, %gs - mov %ax, %ss + setup_segments mov $-4096, %esp lock/xaddl %esp, smp_stacktop setup_percpu_area
Applied, thanks.
Paolo
Quoted 11 lines · Nadav Amit
Currently, the BSP's segment selectors are not initialized in 32-bit (
cstart.S). As a result the tests implicitly rely on the segment selector values that are set by the BIOS. If this assumption is not kept, the task-switch test fails.Fix it by initializing them.
Signed-off-by Nadav Amit <[email protected]>--- x86/cstart.S | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
I'm sorry to be the bearer of bad news again, but this commit broke another set of tests in the Travis CI:
https://travis-ci.com/github/huth/kvm-unit-tests/jobs/353103187#L796
smptest, smptest3, kvmclock_test, hyperv_synic and hyperv_stimer are failing now in the 32-bit kvm-unit-tests :-(
Thomas
Paolo Bonzini
maintainerQuoted 13 lines · Thomas Huth
Quoted 11 lines · Nadav Amit
Currently, the BSP's segment selectors are not initialized in 32-bit (
cstart.S). As a result the tests implicitly rely on the segment selector values that are set by the BIOS. If this assumption is not kept, the task-switch test fails.Fix it by initializing them.
Signed-off-by Nadav Amit <[email protected]>---
x86/cstart.S| 17 +++++++++++------1 file changed, 11 insertions(+), 6 deletions(-)
Quoted 7 lines
I'm sorry to be the bearer of bad news again, but this commit broke another set of tests in the Travis CI:
https://travis-ci.com/github/huth/kvm-unit-tests/jobs/353103187#L796
smptest, smptest3,
kvmclock_test,hyperv_synicandhyperv_stimerare failing now in the 32-bit kvm-unit-tests :-(
And that's just bad testing (both Nadav's and mine). Writing to %gs clobbers the PERCPU area. The fix is as simple as this:
x86/cstart.S +1 -1
@@ -106,6 +106,7 @@ MSR_GS_BASE = 0xc0000101 .globl start start: mov $stacktop, %esp + setup_segments push %ebx call setup_multiboot call setup_libcflat
@@ -118,7 +119,6 @@ start: prepare_32: lgdtl gdt32_descr - setup_segments mov %cr4, %eax bts $4, %eax // pse
Quoted 45 lines · Paolo Bonzini
Quoted 13 lines · Thomas Huth
Quoted 11 lines · Nadav Amit
Currently, the BSP's segment selectors are not initialized in 32-bit (
cstart.S). As a result the tests implicitly rely on the segment selector values that are set by the BIOS. If this assumption is not kept, the task-switch test fails.Fix it by initializing them.
Signed-off-by Nadav Amit <[email protected]>--- x86/cstart.S | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)Quoted 7 lines
I'm sorry to be the bearer of bad news again, but this commit broke another set of tests in the Travis CI:
https://travis-ci.com/github/huth/kvm-unit-tests/jobs/353103187#L796
smptest, smptest3,
kvmclock_test,hyperv_synicandhyperv_stimerare failing now in the 32-bit kvm-unit-tests :-(And that's just bad testing (both Nadav's and mine). Writing to %gs clobbers the PERCPU area. The fix is as simple as this:
x86/cstart.S +0
-0@@ -106,6 +106,7 @@ MSR_GS_BASE = 0xc0000101.globl start
start:
mov $stacktop, %espx86/cstart.S +1
-0+ setup_segments push %ebx call setup_multiboot call setup_libcflat@@ -118,7 +119,6 @@ start:
prepare_32:
lgdtlgdt32_descrx86/cstart.S +0
-1- setup_segments
Quoted 2 lines
mov %cr4, %eax bts $4, %eax // pse
Unfortunately this change does not work for me (i.e., it breaks the task-switch test on my setup). Admittedly, my patch was wrong.
I actually did not notice this scheme of having the GS descriptor and GS-base out of sync. It seems very fragile, specifically when you have a task-switch that reloads the GS and overwrites the original value.
I will try to investigate it further later.
Paolo Bonzini
maintainerQuoted 8 lines · Nadav Amit
Unfortunately this change does not work for me (i.e., it breaks the task-switch test on my setup). Admittedly, my patch was wrong.
I actually did not notice this scheme of having the GS descriptor and GS-base out of sync. It seems very fragile, specifically when you have a task-switch that reloads the GS and overwrites the original value.
I will try to investigate it further later.
Yeah, it's fragile but it's limited to the task switch tests; and it's the only way to do it without having a separate selector or LDT for each CPU.
Paolo