coreboot-gerrit November 2022

coreboot-gerrit@coreboot.org

1 participants
7286 discussions

[S] Change in coreboot[master]: ec/starlabs/merlin: Rename the Cezanne EC code
by Martin Roth (Code Review) 11 Nov '22

11 Nov '22

Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/68319 ) ( 6 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: ec/starlabs/merlin: Rename the Cezanne EC code ...................................................................... ec/starlabs/merlin: Rename the Cezanne EC code This EC code is for the Byte, a Cezanne Mini PC. The EC is different to the Cezanne StarBook Mk VI. Rename it to `-desktop`, so the laptop variant becomes the primary. Signed-off-by: Sean Rhodes <sean(a)starlabs.systems> Change-Id: I25f812cb1c6cefca1ebbe3bee5d20cf521dd60af Reviewed-on: https://review.coreboot.org/c/coreboot/+/68319 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- R src/ec/starlabs/merlin/variants/cezanne-desktop/ecdefs.h R src/ec/starlabs/merlin/variants/cezanne-desktop/emem.asl R src/ec/starlabs/merlin/variants/cezanne-desktop/events.asl 3 files changed, 19 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/src/ec/starlabs/merlin/variants/cezanne/ecdefs.h b/src/ec/starlabs/merlin/variants/cezanne-desktop/ecdefs.h similarity index 85% rename from src/ec/starlabs/merlin/variants/cezanne/ecdefs.h rename to src/ec/starlabs/merlin/variants/cezanne-desktop/ecdefs.h index 0826456..a6ec9fe 100644 --- a/src/ec/starlabs/merlin/variants/cezanne/ecdefs.h +++ b/src/ec/starlabs/merlin/variants/cezanne-desktop/ecdefs.h @@ -7,8 +7,8 @@ * EC communication interface for ITE Embedded Controller */ -#ifndef _EC_STARLABS_CEZANNE_EC_DEFS_H -#define _EC_STARLABS_CEZANNE_EC_DEFS_H +#ifndef _EC_STARLABS_CEZANNE_DESKTOP_EC_DEFS_H +#define _EC_STARLABS_CEZANNE_DESKTOP_EC_DEFS_H /* IT5570 chip ID byte values */ #define ITE_CHIPID_VAL 0x5570 diff --git a/src/ec/starlabs/merlin/variants/cezanne/emem.asl b/src/ec/starlabs/merlin/variants/cezanne-desktop/emem.asl similarity index 100% rename from src/ec/starlabs/merlin/variants/cezanne/emem.asl rename to src/ec/starlabs/merlin/variants/cezanne-desktop/emem.asl diff --git a/src/ec/starlabs/merlin/variants/cezanne/events.asl b/src/ec/starlabs/merlin/variants/cezanne-desktop/events.asl similarity index 100% rename from src/ec/starlabs/merlin/variants/cezanne/events.asl rename to src/ec/starlabs/merlin/variants/cezanne-desktop/events.asl -- To view, visit https://review.coreboot.org/c/coreboot/+/68319 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I25f812cb1c6cefca1ebbe3bee5d20cf521dd60af Gerrit-Change-Number: 68319 Gerrit-PatchSet: 9 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: arch/x86/memmove: Add 64bit version
by Angel Pons (Code Review) 11 Nov '22

11 Nov '22

Attention is currently required from: Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69231 ) Change subject: arch/x86/memmove: Add 64bit version ...................................................................... Patch Set 12: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/69231 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib43ec19df97194d6b1c18bfacb5fe8211ba0ffe5 Gerrit-Change-Number: 69231 Gerrit-PatchSet: 12 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-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 11 Nov 2022 18:20:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: include/cpu/msr.h: transform into an union
by Angel Pons (Code Review) 11 Nov '22

11 Nov '22

Attention is currently required from: Martin Roth, Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68572 ) Change subject: include/cpu/msr.h: transform into an union ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/68572 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1b026cd3807fd68d805051a74b3d31fcde1c5626 Gerrit-Change-Number: 68572 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 11 Nov 2022 18:19:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/amd/morgana/Kconfig: Remove TODO after review
by Fred Reitberger (Code Review) 11 Nov '22

11 Nov '22

