Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48405 )
Change subject: [WIP] soc/amd: Remove Kconfig X86_RESET_VECTOR ......................................................................
[WIP] soc/amd: Remove Kconfig X86_RESET_VECTOR
The architectural requirement is for the address to be located at the end of bootblock -0x10 bytes, so the definition was redundant with other Kconfig variables.
Change-Id: Ia014470cfadf0b401a12a2de6dce3b1fc1862137 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld M src/soc/amd/picasso/Kconfig 2 files changed, 3 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/48405/1
diff --git a/src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld b/src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld index 2ae72e3..e595065 100644 --- a/src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld +++ b/src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld @@ -84,7 +84,6 @@ #if CONFIG(VBOOT) PSP_SHAREDMEM_DRAM_END(CONFIG_PSP_SHAREDMEM_BASE + CONFIG_PSP_SHAREDMEM_SIZE) #endif - _ = ASSERT((CONFIG_BOOTBLOCK_ADDR + CONFIG_C_ENV_BOOTBLOCK_SIZE - 0x10) == CONFIG_X86_RESET_VECTOR, "Reset vector should be -0x10 from end of bootblock"); _ = ASSERT(CONFIG_BOOTBLOCK_ADDR == ((CONFIG_BOOTBLOCK_ADDR + 0xFFFF) & 0xFFFF0000), "Bootblock must be 16 bit aligned"); BOOTBLOCK(CONFIG_BOOTBLOCK_ADDR, CONFIG_C_ENV_BOOTBLOCK_SIZE) ROMSTAGE(CONFIG_ROMSTAGE_ADDR, CONFIG_ROMSTAGE_SIZE) @@ -105,10 +104,11 @@
SECTIONS { /* Trigger an error if I have an unusable start address */ - _TOO_LOW = CONFIG_X86_RESET_VECTOR - 0xfff0; + _TOO_LOW = _X86_RESET_VECTOR - 0xfff0; _bogus = ASSERT(_start16bit >= _TOO_LOW, "_start16bit too low. Please report.");
- . = CONFIG_X86_RESET_VECTOR; + . = CONFIG_BOOTBLOCK_ADDR + CONFIG_C_ENV_BOOTBLOCK_SIZE - 0x10; + _X86_RESET_VECTOR = .; .reset . : { *(.reset); . = 15; diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 79fc3be..fcb7ddb 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -134,15 +134,6 @@ This variable controls the DRAM allocation size in linker script for bootblock stage.
-config X86_RESET_VECTOR - hex - depends on ARCH_X86 - default 0x203fff0 - help - Sets the reset vector within bootblock where x86 starts execution. - Reset vector is supposed to live at offset -0x10 from end of - bootblock i.e. BOOTBLOCK_ADDR + C_ENV_BOOTBLOCK_SIZE - 0x10. - config ROMSTAGE_ADDR hex default 0x2040000
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48405 )
Change subject: [WIP] soc/amd: Remove Kconfig X86_RESET_VECTOR ......................................................................
Patch Set 2:
looks good to me, but misses the corresponding change to the Cezanne Kconfig file. I'll run a test, but I don't expect any issues there
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48405 )
Change subject: [WIP] soc/amd: Remove Kconfig X86_RESET_VECTOR ......................................................................
Patch Set 2:
just tested it and it doesn't change the resulting image and boots on mandolin. will +2 when the change is also added to the cezanne kconfig
Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Julius Werner, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48405
to look at the new patch set (#3).
Change subject: soc/amd: Remove Kconfig X86_RESET_VECTOR ......................................................................
soc/amd: Remove Kconfig X86_RESET_VECTOR
The architectural requirement is for the address to be located at the end of bootblock -0x10 bytes, so the definition was redundant with other Kconfig variables.
Change-Id: Ia014470cfadf0b401a12a2de6dce3b1fc1862137 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/amd/cezanne/Kconfig M src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld M src/soc/amd/picasso/Kconfig 3 files changed, 3 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/48405/3
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48405 )
Change subject: soc/amd: Remove Kconfig X86_RESET_VECTOR ......................................................................
Patch Set 3: Code-Review+2
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48405 )
Change subject: soc/amd: Remove Kconfig X86_RESET_VECTOR ......................................................................
Patch Set 3: Code-Review+2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48405 )
Change subject: soc/amd: Remove Kconfig X86_RESET_VECTOR ......................................................................
soc/amd: Remove Kconfig X86_RESET_VECTOR
The architectural requirement is for the address to be located at the end of bootblock -0x10 bytes, so the definition was redundant with other Kconfig variables.
Change-Id: Ia014470cfadf0b401a12a2de6dce3b1fc1862137 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/48405 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M src/soc/amd/cezanne/Kconfig M src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld M src/soc/amd/picasso/Kconfig 3 files changed, 3 insertions(+), 21 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved Marshall Dawson: Looks good to me, approved
diff --git a/src/soc/amd/cezanne/Kconfig b/src/soc/amd/cezanne/Kconfig index 2a26d6d..d2badc9 100644 --- a/src/soc/amd/cezanne/Kconfig +++ b/src/soc/amd/cezanne/Kconfig @@ -61,15 +61,6 @@ This variable controls the DRAM allocation size in linker script for bootblock stage.
-config X86_RESET_VECTOR - hex - depends on ARCH_X86 - default 0x203fff0 - help - Sets the reset vector within bootblock where x86 starts execution. - Reset vector is supposed to live at offset -0x10 from end of - bootblock i.e. BOOTBLOCK_ADDR + C_ENV_BOOTBLOCK_SIZE - 0x10. - config ROMSTAGE_ADDR hex default 0x2040000 diff --git a/src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld b/src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld index e6de812..dae69d0 100644 --- a/src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld +++ b/src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld @@ -84,7 +84,6 @@ #if CONFIG(VBOOT) PSP_SHAREDMEM_DRAM_END(CONFIG_PSP_SHAREDMEM_BASE + CONFIG_PSP_SHAREDMEM_SIZE) #endif - _ = ASSERT((CONFIG_BOOTBLOCK_ADDR + CONFIG_C_ENV_BOOTBLOCK_SIZE - 0x10) == CONFIG_X86_RESET_VECTOR, "Reset vector should be -0x10 from end of bootblock"); _ = ASSERT(CONFIG_BOOTBLOCK_ADDR == ((CONFIG_BOOTBLOCK_ADDR + 0xFFFF) & 0xFFFF0000), "Bootblock must be 16 bit aligned"); BOOTBLOCK(CONFIG_BOOTBLOCK_ADDR, CONFIG_C_ENV_BOOTBLOCK_SIZE) ROMSTAGE(CONFIG_ROMSTAGE_ADDR, CONFIG_ROMSTAGE_SIZE) @@ -105,10 +104,11 @@
SECTIONS { /* Trigger an error if I have an unusable start address */ - _TOO_LOW = CONFIG_X86_RESET_VECTOR - 0xfff0; + _TOO_LOW = _X86_RESET_VECTOR - 0xfff0; _bogus = ASSERT(_start16bit >= _TOO_LOW, "_start16bit too low. Please report.");
- . = CONFIG_X86_RESET_VECTOR; + . = CONFIG_BOOTBLOCK_ADDR + CONFIG_C_ENV_BOOTBLOCK_SIZE - 0x10; + _X86_RESET_VECTOR = .; .reset . : { *(.reset); . = 15; diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 79fc3be..fcb7ddb 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -134,15 +134,6 @@ This variable controls the DRAM allocation size in linker script for bootblock stage.
-config X86_RESET_VECTOR - hex - depends on ARCH_X86 - default 0x203fff0 - help - Sets the reset vector within bootblock where x86 starts execution. - Reset vector is supposed to live at offset -0x10 from end of - bootblock i.e. BOOTBLOCK_ADDR + C_ENV_BOOTBLOCK_SIZE - 0x10. - config ROMSTAGE_ADDR hex default 0x2040000
-
Felix Held (Code Review) -
Kyösti Mälkki (Code Review)
-
Marshall Dawson (Code Review)