On 12.03.21 17:53, Grant Likely wrote:
>
>
> On 11/03/2021 20:06, Heinrich Schuchardt wrote:
>> On 3/11/21 5:53 PM, Grant Likely wrote:
> [...]
>>> +.. list-table:: Notable Deviations from UEFI § 2.6.2
>>> +   :widths: 50 50
>>> +   :header-rows: 1
>>> +
>>> +   * - Element
>>> +     - Description of deviation
>>> +   * - `LoadImage()`
>>> +     - The LoadImage() boot service is not required to install an
>>> +       EFI_HII_PACKAGE_LIST_PROTOCOL for an image containing a custom
>>> PE/COFF
>>> +       resource with the type 'HII'. - HII resource images are not
>>> needed to run
>>> +       the UEFI shell or the SCT.
>>> +   * - `ConnectController()`
>>> +     - The ConnectController()` boot service is not required to support
>>> the
>>> +       EFI_PLATFORM_DRIVER_OVERRIDE_PROTOCOL,
>>> +       EFI_DRIVER_FAMILY_OVERRIDE_PROTOCOL, and
>>> +       EFI_BUS_SPECIFIC_DRIVER_OVERRIDE_PROTOCOL. - These override
>>> protocols are
>>> +       only useful if drivers are loaded as EFI binaries by the
>>> firmware.
>>> +   * - `EFI_HII_CONFIG_ACCESS_PROTOCOL`
>>> +     - UEFI requires this for console devices, but it is rarely
>>> necessary in practice.
>>> +       Therefore this protocol is not required.
>>> +   * - `EFI_HII_CONFIG_ROUTING_PROTOCOL`
>>> +     - UEFI requires this for console devices, but it is rarely
>>> necessary in practice.
>>> +       Therefore this protocol is not required.
>>> +   * - Graphical console
>>
>> Hello Grant,
>>
>> thanks for sharing this pre-release.
>>
>> Should we mention the protocols that might not be provided:
>>
>> EFI_GRAPHICS_OUTPUT_PROTOCOL
>> EFI_EDID_DISCOVERED_PROTOCOL
>> EFI_EDID_ACTIVE_PROTOCOL
>
> I don't think it is strictly necessary, but if you want to propose a
> patch to add those protocols to the text then I'd be happy to add it.
>
>>> +.. list-table:: Required UEFI Variables
>>> +   :widths: 25 75
>>> +   :header-rows: 1
>>> +
>>> +   * - Variable Name
>>> +     - Description
>>> +   * - `Boot####`
>>> +     - A boot load option. #### is a numerical hex value
>>> +   * - `BootCurrent`
>>> +     - The boot option that was selected for the current boot
>>> +   * - `BootNext`
>>> +     - The boot option that will be used for the next boot only
>>> +   * - `BootOrder`
>>> +     - An ordered list of boot options.
>>> +       Firmware will attempt each Boot#### entry in this order
>>
>> Firmware will try BootNext and each Boot#### in the order given by
>> BootOrder to find the first bootable image.
>
> Fixed, will push out later.
>
>>
>>> +   * - `OsIndications`
>>> +     - Method for OS to request features from firmware
>>> +   * - `OsIndicationsSupported`
>>> +     - Variable for firmware to indicate which features can be enabled
>>>
>>>   Block device partitioning
>>>   -------------------------
>>> @@ -53,7 +224,7 @@ a hypervisor or a virtualization aware Operating
>>> System.
>>>   UEFI Boot at EL1
>>>   ^^^^^^^^^^^^^^^^
>>>
>>> -Booting of UEFI at EL1 is most likely within a hypervisor hosted Guest
>>> +Booting of UEFI at EL1 is most likely employed within a hypervisor
>>> hosted Guest
>>>   Operating System environment, to allow the subsequent booting of a
>>>   UEFI-compliant Operating System.
>>>   In this instance, the UEFI boot-time environment can be provided, as a
>>> @@ -77,7 +248,7 @@ The default RAM allocated attribute must be
>>> EFI_MEMORY_WB.
>>>   Configuration Tables
>>>   --------------------
>>>
>>> -A UEFI system that complies with this specification may provide the
>>> additional
>>> +A UEFI system that complies with this specification may provide
>>> additional
>>>   tables via the EFI Configuration Table.
>>>
>>>   Compliant systems are required to provide one, but not both, of the
>>> following
>>> @@ -151,26 +322,55 @@ EFI_UNSUPPORTED.
>>>   are required to be implemented during boot services and runtime
>>> services.
>>>
>>>   .. _uefi_runtime_service_requirements:
>>> -.. table:: EFI_RUNTIME_SERVICES Implementation Requirements
>>> -
>>> -   ============================== ============= ================
>>> -   EFI_RUNTIME_SERVICES function  Boot Services Runtime Services
>>> -   ============================== ============= ================
>>> -   EFI_GET_TIME                   Optional      Optional
>>> -   EFI_SET_TIME                   Optional      Optional
>>> -   EFI_GET_WAKEUP_TIME            Optional      Optional
>>> -   EFI_SET_WAKEUP_TIME            Optional      Optional
>>> -   EFI_SET_VIRTUAL_ADDRESS_MAP    N/A           Required
>>> -   EFI_CONVERT_POINTER            N/A           Required
>>> -   EFI_GET_VARIABLE               Required      Optional
>>> -   EFI_GET_NEXT_VARIABLE_NAME     Required      Optional
>>> -   EFI_SET_VARIABLE               Required      Optional
>>> -   EFI_GET_NEXT_HIGH_MONO_COUNT   N/A           Optional
>>> -   EFI_RESET_SYSTEM               Required      Optional
>>> -   EFI_UPDATE_CAPSULE             Optional      Optional
>>> -   EFI_QUERY_CAPSULE_CAPABILITIES Optional      Optional
>>> -   EFI_QUERY_VARIABLE_INFO        Optional      Optional > -  
>>> ============================== ============= ================
>>> +.. list-table:: `EFI_RUNTIME_SERVICES` Implementation Requirements
>>> +   :widths: 40 30 30
>>> +   :header-rows: 1
>>> +
>>> +   * - `EFI_RUNTIME_SERVICES` function > +     - Before
>>> ExitBootServices()
>>> +     - After ExitBootServices()
>>> +   * - `EFI_GET_TIME`
>>
>> We should use the service names (not the type names) throughout this
>> document:
>>
>> GetTime(), SetTime(), ..., QueryVariableInfo().
>
> Good comment. That will require some scrubbing of the document. I'll try
> to get to it, but any patches helping it along would be appreciated.
>
>>
>>> +     - Required if RTC present
>>> +     - Optional
>>> +   * - `EFI_SET_TIME`
>>> +     - Required if RTC present
>>> +     - Optional
>>> +   * - `EFI_GET_WAKEUP_TIME`
>>> +     - Required if wakeup supported
>>> +     - Optional
>>> +   * - `EFI_SET_WAKEUP_TIME`
>>> +     - Required if wakeup supported
>>> +     - Optional
>>> +   * - `EFI_SET_VIRTUAL_ADDRESS_MAP`
>>> +     - N/A
>>> +     - Required
>>> +   * - `EFI_CONVERT_POINTER`
>>> +     - N/A
>>> +     - Required
>>> +   * - `EFI_GET_VARIABLE`
>>> +     - Required
>>> +     - Optional
>>> +   * - `EFI_GET_NEXT_VARIABLE_NAME`
>>> +     - Required
>>> +     - Optional
>>> +   * - `EFI_SET_VARIABLE`
>>> +     - Required
>>> +     - Optional
>>> +   * - `EFI_GET_NEXT_HIGH_MONO_COUNT`
>>> +     - N/A
>>> +     - Optional
>>> +   * - `EFI_RESET_SYSTEM`
>>> +     - Required
>>> +     - Optional
>>> +   * - `EFI_UPDATE_CAPSULE`
>>> +     - Required for in-band update
>>> +     - Optional
>>> +   * - `EFI_QUERY_CAPSULE_CAPABILITIES`
>>> +     - Optional
>>> +     - Optional
>>> +   * - `EFI_QUERY_VARIABLE_INFO`
>>> +     - Optional
>>> +     - Optional
>>>
>>>   Runtime Device Mappings
>>>   -----------------------
>>> @@ -198,8 +398,11 @@ it may not be possible to access the RTC from
>>> runtime services.
>>>   e.g., The RTC may be on a shared I2C bus which runtime services cannot
>>> access
>>>   because it will conflict with the OS.
>>>
>>> -If firmware does not support access to the RTC, then GetTime() and
>>> -SetTime() shall return EFI_UNSUPPORTED,
>>> +If an RTC is present, then GetTime() and SetTime() must be supported
>>> +before ExitBootServices() is called.
>>> +
>>> +However, if firmware does not support access to the RTC after
>>> +ExitBootServices(), then GetTime() and SetTime() shall return
>>> EFI_UNSUPPORTED
>>>   and the OS must use a device driver to control the RTC.
>>>
>>>   UEFI Reset and Shutdown
>>> @@ -209,9 +412,10 @@ ResetSystem() is required to be implemented in boot
>>> services, but it is
>>>   optional for runtime services.
>>>   During runtime services, the operating system should first attempt to
>>>   use ResetSystem() to reset the system.
>>> -If firmware doesn't support ResetSystem() during runtime services,
>>> -then the call will immediately return EFI_UNSUPPORTED, and the OS
>>> should
>>> -fall back to an architecture or platform specific reset mechanism.
>>> +
>>> +If firmware doesn't support ResetSystem() during runtime services, then
>>> the call
>>> +will immediately return, and the OS should fall back to an
>>> architecture or
>>> +platform specific reset mechanism.
>>>
>>>   On AArch64 platforms implementing [PSCI]_,
>>>   if ResetSystem() is not implemented then the Operating System
>>> should fall
>>> @@ -242,6 +446,26 @@ Even when SetVariable() is not supported during
>>> runtime services, firmware
>>>   should cache variable names and values in EfiRuntimeServicesData
>>> memory so
>>>   that GetVariable() and GetNextVeriableName() can behave as specified.
>>>
>>> +Firmware Update
>>> +---------------
>>> +
>>> +Being able to update firmware to address security issues is a key
>>> feature of secure platforms.
>>> +EBBR platforms are required to implement either an in-band or an
>>> out-of-band firmware update mechanism.
>>> +
>>> +If firmware update is performed in-band (firmware on the application
>>> processor updates itself),
>>> +then the firmware shall implement EFI_UPDATE_CAPSULE and accept updates
>>
>> %s/EFI_UPDATE_CAPSULE/the UpdateCapsule() runtime service/
>
> Is this the right way around to change this? Why doesn't it make sense
> to reference EFI_UPDATE_CAPSULE? The question is I suppose part of a
> larger question on how to refer to EFI elements throughout the document.

