Page MenuHomeFreeBSD

Make bsdinstall/autopart put swap before the root filesystem in GPT configurations
Needs ReviewPublic

Authored by allanjude on May 5 2016, 6:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 18, 11:37 AM
Unknown Object (File)
Thu, Apr 18, 11:36 AM
Unknown Object (File)
Sat, Apr 6, 12:56 PM
Unknown Object (File)
Feb 24 2024, 4:03 AM
Unknown Object (File)
Dec 20 2023, 12:45 AM
Unknown Object (File)
Nov 30 2023, 8:55 PM
Unknown Object (File)
Nov 13 2023, 1:22 PM
Unknown Object (File)
Oct 25 2023, 8:53 AM

Details

Summary

Root must come first in MBR, but for GPT, having root last makes it easily resizable

While here, if scheme == GPT, always install BOTH partcodes, so the system can be converted back and forth between EFI/CSM

Todo: align partitions

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 3588
Build 3628: arc lint + arc unit

Event Timeline

allanjude retitled this revision from to Make bsdinstall/autopart put swap before the root filesystem in GPT configurations.
allanjude updated this object.
allanjude edited the test plan for this revision. (Show Details)
allanjude added reviewers: dim, dteske, nwhitehorn, trasz, imp.
usr.sbin/bsdinstall/partedit/partedit_x86.c
162

This hard codes the size of the EFI fat partition. Ideally, we'd stat the boot1.efifat file and use its size.

Even better would be to allocate 5MB of space, newfs_msdos it and copy /boot/boot1.efi to there
as efi\boot\bootx64.efi so that people have a little bit of extra space for upgrades to their BIOSes and
such. OTOH, this may be too esoteric to do by default. So if you don't do this, at least do the stat thing.

188

However here I'd be tempted to leave this as is because the boot blocks can grow up to about 540k in size before there's issues with boot1. boot1.efi can be substantially larger before there's an issue, but boot1 really can't.

rpokala added inline comments.
usr.sbin/bsdinstall/partedit/gpart_ops.c
453

Rather than calling partcode_path() twice, save the result here and reuse it below..

usr.sbin/bsdinstall/partedit/part_wizard.c
370

This space (1MB -> 2MB) is for...?

373

Add a comment explaining why the two cases are handled differently

usr.sbin/bsdinstall/partedit/partedit_x86.c
162

imp

TBH, I never understood why we created boot1.efifat in the first place, rather than just using 'newfs_msdos' and 'cp'.

usr.sbin/bsdinstall/partedit/part_wizard.c
370

the bookcode grew. It was 512 or 800kb, it is now 512+800kb.

usr.sbin/bsdinstall/partedit/partedit_x86.c
162

my understanding was that our newfs_msdos did not always produce a file system that all computer could boot from. I am not sure the details, maybe emaste knows

Aside from the issue with swap and root ordering, this looks mostly reasonable at first pass.

It would be nice if the logic were less convoluted, however, especially on non-x86 systems. Right now, there are functions (bootpart_type(), bootpart_size(), bootcode_path(), partcode_path()) to abstract out the GEOM parts of this code. This patch adds yet another function that bypasses the other ones. It would be really good to have this one way or the other: either have x86 use the abstractions (somehow, potentially expanding them) or have all the platforms use the GEOM bits directly.

usr.sbin/bsdinstall/partedit/part_wizard.c
373

See the other revision for why this is a bad idea from a security POV. Please keep this in a separate patch for the time being until our boot loaders can handle this case properly.

usr.sbin/bsdinstall/partedit/partedit_x86.c
162

We're making boot1.efifat to simplify the logic in the installer. The EFI boot partition is not mounted anywhere in the current scheme of things and we have no other platforms with per-platform file system requirements. Thus, making it with newfs_msdos would require a great deal of specialty logic not otherwise required.

It would be nice if the logic were less convoluted, however, especially on non-x86 systems. Right now, there are functions (bootpart_type(), bootpart_size(), bootcode_path(), partcode_path()) to abstract out the GEOM parts of this code. This patch adds yet another function that bypasses the other ones. It would be really good to have this one way or the other: either have x86 use the abstractions (somehow, potentially expanding them) or have all the platforms use the GEOM bits directly.

