cedarhouse1@comcast.net has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38657 )
Change subject: arch/x86/include/arch: Add ROM_TASK_STATE_SEG ......................................................................
arch/x86/include/arch: Add ROM_TASK_STATE_SEG
This define is used to set up the STM SMM Descriptor table tr entry.
Signed-off-by: Eugene D. Myers edmyers@tycho.nsa.gov Change-Id: Iddb1f45444d03465a66a4ebb9fde5f206dc5b300 --- M src/arch/x86/include/arch/rom_segs.h 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/38657/1
diff --git a/src/arch/x86/include/arch/rom_segs.h b/src/arch/x86/include/arch/rom_segs.h index a19d3de..05cef4d 100644 --- a/src/arch/x86/include/arch/rom_segs.h +++ b/src/arch/x86/include/arch/rom_segs.h @@ -17,5 +17,6 @@ #define ROM_CODE_SEG 0x08 #define ROM_DATA_SEG 0x10 #define ROM_CODE_SEG64 0x18 +#define ROM_TASK_STATE_SEG 0x20
#endif /* ROM_SEGS_H */
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38657 )
Change subject: arch/x86/include/arch: Add ROM_TASK_STATE_SEG ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
https://review.coreboot.org/c/coreboot/+/38657/1/src/arch/x86/include/arch/r... File src/arch/x86/include/arch/rom_segs.h:
https://review.coreboot.org/c/coreboot/+/38657/1/src/arch/x86/include/arch/r... PS1, Line 20: ROM_TASK_STATE_SEG That segment doesn't exist in romstage gdt, see src/arch/x86/gdt_init.S
cedarhouse1@comcast.net has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38657 )
Change subject: arch/x86/include/arch: Add ROM_TASK_STATE_SEG ......................................................................
Patch Set 1:
(1 comment)
I would like get resolution on which header file to use for the smm_relocate_gdt in src/cpu/x86/smm/smm_stub.S
I originally proposed smm_segs.h (arch/x86/include/arc) but was rejected and has since been abandoned.
https://review.coreboot.org/c/coreboot/+/38657/1/src/arch/x86/include/arch/r... File src/arch/x86/include/arch/rom_segs.h:
https://review.coreboot.org/c/coreboot/+/38657/1/src/arch/x86/include/arch/r... PS1, Line 20: ROM_TASK_STATE_SEG
That segment doesn't exist in romstage gdt, see src/arch/x86/gdt_init. […]
The gdt that is referenced for the STM setup, for the SMM descriptor table, is the smm_relocate_gdt, which is in src/cpu/x86/smm_stub.S
I chose rom_segs.h out of the header files that you restricted me to because its defines matched the smm_relocate_gdt's offsets. I had to add the ROM_TASK_STATE_SEG because the STM setup code needs to properly setup the tr reg.
Originally, the smm_segs.h header file was created to match the smm_relocate_gdt but was rejected.
Given this information, what would you like me to do. If you have any questions please ask.
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38657 )
Change subject: arch/x86/include/arch: Add ROM_TASK_STATE_SEG ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38657/1/src/arch/x86/include/arch/r... File src/arch/x86/include/arch/rom_segs.h:
https://review.coreboot.org/c/coreboot/+/38657/1/src/arch/x86/include/arch/r... PS1, Line 20: ROM_TASK_STATE_SEG
The gdt that is referenced for the STM setup, for the SMM descriptor table, is the smm_relocate_gdt, […]
let's keep this here. And let's add a comment: /* This define is placed here to make sure future romstage programmers know about it. * It is used for STM setup code. */ and let's call it
#define SMM_TASK_STATE_SEG 0x20
then let's try to use it for a while until we figure out the best place.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38657 )
Change subject: arch/x86/include/arch: Add ROM_TASK_STATE_SEG ......................................................................
Patch Set 2: -Code-Review
cedarhouse1@comcast.net has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38657 )
Change subject: arch/x86/include/arch: Add ROM_TASK_STATE_SEG ......................................................................
Patch Set 2:
Patch Set 2:
(1 comment)
done
Hello Patrick Rudolph, ron minnich, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38657
to look at the new patch set (#3).
Change subject: arch/x86/include/arch: Add SMM_TASK_STATE_SEG ......................................................................
arch/x86/include/arch: Add SMM_TASK_STATE_SEG
This define is used to set up the STM SMM Descriptor table tr entry.
Signed-off-by: Eugene D. Myers edmyers@tycho.nsa.gov Change-Id: Iddb1f45444d03465a66a4ebb9fde5f206dc5b300 --- M src/arch/x86/include/arch/rom_segs.h 1 file changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/38657/3
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38657 )
Change subject: arch/x86/include/arch: Add SMM_TASK_STATE_SEG ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38657 )
Change subject: arch/x86/include/arch: Add SMM_TASK_STATE_SEG ......................................................................
arch/x86/include/arch: Add SMM_TASK_STATE_SEG
This define is used to set up the STM SMM Descriptor table tr entry.
Signed-off-by: Eugene D. Myers edmyers@tycho.nsa.gov Change-Id: Iddb1f45444d03465a66a4ebb9fde5f206dc5b300 Reviewed-on: https://review.coreboot.org/c/coreboot/+/38657 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: ron minnich rminnich@gmail.com --- M src/arch/x86/include/arch/rom_segs.h 1 file changed, 7 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified ron minnich: Looks good to me, approved
diff --git a/src/arch/x86/include/arch/rom_segs.h b/src/arch/x86/include/arch/rom_segs.h index a19d3de..c11def6 100644 --- a/src/arch/x86/include/arch/rom_segs.h +++ b/src/arch/x86/include/arch/rom_segs.h @@ -18,4 +18,11 @@ #define ROM_DATA_SEG 0x10 #define ROM_CODE_SEG64 0x18
+/* + * This define is placed here to make sure future romstage programmers + * know about it. + * It is used for STM setup code. + */ +#define SMM_TASK_STATE_SEG 0x20 + #endif /* ROM_SEGS_H */
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38657 )
Change subject: arch/x86/include/arch: Add SMM_TASK_STATE_SEG ......................................................................
Patch Set 4:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/434 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/433 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/432
Please note: This test is under development and might not be accurate at all!
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38657 )
Change subject: arch/x86/include/arch: Add SMM_TASK_STATE_SEG ......................................................................
Patch Set 4:
Please, where SMM_TASK_STATE_SEG is used?
cedarhouse1@comcast.net has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38657 )
Change subject: arch/x86/include/arch: Add SMM_TASK_STATE_SEG ......................................................................
Patch Set 4:
Patch Set 4:
Please, where SMM_TASK_STATE_SEG is used?
SMM_TASK_STATE_SEG is used by setup_smm_descriptor() (security/intel/stm/StmPlatformSmm.c) as part of its setting up the smm descriptor. The smm descriptor is used by the STM to setup the virtual machine runtime environment for the SMI handler. In this case, the GDT referenced is the smm_relocate_gdt found in cpu/x86/smm/smm_stub.S
-
9elements QA (Code Review) -
HAOUAS Elyes (Code Review)
-
Name of user not set (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
ron minnich (Code Review)