coreboot-gerrit February 2025

coreboot-gerrit@coreboot.org

1 participants
2886 discussions

[XS] Change in coreboot[main]: mb/amd/birman_plus: Use actual flash size of 64 MiB instead of 16 MiB
by Patrick Rudolph (Code Review) 12 Feb '25

12 Feb '25

Attention is currently required from: Maximilian Brune, Patrick Rudolph, Paul Menzel. Hello Maximilian Brune, Paul Menzel, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86179?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by Paul Menzel, Code-Review+2 by Maximilian Brune, Verified+1 by build bot (Jenkins) Change subject: mb/amd/birman_plus: Use actual flash size of 64 MiB instead of 16 MiB ...................................................................... mb/amd/birman_plus: Use actual flash size of 64 MiB instead of 16 MiB Birman+ has a 64MiB flash chip. Update the mainboards Kconfig comment and fix the FMD to generate a 64MiB ROM. Until now only the first 16MiB are being used. TEST: Still boots on AMD/Birman+ Change-Id: I72e3dcb0c3a308c3b0fd981b56cc7c1ef60095cc Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/mainboard/amd/birman_plus/Kconfig M src/mainboard/amd/birman_plus/board_glinda.fmd 2 files changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/86179/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/86179?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I72e3dcb0c3a308c3b0fd981b56cc7c1ef60095cc Gerrit-Change-Number: 86179 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>

1 0

[XS] Change in coreboot[main]: mb/google/rauru: Add EC suspend pin initial setting
by Yu-Ping Wu (Code Review) 12 Feb '25

12 Feb '25

Attention is currently required from: Hung-Te Lin, Jarried Lin. Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/86381?usp=email ) Change subject: mb/google/rauru: Add EC suspend pin initial setting ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/86381/comment/a2857ad7_41a99666?us… : PS3, Line 7: mb/google/rauru: Add EC suspend pin initial setting : : Set the EC suspend pin to output high. > In particular, why do we not need this for previous SoCs such as geralt? Just recalled that AP_IN_SLEEP_L is the new sleep source for Rauru, which is decoupled from SRCLKENA0 (where previous SoCs use). -- To view, visit https://review.coreboot.org/c/coreboot/+/86381?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6ea56208256bb6f11fb6b0adf7627403963295bc Gerrit-Change-Number: 86381 Gerrit-PatchSet: 3 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Comment-Date: Wed, 12 Feb 2025 15:34:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[XS] Change in coreboot[main]: mb/google/rauru: Add EC suspend pin initial setting
by Yu-Ping Wu (Code Review) 12 Feb '25

12 Feb '25

Attention is currently required from: Hung-Te Lin, Jarried Lin. Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/86381?usp=email ) Change subject: mb/google/rauru: Add EC suspend pin initial setting ...................................................................... Patch Set 3: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/86381/comment/de3be7e3_9b1d97c3?us… : PS3, Line 7: mb/google/rauru: Add EC suspend pin initial setting : : Set the EC suspend pin to output high. > Please revise the commit message. And the purpose of the setting. In particular, why do we not need this for previous SoCs such as geralt? File src/mainboard/google/rauru/chromeos.c: https://review.coreboot.org/c/coreboot/+/86381/comment/d63c0b06_e9a38894?us… : PS3, Line 22: gpio_output(GPIO_EC_SUSPEND_PIN, 1); In ATF plat/mediatek/drivers/spm/mt8196/mt_spm_suspend.c, we set the GPIO mode and direction for `EC_SUSPEND_BK_PIN`. Should we do the same for `EC_SUSPEND_PIN` in ATF instead of (or in addition to) here for consistency? File src/mainboard/google/rauru/gpio.h: https://review.coreboot.org/c/coreboot/+/86381/comment/66a8b0bb_ffbdc2cf?us… : PS3, Line 8: _OD Not related to this patch, but what's `_OD`? The name on the schematics is `BEEP_ON`. Could you upload a separate patch to fix this? -- To view, visit https://review.coreboot.org/c/coreboot/+/86381?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6ea56208256bb6f11fb6b0adf7627403963295bc Gerrit-Change-Number: 86381 Gerrit-PatchSet: 3 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Comment-Date: Wed, 12 Feb 2025 15:23:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[S] Change in coreboot[main]: soc/amd/common/block/graphics: Support non VGA IGDs
by Patrick Rudolph (Code Review) 12 Feb '25

