Re: configure options when building from source

[Andrei Borzenkov]

14.05.2017 20:54, Beeblebrox пишет:
> Hello, building Grub from source on FreeBSD with
> ./autogen.sh --disable-efiemu --disable-nls
> ./configure  --disable-efiemu --disable-nls
> 
> The resulting config is below
> GRUB2 will be compiled with following components:
> Platform: i386-pc
> With devmapper support: No (need libdevmapper header)
> With memory debugging: No
> With disk cache statistics: No
> With boot time statistics: No
> efiemu runtime: No
> grub-mkfont: Yes
> grub-mount: No (need FUSE library)
> starfield theme: No (No DejaVu found)
> With libzfs support: Yes
> Build-time grub-mkfont: No (no fonts)
> Without unifont (no build-time grub-mkfont)
> With liblzma from -llzma (support for XZ-compressed mips images)
> 
> I was unable to find documentation regarding config, so How can I:
> 
> * Enable grub-mount, grub-mkfont, unifont ?

grub-mount requires fuse development (as is quite clearly written in
summary). Build time grub-mkfont is disabled because dejavu font was not
found (could be more clear here what is missing).

> All requirements (fuse, dejavu, fonts) are on the system. Is there a flag I 
> must use? Most probably the path searched by Grub is different than on Linux. 
> Where should I correct those?
> 

in configure.ac

> * For starfield do I need to download a sepaarate repo?
> 

What was not clear in "no DejaVu found"?

> * I install Grub as "gmake install DESTDIR=/usr/local/opt"
> But trying for example "# grub-kbdcomp" I get:
> -> cannot open /usr/local/share/grub/grub-mkconfig_lib: No such file or 
> directory
> 

Paths are hardcoded during build. Installing in different directory does
not change them. This is more intended for packaging use.

> * I need to generate mknetdir for pxe with UEFI option as well. Above config 
> does not seem to have UEFI capability enabled - am I wrong?
> 

You need to build grub for corresponding platform. You have built it for
i386-pc only.


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

Re: GRUB 2.02 release

[Andrei Borzenkov]

26.04.2017 14:27, Vladimir 'phcoder' Serbinenko пишет:
> Hello everyone. GRUB maintainers team is proud to announce GRUB 2.02 that
> we have just released. This release contains fixes for bugs uncovered in
> 2.02~rc2 and updated translations.
> 
> Full list of new features since release 2.00 is available in NEWS file.
> This includes improvements to GUI system originally introduced several
> versions ago. You just need to enable gfxmenu. We also introduced new
> platforms like arm, arm64, Xen and improved coreboot support.
> 
> We would like to thank all the people who have contributed to the project.
> 
> The tarball is available at http://ftp.gnu.org/gnu/grub/grub-2.02.tar.xz
> and signature at http://ftp.gnu.org/gnu/grub/grub-2.02.tar.xz.sig
> 
> Release is signed with the following fingerprint:
>  E53D 497F 3FA4 2AD8 C9B4  D1E8 35A9 3B74 E82E 4209
> 
> It's also available as a signed tag grub-2.02 in official git repository.
> 
> If you don't have xz support alternatively you may consider files
> http://ftp.gnu.org/gnu/grub/grub-2.02.tar.gz and signature at
> http://ftp.gnu.org/gnu/grub/grub-2.02.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://ftp.gnu.org/gnu/grub/grub-2.02-for-windows.zip
> and signature at
> http://ftp.gnu.org/gnu/grub/grub-2.02-for-windows.zip.sig
> 
> 

I updated documentation on https://www.gnu.org/software/grub/manual/



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: grub-install deleting long UEFI entries bug ?

[Andrei Borzenkov]

23.04.2017 23:33, adrian15 пишет:
> El 23/04/17 a las 10:45, Andrei Borzenkov escribió:
>> 23.04.2017 11:21, adrian15 adrian15 пишет:
>>> 2017-04-23 6:36 GMT+02:00 Andrei Borzenkov <arvidj...@gmail.com>:
>>>
>>>> 23.04.2017 03:54, adrian15 пишет:
>>>>> grub-install seems to be deleting long UEFI entries
>>>>>
>>>>> (*) What the bug is
>>>>>
>>>>> * Add an UEFI entry with this label (Remove the single quotes):
>>>>>  '(Rescapp added) \EFI\ubuntu\MokManager.efi'
>>>>>
>>>>> Example:
>>>>>
>>>>> efibootmgr -c \
>>>>>  -d /dev/sda \
>>>>>  -p 2 \
>>>>>  -L '(Rescapp added) \EFI\ubuntu\MokManager.efi' \
>>>>>  -l '\EFI\ubuntu\MokManager.efi'
>>>>>
>>>>> * Run grub-install /dev/sda or maybe just grub-install
>>>>>
>>>>> I expect the newly added uefi entry to be there.
>>>>> What I find is that the entry has been lost or deleted!
>>>>>
>>>>
>>>> What is value of GRUB_DISTRIBUTOR in /etc/default/grub?
>>>>
>>>
>>> After evaluating the bash expression the GRUB_DISTRIBUTOR value is Ubuntu.
>>>
>>
>> Yes, historically grub did case insensitive substring search. This
>> probably is wrong, we should just take everything after boot number
>> literally.
> 
> I see, like removing what you are about to add I guess.
> The problem that I see is that efibootmgr output (even if --verbose
> switch) it's not machine readable.
> 

We are using plain "efibootmgr" without --verbose and output is variable
name followed by either '*' or space followed by space followed by
description if present. Unless description contains newline, it looks
straightforward to parse.

> I guess efibootmgr itself would need an specific switch in order to
> produce output suitable for scripts.

It would need somehow escape newline

> Another option is include some of
> the efibootmgr functionality/libraries into grub itself.
> 

Yes, I have half-done patch but as I see now it is probably not really
needed unless we actually have case of description containing newlines.

>>>
>>> 2) Then there's:
>>>
>>>   if (grub_memcmp (line, "Boot", sizeof ("Boot") - 1) != 0
>>>  || line[sizeof ("Boot") - 1] < '0'
>>>  || line[sizeof ("Boot") - 1] > '9')
>>> continue;
>>>
>>> which might be wrong because of 0 and 9 and maybe not because of the array
>>> indexes.
>>>
>>> Let's go into details about that.
>>>
>>> 2.1) Boot First entry
>>> BootA000 Second entry
>>>
>>> Shouldn't the look for A to F hexadecimal letters too?
>>>
>>
>> Yes. Patches are welcome for both problems. Second one is actually bug
>> fix so should be independent.
>>
>>> And...
> 
> Well, I think just checking 0 to 9 in the first character is a good
> compromise.
> 
> Some outputs have: BootCurrent . So 'BootC' can be found in e.g.
> 'BootC001' too. So that would be adding another problem because
> 'BootCurrent' would be considered as a right entry.
> 

This simply means that we have to check for exactly 4 hexadecimal
numbers, not shortcut this.


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

Re: grub2-common: grub.cfg gains wrong root settings for multi-OS system

[Andrei Borzenkov]

23.04.2017 08:43, Ralph Ronnquist пишет:
> For all installs, I made it mount /dev/sda1 at /boot but only the
> first install formatted it.

Shared /boot never worked reliably. With or without grub. Sorry.

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

Re: grub-install deleting long UEFI entries bug ?

[Andrei Borzenkov]

23.04.2017 11:21, adrian15 adrian15 пишет:
> 2017-04-23 6:36 GMT+02:00 Andrei Borzenkov <arvidj...@gmail.com>:
> 
>> 23.04.2017 03:54, adrian15 пишет:
>>> grub-install seems to be deleting long UEFI entries
>>>
>>> (*) What the bug is
>>>
>>> * Add an UEFI entry with this label (Remove the single quotes):
>>>  '(Rescapp added) \EFI\ubuntu\MokManager.efi'
>>>
>>> Example:
>>>
>>> efibootmgr -c \
>>>  -d /dev/sda \
>>>  -p 2 \
>>>  -L '(Rescapp added) \EFI\ubuntu\MokManager.efi' \
>>>  -l '\EFI\ubuntu\MokManager.efi'
>>>
>>> * Run grub-install /dev/sda or maybe just grub-install
>>>
>>> I expect the newly added uefi entry to be there.
>>> What I find is that the entry has been lost or deleted!
>>>
>>
>> What is value of GRUB_DISTRIBUTOR in /etc/default/grub?
>>
> 
> After evaluating the bash expression the GRUB_DISTRIBUTOR value is Ubuntu.
> 

Yes, historically grub did case insensitive substring search. This
probably is wrong, we should just take everything after boot number
literally.
...
> 1) First of all this matches all the line:
> 
> if (!strcasestr (line, efi_distributor))
> continue;
> 
> That means that if you add a custom label which matches the efi distributor
> then it gets removed. I think that's what happened to me. I would prefer
> something more precise that would check the complete efi file path agains
> e.g. EFI/vendor/ .
> 
> 2) Then there's:
> 
>   if (grub_memcmp (line, "Boot", sizeof ("Boot") - 1) != 0
>  || line[sizeof ("Boot") - 1] < '0'
>  || line[sizeof ("Boot") - 1] > '9')
> continue;
> 
> which might be wrong because of 0 and 9 and maybe not because of the array
> indexes.
> 
> Let's go into details about that.
> 
> 2.1) Boot First entry
> BootA000 Second entry
> 
> Shouldn't the look for A to F hexadecimal letters too?
> 

Yes. Patches are welcome for both problems. Second one is actually bug
fix so should be independent.

> And...
> 
> 2.2) line[sizeof ("Boot") - 1] < '0'
> 
> Am I doing it right?
> 
> sizeof ("Boot") = 4
> 

It is 5.


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

Re: grub-install deleting long UEFI entries bug ?

[Andrei Borzenkov]

23.04.2017 03:54, adrian15 пишет:
> grub-install seems to be deleting long UEFI entries
> 
> (*) What the bug is
> 
> * Add an UEFI entry with this label (Remove the single quotes):
>  '(Rescapp added) \EFI\ubuntu\MokManager.efi'
> 
> Example:
> 
> efibootmgr -c \
>  -d /dev/sda \
>  -p 2 \
>  -L '(Rescapp added) \EFI\ubuntu\MokManager.efi' \
>  -l '\EFI\ubuntu\MokManager.efi'
> 
> * Run grub-install /dev/sda or maybe just grub-install
> 
> I expect the newly added uefi entry to be there.
> What I find is that the entry has been lost or deleted!
> 

What is value of GRUB_DISTRIBUTOR in /etc/default/grub?
...
> 
> Maybe grub-install uses efibootmgr as an auxiliar tool and the problem
> is in Ubuntu's efibootmgr?
> 

Yes.

> It would be nice if someone could point us on where does grub-install
> handles the uefi boot entries so that we can take a deeper look into it.
> 

grub-core/osdep/unix/platform.c

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

Re: [PATCH] * grub-core/fs/udf.c: Add support for UUID

[Andrei Borzenkov]

19.04.2017 20:48, Pali Rohár пишет:
> Hi! I would like to remind this patch. It allows to use UUID of UDF
> partition in grub2. Linux tool blkid is already able to handle it, so it
> is possible to specify UUID of UDF partition in /etc/fstab.
> 
> UDF is filesystem used on optical disks (DVD), but also on hard disks as
> it is natively supported by Windows, Linux and Mac OS X kernels. Which
> is a good benefit for multi-boot environment.
> 
Yes, I wanted to review it, but did not have time. In any case we are in
complete freeze for next release, so it will have to wait until then.
Thank you for the patience.


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

Re: How to deal with `multiboot` and compressed files

[Andrei Borzenkov]

15.04.2017 19:47, Paul Menzel пишет:
> Dear GRUB folks,
> 
> 
> The payloads for coreboot are normally LZMA compressed, as there is
> only little room on a flash ROM chip. Vladimir adapted SeaBIOS so it’s
> Multiboot compatible, so that SeaBIOS and the VGA Option can be easily
> loaded from the hard disk, and not the flash ROM chip.
> 
> Trying to load such a compressed with the multiboot command¹, it does
> abort with the message, that the Multiboot header cannot be found.
> 
> Is the the multiboot command supposed to be able to deal with
> compressed files?
> 
> If not, could you please tell me, if there is a way to load such a
> compressed file, and how?
> 

Try

insmod xzio

before multiboot command.



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: `halt` doesn’t work on Lenovo T60 with coreboot and GRUB payload

[Andrei Borzenkov]

15.04.2017 19:32, Paul Menzel пишет:
> Dear GRUB folks,
> 
> 
> On a Lenovo T60 with coreboot and the GRUB payload, version 2.02-rc1,
> entering `halt` in the GRUB command line, nothing happens. The cursor
> goes one line below, and everything stays that way. After that, the
> system can only be powered off by pressing the power button for ten
> seconds.
> 
> Setting `debug=all` before that – `debug=halt` didn’t work – the last
> lines are below.
> 
> ```
> […]
> commands/acpihalt.c:107: data type = 0x12
> commands/acpihalt.c:241: Opcode 0x8
> commands/acpihalt.c:242: Tell 2dbd
> commands/acpihalt.c:107: data type = 0x12
> commands/acpihalt.c:241: Opcode 0x8
> commands/acpihalt.c:242: Tell 2dcd
> commands/acpihalt.c:269: S5 found
> commands/acpihalt.c:444: SLP_TYP = 7, port = 0x504
> ```
> 

So grub found how to power off system from ACPI table. Next it attempts
to do it. If it fails, you should see at least error message "ACPI
shutdown failed". If you do not see it, it looks like it is stuck
somewhere in firmware. It is quite possible that firmware expects us to
do something else before actually entering S5 state. In particular,
there are several ACPI methods that are expected be executed by OS
before actually performing state transition.

Could you make available ACPI tables (/sys/firmware/acpi/tables/* on
modern Linux kernel)?

> I would have expected at least the monitor to go dark, and maybe also
> the system to power off as there is no specific command `poweroff`
> [1]. 
> 
>> The command halts the computer. If the --no-apm option is specified,
>> no APM BIOS call is performed. Otherwise, the computer is shut down
>> using APM.
> 
> I heard, that SysV implemented `halt` “incorrectly”, so that it also

It is irrelevant here. GRUB halt attempts to call ACPI to initiate S5
(power off) transition.

> powered off the system. Only `halt -p` or `poweroff` was supposed to
> that. so I don’t know, how GRUB’s implementation of `halt` is supposed
> to work.
> 
> 
> Kind regards,
> 
> Paul
> 
> 
> [1] https://www.gnu.org/software/grub/manual/html_node/halt.html#halt
> 
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Support for search by disk id module

[Andrei Borzenkov]

15.04.2017 14:27, Mat628 пишет:
> There is no established standard and this changed over time. GRUB uses
> platform firmware to access devices, so it can only use information
> firmware exports. In general we have no way to query for this information.
> 
> Would it be considered acceptable to call nativedisk.mod to allow for query
> of this information? i.e. instead of hd0 it would ata0 or usb0.
> 

a) You cannot really "call into" nativedisk; it replaces biosdisk which
has some consequences, like not being able to load anything that relies
on BIOS (most obvious is chainloading)

b) any solution should work for all platforms, not only i386-pc.

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

Re: `all_video.mod` missing loading `grub.cfg` generated by grub-pc in GRUB payload (coreboot)

[Andrei Borzenkov]

15.04.2017 13:25, Paul Menzel пишет:
> Dear GRUB folks,
> 
> 
> When using GRUB as a coreboot payload, that means passing `--with-
> platform=coreboot` to configure, and configuring it to load the GRUB
> configuration file from disk, `/boot/grub/grub.cfg` in Debian 8.7
> (Jessie/stable) and 9 (Stretch/testing), generated by GRUB shipped by
> the GNU/Linux distribution, I get a warning about the missing module
> `all_video.mod`, and the enter key has to be pressed to continue
> booting.
> 
...
> 
> Do you know of a way to make that compatible with grub-pc and GRUB
> payload? 

Include all_video in coreboot payload?




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [RFC][PATCH] cmd: test: Add bitwise AND, document the feature

[Andrei Borzenkov]

12.04.2017 21:10, Brad Mouring пишет:
> Currently, there is not a good way to control script flow based on
> bitwise values. This initially led to NI adding the ability to read
> a specific bit from a port-mapped register to control bootflow.
> 
> Here, we add a more generic ability to test the bitwise AND of a
> value available to the grub scripting environment. This obviates the
> need for the inbit command (which is currently OOT).
> 

See recent discussion about arithmetic operations. This could also be

if $((a)); then
 ...
fi

I think it is more generic than single specific test.

> Signed-off-by: Brad Mouring 
> ---
>  docs/grub.texi|  2 ++
>  grub-core/commands/test.c | 10 ++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 82f6fa4..f28063e 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -5019,6 +5019,8 @@ the strings are not equal
>  @var{integer1} is less than @var{integer2}
>  @item @var{integer1} @code{-ne} @var{integer2}
>  @var{integer1} is not equal to @var{integer2}
> +@item @var{integer1} @code{-bwa} @var{integer2}
> +Performs a bitwise AND between @var{integer1} and @var{integer2}
>  @item @var{prefix}@var{integer1} @code{-pgt} @var{prefix}@var{integer2}
>  @var{integer1} is greater than @var{integer2} after stripping off common 
> non-numeric @var{prefix}.
>  @item @var{prefix}@var{integer1} @code{-plt} @var{prefix}@var{integer2}
> diff --git a/grub-core/commands/test.c b/grub-core/commands/test.c
> index 5f06642..af4fad7 100644
> --- a/grub-core/commands/test.c
> +++ b/grub-core/commands/test.c
> @@ -290,6 +290,16 @@ test_parse (char **args, int *argn, int argc)
> continue;
>   }
>  
> +   /* GRUB extension: bitwise AND */
> +   if (grub_strcmp (args[*argn + 1], "-bwa") == 0)
> + {
> +   update_val (grub_strtoul (args[*argn], 0, 0)
> +   & grub_strtoul (args[*argn + 2], 0, 0), );
> +   (*argn) += 3;
> +   continue;
> + }
> +
> +
> /* -nt and -ot tests. GRUB extension: when doing -?t bias
>will be added to the first mtime. */
> if (grub_memcmp (args[*argn + 1], "-nt", 3) == 0
> 


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

Re: Grub2, can the following be done

[Andrei Borzenkov]

12.04.2017 02:28, Vladimir 'phcoder' Serbinenko пишет:
> I documented this in the manual. See "standalone multi-OS setup"
> 

There is no such section. You probably mean Configuration - Multi-boot
manual config.

Unfortunately this is not as simple.

1. Sourcing full configuration file from another distribution is not
safe. It may refer to components (modules, fonts, themes, ...) not
present in "parent" grub. We do not change $prefix when executing
different config. And even if we did we cannot obviously load modules
from completely different build.

2. The most generic is to load another GRUB, but for now there is no
cross-platform method to do it. Chainloader, multiboot, efi - pick your
choice. We probably need platform independent "grub" loader that
resolves to whatever method is valid for current platform.

For PCBIOS having each distribution install its GRUB in own partition
and chainloading it is well understood and established practice, but it
makes it rather hard to auto-detect and incorporate such boot blocks.

For EFI the problem starts already with the fact that we are using
distribution name as \EFI subdirectory, so it is impossible to install
several instances of the same distribution side by side. Some (like
SUSE) expose GRUB_DISTRIBUTOR in system configuration tools, but as it
is also used to build menu, we get something like

openSUSE_1 with linux ...

which is not ideal either. We probably need to have separate
configuration option for subdirectory that can be then set to something
like \EFI\\ where UUID is machine UUID or even simply root
filesystem.

Of course existence of secure boot and shim do not make auto-detection
easier.


> On Wed, Apr 12, 2017, 09:27 Leslie Satenstein 
> wrote:
> 
>> Current-Setup
>> =
>> SYSTEM-A Grub2 >-+ Points to A,B,C,D,E
>>  +-->SYSTEM-B Grub2 Points to B,A,C,D,E
>>  +-->SYSTEM-C Grub2 Points to C,A,B,D,E
>>  +-->SYSTEM-D Grub2 Points to D,A,B,C,E
>>  +-->SYSTEM-E Grub2 Points to E,A,B,C,D
>>
>> Problem
>> ===
>> any update to A,B.C.D.E needs a recreation of grub.cfg for each (all)
>> the others
>>
>>
>> Is it possible to do the following?
>>
>> SYSTEM-A Grub2 >-+A,[MASTER=Grub2]
>>  +-->SYSTEM-B Grub2 Points to B,[MASTER=Grub2]
>>  +-->SYSTEM-C Grub2 Points to C,[MASTER=Grub2]
>>  +-->SYSTEM-D Grub2 Points to D,[MASTER=Grub2]
>>  +-->SYSTEM-E Grub2 Points to E,[MASTER=Grub2]
>>
>> [Master-Grub2]
>>  Copy of all Grub.cfgs, neatly organized?
>>
>> Any update of any system must also update MASTER-GRUB2
>>
>>
>>
>> ___
>> 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: Support for plain dm-crypt and detached LUKS header

[Andrei Borzenkov]

Thank you for the patch. Unfortunately it does not solve fundamental
problem - grub-install is expected to auto-detect everything that is
required to access /boot/grub (it violates it now for encrypted /boot,
but I plan to submit patch after release to decouple it from
/etc/default/grub). So any solution that integrates plain
dm-crypt/detached header into grub-install needs to work without user
intervention and provides some means to auto-detect this.

Although we already have some options that cannot be autodetected (e.g.
keys). So may be this can be relaxed.

But at the very least it needs to check whether LUKS device has detached
header and abort by default if it has, otherwise we cannot even find
device to open at boot time. It also needs code to detect underlying
block device and again abort if partition map does not support PTUUID
(i.e. is not MSDOS or GPT), as this is the only means to find it.

In generally, if grub-install completed without error grub should boot -
otherwise we have a bug.

08.04.2017 19:16, Mat628 пишет:
> Hi, I'd like to first say thanks to the additions you made to cryptomount. I 
> came across your patches 2-3 months ago when I was looking to do FDE 
> including /boot for LVM on LUKS. I created a few patches outlined with their 
> features below. I would have messaged sooner but I didn't know about these 
> posts on grub-help until after finding the link from your github. After also 
> coming across the problem of having no way to reliably predict what a device 
> (hd0,msdos1) will map to, I ended up creating an add-on command, that uses 
> the partition UUID/GUID, to search (--pt-uuid) and an accompanying module 
> search.pt_uuid. These two commands mirror the search --fs-uuid and 
> search.fs_uuid where given a uuid it will return the device. When given a 
> disk/partition UUID/GUID that corresponds to a disk that is either a biosdisk 
> or efidisk, or a partition with a partmap name of either "msdos" or "gpt" on 
> one of the previous disks it will return that device.
> 
> Patch 1 stops search from printing same device twice if called multiple times.
> 
> Patch 2 allows this--->Added module search_pt_uuid. When given a disk/part 
> UUID/GUID it will return the associated device.
> On a biosdisk/mbr it will return the same value for the device 
> (hd0)/(hd0,msdos1) as lsblk -o PARTUUID returns. This should be the same as 
> the NT disk signature located at mbr.code[440] for 4 bytes in little endian 
> format plus the partition number appended if applicable -yy. On an 
> efidisk/gpt it will return the same value for the device (hd0)/(hd0,gpt1) as 
> lsblk -o PARTUUID returns. This will be the disk/partition UUID/GUID.
> 
> An example of using this command inside load.cfg or directly on the grub 
> command line. This example is for a detached header LUKS volume on 
> (hdX,msdosY) which corresponds to a partuuid of 12345678-01 with the header 
> file stored on a plain text partition either on the same device or a 
> different one with a fs_uuid=5432-7654.
> 
> From your site this would look like
> 
> cryptomount -H (hd0,1)/header hd1,1
> 
> But with my patch it would be.
> 
> search.fs_uuid 5432-7654 header_file_device Line for setting (hd0,1)
> search.pt_uuid 12345678-01 cryptodevice Line for setting (hd1,1)
> cryptomount ($cryptodevice) --header=($header_file_device)/header.bin
> 
> Or if the header file is in a separate encrypted LUKS volume. LUKS volume 
> UUID is 12345678-1234-1234-1234-1234567890ab. When the LUKS volume is open 
> the mounted fs_uuid is ----123456654321 for a ext4 
> partition.
> 
> cryptomount -u 12345678-1234-1234-1234-1234567890ab Line for opening the 
> LUKS volume with the header file in it.
> search.fs_uuid ----123456654321 header_file_device 
> Line for setting (hd0,1)
> search.pt_uuid 12345678-01 cryptodevice Line for setting (hd1,1)
> cryptomount ($cryptodevice) --header=($header_file_device)/header.bin 
> Line for opening LUKS volume with /boot
> 
> Patch 3 allows this>Inclusion of altered "load.cfg" to 
> grub-install/mkconfig.
> mattle_opts (More Advanced Than Traditional LUKS Encryption Options). Added 
> mattle_opts.cfg file which allows the user to customize load.cfg to allow for 
> extra options for cryptomount. mattle_opts.cfg is located in 
> user-defined/etc/ folder. Same folder as crypttab and fstab. Only affects 
> cryptomount options in load.cfg and only if 
> GRUB_ENABLE_CRYPTODISK_MATTLE_OPTS=y is set in grub.cfg. If not set then 
> normal 'cryptomount -u $uuid' is printed to load.cfg. I altered grub-install 
> and grub-mkconfig to allow it to have essentially an alternate load.cfg 
> (mattle_opts.cfg) to be easily editted by the user prior to running 
> grub-install to allow grub-install to then read from this file and then 
> fprint this alternate files contents into load.cfg to allow your alternate 
> cryptomount commands to be run during boot up. This 

[PATCH] acpi: do not attempt to create EBDA on EFI (was: Loading DSDT table using 'acpi' or some memory write command?)

[Andrei Borzenkov]

01.04.2017 12:09, Andrei Borzenkov пишет:
> 
> I also have rather weird issue that after "acpi dsdt.aml" I lose
> partitions (only hard disk itself is visible). This is on real hardware
> (Dell Latitude E5450).
> 
> 

Anyone knows why we attempt to write to some arbitrary memory location
on EFI in the first place?
From: Andrei Borzenkov <arvidj...@gmail.com>
Subject: [PATCH] acpi: do not attempt to create EBDA on EFI

There is no gurantee that BDA or EBDA on EFI exists; blindly writing
into memory will likely corrupt it. This fixed problem on Dell Latitude
E5450, where after loading ACPI table GRUB "lost" all disk partitions.

---
 grub-core/commands/acpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/grub-core/commands/acpi.c b/grub-core/commands/acpi.c
index 9f02f22..a395d74 100644
--- a/grub-core/commands/acpi.c
+++ b/grub-core/commands/acpi.c
@@ -119,7 +119,7 @@ grub_acpi_get_rsdpv1 (void)
   return grub_machine_acpi_get_rsdpv1 ();
 }
 
-#if defined (__i386__) || defined (__x86_64__)
+#if defined (__i386__) && !defined (GRUB_MACHINE_EFI)
 
 static inline int
 iszero (grub_uint8_t *reg, int size)
@@ -741,7 +741,7 @@ grub_cmd_acpi (struct grub_extcmd_context *ctxt, int argc, char **args)
 }
   acpi_tables = 0;
 
-#if defined (__i386__) || defined (__x86_64__)
+#if defined (__i386__) && !defined (GRUB_MACHINE_EFI)
   if (! state[9].set)
 {
   grub_err_t err;
-- 
tg: (007f0b4..) u/acpi-skip-ebda-on-efi (depends on: master)
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] btrfs: avoid "used uninitialized" error with GCC7

[Andrei Borzenkov]

03.04.2017 11:59, Vladimir 'phcoder' Serbinenko пишет:
> On Fri, Mar 31, 2017, 06:43 Andrei Borzenkov <arvidj...@gmail.com> wrote:
> 
>> sblock was local and so considered new variable on every loop
>> iteration.
>>
>> While on it, dynamically allocate buffer to reduce stack usage.
>>
> Looks good. Did you check all instances sizeof, so we don't have sizeof of
> pointer?
> Did you check return paths for free'ing on all of them?
> 

There is one common return path, but I committed light version that just
moves variable out of loop; will commit full version after release.


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

[PATCH] acpi: add missing efi_call wrapper to acpi command

[Andrei Borzenkov]

Fixed loading of ACPI tables on EFI (side effect was apparent memory
corruption ranging from unpredictable behavior to system reset).

Reported by Nando Eva 

---

@Nando: please test this patch. It fixed loading for me at least on
QEMU/OVMF. It conflicts with lsacpi patch, but you can actually
verify that RSDP was updated by using "lsefisystab" in grub.

@Vladimir: this looks like 2.02 material as it was obviously wrong.
It does not fix weird behavior on physical system (loss of partitions
afetr loading table using "acpi"); unfortunately, I cannot reproduce
it in QEMU which makes it rather hard to debug. This smells like yet
another meory corruption.

