diff mbox

[U-Boot] distro bootcmd: auto write GPT table in scan_dev_for_boot_part

Message ID 1488782334-31000-1-git-send-email-eddie.cai.linux@gmail.com
State Rejected
Delegated to: Tom Rini
Headers show

Commit Message

Eddie Cai March 6, 2017, 6:38 a.m. UTC
scan_dev_for_boot_part will fail when there is no GPT table. So add auto write
GPT table if fail to get it.

Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
---
 include/config_distro_bootcmd.h | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Tom Rini March 7, 2017, 1:29 p.m. UTC | #1
On Mon, Mar 06, 2017 at 02:38:54PM +0800, Eddie Cai wrote:
> scan_dev_for_boot_part will fail when there is no GPT table. So add auto write
> GPT table if fail to get it.
> 
> Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
> ---
>  include/config_distro_bootcmd.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
> index 0e01e82..3be8ffa 100644
> --- a/include/config_distro_bootcmd.h
> +++ b/include/config_distro_bootcmd.h
> @@ -368,6 +368,14 @@
>  		"\0"                                                      \
>  	\
>  	"scan_dev_for_boot_part="                                         \
> +		"part list ${devtype} ${devnum} -bootable test; "		\
> +		"if env exists test; then "		\
> +			"echo Found valid partition table; "		\
> +		"else "		\
> +			"echo No valid partition table, write the original partition table; "		\
> +			"gpt write ${devtype} ${devnum} ${partitions}; "		\
> +			"mmc rescan;"		\
> +		"fi;"		\
>  		"part list ${devtype} ${devnum} -bootable devplist; "     \
>  		"env exists devplist || setenv devplist 1; "              \
>  		"for distro_bootpart in ${devplist}; do "                 \

Er, this feels rather risky to me.  Why are we writing a table?  How do
we know it exists?
Karsten Merker March 8, 2017, 3 p.m. UTC | #2
On Tue, Mar 07, 2017 at 08:29:52AM -0500, Tom Rini wrote:
> On Mon, Mar 06, 2017 at 02:38:54PM +0800, Eddie Cai wrote:
> > scan_dev_for_boot_part will fail when there is no GPT table. So add auto write
> > GPT table if fail to get it.
> > 
> > Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
> > ---
> >  include/config_distro_bootcmd.h | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
> > index 0e01e82..3be8ffa 100644
> > --- a/include/config_distro_bootcmd.h
> > +++ b/include/config_distro_bootcmd.h
> > @@ -368,6 +368,14 @@
> >  		"\0"                                                      \
> >  	\
> >  	"scan_dev_for_boot_part="                                         \
> > +		"part list ${devtype} ${devnum} -bootable test; "		\
> > +		"if env exists test; then "		\
> > +			"echo Found valid partition table; "		\
> > +		"else "		\
> > +			"echo No valid partition table, write the original partition table; "		\
> > +			"gpt write ${devtype} ${devnum} ${partitions}; "		\
> > +			"mmc rescan;"		\
> > +		"fi;"		\
> >  		"part list ${devtype} ${devnum} -bootable devplist; "     \
> >  		"env exists devplist || setenv devplist 1; "              \
> >  		"for distro_bootpart in ${devplist}; do "                 \
> 
> Er, this feels rather risky to me.  Why are we writing a table?  How do
> we know it exists?

Please _never_ write a partition table as part of the default
distro boot environment, and in particular not a GPT. 

The default boot environment should never perform any write
access to storage media.  Besides this general concern, writing a
GPT regardless of whether a platform can actually use a GPT is a
really bad idea as it will result in non-booting devices on
certain platforms.  An MBR-style partition table only uses the
first sector, but GPT covers the first 34 512-bytes sectors of
the medium and there are a number of systems that have their SPL
inside this area, which means those are effectively dead after
writing a GPT.  One example for this are all sunxi-based systems
on which the boot-ROM loads the SPL from sector 16 of the MMC
device.

Regards,
Karsten
diff mbox

Patch

diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
index 0e01e82..3be8ffa 100644
--- a/include/config_distro_bootcmd.h
+++ b/include/config_distro_bootcmd.h
@@ -368,6 +368,14 @@ 
 		"\0"                                                      \
 	\
 	"scan_dev_for_boot_part="                                         \
+		"part list ${devtype} ${devnum} -bootable test; "		\
+		"if env exists test; then "		\
+			"echo Found valid partition table; "		\
+		"else "		\
+			"echo No valid partition table, write the original partition table; "		\
+			"gpt write ${devtype} ${devnum} ${partitions}; "		\
+			"mmc rescan;"		\
+		"fi;"		\
 		"part list ${devtype} ${devnum} -bootable devplist; "     \
 		"env exists devplist || setenv devplist 1; "              \
 		"for distro_bootpart in ${devplist}; do "                 \