Re: [PATCH v2 3/3] efi: Avoid using dm_scan_other()
Hi Heinrich, On Mon, Dec 18, 2023 at 3:02 PM Simon Glass wrote: > > Hi Heinrich, > > On Sat, 16 Dec 2023 at 14:02, Heinrich Schuchardt wrote: > > > > On 12/16/23 21:46, Simon Glass wrote: > > > Hi, > > > > > > On Tue, 21 Nov 2023 at 06:21, Tom Rini wrote: > > >> > > >> On Tue, Nov 21, 2023 at 01:18:09PM +0100, Heinrich Schuchardt wrote: > > >>> On 11/12/23 21:55, Simon Glass wrote: > > This function is defined by bootstd so using it precludes using that > > feature. Use the board_early_init_r() feature instead. > > > > This requires moving quite a lot of code into the board directory, butt > > this is the normal place for code called by board_early_init_r() > > > > Signed-off-by: Simon Glass > > --- > > > > Changes in v2: > > - Drop duplicate acpi_xsdt patch > > - Put the board_early_init_r code into board/ > > > > board/efi/efi-x86_app/Makefile | 5 + > > board/efi/efi-x86_app/efi_app.c | 205 > > > > >>> > > >>> Our target should be to enable building the EFI app on all > > >>> architectures. > > >>> > > >>> Only x86 specific code should be added to > > >>> board/efi/efi-x86_app/efi_app.c. > > >> > > >> A later enhancement to make U-Boot as an EFI app more generic would be > > >> good, but outside the scope of this patch which is moving generic code > > >> out from "lib" and in to "board". > > > > I cannot see that this patch is moving any code our of lib/. > > > > But why should we move generic code into board? > > It isn't really generic > > > > > > > > > This patch was marked as old /archived in patchwork so has been > > > forgotten. I have marked it new and non-archived in the hope that it > > > can be applied to -master soon. > > > > There is nothing x86 specific about the code. Generic code should be in > > lib/. Please, provide a new version of the patch. > > > > > > > > I believe that having the EFI app support bootstd could be a useful > > > addition. > > > > That was not disputed. OK, I see now that your objection was putting it into board/ so I sent a new patch for that. It is very strange to have board_early_init_r() calling code in lib though. We should think of a better way. Regards, SImon
Re: [PATCH v2 3/3] efi: Avoid using dm_scan_other()
Hi Heinrich, On Sat, 16 Dec 2023 at 14:02, Heinrich Schuchardt wrote: > > On 12/16/23 21:46, Simon Glass wrote: > > Hi, > > > > On Tue, 21 Nov 2023 at 06:21, Tom Rini wrote: > >> > >> On Tue, Nov 21, 2023 at 01:18:09PM +0100, Heinrich Schuchardt wrote: > >>> On 11/12/23 21:55, Simon Glass wrote: > This function is defined by bootstd so using it precludes using that > feature. Use the board_early_init_r() feature instead. > > This requires moving quite a lot of code into the board directory, butt > this is the normal place for code called by board_early_init_r() > > Signed-off-by: Simon Glass > --- > > Changes in v2: > - Drop duplicate acpi_xsdt patch > - Put the board_early_init_r code into board/ > > board/efi/efi-x86_app/Makefile | 5 + > board/efi/efi-x86_app/efi_app.c | 205 > >>> > >>> Our target should be to enable building the EFI app on all architectures. > >>> > >>> Only x86 specific code should be added to board/efi/efi-x86_app/efi_app.c. > >> > >> A later enhancement to make U-Boot as an EFI app more generic would be > >> good, but outside the scope of this patch which is moving generic code > >> out from "lib" and in to "board". > > I cannot see that this patch is moving any code our of lib/. > > But why should we move generic code into board? It isn't really generic > > > > > This patch was marked as old /archived in patchwork so has been > > forgotten. I have marked it new and non-archived in the hope that it > > can be applied to -master soon. > > There is nothing x86 specific about the code. Generic code should be in > lib/. Please, provide a new version of the patch. > > > > > I believe that having the EFI app support bootstd could be a useful > > addition. > > That was not disputed. > > Best regards > > Heinrich
Re: [PATCH v2 3/3] efi: Avoid using dm_scan_other()
Am 16. Dezember 2023 23:14:20 MEZ schrieb Tom Rini : >On Sat, Dec 16, 2023 at 09:57:41PM +0100, Heinrich Schuchardt wrote: >> On 12/16/23 21:46, Simon Glass wrote: >> > Hi, >> > >> > On Tue, 21 Nov 2023 at 06:21, Tom Rini wrote: >> > > >> > > On Tue, Nov 21, 2023 at 01:18:09PM +0100, Heinrich Schuchardt wrote: >> > > > On 11/12/23 21:55, Simon Glass wrote: >> > > > > This function is defined by bootstd so using it precludes using that >> > > > > feature. Use the board_early_init_r() feature instead. >> > > > > >> > > > > This requires moving quite a lot of code into the board directory, >> > > > > butt >> > > > > this is the normal place for code called by board_early_init_r() >> > > > > >> > > > > Signed-off-by: Simon Glass >> > > > > --- >> > > > > >> > > > > Changes in v2: >> > > > > - Drop duplicate acpi_xsdt patch >> > > > > - Put the board_early_init_r code into board/ >> > > > > >> > > > >board/efi/efi-x86_app/Makefile | 5 + >> > > > >board/efi/efi-x86_app/efi_app.c | 205 >> > > > > >> > > > >> > > > Our target should be to enable building the EFI app on all >> > > > architectures. >> > > > >> > > > Only x86 specific code should be added to >> > > > board/efi/efi-x86_app/efi_app.c. >> > > >> > > A later enhancement to make U-Boot as an EFI app more generic would be >> > > good, but outside the scope of this patch which is moving generic code >> > > out from "lib" and in to "board". >> >> I cannot see that this patch is moving any code our of lib/. >> >> But why should we move generic code into board? >> >> > >> > This patch was marked as old /archived in patchwork so has been >> > forgotten. I have marked it new and non-archived in the hope that it >> > can be applied to -master soon. >> >> There is nothing x86 specific about the code. Generic code should be in >> lib/. Please, provide a new version of the patch. > >It's not generic EFI code, it's generic "U-Boot as EFI application" code >and so the whole "board" needs a bit of a clean and re-work as we all >agree that there shouldn't be anything architecture specific about it, >only building the binary for the correct architecture. However, we just >aren't well structured to support that right now. Where on the list of >priorities does that have to fall rather than just allowing for >functionality to be tested / used? There's not yet IIRC efi-arm64_app >but what we have today (and would have had with Simon's patch) should >have just built and worked. > We have /lib/efi/ for that code. Regards Heinrich
Re: [PATCH v2 3/3] efi: Avoid using dm_scan_other()
On Sat, Dec 16, 2023 at 09:57:41PM +0100, Heinrich Schuchardt wrote: > On 12/16/23 21:46, Simon Glass wrote: > > Hi, > > > > On Tue, 21 Nov 2023 at 06:21, Tom Rini wrote: > > > > > > On Tue, Nov 21, 2023 at 01:18:09PM +0100, Heinrich Schuchardt wrote: > > > > On 11/12/23 21:55, Simon Glass wrote: > > > > > This function is defined by bootstd so using it precludes using that > > > > > feature. Use the board_early_init_r() feature instead. > > > > > > > > > > This requires moving quite a lot of code into the board directory, > > > > > butt > > > > > this is the normal place for code called by board_early_init_r() > > > > > > > > > > Signed-off-by: Simon Glass > > > > > --- > > > > > > > > > > Changes in v2: > > > > > - Drop duplicate acpi_xsdt patch > > > > > - Put the board_early_init_r code into board/ > > > > > > > > > >board/efi/efi-x86_app/Makefile | 5 + > > > > >board/efi/efi-x86_app/efi_app.c | 205 > > > > > > > > > > > > > Our target should be to enable building the EFI app on all > > > > architectures. > > > > > > > > Only x86 specific code should be added to > > > > board/efi/efi-x86_app/efi_app.c. > > > > > > A later enhancement to make U-Boot as an EFI app more generic would be > > > good, but outside the scope of this patch which is moving generic code > > > out from "lib" and in to "board". > > I cannot see that this patch is moving any code our of lib/. > > But why should we move generic code into board? > > > > > This patch was marked as old /archived in patchwork so has been > > forgotten. I have marked it new and non-archived in the hope that it > > can be applied to -master soon. > > There is nothing x86 specific about the code. Generic code should be in > lib/. Please, provide a new version of the patch. It's not generic EFI code, it's generic "U-Boot as EFI application" code and so the whole "board" needs a bit of a clean and re-work as we all agree that there shouldn't be anything architecture specific about it, only building the binary for the correct architecture. However, we just aren't well structured to support that right now. Where on the list of priorities does that have to fall rather than just allowing for functionality to be tested / used? There's not yet IIRC efi-arm64_app but what we have today (and would have had with Simon's patch) should have just built and worked. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 3/3] efi: Avoid using dm_scan_other()
On 12/16/23 21:46, Simon Glass wrote: Hi, On Tue, 21 Nov 2023 at 06:21, Tom Rini wrote: On Tue, Nov 21, 2023 at 01:18:09PM +0100, Heinrich Schuchardt wrote: On 11/12/23 21:55, Simon Glass wrote: This function is defined by bootstd so using it precludes using that feature. Use the board_early_init_r() feature instead. This requires moving quite a lot of code into the board directory, butt this is the normal place for code called by board_early_init_r() Signed-off-by: Simon Glass --- Changes in v2: - Drop duplicate acpi_xsdt patch - Put the board_early_init_r code into board/ board/efi/efi-x86_app/Makefile | 5 + board/efi/efi-x86_app/efi_app.c | 205 Our target should be to enable building the EFI app on all architectures. Only x86 specific code should be added to board/efi/efi-x86_app/efi_app.c. A later enhancement to make U-Boot as an EFI app more generic would be good, but outside the scope of this patch which is moving generic code out from "lib" and in to "board". I cannot see that this patch is moving any code our of lib/. But why should we move generic code into board? This patch was marked as old /archived in patchwork so has been forgotten. I have marked it new and non-archived in the hope that it can be applied to -master soon. There is nothing x86 specific about the code. Generic code should be in lib/. Please, provide a new version of the patch. I believe that having the EFI app support bootstd could be a useful addition. That was not disputed. Best regards Heinrich
Re: [PATCH v2 3/3] efi: Avoid using dm_scan_other()
Hi, On Tue, 21 Nov 2023 at 06:21, Tom Rini wrote: > > On Tue, Nov 21, 2023 at 01:18:09PM +0100, Heinrich Schuchardt wrote: > > On 11/12/23 21:55, Simon Glass wrote: > > > This function is defined by bootstd so using it precludes using that > > > feature. Use the board_early_init_r() feature instead. > > > > > > This requires moving quite a lot of code into the board directory, butt > > > this is the normal place for code called by board_early_init_r() > > > > > > Signed-off-by: Simon Glass > > > --- > > > > > > Changes in v2: > > > - Drop duplicate acpi_xsdt patch > > > - Put the board_early_init_r code into board/ > > > > > > board/efi/efi-x86_app/Makefile | 5 + > > > board/efi/efi-x86_app/efi_app.c | 205 > > > > Our target should be to enable building the EFI app on all architectures. > > > > Only x86 specific code should be added to board/efi/efi-x86_app/efi_app.c. > > A later enhancement to make U-Boot as an EFI app more generic would be > good, but outside the scope of this patch which is moving generic code > out from "lib" and in to "board". This patch was marked as old /archived in patchwork so has been forgotten. I have marked it new and non-archived in the hope that it can be applied to -master soon. I believe that having the EFI app support bootstd could be a useful addition. Regards, Simon
Re: [PATCH v2 3/3] efi: Avoid using dm_scan_other()
On Tue, Nov 21, 2023 at 01:18:09PM +0100, Heinrich Schuchardt wrote: > On 11/12/23 21:55, Simon Glass wrote: > > This function is defined by bootstd so using it precludes using that > > feature. Use the board_early_init_r() feature instead. > > > > This requires moving quite a lot of code into the board directory, butt > > this is the normal place for code called by board_early_init_r() > > > > Signed-off-by: Simon Glass > > --- > > > > Changes in v2: > > - Drop duplicate acpi_xsdt patch > > - Put the board_early_init_r code into board/ > > > > board/efi/efi-x86_app/Makefile | 5 + > > board/efi/efi-x86_app/efi_app.c | 205 > > Our target should be to enable building the EFI app on all architectures. > > Only x86 specific code should be added to board/efi/efi-x86_app/efi_app.c. A later enhancement to make U-Boot as an EFI app more generic would be good, but outside the scope of this patch which is moving generic code out from "lib" and in to "board". -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 3/3] efi: Avoid using dm_scan_other()
On 11/12/23 21:55, Simon Glass wrote:
This function is defined by bootstd so using it precludes using that
feature. Use the board_early_init_r() feature instead.
This requires moving quite a lot of code into the board directory, butt
this is the normal place for code called by board_early_init_r()
Signed-off-by: Simon Glass
---
Changes in v2:
- Drop duplicate acpi_xsdt patch
- Put the board_early_init_r code into board/
board/efi/efi-x86_app/Makefile | 5 +
board/efi/efi-x86_app/efi_app.c | 205
Our target should be to enable building the EFI app on all architectures.
Only x86 specific code should be added to board/efi/efi-x86_app/efi_app.c.
Best regards
Heinrich
configs/efi-x86_app64_defconfig | 1 +
lib/efi/efi_app.c | 187 -
4 files changed, 211 insertions(+), 187 deletions(-)
create mode 100644 board/efi/efi-x86_app/Makefile
create mode 100644 board/efi/efi-x86_app/efi_app.c
diff --git a/board/efi/efi-x86_app/Makefile b/board/efi/efi-x86_app/Makefile
new file mode 100644
index ..682f8754b44d
--- /dev/null
+++ b/board/efi/efi-x86_app/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright 2023 Google LLC
+
+obj-y += efi_app.o
diff --git a/board/efi/efi-x86_app/efi_app.c b/board/efi/efi-x86_app/efi_app.c
new file mode 100644
index ..c5e4192fe06c
--- /dev/null
+++ b/board/efi/efi-x86_app/efi_app.c
@@ -0,0 +1,205 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * EFI-app board implementation
+ *
+ * Copyright 2023 Google LLC
+ * Written by Simon Glass
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/**
+ * efi_bind_block() - bind a new block device to an EFI device
+ *
+ * Binds a new top-level EFI_MEDIA device as well as a child block device so
+ * that the block device can be accessed in U-Boot.
+ *
+ * The device can then be accessed using 'part list efi 0', 'fat ls efi 0:1',
+ * for example, just like any other interface type.
+ *
+ * @handle: handle of the controller on which this driver is installed
+ * @blkio: block io protocol proxied by this driver
+ * @device_path: EFI device path structure for this
+ * @len: Length of @device_path in bytes
+ * @devp: Returns the bound device
+ * Return: 0 if OK, -ve on error
+ */
+int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio,
+ struct efi_device_path *device_path, int len,
+ struct udevice **devp)
+{
+ struct efi_media_plat plat;
+ struct udevice *dev;
+ char name[18];
+ int ret;
+
+ plat.handle = handle;
+ plat.blkio = blkio;
+ plat.device_path = malloc(device_path->length);
+ if (!plat.device_path)
+ return log_msg_ret("path", -ENOMEM);
+ memcpy(plat.device_path, device_path, device_path->length);
+ ret = device_bind(dm_root(), DM_DRIVER_GET(efi_media), "efi_media",
+ &plat, ofnode_null(), &dev);
+ if (ret)
+ return log_msg_ret("bind", ret);
+
+ snprintf(name, sizeof(name), "efi_media_%x", dev_seq(dev));
+ device_set_name(dev, name);
+ *devp = dev;
+
+ return 0;
+}
+
+/**
+ * devpath_is_partition() - Figure out if a device path is a partition
+ *
+ * Checks if a device path refers to a partition on some media device. This
+ * works by checking for a valid partition number in a hard-driver media device
+ * as the final component of the device path.
+ *
+ * @path: device path
+ * Return: true if a partition, false if not
+ * (e.g. it might be media which contains partitions)
+ */
+static bool devpath_is_partition(const struct efi_device_path *path)
+{
+ const struct efi_device_path *p;
+ bool was_part = false;
+
+ for (p = path; p->type != DEVICE_PATH_TYPE_END;
+p = (void *)p + p->length) {
+ was_part = false;
+ if (p->type == DEVICE_PATH_TYPE_MEDIA_DEVICE &&
+ p->sub_type == DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH) {
+ struct efi_device_path_hard_drive_path *hd =
+ (void *)path;
+
+ if (hd->partition_number)
+ was_part = true;
+ }
+ }
+
+ return was_part;
+}
+
+/**
+ * setup_block() - Find all block devices and setup EFI devices for them
+ *
+ * Partitions are ignored, since U-Boot has partition handling. Errors with
+ * particular devices produce a warning but execution continues to try to
+ * find others.
+ *
+ * Return: 0 if found, -ENOSYS if there is no boot-services table, -ENOTSUPP
+ * if a required protocol is not supported
+ */
+static int setup_block(void)
+{
+ efi_guid_t efi_blkio_guid = EFI_BLOCK_IO_PROTOCOL_GUID;
+ efi_guid_t efi_devpath_guid = EFI_DEVICE_PATH_PROTOCOL_GUID;
+ e
[PATCH v2 3/3] efi: Avoid using dm_scan_other()
This function is defined by bootstd so using it precludes using that
feature. Use the board_early_init_r() feature instead.
This requires moving quite a lot of code into the board directory, butt
this is the normal place for code called by board_early_init_r()
Signed-off-by: Simon Glass
---
Changes in v2:
- Drop duplicate acpi_xsdt patch
- Put the board_early_init_r code into board/
board/efi/efi-x86_app/Makefile | 5 +
board/efi/efi-x86_app/efi_app.c | 205
configs/efi-x86_app64_defconfig | 1 +
lib/efi/efi_app.c | 187 -
4 files changed, 211 insertions(+), 187 deletions(-)
create mode 100644 board/efi/efi-x86_app/Makefile
create mode 100644 board/efi/efi-x86_app/efi_app.c
diff --git a/board/efi/efi-x86_app/Makefile b/board/efi/efi-x86_app/Makefile
new file mode 100644
index ..682f8754b44d
--- /dev/null
+++ b/board/efi/efi-x86_app/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright 2023 Google LLC
+
+obj-y += efi_app.o
diff --git a/board/efi/efi-x86_app/efi_app.c b/board/efi/efi-x86_app/efi_app.c
new file mode 100644
index ..c5e4192fe06c
--- /dev/null
+++ b/board/efi/efi-x86_app/efi_app.c
@@ -0,0 +1,205 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * EFI-app board implementation
+ *
+ * Copyright 2023 Google LLC
+ * Written by Simon Glass
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/**
+ * efi_bind_block() - bind a new block device to an EFI device
+ *
+ * Binds a new top-level EFI_MEDIA device as well as a child block device so
+ * that the block device can be accessed in U-Boot.
+ *
+ * The device can then be accessed using 'part list efi 0', 'fat ls efi 0:1',
+ * for example, just like any other interface type.
+ *
+ * @handle: handle of the controller on which this driver is installed
+ * @blkio: block io protocol proxied by this driver
+ * @device_path: EFI device path structure for this
+ * @len: Length of @device_path in bytes
+ * @devp: Returns the bound device
+ * Return: 0 if OK, -ve on error
+ */
+int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio,
+ struct efi_device_path *device_path, int len,
+ struct udevice **devp)
+{
+ struct efi_media_plat plat;
+ struct udevice *dev;
+ char name[18];
+ int ret;
+
+ plat.handle = handle;
+ plat.blkio = blkio;
+ plat.device_path = malloc(device_path->length);
+ if (!plat.device_path)
+ return log_msg_ret("path", -ENOMEM);
+ memcpy(plat.device_path, device_path, device_path->length);
+ ret = device_bind(dm_root(), DM_DRIVER_GET(efi_media), "efi_media",
+ &plat, ofnode_null(), &dev);
+ if (ret)
+ return log_msg_ret("bind", ret);
+
+ snprintf(name, sizeof(name), "efi_media_%x", dev_seq(dev));
+ device_set_name(dev, name);
+ *devp = dev;
+
+ return 0;
+}
+
+/**
+ * devpath_is_partition() - Figure out if a device path is a partition
+ *
+ * Checks if a device path refers to a partition on some media device. This
+ * works by checking for a valid partition number in a hard-driver media device
+ * as the final component of the device path.
+ *
+ * @path: device path
+ * Return: true if a partition, false if not
+ * (e.g. it might be media which contains partitions)
+ */
+static bool devpath_is_partition(const struct efi_device_path *path)
+{
+ const struct efi_device_path *p;
+ bool was_part = false;
+
+ for (p = path; p->type != DEVICE_PATH_TYPE_END;
+p = (void *)p + p->length) {
+ was_part = false;
+ if (p->type == DEVICE_PATH_TYPE_MEDIA_DEVICE &&
+ p->sub_type == DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH) {
+ struct efi_device_path_hard_drive_path *hd =
+ (void *)path;
+
+ if (hd->partition_number)
+ was_part = true;
+ }
+ }
+
+ return was_part;
+}
+
+/**
+ * setup_block() - Find all block devices and setup EFI devices for them
+ *
+ * Partitions are ignored, since U-Boot has partition handling. Errors with
+ * particular devices produce a warning but execution continues to try to
+ * find others.
+ *
+ * Return: 0 if found, -ENOSYS if there is no boot-services table, -ENOTSUPP
+ * if a required protocol is not supported
+ */
+static int setup_block(void)
+{
+ efi_guid_t efi_blkio_guid = EFI_BLOCK_IO_PROTOCOL_GUID;
+ efi_guid_t efi_devpath_guid = EFI_DEVICE_PATH_PROTOCOL_GUID;
+ efi_guid_t efi_pathutil_guid = EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID;
+ efi_guid_t efi_pathtext_guid = EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID;
+ struct efi_boot_services *boot = efi_get_boot();
+ struct efi_