Attention is currently required from: Matt DeVillier, Felix Held. Fred Reitberger has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69275 ) Change subject: soc/amd/morgana/Kconfig: Remove TODO after review ...................................................................... Patch Set 3: (1 comment) File src/soc/amd/morgana/Kconfig: https://review.coreboot.org/c/coreboot/+/69275/comment/7a31c451_d7021573 PS2, Line 72: select SOC_AMD_COMMON_BLOCK_PCIE_CLK_REQ # TODO: Check if this is still correct > i'd prefer to select this new kconfig option in a separate patch. […] I noticed it in my review of Kconfig options and wanted to mark it for a follow-up somewhere. Removed for now. -- To view, visit https://review.coreboot.org/c/coreboot/+/69275 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I598daf40a774ec81a956ce8c1aeb1cbbf4b475f3 Gerrit-Change-Number: 69275 Gerrit-PatchSet: 3 Gerrit-Owner: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 11 Nov 2022 18:19:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/amd/morgana/Kconfig: Remove TODO after review
by Fred Reitberger (Code Review) 11 Nov '22

11 Nov '22

Attention is currently required from: Matt DeVillier, Fred Reitberger. Hello Jason Glenesk, build bot (Jenkins), Matt DeVillier, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/69275 to look at the new patch set (#3). Change subject: soc/amd/morgana/Kconfig: Remove TODO after review ...................................................................... soc/amd/morgana/Kconfig: Remove TODO after review Remove TODO comments after reviwing against morgana ppr #57396, rev 1.52 Signed-off-by: Fred Reitberger <reitbergerfred(a)gmail.com> Change-Id: I598daf40a774ec81a956ce8c1aeb1cbbf4b475f3 --- M src/soc/amd/morgana/Kconfig M src/soc/amd/morgana/uart.c 2 files changed, 31 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/69275/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/69275 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I598daf40a774ec81a956ce8c1aeb1cbbf4b475f3 Gerrit-Change-Number: 69275 Gerrit-PatchSet: 3 Gerrit-Owner: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: drivers/intel/fsp2_0/hand_off_block: remove unneeded line breaks
by Felix Held (Code Review) 11 Nov '22

11 Nov '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69461 ) Change subject: drivers/intel/fsp2_0/hand_off_block: remove unneeded line breaks ...................................................................... drivers/intel/fsp2_0/hand_off_block: remove unneeded line breaks Since the characters per line limit was increased from 80 to 96, some line breaks can be removed to improve code readability. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I92aa3fec8c8caba143e418efc999ec4a7c5d93c4 Reviewed-on: https://review.coreboot.org/c/coreboot/+/69461 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> --- M src/drivers/intel/fsp2_0/hand_off_block.c 1 file changed, 25 insertions(+), 16 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved Eric Lai: Looks good to me, approved diff --git a/src/drivers/intel/fsp2_0/hand_off_block.c b/src/drivers/intel/fsp2_0/hand_off_block.c index a40da77..21ce03e 100644 --- a/src/drivers/intel/fsp2_0/hand_off_block.c +++ b/src/drivers/intel/fsp2_0/hand_off_block.c @@ -132,8 +132,7 @@ { const struct hob_resource *res; - for (; hob->type != HOB_TYPE_END_OF_HOB_LIST; - hob = fsp_next_hob(hob)) { + for (; hob->type != HOB_TYPE_END_OF_HOB_LIST; hob = fsp_next_hob(hob)) { if (hob->type != HOB_TYPE_RESOURCE_DESCRIPTOR) continue; @@ -195,8 +194,7 @@ if (!hob) return NULL; - for (; hob->type != HOB_TYPE_END_OF_HOB_LIST; - hob = fsp_next_hob(hob)) { + for (; hob->type != HOB_TYPE_END_OF_HOB_LIST; hob = fsp_next_hob(hob)) { if (hob->type != HOB_TYPE_GUID_EXTENSION) continue; @@ -219,12 +217,10 @@ uint8_t vs; #if CONFIG(DISPLAY_FSP_VERSION_INFO) const FIRMWARE_VERSION_INFO *fvi; - const FIRMWARE_VERSION_INFO_HOB *fvih = - (FIRMWARE_VERSION_INFO_HOB *)hob; + const FIRMWARE_VERSION_INFO_HOB *fvih = (FIRMWARE_VERSION_INFO_HOB *)hob; fvi = (void *)&fvih[1]; - str_ptr = (char *)((uintptr_t)fvi + - (fvih->Count * sizeof(FIRMWARE_VERSION_INFO))); + str_ptr = (char *)((uintptr_t)fvi + (fvih->Count * sizeof(FIRMWARE_VERSION_INFO))); tcount = fvih->Count; #elif CONFIG(DISPLAY_FSP_VERSION_INFO_2) @@ -255,8 +251,7 @@ fvi[index].Version.Revision == 0xFF && fvi[index].Version.BuildNumber == 0xFF && vs == 0) { - str_ptr = (char *)((uintptr_t)str_ptr + cnt + - sizeof(uint8_t)); + str_ptr = (char *)((uintptr_t)str_ptr + cnt + sizeof(uint8_t)); continue; } /* @@ -274,13 +269,11 @@ fvi[index].Version.Revision, fvi[index].Version.BuildNumber); else { - str_ptr = (char *)((uintptr_t)str_ptr + cnt + - sizeof(uint8_t)); + str_ptr = (char *)((uintptr_t)str_ptr + cnt + sizeof(uint8_t)); cnt = strlen(str_ptr); printk(BIOS_DEBUG, "%s\n", str_ptr); } - str_ptr = (char *)((uintptr_t)str_ptr + cnt + - sizeof(uint8_t)); + str_ptr = (char *)((uintptr_t)str_ptr + cnt + sizeof(uint8_t)); } #endif } @@ -294,8 +287,7 @@ return; printk(BIOS_DEBUG, "Display FSP Version Info HOB\n"); - for (; hob->type != HOB_TYPE_END_OF_HOB_LIST; - hob = fsp_next_hob(hob)) { + for (; hob->type != HOB_TYPE_END_OF_HOB_LIST; hob = fsp_next_hob(hob)) { if (hob->type != HOB_TYPE_GUID_EXTENSION) continue; -- To view, visit https://review.coreboot.org/c/coreboot/+/69461 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I92aa3fec8c8caba143e418efc999ec4a7c5d93c4 Gerrit-Change-Number: 69461 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: soc/amd/root_complex: don't skip reporting IOAPIC resource in !hob case
by Felix Held (Code Review) 11 Nov '22

11 Nov '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69460 ) Change subject: soc/amd/root_complex: don't skip reporting IOAPIC resource in !hob case ...................................................................... soc/amd/root_complex: don't skip reporting IOAPIC resource in !hob case When no HOB list is found, not only adding the resources reported by the FSP were skipped, but also adding the GNB IOAPIC resource was skipped. Fix this bug by moving the reporting of the GNB IOAPIC resource before the resources reported in the FSP HOBs to not skip the IOAPIC resource when there's no HOB list. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I9174c8d7e5e94144187d27210e12f2dca3a6010f Reviewed-on: https://review.coreboot.org/c/coreboot/+/69460 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/cezanne/root_complex.c M src/soc/amd/glinda/root_complex.c M src/soc/amd/mendocino/root_complex.c M src/soc/amd/morgana/root_complex.c M src/soc/amd/picasso/root_complex.c 5 files changed, 49 insertions(+), 30 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/cezanne/root_complex.c b/src/soc/amd/cezanne/root_complex.c index ed919b1..5f2024c 100644 --- a/src/soc/amd/cezanne/root_complex.c +++ b/src/soc/amd/cezanne/root_complex.c @@ -142,6 +142,12 @@ mmconf_resource(dev, idx++); + /* GNB IOAPIC resource */ + gnb_apic = new_resource(dev, idx++); + gnb_apic->base = GNB_IO_APIC_ADDR; + gnb_apic->size = 0x00001000; + gnb_apic->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; + if (!hob) { printk(BIOS_ERR, "%s incomplete because no HOB list was found\n", __func__); @@ -168,12 +174,6 @@ printk(BIOS_ERR, "failed to set resources for type %d\n", res->type); } - - /* GNB IOAPIC resource */ - gnb_apic = new_resource(dev, idx++); - gnb_apic->base = GNB_IO_APIC_ADDR; - gnb_apic->size = 0x00001000; - gnb_apic->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; } static void root_complex_init(struct device *dev) diff --git a/src/soc/amd/glinda/root_complex.c b/src/soc/amd/glinda/root_complex.c index 2a4a547..16f1bdd 100644 --- a/src/soc/amd/glinda/root_complex.c +++ b/src/soc/amd/glinda/root_complex.c @@ -157,6 +157,12 @@ mmconf_resource(dev, idx++); + /* GNB IOAPIC resource */ + gnb_apic = new_resource(dev, idx++); + gnb_apic->base = GNB_IO_APIC_ADDR; + gnb_apic->size = 0x00001000; + gnb_apic->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; + if (!hob) { printk(BIOS_ERR, "%s incomplete because no HOB list was found\n", __func__); @@ -183,12 +189,6 @@ printk(BIOS_ERR, "Failed to set resources for type %d\n", res->type); } - - /* GNB IOAPIC resource */ - gnb_apic = new_resource(dev, idx++); - gnb_apic->base = GNB_IO_APIC_ADDR; - gnb_apic->size = 0x00001000; - gnb_apic->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; } static void root_complex_init(struct device *dev) diff --git a/src/soc/amd/mendocino/root_complex.c b/src/soc/amd/mendocino/root_complex.c index 7feb7f7..d728516 100644 --- a/src/soc/amd/mendocino/root_complex.c +++ b/src/soc/amd/mendocino/root_complex.c @@ -157,6 +157,12 @@ mmconf_resource(dev, idx++); + /* GNB IOAPIC resource */ + gnb_apic = new_resource(dev, idx++); + gnb_apic->base = GNB_IO_APIC_ADDR; + gnb_apic->size = 0x00001000; + gnb_apic->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; + if (!hob) { printk(BIOS_ERR, "%s incomplete because no HOB list was found\n", __func__); @@ -183,12 +189,6 @@ printk(BIOS_ERR, "Failed to set resources for type %d\n", res->type); } - - /* GNB IOAPIC resource */ - gnb_apic = new_resource(dev, idx++); - gnb_apic->base = GNB_IO_APIC_ADDR; - gnb_apic->size = 0x00001000; - gnb_apic->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; } static void root_complex_init(struct device *dev) diff --git a/src/soc/amd/morgana/root_complex.c b/src/soc/amd/morgana/root_complex.c index 1a4f0bd..0e6e730 100644 --- a/src/soc/amd/morgana/root_complex.c +++ b/src/soc/amd/morgana/root_complex.c @@ -157,6 +157,12 @@ mmconf_resource(dev, idx++); + /* GNB IOAPIC resource */ + gnb_apic = new_resource(dev, idx++); + gnb_apic->base = GNB_IO_APIC_ADDR; + gnb_apic->size = 0x00001000; + gnb_apic->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; + if (!hob) { printk(BIOS_ERR, "%s incomplete because no HOB list was found\n", __func__); @@ -183,12 +189,6 @@ printk(BIOS_ERR, "Failed to set resources for type %d\n", res->type); } - - /* GNB IOAPIC resource */ - gnb_apic = new_resource(dev, idx++); - gnb_apic->base = GNB_IO_APIC_ADDR; - gnb_apic->size = 0x00001000; - gnb_apic->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; } static void root_complex_init(struct device *dev) diff --git a/src/soc/amd/picasso/root_complex.c b/src/soc/amd/picasso/root_complex.c index d53643e..4892023 100644 --- a/src/soc/amd/picasso/root_complex.c +++ b/src/soc/amd/picasso/root_complex.c @@ -140,6 +140,12 @@ mmconf_resource(dev, idx++); + /* GNB IOAPIC resource */ + gnb_apic = new_resource(dev, idx++); + gnb_apic->base = GNB_IO_APIC_ADDR; + gnb_apic->size = 0x00001000; + gnb_apic->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; + if (!hob) { printk(BIOS_ERR, "%s incomplete because no HOB list was found\n", __func__); @@ -166,12 +172,6 @@ printk(BIOS_ERR, "failed to set resources for type %d\n", res->type); } - - /* GNB IOAPIC resource */ - gnb_apic = new_resource(dev, idx++); - gnb_apic->base = GNB_IO_APIC_ADDR; - gnb_apic->size = 0x00001000; - gnb_apic->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; } static void root_complex_init(struct device *dev) -- To view, visit https://review.coreboot.org/c/coreboot/+/69460 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9174c8d7e5e94144187d27210e12f2dca3a6010f Gerrit-Change-Number: 69460 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> 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: 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-MessageType: merged

1 0

[M] Change in coreboot[master]: mb/emulation/qemu-q35: Fix running qemu-i386 with SMM
by Kyösti Mälkki (Code Review) 11 Nov '22

11 Nov '22

Attention is currently required from: Paul Menzel, Arthur Heymans. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55138 ) Change subject: mb/emulation/qemu-q35: Fix running qemu-i386 with SMM ...................................................................... Patch Set 8: (1 comment) File src/mainboard/emulation/qemu-q35/cpu.c: https://review.coreboot.org/c/coreboot/+/55138/comment/04b329e4_ea6ea6ce PS4, Line 52: case 0x00020000: > My comment was sort of bad; I did not necessarily mean adding new revision here. […] qemu/target/i386/tcg/sysemu/smm_helper.c /* SMM support */ #ifdef TARGET_X86_64 #define SMM_REVISION_ID 0x00020064 #else #define SMM_REVISION_ID 0x00020000 #endif So currently, even if qemu command line -cpu parameter is used, those are the possible revision numbers we could see here. ASEG works with PARALLEL_MP. -- To view, visit https://review.coreboot.org/c/coreboot/+/55138 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic6994c8d6e10fd06655129dbd801f1f9d5fd639f Gerrit-Change-Number: 55138 Gerrit-PatchSet: 8 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 11 Nov 2022 17:44:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: mb/emulation/qemu-q35: Fix running qemu-i386 with SMM
by Kyösti Mälkki (Code Review) 11 Nov '22

