coreboot-gerrit November 2022

coreboot-gerrit@coreboot.org

1 participants
7286 discussions

[S] Change in coreboot[master]: mb/google/brya : Set EPP value for Vell board
by Felix Held (Code Review) 07 Nov '22

07 Nov '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/68900 ) Change subject: mb/google/brya : Set EPP value for Vell board ...................................................................... mb/google/brya : Set EPP value for Vell board The patch sets the EPP to 50% (0x80) for Vell. With EPP at 50%, the Vell system demonstrated better power improvement without sacrificing the performance. PLT Results(Perf) with EPP@40% and EPP@50%: EPP@40%: Device1-656 mins, Device2-664 mins. EPP@50%: Device1-678 mins, Device2-677 mins. In short, with EPP@50%, PLT KPI ran for more than 13 to 22mins compared to EPP@40%. Branch=firmware-brya-14505.B BUG=b:215526166 TEST=Verified code build for Vell board Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Change-Id: I41b15b84025d25cf59dac2d85826a3de9d725bae Reviewed-on: https://review.coreboot.org/c/coreboot/+/68900 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Subrata Banik <subratabanik(a)google.com> --- M src/mainboard/google/brya/variants/vell/overridetree.cb 1 file changed, 32 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved diff --git a/src/mainboard/google/brya/variants/vell/overridetree.cb b/src/mainboard/google/brya/variants/vell/overridetree.cb index 98983ef..0c50eb4 100644 --- a/src/mainboard/google/brya/variants/vell/overridetree.cb +++ b/src/mainboard/google/brya/variants/vell/overridetree.cb @@ -76,6 +76,10 @@ register "tcss_ports[3]" = "TCSS_PORT_DEFAULT(OC3)" register "sagv" = "SaGv_Enabled" + # Set EPP to 50%: 50 * 256 / 100 = 0x80 + register "enable_energy_perf_pref" = "true" + register "energy_perf_pref_value" = "0x80" + # FIVR RFI Spread Spectrum 6% register "fivr_spread_spectrum" = "FIVR_SS_6" -- To view, visit https://review.coreboot.org/c/coreboot/+/68900 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I41b15b84025d25cf59dac2d85826a3de9d725bae Gerrit-Change-Number: 68900 Gerrit-PatchSet: 6 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kane Chen <kane.chen(a)intel.com> Gerrit-Reviewer: Kedar J. Karanje <kedar.j.karanje(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: mb/google: Probe p-sensor only for selected variants
by Felix Held (Code Review) 07 Nov '22