Also our ACPI table mangling code is not clean (e.g. Linux reports
FACS two times). Note that ACPI says normal and extended addresses
(i.e. DSDT/XDSDT and FACS/XFACS) are mutually exclusive, while we
set both to non-zero. This strictly speaking violates specs.

 grub-core/commands/acpi.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/grub-core/commands/acpi.c b/grub-core/commands/acpi.c
index b5c2f27..9f02f22 100644
--- a/grub-core/commands/acpi.c
+++ b/grub-core/commands/acpi.c
@@ -761,10 +761,10 @@ grub_cmd_acpi (struct grub_extcmd_context *ctxt, int 
argc, char **args)
 struct grub_efi_guid acpi = GRUB_EFI_ACPI_TABLE_GUID;
 struct grub_efi_guid acpi20 = GRUB_EFI_ACPI_20_TABLE_GUID;
 
-grub_efi_system_table->boot_services->install_configuration_table
-  (, grub_acpi_get_rsdpv2 ());
-grub_efi_system_table->boot_services->install_configuration_table
-  (, grub_acpi_get_rsdpv1 ());
+efi_call_2 
(grub_efi_system_table->boot_services->install_configuration_table,
+  , grub_acpi_get_rsdpv2 ());
+efi_call_2 
(grub_efi_system_table->boot_services->install_configuration_table,
+  , grub_acpi_get_rsdpv1 ());
   }
 #endif
 
-- 
tg: (8014b7b..) u/acpi-efi_call (depends on: master)

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

Re: Integrating a FreeBSD/GELI change

[Andrei Borzenkov]

01.04.2017 15:57, Eric McCorkle пишет:
> Hello,
> 
> I've been working on a series of changes designed to expand FreeBSD's
> full-disk encryption support via GELI (its preferred disk encryption
> mechanism).  One of the important parts of this landed in HEAD last night:
> 
> https://github.com/freebsd/freebsd/commit/6a205a32527153697eb4df4114ff0cd3c7cd6fd8
> 
> This adds a general mechanism for passing keys into the FreeBSD kernel
> at boot.  At present, this is used exclusively by the GELI subsystem.
> 
> FreeBSD currently supports full-disk encryption for i386 BIOS.  I am
> actively working on EFI support and would like to make sure that GRUB
> also supports full-disk encryption as well (as GRUB is our best option
> for a coreboot setup).
> 
> 
> Basically, to add support for this, I'd need to do two things:
> 
> 1) Ensure that GRUB can handle an entirely GELI-encrypted disk hosting a
> FreeBSD system (I suspect it can, but I've never done a GRUB/GELI setup
> before)
> 
> 2) An additional metadata item needs to get generated when booting the
> FreeBSD kernel that contains all the GELI keys.  (For those who don't
> know, FreeBSD has a kernel metadata mechanism that is used to pass some
> information into the kernel: for example, the EFI console on EFI, some
> BIOS information on i386 BIOS, and so on)
> 
> 
> I've never submitted a patch to GRUB before, so I'm interested in 1) how
> hard would this be,

I suppose like with any other software project of reasonable size.

> 2) where should I look in the source code, and

GELI is in grub-core/disk/geli.c, generic framework for device
encryption (which GELI plugs in) in grub-core/disk/cryptodisk.c and
FreeBSD loader in grub-core/loader/i386/bsd*.

There was proposed patch that stored secret in environment variable that
was later used by loader (I think; I am not sure whether loader part was
actually implemented). Search this list for subject

Patch to support GELI passphrase passthrough​

from Kris Moore (October 2014)

> 3) what is the procedure for submitting patches like this?
>

Just send patches to this list. Better inline using git send-email to
make it easier to comment.



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Loading DSDT table using 'acpi' or some memory write command?

[Andrei Borzenkov]

29.03.2017 20:45, Nando Eva пишет:
>> How exactly do you find RSDP? On EFI RSDP should be retrieved from EFI
>> Configuration Table, which grub tries to update. Please give as much
>> details as possible.
> 
> Good point. I can get the RSDP from tools like 'ru.efi' or even with 'r-w 
> everything'.
> However, 'lsacpi' won't tell me the new RSDP address after doing a 'acpi 
> dsdt.aml'.
> 
> Is there anyway of finding it out using the grub shell, or chainloading EFI 
> shell (or chainload grubx64.efi from EFI shell and exiting in case grub's 
> chainloading is restoring the ACPI tables)?
> 

Please test attached patch. It adds printing of table addresses as well
as "lsacpi --scan" option that forces fetching RSDP from firmware.

It looks like on EFI our call to InstallConfigurationTable does not
work; after "acpi ..." "lsacpi --scan" still displays old RSDP and
tables (although grub itself would use new one).

I also have rather weird issue that after "acpi dsdt.aml" I lose
partitions (only hard disk itself is visible). This is on real hardware
(Dell Latitude E5450).


From: Andrei Borzenkov <arvidj...@gmail.com>
Subject: [PATCH] lsacpi: print table address and allow bypassing stored pointers

1. Print memory address for each table, similat to what Linux kernel does.

2. Add "--scan" option to ignore stored RSDP address (after acpi command)
and use normal platform-specific way to search for it.

This provides for better debugging results of replacing tables with acpi.

---
 grub-core/commands/acpi.c| 34 --
 grub-core/commands/acpihalt.c|  4 +-
 grub-core/commands/lsacpi.c  | 86 ++--
 grub-core/efiemu/i386/pc/cfgtables.c |  4 +-
 grub-core/loader/multiboot_mbi2.c|  6 +--
 include/grub/acpi.h  | 12 -
 6 files changed, 109 insertions(+), 37 deletions(-)

diff --git a/grub-core/commands/acpi.c b/grub-core/commands/acpi.c
index b5c2f27..4163a78 100644
--- a/grub-core/commands/acpi.c
+++ b/grub-core/commands/acpi.c
@@ -100,22 +100,28 @@ static grub_size_t dsdt_size = 0;
 static grub_uint32_t facs_addr = 0;
 
 struct grub_acpi_rsdp_v20 *
-grub_acpi_get_rsdpv2 (void)
+grub_acpi_get_rsdpv2 (int scan)
 {
-  if (rsdpv2_new)
-return rsdpv2_new;
-  if (rsdpv1_new)
-return 0;
+  if (!scan)
+{
+  if (rsdpv2_new)
+	return rsdpv2_new;
+  if (rsdpv1_new)
+	return 0;
+}
   return grub_machine_acpi_get_rsdpv2 ();
 }
 
 struct grub_acpi_rsdp_v10 *
-grub_acpi_get_rsdpv1 (void)
+grub_acpi_get_rsdpv1 (int scan)
 {
-  if (rsdpv1_new)
-return rsdpv1_new;
-  if (rsdpv2_new)
-return 0;
+  if (!scan)
+{
+  if (rsdpv1_new)
+	return rsdpv1_new;
+  if (rsdpv2_new)
+	return 0;
+}
   return grub_machine_acpi_get_rsdpv1 ();
 }
 
@@ -198,8 +204,8 @@ grub_acpi_create_ebda (void)
   *((grub_uint16_t *) targetebda) = ebda_kb_len + 1;
   target = targetebda;
 
-  v1 = grub_acpi_get_rsdpv1 ();
-  v2 = grub_acpi_get_rsdpv2 ();
+  v1 = grub_acpi_get_rsdpv1 (0);
+  v2 = grub_acpi_get_rsdpv2 (0);
   if (v2 && v2->length > 40)
 v2 = 0;
 
@@ -762,9 +768,9 @@ grub_cmd_acpi (struct grub_extcmd_context *ctxt, int argc, char **args)
 struct grub_efi_guid acpi20 = GRUB_EFI_ACPI_20_TABLE_GUID;
 
 grub_efi_system_table->boot_services->install_configuration_table
-  (, grub_acpi_get_rsdpv2 ());
+  (, grub_acpi_get_rsdpv2 (0));
 grub_efi_system_table->boot_services->install_configuration_table
-  (, grub_acpi_get_rsdpv1 ());
+  (, grub_acpi_get_rsdpv1 (0));
   }
 #endif
 
diff --git a/grub-core/commands/acpihalt.c b/grub-core/commands/acpihalt.c
index 9cc7f18..d7c51e1 100644
--- a/grub-core/commands/acpihalt.c
+++ b/grub-core/commands/acpihalt.c
@@ -395,11 +395,11 @@ grub_acpi_halt (void)
   grub_uint32_t port = 0;
   int sleep_type = -1;
 
-  rsdp2 = grub_acpi_get_rsdpv2 ();
+  rsdp2 = grub_acpi_get_rsdpv2 (0);
   if (rsdp2)
 rsdp1 = &(rsdp2->rsdpv1);
   else
-rsdp1 = grub_acpi_get_rsdpv1 ();
+rsdp1 = grub_acpi_get_rsdpv1 (0);
   grub_dprintf ("acpi", "rsdp1=%p\n", rsdp1);
   if (!rsdp1)
 return;
diff --git a/grub-core/commands/lsacpi.c b/grub-core/commands/lsacpi.c
index 0824914..34c1220 100644
--- a/grub-core/commands/lsacpi.c
+++ b/grub-core/commands/lsacpi.c
@@ -43,6 +43,7 @@ print_strn (grub_uint8_t *str, grub_size_t len)
 static void
 disp_acpi_table (struct grub_acpi_table_header *t)
 {
+  grub_printf ("%p: ", t);
   print_field (t->signature);
   grub_printf ("%4" PRIuGRUB_UINT32_T "B rev=%u chksum=0x%02x (%s) OEM=", t->length, t->revision, t->checksum,
 	   grub_byte_checksum (t, t->length) == 0 ? "valid" : "invalid");
@@ -59,7 +60,6 @@ disp_madt_table (struct grub_acpi_madt *t)
   struct grub_acpi_madt_entry_header *d;
   grub_uint32_t len;

[PATCH] btrfs: avoid "used uninitialized" error with GCC7

[Andrei Borzenkov]

sblock was local and so considered new variable on every loop
iteration.

While on it, dynamically allocate buffer to reduce stack usage.

Closes: 50597

---
 grub-core/fs/btrfs.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index 9cffa91..99e81f9 100644
--- a/grub-core/fs/btrfs.c
+++ b/grub-core/fs/btrfs.c
@@ -229,24 +229,29 @@ read_sblock (grub_disk_t disk, struct 
grub_btrfs_superblock *sb)
 {
   unsigned i;
   grub_err_t err = GRUB_ERR_NONE;
+  struct grub_btrfs_superblock *sblock;
+
+  sblock = grub_malloc (sizeof (*sblock));
+  if (sblock == NULL)
+return grub_errno;
+
   for (i = 0; i < ARRAY_SIZE (superblock_sectors); i++)
 {
-  struct grub_btrfs_superblock sblock;
   /* Don't try additional superblocks beyond device size.  */
-  if (i && (grub_le_to_cpu64 (sblock.this_device.size)
+  if (i && (grub_le_to_cpu64 (sblock->this_device.size)
>> GRUB_DISK_SECTOR_BITS) <= superblock_sectors[i])
break;
   err = grub_disk_read (disk, superblock_sectors[i], 0,
-   sizeof (sblock), );
+   sizeof (*sblock), sblock);
   if (err == GRUB_ERR_OUT_OF_RANGE)
break;
 
-  if (grub_memcmp ((char *) sblock.signature, GRUB_BTRFS_SIGNATURE,
+  if (grub_memcmp ((char *) sblock->signature, GRUB_BTRFS_SIGNATURE,
   sizeof (GRUB_BTRFS_SIGNATURE) - 1) != 0)
break;
-  if (i == 0 || grub_le_to_cpu64 (sblock.generation)
+  if (i == 0 || grub_le_to_cpu64 (sblock->generation)
  > grub_le_to_cpu64 (sb->generation))
-   grub_memcpy (sb, , sizeof (sblock));
+   grub_memcpy (sb, sblock, sizeof (*sblock));
 }
 
   if ((err == GRUB_ERR_OUT_OF_RANGE || !err) && i == 0)
@@ -255,6 +260,7 @@ read_sblock (grub_disk_t disk, struct grub_btrfs_superblock 
*sb)
   if (err == GRUB_ERR_OUT_OF_RANGE)
 grub_errno = err = GRUB_ERR_NONE;
 
+  grub_free (sblock);
   return err;
 }
 
-- 
tg: (8014b7b..) bug/50597 (depends on: master)

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

[PATCH] i386, x86_64, ppc: fix switch fallthrough cases with GCC7

[Andrei Borzenkov]

In util/getroot and efidisk slightly modify exitsing comment to mostly
retain it but still make GCC7 compliant with respect to fall through
annotation.

In grub-core/lib/xzembed/xz_dec_lzma2.c it adds same comments as
upstream.

In grub-core/tests/setjmp_tets.c declare functions as "noreturn" to
suppress GCC7 warning.

In grub-core/gnulib/regexec.c use new __attribute__, because existing
annotation is not recognized by GCC7 parser (which requires that comment
immediately precedes case statement).

Otherwise add FALLTHROUGH comment.

Closes: 50598

---
 grub-core/commands/hdparm.c   | 1 +
 grub-core/commands/nativedisk.c   | 1 +
 grub-core/disk/cryptodisk.c   | 1 +
 grub-core/disk/efi/efidisk.c  | 2 +-
 grub-core/efiemu/mm.c | 1 +
 grub-core/gdb/cstub.c | 1 +
 grub-core/gnulib/regexec.c| 3 +++
 grub-core/lib/xzembed/xz_dec_lzma2.c  | 4 
 grub-core/lib/xzembed/xz_dec_stream.c | 6 ++
 grub-core/loader/i386/linux.c | 3 +++
 grub-core/tests/setjmp_test.c | 5 -
 grub-core/video/ieee1275.c| 1 +
 grub-core/video/readers/jpeg.c| 1 +
 util/getroot.c| 2 +-
 util/grub-install.c   | 1 +
 util/grub-mkimagexx.c | 1 +
 util/grub-mount.c | 1 +
 17 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/grub-core/commands/hdparm.c b/grub-core/commands/hdparm.c
index f6b178e..d3fa966 100644
--- a/grub-core/commands/hdparm.c
+++ b/grub-core/commands/hdparm.c
@@ -328,6 +328,7 @@ grub_cmd_hdparm (grub_extcmd_context_t ctxt, int argc, char 
**args)
  ata = ((struct grub_scsi *) disk->data)->data;
  break;
}
+  /* FALLTHROUGH */
 default:
   grub_disk_close (disk);
   return grub_error (GRUB_ERR_IO, "not an ATA device");
diff --git a/grub-core/commands/nativedisk.c b/grub-core/commands/nativedisk.c
index 345f97c..2f56a87 100644
--- a/grub-core/commands/nativedisk.c
+++ b/grub-core/commands/nativedisk.c
@@ -79,6 +79,7 @@ get_uuid (const char *name, char **uuid, int getnative)
 case GRUB_DISK_DEVICE_XEN:
   if (getnative)
break;
+  /* FALLTHROUGH */
 
   /* Virtual disks.  */
   /* GRUB dynamically generated files.  */
diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 1e03a09..bd60a66 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -282,6 +282,7 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev,
  break;
case GRUB_CRYPTODISK_MODE_IV_PLAIN64:
  iv[1] = grub_cpu_to_le32 (sector >> 32);
+ /* FALLTHROUGH */
case GRUB_CRYPTODISK_MODE_IV_PLAIN:
  iv[0] = grub_cpu_to_le32 (sector & 0x);
  break;
diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
index e66b35d..5d2400f 100644
--- a/grub-core/disk/efi/efidisk.c
+++ b/grub-core/disk/efi/efidisk.c
@@ -224,7 +224,7 @@ name_devices (struct grub_efidisk_data *devices)
{
case GRUB_EFI_HARD_DRIVE_DEVICE_PATH_SUBTYPE:
  is_hard_drive = 1;
- /* Fall through by intention.  */
+ /* Intentionally fall through.  */
case GRUB_EFI_CDROM_DEVICE_PATH_SUBTYPE:
  {
struct grub_efidisk_data *parent, *parent2;
diff --git a/grub-core/efiemu/mm.c b/grub-core/efiemu/mm.c
index e606dbf..52a032f 100644
--- a/grub-core/efiemu/mm.c
+++ b/grub-core/efiemu/mm.c
@@ -417,6 +417,7 @@ fill_hook (grub_uint64_t addr, grub_uint64_t size, 
grub_memory_type_t type,
   default:
grub_dprintf ("efiemu",
  "Unknown memory type %d. Assuming unusable\n", type);
+   /* FALLTHROUGH */
   case GRUB_MEMORY_RESERVED:
return grub_efiemu_add_to_mmap (addr, size,
GRUB_EFI_UNUSABLE_MEMORY);
diff --git a/grub-core/gdb/cstub.c b/grub-core/gdb/cstub.c
index c94411b..b64acd7 100644
--- a/grub-core/gdb/cstub.c
+++ b/grub-core/gdb/cstub.c
@@ -336,6 +336,7 @@ grub_gdb_trap (int trap_no)
/* sAA..AA: Step one instruction from AA..AA(optional).  */
case 's':
  stepping = 1;
+ /* FALLTHROUGH */
 
/* cAA..AA: Continue at address AA..AA(optional).  */
case 'c':
diff --git a/grub-core/gnulib/regexec.c b/grub-core/gnulib/regexec.c
index f632cd4..a7776f0 100644
--- a/grub-core/gnulib/regexec.c
+++ b/grub-core/gnulib/regexec.c
@@ -4099,6 +4099,9 @@ check_node_accept (const re_match_context_t *mctx, const 
re_token_t *node,
 case OP_UTF8_PERIOD:
   if (ch >= ASCII_CHARS)
 return false;
+#if defined __GNUC__ && __GNUC__ >= 7
+  __attribute__ ((fallthrough));
+#endif
   /* FALLTHROUGH */
 #endif
 case OP_PERIOD:
diff --git a/grub-core/lib/xzembed/xz_dec_lzma2.c 
b/grub-core/lib/xzembed/xz_dec_lzma2.c
index 8a2a118..af7b770 100644
--- a/grub-core/lib/xzembed/xz_dec_lzma2.c
+++ 

Re: Loading DSDT table using 'acpi' or some memory write command?

[Andrei Borzenkov]

29.03.2017 20:45, Nando Eva пишет:
>> How exactly do you find RSDP? On EFI RSDP should be retrieved from EFI
>> Configuration Table, which grub tries to update. Please give as much
>> details as possible.
> 
> Good point. I can get the RSDP from tools like 'ru.efi' or even with 'r-w 
> everything'.
> However, 'lsacpi' won't tell me the new RSDP address after doing a 'acpi 
> dsdt.aml'.
> 
> Is there anyway of finding it out using the grub shell, or chainloading EFI 
> shell (or chainload grubx64.efi from EFI shell and exiting in case grub's 
> chainloading is restoring the ACPI tables)?
> 

Add printing of RSDP address to lsacpi.

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

Re: Loading DSDT table using 'acpi' or some memory write command?

[Andrei Borzenkov]

27.03.2017 17:12, Nando Eva пишет:
> OK, I found the cause if the problem:
> - the ACPI tables specified to the 'acpi' command are loaded to new addresses 
> and a new RSDT and XSDT created with pointers to them. This can be confirmed 
> by the 'lsacpi' command.
> 
> - the RSDP isn't updated to point to the new RSDT and XSDT. 
> 

How exactly do you find RSDP? On EFI RSDP should be retrieved from EFI
Configuration Table, which grub tries to update. Please give as much
details as possible.

> So when chainload the OS, the original RSDT and XSDT are referenced from the 
> RSDP.
> I'm now manually doing the 'acpi [DSDT]' table load, then a 'lsacpi -2' to 
> find the new RSDT+XSDT addresses and updating the RSDP via two write_dword 
> memory writes (though not doing checksum correction).

Again - what address are you updating?

> Can a pach be made for grub2 to correct this behaviour to be automated 
> without such manual intervention? 

We need to understand why it fails first. GRUB *does* attempt to
override RSDP if it finds it.

> Thank you,Nando 
> 
> On Tuesday, 28 February 2017, 5:21, Nando Eva  wrote:
>  
> 
>  Vladimir 'phcoder' Serbinenko wrote:
> 
> I reproduced the bug. I'm investigating
> 
>>> Apparently finish_boot_services rewrites acpi tables. It's possible to 
>>> workaround this, possibly by using acpi table protocol. >> But it's 
>>> certainely not for 2.02 at this point.
> Thank you for investigating the issue. Earlier I did a test to check to see 
> if UEFI chainloading was altering ACPI tables. I did this by:
> 1. performing two grub2 "write_dword" console memory commands to enable ASPM 
> in the ACPI FADT (FACP on my system) table as per 
> http://smackerelofopinion.blogspot.com.au/2011/03/making-sense-of-pcie-aspm.html
>  
> 
> 2. then chainloaded from Grub2 to windows: chainloader 
> /efi/Microsoft/Boot/bootmgfw.efi
> The ASPM enabling change was there as found by using r-w everything and 
> reported by 'powercfg /energy', indicating the UEFI chainloading isn't, at 
> least for ACPI FADT (FACP), altering the in-memory table.
> As the DSDT table is much larger, I can't really write_dword it. Hence asking 
> for some other memory loading module.
> As another lead, I found the mentioned finish_boot_services routine in mm.c, 
> quoted below. Would you mind pointing out which code is rewriting the ACPI 
> tables?

The whole ACPI tables mangling happens in grub-core/commands/acpi.c


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

Re: [wea...@debian.org: Bug#858832: calls efibootmgr with invalid options]

[Andrei Borzenkov]

27.03.2017 15:41, Colin Watson пишет:
> I guess that the attached bug happens because grub_install_register_efi
> is called on non-biosdisk-ish systems but uses
> grub_util_biosdisk_get_osdev to get an OS device name for
> efidir_grub_dev->disk, which isn't going to work so well.  I'm a bit
> rusty here - would anyone care to venture a guess at suitable
> replacement code that would work on devices other than biosdisk?
> 

"biosdisk" here means "firmware device", not necessary BIOS.

Where /boot/efi is located? Output of lsblk and "grub-probe -t device
/boot/efi".

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

Re: [wea...@debian.org: Bug#858832: calls efibootmgr with invalid options]

[Andrei Borzenkov]

Link to the bug report would be helpful. I assume it contains actual
invocation with arguments, I do not see them in this mail.

On Mon, Mar 27, 2017 at 3:41 PM, Colin Watson  wrote:
> I guess that the attached bug happens because grub_install_register_efi
> is called on non-biosdisk-ish systems but uses
> grub_util_biosdisk_get_osdev to get an OS device name for
> efidir_grub_dev->disk, which isn't going to work so well.  I'm a bit
> rusty here - would anyone care to venture a guess at suitable
> replacement code that would work on devices other than biosdisk?
>
> Thanks,
>
> --
> Colin Watson   [cjwat...@debian.org]
>
>
> -- Forwarded message --
> From: Peter Palfrader 
> To: Debian Bug Tracking System 
> Cc:
> Bcc:
> Date: Mon, 27 Mar 2017 12:24:16 +
> Subject: Bug#858832: calls efibootmgr with invalid options
> Package: grub-efi-arm64
> Version: 2.02~beta3-5
> Severity: serious
> User: debian-ad...@lists.debian.org
> Usertags: needed-by-DSA-Team
>
> On upgrading from jessie to stretch on our arm64 box acker.debian.org I
> noticed this issue with grub-efi-arm64:
>
>
> acker:~# apt-get install --reinstall grub-efi-arm64
> Reading package lists... Done
> Building dependency tree
> Reading state information... Done
> 0 upgraded, 0 newly installed, 1 reinstalled, 0 to remove and 1 not upgraded.
> Need to get 73.0 kB of archives.
> After this operation, 0 B of additional disk space will be used.
> Get:1 http://mirror.netcologne.de/debian stretch/main arm64 grub-efi-arm64 
> arm64 2.02~beta3-5 [73.0 kB]
> Fetched 73.0 kB in 0s (603 kB/s)
> Preconfiguring packages ...
> (Reading database ... 62883 files and directories currently installed.)
> Preparing to unpack .../grub-efi-arm64_2.02~beta3-5_arm64.deb ...
> Unpacking grub-efi-arm64 (2.02~beta3-5) over (2.02~beta3-5) ...
> Setting up grub-efi-arm64 (2.02~beta3-5) ...
> Installing for arm64-efi platform.
> efibootmgr: option requires an argument -- 'd'
> efibootmgr version 14
> usage: efibootmgr [options]
> -a | --active sets bootnum active
> -A | --inactive   sets bootnum inactive
> -b | --bootnum    modify Boot (hex)
> -B | --delete-bootnum delete bootnum
> -c | --create create new variable bootnum and add to bootorder
> -C | --create-only  create new variable bootnum and do not add to 
> bootorder
> -D | --remove-dups  remove duplicate values from BootOrder
> -d | --disk disk   (defaults to /dev/sda) containing loader
> -r | --driver Operate on Driver variables, not Boot Variables.
> -e | --edd [1|3|-1]   force EDD 1.0 or 3.0 creation variables, or 
> guess
> -E | --device num  EDD 1.0 device number (defaults to 0x80)
> -g | --gptforce disk with invalid PMBR to be treated as 
> GPT
> -i | --iface name create a netboot entry for the named interface
> -l | --loader name (defaults to \EFI\redhat\grub.efi)
> -L | --label label Boot manager display label (defaults to 
> "Linux")
> -m | --mirror-below-4G t|f mirror memory below 4GB
> -M | --mirror-above-4G X percentage memory to mirror above 4GB
> -n | --bootnext    set BootNext to  (hex)
> -N | --delete-bootnext delete BootNext
> -o | --bootorder ,,,... explicitly set BootOrder (hex)
> -O | --delete-bootorder delete BootOrder
> -p | --part part(defaults to 1) containing loader
> -q | --quietbe quiet
> -t | --timeout seconds  set boot manager timeout waiting for user 
> input.
> -T | --delete-timeout   delete Timeout.
> -u | --unicode | --UCS-2  pass extra args as UCS-2 (default is ASCII)
> -v | --verbose  print additional information
> -V | --version  return version and exit
> -w | --write-signature  write unique sig to MBR if needed
> -y | --sysprep  Operate on SysPrep variables, not Boot 
> Variables.
> -@ | --append-binary-args file  append extra args from file (use "-" 
> for stdin)
> -h | --help show help/usage
> grub-install: error: efibootmgr failed to register the boot entry: Operation 
> not permitted.
> Failed: grub-install --target=arm64-efi
> WARNING: Bootloader is not properly installed, system may not be bootable
> Generating grub configuration file ...
> Found linux image: /boot/vmlinuz-4.9.0-2-arm64
> Found initrd image: /boot/initrd.img-4.9.0-2-arm64
> Found linux image: /boot/vmlinuz-4.9.0-0.bpo.2-arm64
> Found initrd image: /boot/initrd.img-4.9.0-0.bpo.2-arm64
> Found linux image: /boot/vmlinuz-4.8.0-0.bpo.2-arm64
> Found initrd image: /boot/initrd.img-4.8.0-0.bpo.2-arm64
> Adding boot menu entry for EFI firmware configuration
> done
> acker:~#
>
> Cheers,
> --
> 

Re: cbfsdisk not found

[Andrei Borzenkov]

24.03.2017 21:38, Gailu Singh пишет:
> This is how it is calculated in coreboot
> 
> https://review.coreboot.org:4430/cgit/coreboot.git/tree/src/lib/cbfs.c?id=refs/heads/master#n268
> 
> ___FMAP__COREBOOT_BASE and ___FMAP__COREBOOT_SIZE are based on values
> available in fmd files (depending on 8MB/16Mb coreboot).
> 
> 
> #grep -r FMAP__COREBOOT_BASE src/lib/cbfs.c: size_t fmap_top =
> ___FMAP__COREBOOT_BASE + ___FMAP__COREBOOT_SIZE;
> build/fmap_config.h:#define ___FMAP__COREBOOT_BASE 0x300800
> 
> #grep -r FMAP__COREBOOT_SIZE
> src/soc/intel/baytrail/romstage/cache_as_ram.inc:#define CODE_CACHE_SIZE
>  _ALIGN_UP_POW2(___FMAP__COREBOOT_SIZE)
> src/lib/cbfs.c: size_t fmap_top = ___FMAP__COREBOOT_BASE +
> ___FMAP__COREBOOT_SIZE;
> build/fmap_config.h:#define ___FMAP__COREBOOT_SIZE 0xc1d800
> 
> #cat src/mainboard/intel/leafhill/leafhill.16384.fmd
> FLASH 16M {
> SI_DESC@0x0 0x1000
> IFWI@0x1000 0x2ff000
> FMAP@0x30 0x800
> COREBOOT(CBFS)@0x300800 0xc1d800
> UNIFIED_MRC_CACHE@0xf1e000 0x21000 {
> RECOVERY_MRC_CACHE@0x0 0x1
> RW_MRC_CACHE@0x1 0x1
> RW_VAR_MRC_CACHE@0x2 0x1000
> }
> BIOS_UNUSABLE@0xf3f000 0x4
> DEVICE_EXTENSION@0xf7f000 0x7f000
> UNUSED_HOLE@0xfff000 0x1000
> }
> 
> 

How does it help us? We have no idea which board grub currently runs on.
Do you suggest scanning the whole memory for "__FMAP__" signature?
"
> 
> 
> 
> 
> 
> On Fri, Mar 24, 2017 at 11:12 PM, Andrei Borzenkov <arvidj...@gmail.com>
> wrote:
> 
>> 24.03.2017 20:32, Gailu Singh пишет:
>>>>> Could you please reference the solution for future users with the same
>>> problem?
>>> Sure. Infect I should have detailed it without asking. Sorry for that.
>>>
>>> Solution can be referenced to following thread in coreboot mailing list
>>> that provide details of memory mapping on the board answered by Adrian
>>>
>>> https://www.coreboot.org/pipermail/coreboot/2017-March/083681.html
>>>
>>
>> Enters FMAP ... I like the
>>
>> "However, you'd have to know where the FMAP is in order to parse things"
>>
>> And how should we know this location?
>>
>> ___
>> 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: cbfsdisk not found

[Andrei Borzenkov]

24.03.2017 20:32, Gailu Singh пишет:
>>> Could you please reference the solution for future users with the same
> problem?
> Sure. Infect I should have detailed it without asking. Sorry for that.
> 
> Solution can be referenced to following thread in coreboot mailing list
> that provide details of memory mapping on the board answered by Adrian
> 
> https://www.coreboot.org/pipermail/coreboot/2017-March/083681.html
> 

Enters FMAP ... I like the

"However, you'd have to know where the FMAP is in order to parse things"

And how should we know this location?

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

Re: Disable graphics completly

[Andrei Borzenkov]

On Mon, Mar 20, 2017 at 2:57 PM, Andrei Borzenkov <arvidj...@gmail.com> wrote:
> On Mon, Mar 20, 2017 at 1:47 PM, Gailu Singh <gail...@gmail.com> wrote:
>> Thanks Daniel.
>>
>> I do not see video specific modules in the below list (guessing based on the
>> names though, so may be wrong)
>>
>
> Default output driver on coreboot platform in text mode VGA.
> Unfortunately GRUB starts output even before evaluating embedded
> config so it will probably reset screen anyway. For testing
>
> 1. use terminal_output to change output driver in embedded config
> 2. Comment out
>

[I hate webmail]

  /* Hello.  */
  grub_setcolorstate (GRUB_TERM_COLOR_HIGHLIGHT);
  grub_printf ("Welcome to GRUB!\n\n");
  grub_setcolorstate (GRUB_TERM_COLOR_STANDARD);

In grub-core/kern/main.c:main()

Does it help?

>
>> ./grub-mkstandalone --grub-mkimage=./grub-mkimage -O i386-coreboot -o
>> default_payload.elf --modules='ls ahci pata ehci uhci ohci usb_keyboard
>> usbms part_msdos ext2 fat at_keyboard part_gpt usbserial_usbdebug cbfs'
>> --install-modules='linux search configfile normal cbtime cbls memrw iorw
>> minicmd lsmmap lspci halt reboot hexdump pcidump regexp setpci lsacpi chain
>> test serial multiboot cbmemc linux16 gzio echo help syslinuxcfg xnu affs afs
>> bfs btrfs cbfs cpio cpio_be exfat ext2 fat hfs hfsplus iso9660 jfs minix
>> minix2 minix2_be minix3 minix3_be minix_be newc nilfs2 ntfs odc procfs
>> reiserfs romfs sfs squash4 tar udf ufs1 ufs1_be ufs2 xfs zfs password_pbkdf2
>> ' --fonts= --themes= --locales= -d grub-core/
>> /boot/grub/grub.cfg=./coreboot.cfg
>>
>> On Mon, Mar 20, 2017 at 4:06 PM, Daniel Kiper <dki...@net-space.pl> wrote:
>>>
>>> On Mon, Mar 20, 2017 at 02:58:01PM +0530, Gailu Singh wrote:
>>> > Hi Experts,
>>> >
>>> > We are using coreboot + grub to load Linux. We display splash screen in
>>> > coreboot but this splash screen is wiped out by GRUB. Our requirement is
>>> > to
>>> > continue same splash screen (loaded by coreboot) on display until Linux
>>> > booting is completed. Grub console/menu is needed on on serial port but
>>> > not
>>> > on display
>>> >
>>> > Is this possible? If yes how can I achieve it in grub.cfg.
>>>
>>> I am not sure but you can try to not load any of video modules
>>> and fiddle with terminal_input and terminal_output commands.
>>>
>>> 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: Disable graphics completly

[Andrei Borzenkov]

On Mon, Mar 20, 2017 at 1:47 PM, Gailu Singh  wrote:
> Thanks Daniel.
>
> I do not see video specific modules in the below list (guessing based on the
> names though, so may be wrong)
>

Default output driver on coreboot platform in text mode VGA.
Unfortunately GRUB starts output even before evaluating embedded
config so it will probably reset screen anyway. For testing

1. use terminal_output to change output driver in embedded config
2. Comment out


> ./grub-mkstandalone --grub-mkimage=./grub-mkimage -O i386-coreboot -o
> default_payload.elf --modules='ls ahci pata ehci uhci ohci usb_keyboard
> usbms part_msdos ext2 fat at_keyboard part_gpt usbserial_usbdebug cbfs'
> --install-modules='linux search configfile normal cbtime cbls memrw iorw
> minicmd lsmmap lspci halt reboot hexdump pcidump regexp setpci lsacpi chain
> test serial multiboot cbmemc linux16 gzio echo help syslinuxcfg xnu affs afs
> bfs btrfs cbfs cpio cpio_be exfat ext2 fat hfs hfsplus iso9660 jfs minix
> minix2 minix2_be minix3 minix3_be minix_be newc nilfs2 ntfs odc procfs
> reiserfs romfs sfs squash4 tar udf ufs1 ufs1_be ufs2 xfs zfs password_pbkdf2
> ' --fonts= --themes= --locales= -d grub-core/
> /boot/grub/grub.cfg=./coreboot.cfg
>
> On Mon, Mar 20, 2017 at 4:06 PM, Daniel Kiper  wrote:
>>
>> On Mon, Mar 20, 2017 at 02:58:01PM +0530, Gailu Singh wrote:
>> > Hi Experts,
>> >
>> > We are using coreboot + grub to load Linux. We display splash screen in
>> > coreboot but this splash screen is wiped out by GRUB. Our requirement is
>> > to
>> > continue same splash screen (loaded by coreboot) on display until Linux
>> > booting is completed. Grub console/menu is needed on on serial port but
>> > not
>> > on display
>> >
>> > Is this possible? If yes how can I achieve it in grub.cfg.
>>
>> I am not sure but you can try to not load any of video modules
>> and fiddle with terminal_input and terminal_output commands.
>>
>> 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: cbfsdisk not found

[Andrei Borzenkov]

On Fri, Mar 17, 2017 at 4:24 PM, Gailu Singh  wrote:
> @Vladimir
> I do not have serial console (board has memory mapped uart not support in
> grub) to capture memory details. Also on graphics console I can not give
> commands from usb keyboard to  dump memory because USB port on board are 3.0
> and USB 3.0 is not supported in Grub. Any other thing I could try?
>
> @Andrei
> What is the expected address supposed to be printed for successful
> identification of CBFS.
>

GRUB reads 4 bytes at offset 0xffc0. All ones likely means that
this address simply is not valid.

According to 
https://github.com/mrnuke/coreboot/blob/master/documentation/cbfs.txt,
"A pointer to the location of the header will be located at offset -4
from the end of the ROM. This translates to address 0xFFFC on a
normal x86 system." Any chance ROM is mapped somewhere else in your
case?

> I am building coreboot image myself and stitching with other intel firmware
> as documented by Intel.
>
> On Fri, Mar 17, 2017 at 12:45 PM, Vladimir 'phcoder' Serbinenko
>  wrote:
>>
>>
>>
>> On Thu, Mar 16, 2017, 22:30 Gailu Singh  wrote:
>>>
>>> Please find coreboot image attached. Zipped it to reduce size
>>
>> This image is actually fine and is read correctly by fstest. Apparently
>> for some reason reading image at the end of 4GiB doesn't work. Can you try
>> dumping last 8M before 4G mark from /dev/mem ?
>>>
>>>
>>> On Fri, Mar 17, 2017 at 1:19 AM, Vladimir 'phcoder' Serbinenko
>>>  wrote:



 On Thu, Mar 16, 2017, 12:43 Gailu Singh  wrote:
>
> >>Try insmod cbfs
>
>
> Trieed, it does not help. BTW cbfs is already included in my grub elf
> image so I don't think I need to do insmod.
>
> ./grub-mkstandalone --grub-mkimage=./grub-mkimage -O i386-coreboot -o
> default_payload.elf --modules='ls ahci pata ehci uhci ohci usb_keyboard
> usbms part_msdos ext2 fat at_keyboard part_gpt usbserial_usbdebug cbfs'
> --install-modules='linux search configfile normal cbtime cbls memrw iorw
> minicmd lsmmap lspci halt reboot hexdump pcidump regexp setpci lsacpi 
> chain
> test serial multiboot cbmemc linux16 gzio echo help syslinuxcfg xnu affs 
> afs
> bfs btrfs cbfs cpio cpio_be exfat ext2 fat hfs hfsplus iso9660 jfs minix
> minix2 minix2_be minix3 minix3_be minix_be newc nilfs2 ntfs odc procfs
> reiserfs romfs sfs squash4 tar udf ufs1 ufs1_be ufs2 xfs zfs 
> password_pbkdf2
> ' --fonts= --themes= --locales= -d grub-core/
> /boot/grub/grub.cfg=./coreboot.cfg
>
> This must be something else. AFAIK memdisk itself resides in CBFS,
> Strangely ls comman shows memdisk but not cbfsdisk

 Please share your coreboot image.
>
>
>
> On Fri, Mar 17, 2017 at 12:52 AM, Vladimir 'phcoder' Serbinenko
>  wrote:
>>
>>
>>
>> On Thu, Mar 16, 2017, 12:18 Gailu Singh  wrote:
>>>
>>> Hi Experts,
>>>
>>> I am using grub2 with coreboot and configured with
>>> --with-platform=coreboot.
>>>
>>> I am able to load grub2 from coreboot. However when I run ls command,
>>> I do not see cbfsdisk. ls only shows
>>>
>>> (memdisk) (ahci0) (ahci0,msdos1) (ahci0,msdos2)
>>>
>>> I have added file in the cbfs as follows and need to access it but
>>> when I try (cbfsdisk)/myfile I get error disk 'cbfsdisk' not found.
>>>
>>> build/util/cbfstool/cbfstool build/coreboot.rom add -f myfile -n
>>> myfile -t raw
>>>
>>> Any idea how to access myfile on cbfsdisk?
>>
>> Try insmod cbfs
>>>
>>>
>>> Thanks.
>>> ___
>>> 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


 ___
 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
>>
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

___
Grub-devel 

Re: cbfsdisk not found

[Andrei Borzenkov]

On Fri, Mar 17, 2017 at 9:19 AM, Gailu Singh <gail...@gmail.com> wrote:
>>>What address it prints?
>
> I followed suggested steps and I see following print
>
> fs/cbfs.c:347: head=0x
>

Then you need to contact coreboot developers and discuss it. GRUB
looks at predefined location at address of cbfs. If this location
contains wrong address, there is nothing we can do here. We need
information how to discover cbfs using other means.

>
>
> On Fri, Mar 17, 2017 at 8:57 AM, Andrei Borzenkov <arvidj...@gmail.com>
> wrote:
>>
>> 16.03.2017 22:43, Gailu Singh пишет:
>> >>> Try insmod cbfs
>> >
>> >>
>> > Trieed, it does not help. BTW cbfs is already included in my grub elf
>> > image
>> > so I don't think I need to do insmod.
>> >
>> > ./grub-mkstandalone --grub-mkimage=./grub-mkimage -O i386-coreboot -o
>> > default_payload.elf --modules='ls ahci pata ehci uhci ohci usb_keyboard
>> > usbms part_msdos ext2 fat at_keyboard part_gpt usbserial_usbdebug cbfs'
>> > --install-modules='linux search configfile normal cbtime cbls memrw iorw
>> > minicmd lsmmap lspci halt reboot hexdump pcidump regexp setpci lsacpi
>> > chain
>> > test serial multiboot cbmemc linux16 gzio echo help syslinuxcfg xnu affs
>> > afs bfs btrfs cbfs cpio cpio_be exfat ext2 fat hfs hfsplus iso9660 jfs
>> > minix minix2 minix2_be minix3 minix3_be minix_be newc nilfs2 ntfs odc
>> > procfs reiserfs romfs sfs squash4 tar udf ufs1 ufs1_be ufs2 xfs zfs
>> > password_pbkdf2 ' --fonts= --themes= --locales= -d grub-core/
>> > /boot/grub/grub.cfg=./coreboot.cfg
>> >
>> > This must be something else. AFAIK memdisk itself resides in CBFS,
>> > Strangely ls comman shows memdisk but not cbfsdisk
>> >
>>
>> This implies that cbfs address fails validation. Do not include cbfs in
>> image, boot and run
>>
>> set debug=cbfs
>> insmod cbfs
>>
>> What address it prints?
>>
>> >
>> > On Fri, Mar 17, 2017 at 12:52 AM, Vladimir 'phcoder' Serbinenko <
>> > phco...@gmail.com> wrote:
>> >
>> >>
>> >>
>> >> On Thu, Mar 16, 2017, 12:18 Gailu Singh <gail...@gmail.com> wrote:
>> >>
>> >>> Hi Experts,
>> >>>
>> >>> I am using grub2 with coreboot and configured with
>> >>> --with-platform=coreboot.
>> >>>
>> >>>
>> >>> I am able to load grub2 from coreboot. However when I run ls command,
>> >>> I
>> >>> do not see cbfsdisk. ls only shows
>> >>>
>> >>> (memdisk) (ahci0) (ahci0,msdos1) (ahci0,msdos2)
>> >>>
>> >>> I have added file in the cbfs as follows and need to access it but
>> >>> when I
>> >>> try (cbfsdisk)/myfile I get error disk 'cbfsdisk' not found.
>> >>>
>> >>> build/util/cbfstool/cbfstool build/coreboot.rom add -f myfile -n
>> >>> myfile
>> >>> -t raw
>> >>>
>> >>> Any idea how to access myfile on cbfsdisk?
>> >>>
>> >> Try insmod cbfs
>> >>
>> >>>
>> >>> Thanks.
>> >>> ___
>> >>> 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
>> >
>>
>>
>> ___
>> 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: cbfsdisk not found

[Andrei Borzenkov]

16.03.2017 22:43, Gailu Singh пишет:
>>> Try insmod cbfs
> 
>>
> Trieed, it does not help. BTW cbfs is already included in my grub elf image
> so I don't think I need to do insmod.
> 
> ./grub-mkstandalone --grub-mkimage=./grub-mkimage -O i386-coreboot -o
> default_payload.elf --modules='ls ahci pata ehci uhci ohci usb_keyboard
> usbms part_msdos ext2 fat at_keyboard part_gpt usbserial_usbdebug cbfs'
> --install-modules='linux search configfile normal cbtime cbls memrw iorw
> minicmd lsmmap lspci halt reboot hexdump pcidump regexp setpci lsacpi chain
> test serial multiboot cbmemc linux16 gzio echo help syslinuxcfg xnu affs
> afs bfs btrfs cbfs cpio cpio_be exfat ext2 fat hfs hfsplus iso9660 jfs
> minix minix2 minix2_be minix3 minix3_be minix_be newc nilfs2 ntfs odc
> procfs reiserfs romfs sfs squash4 tar udf ufs1 ufs1_be ufs2 xfs zfs
> password_pbkdf2 ' --fonts= --themes= --locales= -d grub-core/
> /boot/grub/grub.cfg=./coreboot.cfg
> 
> This must be something else. AFAIK memdisk itself resides in CBFS,
> Strangely ls comman shows memdisk but not cbfsdisk
> 

This implies that cbfs address fails validation. Do not include cbfs in
image, boot and run

set debug=cbfs
insmod cbfs

What address it prints?

> 
> On Fri, Mar 17, 2017 at 12:52 AM, Vladimir 'phcoder' Serbinenko <
> phco...@gmail.com> wrote:
> 
>>
>>
>> On Thu, Mar 16, 2017, 12:18 Gailu Singh  wrote:
>>
>>> Hi Experts,
>>>
>>> I am using grub2 with coreboot and configured with --with-platform=coreboot.
>>>
>>>
>>> I am able to load grub2 from coreboot. However when I run ls command, I
>>> do not see cbfsdisk. ls only shows
>>>
>>> (memdisk) (ahci0) (ahci0,msdos1) (ahci0,msdos2)
>>>
>>> I have added file in the cbfs as follows and need to access it but when I
>>> try (cbfsdisk)/myfile I get error disk 'cbfsdisk' not found.
>>>
>>> build/util/cbfstool/cbfstool build/coreboot.rom add -f myfile -n myfile
>>> -t raw
>>>
>>> Any idea how to access myfile on cbfsdisk?
>>>
>> Try insmod cbfs
>>
>>>
>>> Thanks.
>>> ___
>>> 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
> 


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

Re: [PATCH] disk/efi: skip iPXE dummy block devices

[Andrei Borzenkov]

14.03.2017 06:03, Michael Chang пишет:
> On Mon, Mar 13, 2017 at 06:45:02AM +0300, Andrei Borzenkov wrote:
>> iPXE adds Simple File System Protocol to loaded image handle, as side
>> effect it also adds Block IO protocol (according to comments, to work
>> around some bugs in EDK2). GRUB assumes that every device with Block IO
>> is disk and skips network initialization entirely. But iPXE Block IO
>> implementation is just a stub which always fails for every operation
>> so cannot be used. Attempt to detect and skip such devices.
>>
>> We are using media ID which iPXE sets to "iPXE" and block IO size in
>> hope that no real device would announce 1B block ...
>>
>> Closes: 50518
>>
>> @Vladimir, @Daniel - this is probably 2.02 material. We cannot use
>> any device that advertises block size less than 512B anyway, so it
>> should not cause regression for disk boot.
>>
>> It is my fault, I have seen it earlier but was busy with another problem
>> so just worked around it to continue testing.
>>
>> ---
>>  grub-core/disk/efi/efidisk.c | 9 +
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
>> index 3b79f7b..47a4e99 100644
>> --- a/grub-core/disk/efi/efidisk.c
>> +++ b/grub-core/disk/efi/efidisk.c
>> @@ -80,6 +80,15 @@ make_devices (void)
>>  /* This should not happen... Why?  */
>>  continue;
>>  
>> +  /* iPXE adds stub Block IO protocol to loaded image device handle. It 
>> is
>> + completely non-functional and simply returns an error for every 
>> method.
>> + So attempt to detect and skip it. Magic number is literal "iPXE" and
>> + check block size as well */
>> +  /* FIXME: shoud we close it? We do not do it elsewhere */
>> +  if (bio->media && bio->media->media_id == 0x69505845U &&
> 
> It seems that BE to LE conversion is required here as it tests for 'EXPi' on 
> LE
> systems. 
> 

iPXE sets it to this literal number:

#define EFI_MEDIA_ID_MAGIC 0x69505845

/** Dummy block I/O media */
static EFI_BLOCK_IO_MEDIA efi_block_io_media = {
.MediaId = EFI_MEDIA_ID_MAGIC,
.MediaPresent = TRUE,
.ReadOnly = TRUE,
.BlockSize = 1,
};


> Thanks,
> Michael
> 
>> +  bio->media->block_size == 1)
>> +  continue;
>> +
>>d = grub_malloc (sizeof (*d));
>>if (! d)
>>  {
>> -- 
>> tg: (bcf3c55..) bug/50518 (depends on: master)
>>
>> ___
>> 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

[PATCH] disk/efi: skip iPXE dummy block devices

[Andrei Borzenkov]

iPXE adds Simple File System Protocol to loaded image handle, as side
effect it also adds Block IO protocol (according to comments, to work
around some bugs in EDK2). GRUB assumes that every device with Block IO
is disk and skips network initialization entirely. But iPXE Block IO
implementation is just a stub which always fails for every operation
so cannot be used. Attempt to detect and skip such devices.

We are using media ID which iPXE sets to "iPXE" and block IO size in
hope that no real device would announce 1B block ...

Closes: 50518

@Vladimir, @Daniel - this is probably 2.02 material. We cannot use
any device that advertises block size less than 512B anyway, so it
should not cause regression for disk boot.

It is my fault, I have seen it earlier but was busy with another problem
so just worked around it to continue testing.

---
 grub-core/disk/efi/efidisk.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
index 3b79f7b..47a4e99 100644
--- a/grub-core/disk/efi/efidisk.c
+++ b/grub-core/disk/efi/efidisk.c
@@ -80,6 +80,15 @@ make_devices (void)
/* This should not happen... Why?  */
continue;
 
+  /* iPXE adds stub Block IO protocol to loaded image device handle. It is
+ completely non-functional and simply returns an error for every 
method.
+So attempt to detect and skip it. Magic number is literal "iPXE" and
+check block size as well */
+  /* FIXME: shoud we close it? We do not do it elsewhere */
+  if (bio->media && bio->media->media_id == 0x69505845U &&
+ bio->media->block_size == 1)
+ continue;
+
   d = grub_malloc (sizeof (*d));
   if (! d)
{
-- 
tg: (bcf3c55..) bug/50518 (depends on: master)

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

Re: Menu for booting BTRFS snapshots

[Andrei Borzenkov]

Отправлено с iPhone

> 4 марта 2017 г., в 4:02, Robert LeBlanc  написал(а):
> 
> 
> menuentry 'RXE Snapshot' {
>   load_env -f /snaps/rxe-boot/grub2/grubenv
>   configfile /snaps/rxe-boot/grub2/grub.cfg
> }
> 

Grub.cfg as created by grub-mkconfig loads $prefix/grubenv which overrides 
whatever you got from your other grubenv. $prefix does not change when you run 
different config.

It is not the first time I see similar issue. I think grubenv should be 
searched relative to current $config_directory, it is not as much property of 
loaded binary, as property of current configuration.

Could you try to change your snapshot grub.cfg this way and see if it helps? 
You can then remove explicit grubenv load.


> 
> # cat /mnt/btrfs/boot/grub2/grubenv
> # GRUB Environment Block
> saved_entry=RXE Snapshot
> ###

Grub-editenv list would be enough.



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

Re: Menu for booting BTRFS snapshots

[Andrei Borzenkov]

03.03.2017 00:40, Robert LeBlanc пишет:
> I'm trying to add some menuentry items to move between btrfs
> snapshots. I've added the following in the non-snapshot
> /etc/grub.d/40_custom.
> 
> menuentry 'RXE Snapshot' {
>load_env -f /snaps/rxe-boot/grub2/grubenv
>configfile /snaps/rxe-boot/grub2/grub.cfg
> }
> 
> It loads the config for my RXE snapshot, but in doesn't select the
> default menuentry as specified in the snapshot's grubenv. This makes

What is exact "default" value in loaded grubenv?

> it so that it won't boot automatically into a snapshot kernel (I
> created a 09_header file to add a noop menuentry that tells me I'm
> using the RXE snapshot config and that is what is selected). It
> doesn't matter if load_env is before or after configfile. Other than
> this, it works as expected.
> 
> I don't think I can use chainload because it is a 'directory' on the
> btrfs file system.
> 
> Any ideas on how to accomplish what I want?
> 

You can load other grub image directly without chainloading.


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

Re: USB3.0 XHCI support on GRUB2

[Andrei Borzenkov]

02.03.2017 06:00, Gailu Singh пишет:
> Thanks for the update. I will try your patches on Apollo lake and report
> back the results. Intel strategy seems to be XHCI only option on newer
> boards so sooner or later xhci support is required in Grub. Thanks for your
> effort to take initiative.
> 

On physical platforms grub normally relies on firmware to access
devices. What exact problem do you solve? Why do you need direct xHCI
access? What firmware this board has (I presume, EFI)?

> On Wed, Mar 1, 2017 at 4:07 PM, Bjørn Forsman <bjorn.fors...@gmail.com>
> wrote:
> 
>> On 1 March 2017 at 07:57, Andrei Borzenkov <arvidj...@gmail.com> wrote:
>>> Yes, Bjørn Forsman intended to work on implementation.
>>
>> So now is apparently a good time to tell you about the status of that
>> implementation :-)
>>
>>
>> The good:
>>
>> I ported the xHCI driver from Coreboot / libpayload to GRUB. In basic
>> tests, it works. (I tested a Lenovo G50-80 laptop with Intel Wildcat
>> Point-LP USB xHCI controller [8086:9cb1].)
>>
>>
>> The bad:
>>
>> The code doesn't support HUBs and completely hangs on one xHCI
>> controller I tested (Intel NUC6i5SYH Skylake with Sunrise Point-LP
>> xHCI USB controller [8086:9d2f]).
>>
>> The HUB problem is due to GRUB using a different code path for HUBs
>> than normal devices. I implemented hooks for "control_transfer" and
>> "bulk_transfer" in GRUB USB stack, because that required little
>> changes and it made it easy to hook it up with the xHCI driver. But
>> when GRUB sees a HUB, it uses the "old" setup_transfer code path which
>> isn't implemented in the xHCI driver.
>>
>> I'm thinking the "hanging" issue (the controller never responds to a
>> basic "NOP" command) is a problem with the Coreboot driver itself, or
>> more specifically, the lack of a "quirk" handling for this controller.
>> But blaming that driver doesn't fix it.
>>
>> About the implementation. I tried to change as little code as
>> possible, both in GRUB and the Coreboot driver, thinking that it'd
>> make maintenance easier. My reasoning was (perhaps wrongly?) that
>> there is little help to get in the community, and if I reuse enough of
>> the Coreboot driver, I can easiliy re-import the driver if/when they
>> fix some bugs. This "change as little as possible" policy means that
>> now the xHCI driver initializes and tracks some state that also GRUB
>> USB stack does. It works but it's not something I think is good enough
>> to integrate in mainline GRUB. (That's one of the reasons why I didn't
>> tell you about the status earlier. I don't see how to make it "clean"
>> enough for mainline. Also, the above issues are sort of blockers in
>> themselves.)
>>
>>
>> The ugly :-)
>>
>> https://github.com/bjornfor/grub/tree/add-coreboot-xhci-
>> driver-2nd-attempt-v2
>>
>> (I just cleaned up history and rebased on master, so it's less ugly
>> than it was a few hours ago.)
>>
>> Best regards,
>> Bjørn Forsman
>>
> 


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

Re: 8250 memory mapped UART

[Andrei Borzenkov]

please test patches from Matthias Lange

https://lists.gnu.org/archive/html/grub-devel/2017-02/msg00104.html


On Wed, Mar 1, 2017 at 9:15 AM, Gailu Singh  wrote:
> Hi Experts,
>
> I am using GRUB2 on intel apollo lake board. This board does not have IO
> mapped uart instead it has 8250 memory mapped UART.
>
> GRUB2 does not recognize memory mapped uart and gives error ("serial port
> COM0 not found). There is a 8250 memory mapped driver available in coreboot.
> Is it possible to port that driver to Grub2?
>
> Thanks
>
>
> ___
> 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: USB3.0 XHCI support on GRUB2

[Andrei Borzenkov]

Yes, Bjørn Forsman intended to work on implementation.

On Wed, Mar 1, 2017 at 9:14 AM, Gailu Singh  wrote:
> Hi Experts,
>
> Is there any development on XHCI support on GRUB2? New Intel boards (e.g.
> Apollo Lake) has only XHCI controller so none my USB device (keyboarrd, mass
> storage) works on GRUB2 on this board.
>
> If there is any work going on, I can help with testing/development but don't
> know where to start from scratch.
>
> Looking forward to your help.
>
>
> ___
> 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-probe for nested BSD partition on Linux

[Andrei Borzenkov]

01.03.2017 01:05, Lennart Sorensen пишет:
> On Tue, Feb 28, 2017 at 09:50:27PM +0300, Andrei Borzenkov wrote:
>> 28.02.2017 21:31, Lennart Sorensen пишет:
>>> On Tue, Feb 28, 2017 at 08:13:53PM +0300, Andrei Borzenkov wrote:
>>>> Sorry? vda7 is 256M, how can you suddenly pretend it is 2G?
>>>>
>>>> 10:~ # fdisk -l /dev/vda
>>>> Disk /dev/vda: 5 GiB, 5368709120 bytes, 10485760 sectors
>>>> Units: sectors of 1 * 512 = 512 bytes
>>>> Sector size (logical/physical): 512 bytes / 512 bytes
>>>> I/O size (minimum/optimal): 512 bytes / 512 bytes
>>>> Disklabel type: dos
>>>> Disk identifier: 0x882b18da
>>>>
>>>> Device Boot   Start  End Sectors  Size Id Type
>>>> /dev/vda1  2048  2099199 20971521G 83 Linux
>>>> /dev/vda2   2099200  6293503 41943042G a6 OpenBSD
>>>> /dev/vda3   6293504 10485759 41922562G  5 Extended
>>>> /dev/vda5   6295552  6819839  524288  256M 83 Linux
>>>> /dev/vda6   6821888  7346175  524288  256M 83 Linux
>>>> 10:~ # fdisk -l /dev/vda2
>>>> Disk /dev/vda2: 2 GiB, 2147483648 bytes, 4194304 sectors
>>>> Geometry: 16 heads, 63 sectors/track, 10402 cylinders
>>>> Units: sectors of 1 * 512 = 512 bytes
>>>> Sector size (logical/physical): 512 bytes / 512 bytes
>>>> I/O size (minimum/optimal): 512 bytes / 512 bytes
>>>> Disklabel type: bsd
>>>>
>>>> Slice   Start  End  Sectors  Size Type Fsize Bsize Cpg
>>>> a 2099200  2623488   524289  256M boot 0 0   0
>>>> b 2099200  2623488   524289  256M unused   0 0   0
>>>> c 2099200  6293503  41943042G unused   0 0   0
>>>> d   0 10485215 104852165G unused   0 0   0
>>>>
>>>> Partition table entries are not in disk order.
>>>
>>> Well that does look mostly sane for an ancient BSD version.
>>>
>>
>> In case I was not clear.
>>
>> BSD partition a is represented in Linux as /dev/vda7. grub-probe -t
>> compatibility_hints -d /dev/vda7 returns hd0,msdos2 which is obviously
>> wrong, because it refers to 2GB area on disk while partition /dev/vda7
>> is just 256M. I would expect either hd0,msdos2,openbsd1 or nothing
>> depending on whether such nested partition is supported.
> 
> Oh now I see what you mean.  Hmm, that is an interesting problem.
> 
> It seems FFS and UFS reserve either 1 or 8KB at the start for bootblock,
> which I suppose would save the disklabel from being overwritten if using
> one of those filesystems. 

Solaris usually does not have issues with slice containing label as the
first block. Native tools (UFS, ZFS, SVM) and non-native (Veritas) all
preserve it. So I never considered it something unusual.


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

Re: grub-probe for nested BSD partition on Linux

[Andrei Borzenkov]

28.02.2017 21:31, Lennart Sorensen пишет:
> On Tue, Feb 28, 2017 at 08:13:53PM +0300, Andrei Borzenkov wrote:
>> Sorry? vda7 is 256M, how can you suddenly pretend it is 2G?
>>
>> 10:~ # fdisk -l /dev/vda
>> Disk /dev/vda: 5 GiB, 5368709120 bytes, 10485760 sectors
>> Units: sectors of 1 * 512 = 512 bytes
>> Sector size (logical/physical): 512 bytes / 512 bytes
>> I/O size (minimum/optimal): 512 bytes / 512 bytes
>> Disklabel type: dos
>> Disk identifier: 0x882b18da
>>
>> Device Boot   Start  End Sectors  Size Id Type
>> /dev/vda1  2048  2099199 20971521G 83 Linux
>> /dev/vda2   2099200  6293503 41943042G a6 OpenBSD
>> /dev/vda3   6293504 10485759 41922562G  5 Extended
>> /dev/vda5   6295552  6819839  524288  256M 83 Linux
>> /dev/vda6   6821888  7346175  524288  256M 83 Linux
>> 10:~ # fdisk -l /dev/vda2
>> Disk /dev/vda2: 2 GiB, 2147483648 bytes, 4194304 sectors
>> Geometry: 16 heads, 63 sectors/track, 10402 cylinders
>> Units: sectors of 1 * 512 = 512 bytes
>> Sector size (logical/physical): 512 bytes / 512 bytes
>> I/O size (minimum/optimal): 512 bytes / 512 bytes
>> Disklabel type: bsd
>>
>> Slice   Start  End  Sectors  Size Type Fsize Bsize Cpg
>> a 2099200  2623488   524289  256M boot 0 0   0
>> b 2099200  2623488   524289  256M unused   0 0   0
>> c 2099200  6293503  41943042G unused   0 0   0
>> d   0 10485215 104852165G unused   0 0   0
>>
>> Partition table entries are not in disk order.
> 
> Well that does look mostly sane for an ancient BSD version.
> 

In case I was not clear.

BSD partition a is represented in Linux as /dev/vda7. grub-probe -t
compatibility_hints -d /dev/vda7 returns hd0,msdos2 which is obviously
wrong, because it refers to 2GB area on disk while partition /dev/vda7
is just 256M. I would expect either hd0,msdos2,openbsd1 or nothing
depending on whether such nested partition is supported.

> Slice a is the rootfs for BSD
> 
> Slice b should traditionally be swap, but appears to be unused here,
> and for some reason uses the same part of the disk as slice a, which
> does look odd.  Maybe since the type is unknown, it doesn't matter.
> 
> Slice c is the whole partition containing the slices (vda2)
> 
> Slice d is the whole disk (vda), which apparently hasn't been used in
> BSD for a couple of decades now, if wikipedia can be trusted.
> 
> So if linux only bothers to count the partition with a type that isn't
> unused, then it probably makes sense.  That appears to be what the
> partition parsing code does.
> 
> The start being the same as the start of vda2 does seem odd given it
> appears that you are supposed to leave space at the start (1 track
> recommended, or 64 sectors), and it appears the disklabel is at sector
> 1 (0 being the first), so it doesn't seem like having your filesystem
> start there would be safe unless the filesystem has reserved space at
> the start for such things.
> 

I just pressed 'b' in Linux fdisk and only change two partition sizes. I
have no idea how fdisk choses partition start. Could be a bug in fdisk.

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

grub-probe for nested BSD partition on Linux (was: [PATCH 1/1] add --partuuid to probe)

[Andrei Borzenkov]

28.02.2017 17:08, Vladimir 'phcoder' Serbinenko пишет:
>>
>> OK I see, kernel skips BSD partition marked as "unused".
>>
>> So it appears that kernel always puts special nested partitions after
>> normal logical MSDOS partitions, so it will not skew MSDOS partition
>> numbers.
>>
>> [1.529752]  vda: vda1 vda2 vda3 < vda5 vda6 >
>> vda2: 
>>
>>
>> /*
>>  * Look for partitions in two passes:
>>  * First find the primary and DOS-type extended partitions.
>>  * On the second pass look inside *BSD, Unixware and Solaris
>> partitions.
>>  */
>>
>> For such partition (vda7) PARTUUID is empty.
>>
>> P.S. I wonder whether we correctly map such partition ... no, we do not.
>>
>> 10:~ # cat /tmp/foo
>> (hd0) /dev/vda
>> 10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda2
>> hd0,msdos2
>> 10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda5
>> hd0,msdos5
>> 10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda7
>> hd0,msdos2
>>
> WAI. In case when subpartition starts at the same sector as partition
> itself.
> 

Sorry? vda7 is 256M, how can you suddenly pretend it is 2G?

10:~ # fdisk -l /dev/vda
Disk /dev/vda: 5 GiB, 5368709120 bytes, 10485760 sectors
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: dos
Disk identifier: 0x882b18da

Device Boot   Start  End Sectors  Size Id Type
/dev/vda1  2048  2099199 20971521G 83 Linux
/dev/vda2   2099200  6293503 41943042G a6 OpenBSD
/dev/vda3   6293504 10485759 41922562G  5 Extended
/dev/vda5   6295552  6819839  524288  256M 83 Linux
/dev/vda6   6821888  7346175  524288  256M 83 Linux
10:~ # fdisk -l /dev/vda2
Disk /dev/vda2: 2 GiB, 2147483648 bytes, 4194304 sectors
Geometry: 16 heads, 63 sectors/track, 10402 cylinders
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: bsd

Slice   Start  End  Sectors  Size Type Fsize Bsize Cpg
a 2099200  2623488   524289  256M boot 0 0   0
b 2099200  2623488   524289  256M unused   0 0   0
c 2099200  6293503  41943042G unused   0 0   0
d   0 10485215 104852165G unused   0 0   0

Partition table entries are not in disk order.


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

Re: [PATCH 1/1] add --partuuid to probe

[Andrei Borzenkov]

27.02.2017 21:20, Vladimir 'phcoder' Serbinenko пишет:
> On Mon, Feb 27, 2017, 09:55 Andrei Borzenkov <arvidj...@gmail.com> wrote:
> 
>> 27.02.2017 03:37, Vladimir 'phcoder' Serbinenko пишет:
>> ...
>>>>>>> This is not NT-style. NT uses partition offset. Who uses this format?
>>>> Are
>>>>>>
>>>>>> This is used by util-linux and Linux kernel.
>>>>>>
>>>>>>
>>>>>>  *  6) PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF representing
>>>> the
>>>>>>  * unique id of a partition if the partition table provides
>> it.
>>>>>>  * The UUID may be either an EFI/GPT UUID, or refer to an
>> MSDOS
>>>>>>  * partition using the format -PP, where  is a
>>>>>> zero-
>>>>>>  * filled hex representation of the 32-bit "NT disk
>> signature",
>>>>>> and PP
>>>>>>  * is a zero-filled hex representation of the 1-based
>> partition
>>>>>> number.
>>>>>>
>>>>>>> you sure that partition numbers are synced with user? Even in
>> presence
>>>> of
>>>>>>> Solaris and bsd partitions.
>>>>>>>
>>>>>>
>>>>>> It is not clear what we should return for nested partition. I'm not
>> sure
>>>>>> whether linux kernel scans nested partitions at all in which case we
>>>>>> probably should follow the suite and assign PARTUUID to top-level
>>>>>> partitions only.
>>>>>>
>>>>> Linux scans nested partitions and it uses though numeration in
>> dev/sdaX,
>>>> in
>>>>> some cases shifting numbering of normal partitions. In those cases grub
>>>> and
>>>>> Linux numeration get out of sync
>>>>>
>>>>
>>>> Can you provide example?
>>>
>>> Bsd and Solaris partitions. I remember we had problem with numbering of
>>> those.
>>>
>>
>> Linux ignores nested BSD partitions (just tested). There are no special
>> files created. Of course someone needs to test what happens under
>> *BSD/Solaris in this case.
>>
> Kpartx or normal sdX? Is bsd-partition support enabled in kernel build?
> 

OK I see, kernel skips BSD partition marked as "unused".

So it appears that kernel always puts special nested partitions after
normal logical MSDOS partitions, so it will not skew MSDOS partition
numbers.

[1.529752]  vda: vda1 vda2 vda3 < vda5 vda6 >
vda2: 


/*
 * Look for partitions in two passes:
 * First find the primary and DOS-type extended partitions.
 * On the second pass look inside *BSD, Unixware and Solaris
partitions.
 */

For such partition (vda7) PARTUUID is empty.

P.S. I wonder whether we correctly map such partition ... no, we do not.

10:~ # cat /tmp/foo
(hd0) /dev/vda
10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda2
hd0,msdos2
10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda5
hd0,msdos5
10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda7
hd0,msdos2


>>
>> I never liked idea of artificial partition GUIDs for MBR, but as long as
>> only Linux is using them and we are consistent with its usage - so be it.
>>
>>>> I tried to create nested partition table, but
>>>> Linux will not display it (actually attempt to "blockdev --rereadpt
>>>> /dev/vda5" fails with "Invalid argument").
>>>>
>>>> if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains)
>>>> return -EINVAL;
>>>>
>>>> Where bdev->bd_contains points to containing device for partition and to
>>>> itself for the whole disk.
>>>>
>>>> As util-linux does not scan partition table itself, it does show these
>>>> nested partitions either.


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

Re: [GRUB PARTUUID PATCH V3 5/5] Harmonize patches

[Andrei Borzenkov]

27.02.2017 03:34, Vladimir 'phcoder' Serbinenko пишет:
>>if (state[6].set)
>>  {
>>char *partuuid = NULL; /* NULL to silence a spurious GCC warning */
>> -  /* Nested partitions are not supported for now. */
>> -  /* Non-nested partitions must have dev->disk->partition->parent ==
>> NULL */
>> -  if (dev->disk && dev->disk->partition &&
>> dev->disk->partition->parent == NULL)
>> +  /*
>> +   * Nested partitions are not supported for now.
>> +   * Non-nested partitions must have dev->disk->partition->parent ==
>> NULL
>> +   */
>>
> I'm not sure that this is enough. In presence of nested partitions Linux
> may shift extended partitions as well. I think we need to do tests with
> qemu and bsd-formatted partitions
> 

In my testing Linux never sees nested partitions (it never scans
partitions for nested partitions). For all I can tell this check has
been there at least since 2006 (I'm lacy to look beyond introduction of
block/ directory).

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

Re: [PATCH 1/1] add --partuuid to probe

[Andrei Borzenkov]

27.02.2017 03:37, Vladimir 'phcoder' Serbinenko пишет:
...
> This is not NT-style. NT uses partition offset. Who uses this format?
>> Are

 This is used by util-linux and Linux kernel.


  *  6) PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF representing
>> the
  * unique id of a partition if the partition table provides it.
  * The UUID may be either an EFI/GPT UUID, or refer to an MSDOS
  * partition using the format -PP, where  is a
 zero-
  * filled hex representation of the 32-bit "NT disk signature",
 and PP
  * is a zero-filled hex representation of the 1-based partition
 number.

> you sure that partition numbers are synced with user? Even in presence
>> of
> Solaris and bsd partitions.
>

 It is not clear what we should return for nested partition. I'm not sure
 whether linux kernel scans nested partitions at all in which case we
 probably should follow the suite and assign PARTUUID to top-level
 partitions only.

>>> Linux scans nested partitions and it uses though numeration in dev/sdaX,
>> in
>>> some cases shifting numbering of normal partitions. In those cases grub
>> and
>>> Linux numeration get out of sync
>>>
>>
>> Can you provide example?
> 
> Bsd and Solaris partitions. I remember we had problem with numbering of
> those.
> 

Linux ignores nested BSD partitions (just tested). There are no special
files created. Of course someone needs to test what happens under
*BSD/Solaris in this case.

I never liked idea of artificial partition GUIDs for MBR, but as long as
only Linux is using them and we are consistent with its usage - so be it.

>> I tried to create nested partition table, but
>> Linux will not display it (actually attempt to "blockdev --rereadpt
>> /dev/vda5" fails with "Invalid argument").
>>
>> if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains)
>> return -EINVAL;
>>
>> Where bdev->bd_contains points to containing device for partition and to
>> itself for the whole disk.
>>
>> As util-linux does not scan partition table itself, it does show these
>> nested partitions either.
>>


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

Re: Loading DSDT table using 'acpi' or some memory write command?

[Andrei Borzenkov]

On Mon, Feb 27, 2017 at 2:29 PM, Nando Eva <nando4...@ymail.com> wrote:
> @Andrei Borzenkov, I've confirmed that any of the following do not alter the
> Win10 DSDT table across numerous Macbooks and my Dell E6540.
>
> acpi dsdt.aml
> acpi --load-only dsdt dsdt.aml
> acpi -2 dsdt.aml
>
> Followed by:
> chainloader /efi/Microsoft/Boot/bootmgfw.efi
>
> Clover has a DSDT override function and using it works on my Dell E6540.
> However it does it by writing to the UEFI firmware volume, which for
> Macbooks, has had one test instance of bricking the system.
>
> Any chance of a simple  'load_dsdt [file]' command that acquires the
> in-memory DSDT table address, checks it's size, then overwrites it with the
> file name specified if it's the same size or smaller?
>

That's more or less what grub tries to do. We cannot really overwrite
ACPI tables because they may be located in read-only memory, but it
attempts to create EBDA and place new RSDP there and update EBDA
address as well as update RSDP pointer in EFI system table. I would
suggest trying to load something else (like Linux) and check whether
it sees new or old table.

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

Re: [GRUB PARTUUID PATCH V3 4/5] Support both EFI and NT Disk Signature for passing to kernel as root=PARTUUID=$val

[Andrei Borzenkov]

Отправлено с iPhone

> 27 февр. 2017 г., в 3:35, Vladimir 'phcoder' Serbinenko  
> написал(а):
> 
> 
> 
>> On Sun, Feb 26, 2017, 16:22 Nicholas Vinson  wrote:
>> From: Steve Kenton 
> 
> Please avoid resubmitting patches made by someone else in most cases. It 
> obscures proper attribution. We can review his patch in his thread


In defense, the idea of consolidated patch series was mine, so blame me if you 
think it was wrong.


>> 
>> ---
>>  grub-core/commands/probe.c | 59 
>> ++
>>  1 file changed, 59 insertions(+)
>> 
>> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
>> index cf2793e1d..5dd1a6bc5 100644
>> --- a/grub-core/commands/probe.c
>> +++ b/grub-core/commands/probe.c
>> @@ -16,6 +16,7 @@
>>   *  along with GRUB.  If not, see .
>>   */
>> 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -24,6 +25,8 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -45,6 +48,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",   'g', 0, N_("Determine partition GUID/UUID."), 0, 0}, 
>> /* GUID but Linux kernel calls it "PARTUUID" */
>>  {0, 0, 0, 0, 0, 0}
>>};
>> 
>> @@ -154,6 +158,61 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc, 
>> char **args)
>>grub_device_close (dev);
>>return GRUB_ERR_NONE;
>>  }
>> +  if (state[6].set)
>> +{
>> +  char *partuuid = NULL; /* NULL to silence a spurious GCC warning */
>> +  /* Nested partitions are not supported for now. */
>> +  /* Non-nested partitions must have dev->disk->partition->parent == 
>> NULL */
>> +  if (dev->disk && dev->disk->partition && dev->disk->partition->parent 
>> == NULL)
>> +   {
>> +grub_partition_t p = dev->disk->partition;
>> +if (grub_strcmp (p->partmap->name, "msdos") == 0)
>> +  {
>> + /* little-endian 4-byte NT disk id "GUID" in the MBR */
>> + grub_uint8_t diskid[4];
>> +dev->disk->partition = p->parent;
>> +grub_uint32_t nt_disk_sig;
>> +err = grub_disk_read (dev->disk, 0, 
>> GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC, sizeof(diskid), diskid);
>> +dev->disk->partition = p;
>> +if (err)
>> +  return grub_errno;
>> +/* partition numbers are one-based */
>> +partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
>> +   diskid[3], diskid[2], diskid[1], 
>> disk[0],
>> +   p->number + 1);
>> +  }
>> +else if (grub_strcmp (p->partmap->name, "gpt") == 0)
>> +  {
>> +struct grub_gpt_partentry e;
>> +dev->disk->partition = p->parent;
>> +err = grub_disk_read (dev->disk, p->offset, p->index, sizeof e, 
>> );
>> +dev->disk->partition = p;
>> +if (err)
>> +  return grub_errno;
>> +
>> +partuuid = grub_xasprintf 
>> ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
>> +   e.guid[3], e.guid[2], e.guid[1], 
>> e.guid[0],
>> +   e.guid[5], e.guid[4],
>> +   e.guid[7], e.guid[6],
>> +   e.guid[8], e.guid[9],
>> +   e.guid[10], e.guid[11], e.guid[12], 
>> e.guid[13], e.guid[14], e.guid[15]);
>> +  }
>> +else
>> +  return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
>> + N_("partition map %s does not support 
>> partition UUIDs"),
>> + dev->disk->partition->partmap->name);
>> +   }
>> +  else
>> +   partuuid = grub_strdup (""); /* a freeable empty string */
>> +
>> +  if (state[0].set)
>> +   grub_env_set (state[0].arg, partuuid);
>> +  else
>> +   grub_printf ("%s", partuuid);
>> +  grub_free (partuuid);
>> +  grub_device_close (dev);
>> +  return GRUB_ERR_NONE;
>> +}
>>grub_device_close (dev);
>>return grub_error (GRUB_ERR_BAD_ARGUMENT, "unrecognised target");
>>  }
>> --
>> 2.12.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
___
Grub-devel mailing list

sqaush4 tests (was: Re: [PATCH] squash4: fix handling of fragments and sparse files)

[Andrei Borzenkov]

25.02.2017 18:23, Andrei Borzenkov пишет:
> 24.02.2017 19:19, Andrei Borzenkov пишет:
>>> How do you create those files reliably? Feel free to add any files to
>>> grub-fs-tester. Feel free to commit without tests, we can add tests later.
>>>
>>
>> I would say, we need to generally create less "nice" test files
>>
>> 1. Generate test files that are not exact multiple of filesystem block.
>> This would cover squash tail packing and may uncover similar issues in
>> other drivers as well.
>>
>> 2. Generate sparse files by seeking beyond end of file. We already had
>> similar problem with indirect ext* blocks that was not caught by tests.
>> We should produce holes both for direct and indirect blocks (we may pick
>> common offset or make it fs-specific if necessary).
>>
>> 3. We need to produce really small files also to test inlining. Again we
>> can pick common size or make it fs-specific.
>>
>> Assuming we generate files as described, for squash4 just call it with
>> -always-use-fragments to force tail packing. -nopad may be interesting
>> as well to stress our driver even more.
>>
>>
> 
> Attached patch implements 1 + -always-use-fragments. It reliably causes
> squash4 test to fail without my patch. I can change BLOCKCNT for squash4
> only, but I think it makes sense to do globally.
> 
> sparse files need some tweaking in garbage-gen, will look into it.
> 

Sorry, wrong patch attached.
From: Andrei Borzenkov <arvidj...@gmail.com>
Subject: [PATCH] grub-fs-tester: improve squash4 tests

1. Make sure files are not multiple of block size. This will ensure tail packing
for squash4 and may also trigger more codes paths in other filesystems.

2. Call mksquashfs with -always-use-fragments to force tail packing.


---
 tests/util/grub-fs-tester.in | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index f363d6f..2337771 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -913,6 +913,9 @@ for ((LOGSECSIZE=MINLOGSECSIZE;LOGSECSIZE<=MAXLOGSECSIZE;LOGSECSIZE=LOGSECSIZE +
 		*)
 		BLOCKCNT=5242880;;
 	esac
+	# Make sure file is not exact multiple of block size. This helps to force
+	# tail packing in case of squash4.
+	: $((BLOCKCNT--))
 	case x"$fs" in
 		x"ntfscomp")
 		setfattr -h -v 0x0800 -n system.ntfs_attrib_be "$MNTPOINTRW/$OSDIR";;
@@ -998,8 +1001,8 @@ for ((LOGSECSIZE=MINLOGSECSIZE;LOGSECSIZE<=MAXLOGSECSIZE;LOGSECSIZE=LOGSECSIZE +
 		x"romfs")
 		genromfs -V "$FSLABEL" -f "${FSIMAGES[0]}" -d "$MASTER" ;;
 		xsquash4_*)
-		echo mksquashfs "$MASTER" "${FSIMAGES[0]}" -comp "${fs/squash4_/}" -b $BLKSIZE
-		mksquashfs "$MASTER" "${FSIMAGES[0]}" -comp "${fs/squash4_/}" -b $BLKSIZE ;;
+		echo mksquashfs "$MASTER" "${FSIMAGES[0]}" -always-use-fragments -comp "${fs/squash4_/}" -b $BLKSIZE
+		mksquashfs "$MASTER" "${FSIMAGES[0]}" -always-use-fragments -comp "${fs/squash4_/}" -b $BLKSIZE ;;
 		x"bfs")
 		sleep 1
 		fusermount -u "$MNTPOINTRW"
-- 
tg: (892dfbe..) u/squash4-tests (depends on: master)
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] squash4: fix handling of fragments and sparse files

[Andrei Borzenkov]

24.02.2017 19:19, Andrei Borzenkov пишет:
>> How do you create those files reliably? Feel free to add any files to
>> grub-fs-tester. Feel free to commit without tests, we can add tests later.
>>
> 
> I would say, we need to generally create less "nice" test files
> 
> 1. Generate test files that are not exact multiple of filesystem block.
> This would cover squash tail packing and may uncover similar issues in
> other drivers as well.
> 
> 2. Generate sparse files by seeking beyond end of file. We already had
> similar problem with indirect ext* blocks that was not caught by tests.
> We should produce holes both for direct and indirect blocks (we may pick
> common offset or make it fs-specific if necessary).
> 
> 3. We need to produce really small files also to test inlining. Again we
> can pick common size or make it fs-specific.
> 
> Assuming we generate files as described, for squash4 just call it with
> -always-use-fragments to force tail packing. -nopad may be interesting
> as well to stress our driver even more.
> 
> 

Attached patch implements 1 + -always-use-fragments. It reliably causes
squash4 test to fail without my patch. I can change BLOCKCNT for squash4
only, but I think it makes sense to do globally.

sparse files need some tweaking in garbage-gen, will look into it.
From: Andrei Borzenkov <arvidj...@gmail.com>
Subject: [PATCH] grub-fs-tester: improve squash4 tests

1. Make sure files are not multiple of block size. This will ensure tail packing
for squash4 and may also trigger more codes paths in other filesystems.

2. Call mksquashfs with -always-use-fragments to force tail packing.


---
 tests/util/grub-fs-tester.in | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index f363d6f..a9771f3 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -913,6 +913,8 @@ for ((LOGSECSIZE=MINLOGSECSIZE;LOGSECSIZE<=MAXLOGSECSIZE;LOGSECSIZE=LOGSECSIZE +
 		*)
 		BLOCKCNT=5242880;;
 	esac
+	# Make sure file is not exact multiple of block size
+	: $((BLOCKCNT--))
 	case x"$fs" in
 		x"ntfscomp")
 		setfattr -h -v 0x0800 -n system.ntfs_attrib_be "$MNTPOINTRW/$OSDIR";;
@@ -998,8 +1000,8 @@ for ((LOGSECSIZE=MINLOGSECSIZE;LOGSECSIZE<=MAXLOGSECSIZE;LOGSECSIZE=LOGSECSIZE +
 		x"romfs")
 		genromfs -V "$FSLABEL" -f "${FSIMAGES[0]}" -d "$MASTER" ;;
 		xsquash4_*)
-		echo mksquashfs "$MASTER" "${FSIMAGES[0]}" -comp "${fs/squash4_/}" -b $BLKSIZE
-		mksquashfs "$MASTER" "${FSIMAGES[0]}" -comp "${fs/squash4_/}" -b $BLKSIZE ;;
+		echo mksquashfs "$MASTER" "${FSIMAGES[0]}" -always-use-fragments -nopad -comp "${fs/squash4_/}" -b $BLKSIZE
+		mksquashfs "$MASTER" "${FSIMAGES[0]}" -always-use-fragments -nopad -comp "${fs/squash4_/}" -b $BLKSIZE ;;
 		x"bfs")
 		sleep 1
 		fusermount -u "$MNTPOINTRW"
-- 
tg: (892dfbe..) u/squash4-tests (depends on: master)
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] efi: strip off final NULL from File Path in grub_efi_get_filename

[Andrei Borzenkov]

25.02.2017 02:21, Vladimir 'phcoder' Serbinenko пишет:
> On Fri, Feb 24, 2017, 09:47 Andrei Borzenkov <arvidj...@gmail.com> wrote:
> 
>> UEFI 2.6 9.3.6.4 File Path Media Device Path says that Path Name is
>> "A NULL-terminated Path string including directory and file names".
>>
>> Strip final NULL from Path Name in each File Path node when constructing
>> full path. To be on safe side, strip all of them.
>>
>> Fixes failure chainloading grub from grub, when loaded grub truncates
>> image path and does not find its grub.cfg.
>>
>> https://bugzilla.opensuse.org/show_bug.cgi?id=1026344
>>
>> This was triggered by commit ce95549cc54b5d6f494608a7c390dba3aab4fba7;
>> before it we built Path Name without trailing NULL, and apparently all
>> other bootloaders use single File Path node, thus not exposing this bug.
>>
>> ---
>> @Vladimir, @Daniel - this fixes regression in rc1 and is real fix for
>> the mentioned problem (previous patch only fixed the case of grub - grub
>> chainloading, this patch should properly parse image path also when called
>> from other EFI application).
>>
> I'm OK with this patch. Especially that it's unlikely to break anything. Is

Committed.

> the other patch still needed? If other loaders do it with a single path
> element we should probably too, to avoid this kind of bugs. Question is
> mostly whether it's rc1 material.
> 

It is not needed. I would still consider it long term - it makes code
less confusing and avoids probably less-utilized code paths elsewhere.

