On 11/10/21 09:11, Heinrich Schuchardt wrote:


On 11/10/21 07:24, AKASHI Takahiro wrote:
On Tue, Nov 09, 2021 at 10:50:37AM +0100, Heinrich Schuchardt wrote:
On 11/9/21 02:32, AKASHI Takahiro wrote:
Support for booting from removable media is now added to UEFI boot
manager. Here we should modify efidebug command in order to define
a proper "BootXXXX" variable.

With this patch applied, you will be able to specify the boot order,
usb and scsi:

I guess for a removable device this should work even if the device is not
present. But currently:

=> efidebug boot add -b 1000 USB_present usb 0:1 EFI/BOOT/BOOTARM.EFI
=> efidebug boot add -b 1000 USB_not_present usb 1:1 EFI/BOOT/BOOTARM.EFI
Cannot create device path for "usb 1:1"

# In the second, I don't expect you to specify the file path,
# "EFI/BOOT/BOOTARM.EFI", for removable media support.

Yes, I have been aware of this issue but it is not this-patch specific
but an existing issue as you clearly mentioned above.

A media device path like:

/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x0,0x0,0x9,0x0,0x1)/UsbClass(0x781,0x5571,0x0,0x0,0x0)/HD(1,MBR,0x0c449046,0x800,0x800)

is not very helpful because the next device that you insert may have a
different location of the ESP partition.

I think you should store

/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x0,0x0,0x9,0x0,0x1)/UsbClass(0x781,0x5571,0x0,0x0,0x0)

Apparently it is promising but actually not because
"UsbClass(0x781,0x5571,0x0,0x0,0x0)" contains Vendor ID and
Product ID which can only be retrieved from a real device attached
to the board.

0x781,0x5571 relates to
Vendor: SanDisk Corp.
Device: Cruzer Fit

This is how Linux sees my OrangePi PC:

$ lsusb
Bus 009 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 007 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 008 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 006 Device 002: ID 0781:5571 SanDisk Corp. Cruzer Fit
Bus 006 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 004 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

The bus numbering seems not to be stable:

$ lsusb
Bus 009 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 008 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 007 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 005 Device 002: ID 0781:5571 SanDisk Corp. Cruzer Fit
Bus 005 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 006 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 004 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub


And this is in U-Boot:

usb           0  [ + ]   ehci_generic          |   |-- usb@1c1a000
usb_hub       0  [ + ]   usb_hub               |   |   `-- usb_hub
usb           1  [ + ]   ohci_generic          |   |-- usb@1c1a400
usb_hub       1  [ + ]   usb_hub               |   |   `-- usb_hub
usb           2  [ + ]   ehci_generic          |   |-- usb@1c1b000
usb_hub       2  [ + ]   usb_hub               |   |   `-- usb_hub
usb           3  [ + ]   ohci_generic          |   |-- usb@1c1b400
usb_hub       3  [ + ]   usb_hub               |   |   `-- usb_hub
usb           4  [ + ]   ehci_generic          |   |-- usb@1c1c000
usb_hub       4  [ + ]   usb_hub               |   |   `-- usb_hub
usb_mass_s    0  [ + ]   usb_mass_storage      |   |       `-- usb_mass_storage blk           1  [   ]   usb_storage_blk       |   |           `-- usb_mass_storage.lun0
usb           5  [ + ]   ohci_generic          |   |-- usb@1c1c400
usb_hub       5  [ + ]   usb_hub               |   |   `-- usb_hub
usb           6  [ + ]   ehci_generic          |   |-- usb@1c1d000
usb_hub       6  [ + ]   usb_hub               |   |   `-- usb_hub
usb           7  [ + ]   ohci_generic          |   |-- usb@1c1d400
usb_hub       7  [ + ]   usb_hub               |   |   `-- usb_hub

The location where a USB is plugged is identified by the port of the USB hub it is connected to.

I think before we can properly support removable media we will have to get the UEFI device path right. We need a node on all of the levels of the DM tree of which more than one instance can exist.

Here is an example of device path created by an AMI BIOS for an ESP partition on a USB stick:

PciRoot(0x0)/Pci(0x1,0x2)/Pci(0x0,0x0)/Pci(0x8,0x0)/Pci(0x0,0x1)/USB(0x2,0x0)/USB(0x3,0x0)/HD(1,GPT,1AD3DE75-504A-47DF-A5CE-81D9EF0252A2,0x40,0x55D824)

The two number in PCI(foo, bar) are:
PCI device
PCI function