Throughout the UEFI spec services are referenced by there function name
and not by their type.

We should opt for the way of least surprise, i.e. use the function names.

Best regards

Heinrich

>
>>
>>> in the
>>> +"Firmware Management Protocol Data Capsule Structure" format as
>>> described in [UEFI]_ § 23.3,
>>> +"Delivering Capsules Containing Updates to Firmware Management
>>> Protocol.  [#FMPNote]_
>>> +Firmware is also required to provide an EFI System Resource Table
>>> (ESRT). [UEFI]_ § 23.4
>>> +Every firmware image that is updated in-band must be described in the
>>
>> %s/is updated/can be updated/
>
> Fixed
>
>>
>>> ESRT.
>>> +
>>> +If firmware update is performed out-of-band (e.g., by an independent
>>> Baseboard
>>> +Management Controller (BMC), or firmware is provided by a hypervisor),
>>> +then the platform is not required to implement EFI_UPDATE_CAPSULE.
>>
>> the UpdateCapsule() runtime service
>>
>>> +
>>> +EFI_UPDATE_CAPSULE is only required before ExitBootServices() is
>>> called.
>>
>> UpdateCapsule() is only required to be supported ...
>>
>>> +
>>> +
>>>   .. [#OPTEESupplicant] It is worth noting that OP-TEE has a similar
>>> problem
>>>      regarding secure storage.
>>>      OP-TEE's chosen solution is to rely on an OS supplicant agent to
>>> perform
>>> @@ -251,4 +475,12 @@ that GetVariable() and GetNextVeriableName() can
>>> behave as specified.
>>>      Regardless, EBBR compliance does not require SetVariable() support
>>>      during runtime services.
>>>
>>> -
>>> https://github.com/OP-TEE/optee_os/blob/master/documentation/secure_storage.md
>>>
>>>
>>> +  
>>> https://optee.readthedocs.io/en/latest/architecture/secure_storage.html
>>> +
>>> +.. [#FMPNote] The `EFI_UPDATE_CAPSULE` implementation is expected to be
>>
>> UpdateCapsuel()
>>
>>> suitable
>>> +   for use by generic firmware update services like fwupd and Windows
>>> Update.
>>> +   Both fwupd and Windows Update read the ESRT table to determine what
>>> firmware
>>> +   can be updated, and use an EFI helper application to call
>>> `EFI_UPDATE_CAPSULE`
>>
>> UpdateCapsule()
>>
>>> +   before ExitBootServices() is called.
>>> +
>>> +   https://fwupd.org/
>>> diff --git a/source/chapter4-firmware-media.rst
>>> b/source/chapter4-firmware-media.rst
>>> index fc71274..cfcc8bd 100644
>>> --- a/source/chapter4-firmware-media.rst
>>> +++ b/source/chapter4-firmware-media.rst
>>> @@ -47,13 +47,19 @@ conflict with normal usage of the media by an OS.
>>>   Partitioning of Shared Storage
>>>   ==============================
>>>
>>> -A shared storage device shall use GPT partitioning unless it is
>>> incompatible
>>> -with the platform boot sequence.
>>> -In which case, MBR partitioning shall be used. [#MBRReqExample]_
>>> -
>>> -.. [#MBRReqExample] For example, if the boot ROM doesn't understand GPT
>>> -   partitioning, and will only work with an MBR, then the storage
>>> must be
>>> -   partitioned using an MBR.
>>> +The shared storage device must use the GUID Partition Table (GPT) disk
>>> +layout as defined in [UEFI]_ § 5.3, unless the platform boot
>>> sequence is
>>> +fundamentally incompatible with the GPT disk layout.
>>> +In which case, a legacy Master Boot Recored (MBR) must be used.
>>> +[#MBRReqExample]_
>>> +
>>> +.. [#MBRReqExample] For example, if the SoC boot ROM requires an MBR to
>>> +   find the next stage firmware image, then it is incompatible with
>>> +   the GPT boot layout.
>>> +   Similarly, if the boot ROM expects the next stage firmware to be
>>> located
>>> +   at LBA1 (the location of the GPT Header), then it is incompatible
>>> with
>>> +   the GPT disk layout.
>>> +   In both cases the shared storage device must use legacy MBR
>>> partitioning.
>>>
>>>   .. warning::
>>>
>>> @@ -62,7 +68,8 @@ In which case, MBR partitioning shall be used.
>>> [#MBRReqExample]_
>>>      GPT partitioning supports a much larger number of partitions, and
>>>      has built in resiliency.
>>>
>>> -   A future issue of this specification will remove the MBR allowance.
>>> +   A future issue of this specification will disallow the use of MBR
>>
>> %s/issue/version/
>>
>> Iterations of this spec are referred to as versions not issues.
>
> fixed.
>
>>
>> Best regards
>>
>> Heinrich
>
> Thanks for the comments Heinrich.
>
> g.
> _______________________________________________
> boot-architecture mailing list
> boot-architecture@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/boot-architecture

_______________________________________________
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture

Reply via email to