It also demonstrates that we desperately need EFI boot tests. Much
thanks to SUSE for setting up comprehensive test suite!


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

Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8

[Andrei Borzenkov]

25.02.2017 02:22, Vladimir 'phcoder' Serbinenko пишет:
> On Sun, Feb 12, 2017, 11:52 Vladimir 'phcoder' Serbinenko <phco...@gmail.com>
> wrote:
> 
>>
>>
>> On Sun, 12 Feb 2017, 08:06 Andrei Borzenkov <arvidj...@gmail.com> wrote:
>>
>> 12.02.2017 00:07, Vladimir 'phcoder' Serbinenko пишет:
>> ...
>>>>
>>> For multiboot module old grub was passing module file name on module
>>> command line, so legacy_configfile should replicate this behaviour. But
>> in
>>> Linux case it shouldn't have ended up loading twice. We need additional
>>> code in legacy_initrd. Please try the attached patch
>>>
>> ...
>>>
>>>
>>> diff --git a/grub-core/commands/legacycfg.c
>> b/grub-core/commands/legacycfg.c
>>> index dd9d9f1..24546bf 100644
>>> --- a/grub-core/commands/legacycfg.c
>>> +++ b/grub-core/commands/legacycfg.c
>>> @@ -517,7 +517,7 @@ grub_cmd_legacy_initrd (struct grub_command *mycmd
>> __attribute__ ((unused)),
>>>  #endif
>>>  );
>>>
>>> -  return cmd->func (cmd, argc, args);
>>> +  return cmd->func (cmd, argc ? argc - 1 : 0, args + 1);
>>
>> If the goal is to be compatible with legacy grub, it is not compatible
>> (nor was) - legacy grub "initrd" ignored all extra arguments to "initrd"
>> command while we attempt to load them as additional initrd components.
>>
>> Nor did legacy grub attempt to quote command line for multiboot kernel
>> or modules.
>>
>> Agreed on both. That's why I said that previous patch is wrong and I'm
>> working on new one
>>
> For first point, see attached patch.
> Second point isn't a problem as legacycfg will already split along spaces
> and hence quoting code will not be triggered.
> 

It will.

  while (*c)
{
  if (*c == '\\' || *c == '\'' || *c == '"')
*buf++ = '\\';
  *buf++ = *c;
  c++;
}

