coreboot-gerrit May 2017

coreboot-gerrit@coreboot.org

1 participants
3888 discussions

Change in coreboot[master]: console/flashsconsole: Add spi flash console for debugging
by Youness Alaoui (Code Review) 24 May '17

24 May '17

Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/19849 ) Change subject: console/flashsconsole: Add spi flash console for debugging ...................................................................... Patch Set 4: (7 comments) https://review.coreboot.org/#/c/19849/2/src/console/Kconfig File src/console/Kconfig: Line 258: file in the CBFS. > As noted on irc by Nico cbfstool can extract an fmap region from the image Yep, very happy about that. Now the only annoyance is the custom fmd. Do we want to delete the tested/working cbfs code ? or keep it to give the choice to the user? Now that I know how to use the fmap, either option is fine with me, so it really depends on what you want to see upstreamed. https://review.coreboot.org/#/c/19849/4/src/console/Kconfig File src/console/Kconfig: Line 239: select BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY > select BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY if !COMMON_CBFS_SPI_WRAPPER Done https://review.coreboot.org/#/c/19849/2/src/drivers/spi/flashconsole.c File src/drivers/spi/flashconsole.c: Line 55: cbfs_file_data(&cbfs_region, &file); > prepare_program_locate() is really only for ensuring vboot hooks in. I don' I see, the only one to implement prepare is the vboot locator, and the boot_device_init() is also called in the boot_region_properties, so the prepare is not needed. thx. Good point also about removing boot_device_init. will do. Line 90: if (offset >= cbfs_size) { > You should have a struct region_device that you rdev_chain() from the root Yeah, I did exactly that in the commit that adds FMAP support: https://review.coreboot.org/#/c/19866/2 I removed the size var and use the region_device_sz() API instead as well. https://review.coreboot.org/#/c/19849/4/src/drivers/spi/flashconsole.c File src/drivers/spi/flashconsole.c: PS4, Line 46: car_set_var(g_rdev, NULL); : car_set_var(g_offset, 0); : car_set_var(g_line_offset, 0); > bss and CAR_GLOBAL are by definition set to 0. No need to do this. It doesn't matter. It's the init function, it needs to set them to sensible values, regardless of what they were set to before. init does get called again at the start of each stage, is the value in car kept as it was or reset to 0? Better to avoid the confusion. Line 57: cbfs_size = region_device_sz(&cbfs_region); > So at this point you should just initialize your global struct region_devic Yeah, that's what I did, once I squash my commits, it will be easier to review. PS4, Line 98: memset(line_buffer, 0, LINE_BUFFER_SIZE); > unnecessary for same reasons above. The buffer is used locally to find the offset to start from. Since adding the g_line_offset variable, it's not needed and I've removed it in the most recent commit. A squash is indeed necessary.. -- To view, visit https://review.coreboot.org/19849 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74a297b94f6881d8c27cbe5168f161d8331c3df3 Gerrit-PatchSet: 4 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in coreboot[master]: console/flashconsole: Add support for FMAP based flash console
by build bot (Jenkins) (Code Review) 24 May '17

24 May '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19866 ) Change subject: console/flashconsole: Add support for FMAP based flash console ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-checkpatch/9987/ : SUCCESS https://qa.coreboot.org/job/coreboot-gerrit/54258/ : SUCCESS -- To view, visit https://review.coreboot.org/19866 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7905480feebd906dd100228ca22c7f506d5a43b8 Gerrit-PatchSet: 2 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Change in coreboot[master]: console/flashsconsole: Add spi flash console for debugging
by build bot (Jenkins) (Code Review) 24 May '17

24 May '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19849 ) Change subject: console/flashsconsole: Add spi flash console for debugging ...................................................................... Patch Set 4: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-checkpatch/9986/ : SUCCESS https://qa.coreboot.org/job/coreboot-gerrit/54257/ : SUCCESS -- To view, visit https://review.coreboot.org/19849 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I74a297b94f6881d8c27cbe5168f161d8331c3df3 Gerrit-PatchSet: 4 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Change in coreboot[master]: console/flashconsole: Add support for tx_flush
by build bot (Jenkins) (Code Review) 24 May '17

24 May '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19865 ) Change subject: console/flashconsole: Add support for tx_flush ...................................................................... Patch Set 2: Build Successful https://qa.coreboot.org/job/coreboot-checkpatch/9988/ : SUCCESS https://qa.coreboot.org/job/coreboot-gerrit/54256/ : SUCCESS -- To view, visit https://review.coreboot.org/19865 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6516ecefddf676bdac6fb97536a08a5c5ecc0d4d Gerrit-PatchSet: 2 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Change in coreboot[master]: mainboard/google/eve: Update VR config settings
by Duncan Laurie (Code Review) 24 May '17

24 May '17

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/19826 ) Change subject: mainboard/google/eve: Update VR config settings ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/#/c/19826/2/src/mainboard/google/eve/devicetree… File src/mainboard/google/eve/devicetree.cb: PS2, Line 91: 4.1 FWIW making this 4.1 doesn't really do much as it just gets *4 and then assigned to an integer variable. -- To view, visit https://review.coreboot.org/19826 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I274245821f68fb3151e5563ea0c75eaa1ad32c08 Gerrit-PatchSet: 2 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in coreboot[master]: console/flashconsole: Add support for FMAP based flash console
by build bot (Jenkins) (Code Review) 24 May '17

24 May '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19866 ) Change subject: console/flashconsole: Add support for FMAP based flash console ...................................................................... Patch Set 1: Verified-1 Build Failed https://qa.coreboot.org/job/coreboot-gerrit/54255/ : ABORTED https://qa.coreboot.org/job/coreboot-checkpatch/9985/ : SUCCESS -- To view, visit https://review.coreboot.org/19866 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7905480feebd906dd100228ca22c7f506d5a43b8 Gerrit-PatchSet: 1 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Change in coreboot[master]: console/flashsconsole: Add spi flash console for debugging
by Youness Alaoui (Code Review) 24 May '17

