coreboot-gerrit October 2023

coreboot-gerrit@coreboot.org

1 participants
3353 discussions

[M] Change in coreboot[main]: cpu/x86: Support SMBASE relocation-only use-case
by Benjamin Doron (Code Review) 30 Oct '23

30 Oct '23

Attention is currently required from: Angel Pons, Dinesh Gehlot, Eran Mitrani, Felix Held, Fred Reitberger, Jason Glenesk, Kapil Porwal, Martin L Roth, Matt DeVillier, Matt DeVillier, Maulik Vaghela, Raul Rangel, Subrata Banik, Tarun, Tarun Tuli, ron minnich. Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70376?usp=email ) Change subject: cpu/x86: Support SMBASE relocation-only use-case ...................................................................... Patch Set 7: (3 comments) Patchset: PS7: > i'm a bit concerned if this approach has any security implications; moving away from coreboot only i […] It's a fair concern, but if we want to use payload SMI handlers, the payload must put code in SMM. Yes, this puts the payload in the TCB/a more privileged position than before, though the user should be allowed to make that trade-off. It's not a 'true' HOB (that is, in ring-0 memory that's created by PEI and available to DXE). When this design was first created for SBL + EDK2, they reused the HOB data struct, but it's just a single struct at the start of SMRAM. It's created by SMM code, then locked, then (SMRAM is) available again to the bootloader on an S3 resume. File src/cpu/x86/Kconfig: https://review.coreboot.org/c/coreboot/+/70376/comment/57c827a6_208360ad : PS7, Line 159: bool : default n > I don't see this Kconfig option being used in this patch. […] It's used above by HAVE_NATIVE_SMI_HANDLER, which determines if smm.manual ("permanent handlers") is linked in, among other things. This Kconfig can be used to disable that one. If by 'not being used' you mean that it has no effect, well, we could remove it, but I think that would leave this setup/patch incomplete. File src/southbridge/intel/common/Kconfig.common: https://review.coreboot.org/c/coreboot/+/70376/comment/0debe57d_ff9c1913 : PS7, Line 93: HAVE_SMI_HANDLER > 2nd Ah, because from memory, this would leave SPI, etc. unlocked for the external SMM case, and it's safe to drop because apmc_trigger won't actually initiate the SMI. This will leave specific parts of lockdown incomplete, I'll check whether they're necessary (we actually want SMRAM unlocked so that the payload can use it). The point is to get as much lockdown done here as possible. -- To view, visit https://review.coreboot.org/c/coreboot/+/70376?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iec96bab19cdcf80622756f02a3dae49b42036c8d Gerrit-Change-Number: 70376 Gerrit-PatchSet: 7 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.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: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Maulik Vaghela <maulikvaghela(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-CC: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Maulik Vaghela <maulikvaghela(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 30 Oct 2023 19:39:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com> Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: soc/amd/mendocino: Update FSP-S UPD to pass boot logo
by Karthik Ramasubramanian (Code Review) 30 Oct '23

30 Oct '23

Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/78817?usp=email ) Change subject: soc/amd/mendocino: Update FSP-S UPD to pass boot logo ...................................................................... soc/amd/mendocino: Update FSP-S UPD to pass boot logo A new FSP-S UPD is added to allow passing a buffer containing boot logo in BMP format. Update the FSP-S UPD and add a SoC specific callback to populate the UPD. BUG=b:294055390 TEST=Build and boot to OS in Skyrim. Pass the BMP logo buffer through the UPD to FSP-S. Ensure that the concerned driver in FSP-S handles the buffer. Change-Id: Ie522956b6dfe2400ef91d43c80f2adc6d52c8415 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- M src/soc/amd/mendocino/fsp_s_params.c M src/vendorcode/amd/fsp/mendocino/FspsUpd.h 2 files changed, 9 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/78817/1 diff --git a/src/soc/amd/mendocino/fsp_s_params.c b/src/soc/amd/mendocino/fsp_s_params.c index ce450e8..e1eff32 100644 --- a/src/soc/amd/mendocino/fsp_s_params.c +++ b/src/soc/amd/mendocino/fsp_s_params.c @@ -6,6 +6,7 @@ #include <amdblocks/apob_cache.h> #include <amdblocks/vbios_cache.h> #include <bootmode.h> +#include <bootsplash.h> #include <console/console.h> #include <device/pci.h> #include <fsp/api.h> @@ -52,3 +53,9 @@ if (!acpi_is_wakeup_s3()) payload_preload(); } + +void soc_load_logo(FSPS_UPD *supd) +{ + uint32_t logo_size; + bmp_load_logo(&supd->FspsConfig.logo_bmp_buffer, &logo_size); +} diff --git a/src/vendorcode/amd/fsp/mendocino/FspsUpd.h b/src/vendorcode/amd/fsp/mendocino/FspsUpd.h index 3ac52c0..fd250d1 100644 --- a/src/vendorcode/amd/fsp/mendocino/FspsUpd.h +++ b/src/vendorcode/amd/fsp/mendocino/FspsUpd.h @@ -11,7 +11,8 @@ typedef struct __packed { /** Offset 0x0020**/ uint32_t vbios_buffer; - /** Offset 0x0024**/ uint64_t gop_reserved; + /** Offset 0x0024**/ uint32_t gop_reserved; + /** Offset 0x0028**/ uint32_t logo_bmp_buffer; /** Offset 0x002C**/ uint32_t reserved1; /** Offset 0x0030**/ uint16_t UpdTerminator; } FSP_S_CONFIG; -- To view, visit https://review.coreboot.org/c/coreboot/+/78817?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie522956b6dfe2400ef91d43c80f2adc6d52c8415 Gerrit-Change-Number: 78817 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: soc/intel/cse: Add API to check if CSE Firmware update is required
by Jérémy Compostella (Code Review) 30 Oct '23

