Attention is currently required from: Arthur Heymans, Christian Walter, Felix Held, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Shuo Liu, Tim Chu.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82034?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: soc/intel/xeon_sp: Add _OSC ASL generation utils for IIO stacks
......................................................................
Patch Set 2:
(8 comments)
Patchset:
PS2:
I wanted to give this more thought, but seeing superficial reviews
(which I blame for the accumulated days of unnecessary work on this
topic) again, put me into damage-control mode. Where I'll stay for
a while.
File src/soc/intel/xeon_sp/acpi/iiostack.asl:
https://review.coreboot.org/c/coreboot/+/82034/comment/a6f6fab1_b8127e07 :
PS2, Line 75: CreateDWordField (Local7, Zero, QSUP)
I fail to find any use of this.
https://review.coreboot.org/c/coreboot/+/82034/comment/a5ba77fb_78600255 :
PS2, Line 87: (OscArg0 == ToUUID (CXL_HOST_BRIDGE_OSC_UUID)))
Doesn't this need an `IsCxlDomain != 0` too?
https://review.coreboot.org/c/coreboot/+/82034/comment/340e1971_919165f5 :
PS2, Line 89: 0x02
3?
https://review.coreboot.org/c/coreboot/+/82034/comment/9e53b03b_36aeb66e :
PS2, Line 99: ToInteger
Isn't a DWord an Integer already?
https://review.coreboot.org/c/coreboot/+/82034/comment/323b5619_e08b2163 :
PS2, Line 100: Local1 = GrantedPCIeFeatures
Is it actually necessary to use local variables for this?
https://review.coreboot.org/c/coreboot/+/82034/comment/232aab9e_edcacedc :
PS2, Line 104: SUPL
What is SUPL?
https://review.coreboot.org/c/coreboot/+/82034/comment/2414c0aa_fafefd2e :
PS2, Line 117: 0x04
5?
--
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: 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: Shuo Liu <shuo.liu(a)intel.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 12:38:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Federico Amedeo Izzo.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/82010?usp=email
to look at the new patch set (#6).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: mb/aoostar: Add AOOSTAR R1 (WTR_R1)
......................................................................
mb/aoostar: Add AOOSTAR R1 (WTR_R1)
AOOSTAR R1 is a Chinese NAS based on Intel N100 (Alder Lake N), with
two 3.5" HDD slots, an M.2 NVMe 2280 SSD slot and a single DDR4 SODIMM
slot. It also comes with 2x 2.5Gb Intel NICs, Intel AX200 WiFi + BT
and USB-C Alt-DP Power Delivery.
Working:
- Automatic FAN control (IT8613E SuperIO)
- M.2 NVME slot
- 2x SATA ports (Issue on 3.5" HDD, see below)
- 2.0 USB ports
- USB-C port with Alt-DP and PD
- HDMI / DisplayPort ports
- 2x 2.5Gb NICs
- WiFi + BT
- ASPM (Unavailable on stock)
- Linux/Windows UEFI booting with EDK2
Broken:
- Power button (OFF->ON broken, ON->OFF works)
- 3.5" SATA HDDs (Detected only after reboot)
- USB 3.0 ports
Untested:
- Internal audio
- MicroSD card reader
- S3
My motivation for doing this port is enabling ASPM, as it makes a
great difference on idle power consumption (from 8.4W to 5W measured
from the wall).
The last remaining annoyance of this port is the power button not
working. I spent a few hours double checking the SuperIO registers but
then I gave up. A workaround for this is to use the "ON after power
loss" feature and reconnect the power cord to turn on the board.
It's not a big problem for a NAS that will stay ON 24/7.
VDT extracted from vendor BIOS.
Compiled with FSP GOP video initialization, using IFD descriptor,
ME blob and vgabios blob (ID 8086,0406) extracted from vendor BIOS.
The board can be flashed externally using a 1.8V adapter, I used a
CH341a modded for 3.3V I/O. Internal flashing could work too as SPI
flash is not read/write protected, but I haven't tried.
Patchset 5: Re-enabled dptf, added default options to Kconfig.
Change-Id: I9414eb742b6b90459e010b038c1994537e9801a5
Signed-off-by: Federico Amedeo Izzo <federico(a)izzo.pro>
---
A src/mainboard/aoostar/Kconfig
A src/mainboard/aoostar/Kconfig.name
A src/mainboard/aoostar/wtr_r1/Kconfig
A src/mainboard/aoostar/wtr_r1/Kconfig.name
A src/mainboard/aoostar/wtr_r1/Makefile.mk
A src/mainboard/aoostar/wtr_r1/board_info.txt
A src/mainboard/aoostar/wtr_r1/bootblock.c
A src/mainboard/aoostar/wtr_r1/data.vbt
A src/mainboard/aoostar/wtr_r1/devicetree.cb
A src/mainboard/aoostar/wtr_r1/dsdt.asl
A src/mainboard/aoostar/wtr_r1/gpio.h
A src/mainboard/aoostar/wtr_r1/romstage_fsp_params.c
12 files changed, 753 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/82010/6
--
To view, visit https://review.coreboot.org/c/coreboot/+/82010?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: I9414eb742b6b90459e010b038c1994537e9801a5
Gerrit-Change-Number: 82010
Gerrit-PatchSet: 6
Gerrit-Owner: Federico Amedeo Izzo <federico(a)izzo.pro>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Federico Amedeo Izzo <federico(a)izzo.pro>
Gerrit-MessageType: newpatchset
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:
(9 comments)
Patchset:
PS1:
> > > Why duplicate the x86 code into x64? Why not capture the differences with preprocessor as done i […]
Acknowledged
File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/81968/comment/4ccd8df5_959d3420 :
PS20, Line 168: default 0x80100000 if ARCH_X86_64
> why use a different address than 32bit code?
no reason, should be using the same addr
File payloads/libpayload/arch/x86/exec_64.S:
https://review.coreboot.org/c/coreboot/+/81968/comment/cff9c92c_d68a2688 :
PS20, Line 41: i386_do_exec
> I would expect 32bit code with this name.
Acknowledged
File payloads/libpayload/arch/x86/gdb.c:
https://review.coreboot.org/c/coreboot/+/81968/comment/4123917c_ecf936c5 :
PS20, Line 59: esp
> should be larger size if you do movq. […]
Acknowledged
File payloads/libpayload/arch/x86/head_64.S:
https://review.coreboot.org/c/coreboot/+/81968/comment/d478ccaf_e480cc67 :
PS20, Line 81: /* Enable special x86 functions if present. */
: push %rdi
: push %rsi
: push %rdx
: push %rcx
: push %r8
: push %r9
> Why save/restore registers that you don't use but not the ones you actually use like rax, rbx, rcx, rdx?
my bad, I misunderstood it with 64-bit ABIs.
File payloads/libpayload/arch/x86/libpayload_64.ldscript:
https://review.coreboot.org/c/coreboot/+/81968/comment/7b710074_e9309f38 :
PS20, Line 29: OUTPUT_FORMAT(elf64-x86-64)
: OUTPUT_ARCH(x86_64)
> Any reason for a separate linker script with the same content? You can add multiple OUTPUT_FORMAT and ARCH in here afaik?
as you know that coreinfo/nvramciu doesn't have support for x86_64 (and can only support i386 aka 32-bit mode) hence, we're temporarily using a separate linker script to avoid compilation issues. Once 64-bit support is added, we can consolidate to a single linker script using OUTPUT_ARCH(i386:x86_64) for multi-arch support
File payloads/libpayload/arch/x86/multiboot.c:
https://review.coreboot.org/c/coreboot/+/81968/comment/bec6268b_8e0a7174 :
PS20, Line 1: *
: *
: * Copyright (C) 2008 Advanced Micro Devices, Inc.
: *
> Multiboot2 predates long mode. I don't think this compilation unit should be added with long mode.
Acknowledged
https://review.coreboot.org/c/coreboot/+/81968/comment/d29fca17_150b9fb5 :
PS20, Line 117: if (loader_rdi != MULTIBOOT_MAGIC)
> Looking at how you intend to boot with will simply never be the case as RDI contains a pointer to the coreboot tables.
marking resolved based on https://review.coreboot.org/c/coreboot/+/81968/comment/9ec1549f_6d1a9277/
File payloads/libpayload/arch/x86/string.c:
https://review.coreboot.org/c/coreboot/+/81968/comment/9113a98c_2d26427d :
PS20, Line 84: asm volatile(
: "rep ; movsl\n\t"
: #if CONFIG(LP_ARCH_X86_64)
: "movq %4,%%rcx\n\t"
: #else
: "movl %4,%%ecx\n\t"
: #endif
: "rep ; movsb\n\t"
: : "=&c" (d0), "=&D" (d1), "=&S" (d2)
: : "0" (n >> 2), "g" (n & 3), "1" (dest), "2" (src)
: : "memory"
: );
> Increment by 8 bytes instead of 4.
>
>
> #if CONFIG(LP_ARCH_X86_64)
> asm volatile(
> "rep ; movsq\n\t"
> "mov %4,%%rcx\n\t"
> "rep ; movsb\n\t"
> : "=&c" (d0), "=&D" (d1), "=&S" (d2)
> : "0" (n >> 3), "g" (n & 7), "1" (dest), "2" (src)
> : "memory"
> );
> #else
> asm volatile(
> "rep ; movsl\n\t"
> "movl %4,%%ecx\n\t"
> "rep ; movsb\n\t"
> : "=&c" (d0), "=&D" (d1), "=&S" (d2)
> : "0" (n >> 2), "g" (n & 3), "1" (dest), "2" (src)
> : "memory"
> );
> #endif
thanks for identifying the mistake.
--
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 11:35:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Kapil Porwal, Subrata Banik.
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 (#21).
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, 895 insertions(+), 138 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/81968/21
--
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: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/81854?usp=email )
Change subject: gfxtest: Fix out-of-order components, permanently
......................................................................
Patch Set 1: Code-Review+1
(3 comments)
Commit Message:
https://review.coreboot.org/c/libgfxinit/+/81854/comment/fd3b540c_507166c8 :
PS1, Line 7: gfxtest: Fix out-of-order components, permanently
Technically, there is nothing to fix, how about:
gfxtest: Introduce From_RGB constructor for Pixel_Type
File gfxtest/hw-gfx-gma-gfx_test.adb:
https://review.coreboot.org/c/libgfxinit/+/81854/comment/ea109250_50215ab3 :
PS1, Line 60: for Pixel_Type use record
: Blue at 0 range 0 .. 7;
: Green at 1 range 0 .. 7;
: Red at 2 range 0 .. 7;
: Alpha at 3 range 0 .. 7;
: end record;
Could replace this with a `with Packed` for the record then.
https://review.coreboot.org/c/libgfxinit/+/81854/comment/296fd00a_b18bb3cf :
PS1, Line 67: RGB
or `From_RGBA`?
--
To view, visit https://review.coreboot.org/c/libgfxinit/+/81854?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: libgfxinit
Gerrit-Branch: main
Gerrit-Change-Id: I77dbdcd6c235e411585585779c31777adcef57d0
Gerrit-Change-Number: 81854
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Mon, 22 Apr 2024 11:01:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Chen, Gang C, David Hendricks, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Paul Menzel, TangYiwei.
Hello Arthur Heymans, Chen, Gang C, David Hendricks, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, TangYiwei, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/81322?usp=email
to look at the new patch set (#22).
Change subject: mb/intel/beechnutcity_crb: Add GNR/SRF-SP 2S server board Beechnut City
......................................................................
mb/intel/beechnutcity_crb: Add GNR/SRF-SP 2S server board Beechnut City
Beechnut City CRB is the 2 socket reference board for 6th Gen Xeon-SP
SP SoCs (Granite Rapids SP and Sierra Forest SP).
This patch initially sets the code set up as a compilation target with
GNR N-1 FSP, and with basic feature supports (Integrated IO Controller
(IIO) configuration, BMC, UART, HPET).
TEST=Build on intel/beechnutcity CRB
Change-Id: I3f6a0fb97b62baadb438fb9f11fdd78fccb3f89a
Signed-off-by: Shuo Liu <shuo.liu(a)intel.com>
Co-authored-by: Gang Chen <gang.c.chen(a)intel.com>
---
A configs/builder/config.intel.crb.bnc
A src/mainboard/intel/beechnutcity_crb/Kconfig
A src/mainboard/intel/beechnutcity_crb/Kconfig.name
A src/mainboard/intel/beechnutcity_crb/Makefile.mk
A src/mainboard/intel/beechnutcity_crb/board.fmd
A src/mainboard/intel/beechnutcity_crb/board_info.txt
A src/mainboard/intel/beechnutcity_crb/bootblock.c
A src/mainboard/intel/beechnutcity_crb/config/iio.c
A src/mainboard/intel/beechnutcity_crb/devicetree.cb
A src/mainboard/intel/beechnutcity_crb/dsdt.asl
A src/mainboard/intel/beechnutcity_crb/ramstage.c
A src/mainboard/intel/beechnutcity_crb/romstage.c
12 files changed, 269 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/81322/22
--
To view, visit https://review.coreboot.org/c/coreboot/+/81322?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: I3f6a0fb97b62baadb438fb9f11fdd78fccb3f89a
Gerrit-Change-Number: 81322
Gerrit-PatchSet: 22
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: TangYiwei
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: TangYiwei
Gerrit-MessageType: newpatchset