Hello Kyösti Mälkki, Aaron Durbin, Arthur Heymans, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36596
to look at the new patch set (#3).
Change subject: include: introduce update* for mmio operations
......................................................................
include: introduce update* for mmio operations
Introduce update* as equivalent of pci_update* for mmio operations.
Change-Id: I9f188d586c09ee9f1e17290563f68970603204fb
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
A src/include/mmio.h
1 file changed, 45 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/36596/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/36596
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9f188d586c09ee9f1e17290563f68970603204fb
Gerrit-Change-Number: 36596
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner
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: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36596 )
Change subject: include: introduce update* for mmio operations
......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36596/2/src/include/mmio.h
File src/include/mmio.h:
https://review.coreboot.org/c/coreboot/+/36596/2/src/include/mmio.h@13
PS2, Line 13:
> Ack
Done
https://review.coreboot.org/c/coreboot/+/36596/2/src/include/mmio.h@20
PS2, Line 20: const
> um, what? […]
oooops :'D
https://review.coreboot.org/c/coreboot/+/36596/1/src/include/mmio.h
File src/include/mmio.h:
https://review.coreboot.org/c/coreboot/+/36596/1/src/include/mmio.h@19
PS1, Line 19: static __always_inline void update8(const volatile void *addr, uint8_t mask, uint8_t or)
> now I hate readX/writeX being bare w/o a namespace. […]
I'm a bit lost now as my parser doesn't seem to work today... :D so, did I get this right that we leave them as is and keep short names for update*(), too?
--
To view, visit https://review.coreboot.org/c/coreboot/+/36596
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9f188d586c09ee9f1e17290563f68970603204fb
Gerrit-Change-Number: 36596
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner
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: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Wed, 06 Nov 2019 19:02:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org>
Comment-In-Reply-To: Michael Niewöhner
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36553 )
Change subject: soc/intel/tigerlake/acpi: Copy acpi directory from icelake
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/36553/4//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/36553/4//COMMIT_MSG@12
PS4, Line 12: .
> Any other changes made from icelake in recent patchsets?
I don't think BDF has changed for any controller. It looks the same as ICL.
I do notice the following changes:
1. Removed Descriptor Name for Memory mapped SPI flash and local APIC in northbridge.asl
2. Rearranged code in gpio.asl to move RBUF object under _CRS and made the file use ASL2.0 syntax.
Can you please add these to commit message?
--
To view, visit https://review.coreboot.org/c/coreboot/+/36553
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If967cb5904f543ce21eb6e89421df0e5673d2238
Gerrit-Change-Number: 36553
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Wed, 06 Nov 2019 19:01:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36087 )
Change subject: soc/intel/tigerlake: Do initial SoC commit till ramstage
......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36087/15//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/36087/15//COMMIT_MSG@22
PS15, Line 22:
Subrata, there are a number of patchsets here. Just want to ensure that this list of changes is still valid and updated? Or are there more changes in this patchset than captured here?
--
To view, visit https://review.coreboot.org/c/coreboot/+/36087
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id95e2fa9b7a7c6b3b9233d2c438b25a6c4904bbb
Gerrit-Change-Number: 36087
Gerrit-PatchSet: 15
Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
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: John Zhao <john.zhao(a)intel.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Wed, 06 Nov 2019 18:20:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36589 )
Change subject: cpu/x86/smm: Add helper functions to deal with different save states
......................................................................
Patch Set 3:
> Patch Set 3:
>
> We have something similar in src/soc/intel/common/block/include/intelblocks/smihandler.h and src/soc/intel/common/block/smm/smihandler.c where the ops are about getting and setting a register.
>
> 1. Why is this about pointers to registers and not just getting and setting the explicit register?
Because the size of the registers is not always the same (eax on legacy vs rax on x86_64). Should legacy just pretend to be 64bit and ignore the upper values on get/set?
> 2. I prefer the ops-based approach because the chipset can say "these are my ops" and the other save state versions are garbage collected instead of carrying support for save state versions in the binary.
sounds good.
> 3. Are we planning on converging/sharing the implementation?
It's probably not hard to do that, so yes?
--
To view, visit https://review.coreboot.org/c/coreboot/+/36589
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I85325a25dc38d7cfc7eb0dfde621e5ebfeef0e15
Gerrit-Change-Number: 36589
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
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-CC: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Wed, 06 Nov 2019 17:56:22 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36341 )
Change subject: security/vboot: Build vboot library with same .a that depthcharge uses
......................................................................
Patch Set 18:
> Patch Set 18:
>
> This is all good but it still leaves the problem that we need to deal with the TLCL duplicate references (see my comment early on). Are you guys clear on who's going to deal with that (Tim or Joel)?
I'm going to look at that today.
--
To view, visit https://review.coreboot.org/c/coreboot/+/36341
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I905b781c3596965ec7ef45a2a7eafe15fdd4d9cc
Gerrit-Change-Number: 36341
Gerrit-PatchSet: 18
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Joel Kitching <kitching(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Wed, 06 Nov 2019 16:46:14 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36589 )
Change subject: cpu/x86/smm: Add helper functions to deal with different save states
......................................................................
Patch Set 3:
We have something similar in src/soc/intel/common/block/include/intelblocks/smihandler.h and src/soc/intel/common/block/smm/smihandler.c where the ops are about getting and setting a register.
1. Why is this about pointers to registers and not just getting and setting the explicit register?
2. I prefer the ops-based approach because the chipset can say "these are my ops" and the other save state versions are garbage collected instead of carrying support for save state versions in the binary.
3. Are we planning on converging/sharing the implementation?
--
To view, visit https://review.coreboot.org/c/coreboot/+/36589
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I85325a25dc38d7cfc7eb0dfde621e5ebfeef0e15
Gerrit-Change-Number: 36589
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
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-CC: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Wed, 06 Nov 2019 16:22:35 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment