Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47070 )
Change subject: cpu/x86/smm/smm_module_loaderv2.c: Use more variables ......................................................................
cpu/x86/smm/smm_module_loaderv2.c: Use more variables
Reusing the size variable for a different purpose later on in the function makes the code harder to read.
Change-Id: Iceb10aa40ad473b41b7da0310554725585e3c2c2 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/x86/smm/smm_module_loaderv2.c 1 file changed, 4 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/47070/1
diff --git a/src/cpu/x86/smm/smm_module_loaderv2.c b/src/cpu/x86/smm/smm_module_loaderv2.c index 3fa5871..821aa3f 100644 --- a/src/cpu/x86/smm/smm_module_loaderv2.c +++ b/src/cpu/x86/smm/smm_module_loaderv2.c @@ -413,12 +413,13 @@ * for default handler, but for relocated handler it lives at the beginning * of SMRAM which is TSEG base */ - size = params->num_concurrent_stacks * params->per_cpu_stack_size; - stacks_top = smm_stub_place_stacks((char *)params->smram_start, size, params); + const total_stack_size = params->num_concurrent_stacks * params->per_cpu_stack_size; + stacks_top = smm_stub_place_stacks((char *)params->smram_start, total_stack_size, + params); if (stacks_top == NULL) { printk(BIOS_ERR, "%s: not enough space for stacks\n", __func__); printk(BIOS_ERR, "%s: ....need -> %p : available -> %zx\n", __func__, - base, size); + base, total_stack_size); return -1; } params->stack_top = stacks_top;
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47070 )
Change subject: cpu/x86/smm/smm_module_loaderv2.c: Use more variables ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/47070/1/src/cpu/x86/smm/smm_module_... File src/cpu/x86/smm/smm_module_loaderv2.c:
https://review.coreboot.org/c/coreboot/+/47070/1/src/cpu/x86/smm/smm_module_... PS1, Line 416: const `const size_t`?
Hello build bot (Jenkins), Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47070
to look at the new patch set (#2).
Change subject: cpu/x86/smm/smm_module_loaderv2.c: Use more variables ......................................................................
cpu/x86/smm/smm_module_loaderv2.c: Use more variables
Reusing the size variable for a different purpose later on in the function makes the code harder to read.
Change-Id: Iceb10aa40ad473b41b7da0310554725585e3c2c2 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/x86/smm/smm_module_loaderv2.c 1 file changed, 5 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/47070/2
Hello build bot (Jenkins), Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47070
to look at the new patch set (#3).
Change subject: cpu/x86/smm/smm_module_loaderv2.c: Use more variables ......................................................................
cpu/x86/smm/smm_module_loaderv2.c: Use more variables
Reusing the 'size' variable for a different purpose later on in the function makes the code harder to read.
Change-Id: Iceb10aa40ad473b41b7da0310554725585e3c2c2 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/x86/smm/smm_module_loaderv2.c 1 file changed, 5 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/47070/3
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47070 )
Change subject: cpu/x86/smm/smm_module_loaderv2.c: Use more variables ......................................................................
Patch Set 3: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47070 )
Change subject: cpu/x86/smm/smm_module_loaderv2.c: Use more variables ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/47070/1/src/cpu/x86/smm/smm_module_... File src/cpu/x86/smm/smm_module_loaderv2.c:
https://review.coreboot.org/c/coreboot/+/47070/1/src/cpu/x86/smm/smm_module_... PS1, Line 416: const
`const size_t`?
Done
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47070 )
Change subject: cpu/x86/smm/smm_module_loaderv2.c: Use more variables ......................................................................
cpu/x86/smm/smm_module_loaderv2.c: Use more variables
Reusing the 'size' variable for a different purpose later on in the function makes the code harder to read.
Change-Id: Iceb10aa40ad473b41b7da0310554725585e3c2c2 Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/47070 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: David Hendricks david.hendricks@gmail.com Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/cpu/x86/smm/smm_module_loaderv2.c 1 file changed, 5 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/cpu/x86/smm/smm_module_loaderv2.c b/src/cpu/x86/smm/smm_module_loaderv2.c index 3fa5871..4bc385f 100644 --- a/src/cpu/x86/smm/smm_module_loaderv2.c +++ b/src/cpu/x86/smm/smm_module_loaderv2.c @@ -413,12 +413,14 @@ * for default handler, but for relocated handler it lives at the beginning * of SMRAM which is TSEG base */ - size = params->num_concurrent_stacks * params->per_cpu_stack_size; - stacks_top = smm_stub_place_stacks((char *)params->smram_start, size, params); + const size_t total_stack_size = params->num_concurrent_stacks * + params->per_cpu_stack_size; + stacks_top = smm_stub_place_stacks((char *)params->smram_start, total_stack_size, + params); if (stacks_top == NULL) { printk(BIOS_ERR, "%s: not enough space for stacks\n", __func__); printk(BIOS_ERR, "%s: ....need -> %p : available -> %zx\n", __func__, - base, size); + base, total_stack_size); return -1; } params->stack_top = stacks_top;
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
David Hendricks (Code Review)
-
Patrick Georgi (Code Review)