12 Feb '25

Attention is currently required from: Ana Carolina Cabral, Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier. Patrick Rudolph has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/86300?usp=email ) Change subject: soc/amd/common/block/graphics: Support non VGA IGDs ...................................................................... Patch Set 3: (1 comment) File src/soc/amd/common/block/graphics/Kconfig: https://review.coreboot.org/c/coreboot/+/86300/comment/dccebf67_4d140639?us… : PS2, Line 39: SOC_AMD_COMMON_BLOCK_GRAPHICS_VGA > i'd invert this option and have boards having a non-VGA-compatible iGPU select that option. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/86300?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6ab28aab74f3169d45d7d852a37ddfcfc75b7c88 Gerrit-Change-Number: 86300 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.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: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ana Carolina Cabral <ana.cpmelo95(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: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 12 Feb 2025 15:18:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[M] Change in coreboot[main]: soc/amd/common/block/graphics: Use vbt_get()
by Patrick Rudolph (Code Review) 12 Feb '25

12 Feb '25

Attention is currently required from: Andrey Petrov, Felix Held, Fred Reitberger, Intel coreboot Reviewers, Jason Glenesk, Jérémy Compostella, Matt DeVillier, Ronak Kanabar. Patrick Rudolph has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/86299?usp=email ) Change subject: soc/amd/common/block/graphics: Use vbt_get() ...................................................................... Patch Set 3: (2 comments) File src/soc/amd/common/block/include/amdblocks/vbt.h: https://review.coreboot.org/c/coreboot/+/86299/comment/92bbc0b6_f43fbb2b?us… : PS2, Line 12: * notify is called. FSP expects a pointer to the PCI option rom instead > Option ROM […] Done https://review.coreboot.org/c/coreboot/+/86299/comment/fd3c9f9c_091c9a3f?us… : PS2, Line 20: void *vbt_get(void); > i'd add an empty line between the function prototype and the #endif Done -- To view, visit https://review.coreboot.org/c/coreboot/+/86299?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I92d76fc4df88fbce792b9d7c912c6799617704a0 Gerrit-Change-Number: 86299 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.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: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 12 Feb 2025 15:18:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[XS] Change in coreboot[main]: device/pci_rom: Support non VGA iGPUs
by Patrick Rudolph (Code Review) 12 Feb '25

12 Feb '25

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/86386?usp=email ) Change subject: device/pci_rom: Support non VGA iGPUs ...................................................................... device/pci_rom: Support non VGA iGPUs Recent AMD iGPUs are not VGA compatible any more, thus they don't identify themself as "VGA compatible" any more by the PCI class code. Since the PCI VGA Option ROM code assumes it only runs on VGA compatible devices, relax the ACPI code part to handle display devices as well. In order to run a VBIOS in coreboot it still must be VGA compatible, but for ACPI table generation, where no code is run, it's not necessary any more. The new code allows to use Linux's amdgpu driver on AMD/glinda. TEST: On amd/birman+ the amdgpu kernel drivers starts and dmesg shows: [ 3.010224] [drm] amdgpu kernel modesetting enabled. The coreboot log shows: [INFO ] CBFS: Found 'pci1002,150e.rom' @0x10a40 size 0x4400 in mcache @0x1b7dd184 [DEBUG] In CBFS, ROM address for PCI: 00:02:00.0 = 0xff012a6c [DEBUG] Class Code mismatch ROM 00030000, dev 00038000 [DEBUG] Copying non-VGA ROM image from 0xff012a6c to 0x000d0000, 0x4400 bytes [...] Copying initialized VBIOS image from 0x000d0000 [DEBUG] ACPI: * VFCT at 1b5cb960 Change-Id: I623cd80b45b148b91f2796b22a589bbede0feeeb Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/device/pci_rom.c 1 file changed, 3 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/86386/1 diff --git a/src/device/pci_rom.c b/src/device/pci_rom.c index 70e18ab..b05cc8b 100644 --- a/src/device/pci_rom.c +++ b/src/device/pci_rom.c @@ -210,10 +210,7 @@ } printk(BIOS_DEBUG, " Copying %sVBIOS image from %p\n", - rom == (struct rom_header *) - (uintptr_t)PCI_VGA_RAM_IMAGE_START ? - "initialized " : "", - rom); + ((uintptr_t)rom < MiB) ? "initialized " : "", rom); header->DeviceID = device->device; header->VendorID = device->vendor; @@ -233,8 +230,8 @@ pci_rom_write_acpi_tables(const struct device *device, unsigned long current, struct acpi_rsdp *rsdp) { - /* Only handle VGA devices */ - if ((device->class >> 8) != PCI_CLASS_DISPLAY_VGA) + /* Only handle display devices */ + if ((device->class >> 16) != PCI_BASE_CLASS_DISPLAY) return current; /* Only handle enabled devices */ -- To view, visit https://review.coreboot.org/c/coreboot/+/86386?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I623cd80b45b148b91f2796b22a589bbede0feeeb Gerrit-Change-Number: 86386 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>

