[U-Boot] [PATCH] x86: fix memalign() parameter order
From: Stephen Warren swarren@nvidia.com
Purely by code inspection, it looks like the parameter order to memalign() is swapped; its parameters are (align, size). 4096 is a likely desired alignment, and a variable named size sounds like a size:-)
Fixes: 45b5a37836d5 ("x86: Add multi-processor init") Signed-off-by: Stephen Warren swarren@nvidia.com --- I've taken a quick look at all the other memalign() calls in U-Boot, and I /think/ they're all correct. --- arch/x86/cpu/mp_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c index 7917350bff26..fc2fb5bf445c 100644 --- a/arch/x86/cpu/mp_init.c +++ b/arch/x86/cpu/mp_init.c @@ -243,7 +243,7 @@ static int load_sipi_vector(atomic_t **ap_countp, int num_cpus)
params->stack_size = CONFIG_AP_STACK_SIZE; size = params->stack_size * num_cpus; - stack = memalign(size, 4096); + stack = memalign(4096, size); if (!stack) return -ENOMEM; params->stack_top = (u32)(stack + size);
Hi Stephen,
On Sat, Feb 13, 2016 at 5:27 AM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
Purely by code inspection, it looks like the parameter order to memalign() is swapped; its parameters are (align, size). 4096 is a likely desired alignment, and a variable named size sounds like a size:-)
Good catch! It was lucky that this did not corrupt any important data before ..
Fixes: 45b5a37836d5 ("x86: Add multi-processor init") Signed-off-by: Stephen Warren swarren@nvidia.com
Reviewed-by: Bin Meng bmeng.cn@gmail.com
I've taken a quick look at all the other memalign() calls in U-Boot, and I /think/ they're all correct.
arch/x86/cpu/mp_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c index 7917350bff26..fc2fb5bf445c 100644 --- a/arch/x86/cpu/mp_init.c +++ b/arch/x86/cpu/mp_init.c @@ -243,7 +243,7 @@ static int load_sipi_vector(atomic_t **ap_countp, int num_cpus)
params->stack_size = CONFIG_AP_STACK_SIZE; size = params->stack_size * num_cpus;
stack = memalign(size, 4096);
stack = memalign(4096, size); if (!stack) return -ENOMEM; params->stack_top = (u32)(stack + size);--
Regards, Bin
On 12 February 2016 at 14:27, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
Purely by code inspection, it looks like the parameter order to memalign() is swapped; its parameters are (align, size). 4096 is a likely desired alignment, and a variable named size sounds like a size:-)
Fixes: 45b5a37836d5 ("x86: Add multi-processor init") Signed-off-by: Stephen Warren swarren@nvidia.com
I've taken a quick look at all the other memalign() calls in U-Boot, and I /think/ they're all correct.
arch/x86/cpu/mp_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c index 7917350bff26..fc2fb5bf445c 100644 --- a/arch/x86/cpu/mp_init.c +++ b/arch/x86/cpu/mp_init.c @@ -243,7 +243,7 @@ static int load_sipi_vector(atomic_t **ap_countp, int
num_cpus)
params->stack_size = CONFIG_AP_STACK_SIZE; size = params->stack_size * num_cpus;
- stack = memalign(size, 4096);
- stack = memalign(4096, size);
if (!stack) return -ENOMEM; params->stack_top = (u32)(stack + size); -- 2.7.0
Reviewed-by: Simon Glass sjg@chromium.org
Thanks. I'm a little surprised this hasn't caused problems with CPU start-up!
- Simon
On Sun, Feb 14, 2016 at 11:19 AM, Simon Glass sjg@chromium.org wrote:
On 12 February 2016 at 14:27, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
Purely by code inspection, it looks like the parameter order to memalign() is swapped; its parameters are (align, size). 4096 is a likely desired alignment, and a variable named size sounds like a size:-)
Fixes: 45b5a37836d5 ("x86: Add multi-processor init") Signed-off-by: Stephen Warren swarren@nvidia.com
I've taken a quick look at all the other memalign() calls in U-Boot, and I /think/ they're all correct.
arch/x86/cpu/mp_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c index 7917350bff26..fc2fb5bf445c 100644 --- a/arch/x86/cpu/mp_init.c +++ b/arch/x86/cpu/mp_init.c @@ -243,7 +243,7 @@ static int load_sipi_vector(atomic_t **ap_countp, int num_cpus)
params->stack_size = CONFIG_AP_STACK_SIZE; size = params->stack_size * num_cpus;
- stack = memalign(size, 4096);
- stack = memalign(4096, size);
if (!stack) return -ENOMEM; params->stack_top = (u32)(stack + size); -- 2.7.0
Reviewed-by: Simon Glass sjg@chromium.org
Thanks. I'm a little surprised this hasn't caused problems with CPU start-up!
Tested on QEMU Tested-by: Bin Meng bmeng.cn@gmail.com
On Wed, Feb 17, 2016 at 4:20 PM, Bin Meng bmeng.cn@gmail.com wrote:
On Sun, Feb 14, 2016 at 11:19 AM, Simon Glass sjg@chromium.org wrote:
On 12 February 2016 at 14:27, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
Purely by code inspection, it looks like the parameter order to memalign() is swapped; its parameters are (align, size). 4096 is a likely desired alignment, and a variable named size sounds like a size:-)
Fixes: 45b5a37836d5 ("x86: Add multi-processor init") Signed-off-by: Stephen Warren swarren@nvidia.com
I've taken a quick look at all the other memalign() calls in U-Boot, and I /think/ they're all correct.
arch/x86/cpu/mp_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c index 7917350bff26..fc2fb5bf445c 100644 --- a/arch/x86/cpu/mp_init.c +++ b/arch/x86/cpu/mp_init.c @@ -243,7 +243,7 @@ static int load_sipi_vector(atomic_t **ap_countp, int num_cpus)
params->stack_size = CONFIG_AP_STACK_SIZE; size = params->stack_size * num_cpus;
- stack = memalign(size, 4096);
- stack = memalign(4096, size);
if (!stack) return -ENOMEM; params->stack_top = (u32)(stack + size); -- 2.7.0
Reviewed-by: Simon Glass sjg@chromium.org
Thanks. I'm a little surprised this hasn't caused problems with CPU start-up!
Tested on QEMU Tested-by: Bin Meng bmeng.cn@gmail.com
applied to u-boot-x86/master, thanks!
participants (3)
-
Bin Meng -
Simon Glass -
Stephen Warren