30 Oct '23

Attention is currently required from: Anil Kumar K, Krishna P Bhat D, Paul Menzel, Subrata Banik. Jérémy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78243?usp=email ) Change subject: soc/intel/cse: Add API to check if CSE Firmware update is required ...................................................................... Patch Set 14: (1 comment) Patchset: PS14: > can you please capture the boot time w/o and w/ this CL. […] As I was testing this feature, I added timestamps around this function call and it takes about 17 ms to execute. -- To view, visit https://review.coreboot.org/c/coreboot/+/78243?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If5fae95786d28d586566881bc4436812754636ae Gerrit-Change-Number: 78243 Gerrit-PatchSet: 14 Gerrit-Owner: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Comment-Date: Mon, 30 Oct 2023 19:11:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: mb/gigabyte/ga-h77m-d3h: Add Sandy/Ivy Bridge board GA-H77M-D3H
by Fabian Groffen (Code Review) 30 Oct '23

30 Oct '23

Attention is currently required from: Keith Hui, Patrick Rudolph. Fabian Groffen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77046?usp=email ) Change subject: mb/gigabyte/ga-h77m-d3h: Add Sandy/Ivy Bridge board GA-H77M-D3H ...................................................................... Patch Set 5: (3 comments) Patchset: PS5: I've cleaned the gpio structs from default values, and verified that gpio25 GPIO_MODE_GPIO works File src/mainboard/gigabyte/ga-h77m-d3h/gpio.c: https://review.coreboot.org/c/coreboot/+/77046/comment/c2062eac_82c79cec : PS3, Line 30: GPIO_MODE_NATIVE > Should be GPIO_MODE_GPIO as DRAM_RESET_GATE_GPIO is 25. Done https://review.coreboot.org/c/coreboot/+/77046/comment/d4e6a4f8_fbe705f0 : PS3, Line 40: GPIO_DIR_OUTPUT > please clean the structs. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/77046?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Icb3e74326a0a7aaf770d1917a2a0931feadd7eab Gerrit-Change-Number: 77046 Gerrit-PatchSet: 5 Gerrit-Owner: Fabian Groffen <grobian(a)gentoo.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Keith Hui <buurin(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Comment-Date: Mon, 30 Oct 2023 18:30:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: soc/intel/cmn/blk/cse: implement ME disable by HAP
by Michał Kopeć (Code Review) 30 Oct '23

30 Oct '23