1 0

[M] Change in coreboot[main]: device/pci_rom: Keep track of VBIOS
by Patrick Rudolph (Code Review) 12 Feb '25

12 Feb '25

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/86385?usp=email ) Change subject: device/pci_rom: Keep track of VBIOS ...................................................................... device/pci_rom: Keep track of VBIOS Currently the VBIOS is placed somewhere in DRAM when necessary. While generating ACPI tables the code attempts to find the VBIOS by looking at "known" places. Simplify the code and keep track of the VBIOS using a pointer in struct device by filling it in pci_rom_load(). The following patches will reuse this pointer to generalize the code even more. Change-Id: Ib27d30e3b07740d6098d4f7a5c4f5d8bce976f00 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/device/pci_rom.c M src/include/device/device.h 2 files changed, 59 insertions(+), 55 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/86385/1 diff --git a/src/device/pci_rom.c b/src/device/pci_rom.c index 0fe94db..70e18ab 100644 --- a/src/device/pci_rom.c +++ b/src/device/pci_rom.c @@ -169,12 +169,18 @@ memcpy((void *)PCI_VGA_RAM_IMAGE_START, rom_header, rom_size); } +#if CONFIG(HAVE_ACPI_TABLES) + dev->pci_vga_option_rom = (struct rom_header *)(PCI_VGA_RAM_IMAGE_START); +#endif return (struct rom_header *)(PCI_VGA_RAM_IMAGE_START); } printk(BIOS_DEBUG, "Copying non-VGA ROM image from %p to %p, 0x%x bytes\n", rom_header, pci_ram_image_start, rom_size); +#if CONFIG(HAVE_ACPI_TABLES) + dev->pci_vga_option_rom = (struct rom_header *)pci_ram_image_start; +#endif memcpy(pci_ram_image_start, rom_header, rom_size); pci_ram_image_start += rom_size; return (struct rom_header *)(pci_ram_image_start-rom_size); @@ -183,30 +189,6 @@ /* ACPI */ #if CONFIG(HAVE_ACPI_TABLES) -/* VBIOS may be modified after oprom init so use the copy if present. */ -static struct rom_header *check_initialized(const struct device *dev) -{ - struct rom_header *run_rom; - struct pci_data *rom_data; - - if (!CONFIG(VGA_ROM_RUN) && !CONFIG(RUN_FSP_GOP)) - return NULL; - - run_rom = (struct rom_header *)(uintptr_t)PCI_VGA_RAM_IMAGE_START; - if (read_le16(&run_rom->signature) != PCI_ROM_HDR) - return NULL; - - rom_data = (struct pci_data *)((u8 *)run_rom - + read_le16(&run_rom->data)); - - if (read_le32(&rom_data->signature) == PCI_DATA_HDR - && read_le16(&rom_data->device) == dev->device - && read_le16(&rom_data->vendor) == dev->vendor) - return run_rom; - else - return NULL; -} - static unsigned long ati_rom_acpi_fill_vfct(const struct device *device, acpi_vfct_t *vfct_struct, unsigned long current) @@ -214,7 +196,8 @@ acpi_vfct_image_hdr_t *header = &vfct_struct->image_hdr; struct rom_header *rom; - rom = check_initialized(device); + /* Already loaded into DRAM? */ + rom = device->pci_vga_option_rom; if (!rom) rom = pci_rom_probe(device); if (!rom) { @@ -277,6 +260,7 @@ void pci_rom_ssdt(const struct device *device) { + const struct rom_header *rom; static size_t ngfx; /* Only handle display devices */ @@ -287,48 +271,54 @@ if (!device->enabled) return; - /* Probe for option rom */ - const struct rom_header *rom = pci_rom_probe(device); - if (!rom || !rom->size) { - printk(BIOS_WARNING, "%s: Missing PCI Option ROM\n", - dev_path(device)); - return; - } - const char *scope = acpi_device_path(device); if (!scope) { printk(BIOS_ERR, "%s: Missing ACPI scope\n", dev_path(device)); return; } - /* Supports up to four devices. */ - if ((CBMEM_ID_ROM0 + ngfx) > CBMEM_ID_ROM3) { - printk(BIOS_ERR, "%s: Out of CBMEM IDs.\n", dev_path(device)); - return; - } + /* Already loaded into DRAM? */ + rom = device->pci_vga_option_rom; + if (!rom) { + /* Probe for option rom */ + rom = pci_rom_probe(device); + if (!rom || !rom->size) { + printk(BIOS_WARNING, "%s: Missing PCI Option ROM\n", + dev_path(device)); + return; + } - /* Prepare memory */ - const size_t cbrom_length = rom->size * 512; - if (!cbrom_length) { - printk(BIOS_ERR, "%s: ROM has zero length!\n", - dev_path(device)); - return; - } + /* Supports up to four devices. */ + if ((CBMEM_ID_ROM0 + ngfx) > CBMEM_ID_ROM3) { + printk(BIOS_ERR, "%s: Out of CBMEM IDs.\n", dev_path(device)); + return; + } - void *cbrom = cbmem_add(CBMEM_ID_ROM0 + ngfx, cbrom_length); - if (!cbrom) { - printk(BIOS_ERR, "%s: Failed to allocate CBMEM.\n", - dev_path(device)); - return; - } - /* Increment CBMEM id for next device */ - ngfx++; + /* Prepare memory */ + const size_t cbrom_length = rom->size * 512; + if (!cbrom_length) { + printk(BIOS_ERR, "%s: ROM has zero length!\n", + dev_path(device)); + return; + } - memcpy(cbrom, rom, cbrom_length); + void *cbrom = cbmem_add(CBMEM_ID_ROM0 + ngfx, cbrom_length); + if (!cbrom) { + printk(BIOS_ERR, "%s: Failed to allocate CBMEM.\n", + dev_path(device)); + return; + } + /* Increment CBMEM id for next device */ + ngfx++; + + memcpy(cbrom, rom, cbrom_length); + + rom = cbrom; + } /* write _ROM method */ acpigen_write_scope(scope); - acpigen_write_rom(cbrom, cbrom_length); + acpigen_write_rom((void *)rom, rom->size * 512); acpigen_pop_len(); /* pop scope */ } #endif diff --git a/src/include/device/device.h b/src/include/device/device.h index 901f717..2cee563 100644 --- a/src/include/device/device.h +++ b/src/include/device/device.h @@ -36,6 +36,7 @@ struct bus; struct acpi_rsdp; +struct rom_header; struct device_operations { void (*read_resources)(struct device *dev); @@ -127,6 +128,19 @@ struct device_operations *ops; struct chip_operations *chip_ops; const char *name; +#if CONFIG(HAVE_ACPI_TABLES) + /* + * A pointer to the corresponding PCI Option ROM. + * + * When set the Option ROM has been placed in usable DRAM in + * an area that is marked as reserved. This can be for example + * the legacy C-segment or a CBMEM area. The Option ROM is + * read writeable and guaranted to be used by the device. + * The PCIR data header might still have a different vendor and + * device ID. + */ + struct rom_header *pci_vga_option_rom; +#endif #if CONFIG(GENERATE_SMBIOS_TABLES) u8 smbios_slot_type; u8 smbios_slot_data_width; -- To view, visit https://review.coreboot.org/c/coreboot/+/86385?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ib27d30e3b07740d6098d4f7a5c4f5d8bce976f00 Gerrit-Change-Number: 86385 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>

