Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images

[Jan Kiszka]

On 2017-02-19 17:33, Bryan O'Donoghue wrote:
> 
> 
> On 19/02/17 13:33, Jan Kiszka wrote:
>>> I would not object strongly to having conditionally compiled code in
>>> mainline that adds support for this, but bodging the default code path
>>> like this for a Quark quirk is out of the question imo.
>> I'm open for any consensus that avoids bending mainline too much and
>> still helps us (and maybe also other Quark X1020 integrators) getting
>> rid of additional patches.
> 
> We could make efi_capsule_setup_info() a weak symbol just like
> 
> drivers/firmware/efi/reboot.c:
> bool __weak efi_poweroff_required(void)
> 
> that way Arm is none the wiser and we can bury the Quark Quirk in
> x86/platform/efi/quirks.c - where you're right Ard it arguably belongs -
> not in the core code.
> 
> diff --git a/arch/x86/platform/efi/quirks.c
> b/arch/x86/platform/efi/quirks.c
> index 30031d5..950663da 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -495,3 +495,19 @@ bool efi_poweroff_required(void)
>  {
>         return acpi_gbl_reduced_hardware || acpi_no_s5;
>  }
> +
> +ssize_t csh_efi_capsule_setup_info(struct capsule_info *cap_info,
> +                                  void *kbuff, size_t hdr_bytes)
> +{
> +       /* Code to deal with the CSH goes here */
> +       return 0;
> +}
> +
> +ssize_t efi_capsule_setup_info(struct capsule_info *cap_info,
> +                              void *kbuff, size_t hdr_bytes)
> +{
> +       if (quark)
> +               return csh_efi_capsule_setup_info(cap_info, kbuff,
> hdr_bytes);
> +       else
> +               return __efi_capsule_setup_info(cap_info, kbuff,
> hdr_bytes);
> +}
> 
> diff --git a/drivers/firmware/efi/capsule-loader.c
> b/drivers/firmware/efi/capsule-loader.c
> index 9ae6c11..d8bdc6f 100644
> --- a/drivers/firmware/efi/capsule-loader.c
> +++ b/drivers/firmware/efi/capsule-loader.c
> @@ -53,7 +53,7 @@ static void efi_free_all_buff_pages(struct
> capsule_info *cap_info)
>   * @kbuff: a mapped first page buffer pointer
>   * @hdr_bytes: the total received number of bytes for efi header
>   **/
> -static ssize_t efi_capsule_setup_info(struct capsule_info *cap_info,
> +ssize_t __efi_capsule_setup_info(struct capsule_info *cap_info,
>                                       void *kbuff, size_t hdr_bytes)
>  {
>         efi_capsule_header_t *cap_hdr;
> @@ -98,6 +98,13 @@ static ssize_t efi_capsule_setup_info(struct
> capsule_info *cap_info,
> 
>         return 0;
>  }
> +EXPORT_SYMBOL_GPL(__efi_capsule_setup_info);
> +
> +ssize_t __weak efi_capsule_setup_info(struct capsule_info *cap_info,
> +                                            void *kbuff, size_t hdr_bytes)
> +{
> +       return __efi_capsule_setup_info(cap_info, kbuff, hdr_bytes);
> +}
>

Good idea.

> One thing we want is to continue to have Quark work on ia32 builds
> without having to compile a Quark specific kernel just to get this
> feature working.
> 
> Jan I haven't had time to look at what you said about the BSP code not
> working with capsules on Gen2 (I will during the week though). If you
> currently have to strip the CSH to make this work then we're missing a
> trick on tip-of-tree and need to sort that out for the final version of
> this.

Yes, I agree. I will look into this when I'm back from ELC next week (no
related hardware with me).

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux