On Tue, May 22, 2018 at 09:31:16PM +0100, Grant Likely wrote:
Hi Daniel,
Thanks for writing this. Good job. Comments below.
On 22/05/2018 20:17, Daniel Thompson wrote:
Fixes: #3 Fixed: #8 Signed-off-by: Daniel Thompson daniel.thompson@linaro.org
Nit: I'd like to maintain good patch practice. Please include commit description in patches.
Will do.
Notes: This patch tries to capture contributions from a long a varied discussion. I hope I haven't missed anything major.
Thanks to all the contributors to this topic so far!source/ebbr.rst | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
diff --git a/source/ebbr.rst b/source/ebbr.rst index 40f03f173bd9..30a9c6ac2666 100644 --- a/source/ebbr.rst +++ b/source/ebbr.rst @@ -185,6 +185,53 @@ System Volume Format
The system firmware must support GPT partitioning. +It may optionally also support MBR partitioning.
I think there are two sides of this; what the firmware supports, and what the OS supports. Should we specify this?
For the firmware: GPT required, MBR optional For the OS: GPT & MBR required
Looking at this fresh eyes, the UEFI spec requires GPT, MBR and el torito.
Is there any need to EBBR to say *anything* on this topic of what firmware and OS supports, except to require (where not impossible) that any pre-formatted media use GPT?
In other words maybe just delete this paragraph.
Should it also specify that GPT partitioning must strictly follow the UEFI specification, and must not use a hybrid-MBR?
Can do, although like the above, we would merely be restating part of the UEFI spec.
+On systems where the system firmware binaries reside on the System Volume then +the System Volume must be pre-configured with a partition table and include +protective partitions to reduce risk of accidental destruction of the system +firmware.
+All pre-configured partition tables must use GPT partitioning unless +some immutable feature of the platform (such as a mask programmed boot ROM) +makes this impossible; on such platforms MBR partitioning may be +used as an alternative
Looks good. It would be useful to have some discussion about the possible scenarios, and how this plays out.
In this patch or in examples in the informative sections of the doc?
+GPT partitioning +^^^^^^^^^^^^^^^^
+Protective partitions should have the Platform Required Attribute Flag set +unless some immutable feature of the platform makes this impossible.
Do we know of any platforms where Platform Required cannot be set? If not, then I would drop the caveat.
Nothing in the wiki, although there is the certainly evidence of ROMish parsing of GPT tables [and therefore potential for touble ;-) ].
+It is recommended that automatic system disk partitioning utilities > +preserve Platform Required partitions as is, and that manual disk +partitioning utilities provide warnings and/or other safe guards to +reduce risk of accidental removal
Suggested reword: Automatic system disk partitioning utilities shall preserve Platform Required partitions as is. Manual disk partitioning utilities should provide warnings and/or other safe guards to reduce the risk of accidental removal.
Looks good, but if we're upgrading from should to shall then I want to work in the "unless it recognizes this partition" language from UEFI spec (in case of spurious bad labelling).
Will do this in the next revision.
+It is recommended that an implementer on a platform where Platform Required +cannot be set contribute a list of Partition type GUIDs for protective +partitions to the table below. It is further recommended that disk partitioning +utilities treat such partitions in the same manner as those with the Platform +Required Attribute Flag set.
++--------------------------------------+---------------------------------------+ +| Partition type GUID | Comment | ++======================================+=======================================+ +| 00000000-0000-0000-0000-000000000000 | Unused entry (example; do not honour) | ++--------------------------------------+---------------------------------------+
+MBR partitioning +^^^^^^^^^^^^^^^^
+Protective partitions should have a partition type of 0xF8 unless some +immutable feature of the platform makes this impossible.
I'd like to be rid of the caveat here too, but there is a lot more legacy to support with MBR.
The known behaviour wiki mentions that RPi dictates 0x0C.
+It is recommended that disk partitioning utilities treat such +partitions in the same manner as GPT partitions with the Platform +Required Attribute Flag set.
+It is strongly recommended that protective partitions with a type other +than 0xF8 be placed within 1MB of the start of the disk.
... I was going to suggest "s/disk/system volume/", but then I went and looked at the UEFI spec and it uses the 'disk' terminology throughout. System Volume is only used by the SBBR/EBBR specs. It should probably be changed to "system disk" since the term Volume is more frequently associated with partitions instead of the whole disk.
"disk" may not be strictly accurate these days, but it is established language.
Agree. I've used System Volume fairly liberally in this patch only because it was already in use and we define no other terminology so I figured adding it could then be someone else's problem (or at least not my problem for this patch). Does this need to be tackled in this patch or elsewhere?
Need to also specify that partitioning tools will not use or modify the first 1MB of block storage.
Agree.
Reading this, I think there also needs to be a discusson of how to handle the System Partition w.r.t. firmware (for a separate patch)
+1 [especially about the "separate patch" bit :-) ]
Daniel.