Re: [PATCH] habv4: add the possibility to changing the signing area from Kconfig
Hi Sascha, in my opinion it is better to have it configurable, because ther are different use cases and security requirements. i found the problem by creating a sd-card \emmc image with wic. The mbr, the partition table and bootloader became be signed at barebox build and wic changes the partition table at the end of the build process. Then the sd card image could not boot , because the signature was wrong. yeah secure boot works :-) the highest protection you have, when mbr and partition table is signed with the bootloader, but it is not always necessary. there was implemented skip-mbr, dcdofs and full, but full was by default in the code. at the moment i think , it is a good and easy choice. Best regards Maik Am 10.12.2019 um 16:21 schrieb Sascha Hauer: > On Tue, Dec 10, 2019 at 04:06:27PM +0100, Maik Otto wrote: >> the whole barebox with mbr and partition table will be signed by default >> add the possibility in the Kconfig to change from full signing to skip-mbr >> and from-dcdofs > Not signing the MBR seems the right thing to do. Do we need it > configurable at all or would it be better to just always skip the first > KiB? > > Sascha > ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 2/4] ARM: zynq: zedboard: allow lowlevel init to be called as second stage
If the code is already executing in DRAM, the PS7 init must not be executed, as it initializes the DRAM controller. As the OCM can be configured to an address which aliases with the DRAM address space we can't reliably infer if we are running from OCM or DRAM from the execution address. So instead of using the address, look at the OCM mapping, as the BootROM leaves a quite unique mapping behind with 192KB OCM mapped at the low address and 64KB mapped to the high address. Signed-off-by: Lucas Stach --- arch/arm/boards/avnet-zedboard/lowlevel.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm/boards/avnet-zedboard/lowlevel.c b/arch/arm/boards/avnet-zedboard/lowlevel.c index b50c36b28869..9b90ef112b46 100644 --- a/arch/arm/boards/avnet-zedboard/lowlevel.c +++ b/arch/arm/boards/avnet-zedboard/lowlevel.c @@ -31,6 +31,16 @@ extern char __dtb_zynq_zed_start[]; static void avnet_zedboard_ps7_init(void) { + /* +* Read OCM mapping configuration, if only the upper 64 KByte are +* mapped to the high address, it's very likely that we just got control +* from the BootROM. If the mapping is changed something other than the +* BootROM was running before us. Skip PS7 init to avoid cutting the +* branch we are sitting on in that case. +*/ + if ((readl(0xf8000910) & 0xf) != 0x8) + return; + /* open sesame */ writel(0xDF0D, ZYNQ_SLCR_UNLOCK); -- 2.23.0 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 1/4] ARM: zynq: zedboard: split out PS7 init
Move the PS7 inititalization into its own function. This helps readability and logically splits the FPGA toolchain generated setup from the reset of the board init. Also execute the PS7 setup after the lowlevel CPU init, as this is the regular order used in the Barebox codebase. Signed-off-by: Lucas Stach --- arch/arm/boards/avnet-zedboard/lowlevel.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/arch/arm/boards/avnet-zedboard/lowlevel.c b/arch/arm/boards/avnet-zedboard/lowlevel.c index 93e4da96ecd4..b50c36b28869 100644 --- a/arch/arm/boards/avnet-zedboard/lowlevel.c +++ b/arch/arm/boards/avnet-zedboard/lowlevel.c @@ -29,11 +29,8 @@ extern char __dtb_zynq_zed_start[]; -ENTRY_FUNCTION(start_avnet_zedboard, r0, r1, r2) +static void avnet_zedboard_ps7_init(void) { - - void *fdt = __dtb_zynq_zed_start + get_runtime_offset(); - /* open sesame */ writel(0xDF0D, ZYNQ_SLCR_UNLOCK); @@ -260,8 +257,16 @@ ENTRY_FUNCTION(start_avnet_zedboard, r0, r1, r2) /* lock up. secure, secure */ writel(0x767B, ZYNQ_SLCR_LOCK); +} + +ENTRY_FUNCTION(start_avnet_zedboard, r0, r1, r2) +{ + + void *fdt = __dtb_zynq_zed_start + get_runtime_offset(); arm_cpu_lowlevel_init(); + avnet_zedboard_ps7_init(); + barebox_arm_entry(0, SZ_512M, fdt); } -- 2.23.0 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 3/4] filetype: add Zynq image type
Use the 2 invariant words (width detection and image identification) from the Zynq image header to detect the filetype. Signed-off-by: Lucas Stach --- common/filetype.c | 4 include/filetype.h | 1 + 2 files changed, 5 insertions(+) diff --git a/common/filetype.c b/common/filetype.c index 39fea45edff5..fd6e8e3d1c35 100644 --- a/common/filetype.c +++ b/common/filetype.c @@ -80,6 +80,7 @@ static const struct filetype_str filetype_str[] = { [filetype_ubootvar] = { "U-Boot environmemnt variable data", "ubootvar" }, [filetype_stm32_image_v1] = { "STM32 image (v1)", "stm32-image-v1" }, + [filetype_zynq_image] = { "Zynq image", "zynq-image" }, }; const char *file_type_to_string(enum filetype f) @@ -392,6 +393,9 @@ enum filetype file_detect_type(const void *_buf, size_t bufsize) if (is_imx_flash_header_v2(_buf)) return filetype_imx_image_v2; + if (buf[8] == 0xAA995566 && buf[9] == 0x584C4E58) + return filetype_zynq_image; + return filetype_unknown; } diff --git a/include/filetype.h b/include/filetype.h index 90a03de58105..db95fdaa0a5f 100644 --- a/include/filetype.h +++ b/include/filetype.h @@ -49,6 +49,7 @@ enum filetype { filetype_layerscape_qspi_image, filetype_ubootvar, filetype_stm32_image_v1, + filetype_zynq_image, filetype_max, }; -- 2.23.0 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 4/4] ARM: zynq: add Zynq image bootm handler
This adds a bootm handler for the Zynq image, to allow second stage booting of a unchanged Zynq boot image. Signed-off-by: Lucas Stach --- arch/arm/mach-zynq/Makefile| 2 +- arch/arm/mach-zynq/bootm-zynqimg.c | 49 ++ 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 arch/arm/mach-zynq/bootm-zynqimg.c diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile index 3252247d176d..c88ab4666f0f 100644 --- a/arch/arm/mach-zynq/Makefile +++ b/arch/arm/mach-zynq/Makefile @@ -1 +1 @@ -obj-y += zynq.o +obj-y += zynq.o bootm-zynqimg.o diff --git a/arch/arm/mach-zynq/bootm-zynqimg.c b/arch/arm/mach-zynq/bootm-zynqimg.c new file mode 100644 index ..e903ab667905 --- /dev/null +++ b/arch/arm/mach-zynq/bootm-zynqimg.c @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include +#include +#include +#include + +static int do_bootm_zynqimage(struct image_data *data) +{ + resource_size_t start, end; + void (*barebox)(void); + u32 *header; + int ret; + + ret = memory_bank_first_find_space(, ); + if (ret) + return ret; + + ret = bootm_load_os(data, start); + if (ret) + return ret; + + header = (u32*)start; + barebox = (void*)start + header[12]; + + if (data->verbose) + printf("Loaded barebox image to 0x%08lx\n", + (unsigned long)barebox); + + shutdown_barebox(); + + barebox(); + + return -EIO; +} + +static struct image_handler zynq_image_handler = { + .name = "Zynq image", + .bootm = do_bootm_zynqimage, + .filetype = filetype_zynq_image, +}; + +static int zynq_register_image_handler(void) +{ + register_image_handler(_image_handler); + + return 0; +} +late_initcall(zynq_register_image_handler); -- 2.23.0 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH] of: demote most debug output to loglevel 8 (vdebug)
Loglevel 7 (debug) is often useful to get barebox running on new hardware; however, the heavy output of "OF:" messages makes most boards take several minutes to boot to a prompt, printing things which are probably interesting when debugging OF code itself, but not very helpful otherwise. Degrade most of the message to loglevel 8 (vdebug). Signed-off-by: Roland Hieber --- For illustration, here is the output before this patch with level 7 for a single OF node: of_platform_device_create: register device 44e0.target-mod...@0.of, io=0x44e0 register_device: 44e0.target-mod...@0.of create child: /ocp/interconnect@44c0/segment@20/target-module@3000 create child: /ocp/interconnect@44c0/segment@20/target-module@5000 create child: /ocp/interconnect@44c0/segment@20/target-module@7000 OF: ** translation for device /ocp/interconnect@44c0/segment@20/target-module@7000 ** OF: bus is default (na=1, ns=1) on /ocp/interconnect@44c0/segment@20 OF: parent bus is default (na=1, ns=1) on /ocp/interconnect@44c0 OF: walking ranges... OF: default map, cp=0, s=2000, da=7000 OF: default map, cp=2000, s=1000, da=7000 OF: default map, cp=3000, s=1000, da=7000 OF: default map, cp=4000, s=1000, da=7000 OF: default map, cp=5000, s=1000, da=7000 OF: default map, cp=6000, s=1000, da=7000 OF: default map, cp=7000, s=1000, da=7000 OF: with offset: 0 OF: parent bus is default (na=1, ns=1) on /ocp OF: walking ranges... OF: default map, cp=0, s=10, da=207000 OF: default map, cp=10, s=10, da=207000 OF: default map, cp=20, s=10, da=207000 OF: with offset: 7000 OF: parent bus is default (na=1, ns=1) on OF: empty ranges; 1:1 translation OF: with offset: 44e07000 OF: reached root node OF: ** translation for device /ocp/interconnect@44c0/segment@20/target-module@7000 ** OF: bus is default (na=1, ns=1) on /ocp/interconnect@44c0/segment@20 OF: parent bus is default (na=1, ns=1) on /ocp/interconnect@44c0 OF: walking ranges... OF: default map, cp=0, s=2000, da=7010 OF: default map, cp=2000, s=1000, da=7010 OF: default map, cp=3000, s=1000, da=7010 OF: default map, cp=4000, s=1000, da=7010 OF: default map, cp=5000, s=1000, da=7010 OF: default map, cp=6000, s=1000, da=7010 OF: default map, cp=7000, s=1000, da=7010 OF: with offset: 10 OF: parent bus is default (na=1, ns=1) on /ocp OF: walking ranges... OF: default map, cp=0, s=10, da=207010 OF: default map, cp=10, s=10, da=207010 OF: default map, cp=20, s=10, da=207010 OF: with offset: 7010 OF: parent bus is default (na=1, ns=1) on OF: empty ranges; 1:1 translation OF: with offset: 44e07010 OF: reached root node OF: ** translation for device /ocp/interconnect@44c0/segment@20/target-module@7000 ** OF: bus is default (na=1, ns=1) on /ocp/interconnect@44c0/segment@20 OF: parent bus is default (na=1, ns=1) on /ocp/interconnect@44c0 OF: walking ranges... OF: default map, cp=0, s=2000, da=7114 OF: default map, cp=2000, s=1000, da=7114 OF: default map, cp=3000, s=1000, da=7114 OF: default map, cp=4000, s=1000, da=7114 OF: default map, cp=5000, s=1000, da=7114 OF: default map, cp=6000, s=1000, da=7114 OF: default map, cp=7000, s=1000, da=7114 OF: with offset: 114 OF: parent bus is default (na=1, ns=1) on /ocp OF: walking ranges... OF: default map, cp=0, s=10, da=207114 OF: default map, cp=10, s=10, da=207114 OF: default map, cp=20, s=10, da=207114 OF: with offset: 7114 OF: parent bus is default (na=1, ns=1) on OF: empty ranges; 1:1 translation OF: with offset: 44e07114 OF: reached root node OF: ** translation for device /ocp/interconnect@44c0/segment@20/target-module@7000 ** OF: bus is default (na=1, ns=1) on /ocp/interconnect@44c0/segment@20 OF: parent bus is default (na=1, ns=1) on /ocp/interconnect@44c0 OF: walking ranges... OF: default map, cp=0, s=2000, da=7000 OF: default map, cp=2000, s=1000, da=7000 OF: default map, cp=3000, s=1000, da=7000 OF: default map, cp=4000, s=1000, da=7000 OF: default map, cp=5000, s=1000, da=7000 OF: default map, cp=6000, s=1000, da=7000 OF: default map, cp=7000, s=1000, da=7000 OF: with offset: 0 OF: parent bus is default (na=1, ns=1) on /ocp OF: walking ranges... OF: default map, cp=0, s=10, da=207000 OF: default map, cp=10, s=10, da=207000 OF: default map, cp=20, s=10, da=207000 OF: with offset: 7000 OF: parent bus is default (na=1, ns=1) on OF: empty ranges; 1:1 translation OF: with offset: 44e07000 OF: reached root node OF: ** translation for device
Re: [BUG] imx6qdl: degraded eMMC write performance
Am Di., 10. Dez. 2019 um 17:18 Uhr schrieb Sascha Hauer : [...] > > > We could adjust RW_BUF_SIZE (used by copy_file as buffer size) to a full > > > chunk size (16KiB). Does this give you back some lost performance? > > No, changing RW_BUF_SIZE did not help. > > And I also see why :-/ > > block_op_write() works around block sizes (512bytes), not around chunk > sizes. This means we always read before we write. This could probably be > optimized somehow, but this would only speed up the write case you > described in your initial mail. It seems what you are more interested in > is the read performance. Well, I'm interested in both. Of course the read-performance is important for booting. But the write-performance is important for production. It simply matters if you have to wait for another 200 seconds for a device to get programmed. Regards Hubert ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH] commands: digest: don't be to restrictive with the length of verification files
Otherwise it is not possible to verify against a file created by shaXsum from linux. Usually there is the filename appended after the hash. Signed-off-by: Hubert Feurstein --- commands/digest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/digest.c b/commands/digest.c index 99b27dcc2..21a091717 100644 --- a/commands/digest.c +++ b/commands/digest.c @@ -164,7 +164,7 @@ static int do_digest(int argc, char *argv[]) if (sig) { digestlen = digest_length(d); - if (siglen == 2 * digestlen) { + if (siglen >= 2 * digestlen) { if (!tmp_sig) tmp_sig = xmalloc(digestlen); -- 2.24.0 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 1/4] led: unify led trigger tables
Currently we have two slightly different led-trigger tables. One in the core module, and the other one in the led-triggers module. The one in the core module, which is used to parse the device-tree triggers, is lacking net-rx and net-tx. Signed-off-by: Hubert Feurstein --- drivers/led/core.c | 46 +++--- drivers/led/led-triggers.c | 25 - 2 files changed, 28 insertions(+), 43 deletions(-) diff --git a/drivers/led/core.c b/drivers/led/core.c index 431966d06..fec7324e7 100644 --- a/drivers/led/core.c +++ b/drivers/led/core.c @@ -256,22 +256,35 @@ void led_unregister(struct led *led) list_del(>list); } -struct led_trg { - const char *str; - enum led_trigger trg; +static char *trigger_names[] = { + [LED_TRIGGER_PANIC] = "panic", + [LED_TRIGGER_HEARTBEAT] = "heartbeat", + [LED_TRIGGER_NET_RX] = "net-rx", + [LED_TRIGGER_NET_TX] = "net-tx", + [LED_TRIGGER_NET_TXRX] = "net", + [LED_TRIGGER_DEFAULT_ON] = "default-on", }; -static struct led_trg triggers[] = { - { .str = "heartbeat", LED_TRIGGER_HEARTBEAT, }, - { .str = "panic", LED_TRIGGER_PANIC, }, - { .str = "net", LED_TRIGGER_NET_TXRX, }, - { .str = "default-on", LED_TRIGGER_DEFAULT_ON, }, -}; +const char *trigger_name(enum led_trigger trigger) +{ + return trigger_names[trigger]; +} + +enum led_trigger trigger_by_name(const char *name) +{ + int i; + + for (i = 0; i < LED_TRIGGER_MAX; i++) + if (!strcmp(name, trigger_names[i])) + return i; + + return LED_TRIGGER_MAX; +} void led_of_parse_trigger(struct led *led, struct device_node *np) { + enum led_trigger trg; const char *trigger; - int i; trigger = of_get_property(np, "linux,default-trigger", NULL); if (!trigger) @@ -280,13 +293,10 @@ void led_of_parse_trigger(struct led *led, struct device_node *np) if (!trigger) return; - for (i = 0; i < ARRAY_SIZE(triggers); i++) { - struct led_trg *trg = [i]; - if (!strcmp(trg->str, trigger)) { - /* disable LED before installing trigger */ - led_set(led, 0); - led_set_trigger(trg->trg, led); - return; - } + trg = trigger_by_name(trigger); + if (trg != LED_TRIGGER_MAX) { + /* disable LED before installing trigger */ + led_set(led, 0); + led_set_trigger(trg, led); } } diff --git a/drivers/led/led-triggers.c b/drivers/led/led-triggers.c index 76a1481e1..216c8639b 100644 --- a/drivers/led/led-triggers.c +++ b/drivers/led/led-triggers.c @@ -143,31 +143,6 @@ int led_set_trigger(enum led_trigger trigger, struct led *led) return 0; } -static char *trigger_names[] = { - [LED_TRIGGER_PANIC] = "panic", - [LED_TRIGGER_HEARTBEAT] = "heartbeat", - [LED_TRIGGER_NET_RX] = "net-rx", - [LED_TRIGGER_NET_TX] = "net-tx", - [LED_TRIGGER_NET_TXRX] = "net", - [LED_TRIGGER_DEFAULT_ON] = "default-on", -}; - -const char *trigger_name(enum led_trigger trigger) -{ - return trigger_names[trigger]; -} - -enum led_trigger trigger_by_name(const char *name) -{ - int i; - - for (i = 0; i < LED_TRIGGER_MAX; i++) - if (!strcmp(name, trigger_names[i])) - return i; - - return LED_TRIGGER_MAX; -} - /** * led_triggers_show_info - Show information about all registered * triggers -- 2.24.0 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 4/4] led: parse panic-indicator from device-tree
Signed-off-by: Hubert Feurstein --- Documentation/devicetree/bindings/leds/common.rst | 3 +++ drivers/led/core.c| 11 --- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/leds/common.rst b/Documentation/devicetree/bindings/leds/common.rst index 5a592d67d..911a55f4f 100644 --- a/Documentation/devicetree/bindings/leds/common.rst +++ b/Documentation/devicetree/bindings/leds/common.rst @@ -12,3 +12,6 @@ Common leds properties * ``label``: The label for this LED. If omitted, the label is taken from the node name (excluding the unit address). + +* ``panic-indicator`` - This property specifies that the LED should be used as a + panic indicator. diff --git a/drivers/led/core.c b/drivers/led/core.c index 4bf19abcc..e727148a2 100644 --- a/drivers/led/core.c +++ b/drivers/led/core.c @@ -286,11 +286,16 @@ enum led_trigger trigger_by_name(const char *name) void led_of_parse_trigger(struct led *led, struct device_node *np) { - enum led_trigger trg; + enum led_trigger trg = LED_TRIGGER_MAX; const char *trigger; - trigger = of_get_property(np, "linux,default-trigger", NULL); - trg = trigger_by_name(trigger); + if (of_property_read_bool(np, "panic-indicator")) + trg = LED_TRIGGER_PANIC; + + if (trg == LED_TRIGGER_MAX) { + trigger = of_get_property(np, "linux,default-trigger", NULL); + trg = trigger_by_name(trigger); + } if (trg == LED_TRIGGER_MAX) { trigger = of_get_property(np, "barebox,default-trigger", NULL); -- 2.24.0 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 2/4] led: add documentation for net-tx and net-rx triggers
Signed-off-by: Hubert Feurstein --- Documentation/devicetree/bindings/leds/common.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/leds/common.rst b/Documentation/devicetree/bindings/leds/common.rst index 1f5ae42ca..5a592d67d 100644 --- a/Documentation/devicetree/bindings/leds/common.rst +++ b/Documentation/devicetree/bindings/leds/common.rst @@ -6,7 +6,9 @@ Common leds properties * ``heartbeat`` - LED flashes at a constant rate * ``panic`` - LED turns on when barebox panics -* ``net`` - LED indicates network activity +* ``net`` - LED indicates network activity (tx and rx) +* ``net-rx`` - LED indicates network activity (rx only) +* ``net-tx`` - LED indicates network activity (tx only) * ``label``: The label for this LED. If omitted, the label is taken from the node name (excluding the unit address). -- 2.24.0 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 3/4] led: check for 'barebox, default-trigger' when 'linux, default-trigger' is not found
When the linux,default-trigger is not found by barebox, then also check if there might be a barebox,default-trigger. Signed-off-by: Hubert Feurstein --- drivers/led/core.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/led/core.c b/drivers/led/core.c index fec7324e7..4bf19abcc 100644 --- a/drivers/led/core.c +++ b/drivers/led/core.c @@ -274,6 +274,9 @@ enum led_trigger trigger_by_name(const char *name) { int i; + if (!name) + return LED_TRIGGER_MAX; + for (i = 0; i < LED_TRIGGER_MAX; i++) if (!strcmp(name, trigger_names[i])) return i; @@ -287,13 +290,13 @@ void led_of_parse_trigger(struct led *led, struct device_node *np) const char *trigger; trigger = of_get_property(np, "linux,default-trigger", NULL); - if (!trigger) - trigger = of_get_property(np, "barebox,default-trigger", NULL); + trg = trigger_by_name(trigger); - if (!trigger) - return; + if (trg == LED_TRIGGER_MAX) { + trigger = of_get_property(np, "barebox,default-trigger", NULL); + trg = trigger_by_name(trigger); + } - trg = trigger_by_name(trigger); if (trg != LED_TRIGGER_MAX) { /* disable LED before installing trigger */ led_set(led, 0); -- 2.24.0 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [BUG] imx6qdl: degraded eMMC write performance
On Tue, Dec 10, 2019 at 04:43:54PM +0100, Hubert Feurstein wrote: > Hi Sascha, > > Am Di., 10. Dez. 2019 um 16:10 Uhr schrieb Sascha Hauer > : > [...] > > Well the commit message says it: > > | commit b6fef20c1215c6ef0004f6af4a9c4b77af51dc43 > > | Author: Sascha Hauer > > | Date: Thu Mar 29 13:49:45 2018 +0200 > > | > > | block: Adjust cache sizes > > | > > | Use four times more cache entries and divide the memory for each entry > > | by four. This lowers the linear read throughput somewhat but increases > > | the access speed for filesystems. > Yes, I know. I've read the commit message. But this patch even costs me ~200ms > in boottime (loading kernel and dts from ext4 root partition). > > [...] > > We could adjust RW_BUF_SIZE (used by copy_file as buffer size) to a full > > chunk size (16KiB). Does this give you back some lost performance? > No, changing RW_BUF_SIZE did not help. And I also see why :-/ block_op_write() works around block sizes (512bytes), not around chunk sizes. This means we always read before we write. This could probably be optimized somehow, but this would only speed up the write case you described in your initial mail. It seems what you are more interested in is the read performance. We could make the number of chunks and the chunk size configurable during runtime. This is less than ideal, but at least we could get a better performance with manageable effort. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [BUG] imx6qdl: degraded eMMC write performance
Hi Sascha, Am Di., 10. Dez. 2019 um 16:10 Uhr schrieb Sascha Hauer : [...] > Well the commit message says it: > | commit b6fef20c1215c6ef0004f6af4a9c4b77af51dc43 > | Author: Sascha Hauer > | Date: Thu Mar 29 13:49:45 2018 +0200 > | > | block: Adjust cache sizes > | > | Use four times more cache entries and divide the memory for each entry > | by four. This lowers the linear read throughput somewhat but increases > | the access speed for filesystems. Yes, I know. I've read the commit message. But this patch even costs me ~200ms in boottime (loading kernel and dts from ext4 root partition). [...] > We could adjust RW_BUF_SIZE (used by copy_file as buffer size) to a full > chunk size (16KiB). Does this give you back some lost performance? No, changing RW_BUF_SIZE did not help. Regards Hubert ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH] habv4: add the possibility to changing the signing area from Kconfig
On Tue, Dec 10, 2019 at 04:06:27PM +0100, Maik Otto wrote: > the whole barebox with mbr and partition table will be signed by default > add the possibility in the Kconfig to change from full signing to skip-mbr > and from-dcdofs Not signing the MBR seems the right thing to do. Do we need it configurable at all or would it be better to just always skip the first KiB? Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [BUG] imx6qdl: degraded eMMC write performance
Hi Hubert, On Tue, Dec 10, 2019 at 03:44:52PM +0100, Hubert Feurstein wrote: > Hi, > > I've updated barebox for our custom platform from v2015.06.0 to > v2019.12.0. With the new version I have noticed a much worse write > performance onto the onboard eMMC. > > With v2015.06.0 the indicated progress of the copy command is very > smooth. Calling "cp -v /dev/zero /dev/mmc3.root" takes about 80 > seconds for 256MB. But with v2019.12.0 the progress is very bumpy and > the copy takes about 280 seconds. > > I've tracked this down to this commit which destroys the performance: > "block: Adjust cache sizes" (b6fef20c1215c6ef0004f6af4a9c4b77af51dc43) Well the commit message says it: | commit b6fef20c1215c6ef0004f6af4a9c4b77af51dc43 | Author: Sascha Hauer | Date: Thu Mar 29 13:49:45 2018 +0200 | | block: Adjust cache sizes | | Use four times more cache entries and divide the memory for each entry | by four. This lowers the linear read throughput somewhat but increases | the access speed for filesystems. The barebox block layer is quite simple. It always handles data in chunks of a hardcoded size. For a high throughput these should be large as this increases the buffer size we can write in the MMC controller in one go. Increasing the size is bad for filesystems though, as for each small block a fs wants to read at minimum a whole chunk must be read, or for write, read-modified-written. I remember I once tried to make the caching more intelligent. It seems I didn't even get far enough to store a branch in my working copy :( I just saw that in block_op_write() we must first read any partially written chunk, then modify it and write it back. Only full chunks can be written without reading them first. This means we should gain performance when we enter block_op_write() with only full chunks. We could adjust RW_BUF_SIZE (used by copy_file as buffer size) to a full chunk size (16KiB). Does this give you back some lost performance? Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 1/1] mci: imx-esdhc: fix termination of statement
Use semicolon instead of comma to terminate statement. Signed-off-by: Hubert Feurstein --- drivers/mci/imx-esdhc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mci/imx-esdhc.c b/drivers/mci/imx-esdhc.c index db3450a26..4f9fe2996 100644 --- a/drivers/mci/imx-esdhc.c +++ b/drivers/mci/imx-esdhc.c @@ -697,7 +697,7 @@ static int fsl_esdhc_probe(struct device_d *dev) host->mci.card_present = esdhc_card_present; host->mci.hw_dev = dev; - dev->detect = fsl_esdhc_detect, + dev->detect = fsl_esdhc_detect; rate = clk_get_rate(host->clk); host->mci.f_min = rate >> 12; -- 2.24.0 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[BUG] imx6qdl: degraded eMMC write performance
Hi, I've updated barebox for our custom platform from v2015.06.0 to v2019.12.0. With the new version I have noticed a much worse write performance onto the onboard eMMC. With v2015.06.0 the indicated progress of the copy command is very smooth. Calling "cp -v /dev/zero /dev/mmc3.root" takes about 80 seconds for 256MB. But with v2019.12.0 the progress is very bumpy and the copy takes about 280 seconds. I've tracked this down to this commit which destroys the performance: "block: Adjust cache sizes" (b6fef20c1215c6ef0004f6af4a9c4b77af51dc43) Regards Hubert ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v2 5/8] ARM: dts: imx6: phycard: Use partname for environment device-path
Hi Sascha, On 10.12.19 11:45, Sascha Hauer wrote: On Tue, Dec 10, 2019 at 09:15:52AM +0100, Stefan Riedmüller wrote: Hi Sascha, On 10.12.19 09:05, Sascha Hauer wrote: On Tue, Dec 10, 2019 at 08:50:41AM +0100, Stefan Riedmüller wrote: Hi Sascha, On 09.12.19 16:37, Sascha Hauer wrote: On Mon, Dec 09, 2019 at 01:31:40PM +0100, Stefan Riedmueller wrote: Change environment device-path from using a separate label to referencing a device plus partname. Why? The way it was is fine. Just to be consistent with phyCORE and phyFLEX. Is there a downside I'm not aware of? It feels more natural to directly point to the partition, that's why I prefer that way. So if you don't have a good reason I suggest to convert it the other way round for consistency I will recheck if there was a specific reason for us to use the partname approach. But you can drop this patch for now. I just tried. The series doesn't apply on current master. Could you rebase it? Yes, of course. Stefan Sascha ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH v2] firmware: zynqmp-fpga: drop example bin format header
Avoid the example bitstream header to validate the bitstream that should be loaded into the FPGA. The header is mostly 0x with a few special values at a certain offsets and can be better described with the offsets and their magic values. As a drive by, this fixes/removes a broken check in the header validation. The != operator has a higher precedence than ?: and this check should have had parenthesis around the ?: expression: bin_header[i] != (byte_order == XILINX_BYTE_ORDER_BIT) ? bin_format[i] : __swab32(bin_format[i]) Signed-off-by: Michael Tretter Reviewed-by: Thomas Haemmerle --- Changes in v2: - Fix subject zynmp-fpga -> zynqmp-fpga drivers/firmware/zynqmp-fpga.c | 121 + 1 file changed, 62 insertions(+), 59 deletions(-) diff --git a/drivers/firmware/zynqmp-fpga.c b/drivers/firmware/zynqmp-fpga.c index 1728e2a954..e02667355f 100644 --- a/drivers/firmware/zynqmp-fpga.c +++ b/drivers/firmware/zynqmp-fpga.c @@ -24,11 +24,32 @@ #define ZYNQMP_PM_VERSION_1_1_FEATURES (ZYNQMP_PM_FEATURE_BYTE_ORDER_IRREL | \ ZYNQMP_PM_FEATURE_SIZE_NOT_NEEDED) +/* + * Xilinx KU040 Bitstream Composition: + * + * Bitstream can be provided with an optinal header (`struct bs_header`). + * The true bitstream starts with the binary-header composed of 21 words: + * + * 0: 0x (Dummy pad word) + * ... + * 15: 0x (Dummy pad word) + * 16: 0x00BB (Bus width auto detect word 1) + * 17: 0x11220044 (Bus width auto detect word 2) + * 18: 0x (Dummy pad word) + * 19: 0x (Dummy pad word) + * 20: 0xAA995566 (Sync word) + * + * See Xilinx UG570 (v1.11) September 30 2019, Chapter 9 "Configuration + * Details - Bitstream Composition" for further details. + */ #define DUMMY_WORD 0x -#define BUS_WIDTH_WORD_1 0x00BB -#define BUS_WIDTH_WORD_2 0x11220044 +#define BUS_WIDTH_AUTO_DETECT1_OFFSET 16 +#define BUS_WIDTH_AUTO_DETECT1 0x00BB +#define BUS_WIDTH_AUTO_DETECT2_OFFSET 17 +#define BUS_WIDTH_AUTO_DETECT2 0x11220044 +#define SYNC_WORD_OFFSET 20 #define SYNC_WORD 0xAA995566 -#define SYNC_WORD_OFFS 20 +#define BIN_HEADER_LENGTH 21 enum xilinx_byte_order { XILINX_BYTE_ORDER_BIT, @@ -58,48 +79,6 @@ struct bs_header_entry { char data[0]; } __attribute__ ((packed)); -/* - * Xilinx KU040 Bitstream Composition: - * Bitstream can be provided with an optinal header (`struct bs_header`). - * The true bitstream starts with the binary-header composed of 21 words: - * - * 1: 0x (Dummy pad word) - * ... - * 16: 0x (Dummy pad word) - * 17: 0x00BB (Bus width auto detect word 1) - * 18: 0x11220044 (Bus width auto detect word 2) - * 19: 0x (Dummy pad word) - * 20: 0x (Dummy pad word) - * 21: 0xAA995566 (Sync word) - * - * Please refer to Xilinx UG570 (v1.11) September 30 2019, - * Chapter 9 Configuration Details - Bitstream Composition - * for further details! - */ -static const u32 bin_format[] = { - DUMMY_WORD, - DUMMY_WORD, - DUMMY_WORD, - DUMMY_WORD, - DUMMY_WORD, - DUMMY_WORD, - DUMMY_WORD, - DUMMY_WORD, - DUMMY_WORD, - DUMMY_WORD, - DUMMY_WORD, - DUMMY_WORD, - DUMMY_WORD, - DUMMY_WORD, - DUMMY_WORD, - DUMMY_WORD, - BUS_WIDTH_WORD_1, - BUS_WIDTH_WORD_2, - DUMMY_WORD, - DUMMY_WORD, - SYNC_WORD, -}; - static void copy_words_swapped(u32 *dst, const u32 *src, size_t size) { int i; @@ -111,36 +90,59 @@ static void copy_words_swapped(u32 *dst, const u32 *src, size_t size) static int get_byte_order(const u32 *bin_header, size_t size, enum xilinx_byte_order *byte_order) { - if (size < sizeof(bin_format)) + if (size < BIN_HEADER_LENGTH * sizeof(*bin_header)) return -EINVAL; - if (bin_header[SYNC_WORD_OFFS] == SYNC_WORD) { + if (bin_header[SYNC_WORD_OFFSET] == SYNC_WORD) { *byte_order = XILINX_BYTE_ORDER_BIT; return 0; } - if (bin_header[SYNC_WORD_OFFS] == __swab32(SYNC_WORD)) { - *byte_order = XILINX_BYTE_ORDER_BIN; + if (bin_header[SYNC_WORD_OFFSET] == __swab32(SYNC_WORD)) { + *byte_order = XILINX_BYTE_ORDER_BIN; return 0; } return -EINVAL; } -static int is_bin_header_valid(const u32 *bin_header, size_t size, - enum xilinx_byte_order byte_order) +static bool is_bin_header_valid(const u32 *bin_header, size_t size, + enum xilinx_byte_order byte_order) { - int i; + size_t i; - if (size < ARRAY_SIZE(bin_format)) - return 0; + if (size < BIN_HEADER_LENGTH * sizeof(*bin_header)) +
Re: [PATCH] firmware: zynmp-fpga: drop example bin format header
typo in subject: zynmp-fpga -> zynqmp-fpga On 09.12.19 14:59, Michael Tretter wrote: > Avoid the example bitstream header to validate the bitstream that should > be loaded into the FPGA. The header is mostly 0x with a few > special values at a certain offsets and can be better described with the > offsets and their magic values. > > As a drive by, this fixes/removes a broken check in the header > validation. The != operator has a higher precedence than ?: and this > check should have had parenthesis around the ?: expression: > > bin_header[i] != (byte_order == XILINX_BYTE_ORDER_BIT) ? >bin_format[i] : __swab32(bin_format[i]) > > Signed-off-by: Michael Tretter Reviewed-by: Thomas Haemmerle > --- > drivers/firmware/zynqmp-fpga.c | 121 + > 1 file changed, 62 insertions(+), 59 deletions(-) > > diff --git a/drivers/firmware/zynqmp-fpga.c b/drivers/firmware/zynqmp-fpga.c > index 1728e2a954..e02667355f 100644 > --- a/drivers/firmware/zynqmp-fpga.c > +++ b/drivers/firmware/zynqmp-fpga.c > @@ -24,11 +24,32 @@ > #define ZYNQMP_PM_VERSION_1_1_FEATURES > (ZYNQMP_PM_FEATURE_BYTE_ORDER_IRREL | \ >ZYNQMP_PM_FEATURE_SIZE_NOT_NEEDED) > > +/* > + * Xilinx KU040 Bitstream Composition: > + * > + * Bitstream can be provided with an optinal header (`struct bs_header`). > + * The true bitstream starts with the binary-header composed of 21 words: > + * > + * 0: 0x (Dummy pad word) > + * ... > + * 15: 0x (Dummy pad word) > + * 16: 0x00BB (Bus width auto detect word 1) > + * 17: 0x11220044 (Bus width auto detect word 2) > + * 18: 0x (Dummy pad word) > + * 19: 0x (Dummy pad word) > + * 20: 0xAA995566 (Sync word) > + * > + * See Xilinx UG570 (v1.11) September 30 2019, Chapter 9 "Configuration > + * Details - Bitstream Composition" for further details. > + */ > #define DUMMY_WORD 0x > -#define BUS_WIDTH_WORD_1 0x00BB > -#define BUS_WIDTH_WORD_2 0x11220044 > +#define BUS_WIDTH_AUTO_DETECT1_OFFSET16 > +#define BUS_WIDTH_AUTO_DETECT1 0x00BB > +#define BUS_WIDTH_AUTO_DETECT2_OFFSET17 > +#define BUS_WIDTH_AUTO_DETECT2 0x11220044 > +#define SYNC_WORD_OFFSET 20 > #define SYNC_WORD 0xAA995566 > -#define SYNC_WORD_OFFS 20 > +#define BIN_HEADER_LENGTH21 > > enum xilinx_byte_order { > XILINX_BYTE_ORDER_BIT, > @@ -58,48 +79,6 @@ struct bs_header_entry { > char data[0]; > } __attribute__ ((packed)); > > -/* > - * Xilinx KU040 Bitstream Composition: > - * Bitstream can be provided with an optinal header (`struct bs_header`). > - * The true bitstream starts with the binary-header composed of 21 words: > - * > - * 1: 0x (Dummy pad word) > - * ... > - * 16: 0x (Dummy pad word) > - * 17: 0x00BB (Bus width auto detect word 1) > - * 18: 0x11220044 (Bus width auto detect word 2) > - * 19: 0x (Dummy pad word) > - * 20: 0x (Dummy pad word) > - * 21: 0xAA995566 (Sync word) > - * > - * Please refer to Xilinx UG570 (v1.11) September 30 2019, > - * Chapter 9 Configuration Details - Bitstream Composition > - * for further details! > - */ > -static const u32 bin_format[] = { > - DUMMY_WORD, > - DUMMY_WORD, > - DUMMY_WORD, > - DUMMY_WORD, > - DUMMY_WORD, > - DUMMY_WORD, > - DUMMY_WORD, > - DUMMY_WORD, > - DUMMY_WORD, > - DUMMY_WORD, > - DUMMY_WORD, > - DUMMY_WORD, > - DUMMY_WORD, > - DUMMY_WORD, > - DUMMY_WORD, > - DUMMY_WORD, > - BUS_WIDTH_WORD_1, > - BUS_WIDTH_WORD_2, > - DUMMY_WORD, > - DUMMY_WORD, > - SYNC_WORD, > -}; > - > static void copy_words_swapped(u32 *dst, const u32 *src, size_t size) > { > int i; > @@ -111,36 +90,59 @@ static void copy_words_swapped(u32 *dst, const u32 *src, > size_t size) > static int get_byte_order(const u32 *bin_header, size_t size, > enum xilinx_byte_order *byte_order) > { > - if (size < sizeof(bin_format)) > + if (size < BIN_HEADER_LENGTH * sizeof(*bin_header)) > return -EINVAL; > > - if (bin_header[SYNC_WORD_OFFS] == SYNC_WORD) { > + if (bin_header[SYNC_WORD_OFFSET] == SYNC_WORD) { > *byte_order = XILINX_BYTE_ORDER_BIT; > return 0; > } > > - if (bin_header[SYNC_WORD_OFFS] == __swab32(SYNC_WORD)) { > - *byte_order = XILINX_BYTE_ORDER_BIN; > + if (bin_header[SYNC_WORD_OFFSET] == __swab32(SYNC_WORD)) { > + *byte_order = XILINX_BYTE_ORDER_BIN; > return 0; > } > > return -EINVAL; > } > > -static int is_bin_header_valid(const u32 *bin_header, size_t size, > -enum xilinx_byte_order byte_order) > +static bool is_bin_header_valid(const u32 *bin_header, size_t
Re: [PATCH v2 5/8] ARM: dts: imx6: phycard: Use partname for environment device-path
On Tue, Dec 10, 2019 at 09:15:52AM +0100, Stefan Riedmüller wrote: > Hi Sascha, > > On 10.12.19 09:05, Sascha Hauer wrote: > > On Tue, Dec 10, 2019 at 08:50:41AM +0100, Stefan Riedmüller wrote: > > > Hi Sascha, > > > > > > On 09.12.19 16:37, Sascha Hauer wrote: > > > > On Mon, Dec 09, 2019 at 01:31:40PM +0100, Stefan Riedmueller wrote: > > > > > Change environment device-path from using a separate label to > > > > > referencing a device plus partname. > > > > > > > > Why? The way it was is fine. > > > > > > Just to be consistent with phyCORE and phyFLEX. Is there a downside I'm > > > not > > > aware of? > > > > It feels more natural to directly point to the partition, that's why I > > prefer that way. So if you don't have a good reason I suggest to convert > > it the other way round for consistency > > I will recheck if there was a specific reason for us to use the partname > approach. But you can drop this patch for now. I just tried. The series doesn't apply on current master. Could you rebase it? Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v2 5/8] ARM: dts: imx6: phycard: Use partname for environment device-path
Hi Sascha, On 10.12.19 09:05, Sascha Hauer wrote: On Tue, Dec 10, 2019 at 08:50:41AM +0100, Stefan Riedmüller wrote: Hi Sascha, On 09.12.19 16:37, Sascha Hauer wrote: On Mon, Dec 09, 2019 at 01:31:40PM +0100, Stefan Riedmueller wrote: Change environment device-path from using a separate label to referencing a device plus partname. Why? The way it was is fine. Just to be consistent with phyCORE and phyFLEX. Is there a downside I'm not aware of? It feels more natural to directly point to the partition, that's why I prefer that way. So if you don't have a good reason I suggest to convert it the other way round for consistency I will recheck if there was a specific reason for us to use the partname approach. But you can drop this patch for now. Thanks Stefan Sascha ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v2 5/8] ARM: dts: imx6: phycard: Use partname for environment device-path
On Tue, Dec 10, 2019 at 08:50:41AM +0100, Stefan Riedmüller wrote: > Hi Sascha, > > On 09.12.19 16:37, Sascha Hauer wrote: > > On Mon, Dec 09, 2019 at 01:31:40PM +0100, Stefan Riedmueller wrote: > > > Change environment device-path from using a separate label to > > > referencing a device plus partname. > > > > Why? The way it was is fine. > > Just to be consistent with phyCORE and phyFLEX. Is there a downside I'm not > aware of? It feels more natural to directly point to the partition, that's why I prefer that way. So if you don't have a good reason I suggest to convert it the other way round for consistency Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox