Skip to content

Commit 38f0c58

Browse files
dgarskedgarske
committed
Peer review feedback
1 parent 439d8be commit 38f0c58

6 files changed

Lines changed: 127 additions & 39 deletions

File tree

‎hal/nxp_ppc.h‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -202,18 +202,15 @@
202202
#define ENABLE_FMAN
203203

204204
#ifdef BOARD_CW_VPX3152
205-
/* Relocate CCSRBAR: default 0xFE000000 (16MB) falls within 256MB flash
206-
* VA range 0xF0000000-0xFFFFFFFF. Move to 0xEF000000 (just below flash).
207-
* The existing relocation code in boot_ppc_start.S handles the hardware
208-
* CCSRBAR register write when CCSRBAR_DEF != CCSRBAR_PHYS.
205+
/* Relocate CCSRBAR: default 0xFE000000 (16MB) falls within the 256MB
206+
* flash VA range 0xF0000000-0xFFFFFFFF. Move to 0xEF000000 (just below
207+
* flash). The existing relocation code in boot_ppc_start.S handles the
208+
* hardware CCSRBAR register write when CCSRBAR_DEF != CCSRBAR_PHYS.
209209
*
210-
* Two physical-address profiles supported here:
211-
* PHYS_HIGH=0xF: 36-bit-aliased CCSR (PA=0xF_EF000000). Matches the
212-
* FDT cw_152_64.dtb soc.ranges entry for VxWorks 7
213-
* but caused VxWorks 7 to silent-boot from wolfBoot.
214-
* PHYS_HIGH=0: 32-bit CCSR (PA=0x0_EF000000). Matches the
215-
* *production* CW U-Boot tlb_table, which boots
216-
* VxWorks 7 64-bit fully (CPU Count: 8, ...). */
210+
* CCSR is mapped at PA=0xF_EF000000 (36-bit-aliased, PHYS_HIGH=0xF) to
211+
* match the cw_152_64.dtb soc.ranges entry. A 32-bit alternative
212+
* (PA=0x0_EF000000) was tested during bring-up but is not selectable
213+
* in this header. */
217214
#define CCSRBAR 0xEF000000
218215
#define CCSRBAR_PHYS_HIGH 0xF
219216
#define CCSRBAR_NEW_REG 0x00FEF000 /* (PHYS_HIGH << 20) | (CCSRBAR >> 12) */
@@ -290,6 +287,13 @@
290287
#ifndef CCSRBAR_PHYS_HIGH
291288
#define CCSRBAR_PHYS_HIGH 0
292289
#endif
290+
/* Encoding written into CCSRBAR for the legacy (e500/e500mc) relocation
291+
* path: PHYS_HIGH in [23:20], PA[35:12] in the low bits. Boards that
292+
* relocate CCSRBAR but don't use USE_CORENET_INTERFACE need this
293+
* pre-computed because GAS @h/@l can't evaluate the shift/OR. */
294+
#ifndef CCSRBAR_NEW_REG
295+
#define CCSRBAR_NEW_REG ((CCSRBAR_PHYS_HIGH << 20) | (CCSRBAR >> 12))
296+
#endif
293297

294298

295299
/* DDR */

