mbox series

[0/8] efi: add support for non-standard capsule headers

Message ID 20170405092317.27921-1-ard.biesheuvel@linaro.org
Headers show
Series efi: add support for non-standard capsule headers | expand

Message

Ard Biesheuvel April 5, 2017, 9:23 a.m. UTC
This is a followup to Jan's series [0] to add support for the non-standard
and awkward capsule header layout that is used by the Quark platform.

While we would prefer to adhere to the standard rigorously, the reality
(and common practice) in Linux is that we sometimes have to deal with
quirks. So while Jan's aim is to get Quark to work, the reason for my
involvement is to try and accommodate this in a flexible way without
putting any handling specific to this quirk in the common code.

Patches #1 to #4 are minor preparatory cleanups.

Patch #5 reworks the capsule loader code to use a cached copy of the
header rather than load it from memory multiple times (which may involve
a kmap/kunmap sequence if it is in highmem). This also allows some mangling
to be performed by quirks code.

Patch #6 splits up efi_capsule_setup_info() into a primary part called
__efi_capsule_setup_info(), and a __weak wrapper under the original name,
allowing it to be overridden externally.

Patch #7 changes the array of struct page pointers maintained by the capsule
loader into an array of physical addresses. This allows special versions of
efi_capsule_setup_info() to mangle the contents of the capsule (and skip
headers by moving pointers around) without putting any intimate knowledge
of the quirks handling into the common code.

Patch #8 is Jan's original patch to add the Quark specific quirk to arch/x86,
but reworked to take advantage of the facilities added in #6 and #7.

This has been tested by Jan on Quark, but this needs testing on other
platforms to ensure that the common code still works as expected on
conforming firmware implementations.

Ard Biesheuvel (3):
  efi/capsule-loader: use cached copy of capsule header
  efi/capsule-loader: indirect calls to efi_capsule_setup_info via weak
    alias
  efi/capsule-loader: use page addresses rather than struct page
    pointers

Jan Kiszka (5):
  efi/capsule: Fix return code on failing kmap/vmap
  efi/capsule: Remove pr_debug on ENOMEM or EFAULT
  efi/capsule: Clean up pr_err/info messages
  efi/capsule: Adjust return type of efi_capsule_setup_info
  efi/capsule: Add support for Quark security header

 arch/x86/platform/efi/quirks.c        | 112 ++++++++++++++++++++
 drivers/firmware/efi/Kconfig          |   9 ++
 drivers/firmware/efi/capsule-loader.c | 111 ++++++++-----------
 drivers/firmware/efi/capsule.c        |   7 +-
 include/linux/efi.h                   |  14 ++-
 5 files changed, 184 insertions(+), 69 deletions(-)

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jan Kiszka April 10, 2017, 4:34 p.m. UTC | #1
On 2017-04-05 11:23, Ard Biesheuvel wrote:
> This is a followup to Jan's series [0] to add support for the non-standard

> and awkward capsule header layout that is used by the Quark platform.

> 

> While we would prefer to adhere to the standard rigorously, the reality

> (and common practice) in Linux is that we sometimes have to deal with

> quirks. So while Jan's aim is to get Quark to work, the reason for my

> involvement is to try and accommodate this in a flexible way without

> putting any handling specific to this quirk in the common code.

> 

> Patches #1 to #4 are minor preparatory cleanups.

> 

> Patch #5 reworks the capsule loader code to use a cached copy of the

> header rather than load it from memory multiple times (which may involve

> a kmap/kunmap sequence if it is in highmem). This also allows some mangling

> to be performed by quirks code.

> 

> Patch #6 splits up efi_capsule_setup_info() into a primary part called

> __efi_capsule_setup_info(), and a __weak wrapper under the original name,

> allowing it to be overridden externally.

> 

> Patch #7 changes the array of struct page pointers maintained by the capsule

> loader into an array of physical addresses. This allows special versions of

> efi_capsule_setup_info() to mangle the contents of the capsule (and skip

> headers by moving pointers around) without putting any intimate knowledge

> of the quirks handling into the common code.

> 

> Patch #8 is Jan's original patch to add the Quark specific quirk to arch/x86,

> but reworked to take advantage of the facilities added in #6 and #7.

> 

> This has been tested by Jan on Quark, but this needs testing on other

> platforms to ensure that the common code still works as expected on

> conforming firmware implementations.

> 

> Ard Biesheuvel (3):

>   efi/capsule-loader: use cached copy of capsule header

>   efi/capsule-loader: indirect calls to efi_capsule_setup_info via weak

>     alias

>   efi/capsule-loader: use page addresses rather than struct page

>     pointers

> 

> Jan Kiszka (5):

>   efi/capsule: Fix return code on failing kmap/vmap

>   efi/capsule: Remove pr_debug on ENOMEM or EFAULT

>   efi/capsule: Clean up pr_err/info messages

>   efi/capsule: Adjust return type of efi_capsule_setup_info

>   efi/capsule: Add support for Quark security header

> 

>  arch/x86/platform/efi/quirks.c        | 112 ++++++++++++++++++++

>  drivers/firmware/efi/Kconfig          |   9 ++

>  drivers/firmware/efi/capsule-loader.c | 111 ++++++++-----------

>  drivers/firmware/efi/capsule.c        |   7 +-

>  include/linux/efi.h                   |  14 ++-

>  5 files changed, 184 insertions(+), 69 deletions(-)

> 


Thanks again for pushing this! I've retested the series in our queue
[1], and it works nicely.

Let me know if you need anything else from us to accelerate the merge.
Unfortunately, I don't think I have a different device with capsule
update support around (does the Seattle / Softiron have that feature?).

Jan

[1] https://github.com/siemens/linux/commits/queues/iot2000

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel April 18, 2017, 4:23 p.m. UTC | #2
On 18 April 2017 at 17:26, Bryan O'Donoghue
<pure.logic@nexus-software.ie> wrote:
>

>

> On 05/04/17 10:23, Ard Biesheuvel wrote:

>>

>> This is a followup to Jan's series [0] to add support for the non-standard

>> and awkward capsule header layout that is used by the Quark platform.

>>

>> While we would prefer to adhere to the standard rigorously, the reality

>> (and common practice) in Linux is that we sometimes have to deal with

>> quirks. So while Jan's aim is to get Quark to work, the reason for my

>> involvement is to try and accommodate this in a flexible way without

>> putting any handling specific to this quirk in the common code.

>

>

> Hi Ard, Jan.

>

> I've run this series on my Galileo Gen2 and it appears to work well.

>

> Please take my;

>

> Tested-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

>


Thanks Bryan,

I take it this is a board that is affected by the quirk?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bryan O'Donoghue April 18, 2017, 4:26 p.m. UTC | #3
On 05/04/17 10:23, Ard Biesheuvel wrote:
> This is a followup to Jan's series [0] to add support for the non-standard

> and awkward capsule header layout that is used by the Quark platform.

>

> While we would prefer to adhere to the standard rigorously, the reality

> (and common practice) in Linux is that we sometimes have to deal with

> quirks. So while Jan's aim is to get Quark to work, the reason for my

> involvement is to try and accommodate this in a flexible way without

> putting any handling specific to this quirk in the common code.


Hi Ard, Jan.

I've run this series on my Galileo Gen2 and it appears to work well.

Please take my;

Tested-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>


for the series.

---
bod
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kiszka April 18, 2017, 4:28 p.m. UTC | #4
On 2017-04-18 18:23, Ard Biesheuvel wrote:
> On 18 April 2017 at 17:26, Bryan O'Donoghue

> <pure.logic@nexus-software.ie> wrote:

>>

>>

>> On 05/04/17 10:23, Ard Biesheuvel wrote:

>>>

>>> This is a followup to Jan's series [0] to add support for the non-standard

>>> and awkward capsule header layout that is used by the Quark platform.

>>>

>>> While we would prefer to adhere to the standard rigorously, the reality

>>> (and common practice) in Linux is that we sometimes have to deal with

>>> quirks. So while Jan's aim is to get Quark to work, the reason for my

>>> involvement is to try and accommodate this in a flexible way without

>>> putting any handling specific to this quirk in the common code.

>>

>>

>> Hi Ard, Jan.

>>

>> I've run this series on my Galileo Gen2 and it appears to work well.

>>

>> Please take my;

>>

>> Tested-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

>>

> 

> Thanks Bryan,

> 

> I take it this is a board that is affected by the quirk?

> 


The Galileo Gen2 firmware doesn't technically require the CSH for
flashing (there is no secure boot with its SoC variant, thus no
verification of the header), but it can swallow such an update format.
It's a third case, so to say.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bryan O'Donoghue April 18, 2017, 4:31 p.m. UTC | #5
On 18/04/17 17:23, Ard Biesheuvel wrote:
> On 18 April 2017 at 17:26, Bryan O'Donoghue

> <pure.logic@nexus-software.ie> wrote:

>>

>>

>> On 05/04/17 10:23, Ard Biesheuvel wrote:

>>>

>>> This is a followup to Jan's series [0] to add support for the non-standard

>>> and awkward capsule header layout that is used by the Quark platform.

>>>

>>> While we would prefer to adhere to the standard rigorously, the reality

>>> (and common practice) in Linux is that we sometimes have to deal with

>>> quirks. So while Jan's aim is to get Quark to work, the reason for my

>>> involvement is to try and accommodate this in a flexible way without

>>> putting any handling specific to this quirk in the common code.

>>

>>

>> Hi Ard, Jan.

>>

>> I've run this series on my Galileo Gen2 and it appears to work well.

>>

>> Please take my;

>>

>> Tested-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

>>

>

> Thanks Bryan,

>

> I take it this is a board that is affected by the quirk?

>


It is indeed.

Galileo Gen2 is a Quark x1000 board. All x1000 parts have the CSH header.

https://tinyurl.com/lhlygzh

cheers
---
bod
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html