[U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time

Thu Jan 24 21:19:28 UTC 2019

On 1/24/19 9:18 PM, Simon Glass wrote:
> Hi Takahiro,
> 
> On Thu, 24 Jan 2019 at 13:53, AKASHI Takahiro
> <takahiro.akashi at linaro.org> wrote:
>>
>> Simon,
>>
>> On Thu, Jan 24, 2019 at 10:58:53AM +1300, Simon Glass wrote:
>>> Hi Heinrich,
>>>
>>> On Wed, 23 Jan 2019 at 10:05, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>>>
>>>> On 1/22/19 8:39 PM, Simon Glass wrote:
>>>>> Hi Alex,
>>>>>
>>>>> On Tue, 22 Jan 2019 at 22:08, Alexander Graf <agraf at suse.de> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 22.01.19 09:29, AKASHI Takahiro wrote:
>>>>>>> Alex, Simon,
>>>>>>>
>>>>>>> Apologies for my slow response on this matter,
>>>>>>>
>>>>>>> On Fri, Jan 11, 2019 at 08:57:05AM +0100, Alexander Graf wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 11.01.19 05:29, AKASHI Takahiro wrote:
>>>>>>>>> Alex, Heinrich and Simon,
>>>>>>>>>
>>>>>>>>> Thank you for your comments, they are all valuable but also make me
>>>>>>>>> confused as different people have different requirements :)
>>>>>>>>> I'm not sure that all of us share the same *ultimate* goal here.
>>>>>>>>
>>>>>>>> The shared ultimate goal is to "merge" (as Simon put it) dm and efi objects.
>>>>>>>
>>>>>>> I don't still understand what "merge" means very well.
>>>>>>
>>>>>> It basically means that "struct efi_object" moves into "struct udevice".
>>>>>> Every udevice instance of type UCLASS_BLK would expose the block and
>>>>>> device_path protocols.
>>>>>>
>>>>>> This will be a slightly bigger rework, but eventually allows us to
>>>>>> basically get rid of efi_init_obj_list() I think.
>>>>>
>>>>> I envisaged something like:
>>>>>
>>>>> - EFI objects have their own UCLASS_EFI uclass
>>>>> - DM uclasses which support EFI would create a child EFI device (e.g.
>>>>> a UCLASS_EFI child of each UCLASS_BLK device)
>>>>> - EFI-uclass devices would thus be bound as needed
>>>>> - Probing an EFI device would causes its parents to be probed
>>>>> - We can use all the existing DM hooks (probe, remove, parent/child
>>>>> data, operations), to implement EFI things
>>>>>
>>>>> I'm assuming that a small percentage of devices would have EFI
>>>>> children, so that this is more efficient than trying to merge the data
>>>>> structures. It also allows EFI to maintain some separate from the core
>>>>> DM code.
>>>>
>>>> Dear Simon,
>>>>
>>>> thanks to your suggestions.
>>>>
>>>> I am not yet convinced that an UCLASS_EFI child device will be helpful.
>>>> It is not an EFI object.
>>>>
>>>> A DM uclass is the equivalent to an EFI driver, i.e. a handle with the
>>>> EFI_DRIVER_BINDING_PROTOCOL installed on it [1]. So the natural thing
>>>> for a uclass supporting EFI would be to provide such a handle.
>>>>
>>>> For the actual devices we also need handles.
>>>
>>> Yes but I understand why this is problematic?
>>>
>>>>
>>>> In the EFI world partitions are devices. How does this fit into your
>>>> picture?
>>>
>>> Perhaps we could consider adding a UCLASS_PARTITION? The rework of the
>>> FS layer may not be too much - at least once we drop the non-BLK code
>>> (as you say at [1]).
>>
>> IMO, instead of considering UCLASS_PARTITION,
>> 1) add IF_TYPE_BLK_PARTITION and set its type_class_id to UCLASS_BLK
>>    So any partition has a parent node (I don't know the correct language
>>    in DM world) that is a real raw disk device, and yet is seen
>>    as a UCLASS_BLK device, or
> 
> Well 'parent node' is used in device tree. For driver model, we say
> 'parent device'.
> 
> In fact enum if_type is not so necessary in the DM world. We could
> instead use the uclass of the parent. For example, we could use
> UCLASS_MMC instead of IF_TYPE_MMC.
> 
> So I'm not sure that it is right to set the if_type to UCLASS_BLK.
> There are many U-Boot commands which number the different 'interface
> types' separately. For example 'mmc' devices create a block device
> with blk_desc containing devnum values numbered from 0, as do 'sata'
> devices, etc. So what does it mean to have a generic 'block' interface
> type?
> 
>>
>> 2) create a block device (blk_desc) for each partition, either in bind/probe
>>    or in enumerating devices, as I mentioned in the previous e-mail
> 
> Here you are really just creating a device of UCLASS_BLK, since there
> is a 1:1 correspondence between a UCLASS_BLK device and blk_desc.
> 
> struct blk_desc *desc = dev_get_uclass_platdata(blk_dev);
> 
>>
>> What's different between (1) and (2),
>> we may enumerate block devices and identify the given one by scanning
>> a UCLASS_BLK list with (1), while by scanning a blk_desc list with (2)
>> at do_load()/fs_set_blk_dev().
> 
> As above these are really the same, in that a blk_desc can only exist
> as an attachment to a UCLASS_BLK / block device.
> 
>>
>> # In any way, we will need
>> a) a bi-directional link between
>>      UCLASS_BLK          efi_obj
>>          or        and       or
>>      blk_desc            efi_disk_obj, and
>> b) a event notification mechanism, in your language, as as to maintain
>>    (create/delete) those links
>>
>> If you agree to approach (1) or (2), I will be able to start a prototyping.
> 
> I think those two are actually the same, but I suspect I misunderstand
> something.
> 
> If you look at how Unix handles devices versus partitions:
> 
> /dev/sdb - whole block device
> /dev/sdb1 - first partition
> /dev/sdb2 - second partition
> 
> So having the partitions as separate devices at least has precedent. I
> do think it would work pretty well in U-Boot too.
> 
> Another thought might be that we make the block devices be siblings of
> the 'whole' block device instead of children. Then a SATA device may
> have 4 children, for example, all UCLASS_BLK, with the first one
> containing the whole device contents and the other three containing
> subsets of the first.

There are different types of devices nodes in the UEFI spec.

Examples of messaging devices are Sata, NVMe, eMMC, SD, iSCSI,
UsbMassStorage, SAS (serial attached SCSI), Scsi, Ata (ATAPI).

Examples of media devices are HD (partition), CDROM, RamDisk.

So a partition belongs into a different catagory of device then a disk
drive.

Here is the output of the EFI shell command `devtree` on a 2018 Lenovo
computer:

Ctrl[C1] PciRoot(0x0)
 Ctrl[1E7] NVM Express Controller
  Ctrl[225] SAMSUNG MZVLW256HEHP
   Ctrl[233] FAT File System
   Ctrl[234]
PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/NVMe(0x1,..)/HD(2,GPT,..)
   Ctrl[235]
PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/NVMe(0x1,..)/HD(3,GPT,..)
   Ctrl[236]
PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/NVMe(0x1,..)/HD(4,GPT,..)
 Ctrl[1F7] AHCI Bus Driver
  Ctrl[228] Crucial_CT525MX300SSD1
   Ctrl[229] FAT File System
   Ctrl[22A]
PciRoot(0x0)/Pci(0x8,0x2)/Pci(0x0,0x0)/Sata(0x0,..)/HD(2,GPT,..)
   Ctrl[22B]
PciRoot(0x0)/Pci(0x8,0x2)/Pci(0x0,0x0)/Sata(0x0,..)/HD(3,GPT,..)

The disk Sata(0x0,0x0,0x0) or NVMe(0x1,<DevUID>) is the parent of its
respective partitions. There is no pseudo partition device HD(0, ...)
for block I/O to the disk.

On U-Boot I see trees like:
root_driver
|-- pcie at 10000000
|   |-- pci_0:0.0
|   |-- e1000#0
|   `-- ahci_pci
|       `-- ahci_scsi
|           `-- ahci_scsi.id0lun0

To keep the U-Boot and the UEFI model in sync we should define
partitions as child devices of block devices.

Best regards

Heinrich

> 
> But it seems to me that having the partitions be children of a parent
> BLK device makes more sense, since if you write to the parent you are
> implicitly writing from one of the children. Also, if you write the
> partition table, you do that with the parent BLK device, but that will
> cause the children to be deleted and recreated. Also the operations on
> a partition are not quite the same as those on a whole device: they
> can only see a window into the whole device, so will likely add their
> offset and then call the parent-device operations.
> 
> [..]
> 
> Regards,
> Simon
> 