coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: *.asl: Don't define Devices by absolute path
by Arthur Heymans (Code Review) 21 Apr '22

21 Apr '22

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36600 ) Change subject: *.asl: Don't define Devices by absolute path ...................................................................... *.asl: Don't define Devices by absolute path Be consistent with the rest tree and set the scope first when defining devices. In general make the resulting code more readable as it avoid '^' symbols. Change-Id: I0b8dd33fac7cb1524d920b77e95c08b839157e8f Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/mainboard/asus/h61m-cs/dsdt.asl M src/mainboard/asus/p8z77-m_pro/dsdt.asl M src/mainboard/gigabyte/ga-h61m-s2pv/dsdt.asl M src/mainboard/lenovo/t440p/dsdt.asl M src/mainboard/msi/ms7707/dsdt.asl M src/mainboard/supermicro/x10slm-f/dsdt.asl M util/autoport/main.go 7 files changed, 42 insertions(+), 32 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/36600/1 diff --git a/src/mainboard/asus/h61m-cs/dsdt.asl b/src/mainboard/asus/h61m-cs/dsdt.asl index 0e1cc3a..a1a70c0 100644 --- a/src/mainboard/asus/h61m-cs/dsdt.asl +++ b/src/mainboard/asus/h61m-cs/dsdt.asl @@ -32,9 +32,11 @@ #include <southbridge/intel/bd82x6x/acpi/globalnvs.asl> #include <southbridge/intel/common/acpi/sleepstates.asl> - Device (\_SB.PCI0) { - #include <northbridge/intel/sandybridge/acpi/sandybridge.asl> - #include <drivers/intel/gma/acpi/default_brightness_levels.asl> - #include <southbridge/intel/bd82x6x/acpi/pch.asl> + Scope (\_SB) { + Device (PCI0) { + #include <northbridge/intel/sandybridge/acpi/sandybridge.asl> + #include <drivers/intel/gma/acpi/default_brightness_levels.asl> + #include <southbridge/intel/bd82x6x/acpi/pch.asl> + } } } diff --git a/src/mainboard/asus/p8z77-m_pro/dsdt.asl b/src/mainboard/asus/p8z77-m_pro/dsdt.asl index 1c7a620..2c14e8f 100644 --- a/src/mainboard/asus/p8z77-m_pro/dsdt.asl +++ b/src/mainboard/asus/p8z77-m_pro/dsdt.asl @@ -34,10 +34,11 @@ #include <southbridge/intel/bd82x6x/acpi/globalnvs.asl> #include <southbridge/intel/common/acpi/sleepstates.asl> - Device (\_SB.PCI0) - { - #include <northbridge/intel/sandybridge/acpi/sandybridge.asl> - #include <drivers/intel/gma/acpi/default_brightness_levels.asl> - #include <southbridge/intel/bd82x6x/acpi/pch.asl> + Scope (\_SB) { + Device (PCI0) { + #include <northbridge/intel/sandybridge/acpi/sandybridge.asl> + #include <drivers/intel/gma/acpi/default_brightness_levels.asl> + #include <southbridge/intel/bd82x6x/acpi/pch.asl> + } } } diff --git a/src/mainboard/gigabyte/ga-h61m-s2pv/dsdt.asl b/src/mainboard/gigabyte/ga-h61m-s2pv/dsdt.asl index 7ee12a0..e54b01b 100644 --- a/src/mainboard/gigabyte/ga-h61m-s2pv/dsdt.asl +++ b/src/mainboard/gigabyte/ga-h61m-s2pv/dsdt.asl @@ -37,10 +37,11 @@ #include <southbridge/intel/bd82x6x/acpi/globalnvs.asl> #include <southbridge/intel/common/acpi/sleepstates.asl> - Device (\_SB.PCI0) - { - #include <northbridge/intel/sandybridge/acpi/sandybridge.asl> - #include <drivers/intel/gma/acpi/default_brightness_levels.asl> - #include <southbridge/intel/bd82x6x/acpi/pch.asl> + Scope (\_SB) { + Device (PCI0) { + #include <northbridge/intel/sandybridge/acpi/sandybridge.asl> + #include <drivers/intel/gma/acpi/default_brightness_levels.asl> + #include <southbridge/intel/bd82x6x/acpi/pch.asl> + } } } diff --git a/src/mainboard/lenovo/t440p/dsdt.asl b/src/mainboard/lenovo/t440p/dsdt.asl index 82358aa..fd37f9e 100644 --- a/src/mainboard/lenovo/t440p/dsdt.asl +++ b/src/mainboard/lenovo/t440p/dsdt.asl @@ -37,10 +37,11 @@ #include <southbridge/intel/lynxpoint/acpi/globalnvs.asl> #include <southbridge/intel/common/acpi/sleepstates.asl> - Device (\_SB.PCI0) - { - #include <northbridge/intel/haswell/acpi/haswell.asl> - #include <drivers/intel/gma/acpi/default_brightness_levels.asl> - #include <southbridge/intel/lynxpoint/acpi/pch.asl> + Scope (\_SB) { + Device (PCI0) { + #include <northbridge/intel/sandybridge/acpi/sandybridge.asl> + #include <drivers/intel/gma/acpi/default_brightness_levels.asl> + #include <southbridge/intel/bd82x6x/acpi/pch.asl> + } } } diff --git a/src/mainboard/msi/ms7707/dsdt.asl b/src/mainboard/msi/ms7707/dsdt.asl index d3e3602..6df1394 100644 --- a/src/mainboard/msi/ms7707/dsdt.asl +++ b/src/mainboard/msi/ms7707/dsdt.asl @@ -32,10 +32,11 @@ #include <southbridge/intel/bd82x6x/acpi/globalnvs.asl> #include <southbridge/intel/common/acpi/sleepstates.asl> - Device (\_SB.PCI0) - { - #include <northbridge/intel/sandybridge/acpi/sandybridge.asl> - #include <drivers/intel/gma/acpi/default_brightness_levels.asl> - #include <southbridge/intel/bd82x6x/acpi/pch.asl> + Scope (\_SB) { + Device (PCI0) { + #include <northbridge/intel/sandybridge/acpi/sandybridge.asl> + #include <drivers/intel/gma/acpi/default_brightness_levels.asl> + #include <southbridge/intel/bd82x6x/acpi/pch.asl> + } } } diff --git a/src/mainboard/supermicro/x10slm-f/dsdt.asl b/src/mainboard/supermicro/x10slm-f/dsdt.asl index 6ed19ee..8649a95 100644 --- a/src/mainboard/supermicro/x10slm-f/dsdt.asl +++ b/src/mainboard/supermicro/x10slm-f/dsdt.asl @@ -24,10 +24,11 @@ #include <southbridge/intel/common/acpi/sleepstates.asl> #include <cpu/intel/common/acpi/cpu.asl> - Device (\_SB.PCI0) - { - #include <northbridge/intel/haswell/acpi/haswell.asl> - #include <southbridge/intel/lynxpoint/acpi/pch.asl> - #include <drivers/intel/gma/acpi/default_brightness_levels.asl> + Scope (\_SB) { + Device (PCI0) { + #include <northbridge/intel/sandybridge/acpi/sandybridge.asl> + #include <drivers/intel/gma/acpi/default_brightness_levels.asl> + #include <southbridge/intel/bd82x6x/acpi/pch.asl> + } } } diff --git a/util/autoport/main.go b/util/autoport/main.go index 03523a2..51d10cf 100644 --- a/util/autoport/main.go +++ b/util/autoport/main.go @@ -876,17 +876,20 @@ } dsdt.WriteString(` - Device (\_SB.PCI0) + Scope (\_SB) { + Device (PCI0) + { `) for _, x := range DSDTPCI0Includes { if x.Comment != "" { fmt.Fprintf(dsdt, "\t/* %s. */\n", x.Comment) } - fmt.Fprintf(dsdt, "\t\t#include <%s>\n", x.File) + fmt.Fprintf(dsdt, "\t\t\t#include <%s>\n", x.File) } dsdt.WriteString( - ` } + ` } + } } `) -- To view, visit https://review.coreboot.org/c/coreboot/+/36600 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0b8dd33fac7cb1524d920b77e95c08b839157e8f Gerrit-Change-Number: 36600 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

4 11

Change in coreboot[master]: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place
by Arthur Heymans (Code Review) 21 Apr '22

21 Apr '22

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place include/device/pci_mmio_cfg.h now has a compile time error if MMCONF_BASE_ADDRESS is not defined. This requires soc/intel target to explicitly set this in the SOC dir, instead of potentially using a possibly wrong common default. Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/common/block/systemagent/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/skylake/Kconfig 5 files changed, 16 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/35884/1 diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig index a1d3c07..24a4b7a 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -128,6 +128,10 @@ TPM part is conntected on Fast SPI interface, but the LPC MMIO TPM transactions are decoded and serialized over the SPI interface. +config MMCONF_BASE_ADDRESS + hex + default 0xe0000000 + config PCR_BASE_ADDRESS hex default 0xd0000000 diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index c1fda95..aeef75c 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -109,6 +109,10 @@ select FSP_T_XIP if FSP_CAR select HECI_DISABLE_USING_SMM if !SOC_INTEL_COFFEELAKE && !SOC_INTEL_WHISKEYLAKE && !SOC_INTEL_COMETLAKE +config MMCONF_BASE_ADDRESS + hex + default 0xe0000000 + config DCACHE_RAM_BASE default 0xfef00000 diff --git a/src/soc/intel/common/block/systemagent/Kconfig b/src/soc/intel/common/block/systemagent/Kconfig index 1222573..d7619a0 100644 --- a/src/soc/intel/common/block/systemagent/Kconfig +++ b/src/soc/intel/common/block/systemagent/Kconfig @@ -3,10 +3,6 @@ help Intel Processor common System Agent support -config MMCONF_BASE_ADDRESS - hex - default 0xe0000000 - config SA_PCIEX_LENGTH hex default 0x10000000 if (PCIEX_LENGTH_256MB) diff --git a/src/soc/intel/denverton_ns/Kconfig b/src/soc/intel/denverton_ns/Kconfig index 2aadcae..d12e4e3 100644 --- a/src/soc/intel/denverton_ns/Kconfig +++ b/src/soc/intel/denverton_ns/Kconfig @@ -91,6 +91,10 @@ int default 16 +config MMCONF_BASE_ADDRESS + hex + default 0xe0000000 + config PCR_BASE_ADDRESS hex default 0xfd000000 diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 901e5f9..a9b8aaa 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -120,6 +120,10 @@ hex default 0x200000 +config MMCONF_BASE_ADDRESS + hex + default 0xe0000000 + config DCACHE_RAM_BASE hex default 0xfef00000 -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

9 19

Change in coreboot[master]: soc/intel/common/block/sa: Add compiletime error on undefined SA_PCIE...
by Arthur Heymans (Code Review) 21 Apr '22

21 Apr '22

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35887 ) Change subject: soc/intel/common/block/sa: Add compiletime error on undefined SA_PCIE_LENGTH ...................................................................... soc/intel/common/block/sa: Add compiletime error on undefined SA_PCIE_LENGTH Change-Id: Ib9facecc4caa869e65f938a55508741dd48b09b0 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/intel/common/block/systemagent/Kconfig M src/soc/intel/common/block/systemagent/systemagent_early.c 2 files changed, 4 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/35887/1 diff --git a/src/soc/intel/common/block/systemagent/Kconfig b/src/soc/intel/common/block/systemagent/Kconfig index d7619a0..81a7f9d 100644 --- a/src/soc/intel/common/block/systemagent/Kconfig +++ b/src/soc/intel/common/block/systemagent/Kconfig @@ -8,7 +8,6 @@ default 0x10000000 if (PCIEX_LENGTH_256MB) default 0x8000000 if (PCIEX_LENGTH_128MB) default 0x4000000 if (PCIEX_LENGTH_64MB) - default 0x10000000 help This option allows you to select length of PCIEX region. diff --git a/src/soc/intel/common/block/systemagent/systemagent_early.c b/src/soc/intel/common/block/systemagent/systemagent_early.c index 8c89c07..5411238 100644 --- a/src/soc/intel/common/block/systemagent/systemagent_early.c +++ b/src/soc/intel/common/block/systemagent/systemagent_early.c @@ -26,6 +26,10 @@ #include "systemagent_def.h" +#if CONFIG_SA_PCIEX_LENGTH == 0 +#error "SA_PCIEX_LENGTH undefined!" +#endif + void bootblock_systemagent_early_init(void) { uint32_t reg; -- To view, visit https://review.coreboot.org/c/coreboot/+/35887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9facecc4caa869e65f938a55508741dd48b09b0 Gerrit-Change-Number: 35887 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

3 5

Change in coreboot[master]: arch/x86/car.ld: Use REGION macro
by Arthur Heymans (Code Review) 21 Apr '22

21 Apr '22

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36696 ) Change subject: arch/x86/car.ld: Use REGION macro ...................................................................... arch/x86/car.ld: Use REGION macro The advantages of using the REGION macro: - less boilerplate for defining start and end symbols - overlap between regions checking - automatic alignment Change-Id: I9c327343eb40d3e2cc8513354ec3a83d803ff1ee Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/arch/x86/car.ld 1 file changed, 4 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/36696/1 diff --git a/src/arch/x86/car.ld b/src/arch/x86/car.ld index 74fc74b..c601d2e 100644 --- a/src/arch/x86/car.ld +++ b/src/arch/x86/car.ld @@ -18,9 +18,7 @@ #if CONFIG(PAGING_IN_CACHE_AS_RAM) /* Page table pre-allocation. CONFIG_DCACHE_RAM_BASE should be 4KiB * aligned when using this option. */ - _pagetables = . ; - . += 4096 * CONFIG_NUM_CAR_PAGE_TABLE_PAGES; - _epagetables = . ; + REGION(pagetables, ., 4096 * CONFIG_NUM_CAR_PAGE_TABLE_PAGES, 4096) #endif /* Vboot work buffer only needs to be available when verified boot * starts in bootblock. */ @@ -37,9 +35,7 @@ * use CAR it can be reused. The chipset/SoC is expected to provide * the stack size. */ #if CONFIG(C_ENVIRONMENT_BOOTBLOCK) - _car_stack = .; - . += CONFIG_DCACHE_BSP_STACK_SIZE; - _ecar_stack = .; + REGION(car_stack, ., CONFIG_DCACHE_BSP_STACK_SIZE, 16) #endif /* The pre-ram cbmem console as well as the timestamp region are fixed * in size. Therefore place them above the car global section so that @@ -47,22 +43,17 @@ * link address of these shared objects. */ PRERAM_CBMEM_CONSOLE(., CONFIG_PRERAM_CBMEM_CONSOLE_SIZE) #if CONFIG(PAGING_IN_CACHE_AS_RAM) - . = ALIGN(32); /* Page directory pointer table resides here. There are 4 8-byte entries * totalling 32 bytes that need to be 32-byte aligned. The reason the * pdpt are not colocated with the rest of the page tables is to reduce * fragmentation of the CAR space that persists across stages. */ - _pdpt = .; - . += 32; - _epdpt = .; + REGION(pdpt, ., 32, 32) #endif TIMESTAMP(., 0x200) - _car_ehci_dbg_info = .; /* Reserve sizeof(struct ehci_dbg_info). */ - . += 80; - _ecar_ehci_dbg_info = .; + REGION(car_ehci_dbg_info, ., 80, ARCH_POINTER_ALIGN_SIZE) /* _bss and _ebss provide symbols to per-stage * variables that are not shared like the timestamp and the pre-ram @@ -114,9 +105,6 @@ } _bogus = ASSERT((CONFIG_DCACHE_RAM_SIZE == 0) || (SIZEOF(.car.data) <= CONFIG_DCACHE_RAM_SIZE), "Cache as RAM area is too full"); -#if CONFIG(PAGING_IN_CACHE_AS_RAM) -_bogus2 = ASSERT(_pagetables == ALIGN(_pagetables, 4096), "_pagetables aren't 4KiB aligned"); -#endif #if CONFIG(C_ENVIRONMENT_BOOTBLOCK) _bogus3 = ASSERT(CONFIG_DCACHE_BSP_STACK_SIZE > 0x0, "BSP stack size not configured"); #endif -- To view, visit https://review.coreboot.org/c/coreboot/+/36696 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9c327343eb40d3e2cc8513354ec3a83d803ff1ee Gerrit-Change-Number: 36696 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

3 8

Change in coreboot[master]: src/soc/intel/fsp-car: Align stack before jumping to C code
by Arthur Heymans (Code Review) 21 Apr '22

21 Apr '22

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37279 ) Change subject: src/soc/intel/fsp-car: Align stack before jumping to C code ...................................................................... src/soc/intel/fsp-car: Align stack before jumping to C code GCC has had a 16 byte stack requirement since quite some time now... Change-Id: I0f5b62b5fef3ca7ec6d9010723527a356eb566ed Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/37279/1 diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S b/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S index d7ef834..4174f36 100644 --- a/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S +++ b/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S @@ -99,6 +99,10 @@ xorl %eax, %eax rep stosl + /* Need to align stack to 16 bytes at call instruction. Account for + the two pushes below. */ + andl $0xfffffff0, %esp + sub $8, %esp /* Restore the timestamp from bootblock_crt0.S (ebp:mm1) */ push %ebp movd %mm1, %eax -- To view, visit https://review.coreboot.org/c/coreboot/+/37279 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0f5b62b5fef3ca7ec6d9010723527a356eb566ed Gerrit-Change-Number: 37279 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

3 5

Change in coreboot[master]: soc/intel/fsp-car: Clear the whole usable CAR region
by Arthur Heymans (Code Review) 21 Apr '22

21 Apr '22

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37277 ) Change subject: soc/intel/fsp-car: Clear the whole usable CAR region ...................................................................... soc/intel/fsp-car: Clear the whole usable CAR region Other symbols in car.ld likely expect to be zero initialized. Change-Id: I44a0d3947eb6dd67dfd2d7f03e5c0766b043f61b Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S 1 file changed, 6 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/37277/1 diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S b/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S index 091fc4a..ec43311 100644 --- a/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S +++ b/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S @@ -89,13 +89,14 @@ /* Setup bootblock stack */ mov %edx, %esp - /* clear .bss section as it is not shared */ + /* coreboot assumes CAR region will be zero */ cld - xor %eax, %eax - movl $(_ebss), %ecx - movl $(_bss), %edi - sub %edi, %ecx + movl %ecx, %edi + neg %ecx + /* Clear up to Temp Ram top. */ + add %edx, %ecx shrl $2, %ecx + xorl %eax, %eax rep stosl /* Restore the timestamp from bootblock_crt0.S (ebp:mm1) */ -- To view, visit https://review.coreboot.org/c/coreboot/+/37277 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I44a0d3947eb6dd67dfd2d7f03e5c0766b043f61b Gerrit-Change-Number: 37277 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

3 4

Change in coreboot[master]: drivers/intel/fsp1.1: Use the coreboot symbols for the stack
by Arthur Heymans (Code Review) 21 Apr '22

