coreboot-gerrit November 2022

coreboot-gerrit@coreboot.org

1 participants
7286 discussions

[M] Change in coreboot[master]: drivers/fsp2_0/mp_service_ppi: Use struct device to fill in buffer
by Arthur Heymans (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Andrey Petrov. Hello build bot (Jenkins), Andrey Petrov, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/69223 to look at the new patch set (#6). Change subject: drivers/fsp2_0/mp_service_ppi: Use struct device to fill in buffer ...................................................................... drivers/fsp2_0/mp_service_ppi: Use struct device to fill in buffer Now the CPU topology is filled in struct device during mp_init. Change-Id: I7322b43f5b95dda5fbe81e7427f5269c9d6f8755 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c M src/soc/intel/common/block/cpu/cpulib.c M src/soc/intel/common/block/include/intelblocks/cpulib.h 3 files changed, 19 insertions(+), 67 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/69223/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/69223 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7322b43f5b95dda5fbe81e7427f5269c9d6f8755 Gerrit-Change-Number: 69223 Gerrit-PatchSet: 6 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: cpu/mp_init.c: Only enable CPUs once they execute code
by Arthur Heymans (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Arthur Heymans. Hello build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/68892 to look at the new patch set (#10). Change subject: cpu/mp_init.c: Only enable CPUs once they execute code ...................................................................... cpu/mp_init.c: Only enable CPUs once they execute code On some systems the BSP cannot know how many CPUs are present in the system. A typical use case is a multi socket system. Setting the enable flag only on CPUs that actually exist makes it more flexible. Change-Id: I6c8042b4d6127239175924f996f735bf9c83c6e8 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/x86/mp_init.c 1 file changed, 16 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/68892/10 -- To view, visit https://review.coreboot.org/c/coreboot/+/68892 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6c8042b4d6127239175924f996f735bf9c83c6e8 Gerrit-Change-Number: 68892 Gerrit-PatchSet: 10 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: cpu/smm_module_loader.c: Fix up CPU index locally
by Arthur Heymans (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Arthur Heymans. Hello Arthur Heymans, build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/64802 to look at the new patch set (#14). Change subject: cpu/smm_module_loader.c: Fix up CPU index locally ...................................................................... cpu/smm_module_loader.c: Fix up CPU index locally Don't pass the stub params to the mp_init code. Change-Id: I070bc00ae5e5bceb6c5b90ea833cc057dd41f6cc Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/x86/mp_init.c M src/cpu/x86/smm/smm_module_loader.c M src/include/cpu/x86/smm.h 3 files changed, 28 insertions(+), 20 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/64802/14 -- To view, visit https://review.coreboot.org/c/coreboot/+/64802 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I070bc00ae5e5bceb6c5b90ea833cc057dd41f6cc Gerrit-Change-Number: 64802 Gerrit-PatchSet: 14 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: cpu/x86/mp_init.c: Keep track of initial lapic ID inside device_path
by Arthur Heymans (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Arthur Heymans, Angel Pons, Kyösti Mälkki. Hello build bot (Jenkins), Angel Pons, Elyes Haouas, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/64342 to look at the new patch set (#13). Change subject: cpu/x86/mp_init.c: Keep track of initial lapic ID inside device_path ...................................................................... cpu/x86/mp_init.c: Keep track of initial lapic ID inside device_path It's quite confusing to keep track of lapic ID inside the device struct and initial lapic ID inside an array. Change-Id: I4d9f8d23c0b0e5c142f6907593428d8509e4e7bb Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/arch/x86/cpu.c M src/cpu/x86/mp_init.c M src/device/cpu_device.c M src/include/cpu/cpu.h M src/include/device/path.h 5 files changed, 22 insertions(+), 31 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/64342/13 -- To view, visit https://review.coreboot.org/c/coreboot/+/64342 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d9f8d23c0b0e5c142f6907593428d8509e4e7bb Gerrit-Change-Number: 64342 Gerrit-PatchSet: 13 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: drivers/ocp: add VPD processing framework
by Jonathan Zhang (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Marc Jones, Johnny Lin, Paul Menzel, Johnny Li, Arthur Heymans, Shuming Chu (Shuming), Angel Pons, Tim Chu. Hello build bot (Jenkins), Marc Jones, Johnny Lin, Paul Menzel, Johnny Li, Angel Pons, Tim Chu, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/68784 to look at the new patch set (#3). Change subject: drivers/ocp: add VPD processing framework ...................................................................... drivers/ocp: add VPD processing framework Add VPD processing framework to be shared by OCP mainboards: * define VPD configruation items in vpd.h. * add helper functions: ** get_bool_from_vpd() ** get_int_from_vpd_range() Change-Id: I705bea348b1611f25ccbd798b77cfee22ec30f0f Signed-off-by: Johnny Lin <johnny_lin(a)wiwynn.com> Signed-off-by: Tim Chu <Tim.Chu(a)quantatw.com> Signed-off-by: Marc Jones <marcjones(a)sysproconsulting.com> Signed-off-by: Jonathan Zhang <jonzhang(a)meta.com> --- A src/drivers/ocp/include/vpd.h A src/drivers/ocp/vpd/Kconfig A src/drivers/ocp/vpd/Makefile.inc A src/drivers/ocp/vpd/vpd_util.c 4 files changed, 153 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/68784/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/68784 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I705bea348b1611f25ccbd798b77cfee22ec30f0f Gerrit-Change-Number: 68784 Gerrit-PatchSet: 3 Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com> Gerrit-Reviewer: Johnny Li <johnny_li(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Shuming Chu (Shuming) <s1218944(a)gmail.com> Gerrit-Attention: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Johnny Li <johnny_li(a)wistron.corp-partner.google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Shuming Chu (Shuming) <s1218944(a)gmail.com> Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: drivers/ocp: add VPD processing framework
by Jonathan Zhang (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Marc Jones, Johnny Lin, Paul Menzel, Johnny Li, Arthur Heymans, Shuming Chu (Shuming), Angel Pons, Tim Chu. Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68784 ) Change subject: drivers/ocp: add VPD processing framework ...................................................................... Patch Set 2: (2 comments) Patchset: PS2: Johnny, please check out this thread. File src/drivers/ocp/vpd/Makefile.inc: https://review.coreboot.org/c/coreboot/+/68784/comment/ef1b035e_f7f0d8bf PS2, Line 3: smm-$(C > It's not a good idea to fetch VPD in the smihandler. Do you really need this? […] We do need to customize SMI handler behavior based on firmware configuration (in this case, VPD), to configure things such as the SMI handler log level. We think that we need to fetch VPD at least once in the smihandler. Another option is to fetch it in the ramstage and save it somewhere for smi handlers to pick up. @Johnny, could you comment on this? CONFIG_VPD_SMM_SUPPORT is defined in our SPR production codebase, and we could use it for DeltaLake as well. For this patch itself, let me remove this statement, while we continue the discussion on CONFIG_VPD_SMM_SUPPORT. -- To view, visit https://review.coreboot.org/c/coreboot/+/68784 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I705bea348b1611f25ccbd798b77cfee22ec30f0f Gerrit-Change-Number: 68784 Gerrit-PatchSet: 2 Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com> Gerrit-Reviewer: Johnny Li <johnny_li(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Shuming Chu (Shuming) <s1218944(a)gmail.com> Gerrit-Attention: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Johnny Li <johnny_li(a)wistron.corp-partner.google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Shuming Chu (Shuming) <s1218944(a)gmail.com> Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Sat, 12 Nov 2022 18:55:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/amd/picasso: Add support for 64bit builds
by Fred Reitberger (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Matt DeVillier, Arthur Heymans. Fred Reitberger has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63727 ) Change subject: soc/amd/picasso: Add support for 64bit builds ...................................................................... Patch Set 8: (1 comment) File src/soc/amd/picasso/Makefile.inc: https://review.coreboot.org/c/coreboot/+/63727/comment/e47eaca3_44a382fc PS8, Line 146: ifeq ($(CONFIG_ARCH_BOOTBLOCK_X86_64),y) : PSP_BIOSBIN_SIZE=$(shell $(READELF_bootblock) -l $(PSP_ELF_FILE) | grep -A1 LOAD | tr -d '\n' | awk '{print $$5}') : else : PSP_BIOSBIN_SIZE=$(shell $(READELF_bootblock) -l $(PSP_ELF_FILE) | grep LOAD | awk '{print $$5}') : endif If you pass the wide option to readelf (-lW instead of -l), it will still be on a single line for the grep/awk commands. Should work for both 32-bit and 64-bit cases. -- To view, visit https://review.coreboot.org/c/coreboot/+/63727 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icd72c4eef7805aacba6378632cbac7de9527673b Gerrit-Change-Number: 63727 Gerrit-PatchSet: 8 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Sat, 12 Nov 2022 18:12:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: treewide: Replace ALIGN(x, a) by ALIGN_UP(x, a) for clarity
by Felix Held (Code Review) 12 Nov '22

12 Nov '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/68267 ) Change subject: treewide: Replace ALIGN(x, a) by ALIGN_UP(x, a) for clarity ...................................................................... treewide: Replace ALIGN(x, a) by ALIGN_UP(x, a) for clarity Change-Id: I2a255cdcbcd38406f008a26fc0ed68d532e7a721 Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/68267 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> --- M src/soc/amd/morgana/agesa_acpi.c M src/vendorcode/google/chromeos/vpd_calibration.c M util/cbfstool/cbfs-payload-linux.c M util/cbfstool/elfheaders.c M util/cbfstool/ifwitool.c 5 files changed, 27 insertions(+), 12 deletions(-) Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved diff --git a/src/soc/amd/morgana/agesa_acpi.c b/src/soc/amd/morgana/agesa_acpi.c index 78c0e4c..8838d9d 100644 --- a/src/soc/amd/morgana/agesa_acpi.c +++ b/src/soc/amd/morgana/agesa_acpi.c @@ -4,6 +4,7 @@ #include <acpi/acpi.h> #include <amdblocks/acpi.h> +#include <commonlib/bsd/helpers.h> #include <device/device.h> #include <FspGuids.h> #include <soc/acpi.h> @@ -18,7 +19,7 @@ current = add_agesa_fsp_acpi_table(AMD_FSP_ACPI_ALIB_HOB_GUID, "ALIB", rsdp, current); /* IVRS */ - current = ALIGN(current, 8); + current = ALIGN_UP(current, 8); ivrs = (acpi_ivrs_t *)current; acpi_create_ivrs(ivrs, acpi_fill_ivrs); current += ivrs->header.length; diff --git a/src/vendorcode/google/chromeos/vpd_calibration.c b/src/vendorcode/google/chromeos/vpd_calibration.c index 460d7dc..e5f5b57 100644 --- a/src/vendorcode/google/chromeos/vpd_calibration.c +++ b/src/vendorcode/google/chromeos/vpd_calibration.c @@ -126,7 +126,7 @@ cache->key_size = key_length; cache->value_size = decoded_size; cache->blob_size = - ALIGN(sizeof(struct calibration_blob) + + ALIGN_UP(sizeof(struct calibration_blob) + cache->key_size + cache->value_size, 4); cbmem_entry_size += cache->blob_size; diff --git a/util/cbfstool/cbfs-payload-linux.c b/util/cbfstool/cbfs-payload-linux.c index 9a29e68..f9d0d11 100644 --- a/util/cbfstool/cbfs-payload-linux.c +++ b/util/cbfstool/cbfs-payload-linux.c @@ -253,7 +253,7 @@ /* 16 MB should be way outside coreboot's playground, * so if possible (relocatable kernel) use that to * avoid a trampoline copy. */ - kernel_base = ALIGN(16*1024*1024, params.kernel_alignment); + kernel_base = ALIGN_UP(16*1024*1024, params.kernel_alignment); if (hdr->init_size == 0) { ERROR("init_size 0 for relocatable kernel\n"); return -1; @@ -275,7 +275,7 @@ * close to the kernel, so give it some room. */ initrd_base = kernel_base + buffer_size(&bzp.kernel); - initrd_base = ALIGN(initrd_base, 64*1024*1024); + initrd_base = ALIGN_UP(initrd_base, 64*1024*1024); params.initrd_start = initrd_base; params.initrd_size = buffer_size(&bzp.initrd); diff --git a/util/cbfstool/elfheaders.c b/util/cbfstool/elfheaders.c index 3a53735..39faff2 100644 --- a/util/cbfstool/elfheaders.c +++ b/util/cbfstool/elfheaders.c @@ -1174,7 +1174,7 @@ metadata_size += (Elf64_Xword)ew->ehdr.e_phnum * ew->ehdr.e_phentsize; shstroffset = metadata_size; /* Align up section header string size and metadata size to 4KiB */ - metadata_size = ALIGN(metadata_size + shstrlen, 4096); + metadata_size = ALIGN_UP(metadata_size + shstrlen, 4096); if (buffer_create(out, metadata_size + program_size, "elfout")) { ERROR("Could not create output buffer for ELF.\n"); diff --git a/util/cbfstool/ifwitool.c b/util/cbfstool/ifwitool.c index 8eda4c7..efcb594 100644 --- a/util/cbfstool/ifwitool.c +++ b/util/cbfstool/ifwitool.c @@ -1,6 +1,7 @@ /* ifwitool, CLI utility for IFWI manipulation */ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <commonlib/bsd/helpers.h> #include <commonlib/endian.h> #include <getopt.h> #include <stdlib.h> @@ -909,7 +910,7 @@ * part of the image. * ASSUMPTION: IFWI image always ends on a 4K boundary. */ - ifwi_image.input_ifwi_end_offset = ALIGN(end_offset, 4 * KiB); + ifwi_image.input_ifwi_end_offset = ALIGN_UP(end_offset, 4 * KiB); DEBUG("Parsing done.\n"); return 0; @@ -979,7 +980,7 @@ __bpdt_reset(&ifwi_image.bpdt, bpdt_count, bpdt_size); /* Update S-BPDT if required. */ - bpdt_size = ALIGN(BPDT_HEADER_SIZE + sbpdt_count * BPDT_ENTRY_SIZE, + bpdt_size = ALIGN_UP(BPDT_HEADER_SIZE + sbpdt_count * BPDT_ENTRY_SIZE, 4 * KiB); __bpdt_reset(&ifwi_image.subpart_buf[S_BPDT_TYPE], sbpdt_count, bpdt_size); @@ -1091,7 +1092,7 @@ } /* Pad ff bytes if there is any empty space left in BPDT 4K. */ - curr_end = ALIGN(curr_offset, 4 * KiB); + curr_end = ALIGN_UP(curr_offset, 4 * KiB); pad_buffer(last_bpdt_buff, buffer_size(last_bpdt_buff) + (curr_end - curr_offset)); curr_offset = curr_end; @@ -1107,9 +1108,9 @@ if (subparts[type].attr & LIES_WITHIN_BPDT_4K) continue; - assert(curr_offset == ALIGN(curr_offset, 4 * KiB)); + assert(curr_offset == ALIGN_UP(curr_offset, 4 * KiB)); curr->offset = curr_offset; - curr_end = ALIGN(curr->offset + curr->size, 4 * KiB); + curr_end = ALIGN_UP(curr->offset + curr->size, 4 * KiB); curr->size = curr_end - curr->offset; pad_buffer(&ifwi_image.subpart_buf[type], curr->size); @@ -1130,7 +1131,7 @@ curr = find_entry_by_type(S_BPDT_TYPE); assert(curr); - assert(curr_offset == ALIGN(curr_offset, 4 * KiB)); + assert(curr_offset == ALIGN_UP(curr_offset, 4 * KiB)); curr->size = curr_offset - curr->offset; } @@ -1190,7 +1191,7 @@ size_t ifwi_start, ifwi_end, file_end; ifwi_start = ifwi_image.input_ifwi_start_offset; - ifwi_end = ifwi_start + ALIGN(s->offset + s->size, 4 * KiB); + ifwi_end = ifwi_start + ALIGN_UP(s->offset + s->size, 4 * KiB); file_end = ifwi_end + (buffer_size(&ifwi_image.input_buff) - ifwi_image.input_ifwi_end_offset); -- To view, visit https://review.coreboot.org/c/coreboot/+/68267 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2a255cdcbcd38406f008a26fc0ed68d532e7a721 Gerrit-Change-Number: 68267 Gerrit-PatchSet: 4 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: treewide: Replace ALIGN(x, a) by ALIGN_UP(x, a) for clarity
by Felix Held (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Vadim Bendebury, Julius Werner, Aaron Durbin, Elyes Haouas. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68267 ) Change subject: treewide: Replace ALIGN(x, a) by ALIGN_UP(x, a) for clarity ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/68267 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2a255cdcbcd38406f008a26fc0ed68d532e7a721 Gerrit-Change-Number: 68267 Gerrit-PatchSet: 3 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Sat, 12 Nov 2022 18:00:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: util/amdfwtool/amdfwtool: Don't rewrite macros
by Felix Held (Code Review) 12 Nov '22

12 Nov '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/68376 ) Change subject: util/amdfwtool/amdfwtool: Don't rewrite macros ...................................................................... util/amdfwtool/amdfwtool: Don't rewrite macros Change-Id: Iea9dc65584c751e4d02524582b744ec9732e2c04 Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/68376 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> --- M util/amdfwtool/amdfwtool.c M util/amdfwtool/amdfwtool.h 2 files changed, 41 insertions(+), 29 deletions(-) Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 07749bd..e28d9c4 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -46,6 +46,7 @@ * */ +#include <commonlib/bsd/helpers.h> #include <fcntl.h> #include <errno.h> #include <openssl/sha.h> @@ -65,7 +66,6 @@ #define AMD_ROMSIG_OFFSET 0x20000 #define MIN_ROM_KB 256 -#define ALIGN(val, by) (((val) + (by) - 1) & ~((by) - 1)) #define _MAX(A, B) (((A) > (B)) ? (A) : (B)) #define ERASE_ALIGNMENT 0x1000U #define TABLE_ALIGNMENT 0x1000U @@ -443,9 +443,9 @@ * if secondary is reprogrammed. */ if (multi) - ctx->current = ALIGN(ctx->current, TABLE_ERASE_ALIGNMENT); + ctx->current = ALIGN_UP(ctx->current, TABLE_ERASE_ALIGNMENT); else - ctx->current = ALIGN(ctx->current, TABLE_ALIGNMENT); + ctx->current = ALIGN_UP(ctx->current, TABLE_ALIGNMENT); ptr = BUFF_CURRENT(*ctx); ((psp_directory_header *)ptr)->num_entries = 0; @@ -459,7 +459,7 @@ static void *new_ish_dir(context *ctx) { void *ptr; - ctx->current = ALIGN(ctx->current, TABLE_ALIGNMENT); + ctx->current = ALIGN_UP(ctx->current, TABLE_ALIGNMENT); ptr = BUFF_CURRENT(*ctx); ctx->current += TABLE_ALIGNMENT; return ptr; @@ -469,7 +469,7 @@ { void *ptr; - ctx->current = ALIGN(ctx->current, TABLE_ALIGNMENT); + ctx->current = ALIGN_UP(ctx->current, TABLE_ALIGNMENT); ptr = BUFF_CURRENT(*ctx); ctx->current += sizeof(psp_combo_header) + MAX_COMBO_ENTRIES * sizeof(psp_combo_entry); @@ -491,7 +491,7 @@ } /* The table size needs to be 0x1000 aligned. So align the end of table. */ - ctx->current = ALIGN(ctx->current, TABLE_ALIGNMENT); + ctx->current = ALIGN_UP(ctx->current, TABLE_ALIGNMENT); switch (cookie) { case PSP2_COOKIE: @@ -782,13 +782,13 @@ uint32_t i; ctx->current += sizeof(embedded_firmware); - ctx->current = ALIGN(ctx->current, BLOB_ALIGNMENT); + ctx->current = ALIGN_UP(ctx->current, BLOB_ALIGNMENT); for (i = 0; fw_table[i].type != AMD_FW_INVALID; i++) { if (fw_table[i].filename != NULL) { switch (fw_table[i].type) { case AMD_FW_IMC: - ctx->current = ALIGN(ctx->current, 0x10000U); + ctx->current = ALIGN_UP(ctx->current, 0x10000U); romsig->imc_entry = RUN_CURRENT(*ctx); break; case AMD_FW_GEC: @@ -809,7 +809,7 @@ exit(1); } - ctx->current = ALIGN(ctx->current + bytes, + ctx->current = ALIGN_UP(ctx->current + bytes, BLOB_ALIGNMENT); } } @@ -1194,7 +1194,7 @@ } current_table_save = ctx->current_table; ctx->current_table = (char *)pspdir - ctx->rom; - ctx->current = ALIGN(ctx->current, TABLE_ALIGNMENT); + ctx->current = ALIGN_UP(ctx->current, TABLE_ALIGNMENT); for (i = 0, count = 0; fw_table[i].type != AMD_FW_INVALID; i++) { if (!(fw_table[i].level & level)) @@ -1205,14 +1205,14 @@ if (fw_table[i].type == AMD_TOKEN_UNLOCK) { if (!fw_table[i].other) continue; - ctx->current = ALIGN(ctx->current, ERASE_ALIGNMENT); + ctx->current = ALIGN_UP(ctx->current, ERASE_ALIGNMENT); pspdir->entries[count].type = fw_table[i].type; pspdir->entries[count].size = 4096; /* TODO: doc? */ pspdir->entries[count].addr = RUN_CURRENT(*ctx); pspdir->entries[count].address_mode = SET_ADDR_MODE_BY_TABLE(pspdir); pspdir->entries[count].subprog = fw_table[i].subprog; pspdir->entries[count].rsvd = 0; - ctx->current = ALIGN(ctx->current + 4096, 0x100U); + ctx->current = ALIGN_UP(ctx->current + 4096, 0x100U); count++; } else if (fw_table[i].type == AMD_PSP_FUSE_CHAIN) { pspdir->entries[count].type = fw_table[i].type; @@ -1228,13 +1228,13 @@ continue; size = fw_table[i].size; addr = fw_table[i].dest; - if (addr != ALIGN(addr, ERASE_ALIGNMENT)) { + if (addr != ALIGN_UP(addr, ERASE_ALIGNMENT)) { fprintf(stderr, "Error: PSP NVRAM section not aligned with erase block size.\n\n"); exit(1); } } else { - ctx->current = ALIGN(ctx->current, ERASE_ALIGNMENT); + ctx->current = ALIGN_UP(ctx->current, ERASE_ALIGNMENT); bytes = copy_blob(BUFF_CURRENT(*ctx), fw_table[i].filename, BUFF_ROOM(*ctx)); if (bytes <= 0) { @@ -1242,9 +1242,9 @@ exit(1); } - size = ALIGN(bytes, ERASE_ALIGNMENT); + size = ALIGN_UP(bytes, ERASE_ALIGNMENT); addr = RUN_CURRENT(*ctx); - ctx->current = ALIGN(ctx->current + bytes, + ctx->current = ALIGN_UP(ctx->current + bytes, BLOB_ERASE_ALIGNMENT); } @@ -1275,7 +1275,7 @@ pspdir->entries[count].addr = RUN_CURRENT(*ctx); pspdir->entries[count].address_mode = SET_ADDR_MODE_BY_TABLE(pspdir); - ctx->current = ALIGN(ctx->current + bytes, + ctx->current = ALIGN_UP(ctx->current + bytes, BLOB_ALIGNMENT); } @@ -1373,9 +1373,9 @@ * if secondary is reprogrammed. */ if (multi) - ctx->current = ALIGN(ctx->current, TABLE_ERASE_ALIGNMENT); + ctx->current = ALIGN_UP(ctx->current, TABLE_ERASE_ALIGNMENT); else - ctx->current = ALIGN(ctx->current, TABLE_ALIGNMENT); + ctx->current = ALIGN_UP(ctx->current, TABLE_ALIGNMENT); ptr = BUFF_CURRENT(*ctx); ((bios_directory_hdr *) ptr)->additional_info = 0; ((bios_directory_hdr *) ptr)->additional_info_fields.address_mode = ctx->address_mode; @@ -1457,7 +1457,7 @@ else level = BDT_BOTH; - ctx->current = ALIGN(ctx->current, TABLE_ALIGNMENT); + ctx->current = ALIGN_UP(ctx->current, TABLE_ALIGNMENT); for (i = 0, count = 0; fw_table[i].type != AMD_BIOS_INVALID; i++) { if (!(fw_table[i].level & level)) @@ -1541,7 +1541,7 @@ SET_ADDR_MODE_BY_TABLE(biosdir); memset(BUFF_CURRENT(*ctx), 0xff, biosdir->entries[count].size); - ctx->current = ALIGN(ctx->current + ctx->current = ALIGN_UP(ctx->current + biosdir->entries[count].size, 0x100U); break; case AMD_BIOS_APOB: @@ -1558,11 +1558,11 @@ biosdir->entries[count].size = fw_table[i].size; } else { /* Else reserve size bytes within amdfw.rom */ - ctx->current = ALIGN(ctx->current, ERASE_ALIGNMENT); + ctx->current = ALIGN_UP(ctx->current, ERASE_ALIGNMENT); biosdir->entries[count].source = RUN_CURRENT(*ctx); biosdir->entries[count].address_mode = SET_ADDR_MODE(biosdir, AMD_ADDR_REL_BIOS); - biosdir->entries[count].size = ALIGN( + biosdir->entries[count].size = ALIGN_UP( fw_table[i].size, ERASE_ALIGNMENT); memset(BUFF_CURRENT(*ctx), 0xff, biosdir->entries[count].size); @@ -1602,7 +1602,7 @@ biosdir->entries[count].address_mode = SET_ADDR_MODE(biosdir, AMD_ADDR_REL_BIOS); - ctx->current = ALIGN(ctx->current + bytes, 0x100U); + ctx->current = ALIGN_UP(ctx->current + bytes, 0x100U); break; case AMD_BIOS_PSP_SHARED_MEM: biosdir->entries[count].dest = fw_table[i].dest; @@ -1612,7 +1612,7 @@ default: /* everything else is copied from input */ if (fw_table[i].type == AMD_BIOS_APCB || fw_table[i].type == AMD_BIOS_APCB_BK) - ctx->current = ALIGN( + ctx->current = ALIGN_UP( ctx->current, ERASE_ALIGNMENT); bytes = copy_blob(BUFF_CURRENT(*ctx), fw_table[i].filename, BUFF_ROOM(*ctx)); @@ -1625,7 +1625,7 @@ biosdir->entries[count].source = RUN_CURRENT(*ctx); biosdir->entries[count].address_mode = SET_ADDR_MODE_BY_TABLE(biosdir); - ctx->current = ALIGN(ctx->current + bytes, 0x100U); + ctx->current = ALIGN_UP(ctx->current + bytes, 0x100U); break; } @@ -2382,7 +2382,7 @@ integrate_firmwares(&ctx, amd_romsig, amd_fw_table); - ctx.current = ALIGN(ctx.current, 0x10000U); /* TODO: is it necessary? */ + ctx.current = ALIGN_UP(ctx.current, 0x10000U); /* TODO: is it necessary? */ ctx.current_table = 0; /* If the tool is invoked with command-line options to keep the signed PSP diff --git a/util/amdfwtool/amdfwtool.h b/util/amdfwtool/amdfwtool.h index cd36085..3afa80e 100644 --- a/util/amdfwtool/amdfwtool.h +++ b/util/amdfwtool/amdfwtool.h @@ -4,6 +4,7 @@ #define _AMD_FW_TOOL_H_ #include <commonlib/bsd/compiler.h> +#include <commonlib/bsd/helpers.h> #include <openssl/sha.h> #include <stdint.h> #include <stdbool.h> @@ -373,6 +374,4 @@ #define LINE_EOF (1) #define LINE_TOO_LONG (2) -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) - #endif /* _AMD_FW_TOOL_H_ */ -- To view, visit https://review.coreboot.org/c/coreboot/+/68376 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iea9dc65584c751e4d02524582b744ec9732e2c04 Gerrit-Change-Number: 68376 Gerrit-PatchSet: 2 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> 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