‎hal/nxp_t2080.c‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,7 +1166,9 @@ static void hal_mp_init(void)
11661166
{
11671167
uint32_t *fixup = (uint32_t*)&_secondary_start_page;
11681168
uint32_t bootpg, second_half_ddr, spin_table_ddr;
1169+
#ifdef BOARD_CW_VPX3152
11691170
volatile uint32_t *bp, *st;
1171+
#endif
11701172
size_t i;
11711173
const volatile uint32_t *s;
11721174
volatile uint32_t *d;
@@ -1344,11 +1346,28 @@ int hal_dts_fixup(void* dts_addr)
13441346
* own spin-table page (0x7fee4000); wolfBoot's spin table lives
13451347
* elsewhere, so we reserve based on the actual runtime address. */
13461348
{
1349+
int rsv_ret;
13471350
uint64_t spin_pg = (uint64_t)(g_spin_table_ddr & ~0xFFFU);
1348-
fdt_add_mem_rsv(fdt, spin_pg, 0x1000ULL); /* wolfBoot spin table */
1351+
1352+
rsv_ret = fdt_add_mem_rsv(fdt, spin_pg, 0x1000ULL);
1353+
if (rsv_ret != 0) {
1354+
wolfBoot_printf("FDT: failed to reserve spin-table page "
1355+
"@ 0x%llx: %d\n", spin_pg, rsv_ret);
1356+
return rsv_ret;
1357+
}
1358+
rsv_ret = fdt_add_mem_rsv(fdt, 0x7ffff000ULL, 0x1000ULL);
1359+
if (rsv_ret != 0) {
1360+
wolfBoot_printf("FDT: failed to reserve boot page "
1361+
"@ 0x7ffff000: %d\n", rsv_ret);
1362+
return rsv_ret;
1363+
}
1364+
rsv_ret = fdt_add_mem_rsv(fdt, 0xfffff000ULL, 0x1000ULL);
1365+
if (rsv_ret != 0) {
1366+
wolfBoot_printf("FDT: failed to reserve top-of-4GB page "
1367+
"@ 0xfffff000: %d\n", rsv_ret);
1368+
return rsv_ret;
1369+
}
13491370
}
1350-
fdt_add_mem_rsv(fdt, 0x7ffff000ULL, 0x1000ULL); /* boot page (top of 2GB) */
1351-
fdt_add_mem_rsv(fdt, 0xfffff000ULL, 0x1000ULL); /* top of 4GB DDR */
13521371
#endif
13531372

13541373
/* fixup the memory region.

‎src/boot_ppc.c‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -401,22 +401,36 @@ void RAMFUNCTION wolfBoot_os64bit_jump(os64bit_entry_t entry,
401401
{
402402
extern void hal_flash_cache_disable_pre_os(void);
403403
extern unsigned int isr_empty;
404+
extern unsigned int isr_empty_end;
404405
{
405406
volatile uint32_t *src = (volatile uint32_t *)&isr_empty;
406407
volatile uint32_t *dst = (volatile uint32_t *)
407408
WOLFBOOT_OS64BIT_IVPR_DDR;
408-
int i;
409-
/* Copy 256 bytes -- isr_empty is ~208 bytes; round up to a
410-
* power-of-two so dcbst/icbi loop is simple. */
411-
for (i = 0; i < 64; i++) {
409+
uintptr_t bytes = (uintptr_t)&isr_empty_end -
410+
(uintptr_t)&isr_empty;
411+
/* Word count, rounded up. Cache-line range, rounded up to
412+
* 64-byte lines (one e6500 dcbz/dcbst granule). */
413+
uintptr_t copy_words = (bytes + sizeof(uint32_t) - 1U) /
414+
sizeof(uint32_t);
415+
uintptr_t cache_words = ((copy_words + 15U) / 16U) * 16U;
416+
uintptr_t i;
417+
418+
/* Reserve enough headroom in the IVPR landing zone for the
419+
* handler to grow without silently truncating the copy. */
420+
#define WOLFBOOT_OS64BIT_IVPR_MAX 0x400U /* 1 KB */
421+
if (bytes > WOLFBOOT_OS64BIT_IVPR_MAX) {
422+
return; /* refuse to jump with a truncated handler */
423+
}
424+
425+
for (i = 0; i < copy_words; i++) {
412426
dst[i] = src[i];
413427
}
414428
/* Make the DDR copy visible to the I-cache fetcher. */
415-
for (i = 0; i < 64; i += 16) { /* one cache line per 64 bytes */
429+
for (i = 0; i < cache_words; i += 16U) { /* 64 bytes per line */
416430
__asm__ __volatile__("dcbst 0,%0" :: "r"(&dst[i]) : "memory");
417431
}
418432
__asm__ __volatile__("sync" ::: "memory");
419-
for (i = 0; i < 64; i += 16) {
433+
for (i = 0; i < cache_words; i += 16U) {
420434
__asm__ __volatile__("icbi 0,%0" :: "r"(&dst[i]) : "memory");
421435
}
422436
__asm__ __volatile__("sync; isync" ::: "memory");
@@ -479,9 +493,11 @@ void do_boot(const uint32_t *app_offset)
479493
wolfBoot_printf("do_boot: pre-transition\n");
480494
#endif
481495

482-
#ifdef MMU
496+
#if defined(MMU) && !defined(BUILD_LOADER_STAGE1)
483497
/* Flush FDT from D-cache to DDR so the OS sees all fixup changes.
484-
* Must be done after hal_dts_fixup and before entry(). */
498+
* Must be done after hal_dts_fixup and before entry(). Stage1
499+
* doesn't run hal_dts_fixup, so the flush isn't needed (and
500+
* flush_cache itself is excluded from stage1). */
485501
flush_cache((uint32_t)dts_offset, WOLFBOOT_DTS_MAX_SIZE);
486502
#endif
487503

‎src/boot_ppc_mp.S‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,8 @@ branch_prediction:
182182
#if defined(CORE_E5500) || defined(CORE_E6500)
183183
rlwinm r8, r0, 29, 0x03 /* r8 = core within cluster */
184184
srwi r10, r0, 5 /* r10 = cluster */
185+
mr r9, r10 /* r9 = cluster (preserved across r10 reuse
186+
* below as spin-table base) */
185187

186188
mulli r5, r10, CORES_PER_CLUSTER
187189
add r5, r5, r8 /* r5 = linear core ID */
@@ -221,9 +223,12 @@ branch_prediction:
221223
* wolfBoot releases secondaries during hal_mp_init before the OS
222224
* jump, so we MUST gate L2FI on (cluster != primary's cluster).
223225
*
224-
* r10 already holds the cluster ID from the PIR-decoding above.
225-
* Primary boots on cluster 0, so skip L2 init when r10 == 0. */
226-
cmpwi r10, 0
226+
* r9 holds the cluster ID (preserved before r10 was repurposed as
227+
* the spin-table base). Primary boots on cluster 0, so skip L2
228+
* init when r9 == 0. Note: gating on r5 (linear core ID) would be
229+
* wrong -- secondaries in cluster 0 have cluster=0 but
230+
* linear core ID != 0 and would still hit L2FI. */
231+
cmpwi r9, 0
227232
beq l2_init_skip_e6500
228233

229234
msync

‎src/boot_ppc_start.S‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,6 +1450,8 @@ isr_empty:
14501450
stw r4, 0x1C(r3)
14511451
#endif
14521452
1: b 1b
1453+
.global isr_empty_end
1454+
isr_empty_end:
14531455
#endif
14541456

14551457
/* reset entry point - must be at end of .S */

‎src/string.c‎

Lines changed: 56 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -330,30 +330,42 @@ void *memmove(void *dst, const void *src, size_t n)
330330
#endif /* WOLFBOOT_USE_STDLIBC */
331331

332332
#if defined(PRINTF_ENABLED) && defined(DEBUG_UART)
333-
void uart_writenum(int num, int base, int zeropad, int maxdigits)
333+
/* The widest unsigned/signed types we'll format. With PRINTF_LONG_LONG
334+
* this is 64-bit (so %llu/%llx don't truncate); without, it's 32-bit
335+
* (no libgcc 64-bit divide pulled in for size-constrained targets). */
336+
#ifdef PRINTF_LONG_LONG
337+
typedef unsigned long long uart_writenum_uval;
338+
typedef long long uart_writenum_sval;
339+
#else
340+
typedef unsigned long uart_writenum_uval;
341+
typedef long uart_writenum_sval;
342+
#endif
343+
344+
/* Core: prints an already-absolute magnitude with an explicit
345+
* negative-sign flag. Bases up to 16. */
346+
static void uart_writenum_core(uart_writenum_uval val, int is_negative,
347+
int base, int zeropad, int maxdigits)
334348
{
335349
int i = 0;
336-
char buf[sizeof(unsigned long)*2+1];
350+
char buf[sizeof(uart_writenum_uval)*2+2]; /* hex digits + sign + NUL */
337351
const char* kDigitLut = "0123456789ABCDEF";
338-
unsigned int val = (unsigned int)num;
339352
int sz = 0;
340353
if (maxdigits == 0)
341354
maxdigits = 8;
342355
if (maxdigits > (int)sizeof(buf))
343356
maxdigits = (int)sizeof(buf);
344357
memset(buf, 0, sizeof(buf));
345-
if (base == 10 && num < 0) { /* handle negative */
358+
if (is_negative) {
346359
buf[i++] = '-';
347-
val = -num;
348360
}
349361
if (zeropad) {
350362
memset(&buf[i], '0', maxdigits);
351363
}
352364
do {
353-
buf[sizeof(buf)-sz-1] = kDigitLut[(val % base)];
365+
buf[sizeof(buf)-sz-1] = kDigitLut[(val % (unsigned)base)];
354366
sz++;
355-
val /= base;
356-
} while (val > 0U);
367+
val /= (unsigned)base;
368+
} while (val > 0);
357369
if (zeropad && sz < maxdigits) {
358370
i += maxdigits-sz;
359371
}
@@ -362,6 +374,20 @@ void uart_writenum(int num, int base, int zeropad, int maxdigits)
362374
uart_write(buf, i);
363375
}
364376

377+
void uart_writenum(int num, int base, int zeropad, int maxdigits)
378+
{
379+
int is_neg = 0;
380+
uart_writenum_uval val;
381+
if (base == 10 && num < 0) {
382+
is_neg = 1;
383+
val = (uart_writenum_uval)(-(uart_writenum_sval)num);
384+
}
385+
else {
386+
val = (uart_writenum_uval)(unsigned int)num;
387+
}
388+
uart_writenum_core(val, is_neg, base, zeropad, maxdigits);
389+
}
390+
365391
void uart_vprintf(const char* fmt, va_list argp)
366392
{
367393
char* fmtp = (char*)fmt;
@@ -434,9 +460,24 @@ void uart_vprintf(const char* fmt, va_list argp)
434460
{
435461
#ifdef PRINTF_LONG_LONG
436462
if (islong >= 2) {
437-
/* %llu / %lld: consume 64-bit arg, print low 32 bits */
438-
unsigned long long ll = va_arg(argp, unsigned long long);
439-
uart_writenum((int)(unsigned int)ll, 10, zeropad, maxdigits);
463+
/* %llu / %lld: full 64-bit value */
464+
int is_neg = 0;
465+
uart_writenum_uval val;
466+
if (*fmtp != 'u') {
467+
long long sll = va_arg(argp, long long);
468+
if (sll < 0) {
469+
is_neg = 1;
470+
val = (uart_writenum_uval)(-sll);
471+
}
472+
else {
473+
val = (uart_writenum_uval)sll;
474+
}
475+
}
476+
else {
477+
val = (uart_writenum_uval)
478+
va_arg(argp, unsigned long long);
479+
}
480+
uart_writenum_core(val, is_neg, 10, zeropad, maxdigits);
440481
}
441482
else
442483
#endif
@@ -454,9 +495,10 @@ void uart_vprintf(const char* fmt, va_list argp)
454495
{
455496
#ifdef PRINTF_LONG_LONG
456497
if (islong >= 2) {
457-
/* %llx: consume 64-bit arg, print low 32 bits */
458-
unsigned long long ll = va_arg(argp, unsigned long long);
459-
uart_writenum((int)(unsigned int)ll, 16, zeropad, maxdigits);
498+
/* %llx: full 64-bit value */
499+
uart_writenum_uval val = (uart_writenum_uval)
500+
va_arg(argp, unsigned long long);
501+
uart_writenum_core(val, 0, 16, zeropad, maxdigits);
460502
}
461503
else
462504
#endif

0 commit comments

Comments
 (0)