[BUG] issues with new bootflow, uefi and virtio
Hi, I am hitting an issue with the new bootflow when booting with UEFI from a virtio device on Qemu Arm. It seems the device number computation in efiload_read_file() does not work in the general virtio case, where it will pick the virtio device number instead of the block device index. On Qemu arm virt machine, many virtio mmio devices are provisioned in the memory map and no assumption can be made on the number of the actual virtio device in use in the general case. This is an extract of the output of `dm tree' on this platform, focused on the virtio device from which I would like to boot: virtio31 [ + ] virtio-mmio |-- virtio_mmio@a003e00 blk0 [ + ] virtio-blk | |-- virtio-blk#31 partition 0 [ + ] blk_partition | | |-- virtio-blk#31:1 partition 1 [ + ] blk_partition | | `-- virtio-blk#31:2 bootdev2 [ + ] virtio_bootdev | `-- virtio-blk#31.bootdev In this extract the actual virtio device number is 31, as will be picked by efiload_read_file(), but the desired block device index is zero, as would be used with e.g. `ls virtio 0'. This can be reproduced for example with Buildroot qemu_aarch64_ebbr_defconfig or qemu_arm_ebbr_defconfig and updating the U-Boot version to v2023.04. This was working properly with U-Boot versions up to v2023.01. This seems to be very specific to virtio, as the numbers align much more nicely for e.g. USB mass storage or NVMe. To help debugging, the following patch forces the device number to zero in the case of virtio, which allows to boot again with UEFI and virtio on Qemu Arm. Hopefully this should give a hint of what is going on. I tried to create a fix by looking for the first child device of UCLASS_BLK, and use its devnum instead in the case of virtio. Sadly, I am not able to confirm if this is a proper fix as I am hitting other boot issues in the case of multiple boot devices of the same class it seems... Vincent. [1]: https://buildroot.org --- boot/bootmeth_efi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 6a97ac02ff5..e5b0d8614ff 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -117,7 +117,9 @@ static int efiload_read_file(struct blk_desc *desc, struct bootflow *bflow) * this can go away. */ media_dev = dev_get_parent(bflow->dev); - snprintf(devnum_str, sizeof(devnum_str), "%x", dev_seq(media_dev)); + snprintf(devnum_str, sizeof(devnum_str), "%x", +device_get_uclass_id(media_dev) == UCLASS_VIRTIO ? 0 : +dev_seq(media_dev)); strlcpy(dirname, bflow->fname, sizeof(dirname)); last_slash = strrchr(dirname, '/'); -- 2.39.2
Re: [BUG] issues with new bootflow, uefi and virtio
On Thu, Apr 06, 2023 at 10:25:15AM +1000, Mathew McBride wrote: .. > I came across the exact same issue a few days ago. Below is a patch which I > believe fixes the problem, by using the devnum of blk uclass (virtio 0) > instead of the sequence number of the parent udevice (e.g virtio-blk#35). Hi Mathew, Thank you for this patch; it works for me and is much simpler that what I had in mind. - Your patch repairs efi with virtio, for Qemu arm and aarch64. - It does not break efi on USB and NVMe (on Qemu). Best regards, Vincent.
Re: [BUG] issues with new bootflow, uefi and virtio
On Thu, Apr 06, 2023 at 06:38:16AM +1200, Simon Glass wrote: (virtio device number 31 vs. blk index 0) > This is strange. Can you try 'dm uclass' to see the sequence number > for the virtio device? I would expect it to be 0, but I might not > fully understand virtio. Hi Simon, Thank you for looking into this. Here is the `dm uclass' extract corresponding to virtio on this Qemu system: uclass 126: virtio 0 * virtio_mmio@a00 @ 7ee977d0, seq 0 1 * virtio_mmio@a000200 @ 7ee97870, seq 1 2 * virtio_mmio@a000400 @ 7ee97910, seq 2 3 * virtio_mmio@a000600 @ 7ee979b0, seq 3 4 * virtio_mmio@a000800 @ 7ee97a50, seq 4 5 * virtio_mmio@a000a00 @ 7ee97af0, seq 5 6 * virtio_mmio@a000c00 @ 7ee97b90, seq 6 7 * virtio_mmio@a000e00 @ 7ee97c30, seq 7 8 * virtio_mmio@a001000 @ 7ee97cd0, seq 8 9 * virtio_mmio@a001200 @ 7ee97d70, seq 9 10 * virtio_mmio@a001400 @ 7ee97e10, seq 10 11 * virtio_mmio@a001600 @ 7ee97eb0, seq 11 12 * virtio_mmio@a001800 @ 7ee97f50, seq 12 13 * virtio_mmio@a001a00 @ 7ee97ff0, seq 13 14 * virtio_mmio@a001c00 @ 7ee98090, seq 14 15 * virtio_mmio@a001e00 @ 7ee98130, seq 15 16 * virtio_mmio@a002000 @ 7ee981d0, seq 16 17 * virtio_mmio@a002200 @ 7ee98270, seq 17 18 * virtio_mmio@a002400 @ 7ee98310, seq 18 19 * virtio_mmio@a002600 @ 7ee983b0, seq 19 20 * virtio_mmio@a002800 @ 7ee98450, seq 20 21 * virtio_mmio@a002a00 @ 7ee984f0, seq 21 22 * virtio_mmio@a002c00 @ 7ee98590, seq 22 23 * virtio_mmio@a002e00 @ 7ee98630, seq 23 24 * virtio_mmio@a003000 @ 7ee986d0, seq 24 25 * virtio_mmio@a003200 @ 7ee98770, seq 25 26 * virtio_mmio@a003400 @ 7ee98810, seq 26 27 * virtio_mmio@a003600 @ 7ee988b0, seq 27 28 * virtio_mmio@a003800 @ 7ee98950, seq 28 29 * virtio_mmio@a003a00 @ 7ee989f0, seq 29 30 * virtio_mmio@a003c00 @ 7ee98a90, seq 30 31 * virtio_mmio@a003e00 @ 7ee98b30, seq 31 (issue with multiple virtio devices) > Please also see this: > > https://patchwork.ozlabs.org/project/uboot/patch/20230402140231.v7.3.Ifa423a8f295b3c11e50821222b0db1e869d0c051@changeid/ > > (or the whole series) Thank you for this patch! When combined with the patch from Mathew[1], it does indeed repair the case of efi boot with two virtio disks, specifically when the first virtio disk is the one we want to boot from. FWIW, the system will not boot when I invert the two virtio disks. Best regards, Vincent. [1]: https://lists.denx.de/pipermail/u-boot/2023-April/514527.html
Re: [BUG] issues with new bootflow, uefi and virtio
On Fri, Apr 07, 2023 at 05:31:06PM +1200, Simon Glass wrote:
..
> > When combined with the patch from Mathew[1], it does indeed repair the case
> > of
> > efi boot with two virtio disks, specifically when the first virtio disk is
> > the
> > one we want to boot from.
> > FWIW, the system will not boot when I invert the two virtio disks.
>
> Is this because it only uses the first virtio device? You could check
> your boot_targets variable. With standard boot you can use 'virtio'
> instead of 'vritio0' and it will find any virtio devices.
Hi Simon,
Thank you for the tips; I did not know that you could use a generic `virtio' or
a more specific `virtio0' specification in boot_targets.
By default the boot_targets variable does indeed contain the generic `virtio' in
my case.
Quick tests matrix:
disk image virtio
(#num) blk index
boot_targets (#30) 0 (#31) 1
--- ---
virtio ok FAIL
virtio0 ok (fail)
virtio1 (fail) ok
This is with both patches, on Qemu.
The fails between () are expected.
I find it interesting that specifying `virtio1' does work when the bootable
image is on the second virtio disk, while auto-detection with `virtio' does not
seem to:
virtio1
~~~
=> setenv boot_targets virtio1
=> boot
Scanning for bootflows in all bootdevs
Seq Method State Uclass Part Name Filename
--- -- - --
Scanning global bootmeth 'efi_mgr':
Scanning bootdev 'virtio-blk#31.bootdev':
0 efiready virtio1 virtio-blk#31.bootdev.par efi/boot/bootaa64.efi
** Booting bootflow 'virtio-blk#31.bootdev.part_1' with efi
Using prior-stage device tree
Missing TPMv2 device for EFI_TCG_PROTOCOL
Booting /efi\boot\bootaa64.efi
virtio
~~
=> setenv boot_targets virtio
=> boot
Scanning for bootflows in all bootdevs
Seq Method State Uclass Part Name Filename
--- -- - --
Scanning global bootmeth 'efi_mgr':
Scanning bootdev 'virtio-blk#30.bootdev':
No more bootdevs
--- -- - --
(0 bootflows, 0 valid)
The messages seem to indicate that virtio #31 / 1 was not even considered when
specifying `virtio'.
(Note that I have edited the logs a bit to avoid wrapping lines.)
Best regards,
Vincent.
>
> >
> > Best regards,
> > Vincent.
> >
> > [1]: https://lists.denx.de/pipermail/u-boot/2023-April/514527.html
>
> Regards,
> Simon
[PATCH] efi_loader: check query_variable_info attributes
QueryVariableInfo() must return EFI_INVALID_PARAMETER when an invalid
combination of attribute bits is supplied.
This fixes three SCT QueryVariableInfo_Conf failures.
Signed-off-by: Vincent Stehlé
Reviewed-by: Grant Likely
Cc: Heinrich Schuchardt
Cc: Alexander Graf
Changes since v1:
- Remove if/else and return directly
---
lib/efi_loader/efi_var_common.c | 4
1 file changed, 4 insertions(+)
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index b11ed91a74a..cbf8685fad5 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -160,6 +160,10 @@ efi_status_t EFIAPI efi_query_variable_info(
EFI_ENTRY("%x %p %p %p", attributes, maximum_variable_storage_size,
remaining_variable_storage_size, maximum_variable_size);
+ if (!attributes || ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) &&
+ !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)))
+ return EFI_EXIT(EFI_INVALID_PARAMETER);
+
ret = efi_query_variable_info_int(attributes,
maximum_variable_storage_size,
remaining_variable_storage_size,
--
2.30.2
[PATCH 0/2] efi: small hii conformance fix
Hi, The following couple of patches fixes a small UEFI HII conformance issue and adds a selftest demonstrating the issue. This is sent in this order to avoid breaking `bootefi selftest' in the middle but feel free to apply in any order if preferred. Best regards, Vincent. Vincent Stehlé (2): efi_loader: fix get_package_list_handle() status efi_selftest: add hii database protocol test case lib/efi_loader/efi_hii.c| 2 +- lib/efi_selftest/efi_selftest_hii.c | 10 ++ 2 files changed, 11 insertions(+), 1 deletion(-) -- 2.35.1
[PATCH 1/2] efi_loader: fix get_package_list_handle() status
When the HII protocol function get_package_list_handle() is called with an
invalid package list handle, it returns EFI_NOT_FOUND but this is not in
its list of possible status codes as per the EFI specification.
Return EFI_INVALID_PARAMETER instead to fix conformance.
Signed-off-by: Vincent Stehlé
Cc: Heinrich Schuchardt
Cc: Ilias Apalodimas
---
lib/efi_loader/efi_hii.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c
index 75ff58aafa5..27db3be6a17 100644
--- a/lib/efi_loader/efi_hii.c
+++ b/lib/efi_loader/efi_hii.c
@@ -780,7 +780,7 @@ get_package_list_handle(const struct
efi_hii_database_protocol *this,
}
}
- return EFI_EXIT(EFI_NOT_FOUND);
+ return EFI_EXIT(EFI_INVALID_PARAMETER);
}
const struct efi_hii_database_protocol efi_hii_database = {
--
2.35.1
[PATCH 2/2] efi_selftest: add hii database protocol test case
Add a test for the case when the HII database protocol
get_package_list_handle() function is called with an invalid package list
handle.
Signed-off-by: Vincent Stehlé
Cc: Heinrich Schuchardt
Cc: Ilias Apalodimas
---
lib/efi_selftest/efi_selftest_hii.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_hii.c
b/lib/efi_selftest/efi_selftest_hii.c
index eaf3b0995d4..8a038d9f534 100644
--- a/lib/efi_selftest/efi_selftest_hii.c
+++ b/lib/efi_selftest/efi_selftest_hii.c
@@ -605,6 +605,16 @@ static int test_hii_database_get_package_list_handle(void)
goto out;
}
+ /* Invalid package list handle. */
+ driver_handle = NULL;
+ ret = hii_database_protocol->get_package_list_handle(
+ hii_database_protocol, NULL, &driver_handle);
+ if (ret != EFI_INVALID_PARAMETER) {
+ efi_st_error("get_package_list_handle returned %u not
invalid\n",
+(unsigned int)ret);
+ goto out;
+ }
+
result = EFI_ST_SUCCESS;
out:
--
2.35.1
[PATCH] efi: adjust ebbr to v2.1 in conformance profile
The EFI Conformance Profile Table entry for EBBR appears in v2.1.0 of the
EBBR specification[1]. Update naming accordingly.
While at it, update the EBBR version referenced in the documentation.
[1]: https://github.com/ARM-software/ebbr/releases/tag/v2.1.0
Signed-off-by: Vincent Stehlé
Cc: Heinrich Schuchardt
Cc: Ilias Apalodimas
---
doc/develop/uefi/uefi.rst| 6 +++---
include/efi_api.h| 2 +-
lib/efi_loader/Kconfig | 6 +++---
lib/efi_loader/efi_conformance.c | 8
lib/efi_selftest/efi_selftest_ecpt.c | 6 +++---
5 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index e0835beba41..a944c0fb803 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -14,7 +14,7 @@ Development target
--
The implementation of UEFI in U-Boot strives to reach the requirements
described
-in the "Embedded Base Boot Requirements (EBBR) Specification - Release v1.0"
+in the "Embedded Base Boot Requirements (EBBR) Specification - Release v2.1.0"
[2]. The "Server Base Boot Requirements System Software on ARM Platforms" [3]
describes a superset of the EBBR specification and may be used as further
reference.
@@ -799,8 +799,8 @@ Links
-
* [1] http://uefi.org/specifications - UEFI specifications
-* [2]
https://github.com/ARM-software/ebbr/releases/download/v1.0/ebbr-v1.0.pdf -
- Embedded Base Boot Requirements (EBBR) Specification - Release v1.0
+* [2]
https://github.com/ARM-software/ebbr/releases/download/v2.1.0/ebbr-v2.1.0.pdf -
+ Embedded Base Boot Requirements (EBBR) Specification - Release v2.1.0
* [3]
https://developer.arm.com/docs/den0044/latest/server-base-boot-requirements-system-software-on-arm-platforms-version-11
-
Server Base Boot Requirements System Software on ARM Platforms - Version 1.1
* [4] :doc:`iscsi`
diff --git a/include/efi_api.h b/include/efi_api.h
index 9bb0d44ac8d..00c98e0984d 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -232,7 +232,7 @@ enum efi_reset_type {
#define EFI_CONFORMANCE_PROFILES_TABLE_VERSION 1
-#define EFI_CONFORMANCE_PROFILE_EBBR_2_0_GUID \
+#define EFI_CONFORMANCE_PROFILE_EBBR_2_1_GUID \
EFI_GUID(0xcce33c35, 0x74ac, 0x4087, 0xbc, 0xe7, \
0x8b, 0x29, 0xb0, 0x2e, 0xeb, 0x27)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index e2b643871bf..b498c72206f 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -384,8 +384,8 @@ config EFI_ECPT
help
Enabling this option created the ECPT UEFI table.
-config EFI_EBBR_2_0_CONFORMANCE
- bool "Add the EBBRv2.0 conformance entry to the ECPT table"
+config EFI_EBBR_2_1_CONFORMANCE
+ bool "Add the EBBRv2.1 conformance entry to the ECPT table"
depends on EFI_ECPT
depends on EFI_LOADER_HII
depends on EFI_RISCV_BOOT_PROTOCOL || !RISCV
@@ -393,7 +393,7 @@ config EFI_EBBR_2_0_CONFORMANCE
depends on EFI_UNICODE_COLLATION_PROTOCOL2
default y
help
- Enabling this option adds the EBBRv2.0 conformance entry to the ECPT
UEFI table.
+ Enabling this option adds the EBBRv2.1 conformance entry to the ECPT
UEFI table.
config EFI_RISCV_BOOT_PROTOCOL
bool "RISCV_EFI_BOOT_PROTOCOL support"
diff --git a/lib/efi_loader/efi_conformance.c b/lib/efi_loader/efi_conformance.c
index a49aae92497..3036d46349a 100644
--- a/lib/efi_loader/efi_conformance.c
+++ b/lib/efi_loader/efi_conformance.c
@@ -12,8 +12,8 @@
#include
static const efi_guid_t efi_ecpt_guid = EFI_CONFORMANCE_PROFILES_TABLE_GUID;
-static const efi_guid_t efi_ebbr_2_0_guid =
- EFI_CONFORMANCE_PROFILE_EBBR_2_0_GUID;
+static const efi_guid_t efi_ebbr_2_1_guid =
+ EFI_CONFORMANCE_PROFILE_EBBR_2_1_GUID;
/**
* efi_ecpt_register() - Install the ECPT system table.
@@ -38,9 +38,9 @@ efi_status_t efi_ecpt_register(void)
return ret;
}
- if (CONFIG_IS_ENABLED(EFI_EBBR_2_0_CONFORMANCE))
+ if (CONFIG_IS_ENABLED(EFI_EBBR_2_1_CONFORMANCE))
guidcpy(&ecpt->conformance_profiles[num_entries++],
- &efi_ebbr_2_0_guid);
+ &efi_ebbr_2_1_guid);
ecpt->version = EFI_CONFORMANCE_PROFILES_TABLE_VERSION;
ecpt->number_of_profiles = num_entries;
diff --git a/lib/efi_selftest/efi_selftest_ecpt.c
b/lib/efi_selftest/efi_selftest_ecpt.c
index e8cc13545db..09c5e96c5e1 100644
--- a/lib/efi_selftest/efi_selftest_ecpt.c
+++ b/lib/efi_selftest/efi_selftest_ecpt.c
@@ -10,7 +10,7 @@
#include
static const efi_guid_t guid_ecpt = EFI_CONFORMANCE_PROFILES_TABLE_GUID;
-static const efi_guid_t guid_ebbr_2_0 = EFI_CONFORMANCE_PROFILE_EBBR_2_0_GUID;
+static const efi_guid_t guid_ebbr_2_1 = EFI_CONFORMANCE_PROFILE_EBBR_2_1_GUID;
/*
* ecpt_find_guid() - find GUID in EFI Conformanc
Re: [PATCH 2/2] efi_selftest: add hii database protocol test case
On Thu, Dec 22, 2022 at 10:45:22AM +0100, Heinrich Schuchardt wrote:
Hi Heinrich,
Happy new year, and thank you for the reviews.
More comments below.
> On 12/13/22 22:39, Vincent Stehlé wrote:
> > Add a test for the case when the HII database protocol
> > get_package_list_handle() function is called with an invalid package list
> > handle.
> >
> > Signed-off-by: Vincent Stehlé
> > Cc: Heinrich Schuchardt
> > Cc: Ilias Apalodimas
> > ---
> > lib/efi_selftest/efi_selftest_hii.c | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/lib/efi_selftest/efi_selftest_hii.c
> > b/lib/efi_selftest/efi_selftest_hii.c
> > index eaf3b0995d4..8a038d9f534 100644
> > --- a/lib/efi_selftest/efi_selftest_hii.c
> > +++ b/lib/efi_selftest/efi_selftest_hii.c
> > @@ -605,6 +605,16 @@ static int
> > test_hii_database_get_package_list_handle(void)
> > goto out;
> > }
> >
> > + /* Invalid package list handle. */
> > + driver_handle = NULL;
> > + ret = hii_database_protocol->get_package_list_handle(
> > + hii_database_protocol, NULL, &driver_handle);
> > + if (ret != EFI_INVALID_PARAMETER) {
>
> Here it is unclear, if you get EFI_INVALID_PARAMETER because the
> PackageListHandle is invalid or DriverHandle is NULL.
In principle, the value used to initialize the (output) parameter driver_handle
should not affect the test. Also, we are initializing driver_handle in exactly
the same way as was done in the previous test, which we know succeeded.
Anyway, if you think it makes the test more readable I can change the
initialization to something like the following:
driver_handle = (efi_handle_t)0x23456789; /* dummy */
>
> We should test both cases separately.
Would you prefer something like the following couple of tests?
/* Invalid package list handle. */
driver_handle = (efi_handle_t)0x23456789; /* dummy */
ret = hii_database_protocol->get_package_list_handle(
hii_database_protocol, NULL, &driver_handle);
/* Invalid driver handle. */
ret = hii_database_protocol->get_package_list_handle(
hii_database_protocol, handle, NULL);
I could verify that EFI_INVALID_PARAMETER is indeed returned in both cases.
Just let me know and I will post a v2.
Best regards,
Vincent.
>
> Best regards
>
> Heinrich
>
> > + efi_st_error("get_package_list_handle returned %u not
> > invalid\n",
> > +(unsigned int)ret);
> > + goto out;
> > + }
> > +
> > result = EFI_ST_SUCCESS;
> >
> > out:
>
[PATCH v2] efi_selftest: add hii database protocol test cases
Add a couple of tests for the cases when the HII database protocol
get_package_list_handle() function is called with invalid parameters.
Signed-off-by: Vincent Stehlé
Cc: Heinrich Schuchardt
Cc: Ilias Apalodimas
---
Hi,
Thanks Heinrich for your review and feedbacks; here is v2 of this patch.
Best regards,
Vincent.
Changes in v2:
- Change initialization of driver_handle to a non-NULL value, which will
never get allocated.
- Add a second test for the case when driver handle is invalid.
v1 reference(s):
- https://lists.denx.de/pipermail/u-boot/2022-December/502213.html
- https://lists.denx.de/pipermail/u-boot/2023-January/503718.html
lib/efi_selftest/efi_selftest_hii.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_hii.c
b/lib/efi_selftest/efi_selftest_hii.c
index eaf3b0995d4..608e3982399 100644
--- a/lib/efi_selftest/efi_selftest_hii.c
+++ b/lib/efi_selftest/efi_selftest_hii.c
@@ -605,6 +605,25 @@ static int test_hii_database_get_package_list_handle(void)
goto out;
}
+ /* Invalid package list handle. */
+ driver_handle = (efi_handle_t)~1UL;
+ ret = hii_database_protocol->get_package_list_handle(
+ hii_database_protocol, NULL, &driver_handle);
+ if (ret != EFI_INVALID_PARAMETER) {
+ efi_st_error("get_package_list_handle returned %u not
invalid\n",
+(unsigned int)ret);
+ goto out;
+ }
+
+ /* Invalid driver handle. */
+ ret = hii_database_protocol->get_package_list_handle(
+ hii_database_protocol, handle, NULL);
+ if (ret != EFI_INVALID_PARAMETER) {
+ efi_st_error("get_package_list_handle returned %u not
invalid\n",
+(unsigned int)ret);
+ goto out;
+ }
+
result = EFI_ST_SUCCESS;
out:
--
2.39.0
[PATCH 0/2] efi: small hii set_keyboard_layout conformance improvement
Hi, The following couple of patches make UEFI HII set_keyboard_layout() more conforming in the case of invalid input parameter and add a selftest. This is sent in this order to avoid breaking `bootefi selftest' in the middle but feel free to apply in any order if preferred. Best regards, Vincent. Vincent Stehlé (2): efi_loader: refine set_keyboard_layout() status efi_selftest: add hii set keyboard layout test case lib/efi_loader/efi_hii.c| 3 +++ lib/efi_selftest/efi_selftest_hii.c | 12 2 files changed, 15 insertions(+) -- 2.39.0
[PATCH 1/2] efi_loader: refine set_keyboard_layout() status
As per the EFI specification, the HII database protocol function
set_keyboard_layout() must return EFI_INVALID_PARAMETER when it is called
with a NULL key_guid argument. Modify the function accordingly to improve
conformance.
Signed-off-by: Vincent Stehlé
Cc: Heinrich Schuchardt
Cc: Ilias Apalodimas
---
lib/efi_loader/efi_hii.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c
index 27db3be6a17..3b54ecb11ac 100644
--- a/lib/efi_loader/efi_hii.c
+++ b/lib/efi_loader/efi_hii.c
@@ -758,6 +758,9 @@ set_keyboard_layout(const struct efi_hii_database_protocol
*this,
{
EFI_ENTRY("%p, %pUs", this, key_guid);
+ if (!key_guid)
+ return EFI_EXIT(EFI_INVALID_PARAMETER);
+
return EFI_EXIT(EFI_NOT_FOUND);
}
--
2.39.0
[PATCH 2/2] efi_selftest: add hii set keyboard layout test case
Add a test for the case when the HII database protocol
set_keyboard_layout() function is called with a NULL key_guid argument.
Signed-off-by: Vincent Stehlé
Cc: Heinrich Schuchardt
Cc: Ilias Apalodimas
---
lib/efi_selftest/efi_selftest_hii.c | 12
1 file changed, 12 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_hii.c
b/lib/efi_selftest/efi_selftest_hii.c
index eaf3b0995d4..f4b55889e29 100644
--- a/lib/efi_selftest/efi_selftest_hii.c
+++ b/lib/efi_selftest/efi_selftest_hii.c
@@ -564,7 +564,19 @@ out:
*/
static int test_hii_database_set_keyboard_layout(void)
{
+ efi_status_t ret;
+
PRINT_TESTNAME;
+
+ /* Invalid key guid. */
+ ret = hii_database_protocol->set_keyboard_layout(
+ hii_database_protocol, NULL);
+ if (ret != EFI_INVALID_PARAMETER) {
+ efi_st_error("set_keyboard_layout returned %u not invalid\n",
+(unsigned int)ret);
+ return EFI_ST_FAILURE;
+ }
+
/* set_keyboard_layout() not implemented yet */
return EFI_ST_SUCCESS;
}
--
2.39.0
Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
On Fri, May 31, 2024 at 03:50:34PM +0200, Caleb Connolly wrote: > As more boards adopt support for the EFI CapsuleUpdate mechanism, there > is a growing issue of being able to target updates to them properly. The > current mechanism of hardcoding UUIDs for each board at compile time is > unsustainable, and maintaining lists of GUIDs is similarly cumbersome. > > In this series, I propose that we adopt v5 GUIDs, these are generated > by using a well-known salt GUID as well as board specific information > (like the model/revision), these are hashed together and the result is > truncated to form a new UUID. Dear Caleb, Thank you for working on this proposal, this looks very useful. Indeed, we found out during SystemReady certifications that it is easy for unique ids to be inadvertently re-used, making them thus non-unique. I have a doubt regarding the format of the generated UUIDs, which end up in the ESRT, though. Here is a quick experiment on the sandbox booting with a DTB using the efidebug command. With the patch series, rebased on the master branch: $ make sandbox_defconfig $ make $ ./u-boot --default_fdt ... U-Boot 2024.07-rc4-00028-g1c96225b0b3 (Jun 19 2024 - 15:29:04 +0200) ... Model: sandbox ... Hit any key to stop autoboot: 0 => efidebug capsule esrt ... ESRT: fw_resource_count=2 ESRT: fw_resource_count_max=2 ESRT: fw_resource_version=1 [entry 0]== ESRT: fw_class=FD5DB83C-12F3-A46B-80A9-E3007C7FF56E ... [entry 1]== ESRT: fw_class=935FE837-FAC8-4394-C008-737D8852C60D ... $ uuid -d FD5DB83C-12F3-A46B-80A9-E3007C7FF56E encode: STR: fd5db83c-12f3-a46b-80a9-e3007c7ff56e SIV: 336781303264349553179461347850802165102 decode: variant: DCE 1.1, ISO/IEC 11578:1996 version: 10 (unknown) content: FD:5D:B8:3C:12:F3:04:6B:00:A9:E3:00:7C:7F:F5:6E (not decipherable: unknown UUID version) Version 10 does not look right. $ uuid -d 935FE837-FAC8-4394-C008-737D8852C60D encode: STR: 935fe837-fac8-4394-c008-737d8852c60d SIV: 195894493536133784175416063449172723213 decode: variant: reserved (Microsoft GUID) version: 4 (random data based) content: 93:5F:E8:37:FA:C8:03:94:00:08:73:7D:88:52:C6:0D (no semantics: random data only) A reserved Microsoft GUID variant does not look right. With the master branch, the (hardcoded) GUIDs are ok: $ ./u-boot --default_fdt ... U-Boot 2024.07-rc4-00021-gfe2ce09a075 (Jun 19 2024 - 15:35:08 +0200) ... Model: sandbox ... => efidebug capsule esrt ... ESRT: fw_resource_count=2 ESRT: fw_resource_count_max=2 ESRT: fw_resource_version=1 [entry 0]== ESRT: fw_class=09D7CF52-0720-4710-91D1-08469B7FE9C8 ... [entry 1]== ESRT: fw_class=5A7021F5-FEF2-48B4-AABA-832E777418C0 ... $ uuid -d 09D7CF52-0720-4710-91D1-08469B7FE9C8 encode: STR: 09d7cf52-0720-4710-91d1-08469b7fe9c8 SIV: 13083600744351929150374221048734280136 decode: variant: DCE 1.1, ISO/IEC 11578:1996 version: 4 (random data based) content: 09:D7:CF:52:07:20:07:10:11:D1:08:46:9B:7F:E9:C8 (no semantics: random data only) $ uuid -d 5A7021F5-FEF2-48B4-AABA-832E777418C0 encode: STR: 5a7021f5-fef2-48b4-aaba-832e777418c0 SIV: 120212745678117161641696128857923655872 decode: variant: DCE 1.1, ISO/IEC 11578:1996 version: 4 (random data based) content: 5A:70:21:F5:FE:F2:08:B4:2A:BA:83:2E:77:74:18:C0 (no semantics: random data only) Also, this is what to expect for a v5 UUID [1]: $ uuid -d a8f6ae40-d8a7-58f0-be05-a22f94eca9ec encode: STR: a8f6ae40-d8a7-58f0-be05-a22f94eca9ec SIV: 224591142595989943290477237735758014956 decode: variant: DCE 1.1, ISO/IEC 11578:1996 version: 5 (name based, SHA-1) content: A8:F6:AE:40:D8:A7:08:F0:3E:05:A2:2F:94:EC:A9:EC (not decipherable: truncated SHA-1 message digest only) Best regards, Vincent. [1] https://uuid.ramsey.dev/en/stable/rfc4122/version5.html > > The well-known salt GUID can be specific to the architecture (SoC > vendor), or OEM. It is defined in the board defconfig so that vendors > can easily bring their own. > > Specifically, the following fields are used to generate a GUID for a > particular fw_image: > > * namespace salt > * board compatible (usually the first entry in the dt root compatible > array). > * fw_image name (the string identifying the specific image, especially > relevant for board that can update multiple images). > > == Usage == > > Boards can integrate dynamic UUID support as follows: > > 1. Adjust Kconfi
Re: [PATCH v3 6/7] tools: add genguid tool
On Fri, May 31, 2024 at 03:50:40PM +0200, Caleb Connolly wrote:
> Add a tool that can generate GUIDs that match those generated internally
> by U-Boot for capsule update fw_images.
Hi Caleb,
Thanks for working on this.
I think there is a leftover "dtb" option; see below.
Best regards,
Vincent.
>
> Dynamic UUIDs in U-Boot work by taking a namespace UUID and hashing it
> with the board model, compatible, and fw_image name.
>
> This tool accepts the same inputs and will produce the same GUID as
> U-Boot would at runtime.
>
> Signed-off-by: Caleb Connolly
> ---
> doc/genguid.1 | 52 +++
> tools/Kconfig | 7 +++
> tools/Makefile | 3 ++
> tools/genguid.c | 154
>
> 4 files changed, 216 insertions(+)
(...)
> diff --git a/tools/genguid.c b/tools/genguid.c
> new file mode 100644
> index ..e71bc1d48f95
> --- /dev/null
> +++ b/tools/genguid.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2024 Linaro Ltd.
> + * Author: Caleb Connolly
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +static struct option options[] = {
> + {"dtb", required_argument, NULL, 'd'},
I think this is unused.
> + {"compat", required_argument, NULL, 'c'},
> + {"help", no_argument, NULL, 'h'},
> + {"verbose", no_argument, NULL, 'v'},
> + {"json", no_argument, NULL, 'j'},
> + {NULL, 0, NULL, 0},
> +};
> +
> +static void usage(const char *progname)
> +{
> + fprintf(stderr, "Usage: %s GUID [-v] -c COMPAT NAME...\n", progname);
> + fprintf(stderr,
> + "Generate a v5 GUID for one of more U-Boot fw_images the same
> way U-Boot does at runtime.\n");
> + fprintf(stderr,
> + "\nOptions:\n"
> + " GUID namespace/salt GUID in 8-4-4-4-12
> format\n"
> + " -h, --help display this help and exit\n"
> + " -c, --compat=COMPAT first compatible property in the
> board devicetree\n"
> + " -v, --verboseprint debug messages\n"
> + " -j, --json output in JSON format\n"
> + " NAME... one or more names of fw_images to
> generate GUIDs for\n"
> + );
> + fprintf(stderr, "\nExample:\n");
> + fprintf(stderr, " %s 2a5aa852-b856-4d97-baa9-5c5f4421551f \\\n"
> + "\t-c \"qcom,qrb4210-rb2\" \\\n"
> + "\tQUALCOMM-UBOOT\n", progname);
> +}
> +
> +static size_t u16_strsize(const uint16_t *in)
> +{
> + size_t i = 0, count = UINT16_MAX;
> +
> + while (count-- && in[i])
> + i++;
> +
> + return (i + 1) * sizeof(uint16_t);
> +}
> +
> +int main(int argc, char **argv)
> +{
> + struct uuid namespace;
> + char *namespace_str;
> + char uuid_str[37];
> + char **image_uuids;
> + char *compatible = NULL;
> + uint16_t **images_u16;
> + char **images;
> + int c, n_images;
> + bool debug = false, json = false;
> +
> + if (argc < 2) {
> + usage(argv[0]);
> + return 1;
> + }
> +
> + namespace_str = argv[1];
> +
> + /* The first arg is the GUID so skip it */
> + while ((c = getopt_long(argc, argv, "c:hvj", options, NULL)) != -1) {
> + switch (c) {
> + case 'c':
> + compatible = strdup(optarg);
> + break;
> + case 'h':
> + usage(argv[0]);
> + return 0;
> + case 'v':
> + debug = true;
> + break;
> + case 'j':
> + json = true;
> + break;
> + default:
> + usage(argv[0]);
> + return 1;
> + }
> + }
> +
> + if (!compatible) {
> + fprintf(stderr, "ERROR: Please specify the compatible
> property.\n\n");
> + usage(argv[0]);
> + return 1;
> + }
> +
> + if (uuid_str_to_bin(namespace_str, (unsigned char *)&namespace,
> UUID_STR_FORMAT_GUID)) {
> + fprintf(stderr, "ERROR: Check that your UUID is formatted
> correctly.\n");
> + exit(EXIT_FAILURE);
> + }
> +
> + /* This is probably not the best way to convert a string to a "u16"
> string */
> + n_images = argc - optind - 1;
> + images = argv + optind + 1;
> + images_u16 = calloc(n_images, sizeof(char *));
> + for (int i = 0; i < n_images; i++) {
> + images_u16[i] = calloc(1, strlen(images[i]) * 2 + 2);
> + for (int j = 0; j < strlen(images[i]); j++)
> + images_u16[i][j] = (uint16_t)images[i][j];
> + }
> +
> + if (debug) {
> + fprintf(stderr, "GUID: ");
> + uuid_bin_to_str((uint8_t *)&namespace, uuid_str,
> UUID_
Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
On Wed, Jun 19, 2024 at 10:15:32PM +0300, Ilias Apalodimas wrote:
> Allô Vincent,
Hi Ilias :)
>
> Thanks for testing!
>
> On Wed, 19 Jun 2024 at 17:02, Vincent Stehlé wrote:
> >
> > On Fri, May 31, 2024 at 03:50:34PM +0200, Caleb Connolly wrote:
> > > As more boards adopt support for the EFI CapsuleUpdate mechanism, there
> > > is a growing issue of being able to target updates to them properly. The
> > > current mechanism of hardcoding UUIDs for each board at compile time is
> > > unsustainable, and maintaining lists of GUIDs is similarly cumbersome.
> > >
> > > In this series, I propose that we adopt v5 GUIDs, these are generated
> > > by using a well-known salt GUID as well as board specific information
> > > (like the model/revision), these are hashed together and the result is
> > > truncated to form a new UUID.
> >
> > Dear Caleb,
> >
> > Thank you for working on this proposal, this looks very useful.
> > Indeed, we found out during SystemReady certifications that it is easy for
> > unique ids to be inadvertently re-used, making them thus non-unique.
> >
> > I have a doubt regarding the format of the generated UUIDs, which end up in
> > the
> > ESRT, though.
> >
> > Here is a quick experiment on the sandbox booting with a DTB using the
> > efidebug
> > command.
> >
> > With the patch series, rebased on the master branch:
> >
> > $ make sandbox_defconfig
> > $ make
> > $ ./u-boot --default_fdt
> > ...
> > U-Boot 2024.07-rc4-00028-g1c96225b0b3 (Jun 19 2024 - 15:29:04 +0200)
> > ...
> > Model: sandbox
> > ...
> > Hit any key to stop autoboot: 0
> > => efidebug capsule esrt
> > ...
> >
> > ESRT: fw_resource_count=2
> > ESRT: fw_resource_count_max=2
> > ESRT: fw_resource_version=1
> > [entry 0]==
> > ESRT: fw_class=FD5DB83C-12F3-A46B-80A9-E3007C7FF56E
> > ...
> > [entry 1]==
> > ESRT: fw_class=935FE837-FAC8-4394-C008-737D8852C60D
> > ...
> >
> >
> > $ uuid -d FD5DB83C-12F3-A46B-80A9-E3007C7FF56E
> > encode: STR: fd5db83c-12f3-a46b-80a9-e3007c7ff56e
> > SIV: 336781303264349553179461347850802165102
> > decode: variant: DCE 1.1, ISO/IEC 11578:1996
> > version: 10 (unknown)
> > content: FD:5D:B8:3C:12:F3:04:6B:00:A9:E3:00:7C:7F:F5:6E
> >(not decipherable: unknown UUID version)
> >
> > Version 10 does not look right.
>
> So, this seems to be an endianess problem.
> Looking at RFC4122 only the space ID needs to be in BE.
>
> >
> > $ uuid -d 935FE837-FAC8-4394-C008-737D8852C60D
> > encode: STR: 935fe837-fac8-4394-c008-737d8852c60d
> > SIV: 195894493536133784175416063449172723213
> > decode: variant: reserved (Microsoft GUID)
> > version: 4 (random data based)
> > content: 93:5F:E8:37:FA:C8:03:94:00:08:73:7D:88:52:C6:0D
> >(no semantics: random data only)
> >
> > A reserved Microsoft GUID variant does not look right.
>
> This seems like an existing bug. RFC4122 defines the MS reserved GUIDs
> in the variant as
> 1 1 0Reserved, Microsoft Corporation backward
> compatibility and the existing UUID_VARIANT_MASK is defined as 0xc0...
I think the variant mask 0xc0 is correct:
- The variant field is in the top three bits of the "clock seq high and
reserved" byte, but...
- The variant we want is 1 0 x (do not care for bit 5, a.k.a. "Msb2").
With the mask 0xc0 we can clear the top two bits as we set the top most bit just
after anyway.
...the mask needs to be used correctly, though; see below.
>
> The patch below should work for you (on top of Calebs')
>
> diff --git a/include/uuid.h b/include/uuid.h
> index b38b20d957ef..78ed5839d2d6 100644
> --- a/include/uuid.h
> +++ b/include/uuid.h
> @@ -81,7 +81,7 @@ struct uuid {
> #define UUID_VERSION_SHIFT 12
> #define UUID_VERSION 0x4
>
> -#define UUID_VARIANT_MASK 0xc0
> +#define UUID_VARIANT_MASK 0xb0
> #define UUID_VARIANT_SHIFT 7
> #define UUID_VARIANT 0x1
>
> diff --git a/lib/uuid.c b/lib/uuid.c
> index 89911b06ccc0..73251eaa397e 100644
> --- a/lib/uuid.c
> +++ b/lib/uuid.c
> @@ -411,9 +411,9 @@ void gen_uuid_v5(const struct uuid *namespace,
> struct uuid *uuid, ...)
> memcpy(uuid, hash, sizeof(*uuid));
&
Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
On Fri, Jun 21, 2024 at 01:00:51PM +0200, Heinrich Schuchardt wrote:
(..)
> The current specification is in RFC 9562, 4.1, "Variant field"
>
> "The variant field consists of a variable number of the most significant
> bits of octet 8 of the UUID.
>
> ...
>
> Specifically for UUIDs in this document, bits 64 and 65 of the UUID
> (bits 0 and 1 of octet 8) MUST be set to 1 and 0 as specified in row 2
> of Table 1."
>
> This reference to byte 8 does not depend on endianness.
Hi Heinrich,
Agreed, variant is not concerned by the endianness.
>
> U-Boot's include/uuid.h has:
>
> /* This is structure is in big-endian */
> struct uuid {
>
> The field time_hi_and_version needs to be stored in big-endian fashion.
Thanks! I thought this structure was used to hold either a big-endian UUID or a
little-endian GUID, but now you have convinced me.
This confirms that the generation of the dynamic GUID is missing something:
gen_uuid_v5(&namespace,
(struct uuid *)&fw_array[i].image_type_id,
compatible, strlen(compatible),
fw_array[i].fw_name, u16_strsize(fw_array[i].fw_name)
- sizeof(uint16_t),
NULL);
It is not possible to cast the little-endian efi_guid_t .image_type_id as the
big-endian struct uuid output of gen_uuid_v5() like this; we need to convert the
three time fields from big to little endianness.
>
>
> tools/genguid uses UUID_STR_FORMAT_GUID which prints low-endian which is
> typical in the EFI context but not understood by 'uuid -d'. Maybe we
> should add a parameter for the output format.
My understanding is that there is a single universal string format for both
UUIDs and GUIDs, which uuid -d understands, and which has no notion of
endianness. (Only the structures in memory have an endianness.)
This means we do not need an output format parameter.
genguid is calling the new gen_uuid_v5() function, which outputs a big-endian
struct uuid. Therefore, genguid should print it with 'STD format, not 'GUID.
Best regards,
Vincent.
>
> Best regards
>
> Heinrich
Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
On Fri, Jun 21, 2024 at 07:11:28PM +0200, Caleb Connolly wrote:
> On Fri, 21 Jun 2024, 19:06 Vincent Stehlé, wrote:
>
> > On Fri, Jun 21, 2024 at 01:00:51PM +0200, Heinrich Schuchardt wrote:
> > (..)
> > > The current specification is in RFC 9562, 4.1, "Variant field"
> > >
> > > "The variant field consists of a variable number of the most significant
> > > bits of octet 8 of the UUID.
> > >
> > > ...
> > >
> > > Specifically for UUIDs in this document, bits 64 and 65 of the UUID
> > > (bits 0 and 1 of octet 8) MUST be set to 1 and 0 as specified in row 2
> > > of Table 1."
> > >
> > > This reference to byte 8 does not depend on endianness.
> >
> > Hi Heinrich,
> >
> > Agreed, variant is not concerned by the endianness.
> >
> > >
> > > U-Boot's include/uuid.h has:
> > >
> > > /* This is structure is in big-endian */
> > > struct uuid {
> > >
> > > The field time_hi_and_version needs to be stored in big-endian fashion.
> >
> > Thanks! I thought this structure was used to hold either a big-endian UUID
> > or a
> > little-endian GUID, but now you have convinced me.
> >
> > This confirms that the generation of the dynamic GUID is missing something:
> >
> > gen_uuid_v5(&namespace,
> > (struct uuid *)&fw_array[i].image_type_id,
> > compatible, strlen(compatible),
> > fw_array[i].fw_name, u16_strsize(fw_array[i].fw_name)
> > - sizeof(uint16_t),
> > NULL);
> >
> > It is not possible to cast the little-endian efi_guid_t .image_type_id as
> > the
> > big-endian struct uuid output of gen_uuid_v5() like this; we need to
> > convert the
> > three time fields from big to little endianness.
> >
>
> I'm inclined to disagree, the comment above struct uuid in include/uuid.h
> state clearly that the format in memory is always big endian, but that a
> GUID when printed has some fields converted to little endian.
Hi Caleb,
I read the comments above struct uuid differently: the GUID has some fields
little-endian when stored in memory (and can thus not be stored in a struct
uuid after Heinrich's comments).
This is consistent with how it is done in U-Boot in various locations; for
example, the EFI_GUID() macro stores a GUID with its time fields little-endian
in memory. Similarly, the callers of uuid_str_to_bin() or uuid_bin_to_str() with
format UUID_STR_FORMAT_GUID have indeed a little-endian GUID in memory (most
often an efi_guid_t). This is also consistent with the UEFI specification's
16-byte buffer format. [1]
When you have a big-endian UUID in memory, the version field is stored in byte
6, which is consistent with the RFC 9562. [2]
If you convert this big-endian UUID with uuid_bin_to_str() and
UUID_STR_FORMAT_GUID as in genguid, the "time high and version" field's bytes
will be printed with byte 7 first and then byte 6, as per guid_char_order[].
This is in contradiction with the RFC, which shows that the version field ("M")
should be printed first.
If you print the UUID with format 'STD, you will indeed have byte 6 containing
the version field printed first as it should (uuid_char_order[]).
This is confirmed by "uuid -d".
Best regards,
Vincent.
[1] https://uefi.org/specs/UEFI/2.10/Apx_A_GUID_and_Time_Formats.html
[2] https://www.rfc-editor.org/rfc/rfc9562
>
>
> > >
> > >
> > > tools/genguid uses UUID_STR_FORMAT_GUID which prints low-endian which is
> > > typical in the EFI context but not understood by 'uuid -d'. Maybe we
> > > should add a parameter for the output format.
> >
> > My understanding is that there is a single universal string format for both
> > UUIDs and GUIDs, which uuid -d understands, and which has no notion of
> > endianness. (Only the structures in memory have an endianness.)
> > This means we do not need an output format parameter.
> >
> > genguid is calling the new gen_uuid_v5() function, which outputs a
> > big-endian
> > struct uuid. Therefore, genguid should print it with 'STD format, not
> > 'GUID.
> >
> > Best regards,
> > Vincent.
> >
> > >
> > > Best regards
> > >
> > > Heinrich
> >
[PATCH] Proposed changes to dynamic UUIDs v3
Here are the changes that I would like to suggest for the "efi:
CapsuleUpdate: support for dynamic UUIDs" v3 patch series:
- Convert from big-endian UUID to little-endian GUID in
efi_capsule_update_info_gen_ids().
- Fix tmp size and masking in gen_uuid_v5().
- Use UUID_STR_FORMAT_STD in all places where we are dealing with a
big-endian UUID.
- Update all GUIDs constants in the code and in the tests accordingly. This
gets rid of the following broken UUIDs:
5af91295-5a99-f62b-80d7-e9574de87170
8ee418dc-7e00-e156-80a7-274fbbc05ba8
935fe837-fac8-4394-c008-737d8852c60d
fd5db83c-12f3-a46b-80a9-e3007c7ff56e
ffd97379-0956-fa94-c003-8bfcf5cc097b
- Also, a few minor modifications here and there.
Signed-off-by: Vincent Stehlé
Cc: Caleb Connolly
Cc: Tom Rini
Cc: Heinrich Schuchardt
Cc: Ilias Apalodimas
Cc: Simon Glass
Cc: Mario Six
Cc: Alper Nebi Yasak
Cc: Abdellatif El Khlifi
Cc: Richard Hughes
---
include/sandbox_efi_capsule.h | 6 +++---
lib/efi_loader/efi_firmware.c | 14 +++---
lib/uuid.c | 8
test/lib/uuid.c| 12 ++--
.../test_efi_capsule/test_capsule_firmware_fit.py | 4 ++--
.../test_efi_capsule/test_capsule_firmware_raw.py | 8
.../test_capsule_firmware_signed_fit.py| 2 +-
.../test_capsule_firmware_signed_raw.py| 4 ++--
test/py/tests/test_efi_capsule/version.dts | 6 +++---
tools/.gitignore | 1 +
tools/binman/etype/efi_capsule.py | 2 +-
tools/binman/ftest.py | 2 +-
tools/genguid.c| 7 +++
13 files changed, 42 insertions(+), 34 deletions(-)
diff --git a/include/sandbox_efi_capsule.h b/include/sandbox_efi_capsule.h
index 25ac496ea24..6f0de5a1e25 100644
--- a/include/sandbox_efi_capsule.h
+++ b/include/sandbox_efi_capsule.h
@@ -6,9 +6,9 @@
#if !defined(_SANDBOX_EFI_CAPSULE_H_)
#define _SANDBOX_EFI_CAPSULE_H_
-#define SANDBOX_UBOOT_IMAGE_GUID "fd5db83c-12f3-a46b-80a9-e3007c7ff56e"
-#define SANDBOX_UBOOT_ENV_IMAGE_GUID "935fe837-fac8-4394-c008-737d8852c60d"
-#define SANDBOX_FIT_IMAGE_GUID "ffd97379-0956-fa94-c003-8bfcf5cc097b"
+#define SANDBOX_UBOOT_IMAGE_GUID "50980990-5af9-5522-86e2-8f05f4d7313c"
+#define SANDBOX_UBOOT_ENV_IMAGE_GUID "3554b655-b9f0-5240-ace2-6f34c2f7fcca"
+#define SANDBOX_FIT_IMAGE_GUID "8b38adc7-df0c-5769-8b89-c090ca3d07a7"
#define SANDBOX_INCORRECT_GUID "058b7d83-50d5-4c47-a195-60d86ad341c4"
#define UBOOT_FIT_IMAGE"u-boot_bin_env.itb"
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index a8dafe4f01a..f0d0c3fa972 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -258,7 +258,7 @@ void efi_firmware_fill_version_info(struct
efi_firmware_image_descriptor *image_
static efi_status_t efi_capsule_update_info_gen_ids(void)
{
int ret, i;
- struct uuid namespace;
+ struct uuid namespace, type;
const char *compatible; /* Full array including null bytes */
struct efi_fw_image *fw_array;
@@ -269,7 +269,7 @@ static efi_status_t efi_capsule_update_info_gen_ids(void)
return EFI_SUCCESS;
ret = uuid_str_to_bin(CONFIG_EFI_CAPSULE_NAMESPACE_UUID,
- (unsigned char *)&namespace, UUID_STR_FORMAT_GUID);
+ (unsigned char *)&namespace, UUID_STR_FORMAT_STD);
if (ret) {
log_debug("%s: CONFIG_EFI_CAPSULE_NAMESPACE_UUID is invalid:
%d\n", __func__, ret);
return EFI_UNSUPPORTED;
@@ -289,12 +289,20 @@ static efi_status_t efi_capsule_update_info_gen_ids(void)
for (i = 0; i < update_info.num_images; i++) {
gen_uuid_v5(&namespace,
- (struct uuid *)&fw_array[i].image_type_id,
+ &type,
compatible, strlen(compatible),
fw_array[i].fw_name,
u16_strsize(fw_array[i].fw_name)
- sizeof(uint16_t),
NULL);
+ /* Convert to little-endian GUID. */
+ fw_array[i].image_type_id = (efi_guid_t)EFI_GUID(
+ be32_to_cpu(type.time_low), be16_to_cpu(type.time_mid),
+ be16_to_cpu(type.time_hi_and_version),
+ type.clock_seq_hi_and_reserved, type.clock_seq_low,
+ type.node[0], type.node[1], type.node[2], type.node[3],
+ type.node[4], type.node[5]);
+
log_debug("Image %ls UUID %pUs\n", fw_array[i].fw_name,
&fw
[PATCH] bootstd: cros: store partition type in an efi_guid_t
The scan_part() function uses a struct uuid to store the little-endian
partition type GUID, but this structure should be used only to contain a
big-endian UUID. Use an efi_guid_t instead.
Signed-off-by: Vincent Stehlé
Cc: Simon Glass
Cc: Tom Rini
---
boot/bootmeth_cros.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/boot/bootmeth_cros.c b/boot/bootmeth_cros.c
index f015f2e1c75..1f83c14aeab 100644
--- a/boot/bootmeth_cros.c
+++ b/boot/bootmeth_cros.c
@@ -148,7 +148,7 @@ static int scan_part(struct udevice *blk, int partnum,
{
struct blk_desc *desc = dev_get_uclass_plat(blk);
struct vb2_keyblock *hdr;
- struct uuid type;
+ efi_guid_t type;
ulong num_blks;
int ret;
@@ -161,7 +161,7 @@ static int scan_part(struct udevice *blk, int partnum,
/* Check for kernel partition type */
log_debug("part %x: type=%s\n", partnum, info->type_guid);
- if (uuid_str_to_bin(info->type_guid, (u8 *)&type, UUID_STR_FORMAT_GUID))
+ if (uuid_str_to_bin(info->type_guid, type.b, UUID_STR_FORMAT_GUID))
return log_msg_ret("typ", -EINVAL);
if (memcmp(&cros_kern_type, &type, sizeof(type)))
--
2.43.0
Re: [PATCH] bootstd: cros: store partition type in an efi_guid_t
On Thu, Jun 27, 2024 at 09:28:04PM +0200, Heinrich Schuchardt wrote:
>
Hi Heinrich,
Thanks for your review.
My comments below.
Best regards,
Vincent.
>
> Am 27. Juni 2024 19:06:29 MESZ schrieb "Vincent Stehlé"
> :
> >The scan_part() function uses a struct uuid to store the little-endian
> >partition type GUID, but this structure should be used only to contain a
> >big-endian UUID. Use an efi_guid_t instead.
> >
> >Signed-off-by: Vincent Stehlé
> >Cc: Simon Glass
> >Cc: Tom Rini
> >---
> > boot/bootmeth_cros.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/boot/bootmeth_cros.c b/boot/bootmeth_cros.c
> >index f015f2e1c75..1f83c14aeab 100644
> >--- a/boot/bootmeth_cros.c
> >+++ b/boot/bootmeth_cros.c
> >@@ -148,7 +148,7 @@ static int scan_part(struct udevice *blk, int partnum,
> > {
> > struct blk_desc *desc = dev_get_uclass_plat(blk);
> > struct vb2_keyblock *hdr;
> >-struct uuid type;
> >+efi_guid_t type;
>
> Does Chrome OS only support GPT partitioning?
>
> > ulong num_blks;
> > int ret;
> >
> >@@ -161,7 +161,7 @@ static int scan_part(struct udevice *blk, int partnum,
> >
> > /* Check for kernel partition type */
> > log_debug("part %x: type=%s\n", partnum, info->type_guid);
> >-if (uuid_str_to_bin(info->type_guid, (u8 *)&type, UUID_STR_FORMAT_GUID))
> >+if (uuid_str_to_bin(info->type_guid, type.b, UUID_STR_FORMAT_GUID))
> > return log_msg_ret("typ", -EINVAL);
>
> struct disk_partition containing a string which is only needed in the CLI
> instead of the 16 byte GUID was a bad idea to start with. Shouldn't we
> replace it or add least add a GUID field instead of first converting to
> string and than back to GUID?
I had a quick look and it seems that converting all those UUIDs from strings to
binary would indeed impact many places; let's separate this longer-term effort
from this change if you agree.
>
> >
> > if (memcmp(&cros_kern_type, &type, sizeof(type)))
>
> You could use the guidcmp() macro here.
Thanks for the tip; I will send a v2 series.
>
> Best regards
>
> Heinrich
>
[PATCH v2 0/2] Respin bootstd cros patch into a series of two
Hi, This is a respin of this patch [1] after discussion [2]. Thanks to Simon and Heinrich for their reviews. To use the guidcmp() function, as suggested by Heinrich, we need to make it available to bootmeth_cros.c and I think that the cleanest way to do that is (arguably) to move the guid helper functions to efi.h near the efi_guid_t definition; this is why the original patch has now become a series of two patches. The alternative would be to include efi_loader.h from bootmeth_cros.c but I think this does not sound "right". If this is in fact the preferred approach just let me know and I will respin. There is no difference in the sandbox binaries before/after this series on Arm and on PC, and all the tests I have run on the sandbox are unchanged. Best regards, Vincent. [1] https://patchwork.ozlabs.org/project/uboot/patch/20240627170629.2696427-1-vincent.ste...@arm.com/ [2] https://lists.denx.de/pipermail/u-boot/2024-June/557588.html Vincent Stehlé (2): efi: move guid helper functions to efi.h bootstd: cros: store partition type in an efi_guid_t boot/bootmeth_cros.c | 6 +++--- include/efi.h| 10 ++ include/efi_loader.h | 10 -- 3 files changed, 13 insertions(+), 13 deletions(-) -- 2.43.0
[PATCH v2 1/2] efi: move guid helper functions to efi.h
Move the guidcmp() and guidcpy() functions to efi.h, near the definition of
the efi_guid_t type those functions deal with.
Signed-off-by: Vincent Stehlé
Cc: Heinrich Schuchardt
Cc: Ilias Apalodimas
Cc: Tom Rini
---
include/efi.h| 10 ++
include/efi_loader.h | 10 --
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/efi.h b/include/efi.h
index c3c4b93f860..d5af2139946 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -78,6 +78,16 @@ typedef struct {
u8 b[16];
} efi_guid_t __attribute__((aligned(4)));
+static inline int guidcmp(const void *g1, const void *g2)
+{
+ return memcmp(g1, g2, sizeof(efi_guid_t));
+}
+
+static inline void *guidcpy(void *dst, const void *src)
+{
+ return memcpy(dst, src, sizeof(efi_guid_t));
+}
+
#define EFI_BITS_PER_LONG (sizeof(long) * 8)
/* Bit mask for EFI status code with error */
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 6c993e1a694..ca8fc0820f6 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -21,16 +21,6 @@
struct blk_desc;
struct jmp_buf_data;
-static inline int guidcmp(const void *g1, const void *g2)
-{
- return memcmp(g1, g2, sizeof(efi_guid_t));
-}
-
-static inline void *guidcpy(void *dst, const void *src)
-{
- return memcpy(dst, src, sizeof(efi_guid_t));
-}
-
#if CONFIG_IS_ENABLED(EFI_LOADER)
/**
--
2.43.0
[PATCH v2 2/2] bootstd: cros: store partition type in an efi_guid_t
The scan_part() function uses a struct uuid to store the little-endian
partition type GUID, but this structure should be used only to contain a
big-endian UUID. Use an efi_guid_t instead and use guidcmp() for the
comparison.
Suggested-by: Heinrich Schuchardt
Signed-off-by: Vincent Stehlé
Cc: Simon Glass
Cc: Tom Rini
---
Changes for v2:
- Use guidcmp() for the comparison, as suggested by Heinrich.
boot/bootmeth_cros.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/boot/bootmeth_cros.c b/boot/bootmeth_cros.c
index 645b8bed102..676f550ca25 100644
--- a/boot/bootmeth_cros.c
+++ b/boot/bootmeth_cros.c
@@ -147,7 +147,7 @@ static int scan_part(struct udevice *blk, int partnum,
{
struct blk_desc *desc = dev_get_uclass_plat(blk);
struct vb2_keyblock *hdr;
- struct uuid type;
+ efi_guid_t type;
ulong num_blks;
int ret;
@@ -160,10 +160,10 @@ static int scan_part(struct udevice *blk, int partnum,
/* Check for kernel partition type */
log_debug("part %x: type=%s\n", partnum, info->type_guid);
- if (uuid_str_to_bin(info->type_guid, (u8 *)&type, UUID_STR_FORMAT_GUID))
+ if (uuid_str_to_bin(info->type_guid, type.b, UUID_STR_FORMAT_GUID))
return log_msg_ret("typ", -EINVAL);
- if (memcmp(&cros_kern_type, &type, sizeof(type)))
+ if (guidcmp(&cros_kern_type, &type))
return log_msg_ret("typ", -ENOEXEC);
/* Make a buffer for the header information */
--
2.43.0
Re: [PATCH v4 09/10] tools: mkeficapsule: support generating dynamic GUIDs
On Tue, Jul 02, 2024 at 03:30:49PM +0200, Caleb Connolly wrote:
Hi Caleb,
Thanks for re-spinning this series; it looks very good.
My comments below.
Best regards,
Vincent.
> Add a tool that can generate GUIDs that match those generated internally
> by U-Boot for capsule update fw_images.
Nit picking: I think you are not "adding a tool" anymore but rather, modifying
an existing one.
>
> Dynamic UUIDs in U-Boot work by taking a namespace UUID and hashing it
> with the board compatible and fw_image name.
>
> This tool accepts the same inputs and will produce the same GUID as
> U-Boot would at runtime.
Same here for "this tool".
>
> Signed-off-by: Caleb Connolly
> ---
> doc/mkeficapsule.1 | 23
> tools/Makefile | 3 +
> tools/mkeficapsule.c | 157
> +--
> 3 files changed, 178 insertions(+), 5 deletions(-)
>
> diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1
> index c4c2057d5c7a..bf735295effa 100644
> --- a/doc/mkeficapsule.1
> +++ b/doc/mkeficapsule.1
> @@ -9,8 +9,11 @@ mkeficapsule \- Generate EFI capsule file for U-Boot
> .SH SYNOPSIS
> .B mkeficapsule
> .RI [ options ] " " [ image-blob ] " " capsule-file
>
> +.B mkeficapsule
> +.RI guidgen " " [ GUID ] " " DTB " " IMAGE_NAME...
> +
> .SH "DESCRIPTION"
> The
> .B mkeficapsule
> command is used to create an EFI capsule file to be used by U-Boot for
> firmware
> @@ -41,8 +44,12 @@ format is the same as used in the new uImage format and
> allows for
> multiple binary blobs in a single capsule file.
> This type of image file can be generated by
> .BR mkimage .
>
> +mkeficapsule can also be used to simulate the dynamic GUID generation used to
> +identify firmware images in capsule updates by providing the namespace guid,
> dtb
> +for the board, and a list of firmware images.
> +
> .SH "OPTIONS"
>
> .TP
> .BI "-g\fR,\fB --guid " guid-string
> @@ -112,8 +119,24 @@ at every firmware update.
> .TP
> .B "-d\fR,\fB --dump_sig"
> Dump signature data into *.p7 file
>
> +.SH "GUIDGEN OPTIONS"
> +
> +.TP
> +.B "[GUID]"
> +The namespace/salt GUID, by default this is EFI_CAPSULE_NAMESPACE_GUID.
> +The format is:
> +----
> +
> +.TP
> +.B DTB
> +The device tree blob file for the board.
> +
> +.TP
> +.B IMAGE_NAME...
> +The names of the firmware images to generate GUIDs for.
> +
> .PP
> .SH FILES
> .TP
> .I /EFI/UpdateCapsule
> diff --git a/tools/Makefile b/tools/Makefile
> index ee08a9675df8..7d1b29943471 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -253,8 +253,11 @@ mkeficapsule-objs := generated/lib/uuid.o \
> $(LIBFDT_OBJS) \
> mkeficapsule.o
> hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
>
> +genguid-objs := generated/lib/uuid.o generated/lib/sha1.o genguid.o
> +hostprogs-$(CONFIG_TOOLS_GENGUID) += genguid
> +
As far as I can tell those lines above should be removed, now that you have
integrated genguid into mkeficapsule.
> mkfwumdata-objs := mkfwumdata.o generated/lib/crc32.o
> HOSTLDLIBS_mkfwumdata += -luuid
> hostprogs-$(CONFIG_TOOLS_MKFWUMDATA) += mkfwumdata
>
> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> index 54fb4dee3ee5..593380e4236a 100644
> --- a/tools/mkeficapsule.c
> +++ b/tools/mkeficapsule.c
> @@ -19,12 +19,16 @@
> #include
> #include
> #include
>
> +#include
> #include
>
> #include "eficapsule.h"
>
> +// Matches CONFIG_EFI_CAPSULE_NAMESPACE_GUID
> +#define DEFAULT_NAMESPACE_GUID "8c9f137e-91dc-427b-b2d6-b420faebaf2a"
> +
> static const char *tool_name = "mkeficapsule";
>
> efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
> @@ -38,8 +42,9 @@ enum {
> } capsule_type;
>
> static struct option options[] = {
> {"guid", required_argument, NULL, 'g'},
> + {"dtb", required_argument, NULL, 'd'},
I think this option entry should be removed. This seems unused and the existing
`-d' option of mkeficapsule means something completely unrelated ("dump
signature").
> {"index", required_argument, NULL, 'i'},
> {"instance", required_argument, NULL, 'I'},
> {"fw-version", required_argument, NULL, 'v'},
> {"private-key", required_argument, NULL, 'p'},
> @@ -53,11 +58,23 @@ static struct option options[] = {
> {"help", no_argument, NULL, 'h'},
> {NULL, 0, NULL, 0},
> };
>
> -static void print_usage(void)
> +
> +static void print_usage_guidgen(void)
> {
> - fprintf(stderr, "Usage: %s [options] \n"
> + fprintf(stderr, "%s guidgen [GUID] DTB IMAGE_NAME...\n"
> + "Options:\n"
> +
> + "\tGUIDNamespace GUID (default: %s)\n"
> + "\tDTB Device Tree Blob\n"
> + "\tIMAGE_NAME... One or more names of fw_images
> to generate GUIDs for\n",
> + tool_name, DEFAULT_NAMESPACE_GUID);
> +}
> +
[PATCH] trace: use dynamic string buffer in make_flamegraph()
The str[] buffer declared in make_flamegraph() is used to hold strings
representing the full call-stacks recorded in traces. The size of this
buffer is currently 500 characters and this works well for the documented
examples.
However, it is possible to exhaust this buffer when processing traces
captured when running the UEFI shell on aarch64 sandbox for example.
Indeed, the maximum length needed for such traces can reach 780 characters.
As it is difficult to evaluate the maximum size that would ever be needed
for all the possible traces, let's use a dynamically allocated `abuf'
instead, which we reallocate when needed.
This fixes the following error:
String too short (500 chars)
While at it, fix a few typos in strings and comments.
Signed-off-by: Vincent Stehlé
Cc: Tom Rini
Cc: Simon Glass
Cc: Michal Simek
---
Hi,
This can be verified using the `test_trace.py' pytest as a starting point.
Configure with `sandbox_defconfig' plus the following options:
CONFIG_TRACE=y
CONFIG_TRACE_BUFFER_SIZE=0x0200
CONFIG_TRACE_EARLY=y
CONFIG_TRACE_EARLY_SIZE=0x0100
Build with:
make FTRACE=1 NO_LTO=1
Run the test with:
./test/py/test.py --bd sandbox --build-dir . -k trace -s
Note that no reallocation happens during the test as the initial size of
500 characters is never exhausted; reduce the size manually if you want to
force re-allocation.
Also, make sure to delete /tmp/test_trace between tests.
Best regards,
Vincent Stehlé
tools/proftool.c | 58
1 file changed, 39 insertions(+), 19 deletions(-)
diff --git a/tools/proftool.c b/tools/proftool.c
index fca45e4a5af..c2e38099354 100644
--- a/tools/proftool.c
+++ b/tools/proftool.c
@@ -290,7 +290,7 @@ static void usage(void)
"Options:\n"
" -c \tSpecify config file\n"
" -f \tSpecify output subtype\n"
- " -m \tSpecify Systen.map file\n"
+ " -m \tSpecify System.map file\n"
" -o \tSpecify output file\n"
" -t \tSpecify trace data file (from U-Boot 'trace
calls')\n"
" -v <0-4>\tSpecify verbosity\n"
@@ -306,7 +306,7 @@ static void usage(void)
}
/**
- * h_cmp_offset - bsearch() function to compare two functions bny their offset
+ * h_cmp_offset - bsearch() function to compare two functions by their offset
*
* @v1: Pointer to first function (struct func_info)
* @v2: Pointer to second function (struct func_info)
@@ -431,7 +431,7 @@ static struct func_info *find_func_by_offset(uint offset)
static struct func_info *find_caller_by_offset(uint offset)
{
int low;/* least function that could be a match */
- int high; /* greated function that could be a match */
+ int high; /* greatest function that could be a match */
struct func_info key;
low = 0;
@@ -1352,7 +1352,7 @@ static int write_pages(struct twriter *tw, enum
out_format_t out_format,
}
if (!(func->flags & FUNCF_TRACE)) {
- debug("Funcion '%s' is excluded from trace\n",
+ debug("Function '%s' is excluded from trace\n",
func->name);
skip_count++;
continue;
@@ -1781,7 +1781,8 @@ static int make_flame_tree(enum out_format_t out_format,
*
* This works by maintaining a string shared across all recursive calls. The
* function name for this node is added to the existing string, to make up the
- * full call-stack description. For example, on entry, @str might contain:
+ * full call-stack description. For example, on entry, @str_buf->data might
+ * contain:
*
*"initf_bootstage;bootstage_mark_name"
*^ @base
@@ -1795,18 +1796,18 @@ static int make_flame_tree(enum out_format_t out_format,
* @fout: Output file
* @out_format: Output format to use
* @node: Node to output (pass the whole tree at first)
- * @str: String to use to build the output line (e.g. 500 charas long)
- * @maxlen: Maximum length of string
+ * @str_buf: String buffer to use to build the output line
* @base: Current base position in the string
* @treep: Returns the resulting flamegraph tree
* Returns 0 if OK, -1 on error
*/
static int output_tree(FILE *fout, enum out_format_t out_format,
- const struct flame_node *node, char *str, int maxlen,
+ const struct flame_node *node, struct abuf *str_buf,
int base)
{
const struct flame_node *child;
int pos;
+ char *str = abuf_data(str_buf);
if (node->count) {
if (out_format == OUT_FMT_FLAMEGRAPH_CALLS) {
@@ -1832,18 +1833,29 @@ s
[PATCH] Fix references to trace doc
The README.trace has been moved and converted to rst in commit dce26c7d56ed
("doc: move README.trace to HTML documentation"); fix all the remaining
references to this file.
Signed-off-by: Vincent Stehlé
Cc: Tom Rini
Cc: Simon Glass
Cc: Heinrich Schuchardt
---
cmd/Kconfig | 4 ++--
doc/develop/tests_sandbox.rst | 2 +-
lib/Kconfig | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 38305197602..f9f271616d3 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2832,8 +2832,8 @@ config CMD_TRACE
Enables a command to control using of function tracing within
U-Boot. This allows recording of call traces including timing
information. The command can write data to memory for exporting
- for analysis (e.g. using bootchart). See doc/README.trace for full
- details.
+ for analysis (e.g. using bootchart). See doc/develop/trace.rst
+ for full details.
config CMD_AVB
bool "avb - Android Verified Boot 2.0 operations"
diff --git a/doc/develop/tests_sandbox.rst b/doc/develop/tests_sandbox.rst
index bfd3bdb9270..c2824834d07 100644
--- a/doc/develop/tests_sandbox.rst
+++ b/doc/develop/tests_sandbox.rst
@@ -29,7 +29,7 @@ Some of the available tests are:
- test/image/test-imagetools.sh - multi-file images
- test/py/tests/test-fit.py - FIT images
- tracing: test/trace/test-trace.sh tests the tracing system (see
- README.trace)
+ doc/develop/trace.rst)
- verified boot: test/py/tests/test_vboot.py
If you change or enhance any U-Boot subsystem, you should write or expand a
diff --git a/lib/Kconfig b/lib/Kconfig
index 37ac14f7bab..efb77978a65 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -348,7 +348,7 @@ config TRACE
Enables function tracing within U-Boot. This allows recording of call
traces including timing information. The command can write data to
memory for exporting for analysis (e.g. using bootchart).
- See doc/README.trace for full details.
+ See doc/develop/trace.rst for full details.
config TRACE_BUFFER_SIZE
hex "Size of trace buffer in U-Boot"
--
2.43.0
[PATCH] sandbox: dtsi: add rng
Having an rng in the sandbox is useful not only for tests but also for e.g.
UEFI. Therefore, copy the rng node from test.dts to sandbox.dtsi.
In the case of UEFI, it can then be verified with `efidebug dh' that a
"Random Number Generator" protocol is indeed present.
This also fixes the following `bootefi' error:
Missing RNG device for EFI_RNG_PROTOCOL
Signed-off-by: Vincent Stehlé
Cc: Simon Glass
---
arch/sandbox/dts/sandbox.dtsi | 4
1 file changed, 4 insertions(+)
diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
index dc933f3bfc7..43b9342ce56 100644
--- a/arch/sandbox/dts/sandbox.dtsi
+++ b/arch/sandbox/dts/sandbox.dtsi
@@ -196,6 +196,10 @@
compatible = "sandbox,reset";
};
+ rng {
+ compatible = "sandbox,sandbox-rng";
+ };
+
sound {
compatible = "sandbox,sound";
cpu {
--
2.30.1
Re: [PATCH 1/1] sandbox: fix sandbox_reset()
On Wed, May 12, 2021 at 06:38:51PM +0200, Heinrich Schuchardt wrote: > state_uninit() and dm_uninit() are mutually exclusive: > > state_uninit() prints via drivers. So it cannot be executed after > dm_uninit(). > > dm_uninit() requires memory. So it cannot be executed after state_uninit() > which releases all memory. > > Just skip dm_uninit() when resetting the sandbox. We will wake up in a new > process and allocate new memory. So this cleanup is not required. We don't > do it in sandbox_exit() either. > > This avoids a segmentation error when efi_reset_system_boottime() is > invoked by a UEFI application. Hi Heinrich, Thanks for fixing this! Before, I was hitting the following segfault with the sandbox under qemu arm64 when running the UEFI SCT: Boot services test: ExitBootServices_Conf Iterations: 1/1 System will cold reset after 2 second and test will be resumed after reboot.resetting ... Writing sandbox state qemu: uncaught target signal 11 (Segmentation fault) - core dumped With your patch I do not hit this segfault anymore. FWIW, feel free to add (or not): Tested-by: Vincent Stehlé Best regards, Vincent. > > Signed-off-by: Heinrich Schuchardt > --- > arch/sandbox/cpu/start.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c > index e87365e800..4ffd97ccbc 100644 > --- a/arch/sandbox/cpu/start.c > +++ b/arch/sandbox/cpu/start.c > @@ -425,9 +425,6 @@ void sandbox_reset(void) > if (state_uninit()) > os_exit(2); > > - if (dm_uninit()) > - os_exit(2); > - > /* Restart U-Boot */ > os_relaunch(os_argv); > } > -- > 2.30.2 >
[PATCH] efi_loader: check update capsule parameters
UpdateCapsule() must return EFI_INVALID_PARAMETER in a number of cases,
listed by the UEFI specification and tested by the SCT. Add a common
function to do that.
This fixes SCT UpdateCapsule_Conf failures.
Reviewed-by: Grant Likely
Signed-off-by: Vincent Stehlé
Cc: Heinrich Schuchardt
Cc: Alexander Graf
---
include/efi_loader.h | 24
lib/efi_loader/efi_capsule.c | 8
lib/efi_loader/efi_runtime.c | 8
3 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 0a9c82a257e..426d1c72d7d 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -910,6 +910,30 @@ extern const struct efi_firmware_management_protocol
efi_fmp_fit;
extern const struct efi_firmware_management_protocol efi_fmp_raw;
/* Capsule update */
+static inline efi_status_t
+efi_valid_update_capsule_params(struct efi_capsule_header
+ **capsule_header_array,
+ efi_uintn_t capsule_count,
+ u64 scatter_gather_list)
+{
+ u32 flags;
+
+ if (!capsule_count)
+ return EFI_INVALID_PARAMETER;
+
+ flags = capsule_header_array[0]->flags;
+
+ if (((flags & CAPSULE_FLAGS_PERSIST_ACROSS_RESET) &&
+!scatter_gather_list) ||
+ ((flags & CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE) &&
+!(flags & CAPSULE_FLAGS_PERSIST_ACROSS_RESET)) ||
+ ((flags & CAPSULE_FLAGS_INITIATE_RESET) &&
+!(flags & CAPSULE_FLAGS_PERSIST_ACROSS_RESET)))
+ return EFI_INVALID_PARAMETER;
+
+ return EFI_SUCCESS;
+}
+
efi_status_t EFIAPI efi_update_capsule(
struct efi_capsule_header **capsule_header_array,
efi_uintn_t capsule_count,
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 60309d4a07d..380cfd70290 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -442,12 +442,12 @@ efi_status_t EFIAPI efi_update_capsule(
EFI_ENTRY("%p, %zu, %llu\n", capsule_header_array, capsule_count,
scatter_gather_list);
- if (!capsule_count) {
- ret = EFI_INVALID_PARAMETER;
+ ret = efi_valid_update_capsule_params(capsule_header_array,
+ capsule_count,
+ scatter_gather_list);
+ if (ret != EFI_SUCCESS)
goto out;
- }
- ret = EFI_SUCCESS;
for (i = 0, capsule = *capsule_header_array; i < capsule_count;
i++, capsule = *(++capsule_header_array)) {
/* sanity check */
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 93a695fc27e..449ad8b9f36 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -467,6 +467,14 @@ efi_status_t __efi_runtime EFIAPI
efi_update_capsule_unsupported(
efi_uintn_t capsule_count,
u64 scatter_gather_list)
{
+ efi_status_t ret;
+
+ ret = efi_valid_update_capsule_params(capsule_header_array,
+ capsule_count,
+ scatter_gather_list);
+ if (ret != EFI_SUCCESS)
+ return ret;
+
return EFI_UNSUPPORTED;
}
--
2.30.2
[PATCH] virtio: fix off by one device id comparison
VIRTIO_ID_MAX_NUM is the largest device ID plus 1. Therefore a device id
cannot be greater or equal to VIRTIO_ID_MAX_NUM. Fix the comparison
accordingly.
Fixes: 8fb49b4c7a82 ("dm: Add a new uclass driver for VirtIO transport devices")
Signed-off-by: Vincent Stehlé
Cc: Simon Glass
Cc: Bin Meng
---
drivers/virtio/virtio-uclass.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio-uclass.c b/drivers/virtio/virtio-uclass.c
index cf2cfaef2cf..0379536c542 100644
--- a/drivers/virtio/virtio-uclass.c
+++ b/drivers/virtio/virtio-uclass.c
@@ -227,7 +227,7 @@ static int virtio_uclass_post_probe(struct udevice *udev)
struct udevice *vdev;
int ret;
- if (uc_priv->device > VIRTIO_ID_MAX_NUM) {
+ if (uc_priv->device >= VIRTIO_ID_MAX_NUM) {
debug("(%s): virtio device ID %d exceeds maximum num\n",
udev->name, uc_priv->device);
return 0;
--
2.29.2
[PATCH] efi: test/py: repair authenticated capsules tests
The UEFI console initialisation has been modified by commit 68edbed454b8
("efi_loader: initialize console size late"). A corresponding workaround is
now necessary for the automated tests, as added to some of the tests
already by commit e05bd68ed5fc ("test: work around for EFI terminal size
probing").
Add the same workaround to the UEFI authenticated capsules tests to repair
them.
This can be tested with sandbox_defconfig, sandbox64_defconfig or
sandbox_flattree_defconfig, plus CONFIG_EFI_CAPSULE_AUTHENTICATE=y.
Signed-off-by: Vincent Stehlé
Cc: Heinrich Schuchardt
---
.../tests/test_efi_capsule/test_capsule_firmware_signed_fit.py | 3 +++
.../tests/test_efi_capsule/test_capsule_firmware_signed_raw.py | 2 ++
2 files changed, 5 insertions(+)
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
index 4400b8f1368..d6ca9b16745 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
@@ -40,6 +40,7 @@ class TestEfiCapsuleFirmwareSignedFit(object):
with u_boot_console.log.section('Test Case 1-a, before reboot'):
output = u_boot_console.run_command_list([
'host bind 0 %s' % disk_img,
+'printenv -e PlatformLangCodes', # workaround for terminal
size determination
'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi',
'efidebug boot order 1',
'env set -e -nv -bs -rt OsIndications =0x0004',
@@ -115,6 +116,7 @@ class TestEfiCapsuleFirmwareSignedFit(object):
with u_boot_console.log.section('Test Case 2-a, before reboot'):
output = u_boot_console.run_command_list([
'host bind 0 %s' % disk_img,
+'printenv -e PlatformLangCodes', # workaround for terminal
size determination
'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi',
'efidebug boot order 1',
'env set -e -nv -bs -rt OsIndications =0x0004',
@@ -192,6 +194,7 @@ class TestEfiCapsuleFirmwareSignedFit(object):
with u_boot_console.log.section('Test Case 3-a, before reboot'):
output = u_boot_console.run_command_list([
'host bind 0 %s' % disk_img,
+'printenv -e PlatformLangCodes', # workaround for terminal
size determination
'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi',
'efidebug boot order 1',
'env set -e -nv -bs -rt OsIndications =0x0004',
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
index 1b5a1bb42eb..2bbaa9cc55f 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
@@ -112,6 +112,7 @@ class TestEfiCapsuleFirmwareSignedRaw(object):
with u_boot_console.log.section('Test Case 2-a, before reboot'):
output = u_boot_console.run_command_list([
'host bind 0 %s' % disk_img,
+'printenv -e PlatformLangCodes', # workaround for terminal
size determination
'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi',
'efidebug boot order 1',
'env set -e -nv -bs -rt OsIndications =0x0004',
@@ -189,6 +190,7 @@ class TestEfiCapsuleFirmwareSignedRaw(object):
with u_boot_console.log.section('Test Case 3-a, before reboot'):
output = u_boot_console.run_command_list([
'host bind 0 %s' % disk_img,
+'printenv -e PlatformLangCodes', # workaround for terminal
size determination
'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi',
'efidebug boot order 1',
'env set -e -nv -bs -rt OsIndications =0x0004',
--
2.35.1
[PATCH] doc: uefi: fix links
Fix a couple of links so that they are rendered correctly with sphinx. Signed-off-by: Vincent Stehlé Cc: Heinrich Schuchardt Cc: Ilias Apalodimas --- doc/develop/uefi/fwu_updates.rst | 3 ++- doc/develop/uefi/uefi.rst| 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/doc/develop/uefi/fwu_updates.rst b/doc/develop/uefi/fwu_updates.rst index 72c850a7908..e4709d82b41 100644 --- a/doc/develop/uefi/fwu_updates.rst +++ b/doc/develop/uefi/fwu_updates.rst @@ -27,7 +27,8 @@ metadata. Individual drivers can be added based on the type of storage media, and its partitioning method. Details of the storage device containing the FWU metadata partitions are specified through a U-Boot specific device tree property `fwu-mdata-store`. Please refer to -U-Boot `doc `__ +U-Boot :download:`fwu-mdata-gpt.yaml +` for the device tree bindings. Enabling the FWU Multi Bank Update feature diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index a944c0fb803..ffe25ca2318 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -386,8 +386,8 @@ is because the FWU feature supports multiple partitions(banks) of updatable images, and the actual dfu alt number to which the image is to be written to is determined at runtime, based on the value of the update bank to which the image is to be written. For more information -on the FWU Multi Bank Update feature, please refer `doc -`__. +on the FWU Multi Bank Update feature, please refer to +:doc:`/develop/uefi/fwu_updates`. When using the FMP for FIT images, the image index value needs to be set to 1. -- 2.39.1
[PATCH 1/2] doc/efi: add firmware management protocol to the documentation
The firmware management protocol can be used to manage device firmware. U-Boot can be configured to provide an implementation. Document the related functions in the API section. Signed-off-by: Vincent Stehlé Cc: Heinrich Schuchardt --- doc/api/efi.rst | 6 ++ 1 file changed, 6 insertions(+) diff --git a/doc/api/efi.rst b/doc/api/efi.rst index cb2a1c897e6..2b96783828b 100644 --- a/doc/api/efi.rst +++ b/doc/api/efi.rst @@ -166,6 +166,12 @@ Unicode Collation protocol .. kernel-doc:: lib/efi_loader/efi_unicode_collation.c :internal: +Firmware management protocol + + +.. kernel-doc:: lib/efi_loader/efi_firmware.c + :internal: + Unit testing -- 2.35.1
[PATCH 2/2] efi: fix documentation warnings
This fixes the following warnings:
./lib/efi_loader/efi_firmware.c:283: warning: Function parameter or member
'package_version' not described in 'efi_firmware_fit_get_image_info'
./lib/efi_loader/efi_firmware.c:283: warning: Function parameter or member
'package_version_name' not described in 'efi_firmware_fit_get_image_info'
./lib/efi_loader/efi_firmware.c:369: warning: bad line: firmware image
./lib/efi_loader/efi_firmware.c:395: warning: Function parameter or member
'package_version' not described in 'efi_firmware_raw_get_image_info'
./lib/efi_loader/efi_firmware.c:395: warning: Function parameter or member
'package_version_name' not described in 'efi_firmware_raw_get_image_info'
Signed-off-by: Vincent Stehlé
Cc: Heinrich Schuchardt
---
lib/efi_loader/efi_firmware.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 27953fe7699..fe4e084106d 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -197,8 +197,8 @@ static efi_status_t efi_fill_image_desc_array(
* @descriptor_version:Pointer to version number
* @descriptor_count: Pointer to number of descriptors
* @descriptor_size: Pointer to descriptor size
- * package_version:Package version
- * package_version_name: Package version's name
+ * @package_version: Package version
+ * @package_version_name: Package version's name
*
* Return information bout the current firmware image in @image_info.
* @image_info will consist of a number of descriptors.
@@ -296,15 +296,15 @@ const struct efi_firmware_management_protocol efi_fmp_fit
= {
/**
* efi_firmware_raw_get_image_info - return information about the current
-firmware image
+ * firmware image
* @this: Protocol instance
* @image_info_size: Size of @image_info
* @image_info:Image information
* @descriptor_version:Pointer to version number
* @descriptor_count: Pointer to number of descriptors
* @descriptor_size: Pointer to descriptor size
- * package_version:Package version
- * package_version_name: Package version's name
+ * @package_version: Package version
+ * @package_version_name: Package version's name
*
* Return information bout the current firmware image in @image_info.
* @image_info will consist of a number of descriptors.
--
2.35.1
[PATCH 1/2] test/py: efi_capsule: repair image authentication test
Repair the python tests for authenticated EFI capsules, which can be run
with sandbox_defconfig plus CONFIG_EFI_CAPSULE_AUTHENTICATE=y.
- Account for the reset changes done by commit 3e6f81000672 ("efi_loader:
test/py: Reset system after capsule update on disk").
- Fix the capsule GUID typo introduced by commit 2e9c3c6965ba ("test:
capsule: Modify the capsule tests to use GUID values for sandbox").
Signed-off-by: Vincent Stehlé
Cc: Heinrich Schuchardt
---
test/py/tests/test_efi_capsule/conftest.py | 4 ++--
.../tests/test_efi_capsule/test_capsule_firmware_signed.py | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/test/py/tests/test_efi_capsule/conftest.py
b/test/py/tests/test_efi_capsule/conftest.py
index d757415c881..5a8826a5a6b 100644
--- a/test/py/tests/test_efi_capsule/conftest.py
+++ b/test/py/tests/test_efi_capsule/conftest.py
@@ -101,7 +101,7 @@ def efi_capsule_data(request, u_boot_config):
check_call('cd %s; '
'%s/tools/mkeficapsule --index 1 --monotonic-count 1 '
'--private-key SIGNER.key --certificate SIGNER.crt
'
-'--guid 09D7DF52-0720-4710-91D1-08469B7FE9C8 '
+'--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 '
'u-boot.bin.new Test11'
% (data_dir, u_boot_config.build_dir),
shell=True)
@@ -110,7 +110,7 @@ def efi_capsule_data(request, u_boot_config):
'%s/tools/mkeficapsule --index 1 --monotonic-count 1 '
'--private-key SIGNER2.key '
'--certificate SIGNER2.crt '
-'--guid 09D7DF52-0720-4710-91D1-08469B7FE9C8 '
+'--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 '
'u-boot.bin.new Test12'
% (data_dir, u_boot_config.build_dir),
shell=True)
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed.py
b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed.py
index 593b032e901..a0b6a1ac86f 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware_signed.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_signed.py
@@ -85,7 +85,7 @@ class TestEfiCapsuleFirmwareSigned(object):
# need to run uefi command to initiate capsule handling
output = u_boot_console.run_command(
-'env print -e Capsule')
+'env print -e Capsule', wait_for_reboot = True)
output = u_boot_console.run_command_list([
'host bind 0 %s' % disk_img,
@@ -160,7 +160,7 @@ class TestEfiCapsuleFirmwareSigned(object):
# need to run uefi command to initiate capsule handling
output = u_boot_console.run_command(
-'env print -e Capsule')
+'env print -e Capsule', wait_for_reboot = True)
# deleted any way
output = u_boot_console.run_command_list([
@@ -237,7 +237,7 @@ class TestEfiCapsuleFirmwareSigned(object):
# need to run uefi command to initiate capsule handling
output = u_boot_console.run_command(
-'env print -e Capsule')
+'env print -e Capsule', wait_for_reboot = True)
# deleted any way
output = u_boot_console.run_command_list([
--
2.35.1
[PATCH 2/2] efi: test/py: authenticate fit capsules
Add support for the authentication of UEFI capsules containing FIT images.
The authentication code is moved out of the function handling raw images
into a new function efi_firmware_capsule_authenticate(). The special case
for the FMP header coming from edk2 tools is preserved. There is no
functional change for capsules containing raw images.
The python test for signed capsules with raw images is renamed with no
functional change and a new test is added for signed capsules containing
FIT images.
This can be tested with sandbox64_defconfig or sandbox_flattree_defconfig,
plus CONFIG_EFI_CAPSULE_AUTHENTICATE=y.
Signed-off-by: Vincent Stehlé
Cc: Heinrich Schuchardt
---
lib/efi_loader/efi_firmware.c | 115 +++---
test/py/tests/test_efi_capsule/conftest.py| 21 +++-
...py => test_capsule_firmware_signed_fit.py} | 41 ---
...py => test_capsule_firmware_signed_raw.py} | 6 +-
4 files changed, 117 insertions(+), 66 deletions(-)
copy test/py/tests/test_efi_capsule/{test_capsule_firmware_signed.py =>
test_capsule_firmware_signed_fit.py} (89%)
rename test/py/tests/test_efi_capsule/{test_capsule_firmware_signed.py =>
test_capsule_firmware_signed_raw.py} (98%)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index fe4e084106d..cbe29e90789 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -178,6 +178,70 @@ static efi_status_t efi_fill_image_desc_array(
return EFI_SUCCESS;
}
+/**
+ * efi_firmware_capsule_authenticate - authenticate the capsule if enabled
+ * @p_image: Pointer to new image
+ * @p_image_size: Pointer to size of new image
+ *
+ * Authenticate the capsule if authentication is enabled.
+ * The image pointer and the image size are updated in case of success.
+ *
+ * Return: status code
+ */
+static
+efi_status_t efi_firmware_capsule_authenticate(const void **p_image,
+ efi_uintn_t *p_image_size)
+{
+ const void *image = *p_image;
+ efi_uintn_t image_size = *p_image_size;
+ u32 fmp_hdr_signature;
+ struct fmp_payload_header *header;
+ void *capsule_payload;
+ efi_status_t status;
+ efi_uintn_t capsule_payload_size;
+
+ if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE)) {
+ capsule_payload = NULL;
+ capsule_payload_size = 0;
+ status = efi_capsule_authenticate(image, image_size,
+ &capsule_payload,
+ &capsule_payload_size);
+
+ if (status == EFI_SECURITY_VIOLATION) {
+ printf("Capsule authentication check failed. Aborting
update\n");
+ return status;
+ } else if (status != EFI_SUCCESS) {
+ return status;
+ }
+
+ debug("Capsule authentication successful\n");
+ image = capsule_payload;
+ image_size = capsule_payload_size;
+ } else {
+ debug("Capsule authentication disabled. ");
+ debug("Updating capsule without authenticating.\n");
+ }
+
+ fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE;
+ header = (void *)image;
+
+ if (!memcmp(&header->signature, &fmp_hdr_signature,
+ sizeof(fmp_hdr_signature))) {
+ /*
+* When building the capsule with the scripts in
+* edk2, a FMP header is inserted above the capsule
+* payload. Compensate for this header to get the
+* actual payload that is to be updated.
+*/
+ image += header->header_size;
+ image_size -= header->header_size;
+ }
+
+ *p_image = image;
+ *p_image_size = image_size;
+ return EFI_SUCCESS;
+}
+
#ifdef CONFIG_EFI_CAPSULE_FIRMWARE_FIT
/*
* This FIRMWARE_MANAGEMENT_PROTOCOL driver provides a firmware update
@@ -266,12 +330,18 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
efi_status_t (*progress)(efi_uintn_t completion),
u16 **abort_reason)
{
+ efi_status_t status;
+
EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
image_size, vendor_code, progress, abort_reason);
if (!image || image_index != 1)
return EFI_EXIT(EFI_INVALID_PARAMETER);
+ status = efi_firmware_capsule_authenticate(&image, &image_size);
+ if (status != EFI_SUCCESS)
+ return EFI_EXIT(status);
+
if (fit_update(image))
return EFI_EXIT(EFI_DEVICE_ERROR);
@@ -372,11 +442,7 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
efi_status_t (*progress)(efi_uintn_t completion),
u16 **abort_reason)
{
-
[U-Boot] [PATCH] README: align default commands with code
Align the list of default commands mentioned in the configuration options paragraph of the README with the actual definitions found in include/config_cmd_default.h Signed-off-by: Vincent Stehlé --- README | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/README b/README index cd0336c..787d40f 100644 --- a/README +++ b/README @@ -843,7 +843,7 @@ The following options need to be configured: CONFIG_CMD_FDOS * Dos diskette Support CONFIG_CMD_FLASH flinfo, erase, protect CONFIG_CMD_FPGA FPGA device initialization support - CONFIG_CMD_FUSE Device fuse support + CONFIG_CMD_FUSE * Device fuse support CONFIG_CMD_GETTIME * Get time since boot CONFIG_CMD_GO * the 'go' command (exec code) CONFIG_CMD_GREPENV * search environment @@ -853,7 +853,7 @@ The following options need to be configured: CONFIG_CMD_IDE * IDE harddisk support CONFIG_CMD_IMIiminfo CONFIG_CMD_IMLS List all images found in NOR flash - CONFIG_CMD_IMLS_NAND List all images found in NAND flash + CONFIG_CMD_IMLS_NAND* List all images found in NAND flash CONFIG_CMD_IMMAP* IMMR dump support CONFIG_CMD_IMPORTENV* import an environment CONFIG_CMD_INI * import data from an ini file into the env @@ -861,23 +861,24 @@ The following options need to be configured: CONFIG_CMD_ITEST Integer/string test of 2 values CONFIG_CMD_JFFS2* JFFS2 Support CONFIG_CMD_KGDB * kgdb - CONFIG_CMD_LDRINFOldrinfo (display Blackfin loader) + CONFIG_CMD_LDRINFO * ldrinfo (display Blackfin loader) CONFIG_CMD_LINK_LOCAL * link-local IP address auto-configuration (169.254.*.*) CONFIG_CMD_LOADB loadb CONFIG_CMD_LOADS loads - CONFIG_CMD_MD5SUM print md5 message digest + CONFIG_CMD_MD5SUM * print md5 message digest (requires CONFIG_CMD_MEMORY and CONFIG_MD5) CONFIG_CMD_MEMINFO * Display detailed memory information CONFIG_CMD_MEMORY md, mm, nm, mw, cp, cmp, crc, base, loop, loopw - CONFIG_CMD_MEMTESTmtest + CONFIG_CMD_MEMTEST * mtest CONFIG_CMD_MISC Misc functions like sleep etc CONFIG_CMD_MMC * MMC memory mapped support CONFIG_CMD_MII * MII utility commands CONFIG_CMD_MTDPARTS * MTD partition support CONFIG_CMD_NAND * NAND support CONFIG_CMD_NETbootp, tftpboot, rarpboot + CONFIG_CMD_NFSNFS support CONFIG_CMD_PCA953X * PCA953x I2C gpio commands CONFIG_CMD_PCA953X_INFO * PCA953x I2C gpio info command CONFIG_CMD_PCI * pciinfo @@ -896,7 +897,7 @@ The following options need to be configured: CONFIG_CMD_SETGETDCR Support for DCR Register access (4xx only) CONFIG_CMD_SF * Read/write/erase SPI NOR flash - CONFIG_CMD_SHA1SUMprint sha1 memory digest + CONFIG_CMD_SHA1SUM * print sha1 memory digest (requires CONFIG_CMD_MEMORY) CONFIG_CMD_SOFTSWITCH * Soft switch setting command for BF60x CONFIG_CMD_SOURCE "source" command Support @@ -908,6 +909,7 @@ The following options need to be configured: CONFIG_CMD_USB * USB support CONFIG_CMD_CDP * Cisco Discover Protocol support CONFIG_CMD_MFSL * Microblaze FSL support + CONFIG_CMD_XIMG Load part of Multi Image EXAMPLE: If you want all functions except of network -- 1.7.10.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/2] ARM: mmu: Set domain permissions to client access - build warnings!
On 03/02/2013 11:46 PM, Albert ARIBAUD wrote: > (..) Basically, this means we need Vincent's series > to be applied first, then we can apply Sricharan's. Hi, I think this is too much trouble for a "one liner". Please feel free to squash. Best regards, V. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] armv7: do not relocate _start twice
The _start symbol is already relocated, so do not add the relocation the second time in c_runtime_cpu_setup. This fixes e.g. the abort exception handling path, which ended in double fault due to bad address in VBAR. Signed-off-by: Vincent Stehlé Reported-by: Lubomir Popov --- Hello, Here is a fix for a bug reported by Lubomir. He noticed that exceptions were not handled correctly anymore. This can be seen with e.g. the 'dhcp' command on some OMAP platforms. Looking at the code, I would says the fix applies to all armv7 platforms except Tegra but I did only test on OMAP5. On this platform at least the abort is now handled: OMAP5430 EVM # dhcp data abort MAYBE you should read doc/README.arm-unaligned-accesses pc : [] lr : [] sp : feef9dc4 ip : fefed0f8 fp : r10: 0001 r9 : 0001 r8 : feef9f48 r7 : feef9fe0 r6 : r5 : r4 : 0014 r3 : r2 : 0002 r1 : 0014 r0 : fefed0f4 Flags: Nzcv IRQs off FIQs off Mode SVC_32 Resetting CPU ... resetting ... It would be appreciated if folks could verify on other ARMv7 platforms, when running from flash for example (where relocation may differ?) arch/arm/cpu/armv7/start.S |1 - 1 file changed, 1 deletion(-) diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 6b59529d..d06b35f 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -254,7 +254,6 @@ ENTRY(c_runtime_cpu_setup) #if !defined(CONFIG_TEGRA) /* Set vector address in CP15 VBAR register */ ldr r0, =_start - add r0, r0, r9 mcr p15, 0, r0, c12, c0, 0 @Set VBAR #endif /* !Tegra */ -- 1.7.10.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] Patches to add some more omap24xx_i2c/twl6035 error handling
Hi, I am encountering the following i2c error on OMAP5 with mainline u-boot: OMAP5430 EVM # mmc rescan timed out in wait_for_bb: I2C_STAT=1410 It seems the first call to i2c_write for bus I2C1 will fail and will leave the bus with SCL stuck low, preventing further i2c operations. While still debugging this issue, I would like to propose the following patches, which add some more error handling in this error case already: [PATCH 1/2] omap24xx_i2c: Handle wait_for_bb error [PATCH 2/2] power: twl6035: complain on LDO9 error Best regards, V. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/2] omap24xx_i2c: Handle wait_for_bb error
We add a return code to wait_for_bb() to be able to report errors to the
callers properly. We in turn handle this new error code in i2c_read, i2c_write
and i2c_probe.
Signed-off-by: Vincent Stehlé
---
drivers/i2c/omap24xx_i2c.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
index 094305f..1bbb2ca 100644
--- a/drivers/i2c/omap24xx_i2c.c
+++ b/drivers/i2c/omap24xx_i2c.c
@@ -31,7 +31,7 @@ DECLARE_GLOBAL_DATA_PTR;
#define I2C_TIMEOUT1000
-static void wait_for_bb(void);
+static int wait_for_bb(void);
static u16 wait_for_pin(void);
static void flush_fifo(void);
@@ -159,7 +159,8 @@ static int i2c_read_byte(u8 devaddr, u16 regoffset, u8
alen, u8 *value)
u16 w;
/* wait until bus not busy */
- wait_for_bb();
+ if (wait_for_bb())
+ return 1;
/* one byte only */
writew(alen, &i2c_base->cnt);
@@ -260,7 +261,8 @@ int i2c_probe(uchar chip)
return res;
/* wait until bus not busy */
- wait_for_bb();
+ if (wait_for_bb())
+ return res;
/* try to read one byte */
writew(1, &i2c_base->cnt);
@@ -279,7 +281,10 @@ int i2c_probe(uchar chip)
res = 1;
writew(0xff, &i2c_base->stat);
writew (readw (&i2c_base->con) | I2C_CON_STP,
&i2c_base->con);
- wait_for_bb ();
+
+ if (wait_for_bb())
+ res = 1;
+
break;
}
if (status & I2C_STAT_ARDY) {
@@ -351,7 +356,8 @@ int i2c_write(uchar chip, uint addr, int alen, uchar
*buffer, int len)
}
/* wait until bus not busy */
- wait_for_bb();
+ if (wait_for_bb())
+ return 1;
/* start address phase - will write regoffset + len bytes data */
/* TODO consider case when !CONFIG_OMAP243X/34XX/44XX */
@@ -394,7 +400,7 @@ write_exit:
return i2c_error;
}
-static void wait_for_bb(void)
+static int wait_for_bb(void)
{
int timeout = I2C_TIMEOUT;
u16 stat;
@@ -408,8 +414,10 @@ static void wait_for_bb(void)
if (timeout <= 0) {
printf("timed out in wait_for_bb: I2C_STAT=%x\n",
readw(&i2c_base->stat));
+ return 1;
}
writew(0x, &i2c_base->stat); /* clear delayed stuff*/
+ return 0;
}
static u16 wait_for_pin(void)
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/2] power: twl6035: complain on LDO9 error
We handle i2c_write return code and complain in case of error. We propagate the
error, too, to allow better handling at the upper level in the future.
Signed-off-by: Vincent Stehlé
---
drivers/power/twl6035.c | 17 +
include/twl6035.h |2 +-
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/power/twl6035.c b/drivers/power/twl6035.c
index 624c09e..d3de698 100644
--- a/drivers/power/twl6035.c
+++ b/drivers/power/twl6035.c
@@ -50,16 +50,25 @@ void twl6035_init_settings(void)
return;
}
-void twl6035_mmc1_poweron_ldo(void)
+int twl6035_mmc1_poweron_ldo(void)
{
u8 val = 0;
/* set LDO9 TWL6035 to 3V */
val = 0x2b; /* (3 -.9)*28 +1 */
- palmas_write_u8(0x48, LDO9_VOLTAGE, val);
+
+ if (palmas_write_u8(0x48, LDO9_VOLTAGE, val)) {
+ printf("twl6035: could not set LDO9 voltage.\n");
+ return 1;
+ }
/* TURN ON LDO9 */
val = LDO_ON | LDO_MODE_SLEEP | LDO_MODE_ACTIVE;
- palmas_write_u8(0x48, LDO9_CTRL, val);
- return;
+
+ if (palmas_write_u8(0x48, LDO9_CTRL, val)) {
+ printf("twl6035: could not turn on LDO9.\n");
+ return 1;
+ }
+
+ return 0;
}
diff --git a/include/twl6035.h b/include/twl6035.h
index e21ddba..ce74348 100644
--- a/include/twl6035.h
+++ b/include/twl6035.h
@@ -39,4 +39,4 @@
int twl6035_i2c_write_u8(u8 chip_no, u8 val, u8 reg);
int twl6035_i2c_read_u8(u8 chip_no, u8 *val, u8 reg);
void twl6035_init_settings(void);
-void twl6035_mmc1_poweron_ldo(void);
+int twl6035_mmc1_poweron_ldo(void);
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] omap24xx_i2c: Handle OMAP5 like OMAP2,3,4
OMAP5 has 8b i2c data register field, like OMAP2, 3 and 4. Handle in the same
way. This fixes the following error on OMAP5:
OMAP5430 EVM # mmc rescan
timed out in wait_for_bb: I2C_STAT=1410
twl6035: could not turn on LDO9.
Signed-off-by: Vincent Stehlé
---
drivers/i2c/omap24xx_i2c.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
index 1bbb2ca..54e9b15 100644
--- a/drivers/i2c/omap24xx_i2c.c
+++ b/drivers/i2c/omap24xx_i2c.c
@@ -180,7 +180,8 @@ static int i2c_read_byte(u8 devaddr, u16 regoffset, u8
alen, u8 *value)
if (status & I2C_STAT_XRDY) {
w = tmpbuf[i++];
#if !(defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \
- defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX))
+ defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX) || \
+ defined(CONFIG_OMAP54XX))
w |= tmpbuf[i++] << 8;
#endif
writew(w, &i2c_base->data);
@@ -210,7 +211,8 @@ static int i2c_read_byte(u8 devaddr, u16 regoffset, u8
alen, u8 *value)
}
if (status & I2C_STAT_RRDY) {
#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \
- defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX)
+ defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX) || \
+ defined(CONFIG_OMAP54XX)
*value = readb(&i2c_base->data);
#else
*value = readw(&i2c_base->data);
@@ -240,7 +242,8 @@ static void flush_fifo(void)
stat = readw(&i2c_base->stat);
if (stat == I2C_STAT_RRDY) {
#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \
- defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX)
+ defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX) || \
+ defined(CONFIG_OMAP54XX)
readb(&i2c_base->data);
#else
readw(&i2c_base->data);
@@ -294,7 +297,8 @@ int i2c_probe(uchar chip)
if (status & I2C_STAT_RRDY) {
res = 0;
#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \
-defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX)
+ defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX) || \
+ defined(CONFIG_OMAP54XX)
readb(&i2c_base->data);
#else
readw(&i2c_base->data);
@@ -382,7 +386,8 @@ int i2c_write(uchar chip, uint addr, int alen, uchar
*buffer, int len)
if (status & I2C_STAT_XRDY) {
w = (i < 0) ? tmpbuf[2+i] : buffer[i];
#if !(defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \
- defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX))
+ defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX) || \
+ defined(CONFIG_OMAP54XX))
w |= ((++i < 0) ? tmpbuf[2+i] : buffer[i]) << 8;
#endif
writew(w, &i2c_base->data);
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2] ARM: OMAP5: redefine arm_setup_identity_mapping
We introduce an OMAP5 specific version of arm_setup_identity_mapping(), which
makes the first page of the identity mapping invalid.
We want to unmap the region near address zero on HS OMAP devices, to avoid
speculative accesses. Accessing this region causes security violations, which
we want to avoid.
Signed-off-by: Vincent Stehlé
Cc: Tom Rini
---
Changes for v2:
- Fix missing page_table argument
- Add extern definition to fix compilation warning
arch/arm/cpu/armv7/omap5/Makefile |1 +
arch/arm/cpu/armv7/omap5/cache-cp15.c | 46 +
2 files changed, 47 insertions(+)
create mode 100644 arch/arm/cpu/armv7/omap5/cache-cp15.c
diff --git a/arch/arm/cpu/armv7/omap5/Makefile
b/arch/arm/cpu/armv7/omap5/Makefile
index 9b261c4..49c454c 100644
--- a/arch/arm/cpu/armv7/omap5/Makefile
+++ b/arch/arm/cpu/armv7/omap5/Makefile
@@ -29,6 +29,7 @@ COBJS += hwinit.o
COBJS += clocks.o
COBJS += emif.o
COBJS += sdram.o
+COBJS += cache-cp15.o
SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c)
OBJS := $(addprefix $(obj),$(COBJS) $(SOBJS))
diff --git a/arch/arm/cpu/armv7/omap5/cache-cp15.c
b/arch/arm/cpu/armv7/omap5/cache-cp15.c
new file mode 100644
index 000..6ff4548
--- /dev/null
+++ b/arch/arm/cpu/armv7/omap5/cache-cp15.c
@@ -0,0 +1,46 @@
+/*
+ * (C) Copyright 2002
+ * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+ *
+ * (C) Copyright 2012
+ * Vincent Stehlé, Texas Instruments, v-ste...@ti.com.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include
+
+/* OMAP5 specific function to set up the identity mapping. */
+void arm_setup_identity_mapping(u32 *page_table)
+{
+ extern void __arm_setup_identity_mapping(u32 *page_table);
+
+ /*
+* Perform default mapping, which sets up an identity-mapping for all
+* 4GB, rw for everyone.
+*/
+ __arm_setup_identity_mapping(page_table);
+
+ /*
+* First page (starting at 0x0) is made invalid to avoid speculative
+* accesses in secure rom.
+* TODO: use second level descriptors for finer grained mapping.
+*/
+ page_table[0] = 0;
+}
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] ARM: OMAP5: redefine arm_setup_identity_mapping
Tom Rini: > Lets put the extern in arch/arm/include/asm/cache.h and make both files > #include . Sure, here is an updated patches pair: [PATCH v3 1/2] ARM: cache: introduce weak arm_setup_identity_mapping [PATCH v3 2/2] ARM: OMAP5: redefine arm_setup_identity_mapping Thank you for your reviews. Best regards, V. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v3 1/2] ARM: cache: introduce weak arm_setup_identity_mapping
Separate the MMU identity mapping for ARM in a weak function, to allow
redefinition with platform specific function.
This is motivated by the need to unmap the region near address zero on HS OMAP
devices, to avoid speculative accesses. Accessing this region causes security
violations, which we want to avoid.
Signed-off-by: Vincent Stehlé
Cc: Tom Rini
---
Changes for v3:
- Add definition of __arm_setup_identity_mapping() into asm/cache.h
- Fix comments style
arch/arm/include/asm/cache.h |1 +
arch/arm/lib/cache-cp15.c| 22 +++---
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/cache.h b/arch/arm/include/asm/cache.h
index eef6a5a..328377b 100644
--- a/arch/arm/include/asm/cache.h
+++ b/arch/arm/include/asm/cache.h
@@ -41,6 +41,7 @@ static inline void invalidate_l2_cache(void)
void l2_cache_enable(void);
void l2_cache_disable(void);
+void __arm_setup_identity_mapping(u32 *page_table);
/*
* The current upper bound for ARM L1 data cache line sizes is 64 bytes. We
diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index 939de10..f785ff5 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -23,6 +23,8 @@
#include
#include
+#include
+#include
#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
@@ -40,6 +42,17 @@ void __arm_init_before_mmu(void)
void arm_init_before_mmu(void)
__attribute__((weak, alias("__arm_init_before_mmu")));
+void __arm_setup_identity_mapping(u32 *page_table)
+{
+ int i;
+
+ /* Set up an identity-mapping for all 4GB, rw for everyone */
+ for (i = 0; i < 4096; i++)
+ page_table[i] = i << 20 | (3 << 10) | 0x12;
+}
+__weak void arm_setup_identity_mapping(u32 *page_table)
+ __attribute__((alias("__arm_setup_identity_mapping")));
+
static void cp_delay (void)
{
volatile int i;
@@ -72,9 +85,12 @@ static inline void mmu_setup(void)
u32 reg;
arm_init_before_mmu();
- /* Set up an identity-mapping for all 4GB, rw for everyone */
- for (i = 0; i < 4096; i++)
- page_table[i] = i << 20 | (3 << 10) | 0x12;
+
+ /*
+* Set up an identity-mapping. Default version maps all 4GB rw for
+* everyone
+*/
+ arm_setup_identity_mapping(page_table);
for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
dram_bank_mmu_setup(i);
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v3 2/2] ARM: OMAP5: redefine arm_setup_identity_mapping
We introduce an OMAP5 specific version of arm_setup_identity_mapping(), which
makes the first page of the identity mapping invalid.
We want to unmap the region near address zero on HS OMAP devices, to avoid
speculative accesses. Accessing this region causes security violations, which
we want to avoid.
Signed-off-by: Vincent Stehlé
Cc: Tom Rini
---
Changes for v3:
- Use definition of __arm_setup_identity_mapping() from asm/cache.h
Changes for v2:
- Fix missing page_table argument
- Add extern definition to fix compilation warning
arch/arm/cpu/armv7/omap5/Makefile |1 +
arch/arm/cpu/armv7/omap5/cache-cp15.c | 45 +
2 files changed, 46 insertions(+)
create mode 100644 arch/arm/cpu/armv7/omap5/cache-cp15.c
diff --git a/arch/arm/cpu/armv7/omap5/Makefile
b/arch/arm/cpu/armv7/omap5/Makefile
index 9b261c4..49c454c 100644
--- a/arch/arm/cpu/armv7/omap5/Makefile
+++ b/arch/arm/cpu/armv7/omap5/Makefile
@@ -29,6 +29,7 @@ COBJS += hwinit.o
COBJS += clocks.o
COBJS += emif.o
COBJS += sdram.o
+COBJS += cache-cp15.o
SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c)
OBJS := $(addprefix $(obj),$(COBJS) $(SOBJS))
diff --git a/arch/arm/cpu/armv7/omap5/cache-cp15.c
b/arch/arm/cpu/armv7/omap5/cache-cp15.c
new file mode 100644
index 000..8e4388c
--- /dev/null
+++ b/arch/arm/cpu/armv7/omap5/cache-cp15.c
@@ -0,0 +1,45 @@
+/*
+ * (C) Copyright 2002
+ * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+ *
+ * (C) Copyright 2012
+ * Vincent Stehlé, Texas Instruments, v-ste...@ti.com.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include
+#include
+
+/* OMAP5 specific function to set up the identity mapping. */
+void arm_setup_identity_mapping(u32 *page_table)
+{
+ /*
+* Perform default mapping, which sets up an identity-mapping for all
+* 4GB, rw for everyone.
+*/
+ __arm_setup_identity_mapping(page_table);
+
+ /*
+* First page (starting at 0x0) is made invalid to avoid speculative
+* accesses in secure rom.
+* TODO: use second level descriptors for finer grained mapping.
+*/
+ page_table[0] = 0;
+}
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] Update OMAP5 "unmap first page" patch series
Hi, Here is a proposed updated patch series to unmap page 0 on OMAP5, rebased on denx/master: [PATCH v4 1/3] ARM: cache: declare set_section_dcache [PATCH v4 2/3] ARM: cache: introduce weak arm_setup_identity_mapping [PATCH v4 3/3] ARM: OMAP5: redefine arm_setup_identity_mapping Necessary adaptations have been done to follow set_section_dcache() changes. Happy new year, V. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v4 1/3] ARM: cache: declare set_section_dcache
We declare the set_section_dcache function globally in the cache header, for later use by e.g. machine specific code. Signed-off-by: Vincent Stehlé Cc: Tom Rini --- arch/arm/include/asm/cache.h |1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/include/asm/cache.h b/arch/arm/include/asm/cache.h index eef6a5a..416d2c8 100644 --- a/arch/arm/include/asm/cache.h +++ b/arch/arm/include/asm/cache.h @@ -41,6 +41,7 @@ static inline void invalidate_l2_cache(void) void l2_cache_enable(void); void l2_cache_disable(void); +void set_section_dcache(int section, enum dcache_option option); /* * The current upper bound for ARM L1 data cache line sizes is 64 bytes. We -- 1.7.9.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v4 2/3] ARM: cache: introduce weak arm_setup_identity_mapping
Separate the MMU identity mapping for ARM in a weak function, to allow
redefinition with platform specific function.
This is motivated by the need to unmap the region near address zero on HS OMAP
devices, to avoid speculative accesses. Accessing this region causes security
violations, which we want to avoid.
Signed-off-by: Vincent Stehlé
Cc: Tom Rini
---
Changes for v4;
- Use set_section_dcache() function
- Remove page_table argument
Changes for v3:
- Add definition of __arm_setup_identity_mapping() into asm/cache.h
- Fix comments style
arch/arm/include/asm/cache.h |1 +
arch/arm/lib/cache-cp15.c| 22 +++---
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/cache.h b/arch/arm/include/asm/cache.h
index 416d2c8..b898dc5 100644
--- a/arch/arm/include/asm/cache.h
+++ b/arch/arm/include/asm/cache.h
@@ -42,6 +42,7 @@ static inline void invalidate_l2_cache(void)
void l2_cache_enable(void);
void l2_cache_disable(void);
void set_section_dcache(int section, enum dcache_option option);
+void __arm_setup_identity_mapping(void);
/*
* The current upper bound for ARM L1 data cache line sizes is 64 bytes. We
diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index 6edf815..fa7b0c5 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -23,6 +23,8 @@
#include
#include
+#include
+#include
#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
@@ -34,6 +36,17 @@ void __arm_init_before_mmu(void)
void arm_init_before_mmu(void)
__attribute__((weak, alias("__arm_init_before_mmu")));
+void __arm_setup_identity_mapping(void)
+{
+ int i;
+
+ /* Set up an identity-mapping for all 4GB, rw for everyone */
+ for (i = 0; i < 4096; i++)
+ set_section_dcache(i, DCACHE_OFF);
+}
+__weak void arm_setup_identity_mapping(void)
+ __attribute__((alias("__arm_setup_identity_mapping")));
+
static void cp_delay (void)
{
volatile int i;
@@ -101,9 +114,12 @@ static inline void mmu_setup(void)
u32 reg;
arm_init_before_mmu();
- /* Set up an identity-mapping for all 4GB, rw for everyone */
- for (i = 0; i < 4096; i++)
- set_section_dcache(i, DCACHE_OFF);
+
+ /*
+* Set up an identity-mapping. Default version maps all 4GB rw for
+* everyone
+*/
+ arm_setup_identity_mapping();
for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
dram_bank_mmu_setup(i);
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v4 3/3] ARM: OMAP5: redefine arm_setup_identity_mapping
We introduce an OMAP5 specific version of arm_setup_identity_mapping(), which
makes the first page of the identity mapping invalid.
We want to unmap the region near address zero on HS OMAP devices, to avoid
speculative accesses. Accessing this region causes security violations, which
we want to avoid.
Signed-off-by: Vincent Stehlé
Cc: Tom Rini
---
Changes for v4:
- Protect functions according to cache config
- Remove page_table argument
- Declare global data ptr and use it to retrieve page_table
Changes for v3:
- Use definition of __arm_setup_identity_mapping() from asm/cache.h
Changes for v2:
- Fix missing page_table argument
- Add extern definition to fix compilation warning
arch/arm/cpu/armv7/omap5/Makefile |1 +
arch/arm/cpu/armv7/omap5/cache-cp15.c | 52 +
2 files changed, 53 insertions(+)
create mode 100644 arch/arm/cpu/armv7/omap5/cache-cp15.c
diff --git a/arch/arm/cpu/armv7/omap5/Makefile
b/arch/arm/cpu/armv7/omap5/Makefile
index 9b261c4..49c454c 100644
--- a/arch/arm/cpu/armv7/omap5/Makefile
+++ b/arch/arm/cpu/armv7/omap5/Makefile
@@ -29,6 +29,7 @@ COBJS += hwinit.o
COBJS += clocks.o
COBJS += emif.o
COBJS += sdram.o
+COBJS += cache-cp15.o
SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c)
OBJS := $(addprefix $(obj),$(COBJS) $(SOBJS))
diff --git a/arch/arm/cpu/armv7/omap5/cache-cp15.c
b/arch/arm/cpu/armv7/omap5/cache-cp15.c
new file mode 100644
index 000..a9666f7
--- /dev/null
+++ b/arch/arm/cpu/armv7/omap5/cache-cp15.c
@@ -0,0 +1,52 @@
+/*
+ * (C) Copyright 2002
+ * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+ *
+ * (C) Copyright 2012
+ * Vincent Stehlé, Texas Instruments, v-ste...@ti.com.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include
+#include
+
+#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* OMAP5 specific function to set up the identity mapping. */
+void arm_setup_identity_mapping(void)
+{
+ u32 *page_table = (u32 *)gd->tlb_addr;
+
+ /*
+* Perform default mapping, which sets up an identity-mapping for all
+* 4GB, rw for everyone.
+*/
+ __arm_setup_identity_mapping();
+
+ /*
+* First page (starting at 0x0) is made invalid to avoid speculative
+* accesses in secure rom.
+* TODO: use second level descriptors for finer grained mapping.
+*/
+ page_table[0] = 0;
+}
+#endif
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 3/3] ARM: OMAP5: redefine arm_setup_identity_mapping
On 01/08/2013 12:14 PM, R Sricharan wrote: (..) > We had this problem of speculative aborts in the kernel uncompress code > as well, which maps all of 4GB address space. It was solved by setting > the non-DRAM region as non-executable(XN) and with client permissions > to the domain in the DACR register. > > This way speculative prefetches are avoided not only to the page 0, > but also to other read sensitive I/O regions. > > I have created a similar patch in u-boot and posted a RFC now. > I was using your first patch [1] and rest from me. (..) > > Please let me know your take on that. Hi Sricharan, Your solution to this issue looks more elegant to me than my unmapping page 0 completely. I tested your patches and they work for me on both GP (without security) and EMU (with security) OMAP5 ES1.0 devices. I'll keep them, thanks :) You can add my 'Tested-by' if you want: Tested-by: Vincent Stehlé Best regards, V. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] Fix omap5 hyp mode for second cpu
Make it work on PandaBoard 5 with 5432 ES2 and Linux.
Signed-off-by: Vincent Stehlé
---
Hi,
Here are some necessary adaptations for OMAP5 ES2, as magic value has changed.
In the mean time, we make the secondary cpu routine a bit closer to what
romcode does.
Best regards,
V.
arch/arm/lib/bootm.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 02852d6..dd8f42e 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -56,16 +56,21 @@ asm (
"ldr r12, =0x102\n"
"mov r0, pc\n"
"smc 0x1\n"
- "ldr r1, =0x48281804\n" // AUX_CORE_BOOT_1
+ "ldr r1, =0x48281800\n" /* AUX_CORE_BOOT_0 */
"mov r2, #0\n"
- "str r2, [r1]\n"
+ "str r2, [r1]\n"/* AUX_CORE_BOOT_0 */
+ "str r2, [r1, #4]\n"/* AUX_CORE_BOOT_1 */
"isb\n"
"dsb\n"
+ "mov r3, #0xf0\n"
"1: wfe\n"
- "ldr r2, [r1]\n"
- "cmp r2, #0\n"
- "movne pc, r2\n"
- "b 1b\n"
+ "ldr r2, [r1]\n" /* AUX_CORE_BOOT_0 */
+ "ands r2, r2, r3\n"
+ "beq 1b\n"
+ "ldr r2, [r1, #4]\n" /* AUX_CORE_BOOT_1 */
+ "cmp r2, #0\n"
+ "beq 1b\n"
+ "bx r2\n"
".popsection\n"
);
@@ -378,7 +383,7 @@ void hyp_enable(void) {
"ldr r1, =0x48281800\n" // AUX_CORE_BOOT_1
"ldr r2, =__hyp_init_sec\n"
"str r2, [r1, #4]\n"
- "mov r2, #0x200\n"
+ "mov r2, #0x20\n"
"str r2, [r1]\n"// AUX_CORE_BOOT_0
"isb\n"
"dmb\n"
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [RFC, PATCH] omap: Invalidate first page to avoid speculation
Hello u-boot list,
Here is a "request for comments" on the best way to solve a little
"speculation" issue on recent OMAPs. Any guidance/feedback on the way to go
would be greatly appreciated, please.
I am using u-boot on an OMAP5 HS device (with security, that is), and I am
experiencing "security violations" due to speculative accesses done by the
Cortex-A15 processor to the region near address zero. This region is a secure
region, where non-secure accesses are forbidden and reported by the security
firmware on an OMAP HS device. On an OMAP GP device, those accesses may very
well exist, but are silently ignored by the firmware. Note that the speculative
accesses are not actual functional accesses, so their being aborted does not
harm the functionality of u-boot as it is.
A quick (and dirty) solution is to mark the region near address zero as being
invalid, which prevents the processor from doing speculative accesses there
(see patch).
This patch as it is has a number of issues: it impacts all ARM devices and it
unmaps too large a region. I am not sure how to cleanly rework the patch so
that it would be made OMAP-only cleanly. Also, unmapping a smaller region to
better fit the hardware characteristics would require using second level
descriptors, and I do not know if this is recommended. To make this worse,
chips in the OMAP family have differences in their secure rom boundaries.
Does the u-boot community feels this issue needs to be addressed? What would be
the best way to solve this?
Best regards,
V.
Signed-off-by: Vincent Stehlé
---
arch/arm/lib/cache-cp15.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index 939de10..57e1974 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -72,8 +72,13 @@ static inline void mmu_setup(void)
u32 reg;
arm_init_before_mmu();
+
+ /* First page (starting at 0x0) is made invalid to avoid
+* speculative accesses in secure rom. */
+ page_table[0] = 0;
+
/* Set up an identity-mapping for all 4GB, rw for everyone */
- for (i = 0; i < 4096; i++)
+ for (i = 1; i < 4096; i++)
page_table[i] = i << 20 | (3 << 10) | 0x12;
for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC, PATCH v2] omap: Invalidate first page to avoid speculation
Albert Aribaud: > To make this affect only some CPUs or even boards, you can define and use a > weak function which would handle filling the page-table; the weak, default, > function would fill table[0] like others, while OMAP5 would have a strong > version which would clear table[0]. Hi Albert, Thank you very much for your comments. Here is "v2" of the patch, reworked to follow your advice. Comments are welcome. I think it is now cleaner to split it in two patches: [PATCH 1/2] ARM: cache: introduce weak arm_setup_identity_mapping [PATCH 2/2] ARM: OMAP5: redefine arm_setup_identity_mapping I picked the 'arm_setup_identity_mapping' name more or less arbitrarily, please advise if not. I verified this patch series on an OMAP5 HS device (with security) and on a GP device (without security), and both work for me. Best regards, V. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/2] ARM: cache: introduce weak arm_setup_identity_mapping
Separate the MMU identity mapping for ARM in a weak function, to allow
redefinition with platform specific function.
This is motivated by the need to unmap the region near address zero on HS OMAP
devices, to avoid speculative accesses. Accessing this region causes security
violations, which we want to avoid.
Signed-off-by: Vincent Stehlé
---
arch/arm/lib/cache-cp15.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index 939de10..886fe5c 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -40,6 +40,17 @@ void __arm_init_before_mmu(void)
void arm_init_before_mmu(void)
__attribute__((weak, alias("__arm_init_before_mmu")));
+void __arm_setup_identity_mapping(u32 *page_table)
+{
+ int i;
+
+ /* Set up an identity-mapping for all 4GB, rw for everyone */
+ for (i = 0; i < 4096; i++)
+ page_table[i] = i << 20 | (3 << 10) | 0x12;
+}
+void arm_setup_identity_mapping(u32 *page_table)
+ __attribute__((weak, alias("__arm_setup_identity_mapping")));
+
static void cp_delay (void)
{
volatile int i;
@@ -72,9 +83,10 @@ static inline void mmu_setup(void)
u32 reg;
arm_init_before_mmu();
- /* Set up an identity-mapping for all 4GB, rw for everyone */
- for (i = 0; i < 4096; i++)
- page_table[i] = i << 20 | (3 << 10) | 0x12;
+
+ /* Set up an identity-mapping. Default version maps all 4GB rw for
+* everyone */
+ arm_setup_identity_mapping(page_table);
for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
dram_bank_mmu_setup(i);
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/2] ARM: OMAP5: redefine arm_setup_identity_mapping
We introduce an OMAP5 specific version of arm_setup_identity_mapping(), which
makes the first page of the identity mapping invalid.
We want to unmap the region near address zero on HS OMAP devices, to avoid
speculative accesses. Accessing this region causes security violations, which
we want to avoid.
Signed-off-by: Vincent Stehlé
---
arch/arm/cpu/armv7/omap5/Makefile |1 +
arch/arm/cpu/armv7/omap5/cache-cp15.c | 40 +
2 files changed, 41 insertions(+)
create mode 100644 arch/arm/cpu/armv7/omap5/cache-cp15.c
diff --git a/arch/arm/cpu/armv7/omap5/Makefile
b/arch/arm/cpu/armv7/omap5/Makefile
index 9b261c4..49c454c 100644
--- a/arch/arm/cpu/armv7/omap5/Makefile
+++ b/arch/arm/cpu/armv7/omap5/Makefile
@@ -29,6 +29,7 @@ COBJS += hwinit.o
COBJS += clocks.o
COBJS += emif.o
COBJS += sdram.o
+COBJS += cache-cp15.o
SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c)
OBJS := $(addprefix $(obj),$(COBJS) $(SOBJS))
diff --git a/arch/arm/cpu/armv7/omap5/cache-cp15.c
b/arch/arm/cpu/armv7/omap5/cache-cp15.c
new file mode 100644
index 000..506cc00
--- /dev/null
+++ b/arch/arm/cpu/armv7/omap5/cache-cp15.c
@@ -0,0 +1,40 @@
+/*
+ * (C) Copyright 2002
+ * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+ *
+ * (C) Copyright 2012
+ * Vincent Stehlé, Texas Instruments, v-ste...@ti.com.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include
+
+/* OMAP5 specific function to set up the identity mapping. */
+void arm_setup_identity_mapping(u32 *page_table)
+{
+ /* Perform default mapping, which sets up an identity-mapping for all
+* 4GB, rw for everyone */
+ __arm_setup_identity_mapping();
+
+ /* First page (starting at 0x0) is made invalid to avoid speculative
+* accesses in secure rom. TODO: use second level descriptors for finer
+* grained mapping. */
+ page_table[0] = 0;
+}
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [RFC, PATCH v3] omap: Invalidate first page to avoid speculation
Tom Rini: > Aside from that (and the comment to 1/2) I think we're good here, > including the naming of the function. Thanks! Hi Tom, Thank you very much for your review. Here is v3 of the patch series with reworks for __weak and comments style: [PATCH 1/2] ARM: cache: introduce weak arm_setup_identity_mapping [PATCH 2/2] ARM: OMAP5: redefine arm_setup_identity_mapping This boots ok on OMAP5 HS device. Best regards, V. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/2] ARM: cache: introduce weak arm_setup_identity_mapping
Separate the MMU identity mapping for ARM in a weak function, to allow
redefinition with platform specific function.
This is motivated by the need to unmap the region near address zero on HS OMAP
devices, to avoid speculative accesses. Accessing this region causes security
violations, which we want to avoid.
Signed-off-by: Vincent Stehlé
---
arch/arm/lib/cache-cp15.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index 939de10..0b87d93 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -23,6 +23,7 @@
#include
#include
+#include
#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
@@ -40,6 +41,17 @@ void __arm_init_before_mmu(void)
void arm_init_before_mmu(void)
__attribute__((weak, alias("__arm_init_before_mmu")));
+void __arm_setup_identity_mapping(u32 *page_table)
+{
+ int i;
+
+ /* Set up an identity-mapping for all 4GB, rw for everyone */
+ for (i = 0; i < 4096; i++)
+ page_table[i] = i << 20 | (3 << 10) | 0x12;
+}
+__weak void arm_setup_identity_mapping(u32 *page_table)
+ __attribute__((alias("__arm_setup_identity_mapping")));
+
static void cp_delay (void)
{
volatile int i;
@@ -72,9 +84,10 @@ static inline void mmu_setup(void)
u32 reg;
arm_init_before_mmu();
- /* Set up an identity-mapping for all 4GB, rw for everyone */
- for (i = 0; i < 4096; i++)
- page_table[i] = i << 20 | (3 << 10) | 0x12;
+
+ /* Set up an identity-mapping. Default version maps all 4GB rw for
+* everyone */
+ arm_setup_identity_mapping(page_table);
for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
dram_bank_mmu_setup(i);
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/2] ARM: OMAP5: redefine arm_setup_identity_mapping
We introduce an OMAP5 specific version of arm_setup_identity_mapping(), which
makes the first page of the identity mapping invalid.
We want to unmap the region near address zero on HS OMAP devices, to avoid
speculative accesses. Accessing this region causes security violations, which
we want to avoid.
Signed-off-by: Vincent Stehlé
---
arch/arm/cpu/armv7/omap5/Makefile |1 +
arch/arm/cpu/armv7/omap5/cache-cp15.c | 44 +
2 files changed, 45 insertions(+)
create mode 100644 arch/arm/cpu/armv7/omap5/cache-cp15.c
diff --git a/arch/arm/cpu/armv7/omap5/Makefile
b/arch/arm/cpu/armv7/omap5/Makefile
index 9b261c4..49c454c 100644
--- a/arch/arm/cpu/armv7/omap5/Makefile
+++ b/arch/arm/cpu/armv7/omap5/Makefile
@@ -29,6 +29,7 @@ COBJS += hwinit.o
COBJS += clocks.o
COBJS += emif.o
COBJS += sdram.o
+COBJS += cache-cp15.o
SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c)
OBJS := $(addprefix $(obj),$(COBJS) $(SOBJS))
diff --git a/arch/arm/cpu/armv7/omap5/cache-cp15.c
b/arch/arm/cpu/armv7/omap5/cache-cp15.c
new file mode 100644
index 000..afd339a
--- /dev/null
+++ b/arch/arm/cpu/armv7/omap5/cache-cp15.c
@@ -0,0 +1,44 @@
+/*
+ * (C) Copyright 2002
+ * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+ *
+ * (C) Copyright 2012
+ * Vincent Stehlé, Texas Instruments, v-ste...@ti.com.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include
+
+/* OMAP5 specific function to set up the identity mapping. */
+void arm_setup_identity_mapping(u32 *page_table)
+{
+ /*
+* Perform default mapping, which sets up an identity-mapping for all
+* 4GB, rw for everyone.
+*/
+ __arm_setup_identity_mapping();
+
+ /*
+* First page (starting at 0x0) is made invalid to avoid speculative
+* accesses in secure rom.
+* TODO: use second level descriptors for finer grained mapping.
+*/
+ page_table[0] = 0;
+}
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] .gitignore: ignore generated u-boot.lst
Signed-off-by: Vincent Stehlé --- .gitignore |1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 1ac43f2..3f728ca 100644 --- a/.gitignore +++ b/.gitignore @@ -55,6 +55,7 @@ /reloc_off /include/generated/ +/include/u-boot.lst asm-offsets.s # stgit generated dirs -- 1.7.9.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] .gitignore: ignore generated u-boot.lst
On 11/21/2012 09:00 PM, Luka Perkov wrote: > You should have also removed "/u-boot.lst": Hi Luka, I think it is already ignored in denx/master (commit 178d0cc1a4c73c3341afbeb2a93b172de8c96bd1). Best regards, V. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] .gitignore: update path for generated u-boot.lst
Luka: (/u-boot.lst) > It is ignored, but it does not need to be. The file is not in that location. Ah! Thanks for the clarification; here is an updated patch. Best regards, V. --->8--- Signed-off-by: Vincent Stehlé --- .gitignore |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 1ac43f2..9afb504 100644 --- a/.gitignore +++ b/.gitignore @@ -38,7 +38,6 @@ /u-boot.sha1 /u-boot.dis /u-boot.lds -/u-boot.lst /u-boot.ubl /u-boot.ais /u-boot.dtb @@ -55,6 +54,7 @@ /reloc_off /include/generated/ +/include/u-boot.lst asm-offsets.s # stgit generated dirs -- 1.7.9.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] tools/proftool: fix use-after-free
The read_trace_config() can dereference the line pointer after freeing
it on its error path. Avoid that.
This was found by Coverity Scan.
Signed-off-by: Vincent Stehlé
Cc: Simon Glass
---
tools/proftool.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/proftool.c b/tools/proftool.c
index 9ce7a77..ddf870f 100644
--- a/tools/proftool.c
+++ b/tools/proftool.c
@@ -432,9 +432,10 @@ static int read_trace_config(FILE *fin)
err = regcomp(&line->regex, tok, REG_NOSUB);
if (err) {
+ int r = regex_report_error(&line->regex, err,
+ "compile", tok);
free(line);
- return regex_report_error(&line->regex, err, "compile",
- tok);
+ return r;
}
/* link this new one to the end of the list */
--
2.5.3
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] tools/proftool: fix use-after-free
On 10/07/2015 04:19 PM, Tom Rini wrote: .. > Were you in the Coverity talk too? :) Hi Tom, No, I was not following that talk, sorry. .. > free(line); > - return regex_report_error(&line->regex, err, > "compile", > + err = regex_report_error(&line->regex, err, "compile", > tok); > + return err; I am not sure you solve the problem this way. Indeed the structure pointed to by the line pointer will still have been freed before use even this way. Who knows what the memory contains when regerror() will access &line->regex, which is contained into the freed structure? Best regards, V. signature.asc Description: OpenPGP digital signature ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] mx6: invalidate D-cache only when booting from USB
Add a detection at runtime of the boot from USB on i.MX6, and invalidate
the D-cache only in that case.
The USB boot detection method is taken from Freescale u-boot commit
1309b1ed78b3 ("ENGR00315499-8 Auto check if boot from usb").
This repairs u-boot when it is built with CONFIG_SKIP_LOWLEVEL_INIT
defined, and is booted from another u-boot, which booted from SD card,
for example.
Signed-off-by: Vincent Stehlé
Cc: Stefano Babic
Cc: Fabio Estevam
Cc: Frank Li
Cc: Nitin Garg
---
arch/arm/cpu/armv7/mx6/soc.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c
index 21ef9d0..774f078 100644
--- a/arch/arm/cpu/armv7/mx6/soc.c
+++ b/arch/arm/cpu/armv7/mx6/soc.c
@@ -350,6 +350,18 @@ int board_postclk_init(void)
return 0;
}
+/*
+ * Determine if we booted from USB.
+ *
+ * We look at the USBPH0_PWD0.RXPWDRX register to determine if the USB
+ * PHY is powered on or off. If the USB PHY is turned on, we assume that
+ * the ROM booted us from USB.
+ */
+static bool is_boot_from_usb(void)
+{
+ return !(readl(USB_PHY0_BASE_ADDR) & (1<<20));
+}
+
#ifndef CONFIG_SYS_DCACHE_OFF
void enable_caches(void)
{
@@ -360,7 +372,8 @@ void enable_caches(void)
#endif
/* Avoid random hang when download by usb */
- invalidate_dcache_all();
+ if (is_boot_from_usb())
+ invalidate_dcache_all();
/* Enable D-cache. I-cache is already enabled in start.S */
dcache_enable();
--
2.1.4
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6: invalidate D-cache only when booting from USB
On 05/22/2015 05:15 PM, Li Frank-B20596 wrote: .. > How about second uboot save in usb mass storage device? Hi Frank, I cannot test but I guess this will behave the same as without the patch, which means that it will probably not boot. Granted, the detection method in is_boot_from_usb() is an approximation, but it repairs one case already and should not break anything. Best regards, V. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] mx6: repair sl and slx load addresses
Commit 8183058188cd ("imx6: centralise common boot options in
mx6_common.h") changed the LOADADDR and SYS_TEXT_BASE for all mx6
boards, setting them to 0x1200 and 0x1780 respectively.
This does not work for i.MX6SL and i.MX6 SoloX based boards, which have
their DDR starting at 0x8000.
We allow to override LOADADDR from mx6_common.h and restore previous
LOADADDR and SYS_TEXT_BASE for SL and SoloX based boards.
This repairs the boot for i.MX6 SoloX Sabre SD, and should also repair
SL EVK and Warp board.
Signed-off-by: Vincent Stehlé
Cc: Peter Robinson
Cc: Fabio Estevam
Cc: Otavio Salvador
---
Hi,
This patch repairs for sure mx6sxsabresd with latest u-boot.
I am pretty sure that mx6slevk and warp are broken in current u-boot and
are repaired by this patch, but I cannot test, only compile.
Also, I noticed that the following boards have different LOADADDR and/or
SYS_TEST_BASE after the aforementioned commit:
- cm_fx6
- novena
- tbs2910
The current patch does not touch those and assumes they still work with
latest u-boot. Maybe a test or two to confirm would not harm...
Testers welcome!
Best regards,
V.
include/configs/mx6_common.h | 2 ++
include/configs/mx6slevk.h | 3 +++
include/configs/mx6sxsabresd.h | 3 +++
include/configs/warp.h | 3 +++
4 files changed, 11 insertions(+)
diff --git a/include/configs/mx6_common.h b/include/configs/mx6_common.h
index 233c6d2..2367374 100644
--- a/include/configs/mx6_common.h
+++ b/include/configs/mx6_common.h
@@ -53,7 +53,9 @@
#define CONFIG_REVISION_TAG
/* Boot options */
+#ifndef CONFIG_LOADADDR
#define CONFIG_LOADADDR0x1200
+#endif
#define CONFIG_SYS_LOAD_ADDR CONFIG_LOADADDR
#ifndef CONFIG_SYS_TEXT_BASE
#define CONFIG_SYS_TEXT_BASE 0x1780
diff --git a/include/configs/mx6slevk.h b/include/configs/mx6slevk.h
index 2b8bb2a..52f2442 100644
--- a/include/configs/mx6slevk.h
+++ b/include/configs/mx6slevk.h
@@ -9,6 +9,9 @@
#ifndef __CONFIG_H
#define __CONFIG_H
+#define CONFIG_LOADADDR0x8200
+#define CONFIG_SYS_TEXT_BASE 0x8780
+
#include "mx6_common.h"
#define MACH_TYPE_MX6SLEVK 4307
diff --git a/include/configs/mx6sxsabresd.h b/include/configs/mx6sxsabresd.h
index 46e1262..d62b3bf 100644
--- a/include/configs/mx6sxsabresd.h
+++ b/include/configs/mx6sxsabresd.h
@@ -10,6 +10,9 @@
#ifndef __CONFIG_H
#define __CONFIG_H
+#define CONFIG_LOADADDR0x8080
+#define CONFIG_SYS_TEXT_BASE 0x8780
+
#include "mx6_common.h"
#ifdef CONFIG_SPL
diff --git a/include/configs/warp.h b/include/configs/warp.h
index 2673948..25f24c1 100644
--- a/include/configs/warp.h
+++ b/include/configs/warp.h
@@ -13,6 +13,9 @@
#ifndef __CONFIG_H
#define __CONFIG_H
+#define CONFIG_LOADADDR0x8200
+#define CONFIG_SYS_TEXT_BASE 0x8780
+
#include "mx6_common.h"
/* Size of malloc() pool */
--
2.1.4
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3] mx6_common: Fix LOADADDR and SYS_TEXT_BASE for MX6SL and MX6SX
On 05/28/2015 05:33 PM, Fabio Estevam wrote:
..
> Build tested only
Hi Fabio,
Thank you for that fix. I did not realize you had a patch "on-going"
when sending mine; sorry. Your solution is cleaner and more generic than
mine anyway :)
This works fine for me and does indeed repair the boot on i.MX6 SoloX
Sabre SD.
Tested-by: Vincent Stehlé
Best regards,
V.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] Booting Wandboard through USB
Hi,
Is it still possible to boot u-boot on Wandboard through USB, please?
U-boot has switched to SPL for Wandboard recently[1] and I wonder if it
did not lose the possibility to boot the Wandboard through USB in the
mean time.
Granted, I can generate an u-boot-with-spl.imx suitable for boot through
USB, with tools such as imx_usb_loader[2], but I am not able to compile
SPL with usb or ethernet support. Is loading the second stage image from
SD card the only way now?
Thanks for any advice.
Best regards,
V.
[1] Commit 0d1ea05210f3 ("wandboard: Switch to SPL support")
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/220799
[2] https://github.com/boundarydevices/imx_usb_loader
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Booting Wandboard through USB
On 05/30/2015 06:49 PM, Tom Rini wrote: .. > The second would be trying to "fake" things such that for > imx_usb_loader you can pass both SPL and u-boot.img, and SPL is > run, inits memory and just exists and then u-boot.img is loaded > and run, similar to how with the non-SPL case you can use the > loader to pass in u-boot.imx as well as a kernel, ramdisk, etc, > into DDR. I wonder if we could use the i.MX6 ROM "plugin"[1] mechanism with u-boot SPL, to download it through USB, have it configure the DDR and return to the ROM, in a way that would allow us to resume downloading the second stage u-boot through USB... Best regards, V. [1] Chapter "8.8 Plugin Image" of the i.MX6D/Q Reference Manual. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [PATCH] efi_loader: Fix loaded image alignment
On Mon, Oct 11, 2021 at 03:10:23PM +0300, Ilias Apalodimas wrote: > We are ignoring the alignment communicated via the PE/COFF header. > Starting 5.10 the Linux kernel will loudly complain about it. For more > details look at [1] (in linux kernel). > > So add a function that can allocate aligned EFI memory and use it for our > relocated loaded image. Hi Ilias, Thank you for this fix. I verified that Linux v5.14.3 EFI stub complains about not being aligned to 64k without this fix and is happy with it, on the following systems: - qemu with U-Boot latest (after v2021.10) - Pine64 ROCKPro64 with U-Boot "near" v2021.07[1] - Lenovo Leez P710 with U-Boot v2021.07[2] - Compulab IOT-GATE-iMX8 with U-Boot "near" v2021.10-rc3[3] Feel free to add (or not): Tested-by: Vincent Stehlé Best regards, Vincent Stehlé System Architect - Arm [1]: https://gitlab.arm.com/systemready/firmware-build/rk3399-manifest/-/blob/rockpro64-21.09/README.md [2]: https://gitlab.arm.com/systemready/firmware-build/rk3399-manifest/-/blob/leez-21.08/README.md [3]: https://git.linaro.org/people/paul.liu/systemready/build-scripts.git/tree/docs/iotgateimx8_building_running.md
[BUG] USB boot issue on ROCKPro64 with UEFI
Hi U-Boot folks,
Hopefully this is the right way to report bugs. If not, please do not hesitate
to let me know.
I am hitting an issue with U-Boot v2021.07 on the ROCKPro64, when booting Linux
with UEFI from USB. The kernel EFI stub will hang:
EFI stub: Booting Linux Kernel...
EFI stub: Using DTB from configuration table
EFI stub: Exiting boot services and installing virtual address map...
After tracking it down, it appears efi_exit_boot_services() is ultimately
calling OHCI hc_reset(), which hangs at this line:
1804 if (ohci_readl(&ohci->regs->control) & OHCI_CTRL_IR) {
This seems to indicate reading the OHCI hardware control register cannot
complete, which hints at a clocking issue.
Looking in more details at the clocks changes performed on the
efi_exit_boot_services() path, a workaround is indeed to prevent the USB2PHY
clocks from being disabled (see the patch following this e-mail).
I don't know enough about the RK3399 clock tree to fully understand what is
going on here, and what would be a proper fix. Hopefully others will be able to
continue from there.
Best regards,
Vincent Stehlé
System Architect - Arm
Vincent Stehlé (1):
clk: rk3399: do not disable the USB2PHY clk
drivers/clk/rockchip/clk_rk3399.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.30.2
[PATCH WORKAROUND] clk: rk3399: do not disable the USB2PHY clk
When booting from USB with UEFI on the ROCKPro64, the Linux kernel EFI stub will hang in efi_exit_boot_services due to OHCI hc_reset accessing the hardware registers after the USB2PHY clocks have been disabled. As a workaround, prevent the USB2PHY clocks from being disabled to repair the boot. Signed-off-by: Vincent Stehlé --- THIS IS A WORKAROUND PLEASE DO NOT APPLY! drivers/clk/rockchip/clk_rk3399.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/clk/rockchip/clk_rk3399.c b/drivers/clk/rockchip/clk_rk3399.c index f8cbda44551..9cf7d26a026 100644 --- a/drivers/clk/rockchip/clk_rk3399.c +++ b/drivers/clk/rockchip/clk_rk3399.c @@ -1213,10 +1213,10 @@ static int rk3399_clk_disable(struct clk *clk) rk_setreg(&priv->cru->clkgate_con[5], BIT(6)); break; case SCLK_USB2PHY0_REF: - rk_setreg(&priv->cru->clkgate_con[6], BIT(5)); + /* WORKAROUND: leave enabled */ break; case SCLK_USB2PHY1_REF: - rk_setreg(&priv->cru->clkgate_con[6], BIT(6)); + /* WORKAROUND: leave enabled */ break; case ACLK_GMAC: rk_setreg(&priv->cru->clkgate_con[32], BIT(0)); -- 2.30.2
Re: [BUG] USB boot issue on ROCKPro64 with UEFI
On Fri, Sep 03, 2021 at 07:02:13PM +0200, Arnaud Patard wrote: .. > I may be wrong but seems like the issue solved by > https://patchwork.ozlabs.org/project/uboot/patch/20210406151059.1187379-1-icen...@aosc.io/ > Unfortunately, I think that there has been no progress since the > submission of this patch. Hi Arnaud, You are right, this patch solves the issue I am seeing on the ROCKPro64. Thanks for the pointer. Best regards, Vincent Stehlé System Architect - Arm
[PATCH] qemu: common: Fix build with update capsule
The common emulation Makefile has a dependency on a non-existent
qemu_capsule.o when building with support for capsule update enabled
(CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y).
The code which was in qemu_capsule.c has been completely moved to
lib/efi_loader/efi_capsule.c by commit 7a6fb28c8e4b ("efi_loader: capsule:
add back efi_get_public_key_data()").
Remove the false dependency.
This fixes the following build error:
make[1]: *** No rule to make target 'board/emulation/common/qemu_capsule.o',
needed by 'board/emulation/common/built-in.o'. Stop.
Fixes: commit 47a25e81d35c ("Revert "efi_capsule: Move signature from DTB to
.rodata"")
Signed-off-by: Vincent Stehlé
Cc: Simon Glass
---
board/emulation/common/Makefile | 1 -
1 file changed, 1 deletion(-)
diff --git a/board/emulation/common/Makefile b/board/emulation/common/Makefile
index 7ed447a69dc..c5b452e7e34 100644
--- a/board/emulation/common/Makefile
+++ b/board/emulation/common/Makefile
@@ -2,4 +2,3 @@
obj-$(CONFIG_SYS_MTDPARTS_RUNTIME) += qemu_mtdparts.o
obj-$(CONFIG_SET_DFU_ALT_INFO) += qemu_dfu.o
-obj-$(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT) += qemu_capsule.o
--
2.33.0
Re: [PATCH v3 0/3] efi: Start tidying up memory management
On Sun, Sep 01, 2024 at 04:22:56PM -0600, Simon Glass wrote: > We have been discussing the state of EFI memory management for some > years so I thought it might be best to send a short series showing some > of the issues we have talked about. > > This one just deals with memory allocation. It provides a way to detect > EFI 'page allocations' when U-Boot' malloc() should be used. It should > help us to clean up this area of U-Boot. > > It also updates EFI to use U-Boot memory allocation for the pool. Most > functions use that anyway and it is much more efficient. It also avoids > allocating things 'in space' in conflict with the loaded images. Hi Simon, Thank you for this series. I did test it on top of v2024.10-rc4 with AArch64 simulators (FVP & Qemu), with OS boots and ACS-IR, and I see no regression. Same with sandbox on x86 and the python tests. Best regards, Vincent. > > This series also includes a little patch to show more information in > the index for the EFI pages. > > Note that this series is independent from Sugosh's lmb series[1], > although I believe it will point the way to simplifying some of the > later patches in that series. > > Overall, some issues which should be resolved are: > - allocation inconsistency, e.g. efi_add_protocol() uses malloc() but > efi_dp_dup() does not (this series makes a start) > - change efi_bl_init() to register events later, when the devices are > actually used > - rather than efi_set_bootdev(), provide a bootstd way to take note of > the device images came from and allow EFI to query that when needed > - EFI_LOADER_BOUNCE_BUFFER - can use U-Boot bounce buffers? > > Minor questions on my mind: > - is unaligned access useful? Is there a performance penalty? > - API confusion about whether something is an address or a pointer > > [1] https://patchwork.ozlabs.org/project/uboot/list/?series=416441 > > Changes in v3: > - Add new patch to drop the memset() from efi_alloc() > - Drop patch to convert device_path allocation to use malloc() > > Changes in v2: > - Drop patch 'Show more information in efi index' > - Drop patch 'Avoid pool allocation in efi_get_memory_map_alloc()' > - Add the word 'warning', use log_warning() and show the end address > > Simon Glass (3): > efi: Drop the memset() from efi_alloc() > efi: Allow use of malloc() for the EFI pool > efi: Show the location of the bounce buffer > > common/dlmalloc.c| 7 +++ > include/efi_loader.h | 29 - > include/malloc.h | 7 +++ > lib/efi_loader/efi_bootbin.c | 11 > lib/efi_loader/efi_memory.c | 119 --- > 5 files changed, 136 insertions(+), 37 deletions(-) > > -- > 2.34.1 >
[PATCH] arm: qemu: fix update_info declaration
Add a missing comma in the update_info structure declaration.
This fixes the following build error when building with
EFI_RUNTIME_UPDATE_CAPSULE or EFI_CAPSULE_ON_DISK:
board/emulation/qemu-arm/qemu-arm.c:52:9: error: request for member ‘images’
in something not a structure or union
Fixes: cccea18813c4 ("efi_loader: add the number of image entries in
efi_capsule_update_info")
Signed-off-by: Vincent Stehlé
Cc: Masahisa Kojima
Cc: Tuomas Tynkkynen
Cc: Tom Rini
---
board/emulation/qemu-arm/qemu-arm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/emulation/qemu-arm/qemu-arm.c
b/board/emulation/qemu-arm/qemu-arm.c
index 6095cb02b23..e0e18b4dfea 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -48,7 +48,7 @@ struct efi_fw_image fw_images[] = {
};
struct efi_capsule_update_info update_info = {
- .num_images = ARRAY_SIZE(fw_images)
+ .num_images = ARRAY_SIZE(fw_images),
.images = fw_images,
};
--
2.45.2
[PATCH] lib: uuid: support more efi protocols in uuid_guid_get_str()
Add more EFI protocols GUIDs to the translation table used by
uuid_guid_get_str().
Signed-off-by: Vincent Stehlé
Cc: Tom Rini
---
Hi,
While the EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL_GUID, the
EFI_HII_CONFIG_ACCESS_PROTOCOL_GUID and the EFI_LOAD_FILE_PROTOCOL_GUID are
actually used by the efi_loader, this is not the case with the
EFI_DISK_IO_PROTOCOL_GUID.
I think we should add this GUID to the table nonetheless, as it can help
with debugging EFI applications and we already have this GUID's definition
in the headers.
If that is not desired, please do not hesitate to drop the first hunk of
this patch.
Best regards,
Vincent.
lib/uuid.c | 16
1 file changed, 16 insertions(+)
diff --git a/lib/uuid.c b/lib/uuid.c
index 97388f597a6..75658778044 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -119,6 +119,10 @@ static const struct {
"Block IO",
EFI_BLOCK_IO_PROTOCOL_GUID,
},
+ {
+ "Disk IO",
+ EFI_DISK_IO_PROTOCOL_GUID,
+ },
{
"Simple File System",
EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID,
@@ -127,6 +131,10 @@ static const struct {
"Loaded Image",
EFI_LOADED_IMAGE_PROTOCOL_GUID,
},
+ {
+ "Loaded Image Device Path",
+ EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL_GUID,
+ },
{
"Graphics Output",
EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID,
@@ -139,10 +147,18 @@ static const struct {
"HII Database",
EFI_HII_DATABASE_PROTOCOL_GUID,
},
+ {
+ "HII Config Access",
+ EFI_HII_CONFIG_ACCESS_PROTOCOL_GUID,
+ },
{
"HII Config Routing",
EFI_HII_CONFIG_ROUTING_PROTOCOL_GUID,
},
+ {
+ "Load File",
+ EFI_LOAD_FILE_PROTOCOL_GUID,
+ },
{
"Load File2",
EFI_LOAD_FILE2_PROTOCOL_GUID,
--
2.45.2
[PATCH] imx8m: soc: cope with existing optee node
On i.MX8M SoCs, the /firmware/optee Devicetree node is created just before
booting the OS when OP-TEE is found running. If the node already exists,
this results in an error, which prevents the OS to boot:
Could not create optee node.
ERROR: system-specific fdt fixup failed: FDT_ERR_EXISTS
- must RESET the board to recover.
failed to process device tree
On the i.MX8M systems where CONFIG_OF_SYSTEM_SETUP is defined, the
ft_add_optee_node() function is called before booting the OS. It will
create the OP-TEE Devicetree node and populate it with reserved memory
informations gathered at runtime.
On on most i.MX8M systems the Devicetree is built with an optee node if
CONFIG_OPTEE is defined. This node is indeed necessary for commands and
drivers communicating with OP-TEE, even before attempting OS boot.
The aforementioned issue can happen on the Compulab IOT-GATE-iMX8, which is
the only in-tree i.MX8M system where both CONFIG_OPTEE and
CONFIG_OF_SYSTEM_SETUP are defined (see the imx8mm-cl-iot-gate*
defconfigs).
Deal with an existing optee node gracefully at runtime to fix this issue.
Signed-off-by: Vincent Stehlé
Cc: Stefano Babic
Cc: Fabio Estevam
Cc: Tom Rini
Cc: Tim Harvey
---
arch/arm/mach-imx/imx8m/soc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index 85dc8b51a14..567e8e9e81a 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -1270,8 +1270,9 @@ static int ft_add_optee_node(void *fdt, struct bd_info
*bd)
}
}
+ /* Locate the optee node if it exists or create it. */
subpath = "optee";
- offs = fdt_add_subnode(fdt, offs, subpath);
+ offs = fdt_find_or_add_subnode(fdt, offs, subpath);
if (offs < 0) {
printf("Could not create %s node.\n", subpath);
return offs;
--
2.47.2
Re: [PATCH] imx8m: soc: cope with existing optee node
On Wed, Mar 12, 2025 at 11:12:35AM -0300, Fabio Estevam wrote:
> Hi Vincent,
>
> On Mon, Mar 10, 2025 at 9:36 AM Vincent Stehlé wrote:
>
> > --- a/arch/arm/mach-imx/imx8m/soc.c
> > +++ b/arch/arm/mach-imx/imx8m/soc.c
> > @@ -1270,8 +1270,9 @@ static int ft_add_optee_node(void *fdt, struct
> > bd_info *bd)
> > }
> > }
> >
> > + /* Locate the optee node if it exists or create it. */
> > subpath = "optee";
> > - offs = fdt_add_subnode(fdt, offs, subpath);
> > + offs = fdt_find_or_add_subnode(fdt, offs, subpath);
> > if (offs < 0) {
> > printf("Could not create %s node.\n", subpath);
> > return offs;
>
> This looks correct.
Hi Fabio,
Thank you for the review.
>
> However, shouldn't we add the optee node only when CONFIG_OPTEE is selected?
Peng has answered this question already, but I would like to add some more
details:
Currently the ft_add_optee_node() function does add the /reserved-memory and
/firmware/optee nodes to the Devicetree passed to Linux when OP-TEE is detected
at runtime (the information is in the rom_pointer[] array).
If we skip this when CONFIG_OPTEE is not set, Linux will not even have the
reserved memory information anymore and the Linux optee driver will not probe.
Best regards,
Vincent.
>
> --- a/arch/arm/mach-imx/imx8m/soc.c
> +++ b/arch/arm/mach-imx/imx8m/soc.c
> @@ -1235,6 +1235,9 @@ static int ft_add_optee_node(void *fdt, struct
> bd_info *bd)
> int offs;
> int ret;
>
> + if (!IS_ENABLED(CONFIG_OPTEE))
> + return 0;
> +
> /*
> * No TEE space allocated indicating no TEE running, so no
> * need to add optee node in dts
[PATCH] ata: ahci: remove bad free
In the case of a memory allocation error, the ahci_port_start() function
tries to free the `pp' pointer.
This pointer was not dynamically allocated but does in fact point to an
element of the port[] array member of the struct ahci_uc_priv.
Remove the erroneous call to free() to fix this.
Fixes: 4782ac80b02f ("Add AHCI support to u-boot")
Signed-off-by: Vincent Stehlé
Cc: Tom Rini
Cc: Jason Jin
---
drivers/ata/ahci.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 8058d5ff1c3..e593e228685 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -463,7 +463,6 @@ static int ahci_port_start(struct ahci_uc_priv *uc_priv, u8
port)
mem = memalign(2048, AHCI_PORT_PRIV_DMA_SZ);
if (!mem) {
- free(pp);
printf("%s: No mem for table!\n", __func__);
return -ENOMEM;
}
--
2.47.2
[PATCH] efi_loader: fix ipv4 device path node conversion
When converting an IPv4 device path node to text, the
EFI_DEVICE_PATH_TO_TEXT_PROTOCOL will produce the following string:
IPv4(5.6.7.8,TCP,UDP,0x6,DHCP,1.2.3.4,9.10.11.12,255.255.255.0)
This string erroneously contains multiple protocols: TCP, UDP and 0x6.
Add the missing `break' statements in the dp_msging() function to fix this
and obtain the following expected string instead:
IPv4(5.6.7.8,TCP,DHCP,1.2.3.4,9.10.11.12,255.255.255.0)
Fixes: aaf63429a112 ("efi_loader: add IPv4() to device path to text protocol")
Signed-off-by: Vincent Stehlé
Cc: Heinrich Schuchardt
Cc: Ilias Apalodimas
Cc: Adriano Cordova
Cc: Tom Rini
---
lib/efi_loader/efi_device_path_to_text.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/efi_loader/efi_device_path_to_text.c
b/lib/efi_loader/efi_device_path_to_text.c
index f6889cb7399..452ec1b2e8b 100644
--- a/lib/efi_loader/efi_device_path_to_text.c
+++ b/lib/efi_loader/efi_device_path_to_text.c
@@ -181,10 +181,13 @@ static char *dp_msging(char *s, struct efi_device_path
*dp)
switch (idp->protocol) {
case IPPROTO_TCP:
s += sprintf(s, "TCP,");
+ break;
case IPPROTO_UDP:
s += sprintf(s, "UDP,");
+ break;
default:
s += sprintf(s, "0x%x,", idp->protocol);
+ break;
}
s += sprintf(s, idp->static_ip_address ? "Static" : "DHCP");
s += sprintf(s, ",%pI4", &idp->local_ip_address);
--
2.47.2
