Attention is currently required from: Nico Huber, Jeremy Soller, Angel Pons.
Tim Crawford has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49104 )
Change subject: soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP SiliconInit
......................................................................
Patch Set 6:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/49104/comment/384f3082_6189bc00
PS5, Line 14: the TigerLake FSP Integration Guide.
> I will clarify the it to be the table override behavior in the message, and reference the section in […]
Done
https://review.coreboot.org/c/coreboot/+/49104/comment/d990bb8c_16489ca9
PS5, Line 23: Tested by checking lspci output on System76 galp3-c, oryp5, oryp6.
> These are WHL (galp3-c), CFL (oryp5, CB:47892), and CML (oryp6, CB:47768). […]
Done
File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/49104/comment/29a8388b_7c6d80c5
PS5, Line 552: 0
> No, what Nico means is that the resulting value of `segbusdevfuncregister` makes FSP try to rewrite […]
Ack
https://review.coreboot.org/c/coreboot/+/49104/comment/6ca257f9_c870f9ed
PS5, Line 561: dev = pcidev_path_on_root(PCH_DEVFN_XHCI);
: if (dev)
: pci_dev_set_subsystem(dev, dev->subsystem_vendor,
: dev->subsystem_device);
:
: dev = pcidev_path_on_root(PCH_DEVFN_HDA);
: if (dev)
: pci_dev_set_subsystem(dev, dev->subsystem_vendor,
: dev->subsystem_device);
> Yes. Based on your other comments, I believe this is the better option. […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/49104
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ieaa45ef7fa8e0da4a25b9174ded1ea0c5d9c4b4e
Gerrit-Change-Number: 49104
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Fri, 08 Jan 2021 15:13:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Crawford <tcrawford(a)system76.com>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Tim Crawford, Nico Huber, Jeremy Soller.
Hello Felix Singer, build bot (Jenkins), Nico Huber, Jeremy Soller, Angel Pons, Michael Niewöhner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/49104
to look at the new patch set (#6).
Change subject: soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP SiliconInit
......................................................................
soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP SiliconInit
Prevent the FSP from writing its default SVID SDID values of 8086:7270
for internal devices as this locks most of the registers. Allows the
subsystemid values set in devicetree to be used.
A description of this SSID table override behavior, along with example
code, is provided in the TigerLake FSP Integration Guide, section
15.178 ("SI_CONFIG Struct Reference").
The xHCI and HDA devices have RW/L registers rather than RW/O registers.
They can be written to multiple times but cannot be modified after
being locked, which happens during FspSiliconInit. Because coreboot
populates subsystem IDs after SiliconInit, these devices specifically
must be written beforehand or will otherwise be locked with their
default values of 0:0.
Tested by checking lspci output on System76 galp3-c (WHL), oryp5 (CFL),
and oryp6 (CML).
References:
- TigerLake FSP Integration Guide
- Intel Document Number 337868-002
Change-Id: Ieaa45ef7fa8e0da4a25b9174ded1ea0c5d9c4b4e
Signed-off-by: Jeremy Soller <jeremy(a)system76.com>
Signed-off-by: Tim Crawford <tcrawford(a)system76.com>
---
M src/soc/intel/cannonlake/fsp_params.c
1 file changed, 56 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/49104/6
--
To view, visit https://review.coreboot.org/c/coreboot/+/49104
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ieaa45ef7fa8e0da4a25b9174ded1ea0c5d9c4b4e
Gerrit-Change-Number: 49104
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Tim Crawford, Nico Huber, Jeremy Soller.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49104 )
Change subject: soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP SiliconInit
......................................................................
Patch Set 5: Code-Review+1
(1 comment)
File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/49104/comment/6ca82b65_81c5c2fb
PS5, Line 552: 0
> You're right. The PCI ID database doesn't list anything using it, but it does appear to be valid.
No, what Nico means is that the resulting value of `segbusdevfuncregister` makes FSP try to rewrite the PCI vendor/device ID register on the host bridge (PCI B:D.F 0:0.0), and it only works because said register is read-only.
--
To view, visit https://review.coreboot.org/c/coreboot/+/49104
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ieaa45ef7fa8e0da4a25b9174ded1ea0c5d9c4b4e
Gerrit-Change-Number: 49104
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Comment-Date: Fri, 08 Jan 2021 13:10:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Tim Crawford <tcrawford(a)system76.com>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Martin Roth, Paul Menzel, Stefan Reinauer.
Felix Friedlander has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49233 )
Change subject: payloads/external/tianocore/Kconfig: elaborate help for TIANOCORE_BOOTSPLASH_FILE
......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/49233/comment/1415b28f_c2b610f2
PS1, Line 7: payloads/external/tianocore/Kconfig: elaborate help for TIANOCORE_BOOTSPLASH_FILE
> The prefix does not need to be the path (often it can be used though). Some suggestions: […]
tianocore/Kconfig: Extend help for bootspash file
Seem OK?
File payloads/external/tianocore/Kconfig:
https://review.coreboot.org/c/coreboot/+/49233/comment/ca503a1e_12c05175
PS1, Line 100: and will be displayed slightly above the centre of
: the screen,
> I do not understand this. […]
From Microsoft's documentation on "boot screen components" (https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/boot-scre…):
"We recommend that the logo is placed with its center at 38.2% from the screen's top edge."
Someone on IRC seemed to be under the impression that the Coreboot TianoCore payload did exactly this, though I confess I haven't checked.
https://review.coreboot.org/c/coreboot/+/49233/comment/c6124265_554d27b0
PS1, Line 102: than your display.
> If you have it handy, it’d be great if you put the URL to the specification here too.
Happy to - unfortunately, the actual ACPI specification is a PDF, so I can't hyperlink directly to the relevant part. Is "See section 5.2.22 of the ACPI specification: https://uefi.org/specifications" OK?
--
To view, visit https://review.coreboot.org/c/coreboot/+/49233
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c6666bb0e49369fe8fe2ae3dc12c023f668da0
Gerrit-Change-Number: 49233
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Friedlander <felix(a)ffetc.net>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Fri, 08 Jan 2021 13:02:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: comment
Attention is currently required from: Martin Roth, Patrick Rudolph.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49228 )
Change subject: mb/emulation/qemu: Copy page tables to DRAM in assembly
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
File src/mainboard/emulation/qemu-i440fx/Kconfig:
https://review.coreboot.org/c/coreboot/+/49228/comment/fd68b806_95bb51e1
PS1, Line 35: ARCH_X86_64_PGTBL_LOC
Once you can migrate it to cbmem, could use the CAR symbols exposed with CONFIG_PAGING_IN_CACHE_AS_RAM and get rid of this?
--
To view, visit https://review.coreboot.org/c/coreboot/+/49228
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic0bdd2bef7197edd2e7488a8efdeba7eb4ab0dd4
Gerrit-Change-Number: 49228
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Comment-Date: Fri, 08 Jan 2021 12:33:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Sugnan Prabhu S has uploaded a new patch set (#72) to the change originally created by Rizwan Qureshi. ( https://review.coreboot.org/c/coreboot/+/27369 )
Change subject: soc/intel/basecode: Add support for updating ucode loaded via FIT
......................................................................
soc/intel/basecode: Add support for updating ucode loaded via FIT
Intel’s FIT (Firmware Interface Table) based MCU (microcode/pcode patch)
loading mechanism patches the microcode before CPU reset. In the current
Chromebooks, field updatable FW has to be first verified by vboot. Since
the MCU is loaded before reset, vboot cannot verify the same and hence
we end up restricting FIT based MCU update only from RO.
This patch implements a scheme which will allow chromebooks to update
MCU in the field.
Create 2 bootblocks (use INTEL_ADD_TOP_SWAP_BOOTBLOCK) each containing
their own FIT table. First bootblock FIT has pointers to MCUs
(in microcode_blob.bin) which resides in RO section. This will be used
in the recovery scenario and also when booting with top-swap disabled
i.e, RTC reset. Second bootblock (Normal mode) is identical to the first
one except the FIT. Insert an additional pointer to a MCU that will
reside in a staging area. Use the
CONFIG_INTEL_TOP_SWAP_FIT_ENTRY_FMAP_REG config to insert the address
of the staging area into FIT.
Top swap control bit in RTC (BUC) PCR register (0x3414) is used to
switch between the two bootblocks.
Reserve a region in the FMAP which is equal to or greater than the MCU
size specified in the BWG for a particular SoC (e.g., for Skylake/Kaby
Lake it is 192K). This is a RW region just like the RW_MRC_CACHE. MCU
from RW-A/RW-B will be copied to this region during boot. Protect this
staging area with a FPR.
Basic update flow:
In non-recovery mode, Once a slot has been selected and loaded, check if
the current slot MCU and RW staging MCU are same. If not, update the
staging area with the MCU found in the current slot and reset the
system.
Also, make sure that the top-swap is enabled in normal/developer mode
and disabled in recovery mode.
In order to enable the update feature:
* The mainboard chromeos.fmd should include a new region for staging MCU
e.g, RW_UCODE_STAGED.
* Select config INTEL_TOP_SWAP_MULTI_FIT_UCODE_UPDATE.
Add documentation to describing the MCU update procedure.
Update config name and Makefile.inc
TODO: Since this update mechanism is dealing mostly with a single MCU
it is best suited for systems where the CPU is soldered down and not
replaceable (socketed). Extend the update mechanism to systems where the
CPU is replaceable, by including multiple MCU for different CPUs.
TEST=Create an FW image for soraka and flash, create a
chromeos-firmwareupdate shellball with a newer MCU and perform an
update. Make sure that the currently loaded microcode version
matches the one in firmwareupdate.
Change-Id: Iab6ba36a2eb587f331fe522c778e2c430c8eb655
Signed-off-by: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Signed-off-by: dhaval v sharma <dhaval.v.sharma(a)intel.com>
Signed-off-by: Pandya, Varshit B <varshit.b.pandya(a)intel.com>
Signed-off-by: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
---
M Documentation/soc/intel/index.md
A Documentation/soc/intel/ucode_update/flash_layout.svg
A Documentation/soc/intel/ucode_update/microcode_update_model.md
M Makefile.inc
A src/soc/intel/common/basecode/fw_update/Kconfig
A src/soc/intel/common/basecode/fw_update/Makefile.inc
A src/soc/intel/common/basecode/fw_update/ucode_update.c
7 files changed, 667 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/27369/72
--
To view, visit https://review.coreboot.org/c/coreboot/+/27369
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iab6ba36a2eb587f331fe522c778e2c430c8eb655
Gerrit-Change-Number: 27369
Gerrit-PatchSet: 72
Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: dhaval v sharma <dhaval.v.sharma(a)intel.com>
Gerrit-CC: Dhaval Sharma <dhaval.v.sharma(a)intel.corp-partner.google.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-CC: rushikesh s kadam <rushikesh.s.kadam(a)intel.com>
Gerrit-MessageType: newpatchset