In particular, this is a problem with Linux which does not interpret
these quotes at all (it only supports "  " without any way to escape
nested `"'). That's something for post 2.02 now, but I still would like
to know what was the reason for this code in the first place.

> 
> diff --git a/grub-core/commands/legacycfg.c b/grub-core/commands/legacycfg.c
> index dd9d9f1..b32f3c7 100644
> --- a/grub-core/commands/legacycfg.c
> +++ b/grub-core/commands/legacycfg.c
> @@ -517,7 +517,7 @@ grub_cmd_legacy_initrd (struct grub_command *mycmd
> __attribute__ ((unused)),
> #endif
> );
> 
> - return cmd->func (cmd, argc, args);
> + return cmd->func (cmd, argc ? 1 : 0, args);
> }
> if (kernel_type == MULTIBOOT)
> {
> 

Looks OK as stop gap. @Andy, could you test it? 

I still do not understand why initrd is parsed as TYPE_FILE_NO_CONSUME,
nor why we handle multiboot kernel in legacy_initrd in the first place.
Legacy grub only supported Linux kernel with initrd command.

  switch (kernel_type)
{
case KERNEL_TYPE_LINUX:
case KERNEL_TYPE_BIG_LINUX:
  if (! load_initrd (arg))
return 1;
  break;

default:
  errnum = ERR_NEED_LX_KERNEL;
  return 1;
}

Although this probably does not really matter now.

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

Re: Loading DSDT table using 'acpi' or some memory write command?

[Andrei Borzenkov]

23.02.2017 19:00, Nando Eva пишет:
> Hi grub-devel, I'm endeavouring to pre-load a modified DSDT table 
> using grub2. The syntax I've tried being as shown below, after which 
> I chainload to Win10. This being done on a Dell E6540 with Win10 and 
> grub 2.02 (or older.. tried many versions, same result).
> 
> acpi /efi/dsdt.amlacpi --load-table dsdt /efi/dsdt.amlchainloader 
> /efi/Boot/bootx64.efi Then I do 'acpidump -b' to peruse the DSDT 
> table. In all cases, it remains the same system BIOS one, not my 
> modified /efi/dsdt.aml one.
> 
> I've also set 'set debug=all' and can see grub2 is reading my 
> modified dsdt file and doing memory writes. So two questions: 1. Is 
> 'acpi' not capable of loading a DSDT table for use with Win10?

It is supposed to do it. Could you test with explicit "acpi -(1|2)" to
force either version 1 or version 2 of tables.

> 2. if that is the case, is there some 'memload' or 'writefromfile' 
> type command where I give a memory address and a file which grub2
> can then write? The idea is simply to replace the existing DSDT at
> it's address with one given by the file of same size or smaller.
> Thank you,Nando
> 


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

Re: [PATCH] squash4: fix handling of fragments and sparse files

[Andrei Borzenkov]

23.02.2017 16:52, Vladimir 'phcoder' Serbinenko пишет:
> On Sat, Feb 18, 2017, 10:17 Andrei Borzenkov <arvidj...@gmail.com> wrote:
> 
>> 1. Do not assume block list and fragment are mutually exclusive. Squash
>> can pack file tail as fragment (unless -no-fragments is specified); so
>> check read offset and read either from block list or from fragments as
>> appropriate.
>>
>> 2. Support sparse files with zero blocks.
>>
>> 3. Fix fragment read - frag.offset is absolute fragment position,
>> not offset relative to ino.chunk.
>>
> Go ahead.
> 

Done with cosmetic changes (renamed one variable to better match
surrounding code and removed now superfluous assignment).

>>
>> Reported and tested by Carlo Caione <ca...@endlessm.com>
>>
>> ---
>>
>> @Vladimir: we need regression tests for both of these cases. We could real
>> small
>> files only by accident - block list was zero, so it appeared to work.
>>
> How do you create those files reliably? Feel free to add any files to
> grub-fs-tester. Feel free to commit without tests, we can add tests later.
> 

I would say, we need to generally create less "nice" test files

1. Generate test files that are not exact multiple of filesystem block.
This would cover squash tail packing and may uncover similar issues in
other drivers as well.

2. Generate sparse files by seeking beyond end of file. We already had
similar problem with indirect ext* blocks that was not caught by tests.
We should produce holes both for direct and indirect blocks (we may pick
common offset or make it fs-specific if necessary).

3. We need to produce really small files also to test inlining. Again we
can pick common size or make it fs-specific.

Assuming we generate files as described, for squash4 just call it with
-always-use-fragments to force tail packing. -nopad may be interesting
as well to stress our driver even more.



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

[PATCH] efi: strip off final NULL from File Path in grub_efi_get_filename

[Andrei Borzenkov]

UEFI 2.6 9.3.6.4 File Path Media Device Path says that Path Name is
"A NULL-terminated Path string including directory and file names".

Strip final NULL from Path Name in each File Path node when constructing
full path. To be on safe side, strip all of them.

Fixes failure chainloading grub from grub, when loaded grub truncates
image path and does not find its grub.cfg.

https://bugzilla.opensuse.org/show_bug.cgi?id=1026344

This was triggered by commit ce95549cc54b5d6f494608a7c390dba3aab4fba7;
before it we built Path Name without trailing NULL, and apparently all
other bootloaders use single File Path node, thus not exposing this bug.

---
@Vladimir, @Daniel - this fixes regression in rc1 and is real fix for
the mentioned problem (previous patch only fixed the case of grub - grub
chainloading, this patch should properly parse image path also when called
from other EFI application).

 grub-core/kern/efi/efi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index caf9bcc..d467785 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -366,6 +366,9 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0)
  len = ((GRUB_EFI_DEVICE_PATH_LENGTH (dp) - 4)
 / sizeof (grub_efi_char16_t));
  fp = (grub_efi_file_path_device_path_t *) dp;
+ /* According to EFI spec Path Name is NULL terminated */
+ while (len > 0 && fp->path_name[len - 1] == 0)
+   len--;
 
  p = (char *) grub_utf16_to_utf8 ((unsigned char *) p, fp->path_name, 
len);
}
-- 
tg: (2fb8cd2..) u/efi-strip-final-NULL-from-file-path (depends on: master)

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

Should Path Name in File Path Media Device Path node be NULL terminated?

[Andrei Borzenkov]

Historically grub2 built image paths using two File Path nodes - one for
directory and one for file name relative to directory. These nodes had
path names that were not NULL terminated.

Recently we had bug report that secure boot using grub2 failed. It was
tracked down to exactly the fact that paths were not NULL terminated.
See
http://git.savannah.gnu.org/cgit/grub.git/commit/?id=ce95549cc54b5d6f494608a7c390dba3aab4fba7

Unfortunately this caused another regression which looks like firmware
truncating passed image path on first NULL

https://bugzilla.opensuse.org/show_bug.cgi?id=1026344

Could someone clarify what is expected by EFI spec? Should each Path
Name (even intermediate) be NULL terminated, or spec intends to say that
only full path must be NULL terminated?

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

[PATCH RFC] efi/chainloader: build image path as single File Path

[Andrei Borzenkov]

According to EFI 9.3.6.4 File Path Media Device Path, Path Name is

A NULL-terminated Path string including directory and file
names.

EFI chainloader is using two File Path nodes for image name - directory and
file. It appears that at least some firmware implementations interpret NULL
terminated directory path as end of full pathname, thus truncating it.

OTOH leaving path names non-NULL terminated caused other errors (see
commit ce95549cc54b5d6f494608a7c390dba3aab4fba7)

Use single File Path node containing both directory and file name to avoid it.

See also https://bugzilla.opensuse.org/show_bug.cgi?id=1026344

---

Anyone remembers why we build complete path using two nodes? As far as I can
tell no other bootloader does it. I am really uneasy to do it now, but we have
known bugs both with and without ce95549cc54b5d6f494608a7c390dba3aab4fba7 and
this looks like the most straightforward fix.

 grub-core/loader/efi/chainloader.c | 16 +---
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/grub-core/loader/efi/chainloader.c 
b/grub-core/loader/efi/chainloader.c
index adc8563..329dd57 100644
--- a/grub-core/loader/efi/chainloader.c
+++ b/grub-core/loader/efi/chainloader.c
@@ -158,29 +158,23 @@ make_file_path (grub_efi_device_path_t *dp, const char 
*filename)
   d = GRUB_EFI_NEXT_DEVICE_PATH (d);
 }
 
-  /* File Path is NULL terminated. Allocate space for 2 extra characters */
-  /* FIXME why we split path in two components? */
+  /* File Path is NULL terminated. Allocate space for extra character */
   file_path = grub_malloc (size
-  + ((grub_strlen (dir_start) + 2)
+  + ((grub_strlen (dir_start) + 1)
  * GRUB_MAX_UTF16_PER_UTF8
  * sizeof (grub_efi_char16_t))
-  + sizeof (grub_efi_file_path_device_path_t) * 2);
+  + sizeof (grub_efi_file_path_device_path_t));
   if (! file_path)
 return 0;
 
   grub_memcpy (file_path, dp, size);
 
-  /* Fill the file path for the directory.  */
+  /* Fill the complete file path.  */
   d = (grub_efi_device_path_t *) ((char *) file_path
  + ((char *) d - (char *) dp));
   grub_efi_print_device_path (d);
   copy_file_path ((grub_efi_file_path_device_path_t *) d,
- dir_start, dir_end - dir_start);
-
-  /* Fill the file path for the file.  */
-  d = GRUB_EFI_NEXT_DEVICE_PATH (d);
-  copy_file_path ((grub_efi_file_path_device_path_t *) d,
- dir_end + 1, grub_strlen (dir_end + 1));
+ dir_start, grub_strlen (dir_start));
 
   /* Fill the end of device path nodes.  */
   d = GRUB_EFI_NEXT_DEVICE_PATH (d);
-- 
tg: (2fb8cd2..) u/use-single-file-path-in-efi-chainloader (depends on: master)

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

kernel build still pulls in host includes even with -nostdinc

[Andrei Borzenkov]

Could someone comment on this?

gcc -DHAVE_CONFIG_H -I. -I..  -Wall -W  -DGRUB_MACHINE_PCBIOS=1
-DGRUB_MACHINE=I386_PC -m32 -nostdinc -isystem
/usr/lib/gcc/x86_64-linux-gnu/5/include

So we do have definitions from /usr/lib/gcc/x86_64-linux-gnu/5/include used.

How harmless/harmful is it?

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

Re: [PATCH 1/1 V2] add --partuuid to probe

[Andrei Borzenkov]

18.02.2017 18:31, Steve Kenton пишет:
> Signed-off-by: Steve Kenton 
> ---
> V2 coding style changes as suggested in feedback
> In V1 I was treating p->index as an offset into the partition table instead
> of an index from 0-15 of the partition table slot needing to be multipled
> by the size of a gtp partition table entry. Is this correct? There are two

E-h-h ... either it returns the correct GUID or not, right? :)

> temporary comments about the include files used to remove the magic numbers,
> If you would prefer others just let me know.
> 
>  grub-core/commands/probe.c | 59 
> ++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
> index cf2793e..662c0a3 100644
> --- a/grub-core/commands/probe.c
> +++ b/grub-core/commands/probe.c
> @@ -16,6 +16,7 @@
>   *  along with GRUB.  If not, see .
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -24,6 +25,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -45,6 +48,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", 'g', 0, N_("Determine partition GUID/UUID."), 0, 0}, /* 
> GUID but Linux kernel calls it "PARTUUID" */
>  {0, 0, 0, 0, 0, 0}
>};
>  
> @@ -154,6 +158,61 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc, 
> char **args)
>grub_device_close (dev);
>return GRUB_ERR_NONE;
>  }
> +  if (state[6].set)
> +{
> +  char *partuuid = NULL; /* NULL to silence a spurious GCC warning */
> +  grub_uint8_t diskbuf[16];
> +  if (dev->disk && dev->disk->partition)
> + {
> +   grub_partition_t p = dev->disk->partition;
> +   if (grub_strcmp (p->partmap->name, "msdos") == 0)

As discussed in other thread, we probably need to skip nested partitions
here.

> + {
> +   /* 440 location of NT Volume Label in MBR 
> ./include/grub/i386/pc/boot.h */
> +   const int diskid_offset = GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC;
> +   dev->disk->partition = p->parent;
> +   /* little-endian 4-byte NT disk id */
> +   err = grub_disk_read (dev->disk, 0, diskid_offset, 4, diskbuf);
> +   dev->disk->partition = p;
> +   if (err)
> + return grub_errno;
> +   partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
> +  diskbuf[3], diskbuf[2], diskbuf[1], 
> diskbuf[0],
> +  p->number + 1); /* one based partition 
> number */
> + }
> +   else if (grub_strcmp (p->partmap->name, "gpt") == 0)
> + {
> +   const int guid_offset = p->index * sizeof(struct 
> grub_gpt_partentry) + 

p->index is absolute byte offset from the start of partition entries array.

> +   /* 16 location of GUID in entry ./include/grub/gpt_partition.h */
> +   offsetof(struct grub_gpt_partentry, guid);

I scratched my head how it compiled at all and then I noticed that in my
case we have

gcc -DHAVE_CONFIG_H -I. -I..  -Wall -W  -DGRUB_MACHINE_PCBIOS=1
-DGRUB_MACHINE=I386_PC -m32 -nostdinc -isystem
/usr/lib/gcc/x86_64-linux-gnu/5/include ...

So we do pull in definitions from host even into kernel build (and of
course offset() is defined in stddef.h in this directory).

This looks like something to solve irrespectively; in your case I
suggest you simply read the whole grub_gpt_partentry to avoid.

Although having offsetof() is actually not that bad.


> +   dev->disk->partition = p->parent;
> +   /* little-endian 16-byte EFI partition GUID */
> +   err = grub_disk_read (dev->disk, p->offset, guid_offset, 16, 
> diskbuf);
> +   dev->disk->partition = p;
> +   if (err)
> + return grub_errno;
> +   partuuid = grub_xasprintf 
> ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
> +  diskbuf[3], diskbuf[2], diskbuf[1], 
> diskbuf[0],
> +  diskbuf[5], diskbuf[4],
> +  diskbuf[7], diskbuf[6],
> +  diskbuf[8], diskbuf[9],
> +  diskbuf[10], diskbuf[11], diskbuf[12], 
> diskbuf[13], diskbuf[14], diskbuf[15]);
> + }
> +   else
> + return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
> +N_("partition map %s does not support partition 
> UUIDs"),
> +dev->disk->partition->partmap->name);
> + }
> +  else
> + partuuid = grub_strdup (""); /* 

Re: [PATCH 1/1] add --partuuid to probe

[Andrei Borzenkov]

15.02.2017 20:25, Vladimir 'phcoder' Serbinenko пишет:
> On Wed, Feb 15, 2017, 17:27 Andrei Borzenkov <arvidj...@gmail.com> wrote:
> 
>> 15.02.2017 13:56, Vladimir 'phcoder' Serbinenko пишет:
>>> On Tue, Feb 14, 2017, 19:01 Steve Kenton <sken...@ou.edu> wrote:
>>>
>>>> Support both EFI and NT Disk Signature for passing to kernel as
>>>> root=PARTUUID=$val
>>>>
>>>> Signed-off-by: Steve Kenton <sken...@ou.edu>
>>>> ---
>>>> It's been six months so I thought I'd resend this so it does not get
>> lost
>>>> in case I get hit by a meteor or something before the next release
>>>>
>>>>  grub-core/commands/probe.c | 53
>>>> ++
>>>>  1 file changed, 53 insertions(+)
>>>>
>>>> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
>>>> index cf2793e..3afc8b8 100644
>>>> --- a/grub-core/commands/probe.c
>>>> +++ b/grub-core/commands/probe.c
>>>> @@ -45,6 +45,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",   'g', 0, N_("Determine partition GUID/UUID."), 0,
>>>> 0}, /* GUID but Linux kernel calls it "PARTUUID" */
>>>>
>>> I like how it generalizes.
>>>
>>>>  {0, 0, 0, 0, 0, 0}
>>>>};
>>>>
>>>> @@ -154,6 +155,58 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int
>> argc,
>>>> char **args)
>>>>grub_device_close (dev);
>>>>return GRUB_ERR_NONE;
>>>>  }
>>>> +  if (state[6].set)
>>>> +{
>>>> +  char *partuuid = NULL; /* NULL to silence a spurious GCC warning
>> */
>>>> +  grub_uint8_t diskbuf[16];
>>>> +  if (dev->disk && dev->disk->partition)
>>>> +   {
>>>> + grub_partition_t p = dev->disk->partition;
>>>> + if (!grub_strcmp (p->partmap->name, "msdos"))
>>>>
>>> Please use == 0 rather than !
>>>
>>>> +   {
>>>> + const int diskid_offset = 440; /* location in MBR */
>>>>
>>> Please get this from a common header rather than hard coding. I think we
>>> have it in msdos.h
>>>
>>>> + dev->disk->partition = p->parent;
>>>> + /* little-endian 4-byte NT disk signature */
>>>> + err = grub_disk_read (dev->disk, 0, diskid_offset, 4,
>>>> diskbuf);
>>>> + dev->disk->partition = p;
>>>> + if (err)
>>>> +   return grub_errno;
>>>> + partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
>>>> +diskbuf[3], diskbuf[2],
>>>> diskbuf[1], diskbuf[0],
>>>> +p->number + 1); /* one based
>>>> partition number */
>>>>
>>> This is not NT-style. NT uses partition offset. Who uses this format? Are
>>
>> This is used by util-linux and Linux kernel.
>>
>>
>>  *  6) PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF representing the
>>  * unique id of a partition if the partition table provides it.
>>  * The UUID may be either an EFI/GPT UUID, or refer to an MSDOS
>>  * partition using the format -PP, where  is a
>> zero-
>>  * filled hex representation of the 32-bit "NT disk signature",
>> and PP
>>  * is a zero-filled hex representation of the 1-based partition
>> number.
>>
>>> you sure that partition numbers are synced with user? Even in presence of
>>> Solaris and bsd partitions.
>>>
>>
>> It is not clear what we should return for nested partition. I'm not sure
>> whether linux kernel scans nested partitions at all in which case we
>> probably should follow the suite and assign PARTUUID to top-level
>> partitions only.
>>
> Linux scans nested partitions and it uses though numeration in dev/sdaX, in
> some cases shifting numbering of normal partitions. In those cases grub and
> Linux numeration get out of sync
> 

Can you provide example? I tried to create nested partition table, but
Linux will not display it (actually attempt to "blockdev --rereadpt
/dev/vda5" fails with "Invalid argument").

if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains)
return -EINVAL;

Where bdev->bd_contains points to containing device for partition and to
itself for the whole disk.

As util-linux does not scan partition table itself, it does show these
nested partitions either.

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

[PATCH] squash4: fix handling of fragments and sparse files

[Andrei Borzenkov]

1. Do not assume block list and fragment are mutually exclusive. Squash
can pack file tail as fragment (unless -no-fragments is specified); so
check read offset and read either from block list or from fragments as
appropriate.

2. Support sparse files with zero blocks.

3. Fix fragment read - frag.offset is absolute fragment position,
not offset relative to ino.chunk.

Reported and tested by Carlo Caione 

---

@Vladimir: we need regression tests for both of these cases. We could real small
files only by accident - block list was zero, so it appeared to work.

@Carlo, please test. I'm surprised it worked for you even without fragments as
your file is sparse and grub immediately choked on the first zero block.

 grub-core/fs/squash4.c | 55 +-
 1 file changed, 36 insertions(+), 19 deletions(-)

diff --git a/grub-core/fs/squash4.c b/grub-core/fs/squash4.c
index b97b344..deee71a 100644
--- a/grub-core/fs/squash4.c
+++ b/grub-core/fs/squash4.c
@@ -823,7 +823,12 @@ direct_read (struct grub_squash_data *data,
   curread = data->blksz - boff;
   if (curread > len)
curread = len;
-  if (!(ino->block_sizes[i]
+  if (!ino->block_sizes[i])
+   {
+ /* Sparse block */
+ grub_memset (buf, '\0', curread);
+   }
+  else if (!(ino->block_sizes[i]
& grub_cpu_to_le32_compile_time (SQUASH_BLOCK_UNCOMPRESSED)))
{
  char *block;
@@ -873,36 +878,57 @@ direct_read (struct grub_squash_data *data,
 
 
 static grub_ssize_t
-grub_squash_read_data (struct grub_squash_data *data, 
-  struct grub_squash_cache_inode *ino,
-  grub_off_t off, char *buf, grub_size_t len)
+grub_squash_read (grub_file_t file, char *buf, grub_size_t len)
 {
+  struct grub_squash_data *data = file->data;
+  struct grub_squash_cache_inode *ino = >ino;
+  grub_off_t off = file->offset;
   grub_err_t err;
   grub_uint64_t a = 0, b;
   grub_uint32_t fragment = 0;
   int compressed = 0;
   struct grub_squash_frag_desc frag;
+  grub_off_t blk_len;
+  grub_uint64_t mask = grub_le_to_cpu32 (data->sb.block_size) - 1;
+  grub_size_t orig_len = len;
 
   switch (ino->ino.type)
 {
 case grub_cpu_to_le16_compile_time (SQUASH_TYPE_LONG_REGULAR):
-  a = grub_le_to_cpu64 (ino->ino.long_file.chunk);
   fragment = grub_le_to_cpu32 (ino->ino.long_file.fragment);
   break;
 case grub_cpu_to_le16_compile_time (SQUASH_TYPE_REGULAR):
-  a = grub_le_to_cpu32 (ino->ino.file.chunk);
   fragment = grub_le_to_cpu32 (ino->ino.file.fragment);
   break;
 }
 
-  if (fragment == 0x)
-return direct_read (data, ino, off, buf, len);
+  /* Squash may pack file tail as fragment. So read initial part directly and
+ get tail from fragments */
+  blk_len  = fragment == 0x ? file->size : file->size & ~mask;
+  if (off < blk_len)
+{
+  grub_size_t read_len = blk_len - off;
+  grub_ssize_t res;
+
+  if (read_len > len)
+   read_len = len;
+  res = direct_read (data, ino, off, buf, read_len);
+  if ((grub_size_t) res != read_len)
+   return -1; /* FIXME: is short read possible here? */
+  len -= read_len;
+  if (!len)
+   return read_len;
+  buf += read_len;
+  off = 0;
+}
+  else
+off -= blk_len;
  
   err = read_chunk (data, , sizeof (frag),
data->fragments, sizeof (frag) * fragment);
   if (err)
 return -1;
-  a += grub_le_to_cpu64 (frag.offset);
+  a = grub_le_to_cpu64 (frag.offset);
   compressed = !(frag.size & grub_cpu_to_le32_compile_time 
(SQUASH_BLOCK_UNCOMPRESSED));
   if (ino->ino.type == grub_cpu_to_le16_compile_time 
(SQUASH_TYPE_LONG_REGULAR))
 b = grub_le_to_cpu32 (ino->ino.long_file.offset) + off;
@@ -943,16 +969,7 @@ grub_squash_read_data (struct grub_squash_data *data,
   if (err)
return -1;
 }
-  return len;
-}
-
-static grub_ssize_t
-grub_squash_read (grub_file_t file, char *buf, grub_size_t len)
-{
-  struct grub_squash_data *data = file->data;
-
-  return grub_squash_read_data (data, >ino,
-   file->offset, buf, len);
+  return orig_len;
 }
 
 static grub_err_t
-- 
tg: (2fb8cd2..) u/squash-tail-fragment (depends on: master)

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

Re: [PATCH 1/1] add --partuuid to probe

[Andrei Borzenkov]

15.02.2017 13:56, Vladimir 'phcoder' Serbinenko пишет:
> On Tue, Feb 14, 2017, 19:01 Steve Kenton  wrote:
> 
>> Support both EFI and NT Disk Signature for passing to kernel as
>> root=PARTUUID=$val
>>
>> Signed-off-by: Steve Kenton 
>> ---
>> It's been six months so I thought I'd resend this so it does not get lost
>> in case I get hit by a meteor or something before the next release
>>
>>  grub-core/commands/probe.c | 53
>> ++
>>  1 file changed, 53 insertions(+)
>>
>> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
>> index cf2793e..3afc8b8 100644
>> --- a/grub-core/commands/probe.c
>> +++ b/grub-core/commands/probe.c
>> @@ -45,6 +45,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",   'g', 0, N_("Determine partition GUID/UUID."), 0,
>> 0}, /* GUID but Linux kernel calls it "PARTUUID" */
>>
> I like how it generalizes.
> 
>>  {0, 0, 0, 0, 0, 0}
>>};
>>
>> @@ -154,6 +155,58 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc,
>> char **args)
>>grub_device_close (dev);
>>return GRUB_ERR_NONE;
>>  }
>> +  if (state[6].set)
>> +{
>> +  char *partuuid = NULL; /* NULL to silence a spurious GCC warning */
>> +  grub_uint8_t diskbuf[16];
>> +  if (dev->disk && dev->disk->partition)
>> +   {
>> + grub_partition_t p = dev->disk->partition;
>> + if (!grub_strcmp (p->partmap->name, "msdos"))
>>
> Please use == 0 rather than !
> 
>> +   {
>> + const int diskid_offset = 440; /* location in MBR */
>>
> Please get this from a common header rather than hard coding. I think we
> have it in msdos.h
> 
>> + dev->disk->partition = p->parent;
>> + /* little-endian 4-byte NT disk signature */
>> + err = grub_disk_read (dev->disk, 0, diskid_offset, 4,
>> diskbuf);
>> + dev->disk->partition = p;
>> + if (err)
>> +   return grub_errno;
>> + partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
>> +diskbuf[3], diskbuf[2],
>> diskbuf[1], diskbuf[0],
>> +p->number + 1); /* one based
>> partition number */
>>
> This is not NT-style. NT uses partition offset. Who uses this format? Are

This is used by util-linux and Linux kernel.


 *  6) PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF representing the
 * unique id of a partition if the partition table provides it.
 * The UUID may be either an EFI/GPT UUID, or refer to an MSDOS
 * partition using the format -PP, where  is a zero-
 * filled hex representation of the 32-bit "NT disk signature",
and PP
 * is a zero-filled hex representation of the 1-based partition
number.

> you sure that partition numbers are synced with user? Even in presence of
> Solaris and bsd partitions.
> 

It is not clear what we should return for nested partition. I'm not sure
whether linux kernel scans nested partitions at all in which case we
probably should follow the suite and assign PARTUUID to top-level
partitions only.

>> +   }
>> + else if (!grub_strcmp (p->partmap->name, "gpt"))
>>
> Ditto.
> 
>> +   {
>> + const int guid_offset = 16; /* location in entry */
>>
> Ditto.
> 
>> + dev->disk->partition = p->parent;
>> + /* little-endian 16-byte EFI partition GUID */
>> + err = grub_disk_read (dev->disk, p->offset, p->index +
>> guid_offset, 16, diskbuf);
>> + dev->disk->partition = p;
>> + if (err)
>> +   return grub_errno;
>> + partuuid = grub_xasprintf
>> ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
>> +diskbuf[3], diskbuf[2],
>> diskbuf[1], diskbuf[0],
>> +diskbuf[5], diskbuf[4],
>> +diskbuf[7], diskbuf[6],
>> +diskbuf[8], diskbuf[9],
>> +diskbuf[10], diskbuf[11],
>> diskbuf[12], diskbuf[13], diskbuf[14], diskbuf[15]);
>> +   }
>> + else
>> +   return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
>> +  N_("partition map %s does not support
>> partition UUIDs"),
>> +  dev->disk->partition->partmap->name);
>> +   }
>> +  else
>> +   partuuid = grub_strdup (""); /* a freeable empty string */
>> +
>> +  if (state[0].set)
>> +   grub_env_set (state[0].arg, partuuid);
>> +  else
>> +   grub_printf ("%s", 

Re: [PATCH 1/1] add --partuuid to probe

[Andrei Borzenkov]

14.02.2017 21:00, Steve Kenton пишет:
> Support both EFI and NT Disk Signature for passing to kernel as 
> root=PARTUUID=$val
> 

Yes, I guess we need to add it finally. Unfortunately it is too late for
2.02, but it should go after in release. There were also patches for
grub-probe and we also need to support it in search to be on par with
filesystem UUID. May be if you could prepare consolidated patch set it
would be great.

I apologize that it tool so long. Thank you for not giving up!

> Signed-off-by: Steve Kenton 
> ---
> It's been six months so I thought I'd resend this so it does not get lost
> in case I get hit by a meteor or something before the next release
> 
>  grub-core/commands/probe.c | 53 
> ++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
> index cf2793e..3afc8b8 100644
> --- a/grub-core/commands/probe.c
> +++ b/grub-core/commands/probe.c

Please also add patch for manual.

> @@ -45,6 +45,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", 'g', 0, N_("Determine partition GUID/UUID."), 0, 0}, /* 
> GUID but Linux kernel calls it "PARTUUID" */
>  {0, 0, 0, 0, 0, 0}
>};
>  
> @@ -154,6 +155,58 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc, 
> char **args)
>grub_device_close (dev);
>return GRUB_ERR_NONE;
>  }
> +  if (state[6].set)
> +{
> +  char *partuuid = NULL; /* NULL to silence a spurious GCC warning */
> +  grub_uint8_t diskbuf[16];
> +  if (dev->disk && dev->disk->partition)
> + {
> +   grub_partition_t p = dev->disk->partition;
> +   if (!grub_strcmp (p->partmap->name, "msdos"))
> + {
> +   const int diskid_offset = 440; /* location in MBR */
> +   dev->disk->partition = p->parent;
> +   /* little-endian 4-byte NT disk signature */
> +   err = grub_disk_read (dev->disk, 0, diskid_offset, 4, diskbuf);
> +   dev->disk->partition = p;
> +   if (err)
> + return grub_errno;
> +   partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
> +  diskbuf[3], diskbuf[2], diskbuf[1], 
> diskbuf[0],
> +  p->number + 1); /* one based partition 
> number */
> + }
> +   else if (!grub_strcmp (p->partmap->name, "gpt"))
> + {
> +   const int guid_offset = 16; /* location in entry */
> +   dev->disk->partition = p->parent;
> +   /* little-endian 16-byte EFI partition GUID */
> +   err = grub_disk_read (dev->disk, p->offset, p->index + 
> guid_offset, 16, diskbuf);
> +   dev->disk->partition = p;
> +   if (err)
> + return grub_errno;
> +   partuuid = grub_xasprintf 
> ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
> +  diskbuf[3], diskbuf[2], diskbuf[1], 
> diskbuf[0],
> +  diskbuf[5], diskbuf[4],
> +  diskbuf[7], diskbuf[6],
> +  diskbuf[8], diskbuf[9],
> +  diskbuf[10], diskbuf[11], diskbuf[12], 
> diskbuf[13], diskbuf[14], diskbuf[15]);
> + }
> +   else
> + return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
> +N_("partition map %s does not support partition 
> UUIDs"),
> +dev->disk->partition->partmap->name);
> + }
> +  else
> + partuuid = grub_strdup (""); /* a freeable empty string */
> +
> +  if (state[0].set)
> + grub_env_set (state[0].arg, partuuid);
> +  else
> + grub_printf ("%s", partuuid);
> +  grub_free (partuuid);
> +  grub_device_close (dev);
> +  return GRUB_ERR_NONE;
> +}
>grub_device_close (dev);
>return grub_error (GRUB_ERR_BAD_ARGUMENT, "unrecognised target");
>  }
> 


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

Re: [PATCH v2] mips64: Add support for 64-bit MIPS.

[Andrei Borzenkov]

13.02.2017 18:57, wa...@lemote.com пишет:
> From: Heiher 
> 

Before going in more depth for review, could you give more details which
platforms are using it, what ABI is targeted (n64? something else?) Is
MIPS going to be included in official UEFI spec?

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

Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8

[Andrei Borzenkov]

12.02.2017 00:07, Vladimir 'phcoder' Serbinenko пишет:
...
>>
> For multiboot module old grub was passing module file name on module
> command line, so legacy_configfile should replicate this behaviour. But in
> Linux case it shouldn't have ended up loading twice. We need additional
> code in legacy_initrd. Please try the attached patch
> 
...
> 
> 
> diff --git a/grub-core/commands/legacycfg.c b/grub-core/commands/legacycfg.c
> index dd9d9f1..24546bf 100644
> --- a/grub-core/commands/legacycfg.c
> +++ b/grub-core/commands/legacycfg.c
> @@ -517,7 +517,7 @@ grub_cmd_legacy_initrd (struct grub_command *mycmd 
> __attribute__ ((unused)),
>  #endif
>  );
>  
> -  return cmd->func (cmd, argc, args);
> +  return cmd->func (cmd, argc ? argc - 1 : 0, args + 1);

If the goal is to be compatible with legacy grub, it is not compatible
(nor was) - legacy grub "initrd" ignored all extra arguments to "initrd"
command while we attempt to load them as additional initrd components.

Nor did legacy grub attempt to quote command line for multiboot kernel
or modules.

>  }
>if (kernel_type == MULTIBOOT)
>  {
> 
> 
> 
> ___
> 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] script: fix double free in lexer

[Andrei Borzenkov]

yylex_destroy() already frees scanner.

Found by: Coverity scan.
CID: 176636

---
 grub-core/script/lexer.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/grub-core/script/lexer.c b/grub-core/script/lexer.c
index 89cf677..c6bd317 100644
--- a/grub-core/script/lexer.c
+++ b/grub-core/script/lexer.c
@@ -251,7 +251,6 @@ grub_script_lexer_init (struct grub_parser_param *parser, 
char *script,
 {
   parser->lexerstate = 0;
   yylex_destroy (lexerstate->yyscanner);
-  grub_free (lexerstate->yyscanner);
   grub_free (lexerstate->text);
   grub_free (lexerstate);
   return 0;
-- 
tg: (5298187..) coverity/176636 (depends on: master)

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

Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8

[Andrei Borzenkov]

11.02.2017 20:19, Andy Smith пишет:
> Hello,
> 
> On Sat, Feb 11, 2017 at 03:26:43PM +, Andy Smith wrote:
>> Here is what the config looks like if I interrupt the boot and go to
>> edit it:
>>
>> set root='(hd0,1)'; set legacy_hdbias='0'
>> legacy_kernel   '/boot/vmlinuz-3.16.0-4-amd64' 
>> '/boot/vmlinuz-3.16.0-4-amd64' 
>> 'root=UUID=38420e46-6123-477d-ba23-baeba8ac0d59' 'ro'
>> legacy_initrd '/boot/initrd.img-3.16.0-4-amd64' 
>> '/boot/initrd.img-3.16.0-4-amd64'
> 
> Over on the xen-users list, Sarah Newman pointed out that the
> legacy_initrd line should not have a repeated argument:
> 
> https://lists.xenproject.org/archives/html/xen-users/2017-02/msg00029.html
> 
> If I edit away the repetition, so that the config line is:
> 
> legacy_initrd '/boot/initrd.img-3.16.0-4-amd64'
> 
> then this boots fine.
> 

This was explicitly added in commit
9fb175ed9a48102543867f8006842628cc41217c. I do not understand the
purpose of this (is there any requirement that multiboot kernel/module
gets its name as the first parameter?)

@Vladimir? Either we need to revert this, or add similar handling to
legacy_initrd, but I miss why we need it in the first place.

> Should I report this in grub's bugzilla?
> 

Sure. Although I still wonder why it fails. I.e. kernel should simply
extract the same initrd content twice, why it causes such effect?

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

Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8

[Andrei Borzenkov]

11.02.2017 16:53, Andy Smith пишет:
...
> 
> if search -s -f /grub/menu.lst ; then
> set root=(xen/xvda,msdos1)
> echo "Reading (${root})/grub/menu.lst"
> read
> legacy_configfile /grub/menu.lst
> fi
> --
> 
> Contents of /boot/grub/menu.lst in guest:
> 
> --
> default 0
> timeout 5
> color cyan/blue white/blue
> title   Debian GNU/Linux, kernel 3.16.0-4-amd64
> root(hd0,0)
> kernel  /boot/vmlinuz-3.16.0-4-amd64 
> root=UUID=38420e46-6123-477d-ba23-baeba8ac0d59 ro 
> initrd  /boot/initrd.img-3.16.0-4-amd64
> --
> 

What happens if you load the same kernel manually in dom0?

linux /boot/vmlinuz ...
initrd /boot/initrd ...

(adjust paths and $root as needed)?


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

Re: Quation about build grub source code

[Andrei Borzenkov]

09.02.2017 19:46, Andre Bigijani пишет:
> hi,
> I'm develop my own simple os and for boot it i wants to use grub
> how can i compile grub source code with cygwin64 in windows for get
> loader.o file?
> 

INSTALL lists prerequisite packages and how to build from snaoshot. If
you follow INSTALL and get errors during build, post them here.

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

Re: [PATCH] Make grub-install check for errors from efibootmgr

[Andrei Borzenkov]

30.01.2017 22:04, Steve McIntyre пишет:
> Code is currently ignoring errors from efibootmgr, giving users
> clearly bogus output like:
> 
> Setting up grub-efi-amd64 (2.02~beta3-4) ...
> Installing for x86_64-efi platform.
> Could not delete variable: No space left on device
> Could not prepare Boot variable: No space left on device
> Installation finished. No error reported.
> 
> and then potentially unbootable systems. If efibootmgr fails,
> grub-install should know that and report it!
> 

This looks more or less cosmetic to me. First, errors are displayed to
user so it is not that user is not aware. Second, efibootmgr is more or
less optional. This is convenient but by far not the only one
possibility to use newly installed grub. Third, even successful
execution of efibootmgr does not mean it will actually boot grub - there
are enough systems out there that will simply ditch grub entry and
replace it with hard coded Windows one.

So I'm fine with changing "no error reported" to "efibootmgr invocation
failed; check your firmware settings" or similar, but I am not sure we
need to abort grub-install in this case. What exact problem do you solve
by aborting?

> Signed-off-by: Steve McIntyre <93...@debian.org>
> ---
>  grub-core/osdep/unix/platform.c | 24 +++-
>  include/grub/util/install.h |  2 +-
>  util/grub-install.c | 14 +++---
>  3 files changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
> index 28cb37e..f9c376c 100644
> --- a/grub-core/osdep/unix/platform.c
> +++ b/grub-core/osdep/unix/platform.c
> @@ -78,19 +78,20 @@ get_ofpathname (const char *dev)
>  dev);
>  }
>  
> -static void
> +static int
>  grub_install_remove_efi_entries_by_distributor (const char *efi_distributor)
>  {
>int fd;
>pid_t pid = grub_util_exec_pipe ((const char * []){ "efibootmgr", NULL }, 
> );
>char *line = NULL;
>size_t len = 0;
> +  int error = 0;
>  
>if (!pid)
>  {
>grub_util_warn (_("Unable to open stream from %s: %s"),
> "efibootmgr", strerror (errno));
> -  return;
> +  return errno;
>  }
>  
>FILE *fp = fdopen (fd, "r");
> @@ -98,7 +99,7 @@ grub_install_remove_efi_entries_by_distributor (const char 
> *efi_distributor)
>  {
>grub_util_warn (_("Unable to open stream from %s: %s"),
> "efibootmgr", strerror (errno));
> -  return;
> +  return errno;
>  }
>  
>line = xmalloc (80);
> @@ -119,23 +120,25 @@ grub_install_remove_efi_entries_by_distributor (const 
> char *efi_distributor)
>bootnum = line + sizeof ("Boot") - 1;
>bootnum[4] = '\0';
>if (!verbosity)
> - grub_util_exec ((const char * []){ "efibootmgr", "-q",
> + error = grub_util_exec ((const char * []){ "efibootmgr", "-q",
> "-b", bootnum,  "-B", NULL });
>else
> - grub_util_exec ((const char * []){ "efibootmgr",
> + error = grub_util_exec ((const char * []){ "efibootmgr",
> "-b", bootnum, "-B", NULL });
>  }
>  
>free (line);
> +  return error;
>  }
>  
> -void
> +int
>  grub_install_register_efi (grub_device_t efidir_grub_dev,
>  const char *efifile_path,
>  const char *efi_distributor)
>  {
>const char * efidir_disk;
>int efidir_part;
> +  int error = 0;
>efidir_disk = grub_util_biosdisk_get_osdev (efidir_grub_dev->disk);
>efidir_part = efidir_grub_dev->disk->partition ? 
> efidir_grub_dev->disk->partition->number + 1 : 1;
>  
> @@ -151,23 +154,26 @@ grub_install_register_efi (grub_device_t 
> efidir_grub_dev,
>grub_util_exec ((const char * []){ "modprobe", "-q", "efivars", NULL });
>  #endif
>/* Delete old entries from the same distributor.  */
> -  grub_install_remove_efi_entries_by_distributor (efi_distributor);
> +  error = grub_install_remove_efi_entries_by_distributor (efi_distributor);
> +  if (error)
> +return error;
>  
>char *efidir_part_str = xasprintf ("%d", efidir_part);
>  
>if (!verbosity)
> -grub_util_exec ((const char * []){ "efibootmgr", "-q",
> +error = grub_util_exec ((const char * []){ "efibootmgr", "-q",
> "-c", "-d", efidir_disk,
> "-p", efidir_part_str, "-w",
> "-L", efi_distributor, "-l", 
> efifile_path, NULL });
>else
> -grub_util_exec ((const char * []){ "efibootmgr",
> +error = grub_util_exec ((const char * []){ "efibootmgr",
> "-c", "-d", efidir_disk,
> "-p", efidir_part_str, "-w",
> "-L", efi_distributor, "-l", 
> efifile_path, NULL });
>free (efidir_part_str);
> +  return error;
>  }
>  
>  void
> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
> index 9f517a1..58648e2 100644
> --- a/include/grub/util/install.h
> +++ b/include/grub/util/install.h
> @@ -209,7 +209,7 @@ 

Re: explicit refresh?

[Andrei Borzenkov]

08.02.2017 11:29, Laszlo Ersek пишет:
> Hi,
> 
> this is a dumb question and it's not appropriate for a development
> mailing list, but I couldn't find "grub-user" or "grub-users", so I'll
> ask here anyway...
> 

there is help-g...@gnu.org

> My laptop is connected to my workstation via USB-serial (USB for the
> laptop, serial for the workstation). Sometimes, by the time the laptop
> comes up and starts "screen", for viewing the serial output from the
> workstation, the workstation is already displaying the grub menu.
> Occasionally in this case, I can't see anything at all, there's no grub
> countdown, the cursor up/down keys do nothing (they certainly don't
> refresh the screen), but if I blindly press Enter, then the selected
> kernel entry is booted okay (and that kernel does produce serial output,
> and takes the LUKS password from serial fine as well).
> 
> So, Q1: what could cause the loss of serial output from grub when I
> connect to it "too late"?
> 
> Q2: when that happens, can I press something like C-r to force a full
> serial / screen refresh?
> 

There are no hot keys for "refresh everything". May be adding it would
be not so bad idea.

> I know that USB-serial adaptors are generally wacky, but this is a
> Prolific PL2303 adaptor which is (to my knowlede) usually considered
> reliable. Plus, my laptop has no physical RS/232 port (shame on you, all
> laptop vendors of today!), so this is the best I can do.
> 
> Thanks,
> Laszlo
> 
> ___
> 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: xen: Fix handling of GRUB chainloading.

[Andrei Borzenkov]

I'm not familiar with code, so go ahead if you are sure it is bug fix.

Отправлено с iPhone

> 7 февр. 2017 г., в 0:50, Vladimir 'phcoder' Serbinenko  
> написал(а):
> 
> In case of GRUB we put remapper after domain pages and not at 0x0.
> In this case we use max_addr to put remapper. Unfortunately we increment
> max_addr as well in this case resulting in virt mapping mapping page
> at old max_addr and trying to boot using new max_addr.
> 
> Daniel, Andrei, Alexander, do you agree that it should go into 2.02?
> 
> Closes 46014.
> ---
>  grub-core/loader/i386/xen.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> index 51d1ddd..3073f64 100644
> --- a/grub-core/loader/i386/xen.c
> +++ b/grub-core/loader/i386/xen.c
> @@ -419,8 +419,6 @@ grub_xen_pt_alloc (void)
>try_virt_end = ALIGN_UP (xen_state.xen_inf.virt_base +
>  page2offset (nr_need_pages) +
>  ADDITIONAL_SIZE + STACK_SIZE, ALIGN_SIZE);
> -  if (!xen_state.xen_inf.virt_base)
> - try_virt_end += PAGE_SIZE;
>  
>err = get_pgtable_size (xen_state.xen_inf.virt_base, try_virt_end,
> nr_info_pages);
> @@ -433,7 +431,7 @@ grub_xen_pt_alloc (void)
>if (xen_state.xen_inf.virt_base)
>   err = get_pgtable_size (0, PAGE_SIZE, nr_need_pages);
>else
> - err = get_pgtable_size (try_virt_end - PAGE_SIZE, try_virt_end,
> + err = get_pgtable_size (try_virt_end, try_virt_end + PAGE_SIZE,
>   nr_need_pages);
>if (err)
>   return err;
> -- 
> 2.9.3
> 
> ___
> 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: xen: Fix parsing of XZ kernel.

[Andrei Borzenkov]

Отправлено с iPhone

> 7 февр. 2017 г., в 0:51, Vladimir 'phcoder' Serbinenko  
> написал(а):
> 
> In case of xz, the uncompressed size is appended to xz data which confuses
> our xz decompressor. Trim it.
> 
> Daniel, Andrei, Alexander, do you agree that this should go into 2.02?
> ---
>  grub-core/loader/i386/xen_file.c | 30 --
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/grub-core/loader/i386/xen_file.c 
> b/grub-core/loader/i386/xen_file.c
> index 37f9ad8..eabaf8f 100644
> --- a/grub-core/loader/i386/xen_file.c
> +++ b/grub-core/loader/i386/xen_file.c
> @@ -26,6 +26,8 @@ grub_xen_file (grub_file_t file)
>grub_elf_t elf;
>struct linux_kernel_header lh;
>grub_file_t off_file;
> +  grub_uint32_t payload_offset, payload_length;
> +  grub_uint8_t magic[6];
>  
>elf = grub_elf_file (file, file->name);
>if (elf)
> @@ -46,20 +48,36 @@ grub_xen_file (grub_file_t file)
>return NULL;
>  }
>  
> -  if (lh.payload_length < 4)
> +  payload_length = lh.payload_length;
> +  payload_offset = (lh.setup_sects + 1) * 512
> ++ lh.payload_offset;
> +
> +  if (payload_length < sizeof (magic))
>  {
>grub_error (GRUB_ERR_BAD_OS, "payload too short");
>return NULL;
>  }
>  
>grub_dprintf ("xen", "found bzimage payload 0x%llx-0x%llx\n",
> - (unsigned long long) (lh.setup_sects + 1) * 512
> - + lh.payload_offset,
> + (unsigned long long) payload_offset,
>   (unsigned long long) lh.payload_length);
>  
> -  off_file = grub_file_offset_open (file, (lh.setup_sects + 1) * 512
> - + lh.payload_offset,
> - lh.payload_length);
> +  grub_file_seek (file, payload_offset);
> +
> +  if (grub_file_read (file, , sizeof (magic)) != sizeof (magic))
> +{
> +  if (!grub_errno)
> + grub_error (GRUB_ERR_BAD_OS, N_("premature end of file %s"),
> + file->name);
> +  goto fail;
> +}
> +
> +  /* Kernel suffixes xz payload with their uncompressed size.
> + Trim it.  */
> +  if (grub_memcmp (magic, "\3757zXZ\0", 6) == 0)

s/6/sizeof (magic)/ and magic string should better be defined (do not we use it 
already somewhere?)

ok otherwise 

> +payload_length -= 4;
> +  off_file = grub_file_offset_open (file, payload_offset,
> + payload_length);
>if (!off_file)
>  goto fail;
>  
> -- 
> 2.9.3
> 
> ___
> 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: Booting a system in a specific language

[Andrei Borzenkov]

04.02.2017 20:47, scootergrisen пишет:
> Den 04-02-2017 kl. 18:26 skrev Andrei Borzenkov:
>> 04.02.2017 20:04, scootergrisen пишет:
>>> I test out different Linux distributions and some distributions offer to
>>> select language during boot and some does not.
>>>
>>> It helps when i can select language and keyboard layout etc. during boot
>>> so i dont have to change the language etc. from inside GNOME or what
>>> ever.
>>>
>>> But does GRUB have anything to do about booting a system in a specific
>>> language or does each distribution modify GRUB to be able to change
>>> language.
>>>
>>> Some distributions seem to be able to change the language of the GRUB
>>> boot menu and some does not.
>>>
>>
>> Internally GRUB supports l10n (reimplementation of gettext). Some
>> distributions install language catalogs; how they select them is
>> distribution-dependent. But grub-mkconfig and scripts that are shipped
>> with GRUB itself do not make use of this and emit strings in fixed
>> language, as set at the time grub-mkconfig runs so you cannot change
>> menu language without regenerating it. That would be something to
>> consider after 2.02 release.
>>
>> As for keyboard layout, unfortunately only at_keyboard supports run-time
>> switch and it apparently has problems on real hardware. Default BIOS
>> input driver has fixed layout. There was proposed patch to implement
>> layouts on top of extended keyboard BIOS interface.
> 
> For example if i try Ubuntu live image.
> I'm not sure if it uses GRUB but i think so.
> When the image starts and i press a button i can select language.
> 
> And i get something that looks a bit like
> https://help.ubuntu.com/community/Grub2?action=AttachFile=get=grub2.theme.bennett.png
> where you have F2 and F3 at the bottom to select language and keyboard
> layout.
> 
> If i select my language the menus changes to my language and also the
> keyboard layout changes. This i really nice i think but it seems most
> distribution dont offer this on boot... so i have to use GRUB in english
> and change language when i get into GNOME or what ever.
> 
> Where does this "F1 Help F2 Language F3 Keymap..." come from?
> 

Most likely syslinux

> In Debian for example i have to manually type what language to start in
> and keymap to use and this is not so nice because first i have to
> remember what to type (cant use webbrowser to search for it) and
> different distributions seems to use different codes and also the menu
> during boot is in english and the keyboard layout is incorrect so it
> makes it harder to type in the codes like "=" and "_" and "-" because
> they are not on the correct places during boot.
> 
> Basically i'm trying to help get Linux distributions better in my
> langauge so we can have danish translation from the beginning like the
> Ubuntu live image i tested.
> 
> ___
> 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: Booting a system in a specific language

[Andrei Borzenkov]

04.02.2017 20:04, scootergrisen пишет:
> I test out different Linux distributions and some distributions offer to
> select language during boot and some does not.
> 
> It helps when i can select language and keyboard layout etc. during boot
> so i dont have to change the language etc. from inside GNOME or what ever.
> 
> But does GRUB have anything to do about booting a system in a specific
> language or does each distribution modify GRUB to be able to change
> language.
> 
> Some distributions seem to be able to change the language of the GRUB
> boot menu and some does not.
> 

Internally GRUB supports l10n (reimplementation of gettext). Some
distributions install language catalogs; how they select them is
distribution-dependent. But grub-mkconfig and scripts that are shipped
with GRUB itself do not make use of this and emit strings in fixed
language, as set at the time grub-mkconfig runs so you cannot change
menu language without regenerating it. That would be something to
consider after 2.02 release.

As for keyboard layout, unfortunately only at_keyboard supports run-time
switch and it apparently has problems on real hardware. Default BIOS
input driver has fixed layout. There was proposed patch to implement
layouts on top of extended keyboard BIOS interface.

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

Re: [PATCH v2] Clarify documentation for special environment variable "default"

[Andrei Borzenkov]

This landed in rc1. Thanks!


03.02.2017 08:23, Daniel Kahn Gillmor пишет:
> The current documentation for the special environment variable
> "default" is confusing and unclear.  This patch attempts to clean it
> up.
> 
> In particular, the current documentation refers to the "number or
> title", but then in the example it gives, the menu entries and
> submenus all have numbers *in* their title; furthermore, there is no
> example given about how to choose the number, or any indication about
> whether counting is zero-indexed or 1-indexed.
> 
> Having a cleaner example and presenting all variants (numeric, title,
> and id) should make it clearer to the user.
> 
> Signed-off-by: Daniel Kahn Gillmor 
> ---
>  docs/grub.texi | 37 -
>  1 file changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index b9ddb9b8a..e935af33e 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3218,9 +3218,10 @@ source for more details.
>  @node default
>  @subsection default
>  
> -If this variable is set, it identifies a menu entry that should be selected
> -by default, possibly after a timeout (@pxref{timeout}).  The entry may be
> -identified by number or by id.
> +If this variable is set, it identifies a menu entry that should be
> +selected by default, possibly after a timeout (@pxref{timeout}).  The
> +entry may be identified by number (starting from 0 at each level of
> +the hierarchy), by title, or by id.
>  
>  For example, if you have:
>  
> @@ -3236,24 +3237,26 @@ then you can make this the default using:
>  default=example-gnu-linux
>  @end example
>  
> -If the entry is in a submenu, then it must be identified using the titles of
> -each of the submenus starting from the top level followed by the number or
> -title of the menu entry itself, separated by @samp{>}.  For example, take
> -the following menu structure:
> +If the entry is in a submenu, then it must be identified using the
> +number, title, or id of each of the submenus starting from the top
> +level, followed by the number, title, or id of the menu entry itself,
> +with each element separated by @samp{>}.  For example, take the
> +following menu structure:
>  
>  @example
> -Submenu 1
> -  Menu Entry 1
> -  Menu Entry 2
> -Submenu 2
> -  Submenu 3
> -Menu Entry 3
> -Menu Entry 4
> -  Menu Entry 5
> +GNU/Hurd --id gnu-hurd
> +  Standard Boot --id=gnu-hurd-std
> +  Rescue shell --id=gnu-hurd-rescue
> +Other platforms --id=other
> +  Minix --id=minix
> +Version 3.4.0 --id=minix-3.4.0
> +Version 3.3.0 --id=minix-3.3.0
> +  GRUB Invaders --id=grub-invaders
>  @end example
>  
> -``Menu Entry 3'' would then be identified as
> -@samp{Submenu 2>Submenu 3>Menu Entry 3}.
> +The more recent release of Minix would then be identified as
> +@samp{Other platforms>Minix>Version 3.4.0}, or as @samp{1>0>0}, or as
> +@samp{other>minix>minix-3.4.0}.
>  
>  This variable is often set by @samp{GRUB_DEFAULT} (@pxref{Simple
>  configuration}), @command{grub-set-default}, or @command{grub-reboot}.
> 


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

Re: [PATCH] Clarify documentation for special environment variable "default"

[Andrei Borzenkov]

03.02.2017 03:19, Daniel Kahn Gillmor пишет:
> The current documentation for the special environment variable
> "default" is confusing and unclear.  This patch attempts to clean it
> up.
> 
> In particular, the current documentation refers to the "number or
> title", but then in the example it gives, the menu entries and
> submenus all have numbers *in* their title; furthermore, there is no
> example given about how to choose the number (e.g. does it start at 1
> or at 0?).
> 
> Having a cleaner example and presenting both the numeric version and
> the title version will make it clearer.
> 
> In my patch here i've tried to make it clear and understandable.  I've
> also assumed that the number counting starts at 1, but haven't
> verified that.  If that's wrong, please commit the corrected version
> :)

Menu items count start with 0 in every submenu. We really need some
place with overall description of what happens when grub starts (akin to
bootup(7) in systemd). But at least example needs to be correct in this
case.

> 
> Signed-off-by: Daniel Kahn Gillmor 
> ---
>  docs/grub.texi | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index b9ddb9b8a..e868f045d 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3242,18 +3242,18 @@ title of the menu entry itself, separated by 
> @samp{>}.  For example, take
>  the following menu structure:
>  
>  @example
> -Submenu 1
> -  Menu Entry 1
> -  Menu Entry 2
> -Submenu 2
> -  Submenu 3
> -Menu Entry 3
> -Menu Entry 4
> -  Menu Entry 5
> +GNU/Hurd
> +  Standard Boot
> +  Rescue shell
> +Other platforms
> +  Minix
> +Version 3.4.0
> +Version 3.3.0
> +  GRUB Invaders
>  @end example
>  
> -``Menu Entry 3'' would then be identified as
> -@samp{Submenu 2>Submenu 3>Menu Entry 3}.
> +The more recent release of Minix would then be identified as
> +@samp{Other platforms>Minix>Version 3.4.0}, or as @samp{2>1>1}.
>  

Note that preceding paragraph says "using the titles of each of the
submenus" which is already not complete and does not agree with your
second example. Could you extend it?

>  This variable is often set by @samp{GRUB_DEFAULT} (@pxref{Simple
>  configuration}), @command{grub-set-default}, or @command{grub-reboot}.
> 

Preferred way to identify menu entry is to use menu ID (--id option).
This is mentioned in preceding paragraph, but not for submenus. Could
you modify your patch to mention this and give example as well. Also at
the very beginning it says "entry may be identified by number or by id"
missing "by title"; would be good to fix as long as you are on it.

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

Re: [PATCH] i386/relocator: Align stack in grub_relocator64_efi relocator

[Andrei Borzenkov]

 On Thu, Feb 2, 2017 at 5:48 PM, Vladimir 'phcoder' Serbinenko
<phco...@gmail.com> wrote:
>
>
> On Thu, 2 Feb 2017, 15:43 Andrei Borzenkov <arvidj...@gmail.com> wrote:
>>
>> On Thu, Feb 2, 2017 at 5:27 PM, Daniel Kiper <daniel.ki...@oracle.com>
>> wrote:
>> > Unified Extensible Firmware Interface Specification, Version 2.6,
>> > section 2.3.4, x64 Platforms, boot services, says among others:
>> > The stack must be 16-byte aligned. So, do it. Otherwise OS may
>> > boot only by chance as it happens right now.
>> >
>> > Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
>> > ---
>> >  grub-core/lib/i386/relocator64.S |   16 +---
>> >  1 file changed, 13 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/grub-core/lib/i386/relocator64.S
>> > b/grub-core/lib/i386/relocator64.S
>> > index 75725cf..148f38a 100644
>> > --- a/grub-core/lib/i386/relocator64.S
>> > +++ b/grub-core/lib/i386/relocator64.S
>> > @@ -73,14 +73,22 @@ VARIABLE(grub_relocator64_rsp)
>> >
>> > movq%rax, %rsp
>> >
>> > +#ifdef GRUB_MACHINE_EFI
>> > +   jmp LOCAL(skip_efi_stack_align)
>> > +
>> > /*
>> > -* Here is grub_relocator64_efi_start() entry point.
>> > -* Following code is shared between grub_relocator64_efi_start()
>> > +* Here is grub_relocator64_efi_start() entry point. Most of the
>> > +* code below is shared between grub_relocator64_efi_start()
>> >  * and grub_relocator64_start().
>> >  *
>> > -* Think twice before changing anything below!!!
>> > +* Think twice before changing anything there!!!
>> >  */
>> >  VARIABLE(grub_relocator64_efi_start)
>> > +   /* Align the stack as UEFI spec requires. */
>> > +   andq$~15, %rsp
>> > +
>>
>> It sounds like it should be fixed on caller side instead. I mean,
>> caller allocated some memory, we cannot just arbitrary adjust it now.
>> It sounds wrong.
>
> Unlike normal relocator, EFI relocator didn't use .RSP field but instead
> uses EFI stack, so there is no other place to fix this.
>>

OK, I see.

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

Re: [PATCH] i386/relocator: Align stack in grub_relocator64_efi relocator

[Andrei Borzenkov]

On Thu, Feb 2, 2017 at 5:27 PM, Daniel Kiper  wrote:
> Unified Extensible Firmware Interface Specification, Version 2.6,
> section 2.3.4, x64 Platforms, boot services, says among others:
> The stack must be 16-byte aligned. So, do it. Otherwise OS may
> boot only by chance as it happens right now.
>
> Signed-off-by: Daniel Kiper 
> ---
>  grub-core/lib/i386/relocator64.S |   16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/grub-core/lib/i386/relocator64.S 
> b/grub-core/lib/i386/relocator64.S
> index 75725cf..148f38a 100644
> --- a/grub-core/lib/i386/relocator64.S
> +++ b/grub-core/lib/i386/relocator64.S
> @@ -73,14 +73,22 @@ VARIABLE(grub_relocator64_rsp)
>
> movq%rax, %rsp
>
> +#ifdef GRUB_MACHINE_EFI
> +   jmp LOCAL(skip_efi_stack_align)
> +
> /*
> -* Here is grub_relocator64_efi_start() entry point.
> -* Following code is shared between grub_relocator64_efi_start()
> +* Here is grub_relocator64_efi_start() entry point. Most of the
> +* code below is shared between grub_relocator64_efi_start()
>  * and grub_relocator64_start().
>  *
> -* Think twice before changing anything below!!!
> +* Think twice before changing anything there!!!
>  */
>  VARIABLE(grub_relocator64_efi_start)
> +   /* Align the stack as UEFI spec requires. */
> +   andq$~15, %rsp
> +

It sounds like it should be fixed on caller side instead. I mean,
caller allocated some memory, we cannot just arbitrary adjust it now.
It sounds wrong.

> +LOCAL(skip_efi_stack_align):
> +#endif
> /* mov imm64, %rax */
> .byte   0x48
> .byte   0xb8
> @@ -128,8 +136,10 @@ LOCAL(jump_addr):
>  VARIABLE(grub_relocator64_rip)
> .quad   0
>
> +#ifdef GRUB_MACHINE_EFI
> /* Here grub_relocator64_efi_start() ends. Ufff... */
>  VARIABLE(grub_relocator64_efi_end)
> +#endif
>
>  #ifndef __x86_64__
> .p2align4
> --
> 1.7.10.4
>

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

Re: [PATCH] Ignore xen all_video module emptiness

[Andrei Borzenkov]

29.01.2017 12:37, Vladimir 'phcoder' Serbinenko пишет:
> Not tested yet. If there are no opposition I'll test and commit it.
> 
> 
...
> diff --git a/util/grub-module-verifierXX.c b/util/grub-module-verifierXX.c
> index 9c04caa..c79c1cb 100644
> --- a/util/grub-module-verifierXX.c
> +++ b/util/grub-module-verifierXX.c
...
> -check_symbols (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e)
> +check_symbols (const struct grub_module_verifier_arch *arch,
> +Elf_Ehdr *e, const char *modname,
> +const char **whitelist_empty)
>  {
>Elf_Sym *sym;
>Elf_Word size, entsize;
> @@ -196,7 +212,12 @@ check_symbols (const struct grub_module_verifier_arch 
> *arch, Elf_Ehdr *e)

I guess it needs some explanation in comments just before this line,
otherwise OK from me.

>sym = get_symtab (arch, e, , );
>if (!sym)
>  {
> -  Elf_Shdr *s = find_section (arch, e, ".moddeps");
> +  Elf_Shdr *s;
> +
> +  if (is_whitelisted (modname, whitelist_empty))
> + return;
> +
> +  s = find_section (arch, e, ".moddeps");
>  
>if (!s)
>   grub_util_error ("no symbol table and no .moddeps section");


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

Re: [PATCH 1/4] Allow non-default ports for HTTP requests

[Andrei Borzenkov]

24.01.2017 03:35, Matthew Garrett пишет:
> Add support for passing ports in HTTP requests. This takes the form of:
> (http,serverip:portnum)/file
> ---
>  grub-core/net/http.c |  8 ++--
>  grub-core/net/net.c  | 10 +-
>  include/grub/net.h   |  1 +
>  3 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/grub-core/net/http.c b/grub-core/net/http.c
> index 5aa4ad3..389a78e 100644
> --- a/grub-core/net/http.c
> +++ b/grub-core/net/http.c
> @@ -309,7 +309,7 @@ http_establish (struct grub_file *file, grub_off_t 
> offset, int initial)
>  {
>http_data_t data = file->data;
>grub_uint8_t *ptr;
> -  int i;
> +  int i, port;
>struct grub_net_buff *nb;
>grub_err_t err;
>  
> @@ -390,8 +390,12 @@ http_establish (struct grub_file *file, grub_off_t 
> offset, int initial)
>grub_netbuff_put (nb, 2);
>grub_memcpy (ptr, "\r\n", 2);
>  
> +  if (file->device->net->port)

0 is valid port number (at least I am not aware of any RFC that
prohibits its use).

> +port = file->device->net->port;
> +  else
> +port = HTTP_PORT;
>data->sock = grub_net_tcp_open (file->device->net->server,
> -   HTTP_PORT, http_receive,
> +   port, http_receive,
> http_err, http_err,
> file);
>if (!data->sock)
> diff --git a/grub-core/net/net.c b/grub-core/net/net.c
> index 10773fc..585f4f7 100644
> --- a/grub-core/net/net.c
> +++ b/grub-core/net/net.c
> @@ -1261,7 +1261,7 @@ grub_net_open_real (const char *name)
>grub_net_app_level_t proto;
>const char *protname, *server;
>grub_size_t protnamelen;
> -  int try;
> +  int try, port = 0;
>  
>if (grub_strncmp (name, "pxe:", sizeof ("pxe:") - 1) == 0)
>  {
> @@ -1278,7 +1278,14 @@ grub_net_open_real (const char *name)
>else
>  {
>const char *comma;
> +  char *colon;
>comma = grub_strchr (name, ',');
> +  colon = grub_strchr (name, ':');

This conflicts with IPv6 addresses. Similar patches were already posted.
I suggested using (proto,server,port). This also allows empty server and
still using non-standard port.

More general at some point we should hit authentication, and so may need
to stuff user/password there. So we need to come up with extensible
framework.

Putting everything in "server" string may work, but then we need to
support IPv6 literals here - [fe80::1]:444 - at least, this is the only
way to use server:port I see. Also, we probably need to support escaping
of colons. Finally, speaking about user/password, we may need to support

user:password@[fe80::1]:456

May be using (net,proto=http,server=xxx,port=yyy,user=uuu,...) which
allows adding arbitrary keywords at any time.


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

Disable movw/movt with clang

[Andrei Borzenkov]

> diff --git a/configure.ac b/configure.ac
> index 4e980c5..ab7fa92 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1152,6 +1152,16 @@ if test "$target_cpu"-"$platform" = x86_64-efi; then
>  fi
>  
>  if test "x$target_cpu" = xarm; then
> +  AC_CACHE_CHECK([whether option -mllvm -arm-use-movt=0 works], 
> grub_cv_cc_mllvm_arm_use_movt, [
> +CPPFLAGS="$TARGET_CPPFLAGS -mllvm -arm-use-movt=0 -Werror"
> +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
> + [grub_cv_cc_mllvm_arm_use_movt=yes],
> + [grub_cv_cc_mllvm_arm_use_movt=no])
> +  ])
> +  if test "x$grub_cv_cc_mllvm_arm_use_movt" = xyes; then
> +# A trick so that clang doesn't see it on link stage
> +TARGET_CPPFLAGS="$TARGET_CPPFLAGS -mllvm -arm-use-movt=0"
> +  fi
>AC_CACHE_CHECK([whether option -mlong-calls works], 
> grub_cv_cc_mlong_calls, [
>  CFLAGS="$TARGET_CFLAGS -mlong-calls -Werror"
>  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],

Is this option supported by all clang versions? What should we do if
this is *not* supported? As far as I understand commit message this
leads to broken code?

Is "-mllvm -arm-use-movt=0" different from "-mno-movt" which appears to
be "official" way to disable it?

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

Re: [PATCH] Add fwconfig command

[Andrei Borzenkov]

24.01.2017 02:43, Matthew Garrett пишет:
> Add a command to read values from the qemu fwcfg store. This allows data
> to be passed from the qemu command line to grub.
> 
> Example use:
> 
> echo '(hd0,1)' >rootdev
> qemu -fw_cfg opt/rootdev,file=rootdev
> 
> fwconfig opt/rootdev root

The name sounds way too generic. Unless we have plans to unify such
interface on multiple platforms, I would expect something like fw_cfg
(to match QEMU documentation) or even qemu_fw_cfg to make it pretty obvious.

> ---
>  docs/grub.texi|   6 +++
>  grub-core/Makefile.core.def   |   6 +++
>  grub-core/commands/fwconfig.c | 121 
> ++
>  3 files changed, 133 insertions(+)
>  create mode 100644 grub-core/commands/fwconfig.c
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 4469638..4f8a378 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3818,6 +3818,7 @@ you forget a command, you can run the command 
> @command{help}
>  * eval::Evaluate agruments as GRUB commands
>  * export::  Export an environment variable
>  * false::   Do nothing, unsuccessfully
> +* fwconfig::Retrieves a value from the qemu fwcfg store
>  * getenv::  Retrieve an EFI firmware variable
>  * gettext:: Translate a string
>  * gptsync:: Fill an MBR based on GPT entries
> @@ -4259,6 +4260,11 @@ Do nothing, unsuccessfully.  This is mainly useful in 
> control constructs
>  such as @code{if} and @code{while} (@pxref{Shell-like scripting}).
>  @end deffn
>  
> +@node fwconfig
> +@subsection fwconig
> +@deffn Command fwconfig fwpath envvar
> +Retrieves @var{fwpath} from the qemu fwcfg store and stores it in 
> @var{envvar}
> +

Your patch adds "-v" option that is not documented here

>  @node getenv
>  @subsection getenv
>  
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index db77a7f..f6b6f38 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -2362,3 +2362,9 @@ module = {
>common = loader/i386/xen_file64.c;
>extra_dist = loader/i386/xen_fileXX.c;
>  };
> +
> +module = {
> +  name = fwconfig;
> +  common = commands/fwconfig.c;
> +  enable = x86;

QEMU supports fw_cfg at least on ARM (according to documentation), but I
guess this can be done later if needed.

> +};
> diff --git a/grub-core/commands/fwconfig.c b/grub-core/commands/fwconfig.c
> new file mode 100644
> index 000..289d167
> --- /dev/null
> +++ b/grub-core/commands/fwconfig.c
> @@ -0,0 +1,121 @@
> +/* fwconfig.c - command to read config from qemu fwconfig  */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2015  CoreOS, Inc.
> + *

I agree with other comments that at least does not align with copyrights
of other sources.

> + *  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 .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +#define SELECTOR 0x510
> +#define DATA 0x511
> +
> +#define SIGNATURE_INDEX 0x00
> +#define DIRECTORY_INDEX 0x19
> +

QEMU also provdes MMIO interface, but again, can be added later. I
wonder if we could use ACPI interface to discover them here?

> +static grub_extcmd_t cmd_read_fwconfig;
> +
> +struct grub_qemu_fwcfgfile {
> +  grub_uint32_t size;
> +  grub_uint16_t select;
> +  grub_uint16_t reserved;
> +  char name[56];
> +};
> +
> +static const struct grub_arg_option options[] =
> +  {
> +{0, 'v', 0, N_("Save read value into variable VARNAME."),
> + N_("VARNAME"), ARG_TYPE_STRING},

This option is not used anywhere. Also long options, please!

> +{0, 0, 0, 0, 0, 0}
> +  };
> +
> +static grub_err_t
> +grub_cmd_fwconfig (grub_extcmd_context_t ctxt, int argc, char **argv)
> +{
> +  grub_uint32_t i, j, value = 0;
> +  struct grub_qemu_fwcfgfile file;
> +  char fwsig[4], signature[4] = { 'Q', 'E', 'M', 'U' };
> +
> +  if (argc != 2)
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
> +

I would really prefer using options over positional parameters to make
it more extensible. One obvious extension would be --list to see
available files and --test to be able to verify in script whether file
exists.

> +  /* Verify that we have meaningful 

Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

[Andrei Borzenkov]

24.01.2017 23:50, Matthew Garrett пишет:
> On Mon, Jan 23, 2017 at 8:02 PM, Andrei Borzenkov <arvidj...@gmail.com> wrote:
>> 24.01.2017 03:36, Matthew Garrett пишет:
>>> Some DHCP servers (such as dnsmasq) tokenise parameters with commas, making
>>> it impossible to pass boot files with commas in them. Allow using a
>>
>> grub_net_open() operates on devices, not files. Please give more details
>> about your problem.
> 
> The DHCP server will return a string in the boot_file field. If you
> want to indicate that this file should be obtained over http, the
> easiest way to handle this is to provide a boot file in the form
> (http,host)filename. Unfortunately dnsmasq uses commas to tokenise its
> configuration parameters and there appears to be no way to override
> that, which makes it impossible to provide a boot file in this form.

Really?

dnsmasq -d -z --dhcp-range=192.168.11.20,192.168.11.30 --dhcp-option
option:bootfile-name,'(http,1.2.3.4)/foo/bar' -i eth1


Bootstrap Protocol (ACK)
...
Option: (67) Bootfile name
Length: 23
Bootfile name: (http,1.2.3.4)/foo/bar
Option: (255) End
Option End: 255


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

Re: [RFC 0/2] UEFI-based HTTP Boot

[Andrei Borzenkov]

On Wed, Jan 25, 2017 at 11:49 AM, Michael Chang <mch...@suse.com> wrote:
> On Wed, Jan 25, 2017 at 11:19:19AM +0300, Andrei Borzenkov wrote:
>> On Wed, Jan 25, 2017 at 11:09 AM, Michael Chang <mch...@suse.com> wrote:
>> > On Fri, Jan 20, 2017 at 05:50:56PM +0300, Andrei Borzenkov wrote:
>> >> On Fri, Jan 20, 2017 at 4:13 AM, Ken Lin <ken@hpe.com> wrote:
>> >> > This RFC patchset is stacked on the previous HTTP boot patchset:
>> >> > https://lists.gnu.org/archive/html/grub-devel/2016-12/msg00088.html
>> >> > It re-uses some code from it, e.g. the DCHPACK
>> >> > with vendor_class_identifier=HTTPClient.
>> >> >
>> >> > Instead of implementing HTTP and HTTPS boot totally from software,
>> >> > UEFI firmware already defines APIs for HTTP(s).
>> >> > Please check UEFI spec. 2.5 and plus for the detail:
>> >> >
>> >> > 28.6 EFI HTTP Protocols
>> >> >
>> >>
>> >> Without reviewing patches themselves - we usually prefer to rely on
>> >> firmware as little as possible. We already have HTTP support, so what
>> >> is missing in grub that requires what amounts to full
>> >> re-implementation? Cannot we rather fix our HTTP support instead? This
>> >> will automatically benefit all supported platforms, of which EFI is
>> >> just one.
>> >
>> > Nothing wrong in providing firmware based approach in addition to grub's 
>> > native
>> > stack of getting the similar things done.
>>
>> You cannot both shut off all layered protocols on physical adapter and
>> makes use of these layered protocols. This will need to implement
>> alternative networking stack first.
>
> They don't have to be runtime switch to operate at the same time. Make them
> distinct modules, and provid a swith for controlling the image being built. We
> already have --native switch in grub2-install to incorporate native disk 
> modules
> rather than firmware's.
>

These patches hook into network stack of grub. So they either must
comply with this stack (which means - provide alternative network
driver for reasons I mentioned) or they must pretend "efihttp" is
disk.

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

Re: [RFC 0/2] UEFI-based HTTP Boot

[Andrei Borzenkov]

On Wed, Jan 25, 2017 at 11:09 AM, Michael Chang <mch...@suse.com> wrote:
> On Fri, Jan 20, 2017 at 05:50:56PM +0300, Andrei Borzenkov wrote:
>> On Fri, Jan 20, 2017 at 4:13 AM, Ken Lin <ken@hpe.com> wrote:
>> > This RFC patchset is stacked on the previous HTTP boot patchset:
>> > https://lists.gnu.org/archive/html/grub-devel/2016-12/msg00088.html
>> > It re-uses some code from it, e.g. the DCHPACK
>> > with vendor_class_identifier=HTTPClient.
>> >
>> > Instead of implementing HTTP and HTTPS boot totally from software,
>> > UEFI firmware already defines APIs for HTTP(s).
>> > Please check UEFI spec. 2.5 and plus for the detail:
>> >
>> > 28.6 EFI HTTP Protocols
>> >
>>
>> Without reviewing patches themselves - we usually prefer to rely on
>> firmware as little as possible. We already have HTTP support, so what
>> is missing in grub that requires what amounts to full
>> re-implementation? Cannot we rather fix our HTTP support instead? This
>> will automatically benefit all supported platforms, of which EFI is
>> just one.
>
> Nothing wrong in providing firmware based approach in addition to grub's 
> native
> stack of getting the similar things done.

You cannot both shut off all layered protocols on physical adapter and
makes use of these layered protocols. This will need to implement
alternative networking stack first.

> And there's no prioity for what has
> to be implemented first imho. Occasionally people would prefer firmware based
> stack because they need new features it provides that haven't been worked out
> in grub, such as the https or fibre networks, or simply to avoid bug from 
> grub,
> like the SNP woes among some UEFI box.
>
> Thanks,
> Michael
>
>>
>> > Then why two implementations? For older UEFI firmwares (UEFI 2.4 and 
>> > older),
>> > the HTTP(s) APIs are not available. In the case,
>> > Grub can fall back to the software-based implementation.
>> > In the first patch of this patchset, 
>> > grub-core/net/drivers/efi/efihttp.c:76 to 81
>> > is the code to query if the HTTP Protocol is supported by the UEFI 
>> > firmware.
>> >
>> > This patchset was tested on QEMU+OVMF and it works flawlessly.
>> >
>> > The main goals of this RFC is to ask for opinions and suggestion to make
>> > the first patch modularized as much as possible.
>> > In the second patch, there is some codes related TCP re-transmission
>> > that need to pass by for the HTTP Boot to work.
>> >
>> > More details are described in the logs of each patch.
>> >
>> >
>> > Ken Lin (2):
>> >   net: add efihttp to do HTTP(S) Boot by UEFI HTTP Protocol
>> >   net: workaround to bypass corruption of the efihttp function pointer
>> >
>> >  grub-core/Makefile.core.def |   1 +
>> >  grub-core/net/bootp.c   |   6 +
>> >  grub-core/net/drivers/efi/efihttp.c | 386 
>> > 
>> >  grub-core/net/drivers/efi/efinet.c  |   1 +
>> >  grub-core/net/net.c |  39 +++-
>> >  include/grub/efi/api.h  |  17 ++
>> >  include/grub/efi/http.h | 221 +
>> >  include/grub/err.h  |   3 +-
>> >  include/grub/net.h  |   1 +
>> >  9 files changed, 672 insertions(+), 3 deletions(-)
>> >  create mode 100755 grub-core/net/drivers/efi/efihttp.c
>> >  create mode 100755 include/grub/efi/http.h
>> >
>> > --
>> > 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
>
> ___
> 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 4/4] Allow protocol to be separated from host with a semicolon

[Andrei Borzenkov]

On Wed, Jan 25, 2017 at 10:16 AM, Matthew Garrett <mj...@coreos.com> wrote:
> On Tue, Jan 24, 2017 at 10:56 PM, Andrei Borzenkov <arvidj...@gmail.com> 
> wrote:
>> On Wed, Jan 25, 2017 at 7:25 AM, Matthew Garrett <mj...@coreos.com> wrote:
>>> If prefix isn't set then won't bootfile be interpreted as the device plus 
>>> file?
>>>
>>
>> No. That would mean "parsing URI" that I mentioned.
>
> My experience is that configfile (http,example.com)grub/config works
> as you'd expect it to, and that set
> endpoint=$net_efinet0_dhcp_boot_file; configfile $endpoint does the
> same. Am I hitting some corner case where things are being incorrectly
> parsed and so giving me unintended functionality?
>

When grub starts it tries to determine device and path it was booted
from. For network boot it currently means examining DHCP ACK packet
that is normally provided by firmware and setting device to
tftp,$next_ip and path to $bootfile. There is no provision to set
protocol to anything else nor to parse $bootfile to extract
protocol/server.

If you speak about "configfile something" you are past this point so
DHCP $bootfile option is not relevant at all.

>>> We need the ability to pass port as well, so that would still be 
>>> insufficient.
>>
>> Good. So start with design proposal that is extensible enough.
>>
>> But TBH - all of this can already be implemented using grub scripting.
>> Use custom DHCP options or parse bootfile using regex. No code changes
>> is needed - we support generic scripting for a reason.
>
> How are custom DHCP options handled? I can't find anything in the
> documentation about them being interpreted, and a quick look at the
> code only shows it setting known variables.

You have net_get_dhcp_option to fetch arbitrary option value from DHCP
ACK packet and put it in variable for further processing. I'm
definitely open to improving this command to make it more useful if it
turns out lacking something.

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

Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

[Andrei Borzenkov]

On Wed, Jan 25, 2017 at 7:25 AM, Matthew Garrett <mj...@coreos.com> wrote:
> On Tue, Jan 24, 2017 at 8:15 PM, Andrei Borzenkov <arvidj...@gmail.com> wrote:
>> 25.01.2017 07:06, Matthew Garrett пишет:
>>> I don't understand - grub_net_open_real() already handles this case:
>>
>> Because bootfile from DHCP packet is not used to set device part of
>> $prefix. Setting bootfile to (http,host)filename will end up with full
>> prefix "(tftp,$next_ip)(http,host)base-name-of-filename". If you mean
>> something different, please explain.
>
> If prefix isn't set then won't bootfile be interpreted as the device plus 
> file?
>

No. That would mean "parsing URI" that I mentioned.

>>> Because we ship prebuilt images but don't know what IP addresses users
>>> will be using for their deployment servers.
>>>
>>
>> I can think about implementing URI parsing (somewhat in line with UEFI
>> HTTPboot spec) and/or supporting partial $prefix that sets only
>> protocol, something like "grub-mknetdir --prefix=(http,)", where server
>> part is filled from DHCP ACK.
>
> We need the ability to pass port as well, so that would still be insufficient.

Good. So start with design proposal that is extensible enough.

But TBH - all of this can already be implemented using grub scripting.
Use custom DHCP options or parse bootfile using regex. No code changes
is needed - we support generic scripting for a reason.

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

Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

[Andrei Borzenkov]

25.01.2017 07:06, Matthew Garrett пишет:
> On Tue, Jan 24, 2017 at 7:48 PM, Andrei Borzenkov <arvidj...@gmail.com> wrote:
>> 24.01.2017 23:50, Matthew Garrett пишет:
>>> On Mon, Jan 23, 2017 at 8:02 PM, Andrei Borzenkov <arvidj...@gmail.com> 
>>> wrote:
>>>> 24.01.2017 03:36, Matthew Garrett пишет:
>>>>> Some DHCP servers (such as dnsmasq) tokenise parameters with commas, 
>>>>> making
>>>>> it impossible to pass boot files with commas in them. Allow using a
>>>>
>>>> grub_net_open() operates on devices, not files. Please give more details
>>>> about your problem.
>>>
>>> The DHCP server will return a string in the boot_file field. If you
>>> want to indicate that this file should be obtained over http, the
>>> easiest way to handle this is to provide a boot file in the form
>>> (http,host)filename. Unfortunately dnsmasq uses commas to tokenise its
>>> configuration parameters and there appears to be no way to override
>>> that, which makes it impossible to provide a boot file in this form.
>>> Allowing the use of an alternative character avoids this problem.
>>>
>>
>> This won't work because (http,host) will never be interpreted as device
>> in current code so you need to support this first before this patch can
>> even be considered. Also I am not convinced that arbitrary changing
>> syntax is good idea.
> 
> I don't understand - grub_net_open_real() already handles this case:

Because bootfile from DHCP packet is not used to set device part of
$prefix. Setting bootfile to (http,host)filename will end up with full
prefix "(tftp,$next_ip)(http,host)base-name-of-filename". If you mean
something different, please explain.

> 
>   else
> {
>   const char *comma;
>   comma = grub_strchr (name, ',');
>   if (comma)
> {
>   protnamelen = comma - name;
>   server = comma + 1;
>   protname = name;
> }
>   else
> {
>   protnamelen = grub_strlen (name);
>   server = grub_net_default_server;
>   protname = name;
> }
> 
> 
>> You already can set $prefix when generating image. Why is it not enough?
> 
> Because we ship prebuilt images but don't know what IP addresses users
> will be using for their deployment servers.
> 

I can think about implementing URI parsing (somewhat in line with UEFI
HTTPboot spec) and/or supporting partial $prefix that sets only
protocol, something like "grub-mknetdir --prefix=(http,)", where server
part is filled from DHCP ACK.

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

Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

[Andrei Borzenkov]

24.01.2017 23:50, Matthew Garrett пишет:
> On Mon, Jan 23, 2017 at 8:02 PM, Andrei Borzenkov <arvidj...@gmail.com> wrote:
>> 24.01.2017 03:36, Matthew Garrett пишет:
>>> Some DHCP servers (such as dnsmasq) tokenise parameters with commas, making
>>> it impossible to pass boot files with commas in them. Allow using a
>>
>> grub_net_open() operates on devices, not files. Please give more details
>> about your problem.
> 
> The DHCP server will return a string in the boot_file field. If you
> want to indicate that this file should be obtained over http, the
> easiest way to handle this is to provide a boot file in the form
> (http,host)filename. Unfortunately dnsmasq uses commas to tokenise its
> configuration parameters and there appears to be no way to override
> that, which makes it impossible to provide a boot file in this form.
> Allowing the use of an alternative character avoids this problem.
> 

This won't work because (http,host) will never be interpreted as device
in current code so you need to support this first before this patch can
even be considered. Also I am not convinced that arbitrary changing
syntax is good idea.

You already can set $prefix when generating image. Why is it not enough?

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

Re: [PATCH v2] osdep/linux: handle autofs entries in /proc/self/mountinfo

[Andrei Borzenkov]

19.01.2017 19:37, Andrei Borzenkov пишет:
> These entries have placeholder for device name and so are useless for our
> purpose. grub failed with something like
> 
> grub-install: error: failed to get canonical path of `systemd-1'.
> 
> When we see autofs entry, record it (to keep parent-child relationship) but
> continue to look for real mount. If it is found, we process it as usual. If
> only autofs entry exists, attempt to trigger mount by opening mount point
> and retry. Mount point itself is then kept open to avoid timeout.
> 
> Recent systemd is by default using automount for /boot/efi so this should
> become more popular problem on EFI systems.
> 
> Closes: 49942
> 
> ---
> 
> v2:
>   - remove bogus autofs check in main mountinfo read loop, it was result of
> misunderstanding
> 
>   - consolidate cleanup code to avoid memory leak on retry.
> 
> If there are no objections in next days, I'll commit it. It was confirmed to
> fix the problem.
> 


Committed.

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

Re: Why is stdint.h so little used?

[Andrei Borzenkov]

On Tue, Jan 24, 2017 at 10:08 AM, Bjørn Forsman  wrote:
> On 24 January 2017 at 02:33, Vladimir 'phcoder' Serbinenko
>  wrote:
>> All files in posix_wrap are only for porting code to GRUB with little
>> modification. No GRUB-specific code should use it.
>
> That only explains half of it :-)
>
> What is better about writing grub_uint32_t instead of uint32_t? (To me
> it looks like a pointless indirection.)
>

GRUB boot time code is built without any standard headers at all, so
there is absolutely no difference between defining grub_uint32_t or
uint32_t, in both cases it must be defined in one of grub headers.
There is no indirection at all, because no uint32_t exists. For your
project you can add defines to avoid non-functional changes and then
simply do mass replace ones when porting is complete :)

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

Re: [PATCH 4/4] Allow protocol to be separated from host with a semicolon

[Andrei Borzenkov]

24.01.2017 03:36, Matthew Garrett пишет:
> Some DHCP servers (such as dnsmasq) tokenise parameters with commas, making
> it impossible to pass boot files with commas in them. Allow using a

grub_net_open() operates on devices, not files. Please give more details
about your problem.

> semicolon to separate the protocol from host if a comma wasn't found.
> ---
>  grub-core/net/net.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/grub-core/net/net.c b/grub-core/net/net.c
> index 585f4f7..efacb30 100644
> --- a/grub-core/net/net.c
> +++ b/grub-core/net/net.c
> @@ -1280,6 +1280,10 @@ grub_net_open_real (const char *name)
>const char *comma;
>char *colon;
>comma = grub_strchr (name, ',');
> +  if (!comma)
> + {
> +   comma = grub_strchr (name, ';');
> + }
>colon = grub_strchr (name, ':');
>if (colon)
>   {
> 


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

Re: [PATCH 3/4] Don't allocate a new address buffer if we receive multiple DNS responses

[Andrei Borzenkov]

24.01.2017 03:36, Matthew Garrett пишет:
> The current logic in the DNS resolution code allocates an address buffer
> based on the number of addresses in the response packet. If we receive
> multiple response packets in response to a single query packet, this means
> that we will reallocate a new buffer large enough for only the addresses in
> that specific packet, discarding any previous results in the process. Worse,
> we still keep track of the *total* number of addresses resolved in response
> to this query, not merely the number in the packet being currently processed.
> Use realloc() rather than malloc() to avoid overwriting the existing data,
> and allocate a buffer large enough for the total set of addresses rather
> than merely the number in this specific response.
> ---
>  grub-core/net/dns.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/grub-core/net/dns.c b/grub-core/net/dns.c
> index 5d9afe0..5deb1ef 100644
> --- a/grub-core/net/dns.c
> +++ b/grub-core/net/dns.c
> @@ -285,8 +285,8 @@ recv_hook (grub_net_udp_socket_t sock __attribute__ 
> ((unused)),
>ptr++;
>ptr += 4;
>  }
> -  *data->addresses = grub_malloc (sizeof ((*data->addresses)[0])
> -  * grub_be_to_cpu16 (head->ancount));
> +  *data->addresses = grub_realloc (*data->addresses, sizeof 
> ((*data->addresses)[0])
> +  * (grub_be_to_cpu16 (head->ancount) + *data->naddresses));

If *data->addresses was not NULL, we should not reach this code.

  /* Code apparently assumed that only one packet is received as response.
 We may get multiple responses due to network condition, so check here
 and quit early. */
  if (*data->addresses)
{
  grub_netbuff_free (nb);
  return GRUB_ERR_NONE;
}

This was noted previously by Josef, we discussed it and my position is
that resolver code requires redesign to correctly merge multiple answers
and prioritize A vs  requests.

Do you get actual errors with current master? If yes, could you provide
more information what this patch fixes?

>if (!*data->addresses)
>  {
>grub_errno = GRUB_ERR_NONE;
> 


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

Re: [RFC 0/2] UEFI-based HTTP Boot

[Andrei Borzenkov]

21.01.2017 06:37, Lin, Keng-Yu пишет:
> 

> The main reason is HTTPS. We've been looking at it for a while.
Recently a few patches of HTTPS Boot were already merge in the EDK2 main
tree (before this, it was in the EDK2-staging tree).
> 
> Please see:
> https://github.com/tianocore/edk2/search?q=tls=Commits
> 
> For the software-based implementation, it will be a lot of works to
support this, e.g. interfaces to import or remove certificates, grub has

I do not see how your patch solves this either. UEFI provides no
guarantee that necessary certificates are available in non-volatile
store, so either your patch relies on some third-party software to do it
on every boot or on implementation specific behavior that stores
information permanently. Having hooks in grub to manage this information
would free it from these assumptions and then it does not matter whether
grub will manage it via EFI interface or using own internal implementation.

to depend on GnuTLS or OpenSSL libraries. This means that grub has to
release a new version once GnuTLS or OpenSSL bump their ABIs, etc.

GRUB already reuses libgcrypto. It may be possible to reuse gnutls in
the same way. Sure, it is a lot of work, but then this will also be
available on every platform.

> 
> The ideal picture from us is as below: * For the UEFI firmware v2.4
> and older: use the software-based HTTP
boot. The limitation is this does not include HTTPS.
> 
> * For the UEFI firmware v2.5 and newer: use the UEFI-based HTTP(S)
boot (this RFC patchset). Both HTTP and HTTPS are supported.

This hardly can be used in this form. First, grub is using SNP and
relies on being the only consumer of network device. We already had
quite serious problems when firmware raced with grub for card access.
Your patch spawns off additional protocol(s) that compete with grub.
This cannot work reliably unless we switch grub to MNP.

Second your patch completely bypasses grub event loop and busy waits for
transfer completion. It should hook into general grub net polling
framework.

> 
> I hope this help you and the other maintainers understand the
background of the HTTP(s) Boot patches from us.
> ____
> From: Andrei Borzenkov <arvidj...@gmail.com>
> Sent: Friday, January 20, 2017 10:50:56 PM
> To: The development of GNU GRUB
> Cc: Lin, Ken (HPS OE-Linux TDC); Chang, Clay (HPS OE-Linux TDC); Lin, 
> Keng-Yu; Knippers, Linda; Ruan, Michael (HPS OE-Linux/VMware TDC)
> Subject: Re: [RFC 0/2] UEFI-based HTTP Boot
> 
> On Fri, Jan 20, 2017 at 4:13 AM, Ken Lin <ken@hpe.com> wrote:
>> This RFC patchset is stacked on the previous HTTP boot patchset:
>> https://lists.gnu.org/archive/html/grub-devel/2016-12/msg00088.html
>> It re-uses some code from it, e.g. the DCHPACK
>> with vendor_class_identifier=HTTPClient.
>>
>> Instead of implementing HTTP and HTTPS boot totally from software,
>> UEFI firmware already defines APIs for HTTP(s).
>> Please check UEFI spec. 2.5 and plus for the detail:
>>
>> 28.6 EFI HTTP Protocols
>>
> 
> Without reviewing patches themselves - we usually prefer to rely on
> firmware as little as possible. We already have HTTP support, so what
> is missing in grub that requires what amounts to full
> re-implementation? Cannot we rather fix our HTTP support instead? This
> will automatically benefit all supported platforms, of which EFI is
> just one.
> 
>> Then why two implementations? For older UEFI firmwares (UEFI 2.4 and older),
>> the HTTP(s) APIs are not available. In the case,
>> Grub can fall back to the software-based implementation.
>> In the first patch of this patchset, grub-core/net/drivers/efi/efihttp.c:76 
>> to 81
>> is the code to query if the HTTP Protocol is supported by the UEFI firmware.
>>
>> This patchset was tested on QEMU+OVMF and it works flawlessly.
>>
>> The main goals of this RFC is to ask for opinions and suggestion to make
>> the first patch modularized as much as possible.
>> In the second patch, there is some codes related TCP re-transmission
>> that need to pass by for the HTTP Boot to work.
>>
>> More details are described in the logs of each patch.
>>
>>
>> Ken Lin (2):
>>   net: add efihttp to do HTTP(S) Boot by UEFI HTTP Protocol
>>   net: workaround to bypass corruption of the efihttp function pointer
>>
>>  grub-core/Makefile.core.def |   1 +
>>  grub-core/net/bootp.c   |   6 +
>>  grub-core/net/drivers/efi/efihttp.c | 386 
>> 
>>  grub-core/net/drivers/efi/efinet.c  |   1 +
>>  grub-core/net/net.c |  39 +++-
>>  include/grub/efi/api.h  |  17 ++
>

Re: [RFC 0/2] UEFI-based HTTP Boot

[Andrei Borzenkov]

On Fri, Jan 20, 2017 at 4:13 AM, Ken Lin  wrote:
> This RFC patchset is stacked on the previous HTTP boot patchset:
> https://lists.gnu.org/archive/html/grub-devel/2016-12/msg00088.html
> It re-uses some code from it, e.g. the DCHPACK
> with vendor_class_identifier=HTTPClient.
>
> Instead of implementing HTTP and HTTPS boot totally from software,
> UEFI firmware already defines APIs for HTTP(s).
> Please check UEFI spec. 2.5 and plus for the detail:
>
> 28.6 EFI HTTP Protocols
>

Without reviewing patches themselves - we usually prefer to rely on
firmware as little as possible. We already have HTTP support, so what
is missing in grub that requires what amounts to full
re-implementation? Cannot we rather fix our HTTP support instead? This
will automatically benefit all supported platforms, of which EFI is
just one.

> Then why two implementations? For older UEFI firmwares (UEFI 2.4 and older),
> the HTTP(s) APIs are not available. In the case,
> Grub can fall back to the software-based implementation.
> In the first patch of this patchset, grub-core/net/drivers/efi/efihttp.c:76 
> to 81
> is the code to query if the HTTP Protocol is supported by the UEFI firmware.
>
> This patchset was tested on QEMU+OVMF and it works flawlessly.
>
> The main goals of this RFC is to ask for opinions and suggestion to make
> the first patch modularized as much as possible.
> In the second patch, there is some codes related TCP re-transmission
> that need to pass by for the HTTP Boot to work.
>
> More details are described in the logs of each patch.
>
>
> Ken Lin (2):
>   net: add efihttp to do HTTP(S) Boot by UEFI HTTP Protocol
>   net: workaround to bypass corruption of the efihttp function pointer
>
>  grub-core/Makefile.core.def |   1 +
>  grub-core/net/bootp.c   |   6 +
>  grub-core/net/drivers/efi/efihttp.c | 386 
> 
>  grub-core/net/drivers/efi/efinet.c  |   1 +
>  grub-core/net/net.c |  39 +++-
>  include/grub/efi/api.h  |  17 ++
>  include/grub/efi/http.h | 221 +
>  include/grub/err.h  |   3 +-
>  include/grub/net.h  |   1 +
>  9 files changed, 672 insertions(+), 3 deletions(-)
>  create mode 100755 grub-core/net/drivers/efi/efihttp.c
>  create mode 100755 include/grub/efi/http.h
>
> --
> 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 v2] osdep/linux: handle autofs entries in /proc/self/mountinfo

[Andrei Borzenkov]

These entries have placeholder for device name and so are useless for our
purpose. grub failed with something like

grub-install: error: failed to get canonical path of `systemd-1'.

When we see autofs entry, record it (to keep parent-child relationship) but
continue to look for real mount. If it is found, we process it as usual. If
only autofs entry exists, attempt to trigger mount by opening mount point
and retry. Mount point itself is then kept open to avoid timeout.

Recent systemd is by default using automount for /boot/efi so this should
become more popular problem on EFI systems.

Closes: 49942

---

v2:
  - remove bogus autofs check in main mountinfo read loop, it was result of
misunderstanding

  - consolidate cleanup code to avoid memory leak on retry.

If there are no objections in next days, I'll commit it. It was confirmed to
fix the problem.


 grub-core/osdep/linux/getroot.c | 45 +++--
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/grub-core/osdep/linux/getroot.c b/grub-core/osdep/linux/getroot.c
index 09e7e6e..90d92d3 100644
--- a/grub-core/osdep/linux/getroot.c
+++ b/grub-core/osdep/linux/getroot.c
@@ -380,24 +380,30 @@ get_btrfs_fs_prefix (const char *mount_path)
 char **
 grub_find_root_devices_from_mountinfo (const char *dir, char **relroot)
 {
-  FILE *fp;
+  FILE *fp = NULL;
   char *buf = NULL;
   size_t len = 0;
-  grub_size_t entry_len = 0, entry_max = 4;
+  grub_size_t entry_len, entry_max = 4;
   struct mountinfo_entry *entries;
   struct mountinfo_entry parent_entry = { 0, 0, 0, "", "", "", "" };
   int i;
+  int retry = 0;
+  int dir_fd = -1;
+  char **ret = NULL;
 
   if (! *dir)
 dir = "/";
   if (relroot)
 *relroot = NULL;
 
+  entries = xmalloc (entry_max * sizeof (*entries));
+
+again:
   fp = grub_util_fopen ("/proc/self/mountinfo", "r");
   if (! fp)
-return NULL; /* fall through to other methods */
+goto out; /* fall through to other methods */
 
-  entries = xmalloc (entry_max * sizeof (*entries));
+  entry_len = 0;
 
   /* First, build a list of relevant visible mounts.  */
   while (getline (, , fp) > 0)
@@ -484,7 +490,6 @@ grub_find_root_devices_from_mountinfo (const char *dir, 
char **relroot)
   /* Now scan visible mounts for the ones we're interested in.  */
   for (i = entry_len - 1; i >= 0; i--)
 {
-  char **ret = NULL;
   char *fs_prefix = NULL;
   if (!*entries[i].device)
continue;
@@ -515,6 +520,23 @@ grub_find_root_devices_from_mountinfo (const char *dir, 
char **relroot)
  ret = grub_find_root_devices_from_btrfs (dir);
  fs_prefix = get_btrfs_fs_prefix (entries[i].enc_path);
}
+  else if (!retry && grub_strcmp (entries[i].fstype, "autofs") == 0)
+   {
+ /* If the best match is automounted, try to trigger mount. We cannot
+simply return here because stat() on automounted directory does not
+trigger mount and returns bogus (pseudo)device number instead.
+We keep mountpoint open until end of scan to prevent timeout. */
+
+ int flags = O_RDONLY|O_DIRECTORY;
+
+ fclose (fp);
+#ifdef O_LARGEFILE
+ flags |= O_LARGEFILE;
+#endif
+ dir_fd = open (entries[i].enc_path, flags);
+ retry = 1;
+ goto again;
+   }
   if (!ret)
{
  ret = xmalloc (2 * sizeof (ret[0]));
@@ -544,16 +566,17 @@ grub_find_root_devices_from_mountinfo (const char *dir, 
char **relroot)
}
   if (fs_prefix != entries[i].enc_root)
free (fs_prefix);
-  free (buf);
-  free (entries);
-  fclose (fp);
-  return ret;
+  break;
 }
 
+out:
   free (buf);
   free (entries);
-  fclose (fp);
-  return NULL;
+  if (fp)
+fclose (fp);
+  if (dir_fd != -1)
+close (dir_fd);
+  return ret;
 }
 
 static char *
-- 
tg: (972765f..) bug/49942 (depends on: master)

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

[PATCH] osdep/linux: handle autofs entries in /proc/self/mountinfo

[Andrei Borzenkov]

These entries have placeholder for device name and so are useless for our
purpose. grub failed with something like

grub-install: error: failed to get canonical path of `systemd-1'.

When we see autofs entry, record it (to keep parent-child relationship) but
continue to look for real mount. If it is found, we process it as usual. If
only autofs entry exists, attempt to trigger mount by opening mount point
and retry. Mount point itself is then kept open to avoid timeout.

Recent systemd is by default using automount for /boot/efi so this should
become more popular problem on EFI systems.

Closes: 49942

---

@felix, could you give it a try? Works for me.

Note that the problem happens only when probing for exact mount point. Otherwise
realpath() triggers mounting before. It is theoretically possible that this will
timeout before we scan mountinfo, although it is probably unlikely.

 grub-core/osdep/linux/getroot.c | 51 -
 1 file changed, 40 insertions(+), 11 deletions(-)

diff --git a/grub-core/osdep/linux/getroot.c b/grub-core/osdep/linux/getroot.c
index 09e7e6e..d630cf3 100644
--- a/grub-core/osdep/linux/getroot.c
+++ b/grub-core/osdep/linux/getroot.c
@@ -383,21 +383,30 @@ grub_find_root_devices_from_mountinfo (const char *dir, 
char **relroot)
   FILE *fp;
   char *buf = NULL;
   size_t len = 0;
-  grub_size_t entry_len = 0, entry_max = 4;
+  grub_size_t entry_len, entry_max = 4;
   struct mountinfo_entry *entries;
   struct mountinfo_entry parent_entry = { 0, 0, 0, "", "", "", "" };
   int i;
+  int retry = 0;
+  int dir_fd = -1;
+  char **ret = NULL;
 
   if (! *dir)
 dir = "/";
   if (relroot)
 *relroot = NULL;
 
+  entries = xmalloc (entry_max * sizeof (*entries));
+
+again:
   fp = grub_util_fopen ("/proc/self/mountinfo", "r");
   if (! fp)
-return NULL; /* fall through to other methods */
+{
+  free (entries);
+  return NULL; /* fall through to other methods */
+}
 
-  entries = xmalloc (entry_max * sizeof (*entries));
+  entry_len = 0;
 
   /* First, build a list of relevant visible mounts.  */
   while (getline (, , fp) > 0)
@@ -458,6 +467,9 @@ grub_find_root_devices_from_mountinfo (const char *dir, 
char **relroot)
}
   else
{
+ /* If this is automounted directory, continue to search for real 
device */
+ int autofs = (grub_strcmp (entry.fstype, "autofs") == 0);
+
  for (i = entry_len - 1; i >= 0; i--)
{
  if (entries[i].id == parent_entry.id)
@@ -465,7 +477,8 @@ grub_find_root_devices_from_mountinfo (const char *dir, 
char **relroot)
  /* Insert at end, pruning anything previously above this.  */
  entry_len = i + 2;
  entries[i + 1] = entry;
- break;
+ if (!autofs)
+   break;
}
  else if (i == 0 && entries[i].id == entry.id)
{
@@ -475,7 +488,8 @@ grub_find_root_devices_from_mountinfo (const char *dir, 
char **relroot)
   (entry_len - 1) * sizeof (*entries));
  entries[0] = parent_entry;
  entries[1] = entry;
- break;
+ if (!autofs)
+   break;
}
}
}
@@ -484,7 +498,6 @@ grub_find_root_devices_from_mountinfo (const char *dir, 
char **relroot)
   /* Now scan visible mounts for the ones we're interested in.  */
   for (i = entry_len - 1; i >= 0; i--)
 {
-  char **ret = NULL;
   char *fs_prefix = NULL;
   if (!*entries[i].device)
continue;
@@ -515,6 +528,23 @@ grub_find_root_devices_from_mountinfo (const char *dir, 
char **relroot)
  ret = grub_find_root_devices_from_btrfs (dir);
  fs_prefix = get_btrfs_fs_prefix (entries[i].enc_path);
}
+  else if (!retry && grub_strcmp (entries[i].fstype, "autofs") == 0)
+   {
+ /* If the best match is automounted, try to trigger mount. We cannot
+simply return here because stat() on automounted directory does not
+trigger mount and returns bogus (pseudo)device number instead.
+We keep mountpoint open until end of scan to prevent timeout. */
+
+ int flags = O_RDONLY|O_DIRECTORY;
+
+ fclose (fp);
+#ifdef O_LARGEFILE
+ flags |= O_LARGEFILE;
+#endif
+ dir_fd = open (entries[i].enc_path, flags);
+ retry = 1;
+ goto again;
+   }
   if (!ret)
{
  ret = xmalloc (2 * sizeof (ret[0]));
@@ -544,16 +574,15 @@ grub_find_root_devices_from_mountinfo (const char *dir, 
char **relroot)
}
   if (fs_prefix != entries[i].enc_root)
free (fs_prefix);
-  free (buf);
-  free (entries);
-  fclose (fp);
-  return ret;
+  break;
 }
 
   free (buf);
   free (entries);
   fclose (fp);
-  return NULL;
+  if (dir_fd != -1)
+close (dir_fd);
+  return ret;
 }
 
 static 

Re: sgi O2 supported?

[Andrei Borzenkov]

07.01.2017 22:34, Waldemar Brodkorb пишет:
> Hi,
> 
> can I use grub to bootup Linux/mips64 from SCSI disk on a
> SGI O2 machine?
> 
> I cross-compiled grub (git hash
> 07662af7aed55bcec448bc2a6610de1f0cb62100). 
> The toolchain is fine, I can bootup a Linux system via TFTP.
> 
> I create a grub.img

For which platform?

> with following modules included:
> boot linux ext2 part_dvh normal
> 
> I copy grub.img with dvhtool to the volume header and
> try to bootup:
> 
>> setenv OSLoader grub
>> boot
> 341112
> Cannot load scsi(0)disk(1)rdisk(0)partition(8)/grub.
> Range check failure: text start 0x881ffde0, size 0x53478.
> Text section would overwrite an already loaded program.Unable to
> execute scsi(0)disk(1)rdisk(0)partition(8)/grub:  not enough space
> Unable to load scsi(0)disk(1)rdisk(0)partition(8)/grub: not enough
> space
> 
> Seems like the load address is bad?
> 
> Is it possible to use grub for this task?
> 
> best regards
>  Waldemar
> 
> ___
> 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: Build error with clang 4.0

[Andrei Borzenkov]

27.12.2016 18:10, Paul Menzel пишет:
> Dear GRUB folks,
> 
> 
> Using Clang 4.0 the build fails with the error below.
> 
> ```
> $ sudo apt install clang-4.0 # Debian Sid/unstable
> $ clang-4.0 --version
> clang version 4.0.0-svn286225-1 (trunk)
> Target: i686-pc-linux-gnu
> Thread model: posix
> InstalledDir: /usr/bin
> $ git log --oneline -1
> ce95549cc efi: properly terminate filepath with NULL in chainloader
> $ ./autoconf.sh
> $ CC=clang-4.0 ../grub/configure --with-platform=coreboot --enable-boot-time
> $ make -j
> […]
> ../grub/grub-core/fs/hfs.c:699:10: error: taking address of packed member 
> 'catalog_recs' of class or structure 'grub_hfs_sblock' may result in an 
> unaligned pointer
>   value [-Werror,-Waddress-of-packed-member]
> ? (>sblock.catalog_recs)
> ^
> ../grub/grub-core/fs/hfs.c:700:10: error: taking address of packed member 
> 'extent_recs' of class or structure 'grub_hfs_sblock' may result in an 
> unaligned pointer
>   value [-Werror,-Waddress-of-packed-member]
> : (>sblock.extent_recs));
> ^~~~
> […]
> ```
> 
> The build succeeds using gcc (Debian 6.2.1-7) 6.2.1 20161215, so I am
> unsure if this is a Clang issue.
> 

I remember having seen it with LLVM/clang development snapshot before
but got distracted. But currently I cannot reproduce it using LLVM/clang
git as of yesterday, even if I explicitly add -Waddress-of-packed-member
to compile options. May be I miss some steps.

Regarding issue itself - we should not have alignment problem here, at
least on known platforms. Structure elements are properly aligned inside
of structure and structure itself is at the beginning of malloc returned
memory.

I was about to change prototype to (void *) then as I mentioned forgot
about it (or, better, issue did not happen in released clang so I did
not want to add workarounds for development snapshot).

It would be helpful if you could check when issue disappeared (if it
did). Unfortunately, full LLVM/clang build takes half a day on my system
and tears HDD apart :(



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Cannot pass a single backslash in multiboot cmdline

[Andrei Borzenkov]

26.12.2016 21:56, Jakub Jermář пишет:
> On 12/26/2016 07:24 PM, Andrei Borzenkov wrote:
>> 26.12.2016 21:12, Jakub Jermář пишет:
>>>>> I am observing a strange behavior when passing boot arguments with a
>>>>> backslash to the kernel (the multiboot cmd_line via the multiboot
>>>>> command in grub.cfg). I would like to pass foo\bar to the kernel, but to
>>>>> no avail. I tried:
>>>>>
>>>>
>>>> Which kernel? What do you load?
>>>
>>> The kernel is a modified version of HelenOS, but IMHO this issue is
>>> kernel agnostic.
>>
>> Not really. It depends on target program being loaded which is why I
>> asked what you use.
> 
> Ok, I submitted my grub.cfg and stated that HelenOS is
> multiboot-compliant, gets loaded by the multiboot command. We boot from
> an ISO image, which was created like this:
> 
> "The binary files of GRUB boot loader in this directory have been
> created by compiling GRUB for the 'i386-pc' target and then using the
> grub-mkrescue script to create the El Torito boot image."
> 
> Is there anything else I should include in order to answer your question?
> 

Multiboot only passes single "string" as command line. As far as I can
tell this is true for both multiboot1 and multiboot2. Loaded kernel
likely will need to parse this string to split it into individual
parameters. This means that to pass character(s) used to split string
verbatim you need some quoting rules.

So it is up to you to define how your kernel parses string. grub is just
the messenger here.

>>> The cmd_line is already wrong when picked up from the
>>> multiboot info and printed out (so there is no processing on it from the
>>> HelenOS side).
>>
>> Sure, but /if/ your kernel interpreted `\' as quoting character, the
>> result would be correct.
> 
> But instead of the plain actual data there would be escaped data, which
> is a hassle. And also a possible workaround.
> 

Again - how your kernel splits string into multiple parameters? Assuming
your kernel does support multiple parameters of course.

>>> I think I understand why it is eating the single backslash (expected
>>> behavior),
>>
>> You have two levels here. First level is grub command line processing.
>> It is loosely compatible with (Bourne) shell so yes, backslash is
>> treated as escape character. You can still pass backslash using usual
>> shell quoting, e.g. foo\\bar or 'foo\bar'. Second level is
>> grub_create_loader_cmdline() function. This one escapes `\' with second
>> `\'. So in the above case this function receives `foo\bar' as adds
>> second `\'.
> 
> This explains a lot, but does not make much sense for my usecase.
> 

Sure but we need to understand your use case first :)

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