Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34134 )
Change subject: cpu/x86: Declare SMM_ASEG ......................................................................
cpu/x86: Declare SMM_ASEG
This is really an inverse of SMM_TSEG to flag platforms that should potentially move away from ASEG implementation.
Change-Id: I3b9007c55c75a59a9e6acc0a0e701300f7d21f87 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/Kconfig M src/cpu/amd/agesa/Kconfig M src/cpu/amd/pi/Kconfig M src/cpu/intel/model_f2x/Kconfig M src/cpu/qemu-x86/Kconfig M src/cpu/x86/Kconfig M src/cpu/x86/smm/smmrelocate.S M src/southbridge/intel/i82801dx/Makefile.inc M src/southbridge/intel/i82801ix/Makefile.inc 9 files changed, 18 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/34134/1
diff --git a/src/Kconfig b/src/Kconfig index 778f169..38209ee 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -543,10 +543,6 @@ file containing NVRAM/CMOS bit definitions. It defaults to 'n' but can be selected in mainboard/*/Kconfig.
-config HAVE_SMI_HANDLER - bool - default n - config PCI_IO_CFG_EXT bool default n diff --git a/src/cpu/amd/agesa/Kconfig b/src/cpu/amd/agesa/Kconfig index 5f7e0f9..4c5463c 100644 --- a/src/cpu/amd/agesa/Kconfig +++ b/src/cpu/amd/agesa/Kconfig @@ -30,6 +30,7 @@ select LAPIC_MONOTONIC_TIMER select SPI_FLASH if HAVE_ACPI_RESUME select POSTCAR_STAGE + select SMM_ASEG
if CPU_AMD_AGESA
diff --git a/src/cpu/amd/pi/Kconfig b/src/cpu/amd/pi/Kconfig index 8dc7f5a..a902089 100644 --- a/src/cpu/amd/pi/Kconfig +++ b/src/cpu/amd/pi/Kconfig @@ -29,6 +29,7 @@ select LAPIC_MONOTONIC_TIMER select SPI_FLASH if HAVE_ACPI_RESUME select POSTCAR_STAGE if !BINARYPI_LEGACY_WRAPPER + select SMM_ASEG
if CPU_AMD_PI
diff --git a/src/cpu/intel/model_f2x/Kconfig b/src/cpu/intel/model_f2x/Kconfig index 5ef1539..9e70775 100644 --- a/src/cpu/intel/model_f2x/Kconfig +++ b/src/cpu/intel/model_f2x/Kconfig @@ -6,3 +6,4 @@ select ARCH_RAMSTAGE_X86_32 select SMP select SUPPORT_CPU_UCODE_IN_CBFS + select SMM_ASEG diff --git a/src/cpu/qemu-x86/Kconfig b/src/cpu/qemu-x86/Kconfig index 70cce9b..0473e2f 100644 --- a/src/cpu/qemu-x86/Kconfig +++ b/src/cpu/qemu-x86/Kconfig @@ -22,3 +22,4 @@ select SMP select UDELAY_TSC select C_ENVIRONMENT_BOOTBLOCK + select SMM_ASEG diff --git a/src/cpu/x86/Kconfig b/src/cpu/x86/Kconfig index 99a7075..f264cda 100644 --- a/src/cpu/x86/Kconfig +++ b/src/cpu/x86/Kconfig @@ -88,6 +88,13 @@ bool default y
+config HAVE_SMI_HANDLER + def_bool n + depends on (SMM_ASEG || SMM_TSEG) + +config SMM_ASEG + def_bool n + config SMM_TSEG bool default n diff --git a/src/cpu/x86/smm/smmrelocate.S b/src/cpu/x86/smm/smmrelocate.S index c282904..e23b082 100644 --- a/src/cpu/x86/smm/smmrelocate.S +++ b/src/cpu/x86/smm/smmrelocate.S @@ -32,10 +32,9 @@ // ADDR32() macro #include <arch/registers.h>
-#if CONFIG(SMM_TSEG) -#error "Don't use this file with TSEG." - -#endif /* CONFIG_SMM_TSEG */ +#if !CONFIG(SMM_ASEG) +#error "Only use this file with ASEG." +#endif /* CONFIG_SMM_ASEG */
#define LAPIC_ID 0xfee00020
diff --git a/src/southbridge/intel/i82801dx/Makefile.inc b/src/southbridge/intel/i82801dx/Makefile.inc index ae0a10f..a8931ff 100644 --- a/src/southbridge/intel/i82801dx/Makefile.inc +++ b/src/southbridge/intel/i82801dx/Makefile.inc @@ -24,8 +24,11 @@ ramstage-y += usb.c ramstage-y += usb2.c
+ifeq ($(CONFIG_SMM_ASEG),y) ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smi.c ramstage-$(CONFIG_HAVE_SMI_HANDLER) += ../../../cpu/x86/smm/smmrelocate.S +endif + smm-y += smihandler.c
romstage-y += early_smbus.c diff --git a/src/southbridge/intel/i82801ix/Makefile.inc b/src/southbridge/intel/i82801ix/Makefile.inc index 0b9ade8..49db123 100644 --- a/src/southbridge/intel/i82801ix/Makefile.inc +++ b/src/southbridge/intel/i82801ix/Makefile.inc @@ -29,7 +29,7 @@
ramstage-srcs += src/mainboard/$(MAINBOARDDIR)/hda_verb.c
-ifneq ($(CONFIG_SMM_TSEG),y) +ifeq ($(CONFIG_SMM_ASEG),y) ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smi.c ramstage-$(CONFIG_HAVE_SMI_HANDLER) += ../../../cpu/x86/smm/smmrelocate.S endif
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34134 )
Change subject: cpu/x86: Declare SMM_ASEG ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/34134/1/src/cpu/x86/Kconfig File src/cpu/x86/Kconfig:
https://review.coreboot.org/c/coreboot/+/34134/1/src/cpu/x86/Kconfig@96 PS1, Line 96: def_bool n Simply using `bool` instead of `def_bool n` would avoid having a `# CONFIG_SMM_ASEG is not set` in every (even non-x86) `.config`.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34134 )
Change subject: cpu/x86: Declare SMM_ASEG ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34134/1/src/cpu/x86/Kconfig File src/cpu/x86/Kconfig:
https://review.coreboot.org/c/coreboot/+/34134/1/src/cpu/x86/Kconfig@96 PS1, Line 96: def_bool n
Simply using `bool` instead of `def_bool n` would avoid having […]
Will fix. Btw, why no if ARCH_X86 guard for the entire cpu/x86/Kconfig?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34134 )
Change subject: cpu/x86: Declare SMM_ASEG ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34134/1/src/cpu/x86/Kconfig File src/cpu/x86/Kconfig:
https://review.coreboot.org/c/coreboot/+/34134/1/src/cpu/x86/Kconfig@96 PS1, Line 96: def_bool n
Will fix. […]
No particular reason I could see...
Hello Patrick Rudolph, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34134
to look at the new patch set (#2).
Change subject: cpu/x86: Declare SMM_ASEG ......................................................................
cpu/x86: Declare SMM_ASEG
This is really an inverse of SMM_TSEG to flag platforms that should potentially move away from ASEG implementation.
Change-Id: I3b9007c55c75a59a9e6acc0a0e701300f7d21f87 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/Kconfig M src/cpu/amd/agesa/Kconfig M src/cpu/amd/pi/Kconfig M src/cpu/intel/model_f2x/Kconfig M src/cpu/qemu-x86/Kconfig M src/cpu/x86/Kconfig M src/cpu/x86/smm/smmrelocate.S M src/southbridge/intel/i82801dx/Makefile.inc M src/southbridge/intel/i82801ix/Makefile.inc 9 files changed, 20 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/34134/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34134 )
Change subject: cpu/x86: Declare SMM_ASEG ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34134/2/src/southbridge/intel/i8280... File src/southbridge/intel/i82801dx/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34134/2/src/southbridge/intel/i8280... PS2, Line 28: $(CONFIG_HAVE_SMI_HANDLER) Can get rid of this?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34134 )
Change subject: cpu/x86: Declare SMM_ASEG ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34134/2/src/southbridge/intel/i8280... File src/southbridge/intel/i82801dx/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34134/2/src/southbridge/intel/i8280... PS2, Line 28: $(CONFIG_HAVE_SMI_HANDLER)
Can get rid of this?
Probably not, I think lapic_cpu_init.c would pull duplicate smm_init() definition.
I probably have this converted to TSEG soon anyway since this is broken, on paper at least. I have this legacy hardware to play around and for PCI-X slots.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34134 )
Change subject: cpu/x86: Declare SMM_ASEG ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/34134/2/src/southbridge/intel/i8280... File src/southbridge/intel/i82801dx/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34134/2/src/southbridge/intel/i8280... PS2, Line 28: $(CONFIG_HAVE_SMI_HANDLER)
Probably not, I think lapic_cpu_init.c would pull duplicate smm_init() definition. […]
Aah I see.
Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34134 )
Change subject: cpu/x86: Declare SMM_ASEG ......................................................................
cpu/x86: Declare SMM_ASEG
This is really an inverse of SMM_TSEG to flag platforms that should potentially move away from ASEG implementation.
Change-Id: I3b9007c55c75a59a9e6acc0a0e701300f7d21f87 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34134 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/Kconfig M src/cpu/amd/agesa/Kconfig M src/cpu/amd/pi/Kconfig M src/cpu/intel/model_f2x/Kconfig M src/cpu/qemu-x86/Kconfig M src/cpu/x86/Kconfig M src/cpu/x86/smm/smmrelocate.S M src/southbridge/intel/i82801dx/Makefile.inc M src/southbridge/intel/i82801ix/Makefile.inc 9 files changed, 20 insertions(+), 9 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/Kconfig b/src/Kconfig index 778f169..38209ee 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -543,10 +543,6 @@ file containing NVRAM/CMOS bit definitions. It defaults to 'n' but can be selected in mainboard/*/Kconfig.
-config HAVE_SMI_HANDLER - bool - default n - config PCI_IO_CFG_EXT bool default n diff --git a/src/cpu/amd/agesa/Kconfig b/src/cpu/amd/agesa/Kconfig index 5f7e0f9..4c5463c 100644 --- a/src/cpu/amd/agesa/Kconfig +++ b/src/cpu/amd/agesa/Kconfig @@ -30,6 +30,7 @@ select LAPIC_MONOTONIC_TIMER select SPI_FLASH if HAVE_ACPI_RESUME select POSTCAR_STAGE + select SMM_ASEG
if CPU_AMD_AGESA
diff --git a/src/cpu/amd/pi/Kconfig b/src/cpu/amd/pi/Kconfig index 8dc7f5a..a902089 100644 --- a/src/cpu/amd/pi/Kconfig +++ b/src/cpu/amd/pi/Kconfig @@ -29,6 +29,7 @@ select LAPIC_MONOTONIC_TIMER select SPI_FLASH if HAVE_ACPI_RESUME select POSTCAR_STAGE if !BINARYPI_LEGACY_WRAPPER + select SMM_ASEG
if CPU_AMD_PI
diff --git a/src/cpu/intel/model_f2x/Kconfig b/src/cpu/intel/model_f2x/Kconfig index 5ef1539..9e70775 100644 --- a/src/cpu/intel/model_f2x/Kconfig +++ b/src/cpu/intel/model_f2x/Kconfig @@ -6,3 +6,4 @@ select ARCH_RAMSTAGE_X86_32 select SMP select SUPPORT_CPU_UCODE_IN_CBFS + select SMM_ASEG diff --git a/src/cpu/qemu-x86/Kconfig b/src/cpu/qemu-x86/Kconfig index 70cce9b..0473e2f 100644 --- a/src/cpu/qemu-x86/Kconfig +++ b/src/cpu/qemu-x86/Kconfig @@ -22,3 +22,4 @@ select SMP select UDELAY_TSC select C_ENVIRONMENT_BOOTBLOCK + select SMM_ASEG diff --git a/src/cpu/x86/Kconfig b/src/cpu/x86/Kconfig index 99a7075..d230a57 100644 --- a/src/cpu/x86/Kconfig +++ b/src/cpu/x86/Kconfig @@ -88,6 +88,15 @@ bool default y
+config HAVE_SMI_HANDLER + bool + default n + depends on (SMM_ASEG || SMM_TSEG) + +config SMM_ASEG + bool + default n + config SMM_TSEG bool default n diff --git a/src/cpu/x86/smm/smmrelocate.S b/src/cpu/x86/smm/smmrelocate.S index c282904..e23b082 100644 --- a/src/cpu/x86/smm/smmrelocate.S +++ b/src/cpu/x86/smm/smmrelocate.S @@ -32,10 +32,9 @@ // ADDR32() macro #include <arch/registers.h>
-#if CONFIG(SMM_TSEG) -#error "Don't use this file with TSEG." - -#endif /* CONFIG_SMM_TSEG */ +#if !CONFIG(SMM_ASEG) +#error "Only use this file with ASEG." +#endif /* CONFIG_SMM_ASEG */
#define LAPIC_ID 0xfee00020
diff --git a/src/southbridge/intel/i82801dx/Makefile.inc b/src/southbridge/intel/i82801dx/Makefile.inc index ae0a10f..a8931ff 100644 --- a/src/southbridge/intel/i82801dx/Makefile.inc +++ b/src/southbridge/intel/i82801dx/Makefile.inc @@ -24,8 +24,11 @@ ramstage-y += usb.c ramstage-y += usb2.c
+ifeq ($(CONFIG_SMM_ASEG),y) ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smi.c ramstage-$(CONFIG_HAVE_SMI_HANDLER) += ../../../cpu/x86/smm/smmrelocate.S +endif + smm-y += smihandler.c
romstage-y += early_smbus.c diff --git a/src/southbridge/intel/i82801ix/Makefile.inc b/src/southbridge/intel/i82801ix/Makefile.inc index 0b9ade8..49db123 100644 --- a/src/southbridge/intel/i82801ix/Makefile.inc +++ b/src/southbridge/intel/i82801ix/Makefile.inc @@ -29,7 +29,7 @@
ramstage-srcs += src/mainboard/$(MAINBOARDDIR)/hda_verb.c
-ifneq ($(CONFIG_SMM_TSEG),y) +ifeq ($(CONFIG_SMM_ASEG),y) ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smi.c ramstage-$(CONFIG_HAVE_SMI_HANDLER) += ../../../cpu/x86/smm/smmrelocate.S endif
-
Furquan Shaikh (Code Review) -
Kyösti Mälkki (Code Review)
-
Nico Huber (Code Review)