Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44675 )
Change subject: cpu/x86/mp_init: Add support for x86_64 ......................................................................
cpu/x86/mp_init: Add support for x86_64
Fix compilation on x86_64.
Tested on HP Z220: * Still boots on x86_32.
Change-Id: Id7190d24172803e40acaf1495ce20f3ea38016b0 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/x86/mp_init.c 1 file changed, 4 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/44675/1
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 5807831..da0002f 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -113,7 +113,7 @@ /* The SIPI vector is loaded at the SMM_DEFAULT_BASE. The reason is at the * memory range is already reserved so the OS cannot use it. That region is * free to use for AP bringup before SMM is initialized. */ -static const uint32_t sipi_vector_location = SMM_DEFAULT_BASE; +static const uintptr_t sipi_vector_location = SMM_DEFAULT_BASE; static const int sipi_vector_location_size = SMM_DEFAULT_SIZE;
struct mp_flight_plan { @@ -339,16 +339,16 @@
setup_default_sipi_vector_params(sp); /* Setup MSR table. */ - sp->msr_table_ptr = (uint32_t)&mod_loc[module_size]; + sp->msr_table_ptr = (uintptr_t)&mod_loc[module_size]; sp->msr_count = num_msrs; /* Provide pointer to microcode patch. */ - sp->microcode_ptr = (uint32_t)mp_params->microcode_pointer; + sp->microcode_ptr = (uintptr_t)mp_params->microcode_pointer; /* Pass on ability to load microcode in parallel. */ if (mp_params->parallel_microcode_load) sp->microcode_lock = 0; else sp->microcode_lock = ~0; - sp->c_handler = (uint32_t)&ap_init; + sp->c_handler = (uintptr_t)&ap_init; ap_count = &sp->ap_count; atomic_set(ap_count, 0);
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44675 )
Change subject: cpu/x86/mp_init: Add support for x86_64 ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/44675/1/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/c/coreboot/+/44675/1/src/cpu/x86/mp_init.c@351 PS1, Line 351: uintptr_t Has the potential to overflow the uint32_t c_handler?
The problem is that the .module_parametrs has to match the struct entries size. Maybe a macro to be used in assembly for the pointer size could come in handy here and for the smm module parameters?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44675 )
Change subject: cpu/x86/mp_init: Add support for x86_64 ......................................................................
Patch Set 3: Code-Review+2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44675 )
Change subject: cpu/x86/mp_init: Add support for x86_64 ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/44675/1/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/c/coreboot/+/44675/1/src/cpu/x86/mp_init.c@351 PS1, Line 351: uintptr_t
Has the potential to overflow the uint32_t c_handler? […]
LGTM and fix/improve this later on.
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44675 )
Change subject: cpu/x86/mp_init: Add support for x86_64 ......................................................................
Patch Set 3: Code-Review+2
Stefan Reinauer has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44675 )
Change subject: cpu/x86/mp_init: Add support for x86_64 ......................................................................
cpu/x86/mp_init: Add support for x86_64
Fix compilation on x86_64.
Tested on HP Z220: * Still boots on x86_32.
Change-Id: Id7190d24172803e40acaf1495ce20f3ea38016b0 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/44675 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Arthur Heymans arthur@aheymans.xyz Reviewed-by: Stefan Reinauer stefan.reinauer@coreboot.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/cpu/x86/mp_init.c 1 file changed, 4 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved Arthur Heymans: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index db21262..01c1d5f 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -112,7 +112,7 @@ /* The SIPI vector is loaded at the SMM_DEFAULT_BASE. The reason is at the * memory range is already reserved so the OS cannot use it. That region is * free to use for AP bringup before SMM is initialized. */ -static const uint32_t sipi_vector_location = SMM_DEFAULT_BASE; +static const uintptr_t sipi_vector_location = SMM_DEFAULT_BASE; static const int sipi_vector_location_size = SMM_DEFAULT_SIZE;
struct mp_flight_plan { @@ -338,16 +338,16 @@
setup_default_sipi_vector_params(sp); /* Setup MSR table. */ - sp->msr_table_ptr = (uint32_t)&mod_loc[module_size]; + sp->msr_table_ptr = (uintptr_t)&mod_loc[module_size]; sp->msr_count = num_msrs; /* Provide pointer to microcode patch. */ - sp->microcode_ptr = (uint32_t)mp_params->microcode_pointer; + sp->microcode_ptr = (uintptr_t)mp_params->microcode_pointer; /* Pass on ability to load microcode in parallel. */ if (mp_params->parallel_microcode_load) sp->microcode_lock = 0; else sp->microcode_lock = ~0; - sp->c_handler = (uint32_t)&ap_init; + sp->c_handler = (uintptr_t)&ap_init; ap_count = &sp->ap_count; atomic_set(ap_count, 0);
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Patrick Rudolph (Code Review)
-
Stefan Reinauer (Code Review)