From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 75DA0C433FE for ; Fri, 3 Dec 2021 07:12:20 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 45E4B80FF3; Fri, 3 Dec 2021 08:12:18 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="lzxI30mn"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 32DCC80FF4; Fri, 3 Dec 2021 08:12:17 +0100 (CET) Received: from mail-pg1-x52e.google.com (mail-pg1-x52e.google.com [IPv6:2607:f8b0:4864:20::52e]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id EE34A80FCC for ; Fri, 3 Dec 2021 08:12:12 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=takahiro.akashi@linaro.org Received: by mail-pg1-x52e.google.com with SMTP id r5so2181703pgi.6 for ; Thu, 02 Dec 2021 23:12:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=HuGIgW5CqPK+/Eky4vg9N9L6l7T54DwZQ2zW2mwFiJI=; b=lzxI30mnh6eTXroqPPHVncxcUmA2KROVfTcKVDKN7CgMjoAoloFJMnkbK99zYSfgc2 wz8fJ8X9fd0ptM7foiM3+lQShb7jHKX5CESyuj5InvXfU6N7vVAVoOGUOVX/FbE/OWfC vWYA/dOOsScRnW3TGJ0di+VvKXTdRU3kWSRqAhOEJDW3ugFpEefUrRXAqRcfKekVKQsj kdBRshMwwEH0/SV4iFHDzl06RsVV9l5PSeg1aQFglawANIdGXWWQq5m4WZ6r1k3G8u2J mfJ1iOgW8TFB6tkrLoJEIaazsw25ST7YY8XPDSsFZ0ikxZNsdmIXSTIALJfWe9bejIG/ saIA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:subject:message-id:mail-followup-to :references:mime-version:content-disposition:in-reply-to; bh=HuGIgW5CqPK+/Eky4vg9N9L6l7T54DwZQ2zW2mwFiJI=; b=ZA6bi+XhfDhAuUL95G0WSshLsf+83is+ifq7bMup7Kud/g5ebwJYi9mJ/5zpMWM4b2 mb9ZZ/lCpYpFf/KyualKbPmImHzbt0y8CaMm0yspHyPhd2Bmg61Gl+htQzT9vJNrS32s hNnHIDYkhnGjN2H9kaOUBWGBAF4fmXyxeOcZzpRr3NB82WkzvxFGk5t3urMb2Wfz7f5G 51hppnxCl3iCN99j/DBlCrWi8tasvoU15PIIXjDbYOqnpEu4rXm44Pu1A3fBL+fjkSx7 QQQvdrkHnQbN9T9gWy/VPHDnQpQya1suaPd5yN2nq2jd/1bMLJ2kpUSCYWU8Z22bwBh5 /NlQ== X-Gm-Message-State: AOAM533c9QaZjfi9uesKEeLISTsFQEMIwWQHraZplaP1FOyPYSIS6+KC MfHBn/Of9k6vKZCFvL+qlsLDpA== X-Google-Smtp-Source: ABdhPJw4lnJ8Atglngznb7VVJoOSUanrm8WWiQFWaJby+cf6BZcT7RekNPar3P36ZMLUoY+s8p6JbQ== X-Received: by 2002:a62:884c:0:b0:49f:9947:e5cd with SMTP id l73-20020a62884c000000b0049f9947e5cdmr17430541pfd.45.1638515531271; Thu, 02 Dec 2021 23:12:11 -0800 (PST) Received: from laputa ([2400:4050:c3e1:100:59ee:cdb7:e187:43ff]) by smtp.gmail.com with ESMTPSA id v3sm1493119pga.78.2021.12.02.23.12.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Dec 2021 23:12:10 -0800 (PST) Date: Fri, 3 Dec 2021 16:12:02 +0900 From: AKASHI Takahiro To: Heinrich Schuchardt , Simon Glass , U-Boot Mailing List , Ilias Apalodimas , Alexander Graf Subject: Re: [BUG] efi_loader: incorrect creation of device paths Message-ID: <20211203071202.GC17147@laputa> Mail-Followup-To: AKASHI Takahiro , Heinrich Schuchardt , Simon Glass , U-Boot Mailing List , Ilias Apalodimas , Alexander Graf References: <6df46f32-f76e-ef2d-aaee-d0349c480789@gmx.de> <20211124031032.GB9598@laputa> <20211125054428.GC41281@laputa> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211125054428.GC41281@laputa> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.38 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean 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 > >>> 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 > --- > 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 >