Attention is currently required from: Nico Huber, Arthur Heymans, Kyösti Mälkki.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52922 )
Change subject: nb/amd/{agesa,pi}: Avoid overflows during DRAM calculation
......................................................................
Patch Set 3:
(2 comments)
File src/northbridge/amd/agesa/family16kb/northbridge.c:
https://review.coreboot.org/c/coreboot/+/52922/comment/e6ca03cd_07f1531f
PS2, Line 48: temp = pci_read_config32(dev, 0x144 + (nodeid <<3)) & 0xff; //[47:40] at [7:0]
> Also no point in checking those since Kabini CONFIG_CPU_ADDR_BITS is 40, 40 bits are covered by 0x44 […]
Ack
File src/northbridge/amd/pi/00730F01/northbridge.c:
https://review.coreboot.org/c/coreboot/+/52922/comment/d29135a4_ae3a48ea
PS2, Line 53: temp = pci_read_config32(dev, 0x144 + (nodeid <<3)) & 0xff; //[47:40] at [7:0]
> Also for some reason, the 0x144 register is reserved on 00730F01, even in NDA BKDG. […]
Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/52922
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3b5c1df96c308ff50c8de104e213219a98f25e10
Gerrit-Change-Number: 52922
Gerrit-PatchSet: 3
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Comment-Date: Wed, 05 May 2021 13:19:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Arthur Heymans, Kyösti Mälkki.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52922 )
Change subject: nb/amd/{agesa,pi}: Avoid overflows during DRAM calculation
......................................................................
Patch Set 2:
(1 comment)
File src/northbridge/amd/agesa/family15tn/northbridge.c:
https://review.coreboot.org/c/coreboot/+/52922/comment/3fd15abb_2a2de7dd
PS2, Line 48: temp = pci_read_config32(dev, 0x144 + (nodeid <<3)) & 0xff; //[47:40] at [7:0]
> > resource_t is uint64_t so for completeness I could add the higher DRAM limit bits. […]
Added. I also kept the high DRAM base and limit calculations for Trinity, where it may make sense, because of 48 physical address bits on CPU.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52922
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3b5c1df96c308ff50c8de104e213219a98f25e10
Gerrit-Change-Number: 52922
Gerrit-PatchSet: 2
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Comment-Date: Wed, 05 May 2021 13:17:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Michał Żygowski, Kyösti Mälkki.
Hello build bot (Jenkins), Nico Huber, Arthur Heymans, Kyösti Mälkki,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/52922
to look at the new patch set (#3).
Change subject: nb/amd/{agesa,pi}: Avoid overflows during DRAM calculation
......................................................................
nb/amd/{agesa,pi}: Avoid overflows during DRAM calculation
Do not use get_dram_base_mask to calculate system DRAM limits. Shift
operation around values operating on base and mask were causing
overflows and thus incorrect system DRAM limit. Another function
returning base and limit in KiB has been developed to avoid data loss.
Keep DRAM high base and limit in calculations only for Trinity where
the physical CPU address bits is 48. Although it is almost impossible
to have a non-zero value there, the platform would have to support
nearly 256GB of RAM.
TEST=boot PC Engines apu1 2GB, apu2 4GB and apu3 2GB and boot Debian
with Linux 4.14
Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Change-Id: I3b5c1df96c308ff50c8de104e213219a98f25e10
---
M src/northbridge/amd/agesa/family14/northbridge.c
M src/northbridge/amd/agesa/family15tn/northbridge.c
M src/northbridge/amd/agesa/family16kb/northbridge.c
M src/northbridge/amd/pi/00730F01/northbridge.c
4 files changed, 142 insertions(+), 155 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/52922/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/52922
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3b5c1df96c308ff50c8de104e213219a98f25e10
Gerrit-Change-Number: 52922
Gerrit-PatchSet: 3
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Damien Zammit, Arthur Heymans, Patrick Rudolph.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52941 )
Change subject: nb/intel/pineview: Use cbfs mcache
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/52941
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1487ba9decd3aa22424a3ac111de7fbdb867d38d
Gerrit-Change-Number: 52941
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Damien Zammit
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Damien Zammit
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 05 May 2021 13:08:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Michał Żygowski, Kyösti Mälkki.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52922 )
Change subject: nb/amd/{agesa,pi}: Avoid overflows during DRAM calculation
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
File src/northbridge/amd/agesa/family15tn/northbridge.c:
https://review.coreboot.org/c/coreboot/+/52922/comment/54c7d19a_9dc722d7
PS2, Line 48: temp = pci_read_config32(dev, 0x144 + (nodeid <<3)) & 0xff; //[47:40] at [7:0]
> resource_t is uint64_t so for completeness I could add the higher DRAM limit bits.
Ok. Maybe add a comment or mention it in the commit message?
--
To view, visit https://review.coreboot.org/c/coreboot/+/52922
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3b5c1df96c308ff50c8de104e213219a98f25e10
Gerrit-Change-Number: 52922
Gerrit-PatchSet: 2
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Comment-Date: Wed, 05 May 2021 12:57:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Philipp Deppenwiese, Martin Roth, Angel Pons, Julius Werner.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51923 )
Change subject: security/tpm: Add option to init TPM in bootblock
......................................................................
Patch Set 6:
(1 comment)
File src/lib/bootblock.c:
https://review.coreboot.org/c/coreboot/+/51923/comment/faf93cf6_6d70edd0
PS6, Line 62: acpi_is_wakeup_s3
> Ah, I guess because of the power_state thingy? It's something I need to backport from Broadwell to Lynx Point in order to unify both platforms.
Not really. One just needs to be careful in romstage where this gets cleared. It also needs to be implemented in bootblock: https://review.coreboot.org/c/coreboot/+/52929/
--
To view, visit https://review.coreboot.org/c/coreboot/+/51923
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifacba5d9ab19b47968b4f2ed5731ded4aac55022
Gerrit-Change-Number: 51923
Gerrit-PatchSet: 6
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Wed, 05 May 2021 12:55:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph.
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/52929
to look at the new patch set (#2).
Change subject: sb/intel/common: Implement acpi_is_wakeup_s3()
......................................................................
sb/intel/common: Implement acpi_is_wakeup_s3()
acpi_is_wakeup_s3() requires acpi_get_sleep_type().
Change-Id: Ibe01863e685bcbc9652a72be0632cfbd83e18380
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/southbridge/intel/common/pmbase.c
1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/52929/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/52929
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibe01863e685bcbc9652a72be0632cfbd83e18380
Gerrit-Change-Number: 52929
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
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)mailbox.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Arthur Heymans, Kyösti Mälkki.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52922 )
Change subject: nb/amd/{agesa,pi}: Avoid overflows during DRAM calculation
......................................................................
Patch Set 2:
(3 comments)
File src/northbridge/amd/agesa/family15tn/northbridge.c:
https://review.coreboot.org/c/coreboot/+/52922/comment/1f7bb919_88db8007
PS2, Line 48: temp = pci_read_config32(dev, 0x144 + (nodeid <<3)) & 0xff; //[47:40] at [7:0]
> Yes it is gone, we would need to support at least ~256GB of DRAM memory to have any other value than […]
resource_t is uint64_t so for completeness I could add the higher DRAM limit bits.
File src/northbridge/amd/agesa/family16kb/northbridge.c:
https://review.coreboot.org/c/coreboot/+/52922/comment/37c66458_5cecfeed
PS2, Line 48: temp = pci_read_config32(dev, 0x144 + (nodeid <<3)) & 0xff; //[47:40] at [7:0]
> As above.
Also no point in checking those since Kabini CONFIG_CPU_ADDR_BITS is 40, 40 bits are covered by 0x44 register
File src/northbridge/amd/pi/00730F01/northbridge.c:
https://review.coreboot.org/c/coreboot/+/52922/comment/713d2ab0_0a14bf08
PS2, Line 53: temp = pci_read_config32(dev, 0x144 + (nodeid <<3)) & 0xff; //[47:40] at [7:0]
> As above
Also for some reason, the 0x144 register is reserved on 00730F01, even in NDA BKDG. Probably removed due to max 40 physical address bits.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52922
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3b5c1df96c308ff50c8de104e213219a98f25e10
Gerrit-Change-Number: 52922
Gerrit-PatchSet: 2
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Comment-Date: Wed, 05 May 2021 12:53:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Alexander Couzens, Patrick Rudolph.
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/52942 )
Change subject: cpu/intel/socket_p: Increase DCACHE_RAM_SIZE
......................................................................
cpu/intel/socket_p: Increase DCACHE_RAM_SIZE
All CPUs supported by this socket should have plenty of cache.
This allows the use of cbfs mcache on all platforms.
Change-Id: I0d6f7f9151ecd4c9fbbba4ed033dfda8724b6772
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/cpu/intel/socket_p/Kconfig
M src/mainboard/lenovo/t400/Kconfig
2 files changed, 1 insertion(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/52942/1
diff --git a/src/cpu/intel/socket_p/Kconfig b/src/cpu/intel/socket_p/Kconfig
index a7c8ab1..7e9cca3 100644
--- a/src/cpu/intel/socket_p/Kconfig
+++ b/src/cpu/intel/socket_p/Kconfig
@@ -13,7 +13,7 @@
config DCACHE_RAM_SIZE
hex
- default 0x8000
+ default 0x10000
config DCACHE_BSP_STACK_SIZE
hex
diff --git a/src/mainboard/lenovo/t400/Kconfig b/src/mainboard/lenovo/t400/Kconfig
index 809ea45..5b3ecf1 100644
--- a/src/mainboard/lenovo/t400/Kconfig
+++ b/src/mainboard/lenovo/t400/Kconfig
@@ -26,7 +26,6 @@
select MAINBOARD_HAS_LIBGFXINIT
select MAINBOARD_USES_IFD_GBE_REGION if !BOARD_LENOVO_R500
select INTEL_GMA_HAVE_VBT
- select NO_CBFS_MCACHE
config VBOOT
select VBOOT_VBNV_CMOS
--
To view, visit https://review.coreboot.org/c/coreboot/+/52942
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0d6f7f9151ecd4c9fbbba4ed033dfda8724b6772
Gerrit-Change-Number: 52942
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newchange