[PATCH 0/2] efi_loader: provide media ID

[Heinrich Schuchardt]

On 9/28/22 03:54, Simon Glass wrote:
> Hi,
> 
> On Tue, 27 Sept 2022 at 00:53, Heinrich Schuchardt
> <heinrich.schuchardt at canonical.com> 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