Michał Kopeć has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/78816?usp=email ) Change subject: soc/intel/cmn/blk/cse: implement ME disable by HAP ...................................................................... soc/intel/cmn/blk/cse: implement ME disable by HAP Add the option to disable ME by the undocumented HAP bit in the Flash Descriptor. Offset of the HAP bit is platform-dependent. Utilize the existing `me_state` option for this purpose. TEST=Boot MSI Z690-A PRO DDR5 with untouched IFD, verify that after booting the HAP bit is set and the HECI device is hidden, indicating that the ME is disabled. Change-Id: I600c9f3733f6ca68e5e91dff29cc53c8d5d8f50b Signed-off-by: Michał Kopeć <michal.kopec(a)3mdeb.com> --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 31 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/78816/1 diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c index af1ca92..19b7967 100644 --- a/src/soc/intel/common/block/cse/cse.c +++ b/src/soc/intel/common/block/cse/cse.c @@ -3,6 +3,7 @@ #define __SIMPLE_DEVICE__ #include <assert.h> +#include <bootstate.h> #include <commonlib/helpers.h> #include <console/console.h> #include <device/mmio.h> @@ -11,6 +12,7 @@ #include <device/pci_ids.h> #include <device/pci_ops.h> #include <intelblocks/cse.h> +#include <intelblocks/fast_spi.h> #include <intelblocks/me.h> #include <intelblocks/pmclib.h> #include <intelblocks/post_codes.h> @@ -1217,10 +1219,13 @@ /* * Disable the Intel (CS)Management Engine via HECI based on a cmos value - * of `me_state`. A value of `0` will result in a (CS)ME state of `0` (working) - * and value of `1` will result in a (CS)ME state of `3` (disabled). + * of `me_state`. A value of `0` will result in a (CS)ME state of `0` (working), + * a value of `1` will result in a (CS)ME state of `3` (disabled), and a value + * of `2` will result in (CS)ME being disabled by the undocumented HAP bit in\ + * the Flash Descriptor. * - * It isn't advised to use this in combination with me_cleaner. + * It isn't advised to use this in combination with me_cleaner. The HAP disable + * mode also requires that the descriptor is unlocked (writable from AP). * * It is advisable to have a second cmos option called `me_state_counter`. * Whilst not essential, it avoid reboots loops if the (CS)ME fails to @@ -1322,7 +1327,7 @@ */ const unsigned int cmos_me_state = get_uint_option("me_state", UINT_MAX); - if (cmos_me_state == UINT_MAX) + if (cmos_me_state == UINT_MAX || cmos_me_state == ME_MODE_DISABLE_HAP) return; printk(BIOS_DEBUG, "CMOS: me_state = %u\n", cmos_me_state); @@ -1369,6 +1374,22 @@ me_reset_with_count(); } +static void set_hap_bit(void *unused) +{ + const unsigned int cmos_me_state = get_uint_option("me_state", UINT_MAX); + + struct descriptor_byte hap_bytes[] = { + { HAP_OFFSET, cmos_me_state == ME_MODE_DISABLE_HAP}, + }; + + if (HAP_OFFSET != 0) + configure_descriptor(hap_bytes, ARRAY_SIZE(hap_bytes)); + else + printk(BIOS_WARNING, "CSE: HAP is unimplemented for this SoC!\n"); +} + +BOOT_STATE_INIT_ENTRY(BS_PRE_DEVICE, BS_ON_ENTRY, set_hap_bit, NULL); + /* * `cse_final_ready_to_boot` function is native implementation of equivalent events * performed by FSP NotifyPhase(Ready To Boot) API invocations. diff --git a/src/soc/intel/common/block/include/intelblocks/cse.h b/src/soc/intel/common/block/include/intelblocks/cse.h index f84dba8..201ae3d 100644 --- a/src/soc/intel/common/block/include/intelblocks/cse.h +++ b/src/soc/intel/common/block/include/intelblocks/cse.h @@ -307,6 +307,12 @@ uint32_t timestamp[NUM_CSE_BOOT_PERF_DATA]; } __packed; +enum cse_disable_mode { + ME_MODE_ENABLE = 0, + ME_MODE_DISABLE_HECI = 1, + ME_MODE_DISABLE_HAP = 2, +}; + /* * Initialize the CSE device. * -- To view, visit https://review.coreboot.org/c/coreboot/+/78816?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I600c9f3733f6ca68e5e91dff29cc53c8d5d8f50b Gerrit-Change-Number: 78816 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-MessageType: newchange

1 0

[L] Change in coreboot[main]: mb/gigabyte/ga-h77m-d3h: Add Sandy/Ivy Bridge board GA-H77M-D3H
by Fabian Groffen (Code Review) 30 Oct '23

30 Oct '23