24 May '17

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/19849 to look at the new patch set (#4). Change subject: console/flashsconsole: Add spi flash console for debugging ...................................................................... console/flashsconsole: Add spi flash console for debugging If CONSOLE_SPI_FLASH config is enabled, we can write the cbmem messages to the 'console' region in CBFS which allows us to grab the log when we read the flash. This is useful when you don't have usb debugging, and UART lines are hard to find. Since a failure to boot would require a hardware flasher anyways, we can get the log at the same time. This feature should only be used when no alternative is found and only when we can't boot the system, because excessive writes to the flash is not recommended. This has been tested on purism/librem13 v2 and librem 15 v3 which run Intel Skylake hardware. It has not been tested on other archs or with a driver other than the fast_spi. The Kconfig help mentions support for writing to an FMAP area but this isn't supported yet. Change-Id: I74a297b94f6881d8c27cbe5168f161d8331c3df3 Signed-off-by: Youness Alaoui <youness.alaoui(a)puri.sm> --- M src/console/Kconfig M src/console/console.c M src/drivers/spi/Makefile.inc A src/drivers/spi/flashconsole.c A src/include/console/flash.h 5 files changed, 245 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/19849/4 -- To view, visit https://review.coreboot.org/19849 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I74a297b94f6881d8c27cbe5168f161d8331c3df3 Gerrit-PatchSet: 4 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in coreboot[master]: console/flashconsole: Add support for FMAP based flash console
by Youness Alaoui (Code Review) 24 May '17

24 May '17

Youness Alaoui has uploaded a new patch set (#2). ( https://review.coreboot.org/19866 ) Change subject: console/flashconsole: Add support for FMAP based flash console ...................................................................... console/flashconsole: Add support for FMAP based flash console Add support for writing the console log to the FMAP 'CONSOLE' area and keep the CBFS support as an alternative. Also remove the need for the offset variable and instead use a region_device subregion directly. Change-Id: I7905480feebd906dd100228ca22c7f506d5a43b8 Signed-off-by: Youness Alaoui <youness.alaoui(a)puri.sm> --- M src/drivers/spi/flashconsole.c 1 file changed, 44 insertions(+), 40 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/19866/2 -- To view, visit https://review.coreboot.org/19866 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7905480feebd906dd100228ca22c7f506d5a43b8 Gerrit-PatchSet: 2 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com>

1 0

Change in coreboot[master]: console/flashconsole: Add support for FMAP based flash console
by Youness Alaoui (Code Review) 24 May '17

24 May '17

Youness Alaoui has uploaded a new change for review. ( https://review.coreboot.org/19866 ) Change subject: console/flashconsole: Add support for FMAP based flash console ...................................................................... console/flashconsole: Add support for FMAP based flash console Add support for writing the console log to the FMAP 'CONSOLE' area and keep the CBFS support as an alternative. Also remove the need for the offset variable and instead use a region_device subregion directly. Change-Id: I7905480feebd906dd100228ca22c7f506d5a43b8 Signed-off-by: Youness Alaoui <youness.alaoui(a)puri.sm> --- M src/drivers/spi/flashconsole.c 1 file changed, 45 insertions(+), 40 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/19866/1 diff --git a/src/drivers/spi/flashconsole.c b/src/drivers/spi/flashconsole.c index b9b8877..a9d7fc3 100644 --- a/src/drivers/spi/flashconsole.c +++ b/src/drivers/spi/flashconsole.c @@ -17,6 +17,7 @@ #include <region.h> #include <boot_device.h> #include <cbfs.h> +#include <fmap.h> #include <console/flash.h> #include <string.h> #include <assert.h> @@ -24,40 +25,49 @@ #define LINE_BUFFER_SIZE 0x1000 -static const struct region_device *g_rdev CAR_GLOBAL; +static const struct region_device *g_rdev_ptr CAR_GLOBAL; +static struct region_device g_rdev CAR_GLOBAL; static uint8_t g_line_buffer[LINE_BUFFER_SIZE] CAR_GLOBAL; -static size_t g_region_offset CAR_GLOBAL; -static size_t g_region_size CAR_GLOBAL; static uint32_t g_offset CAR_GLOBAL; static uint32_t g_line_offset CAR_GLOBAL; void flashconsole_init(void) { - struct cbfsf file; - struct region_device cbfs_region; - const struct region_device *rdev; + struct region_device *rdev = car_get_var_ptr(&g_rdev); uint8_t *line_buffer = car_get_var_ptr(g_line_buffer); - uint32_t cbfs_offset; - uint32_t cbfs_size; + uint32_t size; uint32_t offset = 0; int i; int len = LINE_BUFFER_SIZE; - car_set_var(g_rdev, NULL); + car_set_var(g_rdev_ptr, NULL); car_set_var(g_offset, 0); car_set_var(g_line_offset, 0); - cbfs_prepare_program_locate(); - if (cbfs_boot_locate(&file, "console", NULL) == 0) { - printk(BIOS_INFO, "Can't find 'console' region in CBFS."); - return; - } - cbfs_file_data(&cbfs_region, &file); - cbfs_offset = region_device_offset(&cbfs_region); - cbfs_size = region_device_sz(&cbfs_region); + if (fmap_locate_area_as_rdev_rw("CONSOLE", rdev) == 0) { + size = region_device_sz(rdev); + } else { + struct cbfsf file; + struct region_device cbfs_region; + const struct region_device *rdev_rw; + size_t cbfs_offset; - boot_device_init(); - rdev = boot_device_rw(); + printk(BIOS_INFO, "Can't find 'CONSOLE' area in FMAP, trying CBFS\n"); + + cbfs_prepare_program_locate(); + if (cbfs_boot_locate(&file, "console", NULL)) { + printk(BIOS_INFO, "Can't find 'console' file in CBFS.\n"); + return; + } + cbfs_file_data(&cbfs_region, &file); + cbfs_offset = region_device_offset(&cbfs_region); + size = region_device_sz(&cbfs_region); + + boot_device_init(); + rdev_rw = boot_device_rw(); + if (rdev_chain(rdev, rdev_rw, cbfs_offset, size)) + return; + } /* * We need to check the region until we find a 0xff indicating @@ -69,12 +79,11 @@ * the sector is already erased, so we would need to read * anyways to check if it's all 0xff). */ - for (i = 0; i < len && offset < cbfs_size; ) { + for (i = 0; i < len && offset < size; ) { // Fill the buffer on first iteration if (i == 0) { - len = min(LINE_BUFFER_SIZE, cbfs_size - offset); - rdev_readat(rdev, line_buffer, cbfs_offset + offset, - len); + len = min(LINE_BUFFER_SIZE, size - offset); + rdev_readat(rdev, line_buffer, offset, len); } if (line_buffer[i] == 0xff) { offset += i; @@ -87,32 +96,28 @@ } } // Make sure there is still space left on the console - if (offset >= cbfs_size) { - printk(BIOS_INFO, "No space left on 'console' region in CBFS."); + if (offset >= size) { + printk(BIOS_INFO, "No space left on 'console' region in SPI flash."); return; } // Now we can enable tx_byte - car_set_var(g_region_offset, cbfs_offset); - car_set_var(g_region_size, cbfs_size); car_set_var(g_offset, offset); - memset(line_buffer, 0, LINE_BUFFER_SIZE); - // Set g_rdev last so tx_byte doesn't get executed early - car_set_var(g_rdev, rdev); + // Set g_rdev_ptr last so tx_byte doesn't get executed early + car_set_var(g_rdev_ptr, rdev); } static const struct region_device *_tx_flush(const struct region_device *rdev) { uint8_t *line_buffer = car_get_var_ptr(g_line_buffer); uint32_t offset = car_get_var(g_offset); - uint32_t region_offset = car_get_var(g_region_offset); - uint32_t region_size = car_get_var(g_region_size); + uint32_t region_size = region_device_sz(rdev); uint32_t len = car_get_var(g_line_offset); // We crop the rest of the line if the region is full if (offset + len >= region_size) len = region_size - offset; - rdev_writeat(rdev, line_buffer, region_offset + offset, len); + rdev_writeat(rdev, line_buffer, offset, len); // If the region is full, stop future write attempts if (offset + len >= region_size) @@ -126,18 +131,18 @@ void flashconsole_tx_byte(unsigned char c) { - const struct region_device *rdev = car_get_var(g_rdev); + const struct region_device *rdev = car_get_var(g_rdev_ptr); if (rdev) { uint8_t *line_buffer = car_get_var_ptr(g_line_buffer); uint32_t offset = car_get_var(g_offset); - uint32_t region_size = car_get_var(g_region_size); uint32_t len = car_get_var(g_line_offset); + uint32_t region_size = region_device_sz(rdev); /* Prevent any recursive loops in case the spi flash driver * calls printk (in case of transaction timeout or * any other error while writing) */ - car_set_var(g_rdev, NULL); + car_set_var(g_rdev_ptr, NULL); /* Do line-buffering on the writes to avoid doing an SPI write * on every byte */ @@ -148,21 +153,21 @@ offset + len >= region_size || c == '\n') { rdev = _tx_flush(rdev); } - car_set_var(g_rdev, rdev); + car_set_var(g_rdev_ptr, rdev); } } void flashconsole_tx_flush(void) { - const struct region_device *rdev = car_get_var(g_rdev); + const struct region_device *rdev = car_get_var(g_rdev_ptr); if (rdev) { /* Prevent any recursive loops in case the spi flash driver * calls printk (in case of transaction timeout or * any other error while writing) */ - car_set_var(g_rdev, NULL); + car_set_var(g_rdev_ptr, NULL); rdev = _tx_flush(rdev); - car_set_var(g_rdev, rdev); + car_set_var(g_rdev_ptr, rdev); } } -- To view, visit https://review.coreboot.org/19866 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7905480feebd906dd100228ca22c7f506d5a43b8 Gerrit-PatchSet: 1 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com>

1 0

Change in coreboot[master]: payloads/Kconfig: Add NO_DEFAULT_PAYLOAD
by build bot (Jenkins) (Code Review) 24 May '17

24 May '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19808 ) Change subject: payloads/Kconfig: Add NO_DEFAULT_PAYLOAD ...................................................................... Patch Set 4: Build Successful https://qa.coreboot.org/job/coreboot-checkpatch/9981/ : SUCCESS https://qa.coreboot.org/job/coreboot-gerrit/54251/ : SUCCESS -- To view, visit https://review.coreboot.org/19808 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2f72d4efc0a428dce377c3d003b2c00a6c8d4c08 Gerrit-PatchSet: 4 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2017