[coreboot-gerrit] Change in coreboot[master]: arch/x86: Align cbmem with CONFIG_STACK_SIZE
Subrata Banik (Code Review)
gerrit at coreboot.org
Tue May 15 13:05:14 CEST 2018
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/26288
Change subject: arch/x86: Align cbmem with CONFIG_STACK_SIZE
......................................................................
arch/x86: Align cbmem with CONFIG_STACK_SIZE
As per c_start.S implementation logic, struct cpu_info pointer
will be present at 8 bytes below stack_top(_estack).
Getting stack_base and stack_end using checkstack(_estack, 0)
function and comparing struct cpu_info *ci pointer address with
different STACK_SIZE.
STACK_SIZE=8K (0x2000)
stack_base = 0x85ccc000
stack_top = 0x85cce000
(struct cpu_info) ci = 0x85ccdff8
STACK_SIZE=16K (0x4000)
stack_base = 0x85cca000
stack_top = 0x85cce000
(struct cpu_info) ci = 0x85ccfff8
Due to wrong alignment issue, cpu_index variable is getting random/wrong
value.
BUG=b:79562868
TEST=Verify cpu_index is returning correct cpu_index of BSP and AP
with different STACK_SIZE 4K/8K/16K/32K on KBL and CNL platform.
Change-Id: Ifdb44a8a9902c9b3b393d01c1e288ac34f5e5e8e
Signed-off-by: Subrata Banik <subrata.banik at intel.com>
---
M src/Kconfig
M src/arch/x86/c_start.S
M src/arch/x86/include/arch/cpu.h
M src/cpu/x86/mp_init.c
M src/include/cbmem.h
M src/lib/ext_stage_cache.c
M src/lib/imd.c
M src/lib/memrange.c
M src/lib/rmodule.c
9 files changed, 27 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/26288/1
diff --git a/src/Kconfig b/src/Kconfig
index 99a704d..4bf76b9 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -395,6 +395,11 @@
default 0x1000 if ARCH_X86
default 0x0
+config STACK_ALIGNMENT_SIZE
+ hex
+ default STACK_SIZE if ARCH_X86
+ default 0x1000
+
config MAX_CPUS
int
default 1
diff --git a/src/arch/x86/c_start.S b/src/arch/x86/c_start.S
index 6426ef3..b9573ba 100644
--- a/src/arch/x86/c_start.S
+++ b/src/arch/x86/c_start.S
@@ -19,7 +19,7 @@
.global _stack
.global _estack
-.align CONFIG_STACK_SIZE
+.align CONFIG_STACK_ALIGNMENT_SIZE
_stack:
.space CONFIG_MAX_CPUS*CONFIG_STACK_SIZE
_estack:
diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h
index 5d44aae..7d0f182 100644
--- a/src/arch/x86/include/arch/cpu.h
+++ b/src/arch/x86/include/arch/cpu.h
@@ -206,8 +206,8 @@
"orl %2, %0 "
#endif
: "=r" (ci)
- : "0" (~(CONFIG_STACK_SIZE - 1)),
- "r" (CONFIG_STACK_SIZE - sizeof(struct cpu_info))
+ : "0" (~(CONFIG_STACK_ALIGNMENT_SIZE - 1)),
+ "r" (CONFIG_STACK_ALIGNMENT_SIZE - sizeof(struct cpu_info))
);
return ci;
}
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c
index ef576ec..8689904 100644
--- a/src/cpu/x86/mp_init.c
+++ b/src/cpu/x86/mp_init.c
@@ -317,8 +317,9 @@
return ap_count;
}
- if (rmodule_load_alignment(&sipi_mod) != 4096) {
- printk(BIOS_CRIT, "SIPI module load alignment(%d) != 4096.\n",
+ if (rmodule_load_alignment(&sipi_mod) != CONFIG_STACK_ALIGNMENT_SIZE) {
+ printk(BIOS_CRIT, "SIPI module load alignment(%d) != "
+ "CONFIG_STACK_ALIGNMENT_SIZE.\n",
rmodule_load_alignment(&sipi_mod));
return ap_count;
}
diff --git a/src/include/cbmem.h b/src/include/cbmem.h
index 007bc54..c1eae80 100644
--- a/src/include/cbmem.h
+++ b/src/include/cbmem.h
@@ -38,7 +38,7 @@
* may be removed, but the last one added is the only that can be removed.
*/
-#define DYN_CBMEM_ALIGN_SIZE (4096)
+#define DYN_CBMEM_ALIGN_SIZE CONFIG_STACK_ALIGNMENT_SIZE
#define CBMEM_ROOT_SIZE DYN_CBMEM_ALIGN_SIZE
/* The root region is at least DYN_CBMEM_ALIGN_SIZE . */
diff --git a/src/lib/ext_stage_cache.c b/src/lib/ext_stage_cache.c
index c3d4aee..5614b0f 100644
--- a/src/lib/ext_stage_cache.c
+++ b/src/lib/ext_stage_cache.c
@@ -40,7 +40,8 @@
imd_handle_init(imd, (void *)(size + (uintptr_t)base));
printk(BIOS_DEBUG, "External stage cache:\n");
- imd_create_tiered_empty(imd, 4096, 4096, 1024, 32);
+ imd_create_tiered_empty(imd, CONFIG_STACK_ALIGNMENT_SIZE,
+ CONFIG_STACK_ALIGNMENT_SIZE, 1024, 32);
if (imd_limit_size(imd, size))
printk(BIOS_DEBUG, "Could not limit stage cache size.\n");
}
diff --git a/src/lib/imd.c b/src/lib/imd.c
index d17cc81..2be5573 100644
--- a/src/lib/imd.c
+++ b/src/lib/imd.c
@@ -26,7 +26,7 @@
static const uint32_t IMD_ROOT_PTR_MAGIC = 0xc0389481;
static const uint32_t IMD_ENTRY_MAGIC = ~0xc0389481;
static const uint32_t SMALL_REGION_ID = CBMEM_ID_IMD_SMALL;
-static const size_t LIMIT_ALIGN = 4096;
+static const size_t LIMIT_ALIGN = CONFIG_STACK_ALIGNMENT_SIZE;
/* In-memory data structures. */
struct imd_root_pointer {
diff --git a/src/lib/memrange.c b/src/lib/memrange.c
index 96d7524..2adc478 100644
--- a/src/lib/memrange.c
+++ b/src/lib/memrange.c
@@ -228,12 +228,14 @@
if (size == 0)
return;
- /* The addresses are aligned to 4096 bytes: the begin address is
- * aligned down while the end address is aligned up to be conservative
- * about the full range covered. */
- begin = ALIGN_DOWN(base, 4096);
+ /*
+ * The addresses are aligned to CONFIG_STACK_ALIGNMENT_SIZE bytes:
+ * the begin address is aligned down while the end address is aligned
+ * up to be conservative about the full range covered.
+ */
+ begin = ALIGN_DOWN(base, CONFIG_STACK_ALIGNMENT_SIZE);
end = begin + size + (base - begin);
- end = ALIGN_UP(end, 4096) - 1;
+ end = ALIGN_UP(end, CONFIG_STACK_ALIGNMENT_SIZE) - 1;
action(ranges, begin, end, tag);
}
diff --git a/src/lib/rmodule.c b/src/lib/rmodule.c
index 66d5120..68939a6 100644
--- a/src/lib/rmodule.c
+++ b/src/lib/rmodule.c
@@ -177,7 +177,7 @@
* The base address where the program is loaded needs to be a multiple
* of the program's starting link address. That way all data alignment
* in the program is preserved. Default to 4KiB. */
- return 4096;
+ return CONFIG_STACK_ALIGNMENT_SIZE;
}
int rmodule_load(void *base, struct rmodule *module)
@@ -210,8 +210,8 @@
if (region_alignment & (region_alignment - 1))
BUG();
- if (region_alignment < 4096)
- region_alignment = 4096;
+ if (region_alignment < CONFIG_STACK_ALIGNMENT_SIZE)
+ region_alignment = CONFIG_STACK_ALIGNMENT_SIZE;
/* Sanity check rmodule_header size. The code below assumes it is less
* than the minimum alignment required. */
@@ -228,7 +228,8 @@
* to place the rmodule so that the program falls on the aligned
* address with the header just before it. Therefore, we need at least
* a page to account for the size of the header. */
- *region_size = ALIGN(rmodule_size + region_alignment, 4096);
+ *region_size = ALIGN(rmodule_size + region_alignment,
+ CONFIG_STACK_ALIGNMENT_SIZE);
/* The program starts immediately after the header. However,
* it needs to be aligned to a 4KiB boundary. Therefore, adjust the
* program location so that the program lands on a page boundary. The
--
To view, visit https://review.coreboot.org/26288
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifdb44a8a9902c9b3b393d01c1e288ac34f5e5e8e
Gerrit-Change-Number: 26288
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subrata.banik at intel.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180515/c4f8656a/attachment-0001.html>
More information about the coreboot-gerrit
mailing list