coreboot-gerrit December 2016

coreboot-gerrit@coreboot.org

1 participants
1140 discussions

Patch set updated for coreboot: riscv: Add support for timer interrupts
by Ronald G. Minnich 14 Dec '16

14 Dec '16

Ronald G. Minnich (rminnich(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17807 -gerrit commit 7b990a9c2acc9a069d181c6834da41ae109564a7 Author: Ronald G. Minnich <rminnich(a)gmail.com> Date: Mon Dec 12 15:09:42 2016 -0800 riscv: Add support for timer interrupts RISCV requires that timer interrupts be handled in machine mode and delegated as necessary. Also you can only reset the timer interrupt by writing to mtimecmp. Further, you must write a number > mtime, not just != mtime. This rather clumsy situation requires that we write some value into the future into mtimecmp lest we never be able to leave machine mode as the interrupt either is not cleared or instantly reoccurs. This current code is tested and works for harvey (Plan 9) timer interrupts. Change-Id: I8538d5fd8d80d9347773c638f5cbf0da18dc1cae Signed-off-by: Ronald G. Minnich <rminnich(a)gmail.com> --- src/arch/riscv/include/arch/encoding.h | 5 +++ src/arch/riscv/trap_handler.c | 77 +++++++++++++++++++++++++++++++++- src/arch/riscv/virtual_memory.c | 13 +++++- 3 files changed, 91 insertions(+), 4 deletions(-) diff --git a/src/arch/riscv/include/arch/encoding.h b/src/arch/riscv/include/arch/encoding.h index 9f9d8ca..ece5949 100644 --- a/src/arch/riscv/include/arch/encoding.h +++ b/src/arch/riscv/include/arch/encoding.h @@ -36,6 +36,10 @@ #define MSTATUS_SPIE 0x00000020 #define MSTATUS_HPIE 0x00000040 #define MSTATUS_MPIE 0x00000080 +#define MSTATUS_UTIE 0x00000010 +#define MSTATUS_STIE 0x00000020 +#define MSTATUS_HTIE 0x00000040 +#define MSTATUS_MTIE 0x00000080 #define MSTATUS_SPP 0x00000100 #define MSTATUS_HPP 0x00000600 #define MSTATUS_MPP 0x00001800 @@ -126,6 +130,7 @@ #define SIP_SSIP MIP_SSIP #define SIP_STIP MIP_STIP +#define SIE_STIE MSTATUS_STIE #define PRV_U 0 #define PRV_S 1 diff --git a/src/arch/riscv/trap_handler.c b/src/arch/riscv/trap_handler.c index c7a11c6..79159ee 100644 --- a/src/arch/riscv/trap_handler.c +++ b/src/arch/riscv/trap_handler.c @@ -20,6 +20,10 @@ #include <mcall.h> #include <string.h> #include <vm.h> +#include <commonlib/configstring.h> + +static uint64_t *time; +static uint64_t *timecmp; void handle_supervisor_call(trapframe *tf) { uintptr_t call = tf->gpr[17]; /* a7 */ @@ -70,7 +74,7 @@ void handle_supervisor_call(trapframe *tf) { returnValue = mcall_query_memory(arg0, (memory_block_info*) arg1); break; default: - printk(BIOS_DEBUG, "ERROR! Unrecognized system call\n"); + printk(BIOS_DEBUG, "ERROR! Unrecognized SBI call\n"); returnValue = 0; break; // note: system call we do not know how to handle } @@ -130,8 +134,74 @@ static void print_trap_information(const trapframe *tf) printk(BIOS_DEBUG, "Stored sp: %p\n", (void*) tf->gpr[2]); } -void trap_handler(trapframe *tf) { +static void gettimer(void) +{ + query_result res; + const char *config; + + config = configstring(); + query_rtc(config, (uintptr_t *)&time); + if (!time) + die("Got timer interrupt but found no timer."); + res = query_config_string(config, "core{0{0{timecmp"); + timecmp = (void *)get_uint(res); + if (!timecmp) + die("Got a timer interrupt but found no timecmp."); +} + +static void interrupt_handler(trapframe *tf) +{ + uint64_t cause = tf->cause & ~0x8000000000000000ULL; + uint32_t ssip, ssie; + + switch (cause) { + case IRQ_M_TIMER: + // The only way to reset the timer interrupt is to + // write mtimecmp. But we also have to ensure the + // comparison fails, for a long time, to let + // supervisor interrupt handler compute a new value + // and set it. Finally, it fires if mtimecmp is <= + // mtime, not =, so setting mtimecmp to 0 won't work + // to clear the interrupt and disable a new one. We + // have to set the mtimecmp far into the future. + // Akward! + // + // Further, maybe the platform doesn't have the + // hardware or the payload never uses it. We hold off + // querying some things until we are sure we need + // them. What to do if we can not find them? There are + // no good options. + + // This hart may have disabled timer interrupts. + // If so, just return. Kernels should only enable + // timer interrupts on one hart. + ssie = read_csr(sie); + if (!(ssie & SIE_STIE)) + break; + + if (!timecmp) + gettimer(); + *timecmp = (uint64_t) -1; + ssip = read_csr(sip); + ssip |= SIP_STIP; + write_csr(sip, ssip); + break; + default: + printk(BIOS_EMERG, "======================================\n"); + printk(BIOS_EMERG, "Coreboot: Unknown machine interrupt: 0x%llx\n", + cause); + printk(BIOS_EMERG, "======================================\n"); + print_trap_information(tf); + break; + } +} +void trap_handler(trapframe *tf) +{ write_csr(mscratch, tf); + if (tf->cause & 0x8000000000000000ULL) { + interrupt_handler(tf); + return; + } switch(tf->cause) { case CAUSE_MISALIGNED_FETCH: @@ -159,6 +229,9 @@ void trap_handler(trapframe *tf) { handle_supervisor_call(tf); break; default: + printk(BIOS_EMERG, "================================\n"); + printk(BIOS_EMERG, "Coreboot: can not handle a trap:\n"); + printk(BIOS_EMERG, "================================\n"); print_trap_information(tf); break; } diff --git a/src/arch/riscv/virtual_memory.c b/src/arch/riscv/virtual_memory.c index 26a0169..aceb72e 100644 --- a/src/arch/riscv/virtual_memory.c +++ b/src/arch/riscv/virtual_memory.c @@ -292,12 +292,21 @@ void initVirtualMemory(void) { void mstatus_init(void) { uintptr_t ms = 0; + ms = INSERT_FIELD(ms, MSTATUS_FS, 3); ms = INSERT_FIELD(ms, MSTATUS_XS, 3); write_csr(mstatus, ms); - clear_csr(mip, MIP_MSIP); - set_csr(mie, MIP_MSIP); + // clear any pending timer interrupts. + clear_csr(mip, MIP_STIP | MIP_SSIP); + + // enable machine and supervisor timer and + // all other supervisor interrupts. + set_csr(mie, MIP_MTIP | MIP_STIP | MIP_SSIP); + + // Delegate supervisor timer and other interrupts + // to supervisor mode. + set_csr(mideleg, MIP_STIP | MIP_SSIP); set_csr(medeleg, delegate);

1 0

New patch to review for coreboot: util/cbfstool: more careful handling of error conditions
by Patrick Georgi 14 Dec '16

14 Dec '16

Patrick Georgi (pgeorgi(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17861 -gerrit commit e469488591cb4cca910aee59ea394c3d6387c301 Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Wed Dec 14 16:16:47 2016 +0100 util/cbfstool: more careful handling of error conditions Change-Id: I72a7776d530d1cf0b8fa39e558990df3dc7f7805 Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1295494 --- util/cbfstool/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/cbfstool/common.c b/util/cbfstool/common.c index 3093ba1..3175f1b 100644 --- a/util/cbfstool/common.c +++ b/util/cbfstool/common.c @@ -72,7 +72,7 @@ int buffer_from_file(struct buffer *buffer, const char *filename) } buffer->offset = 0; buffer->size = get_file_size(fp); - if (buffer->size == -1u) { + if (buffer->size < 0) { fprintf(stderr, "could not determine size of %s\n", filename); fclose(fp); return -1;

1 0

New patch to review for coreboot: util/cbfstool: check that buffer_create worked
by Patrick Georgi 14 Dec '16

14 Dec '16

Patrick Georgi (pgeorgi(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17860 -gerrit commit bbca52e975ae6347aa44d125b29df80a4ac6d70f Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Wed Dec 14 16:11:58 2016 +0100 util/cbfstool: check that buffer_create worked We might not care much about this buffer, but we really use it later on... Change-Id: Ia16270f836d05d8b454e77de7b5babeb6bb05d6d Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1294797 --- util/cbfstool/cbfstool.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index 7a71427..d3c15e0 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -776,7 +776,8 @@ static int cbfs_create(void) struct buffer bootblock; if (!param.bootblock) { DEBUG("-B not given, creating image without bootblock.\n"); - buffer_create(&bootblock, 0, "(dummy)"); + if (buffer_create(&bootblock, 0, "(dummy)") != 0) + return 1; } else if (buffer_from_file(&bootblock, param.bootblock)) { return 1; }

1 0

New patch to review for coreboot: util/cbfstool: Fix memory leak
by Patrick Georgi 14 Dec '16

14 Dec '16

Patrick Georgi (pgeorgi(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17859 -gerrit commit b8c9eced24603f1994037e2d25ab62b2c59397a6 Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Wed Dec 14 16:10:00 2016 +0100 util/cbfstool: Fix memory leak Change-Id: I66cb1c88155ef58610bacfb899e0132e4143c7ac Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1325836 --- util/cbfstool/cbfs_image.c | 1 + 1 file changed, 1 insertion(+) diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index ee229b3..c34ca83 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -1411,6 +1411,7 @@ int cbfs_print_entry_info(struct cbfs_image *image, struct cbfs_file *entry, ntohl(entry->len), hash_type, local_hash, hash_len) != VB2_SUCCESS) { fprintf(fp, "failed to hash '%s'\n", name); + free(hash_str); break; } int valid = memcmp(local_hash, hash->hash_data, hash_len) == 0;

1 0

New patch to review for coreboot: util/cbfstool: Add NULL-ptr check
by Patrick Georgi 14 Dec '16

14 Dec '16

Patrick Georgi (pgeorgi(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17858 -gerrit commit 1f7f70137057864e7ec1e2144c6299c062877024 Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Wed Dec 14 16:08:52 2016 +0100 util/cbfstool: Add NULL-ptr check Change-Id: I8b5caf5423135fe683a24db6700b895a2685cb98 Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1323507 --- util/cbfstool/cbfs_image.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index e1a0e8c..ee229b3 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -175,7 +175,8 @@ static int cbfs_file_get_compression_info(struct cbfs_file *entry, uint32_t *decompressed_size) { unsigned int compression = CBFS_COMPRESS_NONE; - *decompressed_size = ntohl(entry->len); + if (decompressed_size) + *decompressed_size = ntohl(entry->len); for (struct cbfs_file_attribute *attr = cbfs_file_first_attr(entry); attr != NULL; attr = cbfs_file_next_attr(entry, attr)) {

1 0

Patch set updated for coreboot: libpayload/drivers/video: Improve check in if condition
by Patrick Georgi 14 Dec '16

14 Dec '16

Patrick Georgi (pgeorgi(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17837 -gerrit commit f81495f98c5b78a1a1a6327b4e469dacbc9237fb Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Tue Dec 13 15:55:26 2016 +0100 libpayload/drivers/video: Improve check in if condition Coverity considers this a copy&paste error, and maybe it is. In any case, it makes sense to check the variable that (if the condition is true) is changed, and the values are the same before that test, so the change is harmless. Change-Id: I163c6a9f5baa05e715861dc19643b19a9c79c883 Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1347376 --- payloads/libpayload/drivers/video/graphics.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/payloads/libpayload/drivers/video/graphics.c b/payloads/libpayload/drivers/video/graphics.c index 943f8ed..f1d916a 100644 --- a/payloads/libpayload/drivers/video/graphics.c +++ b/payloads/libpayload/drivers/video/graphics.c @@ -322,7 +322,7 @@ static int draw_bitmap_v3(const struct vector *top_left, for (d.y = 0; d.y < dim->height; d.y++, p.y += dir) { s0.y = d.y * scale->y.d / scale->y.n; s1.y = s0.y; - if (s0.y + 1 < dim_org->height) + if (s1.y + 1 < dim_org->height) s1.y++; ty.d = scale->y.n; ty.n = (d.y * scale->y.d) % scale->y.n;

1 0

Patch set updated for coreboot: libpayload/.../PDCurses: Improve compatibility with ncurses
by Patrick Georgi 14 Dec '16

14 Dec '16

Patrick Georgi (pgeorgi(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17836 -gerrit commit a1367022a8f21800d76914910e84f6052c3d4103 Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Tue Dec 13 15:50:23 2016 +0100 libpayload/.../PDCurses: Improve compatibility with ncurses Coverity erroneously complains that we call wmove with x or y == -1, even though our copy of that function properly checks for that. But: setsyx is documented to always return OK (even on errors), so let it do that. (and make coverity happy in the process) Change-Id: I1bc9ba2a075037f0e1a855b67a93883978564887 Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1260797 --- payloads/libpayload/curses/PDCurses/pdcurses/getyx.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/payloads/libpayload/curses/PDCurses/pdcurses/getyx.c b/payloads/libpayload/curses/PDCurses/pdcurses/getyx.c index 1c03917..0f39c48 100644 --- a/payloads/libpayload/curses/PDCurses/pdcurses/getyx.c +++ b/payloads/libpayload/curses/PDCurses/pdcurses/getyx.c @@ -135,9 +135,14 @@ int setsyx(int y, int x) curscr->_leaveit = TRUE; return OK; } + else if (y == -1 || x == -1) + { + return OK; + } else { curscr->_leaveit = FALSE; - return wmove(curscr, y, x); + wmove(curscr, y, x); + return OK; } }

1 0

Patch set updated for coreboot: vendorcode/amd: drop dead code
by Patrick Georgi 14 Dec '16

14 Dec '16

Patrick Georgi (pgeorgi(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17833 -gerrit commit b03bf0194dafacf90f0bb908c700c9f3e9bcbb2c Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Tue Dec 13 15:40:26 2016 +0100 vendorcode/amd: drop dead code Change-Id: Ie67e1f7887e8df497d7dfd956badd9e06fd5d8a3 Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1254651 --- src/vendorcode/amd/pi/00630F01/binaryPI/AGESA.c | 5 ----- src/vendorcode/amd/pi/00660F01/binaryPI/AGESA.c | 5 ----- src/vendorcode/amd/pi/00670F00/binaryPI/AGESA.c | 5 ----- src/vendorcode/amd/pi/00730F01/binaryPI/AGESA.c | 5 ----- 4 files changed, 20 deletions(-) diff --git a/src/vendorcode/amd/pi/00630F01/binaryPI/AGESA.c b/src/vendorcode/amd/pi/00630F01/binaryPI/AGESA.c index c207d67..ce1b5d9 100644 --- a/src/vendorcode/amd/pi/00630F01/binaryPI/AGESA.c +++ b/src/vendorcode/amd/pi/00630F01/binaryPI/AGESA.c @@ -368,13 +368,8 @@ AmdIdsRunApTaskLate ( IN AP_EXE_PARAMS *AmdApExeParams ) { - MODULE_ENTRY Dispatcher = NULL; - const AMD_MODULE_HEADER* module = agesawrapper_locate_module(ModuleIdentifier); AmdApExeParams->StdHeader.Func = -1; return AGESA_UNSUPPORTED; - if (!module) return AGESA_UNSUPPORTED; - Dispatcher = module->ModuleDispatcher; - return Dispatcher(AmdApExeParams); } /********************************************************************** diff --git a/src/vendorcode/amd/pi/00660F01/binaryPI/AGESA.c b/src/vendorcode/amd/pi/00660F01/binaryPI/AGESA.c index 8d2c8e6..66a8393 100644 --- a/src/vendorcode/amd/pi/00660F01/binaryPI/AGESA.c +++ b/src/vendorcode/amd/pi/00660F01/binaryPI/AGESA.c @@ -365,13 +365,8 @@ AmdIdsRunApTaskLate ( IN AP_EXE_PARAMS *AmdApExeParams ) { - MODULE_ENTRY Dispatcher = NULL; - const AMD_MODULE_HEADER* module = agesawrapper_locate_module(ModuleIdentifier); AmdApExeParams->StdHeader.Func = -1; return AGESA_UNSUPPORTED; - if (!module) return AGESA_UNSUPPORTED; - Dispatcher = module->ModuleDispatcher; - return Dispatcher(AmdApExeParams); } /********************************************************************** diff --git a/src/vendorcode/amd/pi/00670F00/binaryPI/AGESA.c b/src/vendorcode/amd/pi/00670F00/binaryPI/AGESA.c index 8d2c8e6..66a8393 100644 --- a/src/vendorcode/amd/pi/00670F00/binaryPI/AGESA.c +++ b/src/vendorcode/amd/pi/00670F00/binaryPI/AGESA.c @@ -365,13 +365,8 @@ AmdIdsRunApTaskLate ( IN AP_EXE_PARAMS *AmdApExeParams ) { - MODULE_ENTRY Dispatcher = NULL; - const AMD_MODULE_HEADER* module = agesawrapper_locate_module(ModuleIdentifier); AmdApExeParams->StdHeader.Func = -1; return AGESA_UNSUPPORTED; - if (!module) return AGESA_UNSUPPORTED; - Dispatcher = module->ModuleDispatcher; - return Dispatcher(AmdApExeParams); } /********************************************************************** diff --git a/src/vendorcode/amd/pi/00730F01/binaryPI/AGESA.c b/src/vendorcode/amd/pi/00730F01/binaryPI/AGESA.c index 82744dd..25364d8 100644 --- a/src/vendorcode/amd/pi/00730F01/binaryPI/AGESA.c +++ b/src/vendorcode/amd/pi/00730F01/binaryPI/AGESA.c @@ -366,13 +366,8 @@ AmdIdsRunApTaskLate ( IN AP_EXE_PARAMS *AmdApExeParams ) { - MODULE_ENTRY Dispatcher = NULL; - const AMD_MODULE_HEADER* module = agesawrapper_locate_module(ModuleIdentifier); AmdApExeParams->StdHeader.Func = -1; return AGESA_UNSUPPORTED; - if (!module) return AGESA_UNSUPPORTED; - Dispatcher = module->ModuleDispatcher; - return Dispatcher(AmdApExeParams); } /**********************************************************************

1 0

Patch merged into coreboot/master: libpayload/.../PDCurses: avoid reading orig before NULL checking it
by gerrit＠coreboot.org 14 Dec '16

14 Dec '16

the following patch was just integrated into master: commit 83f75bfeb93194e1a41a7c69431f5249cc04b4e8 Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Tue Dec 13 15:47:22 2016 +0100 libpayload/.../PDCurses: avoid reading orig before NULL checking it Coverity complains and that (unfortunately) means that some compiler might take advantage of the same fact. Change-Id: I59aff77820c524fa5a0fcb251c1268da475101fb Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1261105 Reviewed-on: https://review.coreboot.org/17835 Tested-by: build bot (Jenkins) Reviewed-by: Martin Roth <martinroth(a)google.com> See https://review.coreboot.org/17835 for details. -gerrit

1 0

Patch merged into coreboot/master: vendorcode/amd: Fix non-terminating loop
by gerrit＠coreboot.org 14 Dec '16

14 Dec '16

the following patch was just integrated into master: commit cf3b306cafdb76493ff605c5dd05bbd602293147 Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Tue Dec 13 15:42:58 2016 +0100 vendorcode/amd: Fix non-terminating loop Code is copied from agesa/common's amdlib.c. Things can probably be deduplicated. Change-Id: I9c8adab5db7e9fd41aecc522136dfa705c1e2ee6 Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Found-by: Coverity Scan #1229662 Reviewed-on: https://review.coreboot.org/17834 Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net> Reviewed-by: Martin Roth <martinroth(a)google.com> Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com> Tested-by: build bot (Jenkins) See https://review.coreboot.org/17834 for details. -gerrit

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2016