Hello Patrick Rudolph, Subrata Banik, Balaji Manigandan, Aamir Bohra, Rizwan Qureshi, V Sowmya, build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35402
to look at the new patch set (#14).
Change subject: soc/intel/common/block/cse: Add boot partition related APIs
......................................................................
soc/intel/common/block/cse: Add boot partition related APIs
GET_BOOT_PARTITION_INFO - provides info on available partitions in the cse region.
The API provides info on boot partitions like start/end offsets of a partition
within CSE region, and their version and partition status.
SET_BOOT_PARTITION_INFO - Sets the next boot partition to boot for CSE.
With the HECI API, firmware can notify CSE to boot from BP1 or BP2 on next boot.
Also, made minor changes to group macro definitions of group ids, command ids and
renamed macro definations of hmrfpo commands.
Change-Id: Iaa62409c0616d5913d21374a8a6804f82258eb4f
Signed-off-by: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
---
M src/soc/intel/common/block/cse/Kconfig
M src/soc/intel/common/block/cse/Makefile.inc
M src/soc/intel/common/block/cse/cse.c
A src/soc/intel/common/block/cse/cse_bp.c
M src/soc/intel/common/block/include/intelblocks/cse.h
5 files changed, 390 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/35402/14
--
To view, visit https://review.coreboot.org/c/coreboot/+/35402
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaa62409c0616d5913d21374a8a6804f82258eb4f
Gerrit-Change-Number: 35402
Gerrit-PatchSet: 14
Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
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: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Andrey Petrov <anpetrov(a)fb.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35402 )
Change subject: soc/intel/common/block/cse: Add boot partition related APIs
......................................................................
Patch Set 13:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35402/13/src/soc/intel/common/bloc…
File src/soc/intel/common/block/cse/cse_bp.c:
https://review.coreboot.org/c/coreboot/+/35402/13/src/soc/intel/common/bloc…
PS13, Line 205: printk(BIOS_ERR, "Could not set CSE's next boot partition to %d\n", next_bp + 1);
line over 96 characters
https://review.coreboot.org/c/coreboot/+/35402/13/src/soc/intel/common/bloc…
File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35402/13/src/soc/intel/common/bloc…
PS13, Line 175: * Boot Partition Info Flag Minimum Recovery Mode Enabled
code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35402/13/src/soc/intel/common/bloc…
PS13, Line 175: * Boot Partition Info Flag Minimum Recovery Mode Enabled
please, no space before tabs
https://review.coreboot.org/c/coreboot/+/35402/13/src/soc/intel/common/bloc…
PS13, Line 234: uint8_t cse_get_bp_entry_version(uint8_t boot_partition, struct partition_fw_version *fw_version);
line over 96 characters
--
To view, visit https://review.coreboot.org/c/coreboot/+/35402
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaa62409c0616d5913d21374a8a6804f82258eb4f
Gerrit-Change-Number: 35402
Gerrit-PatchSet: 13
Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
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: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Andrey Petrov <anpetrov(a)fb.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Tue, 24 Sep 2019 22:35:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Hello Patrick Rudolph, Subrata Banik, Balaji Manigandan, Aamir Bohra, Rizwan Qureshi, V Sowmya, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35402
to look at the new patch set (#13).
Change subject: soc/intel/common/block/cse: Add boot partition related APIs
......................................................................
soc/intel/common/block/cse: Add boot partition related APIs
GET_BOOT_PARTITION_INFO - provides info on available partitions in the cse region.
The API provides info on boot partitions like start/end offsets of a partition
within CSE region, and their version and partition status.
SET_BOOT_PARTITION_INFO - Sets the next boot partition to boot for CSE.
With the HECI API, firmware can notify CSE to boot from BP1 or BP2 on next boot.
Also, made minor changes to group macro definitions of group ids, command ids and
renamed macro definations of hmrfpo commands.
Change-Id: Iaa62409c0616d5913d21374a8a6804f82258eb4f
Signed-off-by: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
---
M src/soc/intel/common/block/cse/Kconfig
M src/soc/intel/common/block/cse/Makefile.inc
M src/soc/intel/common/block/cse/cse.c
A src/soc/intel/common/block/cse/cse_bp.c
M src/soc/intel/common/block/include/intelblocks/cse.h
5 files changed, 389 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/35402/13
--
To view, visit https://review.coreboot.org/c/coreboot/+/35402
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaa62409c0616d5913d21374a8a6804f82258eb4f
Gerrit-Change-Number: 35402
Gerrit-PatchSet: 13
Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
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: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Andrey Petrov <anpetrov(a)fb.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35402 )
Change subject: soc/intel/common/block/cse: Add boot partition related APIs
......................................................................
Patch Set 12:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35402/12/src/soc/intel/common/bloc…
File src/soc/intel/common/block/cse/Kconfig:
https://review.coreboot.org/c/coreboot/+/35402/12/src/soc/intel/common/bloc…
PS12, Line 10: 3
> Prefer this stays 3, since all the current platforms have 3 and as said earlier this is not going to […]
Then, maybe its better to just have this as a macro if you are not expecting it to change?
https://review.coreboot.org/c/coreboot/+/35402/12/src/soc/intel/common/bloc…
File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35402/12/src/soc/intel/common/bloc…
PS12, Line 781: partition_id != BP1 && partition_id != BP2
> The BP contents can be basically categorized as below. […]
Those details should be captured in comments. Also, can you please point me to a spec where these details are covered?
https://review.coreboot.org/c/coreboot/+/35402/12/src/soc/intel/common/bloc…
File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35402/12/src/soc/intel/common/bloc…
PS12, Line 153: enum
> There are more places we will use these named BPs, for example in set_net_boot_partition, caller has […]
Can those instances use something like BP(1) and BP(2)?
--
To view, visit https://review.coreboot.org/c/coreboot/+/35402
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaa62409c0616d5913d21374a8a6804f82258eb4f
Gerrit-Change-Number: 35402
Gerrit-PatchSet: 12
Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
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: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Andrey Petrov <anpetrov(a)fb.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Tue, 24 Sep 2019 17:40:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31936 )
Change subject: [RFC] util/sconfig: Drop topology from early static tree
......................................................................
Patch Set 5:
> Patch Set 5:
>
> (1 comment)
>
> > Patch Set 5: Code-Review+1
> >
> > (1 comment)
> >
> > As long as we don't identify devices by their full path,
> > this makes absolute sense.
> >
> > Why the Kconfig, though? I'm eager to find out what
> > Jenkins has to say :)
>
> We need topology in romstages as long as we use some derivative of pcidev_on_root() to find node configs. While I don't like it, some bootblocks seemed to require static devicetree too. I would like to exclude static.c from bootblock-y, smm-y and maybe postcar-y too.
>
> Kconfig is introduced to allow make the change gradually.
Can you please provide examples of the requirements? One thing I know of is that we have some configuration sitting in device tree that we take advantage of for configuration of the system early. We typically just need those values. This sort of ties into Nico's CL where we could just reference the one object needed and get the chip info. With this change it should be struct device and whatever chip info struct. We could make struct device smaller even if we wanted... or we generate aliases for the chip info objects themselves. That last part is slightly different than what Nico was proposing in his CL.
What sort of requirements do we think we have?
- alias a struct device by name -- under what conditions? We should fill this one out as I'm not sure of all the cases. One of them is totally the code that is written that just has the one device that tries to locate.
- having config data early (bootblock, etc) but remove the bloat from the full device tree. In many of these scenarios we don't need the device itself. We just need the config object.
Anything else?
--
To view, visit https://review.coreboot.org/c/coreboot/+/31936
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icdc63123fb8bcb39a0fc86e1e60b8eebb909223c
Gerrit-Change-Number: 31936
Gerrit-PatchSet: 5
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Tue, 24 Sep 2019 14:33:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment