Re: Additional Relocations (R_ARM_THM_MOVT_ABS) support in mkimage.

[Vladimir 'phcoder' Serbinenko]

On Mon, 28 May 2018, 23:08 amit94,  wrote:

> Hi,
>
> I'm trying to cross compile grub for armv7, and I get these errors on
> running `grub-mkstandalone`
> > grub-mkstandalone: error: relocation 0x2f is not implemented yet.
>
> The issue seems to be that the R_ARM_THM_MOVT_ABS_NC is not supported in
> the util/grub-mkimagexx.c file. I searched a bit and found that support for
> this was added to grub-core/kern/arm/dl.c in commit
> c0f529ea679bdf48cbe1c4b3dd92fb552456a1fe by Leif Lindholm.
>
> Is it possible to implement this here?
>
Not easily because PE format used by EFI doesn't have an equivalent. My
recommendation is to remove -mthumb if you have this problem

>
> Thanks,
> Amit.
>
> ___
> 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

Re: [PATCH] Enable /dev/mapper/dm-[0-9]-* scanning

[Vladimir 'phcoder' Serbinenko]

On Tue, 29 May 2018, 13:58 Oleg Solovyov,  wrote:

> Anybody?
>
Why do you need this? Normally the same devices should be under
/dev/mapper. Using dm-X will mess up some other code most likely

>
>
> 14.05.2018 20:13, Daniel Kiper пишет:
> > On Fri, May 11, 2018 at 02:32:48PM +0300, Oleg Solovyov wrote:
> >> Thanks for review
> >> New version is attached
> >>
> >> From 19e3f13632a20a0b1be12b6d6ff4c52ba4f3b4d6 Mon Sep 17 00:00:00 2001
> >> From: Oleg Solovyov 
> >> Date: Fri, 11 May 2018 13:55:46 +0300
> >> Subject: [PATCH] Don't skip /dev/mapper/dm-* devices
> >>
> >> This patch ensures that grub-probe will find the root device placed in
> >> /dev/mapper/dm-[0-9]+-.*
> >> e.g. device named /dev/mapper/dm-0-luks will be found and grub.cfg will
> >> be updated properly, enabling the system to boot.
> >>
> >> Signed-off-by: Oleg Solovyov 
> > Reviewed-by: Daniel Kiper 
> >
> > If there are no objections I will apply this patch in a week or so.
> >
> > Next time please use "git send-email" to send the patches.
> >
> > Daniel
> >
> > ___
> > 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

Re: Add MD5 support

[Vladimir 'phcoder' Serbinenko]

On Wed, 18 Jul 2018, 03:13 Samir Aguiar,  wrote:

> Hi all,
>
> We need support for MD5 passwords - which were used in grub legacy - so
> we can update to 2.x. Basically we can't migrate existing installations
> since it's one-way hashing and so on.
>
> I will start working on it, but I would like to ask you two things first:
> (1) Is it something that could be merged upstream?
> (2) Would you have any advice to give on this? Maybe someone already
> started working on it or there are some gotchas to be aware of.
>
Look at legacy_password command. It's already there

>
> Thank you in advance.
>
> Kind regards,
> Samir Aguiar
>
> ___
> 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

Re: Recent removal of a.out and COFF support for sparc

[Vladimir 'phcoder' Serbinenko]

I can change code to do conversion to coff ourselves.

вт, 7 авг. 2018 г., 12:56 John Paul Adrian Glaubitz <
glaub...@physik.fu-berlin.de>:

