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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 28F26C433EF for ; Mon, 11 Oct 2021 16:48:53 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 8E1606008E for ; Mon, 11 Oct 2021 16:48:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 8E1606008E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmx.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 9510282DF9; Mon, 11 Oct 2021 18:48:50 +0200 (CEST) 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="hzEA/3fZ"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 9CB1582FC5; Mon, 11 Oct 2021 18:48:48 +0200 (CEST) Received: from mout.gmx.net (mout.gmx.net [212.227.15.19]) (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 86AA7805F9 for ; Mon, 11 Oct 2021 18:48:44 +0200 (CEST) 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=1633970923; bh=exf6WeNT7i0WaAnNuwOOQbHAeH4bGPKLVqulFMIRp6E=; h=X-UI-Sender-Class:Date:Subject:To:Cc:References:From:In-Reply-To; b=hzEA/3fZWpj5d1SMtQAS2zYVdeL/BQhJcoQf3uVEi+owGZVjy6TlPrVLDaLg9xp2v /8IVTRokcBK4JpGS/xfQMru2PPLPiiqyEuiglu9huF+EhdVUzZBzBDDRxfUBX0Hase CpvgDYowdct3t4+PeENYzPLuQUMk0OUsTQSRjCew= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [192.168.123.55] ([88.152.144.157]) by mail.gmx.net (mrgmx004 [212.227.17.190]) with ESMTPSA (Nemesis) id 1MPGW7-1mO5xu0PKX-00PdpC; Mon, 11 Oct 2021 18:48:43 +0200 Message-ID: <9e772e73-c5a3-14dd-657a-a3684c5af492@gmx.de> Date: Mon, 11 Oct 2021 18:48:42 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.2 Subject: Re: [RFC 07/22] dm: blk: add UCLASS_PARTITION Content-Language: en-US To: Simon Glass Cc: AKASHI Takahiro , U-Boot Mailing List , Alex Graf , Ilias Apalodimas References: <20211001050228.55183-1-takahiro.akashi@linaro.org> <20211001050228.55183-15-takahiro.akashi@linaro.org> <20211004032759.GC39720@laputa> <20211008005156.GA34717@laputa> <3455705e-1b04-cf01-3cff-b89f673c5e05@gmx.de> <20211011022925.GE44356@laputa> <64774cd7-2119-347a-4d2b-3715fa2a8029@gmx.de> From: Heinrich Schuchardt In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:/wadwaBKEu6VYgZWfcEnNfK9jEj/0o8MK/SguPDnJAp/hwbu9P0 fYD7klYz2YhunwLWshjeJoqj+RrNecgCGgROVBeH6hVjYGbOnS0pj6Pr5ykbUqNRIYwFFEm OpnyuzyvztiDzLmv3W6OyeorxQ4X+0/8Ve4JdGT1tnfR2W8G5QfPMZfGmFcV630m68ZLC+f BQYYjq7VGfF0BRuSywYZA== X-UI-Out-Filterresults: notjunk:1;V03:K0:Ut4e2xepdoU=:N3mwGpkA8QMBFsJAFSaDTC bAl8P3fWmnQ1ut4OXygDEVeh+zCPY7HURjLhkizCJpM+U9ZEI9XM7vIsnfU3nIA1NnoA1PDmQ RdM2LOuRcZ0EbAAt4ZhBzY15fELL+FICeX1DbTMrcNKUIVXRyv97Y0EN9EAEb60MTn7jkaY60 Ji9QSIlGnaz+EJDQwtp5frJLgB+QhSU1T+WBhAOxjCq039/P8+CkWBQR22QRtH+GVkP+Q+MNZ twqnAeH3HQvIKIbxm1GQ2ZD8b8sn5nX/91427Yy4jS8Bz22jdkg+OD3ZGxWMvf7MQjERf5i8k Y16FFw/0H2y2wCdUu5bndwjIE9s77PtNNpeweXEtCcG3DQgEbbXQ/VcQw0HHbOqVe0vcE6KZU xCw1Q/qBX9qSoawlQNFijrMIuYCiteV0qhfKpzVqq/J7oscOqhF8uAbnNGxKnGce7174QWrdm ACkmcWkJzy+tn9BGnzSffmPSxkSnf5i7swAUjd4lj8AWeHm7uB81eSe5Oxc7d6J6eELeSQxXt ls8UBgUCqkY0NhBjgqi4lqX9RquSkIoQmDXBf/G/6e1ZNbIfAn+MWjxlh8fQP+Z/itR9yMHZ5 3lSXnRDJQBlt6gBGgSHhtNCVEWu0do9cFbXey67MxJdQ6Vzqnr2CpmlVeP7fzJl9fk7Pgla0r AUnca4N4kal5+8NxtNmRmBonLO5ildbUvY2ujqu+eEDnDNljIcavGV+XOO3jg0L1DZ5+jD+DZ I3RnEEHhYVSm+GxlaOhUECJ9O/KGBiOyIoGrSIewNjgI4DagGWnUUjXhdWJvAhzg5cybEJjpk 6HdKlcOM4p7H/kndYrZrOp0afqcWhqCwwwDKLvmadnku+SFJ/gckvUq+fYlSie5PnnWG8BPmx tH/yDn8iJNZWiG5o0MJFjov4LNljYToRt4TjgE6O0jsiPK5ufsMQrcI4LoDGj5TDoTYUeuXFK anqa1SNH2AVccos/kMhsWt+GHT2qDgFMAtjZ0eqbTEqOglVrVsDu7Ao4VzvWF3DN6RpivToZg /bWpR8O3mCE8AVwmAM/K68ngjOBPpJ7zpyZ314Gcev5byXrOYSeIIdPCcoCss5SUbonhA/WD5 ohJlAIE5dRiWa0= X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 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.2 at phobos.denx.de X-Virus-Status: Clean On 10/11/21 18:14, Simon Glass wrote: > Hi Heinrich, > > On Mon, 11 Oct 2021 at 09:02, Heinrich Schuchardt w= rote: >> >> >> >> On 10/11/21 16:54, Simon Glass wrote: >>> Hi Takahiro, >>> >>> On Sun, 10 Oct 2021 at 20:29, AKASHI Takahiro >>> wrote: >>>> >>>> Heinrich, >>>> >>>> On Fri, Oct 08, 2021 at 10:23:52AM +0200, Heinrich Schuchardt wrote: >>>>> >>>>> >>>>> On 10/8/21 02:51, AKASHI Takahiro wrote: >>>>>> On Mon, Oct 04, 2021 at 12:27:59PM +0900, AKASHI Takahiro wrote: >>>>>>> On Fri, Oct 01, 2021 at 11:30:37AM +0200, Heinrich Schuchardt wrot= e: >>>>>>>> >>>>>>>> >>>>>>>> On 10/1/21 07:01, AKASHI Takahiro wrote: >>>>>>>>> UCLASS_PARTITION device will be created as a child node of >>>>>>>>> UCLASS_BLK device. >>>>>>>>> >>>>>>>>> Signed-off-by: AKASHI Takahiro >>>>>>>>> --- >>>>>>>>> drivers/block/blk-uclass.c | 111 ++++++++++++++++++++++++++= +++++++++++ >>>>>>>>> include/blk.h | 9 +++ >>>>>>>>> include/dm/uclass-id.h | 1 + >>>>>>>>> 3 files changed, 121 insertions(+) >>>>>>>>> >>>>>>>>> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-ucla= ss.c >>>>>>>>> index 83682dcc181a..dd7f3c0fe31e 100644 >>>>>>>>> --- a/drivers/block/blk-uclass.c >>>>>>>>> +++ b/drivers/block/blk-uclass.c >>>>>>>>> @@ -12,6 +12,7 @@ >>>>>>>>> #include >>>>>>>>> #include >>>>>>>>> #include >>>>>>>>> +#include >>>>>>>>> #include >>>>>>>>> #include >>>>>>>>> #include >>>>>>>>> @@ -695,6 +696,44 @@ int blk_unbind_all(int if_type) >>>>>>>>> return 0; >>>>>>>>> } >>>>>>>>> >>>>>>>>> +int blk_create_partitions(struct udevice *parent) >>>>>>>>> +{ >>>>>>>>> + int part, count; >>>>>>>>> + struct blk_desc *desc =3D dev_get_uclass_plat(parent); >>>>>>>>> + struct disk_partition info; >>>>>>>>> + struct disk_part *part_data; >>>>>>>>> + char devname[32]; >>>>>>>>> + struct udevice *dev; >>>>>>>>> + int ret; >>>>>>>>> + >>>>>>>>> + if (!CONFIG_IS_ENABLED(PARTITIONS) || >>>>>>>>> + !CONFIG_IS_ENABLED(HAVE_BLOCK_DEVICE)) >>>>>>>>> + return 0; >>>>>>>>> + >>>>>>>>> + /* Add devices for each partition */ >>>>>>>>> + for (count =3D 0, part =3D 1; part <=3D MAX_SEARCH_PARTITI= ONS; part++) { >>>>>>>>> + if (part_get_info(desc, part, &info)) >>>>>>>>> + continue; >>>>>>>>> + snprintf(devname, sizeof(devname), "%s:%d", parent= ->name, >>>>>>>>> + part); >>>>>>>>> + >>>>>>>>> + ret =3D device_bind_driver(parent, "blk_partition"= , >>>>>>>>> + strdup(devname), &dev); >>>>>>>>> + if (ret) >>>>>>>>> + return ret; >>>>>>>>> + >>>>>>>>> + part_data =3D dev_get_uclass_plat(dev); >>>>>>>>> + part_data->partnum =3D part; >>>>>>>>> + part_data->gpt_part_info =3D info; >>>>>>>>> + count++; >>>>>>>>> + >>>>>>>>> + device_probe(dev); >>>>>>>>> + } >>>>>>>>> + debug("%s: %d partitions found in %s\n", __func__, count, = parent->name); >>>>>>>>> + >>>>>>>>> + return 0; >>>>>>>>> +} >>>>>>>>> + >>>>>>>>> static int blk_post_probe(struct udevice *dev) >>>>>>>>> { >>>>>>>>> if (IS_ENABLED(CONFIG_PARTITIONS) && >>>>>>>>> @@ -713,3 +752,75 @@ UCLASS_DRIVER(blk) =3D { >>>>>>>>> .post_probe =3D blk_post_probe, >>>>>>>>> .per_device_plat_auto =3D sizeof(struct blk_desc), >>>>>>>>> }; >>>>>>>>> + >>>>>>>>> +static ulong blk_part_read(struct udevice *dev, lbaint_t start, >>>>>>>>> + lbaint_t blkcnt, void *buffer) >>>>>>>>> +{ >>>>>>>>> + struct udevice *parent; >>>>>>>>> + struct disk_part *part; >>>>>>>>> + const struct blk_ops *ops; >>>>>>>>> + >>>>>>>>> + parent =3D dev_get_parent(dev); >>>>>>>> >>>>>>>> What device type will the parent have if it is a eMMC hardware pa= rtition? >>>>>>>> >>>>>>>>> + ops =3D blk_get_ops(parent); >>>>>>>>> + if (!ops->read) >>>>>>>>> + return -ENOSYS; >>>>>>>>> + >>>>>>>>> + part =3D dev_get_uclass_plat(dev); >>>>>>>> >>>>>>>> You should check that we do not access the block device past the >>>>>>>> partition end: >>>>>>> >>>>>>> Yes, I will fix all of checks. >>>>>>> >>>>>>>> struct blk_desc *desc =3D dev_get_uclass_plat(parent); >>>>>>>> if ((start + blkcnt) * desc->blksz < part->gpt_part_info.blksz) >>>>>>>> return -EFAULT. >>>>>>>> >>>>>>>>> + start +=3D part->gpt_part_info.start; >>>>>> >>>>>> A better solution is: >>>>>> if (start >=3D part->gpt_part_info.size) >>>>>> return 0; >>>>>> >>>>>> if ((start + blkcnt) > part->gpt_part_info.size) >>>>>> blkcnt =3D part->gpt_part_info.size - start; >>>>>> start +=3D part->gpt_part_info.start; >>>>>> instead of returning -EFAULT. >>>>>> (note that start and blkcnt are in "block".) >>>>> >>>>> What is your motivation to support an illegal access? >>>>> >>>>> We will implement the EFI_BLOCK_IO_PROTOCOL based on this function. = The >>>>> ReadBlocks() and WriteBlocks() services must return >>>>> EFI_INVALID_PARAMETER if the read request contains LBAs that are not >>>>> valid. >>>> >>>> I interpreted that 'LBA' was the third parameter to ReadBlocks API, >>>> and that if the starting block is out of partition region, we should >>>> return an error (and if not, we still want to trim IO request to fit >>>> into partition size as other OS's API like linux does). >>>> Do you think it's incorrect? >>> >>> [..] >>> >>> Related to this patch I think that the partition type should be really >>> be a child of the media device: >>> >>> - MMC >>> |- BLK >>> |- PARTITION >>> |- BLK >>> |- PARTITION >>> |- BLK >>> |- PARTITION >>> |- BLK >>> >>> It seems more natural to me that putting the partitions under the >>> top-level BLK device, so that BLK remains a 'terminal' device. >>> >>> The partition uclass is different from BLK, of course. It could >>> contain information about the partition such as its partition number >>> and UUID. >> >> Do you mean hardware partition here? Otherwise I would not know what BL= K >> should model. > > I mean that (I think) we should not use BLK to model partitions. A BLK > should just be a block device. That is fine. But this implies that a software partition is the child of a block partition and not the other way round. So the tree should like: MMC |- BLK (user hardware partition) ||- PARTITION 1 (software partition) ||- PARTITION 2 (software partition) |... ||- PARTITION n (software partition) |- BLK (rpmb hardware partition) |- BLK (boot0 hardware partition) |- BLK (boot1 hardware partition) > > I don't see any difference between a partition and a hardware > partition. We presumably end up with a hierarchy though. Do we need a > HWPARTITION uclass so we can handle the hardware partitions > differently? Software partitions are defined and discovered via partition tables. Hardware partitions are defined in a hardware specific way. All software partitions map to HD() device tree nodes in UEFI. An MMC device maps to an eMMC() node MMC hardware partitions are mapped to Ctrl() nodes by EDK II. We should do the same in U-Boot. An SD-card maps to an SD() node. An NVMe namespace maps to a NVMe() node. An SCSI LUN maps to a Scsi() node. SCSI channels of multiple channel controllers are mapped to Ctrl() nodes. The simple file protocol is only provided by HD() nodes and not by nodes representing hardware partitions. If the whole hardware partition is formatted as a file system you would still create a HD() node with partition number 0. Best regards Heinrich