coreboot-gerrit March 2023

coreboot-gerrit@coreboot.org

1 participants
4880 discussions

[S] Change in coreboot[master]: soc/amd/phoenix: Allow the amdfw.rom to be split into two parts
by Fred Reitberger (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Bao Zheng, Jason Glenesk, Matt DeVillier, Zheng Bao, Karthik Ramasubramanian, Felix Held. Fred Reitberger has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72703 ) Change subject: soc/amd/phoenix: Allow the amdfw.rom to be split into two parts ...................................................................... Patch Set 9: -Code-Review (2 comments) Patchset: PS9: n File src/soc/amd/phoenix/Makefile.inc: https://review.coreboot.org/c/coreboot/+/72703/comment/b18d684e_e3c1447a PS9, Line 212: OPT_AMDFW_BODY_LOCATION=$(if $(FMAP_AMDFW_BODY_LOCATION), --body-location $(FMAP_AMDFW_BODY_LOCATION)) Any reason this is not using `$(call add_opt_prefix, ...)` like the other options? -- To view, visit https://review.coreboot.org/c/coreboot/+/72703 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia8b318f71632a2c9b97ce67486374dc24d23e63e Gerrit-Change-Number: 72703 Gerrit-PatchSet: 9 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Zheng Bao Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 02 Mar 2023 20:26:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: amdfwtool: Remove the limit of spliting EFS and body
by Fred Reitberger (Code Review) 02 Mar '23

02 Mar '23

