coreboot-gerrit August 2016

coreboot-gerrit@coreboot.org

1 participants
1551 discussions

New patch to review for coreboot: [WIP] arch/riscv: Better trap handling diagnostics
by Jonathan Neuschäfer 01 Aug '16

01 Aug '16

Jonathan Neuschäfer (j.neuschaefer(a)gmx.net) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16016 -gerrit commit 9bd8814c73599b25f027b6a955dd3a330ff20148 Author: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Date: Mon Aug 1 19:42:27 2016 +0200 [WIP] arch/riscv: Better trap handling diagnostics WIP: By now I should probably move the diagnistic prints into a function. Change-Id: I57032f958c88ea83a420e93b459df4f620799d84 Signed-off-by: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> --- src/arch/riscv/trap_handler.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/arch/riscv/trap_handler.c b/src/arch/riscv/trap_handler.c index 193be61..afb02de 100644 --- a/src/arch/riscv/trap_handler.c +++ b/src/arch/riscv/trap_handler.c @@ -84,11 +84,24 @@ void handle_supervisor_call(trapframe *tf) { asm volatile("j supervisor_call_return"); } +static const char *mstatus_to_previous_mode(uintptr_t ms) +{ + switch (ms & MSTATUS_MPP) { + case 0x00000000: return "user"; + case 0x00000800: return "supervisor"; + case 0x00001000: return "hypervisor"; + case 0x00001800: return "machine"; + } + + return "unknown"; +} + void trap_handler(trapframe *tf) { write_csr(mscratch, tf); uintptr_t cause; void *epc; void *badAddr; + const char *previous_mode; // extract cause asm("csrr %0, mcause" : "=r"(cause)); @@ -99,6 +112,8 @@ void trap_handler(trapframe *tf) { // extract bad address asm("csrr %0, mbadaddr" : "=r"(badAddr)); + previous_mode = mstatus_to_previous_mode(read_csr(mstatus)); + switch(cause) { case 0: printk(BIOS_DEBUG, "Trap: Instruction address misaligned\n"); @@ -107,23 +122,28 @@ void trap_handler(trapframe *tf) { printk(BIOS_DEBUG, "Trap: Instruction access fault\n"); printk(BIOS_DEBUG, "Bad instruction pc: %p\n", epc); printk(BIOS_DEBUG, "Address: %p\n", badAddr); + printk(BIOS_DEBUG, "Previous mode: %s\n", previous_mode); break; case 2: printk(BIOS_DEBUG, "Trap: Illegal instruction\n"); printk(BIOS_DEBUG, "Bad instruction pc: %p\n", epc); printk(BIOS_DEBUG, "Address: %p\n", badAddr); + printk(BIOS_DEBUG, "Previous mode: %s\n", previous_mode); break; case 3: printk(BIOS_DEBUG, "Trap: Breakpoint\n"); + printk(BIOS_DEBUG, "Bad instruction pc: %p\n", epc); break; case 4: printk(BIOS_DEBUG, "Trap: Load address misaligned\n"); + printk(BIOS_DEBUG, "Previous mode: %s\n", previous_mode); handle_misaligned_load(tf); break; case 5: printk(BIOS_DEBUG, "Trap: Load access fault\n"); printk(BIOS_DEBUG, "Bad instruction pc: %p\n", epc); printk(BIOS_DEBUG, "Load Address: %p\n", badAddr); + printk(BIOS_DEBUG, "Previous mode: %s\n", previous_mode); break; case 6: printk(BIOS_DEBUG, "Trap: Store address misaligned\n"); @@ -135,6 +155,7 @@ void trap_handler(trapframe *tf) { printk(BIOS_DEBUG, "Trap: Store access fault\n"); printk(BIOS_DEBUG, "Bad instruction pc: %p\n", epc); printk(BIOS_DEBUG, "Store Address: %p\n", badAddr); + printk(BIOS_DEBUG, "Previous mode: %s\n", previous_mode); break; case 8: printk(BIOS_DEBUG, "Trap: Environment call from U-mode\n");

1 0

New patch to review for coreboot: [WIP] arch/riscv: Virtual memory fixes
by Jonathan Neuschäfer 01 Aug '16

01 Aug '16

Jonathan Neuschäfer (j.neuschaefer(a)gmx.net) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16015 -gerrit commit 777c596251c00fdb4ede33284c590860e1843c8d Author: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Date: Mon Aug 1 19:42:27 2016 +0200 [WIP] arch/riscv: Virtual memory fixes - I implemented a function to walk the whole pagetable structure. Ideally we'd walk the pagetable for a specific virtual address whenever a trap from a lower-than-machine mode is received, so that pagetable misconfigurations are more easily caught. - plus some other fixes that I need to split up. Change-Id: I52a863d8bc814ab3ed3a1f141d0a77edc6e4044d Signed-off-by: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> --- src/arch/riscv/virtual_memory.c | 69 +++++++++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 10 deletions(-) diff --git a/src/arch/riscv/virtual_memory.c b/src/arch/riscv/virtual_memory.c index cda9057..eae0b26 100644 --- a/src/arch/riscv/virtual_memory.c +++ b/src/arch/riscv/virtual_memory.c @@ -20,13 +20,64 @@ #include <stdint.h> #include <console/console.h> +/* walk_page_table(address) */ + pte_t* root_page_table; +static void indent(int level) +{ + int i; + for (i = 0; i < level; i++) + printk(BIOS_DEBUG, " "); +} + +static uintptr_t index_to_virt_addr(int index, int level) +{ + return ((intptr_t)index) + << (64 - RISCV_PGLEVEL_BITS - level * RISCV_PGLEVEL_BITS) + >> (64 - VA_BITS); +} + +static void walk_page_table_at(pte_t *pt, intptr_t virt_addr, int level) +{ + int i; + + indent(level); + printk(BIOS_DEBUG, "Level %d page table at 0x%p\n", level, pt); + + for (i = 0; i < RISCV_PGSIZE/sizeof(pte_t); i++) { + char urwx[8]; + uintptr_t pointer; + intptr_t next_virt_addr; + + if (!(pt[i] & PTE_V)) + continue; + + urwx[0] = (pt[i] & PTE_U)? 'u' : '-'; + urwx[1] = (pt[i] & PTE_R)? 'r' : '-'; + urwx[2] = (pt[i] & PTE_W)? 'w' : '-'; + urwx[3] = (pt[i] & PTE_X)? 'x' : '-'; + urwx[4] = '\0'; + + next_virt_addr = virt_addr + index_to_virt_addr(i, level); + + indent(level+1); + printk(BIOS_DEBUG, "Valid PTE at index %d (0x%016zx), ", + i, (size_t) next_virt_addr); + if (PTE_TABLE(pt[i])) + printk(BIOS_DEBUG, "page table\n"); + else + printk(BIOS_DEBUG, "protections %s\n", urwx); + + if (PTE_TABLE(pt[i])) { + pointer = ((uintptr_t)pt[i] >> 10) << RISCV_PGSHIFT; + walk_page_table_at((pte_t *)pointer, next_virt_addr, level + 1); + } + } +} + void walk_page_table(void) { - // TODO: implement a full walk to make sure memory was set up - //const size_t pte_per_page = RISCV_PGSIZE/sizeof(void*); - pte_t* t = root_page_table; - printk(BIOS_DEBUG, "root_page_table: %p\n", t); + walk_page_table_at(root_page_table, 0, 0); } void flush_tlb(void) @@ -70,7 +121,7 @@ void init_vm(uintptr_t virtMemStart, uintptr_t physMemStart, uintptr_t pageTable root_pt[(1<<RISCV_PGLEVEL_BITS)-num_middle_pts+i] = ptd_create(((uintptr_t)middle_pt >> RISCV_PGSHIFT) + i); // fill the middle page table - for (uintptr_t vaddr = virtMemStart, paddr = physMemStart; paddr < memorySize; vaddr += SUPERPAGE_SIZE, paddr += SUPERPAGE_SIZE) { + for (uintptr_t vaddr = virtMemStart, paddr = physMemStart; paddr < physMemStart+memorySize; vaddr += SUPERPAGE_SIZE, paddr += SUPERPAGE_SIZE) { int l2_shift = RISCV_PGLEVEL_BITS + RISCV_PGSHIFT; size_t l2_idx = (virtMemStart >> l2_shift) & ((1 << RISCV_PGLEVEL_BITS)-1); l2_idx += ((vaddr - virtMemStart) >> l2_shift); @@ -112,9 +163,9 @@ void initVirtualMemory(void) { } printk(BIOS_DEBUG, "Initializing virtual memory...\n"); - uintptr_t physicalStart = 0x1000000; // TODO: Figure out how to grab this from cbfs - uintptr_t virtualStart = 0xffffffff81000000; - uintptr_t pageTableStart = 0x1400000; + uintptr_t physicalStart = 0x90000000; // TODO: Figure out how to grab this from cbfs + uintptr_t virtualStart = 0xffffffff80000000; + uintptr_t pageTableStart = 0x91400000; init_vm(virtualStart, physicalStart, pageTableStart); mb(); printk(BIOS_DEBUG, "Finished initializing virtual memory, starting walk...\n"); @@ -123,8 +174,6 @@ void initVirtualMemory(void) { void mstatus_init(void) { - // supervisor support is required - uintptr_t ms = 0; ms = INSERT_FIELD(ms, MSTATUS_PRV, PRV_M); ms = INSERT_FIELD(ms, MSTATUS_PRV1, PRV_S);

1 0

New patch to review for coreboot: src/arch/riscv/id.S: Don't hardcode the strings
by Jonathan Neuschäfer 01 Aug '16

01 Aug '16

Jonathan Neuschäfer (j.neuschaefer(a)gmx.net) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16014 -gerrit commit 089a4596ae830e127de4265b2073139ef967b1a2 Author: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Date: Mon Aug 1 19:42:27 2016 +0200 src/arch/riscv/id.S: Don't hardcode the strings Change-Id: Ide87c45806c5e58775c77e7f780efb4cf81a70c9 Signed-off-by: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> --- src/arch/riscv/id.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/arch/riscv/id.S b/src/arch/riscv/id.S index 798aa89..311b989 100644 --- a/src/arch/riscv/id.S +++ b/src/arch/riscv/id.S @@ -18,11 +18,11 @@ .globl __id_start __id_start: ver: - .asciz "1" //COREBOOT_VERSION + .asciz COREBOOT_VERSION vendor: - .asciz "ucb" //CONFIG_MAINBOARD_VENDOR + .asciz CONFIG_MAINBOARD_VENDOR part: - .asciz "1" //CONFIG_MAINBOARD_PART_NUMBER + .asciz CONFIG_MAINBOARD_PART_NUMBER .long __id_end + CONFIG_ID_SECTION_OFFSET - ver /* Reverse offset to the vendor id */ .long __id_end + CONFIG_ID_SECTION_OFFSET - vendor /* Reverse offset to the vendor id */ .long __id_end + CONFIG_ID_SECTION_OFFSET - part /* Reverse offset to the part number */

1 0

Patch set updated for coreboot: cbfstool: Check arguments to strtoul() where appropriate
by Nico Huber 01 Aug '16

01 Aug '16

Nico Huber (nico.h(a)gmx.de) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16012 -gerrit commit 497c0e5696b9f442fa450d50671f1b2084a8d826 Author: Nico Huber <nico.h(a)gmx.de> Date: Mon Aug 1 21:37:42 2016 +0200 cbfstool: Check arguments to strtoul() where appropriate The interface to strtoul() is a weird mess. It may or may not set errno if no conversion is done. So check for empty strings and trailing characters. Change-Id: I82373d2a0102fc89144bd12376b5ea3b10c70153 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- util/cbfstool/cbfstool.c | 82 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 70 insertions(+), 12 deletions(-) diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index 7b30ce9..e1bdf2f 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -1350,24 +1350,51 @@ int main(int argc, char **argv) param.source_region = optarg; break; case 'b': - param.baseaddress = strtoul(optarg, NULL, 0); + param.baseaddress = strtoul(optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid base address '%s'.\n", + optarg); + return 1; + } // baseaddress may be zero on non-x86, so we // need an explicit "baseaddress_assigned". param.baseaddress_assigned = 1; break; case 'l': - param.loadaddress = strtoul(optarg, NULL, 0); + param.loadaddress = strtoul(optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid load address '%s'.\n", + optarg); + return 1; + } break; case 'e': - param.entrypoint = strtoul(optarg, NULL, 0); + param.entrypoint = strtoul(optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid entry point '%s'.\n", + optarg); + return 1; + } break; case 's': param.size = strtoul(optarg, &suffix, 0); - if (tolower((int)suffix[0])=='k') { - param.size *= 1024; + if (!*optarg) { + ERROR("Empty size specified.\n"); + return 1; } - if (tolower((int)suffix[0])=='m') { + switch (tolower((int)suffix[0])) { + case 'k': + param.size *= 1024; + break; + case 'm': param.size *= 1024 * 1024; + break; + case '\0': + break; + default: + ERROR("Invalid suffix for size '%s'.\n", + optarg); + return 1; } break; case 'B': @@ -1375,24 +1402,49 @@ int main(int argc, char **argv) break; case 'H': param.headeroffset = strtoul( - optarg, NULL, 0); + optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid header offset '%s'.\n", + optarg); + return 1; + } param.headeroffset_assigned = 1; break; case 'a': - param.alignment = strtoul(optarg, NULL, 0); + param.alignment = strtoul(optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid alignment '%s'.\n", + optarg); + return 1; + } break; case 'P': - param.pagesize = strtoul(optarg, NULL, 0); + param.pagesize = strtoul(optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid page size '%s'.\n", + optarg); + return 1; + } break; case 'o': - param.cbfsoffset = strtoul(optarg, NULL, 0); + param.cbfsoffset = strtoul(optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid cbfs offset '%s'.\n", + optarg); + return 1; + } param.cbfsoffset_assigned = 1; break; case 'f': param.filename = optarg; break; case 'i': - param.u64val = strtoull(optarg, NULL, 0); + param.u64val = strtoull(optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid int parameter '%s'.\n", + optarg); + return 1; + } break; case 'u': param.fill_partial_upward = true; @@ -1404,7 +1456,13 @@ int main(int argc, char **argv) param.show_immutable = true; break; case 'x': - param.fit_empty_entries = strtol(optarg, NULL, 0); + param.fit_empty_entries = strtol( + optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid number of fit entries " + "'%s'.\n", optarg); + return 1; + } break; case 'v': verbose++;

1 0

Patch set updated for coreboot: Makefile.inc: Strip output of `wc`
by Nico Huber 01 Aug '16

01 Aug '16

Nico Huber (nico.h(a)gmx.de) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16013 -gerrit commit 3bd0fa09965eea65b189f15b58abc3085124041b Author: Nico Huber <nico.h(a)gmx.de> Date: Mon Aug 1 22:42:50 2016 +0200 Makefile.inc: Strip output of `wc` Apparently BSD's wc indents its output. Change-Id: I77f50a4b7d6012782f1c1b42ba20a64721c186c8 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- Makefile.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.inc b/Makefile.inc index 7c471dd..b3d429a 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -129,7 +129,7 @@ int-lt=$(if $(filter 1,$(words $1)),$(strip $1),$(shell expr $(call _toint,$(wor int-gt=$(if $(filter 1,$(words $1)),$(strip $1),$(shell expr $(call _toint,$(word 1,$1)) \> $(call _toint,$(word 2,$1)))) int-eq=$(if $(filter 1,$(words $1)),$(strip $1),$(shell expr $(call _toint,$(word 1,$1)) = $(call _toint,$(word 2,$1)))) int-align=$(shell A=$(call _toint,$1) B=$(call _toint,$2); expr $$A + $ \( $$B - \( $$A % $$B $ \) % $$B \) ) -file-size=$(shell cat $1 | wc -c) +file-size=$(strip $(shell cat $1 | wc -c)) tolower=$(shell echo '$1' | tr '[:upper:]' '[:lower:]') toupper=$(shell echo '$1' | tr '[:lower:]' '[:upper:]') ws_to_under=$(shell echo '$1' | tr ' \t' '_')

1 0

New patch to review for coreboot: Makefile.inc: Strip output of `wc`
by Nico Huber 01 Aug '16

01 Aug '16

Nico Huber (nico.h(a)gmx.de) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16013 -gerrit commit a50aec5dd645ef66b2a845ad05da38ccd713ac96 Author: Nico Huber <nico.h(a)gmx.de> Date: Mon Aug 1 22:42:50 2016 +0200 Makefile.inc: Strip output of `wc` Apparently BSD's wc indents its output. Change-Id: I77f50a4b7d6012782f1c1b42ba20a64721c186c8 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- Makefile.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.inc b/Makefile.inc index 7c471dd..b3d429a 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -129,7 +129,7 @@ int-lt=$(if $(filter 1,$(words $1)),$(strip $1),$(shell expr $(call _toint,$(wor int-gt=$(if $(filter 1,$(words $1)),$(strip $1),$(shell expr $(call _toint,$(word 1,$1)) \> $(call _toint,$(word 2,$1)))) int-eq=$(if $(filter 1,$(words $1)),$(strip $1),$(shell expr $(call _toint,$(word 1,$1)) = $(call _toint,$(word 2,$1)))) int-align=$(shell A=$(call _toint,$1) B=$(call _toint,$2); expr $$A + $ \( $$B - \( $$A % $$B $ \) % $$B \) ) -file-size=$(shell cat $1 | wc -c) +file-size=$(strip $(shell cat $1 | wc -c)) tolower=$(shell echo '$1' | tr '[:upper:]' '[:lower:]') toupper=$(shell echo '$1' | tr '[:lower:]' '[:upper:]') ws_to_under=$(shell echo '$1' | tr ' \t' '_')

1 0

Patch merged into coreboot/master: soc/intel/quark: Enable use of hard reset
by gerrit＠coreboot.org 01 Aug '16

01 Aug '16

the following patch was just integrated into master: commit 6e8233a60b475ff56bfe672293f081653e552510 Author: Lee Leahy <leroy.p.leahy(a)intel.com> Date: Sat Jul 30 10:34:22 2016 -0700 soc/intel/quark: Enable use of hard reset Select HAVE_HARD_RESET in the KCONFIG file to enable use of the hard_reset routine. TEST=Build and run on Galileo Gen2 Change-Id: Ib11a80b64cf1c55aec24f2576d197da9017b9751 Signed-off-by: Lee Leahy <leroy.p.leahy(a)intel.com> Reviewed-on: https://review.coreboot.org/15992 Tested-by: build bot (Jenkins) Reviewed-by: Aaron Durbin <adurbin(a)chromium.org> See https://review.coreboot.org/15992 for details. -gerrit

1 0

Patch merged into coreboot/master: soc/intel/common: Fix build error in reset.c
by gerrit＠coreboot.org 01 Aug '16

01 Aug '16

the following patch was just integrated into master: commit aac31ca22149f7e3a6f90d6c229f3fc87fb53592 Author: Lee Leahy <leroy.p.leahy(a)intel.com> Date: Sat Jul 30 10:29:37 2016 -0700 soc/intel/common: Fix build error in reset.c Fix build error caused by macro substitution in the function definition when the Kconfig value HAVE_HARD_RESET is not selected. src/soc/intel/common/reset.c:36:21: error: macro "hard_reset" passed 1 arguments, but takes just 0 void hard_reset(void) ^ src/soc/intel/common/reset.c:37:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token { ^ make: *** [build/bootblock/soc/intel/common/reset.o] Error 1 TEST=Build and run on Galileo Gen2 Change-Id: I793570e62a0e46cca86cc540c243e363896ceac7 Signed-off-by: Lee Leahy <leroy.p.leahy(a)intel.com> Reviewed-on: https://review.coreboot.org/15988 Tested-by: build bot (Jenkins) Reviewed-by: Aaron Durbin <adurbin(a)chromium.org> See https://review.coreboot.org/15988 for details. -gerrit

1 0

New patch to review for coreboot: cbfstool: Check arguments to strtoul() where appropriate
by Nico Huber 01 Aug '16

01 Aug '16

Nico Huber (nico.h(a)gmx.de) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16012 -gerrit commit 24f5ae8f28e42ffc06357ac611d9f88679690d12 Author: Nico Huber <nico.h(a)gmx.de> Date: Mon Aug 1 21:37:42 2016 +0200 cbfstool: Check arguments to strtoul() where appropriate The interface to strtoul() is a weird mess. It may or may not set errno if no conversion is done. So check for empty strings and trailing characters. Change-Id: I82373d2a0102fc89144bd12376b5ea3b10c70153 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- util/cbfstool/cbfstool.c | 82 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 70 insertions(+), 12 deletions(-) diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index 7b30ce9..f448acf 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -1350,24 +1350,51 @@ int main(int argc, char **argv) param.source_region = optarg; break; case 'b': - param.baseaddress = strtoul(optarg, NULL, 0); + param.baseaddress = strtoul(optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid base address '%s'.\n", + optarg); + return 1; + } // baseaddress may be zero on non-x86, so we // need an explicit "baseaddress_assigned". param.baseaddress_assigned = 1; break; case 'l': - param.loadaddress = strtoul(optarg, NULL, 0); + param.loadaddress = strtoul(optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid load address '%s'.\n", + optarg); + return 1; + } break; case 'e': - param.entrypoint = strtoul(optarg, NULL, 0); + param.entrypoint = strtoul(optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid entry point '%s'.\n", + optarg); + return 1; + } break; case 's': param.size = strtoul(optarg, &suffix, 0); - if (tolower((int)suffix[0])=='k') { - param.size *= 1024; + if (!*optarg) { + ERROR("Empty size specified.\n"); + return 1; } - if (tolower((int)suffix[0])=='m') { + switch (tolower((int)suffix[0])) { + case 'k': + param.size *= 1024; + break; + case 'm': param.size *= 1024 * 1024; + break; + case '0': + break; + default: + ERROR("Invalid suffix for size '%s'.\n", + optarg); + return 1; } break; case 'B': @@ -1375,24 +1402,49 @@ int main(int argc, char **argv) break; case 'H': param.headeroffset = strtoul( - optarg, NULL, 0); + optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid header offset '%s'.\n", + optarg); + return 1; + } param.headeroffset_assigned = 1; break; case 'a': - param.alignment = strtoul(optarg, NULL, 0); + param.alignment = strtoul(optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid alignment '%s'.\n", + optarg); + return 1; + } break; case 'P': - param.pagesize = strtoul(optarg, NULL, 0); + param.pagesize = strtoul(optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid page size '%s'.\n", + optarg); + return 1; + } break; case 'o': - param.cbfsoffset = strtoul(optarg, NULL, 0); + param.cbfsoffset = strtoul(optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid cbfs offset '%s'.\n", + optarg); + return 1; + } param.cbfsoffset_assigned = 1; break; case 'f': param.filename = optarg; break; case 'i': - param.u64val = strtoull(optarg, NULL, 0); + param.u64val = strtoull(optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid int parameter '%s'.\n", + optarg); + return 1; + } break; case 'u': param.fill_partial_upward = true; @@ -1404,7 +1456,13 @@ int main(int argc, char **argv) param.show_immutable = true; break; case 'x': - param.fit_empty_entries = strtol(optarg, NULL, 0); + param.fit_empty_entries = strtol( + optarg, &suffix, 0); + if (!*optarg || (suffix && *suffix)) { + ERROR("Invalid number of fit entries " + "'%s'.\n", optarg); + return 1; + } break; case 'v': verbose++;

1 0

Patch set updated for coreboot: util/lint: Add a lint tool to find non-ascii & unprintable chars
by Martin Roth 01 Aug '16

01 Aug '16

Martin Roth (martinroth(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/15979 -gerrit commit 16b6bd19f2704dd876731b3860bbccd588c88af9 Author: Martin Roth <martinroth(a)google.com> Date: Fri Jul 29 14:20:55 2016 -0600 util/lint: Add a lint tool to find non-ascii & unprintable chars Change-Id: I40f7e61fd403cbad19cf0746e2017c53e7379bf8 Signed-off-by: Martin Roth <martinroth(a)google.com> --- util/lint/lint-016-non-ascii | 45 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/util/lint/lint-016-non-ascii b/util/lint/lint-016-non-ascii new file mode 100755 index 0000000..1f64242 --- /dev/null +++ b/util/lint/lint-016-non-ascii @@ -0,0 +1,45 @@ +#!/bin/sh +# This file is part of the coreboot project. +# +# Copyright (C) 2016 Google Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# DESCR: Check for non-ASCII and unprintable characters + +LC_ALL=C export LC_ALL + +INCLUDED_FILES='\.[chsS]$\|\.asl$\|\.cb$\|\.inc$\|Kconfig\|\.ld$|\.txt\|\.hex' +EXCLUDED_DIRS='^payloads/\|^src/vendorcode/\|^Documentation/\|^build/\|^3rdparty/\|^\.git/\|^coreboot-builds/\|^util/nvidia/cbootimage' +EXCLUDED_FILES='to-wiki/towiki\.sh$\|vga/vga_font\|video/font\|PDCurses.*x11' +EXCLUDED_PHRASES='Intel®\|°C\|°F\|Athlon™\|Copyright.*©\|A-Za-zÀ-ÿ' +EXCLUDED_NAMES='Gröger\|Züpke\|Kyösti\|Mälkki\|François\|Michał\|Mirosław\|Măgherușan\|Neuschäfer' + +# Use git ls-files if the code is in a git repo, otherwise use find. +if [ -n "$(command -v git)" ] && [ -d .git ]; then + FIND_FILES="git ls-files" +else + FIND_FILES="find . " +fi + +# 1. Get the list of files to parse and send them through grep +# 2. Find any characters that aren't TAB, or space (0x20) to ~ (0x7F) +# LF (0x10) isn't included, as it ends the grep line +# 3. Remove common phrases and names that have been found +# 4. Run the result through grep again to highlight the issues that were +# found. Without this step, the characters can be difficult to see. +grep -n "[^ -~]" \ + $(${FIND_FILES} | sed 's|^\./||' | sort | \ + grep "$INCLUDED_FILES" | \ + grep -v "$EXCLUDED_DIRS" | \ + grep -v "$EXCLUDED_FILES") | \ + grep -v "$EXCLUDED_PHRASES" | \ + grep -v "$EXCLUDED_NAMES" | \ + grep --color='auto' "[^ -~]"

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit August 2016