[M] Change in coreboot[main]: soc/intel/xeon_sp: Remove PAM unlock operations
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@intel.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter@9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin@wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang@gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu@quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@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@aheymans.xyz> Gerrit-MessageType: comment
participants (1)
-
Shuo Liu (Code Review)