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 115F6C7618A for ; Mon, 20 Mar 2023 07:58:34 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 2E60385B2A; Mon, 20 Mar 2023 08:58:33 +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="ncaaJU3E"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 50ABB85AEC; Mon, 20 Mar 2023 08:58:31 +0100 (CET) Received: from mail-ed1-x534.google.com (mail-ed1-x534.google.com [IPv6:2a00:1450:4864:20::534]) (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 9927F85A62 for ; Mon, 20 Mar 2023 08:58:27 +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=ilias.apalodimas@linaro.org Received: by mail-ed1-x534.google.com with SMTP id o12so43083196edb.9 for ; Mon, 20 Mar 2023 00:58:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1679299107; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=+12XIkDjZzTgS4Ms19W01BP7KMV/TyynJpX/msE3D80=; b=ncaaJU3EwhGQ6bVySmrXy8y4E+RHSupup1p12G5/NSJ0uChFnLwp0D7ruTkSvo+ttO 9uW6qWEBeOdgvfkcCZHIxaI+udD5PZTqjcbxZpfw7nlB/RWsNJu3LPFLs9+Yhtpt2B2U KelCsszHtjKaT4Gvse0QKegnUMR/wV2LKIk1vb/oxdUHqgiwKCigh+XSnA6oe19KDPuA weE1iJcFV0Ml18eoxHpoXErYvBFgkArafQjsegC3jbEz37J0/wfcemhaNKCqj7Ma4ff/ sqX4EXmMhAfyd677MRuUuzX2gSx6hUeiS2QUhQ/lUp5TqQ3gVhMJDN4rz/4rdq3sZthk IzFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679299107; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=+12XIkDjZzTgS4Ms19W01BP7KMV/TyynJpX/msE3D80=; b=T4XmedLquC4axJukPiR3wDJhhtcxxgIzuNh7bR7y2nvsS9Eg/Xan5MkRsIJosJbJ9w Uxhg6Tq8+Volhh5TnFw9cnDpL+TVEPa8Zpu7CTFTQxxTu980Elq8gHX5vKGMVgZaxmuF URRcTHUVcN3OqNhAbtuOI7LPbE3kt4hRbjEh7UbAOJL6Gzf6oMXczHJlj1o6SVB9wwya q1j46RJYi22ynTK87vL9dY5+W1RBrhmCwAKq8QBUjav30OEDLD9m4i539eXq6zdFt9e3 I3xepaHXc3+4iLtWPv1Q5mblIp+ORXBjogGyk5f+BLPx5lsKsrNpaZkS9TkLBSpgW5kC shtA== X-Gm-Message-State: AO0yUKXb7YSbzzarhUADrJicVnwIOiN/go+9bIW/wqjdbazTtj/G+bi8 x55zYPIoKl1aI/ZleY7UNmfiew== X-Google-Smtp-Source: AK7set/95XlImwfYeIqHshSRTKC/rg9G4XUJtlTVugrmBdAyR+boXkMSnfL81xJjcMwRbudH3QD2VA== X-Received: by 2002:a17:906:6a17:b0:933:c052:a277 with SMTP id qw23-20020a1709066a1700b00933c052a277mr4013741ejc.12.1679299107149; Mon, 20 Mar 2023 00:58:27 -0700 (PDT) Received: from hera (ppp176092130041.access.hol.gr. [176.92.130.41]) by smtp.gmail.com with ESMTPSA id l19-20020a170906079300b00932ed432475sm2899621ejc.124.2023.03.20.00.58.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Mar 2023 00:58:26 -0700 (PDT) Date: Mon, 20 Mar 2023 09:58:24 +0200 From: Ilias Apalodimas To: Heinrich Schuchardt Cc: u-boot@lists.denx.de, Patrick Delaunay Subject: Re: [PATCH 2/2] efi_loader: fix device-path for USB devices Message-ID: References: <20230319151809.185099-1-heinrich.schuchardt@canonical.com> <20230319151809.185099-3-heinrich.schuchardt@canonical.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230319151809.185099-3-heinrich.schuchardt@canonical.com> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.8 at phobos.denx.de X-Virus-Status: Clean On Sun, Mar 19, 2023 at 04:18:09PM +0100, Heinrich Schuchardt wrote: > EFI device paths for block devices must be unique. If a non-unique device > path is discovered, probing of the block device fails. > > Currently we use UsbClass() device path nodes. As multiple devices may > have the same vendor and product id these are non-unique. Instead we > should use Usb() device path nodes. They include the USB port on the > parent hub. Hence they are unique. > > A USB storage device may contain multiple logical units. These can be > modeled as Ctrl() nodes. > > Reported-by: Patrick Delaunay > Signed-off-by: Heinrich Schuchardt > --- > lib/efi_loader/efi_device_path.c | 45 +++++++++++++++++++++++--------- > 1 file changed, 33 insertions(+), 12 deletions(-) > > diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c > index 3b267b713e..b6dd575b13 100644 > --- a/lib/efi_loader/efi_device_path.c > +++ b/lib/efi_loader/efi_device_path.c > @@ -147,7 +147,7 @@ struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp) > * in practice fallback.efi just uses MEDIA:HARD_DRIVE > * so not sure when we would see these other cases. > */ > - if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB_CLASS) || > + if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB) || > EFI_DP_TYPE(dp, MEDIA_DEVICE, HARD_DRIVE_PATH) || > EFI_DP_TYPE(dp, MEDIA_DEVICE, FILE_PATH)) > return dp; > @@ -564,6 +564,11 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev) > return dp_size(dev->parent) > + sizeof(struct efi_device_path_vendor) + 1; > #endif > +#ifdef CONFIG_USB > + case UCLASS_MASS_STORAGE: > + return dp_size(dev->parent) > + + sizeof(struct efi_device_path_controller); > +#endif > #ifdef CONFIG_VIRTIO_BLK > case UCLASS_VIRTIO: > /* > @@ -585,7 +590,7 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev) > case UCLASS_MASS_STORAGE: > case UCLASS_USB_HUB: > return dp_size(dev->parent) + > - sizeof(struct efi_device_path_usb_class); > + sizeof(struct efi_device_path_usb); > default: > /* just skip over unknown classes: */ > return dp_size(dev->parent); > @@ -741,6 +746,19 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) > memcpy(&dp->ns_id, &ns_id, sizeof(ns_id)); > return &dp[1]; > } > +#endif > +#if defined(CONFIG_USB) > + case UCLASS_MASS_STORAGE: { > + struct blk_desc *desc = desc = dev_get_uclass_plat(dev); > + struct efi_device_path_controller *dp = > + dp_fill(buf, dev->parent); > + > + dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; > + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_CONTROLLER; > + dp->dp.length = sizeof(*dp); > + dp->controller_number = desc->lun; > + return &dp[1]; > + } > #endif > default: > debug("%s(%u) %s: unhandled parent class: %s (%u)\n", > @@ -767,19 +785,22 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) > #endif > case UCLASS_MASS_STORAGE: > case UCLASS_USB_HUB: { > - struct efi_device_path_usb_class *udp = > - dp_fill(buf, dev->parent); > - struct usb_device *udev = dev_get_parent_priv(dev); > - struct usb_device_descriptor *desc = &udev->descriptor; > + struct efi_device_path_usb *udp = dp_fill(buf, dev->parent); > + > + switch (device_get_uclass_id(dev->parent)) { > + case UCLASS_USB_HUB: { > + struct usb_device *udev = dev_get_parent_priv(dev); > > + udp->parent_port_number = udev->portnr; > + break; > + } > + default: > + udp->parent_port_number = 0; > + } > udp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; > - udp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_USB_CLASS; > + udp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_USB; > udp->dp.length = sizeof(*udp); > - udp->vendor_id = desc->idVendor; > - udp->product_id = desc->idProduct; > - udp->device_class = desc->bDeviceClass; > - udp->device_subclass = desc->bDeviceSubClass; > - udp->device_protocol = desc->bDeviceProtocol; > + udp->usb_interface = 0; Why is this 0? Can't we derive something reasonable from the uclass? Thanks /Ilias > > return &udp[1]; > } > -- > 2.39.2 >