> Hello!
>
> binutils/bfd recently removed a.out and COFF support for sparc [1].
>
> Unfortunately, this means that we are no longer able to built GRUB
> or SILO for sparc/sparc64 which need to be built as a.out binaries
> as the Open Boot Firmware requires them to be in a.out format [2].
>
> I would therefore like to ask to start a discussion about a potential
> reversal of this commit as I don't think we can forgo being able
> to build a bootloader on sparc/sparc64.
>
> Also, since m68k is still very actively maintained (there is even
> LLVM support in the works now) and since AmigaOS uses COFF, I would
> to ask for the a.out and COFF support for m68k to be reinstated
> as well [3].
>
> Thanks,
> Adrian
>
> > [1]
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=c9098af41e3246586a30f4f0bdb0ee4367e9a5e7
> > [2] https://docs.oracle.com/cd/E19457-01/801-7042/801-7042.pdf
> > [3]
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=dc12032bca08554cf0a72d224e44f755f7789ff3
>
> --
>  .''`.  John Paul Adrian Glaubitz
> : :' :  Debian Developer - glaub...@debian.org
> `. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
>   `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
>
> ___
> 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

Re: questions about relocator16.S

[Vladimir 'phcoder' Serbinenko]

вт, 14 авг. 2018 г., 9:11 Cao jin :

> Hi,
>
>   I am learning these code, and now have 2 questions:
>
> 1. at the bottom of relocator_common.S, there is:
>
> LOCAL(jump_vector):
> /* Jump location. Is filled by the code */
> .long   0
> .long   CODE_SEGMENT
>
> Since it is offset & segment selector value, why is not ".word
> CODE_SEGMENT", is there any subtle difference?
>
i386 is little endian, so effectively it just reserves 2 bytes more and
fills them with zeros.

>
> 2. in relocator16.S, after "RELOAD_GDT", the other segment registers are
> updated two times with DATA_SEGMENT & PSEUDO_REAL_DSEG, they are pretty
> close, there is no memory read/write between them, so why we need the
> updates with DATA_SEGMENT?
>
It's derived from older code that may have had accesses there. But it was
tested on many CPUs and changing it risks triggering obscure bug in an
obscure CPU

>
> --
> Sincerely,
> Cao jin
>
>
>
> ___
> 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

Re: Legacy Firmware/INT 13H supporting >512 byte sectors?

[Vladimir 'phcoder' Serbinenko]

пт, 17 авг. 2018 г., 17:14 Mihai Moldovan :

> Hi
>
>
> Just a quick question: did anyone ever come up a BIOS firmware that
> supports
> sector sizes higher than 512 bytes (say 4096 bytes)?
>
Sure. Any BIOS able to read CD supports 2K sectors.
If you mean HDD then I have never seen one. I wrote support for non-512B
sectors in GRUB but so far it's untested as I couldn't find such a BIOS

>
>
>
> Mihai
>
> ___
> 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

Re: [PATCH v3] Makefile.am: Add `all_video` to `default_payload.elf`

[Vladimir 'phcoder' Serbinenko]

Unfortunately this pulls in some problematic graphics drivers that
shouldn't be in default payload

On Thu, 6 Sep 2018, 14:59 Daniel Kiper,  wrote:

> On Thu, Aug 09, 2018 at 08:20:07AM +0200, Paul Menzel wrote:
> > Date: Sun, 16 Apr 2017 21:02:58 +0200
> >
> > The module `all_video` is used in `util/grub.d/00_header.in`, and
> > included for grub-pc by default.
> >
> > To make it easier to load such a GRUB configuration from disk with a
> > GRUB built as a coreboot payload, add this module also to
> > `default_payload.elf` by default. That avoids a missing module error by
> > GRUB, forcing the user to hit enter during the start process.
> >
> > error: file `/boot/grub/i386-coreboot/all_video.mod' not found.
> >
>
> Lack of of SOB... :-((( Otherwise LGTM. If I get your SOB in a reply
> to this email or something like that then I will get this patch...
>
> Daniel
>
> ___
> 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

Re: [PATCH] osdep/linux: convert partition start to disk sector length.

[Vladimir 'phcoder' Serbinenko]

Grub partition get start returns in 512B blocks. Can I have more details
like which partmap you use?

On Mon, 23 Jul 2018, 09:54 Mihai Moldovan,  wrote:

> When reading data off a disk, sector values are based on the disk sector
> length.
>
> Within grub_util_fd_open_device(), the start of the partition was taken
> directly from grub's partition information structure, which uses the
> internal sector length (currently 512b), but never transformed to the
> disk's sector length.
>
> Subsequent calculations were all wrong for devices that have a diverging
> sector length and the functions eventually skipped to the wrong stream
> location, reading invalid data.
> ---
>  grub-core/osdep/linux/hostdisk.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/osdep/linux/hostdisk.c
> b/grub-core/osdep/linux/hostdisk.c
> index 06179fca7..8b92f8528 100644
> --- a/grub-core/osdep/linux/hostdisk.c
> +++ b/grub-core/osdep/linux/hostdisk.c
> @@ -374,7 +374,8 @@ grub_util_fd_open_device (const grub_disk_t disk,
> grub_disk_addr_t sector, int f
>  char dev[PATH_MAX];
>  grub_disk_addr_t part_start = 0;
>
> -part_start = grub_partition_get_start (disk->partition);
> +part_start = grub_partition_get_start (disk->partition)
> + >> (disk->log_sector_size - GRUB_DISK_SECTOR_BITS);
>
>  strncpy (dev, grub_util_biosdisk_get_osdev (disk), sizeof (dev) - 1);
>  dev[sizeof(dev) - 1] = '\0';
> --
> 2.15.1
>
>
> ___
> 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

Re: [PATCH v2] grub-install: Mention *efibootmgr* in `no-nvram` description

[Vladimir 'phcoder' Serbinenko]

On Thu, 6 Sep 2018, 17:03 Paul Menzel,  wrote:

> Date: Thu, 6 Sep 2018 15:15:45 +0200
>
> If executing `efibootmgr` fails, for example, because the system is
> booted in legacy mode, but with a mounted EFI System Partition, the
> error message mentions *efibootmgr*. To make it easier for users to work
> around that problem by skipping running `efibootmgr`, add the word to
> the description of the switch *no-nvram*, which skips the efibootmgr
> execution.
>
> Signed-off-by: Paul Menzel 
> ---
> v2: Add Signed-off-by line
>
>  util/grub-install.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/util/grub-install.c b/util/grub-install.c
> index a44858198..335662a58 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -276,7 +276,7 @@ static struct argp_option options[] = {
> N_("disk module to use (biosdisk or native). "
>"This option is only available on BIOS target."), 2},
>{"no-nvram", OPTION_NO_NVRAM, 0, 0,
> -   N_("don't update the `boot-device'/`Boot*' NVRAM variables. "
> +   N_("don't run efibootmgr to update the `boot-device'/`Boot*' NVRAM
> variables. "
>"This option is only available on EFI and IEEE1275 targets."), 2},
>
In case of ieee1275 this has nothing to do with efibootmgr

>{"skip-fs-probe",'s',0,  0,
> N_("do not probe for filesystems in DEVICE"), 0},
> --
> 2.17.1
>
> ___
> 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

Re: arm64/xen: Remove duplicate check for arguments

[Vladimir 'phcoder' Serbinenko]

On Tue, 20 Nov 2018, 14:40 Lee Jones,  wrote:

> From: Lee Jones 
>
> arm64/xen: Remove duplicate check for arguments
>
It's not duplicate because of argc-- right before it

>
> Signed-off-by: Lee Jones 
>
> diff --git a/grub-core/loader/arm64/xen_boot.c
> b/grub-core/loader/arm64/xen_boot.c
> index 1b1dd8c65..5af15c70d 100644
> --- a/grub-core/loader/arm64/xen_boot.c
> +++ b/grub-core/loader/arm64/xen_boot.c
> @@ -404,12 +404,6 @@ grub_cmd_xen_module (grub_command_t cmd
> __attribute__((unused)),
>argc--;
>  }
>
> -  if (!argc)
> -{
> -  grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
> -  goto fail;
> -}
> -
>if (!loaded)
>  {
>grub_error (GRUB_ERR_BAD_ARGUMENT,
>
> ___
> 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

Re: [PATCH v3 (part 2)] generic/blocklist: Fix implicit declaration of function ‘grub_file_filter_disable_compression'

[Vladimir 'phcoder' Serbinenko]

LGTM

On Tue, 20 Nov 2018, 14:40 Lee Jones,  wrote:

> From: Lee Jones 
>
> generic/blocklist: Fix implicit declaration of function
> ‘grub_file_filter_disable_compression'
>
> grub_file_filter_disable_compression() no longer exists.
>
> Signed-off-by: Lee Jones 
>
> diff --git a/grub-core/osdep/generic/blocklist.c
> b/grub-core/osdep/generic/blocklist.c
> index 74024fd06..43186949d 100644
> --- a/grub-core/osdep/generic/blocklist.c
> +++ b/grub-core/osdep/generic/blocklist.c
> @@ -59,8 +59,7 @@ grub_install_get_blocklist (grub_device_t root_dev,
>
>grub_disk_cache_invalidate_all ();
>
> -  grub_file_filter_disable_compression ();
> -  file = grub_file_open (core_path_dev);
> +  file = grub_file_open (core_path_dev, FILE_TYPE_NO_DECOMPRESS);
>if (file)
> {
>   if (grub_file_size (file) != core_size)
> @@ -117,8 +116,7 @@ grub_install_get_blocklist (grub_device_t root_dev,
>
>grub_file_t file;
>/* Now read the core image to determine where the sectors are.  */
> -  grub_file_filter_disable_compression ();
> -  file = grub_file_open (core_path_dev);
> +  file = grub_file_open (core_path_dev, FILE_TYPE_NO_DECOMPRESS);
>if (! file)
>  grub_util_error ("%s", grub_errmsg);
>
> ___
> 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 2.04 freeze date announcement

[Vladimir 'phcoder' Serbinenko]

Hello everyone!

We are happy to announce that the upcoming release of Grub 2.04 is expected
in March. In order to prepare the release we would freeze on Monday, March
11, 23:59:59 UTC. From that point no new features are accepted unless there
is a very good reason to. We will still accept bugfixes. In fact we kindly
ask to test systems and features you care about

All the best from  GRUB maintainers team
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] POTFILES: Don't include gnulib in grub.pot

[Vladimir 'phcoder' Serbinenko]

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-POTFILES-Don-t-include-gnulib-in-grub.pot.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] efi/tpm.c: Add missing casts

[Vladimir 'phcoder' Serbinenko]

-- 
Regards
Vladimir 'phcoder' Serbinenko


0002-efi-tpm.c-Add-missing-casts.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Propagate GNU_PRINTF from gnulib vfprintf

[Vladimir 'phcoder' Serbinenko]

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-Propagate-GNU_PRINTF-from-gnulib-vfprintf.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] grub-mkimagexx: Fix RISCV error message

[Vladimir 'phcoder' Serbinenko]

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-grub-mkimagexx-Fix-RISCV-error-message.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Rename grub_disk members

[Vladimir 'phcoder' Serbinenko]

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-Rename-grub_disk-members.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] kern/emu/misc.c: Don't include config-util.h when running as GRUB_BUILD

[Vladimir 'phcoder' Serbinenko]

-- 
Regards
Vladimir 'phcoder' Serbinenko


0003-kern-emu-misc.c-Don-t-include-config-util.h-when-run.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] efi/tpm.h: Fix hash_log_extend_event definition

[Vladimir 'phcoder' Serbinenko]

-- 
Regards
Vladimir 'phcoder' Serbinenko


0002-efi-tpm.h-Fix-hash_log_extend_event-definition.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Support R_PPC_PLTREL24

[Vladimir 'phcoder' Serbinenko]

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-Support-R_PPC_PLTREL24.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] efi/tpm.c: Add missing casts

[Vladimir 'phcoder' Serbinenko]

On Mon, Mar 25, 2019 at 1:09 PM Daniel Kiper  wrote:
>
> On Fri, Mar 22, 2019 at 04:11:10PM +0100, Vladimir 'phcoder' Serbinenko wrote:
> > From 9443713c3d3403a21aac9b39de4bbd097a531dd4 Mon Sep 17 00:00:00 2001
> > From: Vladimir Serbinenko 
> > Date: Fri, 22 Mar 2019 16:07:47 +0100
> > Subject: [PATCH 2/2] efi/tpm.c: Add missing casts
>
> Have you seen any warnings without this patch?
>
Yes, That we make either incompatible pointer type or integer from pointer.
> > ---
> >  grub-core/commands/efi/tpm.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/grub-core/commands/efi/tpm.c b/grub-core/commands/efi/tpm.c
> > index 7475fd87b..32909c192 100644
> > --- a/grub-core/commands/efi/tpm.c
> > +++ b/grub-core/commands/efi/tpm.c
> > @@ -245,7 +245,7 @@ grub_tpm1_log_event (grub_efi_handle_t tpm_handle, 
> > unsigned char *buf,
> >grub_memcpy (event->Event, description, event->EventSize);
> >
> >algorithm = TCG_ALG_SHA;
> > -  status = efi_call_7 (tpm->log_extend_event, tpm, buf, (grub_uint64_t) 
> > size,
> > +  status = efi_call_7 (tpm->log_extend_event, tpm, (grub_addr_t) buf, 
> > (grub_uint64_t) size,
> >  algorithm, event, &eventnum, &lastevent);
> >
> >switch (status)
> > @@ -295,7 +295,7 @@ grub_tpm2_log_event (grub_efi_handle_t tpm_handle, 
> > unsigned char *buf,
> >  sizeof (*event) - sizeof (event->Event) + grub_strlen (description) + 
> > 1;
> >grub_memcpy (event->Event, description, grub_strlen (description) + 1);
> >
> > -  status = efi_call_5 (tpm->hash_log_extend_event, tpm, 0, buf,
> > +  status = efi_call_5 (tpm->hash_log_extend_event, tpm, 0, (grub_addr_t) 
> > buf,
>
> Should not you use grub_efi_physical_address_t instead of grub_addr_t?
Nope. On 32-bit system we can cast the pointer only to 32-bit but
grub_efi_physical_address_t is 64-bit. Casting pointer directly to
64-bit create a warning. This way we have explicit cast to addr_t and
then implicit cat to efi_physical_address_t.
>
> Daniel



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Propagate GNU_PRINTF from gnulib vfprintf

[Vladimir 'phcoder' Serbinenko]

On Mon, Mar 25, 2019 at 1:24 PM Daniel Kiper  wrote:
>
> On Fri, Mar 22, 2019 at 04:15:35PM +0100, Vladimir 'phcoder' Serbinenko wrote:
> > From ce156e3274e53089b7a9ba432262b8aab75d8ad0 Mon Sep 17 00:00:00 2001
> > From: Vladimir Serbinenko 
> > Date: Fri, 22 Mar 2019 16:13:11 +0100
> > Subject: [PATCH] Propagate GNU_PRINTF from gnulib vfprintf
> >
> > gnulib now replaces vfprintf and hence it becomes GNU_PRINTF
>
> Missing SOB.
>
> > ---
> >  include/grub/emu/misc.h | 14 +-
> >  include/grub/err.h  |  3 ++-
> >  2 files changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/grub/emu/misc.h b/include/grub/emu/misc.h
> > index 59b8b35fc..ce464cfd0 100644
> > --- a/include/grub/emu/misc.h
> > +++ b/include/grub/emu/misc.h
> > @@ -24,6 +24,7 @@
> >
> >  #include 
> >
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -43,22 +44,17 @@ char *grub_make_system_path_relative_to_its_root (const 
> > char *path)
> >  int
> >  grub_util_device_is_mapped (const char *dev);
> >
> > -#ifdef __MINGW32__
> > -#define GRUB_HOST_PRIuLONG_LONG "I64u"
> > -#define GRUB_HOST_PRIxLONG_LONG "I64x"
> > -#else
> >  #define GRUB_HOST_PRIuLONG_LONG "llu"
> >  #define GRUB_HOST_PRIxLONG_LONG "llx"
> > -#endif
>
> It seems to me that this does not belong to the patch.
It is. It's all the effect of using GNU format rather than platform one.
>
> >  void * EXPORT_FUNC(xmalloc) (grub_size_t size) WARN_UNUSED_RESULT;
> >  void * EXPORT_FUNC(xrealloc) (void *ptr, grub_size_t size) 
> > WARN_UNUSED_RESULT;
> >  char * EXPORT_FUNC(xstrdup) (const char *str) WARN_UNUSED_RESULT;
> > -char * EXPORT_FUNC(xasprintf) (const char *fmt, ...) __attribute__ 
> > ((format (__printf__, 1, 2))) WARN_UNUSED_RESULT;
> > +char * EXPORT_FUNC(xasprintf) (const char *fmt, ...) __attribute__ 
> > ((format (GNU_PRINTF, 1, 2))) WARN_UNUSED_RESULT;
>
> Commit message seems confusing. You are refering to vfprintf() but here
> and below you are replacing __printf__. Could fix that?
__printf__ is platform format. GNU_PRINTF is GNU format. By replacing
vfprintf basically all format functions ended up using GNU format
>
> Daniel



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Fwd: [PATCH] POTFILES: Don't include gnulib in grub.pot

[Vladimir 'phcoder' Serbinenko]

-- Forwarded message -
From: Vladimir 'phcoder' Serbinenko 
Date: Mon, Mar 25, 2019 at 3:22 PM
Subject: Re: [PATCH] POTFILES: Don't include gnulib in grub.pot
To: Daniel Kiper 


On Mon, Mar 25, 2019 at 12:58 PM Daniel Kiper  wrote:
>
> Sadly this patch does not look complete. ./grub-core/lib/gnulib is still
> included. And as Colin pointed out missing SOB.
This directory still exists, at least in my copy. I didn't spend any
time figuring out if this is intended or stale. In either case I don't
want those file to accidently end up in POTFILES


--
Regards
Vladimir 'phcoder' Serbinenko


-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2] POTFILES: Don't include gnulib in grub.pot

[Vladimir 'phcoder' Serbinenko]

>From 25923d90b80709b11969cd56baddd8e898d7b9b5 Mon Sep 17 00:00:00 2001
From: Vladimir Serbinenko 
Date: Fri, 22 Mar 2019 16:06:18 +0100
Subject: [PATCH] POTFILES: Don't include gnulib in grub.pot

They're translated as a separate project, so we
don't want to submit them again.

Signed-off-by: Vladimir Serbinenko 
---
 autogen.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/autogen.sh b/autogen.sh
index d9df5f9be..ef43270fc 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -13,7 +13,7 @@ fi
 export LC_COLLATE=C
 unset LC_ALL

-find . -iname '*.[ch]' ! -ipath './grub-core/lib/libgcrypt-grub/*' !
-ipath './build-aux/*' ! -ipath './grub-core/lib/libgcrypt/src/misc.c'
! -ipath './grub-core/lib/libgcrypt/src/global.c' ! -ipath
'./grub-core/lib/libgcrypt/src/secmem.c'  ! -ipath
'./util/grub-gen-widthspec.c' ! -ipath './util/grub-gen-asciih.c'
|sort > po/POTFILES.in
+find . -iname '*.[ch]' ! -ipath './grub-core/lib/libgcrypt-grub/*' !
-ipath './build-aux/*' ! -ipath './grub-core/lib/libgcrypt/src/misc.c'
! -ipath './grub-core/lib/libgcrypt/src/global.c' ! -ipath
'./grub-core/lib/libgcrypt/src/secmem.c'  ! -ipath
'./util/grub-gen-widthspec.c' ! -ipath './util/grub-gen-asciih.c' !
-ipath './gnulib/*' ! -iname './grub-core/lib/gnulib/*' |sort >
po/POTFILES.in
 find util -iname '*.in' ! -name Makefile.in  |sort > po/POTFILES-shell.in

 echo "Importing unicode..."
-- 
2.11.0


-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] hostfs: #undef open and close.

[Vladimir 'phcoder' Serbinenko]

>From 0cfd6226d200cce722e99230f5564e1bd9e45eff Mon Sep 17 00:00:00 2001
From: Vladimir Serbinenko 
Date: Mon, 25 Mar 2019 16:14:54 +0100
Subject: [PATCH] hostfs: #undef open and close.

Unlike in case of disks in this case it's just a single place, so it's easier to
just #undef

Signed-off-by: Vladimir Serbinenko 
---
 grub-core/kern/emu/hostfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/grub-core/kern/emu/hostfs.c b/grub-core/kern/emu/hostfs.c
index 7b28c001f..0c6afcfae 100644
--- a/grub-core/kern/emu/hostfs.c
+++ b/grub-core/kern/emu/hostfs.c
@@ -173,6 +173,9 @@ grub_hostfs_label (grub_device_t device
__attribute ((unused)),
   return GRUB_ERR_NONE;
 }

+#undef open
+#undef close
+
 static struct grub_fs grub_hostfs_fs =
   {
 .name = "hostfs",
-- 
2.11.0


-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] configure.ac: Use nostdlib when checking for nostdinc

[Vladimir 'phcoder' Serbinenko]

>From 3269baa84c370e22a1cbda9b8cdeed7906b42631 Mon Sep 17 00:00:00 2001
From: Vladimir Serbinenko 
Date: Mon, 25 Mar 2019 16:09:04 +0100
Subject: [PATCH] configure.ac: Use nostdlib when checking for nostdinc

With clang nostdinc behaviour is influenced by nostdlib. Since we
always add nostdlib, add it in test as well

Signed-off-by: Vladimir Serbinenko 
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 05d040cd6..2f53a5683 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1395,7 +1395,7 @@ AC_SUBST(TARGET_NMFLAGS_DEFINED_ONLY)
 if test "$platform" != emu; then
 AC_CACHE_CHECK([whether -nostdinc -isystem works], [grub_cv_cc_isystem], [
   SAVED_CPPFLAGS="$CPPFLAGS"
-  CPPFLAGS="$TARGET_CPPFLAGS -nostdinc -isystem `$TARGET_CC
-print-file-name=include`"
+  CPPFLAGS="$TARGET_CPPFLAGS -nostdlib -nostdinc -isystem `$TARGET_CC
-print-file-name=include`"
   AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include 
 #include 
 int va_arg_func (int fixed, va_list args);]], [[]])],
-- 
2.11.0


-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] A workaround for clang problem assembling startup_raw.S

[Vladimir 'phcoder' Serbinenko]

>From 5f64575630d30cba24f3c57a865d8f4b094690b7 Mon Sep 17 00:00:00 2001
From: Vladimir Serbinenko 
Date: Tue, 26 Mar 2019 14:56:59 +0100
Subject: [PATCH] A workaround for clang problem assembling startup_raw.S

Signed-off-by: Vladimir Serbinenko 
---
 grub-core/boot/i386/pc/startup_raw.S | 9 +
 1 file changed, 9 insertions(+)

diff --git a/grub-core/boot/i386/pc/startup_raw.S
b/grub-core/boot/i386/pc/startup_raw.S
index 8bce7985c..28974821e 100644
--- a/grub-core/boot/i386/pc/startup_raw.S
+++ b/grub-core/boot/i386/pc/startup_raw.S
@@ -118,7 +118,16 @@ LOCAL (codestart):

 #include "../../../kern/i386/realmode.S"

+/*
+ *
+ * This is a workaround for clang adding a section containing only .addrsig
+ * Since clang itself is unable to assemble this pseudo-opcode, just replace
+ * it with .text
+ *
+ */
+#define addrsig text
 #include 
+#undef addrsig

  .text

-- 
2.11.0


-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Pair -Qn with -Qunused-arguments.

[Vladimir 'phcoder' Serbinenko]

>From fb3c8789bba1ef744acf950b0a36345d9d04e2e6 Mon Sep 17 00:00:00 2001
From: Vladimir Serbinenko 
Date: Mon, 25 Mar 2019 17:43:18 +0100
Subject: [PATCH] Pair -Qn with -Qunused-arguments.

When assembling module wirh clang -Qn ends up on command line but later ignored
To avoid it breaking the compile, add -Qunused-arguments.

Signed-off-by: Vladimir Serbinenko 
---
 configure.ac | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 2f53a5683..8578d01b7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1228,12 +1228,12 @@ if test "x$target_cpu" = xarm; then
 fi

 AC_CACHE_CHECK([whether option -Qn works], grub_cv_target_cc_qn, [
-  CFLAGS="$TARGET_CFLAGS -Qn -Werror"
+  CFLAGS="$TARGET_CFLAGS -Qn -Qunused-arguments -Werror"
   AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
  [grub_cv_target_cc_qn=yes],
  [grub_cv_target_cc_qn=no])])
 if test "x$grub_cv_target_cc_qn" = xyes; then
-  TARGET_CFLAGS="$TARGET_CFLAGS -Qn"
+  TARGET_CFLAGS="$TARGET_CFLAGS -Qn -Qunused-arguments"
 fi

 #
-- 
2.11.0


-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Change fs functions to add fs_ prefix

[Vladimir 'phcoder' Serbinenko]

ar *path,
+  grub_err_t (*fs_dir) (grub_device_t device, const char *path,
   grub_fs_dir_hook_t hook, void *hook_data);

   /* Open a file named NAME and initialize FILE.  */
-  grub_err_t (*open) (struct grub_file *file, const char *name);
+  grub_err_t (*fs_open) (struct grub_file *file, const char *name);

   /* Read LEN bytes data from FILE into BUF.  */
-  grub_ssize_t (*read) (struct grub_file *file, char *buf, grub_size_t len);
+  grub_ssize_t (*fs_read) (struct grub_file *file, char *buf, grub_size_t len);

   /* Close the file FILE.  */
-  grub_err_t (*close) (struct grub_file *file);
+  grub_err_t (*fs_close) (struct grub_file *file);

   /* Return the label of the device DEVICE in LABEL.  The label is
  returned in a grub_malloc'ed buffer and should be freed by the
  caller.  */
-  grub_err_t (*label) (grub_device_t device, char **label);
+  grub_err_t (*fs_label) (grub_device_t device, char **label);

   /* Return the uuid of the device DEVICE in UUID.  The uuid is
  returned in a grub_malloc'ed buffer and should be freed by the
  caller.  */
-  grub_err_t (*uuid) (grub_device_t device, char **uuid);
+  grub_err_t (*fs_uuid) (grub_device_t device, char **uuid);

   /* Get writing time of filesystem. */
-  grub_err_t (*mtime) (grub_device_t device, grub_int32_t *timebuf);
+  grub_err_t (*fs_mtime) (grub_device_t device, grub_int32_t *timebuf);

 #ifdef GRUB_UTIL
   /* Determine sectors available for embedding.  */
-  grub_err_t (*embed) (grub_device_t device, unsigned int *nsectors,
+  grub_err_t (*fs_embed) (grub_device_t device, unsigned int *nsectors,
 unsigned int max_nsectors,
 grub_embed_type_t embed_type,
 grub_disk_addr_t **sectors);
diff --git a/util/grub-fstest.c b/util/grub-fstest.c
index 7074e8e41..f14e02d97 100644
--- a/util/grub-fstest.c
+++ b/util/grub-fstest.c
@@ -470,9 +470,9 @@ fstest (int n)
  fs = grub_fs_probe (dev);
  if (!fs)
grub_util_error ("%s", grub_errmsg);
- if (!fs->uuid)
+ if (!fs->fs_uuid)
grub_util_error ("%s", _("couldn't retrieve UUID"));
- if (fs->uuid (dev, &uuid))
+ if (fs->fs_uuid (dev, &uuid))
grub_util_error ("%s", grub_errmsg);
  if (!uuid)
grub_util_error ("%s", _("couldn't retrieve UUID"));
diff --git a/util/grub-install.c b/util/grub-install.c
index 2046c48c4..8a55ad4b8 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1378,8 +1378,8 @@ main (int argc, char *argv[])
  {
char *uuid = NULL;
/*  generic method (used on coreboot and ata mod).  */
-   if (!force_file_id && grub_fs->uuid && grub_fs->uuid (grub_dev,
- &uuid))
+   if (!force_file_id
+   && grub_fs->fs_uuid && grub_fs->fs_uuid (grub_dev, &uuid))
  {
grub_print_error ();
grub_errno = 0;
diff --git a/util/grub-probe.c b/util/grub-probe.c
index 2e44ec8f4..81d27eead 100644
--- a/util/grub-probe.c
+++ b/util/grub-probe.c
@@ -446,10 +446,10 @@ probe (const char *path, char **device_names, char delim)
   else if (print == PRINT_FS_UUID)
  {
char *uuid;
-   if (! fs->uuid)
+   if (! fs->fs_uuid)
  grub_util_error (_("%s does not support UUIDs"), fs->name);

-   if (fs->uuid (dev, &uuid) != GRUB_ERR_NONE)
+   if (fs->fs_uuid (dev, &uuid) != GRUB_ERR_NONE)
  grub_util_error ("%s", grub_errmsg);

printf ("%s", uuid);
@@ -458,11 +458,11 @@ probe (const char *path, char **device_names, char delim)
   else if (print == PRINT_FS_LABEL)
  {
char *label;
-   if (! fs->label)
+   if (! fs->fs_label)
  grub_util_error (_("filesystem `%s' does not support labels"),
   fs->name);

-   if (fs->label (dev, &label) != GRUB_ERR_NONE)
+   if (fs->fs_label (dev, &label) != GRUB_ERR_NONE)
  grub_util_error ("%s", grub_errmsg);

printf ("%s", label);
diff --git a/util/setup.c b/util/setup.c
index 92a5af9ba..6f88f3cc4 100644
--- a/util/setup.c
+++ b/util/setup.c
@@ -491,7 +491,7 @@ SETUP (const char *dir,
  goto unable_to_embed;
   }

-if (fs && !fs->embed)
+if (fs && !fs->fs_embed)
   {
  grub_util_warn (_("File system `%s' doesn't support embedding"),
  fs->name);
@@ -529,8 +529,8 @@ SETUP (const char *dir,
   err = ctx.dest_partmap->embed (dest_dev->disk, &nsec, maxsec,
   GRUB_EMBED_PCBIOS, §ors);
 else
-  err = fs->embed (dest_dev, &nsec, maxsec,
-GRUB_EMBED_PCBIOS, §ors);
+  err = fs->fs_embed (dest_dev, &nsec, maxsec,
+   GRUB_EMBED_PCBIOS, §ors);
 if (!err && nsec < core_sectors)
   {
  err = grub_error (GRUB_ERR_OUT_OF_RANGE,


-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Change fs functions to add fs_ prefix

[Vladimir 'phcoder' Serbinenko]

On Mon, Apr 8, 2019 at 8:07 PM Daniel Kiper  wrote:
>
> On Mon, Apr 08, 2019 at 03:28:48PM +1000, Vladimir 'phcoder' Serbinenko wrote:
> > commit 820906d7b1e68559bcf1b3e0cb56e46cded3e84b
> > Author: Vladimir Serbinenko 
> > Date:   Mon Apr 8 07:24:24 2019 +0200
> >
> > Change fs functions to add fs_ prefix
> >
> > This avoid conflict with gnulib
> >
> > Signed-off-by: Vladimir Serbinenko 
>
> Is it at least build tested for all supported platforms?
Yes, build-tested
> If yes you can add Reviewed-by: Daniel Kiper 
>
> Daniel



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

GRUB 2.04 RC01 released

[Vladimir 'phcoder' Serbinenko]

Hello all. GRUB maintainers team is proud to announce 2.04~rc1 that we
have just released.

We’re not aware of any of the release-critical bugs but we’re sure
that there are some so we need your help finding them.


We would like to thank to all the people who have contributed to the project.


The tarball is available at

http://alpha.gnu.org/gnu/grub/grub-2.04~rc1.tar.xz

and signature at

http://alpha.gnu.org/gnu/grub/grub-2.04~rc1.tar.xz.sig


Signed with following fingerprint:

E53D 497F 3FA4 2AD8 C9B4  D1E8 35A9 3B74 E82E 4209


It's also available as a signed tag grub-2.04-rc1 in official git

repository.


If you don't have xz support alternatively you may consider files

http://alpha.gnu.org/gnu/grub/grub-2.04~rc1.tar.gz

and signature at

http://alpha.gnu.org/gnu/grub/grub-2.04~rc1.tar.gz.sig


If you want a binary version for windows (i386-pc, i386-efi and
x86_64-efi flavours) it’s available under
http://alpha.gnu.org/gnu/grub/grub-2.04~rc1-for-windows.zip

and signature at

http://alpha.gnu.org/gnu/grub/grub-2.04~rc1-for-windows.zip.sig


Thank you again to all the contributors

GRUB maintenance team



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 0/8] fix gcc9 build with -Werror=address-of-packed-member

[Vladimir 'phcoder' Serbinenko]

On Tue, Apr 9, 2019 at 8:48 PM Michael Chang  wrote:
>
> This patch set attempts to resolve the build failure in openSUSE build
> service equipped with new gcc 9 compiler, which has added a new warning
> flag -Waddress-of-packed-member.
>
> The new warning performs the check for taking the address of packed
> member directly to a pointer variable with higher alignment requirement
> and the outcome is risky to memory alignment fault on some architecture
> when deferencing it.
The problem here is that there is no way to tell the compiler to
create a structure without padding but a pointer to which is aligned.
This is very common in GRUB. Consider a struct like following:
grub_uint16_t a;
grub_uint32_t b;
grub_uint16_t c[40];

This needs packed attribute as otherwise compiler will insert a
padding before b. Yet c is guaranteed to be aligned if the pointer to
the structure is aligned.
>
> Please help to review.
>
> Thanks.
>
> Michael Chang (8):
>   cpio: fix gcc9 error address-of-packed-member
>   jfs: fix gcc9 error address-of-packed-member
>   hfs: fix gcc9 error address-of-packed-member
>   hfsplus: fix gcc9 error address-of-packed-member
>   acpi: fix gcc9 error address-of-packed-member
>   usbtest: fix gcc9 error address-of-packed-member
>   chainloader: fix gcc9 error address-of-packed-member
>   efi: fix gcc9 error address-of-packed-member
>
>  grub-core/commands/usbtest.c   | 13 -
>  grub-core/fs/cpio.c|  5 ++--
>  grub-core/fs/cpio_be.c |  5 ++--
>  grub-core/fs/hfsplus.c | 57 
> ++
>  grub-core/fs/jfs.c |  5 ++--
>  grub-core/kern/efi/efi.c   | 27 --
>  grub-core/loader/efi/chainloader.c | 12 ++--
>  include/grub/acpi.h|  2 +-
>  include/grub/hfs.h |  2 +-
>  9 files changed, 96 insertions(+), 32 deletions(-)
>
> --
> 2.16.4
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 0/8] fix gcc9 build with -Werror=address-of-packed-member

[Vladimir 'phcoder' Serbinenko]

On Tue, Apr 9, 2019 at 9:40 PM Michael Chang  wrote:
>
> On Tue, Apr 09, 2019 at 08:57:25PM +1000, Vladimir 'phcoder' Serbinenko wrote:
> > On Tue, Apr 9, 2019 at 8:48 PM Michael Chang  wrote:
> > >
> > > This patch set attempts to resolve the build failure in openSUSE build
> > > service equipped with new gcc 9 compiler, which has added a new warning
> > > flag -Waddress-of-packed-member.
> > >
> > > The new warning performs the check for taking the address of packed
> > > member directly to a pointer variable with higher alignment requirement
> > > and the outcome is risky to memory alignment fault on some architecture
> > > when deferencing it.
> > The problem here is that there is no way to tell the compiler to
> > create a structure without padding but a pointer to which is aligned.
> > This is very common in GRUB. Consider a struct like following:
> > grub_uint16_t a;
> > grub_uint32_t b;
> > grub_uint16_t c[40];
> >
> > This needs packed attribute as otherwise compiler will insert a
> > padding before b. Yet c is guaranteed to be aligned if the pointer to
> > the structure is aligned.
>
> But sometimes it is not easy to know as the packed structure could be
> embedded to another and also it could map to a specific portion in
> memory read from a file, with no guarantee where will be started (for eg
> via searching occurrence of specific magic number ..)
Yes. It's not easy. But that's why just following warnings isn't
always the good way to go [without going into specific details for
particular patches].
>
> Thanks,
> Michael
>
> > >
> > > Please help to review.
> > >
> > > Thanks.
> > >
> > > Michael Chang (8):
> > >   cpio: fix gcc9 error address-of-packed-member
> > >   jfs: fix gcc9 error address-of-packed-member
> > >   hfs: fix gcc9 error address-of-packed-member
> > >   hfsplus: fix gcc9 error address-of-packed-member
> > >   acpi: fix gcc9 error address-of-packed-member
> > >   usbtest: fix gcc9 error address-of-packed-member
> > >   chainloader: fix gcc9 error address-of-packed-member
> > >   efi: fix gcc9 error address-of-packed-member
> > >
> > >  grub-core/commands/usbtest.c   | 13 -
> > >  grub-core/fs/cpio.c|  5 ++--
> > >  grub-core/fs/cpio_be.c |  5 ++--
> > >  grub-core/fs/hfsplus.c | 57 
> > > ++
> > >  grub-core/fs/jfs.c |  5 ++--
> > >  grub-core/kern/efi/efi.c   | 27 --
> > >  grub-core/loader/efi/chainloader.c | 12 ++++++--
> > >  include/grub/acpi.h|  2 +-
> > >  include/grub/hfs.h |  2 +-
> > >  9 files changed, 96 insertions(+), 32 deletions(-)
> > >
> > > --
> > > 2.16.4
> > >
> > >
> > > ___
> > > Grub-devel mailing list
> > > Grub-devel@gnu.org
> > > https://lists.gnu.org/mailman/listinfo/grub-devel
> >
> >
> >
> > --
> > 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



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] x86/msr: Fix build with older GCC versions

[Vladimir 'phcoder' Serbinenko]

LGTM.
Just a note: it's a correct usage of extern, just not consistent with
rest of codebase and not supported by old gcc

On Sat, Apr 13, 2019 at 12:02 AM Daniel Kiper  wrote:
>
> Some older GCC versions produce following error when x86 MSR modules are 
> build:
>
>   In file included from commands/i386/rdmsr.c:29:0:
>   ../include/grub/i386/rdmsr.h:27:29: error: no previous prototype for 
> ‘grub_msr_read’ [-Werror=missing-prototypes]
>extern inline grub_uint64_t grub_msr_read (grub_uint32_t msr_id)
>^
>   cc1: all warnings being treated as errors
>
> This is due to incorrect use of extern keyword. So, replace it with
> static keyword.
>
> Additionally, fix incorrect coding style.
>
> Reported-by: Eric Snowberg 
> Reported-by: adrian15 
> Signed-off-by: Daniel Kiper 
> ---
>  include/grub/i386/rdmsr.h | 3 ++-
>  include/grub/i386/wrmsr.h | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/grub/i386/rdmsr.h b/include/grub/i386/rdmsr.h
> index bddb7..c0a0c717a 100644
> --- a/include/grub/i386/rdmsr.h
> +++ b/include/grub/i386/rdmsr.h
> @@ -24,7 +24,8 @@
>   *   Accessing a reserved or unimplemented MSR address results in a GP#.
>   */
>
> -extern inline grub_uint64_t grub_msr_read (grub_uint32_t msr_id)
> +static inline grub_uint64_t
> +grub_msr_read (grub_uint32_t msr_id)
>  {
>grub_uint32_t low, high;
>
> diff --git a/include/grub/i386/wrmsr.h b/include/grub/i386/wrmsr.h
> index e14d7807f..dea60aed1 100644
> --- a/include/grub/i386/wrmsr.h
> +++ b/include/grub/i386/wrmsr.h
> @@ -24,7 +24,8 @@
>   *   Accessing a reserved or unimplemented MSR address results in a GP#.
>   */
>
> -extern inline void grub_msr_write(grub_uint32_t msr_id, grub_uint64_t 
> msr_value)
> +static inline void
> +grub_msr_write(grub_uint32_t msr_id, grub_uint64_t msr_value)
>  {
>grub_uint32_t low = msr_value, high = msr_value >> 32;
>
> --
> 2.11.0
>


-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: GRUB fails to load kernel on linux-sparc64

[Vladimir 'phcoder' Serbinenko]

Try
insmod xzio
insmod gzio
insmod lzopio

On Wed, 17 Apr 2019, 07:39 John Paul Adrian Glaubitz, <
glaub...@physik.fu-berlin.de> wrote:

> Hi Eric!
>
> On 4/16/19 9:27 PM, Eric Snowberg wrote:
> >> @Eric: Does GRUB 2.04~rc1 work fine for you on sparc64?
> >
> > I boot with an uncompressed kernel and do not see a problem. If I try
> with a compressed kernel I do see the error you have above.  But this is a
> known platform limitation:
> https://www.gnu.org/software/grub/manual/grub/grub.html#Platform-limitations
>
> That would indeed explain the problem. However, Debian's GRUB version
> does support compressed kernels on sparc64. I wonder whether we have
> a local modification that makes this possible.
>
> Adrian
>
> --
>  .''`.  John Paul Adrian Glaubitz
> : :' :  Debian Developer - glaub...@debian.org
> `. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
>   `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
>
> ___
> 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

Re: grub-mkrescue: Problem with MBR partition table at start of EFI partition

[Vladimir 'phcoder' Serbinenko]

On Mon, 22 Apr 2019, 01:42 Thomas Schmitt,  wrote:

> Hi,
>
> this is about how exactly to solve a diagnosed and worked-around problem.
>
> Guix is one of the few distros which make their installation ISOs by
> help of grub-mkrescue. The EFI boot manager of an old Macbook got stuck
> when such an ISO was presented on USB stick. I.e. it not only did not
> boot the ISO but showed no other EFI partitions either.
>
> A Debian Live 9 ISO does not impose such a problem.
> The same Guix ISO on DVD rather than USB stick works well, too.
>
> The owner of the machine is Florian Pelz (Cc'ed). He characterizes it as:
>   MacBook Pro (13-inch, Mid 2010)
> MacOS Yosemite reports:
>   Model Name:MacBook Pro
>   Model Identifier:  MacBookPro7,1
>   Boot ROM Version:  MBP71.003F.B00
>   SMC Version (system):  1.62f7
>
> During a longish bug hunt it turned out that the decisive difference
> is the first block of the FAT filesystem image which Debian creates by
> mkfs.fat whereas grub-mkrescue creates it by mformat.
>
> The mformat-made image has an MBR partition table entry which claims the
> whole image as partition 1:
>
>   DeviceBoot Start   End Sectors  Size Id Type
>   /mnt/iso/efi.img1 *0  28792880  1.4M  1 FAT12
>
> If this partition entry is zeroed, then the EFI boot manager works even
> when the USB stick with this modified ISO is plugged in.
>
> Actually it suffices to change the start LBA from 0 to 1, so that the
> partition does not start by the partition table. (I count this as proof
> of an endless loop in EFI while it is exploring something like
> extended partitions. Strange is that the main MBR partition 1 of the
> Debian ISO points to that main MBR, too. So it seems to harm only in
> sub partition tables. We repacked the Guix ISO to MBR rather than GPT.
> Still stuck.)
>
> The EFI FAT image in Debian Live 9 has no partition table entry in block 0.
>
> Production of the partition entry can be suppressed by mformat option -k.
> But then block 0 of the result is not marked as MBR and also causes
> program "file" to give out a very sparse characterization:
>
>   DOS floppy 2880k
>
> Here is the "file" report about an image made without -k:
>
>   DOS/MBR boot sector, code offset 0x3c+2, OEM-ID "MTOO4018",
>   sectors/cluster 2, root entries 240, sectors 5760 (volumes <=32 MB) ,
>   sectors/FAT 9, sectors/track 36,
>   serial number 0x6c528b1f, unlabeled, FAT (12 bit), followed by FAT
>
> An image made by a modified grub-mkrescue with mformat option -k
> worked well for Florian Pelz.
>
> ---
>
> Questions:
>
> 1: Is there any use for the partition entry in the EFI partition of a
>grub-mkrescue ISO ?
>
Nope. An artefact of mformat that we don't really want. I just didn't know
about -k (or maybe it didn't exist back then)

>
> 2: Is there any use for the information which mformat does not insert
>if option -k is set ?
>
Probably no

>
> If the partition entry is not needed but the other MBR-like bytes are
> needed:
>
> 3: Is there a good example in GRUB util sources about how to open a file,
>seek to its byte 446, and write 16 zero bytes starting there ?
>(I'd like to propose a patch for grub-mkrescue.c which zeros the
> partition entry directly after the mformat run.)
>
I don't think so.

>
> ---
>
>
> Have a nice day :)
>
> Thomas
>
>
> ___
> 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

Re: [PATCH] arm: Move trampolines into code section

[Vladimir 'phcoder' Serbinenko]

LGTM

On Tue, 30 Apr 2019, 10:12 Alexander Graf,  wrote:

> When creating T32->A32 transition jumps, the relocation code in grub
> will generate trampolines. These trampolines live in the .data section
> of our PE binary which means they are not marked as executable.
>
> This misbehavior was unmasked by commit a51f953f4ee87 ("mkimage: Align
> efi sections on 4k boundary") which made the X/NX boundary more obvious
> because everything became page aligned.
>
> To put things into proper order, let's move the arm trampolines into the
> .text section instead. That way everyone knows they are executable.
>
> Fixes: a51f953f4ee87 ("mkimage: Align efi sections on 4k boundary")
> Reported-by: Julien ROBIN 
> Reported-by: Leif Lindholm 
>
> Signed-off-by: Alexander Graf 
> ---
>  util/grub-mkimagexx.c | 29 +
>  1 file changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
> index 2059890c3..af23fae52 100644
> --- a/util/grub-mkimagexx.c
> +++ b/util/grub-mkimagexx.c
> @@ -2197,25 +2197,10 @@ SUFFIX (locate_sections) (Elf_Ehdr *e, const char
> *kernel_path,
>   }
>}
>
> -  layout->kernel_size = ALIGN_UP (layout->kernel_size +
> image_target->vaddr_offset,
> - image_target->section_align)
> -- image_target->vaddr_offset;
> -  layout->exec_size = layout->kernel_size;
> -
> -  /* .data */
> -  for (i = 0, s = smd->sections;
> -   i < smd->num_sections;
> -   i++, s = (Elf_Shdr *) ((char *) s + smd->section_entsize))
> -if (SUFFIX (is_data_section) (s, image_target))
> -  layout->kernel_size = SUFFIX (put_section) (s, i,
> layout->kernel_size, smd,
> - image_target);
> -
>  #ifdef MKIMAGE_ELF32
>if (image_target->elf_target == EM_ARM)
>  {
>grub_size_t tramp;
> -  layout->kernel_size = ALIGN_UP (layout->kernel_size +
> image_target->vaddr_offset,
> - image_target->section_align) -
> image_target->vaddr_offset;
>
>layout->kernel_size = ALIGN_UP (layout->kernel_size, 16);
>
> @@ -2223,10 +2208,22 @@ SUFFIX (locate_sections) (Elf_Ehdr *e, const char
> *kernel_path,
>smd->num_sections, image_target);
>
>layout->tramp_off = layout->kernel_size;
> -  layout->kernel_size += ALIGN_UP (tramp, 16);
>  }
>  #endif
>
> +  layout->kernel_size = ALIGN_UP (layout->kernel_size +
> image_target->vaddr_offset,
> + image_target->section_align)
> +- image_target->vaddr_offset;
> +  layout->exec_size = layout->kernel_size;
> +
> +  /* .data */
> +  for (i = 0, s = smd->sections;
> +   i < smd->num_sections;
> +   i++, s = (Elf_Shdr *) ((char *) s + smd->section_entsize))
> +if (SUFFIX (is_data_section) (s, image_target))
> +  layout->kernel_size = SUFFIX (put_section) (s, i,
> layout->kernel_size, smd,
> + image_target);
> +
>layout->bss_start = layout->kernel_size;
>layout->end = layout->kernel_size;
>
> --
> 2.16.4
>
>
> ___
> 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

Re: [PATCH] arm: Move trampolines into code section

[Vladimir 'phcoder' Serbinenko]

On Tue, 30 Apr 2019, 22:15 Leif Lindholm,  wrote:

> On Tue, Apr 30, 2019 at 02:12:17AM +0200, Alexander Graf wrote:
> > When creating T32->A32 transition jumps, the relocation code in grub
> > will generate trampolines. These trampolines live in the .data section
> > of our PE binary which means they are not marked as executable.
>
> Which was always a bug.
>
> > This misbehavior was unmasked by commit a51f953f4ee87 ("mkimage: Align
> > efi sections on 4k boundary") which made the X/NX boundary more obvious
> > because everything became page aligned.
> >
> > To put things into proper order, let's move the arm trampolines into the
> > .text section instead. That way everyone knows they are executable.
> >
> > Fixes: a51f953f4ee87 ("mkimage: Align efi sections on 4k boundary")
> > Reported-by: Julien ROBIN 
> > Reported-by: Leif Lindholm 
> >
> > Signed-off-by: Alexander Graf 
>
> Right, so with a brain that's actually awake:
>
> > ---
> >  util/grub-mkimagexx.c | 29 +
> >  1 file changed, 13 insertions(+), 16 deletions(-)
> >
> > diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
> > index 2059890c3..af23fae52 100644
> > --- a/util/grub-mkimagexx.c
> > +++ b/util/grub-mkimagexx.c
> > @@ -2197,25 +2197,10 @@ SUFFIX (locate_sections) (Elf_Ehdr *e, const
> char *kernel_path,
> > }
> >}
> >
> > -  layout->kernel_size = ALIGN_UP (layout->kernel_size +
> image_target->vaddr_offset,
> > -   image_target->section_align)
> > -- image_target->vaddr_offset;
> > -  layout->exec_size = layout->kernel_size;
> > -
> > -  /* .data */
> > -  for (i = 0, s = smd->sections;
> > -   i < smd->num_sections;
> > -   i++, s = (Elf_Shdr *) ((char *) s + smd->section_entsize))
> > -if (SUFFIX (is_data_section) (s, image_target))
> > -  layout->kernel_size = SUFFIX (put_section) (s, i,
> layout->kernel_size, smd,
> > -   image_target);
> > -
>
> This patch only moves the below ifdef/conditional before the above
> stanza, which remains unchanged. So this does not affect !armhf at
> all. The generated diff is less than helpful here.
>
> >  #ifdef MKIMAGE_ELF32
> >if (image_target->elf_target == EM_ARM)
> >  {
> >grub_size_t tramp;
> > -  layout->kernel_size = ALIGN_UP (layout->kernel_size +
> image_target->vaddr_offset,
> > -   image_target->section_align) -
> image_target->vaddr_offset;
>
> *boggle*, so we were double adjusting these on arm? That explains why
> things were confused/confusing.
>
> >
> >layout->kernel_size = ALIGN_UP (layout->kernel_size, 16);
> >
> > @@ -2223,10 +2208,22 @@ SUFFIX (locate_sections) (Elf_Ehdr *e, const
> char *kernel_path,
>
> However, the line just left out of context here
> # tramp = arm_get_trampoline_size (e, smd->sections, smd->section_entsize,
>
> >  smd->num_sections, image_target);
>
> now looks a bit weird. We set "tramp" but never use it.
>
> >
> >layout->tramp_off = layout->kernel_size;
> > -  layout->kernel_size += ALIGN_UP (tramp, 16);
>
> Because we delete this adjustment.
> Why is that no longer needed?
>
Because this point is no longer a section boundary.

>
> /
> Leif
>
> >  }
> >  #endif
> >
> > +  layout->kernel_size = ALIGN_UP (layout->kernel_size +
> image_target->vaddr_offset,
> > +   image_target->section_align)
> > +- image_target->vaddr_offset;
> > +  layout->exec_size = layout->kernel_size;
> > +
> > +  /* .data */
> > +  for (i = 0, s = smd->sections;
> > +   i < smd->num_sections;
> > +   i++, s = (Elf_Shdr *) ((char *) s + smd->section_entsize))
> > +if (SUFFIX (is_data_section) (s, image_target))
> > +  layout->kernel_size = SUFFIX (put_section) (s, i,
> layout->kernel_size, smd,
> > +   image_target);
> > +
> >layout->bss_start = layout->kernel_size;
> >layout->end = layout->kernel_size;
> >
> > --
> > 2.16.4
> >
>
> ___
> 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

Re: grub-mkrescue: Problem with MBR partition table at start of EFI partition

[Vladimir 'phcoder' Serbinenko]

There is an easy way to check if -k can be used always: have a look at
windows install CD/DVD. If it doesn't have partion table then we can drop
it as well.

On Thu, 25 Apr 2019, 18:16 Thomas Schmitt,  wrote:

> Hi,
>
> Daniel Kiper wrote:
> > How do you create USB stick from ISO image?
>
> By plain copy to the base device, as usual for a grub-mkrescue ISO.
> Like
>
>   dd if=image of=usb_stick
>
>
> > Have you tried latest Guix image?
>
> Florian builds Guix ISOs locally. So i assume that it was newest Guix.
> Nevertheless, the problem is clearly not in Guix content but in the
> Start LBA 0 of the partition table entry inside the EFI partition image.
>
>
> > Numbers here and in another email suggest that this is a floppy image.
> > And floppy images do not have MBR. They have something called boot
> > sector which is a bit different thing.
>
> Yes. It already has been pointed out to me privately that it should
> rather be called a "VBR".
> But EFI specs talk of "MBR" in the System Partition. So i am in good
> company. As mathematician i always agree to "If it walks like a duck ...".
>
> UEFI 2.4, 12.3.1 "System Partition"
>
>   "A System Partition supports backward compatibility with legacy systems
>by reserving the first block (sector) of the partition for compatibility
>code. On legacy systems, the first block (sector) of a partition is
>loaded into memory and execution is transferred to this code.
>EFI firmware does not execute the code in the MBR. The EFI firmware
>contains knowledge about the partition structure of various devices,
>and can understand legacy MBR, GPT, and “El Torito.”"
>
> (The description of BIOS loading partition boot code is wrong. It is a
>  class of device MBRs which contain code to chainload partition boot code.
>  I guess, GRUB has such an MBR variant in its collection, too.)
>
> The last sentence might mean that EFI shall support extended MBR
> partitions.
> I doubt that it is an invitation to support a tree of partition tables with
> arbitrary tree width.
> I also doubt that the author had a mental relapse to the MBR of the device,
> which is mentioned in the paragraph before the one above.
>
>   "A System Partition is a partition in the conventional sense of a
>partition on a legacy system. For a hard disk, a partition is a
>contiguous grouping of sectors on the disk where the starting sector
>and size are defined by the Master Boot Record (MBR), which resides
>on LBA 0 (i.e., the first sector of the hard disk) (see Section 5.2),
>or the GUID Partition Table (GPT), which resides on logical block 1
>(the second sector of the hard disk) (see Section 5.3.1). For a
>diskette (floppy) drive, a partition is defined to be the entire media.
>A System Partition can reside on any media that is supported by EFI
>Boot Services."
>
>
> > I have a feeling that simply Macs
> > tested by you does not like floppy images. If you create regular HDD
> > images then probably everything will work.
>
> It was grub-mkrescue which created that floppy image by mformat
> and submits it to xorriso as EFI partition.
>
> My questions were whether the partition entry is necessary (Vladimir
> expects that it is not), and whether an MBR signature and the other
> fields in block 0 are needed, which mformat creates without option -k
> (Vladimir's answer was less firm).
>
> I am now proposing to Guix to use my script-in-the middle
>
>
> https://dev.lovelyhq.com/libburnia/libisoburn/raw/master/frontend/grub-mkrescue-sed.sh
>
> which is submitted to grub-mkrescue by option --xorriso=.
>
> Its main purpose is to achieve other partition layouts with mountable
> ISO partition. Meanwhile it learned how to zeroize the partition table
> in the EFI partition and, if desired additionally, to zeroize the MBR
> signature of the partition.
>
> Be invited to try with a BIOS and EFI enabled GRUB configuration:
>
>   wget
> https://dev.lovelyhq.com/libburnia/libisoburn/raw/master/frontend/grub-mkrescue-sed.sh
>   chmod u+x grub-mkrescue-sed.sh
>
>   export MKRESCUE_SED_MODE=mbr_hfs
>   export MKRESCUE_SED_IN_EFI_NO_PT=yes
>
>   grub-mkrescue --xorriso=./grub-mkrescue-sed.sh \
> -iso_mbr_part_type 0x83 -partition_offset 16 \
> ...your usual options to grub-mkrescue...
>
> (  export MKRESCUE_SED_DEBUG=yes
>  would let the script print input xorriso arguments and the arguments
>  which it actually hands over to xorriso.)
>
> The result has this partition layout:
>
>   $ /sbin/fdisk -l output.iso
>   ...
>   Disklabel type: dos
>   ...
>   Device  Boot Start   End Sectors  Size Id Type
>   output.iso1 *   64 28695   28632   14M 83 Linux
>   output.iso2  28696 344555760  2.8M ef EFI (FAT-12/16/32)
>
> Advantages in my eyes are:
>
> - No GPT backup which will be misplaced on USB stick after dd.
>   (Linux and partition editors complain.)
>
> - Mountable ISO partition with non-zero partition start for pleasing
>   partition e

Re: [PATCH v3] probe: Support probing for GPT partition UUID with -q

[Vladimir 'phcoder' Serbinenko]

On Fri, 10 May 2019, 19:05 Jacob Kroon,  wrote:

> Linux supports root=PARTUUID= boot argument, so add
> support for probing it. Compared to the fs UUID, the partition
> UUID does not change when reformatting a partition.
>
> Signed-off-by: Jacob Kroon 
> ---
>  grub-core/commands/probe.c | 35 +++
>  1 file changed, 35 insertions(+)
>
> Changes since v2:
>
>  * Add a proper commit message
>  * Handle endianess in the same way as is currently done in
>util/grub-probe.c:print_gpt_guid ()
>
> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
> index 95d272287..24742c181 100644
> --- a/grub-core/commands/probe.c
> +++ b/grub-core/commands/probe.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -45,6 +46,7 @@ static const struct grub_arg_option options[] =
>  {"fs", 'f', 0, N_("Determine filesystem type."), 0, 0},
>  {"fs-uuid",'u', 0, N_("Determine filesystem UUID."),
> 0, 0},
>  {"label",  'l', 0, N_("Determine filesystem label."), 0, 0},
> +{"partuuid",   'q', 0, N_("Determine GPT partition uuid."), 0, 0},
>  {0, 0, 0, 0, 0, 0}
>};
>
> @@ -98,6 +100,39 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc,
> char **args)
>grub_device_close (dev);
>return GRUB_ERR_NONE;
>  }
> +  if (state[6].set)
> +{
> +  /* ---- + null terminator */
> +  char val[37] = "none";
> +  if (dev->disk && dev->disk->partition &&
> + grub_strcmp(dev->disk->partition->partmap->name, "gpt") == 0)
> +   {
> + struct grub_gpt_partentry entry;
> + struct grub_partition *p = dev->disk->partition;
> + grub_disk_t disk = grub_disk_open(dev->disk->name);
> + if (!disk)
> +   return grub_errno;
> + if (grub_disk_read(disk, p->offset, p->index, sizeof(entry),
> &entry))
> +   return grub_errno;
> + grub_disk_close(disk);
> + grub_gpt_part_guid_t *guid = &entry.guid;
> + guid->data1 = grub_le_to_cpu32 (guid->data1);
> + guid->data2 = grub_le_to_cpu16 (guid->data2);
> + guid->data3 = grub_le_to_cpu16 (guid->data3);
>
Please don't modify this in place. This can easily create hard to track
endian-dependent bug. Instead you can transform them right before printing,
in the same statement.
Otherwise the patch is fine

> + grub_snprintf (val, sizeof(val),
> +
> "%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
> +guid->data1, guid->data2, guid->data3,
> guid->data4[0],
> +guid->data4[1], guid->data4[2], guid->data4[3],
> +guid->data4[4], guid->data4[5], guid->data4[6],
> +guid->data4[7]);
> +   }
> +  if (state[0].set)
> +   grub_env_set (state[0].arg, val);
> +  else
> +   grub_printf ("%s", val);
> +  grub_device_close (dev);
> +  return GRUB_ERR_NONE;
> +}
>fs = grub_fs_probe (dev);
>if (! fs)
>  return grub_errno;
> --
> 2.20.1
>
>
> ___
> 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

Re: [PATCH v2] Optimization calculation expression of FOR_MODULES(var)

[Vladimir 'phcoder' Serbinenko]

As already noticed this is actually on purpose to align pointer on addr_t
size.

On Thu, 11 Apr 2019, 20:04 Milo Wenxiang Niu,  wrote:

> From: Milo Wenxiang X Niu 
>
> It's just to remove the common factor: "sizeof (grub_addr_t)"  from the
> numerator and denominator of the fractional expression of next var.
> Let me explain it:
> Shortly:
> H: struct grub_module_header ;
> B: grub_uint32_t ;
> A: grub_addr_t;
>
> Thus, original expression can be expressed as:
> var = (H *)((B*)var) + ( offset_exp ))
> ++
>  (((H*)var)->size + sizeof(A) - 1)sizeof(A)  |
> offset_exp = - * --- |
>   sizeof(A)   sizeof(B)  |
> ++
> Remove the common factor "sizeof(A)" of fractional offset_exp, we got the
> result.
> offset_exp_new = ((H*)->size + sizeof(A) - 1) / sizeof(B)
>= ((struct grub_module_header *) var)->size +
> sizeof (grub_addr_t) -1) / sizeof (grub_uint32_t)
>
> so:
> var =(H *)(((B*)var) + ( (((H*)var)->size + sizeof(A) - 1) / sizeof(B) ))
> That's what I do.
>
> Still, the new offset express is meaningfull:
> *numerator: ((struct grub_module_header *) var)->size + sizeof
> (grub_addr_t) -1)
> it present the offset value united by byte.
> *denominator: sizeof (grub_uint32_t)
> it's means "struct grub_module_header" aligned by
> sizeof(grub_uint32_t)
> ---
>  include/grub/kernel.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/grub/kernel.h b/include/grub/kernel.h
> index 133a37c..b257224 100644
> --- a/include/grub/kernel.h
> +++ b/include/grub/kernel.h
> @@ -104,7 +104,7 @@ extern grub_addr_t EXPORT_VAR (grub_modbase);
>var && (grub_addr_t) var \
>  < (grub_modbase + (((struct grub_module_info *)
> grub_modbase)->size));\
>var = (struct grub_module_header *)  \
> -(((grub_uint32_t *) var) + struct grub_module_header *)
> var)->size + sizeof (grub_addr_t) - 1) / sizeof (grub_addr_t)) * (sizeof
> (grub_addr_t) / sizeof (grub_uint32_t
> +(((grub_uint32_t *) var) + struct grub_module_header *)
> var)->size + sizeof (grub_addr_t) - 1) / sizeof (grub_uint32_t
>
>  grub_addr_t grub_modules_get_end (void);
>
> --
> 1.8.3.1
>
>
> ___
> 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

Re: [PATCH v5] probe: Support probing for GPT partition UUID with -g

[Vladimir 'phcoder' Serbinenko]

Ideally the option shouldn't be specific to GPT as other partmaps also have
stable IDs. I think it's better not to have short option at all

On Mon, 13 May 2019, 20:33 Jacob Kroon,  wrote:

> Linux supports root=PARTUUID= boot argument, so add
> support for probing it. Compared to the fs UUID, the partition
> UUID does not change when reformatting a partition.
>
> Signed-off-by: Jacob Kroon 
> ---
>  docs/grub.texi |  2 +-
>  grub-core/commands/probe.c | 34 ++
>  2 files changed, 35 insertions(+), 1 deletion(-)
>
> Changes since v4:
>  * Do endianess byte-swapping on the fly when formatting string
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 308b25074..d85818744 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -4771,7 +4771,7 @@ a rest.
>  @node probe
>  @subsection probe
>
> -@deffn Command probe [@option{--set} var]
> @option{--driver}|@option{--partmap}|@option{--fs}|@option{--fs-uuid}|@option{--label}
> device
> +@deffn Command probe [@option{--set} var]
> @option{--driver}|@option{--partmap}|@option{--fs}|@option{--fs-uuid}|@option{--label}|@option{--part-uuid}
> device
>  Retrieve device information. If option @option{--set} is given, assign
> result
>  to variable @var{var}, otherwise print information on the screen.
>  @end deffn
> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
> index 95d272287..b26cbe2cc 100644
> --- a/grub-core/commands/probe.c
> +++ b/grub-core/commands/probe.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -45,6 +46,7 @@ static const struct grub_arg_option options[] =
>  {"fs", 'f', 0, N_("Determine filesystem type."), 0, 0},
>  {"fs-uuid",'u', 0, N_("Determine filesystem UUID."),
> 0, 0},
>  {"label",  'l', 0, N_("Determine filesystem label."), 0, 0},
> +{"part-uuid",  'g', 0, N_("Determine GPT partition UUID."), 0, 0},
>  {0, 0, 0, 0, 0, 0}
>};
>
> @@ -98,6 +100,38 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc,
> char **args)
>grub_device_close (dev);
>return GRUB_ERR_NONE;
>  }
> +  if (state[6].set)
> +{
> +  /* ---- + null terminator */
> +  char val[37] = "none";
> +  if (dev->disk && dev->disk->partition &&
> + grub_strcmp(dev->disk->partition->partmap->name, "gpt") == 0)
> +   {
> + struct grub_gpt_partentry entry;
> + struct grub_partition *p = dev->disk->partition;
> + grub_disk_t disk = grub_disk_open(dev->disk->name);
> + if (!disk)
> +   return grub_errno;
> + if (grub_disk_read(disk, p->offset, p->index, sizeof(entry),
> &entry))
> +   return grub_errno;
> + grub_disk_close(disk);
> + grub_gpt_part_guid_t *guid = &entry.guid;
> + grub_snprintf (val, sizeof(val),
> +
> "%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
> +grub_le_to_cpu32 (guid->data1),
> +grub_le_to_cpu16 (guid->data2),
> +grub_le_to_cpu16 (guid->data3),
> +guid->data4[0], guid->data4[1], guid->data4[2],
> +guid->data4[3], guid->data4[4], guid->data4[5],
> +guid->data4[6], guid->data4[7]);
> +   }
> +  if (state[0].set)
> +   grub_env_set (state[0].arg, val);
> +  else
> +   grub_printf ("%s", val);
> +  grub_device_close (dev);
> +  return GRUB_ERR_NONE;
> +}
>fs = grub_fs_probe (dev);
>if (! fs)
>  return grub_errno;
> --
> 2.20.1
>
>
> ___
> 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

[PATCH] configure: Add -fno-ident when available

[Vladimir 'phcoder' Serbinenko]

On mingw i386-pc without this option mingw generates a
rdata$zzz symbol that is page-aligned and hence lzma_decompress no
longer fits in its allocated space.
With mingw this also saves a bit of space in modules
On other platforms we should already strip this, so no effect
---
 configure.ac | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/configure.ac b/configure.ac
index 7656f2434..a86951b90 100644
--- a/configure.ac
+++ b/configure.ac
@@ -999,6 +999,29 @@ if test "x$grub_cv_cc_fno_unwind_tables" = xyes; then
   TARGET_CFLAGS="$TARGET_CFLAGS -fno-unwind-tables"
 fi

+# Do not generate .ident sections
+AC_CACHE_CHECK([whether -fno-ident works], [grub_cv_cc_fno_ident], [
+  CFLAGS="$TARGET_CFLAGS -fno-ident"
+  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
+  [grub_cv_cc_fno_ident=yes],
+  [grub_cv_cc_fno_ident=no])
+])
+
+if test "x$grub_cv_cc_fno_ident" = xyes; then
+  TARGET_CFLAGS="$TARGET_CFLAGS -fno-ident"
+fi
+
+AC_CACHE_CHECK([whether -fno-unwind-tables works],
[grub_cv_cc_fno_unwind_tables], [
+  CFLAGS="$TARGET_CFLAGS -fno-unwind-tables"
+  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
+  [grub_cv_cc_fno_unwind_tables=yes],
+  [grub_cv_cc_fno_unwind_tables=no])
+])
+
+if test "x$grub_cv_cc_fno_unwind_tables" = xyes; then
+  TARGET_CFLAGS="$TARGET_CFLAGS -fno-unwind-tables"
+fi
+

 CFLAGS="$TARGET_CFLAGS"

-- 
2.11.0


-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] configure: Add -fno-ident when available

[Vladimir 'phcoder' Serbinenko]

From 03707a618d13a67f947833a7752c97ce3dda55a6 Mon Sep 17 00:00:00 2001
From: Vladimir Serbinenko 
Date: Tue, 9 Jul 2019 17:46:25 +0200
Subject: [PATCH] configure: Add -fno-ident when available

With mingw for i386-pc without this option mingw generates a
rdata$zzz symbol that is page-aligned and hence lzma_decompress no
longer fits in its allocated space.
With mingw this also saves a bit of space in modules
With other compilers we already strip the relevant sections,
so no effect

Signed-off-by: Vladimir Serbinenko 
---
 configure.ac | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/configure.ac b/configure.ac
index 7656f2434..ce7760930 100644
--- a/configure.ac
+++ b/configure.ac
@@ -999,7 +999,18 @@ if test "x$grub_cv_cc_fno_unwind_tables" = xyes; then
   TARGET_CFLAGS="$TARGET_CFLAGS -fno-unwind-tables"
 fi

+# Do not generate .ident sections
+AC_CACHE_CHECK([whether -fno-ident works], [grub_cv_cc_fno_ident], [
+  CFLAGS="$TARGET_CFLAGS -fno-ident"
+  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
+  [grub_cv_cc_fno_ident=yes],
+  [grub_cv_cc_fno_ident=no])
+])

+if test "x$grub_cv_cc_fno_ident" = xyes; then
+  TARGET_CFLAGS="$TARGET_CFLAGS -fno-ident"
+fi
+
 CFLAGS="$TARGET_CFLAGS"


-- 
2.11.0

On Thu, Jul 11, 2019 at 12:32 PM Daniel Kiper  wrote:
>
> On Tue, Jul 09, 2019 at 05:52:58PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> > On mingw i386-pc without this option mingw generates a
> > rdata$zzz symbol that is page-aligned and hence lzma_decompress no
> > longer fits in its allocated space.
>
> Out of curiosity, why is the size of lzma_decompress limited to an
> value? I thought that the total size of core.img is limited not
> lzma_decompress module.
>
> > With mingw this also saves a bit of space in modules
> > On other platforms we should already strip this, so no effect
>
> May I ask you to polish a comment a bit. And please add your SOB.
>
> > ---
> >  configure.ac | 23 +++
> >  1 file changed, 23 insertions(+)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 7656f2434..a86951b90 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -999,6 +999,29 @@ if test "x$grub_cv_cc_fno_unwind_tables" = xyes; then
> >TARGET_CFLAGS="$TARGET_CFLAGS -fno-unwind-tables"
> >  fi
> >
> > +# Do not generate .ident sections
> > +AC_CACHE_CHECK([whether -fno-ident works], [grub_cv_cc_fno_ident], [
> > +  CFLAGS="$TARGET_CFLAGS -fno-ident"
> > +  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
> > +  [grub_cv_cc_fno_ident=yes],
> > +  [grub_cv_cc_fno_ident=no])
> > +])
> > +
> > +if test "x$grub_cv_cc_fno_ident" = xyes; then
> > +  TARGET_CFLAGS="$TARGET_CFLAGS -fno-ident"
> > +fi
>
> OK...
>
> > +
> > +AC_CACHE_CHECK([whether -fno-unwind-tables works],
> > [grub_cv_cc_fno_unwind_tables], [
> > +  CFLAGS="$TARGET_CFLAGS -fno-unwind-tables"
> > +  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
> > +  [grub_cv_cc_fno_unwind_tables=yes],
> > +  [grub_cv_cc_fno_unwind_tables=no])
> > +])
> > +
> > +if test "x$grub_cv_cc_fno_unwind_tables" = xyes; then
> > +  TARGET_CFLAGS="$TARGET_CFLAGS -fno-unwind-tables"
> > +fi
> > +
>
> ...but I am afraid that you copied this part from above by mistake.
> Could you drop that?
>
> Daniel



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] configure: Add -fno-ident when available

[Vladimir 'phcoder' Serbinenko]

On Thu, Jul 11, 2019 at 12:32 PM Daniel Kiper  wrote:
>
> On Tue, Jul 09, 2019 at 05:52:58PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> > On mingw i386-pc without this option mingw generates a
> > rdata$zzz symbol that is page-aligned and hence lzma_decompress no
> > longer fits in its allocated space.
>
> Out of curiosity, why is the size of lzma_decompress limited to an
> value? I thought that the total size of core.img is limited not
> lzma_decompress module.
>
It's not a module but a part of GRUB kernel, just compiled separately
because it's the only part of kernel that is not compressed. On
runtime it's loaded at 0x8200 and main GRUB kernel resides at 0x9000,
so decompressor can't be more that 0xe00. So far this was never a
problem except this case which turned out to be alignment issue and
it's good that we catched it since it wasted a lot of place in core
since this part is not compressed

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] configure: disable arm movw/movt relocations for GCC

[Vladimir 'phcoder' Serbinenko]

LGTM

On Tue, 4 Jun 2019, 13:40 Leif Lindholm,  wrote:

> When building for arm, we already disable movw/movt relocations for clang,
> since they are incompatible with PE.
>
> When building with bare metal GCC toolchains (like the one used in the
> travis ci scripts), we end up with these relocations again. So add an
> additional test for the '-mword-relocations' flag used by GCC.
>
> Reported-by: Alexander Graf 
> Signed-off-by: Leif Lindholm 
> ---
>
> Note: unless this is added before the travis-ci set, the arm ci build
> will fail when enabled.
>
> ---
>
>  configure.ac | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 08b518fcc..e7725a546 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1198,7 +1198,8 @@ if test "x$target_cpu" = xarm; then
>AC_CACHE_CHECK([for options to disable movt and movw],
> grub_cv_target_cc_mno_movt, [
>  grub_cv_target_cc_mno_movt=no
>  for cand in "-mno-movt" \
> -   "-mllvm -arm-use-movt=0"; do
> +   "-mllvm -arm-use-movt=0" \
> +   "-mword-relocations"; do
>if test x"$grub_cv_target_cc_mno_movt" != xno ; then
>  break
>fi
> --
> 2.11.0
>
>
> ___
> 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

Re: [PATCH] sparc64: Fix BIOS Boot Partition support

[Vladimir 'phcoder' Serbinenko]

LGTM. Let's give it another day on ML, then I'll commit it

On Mon, Jul 15, 2019 at 12:40 AM James Clarke  wrote:
>
> Currently, gpt_offset is uninitialised when using a BIOS Boot Partition
> but is used unconditionally inside save_blocklists. Instead, ensure it
> is always initialised to 0 (note that there is already separate code to
> do the equivalent adjustment after we call save_blocklists on this code
> path).
>
> This patch has been tested on a T5-2 LDOM.
>
> Signed-off-by: James Clarke 
> ---
>  util/setup.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/util/setup.c b/util/setup.c
> index 6f88f3cc4..3be88aae1 100644
> --- a/util/setup.c
> +++ b/util/setup.c
> @@ -270,6 +270,9 @@ SETUP (const char *dir,
>  #ifdef GRUB_SETUP_BIOS
>bl.current_segment =
>  GRUB_BOOT_I386_PC_KERNEL_SEG + (GRUB_DISK_SECTOR_SIZE >> 4);
> +#endif
> +#ifdef GRUB_SETUP_SPARC64
> +  bl.gpt_offset = 0;
>  #endif
>bl.last_length = 0;
>
> @@ -730,7 +733,6 @@ unable_to_embed:
>  #ifdef GRUB_SETUP_SPARC64
>{
>  grub_partition_t container = root_dev->disk->partition;
> -bl.gpt_offset = 0;
>
>  if (grub_strstr (container->partmap->name, "gpt"))
>bl.gpt_offset = grub_partition_get_start (container);
> --
> 2.20.1
>
>
> ___________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] grub-core/commands/basic_math.c: New commands to perform basic math operations on variables

[Vladimir 'phcoder' Serbinenko]

We should rather have $[...] syntax in parser rather than ad-hoc math
commands

On Tue, 16 Jul 2019, 19:15 glenn tanner,  wrote:

> Added new commands to add and subtract values from grub variables, was
> desirable for automated testing. Compiled and tested under x86_64-efi.
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 474a63e68..c383f0157 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -2503,3 +2503,8 @@ module = {
>   common = commands/i386/wrmsr.c;
>   enable = x86;
> };
> +
> +module = {
> +  name = basic_math;
> +  common = commands/basic_math.c;
> +};
> diff --git a/grub-core/commands/basic_math.c
> b/grub-core/commands/basic_math.c
> new file mode 100644
> index 0..9337cd5b1
> --- /dev/null
> +++ b/grub-core/commands/basic_math.c
> @@ -0,0 +1,134 @@
> +/* basic_math.c - basic math functions.  */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2008,2009,2010  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see .
> + */
> +
> +/* what any module needs */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +/* what my module needs */
> +#include 
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +static grub_err_t
> +grub_cmd_add (grub_extcmd_context_t ctxt __attribute__ ((unused)), int
> argc, char **args)
> +{
> +  unsigned int i;
> +  int ret, value_int=0, arg_int=0, digit;
> +  const char *value;
> +  char buf[512];
> +
> +  if (argc != 2)
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Variable and value
> required"));
> +
> +  value = grub_env_get (args[0]);
> +  if (!value)
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Variable must be
> defined"));
> +
> +  for (i=0;i +if (grub_isdigit((int)value[i])) {
> +  digit = value[i] - '0';
> +  value_int = (10*value_int) + digit;
> +}
> +  }
> +
> +  for (i=0;i +if (grub_isdigit((int)args[1][i])) {
> +  digit = args[1][i] - '0';
> +  arg_int = (10*arg_int) + digit;
> +} else {
> +  return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Value must be a
> positive interger"));
> +}
> +  }
> +
> +  ret=value_int+arg_int;
> +  /* grub_printf ("%d\n", ret); */
> +
> +  grub_snprintf (buf, sizeof (buf), "%d", ret);
> +  /* grub_printf ("%s\n", buf); */
> +  grub_env_set (args[0], buf);
> +  return 0;
> +}
> +
> +static grub_err_t
> +grub_cmd_sub (grub_extcmd_context_t ctxt __attribute__ ((unused)), int
> argc, char **args)
> +{
> +  unsigned int i;
> +  int ret, value_int=0, arg_int=0, digit;
> +  const char *value;
> +  char buf[512];
> +
> +  if (argc != 2)
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Variable and value
> required"));
> +
> +  value = grub_env_get (args[0]);
> +  if (!value)
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Variable must be
> defined"));
> +
> +  for (i=0;i +if (grub_isdigit((int)value[i])) {
> +  digit = value[i] - '0';
> +  value_int = (10*value_int) + digit;
> +}
> +  }
> +
> +  for (i=0;i +if (grub_isdigit((int)args[1][i])) {
> +  digit = args[1][i] - '0';
> +  arg_int = (10*arg_int) + digit;
> +} else {
> +  return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Value must be a
> positive interger"));
> +}
> +  }
> +
> +  if (arg_int > value_int)
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Result may not be
> negitive"));
> +
> +  ret=value_int-arg_int;
> +  /* grub_printf ("%d\n", ret); */
> +
> +  grub_snprintf (buf, sizeof (buf), "%d", ret);
> +  /* grub_printf ("%s\n", buf); */
> +  grub_env_set (args[0], buf);
> +  return 0;
> +}
> +
> +static grub_extcmd_t cmd_add, cmd_sub;
> +
>
> +GRUB_MOD_INIT(basic_math)
> +{
> +  cmd_add =
> +grub_register_extcmd ("add", grub_cmd_add, 0,
> + N_(" "),
> + N_("Add value to variable."),
> + 0);
> +  cmd_sub =
> +grub_register_extcmd ("sub", grub_cmd_sub, 0,
> + N_(" "),
> + N_("Subtract value from variable."),
> + 0);
> +}
> +
> +GRUB_MOD_FINI(basic_math)
> +{
> +  grub_unregister_extcmd (cmd_add);
> +  grub_unregister_extcmd (cmd_sub);
> +}
>
> --
> Regards,
> Glenn Tanner
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.

Re: [PATCH] grub-core/commands/basic_math.c: New commands to perform basic math operations on variables

[Vladimir 'phcoder' Serbinenko]

On Tue, 16 Jul 2019, 19:15 glenn tanner,  wrote:

> Added new commands to add and subtract values from grub variables, was
> desirable for automated testing. Compiled and tested under x86_64-efi.
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 474a63e68..c383f0157 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -2503,3 +2503,8 @@ module = {
>   common = commands/i386/wrmsr.c;
>   enable = x86;
> };
> +
> +module = {
> +  name = basic_math;
> +  common = commands/basic_math.c;
> +};
> diff --git a/grub-core/commands/basic_math.c
> b/grub-core/commands/basic_math.c
> new file mode 100644
> index 0..9337cd5b1
> --- /dev/null
> +++ b/grub-core/commands/basic_math.c
> @@ -0,0 +1,134 @@
> +/* basic_math.c - basic math functions.  */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2008,2009,2010  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see .
> + */
> +
> +/* what any module needs */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +/* what my module needs */
> +#include 
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +static grub_err_t
> +grub_cmd_add (grub_extcmd_context_t ctxt __attribute__ ((unused)), int
> argc, char **args)
> +{
> +  unsigned int i;
> +  int ret, value_int=0, arg_int=0, digit;
> +  const char *value;
> +  char buf[512];
> +
> +  if (argc != 2)
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Variable and value
> required"));
> +
> +  value = grub_env_get (args[0]);
> +  if (!value)
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Variable must be
> defined"));
> +
> +  for (i=0;i +if (grub_isdigit((int)value[i])) {
> +  digit = value[i] - '0';
> +  value_int = (10*value_int) + digit;
>
This should be grub_strtol and not custom loop

> +}
> +  }
> +
> +  for (i=0;i +if (grub_isdigit((int)args[1][i])) {
> +  digit = args[1][i] - '0';
> +  arg_int = (10*arg_int) + digit;
> +} else {
> +  return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Value must be a
> positive interger"));
> +}
> +  }
> +
> +  ret=value_int+arg_int;
> +  /* grub_printf ("%d\n", ret); */
> +
> +  grub_snprintf (buf, sizeof (buf), "%d", ret);
> +  /* grub_printf ("%s\n", buf); */
> +  grub_env_set (args[0], buf);
> +  return 0;
> +}
> +
> +static grub_err_t
> +grub_cmd_sub (grub_extcmd_context_t ctxt __attribute__ ((unused)), int
> argc, char **args)
> +{
> +  unsigned int i;
> +  int ret, value_int=0, arg_int=0, digit;
> +  const char *value;
> +  char buf[512];
> +
> +  if (argc != 2)
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Variable and value
> required"));
> +
> +  value = grub_env_get (args[0]);
> +  if (!value)
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Variable must be
> defined"));
> +
> +  for (i=0;i +if (grub_isdigit((int)value[i])) {
> +  digit = value[i] - '0';
> +  value_int = (10*value_int) + digit;
> +}
> +  }
> +
> +  for (i=0;i +if (grub_isdigit((int)args[1][i])) {
> +  digit = args[1][i] - '0';
> +  arg_int = (10*arg_int) + digit;
> +} else {
> +  return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Value must be a
> positive interger"));
> +}
> +  }
> +
> +  if (arg_int > value_int)
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Result may not be
> negitive"));
> +
> +  ret=value_int-arg_int;
> +  /* grub_printf ("%d\n", ret); */
> +
> +  grub_snprintf (buf, sizeof (buf), "%d", ret);
> +  /* grub_printf ("%s\n", buf); */
> +  grub_env_set (args[0], buf);
> +  return 0;
> +}
> +
> +static grub_extcmd_t cmd_add, cmd_sub;
> +
>
> +GRUB_MOD_INIT(basic_math)
> +{
> +  cmd_add =
> +grub_register_extcmd ("add", grub_cmd_add, 0,
> + N_(" "),
> + N_("Add value to variable."),
> + 0);
> +  cmd_sub =
> +grub_register_extcmd ("sub", grub_cmd_sub, 0,
> + N_(" "),
> + N_("Subtract value from variable."),
> + 0);
> +}
> +
> +GRUB_MOD_FINI(basic_math)
> +{
> +  grub_unregister_extcmd (cmd_add);
> +  grub_unregister_extcmd (cmd_sub);
> +}
>
> --
> Regards,
> Glenn Tanner
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-deve

Re: [PATCH] sparc64: Fix BIOS Boot Partition support

[Vladimir 'phcoder' Serbinenko]

Committed

On Thu, Jul 18, 2019 at 12:03 AM John Paul Adrian Glaubitz
 wrote:
>
> On 7/16/19 2:02 PM, Vladimir 'phcoder' Serbinenko wrote:
> > LGTM. Let's give it another day on ML, then I'll commit it
> Friendly reminder ;-).
>
> Adrian
>
> --
>  .''`.  John Paul Adrian Glaubitz
> : :' :  Debian Developer - glaub...@debian.org
> `. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
>   `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: GRUB bug with UEFI mode on HP 250 G6 Notebook

[Vladimir 'phcoder' Serbinenko]

We don't manage grub-installer. It's Ubuntu-specific wrapper

On Mon, Jul 15, 2019 at 10:25 PM Artyom Pozharov
 wrote:
>
> Hello, GRUB2 Developers. I noticed the critical error with GRUB-installer 
> package. Please, take attention to it. I can't install GNU/Linux 
> distributions in UEFI, because this error appears. There are no problems with 
> BIOS-compability mode. Thanks in advance! 
> https://bugs.launchpad.net/ubuntu/+source/grub-installer/+bug/1835664
> Yours sincerely, Artyom!
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: APFS support

[Vladimir 'phcoder' Serbinenko]

Same here. I'm curious as to how it differs from ZFS. It looks a bit
like ZFS rip-off and I know that Apple has experimented with ZFS in
the past

On Tue, Jun 18, 2019 at 2:57 PM Daniel Kiper  wrote:
>
> On Mon, Jun 10, 2019 at 12:12:41PM +0200, Steffen Sobiech wrote:
> > is there anything planned to add support for Apple's APFS file system to 
> > grub2?
>
> I have not heard about any such plans. However, if somebody sends patches
> then I will be happy to review them.
>
> Daniel
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 00/10] Travis fixes

[Vladimir 'phcoder' Serbinenko]

LGTM for the whole series. I do care about mips and ia64 but I think
it's fine to disable them for now and iterate from there

On Tue, Jun 4, 2019 at 12:30 PM Alexander Graf  wrote:
>
> This patch set collects a few fixes for Travis CI since the initial
> commit was applied:
>
>   - catch up with the tree
>   - fix targets that need a board specified
>   - make mkimage loop more robust
>   - add QEMU tests for ARM and AArch64 EFI targets
>   - add ARM Thumb mode tests
>   - verify that on demand module and config loading from TFTP works
>   - disable MIPS/IA64 tests
>   - add arm_thumb test
>
> That way we should hopefully catch even more problems going forward.
>
> v1 -> v2:
>
>   - add ARM Thumb mode tests
>   - verify that on demand module and config loading from TFTP works
>   - use local gnulib (saves ~3m git checkout time)
>
> v2 -> v3:
>
>   - add arm_thumb target
>   - disable MIPS/IA64
>   - Use 19.03 release firmware files
>   - add comments to why we still use the system gnulib
>
> Alexander Graf (10):
>   travis: Run bootstrap instead of autogen.sh
>   travis: Fix sparc64 test
>   travis: Fix mips QEMU target
>   travis: Fix arm coreboot test and make loop more robus
>   arm coreboot: Use common directory path
>   travis: Add smoke tests for arm and aarch64
>   travis: Add ARM thumb target to tests
>   travis: Test module loading from tftp as well
>   travis: Disable MIPS target
>   travis: Disable IA64 target
>
>  .travis.yml| 67 
> +-
>  util/mkimage.c |  4 ++--
>  2 files changed, 54 insertions(+), 17 deletions(-)
>
> --
> 2.16.4
>
>
> ___________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Grub2.02 installed with Pythin 3, boots Xen Dom0 but not Xen DomU-s

[Vladimir 'phcoder' Serbinenko]

Pygrub is not part of grub project.

On Sun, 21 Jul 2019, 12:58 Kevin Buckley,  wrote:

> Hi there,
>
> this may well fall into a "Grub2 should not be run under Python3"
> basket, hence not submitting a bug at savannah, however the fact
> that my Grub appears to allow me to boot my Dom0 system, but not
> boot my Xen DomU-s, has me thinking there's more to it than that.
>
>
> OK, so I am trying to build an LFS (Linux From Scratch) system that
> will serve as a basic Xen Dom0.
>
> You can see the way things have been compiled, including my UEFI-aware
> Grub, here (Very much a WIP)
>
>   http://youvegotbuckleys.org.nz/LFS/LFS-BOOK.html
>
>
> Suffice it to say that the LFS Grub boots either of the two
> UEFI enties (non-Xen and Dom0) I have so as to bring up the
> system.
>
>
> I also have a couple of VBD-backed DomU-s that I can bring up,
> using the pygrub from either an Ubuntu 1404, or a Centos 6.10
> with Xen4Centos, Dom0.
>
> When I come to try and have my LFS Xen's pygrub boot the VBD-backed
> DomU, I get errors of the form
>
>
> # cat /mnt/var/log/xen/bootloader.4.log
> Using  to parse /boot/grub/grub.cfg
> Traceback (most recent call last):
>   File "/usr/lib/xen/bin/pygrub", line 928, in 
> raise RuntimeError("Unable to find partition containing kernel")
> RuntimeError: Unable to find partition containing kernel
>
> I originally thought that this might be down to my using a Xen
> source from the Xen Git repo that was in beyond 4.12 (which I
> needed so to get a Xen that was happy with just Python3. See
>
> https://lists.xenproject.org/archives/html/xen-devel/2019-04/msg00996.html
> ) and had prhaps had tripped over a regression, as that error used
> to be a problem with Xen (if you go back far enough).
>
>
> However I have since tracked down my problem to the pygrub, in
> that if I point pygrub to the start of the partition in the VBD
> that has the grub.cfg in it, I see the following failure to parse
> the config file:
>
>
> bash-5.0# /usr/lib/xen/bin/pygrub --debug --offset=1048576
> --list-entries /dev/vg_xen_vbds/lv_4g_02
> Using  to parse /boot/grub/grub.cfg
> Traceback (most recent call last):
>   File "/usr/lib/xen/bin/pygrub", line 907, in 
> chosencfg = run_grub(file, entry, fs, incfg["args"])
>   File "/usr/lib/xen/bin/pygrub", line 625, in run_grub
> g = Grub(file, fs)
>   File "/usr/lib/xen/bin/pygrub", line 249, in __init__
> self.read_config(file, fs)
>   File "/usr/lib/xen/bin/pygrub", line 460, in read_config
> self.cf.parse(buf)
>   File "/usr/lib/python3.7/site-packages/grub/GrubConf.py", line 376, in
> parse
> lines = buf.split("\n")
> TypeError: a bytes-like object is required, not 'str'
>
>
> whereas if I do the same with the pygrub from the CentOs6.10/Xen4Centos
> environment, I see
>
>
> /usr/bin/pygrub  --debug --offset=1048576 --list-entries
> /dev/vg_xen_vbds/lv_4g_02
> title: Ubuntu
>   root: None
>   kernel: /boot/vmlinuz-4.4.0-31-generic
>   args: root=/dev/xvda1 ro  quiet splash $vt_handoff
>   initrd: /boot/initrd.img-4.4.0-31-generic
> title: Ubuntu, with Linux 4.4.0-31-generic
>   root: None
>   kernel: /boot/vmlinuz-4.4.0-31-generic
>   args: root=UUID=5556a819-ced8-4864-9e7f-73792570703e ro  quiet
> splash $vt_handoff
>   initrd: /boot/initrd.img-4.4.0-31-generic
> title: Ubuntu, with Linux 4.4.0-31-generic (recovery mode)
>   root: None
>   kernel: /boot/vmlinuz-4.4.0-31-generic
>   args: root=UUID=5556a819-ced8-4864-9e7f-73792570703e ro recovery
> nomodeset
>   initrd: /boot/initrd.img-4.4.0-31-generic
> Using  to parse /boot/grub/grub.cfg
> INFO:root:Ignored directive if
> INFO:root:Ignored directive load_env
> INFO:root:Ignored directive fi
> INFO:root:Ignored directive if
> WARNING:root:grub2's saved_entry/next_entry not supported
> INFO:root:Ignored directive save_env
> ...
>
>
> and it goes on and suceeds.
>
> Ok, so then I went hunting around for the cause of the error
> I was seeing on the LFS system and got pointed towards the
> cause being a Pythin2->3 issue, with the way 3 now differentiates
> between strings and bytes.
>
>
> FWIW, I tried making some changes to the GrubConf.py file but only
> seemed to see more worms coming out of each can that I opened. The
> first of my attempted changes was to try and decode the buffer but
> that seems to have moved me off onto another error again, whilst the
> second line of attack, where I tried to leave the buffer as "bytes"
> whilst altering the operations on it to be "byte-aware" only
> compilacted things further.
>
> The odd things for me is that the same pygrub that can boot my
> LFS system doesn't appear to be failing.
>
> I present the chnages that I made and the resulting errors from
> my attempts to point pygrub at the partition with the grub.cfg
> file on here:
>
> bash-5.0# /usr/lib/xen/bin/pygrub --debug --offset=1048576
> --list-entries /dev/vg_xen_vbds/lv_4g_02
> Using  to parse /boot/grub/grub.cfg
> Traceback (most recent call last):
>   File "/usr/lib/xen/bin/pygrub", line 907

Re: Can the grub2.02 improve the limit of initrd size in 64-bit system?

[Vladimir 'phcoder' Serbinenko]

> So 32 bit arm requires kernel + initrd less than 512MB, 64 bit
> arm requires kernel + inirrd less than 32GB.  If I read the code
> correctly, booting x86 in efi mode the max initrd file allocation is 1GB
> (0x3fff).  Seems the code is shared for 32 and 64 bit EFI and doesn't
> allow anything more on 64 bit than 32 bit.  Of course no normal system
> would ever need more so no point implementing it.
>
> A 1GB ramdisk would be plenty to start a system that could then load more
> ramdisks if it really wanted to.  Could any sensible use case exist for
> loading an initrd that large?

I agree. I don't think that implementing support of > 1GiB ramdisk
makes sense unless we have a usecase in mind.

Original reporter: can you please ellaborate why you need such a large
ramdisk and why you can't let kernel mount additional images?

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Can the grub2.02 improve the limit of initrd size in 64-bit system? (Lennart Sorensen)

[Vladimir 'phcoder' Serbinenko]

On Thu, 1 Aug 2019, 05:20 Zhang,  wrote:

>
> Yes,I want to an initrd  as big as possible on  the service where ram size
> is large.
>
Why? Please detail your usecase

>
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Can the grub2.02 improve the limit of initrd size in 64-bit system?

[Vladimir 'phcoder' Serbinenko]

пт, 2 авг. 2019 г., 08:45 Zhang :

>
>
> Yes,I want to an initrd  as big as possible on  the service where ram size
> is large. Maybe the initrd can include files > 1GB.
>
> I am sorry that I have a mistake.The right is as follows:
> x86_64:
>
> *It seems good for initrd size between 0x37fe-0x7fff（2GB）.But the 
> grub **seems it is against that.*
>
>
This is still not a usecase. You can't ask people to invest resources into
implementing something without having at least a usecase.
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] grub-mkconfig: Fix typo in --help output

[Vladimir 'phcoder' Serbinenko]

LGTM

On Fri, 23 Aug 2019, 13:02 Colin Watson,  wrote:

> The short form of "--version" that grub-mkconfig accepts is "-V", not
> "-v".
>
> Fixes Debian bug #935504.
>
> Signed-off-by: Colin Watson 
> ---
>  util/grub-mkconfig.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
> index 9f477ff05..7660016b3 100644
> --- a/util/grub-mkconfig.in
> +++ b/util/grub-mkconfig.in
> @@ -59,7 +59,7 @@ usage () {
>  echo
>  print_option_help "-o, --output=$(gettext FILE)" "$(gettext "output
> generated config to FILE [default=stdout]")"
>  print_option_help "-h, --help" "$(gettext "print this message and
> exit")"
> -print_option_help "-v, --version" "$(gettext "print the version
> information and exit")"
> +print_option_help "-V, --version" "$(gettext "print the version
> information and exit")"
>  echo
>  gettext "Report bugs to ."; echo
>  }
> --
> 2.17.1
>
> ___
> 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

Re: Interest in a patch to edit/view EFI Boot* variables

[Vladimir 'phcoder' Serbinenko]

Also we need to add safeguards to never fill more than half of variable
storage, same as Linux. Just to avoid bricking bad firmwares

On Mon, 30 Sep 2019, 13:16 Daniel Kiper,  wrote:

> On Sun, Sep 15, 2019 at 05:14:02PM +0200, Sam van Kampen via Grub-devel
> wrote:
> > Hey all,
> >
> > To solve a problem I faced on my own system I've been working on a
> > patch that adds support for viewing and editing EFI Boot* variables
> > (BootOrder, BootNext, Boot) from GRUB, and was wondering if there
> > was any interest in including the functionality upstream.
> >
> > In its current form, the patch adds a new module (efibootvars.mod),
> > which adds the commands `bootnext`, `bootentries` and `bootorder`.
> >
> > My plan is to send the patch for review once I've finished view and
> > edit functionality for `bootnext` and `bootorder`. `bootentries` can
> > currently only show Boot variables - I might add support for
> > editing them in a subsequent patch, but they are a significantly more
> > complex data structure (EFI_LOAD_OPTION), hence editing is not
> > currently included.
> >
> > Please let me know what you think,
>
> Makes sense for me. So, please post it. Though I would consider
> disabling that functionality if UEFI secure boot is enabled. Please
> take a look at grub-core/commands/efi/shim_lock.c for more details.
>
> Daniel
>
> ___
> 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

Re: [PATCH] usb_gamepad: Add support for Logitech Rumble Gamepad F510

[Vladimir 'phcoder' Serbinenko]

Patch looks good in concept but I have few questions:
1. How is it different from normal HID
2. Why do you do custom mapping I'm the driver, why not just pass the
events pretty much as is
3. Why is it predicated on a particular device model rather than on device
class?

On Sun, 29 Sep 2019, 15:59 Alexey Kutepov,  wrote:

> Hello!
>
> I have a little bit of a strange patch to propose. Some time ago I saw
> an interesting thread on Steam asking if it's possible to control GRUB
> with a Gamepad
> https://steamcommunity.com/groups/steamuniverse/discussions/0/558751660797029626/
> and I thought "this is a pretty interesting use case for some kind of
> custom multimedia stations where you control everything (down to the
> bootloader) with a gamepad!" So I implemented usb_gamepad module
> hoping that it could be useful to somebody.
>
> The idea behind the module is very simple:
>
> - Attach to an HID USB device that is gamepad
> - Create a GRUB terminal input
> - Map actions from the gamepad to GRUB terminal input keys
>
> The module registers a bunch of commands to configure the mapping. A
> basic usb_gamepad configuration in grub.cfg looks like this:
>
> ```
> insmod usb_gamepad
> terminal_input --append usb_gamepad0
> gamepad_dpad U name up
> gamepad_dpad D name down
> gamepad_btn  1 code 13
> ```
>
> It maps D-pad up/down directions to up/down keys correspondingly and
> the first button to Enter key, so you can select an OS to boot in a
> pretty conventional way.
>
> Unfortunately, it only works with Logitech Rumble Gamepad F510
> [046d:c218] in DirectInput mode, because this is the only Gamepad I
> have and I don't understand how XInput works. But I made a genuine
> effort to structure the module so it's relatively easy to extend. So
> if anybody later needs support for a different gamepad they can try to
> extend/modify usb_gamepad module instead of implementing everything
> from scratch.
>
> If this is too "out of scope" for the GRUB project, I'm ok with this
> patch being rejected. If this is something useful I'm ready for your
> feedback and critisim to get this patch to acceptable state.
>
> Regards,
> Alexey
> ---
>  grub-core/Makefile.core.def  |   6 +
>  grub-core/term/usb_gamepad.c | 683 +++
>  2 files changed, 689 insertions(+)
>  create mode 100644 grub-core/term/usb_gamepad.c
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 474a63e68..8046918e9 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -599,6 +599,12 @@ module = {
>enable = usb;
>  };
>
> +module = {
> +  name = usb_gamepad;
> +  common = term/usb_gamepad.c;
> +  enable = usb;
> +};
> +
>  module = {
>name = usbserial_common;
>common = bus/usb/serial/common.c;
> diff --git a/grub-core/term/usb_gamepad.c b/grub-core/term/usb_gamepad.c
> new file mode 100644
> index 0..c7bb25969
> --- /dev/null
> +++ b/grub-core/term/usb_gamepad.c
> @@ -0,0 +1,683 @@
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +GRUB_MOD_LICENSE ("GPLv3");
> +
> +typedef enum {
> +DIR_UP = 0x0,
> +DIR_UPRIGHT,
> +DIR_RIGHT,
> +DIR_DOWNRIGHT,
> +DIR_DOWN,
> +DIR_DOWNLEFT,
> +DIR_LEFT,
> +DIR_UPLEFT,
> +DIR_CENTERED,
> +
> +DIR_COUNT
> +} dir_t;
> +
> +typedef enum {
> +SIDE_LEFT = 0x0,
> +SIDE_RIGHT,
> +
> +SIDE_COUNT
> +} side_t;
> +
> +#define BUTTONS_COUNT 4
> +#define GAMEPADS_CAPACITY 16
> +#define KEY_QUEUE_CAPACITY 32
> +#define USB_REPORT_SIZE 8
> +
> +#define LOGITECH_VENDORID 0x046d
> +#define RUMBLEPAD_PRODUCTID 0xc218
> +
> +static int dpad_mapping[DIR_COUNT] = { GRUB_TERM_NO_KEY };
> +static int button_mapping[BUTTONS_COUNT] = { GRUB_TERM_NO_KEY };
> +static int bumper_mapping[SIDE_COUNT] = { GRUB_TERM_NO_KEY };
> +static int trigger_mapping[SIDE_COUNT] = { GRUB_TERM_NO_KEY };
> +static int stick_mapping[SIDE_COUNT][DIR_COUNT] = { GRUB_TERM_NO_KEY };
> +static int stick_press_mapping[SIDE_COUNT] = { GRUB_TERM_NO_KEY };
> +static int options_mapping[SIDE_COUNT] = { GRUB_TERM_NO_KEY };
> +
> +struct logitech_rumble_f510_report
> +{
> +grub_uint8_t stick_axes[SIDE_COUNT * 2];
> +grub_uint8_t dpad: 4;
> +grub_uint8_t buttons: 4;
> +grub_uint8_t bumpers: 2;
> +grub_uint8_t triggers: 2;
> +grub_uint8_t options: 2;
> +grub_uint8_t sticks: 2;
> +grub_uint8_t mode;
> +grub_uint8_t padding;
> +};
> +
> +struct grub_usb_gamepad_data
> +{
> +grub_usb_device_t usbdev;
> +int configno;
> +int interfno;
> +struct grub_usb_desc_endp *endp;
> +grub_usb_transfer_t transfer;
> +grub_uint8_t prev_report[USB_REPORT_SIZE];
> +grub_uint8_t report[USB_REPORT_SIZE];
> +int key_queue[KEY_QUEUE_CAPACITY];
> +int key_queue_begin;
> +int key_queue_size;
> +};
> +
> +static grub_uint8_t initial_logitech_rumble_f510_report[USB_REPORT_SIZE]
> = {
> +0x00, 0x00, 0x00, 0x00, 0x08, 0x00, 0x04, 0xff
> +};
> +
> +static struct grub_term_i

Re: [PATCH] Fix security issue when reading username and password

[Vladimir 'phcoder' Serbinenko]

This has been committed years ago:
https://git.savannah.gnu.org/cgit/grub.git/commit/grub-core/lib/crypto.c?id=451d80e52d851432e109771bb8febafca7a5f1f2
Please contact whoever you got your GRUB from

On Fri, Oct 18, 2019 at 2:39 PM Javier Martinez Canillas
 wrote:
>
> From: Hector Marco-Gisbert 
>
>   This patch fixes two integer underflows at:
> * grub-core/lib/crypto.c
> * grub-core/normal/auth.c
>
> Resolves: CVE-2015-8370
>
> Signed-off-by: Hector Marco-Gisbert 
> Signed-off-by: Ismael Ripoll-Ripoll 
> Signed-off-by: Javier Martinez Canillas 
> ---
>
>  grub-core/lib/crypto.c  | 2 +-
>  grub-core/normal/auth.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/grub-core/lib/crypto.c b/grub-core/lib/crypto.c
> index ca334d5a40e..e6c78d16d39 100644
> --- a/grub-core/lib/crypto.c
> +++ b/grub-core/lib/crypto.c
> @@ -468,7 +468,7 @@ grub_password_get (char buf[], unsigned buf_size)
>   break;
> }
>
> -  if (key == '\b')
> +  if (key == '\b' && cur_len)
> {
>   if (cur_len)
> cur_len--;
> diff --git a/grub-core/normal/auth.c b/grub-core/normal/auth.c
> index 6be678c0de1..c35ce972473 100644
> --- a/grub-core/normal/auth.c
> +++ b/grub-core/normal/auth.c
> @@ -172,7 +172,7 @@ grub_username_get (char buf[], unsigned buf_size)
>   break;
> }
>
> -  if (key == GRUB_TERM_BACKSPACE)
> +  if (key == GRUB_TERM_BACKSPACE && cur_len)
> {
>   if (cur_len)
> {
> --
> 2.21.0
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Fix security issue when reading username and password

[Vladimir 'phcoder' Serbinenko]

On Thu, 24 Oct 2019, 18:11 Daniel Kiper,  wrote:

> Hi Hector,
>
> On Thu, Oct 24, 2019 at 03:22:43PM +0100, Hector Marco wrote:
> > Hello Daniel,
> >
> > Something went wrong in my last email, what I wanted to say is:
> >
> > The patch prevents that "cur_len" underflows. No negative values for
> > "cur_len" so no way to underflow the "cur_len" variable and therefore no
> > vulnerability.
>
> First of all cur_len is unsigned. So, it does not get negative values at
> all. Though even it was signed I cannot see where in the code it can get
> negative value. Am I missing something?
>
It's an old bug that is already fixed. "Signedness" in C is irrelevant. It
got negative/very large previously and overwrote some stack. But it's long
since fixed

>
> Daniel
>
> > Hector.
> >
> >
> > On 24/10/2019 15:13, Hector Marco wrote:
> > > Hello Daniel,
> > >
> > > The patch prevents that "cur_len" underflows. No negative values for
> > > "cur_len" so way to underflow the "cur_len" variable and therefore
> > >
> > > I hope this helps,
> > >
> > > Hector.
> > >
> > >
> > >
> > > On 23/10/2019 11:14, Daniel Kiper wrote:
> > >> On Fri, Oct 18, 2019 at 02:39:01PM +0200, Javier Martinez Canillas
> wrote:
> > >>> From: Hector Marco-Gisbert 
> > >>>
> > >>>   This patch fixes two integer underflows at:
> > >>> * grub-core/lib/crypto.c
> > >>> * grub-core/normal/auth.c
> > >>>
> > >>> Resolves: CVE-2015-8370
> > >>>
> > >>> Signed-off-by: Hector Marco-Gisbert 
> > >>> Signed-off-by: Ismael Ripoll-Ripoll 
> > >>> Signed-off-by: Javier Martinez Canillas 
> > >>> ---
> > >>>
> > >>>  grub-core/lib/crypto.c  | 2 +-
> > >>>  grub-core/normal/auth.c | 2 +-
> > >>>  2 files changed, 2 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/grub-core/lib/crypto.c b/grub-core/lib/crypto.c
> > >>> index ca334d5a40e..e6c78d16d39 100644
> > >>> --- a/grub-core/lib/crypto.c
> > >>> +++ b/grub-core/lib/crypto.c
> > >>> @@ -468,7 +468,7 @@ grub_password_get (char buf[], unsigned buf_size)
> > >>> break;
> > >>>   }
> > >>>
> > >>> -  if (key == '\b')
> > >>> +  if (key == '\b' && cur_len)
> > >>>   {
> > >>> if (cur_len)
> > >>>   cur_len--;
> > >>> diff --git a/grub-core/normal/auth.c b/grub-core/normal/auth.c
> > >>> index 6be678c0de1..c35ce972473 100644
> > >>> --- a/grub-core/normal/auth.c
> > >>> +++ b/grub-core/normal/auth.c
> > >>> @@ -172,7 +172,7 @@ grub_username_get (char buf[], unsigned buf_size)
> > >>> break;
> > >>>   }
> > >>>
> > >>> -  if (key == GRUB_TERM_BACKSPACE)
> > >>> +  if (key == GRUB_TERM_BACKSPACE && cur_len)
> > >>>   {
> > >>> if (cur_len)
> > >>>   {
> > >>
> > >> TBH, I do not understand how this patch helps. It only delays continue
> > >> execution to the next "if (!grub_isprint (key))" if cur_len == 0.
> > >>
> > >> Daniel
>
> ___
> 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

Re: [PATCH] Enable pager by default

[Vladimir 'phcoder' Serbinenko]

.h   |  4 +++-
> >  include/grub/normal.h|  3 +++
> >  7 files changed, 47 insertions(+), 7 deletions(-)
> >
> > diff --git a/grub-core/commands/help.c b/grub-core/commands/help.c
> > index f0be89baa19..6e20e1447a8 100644
> > --- a/grub-core/commands/help.c
> > +++ b/grub-core/commands/help.c
> > @@ -142,7 +142,8 @@ static grub_extcmd_t cmd;
> >
> >  GRUB_MOD_INIT(help)
> >  {
> > -  cmd = grub_register_extcmd ("help", grub_cmd_help, 0,
> > +  cmd = grub_register_extcmd ("help", grub_cmd_help,
> > +   GRUB_COMMAND_FLAG_PAGINATED,
> > N_("[PATTERN ...]"),
> > N_("Show a help message."), 0);
> >  }
> > diff --git a/grub-core/commands/minicmd.c b/grub-core/commands/minicmd.c
> > index 6bbce3128cf..e6a9242bfa6 100644
> > --- a/grub-core/commands/minicmd.c
> > +++ b/grub-core/commands/minicmd.c
> > @@ -21,6 +21,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -75,7 +76,7 @@ grub_mini_cmd_cat (struct grub_command *cmd __attribute__ 
> > ((unused)),
> >
> >  /* help */
> >  static grub_err_t
> > -grub_mini_cmd_help (struct grub_command *cmd __attribute__ ((unused)),
> > +grub_mini_cmd_help (grub_extcmd_context_t ctxt __attribute__ ((unused)),
> >   int argc __attribute__ ((unused)),
> >   char *argv[] __attribute__ ((unused)))
> >  {
> > @@ -188,7 +189,8 @@ grub_mini_cmd_exit (struct grub_command *cmd 
> > __attribute__ ((unused)),
> >/* Not reached.  */
> >  }
> >
> > -static grub_command_t cmd_cat, cmd_help;
> > +static grub_command_t cmd_cat;
> > +static grub_extcmd_t cmd_help;
> >  static grub_command_t cmd_dump, cmd_rmmod, cmd_lsmod, cmd_exit;
> >
> >  GRUB_MOD_INIT(minicmd)
> > @@ -197,8 +199,9 @@ GRUB_MOD_INIT(minicmd)
> >  grub_register_command ("cat", grub_mini_cmd_cat,
> >  N_("FILE"), N_("Show the contents of a file."));
> >cmd_help =
> > -grub_register_command ("help", grub_mini_cmd_help,
> > -0, N_("Show this message."));
> > +grub_register_extcmd ("help", grub_mini_cmd_help,
> > +  GRUB_COMMAND_FLAG_PAGINATED,
> > +  0, N_("Show this message."), 0);
> >cmd_dump =
> >  grub_register_command ("dump", grub_mini_cmd_dump,
> >  N_("ADDR [SIZE]"), N_("Show memory contents."));
> > @@ -216,7 +219,7 @@ GRUB_MOD_INIT(minicmd)
> >  GRUB_MOD_FINI(minicmd)
> >  {
> >grub_unregister_command (cmd_cat);
> > -  grub_unregister_command (cmd_help);
> > +  grub_unregister_extcmd (cmd_help);
> >grub_unregister_command (cmd_dump);
> >grub_unregister_command (cmd_rmmod);
> >grub_unregister_command (cmd_lsmod);
> > diff --git a/grub-core/normal/dyncmd.c b/grub-core/normal/dyncmd.c
> > index 719ebf477f2..1ef52cbe43b 100644
> > --- a/grub-core/normal/dyncmd.c
> > +++ b/grub-core/normal/dyncmd.c
> > @@ -78,11 +78,18 @@ grub_dyncmd_dispatcher (struct grub_extcmd_context 
> > *ctxt,
> >cmd = grub_command_find (name);
> >if (cmd)
> >  {
> > +  /* Temporarily enable paginated output for commands that asked for 
> > it */
> > +  if (cmd->flags & GRUB_COMMAND_FLAG_PAGINATED)
> > +grub_enable_temp_more ();
> > +
> >    if (cmd->flags & GRUB_COMMAND_FLAG_BLOCKS &&
> > cmd->flags & GRUB_COMMAND_FLAG_EXTCMD)
> >   ret = grub_extcmd_dispatcher (cmd, argc, args, ctxt->script);
> >else
> >   ret = (cmd->func) (cmd, argc, args);
> > +
> > +  if (cmd->flags & GRUB_COMMAND_FLAG_PAGINATED)
> > +grub_disable_temp_more ();
> >  }
> >else
> >  ret = grub_errno;
> > diff --git a/grub-core/normal/term.c b/grub-core/normal/term.c
> > index a1e5c5a0daf..7d4021ff8be 100644
> > --- a/grub-core/normal/term.c
> > +++ b/grub-core/normal/term.c
> > @@ -57,6 +57,7 @@ static struct term_state *term_states = NULL;
> >
> >  /* If the more pager is active.  */
> >  static int grub_more;
> > +static int temp_more;
> >
> >  static void
> >  putcode_real (grub_uint32_t code, struct grub_term_output *term, int 
> > fixed_tab);
> > @@ -128,6 +129,22 @@ grub_set_more (int onoff)
> >grub_normal_reset_more ();
> >  }
> >
> > +void
> > +grub_enable_temp_more (void)
> > +{
> > +  temp_more = grub_more;
> > +
> > +  if (!temp_more)
> > +grub_set_more (1);
>
> Does it change pager variable value? If yes I think
> that you should store its original value here and
> restore below.
>
> > +}
> > +
> > +void
> > +grub_disable_temp_more (void)
> > +{
> > +  if (!temp_more)
> > +grub_set_more (0);
>
> Daniel
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Enable pager by default

[Vladimir 'phcoder' Serbinenko]

On Fri, 25 Oct 2019, 08:20 Michael Chang,  wrote:

> On Thu, Oct 24, 2019 at 04:39:09PM +0200, Daniel Kiper wrote:
> > On Thu, Oct 24, 2019 at 06:54:53AM +, Michael Chang wrote:
> > > On Tue, Oct 22, 2019 at 04:04:28PM +0200, Daniel Kiper wrote:
> > > > On Tue, Oct 22, 2019 at 10:30:20AM +0200, Javier Martinez Canillas
> wrote:
> > > > > Hello Daniel,
> > > > >
> > > > > On 10/21/19 4:56 PM, Daniel Kiper wrote:
> > > > > > On Fri, Oct 18, 2019 at 02:43:18PM +0200, Javier Martinez
> Canillas wrote:
> > > > > >> From: Peter Jones 
> > > > > >>
> > > > > >> When user enters into the GRUB shell and tries to use help
> command, lot of
> > > > > >> information is scrolled out of screen and the user doesn't have
> chance to
> > > > > >> read it. Also, there isn't any information about 'set pager=1'
> at the end
> > > > > >> of the help output, to tell the user how scrolling could be
> enabled.
> > > > > >>
> > > > > >> So just enable pager by default which leads to a much better
> experience.
> > > > > >
> > > > > > Hmmm... What will happen if a command produce tons of output
> during boot
> > > > > > process? I am afraid that it will hang indefinitely waiting for
> an user
> > > > > > input. This should not happen. So, I tend to agree that current
> help
> > > > > > command behavior is annoying but I do not like the solution.
> > > > >
> > > > > Ok. I'll then explore having a paginated output only for the help
> command
> > > > > instead of globally enabling it by default.
> > > >
> > > > Great! Though I would think about something which can be used also in
> > > > other commands producing a lot of output. Maybe we should introduce
> "-p"
> > > > (pause) command line option for such commands. And I am not against
> > > > using existing code to do a pause. We just have to do it carefully.
> > >
> > > I'd like to add option to the list, which is grub could provide the
> > > information to have the commands able to tell they are executed in
> > > shell's interactive (aka command-line) or batch mode. After they could
> > > turn on/off paginated output according to the shell mode they are with.
> >
> > Sounds interesting. However, I would go further. If pager == 1 and we are
> > in batch mode then paging is inactive. If pager == 2 and we are in batch
> > mode then paging is active. If we are in interactive mode then if
> > pager != 0 then paging is active.
>
> I agree completely. In this way we no longer have to worry about setting
> page=1 would disrupt boot process as some command output could go overly
> log and at the same time setting page=2 could enforce paginated output
> everywhere, like what is working now. :)
>
This increases complexity. Like what is batch mode. Is running sourcefile
batch or no? There will be edge cases like this. More complexity is more
risk. Moreover in this case the risk is not contained unlike in e.g.
filesystem modules that are not even loaded unless needed. Also it fails at
informing the user. In fact it does the opposite by making it more
confusing at first glance. Informing the user on welcome message is better

>
> Thanks,
> Michael
>
> >
> > Daniel
>
> ___
> 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

Re: [PATCH v5 4/5] normal/main: Search for specific config files for netboot

[Vladimir 'phcoder' Serbinenko]

What is the problem with main grub.cfg just including the files based on
environment variables?

On Tue, 29 Oct 2019, 15:16 Javier Martinez Canillas, 
wrote:

> From: Paulo Flabiano Smorigo 
>
> This patch implements a search for a specific configuration when the config
> file is on a remoteserver. It uses the following order:
>1) DHCP client UUID option.
>2) MAC address (in lower case hexadecimal with dash separators);
>3) IP (in upper case hexadecimal) or IPv6;
>4) The original grub.cfg file.
>
> This procedure is similar to what is used by pxelinux and yaboot:
> http://www.syslinux.org/wiki/index.php/PXELINUX#config
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=873406
>
> Signed-off-by: Paulo Flabiano Smorigo 
> Signed-off-by: Javier Martinez Canillas 
> Reviewed-by: Daniel Kiper 
>
> ---
>
> Changes in v5:
> - Don't mix varible declarations and code.
> - Don't use variable length arrays.
> - Fix coding style issues.
> - Fix function prototype.
> - Include missing  header.
>
> Changes in v4: None
> Changes in v3:
> - Add Reviewed-by tag from Daniel Kiper.
>
> Changes in v2: None
>
>  grub-core/net/net.c | 131 
>  grub-core/normal/main.c |  19 --
>  include/grub/net.h  |   2 +
>  3 files changed, 148 insertions(+), 4 deletions(-)
>
> diff --git grub-core/net/net.c grub-core/net/net.c
> index d5d726a315e..f82ac15b4ec 100644
> --- grub-core/net/net.c
> +++ grub-core/net/net.c
> @@ -1735,6 +1735,137 @@ grub_net_restore_hw (void)
>return GRUB_ERR_NONE;
>  }
>
> +grub_err_t
> +grub_net_search_config_file (char *config)
> +{
> +  grub_size_t config_len;
> +  char *suffix;
> +
> +  auto int search_through (grub_size_t num_tries, grub_size_t slice_size);
> +  int search_through (grub_size_t num_tries, grub_size_t slice_size)
> +  {
> +while (num_tries-- > 0)
> +  {
> +grub_file_t file;
> +
> +grub_dprintf ("net", "attempt to fetch config %s\n", config);
> +
> +file = grub_file_open (config, GRUB_FILE_TYPE_CONFIG);
> +
> +if (file)
> +  {
> +grub_file_close (file);
> +return 0;
> +  }
> +else
> +  {
> +if (grub_errno == GRUB_ERR_IO)
> +  grub_errno = GRUB_ERR_NONE;
> +  }
> +
> +if (grub_strlen (suffix) < slice_size)
> +  break;
> +
> +config[grub_strlen (config) - slice_size] = '\0';
> +  }
> +
> +return 1;
> +  }
> +
> +  config_len = grub_strlen (config);
> +  config[config_len] = '-';
> +  suffix = config + config_len + 1;
> +
> +  struct grub_net_network_level_interface *inf;
> +  FOR_NET_NETWORK_LEVEL_INTERFACES (inf)
> +{
> +  /* By the Client UUID. */
> +  char *ptr;
> +  int client_uuid_len;
> +  char *client_uuid_var;
> +  const char *client_uuid;
> +
> +  client_uuid_len = sizeof ("net_") + grub_strlen (inf->name) +
> +sizeof ("_clientuuid") + 1;
> +
> +  client_uuid_var = grub_zalloc (client_uuid_len);
> +  if (!client_uuid_var)
> +return grub_errno;
> +
> +  grub_snprintf (client_uuid_var, client_uuid_len,
> + "net_%s_clientuuid", inf->name);
> +
> +  client_uuid = grub_env_get (client_uuid_var);
> +  grub_free (client_uuid_var);
> +
> +  if (client_uuid)
> +{
> +  grub_strcpy (suffix, client_uuid);
> +  if (search_through (1, 0) == 0)
> +return GRUB_ERR_NONE;
> +}
> +
> +  /* By the MAC address. */
> +
> +  /* Add ethernet type */
> +  grub_strcpy (suffix, "01-");
> +
> +  grub_net_hwaddr_to_str (&inf->hwaddress, suffix + 3);
> +
> +  for (ptr = suffix; *ptr; ptr++)
> +if (*ptr == ':')
> +  *ptr = '-';
> +
> +  if (search_through (1, 0) == 0)
> +return GRUB_ERR_NONE;
> +
> +  /* By IP address */
> +
> +  switch ((&inf->address)->type)
> +{
> +case GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4:
> +  {
> +grub_uint32_t n = grub_be_to_cpu32 ((&inf->address)->ipv4);
> +
> +grub_snprintf (suffix, GRUB_NET_MAX_STR_ADDR_LEN,
> "%02X%02X%02X%02X", \
> +   ((n >> 24) & 0xff), ((n >> 16) & 0xff),  \
> +   ((n >> 8) & 0xff), ((n >> 0) & 0xff));
> +
> +if (search_through (8, 1) == 0)
> +  return GRUB_ERR_NONE;
> +break;
> +  }
> +case GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6:
> +  {
> +char buf[GRUB_NET_MAX_STR_ADDR_LEN];
> +struct grub_net_network_level_address base;
> +base.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
> +grub_memcpy (&base.ipv6, ((&inf->address)->ipv6), 16);
> +grub_net_addr_to_str (&base, buf);
> +
> +for (ptr = buf; *ptr; ptr++)
> +  if (*ptr == ':')
> +*ptr = '-';
> +
> +grub_snprintf (suffix, GRUB_

Re: [PATCH v5 4/5] normal/main: Search for specific config files for netboot

[Vladimir 'phcoder' Serbinenko]

We had a discussion with Daniel and agreed on following:
1) we accept the feature
2) we want to have an option to specify grub config file. And if this
option is present, the scanning would be disabled
3) to avoid multiple round-trips can we request all the possible files in
parallel?

On Wed, 30 Oct 2019, 11:40 Javier Martinez Canillas, 
wrote:

> Hello Vladimir,
>
> On 10/29/19 5:18 PM, Vladimir 'phcoder' Serbinenko wrote:
> > What is the problem with main grub.cfg just including the files based on
> > environment variables?
> >
>
> Do you mean having a grub.cfg that fetches the machine specific grub.cfg?
> i.e:
>
> configfile (tftp,$net_default_server)/EFI/BOOT/grub.cfg.$net_default_ip
>
> While is true that this could be done, is not ideal since then all the
> logic
> to determine which config file to fetch will have to be encoded into this
> first
> stage grub.cfg that will be downloaded by all the machines.
>
> This policy will have to be written as a GRUB config script instead of
> being in
> the installation server backend, that could just provide the machine
> specific
> config files using the suffixes convention introduced by this patch-set.
>
> And when managing a big fleet of machines the monolithic approach of
> having all
> this policy in a single grub.cfg file could be hard to maintain and error
> prone.
>
> As mentioned in the commit message, others bootloaders already support
> this. For
> example pxelinux and yaboot. So users migrating from pxelinux and yaboot
> to GRUB
> for PXE booting asked for this since they already have built
> infrastructure that
> rely on this feature.
>
> We have been carrying these patches since 2012 and our users expect this
> feature
> to be available for their network installation environments. I also think
> that's
> useful and gives GRUB feature parity with other bootloaders for network
> booting.
>
> Best regards,
> --
> Javier Martinez Canillas
> Software Engineer - Desktop Hardware Enablement
> Red Hat
>
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] ofnet: Fix incorrect mask for ppc64

[Vladimir 'phcoder' Serbinenko]

Please don't chain swap_bytes32 with le_to_cpu32 as it's a less readable
equivalent of be_to_cpu32

On Thu, 31 Oct 2019, 11:38 Javier Martinez Canillas, 
wrote:

> From: Masahiro Matsuya 
>
> The netmask configured in firmware is not respected on ppc64 (big endian).
> When 255.255.252.0 is set as netmask in firmware, the following is the
> value of bootpath string in grub_ieee1275_parse_bootpath().
>
>  /vdevice/l-lan@3002
> :speed=auto,duplex=auto,192.168.88.10,,192.168.89.113,192.168.88.1,5,5,255.255.252.0,512
>
> The netmask in this bootpath is no problem, since it's a value specified
> in firmware. But, The value of 'subnet_mask.ipv4' was set with 0xfc00,
> and __builtin_ctz (~grub_le_to_cpu32 (subnet_mask.ipv4)) returned 16 (not
> 22).
> As a result, 16 was used for netmask wrongly.
>
>      1100   # subnet_mask.ipv4 (=0xfc00)
>    1100     # grub_le_to_cpu32
> (subnet_mask.ipv4)
>    0011     # ~grub_le_to_cpu32
> (subnet_mask.ipv4)
>
> And, the count of zero with __builtin_ctz can be 16.
> This patch changes it as below.
>
>      1100   # subnet_mask.ipv4 (=0xfc00)
>    1100     # grub_le_to_cpu32
> (subnet_mask.ipv4)
>      1100   #
> grub_swap_bytes32(grub_le_to_cpu32 (subnet_mask.ipv4))
>      0011   #
> ~grub_swap_bytes32(grub_le_to_cpu32 (subnet_mask.ipv4))
>
> The count of zero with __builtin_clz can be 22. (clz counts the number of
> one bits preceding the most significant zero bit)
>
> Signed-off-by: Masahiro Matsuya 
> Signed-off-by: Javier Martinez Canillas 
> ---
>
>  grub-core/net/drivers/ieee1275/ofnet.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git grub-core/net/drivers/ieee1275/ofnet.c
> grub-core/net/drivers/ieee1275/ofnet.c
> index ac4e62a95c9..3860b6f78d8 100644
> --- grub-core/net/drivers/ieee1275/ofnet.c
> +++ grub-core/net/drivers/ieee1275/ofnet.c
> @@ -220,8 +220,7 @@ grub_ieee1275_parse_bootpath (const char *devpath,
> char *bootpath,
>   flags);
>inter->vlantag = vlantag;
>grub_net_add_ipv4_local (inter,
> -  __builtin_ctz (~grub_le_to_cpu32
> (subnet_mask.ipv4)));
> -
> +  __builtin_clz
> (~grub_swap_bytes32(grub_le_to_cpu32 (subnet_mask.ipv4;
>  }
>
>if (gateway_addr.ipv4 != 0)
> --
> 2.21.0
>
>
> ___
> 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

Re: [PATCH] tftp: Normalize slashes in tftp paths

[Vladimir 'phcoder' Serbinenko]

Having // at the beginning of the path may have special meaning according
to posix. I don't know if it applies in particular case and if the special
meaning is useful for grub to begin with, just something to check

On Thu, 31 Oct 2019, 11:37 Javier Martinez Canillas, 
wrote:

> From: Lenny Szubowicz 
>
> Some tftp servers do not handle multiple consecutive slashes correctly;
> this patch avoids sending tftp requests with non-normalized paths.
>
> Signed-off-by: Lenny Szubowicz 
> Signed-off-by: Mark Salter 
> Signed-off-by: Javier Martinez Canillas 
> ---
>
>  grub-core/net/tftp.c | 28 +---
>  1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git grub-core/net/tftp.c grub-core/net/tftp.c
> index 7d90bf66e76..6dbb9cdbb7a 100644
> --- grub-core/net/tftp.c
> +++ grub-core/net/tftp.c
> @@ -300,6 +300,25 @@ destroy_pq (tftp_data_t data)
>grub_priority_queue_destroy (data->pq);
>  }
>
> +/* Create a normalized copy of the filename.
> +   Compress any string of consecutive forward slashes to a single forward
> +   slash. */
> +static void
> +grub_normalize_filename (char *normalized, const char *filename)
> +{
> +  char *dest = normalized;
> +  const char *src = filename;
> +
> +  while (*src != '\0')
> +{
> +  if (src[0] == '/' && src[1] == '/')
> +src++;
> +  else
> +*dest++ = *src++;
> +}
> +  *dest = '\0';
> +}
> +
>  static grub_err_t
>  tftp_open (struct grub_file *file, const char *filename)
>  {
> @@ -337,9 +356,12 @@ tftp_open (struct grub_file *file, const char
> *filename)
>rrqlen = 0;
>
>tftph->opcode = grub_cpu_to_be16_compile_time (TFTP_RRQ);
> -  grub_strcpy (rrq, filename);
> -  rrqlen += grub_strlen (filename) + 1;
> -  rrq += grub_strlen (filename) + 1;
> +
> +  /* Copy and normalize the filename to work-around issues on some tftp
> + servers when file names are being matched for remapping. */
> +  grub_normalize_filename (rrq, filename);
> +  rrqlen += grub_strlen (rrq) + 1;
> +  rrq += grub_strlen (rrq) + 1;
>
>grub_strcpy (rrq, "octet");
>rrqlen += grub_strlen ("octet") + 1;
> --
> 2.21.0
>
>
> ___
> 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

Re: [PATCH v3 1/2] Add grub_util_readlink()

[Vladimir 'phcoder' Serbinenko]

This patch LGTM

On Wed, 30 Oct 2019, 11:49 Javier Martinez Canillas, 
wrote:

> From: Peter Jones 
>
> Currently grub-editenv and related tools are not able to follow symbolic
> links when finding their config file. For example the grub-editenv create
> command will wrongly overwrite a symlink in /boot/grub2/grubenv with a new
> regular file, instead of creating a file in the path the symlink points to.
>
> A following patch will change that and add support in grub-editenv to
> follow symbolic links when finding the grub environment variables file.
>
> Add a grub_util_readlink() helper function that is just a wrapper around
> the platform specific function to read the value of a symbolic link. This
> helper function will be used by the following patch for grub-editenv.
>
> Signed-off-by: Peter Jones 
> Reviewed-by: Adam Jackson 
> Signed-off-by: Javier Martinez Canillas 
> Reviewed-by: Daniel Kiper 
>
> ---
>
> Changes in v3: None
> Changes in v2:
> - Add a better commit message explaining why the change is needed.
> - Add Reviewed-by tag from Daniel Kiper.
>
>  grub-core/osdep/windows/hostdisk.c| 6 ++
>  include/grub/osdep/hostfile_aros.h| 6 ++
>  include/grub/osdep/hostfile_unix.h| 6 ++
>  include/grub/osdep/hostfile_windows.h | 2 ++
>  4 files changed, 20 insertions(+)
>
> diff --git grub-core/osdep/windows/hostdisk.c
> grub-core/osdep/windows/hostdisk.c
> index 355100789a7..87a106c9b82 100644
> --- grub-core/osdep/windows/hostdisk.c
> +++ grub-core/osdep/windows/hostdisk.c
> @@ -365,6 +365,12 @@ grub_util_mkdir (const char *dir)
>free (windows_name);
>  }
>
> +ssize_t
> +grub_util_readlink (const char *name, char *buf, size_t bufsize)
> +{
> +  return readlink(name, buf, bufsize);
> +}
> +
>  int
>  grub_util_rename (const char *from, const char *to)
>  {
> diff --git include/grub/osdep/hostfile_aros.h
> include/grub/osdep/hostfile_aros.h
> index a059c0fa40a..161fbb7bdfd 100644
> --- include/grub/osdep/hostfile_aros.h
> +++ include/grub/osdep/hostfile_aros.h
> @@ -68,6 +68,12 @@ grub_util_rename (const char *from, const char *to)
>return rename (from, to);
>  }
>
> +static inline ssize_t
> +grub_util_readlink (const char *name, char *buf, size_t bufsize)
> +{
> +  return readlink(name, buf, bufsize);
> +}
> +
>  #define grub_util_mkdir(a) mkdir ((a), 0755)
>
>  struct grub_util_fd
> diff --git include/grub/osdep/hostfile_unix.h
> include/grub/osdep/hostfile_unix.h
> index 9ffe46fa3ca..17cd3aa8b30 100644
> --- include/grub/osdep/hostfile_unix.h
> +++ include/grub/osdep/hostfile_unix.h
> @@ -71,6 +71,12 @@ grub_util_rename (const char *from, const char *to)
>return rename (from, to);
>  }
>
> +static inline ssize_t
> +grub_util_readlink (const char *name, char *buf, size_t bufsize)
> +{
> +  return readlink(name, buf, bufsize);
> +}
> +
>  #define grub_util_mkdir(a) mkdir ((a), 0755)
>
>  #if defined (__NetBSD__)
> diff --git include/grub/osdep/hostfile_windows.h
> include/grub/osdep/hostfile_windows.h
> index bf6451b6db4..8c92d0591bb 100644
> --- include/grub/osdep/hostfile_windows.h
> +++ include/grub/osdep/hostfile_windows.h
> @@ -41,6 +41,8 @@ typedef struct grub_util_fd_dir *grub_util_fd_dir_t;
>
>  int
>  grub_util_rename (const char *from, const char *to);
> +ssize_t
> +grub_util_readlink (const char *name, char *buf, size_t bufsize);
>  int
>  grub_util_unlink (const char *name);
>  void
> --
> 2.21.0
>
>
> ___
> 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

Re: [PATCH] arm64: make sure fdt has #address-cells and #size-cells properties

[Vladimir 'phcoder' Serbinenko]

This patch looks incomplete. What if "chosen" is there but does not contain
the relevant fields?

On Thu, 31 Oct 2019, 11:39 Javier Martinez Canillas, 
wrote:

> From: Mark Salter 
>
> Recent upstream changes to kexec-tools relies on #address-cells
> and #size-cells properties in the FDT. If grub2 needs to create
> a chosen node, it is likely because firmware did not provide one.
> In that case, set #address-cells and #size-cells properties to
> make sure they exist.
>
> Signed-off-by: Mark Salter 
> Signed-off-by: Javier Martinez Canillas 
> ---
>
>  grub-core/loader/arm64/linux.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git grub-core/loader/arm64/linux.c grub-core/loader/arm64/linux.c
> index ef3e9f9444c..24d73732d89 100644
> --- grub-core/loader/arm64/linux.c
> +++ grub-core/loader/arm64/linux.c
> @@ -78,7 +78,21 @@ finalize_params_linux (void)
>
>node = grub_fdt_find_subnode (fdt, 0, "chosen");
>if (node < 0)
> -node = grub_fdt_add_subnode (fdt, 0, "chosen");
> +{
> +  /*
> +   * If we have to create a chosen node, Make sure we
> +   * have #address-cells and #size-cells properties.
> +   */
> +  retval = grub_fdt_set_prop32(fdt, 0, "#address-cells", 2);
> +  if (retval)
> +goto failure;
> +
> +  retval = grub_fdt_set_prop32(fdt, 0, "#size-cells", 2);
> +  if (retval)
> +goto failure;
> +
> +  node = grub_fdt_add_subnode (fdt, 0, "chosen");
> +}
>
>if (node < 1)
>  goto failure;
> --
> 2.21.0
>
>
> ___
> 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

Re: [PATCH] btrfs: disable zstd support for i386-pc

[Vladimir 'phcoder' Serbinenko]

Please don't do it this way. The right solution is to move it to separate
module and include zstd module when needed. Not everybody uses btrfs
embedded area. I recommend not to use it. Using mbr gap or BBP is the
recommended way.

On Tue, 5 Nov 2019, 01:25 Michael Chang,  wrote:

> The zstd support in btrfs has dependenciy to zstd module and core.img
> grows its size significantly to 75KB on my system. The resulted image
> cannot be installed into btrfs bootloader area in the size of 64KB and
> eventually fails with following message.
>
> /usr/sbin/grub-install: warning: your core.img is unusually large.  It
> won't fit in the embedding area.
> /usr/sbin/grub-install: error: filesystem `btrfs' doesn't support
> blocklists.
>
> The patch disabled the zstd support of btrfs in pc-bios platform to
> avoid the regression. The resulting size is 56KB, albeit a bit too close
> to the 64KB but works. This is simple workaround until a proper fix
> landed upstream.
>
> Signed-off-by: Michael Chang 
> ---
>  grub-core/fs/btrfs.c | 18 ++
>  1 file changed, 18 insertions(+)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index 48bd3d04a..8f98892d3 100644
> --- a/grub-core/fs/btrfs.c
> +++ b/grub-core/fs/btrfs.c
> @@ -17,6 +17,7 @@
>   *  along with GRUB.  If not, see .
>   */
>
> +#ifndef GRUB_MACHINE_PCBIOS
>  /*
>   * Tell zstd to expose functions that aren't part of the stable API, which
>   * aren't safe to use when linking against a dynamic library. We vendor
> in a
> @@ -24,6 +25,7 @@
>   * functions to provide our own allocator, which uses grub_malloc(), to
> zstd.
>   */
>  #define ZSTD_STATIC_LINKING_ONLY
> +#endif
>
>  #include 
>  #include 
> @@ -35,7 +37,9 @@
>  #include 
>  #include 
>  #include 
> +#ifndef GRUB_MACHINE_PCBIOS
>  #include 
> +#endif
>  #include 
>  #include 
>  #include 
> @@ -56,8 +60,10 @@ GRUB_MOD_LICENSE ("GPLv3+");
>  #define GRUB_BTRFS_LZO_BLOCK_MAX_CSIZE (GRUB_BTRFS_LZO_BLOCK_SIZE + \
>  (GRUB_BTRFS_LZO_BLOCK_SIZE / 16) + 64
> + 3)
>
> +#ifndef GRUB_MACHINE_PCBIOS
>  #define ZSTD_BTRFS_MAX_WINDOWLOG 17
>  #define ZSTD_BTRFS_MAX_INPUT (1 << ZSTD_BTRFS_MAX_WINDOWLOG)
> +#endif
>
>  typedef grub_uint8_t grub_btrfs_checksum_t[0x20];
>  typedef grub_uint16_t grub_btrfs_uuid_t[8];
> @@ -229,7 +235,9 @@ struct grub_btrfs_extent_data
>  #define GRUB_BTRFS_COMPRESSION_NONE 0
>  #define GRUB_BTRFS_COMPRESSION_ZLIB 1
>  #define GRUB_BTRFS_COMPRESSION_LZO  2
> +#ifndef GRUB_MACHINE_PCBIOS
>  #define GRUB_BTRFS_COMPRESSION_ZSTD 3
> +#endif
>
>  #define GRUB_BTRFS_OBJECT_ID_CHUNK 0x100
>
> @@ -1229,6 +1237,7 @@ grub_btrfs_read_inode (struct grub_btrfs_data *data,
>return grub_btrfs_read_logical (data, elemaddr, inode, sizeof (*inode),
> 0);
>  }
>
> +#ifndef GRUB_MACHINE_PCBIOS
>  static void *grub_zstd_malloc (void *state __attribute__((unused)),
> size_t size)
>  {
>return grub_malloc (size);
> @@ -1318,6 +1327,7 @@ err:
>
>return ret;
>  }
> +#endif
>
>  static grub_ssize_t
>  grub_btrfs_lzo_decompress(char *ibuf, grub_size_t isize, grub_off_t off,
> @@ -1494,8 +1504,12 @@ grub_btrfs_extent_read (struct grub_btrfs_data
> *data,
>
>if (data->extent->compression != GRUB_BTRFS_COMPRESSION_NONE
>   && data->extent->compression != GRUB_BTRFS_COMPRESSION_ZLIB
> +#ifndef GRUB_MACHINE_PCBIOS
>   && data->extent->compression != GRUB_BTRFS_COMPRESSION_LZO
>   && data->extent->compression != GRUB_BTRFS_COMPRESSION_ZSTD)
> +#else
> + && data->extent->compression != GRUB_BTRFS_COMPRESSION_LZO)
> +#endif
> {
>   grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
>   "compression type 0x%x not supported",
> @@ -1535,6 +1549,7 @@ grub_btrfs_extent_read (struct grub_btrfs_data *data,
>   != (grub_ssize_t) csize)
> return -1;
> }
> +#ifndef GRUB_MACHINE_PCBIOS
>   else if (data->extent->compression ==
> GRUB_BTRFS_COMPRESSION_ZSTD)
> {
>   if (grub_btrfs_zstd_decompress (data->extent->inl,
> data->extsize -
> @@ -1544,6 +1559,7 @@ grub_btrfs_extent_read (struct grub_btrfs_data *data,
>   != (grub_ssize_t) csize)
> return -1;
> }
> +#endif
>   else
> grub_memcpy (buf, data->extent->inl + extoff, csize);
>   break;
> @@ -1581,10 +1597,12 @@ grub_btrfs_extent_read (struct grub_btrfs_data
> *data,
> ret = grub_btrfs_lzo_decompress (tmp, zsize, extoff
> + grub_le_to_cpu64
> (data->extent->offset),
> buf, csize);
> +#ifndef GRUB_MACHINE_PCBIOS
>   else if (data->extent->compression ==
> GRUB_BTRFS_COMPRESSION_ZSTD)
> ret = grub_btrfs_zstd_decompress (tmp, zsize, extoff
> + grub_le_to_cpu64
> (data->extent->offset),
>   

Re: [PATCH] btrfs: disable zstd support for i386-pc

[Vladimir 'phcoder' Serbinenko]

On Wed, 6 Nov 2019, 20:55 Michael Chang,  wrote:

> On Wed, Nov 06, 2019 at 11:15:04AM -0800, Vladimir 'phcoder' Serbinenko
> wrote:
> > Please don't do it this way. The right solution is to move it to separate
> > module and include zstd module when needed.
>
> I fully agree to work out a proper solution, but before that I think it
> is necessary to stop it from spread out going forward, as the running
> system upgrading to new version could be affected and the consequence is
> fatal - an unbootable system, qualified to be show stopper imho.
>
We don't do a release right now, so I don't think it's justified. Also
increase in size can easily come from a compiler. If an increase in size
can make system unbootable on upgrade, I'd rather be fixing this than just
pepper over it.

>
> > Not everybody uses btrfs
> > embedded area. I recommend not to use it. Using mbr gap or BBP is the
> > recommended way.
>
> The problem here is existing installation would fail to update to new
> grub version if it is using btrfs bootloader area. For new installation
> certainly we could recommend them to do so. or btrfs zstd support will
> be disabled if using a btrfs bootloader area.
>


> Thanks,
> Michael
>
> >
> > On Tue, 5 Nov 2019, 01:25 Michael Chang,  wrote:
> >
> > > The zstd support in btrfs has dependenciy to zstd module and core.img
> > > grows its size significantly to 75KB on my system. The resulted image
> > > cannot be installed into btrfs bootloader area in the size of 64KB and
> > > eventually fails with following message.
> > >
> > > /usr/sbin/grub-install: warning: your core.img is unusually large.  It
> > > won't fit in the embedding area.
> > > /usr/sbin/grub-install: error: filesystem `btrfs' doesn't support
> > > blocklists.
> > >
> > > The patch disabled the zstd support of btrfs in pc-bios platform to
> > > avoid the regression. The resulting size is 56KB, albeit a bit too
> close
> > > to the 64KB but works. This is simple workaround until a proper fix
> > > landed upstream.
> > >
> > > Signed-off-by: Michael Chang 
> > > ---
> > >  grub-core/fs/btrfs.c | 18 ++
> > >  1 file changed, 18 insertions(+)
> > >
> > > diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> > > index 48bd3d04a..8f98892d3 100644
> > > --- a/grub-core/fs/btrfs.c
> > > +++ b/grub-core/fs/btrfs.c
> > > @@ -17,6 +17,7 @@
> > >   *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> > >   */
> > >
> > > +#ifndef GRUB_MACHINE_PCBIOS
> > >  /*
> > >   * Tell zstd to expose functions that aren't part of the stable API,
> which
> > >   * aren't safe to use when linking against a dynamic library. We
> vendor
> > > in a
> > > @@ -24,6 +25,7 @@
> > >   * functions to provide our own allocator, which uses grub_malloc(),
> to
> > > zstd.
> > >   */
> > >  #define ZSTD_STATIC_LINKING_ONLY
> > > +#endif
> > >
> > >  #include 
> > >  #include 
> > > @@ -35,7 +37,9 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#ifndef GRUB_MACHINE_PCBIOS
> > >  #include 
> > > +#endif
> > >  #include 
> > >  #include 
> > >  #include 
> > > @@ -56,8 +60,10 @@ GRUB_MOD_LICENSE ("GPLv3+");
> > >  #define GRUB_BTRFS_LZO_BLOCK_MAX_CSIZE (GRUB_BTRFS_LZO_BLOCK_SIZE + \
> > >  (GRUB_BTRFS_LZO_BLOCK_SIZE / 16)
> + 64
> > > + 3)
> > >
> > > +#ifndef GRUB_MACHINE_PCBIOS
> > >  #define ZSTD_BTRFS_MAX_WINDOWLOG 17
> > >  #define ZSTD_BTRFS_MAX_INPUT (1 << ZSTD_BTRFS_MAX_WINDOWLOG)
> > > +#endif
> > >
> > >  typedef grub_uint8_t grub_btrfs_checksum_t[0x20];
> > >  typedef grub_uint16_t grub_btrfs_uuid_t[8];
> > > @@ -229,7 +235,9 @@ struct grub_btrfs_extent_data
> > >  #define GRUB_BTRFS_COMPRESSION_NONE 0
> > >  #define GRUB_BTRFS_COMPRESSION_ZLIB 1
> > >  #define GRUB_BTRFS_COMPRESSION_LZO  2
> > > +#ifndef GRUB_MACHINE_PCBIOS
> > >  #define GRUB_BTRFS_COMPRESSION_ZSTD 3
> > > +#endif
> > >
> > >  #define GRUB_BTRFS_OBJECT_ID_CHUNK 0x100
> > >
> > > @@ -1229,6 +1237,7 @@ grub_btrfs_read_inode (struct grub_btrfs_data
> *data,
> > >return grub_btrfs_read_logical (data, elemaddr, inode, sizeof
> (*inode

Re: workaround for MS Windows' insanity (ignores partition types / hidden flag since 2018)

[Vladimir 'phcoder' Serbinenko]

On Wed, 13 Nov 2019, 17:12 Marcel Partap,  wrote:

> Dear grub devs,
> as we have just last week released a hundred of our debian-based
> https://github.com/fsfw-dresden/usb-live-linux/ sticks .. and at the
> moment are rolling out a version for primary schools, I have stumbled over
> the pure stupidity that is Windows 10's handling of partitioned usb drives.
> As seems abundantly documented, windows changed its behaviour sometime last
> year and now blatantly ignores the hidden flag, displaying partitions
> regardless of type and prominently offering to format non-windows
> filesystems. I was somewhat shocked at this ridiculous demeanour.. After an
> hour of investigating this I found the very sole workaround was to set the
> partition type to 0 - empty/none. This was the only way to windows not show
> and offer to format partitions beyond the exchange FAT partition.
> Rejoicing, on restart I found out that grub would not proceed booting from
> a partition of type "empty". Now semantically, that is totally correct.
> However, in order to provide us with a coping mechanism for such live linux
> use scenarios, I propose to add a flag that overrides this code:
> >   static inline int
> >   grub_msdos_partition_is_empty (int type)
> >   {
> > return (type == GRUB_PC_PARTITION_TYPE_NONE);
> >   }
> to allow booting from partitions of type 0 empty/none.
>
Type 0 means that entry is empty and changing this is to expose bunch of
garbage partition and this is behavior followed by other consumers like
Linux and BSD.
However we can assist you in finding a better solution. Did you already
consider alternative partition maps? I'd try msdos+gpt or msdos+BSD or
msdos+sunpc. Dinner other schemes might also work. Another workaround is to
put some fake FAT structures that will end up as showing as a FS. You can
even put some explanation files there

> This should cause less confusion to the hundreds of young students atm
> wondering about "what the hell is formatting" ..
>
> Best Regards,
> #marcel
> ___
> 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

Re: Transfering Data from GRUB to userspace?

[Vladimir 'phcoder' Serbinenko]

Did you look at cpio generation facility in initrd command? This may be
what your are looking for

On Mon, 28 Oct 2019, 03:59 Max Tottenham via Grub-devel, 
wrote:

> Hi Folks
>
> I've often found myself in a position where I've needed to perform some
> debugging of remote hardware that does not have a network accessible KVM
> attached. To that end, being able to remotely log into a machine and
> have some method of accessing GRUB2 boot time debug messages from
> userspace (in my case linux), would be rather useful. It also dawned on
> me that this capability might be the sort of thing that folks upstream
> might be keen on.
>
> I have done a little thinking and have thought of two possible
> approaches:
>
>
> 1. Perform some hacking of the grub environment block (load/save env)
> handling to be able to write log data to either the same file in /boot,
> or perhaps a separate distinct debug log file.
>
> 2. Export the debug log to a running kernel as another initrd image. In
> other words, have GRUB2 buffer a bunch of log messages and then append
> that log data as a separate initrd archive. Some early userspace program
> would then be able to copy that archive to the real filesystem, or
> stick it into the kernel log buffer. The logfile in the
> initrd that GRUB generates could have either a configurable name based
> on some setting in grub.cfg, or some well known default name (like
> /grub2-debug.log)
>
>
> I initially started with 1, but I dislike the approach of having GRUB2
> write to the filesystem, it's also a little cumbersome to setup,
>
>
> Approach 2 seems like it would work (at least for linux, I've no
> experience with the BSDs but presumably any OS that can load multiple
> initrd's would be able to use this approach too), and has the advantage
> that it doesn't have to touch any data on disk.
>
> If there is no objection I can whip up a POC patch for 2 and post it to
> the list?
>
> Regards
>
> --
> Max Tottenham   | mtott...@akamai.com
> Senior Software Engineer, Server Platform Engineering
> /(* Akamai Technologies
>
> ___
> 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

Re: Disable CLI in submenu but allow users to access it

[Vladimir 'phcoder' Serbinenko]

On Fri, 29 Nov 2019, 15:17 Paul Menzel,  wrote:

> Dear GRUB folks,
>
>
> Is it possible to allow unprivileged users to access a submenu but
> not enter the command-line interface?
>
> Reading the documentation [1] it’s not clear to me how to do that,
> and my tests to do that also failed.
>
> The warning in [2] also hints that it’s not possible.
>
You need to export variables related to security

>
> > Warning — Adding --unrestricted to a menu entry with submenuentry
> > will allow ANYONE gain superuser authority in the submenuentry ! So
> > make sure the --unrestricted option is not used for a menu entry with
> > submenuentry, Or add GRUB_DISABLE_SUBMENU=y to /etc/default/grub /
> > /etc/sysconfig/grub to disable submenu.
>
>
> Kind regards,
>
> Paul
>
>
> [1]: https://www.gnu.org/software/grub/manual/grub/grub.html#Security
> [2]:
> https://philosophos.github.io/articles/20170304~Password-Protection-of-GRUB-Menu/
>
> ___
> 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

Re: Disable CLI in submenu but allow users to access it

[Vladimir 'phcoder' Serbinenko]

Can you update the documentation?

On Tue, 3 Dec 2019, 12:32 Paul Menzel,  wrote:

> Dear Vladimir,
>
>
> On 2019-11-29 16:34, Vladimir 'phcoder' Serbinenko wrote:
> > On Fri, 29 Nov 2019, 15:17 Paul Menzel,  wrote:
>
> >> Is it possible to allow unprivileged users to access a submenu but
> >> not enter the command-line interface?
> >>
> >> Reading the documentation [1] it’s not clear to me how to do that,
> >> and my tests to do that also failed.
> >>
> >> The warning in [2] also hints that it’s not possible.
>
> […]
>
> > You need to export variables related to security
>
> Thank you. The line
>
> export superusers
>
> helped indeed.
>
>
> Kind regards,
>
> Paul
>
>
> >> [1]: https://www.gnu.org/software/grub/manual/grub/grub.html#Security
> >> [2]:
> https://philosophos.github.io/articles/20170304~Password-Protection-of-GRUB-Menu/
>
> ___
> 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

Fwd: [PATCH] ofnet: Fix incorrect mask for ppc64

[Vladimir 'phcoder' Serbinenko]

-- Forwarded message -
From: Vladimir 'phcoder' Serbinenko 
Date: Fri, 13 Dec 2019, 15:56
Subject: Re: [PATCH] ofnet: Fix incorrect mask for ppc64
To: Javier Martinez Canillas 




On Fri, 13 Dec 2019, 15:33 Javier Martinez Canillas, 
wrote:

> Hello Vladimir,
>
> Thanks a lot for your feedback.
>
> On 10/31/19 12:58 PM, Vladimir 'phcoder' Serbinenko wrote:
> > Please don't chain swap_bytes32 with le_to_cpu32 as it's a less readable
> > equivalent of be_to_cpu32
> >
>
> That's correct. In fact I think that the patch is wrong and what we should
> do is just to drop the grub_le_to_cpu32() instead. I'll get access to a
> ppc64le machine to test and post a v2 doing that.
>
ppc64le machine boots in big-endian mode. The only supported architecture
with BE OFW is i386-ieee1275 but I'm not even sure there are i386-ieee1275
machines with ofnet.

>
> Best regards,
> --
> Javier Martinez Canillas
> Software Engineer - Desktop Hardware Enablement
> Red Hat
>
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] ofnet: Fix incorrect mask for ppc64

[Vladimir 'phcoder' Serbinenko]

On Fri, 13 Dec 2019, 15:56 Vladimir 'phcoder' Serbinenko, 
wrote:

>
> -- Forwarded message -----
> From: Vladimir 'phcoder' Serbinenko 
> Date: Fri, 13 Dec 2019, 15:56
> Subject: Re: [PATCH] ofnet: Fix incorrect mask for ppc64
> To: Javier Martinez Canillas 
>
>
>
>
> On Fri, 13 Dec 2019, 15:33 Javier Martinez Canillas, 
> wrote:
>
>> Hello Vladimir,
>>
>> Thanks a lot for your feedback.
>>
>> On 10/31/19 12:58 PM, Vladimir 'phcoder' Serbinenko wrote:
>> > Please don't chain swap_bytes32 with le_to_cpu32 as it's a less readable
>> > equivalent of be_to_cpu32
>> >
>>
>> That's correct. In fact I think that the patch is wrong and what we should
>> do is just to drop the grub_le_to_cpu32() instead. I'll get access to a
>> ppc64le machine to test and post a v2 doing that.
>>
> ppc64le machine boots in big-endian mode. The only supported architecture
> with BE OFW is i386-ieee1275 but I'm not even sure there are i386-ieee1275
> machines with ofnet.
>
I mean LE OFW. Given that IP addresses are generally stored in BE order, I
think be_to_cpu32 is reasonable but probably no machine is impacted either
way.
Maybe some qemu config could have i386-ieee1275 with ofnet but generally
i386-ieee1275 is dead target

>
>> Best regards,
>> --
>> Javier Martinez Canillas
>> Software Engineer - Desktop Hardware Enablement
>> Red Hat
>>
>>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

[Vladimir 'phcoder' Serbinenko]

Please evaluate size increase for this. In the past passing file and line
number to grub_dprintf was a huge source of increased Kern and core size

Le mer. 4 mars 2020 à 13:01, Javier Martinez Canillas 
a écrit :

> From: Peter Jones 
>
> Add file and line to grub_error() output to make troubleshooting easier.
>
> Signed-off-by: Peter Jones 
> Signed-off-by: Javier Martinez Canillas 
> ---
>
>  grub-core/kern/err.c | 13 +++--
>  include/grub/err.h   |  5 -
>  2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/grub-core/kern/err.c b/grub-core/kern/err.c
> index 53c734de70e..aebfe0cf839 100644
> --- a/grub-core/kern/err.c
> +++ b/grub-core/kern/err.c
> @@ -33,15 +33,24 @@ static struct grub_error_saved
> grub_error_stack_items[GRUB_ERROR_STACK_SIZE];
>  static int grub_error_stack_pos;
>  static int grub_error_stack_assert;
>
> +#ifdef grub_error
> +#undef grub_error
> +#endif
> +
>  grub_err_t
> -grub_error (grub_err_t n, const char *fmt, ...)
> +grub_error (grub_err_t n, const char *file, const int line, const char
> *fmt, ...)
>  {
>va_list ap;
> +  int m;
>
>grub_errno = n;
>
> +  m = grub_snprintf (grub_errmsg, sizeof (grub_errmsg), "%s:%d:", file,
> line);
> +  if (m < 0)
> +m = 0;
> +
>va_start (ap, fmt);
> -  grub_vsnprintf (grub_errmsg, sizeof (grub_errmsg), _(fmt), ap);
> +  grub_vsnprintf (grub_errmsg + m, sizeof (grub_errmsg) - m, _(fmt), ap);
>va_end (ap);
>
>return n;
> diff --git a/include/grub/err.h b/include/grub/err.h
> index 24ba9f5f592..b68bbec3c72 100644
> --- a/include/grub/err.h
> +++ b/include/grub/err.h
> @@ -85,7 +85,10 @@ struct grub_error_saved
>  extern grub_err_t EXPORT_VAR(grub_errno);
>  extern char EXPORT_VAR(grub_errmsg)[GRUB_MAX_ERRMSG];
>
> -grub_err_t EXPORT_FUNC(grub_error) (grub_err_t n, const char *fmt, ...);
> +grub_err_t EXPORT_FUNC(grub_error) (grub_err_t n, const char *file, const
> int line, const char *fmt, ...);
> +
> +#define grub_error(n, fmt, ...) grub_error (n, __FILE__, __LINE__, fmt,
> ##__VA_ARGS__)
> +
>  void EXPORT_FUNC(grub_fatal) (const char *fmt, ...) __attribute__
> ((noreturn));
>  void EXPORT_FUNC(grub_error_push) (void);
>  int EXPORT_FUNC(grub_error_pop) (void);
> --
> 2.24.1
>
>
> ___
> 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

Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

[Vladimir 'phcoder' Serbinenko]

Le ven. 6 mars 2020 à 12:38, Javier Martinez Canillas 
a écrit :

> Hello Vladimir,
>
> Thanks a lot for your feedback.
>
> On 3/5/20 3:22 PM, Vladimir 'phcoder' Serbinenko wrote:
> > Please evaluate size increase for this. In the past passing file and line
> > number to grub_dprintf was a huge source of increased Kern and core size
> >
> > Le mer. 4 mars 2020 à 13:01, Javier Martinez Canillas <
> javi...@redhat.com>
> > a écrit :
> >
>
> For a x86_64-pc build with platform=pc on master with and without the
> patch:
>
> $ find -regex '.*\(mod\|img\|exec\)$' -exec du -c {} + | grep total$
> 2740total
>
> $ find -regex '.*\(mod\|img\|exec\)$' -exec du -c {} + | grep total$
> 2756total
>
> so in this case the increase is a 0.6% in size.
>
This is not the comparison we care about. We care about kernel.img size and
core.img size for several common configs. And in bytes.
We have only 31K in mbr gap and every byte counts
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

[Vladimir 'phcoder' Serbinenko]

Le ven. 6 mars 2020 à 13:43, Daniel Kiper  a écrit :

> Re-adding grub-devel@gnu.org...
>
> On Fri, Mar 06, 2020 at 12:44:05PM +0100, Vladimir 'phcoder' Serbinenko
> wrote:
> > Le ven. 6 mars 2020 à 12:42, Daniel Kiper  a écrit
> :
> >
> > > On Thu, Mar 05, 2020 at 03:22:31PM +0100, Vladimir 'phcoder' Serbinenko
> > > wrote:
> > > > Please evaluate size increase for this. In the past passing file and
> line
> > > > number to grub_dprintf was a huge source of increased Kern and core
> size
> > >
> > > I think it does not matter much if we stop pretending that we support
> > > small MBR gaps right now [1]. Does it?
> > >
> > There are still a lot of installations that used older tools and never
> > reformatted. We still care about them
>
> If we go that way then we have to care about them by the end of the
> universe. And this means more and more issues like [1]. If somebody
> wants to use new GRUB then he/she have to reinstall the machine or
> something like that. IMO we should not care about users who do not want
> upgrade their machines or whatnot. Or at least their choices cannot
> impact GRUB development too much. And I think that this MBR constraint
> is hindering the project too much at this point. So, as above...
>
> Sorry for being blunt...
>
It does not. Core doesn't need to have everything and the kitchen sink.
It's small by design.

>
> Daniel
>
> [1] https://lists.gnu.org/archive/html/grub-devel/2019-11/msg00025.html
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

[Vladimir 'phcoder' Serbinenko]

On Fri, Mar 6, 2020 at 3:02 PM Didier Spaier  wrote:
>
> Le 06/03/2020 à 13:43, Daniel Kiper a écrit :
> > If we go that way then we have to care about them by the end of the
> > universe. And this means more and more issues like [1]. If somebody
> > wants to use new GRUB then he/she have to reinstall the machine or
> > something like that. IMO we should not care about users who do not want
> > upgrade their machines or whatnot. Or at least their choices cannot
> > impact GRUB development too much. And I think that this MBR constraint
> > is hindering the project too much at this point. So, as above...
> >
> > Sorry for being blunt...
>
> Sorry to be off topic... In case of a GUID partition table, if I
> understand the UEFI specification[1], as the first usable LBA should be
> greater than or equal to 34 for a 512 bytes block size or 6 for a
> 4096 bytes logical block size, it could begin after a gap of 24K.
>
> Then, if we assume that the first partition begins @ 1MiB, can't GRUB
> use the space unused between the first usable LBA and 1MiB instead of
> a Bios Boot partition, in case of "legacy" booting and a GPT? I ask as
> then a Bios Boot partition wouldn't be necessary any more.
What's the problem with having bios boot partition? You can put it
exactly in the spot you describe (in fact I do so) and it works as
well, just additionally it marks this space as occupied
>
> I just hope this question is not too silly...
>
> Best regards,
> Didier
>
>
> [1]https://uefi.org/sites/default/files/resources/UEFI_Spec_2_8_A_Feb14.pdf
> § 5.3.1 GPT overview
>
> ___________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

[Vladimir 'phcoder' Serbinenko]

> It would be best to use a boot partition so the core.img space is
> reserved by the GPT, but I once wanted to tack i386-pc GRUB onto an
> existing UEFI GPT disk, so I wrote the commands that do what you're
> describing here:
>
> https://github.com/dm0-/installer/blob/master/examples/systems/fitpc.sh#L151-L178
>
Just use gdisk to add a partition into this 1MiB hole.  you only need
to ask it to reduce alignment.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Warn if MBR gap is small and user uses advanced modules

[Vladimir 'phcoder' Serbinenko]

;hfsplus", "hfspluscomp",
+ "iso9660", "jfs", "minix", "minix2", "minix3", "minix_be",
+ "minix2_be", "minix2_be", "nilfs2", "ntfs", "ntfscomp",
+ "reiserfs", "romfs", "sfs", "squash4", "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 8970b73aa..ba27657d9 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1718,10 +1718,14 @@ main (int argc, char *argv[])
  install_device);

  /*  Now perform the installation.  */
- if (install_bootsector)
+ if (install_bootsector) {
+   int warn_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,
+ warn_short_mbr_gap);
+ }
  break;
   }
 case GRUB_INSTALL_PLATFORM_SPARC64_IEEE1275:
@@ -1748,7 +1752,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..da5f2c07f 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)
 {
   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

Re: [PATCH] Warn if MBR gap is small and user uses advanced modules

[Vladimir 'phcoder' Serbinenko]

On Wed, Mar 11, 2020, 18:08 Didier Spaier  wrote:

>
>
> Le 11/03/2020 à 13:43, Daniel Kiper a écrit :
> > Adding Michael, Mihai, Javier and Peter...
> >
> > Below you can find what more or less Vladimir and I agreed WRT small MBR
> > gap. In general Vladimir convinced me to phase out small MBR gaps
> > support gradually. This is first step in this journey. We think that we
> > have to build some warnings into the code and extend documentation.
> > Please chime in what you think about that...
> >
> > On Tue, Mar 10, 2020 at 01:23:10PM +0100, Vladimir 'phcoder' Serbinenko
> wrote:
> >> Daniel, do you want to adjust the whitelist?
> >>
> >> We don't want to support small MBR gap in pair with anything but
> >> the simplest config of biosdisk+part_msdos+simple filesystem. In this
> >> path "simple filesystems" are all current filesystems except zfs and
> >> btrfs
> >
> > Missing SOB...
> >
> >> ---
> >>  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 | 10 +++---
> >>  util/grub-setup.c   |  2 +-
> >>  util/setup.c|  5 +++--
> >>  8 files changed, 56 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
> >> index 103f6796f..25a5a1934 100644
> >> --- a/grub-core/partmap/gpt.c
> >> +++ b/grub-core/partmap/gpt.c
> >> @@ -25,6 +25,9 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#ifdef GRUB_UTIL
> >> +#include 
> >> +#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) {
> >
> > Could you define constant somewhere?
> >
> > Is it in sectors? Why 450? Should not it be 2048 if 1 MiB below?
>
> Whichs lead to a question: would the slot between 24K (or maybe
> 32K or 64K if it is wise to round it?) and 1M still a good fit for
> a Bios boot partition in case of a gpt? if not in which cases?
>

BIOS boot partition should never be under 960K. Just it was never a problem
to begin with.

>
> > ...and missing "&& warn_short" check...
> >
> >> +grub_util_warn("Your BIOS Boot Partition is under 1 MiB, please
> >> increase its size.");
> >> +  }
> >>if (ctx.len < *nsectors)
> >
> > Could not we use this check somehow instead of adding new one?
> >
> >>  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 7b8e45076..402bce050 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");
> >
> > Ditto?
> >
> >

Re: [PATCH] efi: add missed space in GRUB_EFI_GLOBAL_VARIABLE_GUID

[Vladimir 'phcoder' Serbinenko]

Could you explain why we would need this patch? It changes nothing AFAICT

On Fri, Mar 27, 2020, 12:01 Flavio Suligoi  wrote:

> Signed-off-by: Flavio Suligoi 
> ---
>  include/grub/efi/api.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
> index d6f4de1..937058d 100644
> --- a/include/grub/efi/api.h
> +++ b/include/grub/efi/api.h
> @@ -1295,7 +1295,7 @@ struct grub_efi_runtime_services
>(*convert_pointer) (grub_efi_uintn_t debug_disposition, void **address);
>
>  #define GRUB_EFI_GLOBAL_VARIABLE_GUID \
> -  { 0x8BE4DF61, 0x93CA, 0x11d2, { 0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03,
> 0x2B,0x8C }}
> +  { 0x8BE4DF61, 0x93CA, 0x11d2, { 0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03,
> 0x2B, 0x8C }}
>
>
>grub_efi_status_t
> --
> 2.7.4
>
>
> ___
> 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

[PATCH] mips/cache: Add missing nop's in delay slots

[Vladimir 'phcoder' Serbinenko]

Lack of them causes random instructions to be  executed before the
jump really happens.

Signed-off-by: Vladimir Serbinenko 
---
 grub-core/kern/mips/cache.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/grub-core/kern/mips/cache.S b/grub-core/kern/mips/cache.S
index fa6897e14..fa331eca1 100644
--- a/grub-core/kern/mips/cache.S
+++ b/grub-core/kern/mips/cache.S
@@ -7,6 +7,7 @@
 FUNCTION (grub_arch_sync_caches)
 #include "cache_flush.S"
  j $ra
+ nop

 FUNCTION (grub_arch_sync_dma_caches)
  move $t2, $a0
@@ -66,3 +67,4 @@ FUNCTION (grub_arch_sync_dma_caches)
  sync_op

  jr $ra
+ nop
-- 
2.20.1


-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Warn if MBR gap is small and user uses advanced modules

[Vladimir 'phcoder' Serbinenko]

On Wed, Mar 11, 2020 at 2:34 PM Daniel Kiper  wrote:
>
> Adding Michael, Mihai, Javier and Peter...
>
> Below you can find what more or less Vladimir and I agreed WRT small MBR
> gap. In general Vladimir convinced me to phase out small MBR gaps
> support gradually. This is first step in this journey. We think that we
> have to build some warnings into the code and extend documentation.
> Please chime in what you think about that...
>
> On Tue, Mar 10, 2020 at 01:23:10PM +0100, Vladimir 'phcoder' Serbinenko wrote:
> > Daniel, do you want to adjust the whitelist?
> >
> > We don't want to support small MBR gap in pair with anything but
> > the simplest config of biosdisk+part_msdos+simple filesystem. In this
> > path "simple filesystems" are all current filesystems except zfs and
> > btrfs
>
> Missing SOB...
>
> > ---
> >  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 | 10 +++---
> >  util/grub-setup.c   |  2 +-
> >  util/setup.c|  5 +++--
> >  8 files changed, 56 insertions(+), 11 deletions(-)
> >
> > diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
> > index 103f6796f..25a5a1934 100644
> > --- a/grub-core/partmap/gpt.c
> > +++ b/grub-core/partmap/gpt.c
> > @@ -25,6 +25,9 @@
> >  #include 
> >  #include 
> >  #include 
> > +#ifdef GRUB_UTIL
> > +#include 
> > +#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) {
>
> Could you define constant somewhere?
>
> Is it in sectors? Why 450? Should not it be 2048 if 1 MiB below?
Increased to 1900. So slighlty under 1MiB is accepted but not much
smaller. Made it a const
>
> ...and missing "&& warn_short" check...
It's intentional. On GPT we don't want BBP under 1MiB even if setup is simple.
>
> > +grub_util_warn("Your BIOS Boot Partition is under 1 MiB, please
> > increase its size.");
> > +  }
> >if (ctx.len < *nsectors)
>
> Could not we use this check somehow instead of adding new one?
No, it checks for different thing.
>
> >  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 7b8e45076..402bce050 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");
>
> Ditto?
Changed
>
> > +  }
> > +
> >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,
> >

[PATCH v2] Warn if MBR gap is small and user uses advanced modules

[Vladimir 'phcoder' Serbinenko]

io_be", "newc", "odc", "ext2", "fat", "exfat",
+ "f2fs", "fshelp", "hfs", "hfsplus", "hfspluscomp",
+ "iso9660", "jfs", "minix", "minix2", "minix3", "minix_be",
+ "minix2_be", "minix2_be", "nilfs2", "ntfs", "ntfscomp",
+ "reiserfs", "romfs", "sfs", "squash4", "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 8970b73aa..6db06c99b 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1718,10 +1718,14 @@ main (int argc, char *argv[])
  install_device);

  /*  Now perform the installation.  */
- if (install_bootsector)
+ if (install_bootsector) {
+   int warn_short_mbr_gap = grub_install_is_short_mbrgap_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,
+ warn_short_mbr_gap);
+ }
  break;
   }
 case GRUB_INSTALL_PLATFORM_SPARC64_IEEE1275:
@@ -1748,7 +1752,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..da5f2c07f 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)
 {
   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

Re: [PATCH v2] Warn if MBR gap is small and user uses advanced modules

[Vladimir 'phcoder' Serbinenko]

On Tue, May 12, 2020 at 1:48 PM Daniel Kiper  wrote:
>
> On Mon, Apr 27, 2020 at 05:51:34PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> > We don't want to support small MBR gap in pair with anything but
> > the simplest config of biosdisk+part_msdos+simple filesystem. In this
> > path "simple filesystems" are all current filesystems except zfs and
> > btrfs.
>
> The patch LGTM. If there are no objections I will take it in a week or so.
>
> Additionally, I hope that Vladimir will be able to prepare a doc patch
> describing the issue and why we introduced these warnings.
>
> Daniel
>
> > ---
> >  grub-core/partmap/gpt.c |  9 -
> >  grub-core/partmap/msdos.c   |  7 ++-
> >  include/grub/partition.h|  4 +++-
> >  include/grub/util/install.h |  7 +--
> >  util/grub-install-common.c  | 24 
> >  util/grub-install.c | 10 +++---
> >  util/grub-setup.c   |  2 +-
> >  util/setup.c|  5 +++--
> >  8 files changed, 57 insertions(+), 11 deletions(-)
> >
> > diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
> > index 103f6796f..6b979f7c3 100644
> > --- a/grub-core/partmap/gpt.c
> > +++ b/grub-core/partmap/gpt.c
> > @@ -25,6 +25,9 @@
> >  #include 
> >  #include 
> >  #include 
> > +#ifdef GRUB_UTIL
> > +#include 
> > +#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 < GRUB_MIN_RECOMMENDED_MBRGAP) {
> > +grub_util_warn("Your BIOS Boot Partition is under 1 MiB, please
> > increase its size.");
> > +  }
> >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 7b8e45076..400d7521d 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 < GRUB_MIN_RECOMMENDED_MBRGAP && warn_short) {
> > +grub_util_warn("You have a short MBR gap and use advanced config.
> > Please increase post-MBR gap");
> > +  }
> > +
> >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..adc50d680 100644
> > --- a/include/grub/partition.h
> > +++ b/include/grub/partition.h
> > @@ -52,10 +52,12 @@ struct grub_partition_map
> >   grub_partition_iterate_hook_t hook, void *hook_data);
> >  #ifdef GRUB_UTIL
> >/* Determine sectors available for embedding.  */
> > +#define GRUB_MIN_RECOMMENDED_MBRGAP 1900
> >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..640c71d28 100644
> > --- a/include/grub/util/install.h
> > +++ b/include/grub/util/install.h
> > @@ -193,13

Re: [PATCH v2] Warn if MBR gap is small and user uses advanced modules

[Vladimir 'phcoder' Serbinenko]

Signed-off-by: Bladimir Serbinenko 

On Fri, May 29, 2020 at 3:09 PM Vladimir 'phcoder' Serbinenko
 wrote:
>
> On Tue, May 12, 2020 at 1:48 PM Daniel Kiper  wrote:
> >
> > On Mon, Apr 27, 2020 at 05:51:34PM +0200, Vladimir 'phcoder' Serbinenko 
> > wrote:
> > > We don't want to support small MBR gap in pair with anything but
> > > the simplest config of biosdisk+part_msdos+simple filesystem. In this
> > > path "simple filesystems" are all current filesystems except zfs and
> > > btrfs.
> >
> > The patch LGTM. If there are no objections I will take it in a week or so.
> >
> > Additionally, I hope that Vladimir will be able to prepare a doc patch
> > describing the issue and why we introduced these warnings.
> >
> > Daniel
> >
> > > ---
> > >  grub-core/partmap/gpt.c |  9 -
> > >  grub-core/partmap/msdos.c   |  7 ++-
> > >  include/grub/partition.h|  4 +++-
> > >  include/grub/util/install.h |  7 +--
> > >  util/grub-install-common.c  | 24 
> > >  util/grub-install.c | 10 +++---
> > >  util/grub-setup.c   |  2 +-
> > >  util/setup.c|  5 +++--
> > >  8 files changed, 57 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
> > > index 103f6796f..6b979f7c3 100644
> > > --- a/grub-core/partmap/gpt.c
> > > +++ b/grub-core/partmap/gpt.c
> > > @@ -25,6 +25,9 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#ifdef GRUB_UTIL
> > > +#include 
> > > +#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 < GRUB_MIN_RECOMMENDED_MBRGAP) {
> > > +grub_util_warn("Your BIOS Boot Partition is under 1 MiB, please
> > > increase its size.");
> > > +  }
> > >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 7b8e45076..400d7521d 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 < GRUB_MIN_RECOMMENDED_MBRGAP && warn_short) {
> > > +grub_util_warn("You have a short MBR gap and use advanced config.
> > > Please increase post-MBR gap");
> > > +  }
> > > +
> > >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..adc50d680 100644
> > > --- a/include/grub/partition.h
> > > +++ b/include/grub/partition.h
> > > @@ -52,10 +52,12 @@ struct grub_partition_map
> > >   grub_partition_iterate_hook_t hook, void *hook_data);
> > >  #ifdef GRUB_UTIL
> > >/* Determine sectors available for embedding.  */
> > > +#define GRUB_MIN_RECOMMENDED_MBRGAP 1900
> >

Re: [PATCH] disk/loopback: Don't verify loopback images

[Vladimir 'phcoder' Serbinenko]

On Mon, Jun 1, 2020, 15:21 Chris Coulson 
wrote:

> When a file is verified, the entire contents of the verified file are
> loaded in to memory and retained until the file handle is closed. A
> consequence of this is that opening a loopback image can incur a
> significant memory cost.
>
> As loopback devices are just another disk implementation, don't treat
> loopback images any differently to physical disk images, and skip
> verification of them. Files opened from the filesystem within a loopback
> image will still be passed to verifier modules where required.
>
> Signed-off-by: Chris Coulson 
> ---
>  grub-core/disk/loopback.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/disk/loopback.c b/grub-core/disk/loopback.c
> index cdf9123fa..01267e577 100644
> --- a/grub-core/disk/loopback.c
> +++ b/grub-core/disk/loopback.c
> @@ -93,7 +93,8 @@ grub_cmd_loopback (grub_extcmd_context_t ctxt, int argc,
> char **args)
>  return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
>
>file = grub_file_open (args[1], GRUB_FILE_TYPE_LOOPBACK
> -| GRUB_FILE_TYPE_NO_DECOMPRESS);
> +| GRUB_FILE_TYPE_NO_DECOMPRESS |
> +GRUB_FILE_TYPE_SKIP_SIGNATURE);
>
Maybe the verifier should rather decide to skip verification if fuller type
is loopback?

>if (! file)
>  return grub_errno;
>
> --
> 2.25.1
>
>
> ___
> 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

Re: [PATCH] tests: remove cmdline_cat and gfxterm_menu tests

[Vladimir 'phcoder' Serbinenko]

On Tue, Jun 2, 2020, 10:40 Daniel Axtens  wrote:

> cmdline_cat takes a checksum of a screen that includes the grub
> version number, so it has been broken since commit 94d9926a6640
> ("Release 2.04~rc1"). That was in April 2019.
>
> I have not been able to get the gfxterm_menu tests to pass under
> any configuration.
>
> These tests aren't providing any extra information -


Run them under grub-emu and you will find all the images that you can then
check

> because they
> always fail we don't learn anything from them. They are also stopping
> 'make check' from ever passing, which means we are less likely to find
> out if anything else has broken. It also disincentives people from
> writing more tests.
>
> We could regenerate the checksums, but I don't know if the gfxterm_menu
> ones are correct - the Arabic images don't have the letters joining
> correctly, so I'm a bit suspicious!. It also puts us in the position
> where the tests could be broken again by just bumping the version number.
> Instead, remove both tests.
>
> With this, 'make check' can pass (at least on my laptop) in a number
> of configurations, including x86_64-emu and i386-pc.
>
> Signed-off-by: Daniel Axtens 
>
> ---
>
> I had a crack at getting this into Travis CI, but I had a lot of
> trouble with the existing boot test. Then I realised that there was
> a whole series of Travis improvements that never got merged. Maybe
> it would be worth reviving that at some point but I have burned up
> all the time I can afford to dedicate to Travis for now (and more!!)
> ---
>  grub-core/Makefile.core.def   |  10 --
>  grub-core/tests/checksums.h   |  98 --
>  grub-core/tests/cmdline_cat_test.c| 125 --
>  grub-core/tests/gfxterm_menu.c| 180 --
>  grub-core/tests/lib/functional_test.c |   2 -
>  5 files changed, 415 deletions(-)
>  delete mode 100644 grub-core/tests/cmdline_cat_test.c
>  delete mode 100644 grub-core/tests/gfxterm_menu.c
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 166b444c76be..eb57a5af08a7 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -2197,16 +2197,6 @@ module = {
>common = tests/videotest_checksum.c;
>  };
>
> -module = {
> -  name = gfxterm_menu;
> -  common = tests/gfxterm_menu.c;
> -};
> -
> -module = {
> -  name = cmdline_cat_test;
> -  common = tests/cmdline_cat_test.c;
> -};
> -
>  module = {
>name = bitmap;
>common = video/bitmap.c;
> diff --git a/grub-core/tests/checksums.h b/grub-core/tests/checksums.h
> index 8273bd105dee..1d9f68ce948a 100644
> --- a/grub-core/tests/checksums.h
> +++ b/grub-core/tests/checksums.h
> @@ -1,101 +1,3 @@
> -  { "cmdline_cat", 640, 480, 0x2, 16, 8, 1, 0, 0, 0, 0, 0, 0, 0, 0 /*
> 640x480xi16 */, (grub_uint32_t []) { 0x59c36f00, 0xc69be699, 0xc69be699,
> 0xe17fc97c, 0xe17fc97c, 0xc2512486, 0xc2512486, 0x20260761, 0x20260761,
> 0xe3485349, 0xe3485349, 0x6020b4d9, 0x6020b4d9, 0x1605db98, 0x1605db98,
> 0xdca51b58, 0xdca51b58, 0xf3d66441, 0xf3d66441, 0x18cb7ce2, 0x18cb7ce2,
> 0x23b616d9, 0x23b616d9, 0xa1bf5dbf, 0xa1bf5dbf, 0x5e4a8b3d, 0x5e4a8b3d,
> 0x9933b3d8, 0x9933b3d8, 0xce7be2de, 0xce7be2de, 0x62531729, 0x62531729,
> 0xc39d3788, 0xc39d3788, 0xf7aee2fe, 0xf7aee2fe, 0xc66f4903, 0xc66f4903,
> 0x536c922f, 0x536c922f, 0xcb231652, 0x4ae07b67, 0x146816d5, 0x146816d5, },
> 45 },
> -  { "cmdline_cat", 800, 600, 0x2, 16, 8, 1, 0, 0, 0, 0, 0, 0, 0, 0 /*
> 800x600xi16 */, (grub_uint32_t []) { 0xaa4593fe, 0x29d0cfb7, 0x29d0cfb7,
> 0x4d57ad88, 0x4d57ad88, 0xaf7591cb, 0xaf7591cb, 0xb88e3c0, 0xb88e3c0,
> 0x5d55a031, 0x5d55a031, 0x4363bcff, 0x4363bcff, 0xe1142833, 0xe1142833,
> 0x9578c80d, 0x9578c80d, 0xff1f5174, 0xff1f5174, 0x6828d47, 0x6828d47,
> 0x2a849ae3, 0x2a849ae3, 0x27a811d9, 0x27a811d9, 0x6c42e1cb, 0x6c42e1cb,
> 0x5bba61f5, 0x5bba61f5, 0xc02ab705, 0xc02ab705, 0xeb3e0e05, 0xeb3e0e05,
> 0x6f4ce3c, 0x6f4ce3c, 0x2075c50e, 0x2075c50e, 0x54ac7fba, 0x54ac7fba,
> 0x335d1c21, 0x335d1c21, 0xf923fbd1, 0x1b489d4d, 0x1253eb5e, 0x1253eb5e, },
> 45 },
> -  { "cmdline_cat", 1024, 768, 0x2, 16, 8, 1, 0, 0, 0, 0, 0, 0, 0, 0 /*
> 1024x768xi16 */, (grub_uint32_t []) { 0xc9cbf769, 0x6ff92a98, 0x6ff92a98,
> 0xbd8264b3, 0xbd8264b3, 0x5c50282c, 0x5c50282c, 0xd18d6a5a, 0xd18d6a5a,
> 0x99cea139, 0x99cea139, 0x2590c545, 0x2590c545, 0x55c86045, 0x55c86045,
> 0xad53ef1e, 0xad53ef1e, 0x5b44a23a, 0x5b44a23a, 0xa255f863, 0xa255f863,
> 0x85b0bdca, 0x85b0bdca, 0x4220ee1f, 0x4220ee1f, 0xeb991a9f, 0xeb991a9f,
> 0x86f366dd, 0x86f366dd, 0xfa176b7d, 0xfa176b7d, 0xd7384ca9, 0xd7384ca9,
> 0xceebf5c1, 0xceebf5c1, 0xae62b268, 0xae62b268, 0x6e89bbd6, 0x6e89bbd6,
> 0x3fe07c8e, 0x3fe07c8e, 0xee5358fe, 0x162d1a8a, 0x876f8d10, 0x876f8d10, },
> 45 },
> -  { "cmdline_cat", 640, 480, 0x1, 256, 32, 4, 16, 8, 8, 8, 0, 8, 24, 8 /*
> 640x480xrgba */, (grub_uint32_t []) { 0x9813a416, 0xb96afaea,
> 0xb96afaea, 0xaa16ab1e, 0xaa16ab1e, 0x51ff382, 0x51ff382, 0xec74f127,
> 0xec74f127, 0xde4bca

Re: [PATCH] disk/loopback: Don't verify loopback images

[Vladimir 'phcoder' Serbinenko]

On Tue, Jun 2, 2020, 13:21 Dimitri John Ledkov  wrote:

> On Tue, 2 Jun 2020 at 12:12, Vladimir 'phcoder' Serbinenko
>  wrote:
> >
> >
> >
> > On Mon, Jun 1, 2020, 15:21 Chris Coulson 
> wrote:
> >>
> >> When a file is verified, the entire contents of the verified file are
> >> loaded in to memory and retained until the file handle is closed. A
> >> consequence of this is that opening a loopback image can incur a
> >> significant memory cost.
> >>
> >> As loopback devices are just another disk implementation, don't treat
> >> loopback images any differently to physical disk images, and skip
> >> verification of them. Files opened from the filesystem within a loopback
> >> image will still be passed to verifier modules where required.
> >>
> >> Signed-off-by: Chris Coulson 
> >> ---
> >>  grub-core/disk/loopback.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/grub-core/disk/loopback.c b/grub-core/disk/loopback.c
> >> index cdf9123fa..01267e577 100644
> >> --- a/grub-core/disk/loopback.c
> >> +++ b/grub-core/disk/loopback.c
> >> @@ -93,7 +93,8 @@ grub_cmd_loopback (grub_extcmd_context_t ctxt, int
> argc, char **args)
> >>  return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
> >>
> >>file = grub_file_open (args[1], GRUB_FILE_TYPE_LOOPBACK
> >> -| GRUB_FILE_TYPE_NO_DECOMPRESS);
> >> +| GRUB_FILE_TYPE_NO_DECOMPRESS |
> >> +GRUB_FILE_TYPE_SKIP_SIGNATURE);
> >
> > Maybe the verifier should rather decide to skip verification if fuller
> type is loopback?
>
> How would it be used then? For example, I imagine one can gpg sign the
> .iso or a .squashfs and then assume everything inside it is trusted
> (even if things inside are not signed).
> However, I don't believe any verifier works this way today, nor not
> sure it is desired versus signing individual things inside the
> loopback device.
>
> At the moment, without this patch, a lot of things break. It is common
> to loopback mount .iso which currently eats all the RAM and machines
> crash with out of memory. (I.e. loopback mounting 2.5GB .iso).
> Similarly it is common to use things like WUBI, where .raw image file
> is loopback mounted from NTFS. If one is doing secureboot and tpm is
> present that again results in out of memory.
>
> Taking the measurement / checksum / verifying the loopback device is
> not a problem, but keeping it all in RAM is. And imho trusting
> unsigned things inside verified loopback device is dubious too.
>
> So yeah, probably it's something that verifiers should be able to
> decide upon and perform intelligently. But at the moment, the only
> practical answer for all of them is to skip.
>
GPG one can read the file in chunks and then keep in memory only hash of
every chunk to prevent TOCTOU.
But then the question is when is our makes sense versus signing individual
files. I can see uses like signing squashfs.
We probably need a "secure device" flag somewhere long term

>
> --
> Regards,
>
> Dimitri.
>
> ___
> 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

Re: [RFC PATCH 3/3] efi: mm: implement runtime addition of pages

[Vladimir 'phcoder' Serbinenko]

On Mon, Jun 1, 2020, 21:34 Patrick Steinhardt  wrote:

> Adjust the interface of `grub_efi_mm_add_regions ()` to take a set of
> `GRUB_MM_REGION_*` flags, which most notably is currently only the
> `CONSECUTVE` flag. This allows us to set the function up as callback for
> the memory subsystem and have it call out to us in case there's not
> enough pages available in the current heap.
>
> Signed-off-by: Patrick Steinhardt 
> ---
>  grub-core/kern/efi/mm.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
> index 76f0f6a0f..092097355 100644
> --- a/grub-core/kern/efi/mm.c
> +++ b/grub-core/kern/efi/mm.c
> @@ -468,7 +468,8 @@ static void
>  add_memory_regions (grub_efi_memory_descriptor_t *memory_map,
> grub_efi_uintn_t desc_size,
> grub_efi_memory_descriptor_t *memory_map_end,
> -   grub_efi_uint64_t required_pages)
> +   grub_efi_uint64_t required_pages,
> +   char consecutive)
>
Please don't use char for anything other than strings. It has unfortunate
design decisions. Currently we use int as bool. It's something we might
want to revisit in the future but until then, let's be consistent

>  {
>grub_efi_memory_descriptor_t *desc;
>
> @@ -482,6 +483,10 @@ add_memory_regions (grub_efi_memory_descriptor_t
> *memory_map,
>
>start = desc->physical_start;
>pages = desc->num_pages;
> +
> +  if (pages < required_pages && consecutive)
> +   continue;
> +
>if (pages > required_pages)
> {
>   start += PAGES_TO_BYTES (pages - required_pages);
> @@ -543,7 +548,7 @@ print_memory_map (grub_efi_memory_descriptor_t
> *memory_map,
>  #endif
>
>  static grub_err_t
> -grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
> +grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes, unsigned flags)
>  {
>grub_efi_memory_descriptor_t *memory_map;
>grub_efi_memory_descriptor_t *memory_map_end;
> @@ -597,7 +602,8 @@ grub_efi_mm_add_regions (grub_efi_uint64_t
> required_bytes)
>/* Allocate memory regions for GRUB's memory management.  */
>add_memory_regions (filtered_memory_map, desc_size,
>   filtered_memory_map_end,
> - BYTES_TO_PAGES (required_bytes));
> + BYTES_TO_PAGES (required_bytes),
> + flags & GRUB_MM_REGION_CONSECUTIVE);
>
>  #if 0
>/* For debug.  */
> @@ -622,8 +628,9 @@ grub_efi_mm_add_regions (grub_efi_uint64_t
> required_bytes)
>  void
>  grub_efi_mm_init (void)
>  {
> -  if (grub_efi_mm_add_regions (DEFAULT_HEAP_SIZE) != GRUB_ERR_NONE)
> +  if (grub_efi_mm_add_regions (DEFAULT_HEAP_SIZE, 0) != GRUB_ERR_NONE)
>  grub_fatal (grub_errmsg);
> +  grub_mm_region_fn = grub_efi_mm_add_regions;
>  }
>
>  #if defined (__aarch64__) || defined (__arm__) || defined (__riscv)
> --
> 2.26.2
>
> ___
> 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

[PATCH 0/3] cmdline fixes for acpi_osi

[Vladimir 'phcoder' Serbinenko]

This patch series introduces an escaping of spaces that is compatible
with linux parameter parsing. Also it adds some needed tests for this
part of code

-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH 1/3] Put quote after equal sign when it makes sense

[Vladimir 'phcoder' Serbinenko]

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-Put-quote-after-equal-sign-when-it-makes-sense.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH 2/3] cmdline: Make arguments const

[Vladimir 'phcoder' Serbinenko]

-- 
Regards
Vladimir 'phcoder' Serbinenko


0002-cmdline-Make-arguments-const.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH 3/3] Add tests for cmdline module

[Vladimir 'phcoder' Serbinenko]

-- 
Regards
Vladimir 'phcoder' Serbinenko


0003-Add-tests-for-cmdline-module.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Re: [PATCH 1/1] grub-menulst2cfg.c

[Vladimir 'phcoder' Serbinenko]

On Sat, Jul 11, 2020, 13:01 Vladimir 'phcoder' Serbinenko 
wrote:

>
>
> On Fri, Jul 10, 2020, 11:19 朱光宏  wrote:
>
>>
>> In many cases, entryname is null, so there is no need to call this
>> function often, it is faster to do this, what do you think?
>>
> Is it a tight loop where such an optimisation matters? Did you measure
> performance gain? If not, it's better to go for existing code conventions
> and readadble code
>
>>
>> free already checks for nullness of its argument. Any reason for this
>> patch?
>>
>>
>> On Thu, Jul 9, 2020, 15:11 GuangHong Zhu 
>> wrote:
>>
>>> ---
>>>  util/grub-menulst2cfg.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/util/grub-menulst2cfg.c b/util/grub-menulst2cfg.c
>>> index a39f86939..6f7587347 100644
>>> --- a/util/grub-menulst2cfg.c
>>> +++ b/util/grub-menulst2cfg.c
>>> @@ -124,7 +124,9 @@ main (int argc, char **argv)
>>>
>>>free (buf);
>>>free (suffix);
>>> -  free (entryname);
>>> +
>>> +  if(entryname)
>>> +free (entryname);
>>>
>>>if (in != stdin)
>>>  fclose (in);
>>> --
>>> 2.20.1
>>>
>>>
>>>
>>>
>>>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel