Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30684
Change subject: [WIP] Reduce (void **) pointer usage ......................................................................
[WIP] Reduce (void **) pointer usage
Use uintptr_t * instead.
Change-Id: I8ef5d9fad0298c79806fd94ca053027822022ac2 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/intel/haswell/stage_cache.c M src/cpu/intel/model_206ax/stage_cache.c M src/drivers/intel/fsp1_1/include/fsp/memmap.h M src/drivers/intel/fsp1_1/raminit.c M src/drivers/intel/fsp1_1/ramstage.c M src/drivers/intel/fsp1_1/stack.c M src/drivers/intel/fsp1_1/stage_cache.c M src/drivers/intel/fsp2_0/include/fsp/memmap.h M src/drivers/intel/fsp2_0/stage_cache.c M src/include/stage_cache.h M src/lib/ext_stage_cache.c M src/soc/amd/stoneyridge/cpu.c M src/soc/amd/stoneyridge/include/soc/northbridge.h M src/soc/amd/stoneyridge/ramtop.c M src/soc/amd/stoneyridge/romstage.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/baytrail/stage_cache.c M src/soc/intel/braswell/cpu.c M src/soc/intel/braswell/memmap.c M src/soc/intel/braswell/northcluster.c M src/soc/intel/broadwell/stage_cache.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/cannonlake/smmrelocate.c M src/soc/intel/common/block/include/intelblocks/smm.h M src/soc/intel/common/block/smm/smm.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/denverton_ns/include/soc/smm.h M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/denverton_ns/romstage.c M src/soc/intel/icelake/memmap.c M src/soc/intel/icelake/smmrelocate.c M src/soc/intel/skylake/memmap.c M src/soc/intel/skylake/romstage/romstage_fsp20.c M src/soc/intel/skylake/smmrelocate.c 35 files changed, 102 insertions(+), 124 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/30684/1
diff --git a/src/cpu/intel/haswell/stage_cache.c b/src/cpu/intel/haswell/stage_cache.c index 009cc09..fb4a6c3 100644 --- a/src/cpu/intel/haswell/stage_cache.c +++ b/src/cpu/intel/haswell/stage_cache.c @@ -17,10 +17,10 @@ #include <stage_cache.h> #include "haswell.h"
-void stage_cache_external_region(void **base, size_t *size) +void stage_cache_external_region(uintptr_t *base, size_t *size) { /* The ramstage cache lives in the TSEG region at RESERVED_SMM_OFFSET. * The top of RAM is defined to be the TSEG base address. */ *size = RESERVED_SMM_SIZE; - *base = (void *)((uint32_t)cbmem_top() + RESERVED_SMM_OFFSET); + *base = cbmem_top() + RESERVED_SMM_OFFSET; } diff --git a/src/cpu/intel/model_206ax/stage_cache.c b/src/cpu/intel/model_206ax/stage_cache.c index 26dc5e0..a0aaba5 100644 --- a/src/cpu/intel/model_206ax/stage_cache.c +++ b/src/cpu/intel/model_206ax/stage_cache.c @@ -17,12 +17,12 @@ #include <stage_cache.h> #include "model_206ax.h"
-void stage_cache_external_region(void **base, size_t *size) +void stage_cache_external_region(uintptr_t *base, size_t *size) { /* * The ramstage cache lives in the TSEG region at RESERVED_SMM_OFFSET. * The top of RAM is defined to be the TSEG base address. */ *size = RESERVED_SMM_SIZE; - *base = (void *)((uintptr_t)cbmem_top() + RESERVED_SMM_OFFSET); + *base = cbmem_top() + RESERVED_SMM_OFFSET; } diff --git a/src/drivers/intel/fsp1_1/include/fsp/memmap.h b/src/drivers/intel/fsp1_1/include/fsp/memmap.h index 965bce6..49bd019 100644 --- a/src/drivers/intel/fsp1_1/include/fsp/memmap.h +++ b/src/drivers/intel/fsp1_1/include/fsp/memmap.h @@ -27,7 +27,7 @@
/* Fills in the arguments for the entire SMM region covered by chipset * protections. e.g. TSEG. */ -void smm_region(void **start, size_t *size); +void smm_region(uintptr_t *start, size_t *size);
enum { /* SMM handler area. */ @@ -42,6 +42,6 @@
/* Fills in the start and size for the requested SMM subregion. Returns * 0 on susccess, < 0 on failure. */ -int smm_subregion(int sub, void **start, size_t *size); +int smm_subregion(int sub, uintptr_t *start, size_t *size);
#endif /* _COMMON_MEMMAP_H_ */ diff --git a/src/drivers/intel/fsp1_1/raminit.c b/src/drivers/intel/fsp1_1/raminit.c index 7fed2a1..ad9e9dd 100644 --- a/src/drivers/intel/fsp1_1/raminit.c +++ b/src/drivers/intel/fsp1_1/raminit.c @@ -147,12 +147,12 @@
/* Display SMM area */ #if IS_ENABLED(CONFIG_HAVE_SMI_HANDLER) - char *smm_base; + uintptr_t smm_base; size_t smm_size;
- smm_region((void **)&smm_base, &smm_size); + smm_region(&smm_base, &smm_size); printk(BIOS_DEBUG, "0x%08x: smm_size\n", (unsigned int)smm_size); - printk(BIOS_DEBUG, "0x%p: smm_base\n", smm_base); + printk(BIOS_DEBUG, "0x%08x: smm_base\n", (unsigned int)smm_base); #endif
/* Migrate CAR data */ diff --git a/src/drivers/intel/fsp1_1/ramstage.c b/src/drivers/intel/fsp1_1/ramstage.c index 864ab45..41d5437 100644 --- a/src/drivers/intel/fsp1_1/ramstage.c +++ b/src/drivers/intel/fsp1_1/ramstage.c @@ -33,19 +33,19 @@ /* Display SMM memory map */ static void smm_memory_map(void) { - void *base; + uintptr_t base; size_t size; int i;
printk(BIOS_SPEW, "SMM Memory Map\n");
smm_region(&base, &size); - printk(BIOS_SPEW, "SMRAM : %p 0x%zx\n", base, size); + printk(BIOS_SPEW, "SMRAM : 0x%08x 0x%zx\n", (unsigned int)base, size);
for (i = 0; i < SMM_SUBREGION_NUM; i++) { if (smm_subregion(i, &base, &size)) continue; - printk(BIOS_SPEW, " Subregion %d: %p 0x%zx\n", i, base, size); + printk(BIOS_SPEW, " Subregion %d: 0x%08x 0x%zx\n", i, (unsigned int)base, size); } }
diff --git a/src/drivers/intel/fsp1_1/stack.c b/src/drivers/intel/fsp1_1/stack.c index eb2a637..1b748aa 100644 --- a/src/drivers/intel/fsp1_1/stack.c +++ b/src/drivers/intel/fsp1_1/stack.c @@ -117,9 +117,8 @@ num_mtrrs++;
#if IS_ENABLED(CONFIG_HAVE_SMI_HANDLER) - void *smm_base; + uintptr_t smm_base; size_t smm_size; - uint32_t tseg_base;
/* * Cache the TSEG region at the top of ram. This region is not @@ -128,11 +127,10 @@ * the SMM handler as well as using the TSEG region for other purposes. */ smm_region(&smm_base, &smm_size); - tseg_base = (uint32_t)smm_base; slot = stack_push32(slot, mtrr_mask_upper); /* upper mask */ slot = stack_push32(slot, ~(alignment - 1) | MTRR_PHYS_MASK_VALID); slot = stack_push32(slot, 0); /* upper base */ - slot = stack_push32(slot, tseg_base | MTRR_TYPE_WRBACK); + slot = stack_push32(slot, (uint32_t)smm_base | MTRR_TYPE_WRBACK); num_mtrrs++; #endif
diff --git a/src/drivers/intel/fsp1_1/stage_cache.c b/src/drivers/intel/fsp1_1/stage_cache.c index 2d594e6..4895340 100644 --- a/src/drivers/intel/fsp1_1/stage_cache.c +++ b/src/drivers/intel/fsp1_1/stage_cache.c @@ -18,11 +18,11 @@ #include <fsp/memmap.h> #include <stage_cache.h>
-void stage_cache_external_region(void **base, size_t *size) +void stage_cache_external_region(uintptr_t *base, size_t *size) { if (smm_subregion(SMM_SUBREGION_CACHE, base, size)) { printk(BIOS_ERR, "ERROR: No cache SMM subregion.\n"); - *base = NULL; + *base = 0; *size = 0; } } diff --git a/src/drivers/intel/fsp2_0/include/fsp/memmap.h b/src/drivers/intel/fsp2_0/include/fsp/memmap.h index 965bce6..49bd019 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/memmap.h +++ b/src/drivers/intel/fsp2_0/include/fsp/memmap.h @@ -27,7 +27,7 @@
/* Fills in the arguments for the entire SMM region covered by chipset * protections. e.g. TSEG. */ -void smm_region(void **start, size_t *size); +void smm_region(uintptr_t *start, size_t *size);
enum { /* SMM handler area. */ @@ -42,6 +42,6 @@
/* Fills in the start and size for the requested SMM subregion. Returns * 0 on susccess, < 0 on failure. */ -int smm_subregion(int sub, void **start, size_t *size); +int smm_subregion(int sub, uintptr_t *start, size_t *size);
#endif /* _COMMON_MEMMAP_H_ */ diff --git a/src/drivers/intel/fsp2_0/stage_cache.c b/src/drivers/intel/fsp2_0/stage_cache.c index 434eae9..2ea6656 100644 --- a/src/drivers/intel/fsp2_0/stage_cache.c +++ b/src/drivers/intel/fsp2_0/stage_cache.c @@ -20,11 +20,11 @@ #include <reset.h> #include <program_loading.h>
-void stage_cache_external_region(void **base, size_t *size) +void stage_cache_external_region(uintptr_t *base, size_t *size) { if (smm_subregion(SMM_SUBREGION_CACHE, base, size)) { printk(BIOS_ERR, "ERROR: No cache SMM subregion.\n"); - *base = NULL; + *base = 0; *size = 0; } } diff --git a/src/include/stage_cache.h b/src/include/stage_cache.h index 28cb859..2a6c3bc 100644 --- a/src/include/stage_cache.h +++ b/src/include/stage_cache.h @@ -41,7 +41,7 @@ /* Load the cached stage at given location returning the stage entry point. */ void stage_cache_load_stage(int stage_id, struct prog *stage); /* Fill in parameters for the external stage cache, if utilized. */ -void stage_cache_external_region(void **base, size_t *size); +void stage_cache_external_region(uintptr_t *base, size_t *size);
/* Metadata associated with each stage. */ struct stage_cache { diff --git a/src/lib/ext_stage_cache.c b/src/lib/ext_stage_cache.c index c3d4aee..35bcce4 100644 --- a/src/lib/ext_stage_cache.c +++ b/src/lib/ext_stage_cache.c @@ -32,12 +32,12 @@ static void stage_cache_create_empty(void) { struct imd *imd; - void *base; + uintptr_t base; size_t size;
imd = imd_get(); stage_cache_external_region(&base, &size); - imd_handle_init(imd, (void *)(size + (uintptr_t)base)); + imd_handle_init(imd, (void *)(size + base));
printk(BIOS_DEBUG, "External stage cache:\n"); imd_create_tiered_empty(imd, 4096, 4096, 1024, 32); @@ -48,12 +48,12 @@ static void stage_cache_recover(void) { struct imd *imd; - void *base; + uintptr_t base; size_t size;
imd = imd_get(); stage_cache_external_region(&base, &size); - imd_handle_init(imd, (void *)(size + (uintptr_t)base)); + imd_handle_init(imd, (void *)(size + base)); if (imd_recover(imd)) printk(BIOS_DEBUG, "Unable to recover external stage cache.\n"); } diff --git a/src/soc/amd/stoneyridge/cpu.c b/src/soc/amd/stoneyridge/cpu.c index 1d9804d..e5b8bea 100644 --- a/src/soc/amd/stoneyridge/cpu.c +++ b/src/soc/amd/stoneyridge/cpu.c @@ -62,21 +62,21 @@ static void get_smm_info(uintptr_t *perm_smbase, size_t *perm_smsize, size_t *smm_save_state_size) { - void *smm_base; + uintptr_t smm_base; size_t smm_size; - void *handler_base; + uintptr_t handler_base; size_t handler_size;
/* Initialize global tracking state. */ smm_region_info(&smm_base, &smm_size); smm_subregion(SMM_SUBREGION_HANDLER, &handler_base, &handler_size);
- relo_attrs.smbase = (uint32_t)smm_base; + relo_attrs.smbase = smm_base; relo_attrs.tseg_base = relo_attrs.smbase; relo_attrs.tseg_mask = ALIGN_DOWN(~(smm_size - 1), 128 * KiB); relo_attrs.tseg_mask |= SMM_TSEG_WB;
- *perm_smbase = (uintptr_t)handler_base; + *perm_smbase = handler_base; *perm_smsize = handler_size; *smm_save_state_size = sizeof(amd64_smm_state_save_area_t); } diff --git a/src/soc/amd/stoneyridge/include/soc/northbridge.h b/src/soc/amd/stoneyridge/include/soc/northbridge.h index d62c791..82be938 100644 --- a/src/soc/amd/stoneyridge/include/soc/northbridge.h +++ b/src/soc/amd/stoneyridge/include/soc/northbridge.h @@ -114,12 +114,12 @@ * Fills in the arguments for the entire SMM region covered by chipset * protections. e.g. TSEG. */ -void smm_region_info(void **start, size_t *size); +void smm_region_info(uintptr_t *start, size_t *size); /* * Fills in the start and size for the requested SMM subregion. Returns * 0 on success, < 0 on failure. */ -int smm_subregion(int sub, void **start, size_t *size); +int smm_subregion(int sub, uintptr_t *start, size_t *size); void domain_enable_resources(struct device *dev); void domain_read_resources(struct device *dev); void domain_set_resources(struct device *dev); diff --git a/src/soc/amd/stoneyridge/ramtop.c b/src/soc/amd/stoneyridge/ramtop.c index 15968ab..ea3bd8b 100644 --- a/src/soc/amd/stoneyridge/ramtop.c +++ b/src/soc/amd/stoneyridge/ramtop.c @@ -80,18 +80,18 @@ return CONFIG_SMM_TSEG_SIZE; }
-void stage_cache_external_region(void **base, size_t *size) +void stage_cache_external_region(uintptr_t *base, size_t *size) { if (smm_subregion(SMM_SUBREGION_CACHE, base, size)) { printk(BIOS_ERR, "ERROR: No cache SMM subregion.\n"); - *base = NULL; + *base = 0; *size = 0; } }
-void smm_region_info(void **start, size_t *size) +void smm_region_info(uintptr_t *start, size_t *size) { - *start = (void *)smm_region_start(); + *start = smm_region_start(); *size = smm_region_size(); }
@@ -117,7 +117,7 @@ wrmsr(SMM_MASK_MSR, mask); }
-int smm_subregion(int sub, void **start, size_t *size) +int smm_subregion(int sub, uintptr_t *start, size_t *size) { uintptr_t sub_base; size_t sub_size; @@ -143,7 +143,7 @@ return -1; }
- *start = (void *)sub_base; + *start = sub_base; *size = sub_size;
return 0; diff --git a/src/soc/amd/stoneyridge/romstage.c b/src/soc/amd/stoneyridge/romstage.c index 9314488..e89f9e6 100644 --- a/src/soc/amd/stoneyridge/romstage.c +++ b/src/soc/amd/stoneyridge/romstage.c @@ -82,7 +82,7 @@ { struct postcar_frame pcf; uintptr_t top_of_ram; - void *smm_base; + uintptr_t smm_base; size_t smm_size; uintptr_t tseg_base; msr_t base, mask; diff --git a/src/soc/intel/apollolake/cpu.c b/src/soc/intel/apollolake/cpu.c index bcec28e..89faa85 100644 --- a/src/soc/intel/apollolake/cpu.c +++ b/src/soc/intel/apollolake/cpu.c @@ -196,9 +196,9 @@ static void get_smm_info(uintptr_t *perm_smbase, size_t *perm_smsize, size_t *smm_save_state_size) { - void *smm_base; + uintptr_t smm_base; size_t smm_size; - void *handler_base; + uintptr_t handler_base; size_t handler_size;
/* All range registers are aligned to 4KiB */ diff --git a/src/soc/intel/apollolake/memmap.c b/src/soc/intel/apollolake/memmap.c index be15e16..50ececa 100644 --- a/src/soc/intel/apollolake/memmap.c +++ b/src/soc/intel/apollolake/memmap.c @@ -48,7 +48,7 @@ return tolum; }
-int smm_subregion(int sub, void **start, size_t *size) +int smm_subregion(int sub, uintptr_t *start, size_t *size) { uintptr_t sub_base; size_t sub_size; @@ -56,7 +56,6 @@ const size_t cache_size = CONFIG_SMM_RESERVED_SIZE;
smm_region_info(&smm_base, &sub_size); - sub_base = (uintptr_t)smm_base;
assert(sub_size > CONFIG_SMM_RESERVED_SIZE);
@@ -74,7 +73,7 @@ return -1; }
- *start = (void *)sub_base; + *start = smm_base; *size = sub_size;
return 0; diff --git a/src/soc/intel/baytrail/stage_cache.c b/src/soc/intel/baytrail/stage_cache.c index 4c2a976..8e8c67f 100644 --- a/src/soc/intel/baytrail/stage_cache.c +++ b/src/soc/intel/baytrail/stage_cache.c @@ -17,7 +17,7 @@ #include <stage_cache.h> #include <soc/smm.h>
-void stage_cache_external_region(void **base, size_t *size) +void stage_cache_external_region(uintptr_t *base, size_t *size) { char *smm_base; /* 1MiB cache size */ @@ -27,5 +27,5 @@ * cbmem_top(). */ smm_base = cbmem_top(); *size = cache_size; - *base = &smm_base[smm_region_size() - cache_size]; + *base = smm_base + smm_region_size() - cache_size; } diff --git a/src/soc/intel/braswell/cpu.c b/src/soc/intel/braswell/cpu.c index 12f34fb..b4face2 100644 --- a/src/soc/intel/braswell/cpu.c +++ b/src/soc/intel/braswell/cpu.c @@ -147,7 +147,7 @@ static void get_smm_info(uintptr_t *perm_smbase, size_t *perm_smsize, size_t *smm_save_state_size) { - void *smm_base; + uintptr_t smm_base; size_t smm_size;
/* All range registers are aligned to 4KiB */ diff --git a/src/soc/intel/braswell/memmap.c b/src/soc/intel/braswell/memmap.c index d7c9952..89c3ab0 100644 --- a/src/soc/intel/braswell/memmap.c +++ b/src/soc/intel/braswell/memmap.c @@ -29,9 +29,9 @@ return smm_size; }
-void smm_region(void **start, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - *start = (void *)((iosf_bunit_read(BUNIT_SMRRL) & 0xFFFF) << 20); + *start = (iosf_bunit_read(BUNIT_SMRRL) & 0xFFFF) << 20; *size = smm_region_size(); }
@@ -51,15 +51,13 @@ * | (TSEG) | * +-------------------------+ BUNIT_SMRRL */ -int smm_subregion(int sub, void **start, size_t *size) +int smm_subregion(int sub, uintptr_t *start, size_t *size) { uintptr_t sub_base; - void *sub_ptr; size_t sub_size; const size_t cache_size = CONFIG_SMM_RESERVED_SIZE;
- smm_region(&sub_ptr, &sub_size); - sub_base = (uintptr_t)sub_ptr; + smm_region(&sub_base, &sub_size);
switch (sub) { case SMM_SUBREGION_HANDLER: @@ -75,7 +73,7 @@ return -1; }
- *start = (void *)sub_base; + *start = sub_base; *size = sub_size;
return 0; @@ -83,7 +81,7 @@
void *cbmem_top(void) { - char *smm_base; + uintptr_t smm_base; size_t smm_size;
/* @@ -114,6 +112,6 @@ * +-------------------------+ */
- smm_region((void **)&smm_base, &smm_size); + smm_region(&smm_base, &smm_size); return (void *)smm_base; } diff --git a/src/soc/intel/braswell/northcluster.c b/src/soc/intel/braswell/northcluster.c index 6e148b9..36b4377 100644 --- a/src/soc/intel/braswell/northcluster.c +++ b/src/soc/intel/braswell/northcluster.c @@ -87,7 +87,7 @@ unsigned long mmconf; unsigned long bmbound_k; unsigned long bmbound_hi; - void *smm_base; + uintptr_t smm_base; size_t smm_size; unsigned long tseg_base_k; unsigned long tseg_top_k; diff --git a/src/soc/intel/broadwell/stage_cache.c b/src/soc/intel/broadwell/stage_cache.c index dc59ab7..b812ced 100644 --- a/src/soc/intel/broadwell/stage_cache.c +++ b/src/soc/intel/broadwell/stage_cache.c @@ -18,7 +18,7 @@ #include <stage_cache.h> #include <stdint.h>
-void stage_cache_external_region(void **base, size_t *size) +void stage_cache_external_region(uintptr_t *base, size_t *size) { /* The ramstage cache lives in the TSEG region. * The top of RAM is defined to be the TSEG base address. */ @@ -26,6 +26,6 @@ offset -= CONFIG_IED_REGION_SIZE; offset -= CONFIG_SMM_RESERVED_SIZE;
- *base = (void *)(cbmem_top() + offset); + *base = cbmem_top() + offset; *size = CONFIG_SMM_RESERVED_SIZE; } diff --git a/src/soc/intel/cannonlake/memmap.c b/src/soc/intel/cannonlake/memmap.c index 64e07be..8aeabac 100644 --- a/src/soc/intel/cannonlake/memmap.c +++ b/src/soc/intel/cannonlake/memmap.c @@ -29,9 +29,9 @@ #include <soc/systemagent.h> #include <stdlib.h>
-void smm_region(void **start, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - *start = (void *)sa_get_tseg_base(); + *start = sa_get_tseg_base(); *size = sa_get_tseg_size(); }
@@ -46,16 +46,14 @@ * | (TSEG) | * +-------------------------+ TSEG */ -int smm_subregion(int sub, void **start, size_t *size) +int smm_subregion(int sub, uintptr_t *start, size_t *size) { uintptr_t sub_base; size_t sub_size; - void *smm_base; const size_t ied_size = CONFIG_IED_REGION_SIZE; const size_t cache_size = CONFIG_SMM_RESERVED_SIZE;
- smm_region(&smm_base, &sub_size); - sub_base = (uintptr_t)smm_base; + smm_region(&sub_base, &sub_size);
switch (sub) { case SMM_SUBREGION_HANDLER: @@ -77,7 +75,7 @@ return -1; }
- *start = (void *)sub_base; + *start = sub_base; *size = sub_size;
return 0; diff --git a/src/soc/intel/cannonlake/smmrelocate.c b/src/soc/intel/cannonlake/smmrelocate.c index e688592..59ae790 100644 --- a/src/soc/intel/cannonlake/smmrelocate.c +++ b/src/soc/intel/cannonlake/smmrelocate.c @@ -173,11 +173,11 @@ static void fill_in_relocation_params(struct device *dev, struct smm_relocation_params *params) { - void *handler_base; + uintptr_t handler_base; size_t handler_size; - void *ied_base; + uintptr_t *ied_base; size_t ied_size; - void *tseg_base; + uintptr_t tseg_base; size_t tseg_size; u32 emrr_base; u32 emrr_size; @@ -196,9 +196,9 @@ smm_subregion(SMM_SUBREGION_CHIPSET, &ied_base, &ied_size);
params->smram_size = handler_size; - params->smram_base = (uintptr_t)handler_base; + params->smram_base = handler_base;
- params->ied_base = (uintptr_t)ied_base; + params->ied_base = ied_base; params->ied_size = ied_size;
/* SMRR has 32-bits of valid address aligned to 4KiB. */ @@ -232,24 +232,21 @@
static void setup_ied_area(struct smm_relocation_params *params) { - char *ied_base; - struct ied_header ied = { .signature = "INTEL RSVD", .size = params->ied_size, .reserved = {0}, };
- ied_base = (void *)params->ied_base;
printk(BIOS_DEBUG, "IED base = 0x%08x\n", params->ied_base); printk(BIOS_DEBUG, "IED size = 0x%08x\n", params->ied_size);
/* Place IED header at IEDBASE. */ - memcpy(ied_base, &ied, sizeof(ied)); + memcpy(params->ied_base, &ied, sizeof(ied));
/* Zero out 32KiB at IEDBASE + 1MiB */ - memset(ied_base + 1 * MiB, 0, 32 * KiB); + memset(params->ied_base + 1 * MiB, 0, 32 * KiB); }
void smm_info(uintptr_t *perm_smbase, size_t *perm_smsize, diff --git a/src/soc/intel/common/block/include/intelblocks/smm.h b/src/soc/intel/common/block/include/intelblocks/smm.h index c04ec46..e2d38fd 100644 --- a/src/soc/intel/common/block/include/intelblocks/smm.h +++ b/src/soc/intel/common/block/include/intelblocks/smm.h @@ -33,6 +33,6 @@ void smm_southbridge_clear_state(void); void smm_southbridge_enable(uint16_t pm1_events); /* API to get SMM region start and size based on Host Bridge register */ -void smm_region_info(void **start, size_t *size); +void smm_region_info(uintptr_t *start, size_t *size);
#endif diff --git a/src/soc/intel/common/block/smm/smm.c b/src/soc/intel/common/block/smm/smm.c index d929975..d9b5645 100644 --- a/src/soc/intel/common/block/smm/smm.c +++ b/src/soc/intel/common/block/smm/smm.c @@ -89,9 +89,9 @@ ); }
-void smm_region_info(void **start, size_t *size) +void smm_region_info(uintptr_t *start, size_t *size) { - *start = (void *)sa_get_tseg_base(); + *start = sa_get_tseg_base(); *size = sa_get_tseg_size(); }
diff --git a/src/soc/intel/denverton_ns/cpu.c b/src/soc/intel/denverton_ns/cpu.c index 676fab7..683af8e 100644 --- a/src/soc/intel/denverton_ns/cpu.c +++ b/src/soc/intel/denverton_ns/cpu.c @@ -91,9 +91,9 @@ static void get_smm_info(uintptr_t *perm_smbase, size_t *perm_smsize, size_t *smm_save_state_size) { - void *smm_base; + uintptr_t smm_base; size_t smm_size; - void *handler_base; + uintptr_t handler_base; size_t handler_size;
/* All range registers are aligned to 4KiB */ @@ -108,7 +108,7 @@ relo_attrs.smrr_mask = ~(smm_size - 1) & rmask; relo_attrs.smrr_mask |= MTRR_PHYS_MASK_VALID;
- *perm_smbase = (uintptr_t)handler_base; + *perm_smbase = handler_base; *perm_smsize = handler_size; *smm_save_state_size = sizeof(em64t100_smm_state_save_area_t); } diff --git a/src/soc/intel/denverton_ns/include/soc/smm.h b/src/soc/intel/denverton_ns/include/soc/smm.h index 771c3d8..e597e9d 100644 --- a/src/soc/intel/denverton_ns/include/soc/smm.h +++ b/src/soc/intel/denverton_ns/include/soc/smm.h @@ -33,7 +33,7 @@
/* Fills in the arguments for the entire SMM region covered by chipset * protections. e.g. TSEG. */ -void smm_region(void **start, size_t *size); +void smm_region(uintptr_t *start, size_t *size);
enum { /* SMM handler area. */ @@ -48,7 +48,7 @@
/* Fills in the start and size for the requested SMM subregion. Returns * 0 on success, < 0 on failure. */ -int smm_subregion(int sub, void **start, size_t *size); +int smm_subregion(int sub, uintptr_t *start, size_t *size);
#if !defined(__PRE_RAM__) && !defined(__SMM___) #include <stdint.h> diff --git a/src/soc/intel/denverton_ns/memmap.c b/src/soc/intel/denverton_ns/memmap.c index a42d861..b4812ab 100644 --- a/src/soc/intel/denverton_ns/memmap.c +++ b/src/soc/intel/denverton_ns/memmap.c @@ -71,13 +71,13 @@ return system_agent_region_base(TOLUD) - smm_region_start(); }
-void smm_region(void **start, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - *start = (void *)smm_region_start(); + *start = smm_region_start(); *size = smm_region_size(); }
-int smm_subregion(int sub, void **start, size_t *size) +int smm_subregion(int sub, uintptr_t *start, size_t *size) { uintptr_t sub_base; size_t sub_size; @@ -102,7 +102,7 @@ return -1; }
- *start = (void *)sub_base; + *start = sub_base; *size = sub_size;
return 0; diff --git a/src/soc/intel/denverton_ns/romstage.c b/src/soc/intel/denverton_ns/romstage.c index 617b64a..712ec83 100644 --- a/src/soc/intel/denverton_ns/romstage.c +++ b/src/soc/intel/denverton_ns/romstage.c @@ -147,9 +147,8 @@ uintptr_t top_of_ram;
#if IS_ENABLED(CONFIG_HAVE_SMI_HANDLER) - void *smm_base; + uintptr_t smm_base; size_t smm_size; - uintptr_t tseg_base; #endif
console_init(); @@ -190,8 +189,7 @@ * region for other purposes. */ smm_region(&smm_base, &smm_size); - tseg_base = (uintptr_t)smm_base; - postcar_frame_add_mtrr(&pcf, tseg_base, smm_size, MTRR_TYPE_WRBACK); + postcar_frame_add_mtrr(&pcf, smm_base, smm_size, MTRR_TYPE_WRBACK); #endif
run_postcar_phase(&pcf); diff --git a/src/soc/intel/icelake/memmap.c b/src/soc/intel/icelake/memmap.c index 027b8b0..20ae6e8 100644 --- a/src/soc/intel/icelake/memmap.c +++ b/src/soc/intel/icelake/memmap.c @@ -28,9 +28,9 @@ #include <soc/systemagent.h> #include <stdlib.h>
-void smm_region(void **start, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - *start = (void *)sa_get_tseg_base(); + *start = sa_get_tseg_base(); *size = sa_get_tseg_size(); }
@@ -45,16 +45,14 @@ * | (TSEG) | * +-------------------------+ TSEG */ -int smm_subregion(int sub, void **start, size_t *size) +int smm_subregion(int sub, uintptr_t *start, size_t *size) { - uintptr_t sub_base; size_t sub_size; - void *smm_base; + uintptr_t smm_base; const size_t ied_size = CONFIG_IED_REGION_SIZE; const size_t cache_size = CONFIG_SMM_RESERVED_SIZE;
smm_region(&smm_base, &sub_size); - sub_base = (uintptr_t)smm_base;
switch (sub) { case SMM_SUBREGION_HANDLER: @@ -76,7 +74,7 @@ return -1; }
- *start = (void *)sub_base; + *start = smm_base; *size = sub_size;
return 0; diff --git a/src/soc/intel/icelake/smmrelocate.c b/src/soc/intel/icelake/smmrelocate.c index 57a366a..bb81799 100644 --- a/src/soc/intel/icelake/smmrelocate.c +++ b/src/soc/intel/icelake/smmrelocate.c @@ -172,11 +172,11 @@ static void fill_in_relocation_params(struct device *dev, struct smm_relocation_params *params) { - void *handler_base; + uintptr_t handler_base; size_t handler_size; - void *ied_base; + uintptr_t ied_base; size_t ied_size; - void *tseg_base; + uintptr_t tseg_base; size_t tseg_size; u32 emrr_base; u32 emrr_size; @@ -195,9 +195,9 @@ smm_subregion(SMM_SUBREGION_CHIPSET, &ied_base, &ied_size);
params->smram_size = handler_size; - params->smram_base = (uintptr_t)handler_base; + params->smram_base = handler_base;
- params->ied_base = (uintptr_t)ied_base; + params->ied_base = ied_base; params->ied_size = ied_size;
/* SMRR has 32-bits of valid address aligned to 4KiB. */ @@ -231,24 +231,21 @@
static void setup_ied_area(struct smm_relocation_params *params) { - char *ied_base; - struct ied_header ied = { .signature = "INTEL RSVD", .size = params->ied_size, .reserved = {0}, };
- ied_base = (void *)params->ied_base;
printk(BIOS_DEBUG, "IED base = 0x%08x\n", params->ied_base); printk(BIOS_DEBUG, "IED size = 0x%08x\n", params->ied_size);
/* Place IED header at IEDBASE. */ - memcpy(ied_base, &ied, sizeof(ied)); + memcpy(params->ied_base, &ied, sizeof(ied));
/* Zero out 32KiB at IEDBASE + 1MiB */ - memset(ied_base + 1 * MiB, 0, 32 * KiB); + memset(params->ied_base + 1 * MiB, 0, 32 * KiB); }
void smm_info(uintptr_t *perm_smbase, size_t *perm_smsize, diff --git a/src/soc/intel/skylake/memmap.c b/src/soc/intel/skylake/memmap.c index f0ccb1d..c33787a 100644 --- a/src/soc/intel/skylake/memmap.c +++ b/src/soc/intel/skylake/memmap.c @@ -40,9 +40,9 @@ return 8*MiB; }
-void smm_region(void **start, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - *start = (void *)sa_get_tseg_base(); + *start = sa_get_tseg_base(); *size = sa_get_tseg_size(); }
@@ -57,16 +57,14 @@ * | (TSEG) | * +-------------------------+ TSEG */ -int smm_subregion(int sub, void **start, size_t *size) +int smm_subregion(int sub, uintptr_t *start, size_t *size) { - uintptr_t sub_base; size_t sub_size; - void *smm_base; + uintptr_t smm_base; const size_t ied_size = CONFIG_IED_REGION_SIZE; const size_t cache_size = CONFIG_SMM_RESERVED_SIZE;
smm_region(&smm_base, &sub_size); - sub_base = (uintptr_t)smm_base;
switch (sub) { case SMM_SUBREGION_HANDLER: @@ -88,7 +86,7 @@ return -1; }
- *start = (void *)sub_base; + *start = smm_base; *size = sub_size;
return 0; diff --git a/src/soc/intel/skylake/romstage/romstage_fsp20.c b/src/soc/intel/skylake/romstage/romstage_fsp20.c index 2a60158..248f50f 100644 --- a/src/soc/intel/skylake/romstage/romstage_fsp20.c +++ b/src/soc/intel/skylake/romstage/romstage_fsp20.c @@ -167,7 +167,7 @@ postcar_frame_add_mtrr(&pcf, top_of_ram, 16*MiB, MTRR_TYPE_WRBACK);
if (IS_ENABLED(CONFIG_HAVE_SMI_HANDLER)) { - void *smm_base; + uintptr_t *smm_base; size_t smm_size; uintptr_t tseg_base;
@@ -179,7 +179,7 @@ * region for other purposes. */ smm_region(&smm_base, &smm_size); - tseg_base = (uintptr_t)smm_base; + tseg_base = smm_base; postcar_frame_add_mtrr(&pcf, tseg_base, smm_size, MTRR_TYPE_WRBACK); } diff --git a/src/soc/intel/skylake/smmrelocate.c b/src/soc/intel/skylake/smmrelocate.c index cb4e23f..82aacd4 100644 --- a/src/soc/intel/skylake/smmrelocate.c +++ b/src/soc/intel/skylake/smmrelocate.c @@ -241,24 +241,21 @@
static void setup_ied_area(struct smm_relocation_params *params) { - char *ied_base; - struct ied_header ied = { .signature = "INTEL RSVD", .size = params->ied_size, .reserved = {0}, };
- ied_base = (void *)params->ied_base;
printk(BIOS_DEBUG, "IED base = 0x%08x\n", params->ied_base); printk(BIOS_DEBUG, "IED size = 0x%08x\n", params->ied_size);
/* Place IED header at IEDBASE. */ - memcpy(ied_base, &ied, sizeof(ied)); + memcpy(params->ied_base, &ied, sizeof(ied));
/* Zero out 32KiB at IEDBASE + 1MiB */ - memset(ied_base + (1 << 20), 0, (32 << 10)); + memset(params->ied_base + 1 * MiB, 0, 32 * KiB); }
void smm_info(uintptr_t *perm_smbase, size_t *perm_smsize,
Hello Patrick Rudolph, Julius Werner, build bot (Jenkins), Philipp Hug, ron minnich, Vanessa Eusebio, Huang Jin, York Yang, Lee Leahy, Jonathan Neuschäfer, Philipp Deppenwiese, Nico Huber, Damien Zammit, David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30684
to look at the new patch set (#11).
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Use uintptr_t instead of (void *) where it makes sense
This reduces the number of casts.
Change-Id: I8ef5d9fad0298c79806fd94ca053027822022ac2 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/cbmem.c M src/cpu/allwinner/a10/cbmem.c M src/cpu/allwinner/a10/ram_segs.h M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/cpu/intel/haswell/romstage.c M src/cpu/intel/haswell/stage_cache.c M src/cpu/intel/model_206ax/stage_cache.c M src/cpu/ti/am335x/cbmem.c M src/drivers/amd/agesa/mtrr_fixme.c M src/drivers/intel/fsp1_0/fsp_util.c M src/drivers/intel/fsp1_1/include/fsp/memmap.h M src/drivers/intel/fsp1_1/raminit.c M src/drivers/intel/fsp1_1/ramstage.c M src/drivers/intel/fsp1_1/stack.c M src/drivers/intel/fsp1_1/stage_cache.c M src/drivers/intel/fsp2_0/hob_verify.c M src/drivers/intel/fsp2_0/include/fsp/memmap.h M src/drivers/intel/fsp2_0/stage_cache.c M src/include/cbmem.h M src/include/imd.h M src/include/stage_cache.h M src/lib/ext_stage_cache.c M src/lib/imd.c M src/mainboard/emulation/qemu-armv7/cbmem.c M src/mainboard/emulation/qemu-i440fx/memory.c M src/mainboard/emulation/qemu-power8/cbmem.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/raminit.c M src/northbridge/intel/gm45/northbridge.c M src/northbridge/intel/gm45/ram_calc.c M src/northbridge/intel/haswell/ram_calc.c M src/northbridge/intel/i440bx/ram_calc.c M src/northbridge/intel/i945/northbridge.c M src/northbridge/intel/i945/ram_calc.c M src/northbridge/intel/nehalem/ram_calc.c M src/northbridge/intel/pineview/northbridge.c M src/northbridge/intel/pineview/ram_calc.c M src/northbridge/intel/sandybridge/ram_calc.c M src/northbridge/intel/x4x/northbridge.c M src/northbridge/intel/x4x/ram_calc.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/stoneyridge/cpu.c M src/soc/amd/stoneyridge/include/soc/northbridge.h M src/soc/amd/stoneyridge/northbridge.c M src/soc/amd/stoneyridge/ramtop.c M src/soc/amd/stoneyridge/romstage.c M src/soc/cavium/cn81xx/cbmem.c M src/soc/imgtec/pistachio/cbmem.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/apollolake/romstage.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/baytrail/romstage/romstage.c M src/soc/intel/baytrail/stage_cache.c M src/soc/intel/braswell/cpu.c M src/soc/intel/braswell/memmap.c M src/soc/intel/braswell/northcluster.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/romstage/romstage.c M src/soc/intel/broadwell/stage_cache.c D src/soc/intel/cannonlake/cbmem.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/cannonlake/romstage/romstage.c M src/soc/intel/cannonlake/smmrelocate.c M src/soc/intel/common/block/include/intelblocks/smm.h M src/soc/intel/common/block/smm/smm.c M src/soc/intel/common/block/systemagent/systemagent.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/denverton_ns/include/soc/smm.h M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/denverton_ns/romstage.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_baytrail/northcluster.c M src/soc/intel/fsp_broadwell_de/memmap.c D src/soc/intel/icelake/cbmem.c M src/soc/intel/icelake/memmap.c M src/soc/intel/icelake/romstage/romstage.c M src/soc/intel/icelake/smmrelocate.c M src/soc/intel/quark/memmap.c M src/soc/intel/quark/northcluster.c M src/soc/intel/quark/romstage/fsp2_0.c M src/soc/intel/skylake/memmap.c M src/soc/intel/skylake/romstage/romstage_fsp20.c M src/soc/intel/skylake/smmrelocate.c M src/soc/mediatek/common/cbmem.c M src/soc/nvidia/tegra124/cbmem.c M src/soc/nvidia/tegra210/cbmem.c M src/soc/qualcomm/ipq40xx/cbmem.c M src/soc/qualcomm/ipq806x/cbmem.c M src/soc/qualcomm/sdm845/cbmem.c M src/soc/rockchip/common/cbmem.c M src/soc/samsung/exynos5250/cbmem.c M src/soc/samsung/exynos5420/cbmem.c M src/soc/sifive/fu540/cbmem.c M src/soc/ucb/riscv/cbmem.c 95 files changed, 247 insertions(+), 322 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/30684/11
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30684 )
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/#/c/30684/11/src/drivers/intel/fsp1_1/ramstage.c File src/drivers/intel/fsp1_1/ramstage.c:
https://review.coreboot.org/#/c/30684/11/src/drivers/intel/fsp1_1/ramstage.c... PS11, Line 43: printk(BIOS_SPEW, "SMRAM : 0x%08x 0x%zx\n", (unsigned int)base, size); line over 80 characters
https://review.coreboot.org/#/c/30684/11/src/drivers/intel/fsp1_1/ramstage.c... PS11, Line 48: printk(BIOS_SPEW, " Subregion %d: 0x%08x 0x%zx\n", i, (unsigned int)base, size); line over 80 characters
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30684 )
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Patch Set 11:
(4 comments)
https://review.coreboot.org/#/c/30684/10/src/cpu/allwinner/a10/ram_segs.h File src/cpu/allwinner/a10/ram_segs.h:
https://review.coreboot.org/#/c/30684/10/src/cpu/allwinner/a10/ram_segs.h@27 PS10, Line 27: << 20
Seen this more often, maybe worth to trigger a follow up to replace […]
I'll do that.
https://review.coreboot.org/#/c/30684/10/src/cpu/allwinner/a10/ram_segs.h@30 PS10, Line 30: /* : * By CBFS cache, we mean a cached copy, in RAM, of the entire CBFS region. : */ : static inline uintptr_t a1x_get_cbfs_cache_top(void) : { : /* Arbitrary 16 MiB gap for cbmem tables and bouncebuffer */ : return a1x_get_cbmem_top() - (16 << 20); : } : : static inline uintptr_t a1x_get_cbfs_cache_base(void) : { : return a1x_get_cbfs_cache_top() - CONFIG_ROM_SIZE; : }
I was wondering why we don't have to update any caller... […]
this cpu/soc code was actually never completed. The code that would allow it to read cbfs from the bootmedium (typically SD/emmc) was never pushed for review, so it won't get past the bootblock (raminit is done in there).
https://review.coreboot.org/#/c/30684/10/src/drivers/intel/fsp1_1/raminit.c File src/drivers/intel/fsp1_1/raminit.c:
https://review.coreboot.org/#/c/30684/10/src/drivers/intel/fsp1_1/raminit.c@... PS10, Line 155: (unsigned int)
or use PRIxPTR instead
that macro is not implemented in coreboot.
https://review.coreboot.org/#/c/30684/10/src/mainboard/emulation/qemu-power8... File src/mainboard/emulation/qemu-power8/cbmem.c:
https://review.coreboot.org/#/c/30684/10/src/mainboard/emulation/qemu-power8... PS10, Line 21: /* For now, last 1M of 4G */
Is this a lie? CBMEM will be *below* the last 1MiB...
looks quite weird indeed.
Hello Patrick Rudolph, Julius Werner, build bot (Jenkins), Philipp Hug, ron minnich, Vanessa Eusebio, Huang Jin, York Yang, Lee Leahy, Jonathan Neuschäfer, Philipp Deppenwiese, Nico Huber, Damien Zammit, David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30684
to look at the new patch set (#12).
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Use uintptr_t instead of (void *) where it makes sense
This reduces the number of casts.
Change-Id: I8ef5d9fad0298c79806fd94ca053027822022ac2 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/acpi_bert_storage.c M src/arch/x86/cbmem.c M src/arch/x86/include/arch/bert_storage.h M src/cpu/allwinner/a10/cbmem.c M src/cpu/allwinner/a10/ram_segs.h M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/cpu/intel/haswell/romstage.c M src/cpu/intel/haswell/stage_cache.c M src/cpu/intel/model_206ax/stage_cache.c M src/cpu/ti/am335x/cbmem.c M src/drivers/amd/agesa/mtrr_fixme.c M src/drivers/intel/fsp1_0/fsp_util.c M src/drivers/intel/fsp1_1/include/fsp/memmap.h M src/drivers/intel/fsp1_1/raminit.c M src/drivers/intel/fsp1_1/ramstage.c M src/drivers/intel/fsp1_1/stack.c M src/drivers/intel/fsp1_1/stage_cache.c M src/drivers/intel/fsp2_0/hob_verify.c M src/drivers/intel/fsp2_0/include/fsp/memmap.h M src/drivers/intel/fsp2_0/stage_cache.c M src/include/cbmem.h M src/include/imd.h M src/include/stage_cache.h M src/lib/ext_stage_cache.c M src/lib/imd.c M src/mainboard/emulation/qemu-armv7/cbmem.c M src/mainboard/emulation/qemu-i440fx/memory.c M src/mainboard/emulation/qemu-power8/cbmem.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/raminit.c M src/northbridge/intel/gm45/northbridge.c M src/northbridge/intel/gm45/ram_calc.c M src/northbridge/intel/haswell/ram_calc.c M src/northbridge/intel/i440bx/ram_calc.c M src/northbridge/intel/i945/northbridge.c M src/northbridge/intel/i945/ram_calc.c M src/northbridge/intel/nehalem/ram_calc.c M src/northbridge/intel/pineview/northbridge.c M src/northbridge/intel/pineview/ram_calc.c M src/northbridge/intel/sandybridge/ram_calc.c M src/northbridge/intel/x4x/northbridge.c M src/northbridge/intel/x4x/ram_calc.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/stoneyridge/cpu.c M src/soc/amd/stoneyridge/include/soc/northbridge.h M src/soc/amd/stoneyridge/northbridge.c M src/soc/amd/stoneyridge/ramtop.c M src/soc/amd/stoneyridge/romstage.c M src/soc/cavium/cn81xx/cbmem.c M src/soc/imgtec/pistachio/cbmem.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/apollolake/romstage.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/baytrail/romstage/romstage.c M src/soc/intel/baytrail/stage_cache.c M src/soc/intel/braswell/cpu.c M src/soc/intel/braswell/memmap.c M src/soc/intel/braswell/northcluster.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/romstage/romstage.c M src/soc/intel/broadwell/stage_cache.c D src/soc/intel/cannonlake/cbmem.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/cannonlake/romstage/romstage.c M src/soc/intel/cannonlake/smmrelocate.c M src/soc/intel/common/block/include/intelblocks/smm.h M src/soc/intel/common/block/smm/smm.c M src/soc/intel/common/block/systemagent/systemagent.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/denverton_ns/include/soc/smm.h M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/denverton_ns/romstage.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_baytrail/northcluster.c M src/soc/intel/fsp_broadwell_de/memmap.c D src/soc/intel/icelake/cbmem.c M src/soc/intel/icelake/memmap.c M src/soc/intel/icelake/romstage/romstage.c M src/soc/intel/icelake/smmrelocate.c M src/soc/intel/quark/memmap.c M src/soc/intel/quark/northcluster.c M src/soc/intel/quark/romstage/fsp2_0.c M src/soc/intel/skylake/memmap.c M src/soc/intel/skylake/romstage/romstage_fsp20.c M src/soc/intel/skylake/smmrelocate.c M src/soc/mediatek/common/cbmem.c M src/soc/nvidia/tegra124/cbmem.c M src/soc/nvidia/tegra210/cbmem.c M src/soc/qualcomm/ipq40xx/cbmem.c M src/soc/qualcomm/ipq806x/cbmem.c M src/soc/qualcomm/sdm845/cbmem.c M src/soc/rockchip/common/cbmem.c M src/soc/samsung/exynos5250/cbmem.c M src/soc/samsung/exynos5420/cbmem.c M src/soc/sifive/fu540/cbmem.c M src/soc/ucb/riscv/cbmem.c 97 files changed, 259 insertions(+), 334 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/30684/12
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30684 )
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/#/c/30684/12/src/drivers/intel/fsp1_1/ramstage.c File src/drivers/intel/fsp1_1/ramstage.c:
https://review.coreboot.org/#/c/30684/12/src/drivers/intel/fsp1_1/ramstage.c... PS12, Line 43: printk(BIOS_SPEW, "SMRAM : 0x%08x 0x%zx\n", (unsigned int)base, size); line over 80 characters
https://review.coreboot.org/#/c/30684/12/src/drivers/intel/fsp1_1/ramstage.c... PS12, Line 48: printk(BIOS_SPEW, " Subregion %d: 0x%08x 0x%zx\n", i, (unsigned int)base, size); line over 80 characters
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30684 )
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Patch Set 12:
I think that you need to make a rebase manually: modifié des deux côtés : src/mainboard/emulation/qemu-i440fx/memory.c modifié des deux côtés : src/soc/cavium/cn81xx/cbmem.c
Hello Patrick Rudolph, Julius Werner, build bot (Jenkins), Philipp Hug, ron minnich, Vanessa Eusebio, Huang Jin, York Yang, Lee Leahy, Jonathan Neuschäfer, Philipp Deppenwiese, Nico Huber, Damien Zammit, David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30684
to look at the new patch set (#13).
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Use uintptr_t instead of (void *) where it makes sense
This reduces the number of casts.
Change-Id: I8ef5d9fad0298c79806fd94ca053027822022ac2 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/acpi_bert_storage.c M src/arch/x86/cbmem.c M src/arch/x86/include/arch/bert_storage.h M src/cpu/allwinner/a10/cbmem.c M src/cpu/allwinner/a10/ram_segs.h M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/cpu/intel/haswell/romstage.c M src/cpu/intel/haswell/stage_cache.c M src/cpu/intel/model_206ax/stage_cache.c M src/cpu/ti/am335x/cbmem.c M src/drivers/amd/agesa/mtrr_fixme.c M src/drivers/intel/fsp1_0/fsp_util.c M src/drivers/intel/fsp1_1/include/fsp/memmap.h M src/drivers/intel/fsp1_1/raminit.c M src/drivers/intel/fsp1_1/ramstage.c M src/drivers/intel/fsp1_1/stack.c M src/drivers/intel/fsp1_1/stage_cache.c M src/drivers/intel/fsp2_0/hob_verify.c M src/drivers/intel/fsp2_0/include/fsp/memmap.h M src/drivers/intel/fsp2_0/stage_cache.c M src/include/cbmem.h M src/include/imd.h M src/include/stage_cache.h M src/lib/ext_stage_cache.c M src/lib/imd.c M src/mainboard/emulation/qemu-armv7/cbmem.c M src/mainboard/emulation/qemu-i440fx/memory.c M src/mainboard/emulation/qemu-power8/cbmem.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/raminit.c M src/northbridge/intel/gm45/northbridge.c M src/northbridge/intel/gm45/ram_calc.c M src/northbridge/intel/haswell/ram_calc.c M src/northbridge/intel/i440bx/ram_calc.c M src/northbridge/intel/i945/northbridge.c M src/northbridge/intel/i945/ram_calc.c M src/northbridge/intel/nehalem/ram_calc.c M src/northbridge/intel/pineview/northbridge.c M src/northbridge/intel/pineview/ram_calc.c M src/northbridge/intel/sandybridge/ram_calc.c M src/northbridge/intel/x4x/northbridge.c M src/northbridge/intel/x4x/ram_calc.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/stoneyridge/cpu.c M src/soc/amd/stoneyridge/include/soc/northbridge.h M src/soc/amd/stoneyridge/northbridge.c M src/soc/amd/stoneyridge/ramtop.c M src/soc/amd/stoneyridge/romstage.c M src/soc/cavium/cn81xx/cbmem.c M src/soc/imgtec/pistachio/cbmem.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/apollolake/romstage.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/baytrail/romstage/romstage.c M src/soc/intel/baytrail/stage_cache.c M src/soc/intel/braswell/cpu.c M src/soc/intel/braswell/memmap.c M src/soc/intel/braswell/northcluster.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/romstage/romstage.c M src/soc/intel/broadwell/stage_cache.c D src/soc/intel/cannonlake/cbmem.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/cannonlake/romstage/romstage.c M src/soc/intel/cannonlake/smmrelocate.c M src/soc/intel/common/block/include/intelblocks/smm.h M src/soc/intel/common/block/smm/smm.c M src/soc/intel/common/block/systemagent/systemagent.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/denverton_ns/include/soc/smm.h M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/denverton_ns/romstage.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_baytrail/northcluster.c M src/soc/intel/fsp_broadwell_de/memmap.c D src/soc/intel/icelake/cbmem.c M src/soc/intel/icelake/memmap.c M src/soc/intel/icelake/romstage/romstage.c M src/soc/intel/icelake/smmrelocate.c M src/soc/intel/quark/memmap.c M src/soc/intel/quark/northcluster.c M src/soc/intel/quark/romstage/fsp2_0.c M src/soc/intel/skylake/memmap.c M src/soc/intel/skylake/romstage/romstage_fsp20.c M src/soc/intel/skylake/smmrelocate.c M src/soc/mediatek/common/cbmem.c M src/soc/nvidia/tegra124/cbmem.c M src/soc/nvidia/tegra210/cbmem.c M src/soc/qualcomm/ipq40xx/cbmem.c M src/soc/qualcomm/ipq806x/cbmem.c M src/soc/qualcomm/sdm845/cbmem.c M src/soc/rockchip/common/cbmem.c M src/soc/samsung/exynos5250/cbmem.c M src/soc/samsung/exynos5420/cbmem.c M src/soc/sifive/fu540/cbmem.c M src/soc/ucb/riscv/cbmem.c 97 files changed, 262 insertions(+), 336 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/30684/13
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30684 )
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Patch Set 13:
(2 comments)
https://review.coreboot.org/#/c/30684/13/src/drivers/intel/fsp1_1/ramstage.c File src/drivers/intel/fsp1_1/ramstage.c:
https://review.coreboot.org/#/c/30684/13/src/drivers/intel/fsp1_1/ramstage.c... PS13, Line 43: printk(BIOS_SPEW, "SMRAM : 0x%08x 0x%zx\n", (unsigned int)base, size); line over 80 characters
https://review.coreboot.org/#/c/30684/13/src/drivers/intel/fsp1_1/ramstage.c... PS13, Line 48: printk(BIOS_SPEW, " Subregion %d: 0x%08x 0x%zx\n", i, (unsigned int)base, size); line over 80 characters
Hello Patrick Rudolph, Julius Werner, build bot (Jenkins), Philipp Hug, ron minnich, Vanessa Eusebio, Huang Jin, York Yang, Lee Leahy, Jonathan Neuschäfer, Philipp Deppenwiese, Nico Huber, Damien Zammit, David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30684
to look at the new patch set (#14).
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Use uintptr_t instead of (void *) where it makes sense
This reduces the number of casts.
Change-Id: I8ef5d9fad0298c79806fd94ca053027822022ac2 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/acpi_bert_storage.c M src/arch/x86/cbmem.c M src/arch/x86/include/arch/bert_storage.h M src/cpu/allwinner/a10/cbmem.c M src/cpu/allwinner/a10/ram_segs.h M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/cpu/intel/haswell/romstage.c M src/cpu/intel/haswell/stage_cache.c M src/cpu/intel/model_206ax/stage_cache.c M src/cpu/ti/am335x/cbmem.c M src/drivers/amd/agesa/mtrr_fixme.c M src/drivers/intel/fsp1_0/fsp_util.c M src/drivers/intel/fsp1_0/fsp_util.h M src/drivers/intel/fsp1_1/include/fsp/memmap.h M src/drivers/intel/fsp1_1/raminit.c M src/drivers/intel/fsp1_1/ramstage.c M src/drivers/intel/fsp1_1/stack.c M src/drivers/intel/fsp1_1/stage_cache.c M src/drivers/intel/fsp2_0/hob_verify.c M src/drivers/intel/fsp2_0/include/fsp/memmap.h M src/drivers/intel/fsp2_0/stage_cache.c M src/include/cbmem.h M src/include/imd.h M src/include/stage_cache.h M src/lib/ext_stage_cache.c M src/lib/imd.c M src/mainboard/emulation/qemu-armv7/cbmem.c M src/mainboard/emulation/qemu-i440fx/memory.c M src/mainboard/emulation/qemu-power8/cbmem.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/raminit.c M src/northbridge/intel/gm45/northbridge.c M src/northbridge/intel/gm45/ram_calc.c M src/northbridge/intel/gm45/stage_cache.c M src/northbridge/intel/haswell/ram_calc.c M src/northbridge/intel/i440bx/ram_calc.c M src/northbridge/intel/i945/northbridge.c M src/northbridge/intel/i945/ram_calc.c M src/northbridge/intel/i945/stage_cache.c M src/northbridge/intel/nehalem/ram_calc.c M src/northbridge/intel/pineview/northbridge.c M src/northbridge/intel/pineview/ram_calc.c M src/northbridge/intel/pineview/stage_cache.c M src/northbridge/intel/sandybridge/ram_calc.c M src/northbridge/intel/x4x/northbridge.c M src/northbridge/intel/x4x/ram_calc.c M src/northbridge/intel/x4x/stage_cache.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/stoneyridge/cpu.c M src/soc/amd/stoneyridge/include/soc/northbridge.h M src/soc/amd/stoneyridge/northbridge.c M src/soc/amd/stoneyridge/ramtop.c M src/soc/amd/stoneyridge/romstage.c M src/soc/cavium/cn81xx/cbmem.c M src/soc/imgtec/pistachio/cbmem.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/apollolake/romstage.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/baytrail/romstage/romstage.c M src/soc/intel/baytrail/stage_cache.c M src/soc/intel/braswell/cpu.c M src/soc/intel/braswell/memmap.c M src/soc/intel/braswell/northcluster.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/romstage/romstage.c M src/soc/intel/broadwell/stage_cache.c D src/soc/intel/cannonlake/cbmem.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/cannonlake/romstage/romstage.c M src/soc/intel/cannonlake/smmrelocate.c M src/soc/intel/common/block/include/intelblocks/smm.h M src/soc/intel/common/block/smm/smm.c M src/soc/intel/common/block/systemagent/systemagent.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/denverton_ns/include/soc/smm.h M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/denverton_ns/romstage.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_baytrail/northcluster.c M src/soc/intel/fsp_broadwell_de/memmap.c D src/soc/intel/icelake/cbmem.c M src/soc/intel/icelake/memmap.c M src/soc/intel/icelake/romstage/romstage.c M src/soc/intel/icelake/smmrelocate.c M src/soc/intel/quark/memmap.c M src/soc/intel/quark/northcluster.c M src/soc/intel/quark/romstage/fsp2_0.c M src/soc/intel/skylake/memmap.c M src/soc/intel/skylake/romstage/romstage_fsp20.c M src/soc/intel/skylake/smmrelocate.c M src/soc/mediatek/common/cbmem.c M src/soc/nvidia/tegra124/cbmem.c M src/soc/nvidia/tegra210/cbmem.c M src/soc/qualcomm/ipq40xx/cbmem.c M src/soc/qualcomm/ipq806x/cbmem.c M src/soc/qualcomm/sdm845/cbmem.c M src/soc/rockchip/common/cbmem.c M src/soc/samsung/exynos5250/cbmem.c M src/soc/samsung/exynos5420/cbmem.c M src/soc/sifive/fu540/cbmem.c M src/soc/ucb/riscv/cbmem.c 102 files changed, 271 insertions(+), 349 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/30684/14
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30684 )
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/#/c/30684/14/src/drivers/intel/fsp1_1/ramstage.c File src/drivers/intel/fsp1_1/ramstage.c:
https://review.coreboot.org/#/c/30684/14/src/drivers/intel/fsp1_1/ramstage.c... PS14, Line 43: printk(BIOS_SPEW, "SMRAM : 0x%08x 0x%zx\n", (unsigned int)base, size); line over 80 characters
https://review.coreboot.org/#/c/30684/14/src/drivers/intel/fsp1_1/ramstage.c... PS14, Line 48: printk(BIOS_SPEW, " Subregion %d: 0x%08x 0x%zx\n", i, (unsigned int)base, size); line over 80 characters
Hello Patrick Rudolph, Julius Werner, build bot (Jenkins), Philipp Hug, ron minnich, Vanessa Eusebio, Huang Jin, York Yang, Lee Leahy, Jonathan Neuschäfer, Philipp Deppenwiese, Nico Huber, Damien Zammit, David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30684
to look at the new patch set (#15).
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Use uintptr_t instead of (void *) where it makes sense
This reduces the number of casts.
Change-Id: I8ef5d9fad0298c79806fd94ca053027822022ac2 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/acpi_bert_storage.c M src/arch/x86/cbmem.c M src/arch/x86/include/arch/bert_storage.h M src/cpu/allwinner/a10/cbmem.c M src/cpu/allwinner/a10/ram_segs.h M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/cpu/intel/haswell/romstage.c M src/cpu/intel/haswell/stage_cache.c M src/cpu/intel/model_206ax/stage_cache.c M src/cpu/ti/am335x/cbmem.c M src/drivers/amd/agesa/mtrr_fixme.c M src/drivers/intel/fsp1_0/fsp_util.c M src/drivers/intel/fsp1_0/fsp_util.h M src/drivers/intel/fsp1_1/include/fsp/memmap.h M src/drivers/intel/fsp1_1/raminit.c M src/drivers/intel/fsp1_1/ramstage.c M src/drivers/intel/fsp1_1/stack.c M src/drivers/intel/fsp1_1/stage_cache.c M src/drivers/intel/fsp2_0/hob_verify.c M src/drivers/intel/fsp2_0/include/fsp/memmap.h M src/drivers/intel/fsp2_0/stage_cache.c M src/include/cbmem.h M src/include/imd.h M src/include/stage_cache.h M src/lib/ext_stage_cache.c M src/lib/imd.c M src/mainboard/emulation/qemu-armv7/cbmem.c M src/mainboard/emulation/qemu-i440fx/memory.c M src/mainboard/emulation/qemu-power8/cbmem.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/raminit.c M src/northbridge/intel/gm45/northbridge.c M src/northbridge/intel/gm45/ram_calc.c M src/northbridge/intel/gm45/stage_cache.c M src/northbridge/intel/haswell/ram_calc.c M src/northbridge/intel/i440bx/ram_calc.c M src/northbridge/intel/i945/northbridge.c M src/northbridge/intel/i945/ram_calc.c M src/northbridge/intel/i945/stage_cache.c M src/northbridge/intel/nehalem/ram_calc.c M src/northbridge/intel/pineview/northbridge.c M src/northbridge/intel/pineview/ram_calc.c M src/northbridge/intel/pineview/stage_cache.c M src/northbridge/intel/sandybridge/ram_calc.c M src/northbridge/intel/x4x/northbridge.c M src/northbridge/intel/x4x/ram_calc.c M src/northbridge/intel/x4x/stage_cache.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/stoneyridge/cpu.c M src/soc/amd/stoneyridge/include/soc/northbridge.h M src/soc/amd/stoneyridge/northbridge.c M src/soc/amd/stoneyridge/ramtop.c M src/soc/amd/stoneyridge/romstage.c M src/soc/cavium/cn81xx/cbmem.c M src/soc/imgtec/pistachio/cbmem.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/apollolake/romstage.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/baytrail/romstage/romstage.c M src/soc/intel/baytrail/stage_cache.c M src/soc/intel/braswell/cpu.c M src/soc/intel/braswell/memmap.c M src/soc/intel/braswell/northcluster.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/romstage/romstage.c M src/soc/intel/broadwell/stage_cache.c D src/soc/intel/cannonlake/cbmem.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/cannonlake/romstage/romstage.c M src/soc/intel/cannonlake/smmrelocate.c M src/soc/intel/common/block/include/intelblocks/smm.h M src/soc/intel/common/block/smm/smm.c M src/soc/intel/common/block/systemagent/systemagent.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/denverton_ns/include/soc/smm.h M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/denverton_ns/romstage.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_baytrail/northcluster.c M src/soc/intel/fsp_broadwell_de/memmap.c D src/soc/intel/icelake/cbmem.c M src/soc/intel/icelake/memmap.c M src/soc/intel/icelake/romstage/romstage.c M src/soc/intel/icelake/smmrelocate.c M src/soc/intel/quark/memmap.c M src/soc/intel/quark/northcluster.c M src/soc/intel/quark/romstage/fsp2_0.c M src/soc/intel/skylake/memmap.c M src/soc/intel/skylake/romstage/romstage_fsp20.c M src/soc/intel/skylake/smmrelocate.c M src/soc/mediatek/common/cbmem.c M src/soc/nvidia/tegra124/cbmem.c M src/soc/nvidia/tegra210/cbmem.c M src/soc/qualcomm/ipq40xx/cbmem.c M src/soc/qualcomm/ipq806x/cbmem.c M src/soc/qualcomm/sdm845/cbmem.c M src/soc/rockchip/common/cbmem.c M src/soc/samsung/exynos5250/cbmem.c M src/soc/samsung/exynos5420/cbmem.c M src/soc/sifive/fu540/cbmem.c M src/soc/ucb/riscv/cbmem.c 102 files changed, 272 insertions(+), 350 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/30684/15
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30684 )
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Patch Set 15:
(2 comments)
https://review.coreboot.org/#/c/30684/15/src/drivers/intel/fsp1_1/ramstage.c File src/drivers/intel/fsp1_1/ramstage.c:
https://review.coreboot.org/#/c/30684/15/src/drivers/intel/fsp1_1/ramstage.c... PS15, Line 43: printk(BIOS_SPEW, "SMRAM : 0x%08x 0x%zx\n", (unsigned int)base, size); line over 80 characters
https://review.coreboot.org/#/c/30684/15/src/drivers/intel/fsp1_1/ramstage.c... PS15, Line 48: printk(BIOS_SPEW, " Subregion %d: 0x%08x 0x%zx\n", i, (unsigned int)base, size); line over 80 characters
Hello Patrick Rudolph, Julius Werner, build bot (Jenkins), Philipp Hug, ron minnich, Vanessa Eusebio, Huang Jin, York Yang, Lee Leahy, Jonathan Neuschäfer, Philipp Deppenwiese, Nico Huber, Damien Zammit, David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30684
to look at the new patch set (#16).
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Use uintptr_t instead of (void *) where it makes sense
This reduces the number of casts.
Change-Id: I8ef5d9fad0298c79806fd94ca053027822022ac2 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/acpi_bert_storage.c M src/arch/x86/cbmem.c M src/arch/x86/include/arch/bert_storage.h M src/cpu/allwinner/a10/cbmem.c M src/cpu/allwinner/a10/ram_segs.h M src/cpu/amd/family_10h-family_15h/ram_calc.c M src/cpu/intel/haswell/romstage.c M src/cpu/intel/haswell/stage_cache.c M src/cpu/intel/model_206ax/stage_cache.c M src/cpu/ti/am335x/cbmem.c M src/drivers/amd/agesa/mtrr_fixme.c M src/drivers/intel/fsp1_0/fsp_util.c M src/drivers/intel/fsp1_0/fsp_util.h M src/drivers/intel/fsp1_1/include/fsp/memmap.h M src/drivers/intel/fsp1_1/raminit.c M src/drivers/intel/fsp1_1/ramstage.c M src/drivers/intel/fsp1_1/stack.c M src/drivers/intel/fsp1_1/stage_cache.c M src/drivers/intel/fsp2_0/hob_verify.c M src/drivers/intel/fsp2_0/include/fsp/memmap.h M src/drivers/intel/fsp2_0/stage_cache.c M src/include/cbmem.h M src/include/imd.h M src/include/stage_cache.h M src/lib/ext_stage_cache.c M src/lib/imd.c M src/mainboard/emulation/qemu-armv7/cbmem.c M src/mainboard/emulation/qemu-i440fx/memory.c M src/mainboard/emulation/qemu-power8/cbmem.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/fsp_rangeley/raminit.c M src/northbridge/intel/gm45/northbridge.c M src/northbridge/intel/gm45/ram_calc.c M src/northbridge/intel/gm45/stage_cache.c M src/northbridge/intel/haswell/ram_calc.c M src/northbridge/intel/i440bx/ram_calc.c M src/northbridge/intel/i945/northbridge.c M src/northbridge/intel/i945/ram_calc.c M src/northbridge/intel/i945/stage_cache.c M src/northbridge/intel/nehalem/ram_calc.c M src/northbridge/intel/pineview/northbridge.c M src/northbridge/intel/pineview/ram_calc.c M src/northbridge/intel/pineview/stage_cache.c M src/northbridge/intel/sandybridge/ram_calc.c M src/northbridge/intel/x4x/northbridge.c M src/northbridge/intel/x4x/ram_calc.c M src/northbridge/intel/x4x/stage_cache.c M src/northbridge/via/vx900/memmap.c M src/soc/amd/stoneyridge/cpu.c M src/soc/amd/stoneyridge/include/soc/northbridge.h M src/soc/amd/stoneyridge/northbridge.c M src/soc/amd/stoneyridge/ramtop.c M src/soc/amd/stoneyridge/romstage.c M src/soc/cavium/cn81xx/cbmem.c M src/soc/imgtec/pistachio/cbmem.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/apollolake/romstage.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/baytrail/romstage/romstage.c M src/soc/intel/baytrail/stage_cache.c M src/soc/intel/braswell/cpu.c M src/soc/intel/braswell/memmap.c M src/soc/intel/braswell/northcluster.c M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/romstage/romstage.c M src/soc/intel/broadwell/stage_cache.c D src/soc/intel/cannonlake/cbmem.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/cannonlake/romstage/romstage.c M src/soc/intel/cannonlake/smmrelocate.c M src/soc/intel/common/block/include/intelblocks/smm.h M src/soc/intel/common/block/smm/smm.c M src/soc/intel/common/block/systemagent/systemagent.c M src/soc/intel/denverton_ns/cpu.c M src/soc/intel/denverton_ns/include/soc/smm.h M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/denverton_ns/romstage.c M src/soc/intel/fsp_baytrail/memmap.c M src/soc/intel/fsp_baytrail/northcluster.c M src/soc/intel/fsp_broadwell_de/memmap.c D src/soc/intel/icelake/cbmem.c M src/soc/intel/icelake/memmap.c M src/soc/intel/icelake/romstage/romstage.c M src/soc/intel/icelake/smmrelocate.c M src/soc/intel/quark/memmap.c M src/soc/intel/quark/northcluster.c M src/soc/intel/quark/romstage/fsp2_0.c M src/soc/intel/skylake/memmap.c M src/soc/intel/skylake/romstage/romstage_fsp20.c M src/soc/intel/skylake/smmrelocate.c M src/soc/mediatek/common/cbmem.c M src/soc/nvidia/tegra124/cbmem.c M src/soc/nvidia/tegra210/cbmem.c M src/soc/qualcomm/ipq40xx/cbmem.c M src/soc/qualcomm/ipq806x/cbmem.c M src/soc/qualcomm/sdm845/cbmem.c M src/soc/rockchip/common/cbmem.c M src/soc/samsung/exynos5250/cbmem.c M src/soc/samsung/exynos5420/cbmem.c M src/soc/sifive/fu540/cbmem.c M src/soc/ucb/riscv/cbmem.c 102 files changed, 274 insertions(+), 350 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/30684/16
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30684 )
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/#/c/30684/16/src/lib/imd.c File src/lib/imd.c:
https://review.coreboot.org/#/c/30684/16/src/lib/imd.c@401 PS16, Line 401: imdr_init(&imd->sm, 0); Please mention this additional fixes also in the commit message.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30684 )
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Patch Set 16: Code-Review+1
Quite big change, it would be easier to review if you don't touch that many functions at once.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30684 )
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Patch Set 16:
(16 comments)
Sorry, some things slipped through my earlier review. Some reasoning for the BERT issue: I think if the address is used as a pointer in a subsystem, we shouldn't drag `uintptr_t` into it. A nit, though, as our ACPI code is full of `unsigned long` for addresses.
We are on the right track, I think: $ # removed casts to pointers: $ git show | grep '^-.**)' | wc -l 69 $ # added casts to pointers $ git show | grep '^+.**)' | wc -l 7
Six of the latter are actually unnecessary, they were three, unrelated casts before. So we could get the added count down to 1, the cast for/in BERT.
I understand that this is hard to split up (would add a lot of intermediate casts). Maybe, to reduce the round-trip time, I could fix my comments by myself and we review both?
https://review.coreboot.org/#/c/30684/16/src/arch/x86/acpi_bert_storage.c File src/arch/x86/acpi_bert_storage.c:
https://review.coreboot.org/#/c/30684/16/src/arch/x86/acpi_bert_storage.c@54 PS16, Line 54: void bert_errors_region(uintptr_t *start, size_t *size) I would prefer not to change this signature, having a `uintptr_t` here seems very odd. Also, it doesn't reduce the number of casts, you have one cast, it seems, either for the return value of cbmem_top() in stoney, or for the memset() argument below.
https://review.coreboot.org/#/c/30684/16/src/arch/x86/acpi_bert_storage.c@79 PS16, Line 79: return (void *)((u8 *)bert_region_base + alloc); OTOH, we could spare us the inner cast here now.
On the third? hand, making `bert_region_base` a `uint8_t *` would also spare us the cast.
https://review.coreboot.org/#/c/30684/16/src/drivers/intel/fsp1_1/stack.c File src/drivers/intel/fsp1_1/stack.c:
https://review.coreboot.org/#/c/30684/16/src/drivers/intel/fsp1_1/stack.c@13... PS16, Line 133: slot = stack_push32(slot, (uint32_t)smm_base | MTRR_TYPE_WRBACK); Do we need the cast?
https://review.coreboot.org/#/c/30684/16/src/drivers/intel/fsp2_0/hob_verify... File src/drivers/intel/fsp2_0/hob_verify.c:
https://review.coreboot.org/#/c/30684/16/src/drivers/intel/fsp2_0/hob_verify... PS16, Line 66: range_entry_end(&tolum), cbmem_top()); `lx` wants an `unsigned long`, cast?
https://review.coreboot.org/#/c/30684/16/src/lib/imd.c File src/lib/imd.c:
https://review.coreboot.org/#/c/30684/16/src/lib/imd.c@401 PS16, Line 401: imdr_init(&imd->sm, 0);
Please mention this additional fixes also in the commit message.
This changes because of the change in imd_init()'s signature. It's not really "additional" but part of the same change.
https://review.coreboot.org/#/c/30684/16/src/soc/intel/cannonlake/cbmem.c File src/soc/intel/cannonlake/cbmem.c:
PS16: Unrelated? Do it in advance?
https://review.coreboot.org/#/c/30684/16/src/soc/intel/cannonlake/smmrelocat... File src/soc/intel/cannonlake/smmrelocate.c:
https://review.coreboot.org/#/c/30684/16/src/soc/intel/cannonlake/smmrelocat... PS16, Line 249: memset((void *)params->ied_base + 1 * MiB, 0, 32 * KiB); Seems unrelated and is wrong, pointer arithmetic on `void *` is undefined (or at least implementation defined). I would leave this function as is. Or you add parentheses around the addition.
https://review.coreboot.org/#/c/30684/16/src/soc/intel/icelake/cbmem.c File src/soc/intel/icelake/cbmem.c:
PS16: Same as cannonlake.
https://review.coreboot.org/#/c/30684/16/src/soc/intel/icelake/smmrelocate.c File src/soc/intel/icelake/smmrelocate.c:
https://review.coreboot.org/#/c/30684/16/src/soc/intel/icelake/smmrelocate.c... PS16, Line 248: memset((void *)params->ied_base + 1 * MiB, 0, 32 * KiB); Same as above.
https://review.coreboot.org/#/c/30684/16/src/soc/intel/quark/romstage/fsp2_0... File src/soc/intel/quark/romstage/fsp2_0.c:
https://review.coreboot.org/#/c/30684/16/src/soc/intel/quark/romstage/fsp2_0... PS16, Line 76: postcar_frame_add_mtrr(&pcf, (uintptr_t) top_of_low_usable_memory, Not directly related, but... yes, it's declared as that ;)
https://review.coreboot.org/#/c/30684/16/src/soc/intel/skylake/memmap.c File src/soc/intel/skylake/memmap.c:
https://review.coreboot.org/#/c/30684/16/src/soc/intel/skylake/memmap.c@63 PS16, Line 63: uintptr_t sub_base; You called it `smm_base` above, please align.
https://review.coreboot.org/#/c/30684/16/src/soc/intel/skylake/smmrelocate.c File src/soc/intel/skylake/smmrelocate.c:
https://review.coreboot.org/#/c/30684/16/src/soc/intel/skylake/smmrelocate.c... PS16, Line 208: params->smram_base = (uintptr_t)handler_base; Remove cast, please.
https://review.coreboot.org/#/c/30684/16/src/soc/intel/skylake/smmrelocate.c... PS16, Line 210: params->ied_base = (uintptr_t)ied_base; Here, too.
https://review.coreboot.org/#/c/30684/16/src/soc/intel/skylake/smmrelocate.c... PS16, Line 257: memset((void *)params->ied_base + 1 * MiB, 0, 32 * KiB); And again.
https://review.coreboot.org/#/c/30684/16/src/soc/sifive/fu540/cbmem.c File src/soc/sifive/fu540/cbmem.c:
https://review.coreboot.org/#/c/30684/16/src/soc/sifive/fu540/cbmem.c@25 PS16, Line 25: FU540_MAXDRAM); Does this fit in a single line now?
https://review.coreboot.org/#/c/30684/16/src/soc/ucb/riscv/cbmem.c File src/soc/ucb/riscv/cbmem.c:
https://review.coreboot.org/#/c/30684/16/src/soc/ucb/riscv/cbmem.c@26 PS16, Line 26: return (base + size); Parents not needed any more.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30684 )
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Patch Set 16:
Is this still active? Or will be abandoned? Last comment 2/23/2019.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30684 )
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Patch Set 16:
Patch Set 16:
Is this still active? Or will be abandoned? Last comment 2/23/2019.
It still needs some work, but I haven't had time for it yet,
Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/30684 )
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Abandoned
I let this sleep for too long and close to half the files needed intervention when rebasing, so I started from scratch. CB:36559