From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1Vl6op-0005EP-55 for mharc-grub-devel@gnu.org; Mon, 25 Nov 2013 19:47:59 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45680) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vl6oe-0004vV-BR for grub-devel@gnu.org; Mon, 25 Nov 2013 19:47:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vl6oU-0004pf-BM for grub-devel@gnu.org; Mon, 25 Nov 2013 19:47:48 -0500 Received: from mail-ea0-x236.google.com ([2a00:1450:4013:c01::236]:34597) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vl6oT-0004pJ-UU for grub-devel@gnu.org; Mon, 25 Nov 2013 19:47:38 -0500 Received: by mail-ea0-f182.google.com with SMTP id o10so4343675eaj.27 for ; Mon, 25 Nov 2013 16:47:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:subject:references :in-reply-to:content-type; bh=FN1X+6u3coJ9gi7KUWuRXtvWMK2T3ZzHKTkzSJtfdMQ=; b=P6vSmL9D3E2TwOSweSXwEow+l5SqmnfIIiAzvrHyTN7kWSpIJ9sPmYSW8/UYkt2Kgc ehEk2rJznQ65hIrJpINE/bvUBOHgo75aciy61xopGpmxY6hMERRIcFGIc2ndmsCr5dsH rUlC3A3RH6tuW8u0Bq/fjGv66cPLvQ76STgZT4Ir5M0X1YiLsFujR1J4tr5g8WsmJP5e 7tEdnTFRratcLOx0Pe0veZbeR+ZzFVF0N8diGmPe7mY5Rr4ohJ5ZtZOyMxzTARzpSbP9 DttMhrw4CxuL65D2pX2zu8evf9Ht+SuJ32aGQqEngSq/yvFal2dMwl18+P0xNN5VgqoV G0ZA== X-Received: by 10.15.81.129 with SMTP id x1mr561895eey.55.1385426856671; Mon, 25 Nov 2013 16:47:36 -0800 (PST) Received: from [192.168.1.121] (31-249.1-85.cust.bluewin.ch. [85.1.249.31]) by mx.google.com with ESMTPSA id b42sm3834709eem.9.2013.11.25.16.47.35 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 25 Nov 2013 16:47:35 -0800 (PST) Message-ID: <5293EFA7.2000002@gmail.com> Date: Tue, 26 Nov 2013 01:47:35 +0100 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20131005 Icedove/17.0.9 MIME-Version: 1.0 To: The development of GNU GRUB Subject: Re: [PATCH v2] Support to disable reed-solomon codes References: <1385419038-23566-1-git-send-email-jonmccune@google.com> In-Reply-To: <1385419038-23566-1-git-send-email-jonmccune@google.com> X-Enigmail-Version: 1.5.1 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="----enig2TDPGXFBFREUVCIDEOFRX" X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:4013:c01::236 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Nov 2013 00:47:57 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2TDPGXFBFREUVCIDEOFRX Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 25.11.2013 23:37, Jon McCune wrote: > [v0] new grub-*-setup flag to disable insertion of reed solomon codes > [v0] grub-install support for option --no-rs-codes > [v1] bugfix: also change the maxsec value in util/setup.c > [v2] Modified to support new C-language install utilities, added docum= entation >=20 > Signed-off-by: Jon McCune > --- > docs/grub.texi | 12 +++++++++- > include/grub/util/install.h | 4 ++-- > util/grub-install.c | 20 ++++++++++++----- > util/grub-setup.c | 11 ++++++++-- > util/setup.c | 53 +++++++++++++++++++++++++------------= -------- > 5 files changed, 66 insertions(+), 34 deletions(-) >=20 > diff --git a/docs/grub.texi b/docs/grub.texi > index 6aee292..29547cd 100644 > --- a/docs/grub.texi > +++ b/docs/grub.texi > @@ -5937,8 +5937,18 @@ mounted on > Recheck the device map, even if @file{/boot/grub/device.map} already > exists. You should use this option whenever you add/remove a disk > into/from your computer. > -@end table > =20 > +@item --no-rs-codes > +By default, @command{grub-install} will use some extra space in the It must be clear that you speak about BIOS only. > +bootloader embedding area for Reed-Solomon error-correcting > +codes. This enables GRUB to still boot successfully if one single > +block is corrupted. We can survive more than one defective sector. If we have r sectors of redundancy we can survive up to r/2 corrupted ones > This redundancy may be cumbersome if attempting > +to cryptographically validate the contents of the bootloader embedding= > +area, or in more modern systems with GPT-style partition tables > +(@pxref{BIOS installation}) where GRUB does not reside in any > +unpartitioned space outside of the MBR. Disable the Reed-Solomon What's the reasonning behind GPT part? > +codes with this option. > +@end table > =20 > @node Invoking grub-mkconfig > @chapter Invoking grub-mkconfig > diff --git a/include/grub/util/install.h b/include/grub/util/install.h > index 5cb33fc..2bfd2ec 100644 > --- a/include/grub/util/install.h > +++ b/include/grub/util/install.h > @@ -176,12 +176,12 @@ void > 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 fs_probe, int allow_floppy, int no_rs_codes); > 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 fs_probe, int allow_floppy, int no_rs_codes); > =20 > char * > grub_install_get_image_targets_string (void); > diff --git a/util/grub-install.c b/util/grub-install.c > index 5354a6d..12d0bc2 100644 > --- a/util/grub-install.c > +++ b/util/grub-install.c > @@ -68,6 +68,7 @@ static int have_load_cfg =3D 0; > static FILE * load_cfg_f =3D NULL; > static char *load_cfg; > static int install_bootsector =3D 1; > +static int no_rs_codes =3D 0; >=20 Negative logic variables are unreadable and prone to errors. Could you use positive logic? > enum > { > @@ -93,7 +94,8 @@ enum > OPTION_DEBUG_IMAGE, > OPTION_NO_FLOPPY, > OPTION_DISK_MODULE, > - OPTION_NO_BOOTSECTOR > + OPTION_NO_BOOTSECTOR, > + OPTION_NO_RS_CODES, > }; > =20 > static int fs_probe =3D 1; > @@ -180,6 +182,10 @@ argp_parser (int key, char *arg, struct argp_state= *state) > install_bootsector =3D 0; > return 0; > =20 > + case OPTION_NO_RS_CODES: > + no_rs_codes =3D 1; > + return 0; > + > case OPTION_DEBUG: > verbosity++; > return 0; > @@ -238,6 +244,8 @@ static struct argp_option options[] =3D { > N_("do not probe for filesystems in DEVICE"), 0}, > {"no-bootsector", OPTION_NO_BOOTSECTOR, 0, 0, > N_("do not install bootsector"), 0}, > + {"no-rs-codes", OPTION_NO_RS_CODES, 0, 0, > + N_("Do not apply any reed-solomon codes when embedding core.img, ev= en if there is enough space."), 0}, > =20 > {"debug", OPTION_DEBUG, 0, OPTION_HIDDEN, 0, 2}, > {"no-floppy", OPTION_NO_FLOPPY, 0, OPTION_HIDDEN, 0, 2}, > @@ -1412,12 +1420,13 @@ main (int argc, char *argv[]) > "boot.img"); > grub_install_copy_file (boot_img_src, boot_img, 1); > =20 > - grub_util_info ("%sgrub-bios-setup %s %s %s %s --directory=3D'%s' --d= evice-map=3D'%s' '%s'", > + grub_util_info ("%sgrub-bios-setup %s %s %s %s %s --directory=3D'%s' = --device-map=3D'%s' '%s'", > install_bootsector ? "" : "NOT RUNNING: ", > allow_floppy ? "--allow-floppy " : "", > verbosity ? "--verbose " : "", > force ? "--force " : "", > !fs_probe ? "--skip-fs-probe" : "", > + no_rs_codes ? "--no-rs-codes" : "", > platdir, > device_map, > install_device); > @@ -1426,7 +1435,7 @@ main (int argc, char *argv[]) > if (install_bootsector) > grub_util_bios_setup (platdir, "boot.img", "core.img", > install_drive, force, > - fs_probe, allow_floppy); > + fs_probe, allow_floppy, no_rs_codes); > break; > } > case GRUB_INSTALL_PLATFORM_SPARC64_IEEE1275: > @@ -1438,12 +1447,13 @@ main (int argc, char *argv[]) > "boot.img"); > grub_install_copy_file (boot_img_src, boot_img, 1); > =20 > - grub_util_info ("%sgrub-sparc64-setup %s %s %s %s --directory=3D'%s' = --device-map=3D'%s' '%s'", > + grub_util_info ("%sgrub-sparc64-setup %s %s %s %s %s --directory=3D'%= s' --device-map=3D'%s' '%s'", > install_bootsector ? "" : "NOT RUNNING: ", > allow_floppy ? "--allow-floppy " : "", > verbosity ? "--verbose " : "", > force ? "--force " : "", > !fs_probe ? "--skip-fs-probe" : "", > + no_rs_codes ? "--no-rs-codes" : "", On sparc it has no effect. > platdir, > device_map, > install_drive); > @@ -1452,7 +1462,7 @@ main (int argc, char *argv[]) > if (install_bootsector) > grub_util_sparc_setup (platdir, "boot.img", "core.img", > install_device, force, > - fs_probe, allow_floppy); > + fs_probe, allow_floppy, no_rs_codes); > break; > } > =20 > diff --git a/util/grub-setup.c b/util/grub-setup.c > index 90b9de0..41344d2 100644 > --- a/util/grub-setup.c > +++ b/util/grub-setup.c > @@ -82,7 +82,8 @@ static struct argp_option options[] =3D { > /* TRANSLATORS: The potential breakage isn't limited to floppies bu= t it's > likely to make the install unbootable from HDD. */ > N_("make the drive also bootable as floppy (default for fdX devices= ). May break on some BIOSes."), 0}, > - > + {"no-rs-codes", 'n', 0, 0, > + N_("Do not apply any reed-solomon codes when embedding core.img, ev= en if there is enough space."), 0}, It doesn't seem to be important enough to give it a letter of its own. > { 0, 0, 0, 0, 0, 0 } > }; > =20 Don't add it on sparc, it doesn't have any effect there. > @@ -118,6 +119,7 @@ struct arguments > int fs_probe; > int allow_floppy; > char *device; > + int no_rs_codes; > }; > =20 > static error_t > @@ -173,6 +175,10 @@ argp_parser (int key, char *arg, struct argp_state= *state) > verbosity++; > break; > =20 > + case 'n': > + arguments->no_rs_codes =3D 1; > + break; > + > case ARGP_KEY_ARG: > if (state->arg_num =3D=3D 0) > arguments->device =3D xstrdup(arg); > @@ -292,7 +298,8 @@ main (int argc, char *argv[]) > arguments.boot_file ? : DEFAULT_BOOT_FILE, > arguments.core_file ? : DEFAULT_CORE_FILE, > dest_dev, arguments.force, > - arguments.fs_probe, arguments.allow_floppy); > + arguments.fs_probe, arguments.allow_floppy, > + arguments.no_rs_codes); > =20 > /* Free resources. */ > grub_fini_all (); > diff --git a/util/setup.c b/util/setup.c > index 337c304..b60a109 100644 > --- a/util/setup.c > +++ b/util/setup.c > @@ -248,7 +248,7 @@ void > SETUP (const char *dir, > const char *boot_file, const char *core_file, > const char *dest, int force, > - int fs_probe, int allow_floppy) > + int fs_probe, int allow_floppy, int no_rs_codes) > { > char *core_path; > char *boot_img, *core_img, *boot_path; > @@ -486,7 +486,11 @@ SETUP (const char *dir, > =20 > nsec =3D core_sectors; > =20 > - maxsec =3D 2 * core_sectors; > + if (!no_rs_codes) > + maxsec =3D 2 * core_sectors; > + else > + maxsec =3D core_sectors; > + > if (maxsec > ((0x78000 - GRUB_KERNEL_I386_PC_LINK_ADDR) > >> GRUB_DISK_SECTOR_BITS)) > maxsec =3D ((0x78000 - GRUB_KERNEL_I386_PC_LINK_ADDR) > @@ -547,27 +551,29 @@ SETUP (const char *dir, > bl.first_block =3D (struct grub_boot_blocklist *) (core_img > + GRUB_DISK_SECTOR_SIZE > - sizeof (*bl.block)); > - > - grub_size_t no_rs_length; > - grub_set_unaligned32 ((core_img + GRUB_DISK_SECTOR_SIZE > - + GRUB_KERNEL_I386_PC_REED_SOLOMON_REDUNDANCY), > - grub_host_to_target32 (nsec * GRUB_DISK_SECTOR_SIZE - core_size))= ; > - no_rs_length =3D grub_target_to_host16=20 > - (grub_get_unaligned16 (core_img > - + GRUB_DISK_SECTOR_SIZE > - + GRUB_KERNEL_I386_PC_NO_REED_SOLOMON_LENGTH)); > - > - if (no_rs_length =3D=3D 0xffff) > - grub_util_error ("%s", _("core.img version mismatch")); > - > - void *tmp =3D xmalloc (core_size); > - grub_memcpy (tmp, core_img, core_size); > - grub_reed_solomon_add_redundancy (core_img + no_rs_length + GRUB_D= ISK_SECTOR_SIZE, > - core_size - no_rs_length - GRUB_DISK_SECTOR_SIZE, > - nsec * GRUB_DISK_SECTOR_SIZE > - - core_size); > - assert (grub_memcmp (tmp, core_img, core_size) =3D=3D 0); > - free (tmp); > + if (!no_rs_codes) > + { > + grub_size_t no_rs_length; > + grub_set_unaligned32 ((core_img + GRUB_DISK_SECTOR_SIZE > + + GRUB_KERNEL_I386_PC_REED_SOLOMON_REDU= NDANCY), > + grub_host_to_target32 (nsec * GRUB_DISK_= SECTOR_SIZE - core_size)); > + no_rs_length =3D grub_target_to_host16 > + (grub_get_unaligned16 (core_img > + + GRUB_DISK_SECTOR_SIZE > + + GRUB_KERNEL_I386_PC_NO_REED_SOLOM= ON_LENGTH)); > + > + if (no_rs_length =3D=3D 0xffff) > + grub_util_error ("%s", _("core.img version mismatch")); > + I'd keep this check > + void *tmp =3D xmalloc (core_size); > + grub_memcpy (tmp, core_img, core_size); > + grub_reed_solomon_add_redundancy (core_img + no_rs_length + GR= UB_DISK_SECTOR_SIZE, > + core_size - no_rs_length - G= RUB_DISK_SECTOR_SIZE, > + nsec * GRUB_DISK_SECTOR_SIZE= > + - core_size); > + assert (grub_memcmp (tmp, core_img, core_size) =3D=3D 0); > + free (tmp); > + } > =20 > /* Write the core image onto the disk. */ > for (i =3D 0; i < nsec; i++) > @@ -762,4 +768,3 @@ unable_to_embed: > grub_device_close (dest_dev); > grub_device_close (root_dev); > } > - >=20 ------enig2TDPGXFBFREUVCIDEOFRX Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.15 (GNU/Linux) Comment: Using GnuPG with Icedove - http://www.enigmail.net/ iF4EAREKAAYFAlKT76cACgkQmBXlbbo5nOsTnwD7BYQ1n+oROJgPSc/mJSh9V0gQ WkGXhtgdiDIh5MTFJlwA/1FAEw2J6QmhsCGbQOTOqta0tY4wcDpharQkL+JQWQ2v =AaPY -----END PGP SIGNATURE----- ------enig2TDPGXFBFREUVCIDEOFRX--