Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

efi: new 'connectefi' command #108

Open
wants to merge 291 commits into
base: rhel-9-main
Choose a base branch
from

Conversation

rmetrich
Copy link
Contributor

Integrated Glen's comments (except connectefi):

  • use grub_list_* instead of own list
  • added all option

vathpela and others added 30 commits June 11, 2021 12:11
This makes us use pretty names in the titles we generate in
grub2-mkconfig when GRUB_DISTRIBUTOR isn't set.

Resolves: rhbz#996794

Signed-off-by: Peter Jones <[email protected]>
Resolves: rhbz#1065360
Signed-off-by: Peter Jones <[email protected]>
Related: rhbz#1148652

Signed-off-by: Peter Jones <[email protected]>
Users reported that newly installed kernels on their systems installed
with grub-mkconfig would not appear on the grub boot list in order
starting with the most recent. Added an option for rpm-based systems to
use the rpm-sort library to sort kernels instead.

Resolves rhbz#1124074

Signed-off-by: Robert Marshall <[email protected]>
[pjones: fix --enable-rpm-sort configure option]
Signed-off-by: Peter Jones <[email protected]>
[thierry.vignaud: fix build with rpm-4.16]
Signed-off-by: Thierry Vignaud <[email protected]>
…elsewhere.

Resolves: rhbz#1215839

Signed-off-by: Peter Jones <[email protected]>
Provided a tool for users to reset the grub2 root user password
without having to alter the grub.cfg. The hashed password now
lives in a root-only-readable configuration file.

Resolves: rhbz#985962

Signed-off-by: Robert Marshall <[email protected]>
[pjones: fix the efidir in grub-setpassword and rename tool]
Signed-off-by: Peter Jones <[email protected]>
[luto: fix grub-setpassword -o's output path]
Andy Lutomirski <[email protected]>
Sometimes we have to provision boxes across regions, such as California to
Sweden.  The http server has a 10 minute timeout, so if we can't get our 250mb
image transferred fast enough our provisioning fails, which is not ideal.  So
add tcp window scaling on open connections and set the window size to 1mb.  With
this change we're able to get higher sustained transfers between regions and can
transfer our image in well below 10 minutes.  Without this patch we'd time out
every time halfway through the transfer.  Thanks,

Signed-off-by: Josef Bacik <[email protected]>
This patch adds grub-get-kernel-settings, which reads the system kernel
installation configuration from /etc/sysconfig/kernel, and outputs
${GRUB_...} variables suitable for evaluation by grub-mkconfig.  Those
variables are then used by 10_linux to choose whether or not to create
debug stanzas.

Resolves: rhbz#1226325
The netmask configured in firmware is not respected on ppc64 (big endian).
When 255.255.252.0 is set as netmask in firmware, the following is the value of bootpath string in grub_ieee1275_parse_bootpath().

 /vdevice/l-lan@30000002:speed=auto,duplex=auto,192.168.88.10,,192.168.89.113,192.168.88.1,5,5,255.255.252.0,512

The netmask in this bootpath is no problem, since it's a value specified in firmware. But,
The value of 'subnet_mask.ipv4' was set with 0xfffffc00, and __builtin_ctz (~grub_le_to_cpu32 (subnet_mask.ipv4)) returned 16 (not 22).
As a result, 16 was used for netmask wrongly.

1111 1111 1111 1111 1111 1100 0000 0000 # subnet_mask.ipv4 (=0xfffffc00)
0000 0000 1111 1100 1111 1111 1111 1111 # grub_le_to_cpu32 (subnet_mask.ipv4)
1111 1111 0000 0011 0000 0000 0000 0000 # ~grub_le_to_cpu32 (subnet_mask.ipv4)

And, the count of zero with __builtin_ctz can be 16.
This patch changes it as below.

1111 1111 1111 1111 1111 1100 0000 0000 # subnet_mask.ipv4 (=0xfffffc00)
0000 0000 1111 1100 1111 1111 1111 1111 # grub_le_to_cpu32 (subnet_mask.ipv4)
1111 1111 1111 1111 1111 1100 0000 0000 # grub_swap_bytes32(grub_le_to_cpu32 (subnet_mask.ipv4))
0000 0000 0000 0000 0000 0011 1111 1111 # ~grub_swap_bytes32(grub_le_to_cpu32 (subnet_mask.ipv4))

The count of zero with __builtin_clz can be 22. (clz counts the number of one bits preceding the most significant zero bit)
This needs to be hooked up to --program-transform=, but I haven't had
time.

Signed-off-by: Peter Jones <[email protected]>
Since our bugs tell us that the xnu boot entries really just don't work
most of the time, and they create piles of extra boot entries, because
they can't quite figure out 32-vs-64 and other stuff like that.

It's rediculous, and we should just boot their bootloader through the
chainloader instead.

So this patch does that.

Resolves: rhbz#893179

Signed-off-by: Peter Jones <[email protected]>
This patch adds the ability to specify a different root on a btrfs
filesystem too boot from other than the default one.

btrfs-list-snapshots <dev> will list the subvolumes available on the
filesystem.

set btrfs_subvol=<path> and set btrfs_subvolid=<subvolid> will specify
which subvolume to use and any pathnames provided with either of those
variables set will start using that root. If the subvolume or subvolume id
doesn't exist, then an error case will result.

It is possible to boot into a separate GRUB instance by exporting the
variable and loading the config file from the subvolume.

Signed-off-by: Jeff Mahoney <[email protected]>
We should export btrfs_subvol and btrfs_subvolid to have both visible
to subsidiary configuration files loaded using configfile.

Signed-off-by: Michael Chang <[email protected]>
This uses grub_efi_allocate_pool(), grub_efi_free_pool(), and
grub_efi_free_pages() instead of open-coded efi_call_N() calls, so we
get more reasonable type checking.

Signed-off-by: Peter Jones <[email protected]>
This avoids syntax checkers getting confused about if it's llx or lx.

Signed-off-by: Peter Jones <[email protected]>
This should never be trying this, and since we've consolidated the
grubenv to always be on /boot/efi/EFI/fedora/, this code causes it to
always make the wrong decision.

Resolves: rhbz#1484474

Signed-off-by: Peter Jones <[email protected]>
vathpela and others added 15 commits June 3, 2022 14:08
For NX, we need to set the page access permission attributes for write
and execute permissions.

This patch adds two new primitives, grub_set_mem_attrs() and
grub_clear_mem_attrs(), and associated constant definitions, to be used
for that purpose.

For most platforms, it adds a dummy implementation that returns
GRUB_ERR_NONE.  On EFI platforms, it adds a common helper function,
grub_efi_status_to_err(), which translates EFI error codes to grub error
codes, adds headers for the EFI Memory Attribute Protocol (still pending
standardization), and an implementation of the grub nx primitives using
it.

Signed-off-by: Peter Jones <[email protected]>
[rharwood: add pjones's none/nyi fixup]
Signed-off-by: Robbie Harwood <[email protected]>
(cherry picked from commit 35de78a8d32b9fad5291ec96fd3cbb9cf2f4a80b)
For NX, we need to set write and executable permissions on the sections
of grub modules when we load them.

On sections with SHF_ALLOC set, which is typically everything except
.modname and the symbol and string tables, this patch clears the Read
Only flag on sections that have the ELF flag SHF_WRITE set, and clears
the No eXecute flag on sections with SHF_EXECINSTR set.  In all other
cases it sets both flags.

Signed-off-by: Peter Jones <[email protected]>
[rharwood: arm tgptr -> tgaddr]
Signed-off-by: Robbie Harwood <[email protected]>
(cherry-picked from commit ca74904ede0406b594cbedc52ce8e38a6633d2ae)
For NX, our kernel loaders need to set write and execute page
permissions on allocated pages and the stack.

This patch adds those calls.

Signed-off-by: Peter Jones <[email protected]>
[rharwood: fix aarch64 callsites]
(cherry-picked from commit a9f79a997f01a83b36cdfa89ef2e72ac2a17c06c)
[rharwood: uninitialized stack_attrs, double verification]
Signed-off-by: Robbie Harwood <[email protected]>
For NX, we need the grub binary to announce that it is compatible with
the NX feature.  This implies that when loading the executable grub
image, several attributes are true:

- the binary doesn't need an executable stack
- the binary doesn't need sections to be both executable and writable
- the binary knows how to use the EFI Memory Attributes protocol on code
  it is loading.

This patch adds a definition for the PE DLL Characteristics flag
GRUB_PE32_NX_COMPAT, and changes grub-mkimage to set that flag.

Signed-off-by: Peter Jones <[email protected]>
(cherry picked from commit 0c7f1aed5a87f75051b421903a900ccb4bbd795a)
If one of the file filters breaks things, it's hard to figure out where
it has happened.

This makes grub log which filter is being run, which makes it easier to
figure out where you are in the sequence of events.

Signed-off-by: Peter Jones <[email protected]>
(cherry picked from commit d3d6518)
Currently when populating the initial memory arena on EFI systems, we
count the available regions below GRUB_EFI_MAX_ALLOCATION_ADDRESS from
the EFI memory map and then allocates one quarter of that for our arena.

Because many systems come up without IOMMUs, we currently set
GRUB_EFI_MAX_ALLOCATION_ADDRESS to 0x7fffffff, i.e. all addresses
allocated must be below 2G[0].  Due to firmware and other
considerations, this makes the most memory we can possibly have in our
arena 512M.

Because our EFI loader doesn't get kernel and initrd memory from grub's
allocator, but rather reserves it directly from UEFI and then simply
marks those as allocated if they're within grub's arena, it was
historically possible to have initrds that are larger than 512M, because
we could use any memory region below 4G, without concern for grub's
choice of arena size.

Unfortunately, when we switched to using the "verifiers" API (and thus
the file_filter_t API) to do measurement of kernel and initrd, this
introduced a pattern that allocates the entire file when we call
grub_file_open(), and buffers it to pass to the filter.  This results in
needing to have enough space for the initramfs in the grub arena.

This is bad.

Since it's unlikely you're going to do anything *other* than loading a
kernel and initramfs that takes much of the available free memory from
UEFI, this patch introduces a workaround by changing the amount we give
to the arena be three quarters of the available memory, rather than one
quarter, thus changing our theoretical initrd limit to 1.5G.  In
practice, it may still be smaller than that depending on allocation
fragmentation, but generally it will be most of it.

Note that this doesn't fix the underlying flaw, which is that there is
no safe way to do the validation correctly using the "verifiers" system
with the current file API without buffering the whole file before
grub_file_read() is ever called, and thus you can't set an allocation
policy for the initial buffer of the file at all, so unless we raise the
allocation limit to >4G, it can't be allocated in the big region.

[0] I'm not sure there was a good reason not to pick 4G, but even if we
    had, at least one common firmware routes the first 2G of physical
    RAM to 0x0, and any additional memory starting at 0x100000000.

Related: rhbz#2112134

Signed-off-by: Peter Jones <[email protected]>
(cherry picked from commit 005a0aa)
In our kernel allocator on EFI systems, we currently have a growing
amount of code that references the various allocation policies by
position in the array, and of course maintenance of this code scales
very poorly.

This patch changes them to be enumerated, so they're easier to refer to
farther along in the code without confusion.

Signed-off-by: Peter Jones <[email protected]>
(cherry picked from commit 6768026)
Currently in our kernel allocator, we use the same set of choices for
all of our various kernel and initramfs allocations, though they do not
have exactly the same constraints.

This patch adds the concept of an allocation purpose, which currently
can be KERNEL_MEM or INITRD_MEM, and updates kernel_alloc() calls
appropriately, but does not change any current policy decision.  It
also adds a few debug prints.

Signed-off-by: Peter Jones <[email protected]>
(cherry picked from commit 36307be)
Currently on x86, only linux kernels built with CONFIG_RELOCATABLE for
x86_64 can be loaded above 4G, but the maximum address for the initramfs
is specified via a HdrS field.  This allows us to utilize that value,
and unless loading the kernel above 4G, uses the value present there.
If loading kernel above 4G is allowed, we assume loading the initramfs
above 4G also works; in practice this has been true in the kernel code
for quite some time.

Resolves: rhbz#2112134

Signed-off-by: Peter Jones <[email protected]>
(cherry picked from commit 3e08c35)
At some point due to an erroneous kernel warning, we switched kernel and
initramfs to being loaded in EFI_RUNTIME_SERVICES_CODE and
EFI_RUNTIME_SERVICES_DATA memory pools.  This doesn't appear to be
correct according to the spec, and that kernel warning has gone away.

This patch puts them back in EFI_LOADER_CODE and EFI_LOADER_DATA
allocations, respectively.

Resolves: rhbz#2108456

Signed-off-by: Peter Jones <[email protected]>
(cherry picked from commit 35b5d5f)
Signed-off-by: Robbie Harwood <[email protected]>
(cherry picked from commit 0837dcd)
As a legacy support, if the vector 5 is not implemented, Power
Hypervisor will consider the max CPUs as 64 instead 256 currently
supported during client-architecture-support negotiation.

This patch implements the vector 5 and set the MAX CPUs to 256 while
setting the others values to 0 (default).

Signed-off-by: Diego Domingos <[email protected]>
Signed-off-by: Robbie Harwood <[email protected]>
(cherry picked from commit f735c65)
Signed-off-by: Robbie Harwood <[email protected]>
(cherry picked from commit 275a048)
Signed-off-by: Robbie Harwood <[email protected]>
(cherry picked from commit 12354f5)
On OSTree systems, `grub2-mkconfig` is run with `/etc` mounted read-only
because as part of the promise of transactional updates, we want to make
sure that we're not modifying the current deployment's state (`/etc` or
`/var`).

This conflicts with 0837dcd ("BLS: create /etc/kernel/cmdline during
mkconfig") which wants to write to `/etc/kernel/cmdline`. I'm not
exactly sure on the background there, but based on the comment I think
the intent is to fulfill grubby's expectation that the file exists.

However, in systems like Silverblue, kernel arguments are managed by the
rpm-ostree stack and grubby is not shipped at all.

Adjust the script slightly so that we only write `/etc/kernel/cmdline`
if the parent directory is writable.

In the future, we're hoping to simplify things further on rpm-ostree
systems by not running `grub2-mkconfig` at all since libostree already
directly writes BLS entries. Doing that would also have avoided this,
but ratcheting it into existing systems needs more careful thought.

Signed-off-by: Jonathan Lebon <[email protected]>

Fixes: fedora-silverblue/issue-tracker#322
(cherry picked from commit 3c3d1a3)
@frozencemetery frozencemetery changed the base branch from rhel-9-main to master August 31, 2022 17:37
@frozencemetery frozencemetery changed the base branch from master to rhel-9-main August 31, 2022 17:44
@frozencemetery
Copy link
Member

PR was opened against rhel-9-main, which is fine for my use, but it doesn't apply to upstream master. I'm not sure I understand in general - is the intent to squash this with your previous patch and propose that upstream? If so, that's fine - your changes look reasonable at a glance. I have no opinion on "connectefi" vs. "eficonnect", though it sounds like you and Glenn might :)

@rmetrich
Copy link
Contributor Author

rmetrich commented Aug 31, 2022 via email

When efi.quickboot is enabled on VMWare (which is the default for
hardware release 16 and later), it may happen that not all EFI devices
are connected. Due to this, browsing the devices in make_devices() just
fails to find devices, in particular disks or partitions for a given
disk.
This typically happens when network booting, then trying to chainload to
local disk (this is used in deployment tools such as Red Hat Satellite),
which is done through using the following grub.cfg snippet:
-------- 8< ---------------- 8< ---------------- 8< --------
unset prefix
search --file --set=prefix /EFI/redhat/grubx64.efi
if [ -n "$prefix" ]; then
  chainloader ($prefix)/EFI/redhat/grubx64/efi
...
-------- 8< ---------------- 8< ---------------- 8< --------

With efi.quickboot, none of the devices are connected, causing "search"
to fail. Sometimes devices are connected but not the partition of the
disk matching $prefix, causing partition to not be found by
"chainloader".

This patch introduces a new "eficonnect pciroot|scsi|all" command whic
recursively connects all EFI devices starting from a given controller
type:
- if 'pciroot' is specified, recursion is performed for all PCI root
  handles
- if 'scsi' is specified, recursion is performed for all SCSI I/O
  handles (recommended usage to avoid connecting unwanted handles which
  may impact Grub performances)
- if 'all' is specified, recursion is performed on all handles (not
  recommended since it may heavily impact Grub performances)

Typical grub.cfg snippet would then be:
-------- 8< ---------------- 8< ---------------- 8< --------
eficonnect scsi
unset prefix
search --file --set=prefix /EFI/redhat/grubx64.efi
if [ -n "$prefix" ]; then
  chainloader ($prefix)/EFI/redhat/grubx64/efi
...
-------- 8< ---------------- 8< ---------------- 8< --------

The code is easily extensible to handle other arguments in the future if
needed.

Signed-off-by: Renaud Métrich <[email protected]>
@rmetrich
Copy link
Contributor Author

rmetrich commented Sep 1, 2022

@frozencemetery This is now a mirror of the patchset from Upstream (currently v4 submitted today)

@frozencemetery
Copy link
Member

frozencemetery commented Sep 1, 2022

Regarding connectefi, if we change into eficonnect, how will we manage this on RHEL, through some aliasing?

We wouldn't, unless you want it to be. "We're already shipping it this way on RHEL" is a good counter to "please rename this to something else" - upstream generally respects reasonable compatibility with shipped code.

@rmetrich
Copy link
Contributor Author

rmetrich commented Sep 1, 2022

ok that has to be "fixed" somehow. For now there are no clients for this feature anyway, except Satellite but I'm not even sure the template is already out.
Somehow I now prefer "eficonnect" because it's not a "user command" since there is no output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.