| Message ID | 20210126135623.723327-1-gary.bisson@boundarydevices.com |
|---|---|
| State | Accepted, archived |
| Commit | 4e9bd0623775768be4e1e80ee32a7e42b8c9d3c5 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | disk: part_efi: update partition table entries after write | expand |
On 26.01.21 14:56, Gary Bisson wrote: > Fixes fastboot issues when switching from mbr to gpt partition tables. > > Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com> > --- > Hi, > > I hesitated calling this a RFC as I'm not sure everyone will like the > idea. > > Basically the issue encountered was the following: > - Device with its storage flashed with a MBR partition table > - Run fastboot to flash a new OS (and new partition table) > - After flashing a GPT partition table, U-Boot couldn't find any of the > partition names from the GPT > -> reason is that the partition table entries aren't updated > -> so U-Boot still believes it's a MBR table inside and so its parsing > fails > > This commit adds an update of the table entries inside the > write_mbr_and_gpt_partitions() function. At first I thought maybe this > should be added in fastboot file (fb_mmc.c) but couldn't think of a good > reason why this shouldn't happened in part_efi directly. > > Let me know if you have any questions. > > Regards, > Gary > --- > disk/part_efi.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/disk/part_efi.c b/disk/part_efi.c > index 2f922662e6e..7a24e33d189 100644 > --- a/disk/part_efi.c > +++ b/disk/part_efi.c > @@ -867,6 +867,9 @@ int write_mbr_and_gpt_partitions(struct blk_desc *dev_desc, void *buf) > return 1; > } > > + /* Update the partition table entries*/ > + part_init(dev_desc); > + > return 0; > } > #endif > This change is for GPT only. Don't we need the same for MBR partition tables (write_mbr_partitions())? @Simon: Essentially this reflashing is similar to changing media. What I am missing in part_init() is setting a field with a counter to indicate the media change. The EFI_BLOCK_IO_PROTOCOL has a u32 field media_id. Would struct blk_desc be the right place to add such a field. Best regards Heinrich
Hi Heinrich, On Tue, Jan 26, 2021 at 04:08:13PM +0100, Heinrich Schuchardt wrote: > On 26.01.21 14:56, Gary Bisson wrote: > > Fixes fastboot issues when switching from mbr to gpt partition tables. > > > > Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com> > > --- > > Hi, > > > > I hesitated calling this a RFC as I'm not sure everyone will like the > > idea. > > > > Basically the issue encountered was the following: > > - Device with its storage flashed with a MBR partition table > > - Run fastboot to flash a new OS (and new partition table) > > - After flashing a GPT partition table, U-Boot couldn't find any of the > > partition names from the GPT > > -> reason is that the partition table entries aren't updated > > -> so U-Boot still believes it's a MBR table inside and so its parsing > > fails > > > > This commit adds an update of the table entries inside the > > write_mbr_and_gpt_partitions() function. At first I thought maybe this > > should be added in fastboot file (fb_mmc.c) but couldn't think of a good > > reason why this shouldn't happened in part_efi directly. > > > > Let me know if you have any questions. > > > > Regards, > > Gary > > --- > > disk/part_efi.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/disk/part_efi.c b/disk/part_efi.c > > index 2f922662e6e..7a24e33d189 100644 > > --- a/disk/part_efi.c > > +++ b/disk/part_efi.c > > @@ -867,6 +867,9 @@ int write_mbr_and_gpt_partitions(struct blk_desc *dev_desc, void *buf) > > return 1; > > } > > > > + /* Update the partition table entries*/ > > + part_init(dev_desc); > > + > > return 0; > > } > > #endif > > > > This change is for GPT only. Don't we need the same for MBR partition > tables (write_mbr_partitions())? I actually have another commit for the MBR partition[1], I just wanted to send the GPT one first to see if it was an acceptable approach. Regards, Gary [1] https://github.com/boundarydevices/u-boot-imx6/commit/5fc23bce
On Tue, Jan 26, 2021 at 04:08:13PM +0100, Heinrich Schuchardt wrote: > On 26.01.21 14:56, Gary Bisson wrote: > > Fixes fastboot issues when switching from mbr to gpt partition tables. > > > > Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com> > > --- > > Hi, > > > > I hesitated calling this a RFC as I'm not sure everyone will like the > > idea. > > > > Basically the issue encountered was the following: > > - Device with its storage flashed with a MBR partition table > > - Run fastboot to flash a new OS (and new partition table) > > - After flashing a GPT partition table, U-Boot couldn't find any of the > > partition names from the GPT > > -> reason is that the partition table entries aren't updated > > -> so U-Boot still believes it's a MBR table inside and so its parsing > > fails > > > > This commit adds an update of the table entries inside the > > write_mbr_and_gpt_partitions() function. At first I thought maybe this > > should be added in fastboot file (fb_mmc.c) but couldn't think of a good > > reason why this shouldn't happened in part_efi directly. > > > > Let me know if you have any questions. > > > > Regards, > > Gary > > --- > > disk/part_efi.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/disk/part_efi.c b/disk/part_efi.c > > index 2f922662e6e..7a24e33d189 100644 > > --- a/disk/part_efi.c > > +++ b/disk/part_efi.c > > @@ -867,6 +867,9 @@ int write_mbr_and_gpt_partitions(struct blk_desc *dev_desc, void *buf) > > return 1; > > } > > > > + /* Update the partition table entries*/ > > + part_init(dev_desc); > > + > > return 0; > > } > > #endif > > > > This change is for GPT only. Don't we need the same for MBR partition > tables (write_mbr_partitions())? > > @Simon: > > Essentially this reflashing is similar to changing media. What I am > missing in part_init() is setting a field with a counter to indicate the > media change. The EFI_BLOCK_IO_PROTOCOL has a u32 field media_id. Would > struct blk_desc be the right place to add such a field. As far as "efi_loader" is concerned, none of partitions on the media, either in case of repartitioning or changing media, will no longer be recognized as a "efi disk" after efi subsystem is initialized. All we can and have to do is to just reboot the system. I think we should take one step further to fixing the issue along with this patch. I have posted a couple of proposals in the past, for example, see [1]. [1] https://lists.denx.de/pipermail/u-boot/2019-January/356499.html -Takahiro Akashi > Best regards > > Heinrich >
On 1/27/21 7:33 AM, AKASHI Takahiro wrote: > On Tue, Jan 26, 2021 at 04:08:13PM +0100, Heinrich Schuchardt wrote: >> On 26.01.21 14:56, Gary Bisson wrote: >>> Fixes fastboot issues when switching from mbr to gpt partition tables. >>> >>> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com> >>> --- >>> Hi, >>> >>> I hesitated calling this a RFC as I'm not sure everyone will like the >>> idea. >>> >>> Basically the issue encountered was the following: >>> - Device with its storage flashed with a MBR partition table >>> - Run fastboot to flash a new OS (and new partition table) >>> - After flashing a GPT partition table, U-Boot couldn't find any of the >>> partition names from the GPT >>> -> reason is that the partition table entries aren't updated >>> -> so U-Boot still believes it's a MBR table inside and so its parsing >>> fails >>> >>> This commit adds an update of the table entries inside the >>> write_mbr_and_gpt_partitions() function. At first I thought maybe this >>> should be added in fastboot file (fb_mmc.c) but couldn't think of a good >>> reason why this shouldn't happened in part_efi directly. >>> >>> Let me know if you have any questions. >>> >>> Regards, >>> Gary >>> --- >>> disk/part_efi.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/disk/part_efi.c b/disk/part_efi.c >>> index 2f922662e6e..7a24e33d189 100644 >>> --- a/disk/part_efi.c >>> +++ b/disk/part_efi.c >>> @@ -867,6 +867,9 @@ int write_mbr_and_gpt_partitions(struct blk_desc *dev_desc, void *buf) >>> return 1; >>> } >>> >>> + /* Update the partition table entries*/ >>> + part_init(dev_desc); >>> + >>> return 0; >>> } >>> #endif >>> >> >> This change is for GPT only. Don't we need the same for MBR partition >> tables (write_mbr_partitions())? >> >> @Simon: >> >> Essentially this reflashing is similar to changing media. What I am >> missing in part_init() is setting a field with a counter to indicate the >> media change. The EFI_BLOCK_IO_PROTOCOL has a u32 field media_id. Would >> struct blk_desc be the right place to add such a field. > > As far as "efi_loader" is concerned, none of partitions on the media, > either in case of repartitioning or changing media, will no longer be > recognized as a "efi disk" after efi subsystem is initialized. > All we can and have to do is to just reboot the system. > > I think we should take one step further to fixing the issue along with > this patch. I have posted a couple of proposals in the past, for example, > see [1]. > > [1] https://lists.denx.de/pipermail/u-boot/2019-January/356499.html We have a deficiency in the UEFI sub-system with adding and removing block devices. But this is not a problem of Gary's patch. So from my side: Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
diff --git a/disk/part_efi.c b/disk/part_efi.c index 2f922662e6e..7a24e33d189 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -867,6 +867,9 @@ int write_mbr_and_gpt_partitions(struct blk_desc *dev_desc, void *buf) return 1; } + /* Update the partition table entries*/ + part_init(dev_desc); + return 0; } #endif
Fixes fastboot issues when switching from mbr to gpt partition tables. Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com> --- Hi, I hesitated calling this a RFC as I'm not sure everyone will like the idea. Basically the issue encountered was the following: - Device with its storage flashed with a MBR partition table - Run fastboot to flash a new OS (and new partition table) - After flashing a GPT partition table, U-Boot couldn't find any of the partition names from the GPT -> reason is that the partition table entries aren't updated -> so U-Boot still believes it's a MBR table inside and so its parsing fails This commit adds an update of the table entries inside the write_mbr_and_gpt_partitions() function. At first I thought maybe this should be added in fastboot file (fb_mmc.c) but couldn't think of a good reason why this shouldn't happened in part_efi directly. Let me know if you have any questions. Regards, Gary --- disk/part_efi.c | 3 +++ 1 file changed, 3 insertions(+)