21 Apr '22

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37276 ) Change subject: drivers/intel/fsp1.1: Use the coreboot symbols for the stack ...................................................................... drivers/intel/fsp1.1: Use the coreboot symbols for the stack We want to know where the stack is, to make sure that it does not override other CAR symbols. Change-Id: I9edf615bff11aa38fc307b4e2e2c97f1757a05a1 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/drivers/intel/fsp1_1/cache_as_ram.S 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/37276/1 diff --git a/src/drivers/intel/fsp1_1/cache_as_ram.S b/src/drivers/intel/fsp1_1/cache_as_ram.S index fea7acb..fb39884 100644 --- a/src/drivers/intel/fsp1_1/cache_as_ram.S +++ b/src/drivers/intel/fsp1_1/cache_as_ram.S @@ -143,8 +143,8 @@ cmp $0, %eax jne halt2 - /* Setup bootloader stack */ - movl %edx, %esp + /* Setup BSP stack */ + movl _ecar_stack, %esp /* * ebp: FSP_INFO_HEADER address -- To view, visit https://review.coreboot.org/c/coreboot/+/37276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9edf615bff11aa38fc307b4e2e2c97f1757a05a1 Gerrit-Change-Number: 37276 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

3 4

Change in ...coreboot[master]: cpu/intel: Remove unused CPU_ADDR_BITS Kconfig option
by Arthur Heymans (Code Review) 21 Apr '22

21 Apr '22

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33169 Change subject: cpu/intel: Remove unused CPU_ADDR_BITS Kconfig option ...................................................................... cpu/intel: Remove unused CPU_ADDR_BITS Kconfig option Change-Id: Ib5529ceeec9481e05c56f20af0d201f3d20ea3f9 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/intel/model_106cx/Kconfig M src/lib/cbfs.c M src/mainboard/lenovo/x201/Kconfig M src/mainboard/packardbell/ms2290/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/skylake/Kconfig 6 files changed, 0 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/33169/1 diff --git a/src/cpu/intel/model_106cx/Kconfig b/src/cpu/intel/model_106cx/Kconfig index f365cf1..91b5189 100644 --- a/src/cpu/intel/model_106cx/Kconfig +++ b/src/cpu/intel/model_106cx/Kconfig @@ -14,10 +14,4 @@ select SERIALIZED_SMM_INITIALIZATION select CPU_INTEL_COMMON -if CPU_INTEL_MODEL_106CX - -config CPU_ADDR_BITS - int - default 32 - endif diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index 91368fb..d4433df 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -273,7 +273,6 @@ memset(&load[fsize], 0, stage.memlen - fsize); prog_segment_loaded((uintptr_t)load, stage.memlen, SEG_FINAL); - out: prog_set_area(pstage, load, stage.memlen); prog_set_entry(pstage, entry, NULL); diff --git a/src/mainboard/lenovo/x201/Kconfig b/src/mainboard/lenovo/x201/Kconfig index 69e2038..ad22cc4 100644 --- a/src/mainboard/lenovo/x201/Kconfig +++ b/src/mainboard/lenovo/x201/Kconfig @@ -47,10 +47,6 @@ int default 4 -config CPU_ADDR_BITS - int - default 36 - # Without the Intel ME's EFFS partition some PCIe devices (like the USB and SATA # controllers) don't work as expected config ME_CLEANER_ARGS diff --git a/src/mainboard/packardbell/ms2290/Kconfig b/src/mainboard/packardbell/ms2290/Kconfig index c6a1e12..20b520d 100644 --- a/src/mainboard/packardbell/ms2290/Kconfig +++ b/src/mainboard/packardbell/ms2290/Kconfig @@ -49,8 +49,4 @@ int default 4 -config CPU_ADDR_BITS - int - default 36 - endif diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig index 217d1ea..91476c1 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -158,10 +158,6 @@ The amount of anticipated stack usage in CAR by bootblock and other stages. -config CPU_ADDR_BITS - int - default 39 - config SOC_INTEL_COMMON_BLOCK_GSPI_CLOCK_MHZ int default 100 diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index fcfe2b6..b67ddd7 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -116,10 +116,6 @@ hex default 0x200000 -config CPU_ADDR_BITS - int - default 36 - config DCACHE_RAM_BASE hex default 0xfef00000 -- To view, visit https://review.coreboot.org/c/coreboot/+/33169 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib5529ceeec9481e05c56f20af0d201f3d20ea3f9 Gerrit-Change-Number: 33169 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

4 5

Change in coreboot[master]: [UNTESTED]sb/intel/common: Deal with different save states
by Arthur Heymans (Code Review) 21 Apr '22

21 Apr '22

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36590 ) Change subject: [UNTESTED]sb/intel/common: Deal with different save states ...................................................................... [UNTESTED]sb/intel/common: Deal with different save states Some CPUs have different save states and have to be dealt with accordingly. updating the gnvs pointer is now done in a common place and fixes this on i82801xx targets. Change-Id: I63f77432cf4a2ef484ad320bfc321e9a923a19ab Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/southbridge/intel/bd82x6x/smihandler.c M src/southbridge/intel/common/pmutil.h M src/southbridge/intel/common/smihandler.c M src/southbridge/intel/i82801gx/smihandler.c M src/southbridge/intel/i82801ix/smihandler.c M src/southbridge/intel/i82801jx/smihandler.c M src/southbridge/intel/ibexpeak/smihandler.c 7 files changed, 47 insertions(+), 56 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/36590/1 diff --git a/src/southbridge/intel/bd82x6x/smihandler.c b/src/southbridge/intel/bd82x6x/smihandler.c index ceac598..c4f1efb 100644 --- a/src/southbridge/intel/bd82x6x/smihandler.c +++ b/src/southbridge/intel/bd82x6x/smihandler.c @@ -211,16 +211,9 @@ xhci_sleep(slp_type); } -void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) +void set_gnvs_ptr(uintptr_t offset) { - em64t101_smm_state_save_area_t *state = - smi_apmc_find_state_save(apm_cnt); - if (state) { - /* EBX in the state save contains the GNVS pointer */ - gnvs = (global_nvs_t *)((u32)state->rbx); - *smm_done = 1; - printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs); - } + gnvs = (global_nvs_t *)offset; } void southbridge_finalize_all(void) diff --git a/src/southbridge/intel/common/pmutil.h b/src/southbridge/intel/common/pmutil.h index cea5c82..a5cd921 100644 --- a/src/southbridge/intel/common/pmutil.h +++ b/src/southbridge/intel/common/pmutil.h @@ -142,10 +142,10 @@ void southbridge_smm_xhci_sleep(u8 slp_type); void gpi_route_interrupt(u8 gpi, u8 mode); void southbridge_gate_memory_reset(void); -void southbridge_update_gnvs(u8 apm_cnt, int *smm_done); +void set_gnvs_ptr(uintptr_t ptr); void southbridge_finalize_all(void); void southbridge_smi_monitor(void); -em64t101_smm_state_save_area_t *smi_apmc_find_state_save(u8 cmd); +int smi_apmc_find_state_save(u8 cmd); void pch_log_state(void); #endif /*INTEL_COMMON_PMUTIL_H */ diff --git a/src/southbridge/intel/common/smihandler.c b/src/southbridge/intel/common/smihandler.c index 5582051..e85031d 100644 --- a/src/southbridge/intel/common/smihandler.c +++ b/src/southbridge/intel/common/smihandler.c @@ -215,13 +215,17 @@ * core in case we are not running on the same core that * initiated the IO transaction. */ -em64t101_smm_state_save_area_t *smi_apmc_find_state_save(u8 cmd) +int smi_apmc_find_state_save(u8 cmd) { em64t101_smm_state_save_area_t *state; int node; /* Check all nodes looking for the one that issued the IO */ for (node = 0; node < CONFIG_MAX_CPUS; node++) { + if (get_save_state_type() == LEGACY) + if (((*(uintptr_t *)save_state_reg(node, SS_EAX)) & 0xff) != cmd) + continue; + state = smm_get_save_state(node); /* Check for Synchronous IO (bit0 == 1) */ @@ -240,10 +244,11 @@ if ((state->rax & 0xff) != cmd) continue; - return state; + return node; } - return NULL; + printk(BIOS_WARNING, "APMC node not found!\n"); + return -1; } #if CONFIG(ELOG_GSMI) @@ -251,18 +256,17 @@ { u32 *ret, *param; u8 sub_command; - em64t101_smm_state_save_area_t *io_smi = - smi_apmc_find_state_save(APM_CNT_ELOG_GSMI); + int apm_node = smi_apmc_find_state_save(APM_CNT_ELOG_GSMI); - if (!io_smi) + if (apm_node < 0) return; /* Command and return value in EAX */ - ret = (u32*)&io_smi->rax; + ret = (u32*)save_state_reg(apm_node, SS_EAX); sub_command = (u8)(*ret >> 8); /* Parameter buffer in EBX */ - param = (u32*)&io_smi->rbx; + param = (u32*)save_state_reg(apm_node, SS_EBX); /* drivers/elog/gsmi.c */ *ret = gsmi_exec(sub_command, param); @@ -272,21 +276,33 @@ static void southbridge_smi_store(void) { u8 sub_command, ret; - em64t101_smm_state_save_area_t *io_smi = - smi_apmc_find_state_save(APM_CNT_SMMSTORE); - uintptr_t reg_rbx; + int apm_node = smi_apmc_find_state_save(APM_CNT_SMMSTORE); + void *reg_rbx; - if (!io_smi) + if (apm_node < 0) return; /* Command and return value in EAX */ - sub_command = (io_smi->rax >> 8) & 0xff; + sub_command = (*(uintptr_t *)save_state_reg(apm_node, SS_EAX) >> 8) & 0xff; /* Parameter buffer in EBX */ - reg_rbx = (uintptr_t)io_smi->rbx; + reg_rbx = save_state_reg(apm_node, SS_EBX); /* drivers/smmstore/smi.c */ - ret = smmstore_exec(sub_command, (void *)reg_rbx); - io_smi->rax = ret; + ret = smmstore_exec(sub_command, reg_rbx); + *(uintptr_t *)save_state_reg(apm_node, SS_EAX) = ret; +} + +static void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) +{ + int apm_node = smi_apmc_find_state_save(apm_cnt); + uintptr_t gnvs; + if (apm_node > 0) { + /* EBX in the state save contains the GNVS pointer */ + gnvs = *(uintptr_t *)save_state_reg(apm_node, SS_EBX); + set_gnvs_ptr(gnvs); + *smm_done = 1; + printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs); + } } static int mainboard_finalized = 0; diff --git a/src/southbridge/intel/i82801gx/smihandler.c b/src/southbridge/intel/i82801gx/smihandler.c index 16ceb13..c697cf5 100644 --- a/src/southbridge/intel/i82801gx/smihandler.c +++ b/src/southbridge/intel/i82801gx/smihandler.c @@ -41,12 +41,11 @@ /* GNVS needs to be updated by an 0xEA PM Trap (B2) after it has been located * by coreboot. */ -global_nvs_t *gnvs = (global_nvs_t *)0x0; +static global_nvs_t *gnvs; -void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) +void set_gnvs_ptr(uintptr_t offset) { - gnvs = *(global_nvs_t **)0x500; - *smm_done = 1; + gnvs = (global_nvs_t *)offset; } int southbridge_io_trap_handler(int smif) diff --git a/src/southbridge/intel/i82801ix/smihandler.c b/src/southbridge/intel/i82801ix/smihandler.c index 8090a09..2edba39 100644 --- a/src/southbridge/intel/i82801ix/smihandler.c +++ b/src/southbridge/intel/i82801ix/smihandler.c @@ -28,9 +28,7 @@ /* GNVS needs to be updated by an 0xEA PM Trap (B2) after it has been located * by coreboot. */ -global_nvs_t *gnvs = (global_nvs_t *)0x0; -void *tcg = (void *)0x0; -void *smi1 = (void *)0x0; +static global_nvs_t *gnvs; int southbridge_io_trap_handler(int smif) { @@ -49,12 +47,9 @@ return 0; } -void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) +void set_gnvs_ptr(uintptr_t offset) { - gnvs = *(global_nvs_t **)0x500; - tcg = *(void **)0x504; - smi1 = *(void **)0x508; - *smm_done = 1; + gnvs = (global_nvs_t *)offset; } void southbridge_smi_monitor(void) diff --git a/src/southbridge/intel/i82801jx/smihandler.c b/src/southbridge/intel/i82801jx/smihandler.c index 667a853..69a9e7a 100644 --- a/src/southbridge/intel/i82801jx/smihandler.c +++ b/src/southbridge/intel/i82801jx/smihandler.c @@ -35,9 +35,7 @@ /* GNVS needs to be updated by an 0xEA PM Trap (B2) after it has been located * by coreboot. */ -global_nvs_t *gnvs = (global_nvs_t *)0x0; -void *tcg = (void *)0x0; -void *smi1 = (void *)0x0; +static global_nvs_t *gnvs; int southbridge_io_trap_handler(int smif) { @@ -56,12 +54,9 @@ return 0; } -void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) +void set_gnvs_ptr(uintptr_t offset) { - gnvs = *(global_nvs_t **)0x500; - tcg = *(void **)0x504; - smi1 = *(void **)0x508; - *smm_done = 1; + gnvs = (global_nvs_t *)offset; } void southbridge_smi_monitor(void) diff --git a/src/southbridge/intel/ibexpeak/smihandler.c b/src/southbridge/intel/ibexpeak/smihandler.c index 3668842..961d207 100644 --- a/src/southbridge/intel/ibexpeak/smihandler.c +++ b/src/southbridge/intel/ibexpeak/smihandler.c @@ -171,16 +171,9 @@ #undef IOTRAP } -void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) +void set_gnvs_ptr(uintptr_t offset) { - em64t101_smm_state_save_area_t *state = - smi_apmc_find_state_save(apm_cnt); - if (state) { - /* EBX in the state save contains the GNVS pointer */ - gnvs = (global_nvs_t *)((u32)state->rbx); - *smm_done = 1; - printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs); - } + gnvs = (global_nvs_t *)offset; } void southbridge_finalize_all(void) -- To view, visit https://review.coreboot.org/c/coreboot/+/36590 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I63f77432cf4a2ef484ad320bfc321e9a923a19ab Gerrit-Change-Number: 36590 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

2 6

Change in ...coreboot[master]: [WIP]Makefile.inc: Simplify generating the default fmap
by Arthur Heymans (Code Review) 21 Apr '22

21 Apr '22

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33275 Change subject: [WIP]Makefile.inc: Simplify generating the default fmap ...................................................................... [WIP]Makefile.inc: Simplify generating the default fmap Apparently you don't always need to specify base/size as fmap tool fills in the gaps, so make use of that to remove some ugly Makefile arithmetics. Change-Id: Ib138249140fc87307c3dc76692f343587117c7e0 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M Makefile.inc M util/cbfstool/default-x86.fmd 2 files changed, 7 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/33275/1 diff --git a/Makefile.inc b/Makefile.inc index d4f7597..13cb6ed 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -865,10 +865,9 @@ # X86 CONSOLE FMAP region # # position, size and entry line of CONSOLE relative to BIOS_BASE, if enabled -FMAP_CONSOLE_BASE := 0 ifeq ($(CONFIG_CONSOLE_SPI_FLASH),y) FMAP_CONSOLE_SIZE := $(CONFIG_CONSOLE_SPI_FLASH_BUFFER_SIZE) -FMAP_CONSOLE_ENTRY := CONSOLE@$(FMAP_CONSOLE_BASE) $(FMAP_CONSOLE_SIZE) +FMAP_CONSOLE_ENTRY := CONSOLE $(FMAP_CONSOLE_SIZE) else # ifeq ($(CONFIG_CONSOLE_SPI_FLASH),y) FMAP_CONSOLE_SIZE := 0 FMAP_CONSOLE_ENTRY := @@ -879,12 +878,9 @@ # # position, size and entry line of MRC_CACHE relative to BIOS_BASE, if enabled ifeq ($(CONFIG_CACHE_MRC_SETTINGS),y) -FMAP_MRC_CACHE_BASE := $(call int-align, $(call int-add, $(FMAP_CONSOLE_BASE) \ - $(FMAP_CONSOLE_SIZE)), 0x10000) FMAP_MRC_CACHE_SIZE := $(CONFIG_MRC_SETTINGS_CACHE_SIZE) -FMAP_MRC_CACHE_ENTRY := RW_MRC_CACHE@$(FMAP_MRC_CACHE_BASE) $(FMAP_MRC_CACHE_SIZE) +FMAP_MRC_CACHE_ENTRY := RW_MRC_CACHE $(FMAP_MRC_CACHE_SIZE) else # ifeq ($(CONFIG_CACHE_MRC_SETTINGS),y) -FMAP_MRC_CACHE_BASE := 0 FMAP_MRC_CACHE_SIZE := 0 FMAP_MRC_CACHE_ENTRY := endif # ifeq ($(CONFIG_CACHE_MRC_SETTINGS),y) @@ -894,12 +890,9 @@ # # position, size and entry line of SMMSTORE relative to BIOS_BASE, if enabled ifeq ($(CONFIG_SMMSTORE),y) -FMAP_SMMSTORE_BASE := $(call int-align, $(call int-add, $(FMAP_CONSOLE_BASE) \ - $(FMAP_CONSOLE_SIZE) $(FMAP_MRC_CACHE_SIZE)), 0x10000) FMAP_SMMSTORE_SIZE := $(CONFIG_SMMSTORE_SIZE) -FMAP_SMMSTORE_ENTRY := SMMSTORE@$(FMAP_SMMSTORE_BASE) $(FMAP_SMMSTORE_SIZE) +FMAP_SMMSTORE_ENTRY := SMMSTORE $(FMAP_SMMSTORE_SIZE) else # ifeq ($(CONFIG_SMMSTORE),y) -FMAP_SMMSTORE_BASE := 0 FMAP_SMMSTORE_SIZE := 0 FMAP_SMMSTORE_ENTRY := endif # ifeq ($(CONFIG_CACHE_MRC_SETTINGS),y) @@ -909,16 +902,14 @@ # # # position, size -FMAP_FMAP_BASE := $(call int-add, $(FMAP_CONSOLE_BASE) $(FMAP_CONSOLE_SIZE) \ - $(FMAP_MRC_CACHE_SIZE) $(FMAP_SMMSTORE_SIZE)) FMAP_FMAP_SIZE := 0x200 # # X86 COREBOOT default cbfs FMAP region # # position and size of CBFS, relative to BIOS_BASE -FMAP_CBFS_BASE := $(call int-add, $(FMAP_FMAP_BASE) $(FMAP_FMAP_SIZE)) -FMAP_CBFS_SIZE := $(call int-subtract, $(FMAP_BIOS_SIZE) $(FMAP_CBFS_BASE)) +FMAP_CBFS_SIZE := $(shell echo $($(FMAP_BIOS_SIZE) - $(FMAP_FMAP_SIZE) \ + - $(FMAP_CONSOLE_SIZE) - $(FMAP_MRC_CACHE_SIZE) - $(FMAP_SMMSTORE_SIZE))) else # ifeq ($(CONFIG_ARCH_X86),y) DEFAULT_FLASHMAP:=$(top)/util/cbfstool/default.fmd # entire flash diff --git a/util/cbfstool/default-x86.fmd b/util/cbfstool/default-x86.fmd index f0143e9..1d1fec9 100644 --- a/util/cbfstool/default-x86.fmd +++ b/util/cbfstool/default-x86.fmd @@ -12,7 +12,7 @@ ##CONSOLE_ENTRY## ##MRC_CACHE_ENTRY## ##SMMSTORE_ENTRY## - FMAP@##FMAP_BASE## ##FMAP_SIZE## - COREBOOT(CBFS)@##CBFS_BASE## ##CBFS_SIZE## + FMAP ##FMAP_SIZE## + COREBOOT(CBFS) ##CBFS_SIZE## } } -- To view, visit https://review.coreboot.org/c/coreboot/+/33275 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib138249140fc87307c3dc76692f343587117c7e0 Gerrit-Change-Number: 33275 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

2 2

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021