11 Nov '22

Attention is currently required from: Paul Menzel, Arthur Heymans, Kyösti Mälkki. Kyösti Mälkki has uploaded a new patch set (#8) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/55138 ) Change subject: mb/emulation/qemu-q35: Fix running qemu-i386 with SMM ...................................................................... mb/emulation/qemu-q35: Fix running qemu-i386 with SMM Depending on whether qemu emulates an amd64 or i386 machine the SMM save state will differ. The smbase offsets are incompatible between those save states. TESTED: Both qemu-system-i386 and qemu-system-x86_64 (v7.0.50) have a working smihandler, ASEG and TSEG. Change-Id: Ic6994c8d6e10fd06655129dbd801f1f9d5fd639f Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/mainboard/emulation/qemu-q35/cpu.c 1 file changed, 53 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/55138/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/55138 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic6994c8d6e10fd06655129dbd801f1f9d5fd639f Gerrit-Change-Number: 55138 Gerrit-PatchSet: 8 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: drivers/fsp2: Don't die if the FSP signature doesn't match
by Subrata Banik (Code Review) 11 Nov '22

11 Nov '22

Attention is currently required from: Jason Glenesk, Martin L Roth, Eric Lai, Andrey Petrov, Fred Reitberger, Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69412 ) Change subject: drivers/fsp2: Don't die if the FSP signature doesn't match ...................................................................... Patch Set 3: (2 comments) Patchset: PS3: > coreboot should not (and honestly cannot) be responsible for issues inside the FSP. > > If the FSP cannot run, it should be responsible for doing the check instead of pushing the responsibility to coreboot. We don't do a check for the payload "Oh, this payload is running on a platform it wasn't compiled for" and coreboot shouldn't be responsible for the FSP either. I agree that coreboot shouldn’t bother to own responsibility to check those and it's the FSP's responsibility to make those additional check but unfortunately Intel FSP doesn’t perform those additional checks. I can ask them by filling a bug to incorporate this for future SoC. But comparing the payload against SoC ref code may not be applied as payload should be generic in nature and such header signature check doesn't seems required for payload. File src/drivers/intel/fsp2_0/util.c: https://review.coreboot.org/c/coreboot/+/69412/comment/e9adb582_078eb387 PS3, Line 177: If it doesn't match, the : FSP binaries in CBFS are for a different platform than the platform code trying to use it > Sorry, I think I'm missing your point here. How is this coreboot's responsibility? I was trying to stitch ADL FSP with MTL coreboot with this CL and I'm able to call into the ADL FSP but later it failed due to PCH ID mismatch inside FSP. IMO, the filtering inside coreboot is much better (as FSP is not doing such a check) as coreboot is stitching those binaries and calling into those, hence, better we just avoid calling into anything blindly and it results in a failure. Additionally, Intel FSP specification mentions that each platform has a unique signature hence, IMO, it's better to just make sure we are calling into the appropriate ones. 0x00 – 0x07 UPD Region Signature. The signature will be “XXXXXX_T” for FSP-T “XXXXXX_M” for FSP-M “XXXXXX_S” for FSP-S “XXXXXX_I” for FSP-I Where XXXXXX is a unique signature wondering if you/AMD FSP planning to use a generic UPD ID across different SOC platform hence, you would like to skip the `die` or in general you think that `die` is bad idea on production AP image ? -- To view, visit https://review.coreboot.org/c/coreboot/+/69412 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I12aca7cad3298ac36b1fed09efaa190c958cf126 Gerrit-Change-Number: 69412 Gerrit-PatchSet: 3 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(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)amd.corp-partner.google.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 11 Nov 2022 17:31:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2022