Re: [PATCH 4/6] arch: arm: dts: k3-am68-sk-r5: Sync with J721s2 R5 file
Hi Manorit
On 09/05/24 11:04, Manorit Chawdhry wrote:
Hi Neha,
On 10:37-20240509, Manorit Chawdhry wrote:
Hi Neha,
On 16:09-20240508, Neha Malcom Francis wrote:
Hi Manorit,
On 08/05/24 12:56, Manorit Chawdhry wrote:
Update the file with the required nodes from J721s2 R5 file to start
using k3-am68-sk-r5 file for AM68.
Signed-off-by: Manorit Chawdhry
---
What's the motivation behind this patch vs. squashing it into patch 5/6?
Could've squashed it ig, I think developed it in this order so this
remained. Would squash it. Also realised that I should be putting that
patch before the config split otherwise am68 boot would break again.
Would take that up as well in v2. Thanks for the review!
Though on second thoughts.. I think it's good as it tells that AM68 R5
DT had been missing some changes. If someone wants to track what changed
then ig it's better that they don't have to debug the merge commit which
ends up altering the contents of AM68 R5 DT ( in-case this patch ain't
there ) and people will have to manually check the diff as to what
altered. Do you think it's better to keep this patch with the following
reasoning?
Yes you can do that but I think this commit message is confusing. The "start
using k3-am68-sk-r5 file for AM68" threw me off, maybe modify it to say that
AM68 R5 DT is missing these changes and needs them why? After that grabbing the
common bits into an SoC R5 file in patch 5/6 makes sense.
Regards,
Manorit
Regards,
Manorit
arch/arm/dts/k3-am68-sk-r5-base-board.dts | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/arm/dts/k3-am68-sk-r5-base-board.dts
b/arch/arm/dts/k3-am68-sk-r5-base-board.dts
index 695aadc287bd..038b08dc3e01 100644
--- a/arch/arm/dts/k3-am68-sk-r5-base-board.dts
+++ b/arch/arm/dts/k3-am68-sk-r5-base-board.dts
@@ -24,7 +24,8 @@
compatible = "ti,am654-rproc";
reg = <0x0 0x00a9 0x0 0x10>;
power-domains = <_pds 61 TI_SCI_PD_EXCLUSIVE>,
- <_pds 202 TI_SCI_PD_EXCLUSIVE>;
+ <_pds 202 TI_SCI_PD_EXCLUSIVE>,
+ <_pds 4 TI_SCI_PD_EXCLUSIVE>;
resets = <_reset 202 0>;
clocks = <_clks 61 1>;
assigned-clocks = <_clks 61 1>, <_clks 202 0>;
@@ -54,10 +55,12 @@
_proxy_mcu {
bootph-pre-ram;
+ status = "okay";
};
_proxy_sa3 {
bootph-pre-ram;
+ status = "okay";
};
_mcu_wakeup {
--
Thanking You
Neha Malcom Francis
--
Thanking You
Neha Malcom Francis
Re: [PATCH 4/6] arch: arm: dts: k3-am68-sk-r5: Sync with J721s2 R5 file
Hi Neha,
On 10:37-20240509, Manorit Chawdhry wrote:
> Hi Neha,
>
> On 16:09-20240508, Neha Malcom Francis wrote:
> > Hi Manorit,
> >
> > On 08/05/24 12:56, Manorit Chawdhry wrote:
> > > Update the file with the required nodes from J721s2 R5 file to start
> > > using k3-am68-sk-r5 file for AM68.
> > >
> > > Signed-off-by: Manorit Chawdhry
> > > ---
> >
> > What's the motivation behind this patch vs. squashing it into patch 5/6?
> >
>
> Could've squashed it ig, I think developed it in this order so this
> remained. Would squash it. Also realised that I should be putting that
> patch before the config split otherwise am68 boot would break again.
> Would take that up as well in v2. Thanks for the review!
>
Though on second thoughts.. I think it's good as it tells that AM68 R5
DT had been missing some changes. If someone wants to track what changed
then ig it's better that they don't have to debug the merge commit which
ends up altering the contents of AM68 R5 DT ( in-case this patch ain't
there ) and people will have to manually check the diff as to what
altered. Do you think it's better to keep this patch with the following
reasoning?
Regards,
Manorit
> Regards,
> Manorit
>
> > > arch/arm/dts/k3-am68-sk-r5-base-board.dts | 5 -
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/dts/k3-am68-sk-r5-base-board.dts
> > > b/arch/arm/dts/k3-am68-sk-r5-base-board.dts
> > > index 695aadc287bd..038b08dc3e01 100644
> > > --- a/arch/arm/dts/k3-am68-sk-r5-base-board.dts
> > > +++ b/arch/arm/dts/k3-am68-sk-r5-base-board.dts
> > > @@ -24,7 +24,8 @@
> > > compatible = "ti,am654-rproc";
> > > reg = <0x0 0x00a9 0x0 0x10>;
> > > power-domains = <_pds 61 TI_SCI_PD_EXCLUSIVE>,
> > > - <_pds 202 TI_SCI_PD_EXCLUSIVE>;
> > > + <_pds 202 TI_SCI_PD_EXCLUSIVE>,
> > > + <_pds 4 TI_SCI_PD_EXCLUSIVE>;
> > > resets = <_reset 202 0>;
> > > clocks = <_clks 61 1>;
> > > assigned-clocks = <_clks 61 1>, <_clks 202 0>;
> > > @@ -54,10 +55,12 @@
> > > _proxy_mcu {
> > > bootph-pre-ram;
> > > + status = "okay";
> > > };
> > > _proxy_sa3 {
> > > bootph-pre-ram;
> > > + status = "okay";
> > > };
> > > _mcu_wakeup {
> > >
> >
> > --
> > Thanking You
> > Neha Malcom Francis
Re: [PATCH 4/6] arch: arm: dts: k3-am68-sk-r5: Sync with J721s2 R5 file
Hi Neha,
On 16:09-20240508, Neha Malcom Francis wrote:
> Hi Manorit,
>
> On 08/05/24 12:56, Manorit Chawdhry wrote:
> > Update the file with the required nodes from J721s2 R5 file to start
> > using k3-am68-sk-r5 file for AM68.
> >
> > Signed-off-by: Manorit Chawdhry
> > ---
>
> What's the motivation behind this patch vs. squashing it into patch 5/6?
>
Could've squashed it ig, I think developed it in this order so this
remained. Would squash it. Also realised that I should be putting that
patch before the config split otherwise am68 boot would break again.
Would take that up as well in v2. Thanks for the review!
Regards,
Manorit
> > arch/arm/dts/k3-am68-sk-r5-base-board.dts | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/dts/k3-am68-sk-r5-base-board.dts
> > b/arch/arm/dts/k3-am68-sk-r5-base-board.dts
> > index 695aadc287bd..038b08dc3e01 100644
> > --- a/arch/arm/dts/k3-am68-sk-r5-base-board.dts
> > +++ b/arch/arm/dts/k3-am68-sk-r5-base-board.dts
> > @@ -24,7 +24,8 @@
> > compatible = "ti,am654-rproc";
> > reg = <0x0 0x00a9 0x0 0x10>;
> > power-domains = <_pds 61 TI_SCI_PD_EXCLUSIVE>,
> > - <_pds 202 TI_SCI_PD_EXCLUSIVE>;
> > + <_pds 202 TI_SCI_PD_EXCLUSIVE>,
> > + <_pds 4 TI_SCI_PD_EXCLUSIVE>;
> > resets = <_reset 202 0>;
> > clocks = <_clks 61 1>;
> > assigned-clocks = <_clks 61 1>, <_clks 202 0>;
> > @@ -54,10 +55,12 @@
> > _proxy_mcu {
> > bootph-pre-ram;
> > + status = "okay";
> > };
> > _proxy_sa3 {
> > bootph-pre-ram;
> > + status = "okay";
> > };
> > _mcu_wakeup {
> >
>
> --
> Thanking You
> Neha Malcom Francis
[PATCH] board: starfive: support Pine64 Star64 board
On Tuesday, May 7th, 2024 at 12:31 AM, E Shattow wrote: > On Mon, May 6, 2024 at 8:30 AM H Bell wrote: > > + static const char compat[] = "starfive,star64\0starfive,jh7110"; > Should be "pine64,star64\nstarfive,jh7110" ? changed to "pine64,star64\0starfive,jh7110" in v2 On Tuesday, May 7th, 2024 at 12:31 AM, E Shattow wrote: > New property names are not made clear to me how to choose these > different delays for 10/100 Mbps vs 1000 Mbps operation. If the > fishwaldo repo is a valid reference then it seems different than the > content in starfive_vf2_pro starfive_verb[]. No Star64 here to test > with, does this work as-is for both network interfaces? Agreed. I've created a separate object in v2 for some further investigations with the delays/timings to make sure they're both working On Tuesday, May 7th, 2024 at 12:31 AM, E Shattow wrote: > On Mon, May 6, 2024 at 8:30 AM H Bell wrote: > > +#define FDTFILE_PINE64_STAR64 \ > > + "starfive/pine64-star64.dtb" > Should be "starfive/jh7110-pine64-star64.dtb" ? fixed in v2
[PATCH 2/2 v2] board: starfive: support Pine64 Star64 board
Add documentation files Signed-off-by: Henry Bell Cc: ycli...@andestech.com Cc: heinrich.schucha...@canonical.com --- Changes since v1 - New patch --- doc/board/starfive/index.rst | 1 + doc/board/starfive/pine64_star64.rst | 109 +++ 2 files changed, 110 insertions(+) create mode 100644 doc/board/starfive/pine64_star64.rst diff --git a/doc/board/starfive/index.rst b/doc/board/starfive/index.rst index 2762bf74c1..f05f8a0765 100644 --- a/doc/board/starfive/index.rst +++ b/doc/board/starfive/index.rst @@ -7,4 +7,5 @@ StarFive :maxdepth: 1 milk-v_mars.rst + pine64_star64 visionfive2 diff --git a/doc/board/starfive/pine64_star64.rst b/doc/board/starfive/pine64_star64.rst new file mode 100644 index 00..047f109028 --- /dev/null +++ b/doc/board/starfive/pine64_star64.rst @@ -0,0 +1,109 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Pine64 Star64 +=== + +U-Boot for the Star64 uses the same U-Boot binaries as the VisionFive 2 board. +In U-Boot SPL the actual board is detected and the device-tree patched +accordingly. + +Building + + +1. Add the RISC-V toolchain to your PATH. +2. Setup ARCH & cross compilation environment variable: + +.. code-block:: none + + export CROSS_COMPILE= + +The M-mode software OpenSBI provides the supervisor binary interface (SBI) and +is responsible for the switch to S-Mode. It is a prerequisite to build U-Boot. +Support for the JH7110 was introduced in OpenSBI 1.2. It is recommended to use +a current release. + +.. code-block:: console + + git clone https://github.com/riscv/opensbi.git + cd opensbi + make PLATFORM=generic FW_TEXT_START=0x4000 FW_OPTIONS=0 + +Now build the U-Boot SPL and U-Boot proper. + +.. code-block:: console + + cd + make starfive_visionfive2_defconfig + make OPENSBI=$(opensbi_dir)/build/platform/generic/firmware/fw_dynamic.bin + +This will generate the U-Boot SPL image (spl/u-boot-spl.bin.normal.out) as well +as the FIT image (u-boot.itb) with OpenSBI and U-Boot. + +Device-tree selection +~ + +U-Boot will set variable $fdtfile to starfive/jh7110-pine64-star64.dtb. + +To overrule this selection the variable can be set manually and saved in the +environment + +:: + +setenv fdtfile my_device-tree.dtb +env save + +or the configuration variable CONFIG_DEFAULT_FDT_FILE can be used to set to +provide a default value. + +Boot source selection +~ + +The board provides the DIP switches MSEL[1:0] to select the boot device out of +SPI flash, eMMC, SD-card, UART. To select booting from SD-card set the DIP +switches MSEL[1:0] to 10. + +Preparing the SD-Card +~ + +The device firmware loads U-Boot SPL (u-boot-spl.bin.normal.out) from the +partition with type GUID 2E54B353-1271-4842-806F-E436D6AF6985. You are free +to choose any partition number. + +With the default configuration U-Boot SPL loads the U-Boot FIT image +(u-boot.itb) from partition 2 (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION=0x2). +When formatting it is recommended to use GUID +BC13C2FF-59E6-4262-A352-B275FD6F7172 for this partition. + +The FIT image (u-boot.itb) is a combination of OpenSBI's fw_dynamic.bin, +u-boot-nodtb.bin and the device tree blob. + +Format the SD card (make sure the disk has GPT, otherwise use gdisk to switch) + +.. code-block:: bash + + sudo sgdisk --clear \ + --set-alignment=2 \ + --new=1:4096:8191 --change-name=1:spl --typecode=1:2E54B353-1271-4842-806F-E436D6AF6985\ + --new=2:8192:16383 --change-name=2:uboot --typecode=2:BC13C2FF-59E6-4262-A352-B275FD6F7172 \ + --new=3:16384:1654784 --change-name=3:system --typecode=3:EBD0A0A2-B9E5-4433-87C0-68B6B72699C7 \ + /dev/sdb + +Copy U-Boot to the SD card + +.. code-block:: bash + + sudo dd if=u-boot-spl.bin.normal.out of=/dev/sdb1 + sudo dd if=u-boot.itb of=/dev/sdb2 + + sudo mount /dev/sdb3 /mnt/ + sudo cp u-boot-spl.bin.normal.out /mnt/ + sudo cp u-boot.itb /mnt/ + sudo cp Image.gz /mnt/ + sudo cp initramfs.cpio.gz /mnt/ + sudo cp jh7110-starfive-visionfive-2.dtb /mnt/ + sudo umount /mnt + +Booting +~~~ + +Once you plugin the sdcard and power up, you should see the U-Boot prompt. -- 2.44.0
[PATCH 1/2 v2] board: starfive: support Pine64 Star64 board
Similar to the Milk-V Mars, The Star64 board contains few differences to the
VisionFive 2 boards, so can be part of the same U-boot build.
Signed-off-by: Henry Bell
Cc: ycli...@andestech.com
Cc: heinrich.schucha...@canonical.com
---
Changes since v1
- Fix typos on naming
- Create pine64_star64 struct to be populated with PHY values once confirmed
---
board/starfive/visionfive2/spl.c | 85 +++
.../visionfive2/starfive_visionfive2.c| 4 +
2 files changed, 89 insertions(+)
diff --git a/board/starfive/visionfive2/spl.c b/board/starfive/visionfive2/spl.c
index ca61b5be22..248c6ba01d 100644
--- a/board/starfive/visionfive2/spl.c
+++ b/board/starfive/visionfive2/spl.c
@@ -86,6 +86,39 @@ static const struct starfive_vf2_pro starfive_verb[] = {
"tx-internal-delay-ps", "0"},
};
+static const struct starfive_vf2_pro star64_pine64[] = {
+ {"/soc/ethernet@1603", "starfive,tx-use-rgmii-clk", NULL},
+ {"/soc/ethernet@1604", "starfive,tx-use-rgmii-clk", NULL},
+
+ {"/soc/ethernet@1603/mdio/ethernet-phy@0",
+ "motorcomm,tx-clk-adj-enabled", NULL},
+ {"/soc/ethernet@1603/mdio/ethernet-phy@0",
+ "motorcomm,tx-clk-100-inverted", NULL},
+ {"/soc/ethernet@1603/mdio/ethernet-phy@0",
+ "motorcomm,tx-clk-1000-inverted", NULL},
+ {"/soc/ethernet@1603/mdio/ethernet-phy@0",
+ "motorcomm,rx-clk-drv-microamp", "3970"},
+ {"/soc/ethernet@1603/mdio/ethernet-phy@0",
+ "motorcomm,rx-data-drv-microamp", "2910"},
+ {"/soc/ethernet@1603/mdio/ethernet-phy@0",
+ "rx-internal-delay-ps", "1900"},
+ {"/soc/ethernet@1603/mdio/ethernet-phy@0",
+ "tx-internal-delay-ps", "1500"},
+
+ {"/soc/ethernet@1604/mdio/ethernet-phy@1",
+ "motorcomm,tx-clk-adj-enabled", NULL},
+ { "/soc/ethernet@1604/mdio/ethernet-phy@1",
+ "motorcomm,tx-clk-100-inverted", NULL},
+ {"/soc/ethernet@1604/mdio/ethernet-phy@1",
+ "motorcomm,rx-clk-drv-microamp", "3970"},
+ {"/soc/ethernet@1604/mdio/ethernet-phy@1",
+ "motorcomm,rx-data-drv-microamp", "2910"},
+ {"/soc/ethernet@1604/mdio/ethernet-phy@1",
+ "rx-internal-delay-ps", "0"},
+ {"/soc/ethernet@1604/mdio/ethernet-phy@1",
+ "tx-internal-delay-ps", "0"},
+};
+
void spl_fdt_fixup_mars(void *fdt)
{
static const char compat[] = "milkv,mars\0starfive,jh7110";
@@ -226,6 +259,56 @@ void spl_fdt_fixup_version_b(void *fdt)
}
}
+void spl_fdt_fixup_star64(void *fdt)
+{
+ static const char compat[] = "pine64,star64\0starfive,jh7110";
+ u32 phandle;
+ u8 i;
+ int offset;
+ int ret;
+
+ fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat,
sizeof(compat));
+ fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model",
+ "Pine64 Star64");
+
+ /* gmac0 */
+ offset = fdt_path_offset(fdt, "/soc/clock-controller@1700");
+ phandle = fdt_get_phandle(fdt, offset);
+ offset = fdt_path_offset(fdt, "/soc/ethernet@1603");
+
+ fdt_setprop_u32(fdt, offset, "assigned-clocks", phandle);
+ fdt_appendprop_u32(fdt, offset, "assigned-clocks",
JH7110_AONCLK_GMAC0_TX);
+ fdt_setprop_u32(fdt, offset, "assigned-clock-parents", phandle);
+ fdt_appendprop_u32(fdt, offset, "assigned-clock-parents",
+ JH7110_AONCLK_GMAC0_RMII_RTX);
+
+ /* gmac1 */
+ offset = fdt_path_offset(fdt, "/soc/clock-controller@1302");
+ phandle = fdt_get_phandle(fdt, offset);
+ offset = fdt_path_offset(fdt, "/soc/ethernet@1604");
+
+ fdt_setprop_u32(fdt, offset, "assigned-clocks", phandle);
+ fdt_appendprop_u32(fdt, offset, "assigned-clocks",
JH7110_SYSCLK_GMAC1_TX);
+ fdt_setprop_u32(fdt, offset, "assigned-clock-parents", phandle);
+ fdt_appendprop_u32(fdt, offset, "assigned-clock-parents",
+ JH7110_SYSCLK_GMAC1_RMII_RTX);
+
+ for (i = 0; i < ARRAY_SIZE(star64_pine64); i++) {
+ offset = fdt_path_offset(fdt, star64_pine64[i].path);
+
+ if (star64_pine64[i].value)
+ ret = fdt_setprop_u32(fdt, offset,
star64_pine64[i].name,
+ dectoul(star64_pine64[i].value,
NULL));
+ else
+ ret = fdt_setprop_empty(fdt, offset,
star64_pine64[i].name);
+
+ if (ret) {
+ pr_err("%s set prop %s fail.\n", __func__,
star64_pine64[i].name);
+ break;
+ }
+ }
+}
+
void spl_perform_fixups(struct spl_image_info *spl_image)
{
u8 version;
@@ -252,6 +335,8 @@ void spl_perform_fixups(struct spl_image_info *spl_image)
[PATCH v8 3/3] net: bootp: add config option BOOTP_RANDOM_XID
The new config option BOOTP_RANDOM_XID will randomize the transaction ID
for each new BOOT/DHCPv4 exchange.
Signed-off-by: Sean Edmond
Reviewed-by: Tom Rini
---
(no changes since v5)
Changes in v5:
- fix depends for BOOTP_RANDOM_XID:
"depends on CMD_BOOTP && (LIB_RAND || LIB_HW_RAND)"
Changes in v3:
- Add depends for BOOTP_RANDOM_XID
cmd/Kconfig | 7 +++
net/bootp.c | 31 +--
2 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 54c4ab8570c..82f6a089586 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1909,6 +1909,13 @@ config BOOTP_VCI_STRING
default "U-Boot.arm" if ARM
default "U-Boot"
+config BOOTP_RANDOM_XID
+ bool "Send random transaction ID to BOOTP/DHCP server"
+ depends on CMD_BOOTP && (LIB_RAND || LIB_HW_RAND)
+ help
+ Selecting this will allow for a random transaction ID to get
+ selected for each BOOTP/DHCPv4 exchange.
+
if CMD_DHCP6
config DHCP6_PXE_CLIENTARCH
diff --git a/net/bootp.c b/net/bootp.c
index c34ffe27854..285fcba7348 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -834,22 +834,25 @@ void bootp_request(void)
/* Only generate a new transaction ID for each new BOOTP request */
if (bootp_try == 1) {
- /*
-* Bootp ID is the lower 4 bytes of our ethernet address
-* plus the current time in ms.
-*/
- bootp_id = ((u32)net_ethaddr[2] << 24)
- | ((u32)net_ethaddr[3] << 16)
- | ((u32)net_ethaddr[4] << 8)
- | (u32)net_ethaddr[5];
- bootp_id += get_timer(0);
- bootp_id = htonl(bootp_id);
- bootp_add_id(bootp_id);
- net_copy_u32(>bp_id, _id);
- } else {
- net_copy_u32(>bp_id, _id);
+ if (IS_ENABLED(CONFIG_BOOTP_RANDOM_XID)) {
+ srand(get_ticks() + rand());
+ bootp_id = rand();
+ } else {
+ /*
+* Bootp ID is the lower 4 bytes of our ethernet
address
+* plus the current time in ms.
+*/
+ bootp_id = ((u32)net_ethaddr[2] << 24)
+ | ((u32)net_ethaddr[3] << 16)
+ | ((u32)net_ethaddr[4] << 8)
+ | (u32)net_ethaddr[5];
+ bootp_id += get_timer(0);
+ bootp_id = htonl(bootp_id);
+ }
}
+ bootp_add_id(bootp_id);
+ net_copy_u32(>bp_id, _id);
/*
* Calculate proper packet lengths taking into account the
* variable size of the options field
--
2.42.0
[PATCH v8 2/3] net: bootp: BOOTP/DHCPv4 retransmission improvements
This patch introduces 3 improvements to align with RFC 951:
- retransmission backoff interval maximum is configurable
- initial retranmission backoff interval is configurable
- transaction ID is kept the same for each BOOTP/DHCPv4 request
In applications where thousands of nodes are serviced by a single DHCP
server, maximizing the retransmission backoff interval at 2 seconds (the
current u-boot default) exerts high pressure on the DHCP server and
network layer.
RFC 951 “7.2. Client Retransmission Strategy” states that the
retransmission backoff interval should be limited to 60 seconds. This
patch allows the interval to be configurable using the environment
variable "bootpretransmitperiodmax"
The initial retranmission backoff period defaults to 250ms, which is
also too small for these scenarios with many clients. This patch makes
the initial retransmission interval to be configurable using the
environment variable "bootpretransmitperiodinit".
Also, on a retransmission it is not expected for the transaction ID to
change (only the 'secs' field should be updated). Let's save the
transaction ID and use the same transaction ID for each BOOTP/DHCPv4
exchange.
Signed-off-by: Sean Edmond
---
Changes in v8:
- remove unused variable "char *ep" from bootp_reset()
Changes in v7:
- Remove "depends on LIB_RAND || LIB_HW_RAND" from CMD_BOOTP
- Add CMD_BOOTP/CMD_DHCP/CMD_DHCP6 to LIB_RAND menu choice
- Add space after "config CMD_DHCP"
Changes in v6:
- CMD_BOOTP should "depends on LIB_RAND || LIB_HW_RAND"
Changes in v4:
- Add "select LIB_RAND" for "config CMD_BOOTP" (retransmission
improvements require rand())
Changes in v3:
- Set RETRANSMIT_PERIOD_MAX_MS=6
- Add randomization factor to retransmission timeout
Changes in v2:
- use env_get_ulong() to get environment variables
lib/Kconfig | 3 ++-
net/bootp.c | 68 -
2 files changed, 54 insertions(+), 17 deletions(-)
diff --git a/lib/Kconfig b/lib/Kconfig
index 189e6eb31aa..9b62b3bbea7 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -265,7 +265,8 @@ config REGEX
choice
prompt "Pseudo-random library support type"
depends on NET_RANDOM_ETHADDR || RANDOM_UUID || CMD_UUID || \
- RNG_SANDBOX || UT_LIB && AES || FAT_WRITE
+ RNG_SANDBOX || UT_LIB && AES || FAT_WRITE || CMD_BOOTP || \
+ CMD_DHCP || CMD_DHCP6
default LIB_RAND
help
Select the library to provide pseudo-random number generator
diff --git a/net/bootp.c b/net/bootp.c
index b9e3cccb4f9..c34ffe27854 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -42,6 +42,22 @@
*/
#define TIMEOUT_MS ((3 + (CONFIG_NET_RETRY_COUNT * 5)) * 1000)
+/*
+ * According to rfc951 : 7.2. Client Retransmission Strategy
+ * "After the 'average' backoff reaches about 60 seconds, it should be
+ * increased no further, but still randomized."
+ *
+ * U-Boot has saturated this backoff at 2 seconds for a long time.
+ * To modify, set the environment variable "bootpretransmitperiodmax"
+ */
+#define RETRANSMIT_PERIOD_MAX_MS 6
+
+/* Retransmission timeout for the initial packet (in milliseconds).
+ * This timeout will double on each retry. To modify, set the
+ * environment variable bootpretransmitperiodinit.
+ */
+#define RETRANSMIT_PERIOD_INIT_MS 250
+
#ifndef CFG_DHCP_MIN_EXT_LEN /* minimal length of extension list */
#define CFG_DHCP_MIN_EXT_LEN 64
#endif
@@ -53,6 +69,7 @@
u32bootp_ids[CFG_BOOTP_ID_CACHE_SIZE];
unsigned int bootp_num_ids;
intbootp_try;
+u32bootp_id;
ulong bootp_start;
ulong bootp_timeout;
char net_nis_domain[32] = {0,}; /* Our NIS domain */
@@ -60,6 +77,7 @@ char net_hostname[32] = {0,}; /* Our hostname */
char net_root_path[CONFIG_BOOTP_MAX_ROOT_PATH_LEN] = {0,}; /* Our bootpath */
static ulong time_taken_max;
+static u32 retransmit_period_max_ms;
#if defined(CONFIG_CMD_DHCP)
static dhcp_state_t dhcp_state = INIT;
@@ -396,6 +414,7 @@ static void bootp_handler(uchar *pkt, unsigned dest, struct
in_addr sip,
static void bootp_timeout_handler(void)
{
ulong time_taken = get_timer(bootp_start);
+ int rand_minus_plus_100;
if (time_taken >= time_taken_max) {
#ifdef CONFIG_BOOTP_MAY_FAIL
@@ -414,8 +433,17 @@ static void bootp_timeout_handler(void)
}
} else {
bootp_timeout *= 2;
- if (bootp_timeout > 2000)
- bootp_timeout = 2000;
+ if (bootp_timeout > retransmit_period_max_ms)
+ bootp_timeout = retransmit_period_max_ms;
+
+ /* Randomize by adding bootp_timeout*RAND, where RAND
+* is a randomization factor between -0.1..+0.1
+*/
+ srand(get_ticks() + rand());
+ rand_minus_plus_100 = ((rand() % 200) - 100);
+ bootp_timeout = bootp_timeout +
+
[PATCH v8 1/3] net: Enhancements for dhcp option 209
- Enable option 209 by default
- Set pxelinux_configfile to NULL to avoid potential double free
- change hardcoded 209 to a define
Signed-off-by: Sean Edmond
---
(no changes since v7)
Changes in v7:
- Reword this commit
Changes in v6:
- Reword this commit
Changes in v4:
- rebase master and resolve conflicts
- default y for BOOTP_PXE_DHCP_OPTION (feedback from review)
- change commit description for this patch (this is now
an enhancement patch)
Changes in v3:
- add define for option 209 and rfc5071 reference
cmd/Kconfig | 1 +
cmd/pxe.c | 2 ++
net/bootp.c | 4 ++--
net/bootp.h | 2 ++
4 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index c182d73ddbd..54c4ab8570c 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1898,6 +1898,7 @@ config BOOTP_PXE_CLIENTARCH
config BOOTP_PXE_DHCP_OPTION
bool "Request & store 'pxe_configfile' from BOOTP/DHCP server"
+ default y
depends on BOOTP_PXE
config BOOTP_VCI_STRING
diff --git a/cmd/pxe.c b/cmd/pxe.c
index 21134eb7a30..9404f445187 100644
--- a/cmd/pxe.c
+++ b/cmd/pxe.c
@@ -65,6 +65,8 @@ static int pxe_dhcp_option_path(struct pxe_context *ctx,
unsigned long pxefile_a
int ret = get_pxe_file(ctx, pxelinux_configfile, pxefile_addr_r);
free(pxelinux_configfile);
+ /* set to NULL to avoid double-free if DHCP is tried again */
+ pxelinux_configfile = NULL;
return ret;
}
diff --git a/net/bootp.c b/net/bootp.c
index c15472f5d37..b9e3cccb4f9 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -603,7 +603,7 @@ static int dhcp_extended(u8 *e, int message_type, struct
in_addr server_ip,
*cnt += 1;
#endif
if (IS_ENABLED(CONFIG_BOOTP_PXE_DHCP_OPTION)) {
- *e++ = 209; /* PXELINUX Config File */
+ *e++ = DHCP_OPTION_PXE_CONFIG_FILE; /* PXELINUX Config File
*/
*cnt += 1;
}
/* no options, so back up to avoid sending an empty request list */
@@ -922,7 +922,7 @@ static void dhcp_process_options(uchar *popt, uchar *end)
net_boot_file_name[size] = 0;
}
break;
- case 209: /* PXELINUX Config File */
+ case DHCP_OPTION_PXE_CONFIG_FILE: /* PXELINUX Config File
*/
if (IS_ENABLED(CONFIG_BOOTP_PXE_DHCP_OPTION)) {
/* In case it has already been allocated when
get DHCP Offer packet,
* free first to avoid memory leak.
diff --git a/net/bootp.h b/net/bootp.h
index 4e32b19d424..24b32c73f62 100644
--- a/net/bootp.h
+++ b/net/bootp.h
@@ -91,6 +91,8 @@ typedef enum { INIT,
#define DHCP_NAK 6
#define DHCP_RELEASE 7
+#define DHCP_OPTION_PXE_CONFIG_FILE209 /* "ConfigFile" option
according to rfc5071 */
+
/**/
#endif /* __BOOTP_H__ */
--
2.42.0
[PATCH v8 0/3] BOOTP/DHCPv4 enhancements
In our datacenter application, a single DHCP server is servicing 36000+ clients. Improvements are required to the DHCPv4 retransmission behavior to align with RFC and ensure less pressure is exerted on the server: - retransmission backoff interval maximum is configurable (environment variable bootpretransmitperiodmax) - initial retransmission backoff interval is configurable (environment variable bootpretransmitperiodinit) - transaction ID is kept the same for each BOOTP/DHCPv4 request (not recreated on each retry) For our application we'll use: - bootpretransmitperiodmax=16000 - bootpretransmitperiodinit=2000 A new configuration BOOTP_RANDOM_XID has been added to enable a randomized BOOTP/DHCPv4 transaction ID. Enhance DHCPv4 sending/parsing option 209 (PXE config file). A previous patch was accepted. A new patch fixes a possible double free() and addresses latest review comments. Changes in v8: - remove unused variable "char *ep" from bootp_reset() Changes in v7: - Remove "depends on LIB_RAND || LIB_HW_RAND" from CMD_BOOTP - Add CMD_BOOTP/CMD_DHCP/CMD_DHCP6 to LIB_RAND menu choice - Add space after "config CMD_DHCP" Changes in v6: - CMD_BOOTP should "depends on LIB_RAND || LIB_HW_RAND" Changes in v5: - add change log to individual patches - fix depends for BOOTP_RANDOM_XID: "depends on CMD_BOOTP && (LIB_RAND || LIB_HW_RAND)" Changes in v4: - rebase master and resolve conflicts - default y for BOOTP_PXE_DHCP_OPTION (feedback from review) - Add "select LIB_RAND" for "config CMD_BOOTP" (retransmission improvements require rand()) Changes in v3: - add define for option 209 and rfc5071 reference - Set RETRANSMIT_PERIOD_MAX_MS=6 - Add randomization factor to retransmission timeout - Add depends for BOOTP_RANDOM_XID Changes in v2: - use env_get_ulong() to get environment variables Sean Edmond (3): net: Enhancements for dhcp option 209 net: bootp: BOOTP/DHCPv4 retransmission improvements net: bootp: add config option BOOTP_RANDOM_XID cmd/Kconfig | 8 ++ cmd/pxe.c | 2 ++ lib/Kconfig | 3 ++- net/bootp.c | 73 - net/bootp.h | 2 ++ 5 files changed, 70 insertions(+), 18 deletions(-) -- 2.42.0
[PATCH v2 1/1] andes: Unify naming policy for Andes related source
Signed-off-by: Leo Yu-Chi Liang
---
arch/riscv/Kconfig| 2 +-
arch/riscv/cpu/{andesv5 => andes}/Kconfig | 4 +-
arch/riscv/cpu/{andesv5 => andes}/Makefile| 0
arch/riscv/cpu/{andesv5 => andes}/cache.c | 12 +++---
arch/riscv/cpu/{andesv5 => andes}/cpu.c | 0
arch/riscv/cpu/{andesv5 => andes}/spl.c | 0
board/{AndesTech => andestech}/ae350/Kconfig | 4 +-
.../ae350/MAINTAINERS | 0
board/{AndesTech => andestech}/ae350/Makefile | 0
board/{AndesTech => andestech}/ae350/ae350.c | 2 +-
drivers/cache/Kconfig | 6 +--
drivers/cache/Makefile| 2 +-
.../cache/{cache-v5l2.c => cache-andes-l2.c} | 40 +--
13 files changed, 36 insertions(+), 36 deletions(-)
rename arch/riscv/cpu/{andesv5 => andes}/Kconfig (91%)
rename arch/riscv/cpu/{andesv5 => andes}/Makefile (100%)
rename arch/riscv/cpu/{andesv5 => andes}/cache.c (90%)
rename arch/riscv/cpu/{andesv5 => andes}/cpu.c (100%)
rename arch/riscv/cpu/{andesv5 => andes}/spl.c (100%)
rename board/{AndesTech => andestech}/ae350/Kconfig (94%)
rename board/{AndesTech => andestech}/ae350/MAINTAINERS (100%)
rename board/{AndesTech => andestech}/ae350/Makefile (100%)
rename board/{AndesTech => andestech}/ae350/ae350.c (99%)
rename drivers/cache/{cache-v5l2.c => cache-andes-l2.c} (84%)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 7e20ef63bb..120ee1a01c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -93,7 +93,7 @@ source "board/thead/th1520_lpi4a/Kconfig"
source "board/xilinx/mbv/Kconfig"
# platform-specific options below
-source "arch/riscv/cpu/andesv5/Kconfig"
+source "arch/riscv/cpu/andes/Kconfig"
source "arch/riscv/cpu/cv1800b/Kconfig"
source "arch/riscv/cpu/fu540/Kconfig"
source "arch/riscv/cpu/fu740/Kconfig"
diff --git a/arch/riscv/cpu/andesv5/Kconfig b/arch/riscv/cpu/andes/Kconfig
similarity index 91%
rename from arch/riscv/cpu/andesv5/Kconfig
rename to arch/riscv/cpu/andes/Kconfig
index e3efb0de8f..120fec5e54 100644
--- a/arch/riscv/cpu/andesv5/Kconfig
+++ b/arch/riscv/cpu/andes/Kconfig
@@ -1,4 +1,4 @@
-config RISCV_NDS
+config RISCV_ANDES
bool
select ARCH_EARLY_INIT_R
select SYS_CACHE_SHIFT_6
@@ -8,7 +8,7 @@ config RISCV_NDS
imply ANDES_PLMT_TIMER
imply SPL_ANDES_PLMT_TIMER
imply ANDES_PLICSW if (RISCV_MMODE || SPL_RISCV_MMODE)
- imply V5L2_CACHE
+ imply ANDES_L2_CACHE
imply SPL_CPU
imply SPL_OPENSBI
imply SPL_LOAD_FIT
diff --git a/arch/riscv/cpu/andesv5/Makefile b/arch/riscv/cpu/andes/Makefile
similarity index 100%
rename from arch/riscv/cpu/andesv5/Makefile
rename to arch/riscv/cpu/andes/Makefile
diff --git a/arch/riscv/cpu/andesv5/cache.c b/arch/riscv/cpu/andes/cache.c
similarity index 90%
rename from arch/riscv/cpu/andesv5/cache.c
rename to arch/riscv/cpu/andes/cache.c
index 269bb27f75..7d3df8722d 100644
--- a/arch/riscv/cpu/andesv5/cache.c
+++ b/arch/riscv/cpu/andes/cache.c
@@ -12,21 +12,21 @@
#include
#include
-#ifdef CONFIG_V5L2_CACHE
+#ifdef CONFIG_ANDES_L2_CACHE
void enable_caches(void)
{
struct udevice *dev;
int ret;
ret = uclass_get_device_by_driver(UCLASS_CACHE,
- DM_DRIVER_GET(v5l2_cache),
+ DM_DRIVER_GET(andes_l2_cache),
);
if (ret) {
- log_debug("Cannot enable v5l2 cache\n");
+ log_debug("Cannot enable Andes L2 cache\n");
} else {
ret = cache_enable(dev);
if (ret)
- log_debug("v5l2 cache enable failed\n");
+ log_debug("Failed to enable Andes L2 cache\n");
}
}
@@ -78,7 +78,7 @@ void dcache_enable(void)
asm volatile("csrsi %0, 0x2" :: "i"(CSR_MCACHE_CTL));
#endif
-#ifdef CONFIG_V5L2_CACHE
+#ifdef CONFIG_ANDES_L2_CACHE
cache_ops(cache_enable);
#endif
}
@@ -89,7 +89,7 @@ void dcache_disable(void)
asm volatile("csrci %0, 0x2" :: "i"(CSR_MCACHE_CTL));
#endif
-#ifdef CONFIG_V5L2_CACHE
+#ifdef CONFIG_ANDES_L2_CACHE
cache_ops(cache_disable);
#endif
}
diff --git a/arch/riscv/cpu/andesv5/cpu.c b/arch/riscv/cpu/andes/cpu.c
similarity index 100%
rename from arch/riscv/cpu/andesv5/cpu.c
rename to arch/riscv/cpu/andes/cpu.c
diff --git a/arch/riscv/cpu/andesv5/spl.c b/arch/riscv/cpu/andes/spl.c
similarity index 100%
rename from arch/riscv/cpu/andesv5/spl.c
rename to arch/riscv/cpu/andes/spl.c
diff --git a/board/AndesTech/ae350/Kconfig b/board/andestech/ae350/Kconfig
similarity index 94%
rename from board/AndesTech/ae350/Kconfig
rename to board/andestech/ae350/Kconfig
index a85e7d6351..1c95e8447f 100644
--- a/board/AndesTech/ae350/Kconfig
+++ b/board/andestech/ae350/Kconfig
@@ -1,7 +1,7 @@
if TARGET_ANDES_AE350
config SYS_CPU
- default "andesv5"
+ default
[PATCH 1/1] andes: Unify naming policy for Andes related source
Signed-off-by: Leo Yu-Chi Liang
---
arch/riscv/Kconfig| 2 +-
arch/riscv/cpu/{andesv5 => andes}/Kconfig | 4 +-
arch/riscv/cpu/{andesv5 => andes}/Makefile| 0
arch/riscv/cpu/{andesv5 => andes}/cache.c | 12 +++---
arch/riscv/cpu/{andesv5 => andes}/cpu.c | 0
arch/riscv/cpu/{andesv5 => andes}/spl.c | 0
board/{AndesTech => andestech}/ae350/Kconfig | 4 +-
.../ae350/MAINTAINERS | 0
board/{AndesTech => andestech}/ae350/Makefile | 0
board/{AndesTech => andestech}/ae350/ae350.c | 2 +-
drivers/cache/Kconfig | 6 +--
drivers/cache/Makefile| 2 +-
.../cache/{cache-v5l2.c => cache-andes-l2.c} | 40 +--
13 files changed, 36 insertions(+), 36 deletions(-)
rename arch/riscv/cpu/{andesv5 => andes}/Kconfig (91%)
rename arch/riscv/cpu/{andesv5 => andes}/Makefile (100%)
rename arch/riscv/cpu/{andesv5 => andes}/cache.c (90%)
rename arch/riscv/cpu/{andesv5 => andes}/cpu.c (100%)
rename arch/riscv/cpu/{andesv5 => andes}/spl.c (100%)
rename board/{AndesTech => andestech}/ae350/Kconfig (94%)
rename board/{AndesTech => andestech}/ae350/MAINTAINERS (100%)
rename board/{AndesTech => andestech}/ae350/Makefile (100%)
rename board/{AndesTech => andestech}/ae350/ae350.c (99%)
rename drivers/cache/{cache-v5l2.c => cache-andes-l2.c} (84%)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 7e20ef63bb..120ee1a01c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -93,7 +93,7 @@ source "board/thead/th1520_lpi4a/Kconfig"
source "board/xilinx/mbv/Kconfig"
# platform-specific options below
-source "arch/riscv/cpu/andesv5/Kconfig"
+source "arch/riscv/cpu/andes/Kconfig"
source "arch/riscv/cpu/cv1800b/Kconfig"
source "arch/riscv/cpu/fu540/Kconfig"
source "arch/riscv/cpu/fu740/Kconfig"
diff --git a/arch/riscv/cpu/andesv5/Kconfig b/arch/riscv/cpu/andes/Kconfig
similarity index 91%
rename from arch/riscv/cpu/andesv5/Kconfig
rename to arch/riscv/cpu/andes/Kconfig
index e3efb0de8f..120fec5e54 100644
--- a/arch/riscv/cpu/andesv5/Kconfig
+++ b/arch/riscv/cpu/andes/Kconfig
@@ -1,4 +1,4 @@
-config RISCV_NDS
+config RISCV_ANDES
bool
select ARCH_EARLY_INIT_R
select SYS_CACHE_SHIFT_6
@@ -8,7 +8,7 @@ config RISCV_NDS
imply ANDES_PLMT_TIMER
imply SPL_ANDES_PLMT_TIMER
imply ANDES_PLICSW if (RISCV_MMODE || SPL_RISCV_MMODE)
- imply V5L2_CACHE
+ imply ANDES_L2_CACHE
imply SPL_CPU
imply SPL_OPENSBI
imply SPL_LOAD_FIT
diff --git a/arch/riscv/cpu/andesv5/Makefile b/arch/riscv/cpu/andes/Makefile
similarity index 100%
rename from arch/riscv/cpu/andesv5/Makefile
rename to arch/riscv/cpu/andes/Makefile
diff --git a/arch/riscv/cpu/andesv5/cache.c b/arch/riscv/cpu/andes/cache.c
similarity index 90%
rename from arch/riscv/cpu/andesv5/cache.c
rename to arch/riscv/cpu/andes/cache.c
index 269bb27f75..801857ab8f 100644
--- a/arch/riscv/cpu/andesv5/cache.c
+++ b/arch/riscv/cpu/andes/cache.c
@@ -12,21 +12,21 @@
#include
#include
-#ifdef CONFIG_V5L2_CACHE
+#ifdef CONFIG_ANDES_L2_CACHE
void enable_caches(void)
{
struct udevice *dev;
int ret;
ret = uclass_get_device_by_driver(UCLASS_CACHE,
- DM_DRIVER_GET(v5l2_cache),
+ DM_DRIVER_GET(andes_l2_cache),
);
if (ret) {
- log_debug("Cannot enable v5l2 cache\n");
+ log_debug("Cannot enable andes-l2 cache\n");
} else {
ret = cache_enable(dev);
if (ret)
- log_debug("v5l2 cache enable failed\n");
+ log_debug("andes-l2 cache enable failed\n");
}
}
@@ -78,7 +78,7 @@ void dcache_enable(void)
asm volatile("csrsi %0, 0x2" :: "i"(CSR_MCACHE_CTL));
#endif
-#ifdef CONFIG_V5L2_CACHE
+#ifdef CONFIG_ANDES_L2_CACHE
cache_ops(cache_enable);
#endif
}
@@ -89,7 +89,7 @@ void dcache_disable(void)
asm volatile("csrci %0, 0x2" :: "i"(CSR_MCACHE_CTL));
#endif
-#ifdef CONFIG_V5L2_CACHE
+#ifdef CONFIG_ANDES_L2_CACHE
cache_ops(cache_disable);
#endif
}
diff --git a/arch/riscv/cpu/andesv5/cpu.c b/arch/riscv/cpu/andes/cpu.c
similarity index 100%
rename from arch/riscv/cpu/andesv5/cpu.c
rename to arch/riscv/cpu/andes/cpu.c
diff --git a/arch/riscv/cpu/andesv5/spl.c b/arch/riscv/cpu/andes/spl.c
similarity index 100%
rename from arch/riscv/cpu/andesv5/spl.c
rename to arch/riscv/cpu/andes/spl.c
diff --git a/board/AndesTech/ae350/Kconfig b/board/andestech/ae350/Kconfig
similarity index 94%
rename from board/AndesTech/ae350/Kconfig
rename to board/andestech/ae350/Kconfig
index a85e7d6351..1c95e8447f 100644
--- a/board/AndesTech/ae350/Kconfig
+++ b/board/andestech/ae350/Kconfig
@@ -1,7 +1,7 @@
if TARGET_ANDES_AE350
config SYS_CPU
- default "andesv5"
+ default
[PATCH 3/3] cyclic: make clients embed a struct cyclic_info in their own data structure
There are of course not a whole lot of examples in-tree yet, but
before they appear, let's make this API change: Instead of separately
allocating a 'struct cyclic_info', make the users embed such an
instance in their own structure, and make the convention that the
callback simply receives the 'struct cyclic_info *', from which the
clients can get their own data using the container_of() macro.
This has a number of advantages.
First, it means cyclic_register() simply cannot fail, simplifying the
code. The necessary storage will simply be allocated automatically
when the client's own structure is allocated (often via
uclass_priv_auto or similar).
Second, code for which CONFIG_CYCLIC is just an option can more easily
be written without #ifdefs, if we just provide an empty struct
cyclic_info {}. For example, the nested CONFIG_IS_ENABLED()s in
https://lore.kernel.org/u-boot/20240316201416.211480-1-marek.vasut+rene...@mailbox.org/
are mostly due to the existence of the 'struct cyclic_info *' member
being guarded by #ifdef CONFIG_CYCLIC.
And we do probably want to avoid the extra memory overhead of that
member when !CONFIG_CYCLIC. But that is automatic if, instead of a
'struct cyclic_info *', one simply embeds a 'struct cyclic_info',
which will have size 0 when !CONFIG_CYCLIC. Also, the no-op
cyclic_register() function can just unconditionally be called, and the
compiler will see that (1) the callback is referenced, so not emit a
warning for a maybe-unused function and (2) see that it can actually
never be reached, so not emit any code for it.
Signed-off-by: Rasmus Villemoes
---
board/Marvell/octeon_nic23/board.c | 9 +---
cmd/cyclic.c | 12 +--
common/cyclic.c| 22 +--
doc/develop/cyclic.rst | 26 ++-
drivers/watchdog/wdt-uclass.c | 33 +
include/cyclic.h | 34 +++---
6 files changed, 64 insertions(+), 72 deletions(-)
diff --git a/board/Marvell/octeon_nic23/board.c
b/board/Marvell/octeon_nic23/board.c
index bc9332cb74a..74b9c741b7b 100644
--- a/board/Marvell/octeon_nic23/board.c
+++ b/board/Marvell/octeon_nic23/board.c
@@ -357,10 +357,13 @@ int board_late_init(void)
board_configure_qlms();
/* Register cyclic function for PCIe FLR fixup */
- cyclic = cyclic_register(octeon_board_restore_pf, 100,
-"pcie_flr_fix", NULL);
- if (!cyclic)
+ cyclic = calloc(1, sizeof(*cyclic));
+ if (cyclic) {
+ cyclic_register(cyclic, octeon_board_restore_pf, 100,
+ "pcie_flr_fix");
+ } else {
printf("Registering of cyclic function failed\n");
+ }
return 0;
}
diff --git a/cmd/cyclic.c b/cmd/cyclic.c
index ad7fc3b975e..315546515f0 100644
--- a/cmd/cyclic.c
+++ b/cmd/cyclic.c
@@ -14,14 +14,16 @@
#include
#include
#include
+#include
struct cyclic_demo_info {
+ struct cyclic_info cyclic;
uint delay_us;
};
-static void cyclic_demo(void *ctx)
+static void cyclic_demo(struct cyclic_info *c)
{
- struct cyclic_demo_info *info = ctx;
+ struct cyclic_demo_info *info = container_of(c, struct
cyclic_demo_info, cyclic);
/* Just a small dummy delay here */
udelay(info->delay_us);
@@ -31,7 +33,6 @@ static int do_cyclic_demo(struct cmd_tbl *cmdtp, int flag,
int argc,
char *const argv[])
{
struct cyclic_demo_info *info;
- struct cyclic_info *cyclic;
uint time_ms;
if (argc < 3)
@@ -47,10 +48,7 @@ static int do_cyclic_demo(struct cmd_tbl *cmdtp, int flag,
int argc,
info->delay_us = simple_strtoul(argv[2], NULL, 0);
/* Register demo cyclic function */
- cyclic = cyclic_register(cyclic_demo, time_ms * 1000, "cyclic_demo",
-info);
- if (!cyclic)
- printf("Registering of cyclic_demo failed\n");
+ cyclic_register(>cyclic, cyclic_demo, time_ms * 1000,
"cyclic_demo");
printf("Registered function \"%s\" to be executed all %dms\n",
"cyclic_demo", time_ms);
diff --git a/common/cyclic.c b/common/cyclic.c
index c62e7fa7d19..ec38fad6775 100644
--- a/common/cyclic.c
+++ b/common/cyclic.c
@@ -26,34 +26,22 @@ struct hlist_head *cyclic_get_list(void)
return (struct hlist_head *)>cyclic_list;
}
-struct cyclic_info *cyclic_register(cyclic_func_t func, uint64_t delay_us,
- const char *name, void *ctx)
+void cyclic_register(struct cyclic_info *cyclic, cyclic_func_t func,
+uint64_t delay_us, const char *name)
{
- struct cyclic_info *cyclic;
-
- cyclic = calloc(1, sizeof(struct cyclic_info));
- if (!cyclic) {
- pr_debug("Memory allocation error\n");
- return NULL;
- }
+ memset(cyclic, 0,
[PATCH 0/3] cyclic/watchdog patches
A bit of a mixed bag. I've been wanting to submit something like 3/3 for a while. So when I stumbled on Marek's patch https://lore.kernel.org/u-boot/20240316201416.211480-1-marek.vasut+rene...@mailbox.org/ , I got reminded of that plan, and I think that patch could be more readable if we adopt this model. While actually doing those mostly mechanical changes, I stumbled on two separate issues that probably want fixing regardless of the fate of 3/3. For now only compile-tested. Rasmus Villemoes (3): cyclic: stop strdup'ing name in cyclic_register() wdt-uclass: prevent multiple cyclic_register calls cyclic: make clients embed a struct cyclic_info in their own data structure board/Marvell/octeon_nic23/board.c | 9 --- cmd/cyclic.c | 12 -- common/cyclic.c| 24 +-- doc/develop/cyclic.rst | 26 drivers/watchdog/wdt-uclass.c | 38 -- include/cyclic.h | 36 ++-- 6 files changed, 71 insertions(+), 74 deletions(-) -- 2.40.1.1.g1c60b9335d
[PATCH 1/3] cyclic: stop strdup'ing name in cyclic_register()
We are not checking the return value of strdup(), nor
freeing the string in cyclic_unregister().
However, all current users either pass a string literal or the
dev->name of the client device. So in all cases the name string will
live at least as long as the cyclic_info is registered, so just make
that a requirement.
Signed-off-by: Rasmus Villemoes
---
common/cyclic.c | 2 +-
include/cyclic.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/cyclic.c b/common/cyclic.c
index a49bfc88f5c..c62e7fa7d19 100644
--- a/common/cyclic.c
+++ b/common/cyclic.c
@@ -40,7 +40,7 @@ struct cyclic_info *cyclic_register(cyclic_func_t func,
uint64_t delay_us,
/* Store values in struct */
cyclic->func = func;
cyclic->ctx = ctx;
- cyclic->name = strdup(name);
+ cyclic->name = name;
cyclic->delay_us = delay_us;
cyclic->start_time_us = timer_get_us();
hlist_add_head(>list, cyclic_get_list());
diff --git a/include/cyclic.h b/include/cyclic.h
index 44ad3cb6b80..38946216fb8 100644
--- a/include/cyclic.h
+++ b/include/cyclic.h
@@ -31,7 +31,7 @@
struct cyclic_info {
void (*func)(void *ctx);
void *ctx;
- char *name;
+ const char *name;
uint64_t delay_us;
uint64_t start_time_us;
uint64_t cpu_time_us;
--
2.40.1.1.g1c60b9335d
[PATCH 2/3] wdt-uclass: prevent multiple cyclic_register calls
Currently, the cyclic_register() done in wdt_start() is not undone in
wdt_stop(). Moreover, calling wdt_start multiple times (which is
perfectly allowed on an already started device, e.g. to change the
timeout value) will result in another struct cyclic_info being
registered, referring to the same watchdog device.
This can easily be seen on e.g. a wandboard:
=> cyclic list
function: watchdog@20bc000, cpu-time: 22 us, frequency: 1.01 times/s
=> wdt list
watchdog@20bc000 (imx_wdt)
=> wdt dev watchdog@20bc000
=> wdt start 5
WDT: Started watchdog@20bc000 with servicing every 1000ms (50s timeout)
=> cyclic list
function: watchdog@20bc000, cpu-time: 37 us, frequency: 1.03 times/s
function: watchdog@20bc000, cpu-time: 241 us, frequency: 1.01 times/s
=> wdt start 12345
WDT: Started watchdog@20bc000 with servicing every 1000ms (12s timeout)
=> cyclic list
function: watchdog@20bc000, cpu-time: 36 us, frequency: 1.03 times/s
function: watchdog@20bc000, cpu-time: 100 us, frequency: 1.04 times/s
function: watchdog@20bc000, cpu-time: 299 us, frequency: 1.00 times/s
So properly unregister the watchdog device from the cyclic framework
in wdt_stop(). In wdt_start(), we cannot just skip the registration,
as the (new) timeout value may mean that we have to ask the cyclic
framework to call us more often. So if we're already running,
first unregister the old cyclic instance.
Signed-off-by: Rasmus Villemoes
---
drivers/watchdog/wdt-uclass.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 417e8d7eef9..caa1567e0c3 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -122,10 +122,11 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong
flags)
struct wdt_priv *priv = dev_get_uclass_priv(dev);
char str[16];
- priv->running = true;
-
memset(str, 0, 16);
if (IS_ENABLED(CONFIG_WATCHDOG)) {
+ if (priv->running)
+ cyclic_unregister(priv->cyclic);
+
/* Register the watchdog driver as a cyclic function */
priv->cyclic = cyclic_register(wdt_cyclic,
priv->reset_period *
1000,
@@ -140,6 +141,7 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong
flags)
}
}
+ priv->running = true;
printf("WDT: Started %s with%s servicing %s (%ds timeout)\n",
dev->name, IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out",
str, (u32)lldiv(timeout_ms, 1000));
@@ -160,6 +162,9 @@ int wdt_stop(struct udevice *dev)
if (ret == 0) {
struct wdt_priv *priv = dev_get_uclass_priv(dev);
+ if (IS_ENABLED(CONFIG_WATCHDOG) && priv->running)
+ cyclic_unregister(priv->cyclic);
+
priv->running = false;
}
--
2.40.1.1.g1c60b9335d
Re: [PATCH v1 1/1] net: Add drivers for Sysnopsys Ethernet 10G device
On Wed, Apr 17, 2024 at 03:46:56PM +0800, Boon Khai Ng wrote: > This driver support the Synopsys Designware Ethernet 10G > IP block refer from the driver dwc_eth_qos. > > The driver MAC register mapping is different between > Synopsys QoS IP and Synopsys 10G IP, and thus new file > is created meant for Sysnopsys 10G IP. > > The dwc_eth_xgmac_socfpga.c is specific to a device family, > the driver support the specific configuration used in > Intel SoC FPGA Agilex5. > > This driver is extensible for other device family to use. > > Signed-off-by: Boon Khai Ng Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] test/py: Make the number of SPL banners seen a variable
On Wed, 24 Apr 2024 16:45:37 -0600, Tom Rini wrote: > Currently we have the option to tell the console code that we should > ignore the SPL banner. We also have an option to say that we can see it > a second time, and ignore it. However, some platforms such as TI AM64x > will have us see the SPL banner three times. Rather than add an > "spl3_skipped" option, rework the code. By default we expect to see the > banner once, but boards can specify seeing it as many times as they > expect to. > > [...] Applied to u-boot/next, thanks! -- Tom
Re: [PATCH] board: starfive: support Pine64 Star64 board
On Pine64 Star64 8GB (May 2023 order date: "Star64 v1.1" silkscreen)
by contributor Tenkawa (contact info withheld so posting on their
behalf and listing myself in the tag). Other than the inconclusive
networking result (for discussion via code review) it seems to have
basic functionality.
# mac
Vendor : PINE64
Product full SN: STAR64V1-2310-D008E000-0005
data version: 0x2
PCB revision: 0xc1
BOM revision: A
Ethernet MAC0 address: 6c:cf:39:00:75:61
Ethernet MAC1 address: 6c:cf:39:00:75:62
# mmc info
Device: mmc@1601
Manufacturer ID: 15
OEM: 0
Name: BJTD4R
Bus Speed: 5200
Mode: MMC High Speed (52MHz)
Rd Block Len: 512
MMC version 5.1
High Capacity: Yes
Capacity: 29.1 GiB
Bus Width: 8-bit
Erase Group Size: 512 KiB
HC WP Group Size: 8 MiB
User Capacity: 29.1 GiB WRREL
Boot Capacity: 4 MiB ENH
RPMB Capacity: 4 MiB ENH
Boot area 0 is not write protected
Boot area 1 is not write protected
# mmc dev 1 ; mmc info
switch to partitions #0, OK
mmc1 is current device
Device: mmc@1602
Manufacturer ID: 1b
OEM: 534d
Name: GD2S5
Bus Speed: 5000
Mode: SD High Speed (50MHz)
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 119.4 GiB
Bus Width: 4-bit
Erase Group Size: 512 Bytes
# # Silkscreen: Upper=A Port
# env erase;env load;env set bootcmd 'dhcp;net list'; env save; reset
ethernet@1603 Waiting for PHY auto negotiation to
complete. TIMEOUT !
phy_startup() failed: -110
FAILED: -110
ethernet@1604 Waiting for PHY auto negotiation to complete... done
BOOTP broadcast 1
BOOTP broadcast 2
DHCP client bound to address...
...
eth0 : ethernet@1603 6c:cf:39:00:75:61 active
eth1 : ethernet@1604 6c:cf:39:00:75:62
# # Silkscreen: Lower=B Port
# env erase;env load;env set bootcmd 'dhcp;net list'; env save; reset
ethernet@1603 Waiting for PHY auto negotiation to
complete. TIMEOUT !
phy_startup() failed: -110
FAILED: -110
ethernet@1604 Waiting for PHY auto negotiation to complete... done
BOOTP broadcast 1
BOOTP broadcast 2
DHCP client bound to address...
...
eth0 : ethernet@1603 6c:cf:39:00:75:61
eth1 : ethernet@1604 6c:cf:39:00:75:62 active
Some dropped network packets were observed with the test setup:
Approx 6% packet loss on Upper=A Port
Approx 71% packet loss on Lower=B Port.
The test network was not known to be reliable and might have
introduced other sources of error. No comparison was available to any
other firmware releases.
Tested-by: E Shattow
On Mon, May 6, 2024 at 8:30 AM H Bell wrote:
>
> Similar to the Milk-V Mars, The Star64 board contains few differences to the
> VisionFive 2 boards, so can be part of the same U-boot build.
>
> Signed-off-by: Henry Bell
> Cc: ycli...@andestech.com
> Cc: heinrich.schucha...@canonical.com
> ---
> board/starfive/visionfive2/spl.c | 52 +++
> .../visionfive2/starfive_visionfive2.c| 4 ++
> 2 files changed, 56 insertions(+)
>
> diff --git a/board/starfive/visionfive2/spl.c
> b/board/starfive/visionfive2/spl.c
> index ca61b5be22..b8d29a02af 100644
> --- a/board/starfive/visionfive2/spl.c
> +++ b/board/starfive/visionfive2/spl.c
> @@ -226,6 +226,56 @@ void spl_fdt_fixup_version_b(void *fdt)
> }
> }
>
> +void spl_fdt_fixup_star64(void *fdt)
> +{
> + static const char compat[] = "starfive,star64\0starfive,jh7110";
> + u32 phandle;
> + u8 i;
> + int offset;
> + int ret;
> +
> + fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat,
> sizeof(compat));
> + fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model",
> + "Pine64 Star64");
> +
> + /* gmac0 */
> + offset = fdt_path_offset(fdt, "/soc/clock-controller@1700");
> + phandle = fdt_get_phandle(fdt, offset);
> + offset = fdt_path_offset(fdt, "/soc/ethernet@1603");
> +
> + fdt_setprop_u32(fdt, offset, "assigned-clocks", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clocks",
> JH7110_AONCLK_GMAC0_TX);
> + fdt_setprop_u32(fdt, offset, "assigned-clock-parents", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clock-parents",
> + JH7110_AONCLK_GMAC0_RMII_RTX);
> +
> + /* gmac1 */
> + offset = fdt_path_offset(fdt, "/soc/clock-controller@1302");
> + phandle = fdt_get_phandle(fdt, offset);
> + offset = fdt_path_offset(fdt, "/soc/ethernet@1604");
> +
> + fdt_setprop_u32(fdt, offset, "assigned-clocks", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clocks",
> JH7110_SYSCLK_GMAC1_TX);
> + fdt_setprop_u32(fdt, offset, "assigned-clock-parents", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clock-parents",
> + JH7110_SYSCLK_GMAC1_RMII_RTX);
> +
> + for (i = 0; i < ARRAY_SIZE(starfive_verb); i++) {
> + offset = fdt_path_offset(fdt, starfive_verb[i].path);
> +
> + if (starfive_verb[i].value)
> +
Re: [PATCH 3/5] zfs: Fix unaligned read of uint64
On Thursday, April 25th, 2024 at 4:57 AM, Caleb Connolly wrote: > > On 25/04/2024 06:02, mwle...@mailtundra.com wrote: > > > Hi Caleb, > > > > Thanks for this interesting feedback. I saw these patches were already > > merged > > since you sent this but I added a few thoughts below anyway. > > > Ah, a shame.. It would be good to find a scalable solution to this! While I agree your suggestion of doing two 4 byte reads is more scalable than the malloc() patch, the code path (zfs_nvlist_lookup_uint64) is only hit when reading the ZPool label (check_pool_label) which is a once-per-boot operation operating on some metadata, so I don't think scalability is as much a concern as it would be if we were hitting this code path when reading every block from the disk for example. That said, I may try implementing the patch as you suggested if I get a chance. And in any case I definitely appreciate your engagement and feedback! > > > On Friday, April 12th, 2024 at 11:50 AM, Caleb Connolly > > caleb.conno...@linaro.org wrote: > > > > > Hi Phaedrus, > > > > > > On 07/04/2024 03:47, mwle...@mailtundra.com wrote: > > > > > > > Without this patch, when trying to boot zfs using U-Boot on a Jetson TX2 > > > > NX (which is aarch64), I get a CPU reset error like so: > > > > > > > > "Synchronous Abort" handler, esr 0x9621 > > > > elr: 800c9000 lr : 800c8ffc (reloc) > > > > elr: fff77000 lr : fff76ffc > > > > x0 : ffb40f04 x1 : > > > > x2 : 000a x3 : 0310 > > > > x4 : 0310 x5 : 0034 > > > > x6 : fff9cc6e x7 : 000f > > > > x8 : ff7f84a0 x9 : 0008 > > > > x10: ffb40f04 x11: 0006 > > > > x12: 0001869f x13: 0001 > > > > x14: ff7f84bc x15: 0010 > > > > x16: 2080 x17: 001f > > > > x18: ff7fbdd8 x19: ffb405f8 > > > > x20: ffb40dd0 x21: fffabe5e > > > > x22: 00ea7794 x23: ffb42090 > > > > x24: x25: > > > > x26: x27: > > > > x28: 00bab10c x29: ff7f85f0 > > > > > > > > Code: d1a0 9103a000 94006ac6 f9401ba0 (f940) > > > > Resetting CPU ... > > > > > > > > This happens when be64_to_cpu() is called on a value that exists at a > > > > memory address that's 4 byte aligned but not 8 byte aligned (e.g. an > > > > address ending in 04). The call stack where that happens is: > > > > check_pool_label() -> > > > > zfs_nvlist_lookup_uint64(vdevnvlist, ZPOOL_CONFIG_ASHIFT,...) -> > > > > be64_to_cpu() > > > > > > This is very odd, aarch64 doesn't generally have these restrictions. I > > > got a bit nerdsniped when I saw this so I did some digging and figured > > > this: > > > > > > 1. Your abort exception doesn't include the FAR_ELx register (which > > > should contain the address that was being accessed when the abort > > > occured). This means your board is running in EL3. > > > 2. It turns out there is an "A" flag in the SCTLR_ELx register, when set > > > this flag causes a fault when trying to load from an address that isn't > > > aligned to the size of the data element (see "A, bit" on > > > https://developer.arm.com/documentation/ddi0595/2021-06/AArch64-Registers/SCTLR-EL3--System-Control-Register--EL3- > > > > > > I'm not sure who's in the "wrong" here, maybe the driver should avoid > > > unaligned accesses? But then again, I don't think you should be running > > > a ZFS driver in EL3. > > > > > > I'm not familiar with the Jetson Nano, but maybe there's a documented > > > way to run U-Boot so that it isn't executing in EL3? Or if not you could > > > also try unsetting the A flag. > > > > I may look into this if I get a chance. However if I write some assembly > > code > > to change the execution level or unset the A flag, I worry that the code > > would > > work fine on the hardware I have (Jetson TX2 NX) but behave differently on > > > Well, unsetting the A flag might arguably have some niche negative > outcomes, but I really struggle to see how this is a driver bug to be > honest... > > > another platform. And I don't think I can easily set up testing environments > > with u-boot + zfs on different platforms to find out. > > > fwiw, if you do intend to investigate this further, I'd be happy to > validate your assumptions on a Qualcomm board (where we are rather > boring and running in EL1 or EL2). Thanks, I'll keep that in mind > > > > If this really is something to fix in the driver, I don't think > > > hotpatching every unaligned access with a malloc() is the right solution. > > > > I'm certainly open to other ideas. The difficulty is the data structure > > we're > > parsing in this file is read from disk and it's only 4-byte aligned. > > > According to the ARM docs, the issue is that you're loading an 8-byte > value from a 4-byte aligned address. If you split
[PATCH] net: ti: am65-cpsw-nuss: don't touch DMA after stop
From: Alexander Sverdlin
Contrary to doc/develop/driver-model/ethernet.rst contract, eth_ops
.free_pkt can be called after .stop, there are several error paths in TFTP,
for instance:
eth_halt() <= tftp_handler() <= net_process_received_packet() <= eth_rx()
...
am65_cpsw_free_pkt() <= eth_rx()
Which results in (deliberately "tftpboot"ing non-existing file):
TFTP error: 'File not found' (1)
Not retrying...
am65_cpsw_nuss_port ethernet@800port@1: RX dma free_pkt failed -22
Avoid the error message by checking that the interface is still not stopped
in am65_cpsw_free_pkt().
Fixes: 9d0dca1199d1 ("net: ethernet: ti: Introduce am654 gigabit eth switch
subsystem driver")
Signed-off-by: Alexander Sverdlin
---
drivers/net/ti/am65-cpsw-nuss.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c
index 65ade1afd05..646f618afcf 100644
--- a/drivers/net/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ti/am65-cpsw-nuss.c
@@ -523,6 +523,9 @@ static int am65_cpsw_free_pkt(struct udevice *dev, uchar
*packet, int length)
struct am65_cpsw_common *common = priv->cpsw_common;
int ret;
+ if (!common->started)
+ return -ENETDOWN;
+
if (length > 0) {
u32 pkt = common->rx_next % UDMA_RX_DESC_NUM;
--
2.44.0
Re: [PATCH 062/149] board: friendlyarm: Remove and add needed includes
On 01.05.24 04:41, Tom Rini wrote: Remove from this board vendor directory and when needed add missing include files directly. Signed-off-by: Tom Rini Reviewed-by: Stefan Bosch Tested-by: Stefan Bosch Looks good, tested on FriendlyElec-Board NanoPC-T2. Thanks a lot! --- Cc: Stefan Bosch Cc: Caleb Connolly Cc: Eugen Hristev Cc: Igor Prusov Cc: Patrice Chotard --- board/friendlyarm/nanopi2/board.c | 1 - board/friendlyarm/nanopi2/hwrev.c | 1 - board/friendlyarm/nanopi2/lcds.c| 1 - board/friendlyarm/nanopi2/onewire.c | 1 - 4 files changed, 4 deletions(-) diff --git a/board/friendlyarm/nanopi2/board.c b/board/friendlyarm/nanopi2/board.c index 393c5a447d6f..c8cbc5a15fa8 100644 --- a/board/friendlyarm/nanopi2/board.c +++ b/board/friendlyarm/nanopi2/board.c @@ -5,7 +5,6 @@ */ #include -#include #include #include #include diff --git a/board/friendlyarm/nanopi2/hwrev.c b/board/friendlyarm/nanopi2/hwrev.c index 585e08c944f9..cd9c2414a320 100644 --- a/board/friendlyarm/nanopi2/hwrev.c +++ b/board/friendlyarm/nanopi2/hwrev.c @@ -5,7 +5,6 @@ */ #include -#include #include #include diff --git a/board/friendlyarm/nanopi2/lcds.c b/board/friendlyarm/nanopi2/lcds.c index 7303e53af925..b37367300cf0 100644 --- a/board/friendlyarm/nanopi2/lcds.c +++ b/board/friendlyarm/nanopi2/lcds.c @@ -4,7 +4,6 @@ */ #include -#include #include #include #include diff --git a/board/friendlyarm/nanopi2/onewire.c b/board/friendlyarm/nanopi2/onewire.c index 4f0b1e33c2df..31cc871330ca 100644 --- a/board/friendlyarm/nanopi2/onewire.c +++ b/board/friendlyarm/nanopi2/onewire.c @@ -5,7 +5,6 @@ */ #include -#include #include #include #include
Re: [PATCH 17/33] arm: nexell: Remove and add needed includes
On 30.04.24 15:35, Tom Rini wrote:
Remove from all mach-nexell files and when needed add missing
include files directly.
Signed-off-by: Tom Rini
Reviewed-by: Stefan Bosch
Tested-by: Stefan Bosch
Looks good, tested on FriendlyElec-Board NanoPC-T2.
Thanks a lot!
---
Cc: Stefan Bosch
---
arch/arm/mach-nexell/clock.c | 2 +-
arch/arm/mach-nexell/include/mach/reset.h | 2 ++
arch/arm/mach-nexell/reset.c | 1 -
arch/arm/mach-nexell/tieoff.c | 1 -
arch/arm/mach-nexell/timer.c | 1 -
5 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-nexell/clock.c b/arch/arm/mach-nexell/clock.c
index 59ffa26255f5..3082f6077b73 100644
--- a/arch/arm/mach-nexell/clock.c
+++ b/arch/arm/mach-nexell/clock.c
@@ -4,8 +4,8 @@
* Hyunseok, Jung
*/
-#include
#include
+#include
#include
#include
#include
diff --git a/arch/arm/mach-nexell/include/mach/reset.h
b/arch/arm/mach-nexell/include/mach/reset.h
index e1301d4e53d3..0c6a13043f91 100644
--- a/arch/arm/mach-nexell/include/mach/reset.h
+++ b/arch/arm/mach-nexell/include/mach/reset.h
@@ -7,6 +7,8 @@
#ifndef __NEXELL_RESET__
#define __NEXELL_RESET__
+#include
+
#define NUMBER_OF_RESET_MODULE_PIN 69
enum rstcon {
diff --git a/arch/arm/mach-nexell/reset.c b/arch/arm/mach-nexell/reset.c
index 1f732a3d3732..627f568270b6 100644
--- a/arch/arm/mach-nexell/reset.c
+++ b/arch/arm/mach-nexell/reset.c
@@ -8,7 +8,6 @@
*FIXME : Not support device tree & reset control driver.
*will remove after support device tree & reset control driver.
*/
-#include
#include
#include
#include
diff --git a/arch/arm/mach-nexell/tieoff.c b/arch/arm/mach-nexell/tieoff.c
index 5a4744c296a2..51cca6744d6f 100644
--- a/arch/arm/mach-nexell/tieoff.c
+++ b/arch/arm/mach-nexell/tieoff.c
@@ -4,7 +4,6 @@
* Youngbok, Park
*/
-#include
#include
#include
#include
diff --git a/arch/arm/mach-nexell/timer.c b/arch/arm/mach-nexell/timer.c
index 3b311fd22a56..b35c7b1bb33a 100644
--- a/arch/arm/mach-nexell/timer.c
+++ b/arch/arm/mach-nexell/timer.c
@@ -4,7 +4,6 @@
* Hyunseok, Jung
*/
-#include
#include
#include
Re: Does u-boot support USB CCID communication?
Hi Sourabh, In U-Boot, the only methods of separating storage of secrets from general storage are (as far as I know): * TPM v1/v2 * OP-TEE in combination with an MMC supporting RPMB -- Kind regards Andreas Buschka
Re: [PATCH v6 2/3] net: bootp: BOOTP/DHCPv4 retransmission improvements
On Mon, Apr 29, 2024 at 11:33:57AM -0700, Sean Edmond wrote: > This patch introduces 3 improvements to align with RFC 951: > - retransmission backoff interval maximum is configurable > - initial retranmission backoff interval is configurable > - transaction ID is kept the same for each BOOTP/DHCPv4 request > > In applications where thousands of nodes are serviced by a single DHCP > server, maximizing the retransmission backoff interval at 2 seconds (the > current u-boot default) exerts high pressure on the DHCP server and > network layer. > > RFC 951 “7.2. Client Retransmission Strategy” states that the > retransmission backoff interval should be limited to 60 seconds. This > patch allows the interval to be configurable using the environment > variable "bootpretransmitperiodmax" > > The initial retranmission backoff period defaults to 250ms, which is > also too small for these scenarios with many clients. This patch makes > the initial retransmission interval to be configurable using the > environment variable "bootpretransmitperiodinit". > > Also, on a retransmission it is not expected for the transaction ID to > change (only the 'secs' field should be updated). Let's save the > transaction ID and use the same transaction ID for each BOOTP/DHCPv4 > exchange. > > Signed-off-by: Sean Edmond On numerous platforms we now get: +(imx8qm_dmsse20a1) WARNING 'mx8qm-ahab-container.img' not found, resulting binary is not-functional +(imx8qm_dmsse20a1) net/bootp.c: In function 'bootp_reset': +(imx8qm_dmsse20a1) net/bootp.c:741:15: error: unused variable 'ep' [-Werror=unused-variable] +(imx8qm_dmsse20a1) 741 | char *ep; /* Environment pointer */ +(imx8qm_dmsse20a1) | ^~ +(imx8qm_dmsse20a1) cc1: all warnings being treated as errors +(imx8qm_dmsse20a1) make[2]: *** [scripts/Makefile.build:257: net/bootp.o] Error 1 +(imx8qm_dmsse20a1) make[1]: *** [Makefile:1892: net] Error 2 +(imx8qm_dmsse20a1) make: *** [Makefile:177: sub-make] Error 2 -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 05/28] image: remove redundant hash includes
Hi Ilias, On Wed, 8 May 2024 at 06:14, Ilias Apalodimas wrote: > On Tue, 7 May 2024 at 20:54, Raymond Mao wrote: > > > > Remove the redundant includes of u-boot/md5.h, u-boot/sha1.h, > > u-boot/sha256.h and u-boot/sha512.h > > > > Signed-off-by: Raymond Mao > > --- > > Changes in v2 > > - None. > > > > boot/image-fit.c | 4 > > boot/image.c | 2 -- > > 2 files changed, 6 deletions(-) > > > > diff --git a/boot/image-fit.c b/boot/image-fit.c > > index 89e377563ce..1efc39f4408 100644 > > --- a/boot/image-fit.c > > +++ b/boot/image-fit.c > > @@ -38,10 +38,6 @@ DECLARE_GLOBAL_DATA_PTR; > > #include > > #include > > #include > > -#include > > -#include > > -#include > > -#include > > > > > /*/ > > /* New uImage format routines */ > > diff --git a/boot/image.c b/boot/image.c > > index 073931cd7a3..e57d6eae52d 100644 > > --- a/boot/image.c > > +++ b/boot/image.c > > @@ -26,8 +26,6 @@ > > #endif > > > > #include > > -#include > > -#include > > #include > > #include > > > > -- > > 2.25.1 > > > > This seems unrelated to the series of mbedTLS. Can you send a series > with the fixes independently? Tom can pick them up before mbedTLS and > it would make the review process easier > > I can send this in a new prerequisite series. Regards, Raymond > Reviewed-by: Ilias Apalodimas >
Re: [PATCH v2 15/28] mbedtls/external: update MbedTLS PKCS7 test suites
Hi Ilias, On Wed, 8 May 2024 at 10:33, Ilias Apalodimas wrote: > Hi Raymond, > > On Tue, 7 May 2024 at 20:58, Raymond Mao wrote: > > > > Update the PKCS7 test suites for multiple certs. > > Please explain why. Does this belong to U-Boot or the patch should be > sent to mbedTLS directly? > > It is sent to MbedTLS upstream as well. The patch here is to keep the codebase consistent with the upstream, and for the people who want to run the MbedTLS test suite under U-Boot, until they are merged to MbedTLS upstream. Regards, Raymond Thanks > /Ilias > > > > Signed-off-by: Raymond Mao > > --- > > Changes in v2 > > - None. > > > > .../external/mbedtls/tests/suites/test_suite_pkcs7.data | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git > a/lib/mbedtls/external/mbedtls/tests/suites/test_suite_pkcs7.data > b/lib/mbedtls/external/mbedtls/tests/suites/test_suite_pkcs7.data > > index d3b83cdf0aa..2dd1c56109f 100644 > > --- a/lib/mbedtls/external/mbedtls/tests/suites/test_suite_pkcs7.data > > +++ b/lib/mbedtls/external/mbedtls/tests/suites/test_suite_pkcs7.data > > @@ -14,9 +14,9 @@ PKCS7 Signed Data Parse with zero signers > > depends_on:MBEDTLS_MD_CAN_SHA256 > > > pkcs7_parse:"data_files/pkcs7_data_no_signers.der":MBEDTLS_PKCS7_SIGNED_DATA > > > > -PKCS7 Signed Data Parse Fail with multiple certs #4 > > +PKCS7 Signed Data Parse Pass with multiple certs #4 > > depends_on:MBEDTLS_MD_CAN_SHA256:MBEDTLS_RSA_C > > > -pkcs7_parse:"data_files/pkcs7_data_multiple_certs_signed.der":MBEDTLS_ERR_PKCS7_FEATURE_UNAVAILABLE > > > +pkcs7_parse:"data_files/pkcs7_data_multiple_certs_signed.der":MBEDTLS_PKCS7_SIGNED_DATA > > > > PKCS7 Signed Data Parse Fail with corrupted cert #5.0 > > depends_on:MBEDTLS_MD_CAN_SHA256:MBEDTLS_RSA_C > > -- > > 2.25.1 > > >
Re: [PATCH v2 14/28] mbedtls/external: support decoding multiple signer's cert
Hi Ilias,
On Wed, 8 May 2024 at 10:35, Ilias Apalodimas
wrote:
> Hi Raymond
>
> On Tue, 7 May 2024 at 20:57, Raymond Mao wrote:
> >
> > Support decoding multiple signer's cert in the signed data within
> > a PKCS7 message.
>
> For all similar external mbedTLS patches, try to explain which belong
> to mbedTLS must be sent upstream and which ones we need to carry
> internally. I don't expect us to have to carry anything in U-Boot
>
> I can add a reference link to the MbedTLS PR for the patches marked with
"mbedtls/external".
But for enabling the features, we still need to merge them before the next
git
subtree update when those patches are merged in a MbedTLS release.
Regards,
Raymond
Thanks
> /Ilias
>
> >
> > Signed-off-by: Raymond Mao
> > ---
> > Changes in v2
> > - None.
> >
> > lib/mbedtls/external/mbedtls/library/pkcs7.c | 75
> > 1 file changed, 47 insertions(+), 28 deletions(-)
> >
> > diff --git a/lib/mbedtls/external/mbedtls/library/pkcs7.c
> b/lib/mbedtls/external/mbedtls/library/pkcs7.c
> > index da73fb341d6..01105227d7a 100644
> > --- a/lib/mbedtls/external/mbedtls/library/pkcs7.c
> > +++ b/lib/mbedtls/external/mbedtls/library/pkcs7.c
> > @@ -61,6 +61,36 @@ static int pkcs7_get_next_content_len(unsigned char
> **p, unsigned char *end,
> > return ret;
> > }
> >
> > +/**
> > + * Get and decode one cert from a sequence.
> > + * Return 0 for success,
> > + * Return negative error code for failure.
> > + **/
> > +static int pkcs7_get_one_cert(unsigned char **p, unsigned char *end,
> > + mbedtls_x509_crt *certs)
> > +{
> > +int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
> > +size_t len = 0;
> > +unsigned char *start = *p;
> > +unsigned char *end_cert;
> > +
> > +ret = mbedtls_asn1_get_tag(p, end, , MBEDTLS_ASN1_CONSTRUCTED
> > + | MBEDTLS_ASN1_SEQUENCE);
> > +if (ret != 0) {
> > +return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CERT, ret);
> > +}
> > +
> > +end_cert = *p + len;
> > +
> > +if ((ret = mbedtls_x509_crt_parse_der(certs, start, end_cert -
> start)) < 0) {
> > +return MBEDTLS_ERR_PKCS7_INVALID_CERT;
> > +}
> > +
> > +*p = end_cert;
> > +
> > +return 0;
> > +}
> > +
> > /**
> > * version Version
> > * Version ::= INTEGER
> > @@ -178,11 +208,12 @@ static int pkcs7_get_certificates(unsigned char
> **p, unsigned char *end,
> >mbedtls_x509_crt *certs)
> > {
> > int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
> > -size_t len1 = 0;
> > -size_t len2 = 0;
> > -unsigned char *end_set, *end_cert, *start;
> > +size_t len = 0;
> > +unsigned char *end_set;
> > +int num_of_certs = 0;
> >
> > -ret = mbedtls_asn1_get_tag(p, end, , MBEDTLS_ASN1_CONSTRUCTED
> > +/* Get the set of certs */
> > +ret = mbedtls_asn1_get_tag(p, end, , MBEDTLS_ASN1_CONSTRUCTED
> > | MBEDTLS_ASN1_CONTEXT_SPECIFIC);
> > if (ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG) {
> > return 0;
> > @@ -190,38 +221,26 @@ static int pkcs7_get_certificates(unsigned char
> **p, unsigned char *end,
> > if (ret != 0) {
> > return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret);
> > }
> > -start = *p;
> > -end_set = *p + len1;
> > +end_set = *p + len;
> >
> > -ret = mbedtls_asn1_get_tag(p, end_set, ,
> MBEDTLS_ASN1_CONSTRUCTED
> > - | MBEDTLS_ASN1_SEQUENCE);
> > +ret = pkcs7_get_one_cert(p, end_set, certs);
> > if (ret != 0) {
> > -return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CERT, ret);
> > +return ret;
> > }
> >
> > -end_cert = *p + len2;
> > +num_of_certs++;
> >
> > -/*
> > - * This is to verify that there is only one signer certificate. It
> seems it is
> > - * not easy to differentiate between the chain vs different
> signer's certificate.
> > - * So, we support only the root certificate and the single signer.
> > - * The behaviour would be improved with addition of multiple signer
> support.
> > - */
> > -if (end_cert != end_set) {
> > -return MBEDTLS_ERR_PKCS7_FEATURE_UNAVAILABLE;
> > -}
> > -
> > -if ((ret = mbedtls_x509_crt_parse_der(certs, start, len1)) < 0) {
> > -return MBEDTLS_ERR_PKCS7_INVALID_CERT;
> > +while (*p != end_set) {
> > +ret = pkcs7_get_one_cert(p, end_set, certs);
> > +if (ret != 0) {
> > +return ret;
> > +}
> > +num_of_certs++;
> > }
> >
> > -*p = end_cert;
> > +*p = end_set;
> >
> > -/*
> > - * Since in this version we strictly support single certificate,
> and reaching
> > - * here implies we have parsed successfully, we return 1.
> > - */
> > -return 1;
> > +return num_of_certs;
> > }
> >
> > /**
> > --
> > 2.25.1
> >
>
Re: [PATCH v2 0/4] arm: Add Analog Devices SC5xx Machine Type
On Wed, 24 Apr 2024 20:03:59 -0400, Greg Malysa wrote: > This series adds support for the ADI SC5xx machine type and includes two > core drivers that are required for being able to boot any board--a UART > driver, the gptimer driver which is used as a clock reference (CNTVCNT > is not supported on the armv7 sc5xx SoCs) and the clock tree driver. Our > corresponding Linux support relies on u-boot configuring the clocks > correctly before booting, so it is not possible to boot any board > without the CGU/CDU configuration happening here. There are also no > board files, device trees, or defconfigs included here, but some common > definitions that will be used to build board files currently are. The > sc5xx SoCs themselves include many armv7 families (sc57x, sc58x, and > sc594) all using an ARM Cortex-A5, and one armv8 family (sc598) indended > to be a drop-in replacement for the SC594 in terms of peripherals, with > a Cortex-A55 instead. > > [...] Applied to u-boot/next, thanks! -- Tom
Re: [PATCH 3/5] zfs: Fix unaligned read of uint64
On Thursday, April 25th, 2024 at 4:57 AM, Caleb Connolly wrote: > > On 25/04/2024 06:02, mwle...@mailtundra.com wrote: > > > Hi Caleb, > > > > Thanks for this interesting feedback. I saw these patches were already > > merged > > since you sent this but I added a few thoughts below anyway. > > > Ah, a shame.. It would be good to find a scalable solution to this! While I agree your suggestion of doing two 4 byte reads is more scalable than the malloc() patch, the code path (zfs_nvlist_lookup_uint64) is only hit when reading the ZPool label (check_pool_label) which is a once-per-boot operation operating on some metadata, so I don't think scalability is as much a concern as it would be if we were hitting this code path when reading every block from the disk for example. That said, I may try implementing the patch as you suggested if I get a chance. And in any case I definitely appreciate your engagement and feedback! > > > On Friday, April 12th, 2024 at 11:50 AM, Caleb Connolly > > caleb.conno...@linaro.org wrote: > > > > > Hi Phaedrus, > > > > > > On 07/04/2024 03:47, mwle...@mailtundra.com wrote: > > > > > > > Without this patch, when trying to boot zfs using U-Boot on a Jetson TX2 > > > > NX (which is aarch64), I get a CPU reset error like so: > > > > > > > > "Synchronous Abort" handler, esr 0x9621 > > > > elr: 800c9000 lr : 800c8ffc (reloc) > > > > elr: fff77000 lr : fff76ffc > > > > x0 : ffb40f04 x1 : > > > > x2 : 000a x3 : 0310 > > > > x4 : 0310 x5 : 0034 > > > > x6 : fff9cc6e x7 : 000f > > > > x8 : ff7f84a0 x9 : 0008 > > > > x10: ffb40f04 x11: 0006 > > > > x12: 0001869f x13: 0001 > > > > x14: ff7f84bc x15: 0010 > > > > x16: 2080 x17: 001f > > > > x18: ff7fbdd8 x19: ffb405f8 > > > > x20: ffb40dd0 x21: fffabe5e > > > > x22: 00ea7794 x23: ffb42090 > > > > x24: x25: > > > > x26: x27: > > > > x28: 00bab10c x29: ff7f85f0 > > > > > > > > Code: d1a0 9103a000 94006ac6 f9401ba0 (f940) > > > > Resetting CPU ... > > > > > > > > This happens when be64_to_cpu() is called on a value that exists at a > > > > memory address that's 4 byte aligned but not 8 byte aligned (e.g. an > > > > address ending in 04). The call stack where that happens is: > > > > check_pool_label() -> > > > > zfs_nvlist_lookup_uint64(vdevnvlist, ZPOOL_CONFIG_ASHIFT,...) -> > > > > be64_to_cpu() > > > > > > This is very odd, aarch64 doesn't generally have these restrictions. I > > > got a bit nerdsniped when I saw this so I did some digging and figured > > > this: > > > > > > 1. Your abort exception doesn't include the FAR_ELx register (which > > > should contain the address that was being accessed when the abort > > > occured). This means your board is running in EL3. > > > 2. It turns out there is an "A" flag in the SCTLR_ELx register, when set > > > this flag causes a fault when trying to load from an address that isn't > > > aligned to the size of the data element (see "A, bit" on > > > https://developer.arm.com/documentation/ddi0595/2021-06/AArch64-Registers/SCTLR-EL3--System-Control-Register--EL3- > > > > > > I'm not sure who's in the "wrong" here, maybe the driver should avoid > > > unaligned accesses? But then again, I don't think you should be running > > > a ZFS driver in EL3. > > > > > > I'm not familiar with the Jetson Nano, but maybe there's a documented > > > way to run U-Boot so that it isn't executing in EL3? Or if not you could > > > also try unsetting the A flag. > > > > I may look into this if I get a chance. However if I write some assembly > > code > > to change the execution level or unset the A flag, I worry that the code > > would > > work fine on the hardware I have (Jetson TX2 NX) but behave differently on > > > Well, unsetting the A flag might arguably have some niche negative > outcomes, but I really struggle to see how this is a driver bug to be > honest... > > > another platform. And I don't think I can easily set up testing environments > > with u-boot + zfs on different platforms to find out. > > > fwiw, if you do intend to investigate this further, I'd be happy to > validate your assumptions on a Qualcomm board (where we are rather > boring and running in EL1 or EL2). Thanks, I'll keep that in mind > > > > If this really is something to fix in the driver, I don't think > > > hotpatching every unaligned access with a malloc() is the right solution. > > > > I'm certainly open to other ideas. The difficulty is the data structure > > we're > > parsing in this file is read from disk and it's only 4-byte aligned. > > > According to the ARM docs, the issue is that you're loading an 8-byte > value from a 4-byte
Re: [PATCH v2 12/28] mbedtls/external: support MicroSoft Authentication Code
Hi Ilias, On Wed, 8 May 2024 at 10:32, Ilias Apalodimas wrote: > Hi Raymond > > On Tue, 7 May 2024 at 20:57, Raymond Mao wrote: > > > > Populate MicroSoft Authentication Code from the content data > > into PKCS7 decoding context if it exists in a PKCS7 message. > > Add OIDs for describing objects using for MicroSoft Authentication > > Code. > > > > We will need more accurate commit messages for things like this. > IIRC this is already on a PR for mbedTLS and won't be needed in the > future right? > Generally speaking, we shouldn't carry out of tree patches unless we > can prove there's a very good reason. This one needs to be marked as > 'do not merge' and we should wait until mbedTLS merges it upstream > > I can add a tag here to link to the MbedTLS upstream PR for reference. However I think we still need to merge these patches, since MbedTLS will not merge it in v3.6.0. Without it, the EFI Secure Boot and Capsule functions will be broken. In the future, when MbedTLS merges them in later v3.6.x, we can simply update the git subtree. Regards, Raymond
Re: [PATCH] mach-snapdragon: do carveouts for qcs404 only
On Wed, 8 May 2024 at 18:09, Caleb Connolly wrote: > > Hi Sam, > > On 08/05/2024 13:40, Sam Day wrote: > > Salutations Sumit, > > > > On Wednesday, 8 May 2024 at 11:14 AM, Sumit Garg > > wrote: > > > >> > >> > >> Hi Sam, > >> > >> On Wed, 8 May 2024 at 00:11, Sam Day m...@samcday.com wrote: > >> > >>> The newly introduced carve_out_reserved_memory causes issues when > >>> U-Boot is chained from the lk2nd bootloader. lk2nd provides a > >>> simple-framebuffer device and marks the framebuffer region as no-map in > >>> the supplied /reserved-memory. Consequently, the simple_video driver > >>> triggers a page fault when it tries to write to this region. > >> > >> > >> How does the corresponding Linux kernel driver handle this? > > > > Firstly: I'm something of a middle-man here. I would consider Caleb the > > authoritative source on the carveouts stuff (since they wrote it) and > > Nikita Travkin the authority on the simplefb handoff (since he originally > > wrote it for lk2nd to hand off to Linux simpledrm and then adapted it to > > work with U-Boot simplefb). > > > > I consulted with Nikita on your first question here. He linked me to this > > snippet: > > https://elixir.bootlin.com/linux/v6.9-rc7/source/drivers/gpu/drm/tiny/simpledrm.c#L877 > > > >> Is the > >> framebuffer region required to be mapped as normal memory or device > >> type or something else? > > > > So I guess based on the link above, it's just mapped as normal uncached > > memory. > > > > I tried to do something like this a few days ago in U-Boot, but a) it > > doesn't work and b) I have no idea what I'm doing: > > https://github.com/samcday/u-boot/commit/c100cb3711ddf5b01601691f3e6a9ec890d9a496 Can you start adding some debug prints as to what might be happening? Also, look at my proposal below to hook memory mapping properly for drivers. > > > > After talking with Caleb about this for a bit, they suggested the patch you > > see here as what I guess could be considered a "stopgap" solution that > > hopefully makes it into 2024.07. > > > >> Similarly would normal memory type work for > >> all other reserved memory regions marked as no-map? > > > > I'll let Caleb weigh in here. My understanding is that the other regions > > *should* be marked as PTE_TYPE_FAULT because otherwise drivers might > > inadvertently speculatively access regions that are very much off-limits, > > such as TZ app regions. > > Right, carving out reserved regions and having the driver handle them is > theoretically the "correct" thing to do here. But I'm not sure that the > additional complexity offers much value from a U-Boot context. > > If we can teach the armv8 PT/cache code to handle this better, and teach > all the drivers to map their regions on-demand, this would probably help > us a lot (especially as right now if you attempt to access dead space > between peripherals then you'll hang the bus...). But U-Boot is a ways > away from that. I am not sure if U-Boot is really that far away although you can say the infrastructure is not hooked up for Arm. Have a look into map_physmem() and unmap_physmem() utilized by various drivers which are actually hooked up on MIPS. So I think the right way to approach this feature on Qcom platforms is to start hooking up those APIs on mach-snapdragon to begin with via mmu_set_region_dcache_behaviour() underneath. We can always go ahead and make it more generic if needed on other platforms too. -Sumit
Re: [PATCH v2 04/28] arm: EFI linker script text section alignment
Hi Raymond,
On Tue, 7 May 2024 at 20:53, Raymond Mao wrote:
>
> Add text section alignment to fix sbsign signing warning
> 'gaps in the section table may result in different checksums'
> which causes a failure of efi_image_verify_diges()
>
> Signed-off-by: Raymond Mao
> ---
> Changes in v2
> - None.
>
> arch/arm/lib/elf_aarch64_efi.lds | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/lib/elf_aarch64_efi.lds
> b/arch/arm/lib/elf_aarch64_efi.lds
> index 5dd98091698..bffd9a0447a 100644
> --- a/arch/arm/lib/elf_aarch64_efi.lds
> +++ b/arch/arm/lib/elf_aarch64_efi.lds
> @@ -28,6 +28,7 @@ SECTIONS
> *(.dynamic);
> . = ALIGN(512);
> }
> + . = ALIGN(4096);
There are linker symbols a few lines below marking _etext and _text_size.
Those should be moved within the section alignment. On top of that,
more architectures will need a similar fix.
Heinrich don't you need similar changes on risc-v?
Thanks
/Ilias
> .rela.dyn : { *(.rela.dyn) }
> .rela.plt : { *(.rela.plt) }
> .rela.got : { *(.rela.got) }
> --
> 2.25.1
>
Split this off to a preparation series. This seems like a generic fix.
Re: [PATCH v2 11/28] makefile: add mbedtls include directories
On Tue, 7 May 2024 at 20:56, Raymond Mao wrote: > > Add the mbedtls include directories into the build system. > > Signed-off-by: Raymond Mao > --- > Changes in v2 > - None. > > Makefile | 13 + > 1 file changed, 13 insertions(+) > > diff --git a/Makefile b/Makefile > index 7321fe1499e..80db1dfd8ec 100644 > --- a/Makefile > +++ b/Makefile > @@ -829,6 +829,12 @@ KBUILD_HOSTCFLAGS += $(if $(CONFIG_TOOLS_DEBUG),-g) > UBOOTINCLUDE:= \ > -Iinclude \ > $(if $(KBUILD_SRC), -I$(srctree)/include) \ > + $(if $(KBUILD_SRC), \ > + $(if $(CONFIG_MBEDTLS_LIB), \ > + "-DMBEDTLS_CONFIG_FILE=\"mbedtls_def_config.h\"" \ > + -I$(srctree)/lib/mbedtls \ > + -I$(srctree)/lib/mbedtls/port \ > + -I$(srctree)/lib/mbedtls/external/mbedtls/include)) \ > $(if $(CONFIG_$(SPL_)SYS_THUMB_BUILD), \ > $(if $(CONFIG_HAS_THUMB2), \ > $(if $(CONFIG_CPU_V7M), \ > @@ -840,6 +846,13 @@ UBOOTINCLUDE:= \ > > NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include) > > +ifeq ($(CONFIG_MBEDTLS_LIB),y) > +PLATFORM_CPPFLAGS += "-DMBEDTLS_CONFIG_FILE=\"mbedtls_def_config.h\"" \ > + -I$(srctree)/lib/mbedtls \ > + -I$(srctree)/lib/mbedtls/port \ > + -I$(srctree)/lib/mbedtls/external/mbedtls/include cpp_flags already includes UBOOTINCLUDE. /Ilias > +endif > + > # FIX ME > cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \ > $(NOSTDINC_FLAGS) > -- > 2.25.1 >
Re: [PATCH v2 14/28] mbedtls/external: support decoding multiple signer's cert
Hi Raymond
On Tue, 7 May 2024 at 20:57, Raymond Mao wrote:
>
> Support decoding multiple signer's cert in the signed data within
> a PKCS7 message.
For all similar external mbedTLS patches, try to explain which belong
to mbedTLS must be sent upstream and which ones we need to carry
internally. I don't expect us to have to carry anything in U-Boot
Thanks
/Ilias
>
> Signed-off-by: Raymond Mao
> ---
> Changes in v2
> - None.
>
> lib/mbedtls/external/mbedtls/library/pkcs7.c | 75
> 1 file changed, 47 insertions(+), 28 deletions(-)
>
> diff --git a/lib/mbedtls/external/mbedtls/library/pkcs7.c
> b/lib/mbedtls/external/mbedtls/library/pkcs7.c
> index da73fb341d6..01105227d7a 100644
> --- a/lib/mbedtls/external/mbedtls/library/pkcs7.c
> +++ b/lib/mbedtls/external/mbedtls/library/pkcs7.c
> @@ -61,6 +61,36 @@ static int pkcs7_get_next_content_len(unsigned char **p,
> unsigned char *end,
> return ret;
> }
>
> +/**
> + * Get and decode one cert from a sequence.
> + * Return 0 for success,
> + * Return negative error code for failure.
> + **/
> +static int pkcs7_get_one_cert(unsigned char **p, unsigned char *end,
> + mbedtls_x509_crt *certs)
> +{
> +int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
> +size_t len = 0;
> +unsigned char *start = *p;
> +unsigned char *end_cert;
> +
> +ret = mbedtls_asn1_get_tag(p, end, , MBEDTLS_ASN1_CONSTRUCTED
> + | MBEDTLS_ASN1_SEQUENCE);
> +if (ret != 0) {
> +return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CERT, ret);
> +}
> +
> +end_cert = *p + len;
> +
> +if ((ret = mbedtls_x509_crt_parse_der(certs, start, end_cert - start)) <
> 0) {
> +return MBEDTLS_ERR_PKCS7_INVALID_CERT;
> +}
> +
> +*p = end_cert;
> +
> +return 0;
> +}
> +
> /**
> * version Version
> * Version ::= INTEGER
> @@ -178,11 +208,12 @@ static int pkcs7_get_certificates(unsigned char **p,
> unsigned char *end,
>mbedtls_x509_crt *certs)
> {
> int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
> -size_t len1 = 0;
> -size_t len2 = 0;
> -unsigned char *end_set, *end_cert, *start;
> +size_t len = 0;
> +unsigned char *end_set;
> +int num_of_certs = 0;
>
> -ret = mbedtls_asn1_get_tag(p, end, , MBEDTLS_ASN1_CONSTRUCTED
> +/* Get the set of certs */
> +ret = mbedtls_asn1_get_tag(p, end, , MBEDTLS_ASN1_CONSTRUCTED
> | MBEDTLS_ASN1_CONTEXT_SPECIFIC);
> if (ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG) {
> return 0;
> @@ -190,38 +221,26 @@ static int pkcs7_get_certificates(unsigned char **p,
> unsigned char *end,
> if (ret != 0) {
> return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret);
> }
> -start = *p;
> -end_set = *p + len1;
> +end_set = *p + len;
>
> -ret = mbedtls_asn1_get_tag(p, end_set, , MBEDTLS_ASN1_CONSTRUCTED
> - | MBEDTLS_ASN1_SEQUENCE);
> +ret = pkcs7_get_one_cert(p, end_set, certs);
> if (ret != 0) {
> -return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CERT, ret);
> +return ret;
> }
>
> -end_cert = *p + len2;
> +num_of_certs++;
>
> -/*
> - * This is to verify that there is only one signer certificate. It seems
> it is
> - * not easy to differentiate between the chain vs different signer's
> certificate.
> - * So, we support only the root certificate and the single signer.
> - * The behaviour would be improved with addition of multiple signer
> support.
> - */
> -if (end_cert != end_set) {
> -return MBEDTLS_ERR_PKCS7_FEATURE_UNAVAILABLE;
> -}
> -
> -if ((ret = mbedtls_x509_crt_parse_der(certs, start, len1)) < 0) {
> -return MBEDTLS_ERR_PKCS7_INVALID_CERT;
> +while (*p != end_set) {
> +ret = pkcs7_get_one_cert(p, end_set, certs);
> +if (ret != 0) {
> +return ret;
> +}
> +num_of_certs++;
> }
>
> -*p = end_cert;
> +*p = end_set;
>
> -/*
> - * Since in this version we strictly support single certificate, and
> reaching
> - * here implies we have parsed successfully, we return 1.
> - */
> -return 1;
> +return num_of_certs;
> }
>
> /**
> --
> 2.25.1
>
Re: [PATCH v2 15/28] mbedtls/external: update MbedTLS PKCS7 test suites
Hi Raymond, On Tue, 7 May 2024 at 20:58, Raymond Mao wrote: > > Update the PKCS7 test suites for multiple certs. Please explain why. Does this belong to U-Boot or the patch should be sent to mbedTLS directly? Thanks /Ilias > > Signed-off-by: Raymond Mao > --- > Changes in v2 > - None. > > .../external/mbedtls/tests/suites/test_suite_pkcs7.data | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/mbedtls/external/mbedtls/tests/suites/test_suite_pkcs7.data > b/lib/mbedtls/external/mbedtls/tests/suites/test_suite_pkcs7.data > index d3b83cdf0aa..2dd1c56109f 100644 > --- a/lib/mbedtls/external/mbedtls/tests/suites/test_suite_pkcs7.data > +++ b/lib/mbedtls/external/mbedtls/tests/suites/test_suite_pkcs7.data > @@ -14,9 +14,9 @@ PKCS7 Signed Data Parse with zero signers > depends_on:MBEDTLS_MD_CAN_SHA256 > pkcs7_parse:"data_files/pkcs7_data_no_signers.der":MBEDTLS_PKCS7_SIGNED_DATA > > -PKCS7 Signed Data Parse Fail with multiple certs #4 > +PKCS7 Signed Data Parse Pass with multiple certs #4 > depends_on:MBEDTLS_MD_CAN_SHA256:MBEDTLS_RSA_C > -pkcs7_parse:"data_files/pkcs7_data_multiple_certs_signed.der":MBEDTLS_ERR_PKCS7_FEATURE_UNAVAILABLE > +pkcs7_parse:"data_files/pkcs7_data_multiple_certs_signed.der":MBEDTLS_PKCS7_SIGNED_DATA > > PKCS7 Signed Data Parse Fail with corrupted cert #5.0 > depends_on:MBEDTLS_MD_CAN_SHA256:MBEDTLS_RSA_C > -- > 2.25.1 >
Re: [PATCH v2 12/28] mbedtls/external: support MicroSoft Authentication Code
Hi Raymond
On Tue, 7 May 2024 at 20:57, Raymond Mao wrote:
>
> Populate MicroSoft Authentication Code from the content data
> into PKCS7 decoding context if it exists in a PKCS7 message.
> Add OIDs for describing objects using for MicroSoft Authentication
> Code.
>
We will need more accurate commit messages for things like this.
IIRC this is already on a PR for mbedTLS and won't be needed in the
future right?
Generally speaking, we shouldn't carry out of tree patches unless we
can prove there's a very good reason. This one needs to be marked as
'do not merge' and we should wait until mbedTLS merges it upstream
Thanks
/Ilias
> Signed-off-by: Raymond Mao
> ---
> Changes in v2
> - None.
>
> .../external/mbedtls/include/mbedtls/oid.h| 30 ++
> .../external/mbedtls/include/mbedtls/pkcs7.h | 10
> lib/mbedtls/external/mbedtls/library/pkcs7.c | 60 +++
> 3 files changed, 90 insertions(+), 10 deletions(-)
>
> diff --git a/lib/mbedtls/external/mbedtls/include/mbedtls/oid.h
> b/lib/mbedtls/external/mbedtls/include/mbedtls/oid.h
> index fdc25ebf885..2ee982808fa 100644
> --- a/lib/mbedtls/external/mbedtls/include/mbedtls/oid.h
> +++ b/lib/mbedtls/external/mbedtls/include/mbedtls/oid.h
> @@ -352,6 +352,36 @@
> #define MBEDTLS_OID_PKCS12_PBE_SHA1_RC2_128_CBC MBEDTLS_OID_PKCS12_PBE
> "\x05" /**< pbeWithSHAAnd128BitRC2-CBC OBJECT IDENTIFIER ::= {pkcs-12PbeIds
> 5} */
> #define MBEDTLS_OID_PKCS12_PBE_SHA1_RC2_40_CBC MBEDTLS_OID_PKCS12_PBE
> "\x06" /**< pbeWithSHAAnd40BitRC2-CBC OBJECT IDENTIFIER ::= {pkcs-12PbeIds 6}
> */
>
> +/*
> + * MicroSoft Authenticate Code OIDs
> + */
> +#define MBEDTLS_OID_PRIVATE_ENTERPRISE MBEDTLS_OID_INTERNET
> "\x04\x01" /* {iso(1) identified-organization(3) dod(6) internet(1)
> private(4) enterprise(1) */
> +#define MBEDTLS_OID_MICROSOFT "\x82\x37" /*
> {microsoft(311)} */
> +/*
> + * OID_msIndirectData: (1.3.6.1.4.1.311.2.1.4)
> + * {iso(1) identified-organization(3) dod(6) internet(1) private(4)
> enterprise(1) microsoft(311) 2(2) 1(1) 4(4)}
> + */
> +#define MBEDTLS_OID_MICROSOFT_INDIRECTDATA MBEDTLS_OID_PRIVATE_ENTERPRISE
> MBEDTLS_OID_MICROSOFT \
> +"\x02\x01\x04"
> +/*
> + * OID_msStatementType: (1.3.6.1.4.1.311.2.1.11)
> + * {iso(1) identified-organization(3) dod(6) internet(1) private(4)
> enterprise(1) microsoft(311) 2(2) 1(1) 11(11)}
> + */
> +#define MBEDTLS_OID_MICROSOFT_STATETYPE MBEDTLS_OID_PRIVATE_ENTERPRISE
> MBEDTLS_OID_MICROSOFT \
> +"\x02\x01\x0b"
> +/*
> + * OID_msSpOpusInfo: (1.3.6.1.4.1.311.2.1.12)
> + * {iso(1) identified-organization(3) dod(6) internet(1) private(4)
> enterprise(1) microsoft(311) 2(2) 1(1) 12(12)}
> + */
> +#define MBEDTLS_OID_MICROSOFT_SPOPUSINFO MBEDTLS_OID_PRIVATE_ENTERPRISE
> MBEDTLS_OID_MICROSOFT \
> +"\x02\x01\x0b"
> +/*
> + * OID_msPeImageDataObjId: (1.3.6.1.4.1.311.2.1.15)
> + * {iso(1) identified-organization(3) dod(6) internet(1) private(4)
> enterprise(1) microsoft(311) 2(2) 1(1) 15(15)}
> + */
> +#define MBEDTLS_OID_MICROSOFT_PEIMAGEDATA MBEDTLS_OID_PRIVATE_ENTERPRISE
> MBEDTLS_OID_MICROSOFT \
> +"\x02\x01\x0f"
> +
> /*
> * EC key algorithms from RFC 5480
> */
> diff --git a/lib/mbedtls/external/mbedtls/include/mbedtls/pkcs7.h
> b/lib/mbedtls/external/mbedtls/include/mbedtls/pkcs7.h
> index e9b482208e6..9e29b74af70 100644
> --- a/lib/mbedtls/external/mbedtls/include/mbedtls/pkcs7.h
> +++ b/lib/mbedtls/external/mbedtls/include/mbedtls/pkcs7.h
> @@ -132,12 +132,22 @@ typedef struct mbedtls_pkcs7_signed_data {
> }
> mbedtls_pkcs7_signed_data;
>
> +/* Content Data for MicroSoft Authentication Code using in U-Boot Secure
> Boot */
> +typedef struct mbedtls_pkcs7_conten_data {
> +int data_type; /* Type of Data */
> +size_t data_len;/* Length of Data */
> +size_t data_hdrlen; /* Length of Data ASN.1 header */
> +void *data; /* Content Data */
> +}
> +mbedtls_pkcs7_conten_data;
> +
> /**
> * Structure holding PKCS #7 structure, only signed data for now
> */
> typedef struct mbedtls_pkcs7 {
> mbedtls_pkcs7_buf MBEDTLS_PRIVATE(raw);
> mbedtls_pkcs7_signed_data MBEDTLS_PRIVATE(signed_data);
> +mbedtls_pkcs7_conten_data content_data;
> }
> mbedtls_pkcs7;
>
> diff --git a/lib/mbedtls/external/mbedtls/library/pkcs7.c
> b/lib/mbedtls/external/mbedtls/library/pkcs7.c
> index 3aac662ba69..0c2436b56b7 100644
> --- a/lib/mbedtls/external/mbedtls/library/pkcs7.c
> +++ b/lib/mbedtls/external/mbedtls/library/pkcs7.c
> @@ -29,6 +29,13 @@
> #include
> #endif
>
> +enum OID {
> +/* PKCS#7 {iso(1) member-body(2) us(840) rsadsi(113549) pkcs(1)
> pkcs-7(7)} */
> +MBEDTLS_OID_DATA = 13, /* 1.2.840.113549.1.7.1 */
> +/* Microsoft Authenticode & Software Publishing */
> +MBEDTLS_OID_MS_INDIRECTDATA = 24,/* 1.3.6.1.4.1.311.2.1.4 */
> +};
> +
> /**
> * Initializes the mbedtls_pkcs7 structure.
> */
> @@ -449,7 +456,7 @@ cleanup:
> * signerInfos
Re: [PATCH] efi_loader: capsule: detect possible ESP device path
On Wed, May 8, 2024 at 9:56 PM Ilias Apalodimas
wrote:
>
> >
> > > + *
> > > + * Get a possible efi system partition by expanding a boot option
> > > + * file path.
> > > + *
> > > + * @boot_dev The device path pointing to a boot option
> > > + * Return: The full ESP device path or NULL if fail
> > > + */
> > > +static struct efi_device_path *get_esp_from_boot_option_file_path(struct
> > > efi_device_path *boot_dev)
> > > +{
> > > + efi_status_t ret = EFI_SUCCESS;
> > > + efi_handle_t handle;
> > > + struct efi_device_path *dp = boot_dev;
> > > + struct efi_device_path *full_path = NULL;
> > > +
> > > + ret =
> > > EFI_CALL(efi_locate_device_path(_simple_file_system_protocol_guid,
> > > + ,
> > > + ));
> > > + if (ret != EFI_SUCCESS)
> > > + ret = EFI_CALL(efi_locate_device_path(_block_io_guid,
> > > , ));
> > > +
> > > + /* Expand Media Device Path */
> >
> > I don't see where the device path is expanded in this patch.
> > We already have try_load_from_media() which tries to do something
> > similar. Is anything missing from there?
>
> Looking a bit closer, we do lack calling ConnectController there, but
> all this should be added to try_load_from_media() not in a different
> path.
Yeah, I'll summarize them to the try_load_from_media().
> Also I don't think we need the Fixes tag
Okay.
BR,
Weizhao
>
> Thanks
> /Ilias
> >
> > > + if (ret == EFI_SUCCESS && EFI_DP_TYPE(dp, END, END)) {
> >
> > Can you invert the logic here and return immediately?
> > (if ret != EFI_SUCCESS ...etc )
> > return full_path;
> >
> > The indentation will go away.
> >
> > > + struct efi_device_path *temp_dp;
> > > + struct efi_block_io *block_io;
> > > + void *buffer;
> > > + efi_handle_t *simple_file_system_handle;
> > > + efi_uintn_t number_handles, index;
> > > + u32 size;
> > > + u32 temp_size;
> > > +
> > > + temp_dp = boot_dev;
> > > + ret =
> > > EFI_CALL(efi_locate_device_path(_simple_file_system_protocol_guid,
> > > + _dp,
> > > + ));
> > > + /*
> > > +* For device path pointing to simple file system, it
> > > only expands to one
> > > +* full path
> >
> > Why do we have multiple calls to efi_locate_device_path()? Aren't the
> > arguments identical?
> > Which part of edk2 did you read? Is it BmExpandMediaDevicePath()?
> >
> >
> > > +*/
> > > + if (ret == EFI_SUCCESS && EFI_DP_TYPE(temp_dp, END, END))
> > > {
> > > + if (device_is_present_and_system_part(temp_dp))
> > > + return temp_dp;
> > > + }
> > > +
> > > + /*
> > > +* For device path only pointing to the removable device
> > > handle, try to
> > > +* search all its children handles
> > > +*/
> > > + ret = EFI_CALL(efi_locate_device_path(_block_io_guid,
> > > _dp, ));
> > > + EFI_CALL(efi_connect_controller(handle, NULL, NULL,
> > > true));
> > > +
> > > + /* Align with edk2, issue a dummy read to the device to
> > > check the device change */
> > > + ret = EFI_CALL(efi_handle_protocol(handle,
> > > _block_io_guid, (void **)_io));
> > > + if (ret == EFI_SUCCESS) {
> > > + buffer = memalign(block_io->media->io_align,
> > > block_io->media->block_size);
> > > + if (buffer) {
> > > + ret =
> > > EFI_CALL(block_io->read_blocks(block_io,
> > > +
> > > block_io->media->media_id,
> > > +0,
> > > +
> > > block_io->media->block_size,
> > > +
> > > buffer));
> > > + free(buffer);
> > > + } else {
> > > + return full_path;
> > > + }
> > > + } else {
> > > + return full_path;
> > > + }
> > > +
> > > + /* detect the default boot file from removable media */
> > > + size = efi_dp_size(boot_dev) - sizeof(struct
> > > efi_device_path);
> > > + EFI_CALL(efi_locate_handle_buffer(BY_PROTOCOL,
> > > +
> > > _simple_file_system_protocol_guid,
> > > + NULL,
> > > +
Re: [PATCH] qcom_defconfig: enable msm8916 and msm8996
On Tue, 07 May 2024 18:46:54 +, Sam Day wrote: > Enable the clock/pinctrl drivers for these two SoCs. Previously left out > due to only being used on the db410c and db820c respectively which both > have their own board code. We can still boot these with most features > working without that board code. > > Applied, thanks! [1/1] qcom_defconfig: enable msm8916 and msm8996 https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/9291868db7d9 Best regards, -- Caleb Connolly
Re: [PATCH] efi_loader: capsule: detect possible ESP device path
On Wed, May 8, 2024 at 9:53 PM Heinrich Schuchardt wrote:
>
> On 5/8/24 14:59, Weizhao Ouyang wrote:
> > On Wed, May 8, 2024 at 7:52 PM Heinrich Schuchardt
> > wrote:
> >>
> >> On 5/8/24 13:24, Weizhao Ouyang wrote:
> >>> When using CapsuleApp to execute on-disk update, it will choose the
> >>> first boot option as BootNext entry to perform the capsule update after
> >>> a reboot. But auto-generate boot option will ignore the logical
> >>> partition which might be an ESP, thus it could not find the capsule
> >>> file.
> >>>
> >>> Align with the EDK II, detect the possible ESP device path by expanding
> >>> the media path.
> >>
> >> Hello Wheizhoa,
> >>
> >> From the description I would not know how to reproduce the problem you
> >> face.
> >>
> >> Could you, please, provide an example (including disk image and capsule)
> >> for QEMU and describe expected and observed behavior.
> >>
> >> We should add a CI test case covering the observed bug.
> >
> > Hi Heinrich,
> >
> > Here is a brief description, I'll add a testcase in the next patch.
> >
> > background:
> > - there is an ESP in an mmc logical partition
> > - in the edk2 shell payload, running "CapsuleApp.efi -OD" to perform
> > the capsule on-disk update
> >
> > workflow:
> > - currently, u-boot will auto-generate boot option and install with
> > eliminating logical partitions. So we will have a boot variable
> > Boot that its file_path seems like: "/VenHw/eMMC(0)/eMMC(0)"
> > - then we run the efi app in the edk shell payload. The CapsuleApp
> > will find this boot entry, and it will detect that there is an ESP
> > in this mmc device. According to the on-disk update flow, it will
> > copy the capsule to the ESP, and add the Boot to the BootNext.
>
> Why do you need an EFI app? Shouldn't the operating system place the
> capsule file?
>
> What makes you expect that there is any ESP on whatever Boot points
> to? You should evaluate BootCurrent if you want to know from which boot
> entry the current application was started from.
CapsuleApp[1] is a shell application that can manually trigger
capsule update process. As one of its flow, it will install the
capsule to the ESP and update the BootNext. The purpose of this
patch is to fix the broken update process and align with edk2.
>
> > - After CapsuleApp.efi reset the device, u-boot will check
> > OsIndications and perform the on-disk update flow. Then it will
>
> U-Boot does not check OsIndications.
I mean the check_run_capsules().
[1]
https://github.com/tianocore/edk2/tree/master/MdeModulePkg/Application/CapsuleApp
BR,
Weizhao
>
> Best regards
>
> Heirnich
>
> > search for the capsule file from the boot entry indicated by the
> > BootNext. Unfortunately, its file_path is "/VenHw/eMMC(0)/eMMC(0)",
> > so the search will fail.
> > - Using this patch, u-boot will expand the Boot file path to
> > detect its ESP, so the capsule file will be found in the expanded
> > device path, eg: "/VenHw/eMMC(0)/eMMC(0)/HD(2)".
> >
> > BR,
> > Weizhao
> >
> >>
> >>>
> >>> Fixes: f86fba8adbf3 ("efi_loader: auto-generate boot option for each
> >>> blkio device")
> >>> Signed-off-by: Weizhao Ouyang
> >>> ---
> >>>include/efi_loader.h | 6 ++
> >>>lib/efi_loader/efi_boottime.c | 15 ++---
> >>>lib/efi_loader/efi_capsule.c | 110 +-
> >>>3 files changed, 118 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/include/efi_loader.h b/include/efi_loader.h
> >>> index 9600941aa327..9d78fa936701 100644
> >>> --- a/include/efi_loader.h
> >>> +++ b/include/efi_loader.h
> >>> @@ -683,6 +683,12 @@ efi_status_t efi_protocol_open(struct efi_handler
> >>> *handler,
> >>> void **protocol_interface, void
> >>> *agent_handle,
> >>> void *controller_handle, uint32_t
> >>> attributes);
> >>>
> >>> +/* Connect drivers to controller */
> >>> +efi_status_t EFIAPI efi_connect_controller(efi_handle_t
> >>> controller_handle,
> >>> +efi_handle_t
> >>> *driver_image_handle,
> >>> +struct efi_device_path
> >>> *remain_device_path,
> >>> +bool recursive);
> >>> +
> >>>/* Install multiple protocol interfaces */
> >>>efi_status_t EFIAPI
> >>>efi_install_multiple_protocol_interfaces(efi_handle_t *handle, ...);
> >>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> >>> index 1951291747cd..2c86d78208b2 100644
> >>> --- a/lib/efi_loader/efi_boottime.c
> >>> +++ b/lib/efi_loader/efi_boottime.c
> >>> @@ -103,12 +103,6 @@ static efi_status_t EFIAPI efi_disconnect_controller(
> >>>efi_handle_t driver_image_handle,
> >>>efi_handle_t child_handle);
> >>>
> >>> -static
> >>> -efi_status_t EFIAPI efi_connect_controller(efi_handle_t
> >>> controller_handle,
> >>> -
Re: [PATCH] efi_loader: capsule: detect possible ESP device path
Hi Ilias,
On Wed, May 8, 2024 at 9:47 PM Ilias Apalodimas
wrote:
>
> Hi Weizhao,
>
> On Wed, 8 May 2024 at 14:24, Weizhao Ouyang wrote:
> >
> > When using CapsuleApp to execute on-disk update, it will choose the
> > first boot option as BootNext entry to perform the capsule update after
> > a reboot.
>
> This is not entirely accurate. The capsule update on-disk mechanism
> will look in the ESP pointed to by BootNext or the highest priority
> BootOrder.
> But the problem you are describing isn't tied only to capsules. IIUC
> if you have a logical partition with bootXXX.efi the current code will
> ignore it as well and the automatic selection of the boot option won't
> work.
> Can you adjust the commit message on v2 and mention the scanning as an
> issue with the capsule on disk being the obvious side-effect?
I'll do.
>
> > But auto-generate boot option will ignore the logical
> > partition which might be an ESP, thus it could not find the capsule
> > file.
> >
> > Align with the EDK II, detect the possible ESP device path by expanding
> > the media path.
>
> >
> > Fixes: f86fba8adbf3 ("efi_loader: auto-generate boot option for each blkio
> > device")
> > Signed-off-by: Weizhao Ouyang
> > ---
> > include/efi_loader.h | 6 ++
> > lib/efi_loader/efi_boottime.c | 15 ++---
> > lib/efi_loader/efi_capsule.c | 110 +-
> > 3 files changed, 118 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 9600941aa327..9d78fa936701 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -683,6 +683,12 @@ efi_status_t efi_protocol_open(struct efi_handler
> > *handler,
> >void **protocol_interface, void
> > *agent_handle,
> >void *controller_handle, uint32_t
> > attributes);
> >
> > +/* Connect drivers to controller */
> > +efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
> > + efi_handle_t
> > *driver_image_handle,
> > + struct efi_device_path
> > *remain_device_path,
> > + bool recursive);
> > +
> > /* Install multiple protocol interfaces */
> > efi_status_t EFIAPI
> > efi_install_multiple_protocol_interfaces(efi_handle_t *handle, ...);
> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > index 1951291747cd..2c86d78208b2 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -103,12 +103,6 @@ static efi_status_t EFIAPI efi_disconnect_controller(
> > efi_handle_t driver_image_handle,
> > efi_handle_t child_handle);
> >
> > -static
> > -efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
> > - efi_handle_t
> > *driver_image_handle,
> > - struct efi_device_path
> > *remain_device_path,
> > - bool recursive);
> > -
> > /* Called on every callback entry */
> > int __efi_entry_check(void)
> > {
> > @@ -3670,11 +3664,10 @@ static efi_status_t efi_connect_single_controller(
> > *
> > * Return: status code
> > */
> > -static efi_status_t EFIAPI efi_connect_controller(
> > - efi_handle_t controller_handle,
> > - efi_handle_t *driver_image_handle,
> > - struct efi_device_path *remain_device_path,
> > - bool recursive)
> > +efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
> > + efi_handle_t
> > *driver_image_handle,
> > + struct efi_device_path
> > *remain_device_path,
> > + bool recursive)
> > {
> > efi_status_t r;
> > efi_status_t ret = EFI_NOT_FOUND;
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > index de0d49ebebda..919e3cba071b 100644
> > --- a/lib/efi_loader/efi_capsule.c
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -922,6 +922,105 @@ static bool device_is_present_and_system_part(struct
> > efi_device_path *dp)
> > return true;
> > }
> >
> > +/**
> > + * get_esp_from_boot_option_file_path - get the expand device path
>
> expanded
Okay.
>
> > + *
> > + * Get a possible efi system partition by expanding a boot option
> > + * file path.
> > + *
> > + * @boot_dev The device path pointing to a boot option
> > + * Return: The full ESP device path or NULL if fail
> > + */
> > +static struct efi_device_path *get_esp_from_boot_option_file_path(struct
> > efi_device_path *boot_dev)
> > +{
> > + efi_status_t ret = EFI_SUCCESS;
> > + efi_handle_t handle;
> > + struct
Re: [PATCH] efi_loader: capsule: detect possible ESP device path
>
> > + *
> > + * Get a possible efi system partition by expanding a boot option
> > + * file path.
> > + *
> > + * @boot_dev The device path pointing to a boot option
> > + * Return: The full ESP device path or NULL if fail
> > + */
> > +static struct efi_device_path *get_esp_from_boot_option_file_path(struct
> > efi_device_path *boot_dev)
> > +{
> > + efi_status_t ret = EFI_SUCCESS;
> > + efi_handle_t handle;
> > + struct efi_device_path *dp = boot_dev;
> > + struct efi_device_path *full_path = NULL;
> > +
> > + ret =
> > EFI_CALL(efi_locate_device_path(_simple_file_system_protocol_guid,
> > + ,
> > + ));
> > + if (ret != EFI_SUCCESS)
> > + ret = EFI_CALL(efi_locate_device_path(_block_io_guid,
> > , ));
> > +
> > + /* Expand Media Device Path */
>
> I don't see where the device path is expanded in this patch.
> We already have try_load_from_media() which tries to do something
> similar. Is anything missing from there?
Looking a bit closer, we do lack calling ConnectController there, but
all this should be added to try_load_from_media() not in a different
path.
Also I don't think we need the Fixes tag
Thanks
/Ilias
>
> > + if (ret == EFI_SUCCESS && EFI_DP_TYPE(dp, END, END)) {
>
> Can you invert the logic here and return immediately?
> (if ret != EFI_SUCCESS ...etc )
> return full_path;
>
> The indentation will go away.
>
> > + struct efi_device_path *temp_dp;
> > + struct efi_block_io *block_io;
> > + void *buffer;
> > + efi_handle_t *simple_file_system_handle;
> > + efi_uintn_t number_handles, index;
> > + u32 size;
> > + u32 temp_size;
> > +
> > + temp_dp = boot_dev;
> > + ret =
> > EFI_CALL(efi_locate_device_path(_simple_file_system_protocol_guid,
> > + _dp,
> > + ));
> > + /*
> > +* For device path pointing to simple file system, it only
> > expands to one
> > +* full path
>
> Why do we have multiple calls to efi_locate_device_path()? Aren't the
> arguments identical?
> Which part of edk2 did you read? Is it BmExpandMediaDevicePath()?
>
>
> > +*/
> > + if (ret == EFI_SUCCESS && EFI_DP_TYPE(temp_dp, END, END)) {
> > + if (device_is_present_and_system_part(temp_dp))
> > + return temp_dp;
> > + }
> > +
> > + /*
> > +* For device path only pointing to the removable device
> > handle, try to
> > +* search all its children handles
> > +*/
> > + ret = EFI_CALL(efi_locate_device_path(_block_io_guid,
> > _dp, ));
> > + EFI_CALL(efi_connect_controller(handle, NULL, NULL, true));
> > +
> > + /* Align with edk2, issue a dummy read to the device to
> > check the device change */
> > + ret = EFI_CALL(efi_handle_protocol(handle,
> > _block_io_guid, (void **)_io));
> > + if (ret == EFI_SUCCESS) {
> > + buffer = memalign(block_io->media->io_align,
> > block_io->media->block_size);
> > + if (buffer) {
> > + ret =
> > EFI_CALL(block_io->read_blocks(block_io,
> > +
> > block_io->media->media_id,
> > +0,
> > +
> > block_io->media->block_size,
> > +
> > buffer));
> > + free(buffer);
> > + } else {
> > + return full_path;
> > + }
> > + } else {
> > + return full_path;
> > + }
> > +
> > + /* detect the default boot file from removable media */
> > + size = efi_dp_size(boot_dev) - sizeof(struct
> > efi_device_path);
> > + EFI_CALL(efi_locate_handle_buffer(BY_PROTOCOL,
> > +
> > _simple_file_system_protocol_guid,
> > + NULL,
> > + _handles,
> > +
> > _file_system_handle));
> > + for (index = 0; index < number_handles; index++) {
> > +
> > EFI_CALL(efi_handle_protocol(simple_file_system_handle[index],
> > +_guid_device_path,
> > +
Re: [PATCH] efi_loader: capsule: detect possible ESP device path
On 5/8/24 14:59, Weizhao Ouyang wrote:
On Wed, May 8, 2024 at 7:52 PM Heinrich Schuchardt wrote:
On 5/8/24 13:24, Weizhao Ouyang wrote:
When using CapsuleApp to execute on-disk update, it will choose the
first boot option as BootNext entry to perform the capsule update after
a reboot. But auto-generate boot option will ignore the logical
partition which might be an ESP, thus it could not find the capsule
file.
Align with the EDK II, detect the possible ESP device path by expanding
the media path.
Hello Wheizhoa,
From the description I would not know how to reproduce the problem you
face.
Could you, please, provide an example (including disk image and capsule)
for QEMU and describe expected and observed behavior.
We should add a CI test case covering the observed bug.
Hi Heinrich,
Here is a brief description, I'll add a testcase in the next patch.
background:
- there is an ESP in an mmc logical partition
- in the edk2 shell payload, running "CapsuleApp.efi -OD" to perform
the capsule on-disk update
workflow:
- currently, u-boot will auto-generate boot option and install with
eliminating logical partitions. So we will have a boot variable
Boot that its file_path seems like: "/VenHw/eMMC(0)/eMMC(0)"
- then we run the efi app in the edk shell payload. The CapsuleApp
will find this boot entry, and it will detect that there is an ESP
in this mmc device. According to the on-disk update flow, it will
copy the capsule to the ESP, and add the Boot to the BootNext.
Why do you need an EFI app? Shouldn't the operating system place the
capsule file?
What makes you expect that there is any ESP on whatever Boot points
to? You should evaluate BootCurrent if you want to know from which boot
entry the current application was started from.
- After CapsuleApp.efi reset the device, u-boot will check
OsIndications and perform the on-disk update flow. Then it will
U-Boot does not check OsIndications.
Best regards
Heirnich
search for the capsule file from the boot entry indicated by the
BootNext. Unfortunately, its file_path is "/VenHw/eMMC(0)/eMMC(0)",
so the search will fail.
- Using this patch, u-boot will expand the Boot file path to
detect its ESP, so the capsule file will be found in the expanded
device path, eg: "/VenHw/eMMC(0)/eMMC(0)/HD(2)".
BR,
Weizhao
Fixes: f86fba8adbf3 ("efi_loader: auto-generate boot option for each blkio
device")
Signed-off-by: Weizhao Ouyang
---
include/efi_loader.h | 6 ++
lib/efi_loader/efi_boottime.c | 15 ++---
lib/efi_loader/efi_capsule.c | 110 +-
3 files changed, 118 insertions(+), 13 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 9600941aa327..9d78fa936701 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -683,6 +683,12 @@ efi_status_t efi_protocol_open(struct efi_handler *handler,
void **protocol_interface, void *agent_handle,
void *controller_handle, uint32_t attributes);
+/* Connect drivers to controller */
+efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
+efi_handle_t *driver_image_handle,
+struct efi_device_path
*remain_device_path,
+bool recursive);
+
/* Install multiple protocol interfaces */
efi_status_t EFIAPI
efi_install_multiple_protocol_interfaces(efi_handle_t *handle, ...);
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 1951291747cd..2c86d78208b2 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -103,12 +103,6 @@ static efi_status_t EFIAPI efi_disconnect_controller(
efi_handle_t driver_image_handle,
efi_handle_t child_handle);
-static
-efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
-efi_handle_t *driver_image_handle,
-struct efi_device_path
*remain_device_path,
-bool recursive);
-
/* Called on every callback entry */
int __efi_entry_check(void)
{
@@ -3670,11 +3664,10 @@ static efi_status_t efi_connect_single_controller(
*
* Return: status code
*/
-static efi_status_t EFIAPI efi_connect_controller(
- efi_handle_t controller_handle,
- efi_handle_t *driver_image_handle,
- struct efi_device_path *remain_device_path,
- bool recursive)
+efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
+efi_handle_t *driver_image_handle,
+struct efi_device_path
*remain_device_path,
+bool recursive)
{
Re: [PATCH] efi_loader: capsule: detect possible ESP device path
Hi Weizhao,
On Wed, 8 May 2024 at 14:24, Weizhao Ouyang wrote:
>
> When using CapsuleApp to execute on-disk update, it will choose the
> first boot option as BootNext entry to perform the capsule update after
> a reboot.
This is not entirely accurate. The capsule update on-disk mechanism
will look in the ESP pointed to by BootNext or the highest priority
BootOrder.
But the problem you are describing isn't tied only to capsules. IIUC
if you have a logical partition with bootXXX.efi the current code will
ignore it as well and the automatic selection of the boot option won't
work.
Can you adjust the commit message on v2 and mention the scanning as an
issue with the capsule on disk being the obvious side-effect?
> But auto-generate boot option will ignore the logical
> partition which might be an ESP, thus it could not find the capsule
> file.
>
> Align with the EDK II, detect the possible ESP device path by expanding
> the media path.
>
> Fixes: f86fba8adbf3 ("efi_loader: auto-generate boot option for each blkio
> device")
> Signed-off-by: Weizhao Ouyang
> ---
> include/efi_loader.h | 6 ++
> lib/efi_loader/efi_boottime.c | 15 ++---
> lib/efi_loader/efi_capsule.c | 110 +-
> 3 files changed, 118 insertions(+), 13 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 9600941aa327..9d78fa936701 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -683,6 +683,12 @@ efi_status_t efi_protocol_open(struct efi_handler
> *handler,
>void **protocol_interface, void *agent_handle,
>void *controller_handle, uint32_t attributes);
>
> +/* Connect drivers to controller */
> +efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
> + efi_handle_t *driver_image_handle,
> + struct efi_device_path
> *remain_device_path,
> + bool recursive);
> +
> /* Install multiple protocol interfaces */
> efi_status_t EFIAPI
> efi_install_multiple_protocol_interfaces(efi_handle_t *handle, ...);
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 1951291747cd..2c86d78208b2 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -103,12 +103,6 @@ static efi_status_t EFIAPI efi_disconnect_controller(
> efi_handle_t driver_image_handle,
> efi_handle_t child_handle);
>
> -static
> -efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
> - efi_handle_t *driver_image_handle,
> - struct efi_device_path
> *remain_device_path,
> - bool recursive);
> -
> /* Called on every callback entry */
> int __efi_entry_check(void)
> {
> @@ -3670,11 +3664,10 @@ static efi_status_t efi_connect_single_controller(
> *
> * Return: status code
> */
> -static efi_status_t EFIAPI efi_connect_controller(
> - efi_handle_t controller_handle,
> - efi_handle_t *driver_image_handle,
> - struct efi_device_path *remain_device_path,
> - bool recursive)
> +efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
> + efi_handle_t *driver_image_handle,
> + struct efi_device_path
> *remain_device_path,
> + bool recursive)
> {
> efi_status_t r;
> efi_status_t ret = EFI_NOT_FOUND;
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index de0d49ebebda..919e3cba071b 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -922,6 +922,105 @@ static bool device_is_present_and_system_part(struct
> efi_device_path *dp)
> return true;
> }
>
> +/**
> + * get_esp_from_boot_option_file_path - get the expand device path
expanded
> + *
> + * Get a possible efi system partition by expanding a boot option
> + * file path.
> + *
> + * @boot_dev The device path pointing to a boot option
> + * Return: The full ESP device path or NULL if fail
> + */
> +static struct efi_device_path *get_esp_from_boot_option_file_path(struct
> efi_device_path *boot_dev)
> +{
> + efi_status_t ret = EFI_SUCCESS;
> + efi_handle_t handle;
> + struct efi_device_path *dp = boot_dev;
> + struct efi_device_path *full_path = NULL;
> +
> + ret =
> EFI_CALL(efi_locate_device_path(_simple_file_system_protocol_guid,
> + ,
> + ));
> + if (ret != EFI_SUCCESS)
> + ret =
Re: [PATCH] efi_loader: capsule: detect possible ESP device path
Hi Dan,
Thanks for the suggestion, I'll fix them in the next patch.
BR,
Weizhao
On Wed, May 8, 2024 at 8:04 PM Dan Carpenter wrote:
>
> On Wed, May 08, 2024 at 07:24:01PM +0800, Weizhao Ouyang wrote:
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > index de0d49ebebda..919e3cba071b 100644
> > --- a/lib/efi_loader/efi_capsule.c
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -922,6 +922,105 @@ static bool device_is_present_and_system_part(struct
> > efi_device_path *dp)
> > return true;
> > }
> >
> > +/**
> > + * get_esp_from_boot_option_file_path - get the expand device path
> > + *
> > + * Get a possible efi system partition by expanding a boot option
> > + * file path.
> > + *
> > + * @boot_dev The device path pointing to a boot option
> > + * Return: The full ESP device path or NULL if fail
> > + */
> > +static struct efi_device_path *get_esp_from_boot_option_file_path(struct
> > efi_device_path *boot_dev)
> > +{
> > + efi_status_t ret = EFI_SUCCESS;
> > + efi_handle_t handle;
> > + struct efi_device_path *dp = boot_dev;
> > + struct efi_device_path *full_path = NULL;
> > +
> > + ret =
> > EFI_CALL(efi_locate_device_path(_simple_file_system_protocol_guid,
> > + ,
> > + ));
> > + if (ret != EFI_SUCCESS)
> > + ret = EFI_CALL(efi_locate_device_path(_block_io_guid,
> > , ));
> > +
> > + /* Expand Media Device Path */
> > + if (ret == EFI_SUCCESS && EFI_DP_TYPE(dp, END, END)) {
>
> Flip this around. Always do failure handling. Never do success
> handling. full_path is always NULL. Just return that. Delete
> the variable.
>
> if (ret != EFI_SUCCESS ||
> !EFI_DP_TYPE(dp, END, END))
> return NULL;
>
>
> > + struct efi_device_path *temp_dp;
> > + struct efi_block_io *block_io;
> > + void *buffer;
> > + efi_handle_t *simple_file_system_handle;
> > + efi_uintn_t number_handles, index;
> > + u32 size;
> > + u32 temp_size;
> > +
> > + temp_dp = boot_dev;
> > + ret =
> > EFI_CALL(efi_locate_device_path(_simple_file_system_protocol_guid,
> > + _dp,
> > + ));
> > + /*
> > + * For device path pointing to simple file system, it only
> > expands to one
> > + * full path
> > + */
> > + if (ret == EFI_SUCCESS && EFI_DP_TYPE(temp_dp, END, END)) {
>
> "Always" was hyperbole. This success handling is fine.
>
> > + if (device_is_present_and_system_part(temp_dp))
> > + return temp_dp;
> > + }
> > +
> > + /*
> > + * For device path only pointing to the removable device
> > handle, try to
> > + * search all its children handles
> > + */
> > + ret = EFI_CALL(efi_locate_device_path(_block_io_guid,
> > _dp, ));
>
> ret is not used. Delete.
>
> > + EFI_CALL(efi_connect_controller(handle, NULL, NULL, true));
> > +
> > + /* Align with edk2, issue a dummy read to the device to check
> > the device change */
> > + ret = EFI_CALL(efi_handle_protocol(handle,
> > _block_io_guid, (void **)_io));
> > + if (ret == EFI_SUCCESS) {
>
> if (ret != EFI_SUCCESS)
> return NULL;
>
> > + buffer = memalign(block_io->media->io_align,
> > block_io->media->block_size);
> > + if (buffer) {
>
> if (!buffer)
> return NULL;
>
>
> > + ret = EFI_CALL(block_io->read_blocks(block_io,
> > +
> > block_io->media->media_id,
> > + 0,
> > +
> > block_io->media->block_size,
> > + buffer));
>
> Delete the unused "ret = " assignment.
>
> > + free(buffer);
> > + } else {
> > + return full_path;
> > + }
> > + } else {
> > + return full_path;
> > + }
> > +
> > + /* detect the default boot file from removable media */
> > + size = efi_dp_size(boot_dev) - sizeof(struct efi_device_path);
> > + EFI_CALL(efi_locate_handle_buffer(BY_PROTOCOL,
> > +
> > _simple_file_system_protocol_guid,
> > + NULL,
> > + _handles,
> > +
Re: [PATCH] efi_loader: capsule: detect possible ESP device path
On Wed, May 8, 2024 at 7:52 PM Heinrich Schuchardt wrote:
>
> On 5/8/24 13:24, Weizhao Ouyang wrote:
> > When using CapsuleApp to execute on-disk update, it will choose the
> > first boot option as BootNext entry to perform the capsule update after
> > a reboot. But auto-generate boot option will ignore the logical
> > partition which might be an ESP, thus it could not find the capsule
> > file.
> >
> > Align with the EDK II, detect the possible ESP device path by expanding
> > the media path.
>
> Hello Wheizhoa,
>
> From the description I would not know how to reproduce the problem you
> face.
>
> Could you, please, provide an example (including disk image and capsule)
> for QEMU and describe expected and observed behavior.
>
> We should add a CI test case covering the observed bug.
Hi Heinrich,
Here is a brief description, I'll add a testcase in the next patch.
background:
- there is an ESP in an mmc logical partition
- in the edk2 shell payload, running "CapsuleApp.efi -OD" to perform
the capsule on-disk update
workflow:
- currently, u-boot will auto-generate boot option and install with
eliminating logical partitions. So we will have a boot variable
Boot that its file_path seems like: "/VenHw/eMMC(0)/eMMC(0)"
- then we run the efi app in the edk shell payload. The CapsuleApp
will find this boot entry, and it will detect that there is an ESP
in this mmc device. According to the on-disk update flow, it will
copy the capsule to the ESP, and add the Boot to the BootNext.
- After CapsuleApp.efi reset the device, u-boot will check
OsIndications and perform the on-disk update flow. Then it will
search for the capsule file from the boot entry indicated by the
BootNext. Unfortunately, its file_path is "/VenHw/eMMC(0)/eMMC(0)",
so the search will fail.
- Using this patch, u-boot will expand the Boot file path to
detect its ESP, so the capsule file will be found in the expanded
device path, eg: "/VenHw/eMMC(0)/eMMC(0)/HD(2)".
BR,
Weizhao
>
> >
> > Fixes: f86fba8adbf3 ("efi_loader: auto-generate boot option for each blkio
> > device")
> > Signed-off-by: Weizhao Ouyang
> > ---
> > include/efi_loader.h | 6 ++
> > lib/efi_loader/efi_boottime.c | 15 ++---
> > lib/efi_loader/efi_capsule.c | 110 +-
> > 3 files changed, 118 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 9600941aa327..9d78fa936701 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -683,6 +683,12 @@ efi_status_t efi_protocol_open(struct efi_handler
> > *handler,
> > void **protocol_interface, void *agent_handle,
> > void *controller_handle, uint32_t attributes);
> >
> > +/* Connect drivers to controller */
> > +efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
> > +efi_handle_t *driver_image_handle,
> > +struct efi_device_path
> > *remain_device_path,
> > +bool recursive);
> > +
> > /* Install multiple protocol interfaces */
> > efi_status_t EFIAPI
> > efi_install_multiple_protocol_interfaces(efi_handle_t *handle, ...);
> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > index 1951291747cd..2c86d78208b2 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -103,12 +103,6 @@ static efi_status_t EFIAPI efi_disconnect_controller(
> > efi_handle_t driver_image_handle,
> > efi_handle_t child_handle);
> >
> > -static
> > -efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
> > -efi_handle_t *driver_image_handle,
> > -struct efi_device_path
> > *remain_device_path,
> > -bool recursive);
> > -
> > /* Called on every callback entry */
> > int __efi_entry_check(void)
> > {
> > @@ -3670,11 +3664,10 @@ static efi_status_t efi_connect_single_controller(
> >*
> >* Return: status code
> >*/
> > -static efi_status_t EFIAPI efi_connect_controller(
> > - efi_handle_t controller_handle,
> > - efi_handle_t *driver_image_handle,
> > - struct efi_device_path *remain_device_path,
> > - bool recursive)
> > +efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
> > +efi_handle_t *driver_image_handle,
> > +struct efi_device_path
> > *remain_device_path,
> > +bool recursive)
> > {
> > efi_status_t r;
> > efi_status_t ret = EFI_NOT_FOUND;
> > diff --git a/lib/efi_loader/efi_capsule.c
Re: [PATCH v2 1/2] clk/qcom: apq8016: add support for USB_HS clocks
On 06/05/2024 12:26, Sam Day wrote:
The newer "register map for simple gate clocks" support added for qcom
clocks is used. As a result gcc_apq8016 now has a mixture of the old and
new styles. I didn't (and still don't!) feel comfortable enough in this
area to update the existing code.
Thanks for this!
Reviewed-by: Caleb Connolly
Signed-off-by: Sam Day
---
drivers/clk/qcom/clock-apq8016.c | 32
1 file changed, 32 insertions(+)
diff --git a/drivers/clk/qcom/clock-apq8016.c b/drivers/clk/qcom/clock-apq8016.c
index d3b63b9c1a..3b8c88c769 100644
--- a/drivers/clk/qcom/clock-apq8016.c
+++ b/drivers/clk/qcom/clock-apq8016.c
@@ -17,6 +17,8 @@
#include "clock-qcom.h"
+#define USB_HS_SYSTEM_CLK_CMD_RCGR 0x41010
+
/* Clocks: (from CLK_CTL_BASE) */
#define GPLL0_STATUS (0x2101C)
#define APCS_GPLL_ENA_VOTE(0x45000)
@@ -52,6 +54,11 @@ static struct vote_clk gcc_blsp1_ahb_clk = {
.vote_bit = BIT(10),
};
+static const struct gate_clk apq8016_clks[] = {
+ GATE_CLK(GCC_USB_HS_AHB_CLK,0x41008, 0x0001),
+ GATE_CLK(GCC_USB_HS_SYSTEM_CLK, 0x41004, 0x0001),
+};
+
/* SDHCI */
static int apq8016_clk_init_sdc(struct msm_clk_priv *priv, int slot, uint
rate)
{
@@ -117,13 +124,38 @@ static ulong apq8016_clk_set_rate(struct clk *clk, ulong
rate)
case GCC_BLSP1_UART2_APPS_CLK: /* UART2 */
apq8016_clk_init_uart(priv->base, clk->id);
return 7372800;
+ case GCC_USB_HS_SYSTEM_CLK:
+ if (rate != 8000)
+ log_warning("Unexpected rate %ld requested for
USB_HS_SYSTEM_CLK\n",
+ rate);
+ clk_rcg_set_rate_mnd(priv->base, USB_HS_SYSTEM_CLK_CMD_RCGR,
+10, 0, 0, CFG_CLK_SRC_GPLL0, 0);
+ return rate;
default:
return 0;
}
}
+static int apq8016_clk_enable(struct clk *clk)
+{
+ struct msm_clk_priv *priv = dev_get_priv(clk->dev);
+
+ if (priv->data->num_clks < clk->id) {
+ log_warning("%s: unknown clk id %lu\n", __func__, clk->id);
+ return 0;
+ }
+
+ debug("%s: clk %s\n", __func__, apq8016_clks[clk->id].name);
+ qcom_gate_clk_en(priv, clk->id);
+
+ return 0;
+}
+
static struct msm_clk_data apq8016_clk_data = {
.set_rate = apq8016_clk_set_rate,
+ .clks = apq8016_clks,
+ .num_clks = ARRAY_SIZE(apq8016_clks),
+ .enable = apq8016_clk_enable,
};
static const struct udevice_id gcc_apq8016_of_match[] = {
--
// Caleb (they/them)
Re: [PATCH] mach-snapdragon: do carveouts for qcs404 only
Hi Sam,
On 08/05/2024 13:40, Sam Day wrote:
Salutations Sumit,
On Wednesday, 8 May 2024 at 11:14 AM, Sumit Garg wrote:
Hi Sam,
On Wed, 8 May 2024 at 00:11, Sam Day m...@samcday.com wrote:
The newly introduced carve_out_reserved_memory causes issues when
U-Boot is chained from the lk2nd bootloader. lk2nd provides a
simple-framebuffer device and marks the framebuffer region as no-map in
the supplied /reserved-memory. Consequently, the simple_video driver
triggers a page fault when it tries to write to this region.
How does the corresponding Linux kernel driver handle this?
Firstly: I'm something of a middle-man here. I would consider Caleb the
authoritative source on the carveouts stuff (since they wrote it) and Nikita
Travkin the authority on the simplefb handoff (since he originally wrote it for
lk2nd to hand off to Linux simpledrm and then adapted it to work with U-Boot
simplefb).
I consulted with Nikita on your first question here. He linked me to this
snippet:
https://elixir.bootlin.com/linux/v6.9-rc7/source/drivers/gpu/drm/tiny/simpledrm.c#L877
Is the
framebuffer region required to be mapped as normal memory or device
type or something else?
So I guess based on the link above, it's just mapped as normal uncached memory.
I tried to do something like this a few days ago in U-Boot, but a) it doesn't
work and b) I have no idea what I'm doing:
https://github.com/samcday/u-boot/commit/c100cb3711ddf5b01601691f3e6a9ec890d9a496
After talking with Caleb about this for a bit, they suggested the patch you see here as
what I guess could be considered a "stopgap" solution that hopefully makes it
into 2024.07.
Similarly would normal memory type work for
all other reserved memory regions marked as no-map?
I'll let Caleb weigh in here. My understanding is that the other regions
*should* be marked as PTE_TYPE_FAULT because otherwise drivers might
inadvertently speculatively access regions that are very much off-limits, such
as TZ app regions.
Right, carving out reserved regions and having the driver handle them is
theoretically the "correct" thing to do here. But I'm not sure that the
additional complexity offers much value from a U-Boot context.
If we can teach the armv8 PT/cache code to handle this better, and teach
all the drivers to map their regions on-demand, this would probably help
us a lot (especially as right now if you attempt to access dead space
between peripherals then you'll hang the bus...). But U-Boot is a ways
away from that.
Cheers,
-Sam
-Sumit
As per Caleb's advice, this simple patch only does the carveouts for the
qcs404 SoC for which it was originally designed. The intent is to do the
carveouts for more Qualcomm SoCs in future.
---
I'm not sure if it's feasible to get this in for the 2024.07 release,
but it'd be great if we could - it's the only thing that breaks U-Boot
master on msm8916 devices that chain from lk2nd.
Signed-off-by: Sam Day m...@samcday.com
---
arch/arm/mach-snapdragon/board.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
index 3d5994c878..b439a19ec7 100644
--- a/arch/arm/mach-snapdragon/board.c
+++ b/arch/arm/mach-snapdragon/board.c
@@ -467,10 +467,12 @@ void enable_caches(void)
gd->arch.tlb_addr = tlb_addr;
gd->arch.tlb_size = tlb_size;
- carveout_start = get_timer(0);
- /* Takes ~20-50ms on SDM845 /
- carve_out_reserved_memory();
- debug("carveout time: %lums\n", get_timer(carveout_start));
-
+ / We do the carveouts only for QCS404, for now. /
+ if (fdt_node_check_compatible(gd->fdt_blob, 0, "qcom,qcs404") == 0) {
+ carveout_start = get_timer(0);
+ / Takes ~20-50ms on SDM845 */
+ carve_out_reserved_memory();
+ debug("carveout time: %lums\n", get_timer(carveout_start));
+ }
dcache_enable();
}
---
base-commit: 1c40dda60f5f7e83a6d6f541cf5a57eb7e8ec43c
change-id: 20240507-qcs404-carveout-only-7a15bbf3fd89
Best regards,
--
Sam Day m...@samcday.com
--
// Caleb (they/them)
[PATCH 6/6] arm: dts: k3-j721s2|am68: Migrate to OF_UPSTREAM
Use OF_UPSTREAM to pull Linux DT from dts/ tree
Signed-off-by: Manorit Chawdhry
---
arch/arm/dts/Makefile |4 +-
arch/arm/dts/k3-am68-sk-base-board-u-boot.dtsi | 20 +-
arch/arm/dts/k3-am68-sk-base-board.dts | 611 ---
arch/arm/dts/k3-am68-sk-som.dtsi | 259 ---
arch/arm/dts/k3-j721s2-binman.dtsi |2 +-
.../dts/k3-j721s2-common-proc-board-u-boot.dtsi| 18 +-
arch/arm/dts/k3-j721s2-common-proc-board.dts | 504 -
arch/arm/dts/k3-j721s2-main.dtsi | 1928
arch/arm/dts/k3-j721s2-mcu-wakeup.dtsi | 738
arch/arm/dts/k3-j721s2-som-p0.dtsi | 361
arch/arm/dts/k3-j721s2-thermal.dtsi| 101 -
arch/arm/dts/k3-j721s2.dtsi| 175 --
board/ti/j721s2/MAINTAINERS|8 -
configs/am68_sk_a72_defconfig |6 +-
configs/j721s2_evm_a72_defconfig |5 +-
15 files changed, 23 insertions(+), 4717 deletions(-)
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index c9f1b25ad647..60660f24d942 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -1331,9 +1331,7 @@ dtb-$(CONFIG_SOC_K3_J721E) +=
k3-j721e-common-proc-board.dtb \
k3-j721e-beagleboneai64.dtb \
k3-j721e-r5-beagleboneai64.dtb
-dtb-$(CONFIG_SOC_K3_J721S2) += k3-am68-sk-base-board.dtb\
- k3-am68-sk-r5-base-board.dtb\
- k3-j721s2-common-proc-board.dtb\
+dtb-$(CONFIG_SOC_K3_J721S2) += k3-am68-sk-r5-base-board.dtb\
k3-j721s2-r5-common-proc-board.dtb
dtb-$(CONFIG_SOC_K3_J784S4) += k3-am69-r5-sk.dtb \
diff --git a/arch/arm/dts/k3-am68-sk-base-board-u-boot.dtsi
b/arch/arm/dts/k3-am68-sk-base-board-u-boot.dtsi
index dca588485d41..4b8d73a92d6a 100644
--- a/arch/arm/dts/k3-am68-sk-base-board-u-boot.dtsi
+++ b/arch/arm/dts/k3-am68-sk-base-board-u-boot.dtsi
@@ -19,10 +19,14 @@
_mcu_wakeup {
bootph-all;
+};
- chipid@4314 {
- bootph-all;
- };
+_conf {
+ bootph-all;
+};
+
+ {
+ bootph-all;
};
_navss {
@@ -34,14 +38,6 @@
};
_udmap {
- reg = <0x0 0x285c 0x0 0x100>,
- <0x0 0x284c 0x0 0x4000>,
- <0x0 0x2a80 0x0 0x4>,
- <0x0 0x284a 0x0 0x4000>,
- <0x0 0x2aa0 0x0 0x4>,
- <0x0 0x2840 0x0 0x2000>;
- reg-names = "gcfg", "rchan", "rchanrt", "tchan",
- "tchanrt", "rflow";
bootph-all;
};
@@ -132,7 +128,7 @@
#ifdef CONFIG_TARGET_J721S2_A72_EVM
-#define SPL_AM68_SK_DTB "spl/dts/k3-am68-sk-base-board.dtb"
+#define SPL_AM68_SK_DTB "spl/dts/ti/k3-am68-sk-base-board.dtb"
#define AM68_SK_DTB "u-boot.dtb"
_j721s2_evm_dtb {
diff --git a/arch/arm/dts/k3-am68-sk-base-board.dts
b/arch/arm/dts/k3-am68-sk-base-board.dts
deleted file mode 100644
index 1e1a82f9d2b8..
--- a/arch/arm/dts/k3-am68-sk-base-board.dts
+++ /dev/null
@@ -1,611 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
- *
- * Base Board: https://www.ti.com/lit/zip/SPRR463
- */
-
-/dts-v1/;
-
-#include "k3-am68-sk-som.dtsi"
-#include
-#include
-#include
-
-#include "k3-serdes.h"
-
-/ {
- compatible = "ti,am68-sk", "ti,j721s2";
- model = "Texas Instruments AM68 SK";
-
- chosen {
- stdout-path = "serial2:115200n8";
- };
-
- aliases {
- serial0 = _uart0;
- serial1 = _uart0;
- serial2 = _uart8;
- mmc1 = _sdhci1;
- can0 = _mcan0;
- can1 = _mcan1;
- can2 = _mcan6;
- can3 = _mcan7;
- };
-
- vusb_main: regulator-vusb-main5v0 {
- /* USB MAIN INPUT 5V DC */
- compatible = "regulator-fixed";
- regulator-name = "vusb-main5v0";
- regulator-min-microvolt = <500>;
- regulator-max-microvolt = <500>;
- regulator-always-on;
- regulator-boot-on;
- };
-
- vsys_3v3: regulator-vsys3v3 {
- /* Output of LM5141 */
- compatible = "regulator-fixed";
- regulator-name = "vsys_3v3";
- regulator-min-microvolt = <330>;
- regulator-max-microvolt = <330>;
- vin-supply = <_main>;
- regulator-always-on;
- regulator-boot-on;
- };
-
- vdd_mmc1: regulator-sd {
- /* Output of TPS22918 */
- compatible = "regulator-fixed";
- regulator-name = "vdd_mmc1";
- regulator-min-microvolt = <330>;
- regulator-max-microvolt = <330>;
-
[PATCH ] mtd: spi-nor: Add SPI_NOR_OCTAL_READ flag for mx66uw2g345gx0 flash part
Added SPI_NOR_OCTAL_READ flag for Macronix mx66uw2g345gx0 2Gb(256MB)
NOR Flash memory has been added. Initial testing was conducted on the
Versal NET board using SDR mode, which included basic erase, write,
and read-back operations.
Signed-off-by: Prasad Kummari
---
drivers/mtd/spi/spi-nor-ids.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index 4e83b8c94c..dacc26d7b3 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -275,7 +275,7 @@ const struct flash_info spi_nor_ids[] = {
{ INFO("mx66l2g45g", 0xc2201c, 0, 64 * 1024, 4096, SECT_4K |
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
{ INFO("mx25l1633e", 0xc22415, 0, 64 * 1024, 32, SPI_NOR_QUAD_READ |
SPI_NOR_4B_OPCODES | SECT_4K) },
{ INFO("mx25r6435f", 0xc22817, 0, 64 * 1024, 128, SECT_4K) },
- { INFO("mx66uw2g345gx0", 0xc2943c, 0, 64 * 1024, 4096, SECT_4K |
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+ { INFO("mx66uw2g345gx0", 0xc2943c, 0, 64 * 1024, 4096, SECT_4K |
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES | SPI_NOR_OCTAL_READ) },
{ INFO("mx66lm1g45g",0xc2853b, 0, 64 * 1024, 2048, SECT_4K |
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
{ INFO("mx25lm51245g", 0xc2853a, 0, 64 * 1024, 1024, SECT_4K |
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
{ INFO("mx25lw51245g", 0xc2863a, 0, 64 * 1024, 1024, SECT_4K |
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
--
2.25.1
Re: [PATCH] qcom_defconfig: enable msm8916 and msm8996
Hi Sam, On 07/05/2024 20:46, Sam Day wrote: From: Caleb Connolly Enable the clock/pinctrl drivers for these two SoCs. Previously left out due to only being used on the db410c and db820c respectively which both have their own board code. We can still boot these with most features working without that board code. Signed-off-by: Caleb Connolly --- I nabbed this commit from one of Caleb's work trees. Along with the carveout patch I just sent, this gets U-Boot to successfully boot when chained from lk2nd on msm8916 devices supported by that bootloader. Thanks, it's nice to have this in, even if USB doesn't work. A similar plea on this patch as with the "mach-snapdragon: do carveouts for qcs404 only" patch: if we could scrape these changes into the 2024.07 release that would be positively sublime. Signed-off-by: Sam Day --- configs/qcom_defconfig | 4 1 file changed, 4 insertions(+) diff --git a/configs/qcom_defconfig b/configs/qcom_defconfig index 80ad3b32e1..c438aeef8e 100644 --- a/configs/qcom_defconfig +++ b/configs/qcom_defconfig @@ -37,6 +37,8 @@ CONFIG_OF_LIVE=y CONFIG_OF_BOARD_SETUP=y CONFIG_BUTTON_QCOM_PMIC=y CONFIG_CLK=y +CONFIG_CLK_QCOM_APQ8016=y +CONFIG_CLK_QCOM_APQ8096=y CONFIG_CLK_QCOM_QCM2290=y CONFIG_CLK_QCOM_QCS404=y CONFIG_CLK_QCOM_SDM845=y @@ -73,6 +75,8 @@ CONFIG_PHY_QCOM_QUSB2=y CONFIG_PHY_QCOM_USB_SNPS_FEMTO_V2=y CONFIG_PHY_QCOM_SNPS_EUSB2=y CONFIG_PINCTRL=y +CONFIG_PINCTRL_QCOM_APQ8016=y +CONFIG_PINCTRL_QCOM_APQ8096=y CONFIG_PINCTRL_QCOM_QCM2290=y CONFIG_PINCTRL_QCOM_QCS404=y CONFIG_PINCTRL_QCOM_SDM845=y --- base-commit: 1c40dda60f5f7e83a6d6f541cf5a57eb7e8ec43c change-id: 20240507-qcom-defconfig-msm8916-61c8437a0b2e Best regards, -- // Caleb (they/them)
Re: [PATCH 2/2] soc: ti: pruss: Add support for AM64x
On 30/04/2024 13:46, MD Danish Anwar wrote: > Add support for AM64x by adding it's compatible in pruss driver. > > Signed-off-by: MD Danish Anwar Reviewed-by: Roger Quadros
Re: [PATCH 1/2] remoteproc: pru: Add support for AM64x PRU / RTU cores
On 30/04/2024 13:46, MD Danish Anwar wrote: > Add support for AM64x PRU cores by adding compatibles for AM64x. > > Signed-off-by: MD Danish Anwar Reviewed-by: Roger Quadros
Re: [PATCH] efi_loader: capsule: detect possible ESP device path
On 5/8/24 14:04, Dan Carpenter wrote:
On Wed, May 08, 2024 at 07:24:01PM +0800, Weizhao Ouyang wrote:
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index de0d49ebebda..919e3cba071b 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -922,6 +922,105 @@ static bool device_is_present_and_system_part(struct
efi_device_path *dp)
return true;
}
+/**
+ * get_esp_from_boot_option_file_path - get the expand device path
+ *
+ * Get a possible efi system partition by expanding a boot option
+ * file path.
+ *
+ * @boot_dev The device path pointing to a boot option
+ * Return: The full ESP device path or NULL if fail
+ */
+static struct efi_device_path *get_esp_from_boot_option_file_path(struct
efi_device_path *boot_dev)
+{
+ efi_status_t ret = EFI_SUCCESS;
+ efi_handle_t handle;
+ struct efi_device_path *dp = boot_dev;
+ struct efi_device_path *full_path = NULL;
+
+ ret =
EFI_CALL(efi_locate_device_path(_simple_file_system_protocol_guid,
+ ,
+ ));
+ if (ret != EFI_SUCCESS)
+ ret = EFI_CALL(efi_locate_device_path(_block_io_guid, ,
));
+
+ /* Expand Media Device Path */
+ if (ret == EFI_SUCCESS && EFI_DP_TYPE(dp, END, END)) {
Flip this around. Always do failure handling. Never do success
handling. full_path is always NULL. Just return that. Delete
the variable.
if (ret != EFI_SUCCESS ||
!EFI_DP_TYPE(dp, END, END))
return NULL;
+ struct efi_device_path *temp_dp;
+ struct efi_block_io *block_io;
+ void *buffer;
+ efi_handle_t *simple_file_system_handle;
+ efi_uintn_t number_handles, index;
+ u32 size;
+ u32 temp_size;
+
+ temp_dp = boot_dev;
+ ret =
EFI_CALL(efi_locate_device_path(_simple_file_system_protocol_guid,
+ _dp,
+ ));
+ /*
+* For device path pointing to simple file system, it only
expands to one
+* full path
+*/
+ if (ret == EFI_SUCCESS && EFI_DP_TYPE(temp_dp, END, END)) {
"Always" was hyperbole. This success handling is fine.
+ if (device_is_present_and_system_part(temp_dp))
+ return temp_dp;
+ }
+
+ /*
+* For device path only pointing to the removable device
handle, try to
+* search all its children handles
+*/
+ ret = EFI_CALL(efi_locate_device_path(_block_io_guid, _dp,
));
ret is not used. Delete.
Errors should be handled and not ignored.
+ EFI_CALL(efi_connect_controller(handle, NULL, NULL, true));
+
+ /* Align with edk2, issue a dummy read to the device to check
the device change */
+ ret = EFI_CALL(efi_handle_protocol(handle, _block_io_guid,
(void **)_io));
+ if (ret == EFI_SUCCESS) {
if (ret != EFI_SUCCESS)
return NULL;
+ buffer = memalign(block_io->media->io_align,
block_io->media->block_size);
+ if (buffer) {
if (!buffer)
return NULL;
+ ret = EFI_CALL(block_io->read_blocks(block_io,
+
block_io->media->media_id,
+0,
+
block_io->media->block_size,
+buffer));
Delete the unused "ret = " assignment.
ditto
Best regards
Heinrich
+ free(buffer);
+ } else {
+ return full_path;
+ }
+ } else {
+ return full_path;
+ }
+
+ /* detect the default boot file from removable media */
+ size = efi_dp_size(boot_dev) - sizeof(struct efi_device_path);
+ EFI_CALL(efi_locate_handle_buffer(BY_PROTOCOL,
+
_simple_file_system_protocol_guid,
+ NULL,
+ _handles,
+ _file_system_handle));
+ for (index = 0; index < number_handles; index++) {
+
EFI_CALL(efi_handle_protocol(simple_file_system_handle[index],
+_guid_device_path,
+
Re: [PATCH] efi_loader: capsule: detect possible ESP device path
On Wed, May 08, 2024 at 07:24:01PM +0800, Weizhao Ouyang wrote:
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index de0d49ebebda..919e3cba071b 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -922,6 +922,105 @@ static bool device_is_present_and_system_part(struct
> efi_device_path *dp)
> return true;
> }
>
> +/**
> + * get_esp_from_boot_option_file_path - get the expand device path
> + *
> + * Get a possible efi system partition by expanding a boot option
> + * file path.
> + *
> + * @boot_dev The device path pointing to a boot option
> + * Return: The full ESP device path or NULL if fail
> + */
> +static struct efi_device_path *get_esp_from_boot_option_file_path(struct
> efi_device_path *boot_dev)
> +{
> + efi_status_t ret = EFI_SUCCESS;
> + efi_handle_t handle;
> + struct efi_device_path *dp = boot_dev;
> + struct efi_device_path *full_path = NULL;
> +
> + ret =
> EFI_CALL(efi_locate_device_path(_simple_file_system_protocol_guid,
> + ,
> + ));
> + if (ret != EFI_SUCCESS)
> + ret = EFI_CALL(efi_locate_device_path(_block_io_guid, ,
> ));
> +
> + /* Expand Media Device Path */
> + if (ret == EFI_SUCCESS && EFI_DP_TYPE(dp, END, END)) {
Flip this around. Always do failure handling. Never do success
handling. full_path is always NULL. Just return that. Delete
the variable.
if (ret != EFI_SUCCESS ||
!EFI_DP_TYPE(dp, END, END))
return NULL;
> + struct efi_device_path *temp_dp;
> + struct efi_block_io *block_io;
> + void *buffer;
> + efi_handle_t *simple_file_system_handle;
> + efi_uintn_t number_handles, index;
> + u32 size;
> + u32 temp_size;
> +
> + temp_dp = boot_dev;
> + ret =
> EFI_CALL(efi_locate_device_path(_simple_file_system_protocol_guid,
> + _dp,
> + ));
> + /*
> + * For device path pointing to simple file system, it only
> expands to one
> + * full path
> + */
> + if (ret == EFI_SUCCESS && EFI_DP_TYPE(temp_dp, END, END)) {
"Always" was hyperbole. This success handling is fine.
> + if (device_is_present_and_system_part(temp_dp))
> + return temp_dp;
> + }
> +
> + /*
> + * For device path only pointing to the removable device
> handle, try to
> + * search all its children handles
> + */
> + ret = EFI_CALL(efi_locate_device_path(_block_io_guid,
> _dp, ));
ret is not used. Delete.
> + EFI_CALL(efi_connect_controller(handle, NULL, NULL, true));
> +
> + /* Align with edk2, issue a dummy read to the device to check
> the device change */
> + ret = EFI_CALL(efi_handle_protocol(handle, _block_io_guid,
> (void **)_io));
> + if (ret == EFI_SUCCESS) {
if (ret != EFI_SUCCESS)
return NULL;
> + buffer = memalign(block_io->media->io_align,
> block_io->media->block_size);
> + if (buffer) {
if (!buffer)
return NULL;
> + ret = EFI_CALL(block_io->read_blocks(block_io,
> +
> block_io->media->media_id,
> + 0,
> +
> block_io->media->block_size,
> + buffer));
Delete the unused "ret = " assignment.
> + free(buffer);
> + } else {
> + return full_path;
> + }
> + } else {
> + return full_path;
> + }
> +
> + /* detect the default boot file from removable media */
> + size = efi_dp_size(boot_dev) - sizeof(struct efi_device_path);
> + EFI_CALL(efi_locate_handle_buffer(BY_PROTOCOL,
> +
> _simple_file_system_protocol_guid,
> + NULL,
> + _handles,
> + _file_system_handle));
> + for (index = 0; index < number_handles; index++) {
> +
> EFI_CALL(efi_handle_protocol(simple_file_system_handle[index],
> + _guid_device_path,
> + (void **)_dp));
> +
> +
Re: [PATCH] efi_loader: capsule: detect possible ESP device path
On 5/8/24 13:24, Weizhao Ouyang wrote:
When using CapsuleApp to execute on-disk update, it will choose the
first boot option as BootNext entry to perform the capsule update after
a reboot. But auto-generate boot option will ignore the logical
partition which might be an ESP, thus it could not find the capsule
file.
Align with the EDK II, detect the possible ESP device path by expanding
the media path.
Hello Wheizhoa,
From the description I would not know how to reproduce the problem you
face.
Could you, please, provide an example (including disk image and capsule)
for QEMU and describe expected and observed behavior.
We should add a CI test case covering the observed bug.
Fixes: f86fba8adbf3 ("efi_loader: auto-generate boot option for each blkio
device")
Signed-off-by: Weizhao Ouyang
---
include/efi_loader.h | 6 ++
lib/efi_loader/efi_boottime.c | 15 ++---
lib/efi_loader/efi_capsule.c | 110 +-
3 files changed, 118 insertions(+), 13 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 9600941aa327..9d78fa936701 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -683,6 +683,12 @@ efi_status_t efi_protocol_open(struct efi_handler *handler,
void **protocol_interface, void *agent_handle,
void *controller_handle, uint32_t attributes);
+/* Connect drivers to controller */
+efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
+ efi_handle_t *driver_image_handle,
+ struct efi_device_path
*remain_device_path,
+ bool recursive);
+
/* Install multiple protocol interfaces */
efi_status_t EFIAPI
efi_install_multiple_protocol_interfaces(efi_handle_t *handle, ...);
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 1951291747cd..2c86d78208b2 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -103,12 +103,6 @@ static efi_status_t EFIAPI efi_disconnect_controller(
efi_handle_t driver_image_handle,
efi_handle_t child_handle);
-static
-efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
- efi_handle_t *driver_image_handle,
- struct efi_device_path
*remain_device_path,
- bool recursive);
-
/* Called on every callback entry */
int __efi_entry_check(void)
{
@@ -3670,11 +3664,10 @@ static efi_status_t efi_connect_single_controller(
*
* Return: status code
*/
-static efi_status_t EFIAPI efi_connect_controller(
- efi_handle_t controller_handle,
- efi_handle_t *driver_image_handle,
- struct efi_device_path *remain_device_path,
- bool recursive)
+efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
+ efi_handle_t *driver_image_handle,
+ struct efi_device_path
*remain_device_path,
+ bool recursive)
{
efi_status_t r;
efi_status_t ret = EFI_NOT_FOUND;
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index de0d49ebebda..919e3cba071b 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -922,6 +922,105 @@ static bool device_is_present_and_system_part(struct
efi_device_path *dp)
return true;
}
+/**
+ * get_esp_from_boot_option_file_path - get the expand device path
+ *
+ * Get a possible efi system partition by expanding a boot option
+ * file path.
+ *
+ * @boot_dev The device path pointing to a boot option
+ * Return: The full ESP device path or NULL if fail
+ */
+static struct efi_device_path *get_esp_from_boot_option_file_path(struct
efi_device_path *boot_dev)
+{
+ efi_status_t ret = EFI_SUCCESS;
+ efi_handle_t handle;
+ struct efi_device_path *dp = boot_dev;
+ struct efi_device_path *full_path = NULL;
+
+ ret =
EFI_CALL(efi_locate_device_path(_simple_file_system_protocol_guid,
+ ,
+ ));
+ if (ret != EFI_SUCCESS)
+ ret = EFI_CALL(efi_locate_device_path(_block_io_guid, ,
));
+
+ /* Expand Media Device Path */
+ if (ret == EFI_SUCCESS && EFI_DP_TYPE(dp, END, END)) {
+ struct efi_device_path *temp_dp;
+ struct efi_block_io *block_io;
+ void *buffer;
+ efi_handle_t *simple_file_system_handle;
+ efi_uintn_t number_handles, index;
+ u32 size;
+ u32 temp_size;
+
+
Re: [PATCH] mach-snapdragon: do carveouts for qcs404 only
Salutations Sumit,
On Wednesday, 8 May 2024 at 11:14 AM, Sumit Garg wrote:
>
>
> Hi Sam,
>
> On Wed, 8 May 2024 at 00:11, Sam Day m...@samcday.com wrote:
>
> > The newly introduced carve_out_reserved_memory causes issues when
> > U-Boot is chained from the lk2nd bootloader. lk2nd provides a
> > simple-framebuffer device and marks the framebuffer region as no-map in
> > the supplied /reserved-memory. Consequently, the simple_video driver
> > triggers a page fault when it tries to write to this region.
>
>
> How does the corresponding Linux kernel driver handle this?
Firstly: I'm something of a middle-man here. I would consider Caleb the
authoritative source on the carveouts stuff (since they wrote it) and Nikita
Travkin the authority on the simplefb handoff (since he originally wrote it for
lk2nd to hand off to Linux simpledrm and then adapted it to work with U-Boot
simplefb).
I consulted with Nikita on your first question here. He linked me to this
snippet:
https://elixir.bootlin.com/linux/v6.9-rc7/source/drivers/gpu/drm/tiny/simpledrm.c#L877
> Is the
> framebuffer region required to be mapped as normal memory or device
> type or something else?
So I guess based on the link above, it's just mapped as normal uncached memory.
I tried to do something like this a few days ago in U-Boot, but a) it doesn't
work and b) I have no idea what I'm doing:
https://github.com/samcday/u-boot/commit/c100cb3711ddf5b01601691f3e6a9ec890d9a496
After talking with Caleb about this for a bit, they suggested the patch you see
here as what I guess could be considered a "stopgap" solution that hopefully
makes it into 2024.07.
> Similarly would normal memory type work for
> all other reserved memory regions marked as no-map?
I'll let Caleb weigh in here. My understanding is that the other regions
*should* be marked as PTE_TYPE_FAULT because otherwise drivers might
inadvertently speculatively access regions that are very much off-limits, such
as TZ app regions.
Cheers,
-Sam
>
> -Sumit
>
> > As per Caleb's advice, this simple patch only does the carveouts for the
> > qcs404 SoC for which it was originally designed. The intent is to do the
> > carveouts for more Qualcomm SoCs in future.
> >
> > ---
> > I'm not sure if it's feasible to get this in for the 2024.07 release,
> > but it'd be great if we could - it's the only thing that breaks U-Boot
> > master on msm8916 devices that chain from lk2nd.
> >
> > Signed-off-by: Sam Day m...@samcday.com
> > ---
> > arch/arm/mach-snapdragon/board.c | 12 +++-
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm/mach-snapdragon/board.c
> > b/arch/arm/mach-snapdragon/board.c
> > index 3d5994c878..b439a19ec7 100644
> > --- a/arch/arm/mach-snapdragon/board.c
> > +++ b/arch/arm/mach-snapdragon/board.c
> > @@ -467,10 +467,12 @@ void enable_caches(void)
> > gd->arch.tlb_addr = tlb_addr;
> > gd->arch.tlb_size = tlb_size;
> >
> > - carveout_start = get_timer(0);
> > - /* Takes ~20-50ms on SDM845 /
> > - carve_out_reserved_memory();
> > - debug("carveout time: %lums\n", get_timer(carveout_start));
> > -
> > + / We do the carveouts only for QCS404, for now. /
> > + if (fdt_node_check_compatible(gd->fdt_blob, 0, "qcom,qcs404") == 0) {
> > + carveout_start = get_timer(0);
> > + / Takes ~20-50ms on SDM845 */
> > + carve_out_reserved_memory();
> > + debug("carveout time: %lums\n", get_timer(carveout_start));
> > + }
> > dcache_enable();
> > }
> >
> > ---
> > base-commit: 1c40dda60f5f7e83a6d6f541cf5a57eb7e8ec43c
> > change-id: 20240507-qcs404-carveout-only-7a15bbf3fd89
> >
> > Best regards,
> > --
> > Sam Day m...@samcday.com
[PATCH] efi_loader: capsule: detect possible ESP device path
When using CapsuleApp to execute on-disk update, it will choose the
first boot option as BootNext entry to perform the capsule update after
a reboot. But auto-generate boot option will ignore the logical
partition which might be an ESP, thus it could not find the capsule
file.
Align with the EDK II, detect the possible ESP device path by expanding
the media path.
Fixes: f86fba8adbf3 ("efi_loader: auto-generate boot option for each blkio
device")
Signed-off-by: Weizhao Ouyang
---
include/efi_loader.h | 6 ++
lib/efi_loader/efi_boottime.c | 15 ++---
lib/efi_loader/efi_capsule.c | 110 +-
3 files changed, 118 insertions(+), 13 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 9600941aa327..9d78fa936701 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -683,6 +683,12 @@ efi_status_t efi_protocol_open(struct efi_handler *handler,
void **protocol_interface, void *agent_handle,
void *controller_handle, uint32_t attributes);
+/* Connect drivers to controller */
+efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
+ efi_handle_t *driver_image_handle,
+ struct efi_device_path
*remain_device_path,
+ bool recursive);
+
/* Install multiple protocol interfaces */
efi_status_t EFIAPI
efi_install_multiple_protocol_interfaces(efi_handle_t *handle, ...);
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 1951291747cd..2c86d78208b2 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -103,12 +103,6 @@ static efi_status_t EFIAPI efi_disconnect_controller(
efi_handle_t driver_image_handle,
efi_handle_t child_handle);
-static
-efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
- efi_handle_t *driver_image_handle,
- struct efi_device_path
*remain_device_path,
- bool recursive);
-
/* Called on every callback entry */
int __efi_entry_check(void)
{
@@ -3670,11 +3664,10 @@ static efi_status_t efi_connect_single_controller(
*
* Return: status code
*/
-static efi_status_t EFIAPI efi_connect_controller(
- efi_handle_t controller_handle,
- efi_handle_t *driver_image_handle,
- struct efi_device_path *remain_device_path,
- bool recursive)
+efi_status_t EFIAPI efi_connect_controller(efi_handle_t controller_handle,
+ efi_handle_t *driver_image_handle,
+ struct efi_device_path
*remain_device_path,
+ bool recursive)
{
efi_status_t r;
efi_status_t ret = EFI_NOT_FOUND;
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index de0d49ebebda..919e3cba071b 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -922,6 +922,105 @@ static bool device_is_present_and_system_part(struct
efi_device_path *dp)
return true;
}
+/**
+ * get_esp_from_boot_option_file_path - get the expand device path
+ *
+ * Get a possible efi system partition by expanding a boot option
+ * file path.
+ *
+ * @boot_dev The device path pointing to a boot option
+ * Return: The full ESP device path or NULL if fail
+ */
+static struct efi_device_path *get_esp_from_boot_option_file_path(struct
efi_device_path *boot_dev)
+{
+ efi_status_t ret = EFI_SUCCESS;
+ efi_handle_t handle;
+ struct efi_device_path *dp = boot_dev;
+ struct efi_device_path *full_path = NULL;
+
+ ret =
EFI_CALL(efi_locate_device_path(_simple_file_system_protocol_guid,
+ ,
+ ));
+ if (ret != EFI_SUCCESS)
+ ret = EFI_CALL(efi_locate_device_path(_block_io_guid, ,
));
+
+ /* Expand Media Device Path */
+ if (ret == EFI_SUCCESS && EFI_DP_TYPE(dp, END, END)) {
+ struct efi_device_path *temp_dp;
+ struct efi_block_io *block_io;
+ void *buffer;
+ efi_handle_t *simple_file_system_handle;
+ efi_uintn_t number_handles, index;
+ u32 size;
+ u32 temp_size;
+
+ temp_dp = boot_dev;
+ ret =
EFI_CALL(efi_locate_device_path(_simple_file_system_protocol_guid,
+ _dp,
+ ));
+ /*
+* For device path pointing to simple file system, it only
expands
[PATCH] arm: rockchip: using generic capsule update mechanism
Currently Rockchip's capsule update mechanism only accepts capsules in
form of a mmc partition, but a generic capsule update mechanism should
be used to satisfy the universal requirements.
Signed-off-by: Weizhao Ouyang
---
arch/arm/mach-rockchip/board.c | 153 --
board/radxa/rockpi4-rk3399/rockpi4-rk3399.c | 166 +++-
2 files changed, 158 insertions(+), 161 deletions(-)
diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
index cd226844b6..73fbb3f58c 100644
--- a/arch/arm/mach-rockchip/board.c
+++ b/arch/arm/mach-rockchip/board.c
@@ -35,155 +35,6 @@
#include
#include
-#if IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) &&
IS_ENABLED(CONFIG_EFI_PARTITION)
-
-#define DFU_ALT_BUF_LENSZ_1K
-
-static struct efi_fw_image *fw_images;
-
-static bool updatable_image(struct disk_partition *info)
-{
- int i;
- bool ret = false;
- efi_guid_t image_type_guid;
-
- uuid_str_to_bin(info->type_guid, image_type_guid.b,
- UUID_STR_FORMAT_GUID);
-
- for (i = 0; i < update_info.num_images; i++) {
- if (!guidcmp(_images[i].image_type_id, _type_guid)) {
- ret = true;
- break;
- }
- }
-
- return ret;
-}
-
-static void set_image_index(struct disk_partition *info, int index)
-{
- int i;
- efi_guid_t image_type_guid;
-
- uuid_str_to_bin(info->type_guid, image_type_guid.b,
- UUID_STR_FORMAT_GUID);
-
- for (i = 0; i < update_info.num_images; i++) {
- if (!guidcmp(_images[i].image_type_id, _type_guid)) {
- fw_images[i].image_index = index;
- break;
- }
- }
-}
-
-static int get_mmc_desc(struct blk_desc **desc)
-{
- int ret;
- struct mmc *mmc;
- struct udevice *dev;
-
- /*
-* For now the firmware images are assumed to
-* be on the SD card
-*/
- ret = uclass_get_device(UCLASS_MMC, 1, );
- if (ret)
- return -1;
-
- mmc = mmc_get_mmc_dev(dev);
- if (!mmc)
- return -ENODEV;
-
- if ((ret = mmc_init(mmc)))
- return ret;
-
- *desc = mmc_get_blk_desc(mmc);
- if (!*desc)
- return -1;
-
- return 0;
-}
-
-void set_dfu_alt_info(char *interface, char *devstr)
-{
- const char *name;
- bool first = true;
- int p, len, devnum, ret;
- char buf[DFU_ALT_BUF_LEN];
- struct disk_partition info;
- struct blk_desc *desc = NULL;
-
- ret = get_mmc_desc();
- if (ret) {
- log_err("Unable to get mmc desc\n");
- return;
- }
-
- memset(buf, 0, sizeof(buf));
- name = blk_get_uclass_name(desc->uclass_id);
- devnum = desc->devnum;
- len = strlen(buf);
-
- len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
-"%s %d=", name, devnum);
-
- for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
- if (part_get_info(desc, p, ))
- continue;
-
- /* Add entry to dfu_alt_info only for updatable images */
- if (updatable_image()) {
- if (!first)
- len += snprintf(buf + len,
- DFU_ALT_BUF_LEN - len, ";");
-
- len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
- "%s%d_%s part %d %d",
- name, devnum, info.name, devnum, p);
- first = false;
- }
- }
-
- log_debug("dfu_alt_info => %s\n", buf);
- env_set("dfu_alt_info", buf);
-}
-
-__weak void rockchip_capsule_update_board_setup(void)
-{
-}
-
-static void gpt_capsule_update_setup(void)
-{
- int p, i, ret;
- struct disk_partition info;
- struct blk_desc *desc = NULL;
-
- fw_images = update_info.images;
- rockchip_capsule_update_board_setup();
-
- ret = get_mmc_desc();
- if (ret) {
- log_err("Unable to get mmc desc\n");
- return;
- }
-
- for (p = 1, i = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
- if (part_get_info(desc, p, ))
- continue;
-
- /*
-* Since we have a GPT partitioned device, the updatable
-* images could be stored in any order. Populate the
-* image_index at runtime.
-*/
- if (updatable_image()) {
- set_image_index(, i);
- i++;
- }
- }
-}
-#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT && CONFIG_EFI_PARTITION */
-
__weak int rk_board_late_init(void)
{
return 0;
@@ -193,10 +44,6 @@ int board_late_init(void)
{
[PATCH v2] efi_loader: Fix EFI_VARIABLE_APPEND_WRITE hash check
According to UEFI v2.10 spec section 8.2.6, if a caller invokes the
SetVariables() service, it will produce a digest from hash(VariableName,
VendorGuid, Attributes, TimeStamp, DataNew_variable_content), then the
firmware that implements the SetVariable() service will compare the
digest with the result of applying the signer’s public key to the
signature. For EFI variable append write, efitools sign-efi-sig-list has
an option "-a" to add EFI_VARIABLE_APPEND_WRITE attr, and u-boot will
drop this attribute in efi_set_variable_int(). So if a caller uses
"sign-efi-sig-list -a" to create the authenticated variable, this append
write will fail in the u-boot due to "hash check failed".
This patch resumes writing the EFI_VARIABLE_APPEND_WRITE attr to ensure
that the hash check is correct. And also update the "test_efi_secboot"
test case to compliance with the change.
Signed-off-by: Weizhao Ouyang
---
v2: skip attr for efi_var_mem_ins()
---
lib/efi_loader/efi_variable.c | 6 +++---
test/py/tests/test_efi_secboot/conftest.py | 10 ++
test/py/tests/test_efi_secboot/test_authvar.py | 4 ++--
test/py/tests/test_efi_secboot/test_signed.py | 10 +-
4 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 1cc02acb3b..09651d4675 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -288,7 +288,6 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
/* check if a variable exists */
var = efi_var_mem_find(vendor, variable_name, NULL);
append = !!(attributes & EFI_VARIABLE_APPEND_WRITE);
- attributes &= ~EFI_VARIABLE_APPEND_WRITE;
delete = !append && (!data_size || !attributes);
/* check attributes */
@@ -304,7 +303,7 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
/* attributes won't be changed */
if (!delete &&
- ((ro_check && var->attr != attributes) ||
+ ((ro_check && var->attr != (attributes &
~EFI_VARIABLE_APPEND_WRITE)) ||
(!ro_check && ((var->attr & ~EFI_VARIABLE_READ_ONLY)
!= (attributes &
~EFI_VARIABLE_READ_ONLY) {
return EFI_INVALID_PARAMETER;
@@ -378,7 +377,8 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
for (; *old_data; ++old_data)
;
++old_data;
- ret = efi_var_mem_ins(variable_name, vendor, attributes,
+ ret = efi_var_mem_ins(variable_name, vendor,
+ attributes & ~EFI_VARIABLE_APPEND_WRITE,
var->length, old_data, data_size, data,
time);
} else {
diff --git a/test/py/tests/test_efi_secboot/conftest.py
b/test/py/tests/test_efi_secboot/conftest.py
index ff7ac7c810..0fa0747fc7 100644
--- a/test/py/tests/test_efi_secboot/conftest.py
+++ b/test/py/tests/test_efi_secboot/conftest.py
@@ -64,6 +64,12 @@ def efi_boot_env(request, u_boot_config):
check_call('cd %s; %scert-to-efi-sig-list -g %s db1.crt db1.esl;
%ssign-efi-sig-list -t "2020-04-05" -c KEK.crt -k KEK.key db db1.esl db1.auth'
% (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
shell=True)
+# db2 (APPEND_WRITE)
+check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj
/CN=TEST_db2/ -keyout db2.key -out db2.crt -nodes -days 365'
+ % mnt_point, shell=True)
+check_call('cd %s; %scert-to-efi-sig-list -g %s db2.crt db2.esl;
%ssign-efi-sig-list -a -c KEK.crt -k KEK.key db db2.esl db2.auth'
+ % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
+ shell=True)
# dbx (TEST_dbx certificate)
check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj
/CN=TEST_dbx/ -keyout dbx.key -out dbx.crt -nodes -days 365'
% mnt_point, shell=True)
@@ -84,6 +90,10 @@ def efi_boot_env(request, u_boot_config):
check_call('cd %s; %scert-to-efi-hash-list -g %s -s 256 db1.crt
dbx_hash1.crl; %ssign-efi-sig-list -t "2020-04-06" -c KEK.crt -k KEK.key dbx
dbx_hash1.crl dbx_hash1.auth'
% (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
shell=True)
+# dbx_hash2 (digest of TEST_db2 certificate, with APPEND_WRITE)
+check_call('cd %s; %scert-to-efi-hash-list -g %s -s 256 db2.crt
dbx_hash2.crl; %ssign-efi-sig-list -a -c KEK.crt -k KEK.key dbx dbx_hash2.crl
dbx_hash2.auth'
+ % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
+ shell=True)
# dbx_db (with TEST_db certificate)
check_call('cd %s; %ssign-efi-sig-list -t "2020-04-05" -c KEK.crt -k
KEK.key dbx db.esl dbx_db.auth'
% (mnt_point,
Re: [PATCH 4/6] arch: arm: dts: k3-am68-sk-r5: Sync with J721s2 R5 file
Hi Manorit,
On 08/05/24 12:56, Manorit Chawdhry wrote:
Update the file with the required nodes from J721s2 R5 file to start
using k3-am68-sk-r5 file for AM68.
Signed-off-by: Manorit Chawdhry
---
What's the motivation behind this patch vs. squashing it into patch 5/6?
arch/arm/dts/k3-am68-sk-r5-base-board.dts | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/arm/dts/k3-am68-sk-r5-base-board.dts
b/arch/arm/dts/k3-am68-sk-r5-base-board.dts
index 695aadc287bd..038b08dc3e01 100644
--- a/arch/arm/dts/k3-am68-sk-r5-base-board.dts
+++ b/arch/arm/dts/k3-am68-sk-r5-base-board.dts
@@ -24,7 +24,8 @@
compatible = "ti,am654-rproc";
reg = <0x0 0x00a9 0x0 0x10>;
power-domains = <_pds 61 TI_SCI_PD_EXCLUSIVE>,
- <_pds 202 TI_SCI_PD_EXCLUSIVE>;
+ <_pds 202 TI_SCI_PD_EXCLUSIVE>,
+ <_pds 4 TI_SCI_PD_EXCLUSIVE>;
resets = <_reset 202 0>;
clocks = <_clks 61 1>;
assigned-clocks = <_clks 61 1>, <_clks 202 0>;
@@ -54,10 +55,12 @@
_proxy_mcu {
bootph-pre-ram;
+ status = "okay";
};
_proxy_sa3 {
bootph-pre-ram;
+ status = "okay";
};
_mcu_wakeup {
--
Thanking You
Neha Malcom Francis
Re: [PATCH v2 07/28] lib: Adapt digest header files to MbedTLS
On Tue, 7 May 2024 at 20:54, Raymond Mao wrote:
>
> Adapt digest header files to support both original libs and MbedTLS
> by switching on/off MBEDTLS_LIB_CRYPTO
>
> FIXME:
> `IS_ENABLED` or `CONFIG_IS_ENABLED` is not applicable here, since
> including causes undefined reference on schedule()
> with sandbox build.
> As includes which enables
> `CONFIG_HW_WATCHDOG` and `CONFIG_WATCHDOG` but no schedule() are
> defined in sandbox build.
> `#if defined(CONFIG_MBEDTLS_LIB_CRYPTO)` is a workaround.
>
> Signed-off-by: Raymond Mao
> ---
> Changes in v2
> - Initial patch.
>
> include/u-boot/md5.h| 17 -
> include/u-boot/sha1.h | 21 -
> include/u-boot/sha256.h | 20
> include/u-boot/sha512.h | 22 +++---
> lib/Makefile| 6 +-
> 5 files changed, 76 insertions(+), 10 deletions(-)
>
> diff --git a/include/u-boot/md5.h b/include/u-boot/md5.h
> index d61364c0ae3..3cfd33a8e56 100644
> --- a/include/u-boot/md5.h
> +++ b/include/u-boot/md5.h
> @@ -6,22 +6,29 @@
> #ifndef _MD5_H
> #define _MD5_H
>
> +#if defined(CONFIG_MBEDTLS_LIB_CRYPTO)
> +#include
> +#endif
> #include "compiler.h"
>
> #define MD5_SUM_LEN16
>
> -struct MD5Context {
> +#if defined(CONFIG_MBEDTLS_LIB_CRYPTO)
> +typedef mbedtls_md5_context MD5Context;
> +#else
> +typedef struct MD5Context {
> __u32 buf[4];
> __u32 bits[2];
> union {
> unsigned char in[64];
> __u32 in32[16];
> };
> -};
> +} MD5Context;
> +#endif
>
> -void MD5Init(struct MD5Context *ctx);
> -void MD5Update(struct MD5Context *ctx, unsigned char const *buf, unsigned
> len);
> -void MD5Final(unsigned char digest[16], struct MD5Context *ctx);
> +void MD5Init(MD5Context *ctx);
> +void MD5Update(MD5Context *ctx, unsigned char const *buf, unsigned int len);
> +void MD5Final(unsigned char digest[16], MD5Context *ctx);
>
> /*
> * Calculate and store in 'output' the MD5 digest of 'len' bytes at
> diff --git a/include/u-boot/sha1.h b/include/u-boot/sha1.h
> index 09fee594d26..ee46fe947a0 100644
> --- a/include/u-boot/sha1.h
> +++ b/include/u-boot/sha1.h
> @@ -14,6 +14,21 @@
> #ifndef _SHA1_H
> #define _SHA1_H
>
> +#if defined(CONFIG_MBEDTLS_LIB_CRYPTO)
> +/*
> + * FIXME:
> + * MbedTLS define the members of "mbedtls_sha256_context" as private,
> + * but "state" needs to be access by arch/arm/cpu/armv8/sha1_ce_glue.
> + * MBEDTLS_ALLOW_PRIVATE_ACCESS needs to be enabled to allow the external
> + * access.
> + * Directly including is not allowed,
> + * since this will include and break the sandbox test.
> + */
> +#define MBEDTLS_ALLOW_PRIVATE_ACCESS
> +
> +#include
> +#endif
> +
> #ifdef __cplusplus
> extern "C" {
> #endif
> @@ -24,6 +39,9 @@ extern "C" {
>
> extern const uint8_t sha1_der_prefix[];
>
> +#if defined(CONFIG_MBEDTLS_LIB_CRYPTO)
> +typedef mbedtls_sha1_context sha1_context;
> +#else
> /**
> * \brief SHA-1 context structure
> */
> @@ -34,13 +52,14 @@ typedef struct
> unsigned char buffer[64]; /*!< data block being processed */
> }
> sha1_context;
> +#endif
>
> /**
> * \brief SHA-1 context setup
> *
> * \param ctx SHA-1 context to be initialized
> */
> -void sha1_starts( sha1_context *ctx );
> +void sha1_starts(sha1_context *ctx);
>
> /**
> * \brief SHA-1 process buffer
> diff --git a/include/u-boot/sha256.h b/include/u-boot/sha256.h
> index 9aa1251789a..e2b7fdd41c8 100644
> --- a/include/u-boot/sha256.h
> +++ b/include/u-boot/sha256.h
> @@ -1,6 +1,22 @@
> #ifndef _SHA256_H
> #define _SHA256_H
>
> +#if defined(CONFIG_MBEDTLS_LIB_CRYPTO)
> +/*
> + * FIXME:
> + * MbedTLS define the members of "mbedtls_sha256_context" as private,
> + * but "state" needs to be access by arch/arm/cpu/armv8/sha256_ce_glue.
'be able to access.'
Isn't the MBEDTLS_ALLOW_PRIVATE_ACCESS considered deprecated?
I'd prefer if we fix this properly.
Thanks
/Ilias
> + * MBEDTLS_ALLOW_PRIVATE_ACCESS needs to be enabled to allow the external
> + * access.
> + * Directly including is not allowed,
> + * since this will include and break the sandbox test.
> + */
> +#define MBEDTLS_ALLOW_PRIVATE_ACCESS
> +
> +#include
> +#endif
> +
> +#define SHA224_SUM_LEN 28
> #define SHA256_SUM_LEN 32
> #define SHA256_DER_LEN 19
>
> @@ -9,11 +25,15 @@ extern const uint8_t sha256_der_prefix[];
> /* Reset watchdog each time we process this many bytes */
> #define CHUNKSZ_SHA256 (64 * 1024)
>
> +#if defined(CONFIG_MBEDTLS_LIB_CRYPTO)
> +typedef mbedtls_sha256_context sha256_context;
> +#else
> typedef struct {
> uint32_t total[2];
> uint32_t state[8];
> uint8_t buffer[64];
> } sha256_context;
> +#endif
>
> void sha256_starts(sha256_context * ctx);
> void sha256_update(sha256_context *ctx, const uint8_t *input, uint32_t
> length);
> diff --git a/include/u-boot/sha512.h b/include/u-boot/sha512.h
> index 516729d7750..a0c0de89d60 100644
> --- a/include/u-boot/sha512.h
> +++
Re: [PATCH 3/6] configs: am68_sk: Move to separate defconfig for AM68 SK board
Hi Manorit On 08/05/24 12:56, Manorit Chawdhry wrote: Add defconfig for AM68 SK R5 and A72 configuration. This includes and modifies the AM68 EVM defconfigs: j721s2_evm_r5_defconfig -> am68_sk_r5_defconfig j721s2_evm_a72_defconfig -> am68_sk_a72_defconfig Signed-off-by: Manorit Chawdhry --- arch/arm/dts/k3-am68-sk-base-board-u-boot.dtsi | 23 +++ arch/arm/dts/k3-j721s2-binman.dtsi | 89 +- board/ti/j721s2/MAINTAINERS| 2 + configs/am68_sk_a72_defconfig | 10 +++ configs/am68_sk_r5_defconfig | 10 +++ configs/j721s2_evm_a72_defconfig | 2 +- configs/j721s2_evm_r5_defconfig| 2 +- 7 files changed, 49 insertions(+), 89 deletions(-) This looks good to me. Reviewed-by: Neha Malcom Francis -- Thanking You Neha Malcom Francis
Re: [PATCH v2 06/28] efi_loader: remove redundant hash includes
On Tue, 7 May 2024 at 20:54, Raymond Mao wrote: > > Remove the redundant includes of u-boot/sha1.h, u-boot/sha256.h > and u-boot/sha512.h > > Signed-off-by: Raymond Mao > --- > Changes in v2 > - None. > > lib/efi_loader/efi_signature.c | 1 - > lib/efi_loader/efi_tcg2.c | 3 --- > 2 files changed, 4 deletions(-) > > diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c > index f338e732759..184eac8cddb 100644 > --- a/lib/efi_loader/efi_signature.c > +++ b/lib/efi_loader/efi_signature.c > @@ -17,7 +17,6 @@ > #include > #include > #include > -#include > > const efi_guid_t efi_guid_sha256 = EFI_CERT_SHA256_GUID; > const efi_guid_t efi_guid_cert_rsa2048 = EFI_CERT_RSA2048_GUID; > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > index b07e0099c27..ac056dcfc55 100644 > --- a/lib/efi_loader/efi_tcg2.c > +++ b/lib/efi_loader/efi_tcg2.c > @@ -19,9 +19,6 @@ > #include > #include > #include > -#include > -#include > -#include > #include > #include > #include > -- > 2.25.1 > Please include this in a different cleanup series along with other fixes. Reviewed-by: Ilias Apalodimas
Re: [PATCH v2 01/28] CI: Exclude MbedTLS subtree for CONFIG checks
On Tue, 7 May 2024 at 20:52, Raymond Mao wrote: > > Since MbedTLS is an external repo with its own coding style, > exclude it from Azure and gitlab CI CONFIG checks. > > Signed-off-by: Raymond Mao > --- > Changes in v2 > - Initial patch. > > .azure-pipelines.yml | 3 ++- > .gitlab-ci.yml | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml > index 27f69583c65..c8052771fa8 100644 > --- a/.azure-pipelines.yml > +++ b/.azure-pipelines.yml > @@ -65,7 +65,8 @@ stages: ># have no matches. >- script: git grep -E '^#[[:blank:]]*(define|undef)[[:blank:]]*CONFIG_' >:^doc/ :^arch/arm/dts/ :^scripts/kconfig/lkc.h > - :^include/linux/kconfig.h :^tools/ :^dts/upstream/ && > + :^include/linux/kconfig.h :^tools/ :^dts/upstream/ > + :^lib/mbedtls/external :^lib/mbedtls/mbedtls_def_config.h > && >exit 1 || exit 0 > >- job: docs > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > index 165f765a833..a8f7f1940f3 100644 > --- a/.gitlab-ci.yml > +++ b/.gitlab-ci.yml > @@ -156,7 +156,8 @@ check for new CONFIG symbols outside Kconfig: > # have no matches. > - git grep -E '^#[[:blank:]]*(define|undef)[[:blank:]]*CONFIG_' > :^doc/ :^arch/arm/dts/ :^scripts/kconfig/lkc.h > -:^include/linux/kconfig.h :^tools/ :^dts/upstream/ && > +:^include/linux/kconfig.h :^tools/ :^dts/upstream/ > +:^lib/mbedtls/external :^lib/mbedtls/mbedtls_def_config.h && > exit 1 || exit 0 > > # build documentation > -- > 2.25.1 > Reviewed-by: Ilias Apalodimas
Re: [PATCH v2 05/28] image: remove redundant hash includes
On Tue, 7 May 2024 at 20:54, Raymond Mao wrote: > > Remove the redundant includes of u-boot/md5.h, u-boot/sha1.h, > u-boot/sha256.h and u-boot/sha512.h > > Signed-off-by: Raymond Mao > --- > Changes in v2 > - None. > > boot/image-fit.c | 4 > boot/image.c | 2 -- > 2 files changed, 6 deletions(-) > > diff --git a/boot/image-fit.c b/boot/image-fit.c > index 89e377563ce..1efc39f4408 100644 > --- a/boot/image-fit.c > +++ b/boot/image-fit.c > @@ -38,10 +38,6 @@ DECLARE_GLOBAL_DATA_PTR; > #include > #include > #include > -#include > -#include > -#include > -#include > > > /*/ > /* New uImage format routines */ > diff --git a/boot/image.c b/boot/image.c > index 073931cd7a3..e57d6eae52d 100644 > --- a/boot/image.c > +++ b/boot/image.c > @@ -26,8 +26,6 @@ > #endif > > #include > -#include > -#include > #include > #include > > -- > 2.25.1 > This seems unrelated to the series of mbedTLS. Can you send a series with the fixes independently? Tom can pick them up before mbedTLS and it would make the review process easier Reviewed-by: Ilias Apalodimas
Re: [PATCH v2] rockchip: add support for Theobroma Systems SOM-RK3588-Q7 Tiger module
Hi Quentin, On 2024/5/8 16:34, Quentin Schulz wrote: Hi Kever, On 5/8/24 4:42 AM, Kever Yang wrote: Hi Quentin, Could you please update this patch with OF_UPSTREAM support? No, I cannot yet :/ Tiger is only available in Linux kernel v6.9-rcX and dts/ in U-Boot is currently at v6.8. I see, then we may need to support the board without OF_UPSTREAM during the dts is available in kernel tree but not available in U-Boot dts/upstream. What are we supposed to do for this then? Would bumping dts/ to an -rc tag be ok for U-Boot? Hi Tom, What's policy of the dts/upstream suppose to update? This is the typical issue there is gap between the dts merge in mainline kernel and available in next release. Thanks, - Kever After all, those are rc and not "stable" branches. Do we need to wait for the actual kernel release before bumping dts/ (so wait for up to 2 months before supporting a board in U-Boot)? Should we have an intermediate solution where we use the "old" model (NOT OF_UPSTREAM) until we bump dts/ to the latest kernel release and the DTS is available through OF_UPSTREAM? Cheers, Quentin
Re: [PATCH] mach-snapdragon: do carveouts for qcs404 only
Hi Sam,
On Wed, 8 May 2024 at 00:11, Sam Day wrote:
>
> The newly introduced carve_out_reserved_memory causes issues when
> U-Boot is chained from the lk2nd bootloader. lk2nd provides a
> simple-framebuffer device and marks the framebuffer region as no-map in
> the supplied /reserved-memory. Consequently, the simple_video driver
> triggers a page fault when it tries to write to this region.
How does the corresponding Linux kernel driver handle this? Is the
framebuffer region required to be mapped as normal memory or device
type or something else? Similarly would normal memory type work for
all other reserved memory regions marked as no-map?
-Sumit
>
> As per Caleb's advice, this simple patch only does the carveouts for the
> qcs404 SoC for which it was originally designed. The intent is to do the
> carveouts for more Qualcomm SoCs in future.
>
> ---
> I'm not sure if it's feasible to get this in for the 2024.07 release,
> but it'd be great if we could - it's the only thing that breaks U-Boot
> master on msm8916 devices that chain from lk2nd.
>
> Signed-off-by: Sam Day
> ---
> arch/arm/mach-snapdragon/board.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-snapdragon/board.c
> b/arch/arm/mach-snapdragon/board.c
> index 3d5994c878..b439a19ec7 100644
> --- a/arch/arm/mach-snapdragon/board.c
> +++ b/arch/arm/mach-snapdragon/board.c
> @@ -467,10 +467,12 @@ void enable_caches(void)
> gd->arch.tlb_addr = tlb_addr;
> gd->arch.tlb_size = tlb_size;
>
> - carveout_start = get_timer(0);
> - /* Takes ~20-50ms on SDM845 */
> - carve_out_reserved_memory();
> - debug("carveout time: %lums\n", get_timer(carveout_start));
> -
> + /* We do the carveouts only for QCS404, for now. */
> + if (fdt_node_check_compatible(gd->fdt_blob, 0, "qcom,qcs404") == 0) {
> + carveout_start = get_timer(0);
> + /* Takes ~20-50ms on SDM845 */
> + carve_out_reserved_memory();
> + debug("carveout time: %lums\n", get_timer(carveout_start));
> + }
> dcache_enable();
> }
>
> ---
> base-commit: 1c40dda60f5f7e83a6d6f541cf5a57eb7e8ec43c
> change-id: 20240507-qcs404-carveout-only-7a15bbf3fd89
>
> Best regards,
> --
> Sam Day
>
>
Does u-boot support USB CCID communication?
Hello, I wanted to know if u-boot can communicate with CCID compliant smartcards or not (with a USB-HSM device). I couldn't find any documents related to this online. USB support seems to be limited within u-boot. Can anyone please let me know if they have experimented with this? Thanks in advance
Re: [PATCH v2] rockchip: add support for Theobroma Systems SOM-RK3588-Q7 Tiger module
Hi Kever, On 5/8/24 4:42 AM, Kever Yang wrote: Hi Quentin, Could you please update this patch with OF_UPSTREAM support? No, I cannot yet :/ Tiger is only available in Linux kernel v6.9-rcX and dts/ in U-Boot is currently at v6.8. What are we supposed to do for this then? Would bumping dts/ to an -rc tag be ok for U-Boot? After all, those are rc and not "stable" branches. Do we need to wait for the actual kernel release before bumping dts/ (so wait for up to 2 months before supporting a board in U-Boot)? Should we have an intermediate solution where we use the "old" model (NOT OF_UPSTREAM) until we bump dts/ to the latest kernel release and the DTS is available through OF_UPSTREAM? Cheers, Quentin
[PATCH 5/6] arch: arm: dts: k3-j721s2-r5: Introduce k3-j721s2-r5.dtsi
Create an SoC R5 dtsi file that could be used at board level R5 files. This
would help in keeping the SoC level changes in sync across board files.
Signed-off-by: Manorit Chawdhry
---
arch/arm/dts/k3-am68-sk-r5-base-board.dts | 78 +---
arch/arm/dts/k3-j721s2-r5-common-proc-board.dts | 78 +---
arch/arm/dts/k3-j721s2-r5.dtsi | 81 +
board/ti/j721s2/MAINTAINERS | 1 +
4 files changed, 84 insertions(+), 154 deletions(-)
diff --git a/arch/arm/dts/k3-am68-sk-r5-base-board.dts
b/arch/arm/dts/k3-am68-sk-r5-base-board.dts
index 038b08dc3e01..3b2d7af2e528 100644
--- a/arch/arm/dts/k3-am68-sk-r5-base-board.dts
+++ b/arch/arm/dts/k3-am68-sk-r5-base-board.dts
@@ -9,80 +9,4 @@
#include "k3-j721s2-ddr-evm-lp4-4266.dtsi"
#include "k3-j721s2-ddr.dtsi"
#include "k3-am68-sk-base-board-u-boot.dtsi"
-
-/ {
- chosen {
- tick-timer = _timer0;
- };
-
- aliases {
- remoteproc0 =
- remoteproc1 = _0;
- };
-
- a72_0: a72@0 {
- compatible = "ti,am654-rproc";
- reg = <0x0 0x00a9 0x0 0x10>;
- power-domains = <_pds 61 TI_SCI_PD_EXCLUSIVE>,
- <_pds 202 TI_SCI_PD_EXCLUSIVE>,
- <_pds 4 TI_SCI_PD_EXCLUSIVE>;
- resets = <_reset 202 0>;
- clocks = <_clks 61 1>;
- assigned-clocks = <_clks 61 1>, <_clks 202 0>;
- assigned-clock-parents = <_clks 61 2>;
- assigned-clock-rates = <2>, <20>;
- ti,sci = <>;
- ti,sci-proc-id = <32>;
- ti,sci-host-id = <10>;
- bootph-pre-ram;
- };
-
- dm_tifs: dm-tifs {
- compatible = "ti,j721e-dm-sci";
- ti,host-id = <3>;
- ti,secure-host;
- mbox-names = "rx", "tx";
- mboxes= <_proxy_mcu 21>,
- <_proxy_mcu 23>;
- bootph-pre-ram;
- };
-};
-
-_timer0 {
- clock-frequency = <25000>;
- bootph-pre-ram;
-};
-
-_proxy_mcu {
- bootph-pre-ram;
- status = "okay";
-};
-
-_proxy_sa3 {
- bootph-pre-ram;
- status = "okay";
-};
-
-_mcu_wakeup {
- sysctrler: sysctrler {
- compatible = "ti,am654-system-controller";
- mboxes= <_proxy_mcu 4>, <_proxy_mcu 5>,
<_proxy_sa3 5>;
- mbox-names = "tx", "rx", "boot_notify";
- bootph-pre-ram;
- };
-};
-
- {
- mboxes= <_proxy_mcu 8>, <_proxy_mcu 6>,
<_proxy_mcu 5>;
- mbox-names = "tx", "rx", "notify";
- ti,host-id = <4>;
- ti,secure-host;
-};
-
-_ringacc {
- ti,sci = <_tifs>;
-};
-
-_udmap {
- ti,sci = <_tifs>;
-};
+#include "k3-j721s2-r5.dtsi"
diff --git a/arch/arm/dts/k3-j721s2-r5-common-proc-board.dts
b/arch/arm/dts/k3-j721s2-r5-common-proc-board.dts
index 03bd680f4421..e92b1917df4e 100644
--- a/arch/arm/dts/k3-j721s2-r5-common-proc-board.dts
+++ b/arch/arm/dts/k3-j721s2-r5-common-proc-board.dts
@@ -9,80 +9,4 @@
#include "k3-j721s2-ddr-evm-lp4-4266.dtsi"
#include "k3-j721s2-ddr.dtsi"
#include "k3-j721s2-common-proc-board-u-boot.dtsi"
-
-/ {
- chosen {
- tick-timer = _timer0;
- };
-
- aliases {
- remoteproc0 =
- remoteproc1 = _0;
- };
-
- a72_0: a72@0 {
- compatible = "ti,am654-rproc";
- reg = <0x0 0x00a9 0x0 0x10>;
- power-domains = <_pds 61 TI_SCI_PD_EXCLUSIVE>,
- <_pds 202 TI_SCI_PD_EXCLUSIVE>,
- <_pds 4 TI_SCI_PD_EXCLUSIVE>;
- resets = <_reset 202 0>;
- clocks = <_clks 61 1>;
- assigned-clocks = <_clks 61 1>, <_clks 202 0>;
- assigned-clock-parents = <_clks 61 2>;
- assigned-clock-rates = <2>, <20>;
- ti,sci = <>;
- ti,sci-proc-id = <32>;
- ti,sci-host-id = <10>;
- bootph-pre-ram;
- };
-
- dm_tifs: dm-tifs {
- compatible = "ti,j721e-dm-sci";
- ti,host-id = <3>;
- ti,secure-host;
- mbox-names = "rx", "tx";
- mboxes= <_proxy_mcu 21>,
- <_proxy_mcu 23>;
- bootph-pre-ram;
- };
-};
-
-_timer0 {
- clock-frequency = <25000>;
- bootph-pre-ram;
-};
-
-_proxy_sa3 {
- bootph-pre-ram;
- status = "okay";
-};
-
-_proxy_mcu {
- bootph-pre-ram;
- status = "okay";
-};
-
-_mcu_wakeup {
- sysctrler: sysctrler {
- compatible = "ti,am654-system-controller";
- mboxes= <_proxy_mcu 4>, <_proxy_mcu 5>,
<_proxy_sa3 5>;
- mbox-names = "tx", "rx", "boot_notify";
- bootph-pre-ram;
-
[PATCH 4/6] arch: arm: dts: k3-am68-sk-r5: Sync with J721s2 R5 file
Update the file with the required nodes from J721s2 R5 file to start
using k3-am68-sk-r5 file for AM68.
Signed-off-by: Manorit Chawdhry
---
arch/arm/dts/k3-am68-sk-r5-base-board.dts | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/arm/dts/k3-am68-sk-r5-base-board.dts
b/arch/arm/dts/k3-am68-sk-r5-base-board.dts
index 695aadc287bd..038b08dc3e01 100644
--- a/arch/arm/dts/k3-am68-sk-r5-base-board.dts
+++ b/arch/arm/dts/k3-am68-sk-r5-base-board.dts
@@ -24,7 +24,8 @@
compatible = "ti,am654-rproc";
reg = <0x0 0x00a9 0x0 0x10>;
power-domains = <_pds 61 TI_SCI_PD_EXCLUSIVE>,
- <_pds 202 TI_SCI_PD_EXCLUSIVE>;
+ <_pds 202 TI_SCI_PD_EXCLUSIVE>,
+ <_pds 4 TI_SCI_PD_EXCLUSIVE>;
resets = <_reset 202 0>;
clocks = <_clks 61 1>;
assigned-clocks = <_clks 61 1>, <_clks 202 0>;
@@ -54,10 +55,12 @@
_proxy_mcu {
bootph-pre-ram;
+ status = "okay";
};
_proxy_sa3 {
bootph-pre-ram;
+ status = "okay";
};
_mcu_wakeup {
--
2.43.2
[PATCH 3/6] configs: am68_sk: Move to separate defconfig for AM68 SK board
Add defconfig for AM68 SK R5 and A72 configuration.
This includes and modifies the AM68 EVM defconfigs:
j721s2_evm_r5_defconfig -> am68_sk_r5_defconfig
j721s2_evm_a72_defconfig -> am68_sk_a72_defconfig
Signed-off-by: Manorit Chawdhry
---
arch/arm/dts/k3-am68-sk-base-board-u-boot.dtsi | 23 +++
arch/arm/dts/k3-j721s2-binman.dtsi | 89 +-
board/ti/j721s2/MAINTAINERS| 2 +
configs/am68_sk_a72_defconfig | 10 +++
configs/am68_sk_r5_defconfig | 10 +++
configs/j721s2_evm_a72_defconfig | 2 +-
configs/j721s2_evm_r5_defconfig| 2 +-
7 files changed, 49 insertions(+), 89 deletions(-)
diff --git a/arch/arm/dts/k3-am68-sk-base-board-u-boot.dtsi
b/arch/arm/dts/k3-am68-sk-base-board-u-boot.dtsi
index b8fc62f0dd1c..dca588485d41 100644
--- a/arch/arm/dts/k3-am68-sk-base-board-u-boot.dtsi
+++ b/arch/arm/dts/k3-am68-sk-base-board-u-boot.dtsi
@@ -129,3 +129,26 @@
dr_mode = "peripheral";
bootph-all;
};
+
+#ifdef CONFIG_TARGET_J721S2_A72_EVM
+
+#define SPL_AM68_SK_DTB "spl/dts/k3-am68-sk-base-board.dtb"
+#define AM68_SK_DTB "u-boot.dtb"
+
+_j721s2_evm_dtb {
+ filename = SPL_AM68_SK_DTB;
+};
+
+_evm_dtb {
+ filename = AM68_SK_DTB;
+};
+
+_j721s2_evm_dtb_unsigned {
+ filename = SPL_AM68_SK_DTB;
+};
+
+_evm_dtb_unsigned {
+ filename = AM68_SK_DTB;
+};
+
+#endif
diff --git a/arch/arm/dts/k3-j721s2-binman.dtsi
b/arch/arm/dts/k3-j721s2-binman.dtsi
index 7efb135bdff9..c46fda66b0b2 100644
--- a/arch/arm/dts/k3-j721s2-binman.dtsi
+++ b/arch/arm/dts/k3-j721s2-binman.dtsi
@@ -142,10 +142,7 @@
#ifdef CONFIG_TARGET_J721S2_A72_EVM
#define SPL_J721S2_EVM_DTB "spl/dts/k3-j721s2-common-proc-board.dtb"
-#define SPL_AM68_SK_DTB "spl/dts/k3-am68-sk-base-board.dtb"
-
#define J721S2_EVM_DTB "u-boot.dtb"
-#define AM68_SK_DTB "arch/arm/dts/k3-am68-sk-base-board.dtb"
{
ti-dm {
@@ -306,20 +303,6 @@
};
};
-
- fdt-1 {
- description = "k3-am68-sk-base-board";
- type = "flat_dt";
- arch = "arm";
- compression = "none";
- ti-secure {
- content = <_am68_sk_dtb>;
- keyfile = "custMpk.pem";
- };
- spl_am68_sk_dtb: blob-ext {
- filename = SPL_AM68_SK_DTB;
- };
- };
};
configurations {
@@ -331,13 +314,6 @@
loadables = "tee", "dm", "spl";
fdt = "fdt-0";
};
-
- conf-1 {
- description = "k3-am68-sk-base-board";
- firmware = "atf";
- loadables = "tee", "dm", "spl";
- fdt = "fdt-1";
- };
};
};
};
@@ -370,25 +346,6 @@
algo = "crc32";
};
};
-
- fdt-1 {
- description = "k3-am68-sk-base-board";
- type = "flat_dt";
- arch = "arm";
- compression = "none";
- ti-secure {
- content = <_sk_dtb>;
- keyfile = "custMpk.pem";
- };
- am68_sk_dtb: blob-ext {
- filename = AM68_SK_DTB;
- };
-
- hash {
- algo = "crc32";
- };
- };
-
};
configurations {
@@ -400,13 +357,6 @@
loadables = "uboot";
fdt = "fdt-0";
};
- conf-1 {
- description = "k3-am68-sk-base-board";
- firmware = "uboot";
-
[PATCH 2/6] configs: j721s2_evm_a72_defconfig: Switch to bootstd
From: Neha Malcom Francis Switch to using bootstd. Note that with this change, we will stop using distro_bootcmd and instead depend entirely on bootflow method of starting the system up. Also config_distro_bootcmd.h header file that is no longer needed in j721s2_evm.h. Signed-off-by: Neha Malcom Francis Signed-off-by: Manorit Chawdhry --- configs/j721s2_evm_a72_defconfig | 5 +++-- include/configs/j721s2_evm.h | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/configs/j721s2_evm_a72_defconfig b/configs/j721s2_evm_a72_defconfig index 19cd44b068c0..8b02d07a9f09 100644 --- a/configs/j721s2_evm_a72_defconfig +++ b/configs/j721s2_evm_a72_defconfig @@ -33,9 +33,10 @@ CONFIG_SPL_SPI=y # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set CONFIG_SPL_LOAD_FIT=y CONFIG_SPL_LOAD_FIT_ADDRESS=0x8100 -CONFIG_DISTRO_DEFAULTS=y CONFIG_OF_SYSTEM_SETUP=y -CONFIG_BOOTCOMMAND="run envboot; run distro_bootcmd;" +CONFIG_BOOTSTD_FULL=y +CONFIG_BOOTSTD_DEFAULTS=y +CONFIG_BOOTCOMMAND="run envboot; bootflow scan -lb" CONFIG_LOGLEVEL=7 CONFIG_SPL_MAX_SIZE=0xc CONFIG_SPL_BOARD_INIT=y diff --git a/include/configs/j721s2_evm.h b/include/configs/j721s2_evm.h index 846cfa7531cc..6186ec32b1d8 100644 --- a/include/configs/j721s2_evm.h +++ b/include/configs/j721s2_evm.h @@ -10,7 +10,6 @@ #define __CONFIG_J721S2_EVM_H #include -#include /* SPL Loader Configuration */ #if defined(CONFIG_TARGET_J721S2_A72_EVM) -- 2.43.2
[PATCH 1/6] board: ti: j721s2: j721s2.env: Add explicit boot_targets
From: Neha Malcom Francis
Add explicit boot_targets to indicate the specific boot sequence to
follow.
Signed-off-by: Neha Malcom Francis
Signed-off-by: Manorit Chawdhry
---
board/ti/j721s2/j721s2.env | 1 +
1 file changed, 1 insertion(+)
diff --git a/board/ti/j721s2/j721s2.env b/board/ti/j721s2/j721s2.env
index 9a03b9f30aee..a6b22550809e 100644
--- a/board/ti/j721s2/j721s2.env
+++ b/board/ti/j721s2/j721s2.env
@@ -13,6 +13,7 @@ args_all=setenv optargs earlycon=ns16550a,mmio32,0x0288
${mtdparts}
run_kern=booti ${loadaddr} ${rd_spec} ${fdtaddr}
+boot_targets=mmc1 mmc0 usb pxe dhcp
boot=mmc
mmcdev=1
bootpart=1:2
--
2.43.2
[PATCH 0/6] Enable OF_UPSTREAM for j721s2 and am68
Series splits am68 and j721s2 support along with enabling OF_UPSTREAM and adding stdboot support for both the platforms. Boot logs: https://gist.github.com/manorit2001/6c669e4273933bc46c3b28a631a96ae3 Signed-off-by: Manorit Chawdhry --- Manorit Chawdhry (4): configs: am68_sk: Move to separate defconfig for AM68 SK board arch: arm: dts: k3-am68-sk-r5: Sync with J721s2 R5 file arch: arm: dts: k3-j721s2-r5: Introduce k3-j721s2-r5.dtsi arm: dts: k3-j721s2|am68: Migrate to OF_UPSTREAM Neha Malcom Francis (2): board: ti: j721s2: j721s2.env: Add explicit boot_targets configs: j721s2_evm_a72_defconfig: Switch to bootstd arch/arm/dts/Makefile |4 +- arch/arm/dts/k3-am68-sk-base-board-u-boot.dtsi | 41 +- arch/arm/dts/k3-am68-sk-base-board.dts | 611 --- arch/arm/dts/k3-am68-sk-r5-base-board.dts | 75 +- arch/arm/dts/k3-am68-sk-som.dtsi | 259 --- arch/arm/dts/k3-j721s2-binman.dtsi | 91 +- .../dts/k3-j721s2-common-proc-board-u-boot.dtsi| 18 +- arch/arm/dts/k3-j721s2-common-proc-board.dts | 504 - arch/arm/dts/k3-j721s2-main.dtsi | 1928 arch/arm/dts/k3-j721s2-mcu-wakeup.dtsi | 738 arch/arm/dts/k3-j721s2-r5-common-proc-board.dts| 78 +- arch/arm/dts/k3-j721s2-r5.dtsi | 81 + arch/arm/dts/k3-j721s2-som-p0.dtsi | 361 arch/arm/dts/k3-j721s2-thermal.dtsi| 101 - arch/arm/dts/k3-j721s2.dtsi| 175 -- board/ti/j721s2/MAINTAINERS| 11 +- board/ti/j721s2/j721s2.env |1 + configs/am68_sk_a72_defconfig | 10 + configs/am68_sk_r5_defconfig | 10 + configs/j721s2_evm_a72_defconfig | 10 +- configs/j721s2_evm_r5_defconfig|2 +- include/configs/j721s2_evm.h |1 - 22 files changed, 155 insertions(+), 4955 deletions(-) --- base-commit: 2f1e76bcfee75b9f99ade63002c05ffaaec86afb change-id: 20240506-b4-upstream-j721s2-of-upstream-3c28ff12d664 Best regards, -- Manorit Chawdhry
Re: [PATCH] imx: hab: add documentation about the required keys/certs
Hi Marek,
On 2024-05-07 3:28 pm, Marek Vasut wrote:
On 5/7/24 3:06 PM, Claudius Heine wrote:
For CST to find the certificates and keys for signing, some keys and
certs need to be copied into the u-boot build directory.
Make sure to CC "NXP i.MX U-Boot Team" , else NXP is not informed. Use
scripts/get_maintainer to get the full list or just reuse the CC list
from patches in this thread.
I send the patch with `--to-cmd scripts/get_maintainer.pl`, maybe I
should have used `--cc-cmd`, but that would not change the list of
recipients.
diff --git a/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt
b/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt
index ce1de659d8..42214df21a 100644
--- a/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt
+++ b/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt
@@ -144,6 +144,22 @@ The signing is activated by wrapping SPL and
fitImage sections into nxp-imx8mcst
etype, which is done automatically in
arch/arm/dts/imx8m{m,n,p,q}-u-boot.dtsi
in case CONFIG_IMX_HAB Kconfig symbol is enabled.
+Per default the HAB keys and certificates need to be located in the
build
+directory, this means copying the following files from the HAB keys
directory
+flat (e.g. removing the `keys` and `cert` subdirectory) into the
u-boot build
+directory for the CST Code Signing Tool to locate them:
Do symlink(s) work too ?
I have not tested it, but I don't see any reason why it would not. I
also don't see a reason for mentioning it. I want to keep it simple, if
the dev whats to do things differently, they are free to do so.
+- `crts/SRK_1_2_3_4_table.bin`
+- `crts/CSF1_1_sha256_4096_65537_v3_usr_crt.pem`
+- `keys/CSF1_1_sha256_4096_65537_v3_usr_key.pem`
+- `crts/IMG1_1_sha256_4096_65537_v3_usr_crt.pem`
+- `keys/IMG1_1_sha256_4096_65537_v3_usr_key.pem`
+- `keys/key_pass.txt`
+
+The paths to the SRK table and the certificates can be modified via
changes to
+the nxp_imx8mcst device tree node
"nodes", plural, there are two, one for SPL and one for fitImage.
Well, I was thinking here more generally about the node type and was
assuming that the person reading this knows how many they have of that
type. But I can add a `s` in v2.
It would be good to mention the DT properties which govern the crypto
material paths -- nxp,srk-table, nxp,csf-crt, nxp,img-crt -- somewhere
around this sentence.
This is something that should be documented with the changes where that
code was added, IMO. I only documented here what I found out and have
used myself, I haven't used those.
I would be interested in reading how to best overwrite those paths and
the image structured from board u-boot.dtsi files myself.
If you want to can pickup my patch and integrate it into your series and
extend it.
regards,
Claudius
--
DENX Software Engineering GmbH,Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de
