Re: [PATCH v5 1/4] efi_loader: get version information from device tree
On 5/10/23 22:46, Simon Glass wrote:
Hi Heinrich,
On Thu, 27 Apr 2023 at 22:12, Heinrich Schuchardt wrote:
Am 28. April 2023 01:35:04 MESZ schrieb Simon Glass :
Hi Masahisa,
On Mon, 10 Apr 2023 at 03:07, Masahisa Kojima
wrote:
Current FMP->GetImageInfo() always return 0 for the firmware
version, user can not identify which firmware version is currently
running through the EFI interface.
This commit gets the version information from device tree,
then fills the firmware version, lowest supported version
in FMP->GetImageInfo().
Now FMP->GetImageInfo() and ESRT have the meaningful version number.
Signed-off-by: Masahisa Kojima
---
Changes in v5:
- newly implement a device tree based versioning
.../firmware/firmware-version.txt | 25
lib/efi_loader/efi_firmware.c | 63 +--
2 files changed, 84 insertions(+), 4 deletions(-)
create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt
diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt
b/doc/device-tree-bindings/firmware/firmware-version.txt
new file mode 100644
index 00..6112de4a1d
--- /dev/null
+++ b/doc/device-tree-bindings/firmware/firmware-version.txt
@@ -0,0 +1,25 @@
+firmware-version bindings
+---
+
+Required properties:
+- image-type-id: guid for image blob type
+- image-index : image index
+- fw-version : firmware version
+- lowest-supported-version : lowest supported version
+
+Example:
+
+ firmware-version {
+ image1 {
+ image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
Nit: please use lower-case hex and add a decoder to uuid.c so we can
look it up when debugging.
The GUIDs are board specific. No, we should not clutter uuid.c with strings for
dozens of boards. Our development aim is to keep U-Boot small and these GUIDs
are not printed anywhere.
Instead we should define a string per firmware image. We already have a field
for it:
fw_array[i].fw_name
This is of course madness. NAK to any UUIDs in U-Boot that don't have
a decoder ring.
I never have heard of a "decoder ring" for UUIDs in U-Boot.
git grep -A2 -ni decoder | grep -i ring
does not find such a term.
Our development target is to keep the code size of U-Boot small. Hence,
we should not clutter uuid.c with board specific strings. These should
be kept in board specific files.
Let's use a compatible string (vendor,board) and drop these silly UUIDs.
Please, avoid derogatory language like "silly".
The UEFI specification defines EFI_CAPSULE_HEADER. This structure
contains field CapusuleGuid (A GUID that defines the contents of a capsule).
Best regards
Heinrich
Re: [PATCH v5 1/4] efi_loader: get version information from device tree
Hi Heinrich,
On Thu, 27 Apr 2023 at 22:12, Heinrich Schuchardt wrote:
>
>
>
> Am 28. April 2023 01:35:04 MESZ schrieb Simon Glass :
> >Hi Masahisa,
> >
> >On Mon, 10 Apr 2023 at 03:07, Masahisa Kojima
> > wrote:
> >>
> >> Current FMP->GetImageInfo() always return 0 for the firmware
> >> version, user can not identify which firmware version is currently
> >> running through the EFI interface.
> >>
> >> This commit gets the version information from device tree,
> >> then fills the firmware version, lowest supported version
> >> in FMP->GetImageInfo().
> >>
> >> Now FMP->GetImageInfo() and ESRT have the meaningful version number.
> >>
> >> Signed-off-by: Masahisa Kojima
> >> ---
> >> Changes in v5:
> >> - newly implement a device tree based versioning
> >>
> >> .../firmware/firmware-version.txt | 25
> >> lib/efi_loader/efi_firmware.c | 63 +--
> >> 2 files changed, 84 insertions(+), 4 deletions(-)
> >> create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt
> >>
> >> diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt
> >> b/doc/device-tree-bindings/firmware/firmware-version.txt
> >> new file mode 100644
> >> index 00..6112de4a1d
> >> --- /dev/null
> >> +++ b/doc/device-tree-bindings/firmware/firmware-version.txt
> >> @@ -0,0 +1,25 @@
> >> +firmware-version bindings
> >> +---
> >> +
> >> +Required properties:
> >> +- image-type-id: guid for image blob type
> >> +- image-index : image index
> >> +- fw-version : firmware version
> >> +- lowest-supported-version : lowest supported version
> >> +
> >> +Example:
> >> +
> >> + firmware-version {
> >> + image1 {
> >> + image-type-id =
> >> "09D7CF52-0720-4710-91D1-08469B7FE9C8";
> >
> >Nit: please use lower-case hex and add a decoder to uuid.c so we can
> >look it up when debugging.
>
> The GUIDs are board specific. No, we should not clutter uuid.c with strings
> for dozens of boards. Our development aim is to keep U-Boot small and these
> GUIDs are not printed anywhere.
>
> Instead we should define a string per firmware image. We already have a field
> for it:
>
> fw_array[i].fw_name
This is of course madness. NAK to any UUIDs in U-Boot that don't have
a decoder ring.
Let's use a compatible string (vendor,board) and drop these silly UUIDs.
Regards,
Simon
Re: [PATCH v5 1/4] efi_loader: get version information from device tree
Hi Akashi-san,
On Wed, 10 May 2023 at 09:28, Takahiro Akashi
wrote:
>
> On Tue, May 09, 2023 at 06:57:19PM +0900, Masahisa Kojima wrote:
> > On Mon, 8 May 2023 at 18:44, Heinrich Schuchardt wrote:
> > >
> > > On 5/8/23 10:15, Masahisa Kojima wrote:
> > > > Hi Heinrich,
> > > >
> > > > On Thu, 27 Apr 2023 at 15:09, Heinrich Schuchardt
> > > > wrote:
> > > >>
> > > >> On 4/10/23 11:07, Masahisa Kojima wrote:
> > > >>> Current FMP->GetImageInfo() always return 0 for the firmware
> > > >>> version, user can not identify which firmware version is currently
> > > >>> running through the EFI interface.
> > > >>>
> > > >>> This commit gets the version information from device tree,
> > > >>> then fills the firmware version, lowest supported version
> > > >>> in FMP->GetImageInfo().
> > > >>>
> > > >>> Now FMP->GetImageInfo() and ESRT have the meaningful version number.
> > > >>>
> > > >>> Signed-off-by: Masahisa Kojima
> > > >>> ---
> > > >>> Changes in v5:
> > > >>> - newly implement a device tree based versioning
> > > >>>
> > > >>>.../firmware/firmware-version.txt | 25
> > > >>>lib/efi_loader/efi_firmware.c | 63
> > > >>> +--
> > > >>>2 files changed, 84 insertions(+), 4 deletions(-)
> > > >>>create mode 100644
> > > >>> doc/device-tree-bindings/firmware/firmware-version.txt
> > > >>>
> > > >>> diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt
> > > >>> b/doc/device-tree-bindings/firmware/firmware-version.txt
> > > >>> new file mode 100644
> > > >>> index 00..6112de4a1d
> > > >>> --- /dev/null
> > > >>> +++ b/doc/device-tree-bindings/firmware/firmware-version.txt
> > > >>> @@ -0,0 +1,25 @@
> > > >>> +firmware-version bindings
> > > >>> +---
> > > >>> +
> > > >>> +Required properties:
> > > >>> +- image-type-id : guid for image blob type
> > > >>> +- image-index: image index
> > > >>> +- fw-version : firmware version
> > > >>> +- lowest-supported-version : lowest supported version
> > > >>> +
> > > >>> +Example:
> > > >>> +
> > > >>> + firmware-version {
> > > >>> + image1 {
> > > >>> + image-type-id =
> > > >>> "09D7CF52-0720-4710-91D1-08469B7FE9C8";
> > > >>> + image-index = <1>;
> > > >>> + fw-version = <5>;
> > > >>> + lowest-supported-version = <3>;
> > > >>> + };
> > > >>> + image2 {
> > > >>> + image-type-id =
> > > >>> "5A7021F5-FEF2-48B4-AABA-832E777418C0";
> > > >>> + image-index = <2>;
> > > >>> + fw-version = <10>;
> > > >>> + lowest-supported-version = <7>;
> > > >>> + };
> > > >>> + };
> > > >>> diff --git a/lib/efi_loader/efi_firmware.c
> > > >>> b/lib/efi_loader/efi_firmware.c
> > > >>> index 93e2b01c07..1c6ef468bf 100644
> > > >>> --- a/lib/efi_loader/efi_firmware.c
> > > >>> +++ b/lib/efi_loader/efi_firmware.c
> > > >>> @@ -102,6 +102,56 @@ efi_status_t EFIAPI
> > > >>> efi_firmware_set_package_info_unsupported(
> > > >>>return EFI_EXIT(EFI_UNSUPPORTED);
> > > >>>}
> > > >>>
> > > >>> +/**
> > > >>> + * efi_firmware_get_firmware_version - get firmware version from dtb
> > > >>> + * @image_index: Image index
> > > >>> + * @image_type_id: Image type id
> > > >>> + * @fw_version: Pointer to store the version number
> > > >>> + * @lsv: Pointer to store the lowest supported version
> > > >>> + *
> > > >>> + * Authenticate the capsule if authentication is enabled.
> > > >>> + * The image pointer and the image size are updated in case of
> > > >>> success.
> > > >>> + */
> > > >>> +void efi_firmware_get_firmware_version(u8 image_index,
> > > >>> +efi_guid_t *image_type_id,
> > > >>> +u32 *fw_version, u32 *lsv)
> > > >>> +{
> > > >>> + const void *fdt = gd->fdt_blob;
> > > >>> + const fdt32_t *val;
> > > >>> + const char *guid_str;
> > > >>> + int len, offset, index;
> > > >>> + int parent;
> > > >>> +
> > > >>> + parent = fdt_subnode_offset(fdt, 0, "firmware-version");
> > > >>> + if (parent < 0)
> > > >>> + return;
> > > >>> +
> > > >>> + fdt_for_each_subnode(offset, fdt, parent) {
> > > >>> + efi_guid_t guid;
> > > >>> +
> > > >>> + guid_str = fdt_getprop(fdt, offset, "image-type-id",
> > > >>> );
> > > >>> + if (!guid_str)
> > > >>> + continue;
> > > >>> + uuid_str_to_bin(guid_str, guid.b, UUID_STR_FORMAT_GUID);
> > > >>> +
> > > >>> + val = fdt_getprop(fdt, offset, "image-index", );
> > > >>> + if (!val)
> > > >>> + continue;
> > > >>> + index = fdt32_to_cpu(*val);
> > > >>> +
> > > >>> + if
Re: [PATCH v5 1/4] efi_loader: get version information from device tree
On Tue, May 09, 2023 at 06:57:19PM +0900, Masahisa Kojima wrote:
> On Mon, 8 May 2023 at 18:44, Heinrich Schuchardt wrote:
> >
> > On 5/8/23 10:15, Masahisa Kojima wrote:
> > > Hi Heinrich,
> > >
> > > On Thu, 27 Apr 2023 at 15:09, Heinrich Schuchardt
> > > wrote:
> > >>
> > >> On 4/10/23 11:07, Masahisa Kojima wrote:
> > >>> Current FMP->GetImageInfo() always return 0 for the firmware
> > >>> version, user can not identify which firmware version is currently
> > >>> running through the EFI interface.
> > >>>
> > >>> This commit gets the version information from device tree,
> > >>> then fills the firmware version, lowest supported version
> > >>> in FMP->GetImageInfo().
> > >>>
> > >>> Now FMP->GetImageInfo() and ESRT have the meaningful version number.
> > >>>
> > >>> Signed-off-by: Masahisa Kojima
> > >>> ---
> > >>> Changes in v5:
> > >>> - newly implement a device tree based versioning
> > >>>
> > >>>.../firmware/firmware-version.txt | 25
> > >>>lib/efi_loader/efi_firmware.c | 63
> > >>> +--
> > >>>2 files changed, 84 insertions(+), 4 deletions(-)
> > >>>create mode 100644
> > >>> doc/device-tree-bindings/firmware/firmware-version.txt
> > >>>
> > >>> diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt
> > >>> b/doc/device-tree-bindings/firmware/firmware-version.txt
> > >>> new file mode 100644
> > >>> index 00..6112de4a1d
> > >>> --- /dev/null
> > >>> +++ b/doc/device-tree-bindings/firmware/firmware-version.txt
> > >>> @@ -0,0 +1,25 @@
> > >>> +firmware-version bindings
> > >>> +---
> > >>> +
> > >>> +Required properties:
> > >>> +- image-type-id : guid for image blob type
> > >>> +- image-index: image index
> > >>> +- fw-version : firmware version
> > >>> +- lowest-supported-version : lowest supported version
> > >>> +
> > >>> +Example:
> > >>> +
> > >>> + firmware-version {
> > >>> + image1 {
> > >>> + image-type-id =
> > >>> "09D7CF52-0720-4710-91D1-08469B7FE9C8";
> > >>> + image-index = <1>;
> > >>> + fw-version = <5>;
> > >>> + lowest-supported-version = <3>;
> > >>> + };
> > >>> + image2 {
> > >>> + image-type-id =
> > >>> "5A7021F5-FEF2-48B4-AABA-832E777418C0";
> > >>> + image-index = <2>;
> > >>> + fw-version = <10>;
> > >>> + lowest-supported-version = <7>;
> > >>> + };
> > >>> + };
> > >>> diff --git a/lib/efi_loader/efi_firmware.c
> > >>> b/lib/efi_loader/efi_firmware.c
> > >>> index 93e2b01c07..1c6ef468bf 100644
> > >>> --- a/lib/efi_loader/efi_firmware.c
> > >>> +++ b/lib/efi_loader/efi_firmware.c
> > >>> @@ -102,6 +102,56 @@ efi_status_t EFIAPI
> > >>> efi_firmware_set_package_info_unsupported(
> > >>>return EFI_EXIT(EFI_UNSUPPORTED);
> > >>>}
> > >>>
> > >>> +/**
> > >>> + * efi_firmware_get_firmware_version - get firmware version from dtb
> > >>> + * @image_index: Image index
> > >>> + * @image_type_id: Image type id
> > >>> + * @fw_version: Pointer to store the version number
> > >>> + * @lsv: Pointer to store the lowest supported version
> > >>> + *
> > >>> + * Authenticate the capsule if authentication is enabled.
> > >>> + * The image pointer and the image size are updated in case of success.
> > >>> + */
> > >>> +void efi_firmware_get_firmware_version(u8 image_index,
> > >>> +efi_guid_t *image_type_id,
> > >>> +u32 *fw_version, u32 *lsv)
> > >>> +{
> > >>> + const void *fdt = gd->fdt_blob;
> > >>> + const fdt32_t *val;
> > >>> + const char *guid_str;
> > >>> + int len, offset, index;
> > >>> + int parent;
> > >>> +
> > >>> + parent = fdt_subnode_offset(fdt, 0, "firmware-version");
> > >>> + if (parent < 0)
> > >>> + return;
> > >>> +
> > >>> + fdt_for_each_subnode(offset, fdt, parent) {
> > >>> + efi_guid_t guid;
> > >>> +
> > >>> + guid_str = fdt_getprop(fdt, offset, "image-type-id",
> > >>> );
> > >>> + if (!guid_str)
> > >>> + continue;
> > >>> + uuid_str_to_bin(guid_str, guid.b, UUID_STR_FORMAT_GUID);
> > >>> +
> > >>> + val = fdt_getprop(fdt, offset, "image-index", );
> > >>> + if (!val)
> > >>> + continue;
> > >>> + index = fdt32_to_cpu(*val);
> > >>> +
> > >>> + if (!guidcmp(, image_type_id) && index ==
> > >>> image_index) {
> > >>> + val = fdt_getprop(fdt, offset, "fw-version",
> > >>> );
> > >>> + if (val)
> > >>> + *fw_version = fdt32_to_cpu(*val);
> > >>> +
> > >>> + val =
Re: [PATCH v5 1/4] efi_loader: get version information from device tree
On Mon, 8 May 2023 at 18:44, Heinrich Schuchardt wrote:
>
> On 5/8/23 10:15, Masahisa Kojima wrote:
> > Hi Heinrich,
> >
> > On Thu, 27 Apr 2023 at 15:09, Heinrich Schuchardt
> > wrote:
> >>
> >> On 4/10/23 11:07, Masahisa Kojima wrote:
> >>> Current FMP->GetImageInfo() always return 0 for the firmware
> >>> version, user can not identify which firmware version is currently
> >>> running through the EFI interface.
> >>>
> >>> This commit gets the version information from device tree,
> >>> then fills the firmware version, lowest supported version
> >>> in FMP->GetImageInfo().
> >>>
> >>> Now FMP->GetImageInfo() and ESRT have the meaningful version number.
> >>>
> >>> Signed-off-by: Masahisa Kojima
> >>> ---
> >>> Changes in v5:
> >>> - newly implement a device tree based versioning
> >>>
> >>>.../firmware/firmware-version.txt | 25
> >>>lib/efi_loader/efi_firmware.c | 63 +--
> >>>2 files changed, 84 insertions(+), 4 deletions(-)
> >>>create mode 100644
> >>> doc/device-tree-bindings/firmware/firmware-version.txt
> >>>
> >>> diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt
> >>> b/doc/device-tree-bindings/firmware/firmware-version.txt
> >>> new file mode 100644
> >>> index 00..6112de4a1d
> >>> --- /dev/null
> >>> +++ b/doc/device-tree-bindings/firmware/firmware-version.txt
> >>> @@ -0,0 +1,25 @@
> >>> +firmware-version bindings
> >>> +---
> >>> +
> >>> +Required properties:
> >>> +- image-type-id : guid for image blob type
> >>> +- image-index: image index
> >>> +- fw-version : firmware version
> >>> +- lowest-supported-version : lowest supported version
> >>> +
> >>> +Example:
> >>> +
> >>> + firmware-version {
> >>> + image1 {
> >>> + image-type-id =
> >>> "09D7CF52-0720-4710-91D1-08469B7FE9C8";
> >>> + image-index = <1>;
> >>> + fw-version = <5>;
> >>> + lowest-supported-version = <3>;
> >>> + };
> >>> + image2 {
> >>> + image-type-id =
> >>> "5A7021F5-FEF2-48B4-AABA-832E777418C0";
> >>> + image-index = <2>;
> >>> + fw-version = <10>;
> >>> + lowest-supported-version = <7>;
> >>> + };
> >>> + };
> >>> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> >>> index 93e2b01c07..1c6ef468bf 100644
> >>> --- a/lib/efi_loader/efi_firmware.c
> >>> +++ b/lib/efi_loader/efi_firmware.c
> >>> @@ -102,6 +102,56 @@ efi_status_t EFIAPI
> >>> efi_firmware_set_package_info_unsupported(
> >>>return EFI_EXIT(EFI_UNSUPPORTED);
> >>>}
> >>>
> >>> +/**
> >>> + * efi_firmware_get_firmware_version - get firmware version from dtb
> >>> + * @image_index: Image index
> >>> + * @image_type_id: Image type id
> >>> + * @fw_version: Pointer to store the version number
> >>> + * @lsv: Pointer to store the lowest supported version
> >>> + *
> >>> + * Authenticate the capsule if authentication is enabled.
> >>> + * The image pointer and the image size are updated in case of success.
> >>> + */
> >>> +void efi_firmware_get_firmware_version(u8 image_index,
> >>> +efi_guid_t *image_type_id,
> >>> +u32 *fw_version, u32 *lsv)
> >>> +{
> >>> + const void *fdt = gd->fdt_blob;
> >>> + const fdt32_t *val;
> >>> + const char *guid_str;
> >>> + int len, offset, index;
> >>> + int parent;
> >>> +
> >>> + parent = fdt_subnode_offset(fdt, 0, "firmware-version");
> >>> + if (parent < 0)
> >>> + return;
> >>> +
> >>> + fdt_for_each_subnode(offset, fdt, parent) {
> >>> + efi_guid_t guid;
> >>> +
> >>> + guid_str = fdt_getprop(fdt, offset, "image-type-id", );
> >>> + if (!guid_str)
> >>> + continue;
> >>> + uuid_str_to_bin(guid_str, guid.b, UUID_STR_FORMAT_GUID);
> >>> +
> >>> + val = fdt_getprop(fdt, offset, "image-index", );
> >>> + if (!val)
> >>> + continue;
> >>> + index = fdt32_to_cpu(*val);
> >>> +
> >>> + if (!guidcmp(, image_type_id) && index == image_index)
> >>> {
> >>> + val = fdt_getprop(fdt, offset, "fw-version", );
> >>> + if (val)
> >>> + *fw_version = fdt32_to_cpu(*val);
> >>> +
> >>> + val = fdt_getprop(fdt, offset,
> >>> + "lowest-supported-version", );
> >>> + if (val)
> >>> + *lsv = fdt32_to_cpu(*val);
> >>> + }
> >>> + }
> >>> +}
> >>> +
> >>>/**
> >>> * efi_fill_image_desc_array - populate image descriptor array
> >>>
Re: [PATCH v5 1/4] efi_loader: get version information from device tree
On 5/8/23 10:15, Masahisa Kojima wrote:
Hi Heinrich,
On Thu, 27 Apr 2023 at 15:09, Heinrich Schuchardt wrote:
On 4/10/23 11:07, Masahisa Kojima wrote:
Current FMP->GetImageInfo() always return 0 for the firmware
version, user can not identify which firmware version is currently
running through the EFI interface.
This commit gets the version information from device tree,
then fills the firmware version, lowest supported version
in FMP->GetImageInfo().
Now FMP->GetImageInfo() and ESRT have the meaningful version number.
Signed-off-by: Masahisa Kojima
---
Changes in v5:
- newly implement a device tree based versioning
.../firmware/firmware-version.txt | 25
lib/efi_loader/efi_firmware.c | 63 +--
2 files changed, 84 insertions(+), 4 deletions(-)
create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt
diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt
b/doc/device-tree-bindings/firmware/firmware-version.txt
new file mode 100644
index 00..6112de4a1d
--- /dev/null
+++ b/doc/device-tree-bindings/firmware/firmware-version.txt
@@ -0,0 +1,25 @@
+firmware-version bindings
+---
+
+Required properties:
+- image-type-id : guid for image blob type
+- image-index: image index
+- fw-version : firmware version
+- lowest-supported-version : lowest supported version
+
+Example:
+
+ firmware-version {
+ image1 {
+ image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
+ image-index = <1>;
+ fw-version = <5>;
+ lowest-supported-version = <3>;
+ };
+ image2 {
+ image-type-id = "5A7021F5-FEF2-48B4-AABA-832E777418C0";
+ image-index = <2>;
+ fw-version = <10>;
+ lowest-supported-version = <7>;
+ };
+ };
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 93e2b01c07..1c6ef468bf 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -102,6 +102,56 @@ efi_status_t EFIAPI
efi_firmware_set_package_info_unsupported(
return EFI_EXIT(EFI_UNSUPPORTED);
}
+/**
+ * efi_firmware_get_firmware_version - get firmware version from dtb
+ * @image_index: Image index
+ * @image_type_id: Image type id
+ * @fw_version: Pointer to store the version number
+ * @lsv: Pointer to store the lowest supported version
+ *
+ * Authenticate the capsule if authentication is enabled.
+ * The image pointer and the image size are updated in case of success.
+ */
+void efi_firmware_get_firmware_version(u8 image_index,
+efi_guid_t *image_type_id,
+u32 *fw_version, u32 *lsv)
+{
+ const void *fdt = gd->fdt_blob;
+ const fdt32_t *val;
+ const char *guid_str;
+ int len, offset, index;
+ int parent;
+
+ parent = fdt_subnode_offset(fdt, 0, "firmware-version");
+ if (parent < 0)
+ return;
+
+ fdt_for_each_subnode(offset, fdt, parent) {
+ efi_guid_t guid;
+
+ guid_str = fdt_getprop(fdt, offset, "image-type-id", );
+ if (!guid_str)
+ continue;
+ uuid_str_to_bin(guid_str, guid.b, UUID_STR_FORMAT_GUID);
+
+ val = fdt_getprop(fdt, offset, "image-index", );
+ if (!val)
+ continue;
+ index = fdt32_to_cpu(*val);
+
+ if (!guidcmp(, image_type_id) && index == image_index) {
+ val = fdt_getprop(fdt, offset, "fw-version", );
+ if (val)
+ *fw_version = fdt32_to_cpu(*val);
+
+ val = fdt_getprop(fdt, offset,
+ "lowest-supported-version", );
+ if (val)
+ *lsv = fdt32_to_cpu(*val);
+ }
+ }
+}
+
/**
* efi_fill_image_desc_array - populate image descriptor array
* @image_info_size:Size of @image_info
@@ -148,13 +198,19 @@ static efi_status_t efi_fill_image_desc_array(
*package_version_name = NULL; /* not supported */
for (i = 0; i < num_image_type_guids; i++) {
Currently we define num_image_type_guids per board in a C file. Using
the same line of code once per board makes no sense to me. Please, move
the definition of that variable to lib/efi_loader/efi_firmware.c.
Sorry for the late reply.
num_image_type_guids is calculated with "ARRAY_SIZE(fw_images)",
fw_images[] array is also defined in each board file,
so we can not simply move num_image_type_guids into
lib/efi_loader/efi_firmware.c.
Why can't we have
int num_image_type_guids = ARRAY_SIZE(fw_images);
in lib/efi_loader/efi_firmware.c?
Re: [PATCH v5 1/4] efi_loader: get version information from device tree
Hi Heinrich,
On Thu, 27 Apr 2023 at 15:09, Heinrich Schuchardt wrote:
>
> On 4/10/23 11:07, Masahisa Kojima wrote:
> > Current FMP->GetImageInfo() always return 0 for the firmware
> > version, user can not identify which firmware version is currently
> > running through the EFI interface.
> >
> > This commit gets the version information from device tree,
> > then fills the firmware version, lowest supported version
> > in FMP->GetImageInfo().
> >
> > Now FMP->GetImageInfo() and ESRT have the meaningful version number.
> >
> > Signed-off-by: Masahisa Kojima
> > ---
> > Changes in v5:
> > - newly implement a device tree based versioning
> >
> > .../firmware/firmware-version.txt | 25
> > lib/efi_loader/efi_firmware.c | 63 +--
> > 2 files changed, 84 insertions(+), 4 deletions(-)
> > create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt
> >
> > diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt
> > b/doc/device-tree-bindings/firmware/firmware-version.txt
> > new file mode 100644
> > index 00..6112de4a1d
> > --- /dev/null
> > +++ b/doc/device-tree-bindings/firmware/firmware-version.txt
> > @@ -0,0 +1,25 @@
> > +firmware-version bindings
> > +---
> > +
> > +Required properties:
> > +- image-type-id : guid for image blob type
> > +- image-index: image index
> > +- fw-version : firmware version
> > +- lowest-supported-version : lowest supported version
> > +
> > +Example:
> > +
> > + firmware-version {
> > + image1 {
> > + image-type-id =
> > "09D7CF52-0720-4710-91D1-08469B7FE9C8";
> > + image-index = <1>;
> > + fw-version = <5>;
> > + lowest-supported-version = <3>;
> > + };
> > + image2 {
> > + image-type-id =
> > "5A7021F5-FEF2-48B4-AABA-832E777418C0";
> > + image-index = <2>;
> > + fw-version = <10>;
> > + lowest-supported-version = <7>;
> > + };
> > + };
> > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> > index 93e2b01c07..1c6ef468bf 100644
> > --- a/lib/efi_loader/efi_firmware.c
> > +++ b/lib/efi_loader/efi_firmware.c
> > @@ -102,6 +102,56 @@ efi_status_t EFIAPI
> > efi_firmware_set_package_info_unsupported(
> > return EFI_EXIT(EFI_UNSUPPORTED);
> > }
> >
> > +/**
> > + * efi_firmware_get_firmware_version - get firmware version from dtb
> > + * @image_index: Image index
> > + * @image_type_id: Image type id
> > + * @fw_version: Pointer to store the version number
> > + * @lsv: Pointer to store the lowest supported version
> > + *
> > + * Authenticate the capsule if authentication is enabled.
> > + * The image pointer and the image size are updated in case of success.
> > + */
> > +void efi_firmware_get_firmware_version(u8 image_index,
> > +efi_guid_t *image_type_id,
> > +u32 *fw_version, u32 *lsv)
> > +{
> > + const void *fdt = gd->fdt_blob;
> > + const fdt32_t *val;
> > + const char *guid_str;
> > + int len, offset, index;
> > + int parent;
> > +
> > + parent = fdt_subnode_offset(fdt, 0, "firmware-version");
> > + if (parent < 0)
> > + return;
> > +
> > + fdt_for_each_subnode(offset, fdt, parent) {
> > + efi_guid_t guid;
> > +
> > + guid_str = fdt_getprop(fdt, offset, "image-type-id", );
> > + if (!guid_str)
> > + continue;
> > + uuid_str_to_bin(guid_str, guid.b, UUID_STR_FORMAT_GUID);
> > +
> > + val = fdt_getprop(fdt, offset, "image-index", );
> > + if (!val)
> > + continue;
> > + index = fdt32_to_cpu(*val);
> > +
> > + if (!guidcmp(, image_type_id) && index == image_index) {
> > + val = fdt_getprop(fdt, offset, "fw-version", );
> > + if (val)
> > + *fw_version = fdt32_to_cpu(*val);
> > +
> > + val = fdt_getprop(fdt, offset,
> > + "lowest-supported-version", );
> > + if (val)
> > + *lsv = fdt32_to_cpu(*val);
> > + }
> > + }
> > +}
> > +
> > /**
> >* efi_fill_image_desc_array - populate image descriptor array
> >* @image_info_size:Size of @image_info
> > @@ -148,13 +198,19 @@ static efi_status_t efi_fill_image_desc_array(
> > *package_version_name = NULL; /* not supported */
> >
> > for (i = 0; i < num_image_type_guids; i++) {
>
> Currently we define num_image_type_guids per board in a C file. Using
> the same line of code once per board makes no sense
Re: [PATCH v5 1/4] efi_loader: get version information from device tree
Am 28. April 2023 01:35:04 MESZ schrieb Simon Glass :
>Hi Masahisa,
>
>On Mon, 10 Apr 2023 at 03:07, Masahisa Kojima
> wrote:
>>
>> Current FMP->GetImageInfo() always return 0 for the firmware
>> version, user can not identify which firmware version is currently
>> running through the EFI interface.
>>
>> This commit gets the version information from device tree,
>> then fills the firmware version, lowest supported version
>> in FMP->GetImageInfo().
>>
>> Now FMP->GetImageInfo() and ESRT have the meaningful version number.
>>
>> Signed-off-by: Masahisa Kojima
>> ---
>> Changes in v5:
>> - newly implement a device tree based versioning
>>
>> .../firmware/firmware-version.txt | 25
>> lib/efi_loader/efi_firmware.c | 63 +--
>> 2 files changed, 84 insertions(+), 4 deletions(-)
>> create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt
>>
>> diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt
>> b/doc/device-tree-bindings/firmware/firmware-version.txt
>> new file mode 100644
>> index 00..6112de4a1d
>> --- /dev/null
>> +++ b/doc/device-tree-bindings/firmware/firmware-version.txt
>> @@ -0,0 +1,25 @@
>> +firmware-version bindings
>> +---
>> +
>> +Required properties:
>> +- image-type-id: guid for image blob type
>> +- image-index : image index
>> +- fw-version : firmware version
>> +- lowest-supported-version : lowest supported version
>> +
>> +Example:
>> +
>> + firmware-version {
>> + image1 {
>> + image-type-id =
>> "09D7CF52-0720-4710-91D1-08469B7FE9C8";
>
>Nit: please use lower-case hex and add a decoder to uuid.c so we can
>look it up when debugging.
The GUIDs are board specific. No, we should not clutter uuid.c with strings for
dozens of boards. Our development aim is to keep U-Boot small and these GUIDs
are not printed anywhere.
Instead we should define a string per firmware image. We already have a field
for it:
fw_array[i].fw_name
Best regards
Heinrich
>
>> + image-index = <1>;
>> + fw-version = <5>;
>> + lowest-supported-version = <3>;
>> + };
>> + image2 {
>> + image-type-id =
>> "5A7021F5-FEF2-48B4-AABA-832E777418C0";
>> + image-index = <2>;
>> + fw-version = <10>;
>> + lowest-supported-version = <7>;
>> + };
>> + };
>[..]
>
>Regards,
>Simon
Re: [PATCH v5 1/4] efi_loader: get version information from device tree
Hi Masahisa,
On Mon, 10 Apr 2023 at 03:07, Masahisa Kojima
wrote:
>
> Current FMP->GetImageInfo() always return 0 for the firmware
> version, user can not identify which firmware version is currently
> running through the EFI interface.
>
> This commit gets the version information from device tree,
> then fills the firmware version, lowest supported version
> in FMP->GetImageInfo().
>
> Now FMP->GetImageInfo() and ESRT have the meaningful version number.
>
> Signed-off-by: Masahisa Kojima
> ---
> Changes in v5:
> - newly implement a device tree based versioning
>
> .../firmware/firmware-version.txt | 25
> lib/efi_loader/efi_firmware.c | 63 +--
> 2 files changed, 84 insertions(+), 4 deletions(-)
> create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt
>
> diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt
> b/doc/device-tree-bindings/firmware/firmware-version.txt
> new file mode 100644
> index 00..6112de4a1d
> --- /dev/null
> +++ b/doc/device-tree-bindings/firmware/firmware-version.txt
> @@ -0,0 +1,25 @@
> +firmware-version bindings
> +---
> +
> +Required properties:
> +- image-type-id: guid for image blob type
> +- image-index : image index
> +- fw-version : firmware version
> +- lowest-supported-version : lowest supported version
> +
> +Example:
> +
> + firmware-version {
> + image1 {
> + image-type-id =
> "09D7CF52-0720-4710-91D1-08469B7FE9C8";
Nit: please use lower-case hex and add a decoder to uuid.c so we can
look it up when debugging.
> + image-index = <1>;
> + fw-version = <5>;
> + lowest-supported-version = <3>;
> + };
> + image2 {
> + image-type-id =
> "5A7021F5-FEF2-48B4-AABA-832E777418C0";
> + image-index = <2>;
> + fw-version = <10>;
> + lowest-supported-version = <7>;
> + };
> + };
[..]
Regards,
Simon
Re: [PATCH v5 1/4] efi_loader: get version information from device tree
On 4/10/23 11:07, Masahisa Kojima wrote:
Current FMP->GetImageInfo() always return 0 for the firmware
version, user can not identify which firmware version is currently
running through the EFI interface.
This commit gets the version information from device tree,
then fills the firmware version, lowest supported version
in FMP->GetImageInfo().
Now FMP->GetImageInfo() and ESRT have the meaningful version number.
Signed-off-by: Masahisa Kojima
---
Changes in v5:
- newly implement a device tree based versioning
.../firmware/firmware-version.txt | 25
lib/efi_loader/efi_firmware.c | 63 +--
2 files changed, 84 insertions(+), 4 deletions(-)
create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt
diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt
b/doc/device-tree-bindings/firmware/firmware-version.txt
new file mode 100644
index 00..6112de4a1d
--- /dev/null
+++ b/doc/device-tree-bindings/firmware/firmware-version.txt
@@ -0,0 +1,25 @@
+firmware-version bindings
+---
+
+Required properties:
+- image-type-id: guid for image blob type
+- image-index : image index
+- fw-version : firmware version
+- lowest-supported-version : lowest supported version
+
+Example:
+
+ firmware-version {
+ image1 {
+ image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
+ image-index = <1>;
+ fw-version = <5>;
+ lowest-supported-version = <3>;
+ };
+ image2 {
+ image-type-id = "5A7021F5-FEF2-48B4-AABA-832E777418C0";
+ image-index = <2>;
+ fw-version = <10>;
+ lowest-supported-version = <7>;
+ };
+ };
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 93e2b01c07..1c6ef468bf 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -102,6 +102,56 @@ efi_status_t EFIAPI
efi_firmware_set_package_info_unsupported(
return EFI_EXIT(EFI_UNSUPPORTED);
}
+/**
+ * efi_firmware_get_firmware_version - get firmware version from dtb
+ * @image_index: Image index
+ * @image_type_id: Image type id
+ * @fw_version:Pointer to store the version number
+ * @lsv: Pointer to store the lowest supported version
+ *
+ * Authenticate the capsule if authentication is enabled.
+ * The image pointer and the image size are updated in case of success.
+ */
+void efi_firmware_get_firmware_version(u8 image_index,
+ efi_guid_t *image_type_id,
+ u32 *fw_version, u32 *lsv)
+{
+ const void *fdt = gd->fdt_blob;
+ const fdt32_t *val;
+ const char *guid_str;
+ int len, offset, index;
+ int parent;
+
+ parent = fdt_subnode_offset(fdt, 0, "firmware-version");
+ if (parent < 0)
+ return;
+
+ fdt_for_each_subnode(offset, fdt, parent) {
+ efi_guid_t guid;
+
+ guid_str = fdt_getprop(fdt, offset, "image-type-id", );
+ if (!guid_str)
+ continue;
+ uuid_str_to_bin(guid_str, guid.b, UUID_STR_FORMAT_GUID);
+
+ val = fdt_getprop(fdt, offset, "image-index", );
+ if (!val)
+ continue;
+ index = fdt32_to_cpu(*val);
+
+ if (!guidcmp(, image_type_id) && index == image_index) {
+ val = fdt_getprop(fdt, offset, "fw-version", );
+ if (val)
+ *fw_version = fdt32_to_cpu(*val);
+
+ val = fdt_getprop(fdt, offset,
+ "lowest-supported-version", );
+ if (val)
+ *lsv = fdt32_to_cpu(*val);
+ }
+ }
+}
+
/**
* efi_fill_image_desc_array - populate image descriptor array
* @image_info_size: Size of @image_info
@@ -148,13 +198,19 @@ static efi_status_t efi_fill_image_desc_array(
*package_version_name = NULL; /* not supported */
for (i = 0; i < num_image_type_guids; i++) {
Currently we define num_image_type_guids per board in a C file. Using
the same line of code once per board makes no sense to me. Please, move
the definition of that variable to lib/efi_loader/efi_firmware.c.
+ u32 fw_version = 0;
+ u32 lowest_supported_version = 0;
These assignments should be in efi_firmware_get_firmware_version.
+
image_info[i].image_index = fw_array[i].image_index;
Why did we ever introduce the field image_index? Please, eliminate it it
as the GUID is always sufficient to identify an image.