Yeah, the complexity here came from the fact that I expanded it to install both bootcodes, gptzfsboot and boot1.efifat.

Maybe the idea would be to create 1 abstraction for each platform, that creates the bootcode partitions, rather than having the abstractions to get the size, contents, etc.

I like that idea a lot. Unless you feel like you have time to do that soon, the interface shouldn't be the thing to block the commit.

usr.sbin/bsdinstall/partedit/partedit_x86.c
162

@allanjude newfs_msdos can create them: the boot1.efifat images are created with newfs_msdos -F 12 -L EFI $DEVICE. See sys/boot/efi/boot1/generate-fat.sh.

I do think we want some improved logic for handling the ESP as part of broader installer improvements - ideally the installer could find an existing ESP on the target device and offer to install boot1.efi into it. The pre-canned FAT image was a convenient way to fit EFI into the existing installer/gpart infrastructure, but there's no reason it has to be done that way.

usr.sbin/bsdinstall/partedit/partedit_x86.c
162

Some ARM SoCs require a FAT partition with UBOOT (and often other) images on it. We currently kinda hack around that by DDing images created by NanoBSD, Crochet, or the release process onto an SD card. It's a flaw that the installer doesn't support creating them, but since there's usually only one device, you'd have to netboot the installer, which on many SoCs requires those fat partition and files. The UEFI support is just another instance where that's required, but with different files required.

Some ARM SoCs require a FAT partition with UBOOT (and often other) images on it.

My MIPS ERL has a FAT boot fs as well.

In any case I don't think we want to change the way the installer handles this before 11.0 (coming too quickly) but it is something we should take a look at afterwards.

Some ARM SoCs require a FAT partition with UBOOT (and often other) images on it.

My MIPS ERL has a FAT boot fs as well.

In any case I don't think we want to change the way the installer handles this before 11.0 (coming too quickly) but it is something we should take a look at afterwards.

Agreed on that. The main point that this is a thing we'll have to do more of with arbitrary lists of files in the future, though, remains valid. The timing of when we have to do it is one for debate (though I fall on the post 11.0 side of the spectrum), but the need is real.

It's not that bad to do, some minor refactoring at worst (the installer already knows how to set up FAT partitions). What does need to happen is that we need decide where they get mounted, what files should go onto them, how updates work, etc. It's a policy issue rather than a technical one, and one we can easily fix after 11.

So, what's the status on this?

bcran added inline comments.
usr.sbin/bsdinstall/partedit/partedit_x86.c
162

@imp We'll probably want at least a 200 MB EFI FAT partition, since there's normally only one per system and it gets used in multi-boot scenarios.

https://wiki.archlinux.org/index.php/EFI_system_partition says:

To avoid potential problems with some UEFI implementations, the ESP should be formatted with FAT32 and the size should be at least 512 MiB. 550 MiB is recommended to avoid MiB/MB confusion and accidentally creating FAT16 [1], although larger sizes are fine.

According to a Microsoft note[2], the minimum size for the EFI system partition (ESP) would be 100 MiB, though this is not stated in the UEFI Specification. Note that for Advanced Format 4K Native drives (4-KiB-per-sector) drives, the size is at least 256 MiB, because it is the minimum partition size of FAT32 drives (calculated as sector size (4KiB) x 65527 = 256 MiB), due to a limitation of the FAT32 file format.

usr.sbin/bsdinstall/partedit/partedit_x86.c
162

This diff is old, currently we create a 200MB FAT16. For many images, 500+MB is getting to be a rather large bit of the available space, especially for VMs and USB images etc.

usr.sbin/bsdinstall/partedit/partedit_x86.c
162

Oh, thanks. Good point about VMs etc., I agree 500+ MB is rather large for them.

The UEFI 2.3.1 specification mandates the use of FAT32 though, so FAT16 is non-standard:

