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 <[email protected]> > > 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. _______________________________________________ boot-architecture mailing list [email protected] https://lists.linaro.org/mailman/listinfo/boot-architecture
