Attention is currently required from: Arthur Heymans, Christian Walter, Felix Held, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Nico Huber, Tim Chu.
Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82034?usp=email )
Change subject: soc/intel/xeon_sp: Add _OSC ASL generation utils for IIO stacks
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> I wanted to give this more thought, but seeing superficial reviews […]
Thanks, Nico. For the issues captured, it proves the necessity to bring the codes from dynamic generated to static defined for this case.
I attempted to quickly set this up to make sure that all comments and good ideas are taken care of in priority instead of pending for a unclear state. Let us slow down a bit to make sure a carefully going through on all contents here. Please take your time here.
--
To view, visit https://review.coreboot.org/c/coreboot/+/82034?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: Ibd3bfa2428725fe593754436d5ed75a3a11b4cdc
Gerrit-Change-Number: 82034
Gerrit-PatchSet: 2
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Mon, 22 Apr 2024 13:34:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Kapil Porwal.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81968?usp=email )
Change subject: libpayload: Add x86_64 (64-bit) support
......................................................................
Patch Set 21:
(3 comments)
File payloads/libpayload/arch/x86/exception.c:
https://review.coreboot.org/c/coreboot/+/81968/comment/1b662da3_07ef8ac3 :
PS20, Line 150: printf("EIP: 0x%08x\n", exception_state->regs.eip);
: printf("CS: 0x%04x\n", exception_state->regs.cs);
: printf("EFLAGS: 0x%08x\n", exception_state->regs.eflags);
: printf("EAX: 0x%08x\n", exception_state->regs.eax);
: printf("ECX: 0x%08x\n", exception_state->regs.ecx);
: printf("EDX: 0x%08x\n", exception_state->regs.edx);
: printf("EBX: 0x%08x\n", exception_state->regs.ebx);
: printf("ESP: 0x%08x\n", exception_state->regs.esp);
: printf("EBP: 0x%08x\n", exception_state->regs.ebp);
: printf("ESI: 0x%08x\n", exception_state->regs.esi);
: printf("EDI: 0x%08x\n", exception_state->regs.edi);
: printf("DS: 0x%04x\n", exception_state->regs.ds);
: printf("ES: 0x%04x\n", exception_state->regs.es);
: printf("SS: 0x%04x\n", exception_state->regs.ss);
: printf("FS: 0x%04x\n", exception_state->regs.fs);
: printf("GS: 0x%04x\n", exception_state->regs.gs);
> Needs 64bit version.
Acknowledged
File payloads/libpayload/arch/x86/exec_64.S:
https://review.coreboot.org/c/coreboot/+/81968/comment/283ba466_2aa3ca0b :
PS20, Line 64: /* Jump to the code */
: call *8(%rbp)
> This is not going to work. The ABI uses registers, not stack to pass on the jump addr.
Acknowledged
File payloads/libpayload/arch/x86/main.c:
https://review.coreboot.org/c/coreboot/+/81968/comment/27e40484_bc7b724d :
PS20, Line 37: unsigned long loader_eax; /**< The value of EAX passed from the loader */
: unsigned long loader_ebx; /**< The value of EBX passed from the loader */
> Only used for multiboot stuff. Does not apply to long mode.
Acknowledged
--
To view, visit https://review.coreboot.org/c/coreboot/+/81968?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: I69fda47bedf1a14807b1515c4aed6e3a1d5b8585
Gerrit-Change-Number: 81968
Gerrit-PatchSet: 21
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Mon, 22 Apr 2024 13:24:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Kapil Porwal.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/81968?usp=email
to look at the new patch set (#22).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: libpayload: Add x86_64 (64-bit) support
......................................................................
libpayload: Add x86_64 (64-bit) support
This patch introduces x86_64 (64-bit) support to the payload, building
upon the existing x86 (32-bit) architecture. Files necessary for 64-bit
compilation are now guarded by the `CONFIG_LP_ARCH_X86_64` Kconfig
option.
BUG=b:242829490
TEST=Entered libpayload in long mode, successfully parsed coreboot
table.
Change-Id: I69fda47bedf1a14807b1515c4aed6e3a1d5b8585
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M payloads/libpayload/Kconfig
M payloads/libpayload/Makefile
M payloads/libpayload/Makefile.mk
M payloads/libpayload/arch/x86/Kconfig
M payloads/libpayload/arch/x86/Makefile.mk
M payloads/libpayload/arch/x86/exception.c
A payloads/libpayload/arch/x86/exception_asm_64.S
A payloads/libpayload/arch/x86/exec_64.S
M payloads/libpayload/arch/x86/gdb.c
M payloads/libpayload/arch/x86/head.S
A payloads/libpayload/arch/x86/head_32.S
A payloads/libpayload/arch/x86/head_64.S
A payloads/libpayload/arch/x86/libpayload_64.ldscript
M payloads/libpayload/arch/x86/main.c
M payloads/libpayload/arch/x86/multiboot.c
M payloads/libpayload/arch/x86/string.c
M payloads/libpayload/bin/lpgcc
M payloads/libpayload/drivers/storage/ahci_common.c
M payloads/libpayload/drivers/timer/Kconfig
M payloads/libpayload/drivers/usb/Kconfig
M payloads/libpayload/drivers/usb/uhci.c
M payloads/libpayload/drivers/usb/xhci.c
M payloads/libpayload/include/sysinfo.h
M payloads/libpayload/include/x86/arch/exception.h
M payloads/libpayload/libc/exec.c
M payloads/libpayload/libc/time.c
M payloads/libpayload/sample/Makefile
M payloads/libpayload/vboot/Kconfig
M payloads/libpayload/vboot/Makefile.mk
29 files changed, 924 insertions(+), 139 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/81968/22
--
To view, visit https://review.coreboot.org/c/coreboot/+/81968?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: I69fda47bedf1a14807b1515c4aed6e3a1d5b8585
Gerrit-Change-Number: 81968
Gerrit-PatchSet: 22
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newpatchset
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81967?usp=email )
Change subject: ec/google/chromeec: Do not fill TypeC ACPI device when UCSI is enabled
......................................................................
ec/google/chromeec: Do not fill TypeC ACPI device when UCSI is enabled
Do not fill the ACPI table entry associated with the cros_ec_typec
driver once we switch to the UCSI kernel driver. Skip the ACPI entry if
EC implements the UCSI_PPM feature, and the CBI flag to enable UCSI is
set.
BUG=b:333078787
TEST=emerge-brox coreboot chromeos-bootimage
Cq-Depend: chromium:5416841
Change-Id: I67dff6445aa7ba3ba48a04d1df3541f880d09d0a
Signed-off-by: Pavan Holla <pholla(a)google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/81967
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Caveh Jalali <caveh(a)chromium.org>
Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org>
---
M src/ec/google/chromeec/ec.c
M src/ec/google/chromeec/ec.h
M src/ec/google/chromeec/ec_acpi.c
3 files changed, 37 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Paul Menzel: Looks good to me, but someone else must approve
Caveh Jalali: Looks good to me, approved
diff --git a/src/ec/google/chromeec/ec.c b/src/ec/google/chromeec/ec.c
index 8fdfbfb..a32b2a5 100644
--- a/src/ec/google/chromeec/ec.c
+++ b/src/ec/google/chromeec/ec.c
@@ -723,6 +723,31 @@
return cbi_get_uint32(ssfc, CBI_TAG_SSFC);
}
+bool google_chromeec_get_ucsi_enabled(void)
+{
+ int rv;
+ union ec_common_control cc;
+
+ rv = google_chromeec_check_feature(EC_FEATURE_UCSI_PPM);
+ if (rv < 0) {
+ printk(BIOS_INFO, "Cannot check if EC_FEATURE_UCSI_PPM is available: status = %d\n", rv);
+ return false;
+ }
+
+ if (rv == 0)
+ return false;
+
+ /* Check if PPM is enabled at runtime. */
+ cc.ucsi_enabled = 0;
+ rv = cbi_get_uint32(&cc.raw_value, CBI_TAG_COMMON_CONTROL);
+ if (rv < 0) {
+ printk(BIOS_DEBUG, "Cannot get tag CBI_TAG_COMMON_CONTROL from CBI: status = %d\n", rv);
+ return false;
+ }
+
+ return (cc.ucsi_enabled != 0);
+}
+
static int cbi_get_string(char *buf, size_t bufsize, uint32_t tag)
{
struct ec_params_get_cbi params = {
diff --git a/src/ec/google/chromeec/ec.h b/src/ec/google/chromeec/ec.h
index 0e1df9c..0062df6 100644
--- a/src/ec/google/chromeec/ec.h
+++ b/src/ec/google/chromeec/ec.h
@@ -424,6 +424,13 @@
*/
bool google_chromeec_is_battery_present_and_above_critical_threshold(void);
+/**
+ * Determine if the UCSI stack is currently active.
+ *
+ * @return true if EC implements the UCSI stack
+ */
+bool google_chromeec_get_ucsi_enabled(void);
+
#if CONFIG(HAVE_ACPI_TABLES)
/**
* Writes USB Type-C PD related information to the SSDT
diff --git a/src/ec/google/chromeec/ec_acpi.c b/src/ec/google/chromeec/ec_acpi.c
index cf29636..e24f8ac 100644
--- a/src/ec/google/chromeec/ec_acpi.c
+++ b/src/ec/google/chromeec/ec_acpi.c
@@ -157,6 +157,11 @@
struct acpi_pld pld = {0};
uint32_t pcap_mask = 0;
+ /* UCSI implementations do not require an ACPI device with mux info since the
+ linux kernel doesn't set the muxes. */
+ if (google_chromeec_get_ucsi_enabled())
+ return;
+
rv = google_chromeec_get_num_pd_ports(&num_ports);
if (rv || num_ports == 0)
return;
--
To view, visit https://review.coreboot.org/c/coreboot/+/81967?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: I67dff6445aa7ba3ba48a04d1df3541f880d09d0a
Gerrit-Change-Number: 81967
Gerrit-PatchSet: 14
Gerrit-Owner: Pavan Holla <pholla(a)google.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Daisuke Nojiri <dnojiri(a)chromium.org>
Gerrit-CC: Forest Mittelberg <bmbm(a)google.com>
Gerrit-MessageType: merged