1 0

[S] Change in coreboot[main]: device/pci_rom: Move VBIOS checksum fix
by Patrick Rudolph (Code Review) 12 Feb '25

12 Feb '25

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier. Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/86384?usp=email ) Change subject: device/pci_rom: Move VBIOS checksum fix ...................................................................... device/pci_rom: Move VBIOS checksum fix Move the VBIOS checksum code into the soc/amd folder, as it's specific to AMD's FSP. The code now fixes the VBIOS in place instead only fixing it for the VFCT table. Change-Id: I63aaaefaf01ea456e2ed39cd0891e552a7fb5135 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/device/pci_rom.c M src/soc/amd/common/fsp/fsp_graphics.c 2 files changed, 20 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/86384/1 diff --git a/src/device/pci_rom.c b/src/device/pci_rom.c index 7e19646d..0fe94db 100644 --- a/src/device/pci_rom.c +++ b/src/device/pci_rom.c @@ -242,15 +242,6 @@ vfct_struct->VBIOSImageOffset = (size_t)header - (size_t)vfct_struct; - /* Calculate and set checksum for VBIOS data if FSP GOP driver used, - Since GOP driver modifies ATOMBIOS tables at end of VBIOS */ - if (CONFIG(RUN_FSP_GOP)) { - /* Clear existing checksum before recalculating */ - header->VbiosContent[VFCT_VBIOS_CHECKSUM_OFFSET] = 0; - header->VbiosContent[VFCT_VBIOS_CHECKSUM_OFFSET] = - acpi_checksum(header->VbiosContent, header->ImageLength); - } - current += header->ImageLength; return current; } diff --git a/src/soc/amd/common/fsp/fsp_graphics.c b/src/soc/amd/common/fsp/fsp_graphics.c index 3e082e6..776711b 100644 --- a/src/soc/amd/common/fsp/fsp_graphics.c +++ b/src/soc/amd/common/fsp/fsp_graphics.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ #include <amdblocks/graphics.h> +#include <amdblocks/vbt.h> #include <console/console.h> #include <device/device.h> #include <device/pci.h> @@ -15,4 +16,23 @@ else printk(BIOS_ERR, "%s: Unable to find resource for %s\n", __func__, dev_path(dev)); + + /* + * Calculate and set checksum for VBIOS data if FSP GOP driver used, + * Since GOP driver modifies ATOMBIOS tables at end of BS_DEV_RESOURCES. + * While Linux does not verify the checksum the Windows kernel driver does. + */ + struct rom_header *vbios = (struct rom_header *)vbt_get(); + if (!vbios || !vbios->size) { + printk(BIOS_ERR, "%s: No VGA BIOS loaded for %s\n", + __func__, dev_path(dev)); + return; + } + + uint8_t *data = (uint8_t *)vbios; + + /* Clear existing checksum before recalculating */ + data[VFCT_VBIOS_CHECKSUM_OFFSET] = 0; + data[VFCT_VBIOS_CHECKSUM_OFFSET] = + acpi_checksum(data, vbios->size * 512); } -- To view, visit https://review.coreboot.org/c/coreboot/+/86384?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I63aaaefaf01ea456e2ed39cd0891e552a7fb5135 Gerrit-Change-Number: 86384 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.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: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[S] Change in coreboot[main]: device/pci_rom: Use correct endian conversion
by Patrick Rudolph (Code Review) 12 Feb '25

