Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Cleanup code ......................................................................
cpu/x86/lapic: Cleanup code
* Don't use long as it compiles to 64bit on x86_64, which breaks interrupts in qemu and thus SeaBIOS wouldn't time out the boot menu * Get rid of unused defines * Get rid of unused atomic xchg code
Tested on Qemu Q35 with x86_64 enabled: Interrupts working again. Tested on Qemu Q35 with x86_32 enabled: Interrupts are still working.
Change-Id: Iaed1ad956d090625c7bb5cd9cf55cbae16dd82bd Signed-off-by: Patrick Rudolph siro@das-labor.org --- M src/cpu/x86/lapic/lapic.c M src/cpu/x86/lapic/lapic_cpu_init.c M src/include/cpu/x86/lapic.h 3 files changed, 18 insertions(+), 57 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/36777/1
diff --git a/src/cpu/x86/lapic/lapic.c b/src/cpu/x86/lapic/lapic.c index 755fbe2..6eb72f9 100644 --- a/src/cpu/x86/lapic/lapic.c +++ b/src/cpu/x86/lapic/lapic.c @@ -58,6 +58,6 @@ LAPIC_DELIVERY_MODE_NMI) );
- printk(BIOS_DEBUG, " apic_id: 0x%02lx ", lapicid()); + printk(BIOS_DEBUG, " apic_id: 0x%02x ", lapicid()); printk(BIOS_INFO, "done.\n"); } diff --git a/src/cpu/x86/lapic/lapic_cpu_init.c b/src/cpu/x86/lapic/lapic_cpu_init.c index e7dfc57..1e5c96a 100644 --- a/src/cpu/x86/lapic/lapic_cpu_init.c +++ b/src/cpu/x86/lapic/lapic_cpu_init.c @@ -101,7 +101,7 @@ static int lapic_start_cpu(unsigned long apicid) { int timeout; - unsigned long send_status, accept_status; + uint32_t send_status, accept_status; int j, maxlvt;
/* @@ -133,11 +133,11 @@ printk(BIOS_ERR, "CPU %ld: First APIC write timed out. " "Disabling\n", apicid); // too bad. - printk(BIOS_ERR, "ESR is 0x%lx\n", lapic_read(LAPIC_ESR)); + printk(BIOS_ERR, "ESR is 0x%x\n", lapic_read(LAPIC_ESR)); if (lapic_read(LAPIC_ESR)) { printk(BIOS_ERR, "Try to reset ESR\n"); lapic_write_around(LAPIC_ESR, 0); - printk(BIOS_ERR, "ESR is 0x%lx\n", + printk(BIOS_ERR, "ESR is 0x%x\n", lapic_read(LAPIC_ESR)); } return 0; @@ -230,7 +230,7 @@ if (send_status) printk(BIOS_WARNING, "APIC never delivered???\n"); if (accept_status) - printk(BIOS_WARNING, "APIC delivery error (%lx).\n", + printk(BIOS_WARNING, "APIC delivery error (%x).\n", accept_status); if (send_status || accept_status) return 0; diff --git a/src/include/cpu/x86/lapic.h b/src/include/cpu/x86/lapic.h index 6fd1997..8800ad2 100644 --- a/src/include/cpu/x86/lapic.h +++ b/src/include/cpu/x86/lapic.h @@ -6,14 +6,14 @@ #include <halt.h> #include <smp/node.h>
-static __always_inline unsigned long lapic_read(unsigned long reg) +static __always_inline uint32_t lapic_read(unsigned long reg) { - return *((volatile unsigned long *)(LAPIC_DEFAULT_BASE+reg)); + return *((volatile uint32_t *)(LAPIC_DEFAULT_BASE+reg)); }
-static __always_inline void lapic_write(unsigned long reg, unsigned long v) +static __always_inline void lapic_write(unsigned long reg, uint32_t v) { - *((volatile unsigned long *)(LAPIC_DEFAULT_BASE+reg)) = v; + *((volatile uint32_t *)(LAPIC_DEFAULT_BASE+reg)) = v; }
static __always_inline void lapic_wait_icr_idle(void) @@ -40,7 +40,7 @@ wrmsr(LAPIC_BASE_MSR, msr); }
-static __always_inline unsigned long lapicid(void) +static __always_inline uint32_t lapicid(void) { return lapic_read(LAPIC_ID) >> 24; } @@ -58,58 +58,19 @@ void stop_this_cpu(void); #endif
-#define xchg(ptr, v) ((__typeof__(*(ptr)))__xchg((unsigned long)(v), (ptr), \ - sizeof(*(ptr)))) - -struct __xchg_dummy { unsigned long a[100]; }; -#define __xg(x) ((struct __xchg_dummy *)(x)) - -/* - * Note: no "lock" prefix even on SMP: xchg always implies lock anyway - * Note 2: xchg has side effect, so that attribute volatile is necessary, - * but generally the primitive is invalid, *ptr is output argument. --ANK - */ -static inline unsigned long __xchg(unsigned long x, volatile void *ptr, - int size) +static inline void lapic_write_atomic(unsigned long reg, volatile uint32_t v) { - switch (size) { - case 1: - __asm__ __volatile__("xchgb %b0,%1" - : "=q" (x) - : "m" (*__xg(ptr)), "0" (x) - : "memory"); - break; - case 2: - __asm__ __volatile__("xchgw %w0,%1" - : "=r" (x) - : "m" (*__xg(ptr)), "0" (x) - : "memory"); - break; - case 4: - __asm__ __volatile__("xchgl %0,%1" - : "=r" (x) - : "m" (*__xg(ptr)), "0" (x) - : "memory"); - break; - } - return x; + volatile uint32_t *ptr; + + ptr = (volatile uint32_t *)(LAPIC_DEFAULT_BASE+reg); + + asm volatile ("xchgl %0, %1\n" + : "+r" (v), "+m" (*(ptr)) + : : "memory", "cc"); }
-static inline void lapic_write_atomic(unsigned long reg, unsigned long v) -{ - (void)xchg((volatile unsigned long *)(LAPIC_DEFAULT_BASE+reg), v); -} - - -#ifdef X86_GOOD_APIC -# define FORCE_READ_AROUND_WRITE 0 -# define lapic_read_around(x) lapic_read(x) -# define lapic_write_around(x, y) lapic_write((x), (y)) -#else -# define FORCE_READ_AROUND_WRITE 1 # define lapic_read_around(x) lapic_read(x) # define lapic_write_around(x, y) lapic_write_atomic((x), (y)) -#endif
void do_lapic_init(void);
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Cleanup code ......................................................................
Patch Set 1: Code-Review+1
(5 comments)
https://review.coreboot.org/c/coreboot/+/36777/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36777/1//COMMIT_MSG@7 PS1, Line 7: Cleanup Clean up
https://review.coreboot.org/c/coreboot/+/36777/1//COMMIT_MSG@7 PS1, Line 7: cpu/x86/lapic: Cleanup code Too generic. Maybe:
cpu/x86/lapic: Support x86_64 and clean up code
https://review.coreboot.org/c/coreboot/+/36777/1//COMMIT_MSG@9 PS1, Line 9: interrupts : in qemu Do they need to be 32-bit?
https://review.coreboot.org/c/coreboot/+/36777/1//COMMIT_MSG@14 PS1, Line 14: again Did it ever work before with x86_64?
https://review.coreboot.org/c/coreboot/+/36777/1//COMMIT_MSG@14 PS1, Line 14: working work
Hello Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36777
to look at the new patch set (#2).
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
cpu/x86/lapic: Support x86_64 and clean up code
Most LAPIC registers are 32bit, and thus the use of long is valid on x86_32, however it doesn't work on x86_64.
* Don't use long as it compiles to 64bit on x86_64, which breaks interrupts in qemu and thus SeaBIOS wouldn't time out the boot menu * Get rid of unused defines * Get rid of unused atomic xchg code
Tested on Qemu Q35 with x86_64 enabled: Interrupts work again. Tested on Qemu Q35 with x86_32 enabled: Interrupts are still working.
Change-Id: Iaed1ad956d090625c7bb5cd9cf55cbae16dd82bd Signed-off-by: Patrick Rudolph siro@das-labor.org --- M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/x86/lapic/lapic.c M src/cpu/x86/lapic/lapic_cpu_init.c M src/include/cpu/x86/lapic.h M src/soc/amd/common/block/pi/def_callouts.c 5 files changed, 20 insertions(+), 59 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/36777/2
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/36777/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36777/1//COMMIT_MSG@7 PS1, Line 7: cpu/x86/lapic: Cleanup code
Too generic. Maybe: […]
Done
https://review.coreboot.org/c/coreboot/+/36777/1//COMMIT_MSG@7 PS1, Line 7: Cleanup
Clean up
Done
https://review.coreboot.org/c/coreboot/+/36777/1//COMMIT_MSG@9 PS1, Line 9: interrupts : in qemu
Do they need to be 32-bit?
Done
https://review.coreboot.org/c/coreboot/+/36777/1//COMMIT_MSG@14 PS1, Line 14: again
Did it ever work before with x86_64?
Yes, I had it working and it broke, but I can't say when or why, as the x86_64 topic is being worked on for nearly one year.
https://review.coreboot.org/c/coreboot/+/36777/1//COMMIT_MSG@14 PS1, Line 14: working
work
Done
Patrick Rudolph has uploaded a new patch set (#4) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
cpu/x86/lapic: Support x86_64 and clean up code
Most LAPIC registers are 32bit, and thus the use of long is valid on x86_32, however it didn't work on x86_64.
* Don't use long as it compiles to 64bit on x86_64, which breaks interrupts in qemu and thus SeaBIOS wouldn't time out the boot menu * Get rid of unused defines * Get rid of unused atomic xchg code
Tested on Qemu Q35 with x86_64 enabled: Interrupts work again. Tested on Qemu Q35 with x86_32 enabled: Interrupts are still working. Tested on Lenovo T410 with x86_64 enabled.
Change-Id: Iaed1ad956d090625c7bb5cd9cf55cbae16dd82bd Signed-off-by: Patrick Rudolph siro@das-labor.org --- M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/x86/lapic/lapic.c M src/cpu/x86/lapic/lapic_cpu_init.c M src/include/cpu/x86/lapic.h M src/soc/amd/common/block/pi/def_callouts.c 5 files changed, 20 insertions(+), 59 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/36777/4
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
Patch Set 5: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
Patch Set 6: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/36777/6/src/include/cpu/x86/lapic.h File src/include/cpu/x86/lapic.h:
https://review.coreboot.org/c/coreboot/+/36777/6/src/include/cpu/x86/lapic.h... PS6, Line 3: #include <stdint.h>
https://review.coreboot.org/c/coreboot/+/36777/6/src/include/cpu/x86/lapic.h... PS6, Line 8: unsigned long why leave this `unsigned long` here?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
Patch Set 6:
(4 comments)
Some nits about the commit message.
https://review.coreboot.org/c/coreboot/+/36777/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36777/6//COMMIT_MSG@10 PS6, Line 10: didn't does not
https://review.coreboot.org/c/coreboot/+/36777/6//COMMIT_MSG@12 PS6, Line 12: compiles to is
https://review.coreboot.org/c/coreboot/+/36777/6//COMMIT_MSG@13 PS6, Line 13: qemu QEMU
https://review.coreboot.org/c/coreboot/+/36777/6//COMMIT_MSG@17 PS6, Line 17: Qemu QEMU
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36777/6/src/include/cpu/x86/lapic.h File src/include/cpu/x86/lapic.h:
https://review.coreboot.org/c/coreboot/+/36777/6/src/include/cpu/x86/lapic.h... PS6, Line 10: return *((volatile uint32_t *)(LAPIC_DEFAULT_BASE+reg)); could add spaces around +, while we touch the line anyway
https://review.coreboot.org/c/coreboot/+/36777/6/src/include/cpu/x86/lapic.h... PS6, Line 42: uint32_t Why 32? if it's not 32 exactly, why not use `unsigned int`?
https://review.coreboot.org/c/coreboot/+/36777/6/src/include/cpu/x86/lapic.h... PS6, Line 60: volatile why volatile?
Patrick Rudolph has uploaded a new patch set (#7) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
cpu/x86/lapic: Support x86_64 and clean up code
Most LAPIC registers are 32bit, and thus the use of long is valid on x86_32, however it doesn't work on x86_64.
* Don't use long as it is 64bit on x86_64, which breaks interrupts in QEMU and thus SeaBIOS wouldn't time out the boot menu * Get rid of unused defines * Get rid of unused atomic xchg code
Tested on QEMU Q35 with x86_64 enabled: Interrupts work again. Tested on QEMU Q35 with x86_32 enabled: Interrupts are still working. Tested on Lenovo T410 with x86_64 enabled.
Change-Id: Iaed1ad956d090625c7bb5cd9cf55cbae16dd82bd Signed-off-by: Patrick Rudolph siro@das-labor.org --- M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/x86/lapic/lapic.c M src/cpu/x86/lapic/lapic_cpu_init.c M src/include/cpu/x86/lapic.h M src/soc/amd/common/block/pi/def_callouts.c 5 files changed, 21 insertions(+), 59 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/36777/7
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
Patch Set 6:
(8 comments)
https://review.coreboot.org/c/coreboot/+/36777/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36777/6//COMMIT_MSG@10 PS6, Line 10: didn't
does not
Done
https://review.coreboot.org/c/coreboot/+/36777/6//COMMIT_MSG@12 PS6, Line 12: compiles to
is
Done
https://review.coreboot.org/c/coreboot/+/36777/6//COMMIT_MSG@13 PS6, Line 13: qemu
QEMU
Done
https://review.coreboot.org/c/coreboot/+/36777/6//COMMIT_MSG@17 PS6, Line 17: Qemu
QEMU
Done
https://review.coreboot.org/c/coreboot/+/36777/6/src/include/cpu/x86/lapic.h File src/include/cpu/x86/lapic.h:
https://review.coreboot.org/c/coreboot/+/36777/6/src/include/cpu/x86/lapic.h... PS6, Line 3:
#include <stdint. […]
Done
https://review.coreboot.org/c/coreboot/+/36777/6/src/include/cpu/x86/lapic.h... PS6, Line 8: unsigned long
why leave this `unsigned long` here?
Done
https://review.coreboot.org/c/coreboot/+/36777/6/src/include/cpu/x86/lapic.h... PS6, Line 42: uint32_t
Why 32? if it's not 32 exactly, why not use `unsigned int`?
Done
https://review.coreboot.org/c/coreboot/+/36777/6/src/include/cpu/x86/lapic.h... PS6, Line 60: volatile
why volatile?
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
Patch Set 7: Code-Review+1
Need to test, but looks good
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36777/7/src/include/cpu/x86/lapic.h File src/include/cpu/x86/lapic.h:
https://review.coreboot.org/c/coreboot/+/36777/7/src/include/cpu/x86/lapic.h... PS7, Line 11: return *((volatile uint32_t *)(LAPIC_DEFAULT_BASE + reg)); Why no t just use read32() ?
Patrick Rudolph has uploaded a new patch set (#8) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
cpu/x86/lapic: Support x86_64 and clean up code
Most LAPIC registers are 32bit, and thus the use of long is valid on x86_32, however it doesn't work on x86_64.
* Don't use long as it is 64bit on x86_64, which breaks interrupts in QEMU and thus SeaBIOS wouldn't time out the boot menu * Get rid of unused defines * Get rid of unused atomic xchg code
Tested on QEMU Q35 with x86_64 enabled: Interrupts work again. Tested on QEMU Q35 with x86_32 enabled: Interrupts are still working. Tested on Lenovo T410 with x86_64 enabled.
Change-Id: Iaed1ad956d090625c7bb5cd9cf55cbae16dd82bd Signed-off-by: Patrick Rudolph siro@das-labor.org --- M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/x86/lapic/lapic.c M src/cpu/x86/lapic/lapic_cpu_init.c M src/include/cpu/x86/lapic.h M src/soc/amd/common/block/pi/def_callouts.c 5 files changed, 22 insertions(+), 59 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/36777/8
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36777/7/src/include/cpu/x86/lapic.h File src/include/cpu/x86/lapic.h:
https://review.coreboot.org/c/coreboot/+/36777/7/src/include/cpu/x86/lapic.h... PS7, Line 11: return *((volatile uint32_t *)(LAPIC_DEFAULT_BASE + reg));
Why no t just use read32() ?
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
Patch Set 8: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36777 )
Change subject: cpu/x86/lapic: Support x86_64 and clean up code ......................................................................
cpu/x86/lapic: Support x86_64 and clean up code
Most LAPIC registers are 32bit, and thus the use of long is valid on x86_32, however it doesn't work on x86_64.
* Don't use long as it is 64bit on x86_64, which breaks interrupts in QEMU and thus SeaBIOS wouldn't time out the boot menu * Get rid of unused defines * Get rid of unused atomic xchg code
Tested on QEMU Q35 with x86_64 enabled: Interrupts work again. Tested on QEMU Q35 with x86_32 enabled: Interrupts are still working. Tested on Lenovo T410 with x86_64 enabled.
Change-Id: Iaed1ad956d090625c7bb5cd9cf55cbae16dd82bd Signed-off-by: Patrick Rudolph siro@das-labor.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/36777 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/x86/lapic/lapic.c M src/cpu/x86/lapic/lapic_cpu_init.c M src/include/cpu/x86/lapic.h M src/soc/amd/common/block/pi/def_callouts.c 5 files changed, 22 insertions(+), 59 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/cpu/intel/model_2065x/model_2065x_init.c b/src/cpu/intel/model_2065x/model_2065x_init.c index b4bfce3..6f41ac5 100644 --- a/src/cpu/intel/model_2065x/model_2065x_init.c +++ b/src/cpu/intel/model_2065x/model_2065x_init.c @@ -228,7 +228,7 @@ /* Print processor name */ fill_processor_name(processor_name); printk(BIOS_INFO, "CPU: %s.\n", processor_name); - printk(BIOS_INFO, "CPU:lapic=%ld, boot_cpu=%d\n", lapicid(), + printk(BIOS_INFO, "CPU:lapic=%d, boot_cpu=%d\n", lapicid(), boot_cpu());
/* Setup Page Attribute Tables (PAT) */ diff --git a/src/cpu/x86/lapic/lapic.c b/src/cpu/x86/lapic/lapic.c index f0a6cd7..04ce226 100644 --- a/src/cpu/x86/lapic/lapic.c +++ b/src/cpu/x86/lapic/lapic.c @@ -47,6 +47,6 @@ LAPIC_DELIVERY_MODE_NMI) );
- printk(BIOS_DEBUG, " apic_id: 0x%02lx ", lapicid()); + printk(BIOS_DEBUG, " apic_id: 0x%02x ", lapicid()); printk(BIOS_INFO, "done.\n"); } diff --git a/src/cpu/x86/lapic/lapic_cpu_init.c b/src/cpu/x86/lapic/lapic_cpu_init.c index 58a633b..f89d9e3 100644 --- a/src/cpu/x86/lapic/lapic_cpu_init.c +++ b/src/cpu/x86/lapic/lapic_cpu_init.c @@ -91,7 +91,7 @@ static int lapic_start_cpu(unsigned long apicid) { int timeout; - unsigned long send_status, accept_status; + uint32_t send_status, accept_status; int j, maxlvt;
/* @@ -123,11 +123,11 @@ printk(BIOS_ERR, "CPU %ld: First APIC write timed out. " "Disabling\n", apicid); // too bad. - printk(BIOS_ERR, "ESR is 0x%lx\n", lapic_read(LAPIC_ESR)); + printk(BIOS_ERR, "ESR is 0x%x\n", lapic_read(LAPIC_ESR)); if (lapic_read(LAPIC_ESR)) { printk(BIOS_ERR, "Try to reset ESR\n"); lapic_write_around(LAPIC_ESR, 0); - printk(BIOS_ERR, "ESR is 0x%lx\n", + printk(BIOS_ERR, "ESR is 0x%x\n", lapic_read(LAPIC_ESR)); } return 0; @@ -216,7 +216,7 @@ if (send_status) printk(BIOS_WARNING, "APIC never delivered???\n"); if (accept_status) - printk(BIOS_WARNING, "APIC delivery error (%lx).\n", + printk(BIOS_WARNING, "APIC delivery error (%x).\n", accept_status); if (send_status || accept_status) return 0; diff --git a/src/include/cpu/x86/lapic.h b/src/include/cpu/x86/lapic.h index f8081b5..28978f2 100644 --- a/src/include/cpu/x86/lapic.h +++ b/src/include/cpu/x86/lapic.h @@ -1,18 +1,20 @@ #ifndef CPU_X86_LAPIC_H #define CPU_X86_LAPIC_H
+#include <arch/mmio.h> #include <cpu/x86/lapic_def.h> #include <cpu/x86/msr.h> #include <halt.h> +#include <stdint.h>
-static __always_inline unsigned long lapic_read(unsigned long reg) +static __always_inline uint32_t lapic_read(unsigned int reg) { - return *((volatile unsigned long *)(LAPIC_DEFAULT_BASE+reg)); + return read32((volatile void *)(LAPIC_DEFAULT_BASE + reg)); }
-static __always_inline void lapic_write(unsigned long reg, unsigned long v) +static __always_inline void lapic_write(unsigned int reg, uint32_t v) { - *((volatile unsigned long *)(LAPIC_DEFAULT_BASE+reg)) = v; + write32((volatile void *)(LAPIC_DEFAULT_BASE + reg), v); }
static __always_inline void lapic_wait_icr_idle(void) @@ -39,7 +41,7 @@ wrmsr(LAPIC_BASE_MSR, msr); }
-static __always_inline unsigned long lapicid(void) +static __always_inline unsigned int lapicid(void) { return lapic_read(LAPIC_ID) >> 24; } @@ -57,58 +59,19 @@ void stop_this_cpu(void); #endif
-#define xchg(ptr, v) ((__typeof__(*(ptr)))__xchg((unsigned long)(v), (ptr), \ - sizeof(*(ptr)))) - -struct __xchg_dummy { unsigned long a[100]; }; -#define __xg(x) ((struct __xchg_dummy *)(x)) - -/* - * Note: no "lock" prefix even on SMP: xchg always implies lock anyway - * Note 2: xchg has side effect, so that attribute volatile is necessary, - * but generally the primitive is invalid, *ptr is output argument. --ANK - */ -static inline unsigned long __xchg(unsigned long x, volatile void *ptr, - int size) +static inline void lapic_write_atomic(unsigned long reg, uint32_t v) { - switch (size) { - case 1: - __asm__ __volatile__("xchgb %b0,%1" - : "=q" (x) - : "m" (*__xg(ptr)), "0" (x) - : "memory"); - break; - case 2: - __asm__ __volatile__("xchgw %w0,%1" - : "=r" (x) - : "m" (*__xg(ptr)), "0" (x) - : "memory"); - break; - case 4: - __asm__ __volatile__("xchgl %0,%1" - : "=r" (x) - : "m" (*__xg(ptr)), "0" (x) - : "memory"); - break; - } - return x; + volatile uint32_t *ptr; + + ptr = (volatile uint32_t *)(LAPIC_DEFAULT_BASE + reg); + + asm volatile ("xchgl %0, %1\n" + : "+r" (v), "+m" (*(ptr)) + : : "memory", "cc"); }
-static inline void lapic_write_atomic(unsigned long reg, unsigned long v) -{ - (void)xchg((volatile unsigned long *)(LAPIC_DEFAULT_BASE+reg), v); -} - - -#ifdef X86_GOOD_APIC -# define FORCE_READ_AROUND_WRITE 0 -# define lapic_read_around(x) lapic_read(x) -# define lapic_write_around(x, y) lapic_write((x), (y)) -#else -# define FORCE_READ_AROUND_WRITE 1 # define lapic_read_around(x) lapic_read(x) # define lapic_write_around(x, y) lapic_write_atomic((x), (y)) -#endif
void do_lapic_init(void);
diff --git a/src/soc/amd/common/block/pi/def_callouts.c b/src/soc/amd/common/block/pi/def_callouts.c index 7126c97..56ce995 100644 --- a/src/soc/amd/common/block/pi/def_callouts.c +++ b/src/soc/amd/common/block/pi/def_callouts.c @@ -195,7 +195,7 @@ Status = amd_late_run_ap_task(agesadata.ConfigPtr);
if (Status) - printk(BIOS_DEBUG, "There was a problem with %lx returned %s\n", + printk(BIOS_DEBUG, "There was a problem with %x returned %s\n", lapicid(), decodeAGESA_STATUS(Status)); }
-
Aaron Durbin (Code Review) -
Angel Pons (Code Review)
-
Arthur Heymans (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)