gpart: add options to move the primary and secondary GPT partition tables
Needs ReviewPublic
Actions

Authored by bz on Nov 29 2021, 7:34 PM.

Details

Reviewers

imp
kevans
cem
eugen_grosbein.net
mav

Group Reviewers

manpages

Summary

Contrary to most public documentation (and our code having that hard coded)
the primary partition table does not have to start on the second LBA.
Add -l/-L to the create command to move the beginnings of the
primary and secondary partition tables ("PartitionEntryLBA" or
hdr_lba_table field in the GPT header in spec or code).

This allows one to have firmware in the early (or late) disk blocks
assuming the first two (or last) disk sectors are still available.
This gives a layout like:

| pmbr | pri hdr | ... <gap> ... | pri tbl | <1st usable LBA> .....

..... <last usable LBA> | sec tbl | ... <gap> ... | sec hdr |

MFC after: 2 months

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 43028
Build 39916: arc lint + arc unit

Event Timeline

bz created this revision.Nov 29 2021, 7:34 PM

Herald added a reviewer: manpages. · View Herald TranscriptNov 29 2021, 7:34 PM

bz requested review of this revision.Nov 29 2021, 7:34 PM

Harbormaster completed remote builds in B43028: Diff 99174.Nov 29 2021, 7:34 PM

In case you know someone else to review this please add them; I just checked revision history.
Also cannot find a #geom project to add as Subscriber; if there is a disk group that is concerned by these things please add too.

also if you know someone interested in adding the same functionality to mkimg that would be amazing!

Seems ok in principle.

lib/geom/part/geom_part.c
130–131	Why not use G_TYPE_NUMBER?
sys/geom/part/g_part.c
1008	Should it be somewhat smaller than mediasize?
sys/geom/part/g_part_gpt.c
1293–1299	It seems a bit odd to me to be passing a generic g_part_table between GPT-specific, static functions, and then re-casting it. Might as well just pass `g_part_gpt_table` directly.
1309	Seems weird to accept non-sector multiples at all.
1321	Should we reject secondary tables before primary tables? I.e., `lba <= table->lba[GPT_ELT_PRITBL]`

Thanks a lot; will address those.

lib/geom/part/geom_part.c
130–131	Because g_part_parm_quad() operates on a string (see add -b / -s) and g_part_parm_intmax() only handles an u_int which probably is not large enough for current storage anymore. Not sure why begin and size where chosen to be passed as string (possibly because they are also taking SI suffixes in user space) and it is hard to describe an option as two different things (though not impossible)?
sys/geom/part/g_part.c
1008	Indeed by at least 1 sector.
sys/geom/part/g_part_gpt.c
1293–1299	will do.
1309	Can enforce that instead of rounding up. (should round down below for tblsec anyway in that case)
1321	Yes we should; though I should go and see if UEFI enforces that. I assume it does as first and last LBA need to be ordered too from memory. I wonder if g_gpt_set_defaults() should grow further checks on start/end as well?

As usual, confining my comments to the documentation.

lib/geom/part/gpart.8
322–323	If it's growing backwards, isn't that the offset where that table ends, rather then begins? Needs clarification and disambiguation. (I can't read the code well enough to figure out what it should say.)

eugen_grosbein.net added a reviewer: mav.Nov 30 2021, 3:59 AM

I am curios to know more on how this is to be used. Extended loader in the beginning of the disk seems easy and pretty much alike to the old MBR booting. One at the end is a bit more unusual.

I think whether specification of space to reserve could be more convenient rather than specific offsets, especially at the end of the disk, where now the user has to account for the partition table size, which he does not control, to specify its start.

Leaving myself a not to look at gpart backup / restore as well.

In D33168#750111, @mav wrote:

I am curios to know more on how this is to be used. Extended loader in the beginning of the disk seems easy and pretty much alike to the old MBR booting. One at the end is a bit more unusual.

On the HoneyCOmb SD card the firmware starts at sector 8 (4k) and you probably need 4MB but NXP specifies 64MiB reserved and I have a big card so I am generous (also later when putting the efi partition at 128M rather than at 64.xM):

https://wiki.freebsd.org/BjoernZeeb/HoneyComb

I think whether specification of space to reserve could be more convenient rather than specific offsets, especially at the end of the disk, where now the user has to account for the partition table size, which he does not control, to specify its start.

That needs better understanding. The beginning of the table (by default) is the 2nd last sector; So what you need to do is use any smaller size than that; that's all; and you should know the size of your disk. The code will then write the secondary table out properly knowing its size. So table goes from "-L value - table size" to "-L value".

0mp added a subscriber: 0mp.Dec 2 2021, 10:54 AM

0mp added inline comments.

lib/geom/part/gpart.8
68	It would be great to sort those flags while we are here: f, L, l, n.
309	Cd is for kernel configuration (see `man mdoc`). If necessary, we could use Sy or Em to make this fragment stand out more. From what I see, however, GPT scheme is not emphasized in other parts of this manual. I'd just drop the extra style macro from this line but if you see it's needed, then Sy or Em are more appropriate. Tip: if you want to have live-preview a manual in your tmux session as you are working on a manual page, then check out textproc/mantra.
320	L should go before l.

zlei added a subscriber: zlei.Sep 28 2022, 8:57 AM

Can we get an update on the status of this review?

In D33168#914780, @bcr wrote:

Can we get an update on the status of this review?

Has it become interesting to anyone else? I think I may have a slightly updated version from back then but I think the backup/restore bits were still missing.