On Wed, 11 Nov 2020, 04:39 Michael Chang, <mch...@suse.com> wrote:
> On Tue, Nov 10, 2020 at 08:49:52PM +0100, Vladimir 'phcoder' Serbinenko
> wrote:
> > From: Vladimir Serbinenko <phco...@gmail.com>
> > Date: Tue, 10 Nov 2020 20:42:12 +0100
> > Subject: [PATCH 1/2] Warn on small MBR gaps on complicated setups
> >
> > Signed-off-by: Vladimir Serbinenko <phco...@gmail.com>
> > ---
> > grub-core/partmap/gpt.c | 9 ++++++++-
> > grub-core/partmap/msdos.c | 7 ++++++-
> > include/grub/partition.h | 3 ++-
> > include/grub/util/install.h | 7 +++++--
> > util/grub-install-common.c | 24 ++++++++++++++++++++++++
> > util/grub-install.c | 9 ++++++---
> > util/grub-setup.c | 2 +-
> > util/setup.c | 5 +++--
> > 8 files changed, 55 insertions(+), 11 deletions(-)
> >
> > diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
> > index 72a2e37cd..38e4eaef8 100644
> > --- a/grub-core/partmap/gpt.c
> > +++ b/grub-core/partmap/gpt.c
> > @@ -25,6 +25,9 @@
> > #include <grub/msdos_partition.h>
> > #include <grub/gpt_partition.h>
> > #include <grub/i18n.h>
> > +#ifdef GRUB_UTIL
> > +#include <grub/emu/misc.h>
> > +#endif
> >
> > GRUB_MOD_LICENSE ("GPLv3+");
> >
> > @@ -169,7 +172,8 @@ static grub_err_t
> > gpt_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
> > unsigned int max_nsectors,
> > grub_embed_type_t embed_type,
> > - grub_disk_addr_t **sectors)
> > + grub_disk_addr_t **sectors,
> > + int warn_short)
> > {
> > struct gpt_partition_map_embed_ctx ctx = {
> > .start = 0,
> > @@ -191,6 +195,9 @@ gpt_partition_map_embed (struct grub_disk *disk,
> > unsigned int *nsectors,
> > N_("this GPT partition label contains no BIOS Boot Partition;"
> > " embedding won't be possible"));
> >
> > + if (ctx.len < 450) {
> > + grub_util_warn("Your BIOS Boot Partition is under 1 MiB, please
> > increase its size.");
> > + }
>
> Why did you drop the GRUB_MIN_RECOMMENDED_MBRGAP introduced in v2 patch
> ? Also the limitation is differently put (450 vs 1900) compared with
> previous version.
>
My fault. I have intended to resubmit v2+a small change with additional
docs as v3 but have resubmitted v1+docs. Will resubmit proper version
>
> > if (ctx.len < *nsectors)
> > return grub_error (GRUB_ERR_OUT_OF_RANGE,
> > N_("your BIOS Boot Partition is too small;"
> > diff --git a/grub-core/partmap/msdos.c b/grub-core/partmap/msdos.c
> > index ee3f24982..d1b81cbd0 100644
> > --- a/grub-core/partmap/msdos.c
> > +++ b/grub-core/partmap/msdos.c
> > @@ -236,7 +236,8 @@ static grub_err_t
> > pc_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
> > unsigned int max_nsectors,
> > grub_embed_type_t embed_type,
> > - grub_disk_addr_t **sectors)
> > + grub_disk_addr_t **sectors,
> > + int warn_short)
> > {
> > grub_disk_addr_t end = ~0ULL;
> > struct grub_msdos_partition_mbr mbr;
> > @@ -390,6 +391,10 @@ pc_partition_map_embed (struct grub_disk *disk,
> > unsigned int *nsectors,
> > return GRUB_ERR_NONE;
> > }
> >
> > + if (end < 450 && warn_short) {
> > + grub_util_warn("You have a short MBR gap and use advanced config.
> > Please increase post-MBR gap");
> > + }
> > +
>
> Same above.
>
> In addition, the short mbr warning seems to be emitted only if certain
> embedding error happened while it looked to me that the warning should be
> used to remind user the potential issue before any error encountered.
>
> Thanks,
> Michael
>
>
> > if (end <= 1)
> > return grub_error (GRUB_ERR_FILE_NOT_FOUND,
> > N_("this msdos-style partition label has no "
> > diff --git a/include/grub/partition.h b/include/grub/partition.h
> > index 7adb7ec6e..5697e28d5 100644
> > --- a/include/grub/partition.h
> > +++ b/include/grub/partition.h
> > @@ -55,7 +55,8 @@ struct grub_partition_map
> > grub_err_t (*embed) (struct grub_disk *disk, unsigned int *nsectors,
> > unsigned int max_nsectors,
> > grub_embed_type_t embed_type,
> > - grub_disk_addr_t **sectors);
> > + grub_disk_addr_t **sectors,
> > + int warn_short);
> > #endif
> > };
> > typedef struct grub_partition_map *grub_partition_map_t;
> > diff --git a/include/grub/util/install.h b/include/grub/util/install.h
> > index 2631b1074..982115f57 100644
> > --- a/include/grub/util/install.h
> > +++ b/include/grub/util/install.h
> > @@ -193,13 +193,13 @@ grub_util_bios_setup (const char *dir,
> > const char *boot_file, const char *core_file,
> > const char *dest, int force,
> > int fs_probe, int allow_floppy,
> > - int add_rs_codes);
> > + int add_rs_codes, int warn_short_mbr_gap);
> > void
> > grub_util_sparc_setup (const char *dir,
> > const char *boot_file, const char *core_file,
> > const char *dest, int force,
> > int fs_probe, int allow_floppy,
> > - int add_rs_codes);
> > + int add_rs_codes, int warn_short_mbr_gap);
> >
> > char *
> > grub_install_get_image_targets_string (void);
> > @@ -265,4 +265,7 @@ grub_util_get_target_name (const struct
> > grub_install_image_target_desc *t);
> > extern char *grub_install_copy_buffer;
> > #define GRUB_INSTALL_COPY_BUFFER_SIZE 1048576
> >
> > +int
> > +grub_install_is_short_mgrgap_supported(void);
> > +
> > #endif
> > diff --git a/util/grub-install-common.c b/util/grub-install-common.c
> > index 277eaf4e2..5696507a9 100644
> > --- a/util/grub-install-common.c
> > +++ b/util/grub-install-common.c
> > @@ -234,6 +234,30 @@ char *grub_install_source_directory = NULL;
> > char *grub_install_locale_directory = NULL;
> > char *grub_install_themes_directory = NULL;
> >
> > +int
> > +grub_install_is_short_mgrgap_supported()
> > +{
> > + int i, j;
> > + static const char *whitelist[] =
> > + {
> > + "part_msdos", "biosdisk", "affs", "afs", "bfs", "archelp",
> > + "cpio", "cpio_be", "newc", "odc", "ext2", "fat", "exfat",
> > + "f2fs", "fshelp", "hfs", "hfsplus",
> > + "iso9660", "jfs", "minix", "minix2", "minix3", "minix_be",
> > + "minix2_be", "minix2_be", "nilfs2", "ntfs", "ntfscomp",
> > + "reiserfs", "romfs", "sfs", "tar", "udf",
> > + "ufs1", "ufs1_be", "ufs2", "xfs"
> > + };
> > + for (i = 0; i < modules.n_entries; i++) {
> > + for (j = 0; j < ARRAY_SIZE (whitelist); j++)
> > + if (strcmp(modules.entries[i], whitelist[j]) == 0)
> > + break;
> > + if (j == ARRAY_SIZE (whitelist))
> > + return 0;
> > + }
> > + return 1;
> > +}
> > +
> > void
> > grub_install_push_module (const char *val)
> > {
> > diff --git a/util/grub-install.c b/util/grub-install.c
> > index a35a2e2e8..6813f3144 100644
> > --- a/util/grub-install.c
> > +++ b/util/grub-install.c
> > @@ -1718,10 +1718,13 @@ main (int argc, char *argv[])
> > install_device);
> >
> > /* Now perform the installation. */
> > - if (install_bootsector)
> > + if (install_bootsector) {
> > + int support_short_mbr_gap = grub_install_is_short_mgrgap_supported();
> > grub_util_bios_setup (platdir, "boot.img", "core.img",
> > install_drive, force,
> > - fs_probe, allow_floppy, add_rs_codes);
> > + fs_probe, allow_floppy, add_rs_codes,
> > + !support_short_mbr_gap);
> > + }
> > break;
> > }
> > case GRUB_INSTALL_PLATFORM_SPARC64_IEEE1275:
> > @@ -1748,7 +1751,7 @@ main (int argc, char *argv[])
> > grub_util_sparc_setup (platdir, "boot.img", "core.img",
> > install_drive, force,
> > fs_probe, allow_floppy,
> > - 0 /* unused */ );
> > + 0 /* unused */, 0 /* unused */ );
> > break;
> > }
> >
> > diff --git a/util/grub-setup.c b/util/grub-setup.c
> > index 42b98ad3c..1783224dd 100644
> > --- a/util/grub-setup.c
> > +++ b/util/grub-setup.c
> > @@ -315,7 +315,7 @@ main (int argc, char *argv[])
> > arguments.core_file ? : DEFAULT_CORE_FILE,
> > dest_dev, arguments.force,
> > arguments.fs_probe, arguments.allow_floppy,
> > - arguments.add_rs_codes);
> > + arguments.add_rs_codes, 0);
> >
> > /* Free resources. */
> > grub_fini_all ();
> > diff --git a/util/setup.c b/util/setup.c
> > index 3be88aae1..acba4eca1 100644
> > --- a/util/setup.c
> > +++ b/util/setup.c
> > @@ -254,7 +254,8 @@ SETUP (const char *dir,
> > const char *boot_file, const char *core_file,
> > const char *dest, int force,
> > int fs_probe, int allow_floppy,
> > - int add_rs_codes __attribute__ ((unused))) /* unused on sparc64
> */
> > + int add_rs_codes __attribute__ ((unused)), /* unused on sparc64
> */
> > + int warn_small __attribute__ ((unused))) /* unused on sparc64 */
> > {
> > char *core_path;
> > char *boot_img, *core_img, *boot_path;
> > @@ -530,7 +531,7 @@ SETUP (const char *dir,
> > GRUB_EMBED_PCBIOS, §ors);
> > else if (ctx.dest_partmap)
> > err = ctx.dest_partmap->embed (dest_dev->disk, &nsec, maxsec,
> > - GRUB_EMBED_PCBIOS, §ors);
> > + GRUB_EMBED_PCBIOS, §ors, warn_small);
> > else
> > err = fs->fs_embed (dest_dev, &nsec, maxsec,
> > GRUB_EMBED_PCBIOS, §ors);
> > --
> > 2.20.1
> >
> > --
> > Regards
> > Vladimir 'phcoder' Serbinenko
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
