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 41816C6FA82 for ; Wed, 28 Sep 2022 06:57:55 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 9B054845EF; Wed, 28 Sep 2022 08:57:53 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=canonical.com 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=canonical.com header.i=@canonical.com header.b="G3j4i5eB"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id DAF3684BA7; Wed, 28 Sep 2022 08:57:51 +0200 (CEST) Received: from smtp-relay-internal-1.canonical.com (smtp-relay-internal-1.canonical.com [185.125.188.123]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id A0C0E844E4 for ; Wed, 28 Sep 2022 08:57:48 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=canonical.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=heinrich.schuchardt@canonical.com Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-1.canonical.com (Postfix) with ESMTPS id C960C3F19B for ; Wed, 28 Sep 2022 06:57:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1664348267; bh=yr+WMdAw6dG3SV5Kcs47/MqCoM/Ze8tnfJBIoCEJijo=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=G3j4i5eB5hBdtbBo3TqqY8bSImZPbtmh6KtgoGbzcodwfX1YqmylN61UQKSRh2Tpg PyWmPxnCJoxiVYV4vxg378zc9+grvzbL/WdDbO11zK4TSJK9ufxaIvKxVJnK4L3mCr TF2Mt1C22MkzcC0wkOeWW9KWxv14f9U2usVeIGH0xu3pk3MStSrg+pOKR2brfIVPYT SSlI6kjYLe3bXi4WVLO3mGXhLtncJSMj4ivzjC5z/HsjW+hqjqD+LUdS0RS7unTkLg wFa5nCp2RFSF4srk+ZthvblwOCctmV3Z/0bjua5g+oQV17Mi1yGGLMqYw6gmjlN7gi vQshcJc4+YWOw== Received: by mail-ed1-f71.google.com with SMTP id y14-20020a056402440e00b0044301c7ccd9so9500654eda.19 for ; Tue, 27 Sep 2022 23:57:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date; bh=yr+WMdAw6dG3SV5Kcs47/MqCoM/Ze8tnfJBIoCEJijo=; b=eSTqH7YJ6SPkZsSfBQ/l9nQodnQANKAmXfaiK57EH5CP9L+aJuJ/eSLjViP6q3qr9Q YjHAVCSn9lbxWXk9u8giE/Yj735aJH5Ud9oHbZT9XsSXNFzf+TmJJ5VXnl5vitQDolIr TPSwT940cl0gagg6m4x1UTSgJpAlm1T7Dn/vZMHhlLU5wO/DPJzTivYwQceP7If+zC/j gFhjWm+IS7RP271oI3vxc05hPFD/jMKSA2N9jLzxrsnBlh7aVUchyXIRVjvXlPDoQ1/E B1HRlMZKYkiWR7ycxnZpMNxBr9sGMeRZIOu/DhLQdNTta1Gnd6MkABukW3yfHHB/A03/ duow== X-Gm-Message-State: ACrzQf1pkynO0JdUzFkjROXlGmAiYvrV9/us2/T0TREEYs1AfgqTPgSv SPJuhuEIIvKFlPRd3jWOciBahPo/YN7GOznFF83liW7SJtgIZqEBHk7iuugfRWrOqDsaBm3kxWv t1HhpLcayVhOAmBIZg96PvyoKBOiher4= X-Received: by 2002:a17:907:2d8a:b0:781:cef1:2ceb with SMTP id gt10-20020a1709072d8a00b00781cef12cebmr26657826ejc.470.1664348266846; Tue, 27 Sep 2022 23:57:46 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5EmrjMe+BQ9MNlMJlg+4aW91JUMS0nqoGuG9hVRT2gL/Ec3v5kS4/pXGVy8S5q0nQFnYg69A== X-Received: by 2002:a17:907:2d8a:b0:781:cef1:2ceb with SMTP id gt10-20020a1709072d8a00b00781cef12cebmr26657807ejc.470.1664348266559; Tue, 27 Sep 2022 23:57:46 -0700 (PDT) Received: from [192.168.123.94] (ip-084-118-157-002.um23.pools.vodafone-ip.de. [84.118.157.2]) by smtp.gmail.com with ESMTPSA id v17-20020aa7cd51000000b00457f4b22cebsm283461edw.44.2022.09.27.23.57.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 27 Sep 2022 23:57:45 -0700 (PDT) Message-ID: Date: Wed, 28 Sep 2022 08:57:43 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.0 Subject: Re: [PATCH 0/2] efi_loader: provide media ID To: Simon Glass Cc: AKASHI Takahiro , Ilias Apalodimas , Masahisa Kojima , U-Boot Mailing List References: <20220915200242.18358-1-heinrich.schuchardt@canonical.com> <20220916005805.GA45676@laputa> <933e5bf9-1eef-3e8b-c55d-dad943f9177f@canonical.com> <20220927015117.GA34139@laputa> <422e5a00-b71e-2223-2d36-a856277d8415@canonical.com> Content-Language: en-US From: Heinrich Schuchardt In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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.6 at phobos.denx.de X-Virus-Status: Clean On 9/28/22 03:54, Simon Glass wrote: > Hi, > > On Tue, 27 Sept 2022 at 00:53, Heinrich Schuchardt > wrote: >> >> >> >> On 9/27/22 03:51, AKASHI Takahiro wrote: >>> On Mon, Sep 26, 2022 at 08:06:52AM +0200, Heinrich Schuchardt wrote: >>>> >>>> >>>> On 9/16/22 02:58, AKASHI Takahiro wrote: >>>>> On Thu, Sep 15, 2022 at 10:02:40PM +0200, Heinrich Schuchardt wrote: >>>>>> The medium a device like 'mmc 0' or 'usb 0' points to may change over >>>>>> time. Hence device type and number are not sufficient to identify the >>>>>> inserted medium. The same is true for the device path generated for >>>>>> such a device. >>>>> >>>>> Well, it depends on how a device path is generated in U-Boot's UEFI >>>>> implementation. I believe that a device path represents an "unique path" >>>>> to a given device however this device is enumerated. >>>>> In this sense, the current dp_fill()/efi_dp_from_part() is not a right >>>>> implementation as it relies on device numbers. >>>>> Furthermore, a generated device path here is different from one generated >>>>> by EDK2 (even if both software are run on the same board). >>>>> >>>>> This is an issue that I used to tackle in >>>>> https://lists.denx.de/pipermail/u-boot/2021-November/468216.html >>>>> although I have since had no progress. >>>>> >>>>>> This is why the EFI_BLOCK_IO_PROTOCOL provides a field >>>>>> MediaId. >>>>>> >>>>>> Whenever a removable medium is changed or a new block device with a >>>>>> previously used device path is created we should provide a different >>>>>> MediaID. >>>>>> >>>>>> This series adds a field media_id to the block device descriptor and fills >>>>>> it after probing. The value of the field is then copied to the >>>>>> EFI_BLOCK_IO_PROTOCOL. >>>>> >>>>> I'm afraid that your patch doesn't always work as you expect. >>>>> When "scsi rescan" or "usb stop; usb start", for instance, is invoked, >>>>> all the existing devices and associated blk_desc structures are once freed >>>>> and even if nothing is changed, i.e. a device is neither removed nor added, >>>>> the exact same structures will be re-created. >>>>> With your patch applied, however, a new (and different) "media_id" will be >>>>> assigned to an existing device. UEFI User may be notified of "media change". >>>>> (To be honest, this is quite unlikely because the current UEFI implementation >>>>> doesn't use BLOCK_IO_PROTOCOL internally, say, for file system access.) >>>> >>>> This behavior matches what EDK II does if you remove a device and create a >>>> new device. >>> >>> I don't think that EDK2 has "scsi rescan" or others, which users can invoke >>> at any time. Moreover, I believe that EDK2 code (drivers) checks whether a device >>> is really changed or not before updating a MediaId. >>> >>>> If a device is removed and recreated anything could have happened in between >>>> like complete repartitioning. We cannot assume that any cached state is >>>> valid anymore even if GUIDs are the same. >>> >>> I'm not sure if you fully understand my point. >>> My assumption is the case where a device is NOT removed around "scsi rescan" >>> (or usb stop/start) and stays online. In this case, >>> 1. access to, say, "scsi 0:1", via UEFI BLOCK_IO succeeds >>> 2. "scsi rescan" >>> 3. access to the same device, "scsi 0:1", via UEFI BLOCK_IO >>> currently (3) succeeds, but with your patch, it may potentially fail because >>> of media_id altered. >>> >>> I admit that it will not happen under the current UEFI implementation because >>> non of UEFI applications will survive across command lines and none of information, >>> including media_id or handle, can be carried over from (1) to (3). >>> But unconditionally incrementing an internally-held media_id, as in your patch, >>> is a wrong behavior. >> >> The patch issues a new media ID if a new device is probed which only >> happens to have the same device number if another device of that number >> was removed before. >> >> Commands like 'usb scan' don't necessarily issue the same numbers to the >> same device as before the command if a new device has been attached in >> the meanwhile. >> >> Assuming that a new device contains the same medium as an old one >> because by chance it has the same device number is definitively unsafe. >> >> If a device is probed, we have to assume that it contains a new medium. > > Sorry if I repeat myself, but this sort of thing should be handled in > the driver model code. Can we get some more progress on integrating > the EFI layer better? The last mails where about *whether* the media ID should be bumped after a block device has been created and not about where we will implement it. Best regards Heinrich