Skip to content

Commit 00a3140

Browse files
committed
Peer review feedback
1 parent 439d8be commit 00a3140

6 files changed

Lines changed: 157 additions & 47 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: 86 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,19 @@
2626
#define _FORTIFY_SOURCE 0
2727
#endif
2828

29-
/* Enable %llu/%llx support on platforms with 64-bit types */
29+
/* Enable %llu/%llx support only on platforms where the 64-bit divide
30+
* is either native (64-bit CPUs) or backed by linked libgcc helpers
31+
* (PPC32 toolchain). The auto-enable deliberately excludes 32-bit
32+
* bare-metal targets (Cortex-M, x86 stage1) -- those would link-fail
33+
* on __aeabi_uldivmod / __udivmoddi4 because they don't pull in libgcc.
34+
* Such targets can still opt-in by defining PRINTF_LONG_LONG manually. */
3035
#if !defined(PRINTF_LONG_LONG) && ( \
3136
defined(__alpha__) || defined(__ia64__) || defined(_ARCH_PPC64) || \
3237
defined(__x86_64__) || defined(_M_X64) || \
3338
defined(__aarch64__) || defined(_M_ARM64) || \
3439
defined(__sparc64__) || defined(__s390x__) || \
3540
defined(__ppc64__) || defined(__powerpc64__) || defined(__PPC__) || \
36-
(defined(__riscv_xlen) && (__riscv_xlen == 64)) || \
37-
defined(__SIZEOF_LONG_LONG__))
41+
(defined(__riscv_xlen) && (__riscv_xlen == 64)))
3842
#define PRINTF_LONG_LONG
3943
#endif
4044

@@ -330,37 +334,82 @@ void *memmove(void *dst, const void *src, size_t n)
330334
#endif /* WOLFBOOT_USE_STDLIBC */
331335

332336
#if defined(PRINTF_ENABLED) && defined(DEBUG_UART)
337+
/* Shared digit table -- avoids duplicating the literal in each width's
338+
* formatting loop. */
339+
static const char uart_writenum_digits[] = "0123456789ABCDEF";
340+
341+
/* Shared tail for both widths: applies zeropad spacing, slides the digit
342+
* run into place, and emits to UART. Caller has filled digits at the
343+
* tail of `buf` and counted them in `sz`; `i` is the prefix length
344+
* (sign character only). */
345+
static void uart_writenum_emit(char *buf, int bufsize, int i, int sz,
346+
int zeropad, int maxdigits)
347+
{
348+
if (zeropad && sz < maxdigits) {
349+
i += maxdigits - sz;
350+
}
351+
memmove(&buf[i], &buf[bufsize - sz], sz);
352+
uart_write(buf, i + sz);
353+
}
354+
333355
void uart_writenum(int num, int base, int zeropad, int maxdigits)
334356
{
335-
int i = 0;
336-
char buf[sizeof(unsigned long)*2+1];
337-
const char* kDigitLut = "0123456789ABCDEF";
357+
int i = 0, sz = 0;
358+
char buf[sizeof(unsigned int) * 2 + 2];
338359
unsigned int val = (unsigned int)num;
339-
int sz = 0;
340360
if (maxdigits == 0)
341361
maxdigits = 8;
342362
if (maxdigits > (int)sizeof(buf))
343363
maxdigits = (int)sizeof(buf);
344364
memset(buf, 0, sizeof(buf));
345-
if (base == 10 && num < 0) { /* handle negative */
365+
if (base == 10 && num < 0) {
346366
buf[i++] = '-';
347-
val = -num;
367+
val = (unsigned int)(-num);
348368
}
349369
if (zeropad) {
350370
memset(&buf[i], '0', maxdigits);
351371
}
372+
/* 32-bit divide: stays out of libgcc 64-bit helpers, which aren't
373+
* linked into freestanding stage1 / Cortex-M builds. */
352374
do {
353-
buf[sizeof(buf)-sz-1] = kDigitLut[(val % base)];
375+
buf[sizeof(buf) - sz - 1] =
376+
uart_writenum_digits[(val % (unsigned)base)];
354377
sz++;
355-
val /= base;
378+
val /= (unsigned)base;
356379
} while (val > 0U);
357-
if (zeropad && sz < maxdigits) {
358-
i += maxdigits-sz;
380+
uart_writenum_emit(buf, sizeof(buf), i, sz, zeropad, maxdigits);
381+
}
382+
383+
#ifdef PRINTF_LONG_LONG
384+
/* 64-bit core for %llu/%lld/%llx. Pulls in libgcc 64-bit divide
385+
* (__udivmoddi4 / __aeabi_uldivmod) so it's only compiled when needed
386+
* and only called from the long-long printf paths -- never from the
387+
* 32-bit uart_writenum() fast path above. */
388+
static void uart_writenum_ll(unsigned long long val, int is_negative,
389+
int base, int zeropad, int maxdigits)
390+
{
391+
int i = 0, sz = 0;
392+
char buf[sizeof(unsigned long long) * 2 + 2];
393+
if (maxdigits == 0)
394+
maxdigits = 8;
395+
if (maxdigits > (int)sizeof(buf))
396+
maxdigits = (int)sizeof(buf);
397+
memset(buf, 0, sizeof(buf));
398+
if (is_negative) {
399+
buf[i++] = '-';
359400
}
360-
memmove(&buf[i], &buf[sizeof(buf)-sz], sz);
361-
i+=sz;
362-
uart_write(buf, i);
401+
if (zeropad) {
402+
memset(&buf[i], '0', maxdigits);
403+
}
404+
do {
405+
buf[sizeof(buf) - sz - 1] =
406+
uart_writenum_digits[(val % (unsigned)base)];
407+
sz++;
408+
val /= (unsigned)base;
409+
} while (val > 0ULL);
410+
uart_writenum_emit(buf, sizeof(buf), i, sz, zeropad, maxdigits);
363411
}
412+
#endif /* PRINTF_LONG_LONG */
364413

365414
void uart_vprintf(const char* fmt, va_list argp)
366415
{
@@ -434,9 +483,23 @@ void uart_vprintf(const char* fmt, va_list argp)
434483
{
435484
#ifdef PRINTF_LONG_LONG
436485
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);
486+
/* %llu / %lld: full 64-bit value */
487+
int is_neg = 0;
488+
unsigned long long val;
489+
if (*fmtp != 'u') {
490+
long long sll = va_arg(argp, long long);
491+
if (sll < 0) {
492+
is_neg = 1;
493+
val = (unsigned long long)(-sll);
494+
}
495+
else {
496+
val = (unsigned long long)sll;
497+
}
498+
}
499+
else {
500+
val = va_arg(argp, unsigned long long);
501+
}
502+
uart_writenum_ll(val, is_neg, 10, zeropad, maxdigits);
440503
}
441504
else
442505
#endif
@@ -454,9 +517,10 @@ void uart_vprintf(const char* fmt, va_list argp)
454517
{
455518
#ifdef PRINTF_LONG_LONG
456519
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);
520+
/* %llx: full 64-bit value */
521+
unsigned long long val =
522+
va_arg(argp, unsigned long long);
523+
uart_writenum_ll(val, 0, 16, zeropad, maxdigits);
460524
}
461525
else
462526
#endif

0 commit comments

Comments
 (0)