The two numbers in USB(foo, bar) are:
USB Parent Port Number
USB Interface Number

By using USB() instead of USBClass() the device path becomes generic. If you plug in a device from a different vendor into the same USB port, you will get the same device path for the device. Only the partition information may change.

Best regards

Heinrich



I'm not yet sure where "UsbClass(0x0,0x0,0x9,0x0,0x1)" comes from.

"USB(X, Y)", where X is a parent_port_number and Y is a usb_interface,
would be a more appropriate candidate for this kind of device path,
but we don't utilise this form in the current implementation.

and find the ESP on it at runtime.

I don't think the specification requires that the disk is an ESP
(at least explicitly).


=> efidebug -b 1 SCSI scsi 0:1
=> efidebug boot dump
Boot0001:
attributes: A-- (0x00000001)
    label: SCSI
    file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)/

Contrary to USB, we use Scsi(0,0) for scsi devices but it is NOT identical
to U-Boot's scsi0 (in boot_targets).

    HD(1,GPT,0ed48d12-1b4c-4e08-b3ee-decf20428036,0x800,0xa000)
    data:
      00000000: 00 00
=> efideubg -b 2 USB usb 0:1

efidebug

OK

-Takahiro Akashi

Best regards

Heinrich

=> efidebug boot order 2 1
=> bootefi bootmgr

Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
---
   cmd/efidebug.c | 46 ++++++++++++++++++++++++++++++++++++++++------
   1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index a977ca9c72f5..aaf269cdf47d 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -933,6 +933,29 @@ out:
       return initrd_dp;
   }
+/**
+ * count_arguments - count the number of arguments
+ * @argc:    Total number of arguments
+ * @argv:    Argument array
+ * Return:    Number of arguments
+ *
+ * Count the number of arguments for a given option, "-?"
+ * Specifically if the first argument is not "-?", return 0;
+ */
+static int count_arguments(int argc, char *const argv[])
+{
+    int i;
+
+    if (argv[0][0] != '-')
+        return 0;
+
+    for (i = 1; i < argc; i++)
+        if (argv[i][0] == '-')
+            break;
+
+    return i;
+}
+
   /**
    * do_efi_boot_add() - set UEFI load option
    *
@@ -945,7 +968,7 @@ out:
    *
    * Implement efidebug "boot add" sub-command. Create or change UEFI load option.
    *
- * efidebug boot add -b <id> <label> <interface> <devnum>[:<part>] <file> + * efidebug boot add -b <id> <label> <interface> <devnum>[:<part>] [<file>]     *                   -i <file> <interface2> <devnum2>[:<part>] <initrd>
    *                   -s '<options>'
    */
@@ -979,7 +1002,10 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
       argv++; /* 'add' */
       for (; argc > 0; argc--, argv++) {
           if (!strcmp(argv[0], "-b")) {
-            if (argc <  5 || lo.label) {
+            int num_args;
+
+            num_args = count_arguments(argc, argv);
+            if (num_args <  5 || num_args > 6 || lo.label) {
                   r = CMD_RET_USAGE;
                   goto out;
               }
@@ -1000,7 +1026,8 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
               utf8_utf16_strncpy(&label, argv[2], label_len);
               /* file path */
-            ret = efi_dp_from_name(argv[3], argv[4], argv[5],
+            ret = efi_dp_from_name(argv[3], argv[4],
+                           num_args == 5 ? "" : argv[5],
                              &device_path, &file_path);
               if (ret != EFI_SUCCESS) {
                   printf("Cannot create device path for \"%s %s\"\n",
@@ -1008,10 +1035,16 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
                   r = CMD_RET_FAILURE;
                   goto out;
               }
+            /* if no file name is given, use device_path */
+            if (num_args == 5) {
+                efi_free_pool(file_path);
+                file_path = device_path;
+                device_path = NULL;
+            }
               fp_size += efi_dp_size(file_path) +
                   sizeof(struct efi_device_path);
-            argc -= 5;
-            argv += 5;
+            argc -= num_args;
+            argv += num_args;
           } else if (!strcmp(argv[0], "-i")) {
               if (argc < 3 || initrd_dp) {
                   r = CMD_RET_USAGE;
@@ -1078,7 +1111,8 @@ out:
       free(data);
       efi_free_pool(final_fp);
       efi_free_pool(initrd_dp);
-    efi_free_pool(device_path);
+    if (device_path)
+        efi_free_pool(device_path);
       efi_free_pool(file_path);
       free(lo.label);


Reply via email to