Change in coreboot[master]: nb/intel/sandybridge: Improve cbmem_top_chipset calculation

[Angel Pons (Code Review)]

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45923 )

Change subject: nb/intel/sandybridge: Improve cbmem_top_chipset calculation
......................................................................

nb/intel/sandybridge: Improve cbmem_top_chipset calculation

Lock bit in TSEGMB register wasn't accounted for in `cbmem_top_chipset`.
Align down TSEG base to 1 MiB granularity to avoid surprises.

Change-Id: I74882db99502ae77c94d43d850533a4f76da2773
Signed-off-by: Angel Pons 
---
M src/northbridge/intel/sandybridge/memmap.c
1 file changed, 10 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/45923/1

diff --git a/src/northbridge/intel/sandybridge/memmap.c b/src/northbridge/intel/sandybridge/memmap.c
index b0a5149..7f46d66 100644
--- a/src/northbridge/intel/sandybridge/memmap.c
+++ b/src/northbridge/intel/sandybridge/memmap.c
@@ -13,20 +13,10 @@
 #include <stddef.h>
 #include <stdint.h>

-static uintptr_t smm_region_start(void)
-{
-	/* Base of TSEG is top of usable DRAM */
-	return pci_read_config32(HOST_BRIDGE, TSEGMB);
-}
-
-void *cbmem_top_chipset(void)
-{
-	return (void *)smm_region_start();
-}
-
 static uintptr_t northbridge_get_tseg_base(void)
 {
-	return ALIGN_DOWN(smm_region_start(), 1 * MiB);
+	/* TSEG has 1 MiB granularity, and bit 0 is a lock */
+	return ALIGN_DOWN(pci_read_config32(HOST_BRIDGE, TSEGMB), 1 * MiB);
 }

static size_t northbridge_get_tseg_size(void)
@@ -34,6 +24,14 @@
    return CONFIG_SMM_TSEG_SIZE;
 }

+void *cbmem_top_chipset(void)
+{
+	/* If DPR is disabled, base of TSEG is top of usable DRAM */
+	uintptr_t top_of_ram = northbridge_get_tseg_base();
+
+	return (void *)top_of_ram;
+}
+
 void smm_region(uintptr_t *start, size_t *size)
 {
    *start = northbridge_get_tseg_base();

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45923 )

Change subject: nb/intel/sandybridge: Improve cbmem_top_chipset calculation
......................................................................

Patch Set 2: Code-Review+2

[Angel Pons (Code Review)]

Angel Pons has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/45923 )

Change subject: nb/intel/sandybridge: Improve cbmem_top_chipset calculation
......................................................................

Removed Verified+1 by build bot (Jenkins)

[Angel Pons (Code Review)]

Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45923 )

Change subject: nb/intel/sandybridge: Improve cbmem_top_chipset calculation
......................................................................

nb/intel/sandybridge: Improve cbmem_top_chipset calculation

Lock bit in TSEGMB register wasn't accounted for in `cbmem_top_chipset`.
Align down TSEG base to 1 MiB granularity to avoid surprises.

Change-Id: I74882db99502ae77c94d43d850533a4f76da2773
Signed-off-by: Angel Pons 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/45923
Tested-by: build bot (Jenkins) 
Reviewed-by: Arthur Heymans 
---
M src/northbridge/intel/sandybridge/memmap.c
1 file changed, 10 insertions(+), 12 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Arthur Heymans: Looks good to me, approved

diff --git a/src/northbridge/intel/sandybridge/memmap.c b/src/northbridge/intel/sandybridge/memmap.c
index b0a5149..7f46d66 100644
--- a/src/northbridge/intel/sandybridge/memmap.c
+++ b/src/northbridge/intel/sandybridge/memmap.c
@@ -13,20 +13,10 @@
 #include <stddef.h>
 #include <stdint.h>

-static uintptr_t smm_region_start(void)
-{
-	/* Base of TSEG is top of usable DRAM */
-	return pci_read_config32(HOST_BRIDGE, TSEGMB);
-}
-
-void *cbmem_top_chipset(void)
-{
-	return (void *)smm_region_start();
-}
-
 static uintptr_t northbridge_get_tseg_base(void)
 {
-	return ALIGN_DOWN(smm_region_start(), 1 * MiB);
+	/* TSEG has 1 MiB granularity, and bit 0 is a lock */
+	return ALIGN_DOWN(pci_read_config32(HOST_BRIDGE, TSEGMB), 1 * MiB);
 }

static size_t northbridge_get_tseg_size(void)
@@ -34,6 +24,14 @@
    return CONFIG_SMM_TSEG_SIZE;
 }

+void *cbmem_top_chipset(void)
+{
+	/* If DPR is disabled, base of TSEG is top of usable DRAM */
+	uintptr_t top_of_ram = northbridge_get_tseg_base();
+
+	return (void *)top_of_ram;
+}
+
 void smm_region(uintptr_t *start, size_t *size)
 {
    *start = northbridge_get_tseg_base();