Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Simon, On Mon, Jan 28, 2019 at 05:46:21PM -0700, Simon Glass wrote: > Hi, > > On Mon, 28 Jan 2019 at 01:55, AKASHI Takahiro > wrote: > > > > On Fri, Jan 25, 2019 at 10:31:20AM +0100, Alexander Graf wrote: > > > > > > > > > On 25.01.19 10:18, AKASHI Takahiro wrote: > > > > On Fri, Jan 25, 2019 at 09:52:31AM +0100, Alexander Graf wrote: > > > >> > > > >> > > > >> On 25.01.19 09:27, AKASHI Takahiro wrote: > > > >>> Alex, > > > >>> > > > >>> On Wed, Jan 23, 2019 at 10:51:29AM +0100, Alexander Graf wrote: > > > On 01/22/2019 08:39 PM, Simon Glass wrote: > > > > Hi Alex, > > > > > > > > On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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 > > > > > > ... and then we need to create our own sub object model around the > > > UCLASS_EFI devices again. I' not convinced that's a great idea yet > > > :). I > > > really see little reason not to just expose every dm device as EFI > > > handle. > > > Things would plug in quite naturally I think. > > > >>> > > > >>> You said that the ultimate goal is to remove all efi_object data. > > > >>> Do you think that all the existing efi_object can be mapped to > > > >>> one of existing u-boot uclass devices? > > > >>> > > > >>> If so, what would be an real entity of a UEFI handle? > > > >>> struct udevice *? > > > >>> > > > >>> But Simon seems not to agree to adding any UEFI-specific members > > > >>> in struct udevice. > > > >> > > > >> I think we'll have to experiment with both approaches. I personally > > > >> would like to have struct udevice * be the UEFI handle, yes. > > > >> > > > >>> > > > But either way, someone would need to sit down and prototype things > > > to be > > > sure. > > > > > > >>> > > > >>> The most simplest prototype would include > > > >>> * event mechanism (just registration and execution of hook/handler) > > > >>> event: udevice creation (and deletion) > > > >>> * efi_disk hook for udevice(UCLASS_BLK) creation > > > >>> * modified block device's enumeration code, say, scsi_scan(), > > > >>> to add an event hook at udevice creation > > > >>> * removing efi_disk_register() from efi_init_obj_list() > > > >>> * Optionally(?) UCLASS_PARTITION > > > >>> (Partition udevices would be created in part_init().) > > > >> > > > >> Almost. > > > >> > > > >> The simplest prototype would be to add a struct efi_object into struct > > > >> udevice. Then whenever we're looping over efi_obj_list in the code, we > > > >> additionally loop over all udevices to find the handle. > > > > > > > > Ah, yes. You're going further :) > > > > > > > >> Then, we could slowly give the uclasses explicit knowledge of uefi > > > >> protocols. So most of the logic of efi_disk_register() would move into > > > >> (or get called by) drivers/block/blk-uclass.c:blk_create_device(). > > > > > > > > Via event? Otherwise, we cannot decouple u-boot and UEFI world. > > > > > > For a prototype, just make it explicit and see how far that gets us. > > > > > > >> Instead of creating diskobj and adding calling efi_add_handle(), we > > > >> could then just use existing data structure from the udevice (and its > > > >> platdata). > > > > > > > > I don't have good confidence that we can remove struct efi_disk_obj, > > > > at least, for the time being as some of its members are quite > > > > UEFI-specific. > > > > > > Maybe we can move them into struct blk_desc? It's a matter of > > > experimenting I guess. > > > > > > > > > > >> > > > >> Does this make sense? Less events, more implicity :). > > > > > > > > I'll go for it. > > > > > > Thanks a lot :). Feel free to pick an easier target for starters too if > > >
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Hi, On Mon, 28 Jan 2019 at 01:55, AKASHI Takahiro wrote: > > On Fri, Jan 25, 2019 at 10:31:20AM +0100, Alexander Graf wrote: > > > > > > On 25.01.19 10:18, AKASHI Takahiro wrote: > > > On Fri, Jan 25, 2019 at 09:52:31AM +0100, Alexander Graf wrote: > > >> > > >> > > >> On 25.01.19 09:27, AKASHI Takahiro wrote: > > >>> Alex, > > >>> > > >>> On Wed, Jan 23, 2019 at 10:51:29AM +0100, Alexander Graf wrote: > > On 01/22/2019 08:39 PM, Simon Glass wrote: > > > Hi Alex, > > > > > > On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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 > > > > ... and then we need to create our own sub object model around the > > UCLASS_EFI devices again. I' not convinced that's a great idea yet :). > > I > > really see little reason not to just expose every dm device as EFI > > handle. > > Things would plug in quite naturally I think. > > >>> > > >>> You said that the ultimate goal is to remove all efi_object data. > > >>> Do you think that all the existing efi_object can be mapped to > > >>> one of existing u-boot uclass devices? > > >>> > > >>> If so, what would be an real entity of a UEFI handle? > > >>> struct udevice *? > > >>> > > >>> But Simon seems not to agree to adding any UEFI-specific members > > >>> in struct udevice. > > >> > > >> I think we'll have to experiment with both approaches. I personally > > >> would like to have struct udevice * be the UEFI handle, yes. > > >> > > >>> > > But either way, someone would need to sit down and prototype things to > > be > > sure. > > > > >>> > > >>> The most simplest prototype would include > > >>> * event mechanism (just registration and execution of hook/handler) > > >>> event: udevice creation (and deletion) > > >>> * efi_disk hook for udevice(UCLASS_BLK) creation > > >>> * modified block device's enumeration code, say, scsi_scan(), > > >>> to add an event hook at udevice creation > > >>> * removing efi_disk_register() from efi_init_obj_list() > > >>> * Optionally(?) UCLASS_PARTITION > > >>> (Partition udevices would be created in part_init().) > > >> > > >> Almost. > > >> > > >> The simplest prototype would be to add a struct efi_object into struct > > >> udevice. Then whenever we're looping over efi_obj_list in the code, we > > >> additionally loop over all udevices to find the handle. > > > > > > Ah, yes. You're going further :) > > > > > >> Then, we could slowly give the uclasses explicit knowledge of uefi > > >> protocols. So most of the logic of efi_disk_register() would move into > > >> (or get called by) drivers/block/blk-uclass.c:blk_create_device(). > > > > > > Via event? Otherwise, we cannot decouple u-boot and UEFI world. > > > > For a prototype, just make it explicit and see how far that gets us. > > > > >> Instead of creating diskobj and adding calling efi_add_handle(), we > > >> could then just use existing data structure from the udevice (and its > > >> platdata). > > > > > > I don't have good confidence that we can remove struct efi_disk_obj, > > > at least, for the time being as some of its members are quite > > > UEFI-specific. > > > > Maybe we can move them into struct blk_desc? It's a matter of > > experimenting I guess. > > > > > > > >> > > >> Does this make sense? Less events, more implicity :). > > > > > > I'll go for it. > > > > Thanks a lot :). Feel free to pick an easier target for starters too if > > you prefer. > > Prototyping is done :) > Since it was so easy and simple, now I'm thinking of implementing > UCLASS_PARTITION. But it is not so straightforward as I expected, > and it won't bring us lots of advantages. > (I think that blk_desc should also support a partition in its own.) blk_desc is in UCLASS_BLK. So we
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
> Am 28.01.2019 um 09:56 schrieb AKASHI Takahiro : > >> On Fri, Jan 25, 2019 at 10:31:20AM +0100, Alexander Graf wrote: >> >> >>> On 25.01.19 10:18, AKASHI Takahiro wrote: On Fri, Jan 25, 2019 at 09:52:31AM +0100, Alexander Graf wrote: > On 25.01.19 09:27, AKASHI Takahiro wrote: > Alex, > >> On Wed, Jan 23, 2019 at 10:51:29AM +0100, Alexander Graf wrote: >>> On 01/22/2019 08:39 PM, Simon Glass wrote: >>> Hi Alex, >>> On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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 >> >> ... and then we need to create our own sub object model around the >> UCLASS_EFI devices again. I' not convinced that's a great idea yet :). I >> really see little reason not to just expose every dm device as EFI >> handle. >> Things would plug in quite naturally I think. > > You said that the ultimate goal is to remove all efi_object data. > Do you think that all the existing efi_object can be mapped to > one of existing u-boot uclass devices? > > If so, what would be an real entity of a UEFI handle? > struct udevice *? > > But Simon seems not to agree to adding any UEFI-specific members > in struct udevice. I think we'll have to experiment with both approaches. I personally would like to have struct udevice * be the UEFI handle, yes. > >> But either way, someone would need to sit down and prototype things to be >> sure. >> > > The most simplest prototype would include > * event mechanism (just registration and execution of hook/handler) >event: udevice creation (and deletion) > * efi_disk hook for udevice(UCLASS_BLK) creation > * modified block device's enumeration code, say, scsi_scan(), > to add an event hook at udevice creation > * removing efi_disk_register() from efi_init_obj_list() > * Optionally(?) UCLASS_PARTITION > (Partition udevices would be created in part_init().) Almost. The simplest prototype would be to add a struct efi_object into struct udevice. Then whenever we're looping over efi_obj_list in the code, we additionally loop over all udevices to find the handle. >>> >>> Ah, yes. You're going further :) >>> Then, we could slowly give the uclasses explicit knowledge of uefi protocols. So most of the logic of efi_disk_register() would move into (or get called by) drivers/block/blk-uclass.c:blk_create_device(). >>> >>> Via event? Otherwise, we cannot decouple u-boot and UEFI world. >> >> For a prototype, just make it explicit and see how far that gets us. >> Instead of creating diskobj and adding calling efi_add_handle(), we could then just use existing data structure from the udevice (and its platdata). >>> >>> I don't have good confidence that we can remove struct efi_disk_obj, >>> at least, for the time being as some of its members are quite UEFI-specific. >> >> Maybe we can move them into struct blk_desc? It's a matter of >> experimenting I guess. >> >>> Does this make sense? Less events, more implicity :). >>> >>> I'll go for it. >> >> Thanks a lot :). Feel free to pick an easier target for starters too if >> you prefer. > > Prototyping is done :) > Since it was so easy and simple, now I'm thinking of implementing > UCLASS_PARTITION. But it is not so straightforward as I expected, > and it won't bring us lots of advantages. > (I think that blk_desc should also support a partition in its own.) > > Once it gets working, may I send out a patch? Feel free to even just send a patch of what you have now as RFC, so that we can see if this looks like the right direction. Let's make use of our time zone differences :). Alex
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On Fri, Jan 25, 2019 at 10:31:20AM +0100, Alexander Graf wrote: > > > On 25.01.19 10:18, AKASHI Takahiro wrote: > > On Fri, Jan 25, 2019 at 09:52:31AM +0100, Alexander Graf wrote: > >> > >> > >> On 25.01.19 09:27, AKASHI Takahiro wrote: > >>> Alex, > >>> > >>> On Wed, Jan 23, 2019 at 10:51:29AM +0100, Alexander Graf wrote: > On 01/22/2019 08:39 PM, Simon Glass wrote: > > Hi Alex, > > > > On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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 > > ... and then we need to create our own sub object model around the > UCLASS_EFI devices again. I' not convinced that's a great idea yet :). I > really see little reason not to just expose every dm device as EFI > handle. > Things would plug in quite naturally I think. > >>> > >>> You said that the ultimate goal is to remove all efi_object data. > >>> Do you think that all the existing efi_object can be mapped to > >>> one of existing u-boot uclass devices? > >>> > >>> If so, what would be an real entity of a UEFI handle? > >>> struct udevice *? > >>> > >>> But Simon seems not to agree to adding any UEFI-specific members > >>> in struct udevice. > >> > >> I think we'll have to experiment with both approaches. I personally > >> would like to have struct udevice * be the UEFI handle, yes. > >> > >>> > But either way, someone would need to sit down and prototype things to be > sure. > > >>> > >>> The most simplest prototype would include > >>> * event mechanism (just registration and execution of hook/handler) > >>> event: udevice creation (and deletion) > >>> * efi_disk hook for udevice(UCLASS_BLK) creation > >>> * modified block device's enumeration code, say, scsi_scan(), > >>> to add an event hook at udevice creation > >>> * removing efi_disk_register() from efi_init_obj_list() > >>> * Optionally(?) UCLASS_PARTITION > >>> (Partition udevices would be created in part_init().) > >> > >> Almost. > >> > >> The simplest prototype would be to add a struct efi_object into struct > >> udevice. Then whenever we're looping over efi_obj_list in the code, we > >> additionally loop over all udevices to find the handle. > > > > Ah, yes. You're going further :) > > > >> Then, we could slowly give the uclasses explicit knowledge of uefi > >> protocols. So most of the logic of efi_disk_register() would move into > >> (or get called by) drivers/block/blk-uclass.c:blk_create_device(). > > > > Via event? Otherwise, we cannot decouple u-boot and UEFI world. > > For a prototype, just make it explicit and see how far that gets us. > > >> Instead of creating diskobj and adding calling efi_add_handle(), we > >> could then just use existing data structure from the udevice (and its > >> platdata). > > > > I don't have good confidence that we can remove struct efi_disk_obj, > > at least, for the time being as some of its members are quite UEFI-specific. > > Maybe we can move them into struct blk_desc? It's a matter of > experimenting I guess. > > > > >> > >> Does this make sense? Less events, more implicity :). > > > > I'll go for it. > > Thanks a lot :). Feel free to pick an easier target for starters too if > you prefer. Prototyping is done :) Since it was so easy and simple, now I'm thinking of implementing UCLASS_PARTITION. But it is not so straightforward as I expected, and it won't bring us lots of advantages. (I think that blk_desc should also support a partition in its own.) Once it gets working, may I send out a patch? -Takahiro Akashi > > Alex ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On 25.01.19 10:18, AKASHI Takahiro wrote: > On Fri, Jan 25, 2019 at 09:52:31AM +0100, Alexander Graf wrote: >> >> >> On 25.01.19 09:27, AKASHI Takahiro wrote: >>> Alex, >>> >>> On Wed, Jan 23, 2019 at 10:51:29AM +0100, Alexander Graf wrote: On 01/22/2019 08:39 PM, Simon Glass wrote: > Hi Alex, > > On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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 ... and then we need to create our own sub object model around the UCLASS_EFI devices again. I' not convinced that's a great idea yet :). I really see little reason not to just expose every dm device as EFI handle. Things would plug in quite naturally I think. >>> >>> You said that the ultimate goal is to remove all efi_object data. >>> Do you think that all the existing efi_object can be mapped to >>> one of existing u-boot uclass devices? >>> >>> If so, what would be an real entity of a UEFI handle? >>> struct udevice *? >>> >>> But Simon seems not to agree to adding any UEFI-specific members >>> in struct udevice. >> >> I think we'll have to experiment with both approaches. I personally >> would like to have struct udevice * be the UEFI handle, yes. >> >>> But either way, someone would need to sit down and prototype things to be sure. >>> >>> The most simplest prototype would include >>> * event mechanism (just registration and execution of hook/handler) >>> event: udevice creation (and deletion) >>> * efi_disk hook for udevice(UCLASS_BLK) creation >>> * modified block device's enumeration code, say, scsi_scan(), >>> to add an event hook at udevice creation >>> * removing efi_disk_register() from efi_init_obj_list() >>> * Optionally(?) UCLASS_PARTITION >>> (Partition udevices would be created in part_init().) >> >> Almost. >> >> The simplest prototype would be to add a struct efi_object into struct >> udevice. Then whenever we're looping over efi_obj_list in the code, we >> additionally loop over all udevices to find the handle. > > Ah, yes. You're going further :) > >> Then, we could slowly give the uclasses explicit knowledge of uefi >> protocols. So most of the logic of efi_disk_register() would move into >> (or get called by) drivers/block/blk-uclass.c:blk_create_device(). > > Via event? Otherwise, we cannot decouple u-boot and UEFI world. For a prototype, just make it explicit and see how far that gets us. >> Instead of creating diskobj and adding calling efi_add_handle(), we >> could then just use existing data structure from the udevice (and its >> platdata). > > I don't have good confidence that we can remove struct efi_disk_obj, > at least, for the time being as some of its members are quite UEFI-specific. Maybe we can move them into struct blk_desc? It's a matter of experimenting I guess. > >> >> Does this make sense? Less events, more implicity :). > > I'll go for it. Thanks a lot :). Feel free to pick an easier target for starters too if you prefer. Alex ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On Fri, Jan 25, 2019 at 09:52:31AM +0100, Alexander Graf wrote: > > > On 25.01.19 09:27, AKASHI Takahiro wrote: > > Alex, > > > > On Wed, Jan 23, 2019 at 10:51:29AM +0100, Alexander Graf wrote: > >> On 01/22/2019 08:39 PM, Simon Glass wrote: > >>> Hi Alex, > >>> > >>> On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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 > >> > >> ... and then we need to create our own sub object model around the > >> UCLASS_EFI devices again. I' not convinced that's a great idea yet :). I > >> really see little reason not to just expose every dm device as EFI handle. > >> Things would plug in quite naturally I think. > > > > You said that the ultimate goal is to remove all efi_object data. > > Do you think that all the existing efi_object can be mapped to > > one of existing u-boot uclass devices? > > > > If so, what would be an real entity of a UEFI handle? > > struct udevice *? > > > > But Simon seems not to agree to adding any UEFI-specific members > > in struct udevice. > > I think we'll have to experiment with both approaches. I personally > would like to have struct udevice * be the UEFI handle, yes. > > > > >> But either way, someone would need to sit down and prototype things to be > >> sure. > >> > > > > The most simplest prototype would include > > * event mechanism (just registration and execution of hook/handler) > > event: udevice creation (and deletion) > > * efi_disk hook for udevice(UCLASS_BLK) creation > > * modified block device's enumeration code, say, scsi_scan(), > > to add an event hook at udevice creation > > * removing efi_disk_register() from efi_init_obj_list() > > * Optionally(?) UCLASS_PARTITION > > (Partition udevices would be created in part_init().) > > Almost. > > The simplest prototype would be to add a struct efi_object into struct > udevice. Then whenever we're looping over efi_obj_list in the code, we > additionally loop over all udevices to find the handle. Ah, yes. You're going further :) > Then, we could slowly give the uclasses explicit knowledge of uefi > protocols. So most of the logic of efi_disk_register() would move into > (or get called by) drivers/block/blk-uclass.c:blk_create_device(). Via event? Otherwise, we cannot decouple u-boot and UEFI world. > Instead of creating diskobj and adding calling efi_add_handle(), we > could then just use existing data structure from the udevice (and its > platdata). I don't have good confidence that we can remove struct efi_disk_obj, at least, for the time being as some of its members are quite UEFI-specific. > > Does this make sense? Less events, more implicity :). I'll go for it. Thanks, -Takahiro Akashi > Alex ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On 25.01.19 09:27, AKASHI Takahiro wrote: > Alex, > > On Wed, Jan 23, 2019 at 10:51:29AM +0100, Alexander Graf wrote: >> On 01/22/2019 08:39 PM, Simon Glass wrote: >>> Hi Alex, >>> >>> On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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 >> >> ... and then we need to create our own sub object model around the >> UCLASS_EFI devices again. I' not convinced that's a great idea yet :). I >> really see little reason not to just expose every dm device as EFI handle. >> Things would plug in quite naturally I think. > > You said that the ultimate goal is to remove all efi_object data. > Do you think that all the existing efi_object can be mapped to > one of existing u-boot uclass devices? > > If so, what would be an real entity of a UEFI handle? > struct udevice *? > > But Simon seems not to agree to adding any UEFI-specific members > in struct udevice. I think we'll have to experiment with both approaches. I personally would like to have struct udevice * be the UEFI handle, yes. > >> But either way, someone would need to sit down and prototype things to be >> sure. >> > > The most simplest prototype would include > * event mechanism (just registration and execution of hook/handler) > event: udevice creation (and deletion) > * efi_disk hook for udevice(UCLASS_BLK) creation > * modified block device's enumeration code, say, scsi_scan(), > to add an event hook at udevice creation > * removing efi_disk_register() from efi_init_obj_list() > * Optionally(?) UCLASS_PARTITION > (Partition udevices would be created in part_init().) Almost. The simplest prototype would be to add a struct efi_object into struct udevice. Then whenever we're looping over efi_obj_list in the code, we additionally loop over all udevices to find the handle. Then, we could slowly give the uclasses explicit knowledge of uefi protocols. So most of the logic of efi_disk_register() would move into (or get called by) drivers/block/blk-uclass.c:blk_create_device(). Instead of creating diskobj and adding calling efi_add_handle(), we could then just use existing data structure from the udevice (and its platdata). Does this make sense? Less events, more implicity :). Alex ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Alex, On Wed, Jan 23, 2019 at 10:51:29AM +0100, Alexander Graf wrote: > On 01/22/2019 08:39 PM, Simon Glass wrote: > >Hi Alex, > > > >On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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 > > ... and then we need to create our own sub object model around the > UCLASS_EFI devices again. I' not convinced that's a great idea yet :). I > really see little reason not to just expose every dm device as EFI handle. > Things would plug in quite naturally I think. You said that the ultimate goal is to remove all efi_object data. Do you think that all the existing efi_object can be mapped to one of existing u-boot uclass devices? If so, what would be an real entity of a UEFI handle? struct udevice *? But Simon seems not to agree to adding any UEFI-specific members in struct udevice. > But either way, someone would need to sit down and prototype things to be > sure. > The most simplest prototype would include * event mechanism (just registration and execution of hook/handler) event: udevice creation (and deletion) * efi_disk hook for udevice(UCLASS_BLK) creation * modified block device's enumeration code, say, scsi_scan(), to add an event hook at udevice creation * removing efi_disk_register() from efi_init_obj_list() * Optionally(?) UCLASS_PARTITION (Partition udevices would be created in part_init().) ? Thanks, -Takahiro Akashi > > Alex > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Heinrich, On Thu, Jan 24, 2019 at 10:19:28PM +0100, Heinrich Schuchardt wrote: > On 1/24/19 9:18 PM, Simon Glass wrote: > > Hi Takahiro, > > > > On Thu, 24 Jan 2019 at 13:53, AKASHI Takahiro > > 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 > >>> wrote: > > On 1/22/19 8:39 PM, Simon Glass wrote: > > Hi Alex, > > > > On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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. > > > >> > >> #
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On 1/24/19 9:18 PM, Simon Glass wrote: > Hi Takahiro, > > On Thu, 24 Jan 2019 at 13:53, AKASHI Takahiro > 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 >>> wrote: On 1/22/19 8:39 PM, Simon Glass wrote: > Hi Alex, > > On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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 >> orand or >> blk_descefi_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
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Hi Takahiro, On Thu, 24 Jan 2019 at 13:53, AKASHI Takahiro 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 > > wrote: > > > > > > On 1/22/19 8:39 PM, Simon Glass wrote: > > > > Hi Alex, > > > > > > > > On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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 > orand or > blk_descefi_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
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
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 wrote: > > > > On 1/22/19 8:39 PM, Simon Glass wrote: > > > Hi Alex, > > > > > > On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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 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 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(). # In any way, we will need a) a bi-directional link between UCLASS_BLK efi_obj orand or blk_descefi_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. -Takahiro Akashi > > > > [1] https://lists.denx.de/pipermail/u-boot/2019-January/354359.html > > [RFC] Device model for block devices - integration with EFI subsystem > > > > Best regards > > > > Heinrich > > > > > > > >> > > >>> > > But we have this annoying interim state where we would lose a few > > boards > > because they haven't been converted to DM. That's what keeps us from > > it. > > > > I think what this discussion boils down to is that someone needs to > > start prototyping the DM/EFI integration. Start off with a simple > > subsystem, like BLK. > > >>> > > >>> Even in the current implementation, > > >>> * UEFI disk is implemented using UCLASS_BLK devices > > >>> (We can ignore !CONFIG_BLK case now as we have agreed.) > > >>> * UEFI-specific block device can be seen as UCLASS_BLK/IF_TYPE_EFI > > >>> > > >>> So how essentially different is the *ultimate* goal from the current > > >>> form > > >>> regarding block devices? > > >> > > >> The ultimate goal is that efi_disk_register() and efi_obj_list disappear. > > >> > > >> Functionality wise we should be 100% identical to today, so all test > > >> cases would
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Hi Alex, On Wed, 23 Jan 2019 at 22:51, Alexander Graf wrote: > > On 01/22/2019 08:39 PM, Simon Glass wrote: > > Hi Alex, > > > > On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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 > > ... and then we need to create our own sub object model around the > UCLASS_EFI devices again. I' not convinced that's a great idea yet :). I > really see little reason not to just expose every dm device as EFI > handle. Things would plug in quite naturally I think. I would like to keep some separation between EFI and DM data structures. We can maintain a list of pointers or whatever is needed for the protocol stuff. I'm not convinced either way also, and agree that: > > But either way, someone would need to sit down and prototype things to > be sure. I think a reasonable starting point would be to create a PARTITION uclass and rework things to deal with that. > > > Alex > Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Hi Heinrich,
On Wed, 23 Jan 2019 at 10:05, Heinrich Schuchardt wrote:
>
> On 1/22/19 8:39 PM, Simon Glass wrote:
> > Hi Alex,
> >
> > On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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]).
>
> [1] https://lists.denx.de/pipermail/u-boot/2019-January/354359.html
> [RFC] Device model for block devices - integration with EFI subsystem
>
> Best regards
>
> Heinrich
>
> >
> >>
> >>>
> But we have this annoying interim state where we would lose a few boards
> because they haven't been converted to DM. That's what keeps us from it.
>
> I think what this discussion boils down to is that someone needs to
> start prototyping the DM/EFI integration. Start off with a simple
> subsystem, like BLK.
> >>>
> >>> Even in the current implementation,
> >>> * UEFI disk is implemented using UCLASS_BLK devices
> >>> (We can ignore !CONFIG_BLK case now as we have agreed.)
> >>> * UEFI-specific block device can be seen as UCLASS_BLK/IF_TYPE_EFI
> >>>
> >>> So how essentially different is the *ultimate* goal from the current form
> >>> regarding block devices?
> >>
> >> The ultimate goal is that efi_disk_register() and efi_obj_list disappear.
> >>
> >> Functionality wise we should be 100% identical to today, so all test
> >> cases would still apply the same way as they do now. This is purely
> >> internal rework, nothing visible to UEFI payloads.
> >
> > Yes.
> >
> >>
> >>> In order to identify UEFI disks with u-boot devices transparently, we will
> >>> have to have some sort of *hook* (or hotplug in Alex's language?), either
> >>> in create_block_devices or bind/probe? I don't know, but Simon seems
> >>> to be in denial about this idea.
> >>
> >> Well, if a udevice *is* an efi device, we no longer need hooks. The
> >> object list would simply change.
> >>
> >> We may still need to have event notifications at that stage, but that's
> >> a different story.
> >
> > Yes, it's something that I think will need to be added to DM. I
> > haven't got to this as I have not run into an important use case yet.
> > Maybe something like:
> >
> > Controlled by CONFIG_EVENT
> >
> > - int dev_ev_register(struct udevice *dev, enum event_t type,
> > event_handler_func_t handler, void *userdata)
> >
> > which calls handler(struct udevice *dev, void *userdata) when an event is
> > fired
> >
> > - int dev_ev_unregister() to unregister
> >
> > - int dev_ev_send(struct udevice *dev, enum struct event_info *info)
> >
> > which sends events to registered listeners.
> >
> > struct event_info {
> > enum event_t type;
> > union {
> > struct
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On 01/22/2019 08:39 PM, Simon Glass wrote: Hi Alex, On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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 ... and then we need to create our own sub object model around the UCLASS_EFI devices again. I' not convinced that's a great idea yet :). I really see little reason not to just expose every dm device as EFI handle. Things would plug in quite naturally I think. But either way, someone would need to sit down and prototype things to be sure. Alex ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On 01/23/2019 09:12 AM, AKASHI Takahiro wrote: Alex, On Tue, Jan 22, 2019 at 10:08:53AM +0100, Alexander Graf 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. But we have this annoying interim state where we would lose a few boards because they haven't been converted to DM. That's what keeps us from it. I think what this discussion boils down to is that someone needs to start prototyping the DM/EFI integration. Start off with a simple subsystem, like BLK. Even in the current implementation, * UEFI disk is implemented using UCLASS_BLK devices (We can ignore !CONFIG_BLK case now as we have agreed.) * UEFI-specific block device can be seen as UCLASS_BLK/IF_TYPE_EFI So how essentially different is the *ultimate* goal from the current form regarding block devices? The ultimate goal is that efi_disk_register() and efi_obj_list disappear. Functionality wise we should be 100% identical to today, so all test cases would still apply the same way as they do now. This is purely internal rework, nothing visible to UEFI payloads. In order to identify UEFI disks with u-boot devices transparently, we will have to have some sort of *hook* (or hotplug in Alex's language?), either in create_block_devices or bind/probe? I don't know, but Simon seems to be in denial about this idea. Well, if a udevice *is* an efi device, we no longer need hooks. The object list would simply change. We may still need to have event notifications at that stage, but that's a different story. As transitioning period, we could probably implement 2 efi object roots: efi_obj_list as well as the udevice based one. Every piece of code that iterates through devices then just iterates over both. That way we should be able to slowly move devices from the old object model to the new one. Then provide a DM path and have a non-DM fallback still in its own source file that also provides EFI BLK devices. Eventually we just remove the latter. That way we can then work on getting hotplug working in the DM path, which is the one we want anyway. For non-DM, you simply miss out on that amazing new feature, but we don't regress users. So, first, let me reply to each of your comments. Through this process, I hope we will have better understandings of long-term solution as well as a tentative fix. On Thu, Jan 10, 2019 at 10:22:04AM +0100, Alexander Graf wrote: Am 10.01.2019 um 10:16 schrieb AKASHI Takahiro : On Thu, Jan 10, 2019 at 09:15:36AM +0100, Alexander Graf wrote: Am 10.01.2019 um 09:02 schrieb AKASHI Takahiro : On Thu, Jan 10, 2019 at 08:30:13AM +0100, Alexander Graf wrote: On 10.01.19 08:26, AKASHI Takahiro wrote: Alex, On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote: On 10.01.19 03:13, AKASHI Takahiro wrote: Alex, On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote: On 13.12.18 08:58, AKASHI Takahiro wrote: Heinrich, On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote: On 11/15/18 5:58 AM, AKASHI Takahiro wrote: Currently, efi_init_obj_list() scan disk devices only once, and never change a list of efi disk devices. This will possibly result in failing to find a removable storage which may be added later on. See [1]. In this patch, called is efi_disk_update() which is responsible for re-scanning UCLASS_BLK devices and removing/adding efi disks if necessary. For example, => efishell devices Scanning disk pci_mmc.blk... Found 3 disks Device Name /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) => usb start starting USB... USB0: USB EHCI 1.00 scanning bus 0 for devices... 3 USB Device(s) found scanning usb for storage devices... 1 Storage Device(s) found => efishell devices Scanning disk usb_mass_storage.lun0... Device Name /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Alex, On Tue, Jan 22, 2019 at 10:08:53AM +0100, Alexander Graf 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. > > > > >> But we have this annoying interim state where we would lose a few boards > >> because they haven't been converted to DM. That's what keeps us from it. > >> > >> I think what this discussion boils down to is that someone needs to > >> start prototyping the DM/EFI integration. Start off with a simple > >> subsystem, like BLK. > > > > Even in the current implementation, > > * UEFI disk is implemented using UCLASS_BLK devices > > (We can ignore !CONFIG_BLK case now as we have agreed.) > > * UEFI-specific block device can be seen as UCLASS_BLK/IF_TYPE_EFI > > > > So how essentially different is the *ultimate* goal from the current form > > regarding block devices? > > The ultimate goal is that efi_disk_register() and efi_obj_list disappear. > > Functionality wise we should be 100% identical to today, so all test > cases would still apply the same way as they do now. This is purely > internal rework, nothing visible to UEFI payloads. > > > In order to identify UEFI disks with u-boot devices transparently, we will > > have to have some sort of *hook* (or hotplug in Alex's language?), either > > in create_block_devices or bind/probe? I don't know, but Simon seems > > to be in denial about this idea. > > Well, if a udevice *is* an efi device, we no longer need hooks. The > object list would simply change. > > We may still need to have event notifications at that stage, but that's > a different story. > > As transitioning period, we could probably implement 2 efi object roots: > efi_obj_list as well as the udevice based one. Every piece of code that > iterates through devices then just iterates over both. That way we > should be able to slowly move devices from the old object model to the > new one. > > >> Then provide a DM path and have a non-DM fallback > >> still in its own source file that also provides EFI BLK devices. > >> Eventually we just remove the latter. > >> > >> That way we can then work on getting hotplug working in the DM path, > >> which is the one we want anyway. For non-DM, you simply miss out on that > >> amazing new feature, but we don't regress users. > >> > >>> So, first, let me reply to each of your comments. > >>> Through this process, I hope we will have better understandings > >>> of long-term solution as well as a tentative fix. > >>> > >>> On Thu, Jan 10, 2019 at 10:22:04AM +0100, Alexander Graf wrote: > > > > Am 10.01.2019 um 10:16 schrieb AKASHI Takahiro > > : > > > >> On Thu, Jan 10, 2019 at 09:15:36AM +0100, Alexander Graf wrote: > >> > >> > Am 10.01.2019 um 09:02 schrieb AKASHI Takahiro > : > > On Thu, Jan 10, 2019 at 08:30:13AM +0100, Alexander Graf wrote: > > > > On 10.01.19 08:26, AKASHI Takahiro wrote: > > Alex, > > > >> On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote: > >> > >> > >>> On 10.01.19 03:13, AKASHI Takahiro wrote: > >>> Alex, > >>> > On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote: > > > > On 13.12.18 08:58, AKASHI Takahiro wrote: > > Heinrich, > > > >>> On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt > >>> wrote: > >>> On 11/15/18 5:58 AM, AKASHI Takahiro wrote: > >>> Currently, efi_init_obj_list() scan disk devices only once, > >>> and never > >>> change a list of efi disk devices. This will possibly result > >>> in failing > >>> to find a removable storage which may be added later on. See > >>> [1]. > >>> > >>> In this patch, called is efi_disk_update() which is > >>> responsible for > >>> re-scanning UCLASS_BLK devices and removing/adding efi disks > >>> if necessary. >
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On Tue, Jan 22, 2019 at 10:04:55PM +0100, Heinrich Schuchardt wrote: > On 1/22/19 8:39 PM, Simon Glass wrote: > > Hi Alex, > > > > On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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. > > In the EFI world partitions are devices. How does this fit into your > picture? This is one of my concerns, too. The only solution, I can image, in the *ultimate* goal is that all the partitions of block devices be converted to *real* block devices at probe() or any other enumerating processes, say, "mmc rescan" or "usb start." This, however, requires an extensive change across all the file system types. -Takahiro Akashi > [1] https://lists.denx.de/pipermail/u-boot/2019-January/354359.html > [RFC] Device model for block devices - integration with EFI subsystem > > Best regards > > Heinrich > > > > >> > >>> > But we have this annoying interim state where we would lose a few boards > because they haven't been converted to DM. That's what keeps us from it. > > I think what this discussion boils down to is that someone needs to > start prototyping the DM/EFI integration. Start off with a simple > subsystem, like BLK. > >>> > >>> Even in the current implementation, > >>> * UEFI disk is implemented using UCLASS_BLK devices > >>> (We can ignore !CONFIG_BLK case now as we have agreed.) > >>> * UEFI-specific block device can be seen as UCLASS_BLK/IF_TYPE_EFI > >>> > >>> So how essentially different is the *ultimate* goal from the current form > >>> regarding block devices? > >> > >> The ultimate goal is that efi_disk_register() and efi_obj_list disappear. > >> > >> Functionality wise we should be 100% identical to today, so all test > >> cases would still apply the same way as they do now. This is purely > >> internal rework, nothing visible to UEFI payloads. > > > > Yes. > > > >> > >>> In order to identify UEFI disks with u-boot devices transparently, we will > >>> have to have some sort of *hook* (or hotplug in Alex's language?), either > >>> in create_block_devices or bind/probe? I don't know, but Simon seems > >>> to be in denial about this idea. > >> > >> Well, if a udevice *is* an efi device, we no longer need hooks. The > >> object list would simply change. > >> > >> We may still need to have event notifications at that stage, but that's > >> a different story. > > > > Yes, it's something that I think will need to be added to DM. I > > haven't got to this as I have not run into an important use case yet. > > Maybe something like: > > > > Controlled by CONFIG_EVENT > > > > - int dev_ev_register(struct udevice *dev, enum event_t type, > > event_handler_func_t handler, void *userdata) > > > > which calls handler(struct udevice *dev, void *userdata) when an event is > > fired > > > > - int dev_ev_unregister() to unregister > > > > - int dev_ev_send(struct udevice *dev, enum struct
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On 1/22/19 8:39 PM, Simon Glass wrote:
> Hi Alex,
>
> On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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.
In the EFI world partitions are devices. How does this fit into your
picture?
[1] https://lists.denx.de/pipermail/u-boot/2019-January/354359.html
[RFC] Device model for block devices - integration with EFI subsystem
Best regards
Heinrich
>
>>
>>>
But we have this annoying interim state where we would lose a few boards
because they haven't been converted to DM. That's what keeps us from it.
I think what this discussion boils down to is that someone needs to
start prototyping the DM/EFI integration. Start off with a simple
subsystem, like BLK.
>>>
>>> Even in the current implementation,
>>> * UEFI disk is implemented using UCLASS_BLK devices
>>> (We can ignore !CONFIG_BLK case now as we have agreed.)
>>> * UEFI-specific block device can be seen as UCLASS_BLK/IF_TYPE_EFI
>>>
>>> So how essentially different is the *ultimate* goal from the current form
>>> regarding block devices?
>>
>> The ultimate goal is that efi_disk_register() and efi_obj_list disappear.
>>
>> Functionality wise we should be 100% identical to today, so all test
>> cases would still apply the same way as they do now. This is purely
>> internal rework, nothing visible to UEFI payloads.
>
> Yes.
>
>>
>>> In order to identify UEFI disks with u-boot devices transparently, we will
>>> have to have some sort of *hook* (or hotplug in Alex's language?), either
>>> in create_block_devices or bind/probe? I don't know, but Simon seems
>>> to be in denial about this idea.
>>
>> Well, if a udevice *is* an efi device, we no longer need hooks. The
>> object list would simply change.
>>
>> We may still need to have event notifications at that stage, but that's
>> a different story.
>
> Yes, it's something that I think will need to be added to DM. I
> haven't got to this as I have not run into an important use case yet.
> Maybe something like:
>
> Controlled by CONFIG_EVENT
>
> - int dev_ev_register(struct udevice *dev, enum event_t type,
> event_handler_func_t handler, void *userdata)
>
> which calls handler(struct udevice *dev, void *userdata) when an event is
> fired
>
> - int dev_ev_unregister() to unregister
>
> - int dev_ev_send(struct udevice *dev, enum struct event_info *info)
>
> which sends events to registered listeners.
>
> struct event_info {
> enum event_t type;
> union {
> struct ev_data_probed probed;
> struct ev_data_removed removed;
> ...
> } d;
> };
>
>>
>> As transitioning period, we could probably implement 2 efi object roots:
>> efi_obj_list as well as the udevice based one. Every piece of code that
>> iterates through devices then just iterates over both. That way we
>> should be able to slowly move devices from the old object model to the
>> new one.
>
> Will the uclass I mentioned above you can iterate through UCLASS_EFI
> and thus you have a list of EFI devices.
>
> [...]
>
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Hi Alex,
On Tue, 22 Jan 2019 at 22:08, Alexander Graf 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.
>
> >
> >> But we have this annoying interim state where we would lose a few boards
> >> because they haven't been converted to DM. That's what keeps us from it.
> >>
> >> I think what this discussion boils down to is that someone needs to
> >> start prototyping the DM/EFI integration. Start off with a simple
> >> subsystem, like BLK.
> >
> > Even in the current implementation,
> > * UEFI disk is implemented using UCLASS_BLK devices
> > (We can ignore !CONFIG_BLK case now as we have agreed.)
> > * UEFI-specific block device can be seen as UCLASS_BLK/IF_TYPE_EFI
> >
> > So how essentially different is the *ultimate* goal from the current form
> > regarding block devices?
>
> The ultimate goal is that efi_disk_register() and efi_obj_list disappear.
>
> Functionality wise we should be 100% identical to today, so all test
> cases would still apply the same way as they do now. This is purely
> internal rework, nothing visible to UEFI payloads.
Yes.
>
> > In order to identify UEFI disks with u-boot devices transparently, we will
> > have to have some sort of *hook* (or hotplug in Alex's language?), either
> > in create_block_devices or bind/probe? I don't know, but Simon seems
> > to be in denial about this idea.
>
> Well, if a udevice *is* an efi device, we no longer need hooks. The
> object list would simply change.
>
> We may still need to have event notifications at that stage, but that's
> a different story.
Yes, it's something that I think will need to be added to DM. I
haven't got to this as I have not run into an important use case yet.
Maybe something like:
Controlled by CONFIG_EVENT
- int dev_ev_register(struct udevice *dev, enum event_t type,
event_handler_func_t handler, void *userdata)
which calls handler(struct udevice *dev, void *userdata) when an event is fired
- int dev_ev_unregister() to unregister
- int dev_ev_send(struct udevice *dev, enum struct event_info *info)
which sends events to registered listeners.
struct event_info {
enum event_t type;
union {
struct ev_data_probed probed;
struct ev_data_removed removed;
...
} d;
};
>
> As transitioning period, we could probably implement 2 efi object roots:
> efi_obj_list as well as the udevice based one. Every piece of code that
> iterates through devices then just iterates over both. That way we
> should be able to slowly move devices from the old object model to the
> new one.
Will the uclass I mentioned above you can iterate through UCLASS_EFI
and thus you have a list of EFI devices.
[...]
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
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. > >> But we have this annoying interim state where we would lose a few boards >> because they haven't been converted to DM. That's what keeps us from it. >> >> I think what this discussion boils down to is that someone needs to >> start prototyping the DM/EFI integration. Start off with a simple >> subsystem, like BLK. > > Even in the current implementation, > * UEFI disk is implemented using UCLASS_BLK devices > (We can ignore !CONFIG_BLK case now as we have agreed.) > * UEFI-specific block device can be seen as UCLASS_BLK/IF_TYPE_EFI > > So how essentially different is the *ultimate* goal from the current form > regarding block devices? The ultimate goal is that efi_disk_register() and efi_obj_list disappear. Functionality wise we should be 100% identical to today, so all test cases would still apply the same way as they do now. This is purely internal rework, nothing visible to UEFI payloads. > In order to identify UEFI disks with u-boot devices transparently, we will > have to have some sort of *hook* (or hotplug in Alex's language?), either > in create_block_devices or bind/probe? I don't know, but Simon seems > to be in denial about this idea. Well, if a udevice *is* an efi device, we no longer need hooks. The object list would simply change. We may still need to have event notifications at that stage, but that's a different story. As transitioning period, we could probably implement 2 efi object roots: efi_obj_list as well as the udevice based one. Every piece of code that iterates through devices then just iterates over both. That way we should be able to slowly move devices from the old object model to the new one. >> Then provide a DM path and have a non-DM fallback >> still in its own source file that also provides EFI BLK devices. >> Eventually we just remove the latter. >> >> That way we can then work on getting hotplug working in the DM path, >> which is the one we want anyway. For non-DM, you simply miss out on that >> amazing new feature, but we don't regress users. >> >>> So, first, let me reply to each of your comments. >>> Through this process, I hope we will have better understandings >>> of long-term solution as well as a tentative fix. >>> >>> On Thu, Jan 10, 2019 at 10:22:04AM +0100, Alexander Graf wrote: > Am 10.01.2019 um 10:16 schrieb AKASHI Takahiro > : > >> On Thu, Jan 10, 2019 at 09:15:36AM +0100, Alexander Graf wrote: >> >> Am 10.01.2019 um 09:02 schrieb AKASHI Takahiro : On Thu, Jan 10, 2019 at 08:30:13AM +0100, Alexander Graf wrote: > On 10.01.19 08:26, AKASHI Takahiro wrote: > Alex, > >> On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote: >> >> >>> On 10.01.19 03:13, AKASHI Takahiro wrote: >>> Alex, >>> On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote: > On 13.12.18 08:58, AKASHI Takahiro wrote: > Heinrich, > >>> On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt >>> wrote: >>> On 11/15/18 5:58 AM, AKASHI Takahiro wrote: >>> Currently, efi_init_obj_list() scan disk devices only once, and >>> never >>> change a list of efi disk devices. This will possibly result in >>> failing >>> to find a removable storage which may be added later on. See >>> [1]. >>> >>> In this patch, called is efi_disk_update() which is responsible >>> for >>> re-scanning UCLASS_BLK devices and removing/adding efi disks if >>> necessary. >>> >>> For example, >>> >>> => efishell devices >>> Scanning disk pci_mmc.blk... >>> Found 3 disks >>> Device Name >>> >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) >>>
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
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. > But we have this annoying interim state where we would lose a few boards > because they haven't been converted to DM. That's what keeps us from it. > > I think what this discussion boils down to is that someone needs to > start prototyping the DM/EFI integration. Start off with a simple > subsystem, like BLK. Even in the current implementation, * UEFI disk is implemented using UCLASS_BLK devices (We can ignore !CONFIG_BLK case now as we have agreed.) * UEFI-specific block device can be seen as UCLASS_BLK/IF_TYPE_EFI So how essentially different is the *ultimate* goal from the current form regarding block devices? In order to identify UEFI disks with u-boot devices transparently, we will have to have some sort of *hook* (or hotplug in Alex's language?), either in create_block_devices or bind/probe? I don't know, but Simon seems to be in denial about this idea. > Then provide a DM path and have a non-DM fallback > still in its own source file that also provides EFI BLK devices. > Eventually we just remove the latter. > > That way we can then work on getting hotplug working in the DM path, > which is the one we want anyway. For non-DM, you simply miss out on that > amazing new feature, but we don't regress users. > > > So, first, let me reply to each of your comments. > > Through this process, I hope we will have better understandings > > of long-term solution as well as a tentative fix. > > > > On Thu, Jan 10, 2019 at 10:22:04AM +0100, Alexander Graf wrote: > >> > >> > >>> Am 10.01.2019 um 10:16 schrieb AKASHI Takahiro > >>> : > >>> > On Thu, Jan 10, 2019 at 09:15:36AM +0100, Alexander Graf wrote: > > > >> Am 10.01.2019 um 09:02 schrieb AKASHI Takahiro > >> : > >> > >> On Thu, Jan 10, 2019 at 08:30:13AM +0100, Alexander Graf wrote: > >> > >> > >>> On 10.01.19 08:26, AKASHI Takahiro wrote: > >>> Alex, > >>> > On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote: > > > > On 10.01.19 03:13, AKASHI Takahiro wrote: > > Alex, > > > >> On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote: > >> > >> > >>> On 13.12.18 08:58, AKASHI Takahiro wrote: > >>> Heinrich, > >>> > > On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt > > wrote: > > On 11/15/18 5:58 AM, AKASHI Takahiro wrote: > > Currently, efi_init_obj_list() scan disk devices only once, and > > never > > change a list of efi disk devices. This will possibly result in > > failing > > to find a removable storage which may be added later on. See > > [1]. > > > > In this patch, called is efi_disk_update() which is responsible > > for > > re-scanning UCLASS_BLK devices and removing/adding efi disks if > > necessary. > > > > For example, > > > > => efishell devices > > Scanning disk pci_mmc.blk... > > Found 3 disks > > Device Name > > > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > > => usb start > > starting USB... > > USB0: USB EHCI 1.00 > > scanning bus 0 for devices... 3 USB Device(s) found > > scanning usb for storage devices... 1 Storage Device(s) > > found > > => efishell devices > > Scanning disk usb_mass_storage.lun0... > > Device Name > > > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c) > > > > Without this patch, the last device, USB mass storage, won't > > show up. > > >
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Hi Alex, On Sat, 12 Jan 2019 at 15:00, Alexander Graf wrote: > > > > > Am 12.01.2019 um 22:32 schrieb Simon Glass : > > > > Hi Alex, > > > >> On Fri, 11 Jan 2019 at 00:57, 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. > >> > >> But we have this annoying interim state where we would lose a few boards > >> because they haven't been converted to DM. That's what keeps us from it. > > > > I don't think that is true anymore. The deadline for patches is > > effectively 28th January. > > > >> > >> I think what this discussion boils down to is that someone needs to > >> start prototyping the DM/EFI integration. Start off with a simple > >> subsystem, like BLK. Then provide a DM path and have a non-DM fallback > >> still in its own source file that also provides EFI BLK devices. > >> Eventually we just remove the latter. > > > > No fallback, please. As above, it is time to remove code that does not > > use CONFIG_BLK. > > > >> > >> That way we can then work on getting hotplug working in the DM path, > >> which is the one we want anyway. For non-DM, you simply miss out on that > >> amazing new feature, but we don't regress users. > > > > Of which there will not be any as of 2019.04. I'm sorry to belabour > > this point, but I feel quite strongly that we should not be adding new > > code to old frameworks. We should instead be migrating away from them > > and deleting them. > > I was thinking of an isolated file that we could just remove eventually. > > But if we're getting to a dm only world with 2019.04 already, I'll be happy > to merge code that merges efi and dm objects for that release. > > The one thing I do not want here is any functional regression. If a board > worked with efi in 2019.01, it better works at least as well in 2019.04. So long as it supports BLK, yes. If it doesn't then I suppose it will be removed anyway. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
> Am 12.01.2019 um 22:32 schrieb Simon Glass : > > Hi Alex, > >> On Fri, 11 Jan 2019 at 00:57, 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. >> >> But we have this annoying interim state where we would lose a few boards >> because they haven't been converted to DM. That's what keeps us from it. > > I don't think that is true anymore. The deadline for patches is > effectively 28th January. > >> >> I think what this discussion boils down to is that someone needs to >> start prototyping the DM/EFI integration. Start off with a simple >> subsystem, like BLK. Then provide a DM path and have a non-DM fallback >> still in its own source file that also provides EFI BLK devices. >> Eventually we just remove the latter. > > No fallback, please. As above, it is time to remove code that does not > use CONFIG_BLK. > >> >> That way we can then work on getting hotplug working in the DM path, >> which is the one we want anyway. For non-DM, you simply miss out on that >> amazing new feature, but we don't regress users. > > Of which there will not be any as of 2019.04. I'm sorry to belabour > this point, but I feel quite strongly that we should not be adding new > code to old frameworks. We should instead be migrating away from them > and deleting them. I was thinking of an isolated file that we could just remove eventually. But if we're getting to a dm only world with 2019.04 already, I'll be happy to merge code that merges efi and dm objects for that release. The one thing I do not want here is any functional regression. If a board worked with efi in 2019.01, it better works at least as well in 2019.04. Alex ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Hi Alex, On Fri, 11 Jan 2019 at 00:57, 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. > > But we have this annoying interim state where we would lose a few boards > because they haven't been converted to DM. That's what keeps us from it. I don't think that is true anymore. The deadline for patches is effectively 28th January. > > I think what this discussion boils down to is that someone needs to > start prototyping the DM/EFI integration. Start off with a simple > subsystem, like BLK. Then provide a DM path and have a non-DM fallback > still in its own source file that also provides EFI BLK devices. > Eventually we just remove the latter. No fallback, please. As above, it is time to remove code that does not use CONFIG_BLK. > > That way we can then work on getting hotplug working in the DM path, > which is the one we want anyway. For non-DM, you simply miss out on that > amazing new feature, but we don't regress users. Of which there will not be any as of 2019.04. I'm sorry to belabour this point, but I feel quite strongly that we should not be adding new code to old frameworks. We should instead be migrating away from them and deleting them. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
> From: Alexander Graf > Date: Fri, 11 Jan 2019 09:00:27 +0100 > > On 11.01.19 05:51, AKASHI Takahiro wrote: > > On Thu, Jan 10, 2019 at 05:57:11AM -0700, Simon Glass wrote: > >> Hi, > >> > >> > >> Please no. We don't want EFI hooks around the place. EFI should use > >> DM, not the other way around. > > > > Right, but every efi disk is associated with a backing "u-boot" > > block devices (even more, this is not 1-to-1 mapping but many-to-1 due to > > partitions). > > In this case we may just want to create DM devices for partitions as > well to make things more natural. Be careful here though that this doesn't lead to device paths for partitions that have a different "root" than the disk itself. The OpenBSD bootloader relies on matching the initial EFI device path components to find the entire disk from the partition it was loaded from to find the disk from which to load the kernel. Cheers, Mark ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On 11.01.19 05:51, AKASHI Takahiro wrote:
> On Thu, Jan 10, 2019 at 05:57:11AM -0700, Simon Glass wrote:
>> Hi,
>>
>> On Wed, 9 Jan 2019 at 02:06, Alexander Graf wrote:
>>>
>>>
>>>
>>> On 13.12.18 08:58, AKASHI Takahiro wrote:
Heinrich,
On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote:
> On 11/15/18 5:58 AM, AKASHI Takahiro wrote:
>> Currently, efi_init_obj_list() scan disk devices only once, and never
>> change a list of efi disk devices. This will possibly result in failing
>> to find a removable storage which may be added later on. See [1].
>>
>> In this patch, called is efi_disk_update() which is responsible for
>> re-scanning UCLASS_BLK devices and removing/adding efi disks if
>> necessary.
>>
>> For example,
>>
>> => efishell devices
>> Scanning disk pci_mmc.blk...
>> Found 3 disks
>> Device Name
>>
>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
>> => usb start
>> starting USB...
>> USB0: USB EHCI 1.00
>> scanning bus 0 for devices... 3 USB Device(s) found
>>scanning usb for storage devices... 1 Storage Device(s) found
>> => efishell devices
>> Scanning disk usb_mass_storage.lun0...
>> Device Name
>>
>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c)
>>
>> Without this patch, the last device, USB mass storage, won't show up.
>>
>> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
>>
>> Signed-off-by: AKASHI Takahiro
>
> Why should we try to fix something in the EFI subsystems that goes wrong
> in the handling of device enumeration.
No.
This is a natural result from how efi disks are currently implemented on
u-boot.
Do you want to totally re-write/re-implement efi disks?
>>>
>>> Could we just make this event based for now? Call a hook from the
>>> storage dm subsystem when a new u-boot block device gets created to
>>> issue a sync of that in the efi subsystem?
>>>
>>
>> Please no. We don't want EFI hooks around the place. EFI should use
>> DM, not the other way around.
>
> Right, but every efi disk is associated with a backing "u-boot"
> block devices (even more, this is not 1-to-1 mapping but many-to-1 due to
> partitions).
In this case we may just want to create DM devices for partitions as
well to make things more natural.
> Without any sort of event/hook mechanism, we can know of all block
> devices only by enumerating them at some point as in my current
> approach. Do you want to accept this?
>
> (Even in a hacky way. See efi_disk_register():
> for (if_type = 0; if_type < IF_TYPE_COUNT; if_type++)
> ...
> printf("Scanning disks on %s...\n", if_typename);
> for (i = 0; i < 4; i++) {<= !!!
> desc = blk_get_devnum_by_type(if_type, i);
> ...
> )
>
>>> That hook would obviously only do something (or get registered?) when
> i
>>> the efi object stack is initialized.
>>>
>>> The long term goal IMHO should still be though to just merge DM and EFI
>>> objects. But we're still waiting on the deprecation of non-DM devices
>>> for that.
>>
>> I think think 'merge' is the right word. Perhaps 'create EFI devices
>> in DM' is a better term.
>
> How different is your idea from UCLASS_BLK device(efi_block_device.c)?
The UCLASS_BLK is the reverse path. It's exposing a DM device from an
EFI one.
Alex
>
> The current implementation of UCLASS_BLK, in my opinion, is somehow
> in a half way; an instance of UCLASS_BLK is created only when a
> efi driver is bound to a controller.
> In the meantime, efi disks (efi objects which support UEFI_BLOCK_IO_PROTOCOL)
> is implicitly created in efi_disk_add_dev() without any *binding*.
>
>> Anyway, let's do that now. As I may have mentioned we should never
>> have enabled EFI on pre-DM boards :-) It has just lead to duplication.
>>
>> In any case, the migration deadline for DM_MMC (for example) is the
>> upcoming merge window, so the time is now.
>>
>> As things stand this patch looks reasonable to me. It is a natural
>> consequence of duplicating the DM tables.
>
> I think so, yes.
>
> -Takahiro Akashi
>
>> Regards,
>> Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
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. But we have this annoying interim state where we would lose a few boards because they haven't been converted to DM. That's what keeps us from it. I think what this discussion boils down to is that someone needs to start prototyping the DM/EFI integration. Start off with a simple subsystem, like BLK. Then provide a DM path and have a non-DM fallback still in its own source file that also provides EFI BLK devices. Eventually we just remove the latter. That way we can then work on getting hotplug working in the DM path, which is the one we want anyway. For non-DM, you simply miss out on that amazing new feature, but we don't regress users. > So, first, let me reply to each of your comments. > Through this process, I hope we will have better understandings > of long-term solution as well as a tentative fix. > > On Thu, Jan 10, 2019 at 10:22:04AM +0100, Alexander Graf wrote: >> >> >>> Am 10.01.2019 um 10:16 schrieb AKASHI Takahiro : >>> On Thu, Jan 10, 2019 at 09:15:36AM +0100, Alexander Graf wrote: >> Am 10.01.2019 um 09:02 schrieb AKASHI Takahiro >> : >> >> On Thu, Jan 10, 2019 at 08:30:13AM +0100, Alexander Graf wrote: >> >> >>> On 10.01.19 08:26, AKASHI Takahiro wrote: >>> Alex, >>> On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote: > On 10.01.19 03:13, AKASHI Takahiro wrote: > Alex, > >> On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote: >> >> >>> On 13.12.18 08:58, AKASHI Takahiro wrote: >>> Heinrich, >>> > On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt > wrote: > On 11/15/18 5:58 AM, AKASHI Takahiro wrote: > Currently, efi_init_obj_list() scan disk devices only once, and > never > change a list of efi disk devices. This will possibly result in > failing > to find a removable storage which may be added later on. See [1]. > > In this patch, called is efi_disk_update() which is responsible > for > re-scanning UCLASS_BLK devices and removing/adding efi disks if > necessary. > > For example, > > => efishell devices > Scanning disk pci_mmc.blk... > Found 3 disks > Device Name > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > => usb start > starting USB... > USB0: USB EHCI 1.00 > scanning bus 0 for devices... 3 USB Device(s) found > scanning usb for storage devices... 1 Storage Device(s) found > => efishell devices > Scanning disk usb_mass_storage.lun0... > Device Name > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c) > > Without this patch, the last device, USB mass storage, won't show > up. > > [1] > https://lists.denx.de/pipermail/u-boot/2018-October/345307.html > > Signed-off-by: AKASHI Takahiro Why should we try to fix something in the EFI subsystems that goes wrong in the handling of device enumeration. >>> >>> No. >>> This is a natural result from how efi disks are currently >>> implemented on u-boot. >>> Do you want to totally re-write/re-implement efi disks? >> >> Could we just make this event based for now? Call a hook from the >> storage dm subsystem when a new u-boot block device gets created to >> issue a sync of that in the efi subsystem? > > If I correctly understand you, your suggestion here corresponds > with my proposal#3 in [1] while my current approach is #2. > > [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Heinrich, On Thu, Jan 10, 2019 at 08:22:25PM +0100, Heinrich Schuchardt wrote: > On 1/10/19 10:22 AM, Alexander Graf wrote: > > > > > >> Am 10.01.2019 um 10:16 schrieb AKASHI Takahiro > >> : > >> > >>> On Thu, Jan 10, 2019 at 09:15:36AM +0100, Alexander Graf wrote: > >>> > >>> > > Am 10.01.2019 um 09:02 schrieb AKASHI Takahiro > > : > > > > On Thu, Jan 10, 2019 at 08:30:13AM +0100, Alexander Graf wrote: > > > > > >> On 10.01.19 08:26, AKASHI Takahiro wrote: > >> Alex, > >> > >>> On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote: > >>> > >>> > On 10.01.19 03:13, AKASHI Takahiro wrote: > Alex, > > > On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote: > > > > > >> On 13.12.18 08:58, AKASHI Takahiro wrote: > >> Heinrich, > >> > On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt > wrote: > On 11/15/18 5:58 AM, AKASHI Takahiro wrote: > Currently, efi_init_obj_list() scan disk devices only once, and > never > change a list of efi disk devices. This will possibly result in > failing > to find a removable storage which may be added later on. See [1]. > > In this patch, called is efi_disk_update() which is responsible > for > re-scanning UCLASS_BLK devices and removing/adding efi disks if > necessary. > > For example, > > => efishell devices > Scanning disk pci_mmc.blk... > Found 3 disks > Device Name > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > => usb start > starting USB... > USB0: USB EHCI 1.00 > scanning bus 0 for devices... 3 USB Device(s) found > scanning usb for storage devices... 1 Storage Device(s) > found > => efishell devices > Scanning disk usb_mass_storage.lun0... > Device Name > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c) > > Without this patch, the last device, USB mass storage, won't > show up. > > [1] > https://lists.denx.de/pipermail/u-boot/2018-October/345307.html > > Signed-off-by: AKASHI Takahiro > >>> > >>> Why should we try to fix something in the EFI subsystems that > >>> goes wrong > >>> in the handling of device enumeration. > >> > >> No. > >> This is a natural result from how efi disks are currently > >> implemented on u-boot. > >> Do you want to totally re-write/re-implement efi disks? > > > > Could we just make this event based for now? Call a hook from the > > storage dm subsystem when a new u-boot block device gets created to > > issue a sync of that in the efi subsystem? > > If I correctly understand you, your suggestion here corresponds > with my proposal#3 in [1] while my current approach is #2. > > [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html > >>> > >>> Yes, I think so. > >>> > So we will call, say, efi_disk_create(struct udevice *) in > blk_create_device() and efi_dsik_delete() in blk_unbind_all(). > >>> > >>> I would prefer if we didn't call them directly, but through an event > >>> mechanism. So the efi_disk subsystem registers an event with the dm > >>> block subsystem and that will just call all events when block devices > >>> get created which will automatically also include the efi disk > >>> creation > >>> callback. Same for reverse. > >> > >> Do you mean efi event by "event?" > >> (I don't think there is any generic event interface on DM side.) > >> > >> Whatever an "event" is or whether we call efi_disk_create() directly > >> or indirectly via an event, there is one (big?) issue in this approach > >> (while I've almost finished prototyping): > >> > >> We cannot call efi_disk_create() within blk_create_device() because > >>
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On Thu, Jan 10, 2019 at 05:57:11AM -0700, Simon Glass wrote:
> Hi,
>
> On Wed, 9 Jan 2019 at 02:06, Alexander Graf wrote:
> >
> >
> >
> > On 13.12.18 08:58, AKASHI Takahiro wrote:
> > > Heinrich,
> > >
> > > On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote:
> > >> On 11/15/18 5:58 AM, AKASHI Takahiro wrote:
> > >>> Currently, efi_init_obj_list() scan disk devices only once, and never
> > >>> change a list of efi disk devices. This will possibly result in failing
> > >>> to find a removable storage which may be added later on. See [1].
> > >>>
> > >>> In this patch, called is efi_disk_update() which is responsible for
> > >>> re-scanning UCLASS_BLK devices and removing/adding efi disks if
> > >>> necessary.
> > >>>
> > >>> For example,
> > >>>
> > >>> => efishell devices
> > >>> Scanning disk pci_mmc.blk...
> > >>> Found 3 disks
> > >>> Device Name
> > >>>
> > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> > >>> => usb start
> > >>> starting USB...
> > >>> USB0: USB EHCI 1.00
> > >>> scanning bus 0 for devices... 3 USB Device(s) found
> > >>>scanning usb for storage devices... 1 Storage Device(s) found
> > >>> => efishell devices
> > >>> Scanning disk usb_mass_storage.lun0...
> > >>> Device Name
> > >>>
> > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c)
> > >>>
> > >>> Without this patch, the last device, USB mass storage, won't show up.
> > >>>
> > >>> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
> > >>>
> > >>> Signed-off-by: AKASHI Takahiro
> > >>
> > >> Why should we try to fix something in the EFI subsystems that goes wrong
> > >> in the handling of device enumeration.
> > >
> > > No.
> > > This is a natural result from how efi disks are currently implemented on
> > > u-boot.
> > > Do you want to totally re-write/re-implement efi disks?
> >
> > Could we just make this event based for now? Call a hook from the
> > storage dm subsystem when a new u-boot block device gets created to
> > issue a sync of that in the efi subsystem?
> >
>
> Please no. We don't want EFI hooks around the place. EFI should use
> DM, not the other way around.
Right, but every efi disk is associated with a backing "u-boot"
block devices (even more, this is not 1-to-1 mapping but many-to-1 due to
partitions).
Without any sort of event/hook mechanism, we can know of all block
devices only by enumerating them at some point as in my current
approach. Do you want to accept this?
(Even in a hacky way. See efi_disk_register():
for (if_type = 0; if_type < IF_TYPE_COUNT; if_type++)
...
printf("Scanning disks on %s...\n", if_typename);
for (i = 0; i < 4; i++) {<= !!!
desc = blk_get_devnum_by_type(if_type, i);
...
)
> > That hook would obviously only do something (or get registered?) when
i
> > the efi object stack is initialized.
> >
> > The long term goal IMHO should still be though to just merge DM and EFI
> > objects. But we're still waiting on the deprecation of non-DM devices
> > for that.
>
> I think think 'merge' is the right word. Perhaps 'create EFI devices
> in DM' is a better term.
How different is your idea from UCLASS_BLK device(efi_block_device.c)?
The current implementation of UCLASS_BLK, in my opinion, is somehow
in a half way; an instance of UCLASS_BLK is created only when a
efi driver is bound to a controller.
In the meantime, efi disks (efi objects which support UEFI_BLOCK_IO_PROTOCOL)
is implicitly created in efi_disk_add_dev() without any *binding*.
> Anyway, let's do that now. As I may have mentioned we should never
> have enabled EFI on pre-DM boards :-) It has just lead to duplication.
>
> In any case, the migration deadline for DM_MMC (for example) is the
> upcoming merge window, so the time is now.
>
> As things stand this patch looks reasonable to me. It is a natural
> consequence of duplicating the DM tables.
I think so, yes.
-Takahiro Akashi
> Regards,
> Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
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. So, first, let me reply to each of your comments. Through this process, I hope we will have better understandings of long-term solution as well as a tentative fix. On Thu, Jan 10, 2019 at 10:22:04AM +0100, Alexander Graf wrote: > > > > Am 10.01.2019 um 10:16 schrieb AKASHI Takahiro : > > > >> On Thu, Jan 10, 2019 at 09:15:36AM +0100, Alexander Graf wrote: > >> > >> > Am 10.01.2019 um 09:02 schrieb AKASHI Takahiro > : > > On Thu, Jan 10, 2019 at 08:30:13AM +0100, Alexander Graf wrote: > > > > On 10.01.19 08:26, AKASHI Takahiro wrote: > > Alex, > > > >> On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote: > >> > >> > >>> On 10.01.19 03:13, AKASHI Takahiro wrote: > >>> Alex, > >>> > On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote: > > > > On 13.12.18 08:58, AKASHI Takahiro wrote: > > Heinrich, > > > >>> On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt > >>> wrote: > >>> On 11/15/18 5:58 AM, AKASHI Takahiro wrote: > >>> Currently, efi_init_obj_list() scan disk devices only once, and > >>> never > >>> change a list of efi disk devices. This will possibly result in > >>> failing > >>> to find a removable storage which may be added later on. See [1]. > >>> > >>> In this patch, called is efi_disk_update() which is responsible > >>> for > >>> re-scanning UCLASS_BLK devices and removing/adding efi disks if > >>> necessary. > >>> > >>> For example, > >>> > >>> => efishell devices > >>> Scanning disk pci_mmc.blk... > >>> Found 3 disks > >>> Device Name > >>> > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > >>> => usb start > >>> starting USB... > >>> USB0: USB EHCI 1.00 > >>> scanning bus 0 for devices... 3 USB Device(s) found > >>> scanning usb for storage devices... 1 Storage Device(s) found > >>> => efishell devices > >>> Scanning disk usb_mass_storage.lun0... > >>> Device Name > >>> > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c) > >>> > >>> Without this patch, the last device, USB mass storage, won't show > >>> up. > >>> > >>> [1] > >>> https://lists.denx.de/pipermail/u-boot/2018-October/345307.html > >>> > >>> Signed-off-by: AKASHI Takahiro > >> > >> Why should we try to fix something in the EFI subsystems that goes > >> wrong > >> in the handling of device enumeration. > > > > No. > > This is a natural result from how efi disks are currently > > implemented on u-boot. > > Do you want to totally re-write/re-implement efi disks? > > Could we just make this event based for now? Call a hook from the > storage dm subsystem when a new u-boot block device gets created to > issue a sync of that in the efi subsystem? > >>> > >>> If I correctly understand you, your suggestion here corresponds > >>> with my proposal#3 in [1] while my current approach is #2. > >>> > >>> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html > >> > >> Yes, I think so. > >> > >>> So we will call, say, efi_disk_create(struct udevice *) in > >>> blk_create_device() and efi_dsik_delete() in blk_unbind_all(). > >> > >> I would prefer if we didn't call them directly, but through an event > >> mechanism. So the efi_disk subsystem registers an event with the dm > >> block subsystem and that will just call all events when block devices > >> get created which will automatically also include the efi disk creation > >> callback. Same for reverse. > > > > Do you mean efi event by "event?" > > (I don't think there is any generic event interface on DM side.) > > > > Whatever an "event" is or whether we call efi_disk_create() directly > > or
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On 1/10/19 10:22 AM, Alexander Graf wrote: > > >> Am 10.01.2019 um 10:16 schrieb AKASHI Takahiro : >> >>> On Thu, Jan 10, 2019 at 09:15:36AM +0100, Alexander Graf wrote: >>> >>> > Am 10.01.2019 um 09:02 schrieb AKASHI Takahiro > : > > On Thu, Jan 10, 2019 at 08:30:13AM +0100, Alexander Graf wrote: > > >> On 10.01.19 08:26, AKASHI Takahiro wrote: >> Alex, >> >>> On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote: >>> >>> On 10.01.19 03:13, AKASHI Takahiro wrote: Alex, > On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote: > > >> On 13.12.18 08:58, AKASHI Takahiro wrote: >> Heinrich, >> On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote: On 11/15/18 5:58 AM, AKASHI Takahiro wrote: Currently, efi_init_obj_list() scan disk devices only once, and never change a list of efi disk devices. This will possibly result in failing to find a removable storage which may be added later on. See [1]. In this patch, called is efi_disk_update() which is responsible for re-scanning UCLASS_BLK devices and removing/adding efi disks if necessary. For example, => efishell devices Scanning disk pci_mmc.blk... Found 3 disks Device Name /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) => usb start starting USB... USB0: USB EHCI 1.00 scanning bus 0 for devices... 3 USB Device(s) found scanning usb for storage devices... 1 Storage Device(s) found => efishell devices Scanning disk usb_mass_storage.lun0... Device Name /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c) Without this patch, the last device, USB mass storage, won't show up. [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html Signed-off-by: AKASHI Takahiro >>> >>> Why should we try to fix something in the EFI subsystems that goes >>> wrong >>> in the handling of device enumeration. >> >> No. >> This is a natural result from how efi disks are currently >> implemented on u-boot. >> Do you want to totally re-write/re-implement efi disks? > > Could we just make this event based for now? Call a hook from the > storage dm subsystem when a new u-boot block device gets created to > issue a sync of that in the efi subsystem? If I correctly understand you, your suggestion here corresponds with my proposal#3 in [1] while my current approach is #2. [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html >>> >>> Yes, I think so. >>> So we will call, say, efi_disk_create(struct udevice *) in blk_create_device() and efi_dsik_delete() in blk_unbind_all(). >>> >>> I would prefer if we didn't call them directly, but through an event >>> mechanism. So the efi_disk subsystem registers an event with the dm >>> block subsystem and that will just call all events when block devices >>> get created which will automatically also include the efi disk creation >>> callback. Same for reverse. >> >> Do you mean efi event by "event?" >> (I don't think there is any generic event interface on DM side.) >> >> Whatever an "event" is or whether we call efi_disk_create() directly >> or indirectly via an event, there is one (big?) issue in this approach >> (while I've almost finished prototyping): >> >> We cannot call efi_disk_create() within blk_create_device() because >> some data fields of struct blk_desc, which are to be used by efi disk, >> are initialized *after* blk_create_device() in driver side. >> >> So we need to add a hook at/after every occurrence of blk_create_device() >> on driver side. For example, >> >> === drivers/scsi/scsi.c === >> int do_scsi_scan_one(struct udevice *dev, int id, int
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Hi, On Wed, 9 Jan 2019 at 02:06, Alexander Graf wrote: > > > > On 13.12.18 08:58, AKASHI Takahiro wrote: > > Heinrich, > > > > On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote: > >> On 11/15/18 5:58 AM, AKASHI Takahiro wrote: > >>> Currently, efi_init_obj_list() scan disk devices only once, and never > >>> change a list of efi disk devices. This will possibly result in failing > >>> to find a removable storage which may be added later on. See [1]. > >>> > >>> In this patch, called is efi_disk_update() which is responsible for > >>> re-scanning UCLASS_BLK devices and removing/adding efi disks if necessary. > >>> > >>> For example, > >>> > >>> => efishell devices > >>> Scanning disk pci_mmc.blk... > >>> Found 3 disks > >>> Device Name > >>> > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > >>> => usb start > >>> starting USB... > >>> USB0: USB EHCI 1.00 > >>> scanning bus 0 for devices... 3 USB Device(s) found > >>>scanning usb for storage devices... 1 Storage Device(s) found > >>> => efishell devices > >>> Scanning disk usb_mass_storage.lun0... > >>> Device Name > >>> > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c) > >>> > >>> Without this patch, the last device, USB mass storage, won't show up. > >>> > >>> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html > >>> > >>> Signed-off-by: AKASHI Takahiro > >> > >> Why should we try to fix something in the EFI subsystems that goes wrong > >> in the handling of device enumeration. > > > > No. > > This is a natural result from how efi disks are currently implemented on > > u-boot. > > Do you want to totally re-write/re-implement efi disks? > > Could we just make this event based for now? Call a hook from the > storage dm subsystem when a new u-boot block device gets created to > issue a sync of that in the efi subsystem? > Please no. We don't want EFI hooks around the place. EFI should use DM, not the other way around. > That hook would obviously only do something (or get registered?) when > the efi object stack is initialized. > > The long term goal IMHO should still be though to just merge DM and EFI > objects. But we're still waiting on the deprecation of non-DM devices > for that. I think think 'merge' is the right word. Perhaps 'create EFI devices in DM' is a better term. Anyway, let's do that now. As I may have mentioned we should never have enabled EFI on pre-DM boards :-) It has just lead to duplication. In any case, the migration deadline for DM_MMC (for example) is the upcoming merge window, so the time is now. As things stand this patch looks reasonable to me. It is a natural consequence of duplicating the DM tables. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
> Am 10.01.2019 um 10:16 schrieb AKASHI Takahiro :
>
>> On Thu, Jan 10, 2019 at 09:15:36AM +0100, Alexander Graf wrote:
>>
>>
Am 10.01.2019 um 09:02 schrieb AKASHI Takahiro
:
On Thu, Jan 10, 2019 at 08:30:13AM +0100, Alexander Graf wrote:
> On 10.01.19 08:26, AKASHI Takahiro wrote:
> Alex,
>
>> On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote:
>>
>>
>>> On 10.01.19 03:13, AKASHI Takahiro wrote:
>>> Alex,
>>>
On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote:
> On 13.12.18 08:58, AKASHI Takahiro wrote:
> Heinrich,
>
>>> On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote:
>>> On 11/15/18 5:58 AM, AKASHI Takahiro wrote:
>>> Currently, efi_init_obj_list() scan disk devices only once, and
>>> never
>>> change a list of efi disk devices. This will possibly result in
>>> failing
>>> to find a removable storage which may be added later on. See [1].
>>>
>>> In this patch, called is efi_disk_update() which is responsible for
>>> re-scanning UCLASS_BLK devices and removing/adding efi disks if
>>> necessary.
>>>
>>> For example,
>>>
>>> => efishell devices
>>> Scanning disk pci_mmc.blk...
>>> Found 3 disks
>>> Device Name
>>>
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
>>> => usb start
>>> starting USB...
>>> USB0: USB EHCI 1.00
>>> scanning bus 0 for devices... 3 USB Device(s) found
>>> scanning usb for storage devices... 1 Storage Device(s) found
>>> => efishell devices
>>> Scanning disk usb_mass_storage.lun0...
>>> Device Name
>>>
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c)
>>>
>>> Without this patch, the last device, USB mass storage, won't show
>>> up.
>>>
>>> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
>>>
>>> Signed-off-by: AKASHI Takahiro
>>
>> Why should we try to fix something in the EFI subsystems that goes
>> wrong
>> in the handling of device enumeration.
>
> No.
> This is a natural result from how efi disks are currently implemented
> on u-boot.
> Do you want to totally re-write/re-implement efi disks?
Could we just make this event based for now? Call a hook from the
storage dm subsystem when a new u-boot block device gets created to
issue a sync of that in the efi subsystem?
>>>
>>> If I correctly understand you, your suggestion here corresponds
>>> with my proposal#3 in [1] while my current approach is #2.
>>>
>>> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
>>
>> Yes, I think so.
>>
>>> So we will call, say, efi_disk_create(struct udevice *) in
>>> blk_create_device() and efi_dsik_delete() in blk_unbind_all().
>>
>> I would prefer if we didn't call them directly, but through an event
>> mechanism. So the efi_disk subsystem registers an event with the dm
>> block subsystem and that will just call all events when block devices
>> get created which will automatically also include the efi disk creation
>> callback. Same for reverse.
>
> Do you mean efi event by "event?"
> (I don't think there is any generic event interface on DM side.)
>
> Whatever an "event" is or whether we call efi_disk_create() directly
> or indirectly via an event, there is one (big?) issue in this approach
> (while I've almost finished prototyping):
>
> We cannot call efi_disk_create() within blk_create_device() because
> some data fields of struct blk_desc, which are to be used by efi disk,
> are initialized *after* blk_create_device() in driver side.
>
> So we need to add a hook at/after every occurrence of blk_create_device()
> on driver side. For example,
>
> === drivers/scsi/scsi.c ===
> int do_scsi_scan_one(struct udevice *dev, int id, int lun, bool verbose)
> {
> ...
> ret = blk_create_devicef(dev, "scsi_blk", str, IF_TYPE_SCSI, -1,
> bd.blksz,
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On Thu, Jan 10, 2019 at 09:15:36AM +0100, Alexander Graf wrote:
>
>
> > Am 10.01.2019 um 09:02 schrieb AKASHI Takahiro :
> >
> >> On Thu, Jan 10, 2019 at 08:30:13AM +0100, Alexander Graf wrote:
> >>
> >>
> >>> On 10.01.19 08:26, AKASHI Takahiro wrote:
> >>> Alex,
> >>>
> On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote:
>
>
> > On 10.01.19 03:13, AKASHI Takahiro wrote:
> > Alex,
> >
> >> On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote:
> >>
> >>
> >>> On 13.12.18 08:58, AKASHI Takahiro wrote:
> >>> Heinrich,
> >>>
> On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote:
> > On 11/15/18 5:58 AM, AKASHI Takahiro wrote:
> > Currently, efi_init_obj_list() scan disk devices only once, and
> > never
> > change a list of efi disk devices. This will possibly result in
> > failing
> > to find a removable storage which may be added later on. See [1].
> >
> > In this patch, called is efi_disk_update() which is responsible for
> > re-scanning UCLASS_BLK devices and removing/adding efi disks if
> > necessary.
> >
> > For example,
> >
> > => efishell devices
> > Scanning disk pci_mmc.blk...
> > Found 3 disks
> > Device Name
> >
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> > => usb start
> > starting USB...
> > USB0: USB EHCI 1.00
> > scanning bus 0 for devices... 3 USB Device(s) found
> > scanning usb for storage devices... 1 Storage Device(s) found
> > => efishell devices
> > Scanning disk usb_mass_storage.lun0...
> > Device Name
> >
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c)
> >
> > Without this patch, the last device, USB mass storage, won't show
> > up.
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
> >
> > Signed-off-by: AKASHI Takahiro
>
> Why should we try to fix something in the EFI subsystems that goes
> wrong
> in the handling of device enumeration.
> >>>
> >>> No.
> >>> This is a natural result from how efi disks are currently implemented
> >>> on u-boot.
> >>> Do you want to totally re-write/re-implement efi disks?
> >>
> >> Could we just make this event based for now? Call a hook from the
> >> storage dm subsystem when a new u-boot block device gets created to
> >> issue a sync of that in the efi subsystem?
> >
> > If I correctly understand you, your suggestion here corresponds
> > with my proposal#3 in [1] while my current approach is #2.
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
>
> Yes, I think so.
>
> > So we will call, say, efi_disk_create(struct udevice *) in
> > blk_create_device() and efi_dsik_delete() in blk_unbind_all().
>
> I would prefer if we didn't call them directly, but through an event
> mechanism. So the efi_disk subsystem registers an event with the dm
> block subsystem and that will just call all events when block devices
> get created which will automatically also include the efi disk creation
> callback. Same for reverse.
> >>>
> >>> Do you mean efi event by "event?"
> >>> (I don't think there is any generic event interface on DM side.)
> >>>
> >>> Whatever an "event" is or whether we call efi_disk_create() directly
> >>> or indirectly via an event, there is one (big?) issue in this approach
> >>> (while I've almost finished prototyping):
> >>>
> >>> We cannot call efi_disk_create() within blk_create_device() because
> >>> some data fields of struct blk_desc, which are to be used by efi disk,
> >>> are initialized *after* blk_create_device() in driver side.
> >>>
> >>> So we need to add a hook at/after every occurrence of blk_create_device()
> >>> on driver side. For example,
> >>>
> >>> === drivers/scsi/scsi.c ===
> >>> int do_scsi_scan_one(struct udevice *dev, int id, int lun, bool verbose)
> >>> {
> >>>...
> >>>ret = blk_create_devicef(dev, "scsi_blk", str, IF_TYPE_SCSI, -1,
> >>> bd.blksz, bd.lba, );
> >>>...
> >>>bdesc =
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
> Am 10.01.2019 um 09:02 schrieb AKASHI Takahiro :
>
>> On Thu, Jan 10, 2019 at 08:30:13AM +0100, Alexander Graf wrote:
>>
>>
>>> On 10.01.19 08:26, AKASHI Takahiro wrote:
>>> Alex,
>>>
On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote:
> On 10.01.19 03:13, AKASHI Takahiro wrote:
> Alex,
>
>> On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote:
>>
>>
>>> On 13.12.18 08:58, AKASHI Takahiro wrote:
>>> Heinrich,
>>>
On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote:
> On 11/15/18 5:58 AM, AKASHI Takahiro wrote:
> Currently, efi_init_obj_list() scan disk devices only once, and never
> change a list of efi disk devices. This will possibly result in
> failing
> to find a removable storage which may be added later on. See [1].
>
> In this patch, called is efi_disk_update() which is responsible for
> re-scanning UCLASS_BLK devices and removing/adding efi disks if
> necessary.
>
> For example,
>
> => efishell devices
> Scanning disk pci_mmc.blk...
> Found 3 disks
> Device Name
>
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> => usb start
> starting USB...
> USB0: USB EHCI 1.00
> scanning bus 0 for devices... 3 USB Device(s) found
> scanning usb for storage devices... 1 Storage Device(s) found
> => efishell devices
> Scanning disk usb_mass_storage.lun0...
> Device Name
>
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c)
>
> Without this patch, the last device, USB mass storage, won't show up.
>
> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
>
> Signed-off-by: AKASHI Takahiro
Why should we try to fix something in the EFI subsystems that goes
wrong
in the handling of device enumeration.
>>>
>>> No.
>>> This is a natural result from how efi disks are currently implemented
>>> on u-boot.
>>> Do you want to totally re-write/re-implement efi disks?
>>
>> Could we just make this event based for now? Call a hook from the
>> storage dm subsystem when a new u-boot block device gets created to
>> issue a sync of that in the efi subsystem?
>
> If I correctly understand you, your suggestion here corresponds
> with my proposal#3 in [1] while my current approach is #2.
>
> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
Yes, I think so.
> So we will call, say, efi_disk_create(struct udevice *) in
> blk_create_device() and efi_dsik_delete() in blk_unbind_all().
I would prefer if we didn't call them directly, but through an event
mechanism. So the efi_disk subsystem registers an event with the dm
block subsystem and that will just call all events when block devices
get created which will automatically also include the efi disk creation
callback. Same for reverse.
>>>
>>> Do you mean efi event by "event?"
>>> (I don't think there is any generic event interface on DM side.)
>>>
>>> Whatever an "event" is or whether we call efi_disk_create() directly
>>> or indirectly via an event, there is one (big?) issue in this approach
>>> (while I've almost finished prototyping):
>>>
>>> We cannot call efi_disk_create() within blk_create_device() because
>>> some data fields of struct blk_desc, which are to be used by efi disk,
>>> are initialized *after* blk_create_device() in driver side.
>>>
>>> So we need to add a hook at/after every occurrence of blk_create_device()
>>> on driver side. For example,
>>>
>>> === drivers/scsi/scsi.c ===
>>> int do_scsi_scan_one(struct udevice *dev, int id, int lun, bool verbose)
>>> {
>>>...
>>>ret = blk_create_devicef(dev, "scsi_blk", str, IF_TYPE_SCSI, -1,
>>> bd.blksz, bd.lba, );
>>>...
>>>bdesc = dev_get_uclass_platdata(bdev);
>>>bdesc->target = id;
>>>bdesc->lun = lun;
>>>...
>>>
>>>/*
>>> * We need have efi_disk_create() called here because bdesc->target
>>> * and lun will be used by dp helpers in efi_disk_add_dev().
>>> */
>>>efi_disk_create(bdev);
>>> }
>>>
>>> int scsi_scan_dev(struct udevice
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On Thu, Jan 10, 2019 at 08:30:13AM +0100, Alexander Graf wrote:
>
>
> On 10.01.19 08:26, AKASHI Takahiro wrote:
> > Alex,
> >
> > On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 10.01.19 03:13, AKASHI Takahiro wrote:
> >>> Alex,
> >>>
> >>> On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote:
>
>
> On 13.12.18 08:58, AKASHI Takahiro wrote:
> > Heinrich,
> >
> > On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote:
> >> On 11/15/18 5:58 AM, AKASHI Takahiro wrote:
> >>> Currently, efi_init_obj_list() scan disk devices only once, and never
> >>> change a list of efi disk devices. This will possibly result in
> >>> failing
> >>> to find a removable storage which may be added later on. See [1].
> >>>
> >>> In this patch, called is efi_disk_update() which is responsible for
> >>> re-scanning UCLASS_BLK devices and removing/adding efi disks if
> >>> necessary.
> >>>
> >>> For example,
> >>>
> >>> => efishell devices
> >>> Scanning disk pci_mmc.blk...
> >>> Found 3 disks
> >>> Device Name
> >>>
> >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> >>> => usb start
> >>> starting USB...
> >>> USB0: USB EHCI 1.00
> >>> scanning bus 0 for devices... 3 USB Device(s) found
> >>>scanning usb for storage devices... 1 Storage Device(s) found
> >>> => efishell devices
> >>> Scanning disk usb_mass_storage.lun0...
> >>> Device Name
> >>>
> >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c)
> >>>
> >>> Without this patch, the last device, USB mass storage, won't show up.
> >>>
> >>> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
> >>>
> >>> Signed-off-by: AKASHI Takahiro
> >>
> >> Why should we try to fix something in the EFI subsystems that goes
> >> wrong
> >> in the handling of device enumeration.
> >
> > No.
> > This is a natural result from how efi disks are currently implemented
> > on u-boot.
> > Do you want to totally re-write/re-implement efi disks?
>
> Could we just make this event based for now? Call a hook from the
> storage dm subsystem when a new u-boot block device gets created to
> issue a sync of that in the efi subsystem?
> >>>
> >>> If I correctly understand you, your suggestion here corresponds
> >>> with my proposal#3 in [1] while my current approach is #2.
> >>>
> >>> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
> >>
> >> Yes, I think so.
> >>
> >>> So we will call, say, efi_disk_create(struct udevice *) in
> >>> blk_create_device() and efi_dsik_delete() in blk_unbind_all().
> >>
> >> I would prefer if we didn't call them directly, but through an event
> >> mechanism. So the efi_disk subsystem registers an event with the dm
> >> block subsystem and that will just call all events when block devices
> >> get created which will automatically also include the efi disk creation
> >> callback. Same for reverse.
> >
> > Do you mean efi event by "event?"
> > (I don't think there is any generic event interface on DM side.)
> >
> > Whatever an "event" is or whether we call efi_disk_create() directly
> > or indirectly via an event, there is one (big?) issue in this approach
> > (while I've almost finished prototyping):
> >
> > We cannot call efi_disk_create() within blk_create_device() because
> > some data fields of struct blk_desc, which are to be used by efi disk,
> > are initialized *after* blk_create_device() in driver side.
> >
> > So we need to add a hook at/after every occurrence of blk_create_device()
> > on driver side. For example,
> >
> > === drivers/scsi/scsi.c ===
> > int do_scsi_scan_one(struct udevice *dev, int id, int lun, bool verbose)
> > {
> > ...
> > ret = blk_create_devicef(dev, "scsi_blk", str, IF_TYPE_SCSI, -1,
> >bd.blksz, bd.lba, );
> > ...
> > bdesc = dev_get_uclass_platdata(bdev);
> > bdesc->target = id;
> > bdesc->lun = lun;
> > ...
> >
> > /*
> > * We need have efi_disk_create() called here because bdesc->target
> > * and lun will be used by dp helpers in efi_disk_add_dev().
> > */
> > efi_disk_create(bdev);
> > }
> >
> > int scsi_scan_dev(struct udevice *dev, bool verbose)
> > {
> > for (i = 0; i <
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On 10.01.19 08:26, AKASHI Takahiro wrote:
> Alex,
>
> On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote:
>>
>>
>> On 10.01.19 03:13, AKASHI Takahiro wrote:
>>> Alex,
>>>
>>> On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote:
On 13.12.18 08:58, AKASHI Takahiro wrote:
> Heinrich,
>
> On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote:
>> On 11/15/18 5:58 AM, AKASHI Takahiro wrote:
>>> Currently, efi_init_obj_list() scan disk devices only once, and never
>>> change a list of efi disk devices. This will possibly result in failing
>>> to find a removable storage which may be added later on. See [1].
>>>
>>> In this patch, called is efi_disk_update() which is responsible for
>>> re-scanning UCLASS_BLK devices and removing/adding efi disks if
>>> necessary.
>>>
>>> For example,
>>>
>>> => efishell devices
>>> Scanning disk pci_mmc.blk...
>>> Found 3 disks
>>> Device Name
>>>
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
>>> => usb start
>>> starting USB...
>>> USB0: USB EHCI 1.00
>>> scanning bus 0 for devices... 3 USB Device(s) found
>>>scanning usb for storage devices... 1 Storage Device(s) found
>>> => efishell devices
>>> Scanning disk usb_mass_storage.lun0...
>>> Device Name
>>>
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c)
>>>
>>> Without this patch, the last device, USB mass storage, won't show up.
>>>
>>> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
>>>
>>> Signed-off-by: AKASHI Takahiro
>>
>> Why should we try to fix something in the EFI subsystems that goes wrong
>> in the handling of device enumeration.
>
> No.
> This is a natural result from how efi disks are currently implemented on
> u-boot.
> Do you want to totally re-write/re-implement efi disks?
Could we just make this event based for now? Call a hook from the
storage dm subsystem when a new u-boot block device gets created to
issue a sync of that in the efi subsystem?
>>>
>>> If I correctly understand you, your suggestion here corresponds
>>> with my proposal#3 in [1] while my current approach is #2.
>>>
>>> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
>>
>> Yes, I think so.
>>
>>> So we will call, say, efi_disk_create(struct udevice *) in
>>> blk_create_device() and efi_dsik_delete() in blk_unbind_all().
>>
>> I would prefer if we didn't call them directly, but through an event
>> mechanism. So the efi_disk subsystem registers an event with the dm
>> block subsystem and that will just call all events when block devices
>> get created which will automatically also include the efi disk creation
>> callback. Same for reverse.
>
> Do you mean efi event by "event?"
> (I don't think there is any generic event interface on DM side.)
>
> Whatever an "event" is or whether we call efi_disk_create() directly
> or indirectly via an event, there is one (big?) issue in this approach
> (while I've almost finished prototyping):
>
> We cannot call efi_disk_create() within blk_create_device() because
> some data fields of struct blk_desc, which are to be used by efi disk,
> are initialized *after* blk_create_device() in driver side.
>
> So we need to add a hook at/after every occurrence of blk_create_device()
> on driver side. For example,
>
> === drivers/scsi/scsi.c ===
> int do_scsi_scan_one(struct udevice *dev, int id, int lun, bool verbose)
> {
> ...
> ret = blk_create_devicef(dev, "scsi_blk", str, IF_TYPE_SCSI, -1,
> bd.blksz, bd.lba, );
> ...
> bdesc = dev_get_uclass_platdata(bdev);
> bdesc->target = id;
> bdesc->lun = lun;
> ...
>
> /*
>* We need have efi_disk_create() called here because bdesc->target
>* and lun will be used by dp helpers in efi_disk_add_dev().
>*/
> efi_disk_create(bdev);
> }
>
> int scsi_scan_dev(struct udevice *dev, bool verbose)
> {
> for (i = 0; i < uc_plat->max_id; i++)
> for (lun = 0; lun < uc_plat->max_lun; lun++)
> do_scsi_scan_one(dev, i, lun, verbose);
> ...
> }
>
> int scsi_scan(bool verbose)
> {
> ret = uclass_get(UCLASS_SCSI, );
> ...
> uclass_foreach_dev(dev, uc)
>
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Alex,
On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote:
>
>
> On 10.01.19 03:13, AKASHI Takahiro wrote:
> > Alex,
> >
> > On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 13.12.18 08:58, AKASHI Takahiro wrote:
> >>> Heinrich,
> >>>
> >>> On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote:
> On 11/15/18 5:58 AM, AKASHI Takahiro wrote:
> > Currently, efi_init_obj_list() scan disk devices only once, and never
> > change a list of efi disk devices. This will possibly result in failing
> > to find a removable storage which may be added later on. See [1].
> >
> > In this patch, called is efi_disk_update() which is responsible for
> > re-scanning UCLASS_BLK devices and removing/adding efi disks if
> > necessary.
> >
> > For example,
> >
> > => efishell devices
> > Scanning disk pci_mmc.blk...
> > Found 3 disks
> > Device Name
> >
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> > => usb start
> > starting USB...
> > USB0: USB EHCI 1.00
> > scanning bus 0 for devices... 3 USB Device(s) found
> >scanning usb for storage devices... 1 Storage Device(s) found
> > => efishell devices
> > Scanning disk usb_mass_storage.lun0...
> > Device Name
> >
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c)
> >
> > Without this patch, the last device, USB mass storage, won't show up.
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
> >
> > Signed-off-by: AKASHI Takahiro
>
> Why should we try to fix something in the EFI subsystems that goes wrong
> in the handling of device enumeration.
> >>>
> >>> No.
> >>> This is a natural result from how efi disks are currently implemented on
> >>> u-boot.
> >>> Do you want to totally re-write/re-implement efi disks?
> >>
> >> Could we just make this event based for now? Call a hook from the
> >> storage dm subsystem when a new u-boot block device gets created to
> >> issue a sync of that in the efi subsystem?
> >
> > If I correctly understand you, your suggestion here corresponds
> > with my proposal#3 in [1] while my current approach is #2.
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
>
> Yes, I think so.
>
> > So we will call, say, efi_disk_create(struct udevice *) in
> > blk_create_device() and efi_dsik_delete() in blk_unbind_all().
>
> I would prefer if we didn't call them directly, but through an event
> mechanism. So the efi_disk subsystem registers an event with the dm
> block subsystem and that will just call all events when block devices
> get created which will automatically also include the efi disk creation
> callback. Same for reverse.
Do you mean efi event by "event?"
(I don't think there is any generic event interface on DM side.)
Whatever an "event" is or whether we call efi_disk_create() directly
or indirectly via an event, there is one (big?) issue in this approach
(while I've almost finished prototyping):
We cannot call efi_disk_create() within blk_create_device() because
some data fields of struct blk_desc, which are to be used by efi disk,
are initialized *after* blk_create_device() in driver side.
So we need to add a hook at/after every occurrence of blk_create_device()
on driver side. For example,
=== drivers/scsi/scsi.c ===
int do_scsi_scan_one(struct udevice *dev, int id, int lun, bool verbose)
{
...
ret = blk_create_devicef(dev, "scsi_blk", str, IF_TYPE_SCSI, -1,
bd.blksz, bd.lba, );
...
bdesc = dev_get_uclass_platdata(bdev);
bdesc->target = id;
bdesc->lun = lun;
...
/*
* We need have efi_disk_create() called here because bdesc->target
* and lun will be used by dp helpers in efi_disk_add_dev().
*/
efi_disk_create(bdev);
}
int scsi_scan_dev(struct udevice *dev, bool verbose)
{
for (i = 0; i < uc_plat->max_id; i++)
for (lun = 0; lun < uc_plat->max_lun; lun++)
do_scsi_scan_one(dev, i, lun, verbose);
...
}
int scsi_scan(bool verbose)
{
ret = uclass_get(UCLASS_SCSI, );
...
uclass_foreach_dev(dev, uc)
ret = scsi_scan_dev(dev, verbose);
...
}
=== ===
Since scsn_scan() can be directly called by "scsi rescan"
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On 10.01.19 03:13, AKASHI Takahiro wrote: > Alex, > > On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote: >> >> >> On 13.12.18 08:58, AKASHI Takahiro wrote: >>> Heinrich, >>> >>> On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote: On 11/15/18 5:58 AM, AKASHI Takahiro wrote: > Currently, efi_init_obj_list() scan disk devices only once, and never > change a list of efi disk devices. This will possibly result in failing > to find a removable storage which may be added later on. See [1]. > > In this patch, called is efi_disk_update() which is responsible for > re-scanning UCLASS_BLK devices and removing/adding efi disks if necessary. > > For example, > > => efishell devices > Scanning disk pci_mmc.blk... > Found 3 disks > Device Name > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > => usb start > starting USB... > USB0: USB EHCI 1.00 > scanning bus 0 for devices... 3 USB Device(s) found >scanning usb for storage devices... 1 Storage Device(s) found > => efishell devices > Scanning disk usb_mass_storage.lun0... > Device Name > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c) > > Without this patch, the last device, USB mass storage, won't show up. > > [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html > > Signed-off-by: AKASHI Takahiro Why should we try to fix something in the EFI subsystems that goes wrong in the handling of device enumeration. >>> >>> No. >>> This is a natural result from how efi disks are currently implemented on >>> u-boot. >>> Do you want to totally re-write/re-implement efi disks? >> >> Could we just make this event based for now? Call a hook from the >> storage dm subsystem when a new u-boot block device gets created to >> issue a sync of that in the efi subsystem? > > If I correctly understand you, your suggestion here corresponds > with my proposal#3 in [1] while my current approach is #2. > > [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html Yes, I think so. > So we will call, say, efi_disk_create(struct udevice *) in > blk_create_device() and efi_dsik_delete() in blk_unbind_all(). I would prefer if we didn't call them directly, but through an event mechanism. So the efi_disk subsystem registers an event with the dm block subsystem and that will just call all events when block devices get created which will automatically also include the efi disk creation callback. Same for reverse. > UEFI handles for disks, however, will *not* be deleted actually > because, as Heinrich suggested in the past, no *reference count* > for a handle is maintained in the current implementation. Right? Sure, but we can have an internal state that just starts failing all callbacks (read, write, etc) when the backing device was destroyed. >> That hook would obviously only do something (or get registered?) when >> the efi object stack is initialized. >> >> The long term goal IMHO should still be though to just merge DM and EFI >> objects. But we're still waiting on the deprecation of non-DM devices >> for that. > > Maybe my #4? Not quite. We would basically get rid of the efi object list altogether. Instead, any DM object would also be an EFI object and expose EFI interfaces if awareness exists. That way we can merge into the existing object maintenance logic and wouldn't need any callbacks. Once we're there, we can also start to think about reference counting :). Alex ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Alex, On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote: > > > On 13.12.18 08:58, AKASHI Takahiro wrote: > > Heinrich, > > > > On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote: > >> On 11/15/18 5:58 AM, AKASHI Takahiro wrote: > >>> Currently, efi_init_obj_list() scan disk devices only once, and never > >>> change a list of efi disk devices. This will possibly result in failing > >>> to find a removable storage which may be added later on. See [1]. > >>> > >>> In this patch, called is efi_disk_update() which is responsible for > >>> re-scanning UCLASS_BLK devices and removing/adding efi disks if necessary. > >>> > >>> For example, > >>> > >>> => efishell devices > >>> Scanning disk pci_mmc.blk... > >>> Found 3 disks > >>> Device Name > >>> > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > >>> => usb start > >>> starting USB... > >>> USB0: USB EHCI 1.00 > >>> scanning bus 0 for devices... 3 USB Device(s) found > >>>scanning usb for storage devices... 1 Storage Device(s) found > >>> => efishell devices > >>> Scanning disk usb_mass_storage.lun0... > >>> Device Name > >>> > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c) > >>> > >>> Without this patch, the last device, USB mass storage, won't show up. > >>> > >>> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html > >>> > >>> Signed-off-by: AKASHI Takahiro > >> > >> Why should we try to fix something in the EFI subsystems that goes wrong > >> in the handling of device enumeration. > > > > No. > > This is a natural result from how efi disks are currently implemented on > > u-boot. > > Do you want to totally re-write/re-implement efi disks? > > Could we just make this event based for now? Call a hook from the > storage dm subsystem when a new u-boot block device gets created to > issue a sync of that in the efi subsystem? If I correctly understand you, your suggestion here corresponds with my proposal#3 in [1] while my current approach is #2. [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html So we will call, say, efi_disk_create(struct udevice *) in blk_create_device() and efi_dsik_delete() in blk_unbind_all(). UEFI handles for disks, however, will *not* be deleted actually because, as Heinrich suggested in the past, no *reference count* for a handle is maintained in the current implementation. Right? > That hook would obviously only do something (or get registered?) when > the efi object stack is initialized. > > The long term goal IMHO should still be though to just merge DM and EFI > objects. But we're still waiting on the deprecation of non-DM devices > for that. Maybe my #4? -Takahiro Akashi > > Alex ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On 13.12.18 08:58, AKASHI Takahiro wrote: > Heinrich, > > On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote: >> On 11/15/18 5:58 AM, AKASHI Takahiro wrote: >>> Currently, efi_init_obj_list() scan disk devices only once, and never >>> change a list of efi disk devices. This will possibly result in failing >>> to find a removable storage which may be added later on. See [1]. >>> >>> In this patch, called is efi_disk_update() which is responsible for >>> re-scanning UCLASS_BLK devices and removing/adding efi disks if necessary. >>> >>> For example, >>> >>> => efishell devices >>> Scanning disk pci_mmc.blk... >>> Found 3 disks >>> Device Name >>> >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) >>> => usb start >>> starting USB... >>> USB0: USB EHCI 1.00 >>> scanning bus 0 for devices... 3 USB Device(s) found >>>scanning usb for storage devices... 1 Storage Device(s) found >>> => efishell devices >>> Scanning disk usb_mass_storage.lun0... >>> Device Name >>> >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c) >>> >>> Without this patch, the last device, USB mass storage, won't show up. >>> >>> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html >>> >>> Signed-off-by: AKASHI Takahiro >> >> Why should we try to fix something in the EFI subsystems that goes wrong >> in the handling of device enumeration. > > No. > This is a natural result from how efi disks are currently implemented on > u-boot. > Do you want to totally re-write/re-implement efi disks? Could we just make this event based for now? Call a hook from the storage dm subsystem when a new u-boot block device gets created to issue a sync of that in the efi subsystem? That hook would obviously only do something (or get registered?) when the efi object stack is initialized. The long term goal IMHO should still be though to just merge DM and EFI objects. But we're still waiting on the deprecation of non-DM devices for that. Alex ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Heinrich,
Do you have any further comments here?
Otherwise, I'd like to send out v3 shortly.
-Takahiro Akashi
On Thu, Dec 13, 2018 at 04:58:29PM +0900, AKASHI Takahiro wrote:
> Heinrich,
>
> On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote:
> > On 11/15/18 5:58 AM, AKASHI Takahiro wrote:
> > > Currently, efi_init_obj_list() scan disk devices only once, and never
> > > change a list of efi disk devices. This will possibly result in failing
> > > to find a removable storage which may be added later on. See [1].
> > >
> > > In this patch, called is efi_disk_update() which is responsible for
> > > re-scanning UCLASS_BLK devices and removing/adding efi disks if necessary.
> > >
> > > For example,
> > >
> > > => efishell devices
> > > Scanning disk pci_mmc.blk...
> > > Found 3 disks
> > > Device Name
> > >
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> > > => usb start
> > > starting USB...
> > > USB0: USB EHCI 1.00
> > > scanning bus 0 for devices... 3 USB Device(s) found
> > >scanning usb for storage devices... 1 Storage Device(s) found
> > > => efishell devices
> > > Scanning disk usb_mass_storage.lun0...
> > > Device Name
> > >
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c)
> > >
> > > Without this patch, the last device, USB mass storage, won't show up.
> > >
> > > [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
> > >
> > > Signed-off-by: AKASHI Takahiro
> >
> > Why should we try to fix something in the EFI subsystems that goes wrong
> > in the handling of device enumeration.
>
> No.
> This is a natural result from how efi disks are currently implemented on
> u-boot.
> Do you want to totally re-write/re-implement efi disks?
>
> Furthermore, your comment here doesn't match your previous comment[1].
>
> [1] https://lists.denx.de/pipermail/u-boot/2018-November/346860.html
>
> -Takahiro Akashi
>
> > @Marek
> > Why should a 'usb start' command be needed to make a plugged in device
> > available?
> >
> > Best regards
> >
> > Heirnich
> >
> >
> >
> > > ---
> > > cmd/bootefi.c | 17 +++-
> > > include/efi_loader.h | 4 +
> > > lib/efi_loader/efi_disk.c | 191 ++
> > > 3 files changed, 210 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > > index 3cefb4d0ecaa..82649e211fda 100644
> > > --- a/cmd/bootefi.c
> > > +++ b/cmd/bootefi.c
> > > @@ -56,9 +56,22 @@ efi_status_t efi_init_obj_list(void)
> > >*/
> > > efi_save_gd();
> > >
> > > - /* Initialize once only */
> > > - if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
> > > + if (efi_obj_list_initialized == EFI_SUCCESS) {
> > > +#ifdef CONFIG_PARTITIONS
> > > + ret = efi_disk_update();
> > > + if (ret != EFI_SUCCESS)
> > > + printf("+++ updating disks list failed\n");
> > > +
> > > + /*
> > > + * It may sound odd, but most part of EFI should
> > > + * yet work.
> > > + */
> > > +#endif
> > > return efi_obj_list_initialized;
> > > + } else if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED) {
> > > + /* Initialize once only */
> > > + return efi_obj_list_initialized;
> > > + }
> > >
> > > /* Initialize system table */
> > > ret = efi_initialize_system_table();
> > > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > > index 5cc3bded03fa..3bae1844befb 100644
> > > --- a/include/efi_loader.h
> > > +++ b/include/efi_loader.h
> > > @@ -260,6 +260,10 @@ efi_status_t efi_initialize_system_table(void);
> > > efi_status_t efi_console_register(void);
> > > /* Called by bootefi to make all disk storage accessible as EFI objects
> > > */
> > > efi_status_t efi_disk_register(void);
> > > +/* Check validity of efi disk */
> > > +bool efi_disk_is_valid(efi_handle_t handle);
> > > +/* Called by bootefi to find and update disk storage information */
> > > +efi_status_t efi_disk_update(void);
> > > /* Create handles and protocols for the partitions of a block device */
> > > int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc
> > > *desc,
> > > const char *if_typename, int diskid,
> > > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > > index c037526ad2d0..0c4d79ee3fc9 100644
> > > --- a/lib/efi_loader/efi_disk.c
> > > +++ b/lib/efi_loader/efi_disk.c
> > > @@ -14,10 +14,14 @@
> > >
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Heinrich,
On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote:
> On 11/15/18 5:58 AM, AKASHI Takahiro wrote:
> > Currently, efi_init_obj_list() scan disk devices only once, and never
> > change a list of efi disk devices. This will possibly result in failing
> > to find a removable storage which may be added later on. See [1].
> >
> > In this patch, called is efi_disk_update() which is responsible for
> > re-scanning UCLASS_BLK devices and removing/adding efi disks if necessary.
> >
> > For example,
> >
> > => efishell devices
> > Scanning disk pci_mmc.blk...
> > Found 3 disks
> > Device Name
> >
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> > => usb start
> > starting USB...
> > USB0: USB EHCI 1.00
> > scanning bus 0 for devices... 3 USB Device(s) found
> >scanning usb for storage devices... 1 Storage Device(s) found
> > => efishell devices
> > Scanning disk usb_mass_storage.lun0...
> > Device Name
> >
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c)
> >
> > Without this patch, the last device, USB mass storage, won't show up.
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
> >
> > Signed-off-by: AKASHI Takahiro
>
> Why should we try to fix something in the EFI subsystems that goes wrong
> in the handling of device enumeration.
No.
This is a natural result from how efi disks are currently implemented on u-boot.
Do you want to totally re-write/re-implement efi disks?
Furthermore, your comment here doesn't match your previous comment[1].
[1] https://lists.denx.de/pipermail/u-boot/2018-November/346860.html
-Takahiro Akashi
> @Marek
> Why should a 'usb start' command be needed to make a plugged in device
> available?
>
> Best regards
>
> Heirnich
>
>
>
> > ---
> > cmd/bootefi.c | 17 +++-
> > include/efi_loader.h | 4 +
> > lib/efi_loader/efi_disk.c | 191 ++
> > 3 files changed, 210 insertions(+), 2 deletions(-)
> >
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index 3cefb4d0ecaa..82649e211fda 100644
> > --- a/cmd/bootefi.c
> > +++ b/cmd/bootefi.c
> > @@ -56,9 +56,22 @@ efi_status_t efi_init_obj_list(void)
> > */
> > efi_save_gd();
> >
> > - /* Initialize once only */
> > - if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
> > + if (efi_obj_list_initialized == EFI_SUCCESS) {
> > +#ifdef CONFIG_PARTITIONS
> > + ret = efi_disk_update();
> > + if (ret != EFI_SUCCESS)
> > + printf("+++ updating disks list failed\n");
> > +
> > + /*
> > +* It may sound odd, but most part of EFI should
> > +* yet work.
> > +*/
> > +#endif
> > return efi_obj_list_initialized;
> > + } else if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED) {
> > + /* Initialize once only */
> > + return efi_obj_list_initialized;
> > + }
> >
> > /* Initialize system table */
> > ret = efi_initialize_system_table();
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 5cc3bded03fa..3bae1844befb 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -260,6 +260,10 @@ efi_status_t efi_initialize_system_table(void);
> > efi_status_t efi_console_register(void);
> > /* Called by bootefi to make all disk storage accessible as EFI objects */
> > efi_status_t efi_disk_register(void);
> > +/* Check validity of efi disk */
> > +bool efi_disk_is_valid(efi_handle_t handle);
> > +/* Called by bootefi to find and update disk storage information */
> > +efi_status_t efi_disk_update(void);
> > /* Create handles and protocols for the partitions of a block device */
> > int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
> >const char *if_typename, int diskid,
> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > index c037526ad2d0..0c4d79ee3fc9 100644
> > --- a/lib/efi_loader/efi_disk.c
> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -14,10 +14,14 @@
> >
> > const efi_guid_t efi_block_io_guid = BLOCK_IO_GUID;
> >
> > +#define _EFI_DISK_FLAG_DELETED 0x1 /* to be removed */
> > +#define _EFI_DISK_FLAG_INVALID 0x8000 /* in stale state */
> > +
> > /**
> > * struct efi_disk_obj - EFI disk object
> > *
> > * @header:EFI object header
> > + * @flags: control flags
> > * @ops: EFI disk I/O protocol interface
> >
Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
On 11/15/18 5:58 AM, AKASHI Takahiro wrote:
> Currently, efi_init_obj_list() scan disk devices only once, and never
> change a list of efi disk devices. This will possibly result in failing
> to find a removable storage which may be added later on. See [1].
>
> In this patch, called is efi_disk_update() which is responsible for
> re-scanning UCLASS_BLK devices and removing/adding efi disks if necessary.
>
> For example,
>
> => efishell devices
> Scanning disk pci_mmc.blk...
> Found 3 disks
> Device Name
>
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> => usb start
> starting USB...
> USB0: USB EHCI 1.00
> scanning bus 0 for devices... 3 USB Device(s) found
>scanning usb for storage devices... 1 Storage Device(s) found
> => efishell devices
> Scanning disk usb_mass_storage.lun0...
> Device Name
>
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c)
>
> Without this patch, the last device, USB mass storage, won't show up.
>
> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
>
> Signed-off-by: AKASHI Takahiro
Why should we try to fix something in the EFI subsystems that goes wrong
in the handling of device enumeration.
@Marek
Why should a 'usb start' command be needed to make a plugged in device
available?
Best regards
Heirnich
> ---
> cmd/bootefi.c | 17 +++-
> include/efi_loader.h | 4 +
> lib/efi_loader/efi_disk.c | 191 ++
> 3 files changed, 210 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 3cefb4d0ecaa..82649e211fda 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -56,9 +56,22 @@ efi_status_t efi_init_obj_list(void)
>*/
> efi_save_gd();
>
> - /* Initialize once only */
> - if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
> + if (efi_obj_list_initialized == EFI_SUCCESS) {
> +#ifdef CONFIG_PARTITIONS
> + ret = efi_disk_update();
> + if (ret != EFI_SUCCESS)
> + printf("+++ updating disks list failed\n");
> +
> + /*
> + * It may sound odd, but most part of EFI should
> + * yet work.
> + */
> +#endif
> return efi_obj_list_initialized;
> + } else if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED) {
> + /* Initialize once only */
> + return efi_obj_list_initialized;
> + }
>
> /* Initialize system table */
> ret = efi_initialize_system_table();
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 5cc3bded03fa..3bae1844befb 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -260,6 +260,10 @@ efi_status_t efi_initialize_system_table(void);
> efi_status_t efi_console_register(void);
> /* Called by bootefi to make all disk storage accessible as EFI objects */
> efi_status_t efi_disk_register(void);
> +/* Check validity of efi disk */
> +bool efi_disk_is_valid(efi_handle_t handle);
> +/* Called by bootefi to find and update disk storage information */
> +efi_status_t efi_disk_update(void);
> /* Create handles and protocols for the partitions of a block device */
> int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
> const char *if_typename, int diskid,
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index c037526ad2d0..0c4d79ee3fc9 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -14,10 +14,14 @@
>
> const efi_guid_t efi_block_io_guid = BLOCK_IO_GUID;
>
> +#define _EFI_DISK_FLAG_DELETED 0x1 /* to be removed */
> +#define _EFI_DISK_FLAG_INVALID 0x8000/* in stale state */
> +
> /**
> * struct efi_disk_obj - EFI disk object
> *
> * @header: EFI object header
> + * @flags: control flags
> * @ops: EFI disk I/O protocol interface
> * @ifname: interface name for block device
> * @dev_index: device index of block device
> @@ -30,6 +34,7 @@ const efi_guid_t efi_block_io_guid = BLOCK_IO_GUID;
> */
> struct efi_disk_obj {
> struct efi_object header;
> + int flags;
> struct efi_block_io ops;
> const char *ifname;
> int dev_index;
> @@ -41,6 +46,35 @@ struct efi_disk_obj {
> struct blk_desc *desc;
> };
>
> +static void efi_disk_mark_deleted(struct efi_disk_obj *disk)
> +{
> + disk->flags |= _EFI_DISK_FLAG_DELETED;
> +}
> +
> +static void
