Werner Zeh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37809 )
Change subject: cpu/x86/smm: Fix regression with SMM setup ......................................................................
cpu/x86/smm: Fix regression with SMM setup
Commit 297b6b862a724de70abf33f681f63b6a3d84c24b (include/cpu/x86: Add STM Support) has introduced two new members to struct smm_runtime without taking care of adding these new members to the SMM stub. This leads to a hanging CPU when SMM is set up.
This commit fixes this regression by adding the new structure members to the SMM stub. TEST=Booted siemens/mc_apl4 which is now working again
Change-Id: I4f8a9898cf81349b50a050b2c2f92578205573e4 Signed-off-by: Werner Zeh werner.zeh@siemens.com --- M src/cpu/x86/smm/smm_stub.S 1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/37809/1
diff --git a/src/cpu/x86/smm/smm_stub.S b/src/cpu/x86/smm/smm_stub.S index f0e55f9..e8eefe4 100644 --- a/src/cpu/x86/smm/smm_stub.S +++ b/src/cpu/x86/smm/smm_stub.S @@ -44,6 +44,11 @@ .long 0 save_state_size: .long 0 +num_cpus: +.long 0 +/* STM's 32 bit entry into SMI handler */ +start32_offset: +.long 0 /* apic_to_cpu_num is a table mapping the default APIC id to CPU num. If the * APIC id is found at the given index, the contiguous CPU number is index * into the table. */
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37809 )
Change subject: cpu/x86/smm: Fix regression with SMM setup ......................................................................
Patch Set 1: Code-Review+1
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37809 )
Change subject: cpu/x86/smm: Fix regression with SMM setup ......................................................................
Patch Set 1: Code-Review+2
Solves the issue on facebook monolith
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37809 )
Change subject: cpu/x86/smm: Fix regression with SMM setup ......................................................................
Patch Set 1:
I suggest backing out the field addtions to the struct itself. There's honestly no good reason to add fields to things that aren't actually used in the code. That or revert the original patch.
Name of user not set #1002358 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37809 )
Change subject: cpu/x86/smm: Fix regression with SMM setup ......................................................................
Patch Set 1:
Patch Set 1:
I suggest backing out the field addtions to the struct itself. There's honestly no good reason to add fields to things that aren't actually used in the code. That or revert the original patch.
The fields are used for the STM setup and, hence will be used when the remainder of the STM setup code is merged
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37809 )
Change subject: cpu/x86/smm: Fix regression with SMM setup ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
I suggest backing out the field addtions to the struct itself. There's honestly no good reason to add fields to things that aren't actually used in the code. That or revert the original patch.
The fields are used for the STM setup and, hence will be used when the remainder of the STM setup code is merged
Correct. And those field additions should go along w/ the patch that adds the functionality.
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37809 )
Change subject: cpu/x86/smm: Fix regression with SMM setup ......................................................................
Patch Set 1:
is there any test we could add to jenkins to make sure this won't happen again.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37809 )
Change subject: cpu/x86/smm: Fix regression with SMM setup ......................................................................
Patch Set 1:
Patch Set 1:
is there any test we could add to jenkins to make sure this won't happen again.
runtime boot test would catch things missed like this in review.
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37809 )
Change subject: cpu/x86/smm: Fix regression with SMM setup ......................................................................
Patch Set 1:
I would hope we can take this ssm_stub.S, put it in Eugene's patch with the data structure change, and submit that as a CL. I realize the fields are not used *yet* but we are frequently asked to split CLs up into digestible pieces, which I think he did.
Name of user not set #1002358 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37809 )
Change subject: cpu/x86/smm: Fix regression with SMM setup ......................................................................
Patch Set 1:
Patch Set 1:
I would hope we can take this ssm_stub.S, put it in Eugene's patch with the data structure change, and submit that as a CL. I realize the fields are not used *yet* but we are frequently asked to split CLs up into digestible pieces, which I think he did.
That is correct - I was requested to split the patch into bit size pieces.
Werner Zeh has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/37809 )
Change subject: cpu/x86/smm: Fix regression with SMM setup ......................................................................
Abandoned
Fixed in CB:37096 by reverting the original patch. No need for this patch anymore.
-
Aaron Durbin (Code Review) -
Frans Hendriks (Code Review)
-
Name of user not set (Code Review)
-
Paul Menzel (Code Review)
-
ron minnich (Code Review)
-
Werner Zeh (Code Review)