07 Nov '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69191 ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: mb/google: Probe p-sensor only for selected variants ...................................................................... mb/google: Probe p-sensor only for selected variants Only a subset of variants has proximity sensors. This patch by itself does not introduce functional changes to any board. It is mainly to ease migrating SX9324 from the legacy driver to the linux one - allowing gradual migration variant by variant. BUG=b:242662878 TEST=Dump ACPI SSDT then verify they are identical w/ and w/o this patch Change-Id: Ic00e0d9eafcef2c9eaf32571fecf6190777cec36 Signed-off-by: Victor Ding <victording(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/69191 Reviewed-by: Reka Norman <rekanorman(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/mainboard/google/brya/Kconfig M src/mainboard/google/brya/Kconfig.name M src/mainboard/google/dedede/Kconfig M src/mainboard/google/dedede/Kconfig.name M src/mainboard/google/zork/Kconfig M src/mainboard/google/zork/Kconfig.name 6 files changed, 34 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Reka Norman: Looks good to me, approved diff --git a/src/mainboard/google/brya/Kconfig b/src/mainboard/google/brya/Kconfig index 68c2780..a114860 100644 --- a/src/mainboard/google/brya/Kconfig +++ b/src/mainboard/google/brya/Kconfig @@ -6,7 +6,6 @@ select DRIVERS_I2C_GENERIC select DRIVERS_I2C_HID select DRIVERS_I2C_NAU8825 - select DRIVERS_I2C_SX9324 select DRIVERS_INTEL_DPTF select DRIVERS_INTEL_DPTF_SUPPORTS_TPCH select DRIVERS_INTEL_PMC diff --git a/src/mainboard/google/brya/Kconfig.name b/src/mainboard/google/brya/Kconfig.name index fc91205..4ead1f9 100644 --- a/src/mainboard/google/brya/Kconfig.name +++ b/src/mainboard/google/brya/Kconfig.name @@ -39,6 +39,7 @@ select BOARD_GOOGLE_BASEBOARD_BRYA select SOC_INTEL_RAPTORLAKE select DRIVERS_GENESYSLOGIC_GL9755 + select DRIVERS_I2C_SX9324 select DRIVERS_INTEL_MIPI_CAMERA select SOC_INTEL_COMMON_BLOCK_IPU select SOC_INTEL_CRASHLOG @@ -83,6 +84,7 @@ select BOARD_ROMSIZE_KB_32768 select DRIVERS_GENERIC_GPIO_KEYS select DRIVERS_GENESYSLOGIC_GL9750 + select DRIVERS_I2C_SX9324 select DRIVERS_INTEL_MIPI_CAMERA select HAVE_WWAN_POWER_SEQUENCE @@ -136,6 +138,7 @@ select BOARD_GOOGLE_BASEBOARD_SKOLAS select DRIVERS_GENESYSLOGIC_GL9755 select DRIVERS_INTEL_MIPI_CAMERA + select DRIVERS_I2C_SX9324 select SOC_INTEL_COMMON_BLOCK_IPU config BOARD_GOOGLE_SKOLAS4ES @@ -143,6 +146,7 @@ select BOARD_GOOGLE_BASEBOARD_SKOLAS select DRIVERS_GENESYSLOGIC_GL9755 select DRIVERS_INTEL_MIPI_CAMERA + select DRIVERS_I2C_SX9324 select SOC_INTEL_COMMON_BLOCK_IPU config BOARD_GOOGLE_TAEKO @@ -252,6 +256,7 @@ select DRIVERS_GENERIC_BAYHUB_LV2 select DRIVERS_GENERIC_GPIO_KEYS select DRIVERS_GENESYSLOGIC_GL9750 + select DRIVERS_I2C_SX9324 select HAVE_WWAN_POWER_SEQUENCE config BOARD_GOOGLE_XIVU diff --git a/src/mainboard/google/dedede/Kconfig b/src/mainboard/google/dedede/Kconfig index 4767018..8901da3 100644 --- a/src/mainboard/google/dedede/Kconfig +++ b/src/mainboard/google/dedede/Kconfig @@ -6,8 +6,6 @@ select DRIVERS_I2C_GENERIC select DRIVERS_I2C_GPIO_MUX select DRIVERS_I2C_HID - select DRIVERS_I2C_SX9324 - select DRIVERS_I2C_SX9360 select DRIVERS_INTEL_DPTF select DRIVERS_SPI_ACPI select DRIVERS_USB_ACPI diff --git a/src/mainboard/google/dedede/Kconfig.name b/src/mainboard/google/dedede/Kconfig.name index 3bb6363..45cc117 100644 --- a/src/mainboard/google/dedede/Kconfig.name +++ b/src/mainboard/google/dedede/Kconfig.name @@ -4,6 +4,7 @@ bool "-> Boten (Lenovo 500e Chromebook Gen 3)" select BOARD_GOOGLE_BASEBOARD_DEDEDE_CR50 select BASEBOARD_DEDEDE_LAPTOP + select DRIVERS_I2C_SX9324 select GEO_SAR_ENABLE if CHROMEOS_WIFI_SAR config BOARD_GOOGLE_DEDEDE @@ -106,6 +107,7 @@ bool "-> Storo (Asus Chromebook CR1100)" select BOARD_GOOGLE_BASEBOARD_DEDEDE_CR50 select BASEBOARD_DEDEDE_LAPTOP + select DRIVERS_I2C_SX9324 select DRIVERS_INTEL_MIPI_CAMERA select SOC_INTEL_COMMON_BLOCK_IPU select GEO_SAR_ENABLE if CHROMEOS_WIFI_SAR @@ -121,6 +123,7 @@ select BOARD_GOOGLE_BASEBOARD_DEDEDE_CR50 select BASEBOARD_DEDEDE_LAPTOP select DRIVERS_GENERIC_MAX98357A + select DRIVERS_I2C_SX9324 select GEO_SAR_ENABLE if CHROMEOS_WIFI_SAR config BOARD_GOOGLE_BLIPPER @@ -154,6 +157,7 @@ select BASEBOARD_DEDEDE_LAPTOP select DRIVERS_GENERIC_MAX98357A select DRIVERS_I2C_DA7219 + select DRIVERS_I2C_SX9360 select GEO_SAR_ENABLE if CHROMEOS_WIFI_SAR config BOARD_GOOGLE_CORORI @@ -172,6 +176,7 @@ bool "-> Gooey" select BOARD_GOOGLE_BASEBOARD_DEDEDE_TPM2 select BASEBOARD_DEDEDE_LAPTOP + select DRIVERS_I2C_SX9324 select GEO_SAR_ENABLE if CHROMEOS_WIFI_SAR config BOARD_GOOGLE_BEADRIX @@ -179,6 +184,7 @@ select BOARD_GOOGLE_BASEBOARD_DEDEDE_CR50 select BASEBOARD_DEDEDE_LAPTOP select DRIVERS_GENERIC_MAX98357A + select DRIVERS_I2C_SX9324 config BOARD_GOOGLE_SHOTZO bool "-> Shotzo" diff --git a/src/mainboard/google/zork/Kconfig b/src/mainboard/google/zork/Kconfig index 25d4ca4..d483999 100644 --- a/src/mainboard/google/zork/Kconfig +++ b/src/mainboard/google/zork/Kconfig @@ -21,7 +21,6 @@ select DRIVERS_GENERIC_ALC1015 select DRIVERS_I2C_GENERIC select DRIVERS_I2C_HID - select DRIVERS_I2C_SX9324 select EC_GOOGLE_CHROMEEC select EC_GOOGLE_CHROMEEC_AUDIO_CODEC select EC_GOOGLE_CHROMEEC_BOARDID diff --git a/src/mainboard/google/zork/Kconfig.name b/src/mainboard/google/zork/Kconfig.name index bac7ee4..7ba518a 100644 --- a/src/mainboard/google/zork/Kconfig.name +++ b/src/mainboard/google/zork/Kconfig.name @@ -7,6 +7,7 @@ config BOARD_GOOGLE_VILBOZ bool "-> Vilboz (Lenovo 100e/300e Gen3 AMD)" select BOARD_GOOGLE_BASEBOARD_DALBOZ + select DRIVERS_I2C_SX9324 config BOARD_GOOGLE_EZKINIL bool "-> Ezkinil (Acer Chromebook Spin 514)" -- To view, visit https://review.coreboot.org/c/coreboot/+/69191 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic00e0d9eafcef2c9eaf32571fecf6190777cec36 Gerrit-Change-Number: 69191 Gerrit-PatchSet: 4 Gerrit-Owner: Victor Ding <victording(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: cbfstool: Fix possible memory leak
by Yu-Ping Wu (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: V Sowmya, Shahina Shaik, Haribalaraman Ramasubramanian, Sridhar Siricilla, Balaji Manigandan. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69220 ) Change subject: cbfstool: Fix possible memory leak ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/69220 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I01c4643d1e671d9bd9971ac6db8031634fffd61e Gerrit-Change-Number: 69220 Gerrit-PatchSet: 6 Gerrit-Owner: Shahina Shaik <shahina.shaik(a)intel.com> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Shahina Shaik <shahina.shaik(a)intel.corp-partner.google.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: V Sowmya <v.sowmya(a)intel.com> Gerrit-Attention: Shahina Shaik <shahina.shaik(a)intel.com> Gerrit-Attention: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Comment-Date: Mon, 07 Nov 2022 14:12:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: Allow to build romstage sources inside the bootblock
by Erik van den Bogaert (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Jakub Czapiga, ron minnich, Paul Menzel, Stefan Reinauer, Julius Werner, Angel Pons, Arthur Heymans, Keith Hui, Patrick Rudolph, Branden Waldner, Martin L Roth, Christian Walter, Felix Held. Erik van den Bogaert has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55068 ) Change subject: Allow to build romstage sources inside the bootblock ...................................................................... Patch Set 33: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/55068 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id68390edc1ba228b121cca89b80c64a92553e284 Gerrit-Change-Number: 55068 Gerrit-PatchSet: 33 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)chromium.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Branden Waldner <scruffy99(a)gmail.com> Gerrit-CC: Keith Hui <buurin(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: ron minnich <rminnich(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Branden Waldner <scruffy99(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 07 Nov 2022 14:01:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: mb/*: Remove lapic from devicetree
by Erik van den Bogaert (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Felix Singer, Frans Hendriks, Patrick Rudolph, Jonathan Zhang, Jeremy Soller, Angel Pons, Arthur Heymans, Jonathon Hall, Michael Niewöhner, Piotr Król, Tim Crawford, Tarun Tuli, Nico Huber, Michał Żygowski, Maxim Polyakov, Johnny Lin, Christian Walter, Morgan Jang, Werner Zeh, Tim Chu. Erik van den Bogaert has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69303 ) Change subject: mb/*: Remove lapic from devicetree ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/69303 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5258a769c0f0ee4bbc4facc19737eed187b68c73 Gerrit-Change-Number: 69303 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Mon, 07 Nov 2022 14:00:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: device/resource_allocator_v3: Drop code
by Arthur Heymans (Code Review) 07 Nov '22

07 Nov '22

Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69140 ) Change subject: device/resource_allocator_v3: Drop code ...................................................................... device/resource_allocator_v3: Drop code No platform uses this anymore. Change-Id: Ifccb59ae45daa8fec41a9a2d46c628ff24a0c998 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/69140 Reviewed-by: Elyes Haouas <ehaouas(a)noos.fr> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> --- M src/device/Kconfig M src/device/Makefile.inc D src/device/resource_allocator_v3.c 3 files changed, 18 insertions(+), 564 deletions(-) Approvals: build bot (Jenkins): Verified Elyes Haouas: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved Eric Lai: Looks good to me, but someone else must approve diff --git a/src/device/Kconfig b/src/device/Kconfig index 60ee047..a41f261 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -699,7 +699,6 @@ config PCIEXP_HOTPLUG_PREFETCH_MEM_ABOVE_4G bool - depends on RESOURCE_ALLOCATOR_V4 default y if !PCIEXP_HOTPLUG_PREFETCH_MEM_BELOW_4G default n help @@ -922,28 +921,9 @@ maximum time a device could stretch clock bits before the transfer is aborted and an error returned. -config RESOURCE_ALLOCATOR_V3 - bool - default n - help - This config option enables resource allocator v3 which performs - top down allocation of resources in a single MMIO window. This is the - old resource allocator meant to be used only until the broken AMD - chipsets are fixed. DO NOT USE THIS FOR ANY NEW CHIPSETS! - -config RESOURCE_ALLOCATOR_V4 - bool - default n if RESOURCE_ALLOCATOR_V3 - default y if !RESOURCE_ALLOCATOR_V3 - help - This config option enables resource allocator v4 which uses multiple - ranges for allocating resources. This allows allocation of resources - above 4G boundary as well. - config RESOURCE_ALLOCATION_TOP_DOWN bool "Allocate resources from top down" default n - depends on RESOURCE_ALLOCATOR_V4 help Should be the default, but many platforms don't report resources correctly. Hence, the allocator might cause conflicts. diff --git a/src/device/Makefile.inc b/src/device/Makefile.inc index 808648d..28acd73 100644 --- a/src/device/Makefile.inc +++ b/src/device/Makefile.inc @@ -59,8 +59,7 @@ ramstage-y += mmio.c ramstage-y += resource_allocator_common.c -ramstage-$(CONFIG_RESOURCE_ALLOCATOR_V3) += resource_allocator_v3.c -ramstage-$(CONFIG_RESOURCE_ALLOCATOR_V4) += resource_allocator_v4.c +ramstage-y += resource_allocator_v4.c ramstage-$(CONFIG_XHCI_UTILS) += xhci.c diff --git a/src/device/resource_allocator_v3.c b/src/device/resource_allocator_v3.c deleted file mode 100644 index 236e149..0000000 --- a/src/device/resource_allocator_v3.c +++ /dev/null @@ -1,542 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <console/console.h> -#include <device/device.h> -#include <post.h> - -/** - * Round a number up to an alignment. - * - * @param val The starting value. - * @param pow Alignment as a power of two. - * @return Rounded up number. - */ -static resource_t round(resource_t val, unsigned long pow) -{ - resource_t mask; - mask = (1ULL << pow) - 1ULL; - val += mask; - val &= ~mask; - return val; -} - -static const char *resource2str(struct resource *res) -{ - if (res->flags & IORESOURCE_IO) - return "io"; - if (res->flags & IORESOURCE_PREFETCH) - return "prefmem"; - if (res->flags & IORESOURCE_MEM) - return "mem"; - return "undefined"; -} - -/** - * This function is the guts of the resource allocator. - * - * The problem. - * - Allocate resource locations for every device. - * - Don't overlap, and follow the rules of bridges. - * - Don't overlap with resources in fixed locations. - * - Be efficient so we don't have ugly strategies. - * - * The strategy. - * - Devices that have fixed addresses are the minority so don't - * worry about them too much. Instead only use part of the address - * space for devices with programmable addresses. This easily handles - * everything except bridges. - * - * - PCI devices are required to have their sizes and their alignments - * equal. In this case an optimal solution to the packing problem - * exists. Allocate all devices from highest alignment to least - * alignment or vice versa. Use this. - * - * - So we can handle more than PCI run two allocation passes on bridges. The - * first to see how large the resources are behind the bridge, and what - * their alignment requirements are. The second to assign a safe address to - * the devices behind the bridge. This allows us to treat a bridge as just - * a device with a couple of resources, and not need to special case it in - * the allocator. Also this allows handling of other types of bridges. - * - * @param bus The bus we are traversing. - * @param bridge The bridge resource which must contain the bus' resources. - * @param type_mask This value gets ANDed with the resource type. - * @param type This value must match the result of the AND. - * @return TODO - */ -static void compute_resources(struct bus *bus, struct resource *bridge, - unsigned long type_mask, unsigned long type) -{ - const struct device *dev; - struct resource *resource; - resource_t base; - base = round(bridge->base, bridge->align); - - if (!bus) - return; - - printk(BIOS_SPEW, "%s %s: base: %llx size: %llx align: %d gran: %d" - " limit: %llx\n", dev_path(bus->dev), resource2str(bridge), - base, bridge->size, bridge->align, - bridge->gran, bridge->limit); - - /* For each child which is a bridge, compute the resource needs. */ - for (dev = bus->children; dev; dev = dev->sibling) { - struct resource *child_bridge; - - if (!dev->link_list) - continue; - - /* Find the resources with matching type flags. */ - for (child_bridge = dev->resource_list; child_bridge; - child_bridge = child_bridge->next) { - struct bus* link; - - if (!(child_bridge->flags & IORESOURCE_BRIDGE) - || (child_bridge->flags & type_mask) != type) - continue; - - /* - * Split prefetchable memory if combined. Many domains - * use the same address space for prefetchable memory - * and non-prefetchable memory. Bridges below them need - * it separated. Add the PREFETCH flag to the type_mask - * and type. - */ - link = dev->link_list; - while (link && link->link_num != - IOINDEX_LINK(child_bridge->index)) - link = link->next; - - if (link == NULL) { - printk(BIOS_ERR, "link %ld not found on %s\n", - IOINDEX_LINK(child_bridge->index), - dev_path(dev)); - } - - compute_resources(link, child_bridge, - type_mask | IORESOURCE_PREFETCH, - type | (child_bridge->flags & - IORESOURCE_PREFETCH)); - } - } - - /* Remember we haven't found anything yet. */ - resource = NULL; - - /* - * Walk through all the resources on the current bus and compute the - * amount of address space taken by them. Take granularity and - * alignment into account. - */ - while ((dev = largest_resource(bus, &resource, type_mask, type))) { - - /* Size 0 resources can be skipped. */ - if (!resource->size) - continue; - - /* Propagate the resource alignment to the bridge resource. */ - if (resource->align > bridge->align) - bridge->align = resource->align; - - /* Propagate the resource limit to the bridge register. */ - if (bridge->limit > resource->limit) - bridge->limit = resource->limit; - - /* Warn if it looks like APICs aren't declared. */ - if ((resource->limit == 0xffffffff) && - (resource->flags & IORESOURCE_ASSIGNED)) { - printk(BIOS_ERR, - "Resource limit looks wrong! (no APIC?)\n"); - printk(BIOS_ERR, "%s %02lx limit %08llx\n", - dev_path(dev), resource->index, resource->limit); - } - - if (resource->flags & IORESOURCE_IO) { - /* - * Don't allow potential aliases over the legacy PCI - * expansion card addresses. The legacy PCI decodes - * only 10 bits, uses 0x100 - 0x3ff. Therefore, only - * 0x00 - 0xff can be used out of each 0x400 block of - * I/O space. - */ - if ((base & 0x300) != 0) { - base = (base & ~0x3ff) + 0x400; - } - /* - * Don't allow allocations in the VGA I/O range. - * PCI has special cases for that. - */ - else if ((base >= 0x3b0) && (base <= 0x3df)) { - base = 0x3e0; - } - } - /* Base must be aligned. */ - base = round(base, resource->align); - resource->base = base; - base += resource->size; - - printk(BIOS_SPEW, "%s %02lx * [0x%llx - 0x%llx] %s\n", - dev_path(dev), resource->index, resource->base, - resource->base + resource->size - 1, - resource2str(resource)); - } - - /* - * A PCI bridge resource does not need to be a power of two size, but - * it does have a minimum granularity. Round the size up to that - * minimum granularity so we know not to place something else at an - * address positively decoded by the bridge. - */ - bridge->size = round(base, bridge->gran) - - round(bridge->base, bridge->align); - - printk(BIOS_SPEW, "%s %s: base: %llx size: %llx align: %d gran: %d" - " limit: %llx done\n", dev_path(bus->dev), - resource2str(bridge), - base, bridge->size, bridge->align, bridge->gran, bridge->limit); -} - -/** - * This function is the second part of the resource allocator. - * - * See the compute_resources function for a more detailed explanation. - * - * This function assigns the resources a value. - * - * @param bus The bus we are traversing. - * @param bridge The bridge resource which must contain the bus' resources. - * @param type_mask This value gets ANDed with the resource type. - * @param type This value must match the result of the AND. - * - * @see compute_resources - */ -static void __allocate_resources(struct bus *bus, struct resource *bridge, - unsigned long type_mask, unsigned long type) -{ - const struct device *dev; - struct resource *resource; - resource_t base; - base = bridge->base; - - if (!bus) - return; - - printk(BIOS_SPEW, "%s %s: base:%llx size:%llx align:%d gran:%d " - "limit:%llx\n", dev_path(bus->dev), - resource2str(bridge), - base, bridge->size, bridge->align, bridge->gran, bridge->limit); - - /* Remember we haven't found anything yet. */ - resource = NULL; - - /* - * Walk through all the resources on the current bus and allocate them - * address space. - */ - while ((dev = largest_resource(bus, &resource, type_mask, type))) { - - /* Propagate the bridge limit to the resource register. */ - if (resource->limit > bridge->limit) - resource->limit = bridge->limit; - - /* Size 0 resources can be skipped. */ - if (!resource->size) - continue; - - if (resource->flags & IORESOURCE_IO) { - /* - * Don't allow potential aliases over the legacy PCI - * expansion card addresses. The legacy PCI decodes - * only 10 bits, uses 0x100 - 0x3ff. Therefore, only - * 0x00 - 0xff can be used out of each 0x400 block of - * I/O space. - */ - if ((base & 0x300) != 0) { - base = (base & ~0x3ff) + 0x400; - } - /* - * Don't allow allocations in the VGA I/O range. - * PCI has special cases for that. - */ - else if ((base >= 0x3b0) && (base <= 0x3df)) { - base = 0x3e0; - } - } - - if ((round(base, resource->align) + resource->size - 1) <= - resource->limit) { - /* Base must be aligned. */ - base = round(base, resource->align); - resource->base = base; - resource->limit = resource->base + resource->size - 1; - resource->flags |= IORESOURCE_ASSIGNED; - resource->flags &= ~IORESOURCE_STORED; - base += resource->size; - } else { - printk(BIOS_ERR, "!! Resource didn't fit !!\n"); - printk(BIOS_ERR, " aligned base %llx size %llx " - "limit %llx\n", round(base, resource->align), - resource->size, resource->limit); - printk(BIOS_ERR, " %llx needs to be <= %llx " - "(limit)\n", (round(base, resource->align) + - resource->size) - 1, resource->limit); - printk(BIOS_ERR, " %s%s %02lx * [0x%llx - 0x%llx]" - " %s\n", (resource->flags & IORESOURCE_ASSIGNED) - ? "Assigned: " : "", dev_path(dev), - resource->index, resource->base, - resource->base + resource->size - 1, - resource2str(resource)); - } - - printk(BIOS_SPEW, "%s %02lx * [0x%llx - 0x%llx] %s\n", - dev_path(dev), resource->index, resource->base, - resource->size ? resource->base + resource->size - 1 : - resource->base, resource2str(resource)); - } - - /* - * A PCI bridge resource does not need to be a power of two size, but - * it does have a minimum granularity. Round the size up to that - * minimum granularity so we know not to place something else at an - * address positively decoded by the bridge. - */ - - bridge->flags |= IORESOURCE_ASSIGNED; - - printk(BIOS_SPEW, "%s %s: next_base: %llx size: %llx align: %d " - "gran: %d done\n", dev_path(bus->dev), - resource2str(bridge), base, bridge->size, bridge->align, - bridge->gran); - - /* For each child which is a bridge, __allocate_resources. */ - for (dev = bus->children; dev; dev = dev->sibling) { - struct resource *child_bridge; - - if (!dev->link_list) - continue; - - /* Find the resources with matching type flags. */ - for (child_bridge = dev->resource_list; child_bridge; - child_bridge = child_bridge->next) { - struct bus* link; - - if (!(child_bridge->flags & IORESOURCE_BRIDGE) || - (child_bridge->flags & type_mask) != type) - continue; - - /* - * Split prefetchable memory if combined. Many domains - * use the same address space for prefetchable memory - * and non-prefetchable memory. Bridges below them need - * it separated. Add the PREFETCH flag to the type_mask - * and type. - */ - link = dev->link_list; - while (link && link->link_num != - IOINDEX_LINK(child_bridge->index)) - link = link->next; - if (link == NULL) - printk(BIOS_ERR, "link %ld not found on %s\n", - IOINDEX_LINK(child_bridge->index), - dev_path(dev)); - - __allocate_resources(link, child_bridge, - type_mask | IORESOURCE_PREFETCH, - type | (child_bridge->flags & - IORESOURCE_PREFETCH)); - } - } -} - -static int resource_is(struct resource *res, u32 type) -{ - return (res->flags & IORESOURCE_TYPE_MASK) == type; -} - -struct constraints { - struct resource io, mem; -}; - -static struct resource *resource_limit(struct constraints *limits, - struct resource *res) -{ - struct resource *lim = NULL; - - /* MEM, or I/O - skip any others. */ - if (resource_is(res, IORESOURCE_MEM)) - lim = &limits->mem; - else if (resource_is(res, IORESOURCE_IO)) - lim = &limits->io; - - return lim; -} - -static void constrain_resources(const struct device *dev, - struct constraints* limits) -{ - const struct device *child; - struct resource *res; - struct resource *lim; - struct bus *link; - - /* Constrain limits based on the fixed resources of this device. */ - for (res = dev->resource_list; res; res = res->next) { - if (!(res->flags & IORESOURCE_FIXED)) - continue; - if (!res->size) { - /* It makes no sense to have 0-sized, fixed resources.*/ - printk(BIOS_ERR, "skipping %s@%lx fixed resource, " - "size=0!\n", dev_path(dev), res->index); - continue; - } - - lim = resource_limit(limits, res); - if (!lim) - continue; - - /* - * Is it a fixed resource outside the current known region? - * If so, we don't have to consider it - it will be handled - * correctly and doesn't affect current region's limits. - */ - if (((res->base + res->size -1) < lim->base) - || (res->base > lim->limit)) - continue; - - printk(BIOS_SPEW, "%s: %s %02lx base %08llx limit %08llx %s (fixed)\n", - __func__, dev_path(dev), res->index, res->base, - res->base + res->size - 1, resource2str(res)); - - /* - * Choose to be above or below fixed resources. This check is - * signed so that "negative" amounts of space are handled - * correctly. - */ - if ((signed long long)(lim->limit - (res->base + res->size -1)) - > (signed long long)(res->base - lim->base)) - lim->base = res->base + res->size; - else - lim->limit = res->base -1; - } - - /* Descend into every enabled child and look for fixed resources. */ - for (link = dev->link_list; link; link = link->next) { - for (child = link->children; child; child = child->sibling) { - if (child->enabled) - constrain_resources(child, limits); - } - } -} - -static void avoid_fixed_resources(const struct device *dev) -{ - struct constraints limits; - struct resource *res; - struct resource *lim; - - printk(BIOS_SPEW, "%s: %s\n", __func__, dev_path(dev)); - - /* Initialize constraints to maximum size. */ - limits.io.base = 0; - limits.io.limit = 0xffffffffffffffffULL; - limits.mem.base = 0; - limits.mem.limit = 0xffffffffffffffffULL; - - /* Constrain the limits to dev's initial resources. */ - for (res = dev->resource_list; res; res = res->next) { - if ((res->flags & IORESOURCE_FIXED)) - continue; - printk(BIOS_SPEW, "%s:@%s %02lx limit %08llx\n", __func__, - dev_path(dev), res->index, res->limit); - - lim = resource_limit(&limits, res); - if (!lim) - continue; - - if (res->base > lim->base) - lim->base = res->base; - if (res->limit < lim->limit) - lim->limit = res->limit; - } - - /* Look through the tree for fixed resources and update the limits. */ - constrain_resources(dev, &limits); - - /* Update dev's resources with new limits. */ - for (res = dev->resource_list; res; res = res->next) { - if ((res->flags & IORESOURCE_FIXED)) - continue; - - lim = resource_limit(&limits, res); - if (!lim) - continue; - - /* Is the resource outside the limits? */ - if (lim->base > res->base) - res->base = lim->base; - if (res->limit > lim->limit) - res->limit = lim->limit; - - /* MEM resources need to start at the highest address manageable. */ - if (res->flags & IORESOURCE_MEM) - res->base = resource_max(res); - - printk(BIOS_SPEW, "%s:@%s %02lx base %08llx limit %08llx\n", - __func__, dev_path(dev), res->index, res->base, res->limit); - } -} - -void allocate_resources(const struct device *root) -{ - struct resource *res; - const struct device *child; - - /* Compute resources for all domains. */ - for (child = root->link_list->children; child; child = child->sibling) { - if (!(child->path.type == DEVICE_PATH_DOMAIN)) - continue; - post_log_path(child); - for (res = child->resource_list; res; res = res->next) { - if (res->flags & IORESOURCE_FIXED) - continue; - if (res->flags & IORESOURCE_MEM) { - compute_resources(child->link_list, - res, IORESOURCE_TYPE_MASK, IORESOURCE_MEM); - continue; - } - if (res->flags & IORESOURCE_IO) { - compute_resources(child->link_list, - res, IORESOURCE_TYPE_MASK, IORESOURCE_IO); - continue; - } - } - } - - /* For all domains. */ - for (child = root->link_list->children; child; child=child->sibling) - if (child->path.type == DEVICE_PATH_DOMAIN) - avoid_fixed_resources(child); - - /* Store the computed resource allocations into device registers ... */ - printk(BIOS_INFO, "Setting resources...\n"); - for (child = root->link_list->children; child; child = child->sibling) { - if (!(child->path.type == DEVICE_PATH_DOMAIN)) - continue; - post_log_path(child); - for (res = child->resource_list; res; res = res->next) { - if (res->flags & IORESOURCE_FIXED) - continue; - if (res->flags & IORESOURCE_MEM) { - __allocate_resources(child->link_list, - res, IORESOURCE_TYPE_MASK, IORESOURCE_MEM); - continue; - } - if (res->flags & IORESOURCE_IO) { - __allocate_resources(child->link_list, - res, IORESOURCE_TYPE_MASK, IORESOURCE_IO); - continue; - } - } - } -} -- To view, visit https://review.coreboot.org/c/coreboot/+/69140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifccb59ae45daa8fec41a9a2d46c628ff24a0c998 Gerrit-Change-Number: 69140 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: Kconfig: Set better defaults for building romstage
by Erik van den Bogaert (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Frans Hendriks, Arthur Heymans, Andrey Petrov. Erik van den Bogaert has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63419 ) Change subject: Kconfig: Set better defaults for building romstage ...................................................................... Patch Set 35: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/63419 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4c53b6fa7a3e66415c5b1c539918a6c1cd8defa1 Gerrit-Change-Number: 63419 Gerrit-PatchSet: 35 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Mon, 07 Nov 2022 14:00:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: cpu/x86: Drop !CPU_INFO_V2 code
by Arthur Heymans (Code Review) 07 Nov '22

07 Nov '22

Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69122 ) Change subject: cpu/x86: Drop !CPU_INFO_V2 code ...................................................................... cpu/x86: Drop !CPU_INFO_V2 code Now that all platforms use parallel_mp this is the only codepath used for cpu_info() local thread storage. Change-Id: I119214e703aea8a4fe93f83b784159cf86d859d3 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/69122 Reviewed-by: Elyes Haouas <ehaouas(a)noos.fr> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/Kconfig M src/arch/x86/c_start.S M src/arch/x86/include/arch/cpu.h M src/cpu/x86/Kconfig M src/security/intel/txt/getsec_enteraccs.S 5 files changed, 20 insertions(+), 25 deletions(-) Approvals: build bot (Jenkins): Verified Elyes Haouas: Looks good to me, approved Angel Pons: Looks good to me, approved diff --git a/src/Kconfig b/src/Kconfig index b33ea62..08b2437 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -715,7 +715,7 @@ config COOP_MULTITASKING def_bool n select TIMER_QUEUE - depends on ARCH_X86 && CPU_INFO_V2 + depends on ARCH_X86 help Cooperative multitasking allows callbacks to be multiplexed on the main thread. With this enabled it allows for multiple execution paths diff --git a/src/arch/x86/c_start.S b/src/arch/x86/c_start.S index 84fbed2..02a9b89 100644 --- a/src/arch/x86/c_start.S +++ b/src/arch/x86/c_start.S @@ -80,7 +80,6 @@ push_cpu_info -#if CONFIG(CPU_INFO_V2) /* Allocate the per_cpu_segment_data on the stack */ push_per_cpu_segment_data @@ -93,7 +92,6 @@ mov $per_cpu_segment_selector, %eax movl (%eax), %eax mov %eax, %gs -#endif /* * Now we are finished. Memory is up, data is copied and @@ -222,7 +220,6 @@ .word 0xffff, 0x0000 .byte 0x00, 0x9b, 0xaf, 0x00 #endif -#if CONFIG(CPU_INFO_V2) per_cpu_segment_descriptors: .rept CONFIG_MAX_CPUS /* flat data segment */ @@ -233,14 +230,11 @@ .byte 0x00, 0x93, 0xcf, 0x00 #endif .endr -#endif /* CPU_INFO_V2 */ gdt_end: -#if CONFIG(CPU_INFO_V2) /* Segment selector pointing to the first per_cpu_segment_descriptor. */ per_cpu_segment_selector: .long per_cpu_segment_descriptors - gdt -#endif /* CPU_INFO_V2 */ .section ".text._start", "ax", @progbits #if ENV_X86_64 diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h index 2d2879f6..0087a79 100644 --- a/src/arch/x86/include/arch/cpu.h +++ b/src/arch/x86/include/arch/cpu.h @@ -149,8 +149,6 @@ static inline struct cpu_info *cpu_info(void) { -/* We use a #if because we don't want to mess with the &s below. */ -#if CONFIG(CPU_INFO_V2) struct cpu_info *ci = NULL; __asm__("mov %%gs:%c[offset], %[ci]" @@ -159,11 +157,6 @@ ); return ci; -#else - char s; - uintptr_t info = ALIGN_UP((uintptr_t)&s, CONFIG_STACK_SIZE) - sizeof(struct cpu_info); - return (struct cpu_info *)info; -#endif /* CPU_INFO_V2 */ } struct cpuinfo_x86 { diff --git a/src/cpu/x86/Kconfig b/src/cpu/x86/Kconfig index b136e9e..1d6b489 100644 --- a/src/cpu/x86/Kconfig +++ b/src/cpu/x86/Kconfig @@ -2,7 +2,6 @@ config PARALLEL_MP def_bool y - select CPU_INFO_V2 help This option uses common MP infrastructure for bringing up APs in parallel. It additionally provides a more flexible mechanism @@ -210,12 +209,4 @@ However, modern OSes use PAT to control cacheability instead of using MTRRs. -config CPU_INFO_V2 - bool - depends on PARALLEL_MP - help - Enables the new method of locating struct cpu_info. This new method - uses the %gs segment to locate the cpu_info pointer. The old method - relied on the stack being CONFIG_STACK_SIZE aligned. - endif # ARCH_X86 diff --git a/src/security/intel/txt/getsec_enteraccs.S b/src/security/intel/txt/getsec_enteraccs.S index cbb24b6..ff9db05 100644 --- a/src/security/intel/txt/getsec_enteraccs.S +++ b/src/security/intel/txt/getsec_enteraccs.S @@ -227,7 +227,7 @@ movd %esp, %xmm0 movd %ebp, %xmm1 - /* Backup %gs used by CPU_INFO_V2 */ + /* Backup %gs used by cpu_info() */ movl %gs, %eax movd %eax, %xmm2 @@ -265,7 +265,7 @@ movl %eax, %es movl %eax, %ss movl %eax, %fs - /* Restore %gs used by CPU_INFO_V2 */ + /* Restore %gs used by cpu_info */ movd %xmm2, %eax movl %eax, %gs -- To view, visit https://review.coreboot.org/c/coreboot/+/69122 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I119214e703aea8a4fe93f83b784159cf86d859d3 Gerrit-Change-Number: 69122 Gerrit-PatchSet: 17 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XL] Change in coreboot[master]: cpu/x86: Drop LEGACY_SMP_INIT
by Arthur Heymans (Code Review) 07 Nov '22

07 Nov '22

Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69121 ) Change subject: cpu/x86: Drop LEGACY_SMP_INIT ...................................................................... cpu/x86: Drop LEGACY_SMP_INIT This codepath is deprecated after the 4.18 release. Change-Id: I7e90f457f3979781d06323ef1350d5fb05a6be43 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/69121 Reviewed-by: Elyes Haouas <ehaouas(a)noos.fr> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/cpu/x86/Kconfig M src/cpu/x86/lapic/Makefile.inc D src/cpu/x86/lapic/lapic_cpu_init.c D src/cpu/x86/lapic/secondary.S M src/cpu/x86/smm/Makefile.inc D src/cpu/x86/smm/smihandler.c D src/cpu/x86/smm/smm.ld D src/cpu/x86/smm/smmhandler.S D src/cpu/x86/smm/smmrelocate.S 9 files changed, 17 insertions(+), 1,226 deletions(-) Approvals: build bot (Jenkins): Verified Elyes Haouas: Looks good to me, approved Angel Pons: Looks good to me, approved diff --git a/src/cpu/x86/Kconfig b/src/cpu/x86/Kconfig index 3a0c29b..b136e9e 100644 --- a/src/cpu/x86/Kconfig +++ b/src/cpu/x86/Kconfig @@ -2,7 +2,6 @@ config PARALLEL_MP def_bool y - depends on !LEGACY_SMP_INIT select CPU_INFO_V2 help This option uses common MP infrastructure for bringing up APs @@ -27,9 +26,6 @@ with a stub at 0x30000. This is useful on platforms that have an alternative way to set SMBASE. -config LEGACY_SMP_INIT - bool - config DEFAULT_X2APIC def_bool n help @@ -154,13 +150,7 @@ default y depends on !(NO_SMM || SMM_ASEG) -config SMM_LEGACY_ASEG - bool - default y if HAVE_SMI_HANDLER && SMM_ASEG && LEGACY_SMP_INIT - help - SMM support without PARALLEL_MP, to be deprecated. - -if HAVE_SMI_HANDLER && !SMM_LEGACY_ASEG +if HAVE_SMI_HANDLER config SMM_MODULE_STACK_SIZE hex diff --git a/src/cpu/x86/lapic/Makefile.inc b/src/cpu/x86/lapic/Makefile.inc index 91a41f7..22bd987 100644 --- a/src/cpu/x86/lapic/Makefile.inc +++ b/src/cpu/x86/lapic/Makefile.inc @@ -1,6 +1,4 @@ ramstage-$(CONFIG_AP_IN_SIPI_WAIT) += lapic_cpu_stop.c -ramstage-$(CONFIG_LEGACY_SMP_INIT) += lapic_cpu_init.c -ramstage-$(CONFIG_LEGACY_SMP_INIT) += secondary.S bootblock-$(CONFIG_UDELAY_LAPIC) += apic_timer.c romstage-$(CONFIG_UDELAY_LAPIC) += apic_timer.c diff --git a/src/cpu/x86/lapic/lapic_cpu_init.c b/src/cpu/x86/lapic/lapic_cpu_init.c deleted file mode 100644 index c52a898..0000000 --- a/src/cpu/x86/lapic/lapic_cpu_init.c +++ /dev/null @@ -1,420 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <cpu/x86/cr.h> -#include <cpu/x86/gdt.h> -#include <cpu/x86/lapic.h> -#include <cpu/x86/smi_deprecated.h> -#include <acpi/acpi.h> -#include <delay.h> -#include <lib.h> -#include <string.h> -#include <symbols.h> -#include <console/console.h> -#include <device/device.h> -#include <device/path.h> -#include <smp/atomic.h> -#include <smp/spinlock.h> -#include <cpu/cpu.h> -#include <cpu/intel/speedstep.h> -#include <smp/node.h> -#include <stdlib.h> -#include <thread.h> - -/* This is a lot more paranoid now, since Linux can NOT handle - * being told there is a CPU when none exists. So any errors - * will return 0, meaning no CPU. - * - * We actually handling that case by noting which cpus startup - * and not telling anyone about the ones that don't. - */ - -/* Start-UP IPI vector must be 4kB aligned and below 1MB. */ -#define AP_SIPI_VECTOR 0x1000 - -static char *lowmem_backup; -static char *lowmem_backup_ptr; -static int lowmem_backup_size; - -static inline void setup_secondary_gdt(void) -{ - u16 *gdt_limit; -#if ENV_X86_64 - u64 *gdt_base; -#else - u32 *gdt_base; -#endif - - gdt_limit = (void *)&_secondary_gdt_addr; - gdt_base = (void *)&gdt_limit[1]; - - *gdt_limit = (uintptr_t)&gdt_end - (uintptr_t)&gdt - 1; - *gdt_base = (uintptr_t)&gdt; -} - -static void copy_secondary_start_to_lowest_1M(void) -{ - unsigned long code_size; - - /* Fill in secondary_start's local gdt. */ - setup_secondary_gdt(); - - code_size = (unsigned long)_secondary_start_end - - (unsigned long)_secondary_start; - - if (acpi_is_wakeup_s3()) { - /* need to save it for RAM resume */ - lowmem_backup_size = code_size; - lowmem_backup = malloc(code_size); - lowmem_backup_ptr = (char *)AP_SIPI_VECTOR; - - if (lowmem_backup == NULL) - die("Out of backup memory\n"); - - memcpy(lowmem_backup, lowmem_backup_ptr, lowmem_backup_size); - } - - /* copy the _secondary_start to the RAM below 1M*/ - memcpy((unsigned char *)AP_SIPI_VECTOR, - (unsigned char *)_secondary_start, code_size); - - printk(BIOS_DEBUG, "start_eip=0x%08lx, code_size=0x%08lx\n", - (unsigned long int)AP_SIPI_VECTOR, code_size); -} - -static void recover_lowest_1M(void) -{ - if (acpi_is_wakeup_s3()) - memcpy(lowmem_backup_ptr, lowmem_backup, lowmem_backup_size); -} - -static uint32_t wait_for_ipi_completion(const int timeout_ms) -{ - int loops = timeout_ms * 10; - uint32_t send_status; - - /* wait for the ipi send to finish */ - printk(BIOS_SPEW, "Waiting for send to finish...\n"); - do { - printk(BIOS_SPEW, "+"); - udelay(100); - send_status = lapic_busy(); - } while (send_status && (--loops > 0)); - - return send_status; -} - -static int lapic_start_cpu(unsigned long apicid) -{ - const int timeout_100ms = 100; - uint32_t send_status, accept_status; - int j, maxlvt; - - /* - * Starting actual IPI sequence... - */ - - printk(BIOS_SPEW, "Asserting INIT.\n"); - - /* - * Turn INIT on target chip - */ - lapic_send_ipi(LAPIC_INT_LEVELTRIG | LAPIC_INT_ASSERT | LAPIC_DM_INIT, apicid); - - send_status = wait_for_ipi_completion(timeout_100ms); - if (send_status) { - printk(BIOS_ERR, "CPU %ld: First APIC write timed out. " - "Disabling\n", apicid); - // too bad. - printk(BIOS_ERR, "ESR is 0x%x\n", lapic_read(LAPIC_ESR)); - if (lapic_read(LAPIC_ESR)) { - printk(BIOS_ERR, "Try to reset ESR\n"); - lapic_write(LAPIC_ESR, 0); - printk(BIOS_ERR, "ESR is 0x%x\n", - lapic_read(LAPIC_ESR)); - } - return 0; - } - mdelay(10); - - printk(BIOS_SPEW, "Deasserting INIT.\n"); - - lapic_send_ipi(LAPIC_INT_LEVELTRIG | LAPIC_DM_INIT, apicid); - - send_status = wait_for_ipi_completion(timeout_100ms); - if (send_status) { - printk(BIOS_ERR, "CPU %ld: Second APIC write timed out. " - "Disabling\n", apicid); - // too bad. - return 0; - } - - /* - * Run STARTUP IPI loop. - */ - printk(BIOS_SPEW, "#startup loops: %d.\n", CONFIG_NUM_IPI_STARTS); - - maxlvt = 4; - - for (j = 1; j <= CONFIG_NUM_IPI_STARTS; j++) { - printk(BIOS_SPEW, "Sending STARTUP #%d to %lu.\n", j, apicid); - lapic_read(LAPIC_SPIV); - lapic_write(LAPIC_ESR, 0); - lapic_read(LAPIC_ESR); - printk(BIOS_SPEW, "After apic_write.\n"); - - /* - * STARTUP IPI - */ - - lapic_send_ipi(LAPIC_DM_STARTUP | (AP_SIPI_VECTOR >> 12), apicid); - - /* - * Give the other CPU some time to accept the IPI. - */ - udelay(300); - - printk(BIOS_SPEW, "Startup point 1.\n"); - - send_status = wait_for_ipi_completion(timeout_100ms); - - /* - * Give the other CPU some time to accept the IPI. - */ - udelay(200); - /* - * Due to the Pentium erratum 3AP. - */ - if (maxlvt > 3) { - lapic_read(LAPIC_SPIV); - lapic_write(LAPIC_ESR, 0); - } - accept_status = (lapic_read(LAPIC_ESR) & 0xEF); - if (send_status || accept_status) - break; - } - printk(BIOS_SPEW, "After Startup.\n"); - if (send_status) - printk(BIOS_WARNING, "APIC never delivered???\n"); - if (accept_status) - printk(BIOS_WARNING, "APIC delivery error (%x).\n", - accept_status); - if (send_status || accept_status) - return 0; - return 1; -} - -/* Number of cpus that are currently running in coreboot */ -static atomic_t active_cpus = ATOMIC_INIT(1); - -/* start_cpu_lock covers last_cpu_index and secondary_stack. - * Only starting one CPU at a time let's me remove the logic - * for select the stack from assembly language. - * - * In addition communicating by variables to the CPU I - * am starting allows me to verify it has started before - * start_cpu returns. - */ - -DECLARE_SPIN_LOCK(start_cpu_lock); -static unsigned int last_cpu_index = 0; -static void *stacks[CONFIG_MAX_CPUS]; -volatile unsigned long secondary_stack; -volatile unsigned int secondary_cpu_index; - -static int start_cpu(struct device *cpu) -{ - struct cpu_info *info; - uintptr_t stack_top; - uintptr_t stack_base; - unsigned long apicid; - unsigned int index; - unsigned long count; - int result; - - spin_lock(&start_cpu_lock); - - /* Get the CPU's apicid */ - apicid = cpu->path.apic.apic_id; - - /* Get an index for the new processor */ - index = ++last_cpu_index; - - /* Find boundaries of the new processor's stack */ - stack_top = ALIGN_DOWN((uintptr_t)_estack, CONFIG_STACK_SIZE); - stack_top -= (CONFIG_STACK_SIZE*index); - stack_base = stack_top - CONFIG_STACK_SIZE; - stack_top -= sizeof(struct cpu_info); - printk(BIOS_SPEW, "CPU%d: stack_base %p, stack_top %p\n", index, - (void *)stack_base, (void *)stack_top); - stacks[index] = (void *)stack_base; - - /* Record the index and which CPU structure we are using */ - info = (struct cpu_info *)stack_top; - info->index = index; - info->cpu = cpu; - cpu_add_map_entry(info->index); - - /* Advertise the new stack and index to start_cpu */ - secondary_stack = stack_top; - secondary_cpu_index = index; - - /* Until the CPU starts up report the CPU is not enabled */ - cpu->enabled = 0; - cpu->initialized = 0; - - /* Start the CPU */ - result = lapic_start_cpu(apicid); - - if (result) { - result = 0; - /* Wait 1s or until the new CPU calls in */ - for (count = 0; count < 100000; count++) { - if (secondary_stack == 0) { - result = 1; - break; - } - udelay(10); - } - } - secondary_stack = 0; - spin_unlock(&start_cpu_lock); - return result; -} - -/* C entry point of secondary cpus */ -asmlinkage void secondary_cpu_init(unsigned int index) -{ - atomic_inc(&active_cpus); - - spin_lock(&start_cpu_lock); - -#ifdef __SSE3__ - /* - * Seems that CR4 was cleared when AP start via lapic_start_cpu() - * Turn on CR4.OSFXSR and CR4.OSXMMEXCPT when SSE options enabled - */ - CRx_TYPE cr4_val; - cr4_val = read_cr4(); - cr4_val |= (CR4_OSFXSR | CR4_OSXMMEXCPT); - write_cr4(cr4_val); -#endif - - /* Ensure the local APIC is enabled */ - enable_lapic(); - setup_lapic_interrupts(); - - cpu_initialize(index); - - spin_unlock(&start_cpu_lock); - - atomic_dec(&active_cpus); - - stop_this_cpu(); -} - -static void start_other_cpus(struct bus *cpu_bus, struct device *bsp_cpu) -{ - struct device *cpu; - /* Loop through the cpus once getting them started */ - - for (cpu = cpu_bus->children; cpu; cpu = cpu->sibling) { - if (cpu->path.type != DEVICE_PATH_APIC) - continue; - - if (!cpu->enabled) - continue; - - if (cpu->initialized) - continue; - - if (!start_cpu(cpu)) - /* Record the error in cpu? */ - printk(BIOS_ERR, "CPU 0x%02x would not start!\n", - cpu->path.apic.apic_id); - - udelay(10); - } -} - -static void wait_other_cpus_stop(struct bus *cpu_bus) -{ - struct device *cpu; - int old_active_count, active_count; - long loopcount = 0; - int i; - - /* Now loop until the other cpus have finished initializing */ - old_active_count = 1; - active_count = atomic_read(&active_cpus); - while (active_count > 1) { - if (active_count != old_active_count) { - printk(BIOS_INFO, "Waiting for %d CPUS to stop\n", - active_count - 1); - old_active_count = active_count; - } - udelay(10); - active_count = atomic_read(&active_cpus); - loopcount++; - } - for (cpu = cpu_bus->children; cpu; cpu = cpu->sibling) { - if (cpu->path.type != DEVICE_PATH_APIC) - continue; - if (cpu->path.apic.apic_id == SPEEDSTEP_APIC_MAGIC) - continue; - if (!cpu->initialized) - printk(BIOS_ERR, "CPU 0x%02x did not initialize!\n", - cpu->path.apic.apic_id); - } - printk(BIOS_DEBUG, "All AP CPUs stopped (%ld loops)\n", loopcount); - checkstack(_estack, 0); - for (i = 1; i < CONFIG_MAX_CPUS && i <= last_cpu_index; i++) - checkstack((void *)stacks[i] + CONFIG_STACK_SIZE, i); -} - -void initialize_cpus(struct bus *cpu_bus) -{ - struct device_path cpu_path; - struct cpu_info *info; - - /* Find the info struct for this CPU */ - info = cpu_info(); - - /* Ensure the local APIC is enabled */ - if (is_smp_boot()) { - enable_lapic(); - setup_lapic_interrupts(); - } else { - disable_lapic(); - } - - /* Get the device path of the boot CPU */ - cpu_path.type = DEVICE_PATH_APIC; - cpu_path.apic.apic_id = lapicid(); - - /* Find the device structure for the boot CPU */ - info->cpu = alloc_find_dev(cpu_bus, &cpu_path); - cpu_add_map_entry(info->index); - - // why here? In case some day we can start core1 in amd_sibling_init - if (is_smp_boot()) - copy_secondary_start_to_lowest_1M(); - - if (CONFIG(SMM_LEGACY_ASEG)) - smm_init(); - - /* Initialize the bootstrap processor */ - cpu_initialize(0); - - if (is_smp_boot()) - start_other_cpus(cpu_bus, info->cpu); - - /* Now wait the rest of the cpus stop*/ - if (is_smp_boot()) - wait_other_cpus_stop(cpu_bus); - - if (CONFIG(SMM_LEGACY_ASEG)) - smm_init_completion(); - - if (is_smp_boot()) - recover_lowest_1M(); -} diff --git a/src/cpu/x86/lapic/secondary.S b/src/cpu/x86/lapic/secondary.S deleted file mode 100644 index d2d43a2..0000000 --- a/src/cpu/x86/lapic/secondary.S +++ /dev/null @@ -1,86 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <cpu/x86/lapic_def.h> -#include <arch/ram_segs.h> - - .text - .globl _secondary_start, _secondary_start_end, _secondary_gdt_addr - .balign 4096 -_secondary_start: - .code16 - cli - xorl %eax, %eax - movl %eax, %cr3 /* Invalidate TLB*/ - - /* On hyper threaded cpus, invalidating the cache here is - * very very bad. Don't. - */ - - /* setup the data segment */ - movw %cs, %ax - movw %ax, %ds - - lgdtl gdtaddr - _secondary_start - - movl %cr0, %eax - andl $0x7FFAFFD1, %eax /* PG,AM,WP,NE,TS,EM,MP = 0 */ - orl $0x60000001, %eax /* CD, NW, PE = 1 */ - movl %eax, %cr0 - - ljmpl $RAM_CODE_SEG, $__ap_protected_start - - /* This will get filled in by C code. */ -_secondary_gdt_addr: -gdtaddr: - .word 0 /* the table limit */ -#if ENV_X86_64 - .quad 0 -#else - .long 0 /* we know the offset */ -#endif - -_secondary_start_end: - -ap_protected_start: - .code32 - lgdt gdtaddr - ljmpl $RAM_CODE_SEG, $__ap_protected_start - -__ap_protected_start: - - movw $RAM_DATA_SEG, %ax - movw %ax, %ds - movw %ax, %es - movw %ax, %ss - xor %ax, %ax /* zero out the gs and fs segment index */ - movw %ax, %fs - movw %ax, %gs /* Will be used for cpu_info */ - - /* Load the Interrupt descriptor table */ - lidt idtarg - -#if ENV_X86_64 - /* entry64.inc preserves ebx. */ - #include <cpu/x86/64bit/entry64.inc> - movabs secondary_stack, %rax - mov %rax, %rsp - andl $0xfffffff0, %esp - movabs secondary_cpu_index, %rax - mov %rax, %rdi -#else - /* Set the stack pointer, and flag that we are done */ - xorl %eax, %eax - movl secondary_stack, %esp - - andl $0xfffffff0, %esp - sub $12, %esp /* maintain 16-byte alignment for the call below */ - movl secondary_cpu_index, %edi - pushl %edi - movl %eax, secondary_stack -#endif - - call secondary_cpu_init -1: hlt - jmp 1b - -.code32 diff --git a/src/cpu/x86/smm/Makefile.inc b/src/cpu/x86/smm/Makefile.inc index 5f69590..da93827 100644 --- a/src/cpu/x86/smm/Makefile.inc +++ b/src/cpu/x86/smm/Makefile.inc @@ -76,17 +76,3 @@ $(OBJCOPY_smm) -O binary $< $@ endif - -ifeq ($(CONFIG_SMM_LEGACY_ASEG),y) - -smm-y += smm.ld - -$(obj)/smm/smm: $(obj)/smm/smm.o $(call src-to-obj,smm,$(src)/cpu/x86/smm/smm.ld) - $(LD_smm) $(LDFLAGS_smm) -o $(obj)/smm/smm.elf -T $(call src-to-obj,smm,$(src)/cpu/x86/smm/smm.ld) $(obj)/smm/smm.o - $(NM_smm) -n $(obj)/smm/smm.elf | sort > $(obj)/smm/smm.map - $(OBJCOPY_smm) -O binary $(obj)/smm/smm.elf $@ - -smm-y += smmhandler.S -smm-y += smihandler.c - -endif # CONFIG_SMM_LEGACY_ASEG diff --git a/src/cpu/x86/smm/smihandler.c b/src/cpu/x86/smm/smihandler.c deleted file mode 100644 index 7946e90..0000000 --- a/src/cpu/x86/smm/smihandler.c +++ /dev/null @@ -1,207 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <arch/io.h> -#include <console/console.h> -#include <commonlib/region.h> -#include <cpu/x86/smm.h> -#include <cpu/x86/smi_deprecated.h> -#include <cpu/amd/amd64_save_state.h> -#include <cpu/intel/em64t100_save_state.h> -#include <cpu/intel/em64t101_save_state.h> -#include <cpu/x86/lapic.h> -#include <cpu/x86/lapic_def.h> -#include <cpu/x86/legacy_save_state.h> - -#if CONFIG(SPI_FLASH_SMM) -#include <spi-generic.h> -#endif - -static int do_driver_init = 1; - -typedef enum { SMI_LOCKED, SMI_UNLOCKED } smi_semaphore; - -/* SMI multiprocessing semaphore */ -static __attribute__((aligned(4))) volatile smi_semaphore smi_handler_status - = SMI_UNLOCKED; - -static int smi_obtain_lock(void) -{ - u8 ret = SMI_LOCKED; - - asm volatile ( - "movb %2, %%al\n" - "xchgb %%al, %1\n" - "movb %%al, %0\n" - : "=g" (ret), "=m" (smi_handler_status) - : "g" (SMI_LOCKED) - : "eax" - ); - - return (ret == SMI_UNLOCKED); -} - -static void smi_release_lock(void) -{ - asm volatile ( - "movb %1, %%al\n" - "xchgb %%al, %0\n" - : "=m" (smi_handler_status) - : "g" (SMI_UNLOCKED) - : "eax" - ); -} - -void io_trap_handler(int smif) -{ - /* If a handler function handled a given IO trap, it - * shall return a non-zero value - */ - printk(BIOS_DEBUG, "SMI function trap 0x%x: ", smif); - - if (southbridge_io_trap_handler(smif)) - return; - - if (mainboard_io_trap_handler(smif)) - return; - - printk(BIOS_DEBUG, "Unknown function\n"); -} - -/** - * @brief Set the EOS bit - */ -static void smi_set_eos(void) -{ - southbridge_smi_set_eos(); -} - -static u32 pci_orig; - -/** - * @brief Backup PCI address to make sure we do not mess up the OS - */ -static void smi_backup_pci_address(void) -{ - pci_orig = inl(0xcf8); -} - -/** - * @brief Restore PCI address previously backed up - */ -static void smi_restore_pci_address(void) -{ - outl(pci_orig, 0xcf8); -} - -static inline void *smm_save_state(uintptr_t base, int arch_offset, int node) -{ - base += SMM_SAVE_STATE_BEGIN(arch_offset) - (node * 0x400); - return (void *)base; -} - -/* This returns the SMM revision from the savestate of CPU0, - which is assumed to be the same for all CPU's. See the memory - map in smmhandler.S */ -uint32_t smm_revision(void) -{ - return *(uint32_t *)(SMM_BASE + SMM_ENTRY_OFFSET * 2 - SMM_REVISION_OFFSET_FROM_TOP); -} - -void *smm_get_save_state(int cpu) -{ - switch (smm_revision()) { - case 0x00030002: - case 0x00030007: - return smm_save_state(SMM_BASE, SMM_LEGACY_ARCH_OFFSET, cpu); - case 0x00030100: - return smm_save_state(SMM_BASE, SMM_EM64T100_ARCH_OFFSET, cpu); - case 0x00030101: /* SandyBridge, IvyBridge, and Haswell */ - return smm_save_state(SMM_BASE, SMM_EM64T101_ARCH_OFFSET, cpu); - case 0x00020064: - case 0x00030064: - return smm_save_state(SMM_BASE, SMM_AMD64_ARCH_OFFSET, cpu); - } - - return NULL; -} - -bool smm_region_overlaps_handler(const struct region *r) -{ - const struct region r_smm = {SMM_BASE, SMM_DEFAULT_SIZE}; - - return region_overlap(&r_smm, r); -} - -/** - * @brief Interrupt handler for SMI# - * - * @param smm_revision revision of the smm state save map - */ - -void smi_handler(void) -{ - unsigned int node; - - /* Are we ok to execute the handler? */ - if (!smi_obtain_lock()) { - /* For security reasons we don't release the other CPUs - * until the CPU with the lock is actually done - */ - while (smi_handler_status == SMI_LOCKED) { - asm volatile ( - ".byte 0xf3, 0x90\n" /* hint a CPU we are in - * spinlock (PAUSE - * instruction, REP NOP) - */ - ); - } - return; - } - - smi_backup_pci_address(); - - node = lapicid(); - - console_init(); - - printk(BIOS_SPEW, "\nSMI# #%d\n", node); - - /* Use smm_get_save_state() to see if the smm revision is supported */ - if (smm_get_save_state(node) == NULL) { - printk(BIOS_WARNING, "smm_revision: 0x%08x\n", smm_revision()); - printk(BIOS_WARNING, "SMI# not supported on your CPU\n"); - /* Don't release lock, so no further SMI will happen, - * if we don't handle it anyways. - */ - return; - } - - /* Allow drivers to initialize variables in SMM context. */ - if (do_driver_init) { -#if CONFIG(SPI_FLASH_SMM) - spi_init(); -#endif - do_driver_init = 0; - } - - /* Call chipset specific SMI handlers. */ - southbridge_smi_handler(); - - smi_restore_pci_address(); - - smi_release_lock(); - - /* De-assert SMI# signal to allow another SMI */ - smi_set_eos(); -} - -/* Provide a default implementation for all weak handlers so that relocation - * entries in the modules make sense. Without default implementations the - * weak relocations w/o a symbol have a 0 address which is where the modules - * are linked at. */ -int __weak mainboard_io_trap_handler(int smif) { return 0; } -void __weak southbridge_smi_handler(void) {} -void __weak mainboard_smi_gpi(u32 gpi_sts) {} -int __weak mainboard_smi_apmc(u8 data) { return 0; } -void __weak mainboard_smi_sleep(u8 slp_typ) {} -void __weak mainboard_smi_finalize(void) {} diff --git a/src/cpu/x86/smm/smm.ld b/src/cpu/x86/smm/smm.ld deleted file mode 100644 index e232028..0000000 --- a/src/cpu/x86/smm/smm.ld +++ /dev/null @@ -1,65 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -/* Maximum number of CPUs/cores */ -CPUS = 4; - -_ = ASSERT(CPUS >= CONFIG_MAX_CPUS, "The ASEG SMM code only supports up to 4 CPUS"); - -ENTRY(smm_handler_start); - -SECTIONS -{ - /* This is the actual SMM handler. - * - * We just put code, rodata, data and bss all in a row. - */ - . = 0xa0000; - .handler (.): { - _program = .; - /* Assembler stub */ - *(.handler) - - /* C code of the SMM handler */ - *(.text); - *(.text.*); - - /* C read-only data of the SMM handler */ - . = ALIGN(16); - *(.rodata) - *(.rodata.*) - - /* C read-write data of the SMM handler */ - . = ALIGN(4); - *(.data) - *(.data.*) - - /* C uninitialized data of the SMM handler */ - . = ALIGN(4); - *(.bss) - *(.bss.*) - *(.sbss) - *(.sbss.*) - - /* What is this? (Something we don't need with -fno-common.) */ - *(COMMON) - . = ALIGN(4); - _eprogram = .; - } - - /* We are using the ASEG interleaved to stuff the SMM handlers - * for all CPU cores in there. The jump table redirects the execution - * to the actual SMM handler - */ - . = 0xa8000 - (( CPUS - 1) * 0x400); - .jumptable : { - KEEP(*(.jumptable)); - } - - /DISCARD/ : { - *(.comment) - *(.note) - *(.note.*) - *(.eh_frame) - *(.debug_*) - } -} diff --git a/src/cpu/x86/smm/smmhandler.S b/src/cpu/x86/smm/smmhandler.S deleted file mode 100644 index 9e7108c..0000000 --- a/src/cpu/x86/smm/smmhandler.S +++ /dev/null @@ -1,258 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -/* NOTE: This handler assumes the SMM window goes from 0xa0000 - * to 0xaffff. In fact, at least on Intel Core CPUs (i945 chipset) - * the SMM window is 128K big, covering 0xa0000 to 0xbffff. - * So there is a lot of potential for growth in here. Let's stick - * to 64k if we can though. - */ - -#include <cpu/x86/lapic_def.h> -#include <cpu/x86/msr.h> - -/* - * +--------------------------------+ 0xaffff - * | Save State Map Node 0 | - * | Save State Map Node 1 | - * | Save State Map Node 2 | - * | Save State Map Node 3 | - * | ... | - * +--------------------------------+ 0xaf000 - * | | - * | | - * | | - * +--------------------------------+ 0xa8400 - * | SMM Entry Node 0 (+ stack) | - * +--------------------------------+ 0xa8000 - * | SMM Entry Node 1 (+ stack) | - * | SMM Entry Node 2 (+ stack) | - * | SMM Entry Node 3 (+ stack) | - * | ... | - * +--------------------------------+ 0xa7400 - * | | - * | SMM Handler | - * | | - * +--------------------------------+ 0xa0000 - * - */ - -/* SMM_HANDLER_OFFSET is the 16bit offset within the ASEG - * at which smm_handler_start lives. At the moment the handler - * lives right at 0xa0000, so the offset is 0. - */ - -#define SMM_HANDLER_OFFSET 0x0000 - -#if ENV_X86_64 -.bss -ia32efer_backup_eax: -.long 0 -ia32efer_backup_edx: -.long 0 -#endif - -/* initially SMM is some sort of real mode. Let gcc know - * how to treat the SMM handler stub - */ - -.section ".handler", "a", @progbits - -.code16 - -/** - * SMM code to enable protected mode and jump to the - * C-written function void smi_handler(u32 smm_revision) - * - * All the bad magic is not all that bad after all. - */ -#define SMM_START 0xa0000 -#define SMM_END 0xb0000 -#if SMM_END <= SMM_START -#error invalid SMM configuration -#endif -.global smm_handler_start -smm_handler_start: -#if CONFIG(SMM_LAPIC_REMAP_MITIGATION) - /* Check if the LAPIC register block overlaps with SMM. - * This block needs to work without data accesses because they - * may be routed into the LAPIC register block. - * Code accesses, on the other hand, are never routed to LAPIC, - * which is what makes this work in the first place. - */ - mov $LAPIC_BASE_MSR, %ecx - rdmsr - and $(~0xfff), %eax - sub $(SMM_START), %eax - cmp $(SMM_END - SMM_START), %eax - ja untampered_lapic -1: - /* emit "Crash" on serial */ - mov $(CONFIG_TTYS0_BASE), %dx - mov $'C', %al - out %al, (%dx) - mov $'r', %al - out %al, (%dx) - mov $'a', %al - out %al, (%dx) - mov $'s', %al - out %al, (%dx) - mov $'h', %al - out %al, (%dx) - /* now crash for real */ - ud2 -untampered_lapic: -#endif - movw $(smm_gdtptr16 - smm_handler_start + SMM_HANDLER_OFFSET), %bx - lgdtl %cs:(%bx) - - movl %cr0, %eax - andl $0x7FFAFFD1, %eax /* PG,AM,WP,NE,TS,EM,MP = 0 */ - orl $0x60000001, %eax /* CD, NW, PE = 1 */ - movl %eax, %cr0 - - /* Enable protected mode */ - ljmpl $0x08, $1f - -.code32 -1: - /* flush the cache after disabling it */ - wbinvd - - /* Use flat data segment */ - movw $0x10, %ax - movw %ax, %ds - movw %ax, %es - movw %ax, %ss - xor %ax, %ax /* zero out the gs and fs segment index */ - movw %ax, %fs - movw %ax, %gs /* Will be used for cpu_info */ - - /* FIXME: Incompatible with X2APIC_SUPPORT. */ - /* Get this CPU's LAPIC ID */ - movl $(LAPIC_DEFAULT_BASE | LAPIC_ID), %esi - movl (%esi), %ecx - shr $24, %ecx - - /* This is an ugly hack, and we should find a way to read the CPU index - * without relying on the LAPIC ID. - */ - - /* calculate stack offset by multiplying the APIC ID - * by 1024 (0x400), and save that offset in ebp. - */ - shl $10, %ecx - movl %ecx, %ebp - - /* We put the stack for each core right above - * its SMM entry point. Core 0 starts at 0xa8000, - * we spare 0x10 bytes for the jump to be sure. - */ - movl $0xa8010, %eax - subl %ecx, %eax /* subtract offset, see above */ - movl %eax, %ebx /* Save bottom of stack in ebx */ - -#define SMM_STACK_SIZE (0x400 - 0x10) - /* clear stack */ - cld - movl %eax, %edi - movl $(SMM_STACK_SIZE >> 2), %ecx - xorl %eax, %eax - rep stosl - - /* set new stack */ - addl $SMM_STACK_SIZE, %ebx - movl %ebx, %esp - -#if ENV_X86_64 - /* Backup IA32_EFER. Preserves ebx. */ - movl $(IA32_EFER), %ecx - rdmsr - movl %eax, ia32efer_backup_eax - movl %edx, ia32efer_backup_edx - - /* Enable long mode. Preserves ebx. */ -#include <cpu/x86/64bit/entry64.inc> - -#endif - /* Call C handler */ - call smi_handler - -#if ENV_X86_64 - /* - * The only reason to go back to protected mode is that RSM doesn't restore - * MSR registers and MSR IA32_EFER was modified by entering long mode. - * Drop to protected mode to safely operate on the IA32_EFER MSR. - */ - - /* Disable long mode. */ - #include <cpu/x86/64bit/exit32.inc> - - /* Restore IA32_EFER as RSM doesn't restore MSRs. */ - movl $(IA32_EFER), %ecx - movl ia32efer_backup_eax, %eax - movl ia32efer_backup_edx, %edx - wrmsr -#endif - - /* To return, just do rsm. It will "clean up" protected mode */ - rsm - -.code16 - -.align 4, 0xff - -smm_gdtptr16: - .word smm_gdt_end - smm_gdt - 1 - .long smm_gdt - smm_handler_start + 0xa0000 + SMM_HANDLER_OFFSET - -.code32 - -smm_gdt: - /* The first GDT entry can not be used. Keep it zero */ - .long 0x00000000, 0x00000000 - - /* gdt selector 0x08, flat code segment */ - .word 0xffff, 0x0000 - .byte 0x00, 0x9b, 0xcf, 0x00 /* G=1 and 0x0f, 4GB limit */ - - /* gdt selector 0x10, flat data segment */ - .word 0xffff, 0x0000 - .byte 0x00, 0x93, 0xcf, 0x00 - - /* gdt selector 0x18, flat code segment (64-bit) */ - .word 0xffff, 0x0000 - .byte 0x00, 0x9b, 0xaf, 0x00 -smm_gdt_end: - - -.section ".jumptable", "a", @progbits - -/* This is the SMM jump table. All cores use the same SMM handler - * for simplicity. But SMM Entry needs to be different due to the - * save state area. The jump table makes sure all CPUs jump into the - * real handler on SMM entry. - */ - -/* This code currently supports up to 4 CPU cores. If more than 4 CPU cores - * shall be used, below table has to be updated, as well as smm.ld - */ - -/* GNU AS/LD will always generate code that assumes CS is 0xa000. In reality - * CS will be set to SMM_BASE[19:4] though. Knowing that the smm handler is the - * first thing in the ASEG, we do a far jump here, to set CS to 0xa000. - */ - -.code16 -jumptable: - /* core 3 */ - ljmp $0xa000, $SMM_HANDLER_OFFSET -.align 1024, 0x00 - /* core 2 */ - ljmp $0xa000, $SMM_HANDLER_OFFSET -.align 1024, 0x00 - /* core 1 */ - ljmp $0xa000, $SMM_HANDLER_OFFSET -.align 1024, 0x00 - /* core 0 */ - ljmp $0xa000, $SMM_HANDLER_OFFSET -.align 1024, 0x00 diff --git a/src/cpu/x86/smm/smmrelocate.S b/src/cpu/x86/smm/smmrelocate.S deleted file mode 100644 index b89cf3d..0000000 --- a/src/cpu/x86/smm/smmrelocate.S +++ /dev/null @@ -1,163 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -// FIXME: Is this piece of code southbridge specific, or -// can it be cleaned up so this include is not required? -// It's needed right now because we get our DEFAULT_PMBASE from -// here. -#if CONFIG(SOUTHBRIDGE_INTEL_I82801IX) -#include <southbridge/intel/i82801ix/i82801ix.h> -#else -#error "Southbridge needs SMM handler support." -#endif - -// ADDR32() macro -#include <arch/registers.h> - -#if !CONFIG(SMM_ASEG) -#error "Only use this file with ASEG." -#endif /* CONFIG_SMM_ASEG */ - -#define LAPIC_ID 0xfee00020 - -.global smm_relocation_start -.global smm_relocation_end - -/* initially SMM is some sort of real mode. */ -.code16 - -/** - * When starting up, x86 CPUs have their SMBASE set to 0x30000. However, - * this is not a good place for the SMM handler to live, so it needs to - * be relocated. - * Traditionally SMM handlers used to live in the A segment (0xa0000). - * With growing SMM handlers, more CPU cores, etc. CPU vendors started - * allowing to relocate the handler to the end of physical memory, which - * they refer to as TSEG. - * This trampoline code relocates SMBASE to base address - ( lapicid * 0x400 ) - * - * Why 0x400? It is a safe value to cover the save state area per CPU. On - * current AMD CPUs this area is _documented_ to be 0x200 bytes. On Intel - * Core 2 CPUs the _documented_ parts of the save state area is 48 bytes - * bigger, effectively sizing our data structures 0x300 bytes. - * - * Example (with SMM handler living at 0xa0000): - * - * LAPICID SMBASE SMM Entry SAVE STATE - * 0 0xa0000 0xa8000 0xafd00 - * 1 0x9fc00 0xa7c00 0xaf900 - * 2 0x9f800 0xa7800 0xaf500 - * 3 0x9f400 0xa7400 0xaf100 - * 4 0x9f000 0xa7000 0xaed00 - * 5 0x9ec00 0xa6c00 0xae900 - * 6 0x9e800 0xa6800 0xae500 - * 7 0x9e400 0xa6400 0xae100 - * 8 0x9e000 0xa6000 0xadd00 - * 9 0x9dc00 0xa5c00 0xad900 - * 10 0x9d800 0xa5800 0xad500 - * 11 0x9d400 0xa5400 0xad100 - * 12 0x9d000 0xa5000 0xacd00 - * 13 0x9cc00 0xa4c00 0xac900 - * 14 0x9c800 0xa4800 0xac500 - * 15 0x9c400 0xa4400 0xac100 - * . . . . - * . . . . - * . . . . - * 31 0x98400 0xa0400 0xa8100 - * - * With 32 cores, the SMM handler would need to fit between - * 0xa0000-0xa0400 and the stub plus stack would need to go - * at 0xa8000-0xa8100 (example for core 0). That is not enough. - * - * This means we're basically limited to 16 CPU cores before - * we need to move the SMM handler to TSEG. - * - * Note: Some versions of Pentium M need their SMBASE aligned to 32k. - * On those the above only works for up to 2 cores. But for now we only - * care fore Core (2) Duo/Solo - * - */ - -smm_relocation_start: - /* Check revision to see if AMD64 style SMM_BASE - * Intel Core Solo/Duo: 0x30007 - * Intel Core2 Solo/Duo: 0x30100 - * Intel SandyBridge: 0x30101 - * AMD64: 0x3XX64 - * This check does not make much sense, unless someone ports - * SMI handling to AMD64 CPUs. - */ - - mov $0x38000 + 0x7efc, %ebx - ADDR32(mov) (%ebx), %al - cmp $0x64, %al - je 1f - - mov $0x38000 + 0x7ef8, %ebx - jmp smm_relocate -1: - mov $0x38000 + 0x7f00, %ebx - -smm_relocate: - /* Get this CPU's LAPIC ID */ - movl $LAPIC_ID, %esi - ADDR32(movl) (%esi), %ecx - shr $24, %ecx - - /* calculate offset by multiplying the - * APIC ID by 1024 (0x400) - */ - movl %ecx, %edx - shl $10, %edx - - movl $0xa0000, %eax - subl %edx, %eax /* subtract offset, see above */ - - ADDR32(movl) %eax, (%ebx) - - /* The next section of code is potentially southbridge specific */ - - /* Clear SMI status */ - movw $(DEFAULT_PMBASE + 0x34), %dx - inw %dx, %ax - outw %ax, %dx - - /* Clear PM1 status */ - movw $(DEFAULT_PMBASE + 0x00), %dx - inw %dx, %ax - outw %ax, %dx - - /* Set EOS bit so other SMIs can occur */ - movw $(DEFAULT_PMBASE + 0x30), %dx - inl %dx, %eax - orl $(1 << 1), %eax - outl %eax, %dx - - /* End of southbridge specific section. */ - -#if CONFIG(DEBUG_SMM_RELOCATION) - /* print [SMM-x] so we can determine if CPUx went to SMM */ - movw $CONFIG_TTYS0_BASE, %dx - mov $'[', %al - outb %al, %dx - mov $'S', %al - outb %al, %dx - mov $'M', %al - outb %al, %dx - outb %al, %dx - movb $'-', %al - outb %al, %dx - /* calculate ascii of CPU number. More than 9 cores? -> FIXME */ - movb %cl, %al - addb $'0', %al - outb %al, %dx - mov $']', %al - outb %al, %dx - mov $'\r', %al - outb %al, %dx - mov $'\n', %al - outb %al, %dx -#endif - - /* That's it. return */ - rsm -smm_relocation_end: -- To view, visit https://review.coreboot.org/c/coreboot/+/69121 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7e90f457f3979781d06323ef1350d5fb05a6be43 Gerrit-Change-Number: 69121 Gerrit-PatchSet: 16 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: mb/qemu/x86: Remove option for LEGACY_SMP_INIT
by Arthur Heymans (Code Review) 07 Nov '22

07 Nov '22

Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69128 ) Change subject: mb/qemu/x86: Remove option for LEGACY_SMP_INIT ...................................................................... mb/qemu/x86: Remove option for LEGACY_SMP_INIT This is deprecated after the 4.18 release. Change-Id: I17327c31f8ade51716578e45c2d90a327efcd4ad Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/69128 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/cpu/qemu-x86/Kconfig M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801ix/lpc.c D src/southbridge/intel/i82801ix/smi.c M src/southbridge/intel/i82801ix/smihandler.c 5 files changed, 15 insertions(+), 190 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/src/cpu/qemu-x86/Kconfig b/src/cpu/qemu-x86/Kconfig index 868009c..f3e2c4c 100644 --- a/src/cpu/qemu-x86/Kconfig +++ b/src/cpu/qemu-x86/Kconfig @@ -10,19 +10,6 @@ if CPU_QEMU_X86 -choice - prompt "AP init" - default CPU_QEMU_X86_LAPIC_INIT - -config CPU_QEMU_X86_LAPIC_INIT - bool "Legacy serial LAPIC init" - select LEGACY_SMP_INIT - -config CPU_QEMU_X86_PARALLEL_MP - bool "Parallel MP init" - -endchoice - # coreboot i440fx does not support SMM choice prompt "SMM support" @@ -40,7 +27,6 @@ config CPU_QEMU_X86_TSEG_SMM bool "SMM in TSEG" select SMM_TSEG - depends on CPU_QEMU_X86_PARALLEL_MP endchoice diff --git a/src/southbridge/intel/i82801ix/Makefile.inc b/src/southbridge/intel/i82801ix/Makefile.inc index 54e8c1c..0000913 100644 --- a/src/southbridge/intel/i82801ix/Makefile.inc +++ b/src/southbridge/intel/i82801ix/Makefile.inc @@ -26,9 +26,6 @@ ramstage-y += madt.c endif -ramstage-$(CONFIG_SMM_LEGACY_ASEG) += smi.c -ramstage-$(CONFIG_SMM_LEGACY_ASEG) += ../../../cpu/x86/smm/smmrelocate.S - CPPFLAGS_common += -I$(src)/southbridge/intel/i82801ix/include endif diff --git a/src/southbridge/intel/i82801ix/lpc.c b/src/southbridge/intel/i82801ix/lpc.c index e39c95a..8d44384 100644 --- a/src/southbridge/intel/i82801ix/lpc.c +++ b/src/southbridge/intel/i82801ix/lpc.c @@ -374,12 +374,6 @@ i8259_configure_irq_trigger(9, 1); i82801ix_set_acpi_mode(dev); - - /* Don't allow evil boot loaders, kernels, or - * userspace applications to deceive us: - */ - if (CONFIG(SMM_LEGACY_ASEG)) - aseg_smm_lock(); } static void i82801ix_lpc_read_resources(struct device *dev) diff --git a/src/southbridge/intel/i82801ix/smi.c b/src/southbridge/intel/i82801ix/smi.c deleted file mode 100644 index 1abd8c5..0000000 --- a/src/southbridge/intel/i82801ix/smi.c +++ /dev/null @@ -1,162 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <device/device.h> -#include <device/pci.h> -#include <console/console.h> -#include <arch/io.h> -#include <device/pci_ops.h> -#include <acpi/acpi.h> -#include <cpu/x86/cache.h> -#include <cpu/x86/smm.h> -#include <cpu/x86/smi_deprecated.h> -#include <string.h> -#include <southbridge/intel/common/pmutil.h> -#include "i82801ix.h" - -/* I945/GM45 */ -#define SMRAM 0x9d -#define D_OPEN (1 << 6) -#define D_CLS (1 << 5) -#define D_LCK (1 << 4) -#define G_SMRAME (1 << 3) -#define C_BASE_SEG ((0 << 2) | (1 << 1) | (0 << 0)) - -/* While we read PMBASE dynamically in case it changed, let's - * initialize it with a sane value - */ -static u16 pmbase = DEFAULT_PMBASE; - -extern uint8_t smm_relocation_start, smm_relocation_end; -static void *default_smm_area = NULL; - -static void aseg_smm_relocate(void) -{ - u32 smi_en; - u16 pm1_en; - - printk(BIOS_DEBUG, "Initializing SMM handler..."); - - pmbase = pci_read_config16(pcidev_on_root(0x1f, 0), D31F0_PMBASE) & - 0xfffc; - printk(BIOS_SPEW, " ... pmbase = 0x%04x\n", pmbase); - - smi_en = inl(pmbase + SMI_EN); - if (smi_en & GBL_SMI_EN) { - printk(BIOS_INFO, "SMI# handler already enabled?\n"); - return; - } - - default_smm_area = backup_default_smm_area(); - - /* copy the SMM relocation code */ - memcpy((void *)0x38000, &smm_relocation_start, - &smm_relocation_end - &smm_relocation_start); - wbinvd(); - - printk(BIOS_DEBUG, "\n"); - dump_smi_status(reset_smi_status()); - dump_pm1_status(reset_pm1_status()); - dump_gpe0_status(reset_gpe0_status()); - dump_alt_gp_smi_status(reset_alt_gp_smi_status()); - dump_tco_status(reset_tco_status()); - - /* Enable SMI generation: - * - on TCO events - * - on APMC writes (io 0xb2) - * - on writes to GBL_RLS (bios commands) - * No SMIs: - * - on microcontroller writes (io 0x62/0x66) - */ - - smi_en = 0; /* reset SMI enables */ - smi_en |= TCO_EN; - smi_en |= APMC_EN; - if (CONFIG(DEBUG_PERIODIC_SMI)) - smi_en |= PERIODIC_EN; - smi_en |= BIOS_EN; - - /* The following need to be on for SMIs to happen */ - smi_en |= EOS | GBL_SMI_EN; - - outl(smi_en, pmbase + SMI_EN); - - pm1_en = 0; - pm1_en |= PWRBTN_EN; - pm1_en |= GBL_EN; - outw(pm1_en, pmbase + PM1_EN); - - /** - * There are several methods of raising a controlled SMI# via - * software, among them: - * - Writes to io 0xb2 (APMC) - * - Writes to the Local Apic ICR with Delivery mode SMI. - * - * Using the local APIC is a bit more tricky. According to - * AMD Family 11 Processor BKDG no destination shorthand must be - * used. - * The whole SMM initialization is quite a bit hardware specific, so - * I'm not too worried about the better of the methods at the moment - */ - - /* raise an SMI interrupt */ - printk(BIOS_SPEW, " ... raise SMI#\n"); - apm_control(APM_CNT_NOOP_SMI); -} - -static int smm_handler_copied = 0; - -static void aseg_smm_install(void) -{ - /* The first CPU running this gets to copy the SMM handler. But not all - * of them. - */ - if (smm_handler_copied) - return; - smm_handler_copied = 1; - - /* if we're resuming from S3, the SMM code is already in place, - * so don't copy it again to keep the current SMM state */ - - if (!acpi_is_wakeup_s3()) { - /* enable the SMM memory window */ - pci_write_config8(pcidev_on_root(0, 0), SMRAM, - D_OPEN | G_SMRAME | C_BASE_SEG); - - /* copy the real SMM handler */ - memcpy((void *)0xa0000, _binary_smm_start, - _binary_smm_end - _binary_smm_start); - wbinvd(); - } - - /* close the SMM memory window and enable normal SMM */ - pci_write_config8(pcidev_on_root(0, 0), SMRAM, - G_SMRAME | C_BASE_SEG); -} - -void smm_init(void) -{ - /* Put SMM code to 0xa0000 */ - aseg_smm_install(); - - /* Put relocation code to 0x38000 and relocate SMBASE */ - aseg_smm_relocate(); - - /* We're done. Make sure SMIs can happen! */ - smi_set_eos(); -} - -void smm_init_completion(void) -{ - restore_default_smm_area(default_smm_area); -} - -void aseg_smm_lock(void) -{ - /* LOCK the SMM memory window and enable normal SMM. - * After running this function, only a full reset can - * make the SMM registers writable again. - */ - printk(BIOS_DEBUG, "Locking SMM.\n"); - pci_write_config8(pcidev_on_root(0, 0), SMRAM, - D_LCK | G_SMRAME | C_BASE_SEG); -} diff --git a/src/southbridge/intel/i82801ix/smihandler.c b/src/southbridge/intel/i82801ix/smihandler.c index 3aca874..cfcef79 100644 --- a/src/southbridge/intel/i82801ix/smihandler.c +++ b/src/southbridge/intel/i82801ix/smihandler.c @@ -9,11 +9,6 @@ #include <soc/nvs.h> -#if CONFIG(SMM_LEGACY_ASEG) -/* For qemu/x86-q35 to build properly. */ -struct global_nvs *gnvs; -#endif - int southbridge_io_trap_handler(int smif) { switch (smif) { -- To view, visit https://review.coreboot.org/c/coreboot/+/69128 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I17327c31f8ade51716578e45c2d90a327efcd4ad Gerrit-Change-Number: 69128 Gerrit-PatchSet: 8 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2022