Message ID | 20211001050228.55183-2-takahiro.akashi@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | efi_loader: more tightly integrate UEFI disks to device model | expand |
On 10/1/21 7:01 AM, AKASHI Takahiro wrote: > In blk_get_device_by_str(), the comment says: "Updates the partition table > for the specified hw partition." > Since hw partition is supported only on MMC, it makes no sense to do so > for other devices. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > disk/part.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/disk/part.c b/disk/part.c > index a6a8f7052bd3..b330103a5bc0 100644 > --- a/disk/part.c > +++ b/disk/part.c > @@ -427,7 +427,8 @@ int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str, > * Always should be done, otherwise hw partition 0 will return stale > * data after displaying a non-zero hw partition. > */ > - part_init(*dev_desc); > + if ((*dev_desc)->if_type == IF_TYPE_MMC) > + part_init(*dev_desc); For an eMMC the following logical levels exist: * device * hardware partition * software partition Linux might show the following: /dev/mmcblk0 - user data area /dev/mmcblk0boot0 - boot hardware partition 0 /dev/mmcblk0boot1 - boot hardware partition 1 /dev/mmcblk0rpmb - replay protected memory block How are the different hardware partition modeled in the UEFI device path? Should each hardware partition be a separate udevice? For NOR flash we also have an extra level: => setenv mtdparts mtdparts=30bb0000.qspi:1m(U-Boot),512k(Env),512k(DTB),2m(User_FS),12m(Data_FS),4m(Factory_FS),34m(Ramdisk),10m(Linux) => mtd device nor0 <30bb0000.qspi>, # parts = 8 #: name size offset mask_flags 0: U-Boot 0x00100000 0x00000000 0 1: Env 0x00080000 0x00100000 0 2: DTB 0x00080000 0x00180000 0 3: User_FS 0x00200000 0x00200000 0 4: Data_FS 0x00c00000 0x00400000 0 5: Factory_FS 0x00400000 0x01000000 0 6: Ramdisk 0x02200000 0x01400000 0 7: Linux 0x00a00000 0x03600000 0 active partition: nor0,0 - (U-Boot) 0x00100000 @ 0x00000000 Has part_info() to be called here too? What is the if_type? What are the devicepaths for these partitions? Best regards Heinrich > #endif > > cleanup: >
On 10/1/21 08:41, Heinrich Schuchardt wrote: > > > On 10/1/21 7:01 AM, AKASHI Takahiro wrote: >> In blk_get_device_by_str(), the comment says: "Updates the partition >> table >> for the specified hw partition." >> Since hw partition is supported only on MMC, it makes no sense to do so >> for other devices. >> >> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >> --- >> disk/part.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/disk/part.c b/disk/part.c >> index a6a8f7052bd3..b330103a5bc0 100644 >> --- a/disk/part.c >> +++ b/disk/part.c >> @@ -427,7 +427,8 @@ int blk_get_device_by_str(const char *ifname, >> const char *dev_hwpart_str, >> * Always should be done, otherwise hw partition 0 will return >> stale >> * data after displaying a non-zero hw partition. >> */ >> - part_init(*dev_desc); >> + if ((*dev_desc)->if_type == IF_TYPE_MMC) >> + part_init(*dev_desc); > > For an eMMC the following logical levels exist: > > * device > * hardware partition > * software partition > > Linux might show the following: > > /dev/mmcblk0 - user data area > /dev/mmcblk0boot0 - boot hardware partition 0 > /dev/mmcblk0boot1 - boot hardware partition 1 > /dev/mmcblk0rpmb - replay protected memory block > > How are the different hardware partition modeled in the UEFI device path? This is a list of block devices with their Linux name and the device path in EDK II as seen on my MacchicatoBIN board: /dev/mmcblk0 VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,00006EF00000000000)/eMMC(0x0)/Ctrl(0x0) /dev/mmcblk0p1 VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,00006EF00000000000)/eMMC(0x0)/Ctrl(0x0)/HD(1,GPT,C53259E6-9B1D-40CD-B4EF-39520011EF2B,0x2000,0x3FE001) /dev/mmcblk0p2 VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,00006EF00000000000)/eMMC(0x0)/Ctrl(0x0)/HD(2,GPT,A0AE9901-B847-4150-B947-6C8FF69529A7,0x400800,0xA8F7DF) /dev/mmcblk0boot0 VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,00006EF00000000000)/eMMC(0x0)/Ctrl(0x1) /dev/mmcblk0boot1 VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,00006EF00000000000)/eMMC(0x0)/Ctrl(0x2) > Should each hardware partition be a separate udevice? We need a udevice matching the hardware partition. These child devices should be created when the eMMC device is probed. Best regards Heinrich > > For NOR flash we also have an extra level: > > => setenv mtdparts > mtdparts=30bb0000.qspi:1m(U-Boot),512k(Env),512k(DTB),2m(User_FS),12m(Data_FS),4m(Factory_FS),34m(Ramdisk),10m(Linux) > > => mtd > > device nor0 <30bb0000.qspi>, # parts = 8 > #: name size offset mask_flags > 0: U-Boot 0x00100000 0x00000000 0 > 1: Env 0x00080000 0x00100000 0 > 2: DTB 0x00080000 0x00180000 0 > 3: User_FS 0x00200000 0x00200000 0 > 4: Data_FS 0x00c00000 0x00400000 0 > 5: Factory_FS 0x00400000 0x01000000 0 > 6: Ramdisk 0x02200000 0x01400000 0 > 7: Linux 0x00a00000 0x03600000 0 > > active partition: nor0,0 - (U-Boot) 0x00100000 @ 0x00000000 > > Has part_info() to be called here too? What is the if_type? > What are the devicepaths for these partitions? > > Best regards > > Heinrich > >> #endif >> >> cleanup: >>
On Fri, Oct 1, 2021 at 6:03 AM AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > In blk_get_device_by_str(), the comment says: "Updates the partition table > for the specified hw partition." > Since hw partition is supported only on MMC, it makes no sense to do so > for other devices. Is it not also supported on UFS, and I believe it may also be an option in the NVME spec too. > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > disk/part.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/disk/part.c b/disk/part.c > index a6a8f7052bd3..b330103a5bc0 100644 > --- a/disk/part.c > +++ b/disk/part.c > @@ -427,7 +427,8 @@ int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str, > * Always should be done, otherwise hw partition 0 will return stale > * data after displaying a non-zero hw partition. > */ > - part_init(*dev_desc); > + if ((*dev_desc)->if_type == IF_TYPE_MMC) > + part_init(*dev_desc); > #endif > > cleanup: > -- > 2.33.0 >
Heinrich, On Fri, Oct 01, 2021 at 08:41:52AM +0200, Heinrich Schuchardt wrote: > > > On 10/1/21 7:01 AM, AKASHI Takahiro wrote: > > In blk_get_device_by_str(), the comment says: "Updates the partition table > > for the specified hw partition." > > Since hw partition is supported only on MMC, it makes no sense to do so > > for other devices. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > disk/part.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/disk/part.c b/disk/part.c > > index a6a8f7052bd3..b330103a5bc0 100644 > > --- a/disk/part.c > > +++ b/disk/part.c > > @@ -427,7 +427,8 @@ int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str, > > * Always should be done, otherwise hw partition 0 will return stale > > * data after displaying a non-zero hw partition. > > */ > > - part_init(*dev_desc); > > + if ((*dev_desc)->if_type == IF_TYPE_MMC) > > + part_init(*dev_desc); > > For an eMMC the following logical levels exist: > > * device > * hardware partition > * software partition > > Linux might show the following: > > /dev/mmcblk0 - user data area > /dev/mmcblk0boot0 - boot hardware partition 0 > /dev/mmcblk0boot1 - boot hardware partition 1 > /dev/mmcblk0rpmb - replay protected memory block > > How are the different hardware partition modeled in the UEFI device path? > Should each hardware partition be a separate udevice? > > For NOR flash we also have an extra level: > > => setenv mtdparts > mtdparts=30bb0000.qspi:1m(U-Boot),512k(Env),512k(DTB),2m(User_FS),12m(Data_FS),4m(Factory_FS),34m(Ramdisk),10m(Linux) > => mtd > > device nor0 <30bb0000.qspi>, # parts = 8 > #: name size offset mask_flags > 0: U-Boot 0x00100000 0x00000000 0 > 1: Env 0x00080000 0x00100000 0 > 2: DTB 0x00080000 0x00180000 0 > 3: User_FS 0x00200000 0x00200000 0 > 4: Data_FS 0x00c00000 0x00400000 0 > 5: Factory_FS 0x00400000 0x01000000 0 > 6: Ramdisk 0x02200000 0x01400000 0 > 7: Linux 0x00a00000 0x03600000 0 > > active partition: nor0,0 - (U-Boot) 0x00100000 @ 0x00000000 > > Has part_info() to be called here too? What is the if_type? > What are the devicepaths for these partitions? You have good points. MMC's hw partitions and mtd's "environment variable-based" partitions are quite different from the rest of table-based software partitions in terms of U-Boot block device implementation. Both neither are handled by part_info() (under disk/* framework) nor have dedicated "if_type's". Even if we might have to address those issues at the end, I would like to keep them out of my scope for now as it requires a lot of extra work. # I wonder if we should support mtdparts partitions on U-Boot UEFI # as it is a quite U-Boot specific feature. -Takahiro Akashi > Best regards > > Heinrich > > > #endif > > > > cleanup: > >
Hi Peter, On Fri, Oct 01, 2021 at 12:48:24PM +0100, Peter Robinson wrote: > On Fri, Oct 1, 2021 at 6:03 AM AKASHI Takahiro > <takahiro.akashi@linaro.org> wrote: > > > > In blk_get_device_by_str(), the comment says: "Updates the partition table > > for the specified hw partition." > > Since hw partition is supported only on MMC, it makes no sense to do so > > for other devices. > > Is it not also supported on UFS, and I believe it may also be an > option in the NVME spec too. Yeah, maybe. But under the current implementation, IIUC, neither UFS nor NVME supports hw partitions as both drivers do not provide a "select_hwpart" function in blk_ops. (UFS is seen as an instance of SCSI.) -Takahiro Akashi > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > disk/part.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/disk/part.c b/disk/part.c > > index a6a8f7052bd3..b330103a5bc0 100644 > > --- a/disk/part.c > > +++ b/disk/part.c > > @@ -427,7 +427,8 @@ int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str, > > * Always should be done, otherwise hw partition 0 will return stale > > * data after displaying a non-zero hw partition. > > */ > > - part_init(*dev_desc); > > + if ((*dev_desc)->if_type == IF_TYPE_MMC) > > + part_init(*dev_desc); > > #endif > > > > cleanup: > > -- > > 2.33.0 > >
On Thu, 30 Sept 2021 at 23:02, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > In blk_get_device_by_str(), the comment says: "Updates the partition table > for the specified hw partition." > Since hw partition is supported only on MMC, it makes no sense to do so > for other devices. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > disk/part.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass <sjg@chromium.org>
On 10/1/21 13:48, Peter Robinson wrote: > On Fri, Oct 1, 2021 at 6:03 AM AKASHI Takahiro > <takahiro.akashi@linaro.org> wrote: >> >> In blk_get_device_by_str(), the comment says: "Updates the partition table >> for the specified hw partition." >> Since hw partition is supported only on MMC, it makes no sense to do so >> for other devices. > > Is it not also supported on UFS, and I believe it may also be an > option in the NVME spec too. An NVMe device may expose multiple namespaces. blk_create_devicef() is called for each namespace. A SCSI device may have multiple LUNs. blk_create_devicef() is called for each LUN. This is what the tree shown by 'dm tree' with on NVMe namespace and one LUN. Class Index Driver Name --------------------------------------------------------------------- root 0 root_driver root_driver simple_bus 0 simple_bus |- soc spi 1 sifive_spi | |- spi@10050000 mmc 0 mmc_spi | | `- mmc@0 blk 0 mmc_blk | | `- mmc@0.blk pci 0 pcie_sifive | |- pcie@e00000000 pci 1 pci_bridge_drv | | `- pci_0:0.0 pci 2 pci_bridge_drv | | `- pci_1:0.0 pci 5 pci_bridge_drv | | |- pci_2:3.0 ahci 0 ahci_pci | | | `- ahci_pci scsi 0 ahci_scsi | | | `- ahci_scsi blk 2 scsi_blk | | | `- ahci_scsi.id0lun0 pci 6 pci_bridge_drv | | |- pci_2:4.0 nvme 0 nvme | | | `- nvme#0 blk 1 nvme-blk | | | `- nvme#0.blk#1 Namespaces and LUNs are modeled as block devices (class = 'blk'). Best regards Heinrich > >> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >> --- >> disk/part.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/disk/part.c b/disk/part.c >> index a6a8f7052bd3..b330103a5bc0 100644 >> --- a/disk/part.c >> +++ b/disk/part.c >> @@ -427,7 +427,8 @@ int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str, >> * Always should be done, otherwise hw partition 0 will return stale >> * data after displaying a non-zero hw partition. >> */ >> - part_init(*dev_desc); >> + if ((*dev_desc)->if_type == IF_TYPE_MMC) >> + part_init(*dev_desc); >> #endif >> >> cleanup: >> -- >> 2.33.0 >>
Hi Heinrich, On Mon, 11 Oct 2021 at 04:07, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > On 10/1/21 13:48, Peter Robinson wrote: > > On Fri, Oct 1, 2021 at 6:03 AM AKASHI Takahiro > > <takahiro.akashi@linaro.org> wrote: > >> > >> In blk_get_device_by_str(), the comment says: "Updates the partition table > >> for the specified hw partition." > >> Since hw partition is supported only on MMC, it makes no sense to do so > >> for other devices. > > > > Is it not also supported on UFS, and I believe it may also be an > > option in the NVME spec too. > > An NVMe device may expose multiple namespaces. blk_create_devicef() is > called for each namespace. > > A SCSI device may have multiple LUNs. blk_create_devicef() is called for > each LUN. > > This is what the tree shown by 'dm tree' with on NVMe namespace and one LUN. > > Class Index Driver Name > --------------------------------------------------------------------- > root 0 root_driver root_driver > simple_bus 0 simple_bus |- soc > spi 1 sifive_spi | |- spi@10050000 > mmc 0 mmc_spi | | `- mmc@0 > blk 0 mmc_blk | | `- mmc@0.blk > pci 0 pcie_sifive | |- pcie@e00000000 > pci 1 pci_bridge_drv | | `- pci_0:0.0 > pci 2 pci_bridge_drv | | `- pci_1:0.0 > pci 5 pci_bridge_drv | | |- pci_2:3.0 > ahci 0 ahci_pci | | | `- ahci_pci > scsi 0 ahci_scsi | | | `- ahci_scsi > blk 2 scsi_blk | | | `- ahci_scsi.id0lun0 > pci 6 pci_bridge_drv | | |- pci_2:4.0 > nvme 0 nvme | | | `- nvme#0 > blk 1 nvme-blk | | | `- nvme#0.blk#1 > > Namespaces and LUNs are modeled as block devices (class = 'blk'). So multiple block devices per NVMe device? I did not know that was supported. We need a sandbox driver for NVMe as it has no tests at present. Since it has no tests, I don't think we can expect people to know how to maintain whatever functionality is there. [..] Regards, Simon
On 10/11/21 16:32, Simon Glass wrote: > Hi Heinrich, > > On Mon, 11 Oct 2021 at 04:07, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> >> >> >> On 10/1/21 13:48, Peter Robinson wrote: >>> On Fri, Oct 1, 2021 at 6:03 AM AKASHI Takahiro >>> <takahiro.akashi@linaro.org> wrote: >>>> >>>> In blk_get_device_by_str(), the comment says: "Updates the partition table >>>> for the specified hw partition." >>>> Since hw partition is supported only on MMC, it makes no sense to do so >>>> for other devices. >>> >>> Is it not also supported on UFS, and I believe it may also be an >>> option in the NVME spec too. >> >> An NVMe device may expose multiple namespaces. blk_create_devicef() is >> called for each namespace. >> >> A SCSI device may have multiple LUNs. blk_create_devicef() is called for >> each LUN. >> >> This is what the tree shown by 'dm tree' with on NVMe namespace and one LUN. >> >> Class Index Driver Name >> --------------------------------------------------------------------- >> root 0 root_driver root_driver >> simple_bus 0 simple_bus |- soc >> spi 1 sifive_spi | |- spi@10050000 >> mmc 0 mmc_spi | | `- mmc@0 >> blk 0 mmc_blk | | `- mmc@0.blk >> pci 0 pcie_sifive | |- pcie@e00000000 >> pci 1 pci_bridge_drv | | `- pci_0:0.0 >> pci 2 pci_bridge_drv | | `- pci_1:0.0 >> pci 5 pci_bridge_drv | | |- pci_2:3.0 >> ahci 0 ahci_pci | | | `- ahci_pci >> scsi 0 ahci_scsi | | | `- ahci_scsi >> blk 2 scsi_blk | | | `- ahci_scsi.id0lun0 >> pci 6 pci_bridge_drv | | |- pci_2:4.0 >> nvme 0 nvme | | | `- nvme#0 >> blk 1 nvme-blk | | | `- nvme#0.blk#1 >> >> Namespaces and LUNs are modeled as block devices (class = 'blk'). > > So multiple block devices per NVMe device? I did not know that was supported. > > We need a sandbox driver for NVMe as it has no tests at present. Since > it has no tests, I don't think we can expect people to know how to > maintain whatever functionality is there. NVMe drives with multiple namespaces exist for servers but not for consumer NVMe drives. In QEMU you can define an NVMe device with multiple namespaces. Cf. https://qemu.readthedocs.io/en/latest/system/devices/nvme.html?highlight=namespace#additional-namespaces So for a first glimpse at the handling I suggest to use QEMU. Best regards Heinrich
Hi Heinrich, On Mon, 11 Oct 2021 at 09:09, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > On 10/11/21 16:32, Simon Glass wrote: > > Hi Heinrich, > > > > On Mon, 11 Oct 2021 at 04:07, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > >> > >> > >> > >> On 10/1/21 13:48, Peter Robinson wrote: > >>> On Fri, Oct 1, 2021 at 6:03 AM AKASHI Takahiro > >>> <takahiro.akashi@linaro.org> wrote: > >>>> > >>>> In blk_get_device_by_str(), the comment says: "Updates the partition table > >>>> for the specified hw partition." > >>>> Since hw partition is supported only on MMC, it makes no sense to do so > >>>> for other devices. > >>> > >>> Is it not also supported on UFS, and I believe it may also be an > >>> option in the NVME spec too. > >> > >> An NVMe device may expose multiple namespaces. blk_create_devicef() is > >> called for each namespace. > >> > >> A SCSI device may have multiple LUNs. blk_create_devicef() is called for > >> each LUN. > >> > >> This is what the tree shown by 'dm tree' with on NVMe namespace and one LUN. > >> > >> Class Index Driver Name > >> --------------------------------------------------------------------- > >> root 0 root_driver root_driver > >> simple_bus 0 simple_bus |- soc > >> spi 1 sifive_spi | |- spi@10050000 > >> mmc 0 mmc_spi | | `- mmc@0 > >> blk 0 mmc_blk | | `- mmc@0.blk > >> pci 0 pcie_sifive | |- pcie@e00000000 > >> pci 1 pci_bridge_drv | | `- pci_0:0.0 > >> pci 2 pci_bridge_drv | | `- pci_1:0.0 > >> pci 5 pci_bridge_drv | | |- pci_2:3.0 > >> ahci 0 ahci_pci | | | `- ahci_pci > >> scsi 0 ahci_scsi | | | `- ahci_scsi > >> blk 2 scsi_blk | | | `- ahci_scsi.id0lun0 > >> pci 6 pci_bridge_drv | | |- pci_2:4.0 > >> nvme 0 nvme | | | `- nvme#0 > >> blk 1 nvme-blk | | | `- nvme#0.blk#1 > >> > >> Namespaces and LUNs are modeled as block devices (class = 'blk'). > > > > So multiple block devices per NVMe device? I did not know that was supported. > > > > We need a sandbox driver for NVMe as it has no tests at present. Since > > it has no tests, I don't think we can expect people to know how to > > maintain whatever functionality is there. > > NVMe drives with multiple namespaces exist for servers but not for > consumer NVMe drives. > > In QEMU you can define an NVMe device with multiple namespaces. Cf. > https://qemu.readthedocs.io/en/latest/system/devices/nvme.html?highlight=namespace#additional-namespaces > > So for a first glimpse at the handling I suggest to use QEMU. Well that's fine, but every uclass must have a test and a sandbox emulator as well. Regards, Simon
Simon, Heinrich, On Mon, Oct 11, 2021 at 10:14:02AM -0600, Simon Glass wrote: > Hi Heinrich, > > On Mon, 11 Oct 2021 at 09:09, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > > > On 10/11/21 16:32, Simon Glass wrote: > > > Hi Heinrich, > > > > > > On Mon, 11 Oct 2021 at 04:07, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > >> > > >> > > >> > > >> On 10/1/21 13:48, Peter Robinson wrote: > > >>> On Fri, Oct 1, 2021 at 6:03 AM AKASHI Takahiro > > >>> <takahiro.akashi@linaro.org> wrote: > > >>>> > > >>>> In blk_get_device_by_str(), the comment says: "Updates the partition table > > >>>> for the specified hw partition." > > >>>> Since hw partition is supported only on MMC, it makes no sense to do so > > >>>> for other devices. > > >>> > > >>> Is it not also supported on UFS, and I believe it may also be an > > >>> option in the NVME spec too. > > >> > > >> An NVMe device may expose multiple namespaces. blk_create_devicef() is > > >> called for each namespace. > > >> > > >> A SCSI device may have multiple LUNs. blk_create_devicef() is called for > > >> each LUN. > > >> > > >> This is what the tree shown by 'dm tree' with on NVMe namespace and one LUN. > > >> > > >> Class Index Driver Name > > >> --------------------------------------------------------------------- > > >> root 0 root_driver root_driver > > >> simple_bus 0 simple_bus |- soc > > >> spi 1 sifive_spi | |- spi@10050000 > > >> mmc 0 mmc_spi | | `- mmc@0 > > >> blk 0 mmc_blk | | `- mmc@0.blk > > >> pci 0 pcie_sifive | |- pcie@e00000000 > > >> pci 1 pci_bridge_drv | | `- pci_0:0.0 > > >> pci 2 pci_bridge_drv | | `- pci_1:0.0 > > >> pci 5 pci_bridge_drv | | |- pci_2:3.0 > > >> ahci 0 ahci_pci | | | `- ahci_pci > > >> scsi 0 ahci_scsi | | | `- ahci_scsi > > >> blk 2 scsi_blk | | | `- ahci_scsi.id0lun0 > > >> pci 6 pci_bridge_drv | | |- pci_2:4.0 > > >> nvme 0 nvme | | | `- nvme#0 > > >> blk 1 nvme-blk | | | `- nvme#0.blk#1 > > >> > > >> Namespaces and LUNs are modeled as block devices (class = 'blk'). > > > > > > So multiple block devices per NVMe device? I did not know that was supported. > > > > > > We need a sandbox driver for NVMe as it has no tests at present. Since > > > it has no tests, I don't think we can expect people to know how to > > > maintain whatever functionality is there. > > > > NVMe drives with multiple namespaces exist for servers but not for > > consumer NVMe drives. > > > > In QEMU you can define an NVMe device with multiple namespaces. Cf. > > https://qemu.readthedocs.io/en/latest/system/devices/nvme.html?highlight=namespace#additional-namespaces > > > > So for a first glimpse at the handling I suggest to use QEMU. > > Well that's fine, but every uclass must have a test and a sandbox > emulator as well. Wait, it seems that you're discussing a different thing from my patch. While I don't know whether NVMe namespaces are a kind of "HW partitions", we don't care much here as long as any namespace can be handled simply as a normal block device, like scsi LUN's, in terms of U-Boot driver model. # On the other hand, we have to explicitly switch "hw partitions" # with blk_select_hwpart_devnum() on MMC devices even though we use # the *same* udevice(blk_desc). # See do_mmcrpmb() in cmd/mmc.c So I hope that *your* discussion doesn't make any difference to my patch. Right? -Takahiro Akashi > Regards, > Simon
On 10/12/21 05:26, AKASHI Takahiro wrote: > Simon, Heinrich, > > On Mon, Oct 11, 2021 at 10:14:02AM -0600, Simon Glass wrote: >> Hi Heinrich, >> >> On Mon, 11 Oct 2021 at 09:09, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >>> >>> >>> >>> On 10/11/21 16:32, Simon Glass wrote: >>>> Hi Heinrich, >>>> >>>> On Mon, 11 Oct 2021 at 04:07, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >>>>> >>>>> >>>>> >>>>> On 10/1/21 13:48, Peter Robinson wrote: >>>>>> On Fri, Oct 1, 2021 at 6:03 AM AKASHI Takahiro >>>>>> <takahiro.akashi@linaro.org> wrote: >>>>>>> >>>>>>> In blk_get_device_by_str(), the comment says: "Updates the partition table >>>>>>> for the specified hw partition." >>>>>>> Since hw partition is supported only on MMC, it makes no sense to do so >>>>>>> for other devices. >>>>>> >>>>>> Is it not also supported on UFS, and I believe it may also be an >>>>>> option in the NVME spec too. >>>>> >>>>> An NVMe device may expose multiple namespaces. blk_create_devicef() is >>>>> called for each namespace. >>>>> >>>>> A SCSI device may have multiple LUNs. blk_create_devicef() is called for >>>>> each LUN. >>>>> >>>>> This is what the tree shown by 'dm tree' with on NVMe namespace and one LUN. >>>>> >>>>> Class Index Driver Name >>>>> --------------------------------------------------------------------- >>>>> root 0 root_driver root_driver >>>>> simple_bus 0 simple_bus |- soc >>>>> spi 1 sifive_spi | |- spi@10050000 >>>>> mmc 0 mmc_spi | | `- mmc@0 >>>>> blk 0 mmc_blk | | `- mmc@0.blk >>>>> pci 0 pcie_sifive | |- pcie@e00000000 >>>>> pci 1 pci_bridge_drv | | `- pci_0:0.0 >>>>> pci 2 pci_bridge_drv | | `- pci_1:0.0 >>>>> pci 5 pci_bridge_drv | | |- pci_2:3.0 >>>>> ahci 0 ahci_pci | | | `- ahci_pci >>>>> scsi 0 ahci_scsi | | | `- ahci_scsi >>>>> blk 2 scsi_blk | | | `- ahci_scsi.id0lun0 >>>>> pci 6 pci_bridge_drv | | |- pci_2:4.0 >>>>> nvme 0 nvme | | | `- nvme#0 >>>>> blk 1 nvme-blk | | | `- nvme#0.blk#1 >>>>> >>>>> Namespaces and LUNs are modeled as block devices (class = 'blk'). >>>> >>>> So multiple block devices per NVMe device? I did not know that was supported. >>>> >>>> We need a sandbox driver for NVMe as it has no tests at present. Since >>>> it has no tests, I don't think we can expect people to know how to >>>> maintain whatever functionality is there. >>> >>> NVMe drives with multiple namespaces exist for servers but not for >>> consumer NVMe drives. >>> >>> In QEMU you can define an NVMe device with multiple namespaces. Cf. >>> https://qemu.readthedocs.io/en/latest/system/devices/nvme.html?highlight=namespace#additional-namespaces >>> >>> So for a first glimpse at the handling I suggest to use QEMU. >> >> Well that's fine, but every uclass must have a test and a sandbox >> emulator as well. > > Wait, it seems that you're discussing a different thing from my patch. > > While I don't know whether NVMe namespaces are a kind of "HW partitions", > we don't care much here as long as any namespace can be handled simply > as a normal block device, like scsi LUN's, in terms of U-Boot driver model. > > # On the other hand, we have to explicitly switch "hw partitions" > # with blk_select_hwpart_devnum() on MMC devices even though we use > # the *same* udevice(blk_desc). > # See do_mmcrpmb() in cmd/mmc.c Each hardware partition should be a block device (class blk) which is mirrored in the UEFI world by a CTRL() device. It is not necessary for parent device to be a block device. Best regards Heinrich > > So I hope that *your* discussion doesn't make any difference to my patch. > Right? > > -Takahiro Akashi > > >> Regards, >> Simon
Hi Takahiro, On Mon, 11 Oct 2021 at 21:26, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > Simon, Heinrich, > > On Mon, Oct 11, 2021 at 10:14:02AM -0600, Simon Glass wrote: > > Hi Heinrich, > > > > On Mon, 11 Oct 2021 at 09:09, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > > > > > > > On 10/11/21 16:32, Simon Glass wrote: > > > > Hi Heinrich, > > > > > > > > On Mon, 11 Oct 2021 at 04:07, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > >> > > > >> > > > >> > > > >> On 10/1/21 13:48, Peter Robinson wrote: > > > >>> On Fri, Oct 1, 2021 at 6:03 AM AKASHI Takahiro > > > >>> <takahiro.akashi@linaro.org> wrote: > > > >>>> > > > >>>> In blk_get_device_by_str(), the comment says: "Updates the partition table > > > >>>> for the specified hw partition." > > > >>>> Since hw partition is supported only on MMC, it makes no sense to do so > > > >>>> for other devices. > > > >>> > > > >>> Is it not also supported on UFS, and I believe it may also be an > > > >>> option in the NVME spec too. > > > >> > > > >> An NVMe device may expose multiple namespaces. blk_create_devicef() is > > > >> called for each namespace. > > > >> > > > >> A SCSI device may have multiple LUNs. blk_create_devicef() is called for > > > >> each LUN. > > > >> > > > >> This is what the tree shown by 'dm tree' with on NVMe namespace and one LUN. > > > >> > > > >> Class Index Driver Name > > > >> --------------------------------------------------------------------- > > > >> root 0 root_driver root_driver > > > >> simple_bus 0 simple_bus |- soc > > > >> spi 1 sifive_spi | |- spi@10050000 > > > >> mmc 0 mmc_spi | | `- mmc@0 > > > >> blk 0 mmc_blk | | `- mmc@0.blk > > > >> pci 0 pcie_sifive | |- pcie@e00000000 > > > >> pci 1 pci_bridge_drv | | `- pci_0:0.0 > > > >> pci 2 pci_bridge_drv | | `- pci_1:0.0 > > > >> pci 5 pci_bridge_drv | | |- pci_2:3.0 > > > >> ahci 0 ahci_pci | | | `- ahci_pci > > > >> scsi 0 ahci_scsi | | | `- ahci_scsi > > > >> blk 2 scsi_blk | | | `- ahci_scsi.id0lun0 > > > >> pci 6 pci_bridge_drv | | |- pci_2:4.0 > > > >> nvme 0 nvme | | | `- nvme#0 > > > >> blk 1 nvme-blk | | | `- nvme#0.blk#1 > > > >> > > > >> Namespaces and LUNs are modeled as block devices (class = 'blk'). > > > > > > > > So multiple block devices per NVMe device? I did not know that was supported. > > > > > > > > We need a sandbox driver for NVMe as it has no tests at present. Since > > > > it has no tests, I don't think we can expect people to know how to > > > > maintain whatever functionality is there. > > > > > > NVMe drives with multiple namespaces exist for servers but not for > > > consumer NVMe drives. > > > > > > In QEMU you can define an NVMe device with multiple namespaces. Cf. > > > https://qemu.readthedocs.io/en/latest/system/devices/nvme.html?highlight=namespace#additional-namespaces > > > > > > So for a first glimpse at the handling I suggest to use QEMU. > > > > Well that's fine, but every uclass must have a test and a sandbox > > emulator as well. > > Wait, it seems that you're discussing a different thing from my patch. > > While I don't know whether NVMe namespaces are a kind of "HW partitions", > we don't care much here as long as any namespace can be handled simply > as a normal block device, like scsi LUN's, in terms of U-Boot driver model. > > # On the other hand, we have to explicitly switch "hw partitions" > # with blk_select_hwpart_devnum() on MMC devices even though we use > # the *same* udevice(blk_desc). > # See do_mmcrpmb() in cmd/mmc.c > > So I hope that *your* discussion doesn't make any difference to my patch. > Right? From my POV the patch is fine, which is why I added the review tag. We should continue the discussion on BLK versus PARTITION. Regards, Simon
On Tue, Oct 12, 2021 at 03:30:26PM +0200, Heinrich Schuchardt wrote: > > > On 10/12/21 05:26, AKASHI Takahiro wrote: > > Simon, Heinrich, > > > > On Mon, Oct 11, 2021 at 10:14:02AM -0600, Simon Glass wrote: > > > Hi Heinrich, > > > > > > On Mon, 11 Oct 2021 at 09:09, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > > > > > > > > > > > On 10/11/21 16:32, Simon Glass wrote: > > > > > Hi Heinrich, > > > > > > > > > > On Mon, 11 Oct 2021 at 04:07, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > > > > > > > > > > > > > > > > > > > On 10/1/21 13:48, Peter Robinson wrote: > > > > > > > On Fri, Oct 1, 2021 at 6:03 AM AKASHI Takahiro > > > > > > > <takahiro.akashi@linaro.org> wrote: > > > > > > > > > > > > > > > > In blk_get_device_by_str(), the comment says: "Updates the partition table > > > > > > > > for the specified hw partition." > > > > > > > > Since hw partition is supported only on MMC, it makes no sense to do so > > > > > > > > for other devices. > > > > > > > > > > > > > > Is it not also supported on UFS, and I believe it may also be an > > > > > > > option in the NVME spec too. > > > > > > > > > > > > An NVMe device may expose multiple namespaces. blk_create_devicef() is > > > > > > called for each namespace. > > > > > > > > > > > > A SCSI device may have multiple LUNs. blk_create_devicef() is called for > > > > > > each LUN. > > > > > > > > > > > > This is what the tree shown by 'dm tree' with on NVMe namespace and one LUN. > > > > > > > > > > > > Class Index Driver Name > > > > > > --------------------------------------------------------------------- > > > > > > root 0 root_driver root_driver > > > > > > simple_bus 0 simple_bus |- soc > > > > > > spi 1 sifive_spi | |- spi@10050000 > > > > > > mmc 0 mmc_spi | | `- mmc@0 > > > > > > blk 0 mmc_blk | | `- mmc@0.blk > > > > > > pci 0 pcie_sifive | |- pcie@e00000000 > > > > > > pci 1 pci_bridge_drv | | `- pci_0:0.0 > > > > > > pci 2 pci_bridge_drv | | `- pci_1:0.0 > > > > > > pci 5 pci_bridge_drv | | |- pci_2:3.0 > > > > > > ahci 0 ahci_pci | | | `- ahci_pci > > > > > > scsi 0 ahci_scsi | | | `- ahci_scsi > > > > > > blk 2 scsi_blk | | | `- ahci_scsi.id0lun0 > > > > > > pci 6 pci_bridge_drv | | |- pci_2:4.0 > > > > > > nvme 0 nvme | | | `- nvme#0 > > > > > > blk 1 nvme-blk | | | `- nvme#0.blk#1 > > > > > > > > > > > > Namespaces and LUNs are modeled as block devices (class = 'blk'). > > > > > > > > > > So multiple block devices per NVMe device? I did not know that was supported. > > > > > > > > > > We need a sandbox driver for NVMe as it has no tests at present. Since > > > > > it has no tests, I don't think we can expect people to know how to > > > > > maintain whatever functionality is there. > > > > > > > > NVMe drives with multiple namespaces exist for servers but not for > > > > consumer NVMe drives. > > > > > > > > In QEMU you can define an NVMe device with multiple namespaces. Cf. > > > > https://qemu.readthedocs.io/en/latest/system/devices/nvme.html?highlight=namespace#additional-namespaces > > > > > > > > So for a first glimpse at the handling I suggest to use QEMU. > > > > > > Well that's fine, but every uclass must have a test and a sandbox > > > emulator as well. > > > > Wait, it seems that you're discussing a different thing from my patch. > > > > While I don't know whether NVMe namespaces are a kind of "HW partitions", > > we don't care much here as long as any namespace can be handled simply > > as a normal block device, like scsi LUN's, in terms of U-Boot driver model. > > > > # On the other hand, we have to explicitly switch "hw partitions" > > # with blk_select_hwpart_devnum() on MMC devices even though we use > > # the *same* udevice(blk_desc). > > # See do_mmcrpmb() in cmd/mmc.c > > Each hardware partition should be a block device (class blk) which is > mirrored in the UEFI world by a CTRL() device. Yes, whether it is mirrored or not, a hw partition is to be a separate udevice from its associated raw device. > It is not necessary for > parent device to be a block device. I'm not sure what 'parent device' means here, but I guess that it is the raw MMC device (as a controller handle in UEFI terminology which is set to provide BLOCK_IO_PROTOCOL), isn't it? -Takahiro Akashi > Best regards > > Heinrich > > > > > So I hope that *your* discussion doesn't make any difference to my patch. > > Right? > > > > -Takahiro Akashi > > > > > > > Regards, > > > Simon
diff --git a/disk/part.c b/disk/part.c index a6a8f7052bd3..b330103a5bc0 100644 --- a/disk/part.c +++ b/disk/part.c @@ -427,7 +427,8 @@ int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str, * Always should be done, otherwise hw partition 0 will return stale * data after displaying a non-zero hw partition. */ - part_init(*dev_desc); + if ((*dev_desc)->if_type == IF_TYPE_MMC) + part_init(*dev_desc); #endif cleanup:
In blk_get_device_by_str(), the comment says: "Updates the partition table for the specified hw partition." Since hw partition is supported only on MMC, it makes no sense to do so for other devices. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- disk/part.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.33.0