* [kvm-unit-tests PATCH 0/8] x86: APIC bug fix and cleanup
@ 2022-01-21 23:18 Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 1/8] x86: Always use legacy xAPIC to get APIC ID during TSS setup Sean Christopherson
` (7 more replies)
0 siblings, 8 replies; 10+ messages in thread
From: Sean Christopherson @ 2022-01-21 23:18 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Sean Christopherson
Fix a bug introduced by the UEFI support where setup_tss() can race with
enable_x2apic() and crash the test due to attempting to read an x2APIC MSR
prior to enabling x2APIC (failure manifests in smptest3).
In an attempt to avoid similar bugs in the future, clean up the per-cpu
stuff and convert apic_ops into a per-cpu pointer. 32-bit KUT has a
chick-and-egg problem due to using the APIC ID to choose the selector
for GS (the per-cpu segment), so the original bug "has" to say on a
dedicated helper.
Sean Christopherson (8):
x86: Always use legacy xAPIC to get APIC ID during TSS setup
x86: nVMX: Load actual GS.base for both guest and host
x86: smp: Replace spaces with tabs
x86: desc: Replace spaces with tabs
x86: Add proper helpers for per-cpu reads/writes
x86: apic: Replace spaces with tabs
x86: apic: Track APIC ops on a per-cpu basis
x86: apic: Make xAPIC and I/O APIC pointers static
lib/x86/apic-defs.h | 3 +-
lib/x86/apic.c | 157 ++++++++++++++++++++++++--------------------
lib/x86/apic.h | 5 +-
lib/x86/desc.c | 120 ++++++++++++++++-----------------
lib/x86/desc.h | 68 +++++++++----------
lib/x86/setup.c | 4 +-
lib/x86/smp.c | 128 +++++++++++++++++-------------------
lib/x86/smp.h | 67 +++++++++++++++++++
x86/vmx.c | 4 +-
9 files changed, 311 insertions(+), 245 deletions(-)
base-commit: 3df301615cead4142fe28629d86142de32fc6768
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply [flat|nested] 10+ messages in thread
* [kvm-unit-tests PATCH 1/8] x86: Always use legacy xAPIC to get APIC ID during TSS setup
2022-01-21 23:18 [kvm-unit-tests PATCH 0/8] x86: APIC bug fix and cleanup Sean Christopherson
@ 2022-01-21 23:18 ` Sean Christopherson
2022-01-22 1:18 ` David Matlack
2022-01-21 23:18 ` [kvm-unit-tests PATCH 2/8] x86: nVMX: Load actual GS.base for both guest and host Sean Christopherson
` (6 subsequent siblings)
7 siblings, 1 reply; 10+ messages in thread
From: Sean Christopherson @ 2022-01-21 23:18 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Sean Christopherson
Force use of xAPIC to retrieve the APIC ID during TSS setup to fix an
issue where an AP can switch apic_ops to point at x2apic_ops before
setup_tss() completes, leading to a #GP and triple fault due to trying
to read an x2APIC MSR without x2APIC being enabled.
A future patch will make apic_ops a per-cpu pointer, but that's not of
any help for 32-bit, which uses the APIC ID to determine the GS selector,
i.e. 32-bit KUT has a chicken-and-egg problem. All setup_tss() callers
ensure the local APIC is in xAPIC mode, so just force use of xAPIC in
this case.
Fixes: 7e33895 ("x86: Move 32-bit GDT and TSS to desc.c")
Fixes: dbd3800 ("x86: Move 64-bit GDT and TSS to desc.c")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
lib/x86/apic.c | 5 +++++
lib/x86/apic.h | 2 ++
lib/x86/setup.c | 4 ++--
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/lib/x86/apic.c b/lib/x86/apic.c
index da8f3013..b404d580 100644
--- a/lib/x86/apic.c
+++ b/lib/x86/apic.c
@@ -48,6 +48,11 @@ static uint32_t xapic_id(void)
return xapic_read(APIC_ID) >> 24;
}
+uint32_t pre_boot_apic_id(void)
+{
+ return xapic_id();
+}
+
static const struct apic_ops xapic_ops = {
.reg_read = xapic_read,
.reg_write = xapic_write,
diff --git a/lib/x86/apic.h b/lib/x86/apic.h
index c4821716..7844324b 100644
--- a/lib/x86/apic.h
+++ b/lib/x86/apic.h
@@ -53,6 +53,8 @@ bool apic_read_bit(unsigned reg, int n);
void apic_write(unsigned reg, uint32_t val);
void apic_icr_write(uint32_t val, uint32_t dest);
uint32_t apic_id(void);
+uint32_t pre_boot_apic_id(void);
+
int enable_x2apic(void);
void disable_apic(void);
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index bbd34682..64217add 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -109,7 +109,7 @@ unsigned long setup_tss(u8 *stacktop)
u32 id;
tss64_t *tss_entry;
- id = apic_id();
+ id = pre_boot_apic_id();
/* Runtime address of current TSS */
tss_entry = &tss[id];
@@ -129,7 +129,7 @@ unsigned long setup_tss(u8 *stacktop)
u32 id;
tss32_t *tss_entry;
- id = apic_id();
+ id = pre_boot_apic_id();
/* Runtime address of current TSS */
tss_entry = &tss[id];
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [kvm-unit-tests PATCH 2/8] x86: nVMX: Load actual GS.base for both guest and host
2022-01-21 23:18 [kvm-unit-tests PATCH 0/8] x86: APIC bug fix and cleanup Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 1/8] x86: Always use legacy xAPIC to get APIC ID during TSS setup Sean Christopherson
@ 2022-01-21 23:18 ` Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 3/8] x86: smp: Replace spaces with tabs Sean Christopherson
` (5 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Sean Christopherson @ 2022-01-21 23:18 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Sean Christopherson
Load KUT's actual GS.base on VM-Entry and VM-Exit so that the VMX tests
can access per-cpu data. A future commit will track xAPIC vs. x2APIC ops
on a per-cpu basis.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
x86/vmx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/x86/vmx.c b/x86/vmx.c
index f4fbb94d..756deb38 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1209,7 +1209,7 @@ static void init_vmcs_host(void)
vmcs_write(HOST_BASE_GDTR, gdt_descr.base);
vmcs_write(HOST_BASE_IDTR, idt_descr.base);
vmcs_write(HOST_BASE_FS, 0);
- vmcs_write(HOST_BASE_GS, 0);
+ vmcs_write(HOST_BASE_GS, rdmsr(MSR_GS_BASE));
/* Set other vmcs area */
vmcs_write(PF_ERROR_MASK, 0);
@@ -1261,7 +1261,7 @@ static void init_vmcs_guest(void)
vmcs_write(GUEST_BASE_SS, 0);
vmcs_write(GUEST_BASE_DS, 0);
vmcs_write(GUEST_BASE_FS, 0);
- vmcs_write(GUEST_BASE_GS, 0);
+ vmcs_write(GUEST_BASE_GS, rdmsr(MSR_GS_BASE));
vmcs_write(GUEST_BASE_TR, get_gdt_entry_base(tss_descr));
vmcs_write(GUEST_BASE_LDTR, 0);
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [kvm-unit-tests PATCH 3/8] x86: smp: Replace spaces with tabs
2022-01-21 23:18 [kvm-unit-tests PATCH 0/8] x86: APIC bug fix and cleanup Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 1/8] x86: Always use legacy xAPIC to get APIC ID during TSS setup Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 2/8] x86: nVMX: Load actual GS.base for both guest and host Sean Christopherson
@ 2022-01-21 23:18 ` Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 4/8] x86: desc: " Sean Christopherson
` (4 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Sean Christopherson @ 2022-01-21 23:18 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Sean Christopherson
Replace spaces with tabs in smp.c, and opportunistically clean up a
handful of minor coding style violations.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
lib/x86/smp.c | 129 +++++++++++++++++++++++++-------------------------
1 file changed, 64 insertions(+), 65 deletions(-)
diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index 2ac0ef74..b24675fd 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -21,128 +21,127 @@ static atomic_t active_cpus;
static __attribute__((used)) void ipi(void)
{
- void (*function)(void *data) = ipi_function;
- void *data = ipi_data;
- bool wait = ipi_wait;
+ void (*function)(void *data) = ipi_function;
+ void *data = ipi_data;
+ bool wait = ipi_wait;
- if (!wait) {
- ipi_done = 1;
- apic_write(APIC_EOI, 0);
- }
- function(data);
- atomic_dec(&active_cpus);
- if (wait) {
- ipi_done = 1;
- apic_write(APIC_EOI, 0);
- }
+ if (!wait) {
+ ipi_done = 1;
+ apic_write(APIC_EOI, 0);
+ }
+ function(data);
+ atomic_dec(&active_cpus);
+ if (wait) {
+ ipi_done = 1;
+ apic_write(APIC_EOI, 0);
+ }
}
asm (
- "ipi_entry: \n"
- " call ipi \n"
+ "ipi_entry: \n"
+ " call ipi \n"
#ifndef __x86_64__
- " iret"
+ " iret"
#else
- " iretq"
+ " iretq"
#endif
- );
+ );
int cpu_count(void)
{
- return _cpu_count;
+ return _cpu_count;
}
int smp_id(void)
{
- unsigned id;
+ unsigned id;
- asm ("mov %%gs:0, %0" : "=r"(id));
- return id;
+ asm ("mov %%gs:0, %0" : "=r"(id));
+ return id;
}
static void setup_smp_id(void *data)
{
- asm ("mov %0, %%gs:0" : : "r"(apic_id()) : "memory");
+ asm ("mov %0, %%gs:0" : : "r"(apic_id()) : "memory");
}
-static void __on_cpu(int cpu, void (*function)(void *data), void *data,
- int wait)
+static void __on_cpu(int cpu, void (*function)(void *data), void *data, int wait)
{
- unsigned int target = id_map[cpu];
+ const u32 ipi_icr = APIC_INT_ASSERT | APIC_DEST_PHYSICAL | APIC_DM_FIXED | IPI_VECTOR;
+ unsigned int target = id_map[cpu];
- spin_lock(&ipi_lock);
- if (target == smp_id())
- function(data);
- else {
- atomic_inc(&active_cpus);
- ipi_done = 0;
- ipi_function = function;
- ipi_data = data;
- ipi_wait = wait;
- apic_icr_write(APIC_INT_ASSERT | APIC_DEST_PHYSICAL | APIC_DM_FIXED
- | IPI_VECTOR, target);
- while (!ipi_done)
- ;
- }
- spin_unlock(&ipi_lock);
+ spin_lock(&ipi_lock);
+ if (target == smp_id()) {
+ function(data);
+ } else {
+ atomic_inc(&active_cpus);
+ ipi_done = 0;
+ ipi_function = function;
+ ipi_data = data;
+ ipi_wait = wait;
+ apic_icr_write(ipi_icr, target);
+ while (!ipi_done)
+ ;
+ }
+ spin_unlock(&ipi_lock);
}
void on_cpu(int cpu, void (*function)(void *data), void *data)
{
- __on_cpu(cpu, function, data, 1);
+ __on_cpu(cpu, function, data, 1);
}
void on_cpu_async(int cpu, void (*function)(void *data), void *data)
{
- __on_cpu(cpu, function, data, 0);
+ __on_cpu(cpu, function, data, 0);
}
void on_cpus(void (*function)(void *data), void *data)
{
- int cpu;
+ int cpu;
- for (cpu = cpu_count() - 1; cpu >= 0; --cpu)
- on_cpu_async(cpu, function, data);
+ for (cpu = cpu_count() - 1; cpu >= 0; --cpu)
+ on_cpu_async(cpu, function, data);
- while (cpus_active() > 1)
- pause();
+ while (cpus_active() > 1)
+ pause();
}
int cpus_active(void)
{
- return atomic_read(&active_cpus);
+ return atomic_read(&active_cpus);
}
void smp_init(void)
{
- int i;
- void ipi_entry(void);
+ int i;
+ void ipi_entry(void);
- _cpu_count = fwcfg_get_nb_cpus();
+ _cpu_count = fwcfg_get_nb_cpus();
- setup_idt();
- init_apic_map();
- set_idt_entry(IPI_VECTOR, ipi_entry, 0);
+ setup_idt();
+ init_apic_map();
+ set_idt_entry(IPI_VECTOR, ipi_entry, 0);
- setup_smp_id(0);
- for (i = 1; i < cpu_count(); ++i)
- on_cpu(i, setup_smp_id, 0);
+ setup_smp_id(0);
+ for (i = 1; i < cpu_count(); ++i)
+ on_cpu(i, setup_smp_id, 0);
- atomic_inc(&active_cpus);
+ atomic_inc(&active_cpus);
}
static void do_reset_apic(void *data)
{
- reset_apic();
+ reset_apic();
}
void smp_reset_apic(void)
{
- int i;
+ int i;
- reset_apic();
- for (i = 1; i < cpu_count(); ++i)
- on_cpu(i, do_reset_apic, 0);
+ reset_apic();
+ for (i = 1; i < cpu_count(); ++i)
+ on_cpu(i, do_reset_apic, 0);
- atomic_inc(&active_cpus);
+ atomic_inc(&active_cpus);
}
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [kvm-unit-tests PATCH 4/8] x86: desc: Replace spaces with tabs
2022-01-21 23:18 [kvm-unit-tests PATCH 0/8] x86: APIC bug fix and cleanup Sean Christopherson
` (2 preceding siblings ...)
2022-01-21 23:18 ` [kvm-unit-tests PATCH 3/8] x86: smp: Replace spaces with tabs Sean Christopherson
@ 2022-01-21 23:18 ` Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 5/8] x86: Add proper helpers for per-cpu reads/writes Sean Christopherson
` (3 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Sean Christopherson @ 2022-01-21 23:18 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Sean Christopherson
Replace spaces with tabs in smp.c, and opportunistically clean up a
handful of minor coding style violations.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
lib/x86/desc.c | 121 +++++++++++++++++++++++++------------------------
lib/x86/desc.h | 68 +++++++++++++--------------
2 files changed, 95 insertions(+), 94 deletions(-)
diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 16b72562..25c5ac55 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -58,35 +58,35 @@ void do_handle_exception(struct ex_regs *regs);
void set_idt_entry(int vec, void *addr, int dpl)
{
- idt_entry_t *e = &boot_idt[vec];
- memset(e, 0, sizeof *e);
- e->offset0 = (unsigned long)addr;
- e->selector = read_cs();
- e->ist = 0;
- e->type = 14;
- e->dpl = dpl;
- e->p = 1;
- e->offset1 = (unsigned long)addr >> 16;
+ idt_entry_t *e = &boot_idt[vec];
+ memset(e, 0, sizeof *e);
+ e->offset0 = (unsigned long)addr;
+ e->selector = read_cs();
+ e->ist = 0;
+ e->type = 14;
+ e->dpl = dpl;
+ e->p = 1;
+ e->offset1 = (unsigned long)addr >> 16;
#ifdef __x86_64__
- e->offset2 = (unsigned long)addr >> 32;
+ e->offset2 = (unsigned long)addr >> 32;
#endif
}
void set_idt_dpl(int vec, u16 dpl)
{
- idt_entry_t *e = &boot_idt[vec];
- e->dpl = dpl;
+ idt_entry_t *e = &boot_idt[vec];
+ e->dpl = dpl;
}
void set_idt_sel(int vec, u16 sel)
{
- idt_entry_t *e = &boot_idt[vec];
- e->selector = sel;
+ idt_entry_t *e = &boot_idt[vec];
+ e->selector = sel;
}
struct ex_record {
- unsigned long rip;
- unsigned long handler;
+ unsigned long rip;
+ unsigned long handler;
};
extern struct ex_record exception_table_start, exception_table_end;
@@ -154,20 +154,20 @@ void unhandled_exception(struct ex_regs *regs, bool cpu)
static void check_exception_table(struct ex_regs *regs)
{
- struct ex_record *ex;
- unsigned ex_val;
+ struct ex_record *ex;
+ unsigned ex_val;
- ex_val = regs->vector | (regs->error_code << 16) |
+ ex_val = regs->vector | (regs->error_code << 16) |
(((regs->rflags >> 16) & 1) << 8);
- asm("mov %0, %%gs:4" : : "r"(ex_val));
+ asm("mov %0, %%gs:4" : : "r"(ex_val));
- for (ex = &exception_table_start; ex != &exception_table_end; ++ex) {
- if (ex->rip == regs->rip) {
- regs->rip = ex->handler;
- return;
- }
- }
- unhandled_exception(regs, false);
+ for (ex = &exception_table_start; ex != &exception_table_end; ++ex) {
+ if (ex->rip == regs->rip) {
+ regs->rip = ex->handler;
+ return;
+ }
+ }
+ unhandled_exception(regs, false);
}
static handler exception_handlers[32];
@@ -278,51 +278,52 @@ static void *idt_handlers[32] = {
void setup_idt(void)
{
- int i;
- static bool idt_initialized = false;
+ int i;
+ static bool idt_initialized = false;
- if (idt_initialized) {
- return;
- }
- idt_initialized = true;
- for (i = 0; i < 32; i++)
- if (idt_handlers[i])
- set_idt_entry(i, idt_handlers[i], 0);
- handle_exception(0, check_exception_table);
- handle_exception(6, check_exception_table);
- handle_exception(13, check_exception_table);
+ if (idt_initialized)
+ return;
+
+ idt_initialized = true;
+ for (i = 0; i < 32; i++) {
+ if (idt_handlers[i])
+ set_idt_entry(i, idt_handlers[i], 0);
+ }
+ handle_exception(0, check_exception_table);
+ handle_exception(6, check_exception_table);
+ handle_exception(13, check_exception_table);
}
unsigned exception_vector(void)
{
- unsigned char vector;
+ unsigned char vector;
- asm volatile("movb %%gs:4, %0" : "=q"(vector));
- return vector;
+ asm volatile("movb %%gs:4, %0" : "=q"(vector));
+ return vector;
}
int write_cr4_checking(unsigned long val)
{
- asm volatile(ASM_TRY("1f")
- "mov %0,%%cr4\n\t"
- "1:": : "r" (val));
- return exception_vector();
+ asm volatile(ASM_TRY("1f")
+ "mov %0,%%cr4\n\t"
+ "1:": : "r" (val));
+ return exception_vector();
}
unsigned exception_error_code(void)
{
- unsigned short error_code;
+ unsigned short error_code;
- asm volatile("mov %%gs:6, %0" : "=r"(error_code));
- return error_code;
+ asm volatile("mov %%gs:6, %0" : "=r"(error_code));
+ return error_code;
}
bool exception_rflags_rf(void)
{
- unsigned char rf_flag;
+ unsigned char rf_flag;
- asm volatile("movb %%gs:5, %b0" : "=q"(rf_flag));
- return rf_flag & 1;
+ asm volatile("movb %%gs:5, %b0" : "=q"(rf_flag));
+ return rf_flag & 1;
}
static char intr_alt_stack[4096];
@@ -352,20 +353,20 @@ void set_gdt_entry(int sel, unsigned long base, u32 limit, u8 type, u8 flags)
#ifndef __x86_64__
void set_gdt_task_gate(u16 sel, u16 tss_sel)
{
- set_gdt_entry(sel, tss_sel, 0, 0x85, 0); // task, present
+ set_gdt_entry(sel, tss_sel, 0, 0x85, 0); // task, present
}
void set_idt_task_gate(int vec, u16 sel)
{
- idt_entry_t *e = &boot_idt[vec];
+ idt_entry_t *e = &boot_idt[vec];
- memset(e, 0, sizeof *e);
+ memset(e, 0, sizeof *e);
- e->selector = sel;
- e->ist = 0;
- e->type = 5;
- e->dpl = 0;
- e->p = 1;
+ e->selector = sel;
+ e->ist = 0;
+ e->type = 5;
+ e->dpl = 0;
+ e->p = 1;
}
/*
diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index 9b81da0c..2660300b 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -7,20 +7,20 @@ void setup_idt(void);
void setup_alt_stack(void);
struct ex_regs {
- unsigned long rax, rcx, rdx, rbx;
- unsigned long dummy, rbp, rsi, rdi;
+ unsigned long rax, rcx, rdx, rbx;
+ unsigned long dummy, rbp, rsi, rdi;
#ifdef __x86_64__
- unsigned long r8, r9, r10, r11;
- unsigned long r12, r13, r14, r15;
+ unsigned long r8, r9, r10, r11;
+ unsigned long r12, r13, r14, r15;
#endif
- unsigned long vector;
- unsigned long error_code;
- unsigned long rip;
- unsigned long cs;
- unsigned long rflags;
+ unsigned long vector;
+ unsigned long error_code;
+ unsigned long rip;
+ unsigned long cs;
+ unsigned long rflags;
#ifdef __x86_64__
- unsigned long rsp;
- unsigned long ss;
+ unsigned long rsp;
+ unsigned long ss;
#endif
};
@@ -80,19 +80,19 @@ typedef struct __attribute__((packed)) {
} tss64_t;
#ifdef __x86_64
-#define ASM_TRY(catch) \
- "movl $0, %%gs:4 \n\t" \
- ".pushsection .data.ex \n\t" \
- ".quad 1111f, " catch "\n\t" \
- ".popsection \n\t" \
- "1111:"
+#define ASM_TRY(catch) \
+ "movl $0, %%gs:4 \n\t" \
+ ".pushsection .data.ex \n\t" \
+ ".quad 1111f, " catch "\n\t" \
+ ".popsection \n\t" \
+ "1111:"
#else
-#define ASM_TRY(catch) \
- "movl $0, %%gs:4 \n\t" \
- ".pushsection .data.ex \n\t" \
- ".long 1111f, " catch "\n\t" \
- ".popsection \n\t" \
- "1111:"
+#define ASM_TRY(catch) \
+ "movl $0, %%gs:4 \n\t" \
+ ".pushsection .data.ex \n\t" \
+ ".long 1111f, " catch "\n\t" \
+ ".popsection \n\t" \
+ "1111:"
#endif
/*
@@ -152,18 +152,18 @@ typedef struct __attribute__((packed)) {
#define TSS_MAIN 0x80
typedef struct {
- unsigned short offset0;
- unsigned short selector;
- unsigned short ist : 3;
- unsigned short : 5;
- unsigned short type : 4;
- unsigned short : 1;
- unsigned short dpl : 2;
- unsigned short p : 1;
- unsigned short offset1;
+ unsigned short offset0;
+ unsigned short selector;
+ unsigned short ist : 3;
+ unsigned short : 5;
+ unsigned short type : 4;
+ unsigned short : 1;
+ unsigned short dpl : 2;
+ unsigned short p : 1;
+ unsigned short offset1;
#ifdef __x86_64__
- unsigned offset2;
- unsigned reserved;
+ unsigned offset2;
+ unsigned reserved;
#endif
} idt_entry_t;
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [kvm-unit-tests PATCH 5/8] x86: Add proper helpers for per-cpu reads/writes
2022-01-21 23:18 [kvm-unit-tests PATCH 0/8] x86: APIC bug fix and cleanup Sean Christopherson
` (3 preceding siblings ...)
2022-01-21 23:18 ` [kvm-unit-tests PATCH 4/8] x86: desc: " Sean Christopherson
@ 2022-01-21 23:18 ` Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 6/8] x86: apic: Replace spaces with tabs Sean Christopherson
` (2 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Sean Christopherson @ 2022-01-21 23:18 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Sean Christopherson
Add helpers to read/write per-cpu data instead of open coding access
with gs: and magic numbers. Keeping track of what offsets are used for
what and by whom is a nightmare.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
lib/x86/desc.c | 23 ++++++------------
lib/x86/smp.c | 7 ++----
lib/x86/smp.h | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 74 insertions(+), 21 deletions(-)
diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 25c5ac55..22ff59e9 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -1,6 +1,7 @@
#include "libcflat.h"
#include "desc.h"
#include "processor.h"
+#include "smp.h"
#include <setjmp.h>
#include "apic-defs.h"
@@ -155,11 +156,10 @@ void unhandled_exception(struct ex_regs *regs, bool cpu)
static void check_exception_table(struct ex_regs *regs)
{
struct ex_record *ex;
- unsigned ex_val;
- ex_val = regs->vector | (regs->error_code << 16) |
- (((regs->rflags >> 16) & 1) << 8);
- asm("mov %0, %%gs:4" : : "r"(ex_val));
+ this_cpu_write_exception_vector(regs->vector);
+ this_cpu_write_exception_rflags_rf((regs->rflags >> 16) & 1);
+ this_cpu_write_exception_error_code(regs->error_code);
for (ex = &exception_table_start; ex != &exception_table_end; ++ex) {
if (ex->rip == regs->rip) {
@@ -296,10 +296,7 @@ void setup_idt(void)
unsigned exception_vector(void)
{
- unsigned char vector;
-
- asm volatile("movb %%gs:4, %0" : "=q"(vector));
- return vector;
+ return this_cpu_read_exception_vector();
}
int write_cr4_checking(unsigned long val)
@@ -312,18 +309,12 @@ int write_cr4_checking(unsigned long val)
unsigned exception_error_code(void)
{
- unsigned short error_code;
-
- asm volatile("mov %%gs:6, %0" : "=r"(error_code));
- return error_code;
+ return this_cpu_read_exception_error_code();
}
bool exception_rflags_rf(void)
{
- unsigned char rf_flag;
-
- asm volatile("movb %%gs:5, %b0" : "=q"(rf_flag));
- return rf_flag & 1;
+ return this_cpu_read_exception_rflags_rf() & 1;
}
static char intr_alt_stack[4096];
diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index b24675fd..683b25d1 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -54,15 +54,12 @@ int cpu_count(void)
int smp_id(void)
{
- unsigned id;
-
- asm ("mov %%gs:0, %0" : "=r"(id));
- return id;
+ return this_cpu_read_smp_id();
}
static void setup_smp_id(void *data)
{
- asm ("mov %0, %%gs:0" : : "r"(apic_id()) : "memory");
+ this_cpu_write_smp_id(apic_id());
}
static void __on_cpu(int cpu, void (*function)(void *data), void *data, int wait)
diff --git a/lib/x86/smp.h b/lib/x86/smp.h
index f74845e6..eb037a46 100644
--- a/lib/x86/smp.h
+++ b/lib/x86/smp.h
@@ -1,7 +1,72 @@
#ifndef _X86_SMP_H_
#define _X86_SMP_H_
+
+#include <stddef.h>
#include <asm/spinlock.h>
+/* Offsets into the per-cpu page. */
+struct percpu_data {
+ uint32_t smp_id;
+ union {
+ struct {
+ uint8_t exception_vector;
+ uint8_t exception_rflags_rf;
+ uint16_t exception_error_code;
+ };
+ uint32_t exception_data;
+ };
+};
+
+#define typeof_percpu(name) typeof(((struct percpu_data *)0)->name)
+#define offsetof_percpu(name) offsetof(struct percpu_data, name)
+
+#define BUILD_PERCPU_OP(name) \
+static inline typeof_percpu(name) this_cpu_read_##name(void) \
+{ \
+ typeof_percpu(name) val; \
+ \
+ switch (sizeof(val)) { \
+ case 1: \
+ asm("movb %%gs:%c1, %0" : "=q" (val) : "i" (offsetof_percpu(name))); \
+ break; \
+ case 2: \
+ asm("movw %%gs:%c1, %0" : "=r" (val) : "i" (offsetof_percpu(name))); \
+ break; \
+ case 4: \
+ asm("movl %%gs:%c1, %0" : "=r" (val) : "i" (offsetof_percpu(name))); \
+ break; \
+ case 8: \
+ asm("movq %%gs:%c1, %0" : "=r" (val) : "i" (offsetof_percpu(name))); \
+ break; \
+ default: \
+ asm volatile("ud2"); \
+ } \
+ return val; \
+} \
+static inline void this_cpu_write_##name(typeof_percpu(name) val) \
+{ \
+ switch (sizeof(val)) { \
+ case 1: \
+ asm("movb %0, %%gs:%c1" :: "q" (val), "i" (offsetof_percpu(name))); \
+ break; \
+ case 2: \
+ asm("movw %0, %%gs:%c1" :: "r" (val), "i" (offsetof_percpu(name))); \
+ break; \
+ case 4: \
+ asm("movl %0, %%gs:%c1" :: "r" (val), "i" (offsetof_percpu(name))); \
+ break; \
+ case 8: \
+ asm("movq %0, %%gs:%c1" :: "r" (val), "i" (offsetof_percpu(name))); \
+ break; \
+ default: \
+ asm volatile("ud2"); \
+ } \
+}
+BUILD_PERCPU_OP(smp_id);
+BUILD_PERCPU_OP(exception_vector);
+BUILD_PERCPU_OP(exception_rflags_rf);
+BUILD_PERCPU_OP(exception_error_code);
+
void smp_init(void);
int cpu_count(void);
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [kvm-unit-tests PATCH 6/8] x86: apic: Replace spaces with tabs
2022-01-21 23:18 [kvm-unit-tests PATCH 0/8] x86: APIC bug fix and cleanup Sean Christopherson
` (4 preceding siblings ...)
2022-01-21 23:18 ` [kvm-unit-tests PATCH 5/8] x86: Add proper helpers for per-cpu reads/writes Sean Christopherson
@ 2022-01-21 23:18 ` Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 7/8] x86: apic: Track APIC ops on a per-cpu basis Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 8/8] x86: apic: Make xAPIC and I/O APIC pointers static Sean Christopherson
7 siblings, 0 replies; 10+ messages in thread
From: Sean Christopherson @ 2022-01-21 23:18 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Sean Christopherson
Replace spaces with tabs in apic.c. No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
lib/x86/apic.c | 138 ++++++++++++++++++++++++-------------------------
1 file changed, 69 insertions(+), 69 deletions(-)
diff --git a/lib/x86/apic.c b/lib/x86/apic.c
index b404d580..44a6ad38 100644
--- a/lib/x86/apic.c
+++ b/lib/x86/apic.c
@@ -9,43 +9,43 @@ void *g_ioapic = (void *)0xfec00000;
u8 id_map[MAX_TEST_CPUS];
struct apic_ops {
- u32 (*reg_read)(unsigned reg);
- void (*reg_write)(unsigned reg, u32 val);
- void (*icr_write)(u32 val, u32 dest);
- u32 (*id)(void);
+ u32 (*reg_read)(unsigned reg);
+ void (*reg_write)(unsigned reg, u32 val);
+ void (*icr_write)(u32 val, u32 dest);
+ u32 (*id)(void);
};
static void outb(unsigned char data, unsigned short port)
{
- asm volatile ("out %0, %1" : : "a"(data), "d"(port));
+ asm volatile ("out %0, %1" : : "a"(data), "d"(port));
}
void eoi(void)
{
- apic_write(APIC_EOI, 0);
+ apic_write(APIC_EOI, 0);
}
static u32 xapic_read(unsigned reg)
{
- return *(volatile u32 *)(g_apic + reg);
+ return *(volatile u32 *)(g_apic + reg);
}
static void xapic_write(unsigned reg, u32 val)
{
- *(volatile u32 *)(g_apic + reg) = val;
+ *(volatile u32 *)(g_apic + reg) = val;
}
static void xapic_icr_write(u32 val, u32 dest)
{
- while (xapic_read(APIC_ICR) & APIC_ICR_BUSY)
- ;
- xapic_write(APIC_ICR2, dest << 24);
- xapic_write(APIC_ICR, val);
+ while (xapic_read(APIC_ICR) & APIC_ICR_BUSY)
+ ;
+ xapic_write(APIC_ICR2, dest << 24);
+ xapic_write(APIC_ICR, val);
}
static uint32_t xapic_id(void)
{
- return xapic_read(APIC_ID) >> 24;
+ return xapic_read(APIC_ID) >> 24;
}
uint32_t pre_boot_apic_id(void)
@@ -54,71 +54,71 @@ uint32_t pre_boot_apic_id(void)
}
static const struct apic_ops xapic_ops = {
- .reg_read = xapic_read,
- .reg_write = xapic_write,
- .icr_write = xapic_icr_write,
- .id = xapic_id,
+ .reg_read = xapic_read,
+ .reg_write = xapic_write,
+ .icr_write = xapic_icr_write,
+ .id = xapic_id,
};
static const struct apic_ops *apic_ops = &xapic_ops;
static u32 x2apic_read(unsigned reg)
{
- unsigned a, d;
+ unsigned a, d;
- asm volatile ("rdmsr" : "=a"(a), "=d"(d) : "c"(APIC_BASE_MSR + reg/16));
- return a | (u64)d << 32;
+ asm volatile ("rdmsr" : "=a"(a), "=d"(d) : "c"(APIC_BASE_MSR + reg/16));
+ return a | (u64)d << 32;
}
static void x2apic_write(unsigned reg, u32 val)
{
- asm volatile ("wrmsr" : : "a"(val), "d"(0), "c"(APIC_BASE_MSR + reg/16));
+ asm volatile ("wrmsr" : : "a"(val), "d"(0), "c"(APIC_BASE_MSR + reg/16));
}
static void x2apic_icr_write(u32 val, u32 dest)
{
- mb();
- asm volatile ("wrmsr" : : "a"(val), "d"(dest),
- "c"(APIC_BASE_MSR + APIC_ICR/16));
+ mb();
+ asm volatile ("wrmsr" : : "a"(val), "d"(dest),
+ "c"(APIC_BASE_MSR + APIC_ICR/16));
}
static uint32_t x2apic_id(void)
{
- return x2apic_read(APIC_ID);
+ return x2apic_read(APIC_ID);
}
static const struct apic_ops x2apic_ops = {
- .reg_read = x2apic_read,
- .reg_write = x2apic_write,
- .icr_write = x2apic_icr_write,
- .id = x2apic_id,
+ .reg_read = x2apic_read,
+ .reg_write = x2apic_write,
+ .icr_write = x2apic_icr_write,
+ .id = x2apic_id,
};
u32 apic_read(unsigned reg)
{
- return apic_ops->reg_read(reg);
+ return apic_ops->reg_read(reg);
}
void apic_write(unsigned reg, u32 val)
{
- apic_ops->reg_write(reg, val);
+ apic_ops->reg_write(reg, val);
}
bool apic_read_bit(unsigned reg, int n)
{
- reg += (n >> 5) << 4;
- n &= 31;
- return (apic_read(reg) & (1 << n)) != 0;
+ reg += (n >> 5) << 4;
+ n &= 31;
+ return (apic_read(reg) & (1 << n)) != 0;
}
void apic_icr_write(u32 val, u32 dest)
{
- apic_ops->icr_write(val, dest);
+ apic_ops->icr_write(val, dest);
}
uint32_t apic_id(void)
{
- return apic_ops->id();
+ return apic_ops->id();
}
uint8_t apic_get_tpr(void)
@@ -144,59 +144,59 @@ void apic_set_tpr(uint8_t tpr)
int enable_x2apic(void)
{
- unsigned a, b, c, d;
+ unsigned a, b, c, d;
- asm ("cpuid" : "=a"(a), "=b"(b), "=c"(c), "=d"(d) : "0"(1));
+ asm ("cpuid" : "=a"(a), "=b"(b), "=c"(c), "=d"(d) : "0"(1));
- if (c & (1 << 21)) {
- asm ("rdmsr" : "=a"(a), "=d"(d) : "c"(MSR_IA32_APICBASE));
- a |= 1 << 10;
- asm ("wrmsr" : : "a"(a), "d"(d), "c"(MSR_IA32_APICBASE));
- apic_ops = &x2apic_ops;
- return 1;
- } else {
- return 0;
- }
+ if (c & (1 << 21)) {
+ asm ("rdmsr" : "=a"(a), "=d"(d) : "c"(MSR_IA32_APICBASE));
+ a |= 1 << 10;
+ asm ("wrmsr" : : "a"(a), "d"(d), "c"(MSR_IA32_APICBASE));
+ apic_ops = &x2apic_ops;
+ return 1;
+ } else {
+ return 0;
+ }
}
void disable_apic(void)
{
- wrmsr(MSR_IA32_APICBASE, rdmsr(MSR_IA32_APICBASE) & ~(APIC_EN | APIC_EXTD));
- apic_ops = &xapic_ops;
+ wrmsr(MSR_IA32_APICBASE, rdmsr(MSR_IA32_APICBASE) & ~(APIC_EN | APIC_EXTD));
+ apic_ops = &xapic_ops;
}
void reset_apic(void)
{
- disable_apic();
- wrmsr(MSR_IA32_APICBASE, rdmsr(MSR_IA32_APICBASE) | APIC_EN);
- xapic_write(APIC_SPIV, 0x1ff);
+ disable_apic();
+ wrmsr(MSR_IA32_APICBASE, rdmsr(MSR_IA32_APICBASE) | APIC_EN);
+ xapic_write(APIC_SPIV, 0x1ff);
}
u32 ioapic_read_reg(unsigned reg)
{
- *(volatile u32 *)g_ioapic = reg;
- return *(volatile u32 *)(g_ioapic + 0x10);
+ *(volatile u32 *)g_ioapic = reg;
+ return *(volatile u32 *)(g_ioapic + 0x10);
}
void ioapic_write_reg(unsigned reg, u32 value)
{
- *(volatile u32 *)g_ioapic = reg;
- *(volatile u32 *)(g_ioapic + 0x10) = value;
+ *(volatile u32 *)g_ioapic = reg;
+ *(volatile u32 *)(g_ioapic + 0x10) = value;
}
void ioapic_write_redir(unsigned line, ioapic_redir_entry_t e)
{
- ioapic_write_reg(0x10 + line * 2 + 0, ((u32 *)&e)[0]);
- ioapic_write_reg(0x10 + line * 2 + 1, ((u32 *)&e)[1]);
+ ioapic_write_reg(0x10 + line * 2 + 0, ((u32 *)&e)[0]);
+ ioapic_write_reg(0x10 + line * 2 + 1, ((u32 *)&e)[1]);
}
ioapic_redir_entry_t ioapic_read_redir(unsigned line)
{
- ioapic_redir_entry_t e;
+ ioapic_redir_entry_t e;
- ((u32 *)&e)[0] = ioapic_read_reg(0x10 + line * 2 + 0);
- ((u32 *)&e)[1] = ioapic_read_reg(0x10 + line * 2 + 1);
- return e;
+ ((u32 *)&e)[0] = ioapic_read_reg(0x10 + line * 2 + 0);
+ ((u32 *)&e)[1] = ioapic_read_reg(0x10 + line * 2 + 1);
+ return e;
}
@@ -214,10 +214,10 @@ void ioapic_set_redir(unsigned line, unsigned vec,
void set_mask(unsigned line, int mask)
{
- ioapic_redir_entry_t e = ioapic_read_redir(line);
+ ioapic_redir_entry_t e = ioapic_read_redir(line);
- e.mask = mask;
- ioapic_write_redir(line, e);
+ e.mask = mask;
+ ioapic_write_redir(line, e);
}
void set_irq_line(unsigned line, int val)
@@ -227,14 +227,14 @@ void set_irq_line(unsigned line, int val)
void enable_apic(void)
{
- printf("enabling apic\n");
- xapic_write(APIC_SPIV, 0x1ff);
+ printf("enabling apic\n");
+ xapic_write(APIC_SPIV, 0x1ff);
}
void mask_pic_interrupts(void)
{
- outb(0xff, 0x21);
- outb(0xff, 0xa1);
+ outb(0xff, 0x21);
+ outb(0xff, 0xa1);
}
extern unsigned char online_cpus[(MAX_TEST_CPUS + 7) / 8];
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [kvm-unit-tests PATCH 7/8] x86: apic: Track APIC ops on a per-cpu basis
2022-01-21 23:18 [kvm-unit-tests PATCH 0/8] x86: APIC bug fix and cleanup Sean Christopherson
` (5 preceding siblings ...)
2022-01-21 23:18 ` [kvm-unit-tests PATCH 6/8] x86: apic: Replace spaces with tabs Sean Christopherson
@ 2022-01-21 23:18 ` Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 8/8] x86: apic: Make xAPIC and I/O APIC pointers static Sean Christopherson
7 siblings, 0 replies; 10+ messages in thread
From: Sean Christopherson @ 2022-01-21 23:18 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Sean Christopherson
Track the virtual function table to handle xAPIC vs. x2APIC on a per-cpu
basis. Using a common global is racy as nothing in KUT synchronizes
CPUs when switching to/from x2APIC.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
lib/x86/apic.c | 20 ++++++++++++--------
lib/x86/smp.h | 2 ++
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/lib/x86/apic.c b/lib/x86/apic.c
index 44a6ad38..d7137b61 100644
--- a/lib/x86/apic.c
+++ b/lib/x86/apic.c
@@ -2,6 +2,7 @@
#include "apic.h"
#include "msr.h"
#include "processor.h"
+#include "smp.h"
#include "asm/barrier.h"
void *g_apic = (void *)0xfee00000;
@@ -15,6 +16,11 @@ struct apic_ops {
u32 (*id)(void);
};
+static struct apic_ops *get_apic_ops(void)
+{
+ return this_cpu_read_apic_ops();
+}
+
static void outb(unsigned char data, unsigned short port)
{
asm volatile ("out %0, %1" : : "a"(data), "d"(port));
@@ -60,8 +66,6 @@ static const struct apic_ops xapic_ops = {
.id = xapic_id,
};
-static const struct apic_ops *apic_ops = &xapic_ops;
-
static u32 x2apic_read(unsigned reg)
{
unsigned a, d;
@@ -96,12 +100,12 @@ static const struct apic_ops x2apic_ops = {
u32 apic_read(unsigned reg)
{
- return apic_ops->reg_read(reg);
+ return get_apic_ops()->reg_read(reg);
}
void apic_write(unsigned reg, u32 val)
{
- apic_ops->reg_write(reg, val);
+ get_apic_ops()->reg_write(reg, val);
}
bool apic_read_bit(unsigned reg, int n)
@@ -113,12 +117,12 @@ bool apic_read_bit(unsigned reg, int n)
void apic_icr_write(u32 val, u32 dest)
{
- apic_ops->icr_write(val, dest);
+ get_apic_ops()->icr_write(val, dest);
}
uint32_t apic_id(void)
{
- return apic_ops->id();
+ return get_apic_ops()->id();
}
uint8_t apic_get_tpr(void)
@@ -152,7 +156,7 @@ int enable_x2apic(void)
asm ("rdmsr" : "=a"(a), "=d"(d) : "c"(MSR_IA32_APICBASE));
a |= 1 << 10;
asm ("wrmsr" : : "a"(a), "d"(d), "c"(MSR_IA32_APICBASE));
- apic_ops = &x2apic_ops;
+ this_cpu_write_apic_ops((void *)&x2apic_ops);
return 1;
} else {
return 0;
@@ -162,7 +166,7 @@ int enable_x2apic(void)
void disable_apic(void)
{
wrmsr(MSR_IA32_APICBASE, rdmsr(MSR_IA32_APICBASE) & ~(APIC_EN | APIC_EXTD));
- apic_ops = &xapic_ops;
+ this_cpu_write_apic_ops((void *)&xapic_ops);
}
void reset_apic(void)
diff --git a/lib/x86/smp.h b/lib/x86/smp.h
index eb037a46..bd303c28 100644
--- a/lib/x86/smp.h
+++ b/lib/x86/smp.h
@@ -15,6 +15,7 @@ struct percpu_data {
};
uint32_t exception_data;
};
+ void *apic_ops;
};
#define typeof_percpu(name) typeof(((struct percpu_data *)0)->name)
@@ -66,6 +67,7 @@ BUILD_PERCPU_OP(smp_id);
BUILD_PERCPU_OP(exception_vector);
BUILD_PERCPU_OP(exception_rflags_rf);
BUILD_PERCPU_OP(exception_error_code);
+BUILD_PERCPU_OP(apic_ops);
void smp_init(void);
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [kvm-unit-tests PATCH 8/8] x86: apic: Make xAPIC and I/O APIC pointers static
2022-01-21 23:18 [kvm-unit-tests PATCH 0/8] x86: APIC bug fix and cleanup Sean Christopherson
` (6 preceding siblings ...)
2022-01-21 23:18 ` [kvm-unit-tests PATCH 7/8] x86: apic: Track APIC ops on a per-cpu basis Sean Christopherson
@ 2022-01-21 23:18 ` Sean Christopherson
7 siblings, 0 replies; 10+ messages in thread
From: Sean Christopherson @ 2022-01-21 23:18 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Sean Christopherson
Make the pointers to the xAPIC and I/O APIC static as there are no users
outside of apic.c. Opportunistically use #defines for the default values
instead of open coding magic numbers.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
lib/x86/apic-defs.h | 3 ++-
lib/x86/apic.c | 6 ++++--
lib/x86/apic.h | 3 ---
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/lib/x86/apic-defs.h b/lib/x86/apic-defs.h
index dabefe78..4db73da2 100644
--- a/lib/x86/apic-defs.h
+++ b/lib/x86/apic-defs.h
@@ -14,8 +14,9 @@
* Alan Cox <Alan.Cox@linux.org>, 1995.
* Ingo Molnar <mingo@redhat.com>, 1999, 2000
*/
+#define IO_APIC_DEFAULT_PHYS_BASE 0xfec00000
+#define APIC_DEFAULT_PHYS_BASE 0xfee00000
-#define APIC_DEFAULT_PHYS_BASE 0xfee00000
#define APIC_BSP (1UL << 8)
#define APIC_EXTD (1UL << 10)
#define APIC_EN (1UL << 11)
diff --git a/lib/x86/apic.c b/lib/x86/apic.c
index d7137b61..5d4c7766 100644
--- a/lib/x86/apic.c
+++ b/lib/x86/apic.c
@@ -5,8 +5,10 @@
#include "smp.h"
#include "asm/barrier.h"
-void *g_apic = (void *)0xfee00000;
-void *g_ioapic = (void *)0xfec00000;
+/* xAPIC and I/O APIC are identify mapped, and never relocated. */
+static void *g_apic = (void *)APIC_DEFAULT_PHYS_BASE;
+static void *g_ioapic = (void *)IO_APIC_DEFAULT_PHYS_BASE;
+
u8 id_map[MAX_TEST_CPUS];
struct apic_ops {
diff --git a/lib/x86/apic.h b/lib/x86/apic.h
index 7844324b..6d27f047 100644
--- a/lib/x86/apic.h
+++ b/lib/x86/apic.h
@@ -6,9 +6,6 @@
extern u8 id_map[MAX_TEST_CPUS];
-extern void *g_apic;
-extern void *g_ioapic;
-
typedef struct {
uint8_t vector;
uint8_t delivery_mode:3;
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests PATCH 1/8] x86: Always use legacy xAPIC to get APIC ID during TSS setup
2022-01-21 23:18 ` [kvm-unit-tests PATCH 1/8] x86: Always use legacy xAPIC to get APIC ID during TSS setup Sean Christopherson
@ 2022-01-22 1:18 ` David Matlack
0 siblings, 0 replies; 10+ messages in thread
From: David Matlack @ 2022-01-22 1:18 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, kvm list
On Fri, Jan 21, 2022 at 3:23 PM Sean Christopherson <seanjc@google.com> wrote:
>
> Force use of xAPIC to retrieve the APIC ID during TSS setup to fix an
> issue where an AP can switch apic_ops to point at x2apic_ops before
> setup_tss() completes, leading to a #GP and triple fault due to trying
> to read an x2APIC MSR without x2APIC being enabled.
>
> A future patch will make apic_ops a per-cpu pointer, but that's not of
> any help for 32-bit, which uses the APIC ID to determine the GS selector,
> i.e. 32-bit KUT has a chicken-and-egg problem. All setup_tss() callers
> ensure the local APIC is in xAPIC mode, so just force use of xAPIC in
> this case.
>
> Fixes: 7e33895 ("x86: Move 32-bit GDT and TSS to desc.c")
> Fixes: dbd3800 ("x86: Move 64-bit GDT and TSS to desc.c")
FYI there was recently a report [1] to the KVM mailing list that Fixes
tags should use at least 12 characters for hashes. So I guess this
should be:
Fixes: 7e33895d3232 ("x86: Move 32-bit GDT and TSS to desc.c")
Fixes: dbd380049042 ("x86: Move 64-bit GDT and TSS to desc.c")
[1] https://lore.kernel.org/kvm/20220121081432.5b671602@canb.auug.org.au/
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> lib/x86/apic.c | 5 +++++
> lib/x86/apic.h | 2 ++
> lib/x86/setup.c | 4 ++--
> 3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/lib/x86/apic.c b/lib/x86/apic.c
> index da8f3013..b404d580 100644
> --- a/lib/x86/apic.c
> +++ b/lib/x86/apic.c
> @@ -48,6 +48,11 @@ static uint32_t xapic_id(void)
> return xapic_read(APIC_ID) >> 24;
> }
>
> +uint32_t pre_boot_apic_id(void)
> +{
> + return xapic_id();
> +}
> +
> static const struct apic_ops xapic_ops = {
> .reg_read = xapic_read,
> .reg_write = xapic_write,
> diff --git a/lib/x86/apic.h b/lib/x86/apic.h
> index c4821716..7844324b 100644
> --- a/lib/x86/apic.h
> +++ b/lib/x86/apic.h
> @@ -53,6 +53,8 @@ bool apic_read_bit(unsigned reg, int n);
> void apic_write(unsigned reg, uint32_t val);
> void apic_icr_write(uint32_t val, uint32_t dest);
> uint32_t apic_id(void);
> +uint32_t pre_boot_apic_id(void);
> +
>
> int enable_x2apic(void);
> void disable_apic(void);
> diff --git a/lib/x86/setup.c b/lib/x86/setup.c
> index bbd34682..64217add 100644
> --- a/lib/x86/setup.c
> +++ b/lib/x86/setup.c
> @@ -109,7 +109,7 @@ unsigned long setup_tss(u8 *stacktop)
> u32 id;
> tss64_t *tss_entry;
>
> - id = apic_id();
> + id = pre_boot_apic_id();
>
> /* Runtime address of current TSS */
> tss_entry = &tss[id];
> @@ -129,7 +129,7 @@ unsigned long setup_tss(u8 *stacktop)
> u32 id;
> tss32_t *tss_entry;
>
> - id = apic_id();
> + id = pre_boot_apic_id();
>
> /* Runtime address of current TSS */
> tss_entry = &tss[id];
> --
> 2.35.0.rc0.227.g00780c9af4-goog
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-01-22 1:19 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-21 23:18 [kvm-unit-tests PATCH 0/8] x86: APIC bug fix and cleanup Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 1/8] x86: Always use legacy xAPIC to get APIC ID during TSS setup Sean Christopherson
2022-01-22 1:18 ` David Matlack
2022-01-21 23:18 ` [kvm-unit-tests PATCH 2/8] x86: nVMX: Load actual GS.base for both guest and host Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 3/8] x86: smp: Replace spaces with tabs Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 4/8] x86: desc: " Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 5/8] x86: Add proper helpers for per-cpu reads/writes Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 6/8] x86: apic: Replace spaces with tabs Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 7/8] x86: apic: Track APIC ops on a per-cpu basis Sean Christopherson
2022-01-21 23:18 ` [kvm-unit-tests PATCH 8/8] x86: apic: Make xAPIC and I/O APIC pointers static Sean Christopherson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox