Add a new option to gpart(8) to fix Lenovo BIOS boot issue

[allanjude]

Summary

Add an option to gpart(8) to location the fake GPT (0xee) entry in the pMBR in a non-default location
Writing the 0xee entry to the 2nd partition slot, instead of the first, allows afflicted Lenovo's (x220, t420, t520) to boot GPT in BIOS mode

[allanjude]

sys/geom/part/g_part_gpt.c

998 ↗

(On Diff #6886)

When 'gpart recover' is used, it will undo the lenovo fix. the pmbr is not backed up, so there is no way to tell if the fix had previously been applied

[ae]

I prefer to not introduce new option. I think it will be better add new scheme-specific attribute. Something like

1. gpart set -a lenovofix ada0

or

1. gpart set -a pmbrentry -i 2 ada0

[allanjude]

In D3065#60637, @ae wrote:

I prefer to not introduce new option. I think it will be better add new scheme-specific attribute. Something like

1. gpart set -a lenovofix ada0

or

1. gpart set -a pmbrentry -i 2 ada0

The advantage to doing this as a 'set' command is that it makes it easy to apply after-the-fact as well, instead of only at create time, which resolves the potential issue with 'gpart recover'

I don't think it will be possible to overload the -i flag like in your second example, but considering the use case, the first example seems like it should work.

[ae]

In D3065#60653, @allanjude wrote:

I don't think it will be possible to overload the -i flag like in your second example, but considering the use case, the first example seems like it should work.

In g_part_gpt_setunset() you have the attribute name and pointer to partition entry baseentry, where you know the number of partition entry.

[allanjude]

In D3065#60654, @ae wrote:

In D3065#60653, @allanjude wrote:

I don't think it will be possible to overload the -i flag like in your second example, but considering the use case, the first example seems like it should work.

In g_part_gpt_setunset() you have the attribute name and pointer to partition entry baseentry, where you know the number of partition entry.

Right, but in the case of 'gpart set -a <flag> -i <index> ada0'

the -i refers to the <index>'th GPT partition, which is unrelated to the fake pMBR partition table.

But I think 'gpart set -a lenovofix ada0' makes sense, and I'll rewrite the patch to do it that way.

[allanjude]

Rewrite the patch to use 'gpart set -a lenovofix <provider>'
This is cleaner than adding a new flag to 'gpart create' and allows it to be done at any time

[ae]

Maybe you need to mention that after apply lenovofix, you will lose all PMBR entries. So if you have configured bootcamp, or have set/unset "active" flag - all these changes will be lost. Also you forgot to check/handle, what type of action user asks - set or unset.

sbin/geom/class/part/gpart.8

928 ↗

(On Diff #6907)

Don't forget to bump .Dd

sys/geom/part/g_part_gpt.c

1054 ↗

(On Diff #6907)

This isn't necessary, you didn't touch PMBR's magic.

[wblock]

sbin/geom/class/part/gpart.8

937 ↗

(On Diff #6907)

Is "pMBR" the preferred capitalization? It is not listed anywhere else in gpart.8.

Also:
s/2nd/second/

938 ↗

(On Diff #6907)

Start a new sentence on a new line. The moose out front should have told you.

[marcel]

As for bootcamp: we don't create it and one doesn't need the lenovo fix on machines that have bootcamp. I don't think we should over- engineer a pure theoretical case. Likewise for the active flag -- that is, if the active flag must be set on Lenovo machines, it better be set when the lenovo fix is applied.

While technically speaking the above is not perfect, we're talking about tweaks to make certain machines boot (whether active flag or moving the 0xee partition to the second entry). It's more concerning that recover will undo the tweak, though.

sbin/geom/class/part/gpart.8

937 ↗

(On Diff #6907)

It's PMBR, with a capital P. It's probably also better not to call it a "GPT PMBR", but rather just "PMBR" or alternatively "Protective MBR".

[allanjude]

Update with feedback from everyone

• do not rewrite the DOSMAGIC
• unset now restores an ordinary PMBR
• man page fixes

[marcel]

Thanks Allan!

[allanjude]

In D3065#60972, @marcel wrote:

Thanks Allan!

As I am currently only a doc committer, I need an 'Approved By:' to commit this. Are you willing to provide that?

[wblock]

One note, but man page is otherwise good. Remember to test with igor and mandoc -Tlint before commit.

sbin/geom/class/part/gpart.8

939 ↗

(On Diff #6939)

"in" seems... not right. I'd suggest "on" or "with" instead.

[allanjude]

Updated with feedback from wblock

[wblock]

Man page is good.

[marcel]

You can add me as approver.