EFI encompasses the use of FAT32 for a system partition, and FAT12 or FAT16 for removable media. The FAT32 system partition is identified by an OSType value other than that used to identify previous versions of FAT. This unique partition type distinguishes an EFI defined file system from a normal FAT file system. The file system supported by EFI includes support for long file names.

(seen via https://bugs.launchpad.net/ubuntu/+source/partman-efi/+bug/811485)

Was this review overcome by events?

Make bsdinstall/autopart put swap before the root filesystem in GPT configurations

I did have swap first with an installation of 12.3 that was performed in June 2022:

root@fuji:~ # lsblk
DEVICE         MAJ:MIN SIZE TYPE                                          LABEL MOUNT
ada0             0:127 149G GPT                                               - -
  ada0p1         0:129 512K freebsd-boot                           gpt/gptboot0 -
  <FREE>         -:-   492K -                                                 - -
  ada0p2         0:131  16G freebsd-swap                              gpt/swap0 SWAP
  ada0p2.eli     0:161  16G freebsd-swap                                      - SWAP
  ada0p3         0:133 133G freebsd-zfs                                gpt/zfs0 <ZFS>
  ada0p3.eli     0:140 133G zfs                                               - -
  <FREE>         -:-   836K -                                                 - -
root@fuji:~ # bectl list -c creation
BE                                Active Mountpoint Space Created
default                           -      -          1.30G 2022-06-11 19:03
12.3-RELEASE-p5_2022-06-11_212957 -      -          86.6M 2022-06-11 21:29
13.1-RELEASE_2022-06-11_213806    -      -          2.75M 2022-06-11 21:38
13.1-RELEASE_2022-06-11_214835    -      -          13.3M 2022-06-11 21:48
13.1-RELEASE_2022-06-18           -      -          974M  2022-06-18 19:56
13.1-RELEASE_2022-06-29           -      -          402M  2022-06-29 20:02
13.1-RELEASE_2022-08-02           -      -          208M  2022-08-02 18:18
13.1-RELEASE_2022-08-05           -      -          2.13G 2022-08-05 18:00
13.1-RELEASE_2022-08-11_011205    -      -          51.2M 2022-08-11 01:12
13.1-RELEASE-p1_2022-09-18_042630 -      -          599M  2022-09-18 04:26
13.1-RELEASE_2022-10-10           -      -          754M  2022-10-10 02:58
13.1-RELEASE_2022-10-28           -      -          749M  2022-10-28 23:37
13.1-RELEASE-p2_2022-11-03_174754 -      -          551M  2022-11-03 17:47
13.1-RELEASE-p3_2022-11-20_101333 -      -          608M  2022-11-20 10:13
13.1-RELEASE-p4_2022-12-10_144333 -      -          926M  2022-12-10 14:43
13.1-RELEASE-p5_2023-02-11_093418 -      -          2.74G 2023-02-11 09:34
13.1-RELEASE-p6_2023-02-23_182708 -      -          2.36G 2023-02-23 18:27
13.1-RELEASE-p7_2023-03-19_154504 NR     /          44.1G 2023-03-19 15:45
13.2-RC3_2023-03-19_160134        -      -          5.19M 2023-03-19 16:01
13.2-RC3_2023-03-24               -      -          1.14G 2023-03-24 01:04
13.2-RC3_2023-03-26_045052        -      -          4.62M 2023-03-26 04:50
13.2-RC4_2023-03-26_052745        -      -          4.72M 2023-03-26 05:27
13.2-RC4_2023-04-01_215204        -      -          4.76M 2023-04-01 21:52
13.2-RC6_2023-04-02_023914        -      -          11.3G 2023-04-02 02:39
13.2-RC6_2023-04-09_100730        -      -          198M  2023-04-09 10:07
13.1-RELEASE-p7_2023-04-10_192020 -      -          3.22M 2023-04-10 19:20
13.1-RELEASE-p7_2023-04-10_205201 -      -          4.03M 2023-04-10 20:52
13.2-RELEASE_2023-04-10_223831    -      -          4.54M 2023-04-10 22:38
root@fuji:~ # freebsd-version -kru
13.2-RELEASE
13.2-RELEASE
13.2-RELEASE
root@fuji:~ #