Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81349?usp=email )
Change subject: soc/intel/xeon_sp: Remove PAM unlock operations
......................................................................
Patch Set 15:
(2 comments)
Patchset:
PS15:
> It's moved from romstage to ramstage on platforms that we think need it. […]
Sure, unlock_pam_region() will route the access to PAM-F segment to DRAM, coreboot places ACPI RSDP lower pointer here, without RSDP pointer, LinuxBoot cannot boot.
In SPR, FSP covered such unlock operations (confirmed from source code), and after removed from coreboot, the coreboot ACPI table can still be accessed by LinuxBoot, hence it proves the removal is okay for SPR. (for GNR, the same).
For the calling site of the unlock in CPX and SKX, it is moved from late romstage and early ramstage, which is close to each other. From the ACPI table write point of view, they are similar. However, only compilation test are executed for SKX and CPX.
File src/soc/intel/xeon_sp/cpx/chip.c:
https://review.coreboot.org/c/coreboot/+/81349/comment/95563f6e_6ac2b76e :
PS15, Line 164: /* Only call this code from socket0! */
: static void unlock_pam_regions(void)
: {
: uint32_t pam0123_unlock_dram = 0x33333330;
: uint32_t pam456_unlock_dram = 0x00333333;
: /* Get UBOX(1) for socket0 */
: uint32_t bus1 = socket0_get_ubox_busno(PCU_IIO_STACK);
:
: /* Assume socket0 owns PCI segment 0 */
: pci_io_write_config32(PCI_DEV(bus1, SAD_ALL_DEV, SAD_ALL_FUNC),
: SAD_ALL_PAM0123_CSR, pam0123_unlock_dram);
: pci_io_write_config32(PCI_DEV(bus1, SAD_ALL_DEV, SAD_ALL_FUNC),
: SAD_ALL_PAM456_CSR, pam456_unlock_dram);
:
: uint32_t reg1 = pci_io_read_config32(PCI_DEV(bus1, SAD_ALL_DEV,
: SAD_ALL_FUNC), SAD_ALL_PAM0123_CSR);
: uint32_t reg2 = pci_io_read_config32(PCI_DEV(bus1, SAD_ALL_DEV,
: SAD_ALL_FUNC), SAD_ALL_PAM456_CSR);
: printk(BIOS_DEBUG, "%s:%s pam0123_csr: 0x%x, pam456_csr: 0x%x\n",
: __FILE__, __func__, reg1, reg2);
: }
> Why is this code duplicated in cpx and skx. It was also valid for SPR-SP but not used. […]
This is because this code cannot pass compilation with the GNR target. If we would like to further share the codes, I can make another patch to put this into chip_gen1.c. Your opinion?
--
To view, visit https://review.coreboot.org/c/coreboot/+/81349?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: I3fd1d806807449e6a4d9d4d2c8a47ce61ed53018
Gerrit-Change-Number: 81349
Gerrit-PatchSet: 15
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
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: 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-Comment-Date: Mon, 01 Apr 2024 10:19:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Frank Chu has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/81445?usp=email )
Change subject: mb/google/nissa/var/glassway: Set VccIn Aux Imon IccMax to 25 A
......................................................................
Abandoned
--
To view, visit https://review.coreboot.org/c/coreboot/+/81445?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: I105dc9df53c624fd7fc697408a1097e023a3cd68
Gerrit-Change-Number: 81445
Gerrit-PatchSet: 8
Gerrit-Owner: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Shawn Ku <shawnku(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com>
Gerrit-CC: Derek Huang <derekhuang(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: abandon
Attention is currently required from: Andrey Petrov, Arthur Heymans, Bora Guvendik, Christian Walter, Felix Held, Fred Reitberger, Jason Glenesk, Johnny Lin, Jérémy Compostella, Matt DeVillier, Patrick Rudolph, Paul Menzel, Ronak Kanabar, Shuo Liu, Tim Chu, Wonkyu Kim.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80277?usp=email )
Change subject: drivers/intel/fsp2_0: Support FSP 2.4 64-bits
......................................................................
Patch Set 30:
(3 comments)
Patchset:
PS30:
can you please try to split this changes to first accommodate
1. uint32_t status to efi_return_status_t status and associated changes in function to print the debug msg
2. keep 64-bit related change
File 3rdparty/vboot:
https://review.coreboot.org/c/coreboot/+/80277/comment/20e93ef1_7b909ee2 :
PS30, Line 1: Subproject commit 493f7afc9d6b19c97498713a26e72992c60b3ede
remove this file
File src/include/efi/efi_datatype.h:
https://review.coreboot.org/c/coreboot/+/80277/comment/d257fb00_38e48b01 :
PS30, Line 65: EFI_STATUS
nit: UINTN?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80277?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: If0397f5cc8d0f4f1872bd37a001fe42e0c37ec99
Gerrit-Change-Number: 80277
Gerrit-PatchSet: 30
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Christian Walter <christian.walter(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: Johnny Lin <Johnny_Lin(a)wiwynn.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: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Appukuttan V K <appukuttan.vk(a)intel.com>
Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-CC: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(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: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Mon, 01 Apr 2024 09:25:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Andrey Petrov, Paul Menzel, Ronak Kanabar, Subrata Banik.
Hello Andrey Petrov, Paul Menzel, Ronak Kanabar, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/81615?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Code-Review+1 by Paul Menzel, Verified-1 by build bot (Jenkins)
Change subject: drivers/intel/fsp2_0: Enhance portability with uintptr_t/size_t
......................................................................
drivers/intel/fsp2_0: Enhance portability with uintptr_t/size_t
Replace fixed-width integers for pointers and sizes with uintptr_t and
size_t, promoting portability across 32-bit and 64-bit architectures.
For FSP-API specific UPD assignments, rely on `efi_uintn_t` rather
fixed size datatype uint32_t/uint64_t.
Fix the compilation issue for google/skyrim due to change in fixed-width
integers while calling into bmp_load_logo() function.
BUG=b:242829490
TEST=Firmware splash screen visible on google/rex0 w/ both 32-bit and
64-bit compilation.
Change-Id: Iab5c612e0640441a2a10e77949416de2afdb8985
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/drivers/intel/fsp2_0/fsp_gop_blt.c
M src/drivers/intel/fsp2_0/include/fsp/fsp_gop_blt.h
M src/include/bootsplash.h
M src/lib/bmp_logo.c
M src/soc/amd/mendocino/fsp_s_params.c
5 files changed, 12 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/81615/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/81615?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: Iab5c612e0640441a2a10e77949416de2afdb8985
Gerrit-Change-Number: 81615
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-MessageType: newpatchset
Subrata Banik has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/81617?usp=email )
Change subject: soc/amd/mendocino: Use platform-independent datatype for bmp_load_logo
......................................................................
Abandoned
--
To view, visit https://review.coreboot.org/c/coreboot/+/81617?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: I038ee7cf5b9544c518e6e21e47b89772a10b8e41
Gerrit-Change-Number: 81617
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(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: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-MessageType: abandon