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 8AEB6C433EF for ; Sat, 5 Feb 2022 09:39:54 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 37B6E83A9D; Sat, 5 Feb 2022 10:39:52 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; secure) header.d=gmx.net header.i=@gmx.net header.b="QNLMT/yL"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id EB05583AA3; Sat, 5 Feb 2022 10:39:50 +0100 (CET) Received: from mout.gmx.net (mout.gmx.net [212.227.17.20]) (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 C57C783A90 for ; Sat, 5 Feb 2022 10:39:46 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=xypron.glpk@gmx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1644053981; bh=R+hFMEdg5hwd+VhCcNQhyptGdzMXLvst1a4kOObo7W0=; h=X-UI-Sender-Class:Date:Subject:To:Cc:References:From:In-Reply-To; b=QNLMT/yLZhl9oDTRE1gnvazQ4C09RRpKRHOG0520Cz9tu6r7gDyzEz4OAeDY2ifXD Vwt9r7NPwySfug+nrv6QW6GHthJx8SfaLDQ/gQJtD8qZ2feDj/tJzYYSeYBf/s6V2S qnYvAH60EzmGyn+m4pcDyVdSbdUprS4eqNP+ZKAQ= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [192.168.123.94] ([88.152.144.107]) by mail.gmx.net (mrgmx105 [212.227.17.168]) with ESMTPSA (Nemesis) id 1N33ET-1mDlb61WIP-013R3M; Sat, 05 Feb 2022 10:39:41 +0100 Message-ID: Date: Sat, 5 Feb 2022 10:39:39 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.1 Subject: Re: [PATCH 00/19] efi_loader: more tightly integrate UEFI disks to driver model Content-Language: en-US To: AKASHI Takahiro , sjg@chromium.org Cc: masami.hiramatsu@linaro.org, u-boot@lists.denx.de, lukma@denx.de, peng.fan@nxp.com, peng.ma@nxp.com, bmeng.cn@gmail.com, ilias.apalodimas@linaro.org, jh80.chung@samsung.com, sr@denx.de References: <20220202010853.40405-1-takahiro.akashi@linaro.org> From: Heinrich Schuchardt In-Reply-To: <20220202010853.40405-1-takahiro.akashi@linaro.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:B+WXgYalXDg6qL8TXuef438tpwS20hpyW0b90crIZnye0asJsdS 9gFozrSD237Ew05O+iy7O93xpwuba7e7Nmh3CmV7Vbg4KemPIHSNVIdioYjmMYpDNItsGiz wVi0wKJJZI6Vy6kscd6F1CkvNSTnvUsDywlzHcJcRajA62gDrDnyjnHGtejpPwKJ4bjs0zK fSK4XgvJ54yJgpO7u7R4g== X-UI-Out-Filterresults: notjunk:1;V03:K0:9vYgCcxJ7V8=:5McGpyATR4sOO9kqzbiYBb jDF5RZjGDVkgLLCiHwgdSH1zmdLGOaJ7lAPdaVqRG6vcobkBSfX8cihe2zCTIPkqCpDXh1Vf0 e1yNG7UfUGtk0cRThPmVbvqm1DeFVmtEiPzD7G6G9KsBR9GfBR+uozDhZwIyZzbBbi/dtpdGe YooP8EJnI909aR5/aSLC7xP9H2a9MrTNNr8m8pf85xNybkU4zKlCJTRYRP1vg/gvsH3XSmuow 1clK4FbcORpLFaFjDtWloWcUfcMUsDQv94M2R7+LcUE8rsf+mZY5fDaMG5C86HANsVP3GDKLu 7O+IGU+Vsp6hr0FOlQcK3hdaXYVDIsXMNcepNblM/tlegmQGsZob9l7ZpVCZJM001EJdXYwT0 UnFR6jo1qq6s3wRyoKXLxLs/QzmEL/McE0zTER/Htq8K1p8Erc//4lz1WVl3oVfNDNnr0lVI6 hrlQP6QOUffIoEe+i3IweKxCfkKQjqCgLQ896Iv6l7C8/f8i0pA2o7/NgC51R3Q3+q5Lo1pVG Il+W60IWcKVv/3kySHsv3QKgUyPxNx87pfW8FYcpx1gfwlkn2b6cthSzProna3SNhPFTcAhHu E28fgQpVOJgnRqzi0cgopruxmk9KEUSrQ3+p0BZ7JhH3El5likDnPhSZO9ZcAuns4wCu2cCJP VIOXAYuVBeeRAK7Lb/Sdt0xD2vi6BZvONMyvILuYHITEcanrCNnrGfPL+8L/6HyoSAh7duY1f SgOpjIr8Lw67SX/BCgNsLXVokaC6wUbSDMuaxHpJTkSLVigwdDT8oWENjw46B7rwfBdwODUr9 1TUTmRdGviF4xkvPC8pRspcVD5Fqi6Bdsf+YlTfIIMgWJNhIkXjwWJb9KhBRjPIFUVpHquSCe oAXex20JIlcXhxzmgNf8grF3GfGqYyLMzoFIK6FMzzjqnaKfZAZbd4ZsMZ2tRzP9vQicps5yl K69u4Ed2aEmWlpsSOO37T+KusxI4n26sWfZnOZ+Uyg/56u7zrQ0KV9b5lR4KtmQd8w35ZYAob InCENV5VX7x0OGrC7UMZWCZ8FuZZZ1+COr8g0/+Qq7JIZGKNN3JMhwHmta0dSrQfad8O3sdIP HYGvCNJRI9pXac= 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.5 at phobos.denx.de X-Virus-Status: Clean On 2/2/22 02:08, AKASHI Takahiro wrote: > Background: > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > The purpose of this patch is to reignite the discussion about how UEFI > subystem would best be integrated into U-Boot driver model. > In the past, I poposed a couple of patch series, the latest one[1], > while Heinrich revealed his idea[2], and the approach taken here is > something between them, with a focus on block device handlings. > > Disks in UEFI world: > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > In general in UEFI world, accessing to any device is performed through > a 'protocol' interface which are installed to (or associated with) the d= evice's > UEFI handle (or an opaque pointer to UEFI object data). Protocols are > implemented by either the UEFI system itself or UEFI drivers. > > For block IO's, it is a device which has EFI_BLOCK_IO_PROTOCOL (efi_disk > hereafter). Currently, every efi_disk may have one of two origins: > > a.U-Boot's block devices or related partitions > (lib/efi_loader/efi_disk.c) > b.UEFI objects which are implemented as a block device by UEFI drivers. > (lib/efi_driver/efi_block_device.c) > > All the efi_diskss as (a) will be enumelated and created only once at UE= FI > subsystem initialization (efi_disk_register()), which is triggered by > first executing one of UEFI-related U-Boot commands, like "bootefi", > "setenv -e" or "efidebug". > EFI_BLOCK_IO_PROTOCOL is implemented by UEFI system using blk_desc(->ops= ) > in the corresponding udevice(UCLASS_BLK). > > On the other hand, efi_disk as (b) will be created each time UEFI boot > services' connect_controller() is executed in UEFI app which, as a (devi= ce) > controller, gives the method to access the device's data, > ie. EFI_BLOCK_IO_PROTOCOL. > >>>> more details >>> > Internally, connect_controller() search for UEFI driver that can support > this controller/protocol, 'efi_block' driver(UCLASS_EFI) in this case, > then calls the driver's 'bind' interface, which eventually installs > the controller's EFI_BLOCK_IO_PROTOCOL to efi_disk object. > 'efi_block' driver also create a corresponding udevice(UCLASS_BLK) for > * creating additional partitions efi_disk's, and > * supporting a file system (EFI_SIMPLE_FILE_SYSTEM_PROTOCOL) on it. > <<< <<< > > Issues: > =3D=3D=3D=3D=3D=3D=3D > 1. While an efi_disk represents a device equally for either a whole disk > or a partition in UEFI world, the driver model treats only a whole > disk as a real block device or udevice(UCLASS_BLK). > > 2. efi_disk holds and makes use of "blk_desc" data even though blk_desc > in plat_data is supposed to be private and not to be accessed outsid= e > the driver model. > # This issue, though, exists for all the implmenetation of U-Boot > # file systems as well. > > For efi_disk(a), > 3. A block device can be enumelated dynamically by 'scanning' a device b= us > in U-Boot, but UEFI subsystem is not able to update efi_disks accord= ingly. > For examples, > =3D> scsi rescan; efidebug devices > =3D> usb start; efidebug devices ... (A) > (A) doesn't show any usb devices detected. > > =3D> scsi rescan; efidebug boot add -b 0 TEST scsi 0:1 ... > =3D> scsi rescan ... (B) > =3D> bootefi bootmgr ... (C) > (C) may de-reference a bogus blk_desc pointer which has been freed b= y (B). > (Please note that "scsi rescan" removes all udevices/blk_desc and th= en > re-create them even if nothing is changed on a bus.) > > For efi_disk(b), > 4. A "controller (handle)", combined with efi_block driver, has no > corresponding udevice as a parent of efi_disks in DM tree, unlike, > say, a scsi controller, even though it provides methods for block io > operations. > 5. There is no way supported to remove efi_disk's even after > disconnect_controller() is called. > > > My approach: > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > Due to functional differences in semantics, it would be difficult > to identify "udevice" structure as a handle in UEFI world. Instead, we w= ill > have to somehow maintain a relationship between a udevice and a handle. > > 1-1. add a dedicated uclass, UCLASS_PARTITION, for partitions > Currently, the uclass for paritions is not a UCLASS_BLK. > It can be possible to define partitions as UCLASS_BLK > (with IF_TYPE_PARTION?), but > I'm afraid that it may introduce some chaos since udevice(UCLASS_BLK= ) > is tightly coupled with 'struct blk_desc' data which is still used > as a "structure to a whole disk" in a lot of interfaces. > (I hope that you understand what it means.) > > In DM tree, a UCLASS_PARTITON instance has a UCLASS_BLK parent: > For instance, > UCLASS_SCSI --- UCLASS_BLK --- UCLASS_PARTITION > (IF_TYPE_SCSI) | > +- struct blk_desc +- struct disk_part > +- scsi_blk_ops +- blk_part_ops > > 1-2. create partition udevices in the context of device_probe() > part_init() is already called in blk_post_probe(). See the commit > d0851c893706 ("blk: Call part_init() in the post_probe() method"). > Why not enumelate partitions as well in there. > > 2. add new block access interfaces, which takes a *udevice* as a target > device, in U-Boot and use those functions to implement efi_disk > operations (i.e. EFI_BLOCK_IO_PROTOCOL). > > 3-1. maintain a bi-directional link between a udevice and an efi_disk > by adding > - a UEFI handle pointer as a tag for a udevice > - a udevice pointer in UEFI handle (in fact, in "struct efi_disk_obj= ") > > 3-2. synchronize the lifetime of efi_disk objects in UEFI world with > the driver model using > - event notification associated with device's probe/remove. > > 4. I have no solution to issue(4) and (5) yet. > > > <<>> > =3D> dm tree > Class Driver Name > -------------------------------------------- > root root_driver root_driver > ... > pci pci_generic_ecam |-- pcie@10000000 > pci_generi pci_generic_drv | |-- pci_0:0.0 > virtio virtio-pci.l | |-- virtio-pci.l#0 > ethernet virtio-net | | `-- virtio-net#32 > ahci ahci_pci | |-- ahci_pci > scsi ahci_scsi | | `-- ahci_scsi > blk scsi_blk | | |-- ahci_scsi.id0lun0 > partition blk_partition | | | |-- ahci_scsi.id0lun0:= 1 > partition blk_partition | | | `-- ahci_scsi.id0lun0:= 2 > blk scsi_blk | | `-- ahci_scsi.id1lun0 > partition blk_partition | | |-- ahci_scsi.id1lun0:= 1 > partition blk_partition | | `-- ahci_scsi.id1lun0:= 2 > usb xhci_pci | `-- xhci_pci > usb_hub usb_hub | `-- usb_hub > usb_dev_ge usb_dev_generic_drv | |-- generic_bus_0_dev_2 > usb_mass_s usb_mass_storage | `-- usb_mass_storage > blk usb_storage_blk | `-- usb_mass_storage.l= un0 > partition blk_partition | |-- usb_mass_stora= ge.lun0:1 > partition blk_partition | `-- usb_mass_stora= ge.lun0:2 > ... > =3D> efi devices > Device Device Path > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > 000000013eeea8d0 /VenHw() > 000000013eeed810 /VenHw()/MAC(525252525252,1) > 000000013eefc460 /VenHw()/Scsi(0,0) > 000000013eefc5a0 /VenHw()/Scsi(0,0)/HD(1,GPT,ce86c5a7-b32a-488f-a346-88f= e698e0edc,0x22,0x4c2a) > 000000013eefe320 /VenHw()/Scsi(0,0)/HD(2,GPT,aa80aab9-33e6-42b6-b5db-def= 2cb8d7844,0x5000,0x1a800) > 000000013eeff210 /VenHw()/Scsi(1,0) > 000000013eeff390 /VenHw()/Scsi(1,0)/HD(1,GPT,ce86c5a7-b32a-488f-a346-88f= e698e0edc,0x22,0x4c2a) > 000000013eeff7d0 /VenHw()/Scsi(1,0)/HD(2,GPT,aa80aab9-33e6-42b6-b5db-def= 2cb8d7844,0x5000,0x1a800) > 000000013ef04c20 /VenHw()/UsbClass(0x0,0x0,0x9,0x0,0x3)/UsbClass(0x46f4,= 0x1,0x0,0x0,0x0) > 000000013ef04da0 /VenHw()/UsbClass(0x0,0x0,0x9,0x0,0x3)/UsbClass(0x46f4,= 0x1,0x0,0x0,0x0)/HD(1,0x01,0,0x0,0x99800) > 000000013ef04f70 /VenHw()/UsbClass(0x0,0x0,0x9,0x0,0x3)/UsbClass(0x46f4,= 0x1,0x0,0x0,0x0)/HD(2,0x01,0,0x99800,0x1800) > > > Patchs: > =3D=3D=3D=3D=3D=3D=3D > For easy understandings, patches may be categorized into separate groups > of changes. > > Patch#1-#7: DM: add device_probe() for later use of events > Patch#8-#11: DM: add new features (tag and event notification) > Patch#12-#15: UEFI: dynamically create/remove efi_disk's for a raw disk > and its partitions > For removal case, we may need more consideration since removing handl= es > unconditionally may end up breaking integrity of handles > (as some may still be held and referenced to by a UEFI app). > Patch#16-#17: UEFI: use udevice read/write interfaces > Patch#18-#19: UEFI: fix-up efi_driver, aligning with changes in DM integ= ration > > > [1] https://lists.denx.de/pipermail/u-boot/2019-February/357923.html > [2] https://lists.denx.de/pipermail/u-boot/2021-June/452297.html > > > Change history: > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > v1 (Feb 2, 2022) > * rebased on 2022.04-rc1 > * drop patches that have already been merged > * modify a tag-range check with "tag >=3D DM_TAG_COUNT" (patch#9) > * move dmtag_list to GD (global data) (patch#9) > * add function descriptions and a document about DM tag feature (patch#9= ,10) > * add tests for DM tag support (patch#11) > * change 'depends on EVENT' to 'select EVENT' for EFI_LOADER (patch#14) > * migrate IF_TYPE_EFI to IF_TYPE_EFI_LOADER (patch#18) > > RFCv2 (Dec 10, 2021) > * rebased on 2022-rc3 > * re-order and merge some related commits into ones > * call device_probe() in MMC (not bind, but) probe hook (patch#5) > * fix a wrong name of variable (patch#7) > * add patch#9 > * invoke device_probe() for virtio devices (patch#10) > * add DM event notitication (from Simon) (patch#11) > * add DM tag support (patch#12) > * move UCLASS_PARTITION driver under disk/ (patch#13) > * create partition's dp using its parent's. This change is necessary > in particular for 'efi_blk' efi_disk (patch#13) > * modify the code so that we will use new features like tags and > event notification (patch#13,15,16,20) > * rename new functions from blk_read/write() to dev_read/write() > (patch#17,18) > * isolate changes in efi_driver from the rest (in efi_loader) (patch#19) > * drop the previous patch#22 ("efi_selftest: block device: adjust dp > for a test") due to the fix in patch#13 > > RFC (Nov 16, 2021) > * initial RFC > > AKASHI Takahiro (18): > scsi: call device_probe() after scanning > usb: storage: call device_probe() after scanning > mmc: call device_probe() after scanning > nvme: call device_probe() after scanning > sata: call device_probe() after scanning > block: ide: call device_probe() after scanning > virtio: call device_probe() in scanning > dm: add tag support > dm: tag: add some document > test: dm: add tests for tag support > dm: disk: add UCLASS_PARTITION > dm: blk: add a device-probe hook for scanning disk partitions > efi_loader: disk: a helper function to create efi_disk objects from > udevice > efi_loader: disk: a helper function to delete efi_disk objects > dm: disk: add read/write interfaces with udevice > efi_loader: disk: use udevice instead of blk_desc > efi_loader: disk: not create BLK device for BLK(IF_TYPE_EFI_LOADER) > devices > efi_driver: align with efi_disk-dm integration > > Simon Glass (1): > dm: add event notification > > cmd/virtio.c | 21 +- > common/Kconfig | 11 + > common/Makefile | 2 + > common/board_f.c | 2 + > common/event.c | 103 +++++++++ > common/log.c | 1 + > common/usb_storage.c | 4 + > disk/Makefile | 3 + > disk/disk-uclass.c | 247 +++++++++++++++++++++ > doc/develop/driver-model/design.rst | 20 ++ > drivers/ata/dwc_ahsata.c | 5 + > drivers/ata/fsl_sata.c | 11 + > drivers/ata/sata_mv.c | 5 + > drivers/ata/sata_sil.c | 12 + > drivers/block/blk-uclass.c | 4 + > drivers/block/ide.c | 4 + > drivers/core/Makefile | 2 +- > drivers/core/device-remove.c | 9 + > drivers/core/device.c | 9 + > drivers/core/root.c | 2 + > drivers/core/tag.c | 139 ++++++++++++ > drivers/mmc/mmc-uclass.c | 12 + > drivers/nvme/nvme.c | 4 + > drivers/scsi/scsi.c | 5 + > include/asm-generic/global_data.h | 4 + > include/dm/device-internal.h | 10 + > include/dm/tag.h | 110 ++++++++++ > include/dm/uclass-id.h | 1 + > include/efi_loader.h | 4 +- > include/event.h | 105 +++++++++ > include/event_internal.h | 34 +++ > include/log.h | 2 + > include/part.h | 18 ++ > lib/efi_driver/efi_block_device.c | 34 +-- > lib/efi_loader/Kconfig | 2 + > lib/efi_loader/efi_disk.c | 329 ++++++++++++++++++++-------- > lib/efi_loader/efi_setup.c | 11 +- > test/common/Makefile | 1 + > test/common/event.c | 87 ++++++++ > test/dm/Makefile | 1 + > test/dm/tag.c | 80 +++++++ > test/test-main.c | 5 + > 42 files changed, 1355 insertions(+), 120 deletions(-) > create mode 100644 common/event.c > create mode 100644 disk/disk-uclass.c > create mode 100644 drivers/core/tag.c > create mode 100644 include/dm/tag.h > create mode 100644 include/event.h > create mode 100644 include/event_internal.h > create mode 100644 test/common/event.c > create mode 100644 test/dm/tag.c This code does not even compile. https://source.denx.de/u-boot/custodians/u-boot-efi/-/jobs/387157 I will update the whole series to status "changes requested". Best regards Heinrich