Fred Reitberger has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69857 ) Change subject: amdfwtool: Remove the limit of spliting EFS and body ...................................................................... amdfwtool: Remove the limit of spliting EFS and body To support 32M flash, the non-vboot also need to split amdfw. Just as the deleted comment says, we need this feature now. This is one of series of patches to support 32/64M flash. BUG=b:255374782 Change-Id: Ic058cfaeebd1a947227cfa9be2db4eb22702aa28 Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/69857 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Fred Reitberger <reitbergerfred(a)gmail.com> --- M util/amdfwtool/amdfwtool.c 1 file changed, 19 insertions(+), 11 deletions(-) Approvals: build bot (Jenkins): Verified Fred Reitberger: Looks good to me, approved diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 4c570be..edac06a 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -2423,17 +2423,6 @@ return 1; } - /* - * On boards using vboot, there can be more than one instance of EFS + AMDFW Body. - * For the instance in the RO section, there is no need to split EFS + AMDFW body - * currently. This condition is to ensure that it is not accidentally split. Revisit - * this condition if such a need arises in the future. - */ - if (!any_location && body_location != efs_location) { - fprintf(stderr, "Error: EFS cannot be separate from AMDFW Body.\n"); - return 1; - } - if (body_location != efs_location && body_location < ALIGN(efs_location + sizeof(embedded_firmware), BLOB_ALIGNMENT)) { fprintf(stderr, "Error: Insufficient space between EFS and Blobs.\n"); -- To view, visit https://review.coreboot.org/c/coreboot/+/69857 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic058cfaeebd1a947227cfa9be2db4eb22702aa28 Gerrit-Change-Number: 69857 Gerrit-PatchSet: 14 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: amdfwtool: Change .rom.efs to .rom and .rom to .rom.body
by Fred Reitberger (Code Review) 02 Mar '23

02 Mar '23

Fred Reitberger has submitted this change. ( https://review.coreboot.org/c/coreboot/+/72702 ) Change subject: amdfwtool: Change .rom.efs to .rom and .rom to .rom.body ...................................................................... amdfwtool: Change .rom.efs to .rom and .rom to .rom.body To support 32M flash, the non-vboot also need to split amdfw. The amdfw.rom is the default filename added to CBFS. Keep the default filename and then we don't have to change all the CBFS definition. This is one of series of patches to support 32/64M flash. BUG=b:255374782 Change-Id: Id77b11422d4549cf57a1cd8980c7a9cf3597d1bc Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/72702 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Fred Reitberger <reitbergerfred(a)gmail.com> --- M src/soc/amd/mendocino/Makefile.inc M util/amdfwtool/amdfwtool.c 2 files changed, 62 insertions(+), 36 deletions(-) Approvals: build bot (Jenkins): Verified Fred Reitberger: Looks good to me, approved diff --git a/src/soc/amd/mendocino/Makefile.inc b/src/soc/amd/mendocino/Makefile.inc index 4b971d3..2995037 100644 --- a/src/soc/amd/mendocino/Makefile.inc +++ b/src/soc/amd/mendocino/Makefile.inc @@ -171,8 +171,8 @@ SIGNED_AMDFW_B_POSITION=$(call int-subtract, \ $(shell awk '$$2 == "FMAP_SECTION_SIGNED_AMDFW_B_START" {print $$3}' $(obj)/fmap_config.h) \ $(shell awk '$$2 == "FMAP_SECTION_FLASH_START" {print $$3}' $(obj)/fmap_config.h)) -SIGNED_AMDFW_A_FILE=$(obj)/amdfw_a.rom.signed -SIGNED_AMDFW_B_FILE=$(obj)/amdfw_b.rom.signed +SIGNED_AMDFW_A_FILE=$(obj)/amdfw_a.rom.body.signed +SIGNED_AMDFW_B_FILE=$(obj)/amdfw_b.rom.body.signed endif # CONFIG_SEPARATE_SIGNED_PSPFW # Helper function to return a value with given bit set @@ -306,41 +306,41 @@ --anywhere \ --output $@ -$(obj)/amdfw_a.rom.efs: $(obj)/amdfw_a.rom -$(obj)/amdfw_b.rom.efs: $(obj)/amdfw_b.rom +$(obj)/amdfw_a.rom.body: $(obj)/amdfw_a.rom +$(obj)/amdfw_b.rom.body: $(obj)/amdfw_b.rom ifeq ($(CONFIG_VBOOT_SLOTS_RW_A)$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),yy) cbfs-files-y += apu/amdfw_a -apu/amdfw_a-file := $(obj)/amdfw_a.rom.efs +apu/amdfw_a-file := $(obj)/amdfw_a.rom apu/amdfw_a-position := $(AMD_FW_AB_POSITION) apu/amdfw_a-type := raw cbfs-files-y += apu/amdfw_a_body -apu/amdfw_a_body-file := $(obj)/amdfw_a.rom +apu/amdfw_a_body-file := $(obj)/amdfw_a.rom.body apu/amdfw_a_body-position := $(call int-add, $(AMD_FW_AB_POSITION) $(MENDOCINO_FW_BODY_OFFSET)) apu/amdfw_a_body-type := raw endif ifeq ($(CONFIG_VBOOT_SLOTS_RW_AB)$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),yy) cbfs-files-y += apu/amdfw_b -apu/amdfw_b-file := $(obj)/amdfw_b.rom.efs +apu/amdfw_b-file := $(obj)/amdfw_b.rom apu/amdfw_b-position := $(AMD_FW_AB_POSITION) apu/amdfw_b-type := raw cbfs-files-y += apu/amdfw_b_body -apu/amdfw_b_body-file := $(obj)/amdfw_b.rom +apu/amdfw_b_body-file := $(obj)/amdfw_b.rom.body apu/amdfw_b_body-position := $(call int-add, $(AMD_FW_AB_POSITION) $(MENDOCINO_FW_BODY_OFFSET)) apu/amdfw_b_body-type := raw endif ifeq ($(CONFIG_SEPARATE_SIGNED_PSPFW)$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),yy) -build_complete:: $(obj)/amdfw_a.rom $(obj)/amdfw_b.rom +build_complete:: $(obj)/amdfw_a.rom.body $(obj)/amdfw_b.rom.body @printf " Adding Signed ROM and HASH\n" - $(CBFSTOOL) $(obj)/coreboot.rom write -u -r SIGNED_AMDFW_A -i 0 -f $(obj)/amdfw_a.rom.signed - $(CBFSTOOL) $(obj)/coreboot.rom write -u -r SIGNED_AMDFW_B -i 0 -f $(obj)/amdfw_b.rom.signed - $(CBFSTOOL) $(obj)/coreboot.rom add -r FW_MAIN_A -f $(obj)/amdfw_a.rom.signed.hash \ + $(CBFSTOOL) $(obj)/coreboot.rom write -u -r SIGNED_AMDFW_A -i 0 -f $(obj)/amdfw_a.rom.body.signed + $(CBFSTOOL) $(obj)/coreboot.rom write -u -r SIGNED_AMDFW_B -i 0 -f $(obj)/amdfw_b.rom.body.signed + $(CBFSTOOL) $(obj)/coreboot.rom add -r FW_MAIN_A -f $(obj)/amdfw_a.rom.body.signed.hash \ -n apu/amdfw_a_hash -t raw - $(CBFSTOOL) $(obj)/coreboot.rom add -r FW_MAIN_B -f $(obj)/amdfw_b.rom.signed.hash \ + $(CBFSTOOL) $(obj)/coreboot.rom add -r FW_MAIN_B -f $(obj)/amdfw_b.rom.body.signed.hash \ -n apu/amdfw_b_hash -t raw endif diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 19a9438..4c570be 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -111,6 +111,7 @@ #define HASH_FILE_SUFFIX ".hash" #define EFS_FILE_SUFFIX ".efs" #define TMP_FILE_SUFFIX ".tmp" +#define BODY_FILE_SUFFIX ".body" /* * Beginning with Family 15h Models 70h-7F, a.k.a Stoney Ridge, the PSP @@ -2043,53 +2044,53 @@ return 0; } -static ssize_t write_efs(char *output, embedded_firmware *amd_romsig, context *ctx) +static ssize_t write_body(char *output, void *body_offset, ssize_t body_size, context *ctx) { - char efs_name[PATH_MAX], efs_tmp_name[PATH_MAX]; + char body_name[PATH_MAX], body_tmp_name[PATH_MAX]; int ret; int fd; ssize_t bytes = -1; /* Create a tmp file and rename it at the end so that make does not get confused if amdfwtool is killed for some unexpected reasons. */ - ret = snprintf(efs_tmp_name, sizeof(efs_tmp_name), "%s%s%s", - output, EFS_FILE_SUFFIX, TMP_FILE_SUFFIX); + ret = snprintf(body_tmp_name, sizeof(body_tmp_name), "%s%s%s", + output, BODY_FILE_SUFFIX, TMP_FILE_SUFFIX); if (ret < 0) { - fprintf(stderr, "Error %s forming EFS tmp file name: %d\n", + fprintf(stderr, "Error %s forming BODY tmp file name: %d\n", strerror(errno), ret); amdfwtool_cleanup(ctx); exit(1); - } else if ((unsigned int)ret >= sizeof(efs_tmp_name)) { - fprintf(stderr, "EFS File name %d > %zu\n", ret, sizeof(efs_tmp_name)); + } else if ((unsigned int)ret >= sizeof(body_tmp_name)) { + fprintf(stderr, "BODY File name %d > %zu\n", ret, sizeof(body_tmp_name)); amdfwtool_cleanup(ctx); exit(1); } - fd = open(efs_tmp_name, O_RDWR | O_CREAT | O_TRUNC, 0666); + fd = open(body_tmp_name, O_RDWR | O_CREAT | O_TRUNC, 0666); if (fd < 0) { - fprintf(stderr, "Error: Opening %s file: %s\n", efs_tmp_name, strerror(errno)); + fprintf(stderr, "Error: Opening %s file: %s\n", body_tmp_name, strerror(errno)); amdfwtool_cleanup(ctx); exit(1); } - bytes = write_from_buf_to_file(fd, amd_romsig, sizeof(*amd_romsig)); - if (bytes != sizeof(*amd_romsig)) { - fprintf(stderr, "Error: Writing to file %s failed\n", efs_tmp_name); + bytes = write_from_buf_to_file(fd, body_offset, body_size); + if (bytes != body_size) { + fprintf(stderr, "Error: Writing to file %s failed\n", body_tmp_name); amdfwtool_cleanup(ctx); exit(1); } close(fd); /* Rename the tmp file */ - ret = snprintf(efs_name, sizeof(efs_name), "%s%s", output, EFS_FILE_SUFFIX); + ret = snprintf(body_name, sizeof(body_name), "%s%s", output, BODY_FILE_SUFFIX); if (ret < 0) { - fprintf(stderr, "Error %s forming EFS file name: %d\n", strerror(errno), ret); + fprintf(stderr, "Error %s forming BODY file name: %d\n", strerror(errno), ret); amdfwtool_cleanup(ctx); exit(1); } - if (rename(efs_tmp_name, efs_name)) { - fprintf(stderr, "Error: renaming file %s to %s\n", efs_tmp_name, efs_name); + if (rename(body_tmp_name, body_name)) { + fprintf(stderr, "Error: renaming file %s to %s\n", body_tmp_name, body_name); amdfwtool_cleanup(ctx); exit(1); } @@ -2666,11 +2667,13 @@ targetfd = open(output, O_RDWR | O_CREAT | O_TRUNC, 0666); if (targetfd >= 0) { - ssize_t bytes; - uint32_t offset = body_location ? body_location : AMD_ROMSIG_OFFSET; + uint32_t offset = efs_location; + uint32_t bytes = efs_location == body_location ? + ctx.current - offset : sizeof(*amd_romsig); + uint32_t ret_bytes; - bytes = write(targetfd, BUFF_OFFSET(ctx, offset), ctx.current - offset); - if (bytes != ctx.current - offset) { + ret_bytes = write(targetfd, BUFF_OFFSET(ctx, offset), bytes); + if (bytes != ret_bytes) { fprintf(stderr, "Error: Writing to file %s failed\n", output); retval = 1; } @@ -2683,9 +2686,10 @@ if (efs_location != body_location) { ssize_t bytes; - bytes = write_efs(output, amd_romsig, &ctx); - if (bytes != sizeof(*amd_romsig)) { - fprintf(stderr, "Error: Writing EFS\n"); + bytes = write_body(output, BUFF_OFFSET(ctx, body_location), + ctx.current - body_location, &ctx); + if (bytes != ctx.current - body_location) { + fprintf(stderr, "Error: Writing body\n"); retval = 1; } } -- To view, visit https://review.coreboot.org/c/coreboot/+/72702 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id77b11422d4549cf57a1cd8980c7a9cf3597d1bc Gerrit-Change-Number: 72702 Gerrit-PatchSet: 11 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: [UNTESTED] nb/amd/pi/00730F01: request binaryPI to use \_SB_ scope in...
by Felix Held (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Michał Żygowski. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73386 ) Change subject: [UNTESTED] nb/amd/pi/00730F01: request binaryPI to use \_SB_ scope in PSTATE SSDT ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: not sure when i'll get around to test this, so would be good if you could run a test on pcenines/apu2. this works on stoneyridge, so it'll likely work here too -- To view, visit https://review.coreboot.org/c/coreboot/+/73386 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I411201b55cfee30ae41da4e6814679bdb49e9bf7 Gerrit-Change-Number: 73386 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Comment-Date: Thu, 02 Mar 2023 20:06:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: [UNTESTED] nb/amd/pi/00730F01: request binaryPI to use \_SB_ scope in...
by Felix Held (Code Review) 02 Mar '23

02 Mar '23

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/73386 ) Change subject: [UNTESTED] nb/amd/pi/00730F01: request binaryPI to use \_SB_ scope in PSTATE SSDT ...................................................................... [UNTESTED] nb/amd/pi/00730F01: request binaryPI to use \_SB_ scope in PSTATE SSDT Instead of having binaryPI generate a PSTATE SSDT that uses \_PR_ as the scope for the CPU objects and patching this SSDT in coreboot to use the \_SB_ scope in patch_ssdt_processor_scope, request binaryPI to use the \_SB_ scope instead by setting the late platform configuration option ProcessorScopeInSb to true. TEST=Untested, but equivalent change for Stoneyridge works. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I411201b55cfee30ae41da4e6814679bdb49e9bf7 --- M src/northbridge/amd/pi/00730F01/northbridge.c M src/northbridge/amd/pi/00730F01/state_machine.c 2 files changed, 21 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/73386/1 diff --git a/src/northbridge/amd/pi/00730F01/northbridge.c b/src/northbridge/amd/pi/00730F01/northbridge.c index dab0e72..90d80db 100644 --- a/src/northbridge/amd/pi/00730F01/northbridge.c +++ b/src/northbridge/amd/pi/00730F01/northbridge.c @@ -558,21 +558,6 @@ acpigen_pop_len(); } -static void patch_ssdt_processor_scope(acpi_header_t *ssdt) -{ - unsigned int len = ssdt->length - sizeof(acpi_header_t); - unsigned int i; - - for (i = sizeof(acpi_header_t); i < len; i++) { - /* Search for _PR_ scope and replace it with _SB_ */ - if (*(uint32_t *)((unsigned long)ssdt + i) == 0x5f52505f) - *(uint32_t *)((unsigned long)ssdt + i) = 0x5f42535f; - } - /* Recalculate checksum */ - ssdt->checksum = 0; - ssdt->checksum = acpi_checksum((void *)ssdt, ssdt->length); -} - static unsigned long agesa_write_acpi_tables(const struct device *device, unsigned long current, acpi_rsdp_t *rsdp) @@ -643,7 +628,6 @@ printk(BIOS_DEBUG, "ACPI: * SSDT at %lx\n", current); ssdt = (acpi_header_t *)agesawrapper_getlateinitptr(PICK_PSTATE); if (ssdt != NULL) { - patch_ssdt_processor_scope(ssdt); memcpy((void *)current, ssdt, ssdt->length); ssdt = (acpi_header_t *)current; current += ssdt->length; diff --git a/src/northbridge/amd/pi/00730F01/state_machine.c b/src/northbridge/amd/pi/00730F01/state_machine.c index ba34dab..ef6c624 100644 --- a/src/northbridge/amd/pi/00730F01/state_machine.c +++ b/src/northbridge/amd/pi/00730F01/state_machine.c @@ -68,6 +68,9 @@ Late->GnbLateConfiguration.FchIoapicId = CONFIG_MAX_CPUS; } + /* Make binariPY use \_SB_ as processor object scope in PSTATE SSDT */ + Late->PlatformConfig.ProcessorScopeInSb = true; + /* Code for creating CDIT requires hop count table. If it is not * present AGESA_ERROR is returned, which confuses users. CDIT is not * written to the ACPI tables anyway. */ -- To view, visit https://review.coreboot.org/c/coreboot/+/73386 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I411201b55cfee30ae41da4e6814679bdb49e9bf7 Gerrit-Change-Number: 73386 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[master]: soc/amd/stoneyridge: request binaryPI to use \_SB_ scope in PSTATE SSDT
by Felix Held (Code Review) 02 Mar '23