Attention is currently required from: Keith Hui. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/77046?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: mb/gigabyte/ga-h77m-d3h: Add Sandy/Ivy Bridge board GA-H77M-D3H ...................................................................... mb/gigabyte/ga-h77m-d3h: Add Sandy/Ivy Bridge board GA-H77M-D3H This board is based off ga-b75m-d3h, which uses the same SuperIO chip. It doesn't have the ASMedia SATA3 controller, the H77 chipset comes with 2 SATA3 ports next to the 4 SATA2 ports. Flashing notes: These boards come with dual-BIOS feature. This is set of two unremovable what appears to be identical chips marked M_BIOS and B_BIOS. Flash the B_BIOS chip, and boot the system. Ensure you have a payload and setup ready to boot a Linux system with iomem=relaxed or similar. Immediately use flashrom -p internal to flash the same firmware again. If you skip this step your next boot will show weird exception traces in either coreboot or your payload. Flashing from there via the chip is very difficult (you have to try many times in order to get a booting run), which can all be remedied by doing a flash from internal. I suppose the dual-BIOS feature is somewhat in the way here. Tested with: - CPU Core i7-3770S - RAM single bank 4GB CL11, two banks 4+4GB CL11 - OS Gentoo Linux LiveUSB, KDE desktop (Linux 5.15.72) Working: - GRUB2 payload - Intel ME stripped - Integrated graphics with libgfxinit - (boot from) SATA2, SATA3 ports - Rear and mainboard connector USB ports, supporting boot - Atheros GbE NIC - 2.0 channel audio via lineout jack output - ACPI (power button triggers OS events) - S3 suspend/resume Signed-off-by: Fabian Groffen <grobian(a)gentoo.org> Change-Id: Icb3e74326a0a7aaf770d1917a2a0931feadd7eab --- A src/mainboard/gigabyte/ga-h77m-d3h/Kconfig A src/mainboard/gigabyte/ga-h77m-d3h/Kconfig.name A src/mainboard/gigabyte/ga-h77m-d3h/Makefile.inc A src/mainboard/gigabyte/ga-h77m-d3h/acpi/ec.asl A src/mainboard/gigabyte/ga-h77m-d3h/acpi/mainboard.asl A src/mainboard/gigabyte/ga-h77m-d3h/acpi/pci.asl A src/mainboard/gigabyte/ga-h77m-d3h/acpi/platform.asl A src/mainboard/gigabyte/ga-h77m-d3h/acpi/superio.asl A src/mainboard/gigabyte/ga-h77m-d3h/acpi/thermal.asl A src/mainboard/gigabyte/ga-h77m-d3h/acpi_tables.c A src/mainboard/gigabyte/ga-h77m-d3h/board_info.txt A src/mainboard/gigabyte/ga-h77m-d3h/cmos.default A src/mainboard/gigabyte/ga-h77m-d3h/cmos.layout A src/mainboard/gigabyte/ga-h77m-d3h/data.vbt A src/mainboard/gigabyte/ga-h77m-d3h/devicetree.cb A src/mainboard/gigabyte/ga-h77m-d3h/dsdt.asl A src/mainboard/gigabyte/ga-h77m-d3h/early_init.c A src/mainboard/gigabyte/ga-h77m-d3h/gma-mainboard.ads A src/mainboard/gigabyte/ga-h77m-d3h/gpio.c A src/mainboard/gigabyte/ga-h77m-d3h/hda_verb.c A src/mainboard/gigabyte/ga-h77m-d3h/thermal.h 21 files changed, 669 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/77046/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/77046?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Icb3e74326a0a7aaf770d1917a2a0931feadd7eab Gerrit-Change-Number: 77046 Gerrit-PatchSet: 5 Gerrit-Owner: Fabian Groffen <grobian(a)gentoo.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Keith Hui <buurin(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[L] Change in coreboot[main]: mb/asus/p8x7x-series: Add P8H77-M as a variant
by Fabian Groffen (Code Review) 30 Oct '23

30 Oct '23

Attention is currently required from: Angel Pons, Felix Singer, Keith Hui, Kevin Keijzer. Fabian Groffen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78806?usp=email ) Change subject: mb/asus/p8x7x-series: Add P8H77-M as a variant ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: I had the same result on P8H77M -- To view, visit https://review.coreboot.org/c/coreboot/+/78806?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I846e61303db0ad24569dcb713f0505d670fde1bd Gerrit-Change-Number: 78806 Gerrit-PatchSet: 2 Gerrit-Owner: Kevin Keijzer <kevin(a)quietlife.nl> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Fabian Groffen <grobian(a)gentoo.org> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Kevin Keijzer <kevin(a)quietlife.nl> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 30 Oct 2023 18:18:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: acpi: Add PPTT support
by David Milosevic (Code Review) 30 Oct '23

30 Oct '23

Attention is currently required from: Arthur Heymans, Cliff Huang, Lance Zhao, Maximilian Brune, Patrick Rudolph, Tim Wawrzynczak. David Milosevic has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78071?usp=email ) Change subject: acpi: Add PPTT support ...................................................................... Patch Set 8: (2 comments) File src/acpi/acpi_pptt.c: https://review.coreboot.org/c/coreboot/+/78071/comment/92aeb0a9_87655e3f : PS6, Line 99: node_to_idx(cpu_node, pptt); : } > This is just a monotonically increasing number in the loop over topology elements? Use a static coun […] Then we would have basically a second "current" pointer, which we also would have to carry around. I tried it, and don't like the result. The information (start of PPTT table and current writing position) is already there, so there is no need to introduce yet another counter. The macro is now an inline function. I also was able to ditch the second macro due to refactoring. As requested, I have added an ASCII graph and a short description to explain what we do there. I also decided to re-introduce the global "pptt_start" variable. IMO, removing it, reduces the readability. And since "pptt_start" is effectively a constant value, it should not add further complexity. I have also added a comment to explain all this. https://review.coreboot.org/c/coreboot/+/78071/comment/481a487c_8690aeb3 : PS6, Line 102: root > Maybe rename it to "node"? Since "entry" has the same issue. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/78071?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia119e1ba15756704668116bdbc655190ec94ff10 Gerrit-Change-Number: 78071 Gerrit-PatchSet: 8 Gerrit-Owner: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Mon, 30 Oct 2023 18:11:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: cpu/x86: Support SMBASE relocation-only use-case
by Felix Held (Code Review) 30 Oct '23

30 Oct '23

Attention is currently required from: Angel Pons, Benjamin Doron, Dinesh Gehlot, Eran Mitrani, Fred Reitberger, Jason Glenesk, Kapil Porwal, Martin L Roth, Matt DeVillier, Maulik Vaghela, Raul Rangel, Subrata Banik, Tarun, Tarun Tuli, ron minnich. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70376?usp=email ) Change subject: cpu/x86: Support SMBASE relocation-only use-case ...................................................................... Patch Set 7: (3 comments) Patchset: PS7: i'm a bit concerned if this approach has any security implications; moving away from coreboot only implementing a rather minimal smm handler to having some payload put code into smm sounds like a bad plan to me. the less code we have in smm, the better. also in the follow-up that adds this functionality, i'm not sure if getting the smm handler location from a hob adds some attack surface File src/cpu/x86/smm/smm_module_loader.c: https://review.coreboot.org/c/coreboot/+/70376/comment/628ce78b_076a22da : PS7, Line 127: __weak it might be better if this could be done without the need of weak functions. see below for a possible idea https://review.coreboot.org/c/coreboot/+/70376/comment/c45017d8_f30586b7 : PS7, Line 142: { adding an if (!CONFIG(HAVE_NATIVE_SMI_HANDLER)) return smm_get_cpu_smbase_override(unsigned int cpu_num) would avoid the preprocessor #if -- To view, visit https://review.coreboot.org/c/coreboot/+/70376?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iec96bab19cdcf80622756f02a3dae49b42036c8d Gerrit-Change-Number: 70376 Gerrit-PatchSet: 7 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.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: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Maulik Vaghela <maulikvaghela(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-CC: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Maulik Vaghela <maulikvaghela(a)google.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Mon, 30 Oct 2023 17:56:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: Revert "soc/intel/{tigerlake,meteorlake}: Check ITBT FW version"
by Subrata Banik (Code Review) 30 Oct '23

30 Oct '23

Attention is currently required from: Eran Mitrani, Nick Vaccaro, Paul Menzel, Ravishankar Sarawadi, Sean Rhodes, Tarun. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78457?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: Revert "soc/intel/{tigerlake,meteorlake}: Check ITBT FW version" ...................................................................... Patch Set 9: Code-Review+2 (1 comment) Patchset: PS7: > Done I thought, Kapil tested this CL and verified S0ix was working ``` w/o test and broke MTL/TGL S0ix. ``` @Kapil, can you please comment on this ? -- To view, visit https://review.coreboot.org/c/coreboot/+/78457?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ib8eb11d36eac4e1c94a3349386442fa3eeeaef37 Gerrit-Change-Number: 78457 Gerrit-PatchSet: 9 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Mon, 30 Oct 2023 17:55:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems> Comment-In-Reply-To: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2023