12 Feb '25

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/86383?usp=email ) Change subject: device/pci_rom: Use correct endian conversion ...................................................................... device/pci_rom: Use correct endian conversion The Option ROM contains lots of 16bit values that are being used, thus use the 16bit endianess conversion function over the 32bit variant to avoid confusion. Change-Id: I571be97a930ad018e1d1316117cefe5bd1c68f9b Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/device/pci_rom.c 1 file changed, 7 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/86383/1 diff --git a/src/device/pci_rom.c b/src/device/pci_rom.c index d60720e..7e19646d 100644 --- a/src/device/pci_rom.c +++ b/src/device/pci_rom.c @@ -93,16 +93,16 @@ printk(BIOS_SPEW, "PCI expansion ROM, signature 0x%04x, INIT size 0x%04x, data ptr 0x%04x\n", - le32_to_cpu(rom_header->signature), - rom_header->size * 512, le32_to_cpu(rom_header->data)); + le16_to_cpu(rom_header->signature), + rom_header->size * 512, le16_to_cpu(rom_header->data)); - if (le32_to_cpu(rom_header->signature) != PCI_ROM_HDR) { + if (le16_to_cpu(rom_header->signature) != PCI_ROM_HDR) { printk(BIOS_ERR, "Incorrect expansion ROM header signature %04x\n", - le32_to_cpu(rom_header->signature)); + le16_to_cpu(rom_header->signature)); return NULL; } - rom_data = (((void *)rom_header) + le32_to_cpu(rom_header->data)); + rom_data = (((void *)rom_header) + le16_to_cpu(rom_header->data)); printk(BIOS_SPEW, "PCI ROM image, vendor ID %04x, device ID %04x,\n", rom_data->vendor, rom_data->device); @@ -144,9 +144,9 @@ + image_size); rom_data = (struct pci_data *)((void *)rom_header - + le32_to_cpu(rom_header->data)); + + le16_to_cpu(rom_header->data)); - image_size = le32_to_cpu(rom_data->ilen) * 512; + image_size = le16_to_cpu(rom_data->ilen) * 512; } while ((rom_data->type != 0) && (rom_data->indicator != 0)); // make sure we got x86 version if (rom_data->type != 0) -- To view, visit https://review.coreboot.org/c/coreboot/+/86383?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I571be97a930ad018e1d1316117cefe5bd1c68f9b Gerrit-Change-Number: 86383 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>

