Heinrich,
On Thu, Nov 25, 2021 at 02:44:28PM +0900, AKASHI Takahiro wrote:
> Heinrich,
>
> On Wed, Nov 24, 2021 at 12:10:32PM +0900, AKASHI Takahiro wrote:
> > On Sat, Nov 20, 2021 at 01:54:30PM +0100, Heinrich Schuchardt wrote:
> > > Hello Takahiro,
> > >
> > > in a prior mail we have discussed the creation of device paths for USB
> > > mass storage devices.
> > >
> > > On the sand boxyou get the following devices after 'usb start':
> > >
> > > Class Index Probed Driver Name
> > > -----------------------------------------------------------
> > > usb 0 [ + ] usb_sandbox |-- usb@1
> > > usb_hub 0 [ + ] usb_hub | `-- hub
> > > usb_mass_s 0 [ + ] usb_mass_storage | |--
> > > usb_mass_storage
> > > blk 3 [ ] usb_storage_blk | | `--
> > > usb_mass_storage.lun0
> > > usb_mass_s 1 [ + ] usb_mass_storage | |--
> > > usb_mass_storage
> > > blk 4 [ ] usb_storage_blk | | `--
> > > usb_mass_storage.lun0
> > > usb_mass_s 2 [ + ] usb_mass_storage | `--
> > > usb_mass_storage
> > > blk 5 [ ] usb_storage_blk | `--
> > > usb_mass_storage.lun0
> > >
> > > For of these storage devices we try to create the same device path which
> > > is not allowable:
> > >
> > > => usb start
> > > starting USB...
> > > Bus usb@1: scanning bus usb@1 for devices... 5 USB Device(s) found
> > > scanning usb for storage devices... 3 Storage Device(s) found
> > > =>
> > > => efidebug dh
> > > Scanning disk mmc2.blk...
> > > handle 0000000015e34f00,
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(2)/SD(0)
> > > Scanning disk mmc1.blk...
> > > handle 0000000015e36b30,
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(1)/SD(1)
> > > Scanning disk mmc0.blk...
> > > handle 0000000015e35b00,
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(2)
> > > Scanning disk usb_mass_storage.lun0...
> > > handle 0000000015e35c10,
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x1234,0x5678,0x9,0x0,0x0)/UsbClass(0x1234,0x5678,0x0,0x0,0x0)
> > > fs_devread read outside partition 2
> > > Failed to mount ext2 filesystem...
> > > BTRFS: superblock end 69632 is larger than device size 512
> > > Scanning disk usb_mass_storage.lun0...
> > > handle 0000000015e361f0,
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x1234,0x5678,0x9,0x0,0x0)/UsbClass(0x1234,0x5678,0x0,0x0,0x0)
> > > ERROR: failure to add disk device usb_mass_storage.lun0, r = 20
> > > Error: Cannot initialize UEFI sub-system, r = 20
> > >
> > > I will provide a patch that will allow the first USB device to be used
> > > and avoids stopping the boot process. But we really have to walk the dm
> > > tree to create a device patch for USB devices based on port IDs.
> > >
> > > We should add a new field to struct uclass_driver:
> > >
> > > struct efi_device_path *get_node(udevice *dev);
> > >
> > > This function shall return a pointer to an freshly allocated buffer with
> > > the device node for the device. To build the devicepath we can then walk
> > > the dm tree.
> >
> > I'm not sure this is an acceptable solution.
> >
> > Let me make sure:
> > The goal would be to create a device path like
> > .../USB(0x1,0x0)/HD(1,...)
> > instead of
> > .../UsbHub(0x0,0x0,0x0,0x3)/UsbMassStorage(0x46f4,0x1,0x0,0x0)/HD(1,...)
> > as we already see this format for SCSI:
> > .../Scsi(0,0)/HD(1,..)
> >
> > Right?
>
> Please try the tweak attached below.
> (I think what I did here is trivial.)
>
> If you like, I will post it as a patch.
> (See "10.3.4.5.1 USB Device Path Example".)
Any comment on my proposal?
> -Takahiro Akashi
>
> >>> From cda91e9d8144f89f0d73738b338289a7940bbe0e Mon Sep 17 00:00:00 2001
> >>> From: AKASHI Takahiro <takahiro.aka...@linaro.org>
> >>> Date: Thu, 25 Nov 2021 14:20:08 +0900
> >>> Subject: [PATCH] efi_loader: disk: usb mass-storage
>
> - use path_usb instead of path_usb_class for existing USB dp nodes
> - add a dp node for UCLASS_USB
>
> => usb start
> starting USB...
> Bus xhci_pci: Register 8001040 NbrPorts 8
> Starting the controller
> USB XHCI 1.00
> scanning bus xhci_pci for devices... 4 USB Device(s) found
> scanning usb for storage devices... 2 Storage Device(s) found
> => dm tree
> Class Index Probed Driver Name
> -----------------------------------------------------------
> root 0 [ + ] root_driver root_driver
> ...
> pci 0 [ + ] pci_generic_ecam |-- pcie@10000000
> pci_generi 0 [ ] pci_generic_drv | |-- pci_0:0.0
> virtio 32 [ + ] virtio-pci.l | |-- virtio-pci.l#0
> ethernet 0 [ + ] virtio-net | | `-- virtio-net#32
> usb 0 [ + ] xhci_pci | `-- xhci_pci
> usb_hub 0 [ + ] usb_hub | `-- usb_hub
> usb_dev_ge 0 [ + ] usb_dev_generic_drv | |--
> generic_bus_0_dev_2
> usb_mass_s 0 [ + ] usb_mass_storage | |--
> usb_mass_storage
> blk 0 [ ] usb_storage_blk | | `--
> usb_mass_storage.lun0
> usb_mass_s 1 [ + ] usb_mass_storage | `--
> usb_mass_storage
> blk 1 [ ] usb_storage_blk | `--
> usb_mass_storage.lun0
> ...
> => efidebug devices
> Scanning disk usb_mass_storage.lun0...
> Scanning disk usb_mass_storage.lun0...
> Found 6 disks
> Missing RNG device for EFI_RNG_PROTOCOL
> ** Unable to read file ubootefi.var **
> Failed to load EFI variables
> Unable to find TPMv2 device
> Device Device Path
> ================ ====================
> 000000013eef3a50 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> 000000013eefe840
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USB(0xf3,0x0)/USB(0x0,0x0)/USB(0x2,0x0)
> 000000013eefe990
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USB(0xf3,0x0)/USB(0x0,0x0)/USB(0x2,0x0)/HD(1,0x01,0,0x0,0x99800)
> 000000013eefeae0
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USB(0xf3,0x0)/USB(0x0,0x0)/USB(0x2,0x0)/HD(2,0x01,0,0x99800,0x1800)
> 000000013eeff5f0
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USB(0xf3,0x0)/USB(0x0,0x0)/USB(0x3,0x0)
> 000000013eeff990
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USB(0xf3,0x0)/USB(0x0,0x0)/USB(0x3,0x0)/HD(1,GPT,ce86c5a7-b32a-488f-a346-88fe698e0edc,0x22,0x4c2a)
> 000000013eeffda0
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USB(0xf3,0x0)/USB(0x0,0x0)/USB(0x3,0x0)/HD(2,GPT,aa80aab9-33e6-42b6-b5db-def2cb8d7844,0x5000,0x1a800)
> 000000013ef00270
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/MAC(525252525252,1)
>
> Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
> ---
> lib/efi_loader/efi_device_path.c | 36 ++++++++++++++++++++++--
> lib/efi_loader/efi_device_path_to_text.c | 17 ++++++++---
> 2 files changed, 47 insertions(+), 6 deletions(-)
>
> diff --git a/lib/efi_loader/efi_device_path.c
> b/lib/efi_loader/efi_device_path.c
> index 735ed0bd0f4c..a8ec6809c422 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -557,10 +557,18 @@ __maybe_unused static unsigned int dp_size(struct
> udevice *dev)
> return dp_size(dev->parent) +
> sizeof(struct efi_device_path_sd_mmc_path);
> #endif
> +#if 1
> + case UCLASS_USB:
> + case UCLASS_USB_HUB:
> + case UCLASS_MASS_STORAGE:
> + return dp_size(dev->parent) +
> + sizeof(struct efi_device_path_usb);
> +#else
> case UCLASS_MASS_STORAGE:
> case UCLASS_USB_HUB:
> return dp_size(dev->parent) +
> sizeof(struct efi_device_path_usb_class);
> +#endif
> default:
> /* just skip over unknown classes: */
> return dp_size(dev->parent);
> @@ -740,6 +748,24 @@ __maybe_unused static void *dp_fill(void *buf, struct
> udevice *dev)
> return &sddp[1];
> }
> #endif
> +#if 1
> + case UCLASS_USB:
> + case UCLASS_USB_HUB:
> + case UCLASS_MASS_STORAGE: {
> + struct efi_device_path_usb *udp =
> + dp_fill(buf, dev->parent);
> + struct usb_device *udev = dev_get_parent_priv(dev);
> +
> + udp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
> + udp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_USB;
> + udp->dp.length = sizeof(*udp);
> + udp->parent_port_number = udev->portnr;
> + /* TODO: see usb_new_device() */
> + udp->usb_interface = 0;
> +
> + return &udp[1];
> + }
> +#else
> case UCLASS_MASS_STORAGE:
> case UCLASS_USB_HUB: {
> struct efi_device_path_usb_class *udp =
> @@ -750,14 +776,20 @@ __maybe_unused static void *dp_fill(void *buf, struct
> udevice *dev)
> udp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
> udp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_USB_CLASS;
> udp->dp.length = sizeof(*udp);
> - udp->vendor_id = desc->idVendor;
> - udp->product_id = desc->idProduct;
> +#if 1
> + if (dev->driver->id == UCLASS_MASS_STORAGE)
> + udp->device_class = 8;
> + else if (dev->driver->id == UCLASS_USB_HUB)
> + udp->device_class = 9;
> +#else
> udp->device_class = desc->bDeviceClass;
> +#endif
> udp->device_subclass = desc->bDeviceSubClass;
> udp->device_protocol = desc->bDeviceProtocol;
>
> return &udp[1];
> }
> +#endif
> default:
> debug("%s(%u) %s: unhandled device class: %s (%u)\n",
> __FILE__, __LINE__, __func__,
> diff --git a/lib/efi_loader/efi_device_path_to_text.c
> b/lib/efi_loader/efi_device_path_to_text.c
> index 57fa9d97f712..a21de4157307 100644
> --- a/lib/efi_loader/efi_device_path_to_text.c
> +++ b/lib/efi_loader/efi_device_path_to_text.c
> @@ -158,10 +158,19 @@ static char *dp_msging(char *s, struct efi_device_path
> *dp)
> struct efi_device_path_usb_class *ucdp =
> (struct efi_device_path_usb_class *)dp;
>
> - s += sprintf(s, "UsbClass(0x%x,0x%x,0x%x,0x%x,0x%x)",
> - ucdp->vendor_id, ucdp->product_id,
> - ucdp->device_class, ucdp->device_subclass,
> - ucdp->device_protocol);
> + if (ucdp->device_class == 8)
> + s += sprintf(s, "UsbMassStorage(0x%x,0x%x,0x%x,0x%x)",
> + ucdp->vendor_id, ucdp->product_id,
> + ucdp->device_subclass, ucdp->device_protocol);
> + else if (ucdp->device_class == 9)
> + s += sprintf(s, "UsbHub(0x%x,0x%x,0x%x,0x%x)",
> + ucdp->vendor_id, ucdp->product_id,
> + ucdp->device_subclass, ucdp->device_protocol);
> + else
> + s += sprintf(s, "UsbClass(0x%x,0x%x,0x%x,0x%x,0x%x)",
> + ucdp->vendor_id, ucdp->product_id,
> + ucdp->device_class, ucdp->device_subclass,
> + ucdp->device_protocol);
>
> break;
> }
> --
> 2.33.0
>