02 Mar '23

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/73385 ) Change subject: soc/amd/stoneyridge: request binaryPI to use \_SB_ scope in PSTATE SSDT ...................................................................... soc/amd/stoneyridge: request binaryPI to use \_SB_ scope in PSTATE SSDT Instead of having binaryPI generate a PSTATE SSDT that uses \_PR_ as the scope for the CPU objects and patching this SSDT in coreboot to use the \_SB_ scope in patch_ssdt_processor_scope, request binaryPI to use the \_SB_ scope instead by setting the late platform configuration option ProcessorScopeInSb to true. TEST=Careena still boots and Linux doesn't show any ACPI errors with this patch applied. With only patch_ssdt_processor_scope removed, but the ProcessorScopeInSb option not set, Linux will complain that it can't resolve the \PR.P00x symbols. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: If88820a0f5df923f129e2e3b5335f5f0e38ee7f5 --- M src/soc/amd/common/pi/agesawrapper.c M src/soc/amd/stoneyridge/northbridge.c 2 files changed, 24 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/73385/1 diff --git a/src/soc/amd/common/pi/agesawrapper.c b/src/soc/amd/common/pi/agesawrapper.c index 4085855..07fc16e 100644 --- a/src/soc/amd/common/pi/agesawrapper.c +++ b/src/soc/amd/common/pi/agesawrapper.c @@ -266,6 +266,9 @@ LateParams->GnbLateConfiguration.FchIoapicId = FCH_IOAPIC_ID; } + /* Make binariPY use \_SB_ as processor object scope in PSTATE SSDT */ + LateParams->PlatformConfig.ProcessorScopeInSb = true; + timestamp_add_now(TS_AGESA_INIT_LATE_START); Status = amd_dispatch(LateParams); timestamp_add_now(TS_AGESA_INIT_LATE_END); diff --git a/src/soc/amd/stoneyridge/northbridge.c b/src/soc/amd/stoneyridge/northbridge.c index 6389cc4..e318024 100644 --- a/src/soc/amd/stoneyridge/northbridge.c +++ b/src/soc/amd/stoneyridge/northbridge.c @@ -207,21 +207,6 @@ acpigen_pop_len(); } -static void patch_ssdt_processor_scope(acpi_header_t *ssdt) -{ - unsigned int len = ssdt->length - sizeof(acpi_header_t); - unsigned int i; - - for (i = sizeof(acpi_header_t); i < len; i++) { - /* Search for _PR_ scope and replace it with _SB_ */ - if (*(uint32_t *)((unsigned long)ssdt + i) == 0x5f52505f) - *(uint32_t *)((unsigned long)ssdt + i) = 0x5f42535f; - } - /* Recalculate checksum */ - ssdt->checksum = 0; - ssdt->checksum = acpi_checksum((void *)ssdt, ssdt->length); -} - static unsigned long agesa_write_acpi_tables(const struct device *device, unsigned long current, acpi_rsdp_t *rsdp) @@ -296,7 +281,6 @@ printk(BIOS_DEBUG, "ACPI: * SSDT at %lx\n", current); ssdt = (acpi_header_t *)agesawrapper_getlateinitptr(PICK_PSTATE); if (ssdt != NULL) { - patch_ssdt_processor_scope(ssdt); memcpy((void *)current, ssdt, ssdt->length); ssdt = (acpi_header_t *)current; current += ssdt->length; -- To view, visit https://review.coreboot.org/c/coreboot/+/73385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If88820a0f5df923f129e2e3b5335f5f0e38ee7f5 Gerrit-Change-Number: 73385 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[master]: soc/intel/alderlake: Hook up ucode for RPL-S/HX B0
by Felix Singer (Code Review) 02 Mar '23

02 Mar '23

Felix Singer has submitted this change. ( https://review.coreboot.org/c/coreboot/+/72927 ) Change subject: soc/intel/alderlake: Hook up ucode for RPL-S/HX B0 ...................................................................... soc/intel/alderlake: Hook up ucode for RPL-S/HX B0 Hook up microcode from 3rdparty repo for: - 06-b7-01 (CPUID signature: 0xb0671) Verified microcode blob was in CBFS on Clevo PD50SNE (system76/serw13), which has an i9-13900HX. Change-Id: If91ff9233a5e1dd1db76edf33a76c55f5dddc9b4 Signed-off-by: Tim Crawford <tcrawford(a)system76.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/72927 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> --- M src/soc/intel/alderlake/Makefile.inc 1 file changed, 22 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved diff --git a/src/soc/intel/alderlake/Makefile.inc b/src/soc/intel/alderlake/Makefile.inc index 943b158..5c2b9dc 100644 --- a/src/soc/intel/alderlake/Makefile.inc +++ b/src/soc/intel/alderlake/Makefile.inc @@ -76,6 +76,8 @@ cpu_microcode_bins += 3rdparty/intel-microcode/intel-ucode/06-97-02 # ADL-S H0 cpu_microcode_bins += 3rdparty/intel-microcode/intel-ucode/06-97-05 +# RPL-S/HX B0 +cpu_microcode_bins += 3rdparty/intel-microcode/intel-ucode/06-b7-01 else ifneq ($(CONFIG_SOC_INTEL_ALDERLAKE_PCH_N),y) # 06-9a-00, 06-9a-01 are ADL-P/ADL-M Engineering Samples -- To view, visit https://review.coreboot.org/c/coreboot/+/72927 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If91ff9233a5e1dd1db76edf33a76c55f5dddc9b4 Gerrit-Change-Number: 72927 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jeremy Soller <jeremy(a)system76.com> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: soc/intel/alderlake: Hook up ucode for RPL-S/HX B0
by Felix Singer (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Tim Crawford, Tarun Tuli, Subrata Banik. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72927 ) Change subject: soc/intel/alderlake: Hook up ucode for RPL-S/HX B0 ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/72927 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If91ff9233a5e1dd1db76edf33a76c55f5dddc9b4 Gerrit-Change-Number: 72927 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Comment-Date: Thu, 02 Mar 2023 19:40:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/skyrim: Disable USE_SELECTIVE_GOP_INIT
by Karthik Ramasubramanian (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Martin L Roth, Matt DeVillier, Tim Van Patten, Felix Held. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73393 ) Change subject: mb/google/skyrim: Disable USE_SELECTIVE_GOP_INIT ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/73393 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id4aa441e4b4f76168f8243b6abafa1cf1ea08dbd Gerrit-Change-Number: 73393 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 02 Mar 2023 19:22:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/skyrim: Disable USE_SELECTIVE_GOP_INIT
by Martin L Roth (Code Review) 02 Mar '23

02 Mar '23

Martin L Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/73393 ) Change subject: mb/google/skyrim: Disable USE_SELECTIVE_GOP_INIT ...................................................................... mb/google/skyrim: Disable USE_SELECTIVE_GOP_INIT This is causing some issues, so disable it until those issues can be resolved. BUG=b:271437658, b:271199389, b:270077971 TEST=Screen always lights up on boot & after S0i3 Signed-off-by: Martin Roth <gaumless(a)gmail.com> Change-Id: Id4aa441e4b4f76168f8243b6abafa1cf1ea08dbd --- M src/mainboard/google/skyrim/Kconfig 1 file changed, 16 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/73393/1 diff --git a/src/mainboard/google/skyrim/Kconfig b/src/mainboard/google/skyrim/Kconfig index 22caf51..dfc8030 100644 --- a/src/mainboard/google/skyrim/Kconfig +++ b/src/mainboard/google/skyrim/Kconfig @@ -44,7 +44,6 @@ select SOC_AMD_MENDOCINO select SOC_AMD_COMMON_BLOCK_I2C3_TPM_SHARED_WITH_PSP select SOC_AMD_COMMON_BLOCK_USE_ESPI - select SOC_AMD_GFX_CACHE_VBIOS_IN_FMAP if CHROMEOS && RUN_FSP_GOP select SYSTEM_TYPE_LAPTOP select TPM_GOOGLE_TI50 select PCIEXP_ASPM @@ -118,9 +117,6 @@ default "crystaldrift" if BOARD_GOOGLE_CRYSTALDRIFT default "markarth" if BOARD_GOOGLE_MARKARTH -config USE_SELECTIVE_GOP_INIT - default y if CHROMEOS && RUN_FSP_GOP - config VBOOT select EC_GOOGLE_CHROMEEC_SWITCHES select VBOOT_LID_SWITCH -- To view, visit https://review.coreboot.org/c/coreboot/+/73393 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id4aa441e4b4f76168f8243b6abafa1cf1ea08dbd Gerrit-Change-Number: 73393 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-MessageType: newchange

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2023