1 0

[M] Change in coreboot[main]: soc/amd/common/block/graphics: Use vbt_get()
by Patrick Rudolph (Code Review) 12 Feb '25

12 Feb '25

Attention is currently required from: Andrey Petrov, Fred Reitberger, Intel coreboot Reviewers, Jason Glenesk, Matt DeVillier, Patrick Rudolph, Ronak Kanabar. Hello Andrey Petrov, Felix Held, Fred Reitberger, Intel coreboot Reviewers, Jason Glenesk, Matt DeVillier, Ronak Kanabar, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86299?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: soc/amd/common/block/graphics: Use vbt_get() ...................................................................... soc/amd/common/block/graphics: Use vbt_get() Implement vbt_get() on AMD and return the VBIOS location. This allows to drop the hardcoded addresses used in various places and return an address in DRAM that is reserved for FSP use. TEST: amd/birman+ still gets passed the correct VBIOS address. Change-Id: I92d76fc4df88fbce792b9d7c912c6799617704a0 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/drivers/intel/fsp2_0/silicon_init.c M src/soc/amd/cezanne/fsp_s_params.c M src/soc/amd/common/block/graphics/graphics.c A src/soc/amd/common/block/include/amdblocks/vbt.h M src/soc/amd/glinda/fsp_s_params.c M src/soc/amd/mendocino/fsp_s_params.c M src/soc/amd/phoenix/fsp_s_params.c M src/soc/amd/picasso/fsp_s_params.c 8 files changed, 45 insertions(+), 19 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/86299/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/86299?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I92d76fc4df88fbce792b9d7c912c6799617704a0 Gerrit-Change-Number: 86299 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.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: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2025