From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752003AbdBTBxT (ORCPT ); Sun, 19 Feb 2017 20:53:19 -0500 Received: from goliath.siemens.de ([192.35.17.28]:39623 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751651AbdBTBxR (ORCPT ); Sun, 19 Feb 2017 20:53:17 -0500 Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images To: "Bryan O'Donoghue" , Ard Biesheuvel References: <1bf3c9d8-56aa-818b-350f-deb62ad14e08@siemens.com> <4014c5e6-b5a0-7552-166f-a42992532c09@siemens.com> <3641f43d-ecfd-03dd-9e5d-ca32d587d318@siemens.com> Cc: "Kweh, Hock Leong" , Andy Shevchenko , Matt Fleming , "linux-efi@vger.kernel.org" , Linux Kernel Mailing List , Borislav Petkov , Sascha Weisenberger , Daniel Wagner From: Jan Kiszka Message-ID: Date: Sun, 19 Feb 2017 17:52:37 -0800 User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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