2017-04-19 18:33:55

by Jan Kiszka

[permalink] [raw]
Subject: [PATCH v2 0/3] efi: add support for non-standard capsule headers

This picks up the patches Ard send before in [1], including the
"left-over" patches 6..8.

As Ard suggested, I've taken updated patches 6 and 7 of him from [2]
which address reviewer comments. Furthermore, I've changed patch 8 to
factor out the Quark quirk logic from the overloaded
efi_capsule_setup_info as requested by Matt and also applied Andy's
suggestion to have a quirk dispatcher table with callbacks.

Tested successfully on the IOT2040 - still without a working Galileo
board.

Jan

[1] http://www.spinics.net/lists/linux-efi/msg11194.html
[2] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=quark-capsule

Cc: Matt Fleming <[email protected]>

Ard Biesheuvel (2):
efi/capsule-loader: Redirect calls to efi_capsule_setup_info via weak
alias
efi/capsule-loader: Use page addresses rather than struct page
pointers

Jan Kiszka (1):
efi/capsule: Add support for Quark security header

arch/x86/platform/efi/quirks.c | 137 ++++++++++++++++++++++++++++++++++
drivers/firmware/efi/Kconfig | 9 +++
drivers/firmware/efi/capsule-loader.c | 66 ++++++++--------
drivers/firmware/efi/capsule.c | 7 +-
include/linux/efi.h | 14 +++-
5 files changed, 197 insertions(+), 36 deletions(-)

--
2.12.0


2017-04-19 18:33:23

by Jan Kiszka

[permalink] [raw]
Subject: [PATCH v2 1/3] efi/capsule-loader: Redirect calls to efi_capsule_setup_info via weak alias

From: Ard Biesheuvel <[email protected]>

To allow platform specific code to hook into the capsule loading
routines, indirect calls to efi_capsule_setup_info() via a weak alias
of __efi_capsule_setup_info(), allowing platforms to redefine the former
but still use the latter.

Cc: Matt Fleming <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
---
drivers/firmware/efi/capsule-loader.c | 56 +++++++++++++++++------------------
include/linux/efi.h | 12 ++++++++
2 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index 5b012a467d7d..f7fdeab0bc37 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -20,16 +20,6 @@

#define NO_FURTHER_WRITE_ACTION -1

-struct capsule_info {
- efi_capsule_header_t header;
- int reset_type;
- long index;
- size_t count;
- size_t total_size;
- struct page **pages;
- size_t page_bytes_remain;
-};
-
/**
* efi_free_all_buff_pages - free all previous allocated buffer pages
* @cap_info: pointer to current instance of capsule_info structure
@@ -46,28 +36,13 @@ static void efi_free_all_buff_pages(struct capsule_info *cap_info)
cap_info->index = NO_FURTHER_WRITE_ACTION;
}

-/**
- * efi_capsule_setup_info - obtain the efi capsule header in the binary and
- * setup capsule_info structure
- * @cap_info: pointer to current instance of capsule_info structure
- * @kbuff: a mapped first page buffer pointer
- * @hdr_bytes: the total received number of bytes for efi header
- **/
-static int efi_capsule_setup_info(struct capsule_info *cap_info,
- void *kbuff, size_t hdr_bytes)
+int __efi_capsule_setup_info(struct capsule_info *cap_info)
{
size_t pages_needed;
int ret;
void *temp_page;

- /* Only process data block that is larger than efi header size */
- if (hdr_bytes < sizeof(efi_capsule_header_t))
- return 0;
-
- /* Reset back to the correct offset of header */
- kbuff -= cap_info->count;
- memcpy(&cap_info->header, kbuff, sizeof(cap_info->header));
- pages_needed = ALIGN(cap_info->header.imagesize, PAGE_SIZE) / PAGE_SIZE;
+ pages_needed = ALIGN(cap_info->total_size, PAGE_SIZE) / PAGE_SIZE;

if (pages_needed == 0) {
pr_err("invalid capsule size");
@@ -84,7 +59,6 @@ static int efi_capsule_setup_info(struct capsule_info *cap_info,
return ret;
}

- cap_info->total_size = cap_info->header.imagesize;
temp_page = krealloc(cap_info->pages,
pages_needed * sizeof(void *),
GFP_KERNEL | __GFP_ZERO);
@@ -97,6 +71,30 @@ static int efi_capsule_setup_info(struct capsule_info *cap_info,
}

/**
+ * efi_capsule_setup_info - obtain the efi capsule header in the binary and
+ * setup capsule_info structure
+ * @cap_info: pointer to current instance of capsule_info structure
+ * @kbuff: a mapped first page buffer pointer
+ * @hdr_bytes: the total received number of bytes for efi header
+ *
+ * Platforms with non-standard capsule update mechanisms can override
+ * this __weak function so they can perform any required capsule
+ * image munging. See quark_quirk_function() for an example.
+ **/
+int __weak efi_capsule_setup_info(struct capsule_info *cap_info, void *kbuff,
+ size_t hdr_bytes)
+{
+ /* Only process data block that is larger than efi header size */
+ if (hdr_bytes < sizeof(efi_capsule_header_t))
+ return 0;
+
+ memcpy(&cap_info->header, kbuff, sizeof(cap_info->header));
+ cap_info->total_size = cap_info->header.imagesize;
+
+ return __efi_capsule_setup_info(cap_info);
+}
+
+/**
* efi_capsule_submit_update - invoke the efi_capsule_update API once binary
* upload done
* @cap_info: pointer to current instance of capsule_info structure
@@ -186,7 +184,7 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,

/* Setup capsule binary info structure */
if (cap_info->header.headersize == 0) {
- ret = efi_capsule_setup_info(cap_info, kbuff,
+ ret = efi_capsule_setup_info(cap_info, kbuff - cap_info->count,
cap_info->count + write_byte);
if (ret)
goto fail_unmap;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 94d34e0be24f..af54be874dd0 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -137,6 +137,18 @@ struct efi_boot_memmap {
#define EFI_CAPSULE_POPULATE_SYSTEM_TABLE 0x00020000
#define EFI_CAPSULE_INITIATE_RESET 0x00040000

+struct capsule_info {
+ efi_capsule_header_t header;
+ int reset_type;
+ long index;
+ size_t count;
+ size_t total_size;
+ struct page **pages;
+ size_t page_bytes_remain;
+};
+
+int __efi_capsule_setup_info(struct capsule_info *cap_info);
+
/*
* Allocation types for calls to boottime->allocate_pages.
*/
--
2.12.0

2017-04-19 18:33:30

by Jan Kiszka

[permalink] [raw]
Subject: [PATCH v2 3/3] efi/capsule: Add support for Quark security header

The firmware for Quark X102x prepends a security header to the capsule
which is needed to support the mandatory secure boot on this processor.
The header can be detected by checking for the "_CSH" signature and -
to avoid any GUID conflict - validating its size field to contain the
expected value. Then we need to look for the EFI header right after the
security header and pass the real header to __efi_capsule_setup_info.

To be minimal invasive and maximal safe, the quirk version of
efi_capsule_identify_image is only effective on Quark processors.

Signed-off-by: Jan Kiszka <[email protected]>
---
arch/x86/platform/efi/quirks.c | 137 +++++++++++++++++++++++++++++++++++++++++
drivers/firmware/efi/Kconfig | 9 +++
2 files changed, 146 insertions(+)

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index cdfe8c628959..8f41a75f8604 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -13,12 +13,66 @@
#include <linux/dmi.h>
#include <asm/efi.h>
#include <asm/uv/uv.h>
+#include <asm/cpu_device_id.h>

#define EFI_MIN_RESERVE 5120

#define EFI_DUMMY_GUID \
EFI_GUID(0x4424ac57, 0xbe4b, 0x47dd, 0x9e, 0x97, 0xed, 0x50, 0xf0, 0x9f, 0x92, 0xa9)

+#define QUARK_CSH_SIGNATURE 0x5f435348 /* _CSH */
+#define QUARK_SECURITY_HEADER_SIZE 0x400
+
+/*
+ * Header prepended to the standard EFI capsule on Quark systems the are based
+ * on Intel firmware BSP.
+ * @csh_signature: Unique identifier to sanity check signed module
+ * presence ("_CSH").
+ * @version: Current version of CSH used. Should be one for Quark A0.
+ * @modulesize: Size of the entire module including the module header
+ * and payload.
+ * @security_version_number_index: Index of SVN to use for validation of signed
+ * module.
+ * @security_version_number: Used to prevent against roll back of modules.
+ * @rsvd_module_id: Currently unused for Clanton (Quark).
+ * @rsvd_module_vendor: Vendor Identifier. For Intel products value is
+ * 0x00008086.
+ * @rsvd_date: BCD representation of build date as yyyymmdd, where
+ * yyyy=4 digit year, mm=1-12, dd=1-31.
+ * @headersize: Total length of the header including including any
+ * padding optionally added by the signing tool.
+ * @hash_algo: What Hash is used in the module signing.
+ * @cryp_algo: What Crypto is used in the module signing.
+ * @keysize: Total length of the key data including including any
+ * padding optionally added by the signing tool.
+ * @signaturesize: Total length of the signature including including any
+ * padding optionally added by the signing tool.
+ * @rsvd_next_header: 32-bit pointer to the next Secure Boot Module in the
+ * chain, if there is a next header.
+ * @rsvd: Reserved, padding structure to required size.
+ *
+ * See also QuartSecurityHeader_t in
+ * Quark_EDKII_v1.2.1.1/QuarkPlatformPkg/Include/QuarkBootRom.h
+ * from https://downloadcenter.intel.com/download/23197/Intel-Quark-SoC-X1000-Board-Support-Package-BSP
+ */
+struct quark_security_header {
+ u32 csh_signature;
+ u32 version;
+ u32 modulesize;
+ u32 security_version_number_index;
+ u32 security_version_number;
+ u32 rsvd_module_id;
+ u32 rsvd_module_vendor;
+ u32 rsvd_date;
+ u32 headersize;
+ u32 hash_algo;
+ u32 cryp_algo;
+ u32 keysize;
+ u32 signaturesize;
+ u32 rsvd_next_header;
+ u32 rsvd[2];
+};
+
static efi_char16_t efi_dummy_name[6] = { 'D', 'U', 'M', 'M', 'Y', 0 };

static bool efi_no_storage_paranoia;
@@ -499,3 +553,86 @@ bool efi_poweroff_required(void)
{
return acpi_gbl_reduced_hardware || acpi_no_s5;
}
+
+#ifdef CONFIG_EFI_CAPSULE_QUIRK_QUARK_CSH
+
+static int qrk_capsule_setup_info(struct capsule_info *cap_info, void **pkbuff,
+ size_t hdr_bytes)
+{
+ struct quark_security_header *csh = *pkbuff;
+
+ /* Only process data block that is larger than the security header */
+ if (hdr_bytes < sizeof(struct quark_security_header))
+ return 0;
+
+ if (csh->csh_signature != QUARK_CSH_SIGNATURE ||
+ csh->headersize != QUARK_SECURITY_HEADER_SIZE)
+ return 1;
+
+ /* Only process data block if EFI header is included */
+ if (hdr_bytes < QUARK_SECURITY_HEADER_SIZE +
+ sizeof(efi_capsule_header_t))
+ return 0;
+
+ pr_debug("Quark security header detected\n");
+
+ if (csh->rsvd_next_header != 0) {
+ pr_err("multiple Quark security headers not supported\n");
+ return -EINVAL;
+ }
+
+ *pkbuff += csh->headersize;
+ cap_info->total_size = csh->headersize;
+
+ /*
+ * Update the first page pointer to skip over the CSH header.
+ */
+ cap_info->pages[0] += csh->headersize;
+
+ return 1;
+}
+
+#define ICPU(family, model, quirk_handler) \
+ { X86_VENDOR_INTEL, family, model, X86_FEATURE_ANY, \
+ (unsigned long)&quirk_handler }
+
+static const struct x86_cpu_id efi_capsule_quirk_ids[] = {
+ ICPU(5, 9, qrk_capsule_setup_info), /* Intel Quark X1000 */
+ { }
+};
+
+int efi_capsule_setup_info(struct capsule_info *cap_info, void *kbuff,
+ size_t hdr_bytes)
+{
+ int (*quirk_handler)(struct capsule_info *, void **, size_t);
+ const struct x86_cpu_id *id;
+ int ret;
+
+ if (hdr_bytes < sizeof(efi_capsule_header_t))
+ return 0;
+
+ cap_info->total_size = 0;
+
+ id = x86_match_cpu(efi_capsule_quirk_ids);
+ if (id) {
+ /*
+ * The quirk handler is supposed to return
+ * - a value > 0 if the setup should continue, after advancing
+ * kbuff as needed
+ * - 0 if not enough hdr_bytes are available yet
+ * - a negative error code otherwise
+ */
+ quirk_handler = (typeof(quirk_handler))id->driver_data;
+ ret = quirk_handler(cap_info, &kbuff, hdr_bytes);
+ if (ret <= 0)
+ return ret;
+ }
+
+ memcpy(&cap_info->header, kbuff, sizeof(cap_info->header));
+
+ cap_info->total_size += cap_info->header.imagesize;
+
+ return __efi_capsule_setup_info(cap_info);
+}
+
+#endif
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 2e78b0b96d74..394db40ed374 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -112,6 +112,15 @@ config EFI_CAPSULE_LOADER

Most users should say N.

+config EFI_CAPSULE_QUIRK_QUARK_CSH
+ boolean "Add support for Quark capsules with non-standard headers"
+ depends on X86 && !64BIT
+ select EFI_CAPSULE_LOADER
+ default y
+ help
+ Add support for processing Quark X1000 EFI capsules, whose header
+ layout deviates from the layout mandated by the UEFI specification.
+
config EFI_TEST
tristate "EFI Runtime Service Tests Support"
depends on EFI
--
2.12.0

2017-04-19 18:33:39

by Jan Kiszka

[permalink] [raw]
Subject: [PATCH v2 2/3] efi/capsule-loader: Use page addresses rather than struct page pointers

From: Ard Biesheuvel <[email protected]>

To give some leeway to code that handles non-standard capsule headers,
let's keep an array of page addresses rather than struct page pointers.

This gives special implementations of efi_capsule_setup_info() the
opportunity to mangle the payload a bit before it is presented to the
firmware, without putting any knowledge of the nature of such quirks
into the generic code.

Cc: Matt Fleming <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
---
drivers/firmware/efi/capsule-loader.c | 12 ++++++++----
drivers/firmware/efi/capsule.c | 7 ++++---
include/linux/efi.h | 4 ++--
3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index f7fdeab0bc37..feeafb673c07 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -20,6 +20,10 @@

#define NO_FURTHER_WRITE_ACTION -1

+#ifndef phys_to_page
+#define phys_to_page(x) pfn_to_page((x) >> PAGE_SHIFT)
+#endif
+
/**
* efi_free_all_buff_pages - free all previous allocated buffer pages
* @cap_info: pointer to current instance of capsule_info structure
@@ -31,7 +35,7 @@
static void efi_free_all_buff_pages(struct capsule_info *cap_info)
{
while (cap_info->index > 0)
- __free_page(cap_info->pages[--cap_info->index]);
+ __free_page(phys_to_page(cap_info->pages[--cap_info->index]));

cap_info->index = NO_FURTHER_WRITE_ACTION;
}
@@ -161,12 +165,12 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
goto failed;
}

- cap_info->pages[cap_info->index++] = page;
+ cap_info->pages[cap_info->index++] = page_to_phys(page);
cap_info->page_bytes_remain = PAGE_SIZE;
+ } else {
+ page = phys_to_page(cap_info->pages[cap_info->index - 1]);
}

- page = cap_info->pages[cap_info->index - 1];
-
kbuff = kmap(page);
if (!kbuff) {
ret = -ENOMEM;
diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
index 6eedff45e6d7..57f85256feb2 100644
--- a/drivers/firmware/efi/capsule.c
+++ b/drivers/firmware/efi/capsule.c
@@ -214,7 +214,7 @@ efi_capsule_update_locked(efi_capsule_header_t *capsule,
*
* Return 0 on success, a converted EFI status code on failure.
*/
-int efi_capsule_update(efi_capsule_header_t *capsule, struct page **pages)
+int efi_capsule_update(efi_capsule_header_t *capsule, phys_addr_t *pages)
{
u32 imagesize = capsule->imagesize;
efi_guid_t guid = capsule->guid;
@@ -253,10 +253,11 @@ int efi_capsule_update(efi_capsule_header_t *capsule, struct page **pages)
}

for (j = 0; j < SGLIST_PER_PAGE && count > 0; j++) {
- u64 sz = min_t(u64, imagesize, PAGE_SIZE);
+ u64 sz = min_t(u64, imagesize,
+ PAGE_SIZE - (u64)*pages % PAGE_SIZE);

sglist[j].length = sz;
- sglist[j].data = page_to_phys(*pages++);
+ sglist[j].data = *pages++;

imagesize -= sz;
count--;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index af54be874dd0..2cec67cc18a3 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -143,7 +143,7 @@ struct capsule_info {
long index;
size_t count;
size_t total_size;
- struct page **pages;
+ phys_addr_t *pages;
size_t page_bytes_remain;
};

@@ -1415,7 +1415,7 @@ extern int efi_capsule_supported(efi_guid_t guid, u32 flags,
size_t size, int *reset);

extern int efi_capsule_update(efi_capsule_header_t *capsule,
- struct page **pages);
+ phys_addr_t *pages);

#ifdef CONFIG_EFI_RUNTIME_MAP
int efi_runtime_map_init(struct kobject *);
--
2.12.0

2017-04-19 21:17:48

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] efi/capsule: Add support for Quark security header

On Wed, Apr 19, 2017 at 9:33 PM, Jan Kiszka <[email protected]> wrote:
> The firmware for Quark X102x prepends a security header to the capsule
> which is needed to support the mandatory secure boot on this processor.
> The header can be detected by checking for the "_CSH" signature and -
> to avoid any GUID conflict - validating its size field to contain the
> expected value. Then we need to look for the EFI header right after the
> security header and pass the real header to __efi_capsule_setup_info.
>
> To be minimal invasive and maximal safe, the quirk version of
> efi_capsule_identify_image is only effective on Quark processors.
>

FWIW:
Reviewed-by: Andy Shevchenko <[email protected]>

> Signed-off-by: Jan Kiszka <[email protected]>
> ---
> arch/x86/platform/efi/quirks.c | 137 +++++++++++++++++++++++++++++++++++++++++
> drivers/firmware/efi/Kconfig | 9 +++
> 2 files changed, 146 insertions(+)
>
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index cdfe8c628959..8f41a75f8604 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -13,12 +13,66 @@
> #include <linux/dmi.h>
> #include <asm/efi.h>
> #include <asm/uv/uv.h>
> +#include <asm/cpu_device_id.h>
>
> #define EFI_MIN_RESERVE 5120
>
> #define EFI_DUMMY_GUID \
> EFI_GUID(0x4424ac57, 0xbe4b, 0x47dd, 0x9e, 0x97, 0xed, 0x50, 0xf0, 0x9f, 0x92, 0xa9)
>
> +#define QUARK_CSH_SIGNATURE 0x5f435348 /* _CSH */
> +#define QUARK_SECURITY_HEADER_SIZE 0x400
> +
> +/*
> + * Header prepended to the standard EFI capsule on Quark systems the are based
> + * on Intel firmware BSP.
> + * @csh_signature: Unique identifier to sanity check signed module
> + * presence ("_CSH").
> + * @version: Current version of CSH used. Should be one for Quark A0.
> + * @modulesize: Size of the entire module including the module header
> + * and payload.
> + * @security_version_number_index: Index of SVN to use for validation of signed
> + * module.
> + * @security_version_number: Used to prevent against roll back of modules.
> + * @rsvd_module_id: Currently unused for Clanton (Quark).
> + * @rsvd_module_vendor: Vendor Identifier. For Intel products value is
> + * 0x00008086.
> + * @rsvd_date: BCD representation of build date as yyyymmdd, where
> + * yyyy=4 digit year, mm=1-12, dd=1-31.
> + * @headersize: Total length of the header including including any
> + * padding optionally added by the signing tool.
> + * @hash_algo: What Hash is used in the module signing.
> + * @cryp_algo: What Crypto is used in the module signing.
> + * @keysize: Total length of the key data including including any
> + * padding optionally added by the signing tool.
> + * @signaturesize: Total length of the signature including including any
> + * padding optionally added by the signing tool.
> + * @rsvd_next_header: 32-bit pointer to the next Secure Boot Module in the
> + * chain, if there is a next header.
> + * @rsvd: Reserved, padding structure to required size.
> + *
> + * See also QuartSecurityHeader_t in
> + * Quark_EDKII_v1.2.1.1/QuarkPlatformPkg/Include/QuarkBootRom.h
> + * from https://downloadcenter.intel.com/download/23197/Intel-Quark-SoC-X1000-Board-Support-Package-BSP
> + */
> +struct quark_security_header {
> + u32 csh_signature;
> + u32 version;
> + u32 modulesize;
> + u32 security_version_number_index;
> + u32 security_version_number;
> + u32 rsvd_module_id;
> + u32 rsvd_module_vendor;
> + u32 rsvd_date;
> + u32 headersize;
> + u32 hash_algo;
> + u32 cryp_algo;
> + u32 keysize;
> + u32 signaturesize;
> + u32 rsvd_next_header;
> + u32 rsvd[2];
> +};
> +
> static efi_char16_t efi_dummy_name[6] = { 'D', 'U', 'M', 'M', 'Y', 0 };
>
> static bool efi_no_storage_paranoia;
> @@ -499,3 +553,86 @@ bool efi_poweroff_required(void)
> {
> return acpi_gbl_reduced_hardware || acpi_no_s5;
> }
> +
> +#ifdef CONFIG_EFI_CAPSULE_QUIRK_QUARK_CSH
> +
> +static int qrk_capsule_setup_info(struct capsule_info *cap_info, void **pkbuff,
> + size_t hdr_bytes)
> +{
> + struct quark_security_header *csh = *pkbuff;
> +
> + /* Only process data block that is larger than the security header */
> + if (hdr_bytes < sizeof(struct quark_security_header))
> + return 0;
> +
> + if (csh->csh_signature != QUARK_CSH_SIGNATURE ||
> + csh->headersize != QUARK_SECURITY_HEADER_SIZE)
> + return 1;
> +
> + /* Only process data block if EFI header is included */
> + if (hdr_bytes < QUARK_SECURITY_HEADER_SIZE +
> + sizeof(efi_capsule_header_t))
> + return 0;
> +
> + pr_debug("Quark security header detected\n");
> +
> + if (csh->rsvd_next_header != 0) {
> + pr_err("multiple Quark security headers not supported\n");
> + return -EINVAL;
> + }
> +
> + *pkbuff += csh->headersize;
> + cap_info->total_size = csh->headersize;
> +
> + /*
> + * Update the first page pointer to skip over the CSH header.
> + */
> + cap_info->pages[0] += csh->headersize;
> +
> + return 1;
> +}
> +
> +#define ICPU(family, model, quirk_handler) \
> + { X86_VENDOR_INTEL, family, model, X86_FEATURE_ANY, \
> + (unsigned long)&quirk_handler }
> +
> +static const struct x86_cpu_id efi_capsule_quirk_ids[] = {
> + ICPU(5, 9, qrk_capsule_setup_info), /* Intel Quark X1000 */
> + { }
> +};
> +
> +int efi_capsule_setup_info(struct capsule_info *cap_info, void *kbuff,
> + size_t hdr_bytes)
> +{
> + int (*quirk_handler)(struct capsule_info *, void **, size_t);
> + const struct x86_cpu_id *id;
> + int ret;
> +
> + if (hdr_bytes < sizeof(efi_capsule_header_t))
> + return 0;
> +
> + cap_info->total_size = 0;
> +
> + id = x86_match_cpu(efi_capsule_quirk_ids);
> + if (id) {
> + /*
> + * The quirk handler is supposed to return
> + * - a value > 0 if the setup should continue, after advancing
> + * kbuff as needed
> + * - 0 if not enough hdr_bytes are available yet
> + * - a negative error code otherwise
> + */
> + quirk_handler = (typeof(quirk_handler))id->driver_data;
> + ret = quirk_handler(cap_info, &kbuff, hdr_bytes);
> + if (ret <= 0)
> + return ret;
> + }
> +
> + memcpy(&cap_info->header, kbuff, sizeof(cap_info->header));
> +
> + cap_info->total_size += cap_info->header.imagesize;
> +
> + return __efi_capsule_setup_info(cap_info);
> +}
> +
> +#endif
> diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
> index 2e78b0b96d74..394db40ed374 100644
> --- a/drivers/firmware/efi/Kconfig
> +++ b/drivers/firmware/efi/Kconfig
> @@ -112,6 +112,15 @@ config EFI_CAPSULE_LOADER
>
> Most users should say N.
>
> +config EFI_CAPSULE_QUIRK_QUARK_CSH
> + boolean "Add support for Quark capsules with non-standard headers"
> + depends on X86 && !64BIT
> + select EFI_CAPSULE_LOADER
> + default y
> + help
> + Add support for processing Quark X1000 EFI capsules, whose header
> + layout deviates from the layout mandated by the UEFI specification.
> +
> config EFI_TEST
> tristate "EFI Runtime Service Tests Support"
> depends on EFI
> --
> 2.12.0
>



--
With Best Regards,
Andy Shevchenko

2017-04-20 09:44:39

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] efi: add support for non-standard capsule headers



On 19/04/17 19:32, Jan Kiszka wrote:
> This picks up the patches Ard send before in [1], including the
> "left-over" patches 6..8.
>
> As Ard suggested, I've taken updated patches 6 and 7 of him from [2]
> which address reviewer comments. Furthermore, I've changed patch 8 to
> factor out the Quark quirk logic from the overloaded
> efi_capsule_setup_info as requested by Matt and also applied Andy's
> suggestion to have a quirk dispatcher table with callbacks.
>
> Tested successfully on the IOT2040 - still without a working Galileo
> board.

I'll run it on a Galileo for you today.

2017-04-20 15:22:58

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] efi: add support for non-standard capsule headers



On 19/04/17 19:32, Jan Kiszka wrote:
> This picks up the patches Ard send before in [1], including the
> "left-over" patches 6..8.
>
> As Ard suggested, I've taken updated patches 6 and 7 of him from [2]
> which address reviewer comments. Furthermore, I've changed patch 8 to
> factor out the Quark quirk logic from the overloaded
> efi_capsule_setup_info as requested by Matt and also applied Andy's
> suggestion to have a quirk dispatcher table with callbacks.
>
> Tested successfully on the IOT2040 - still without a working Galileo
> board.
>
> Jan
>
> [1] http://www.spinics.net/lists/linux-efi/msg11194.html
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=quark-capsule
>
> Cc: Matt Fleming <[email protected]>
>
> Ard Biesheuvel (2):
> efi/capsule-loader: Redirect calls to efi_capsule_setup_info via weak
> alias
> efi/capsule-loader: Use page addresses rather than struct page
> pointers
>
> Jan Kiszka (1):
> efi/capsule: Add support for Quark security header
>
> arch/x86/platform/efi/quirks.c | 137 ++++++++++++++++++++++++++++++++++
> drivers/firmware/efi/Kconfig | 9 +++
> drivers/firmware/efi/capsule-loader.c | 66 ++++++++--------
> drivers/firmware/efi/capsule.c | 7 +-
> include/linux/efi.h | 14 +++-
> 5 files changed, 197 insertions(+), 36 deletions(-)
>

Still looks good, nothing's broken on Galileo.

Please feel free to add my

Tested-by: Bryan O'Donoghue <[email protected]>

To your patches

2017-04-25 15:00:16

by Matt Fleming

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] efi: add support for non-standard capsule headers

On Wed, 19 Apr, at 08:32:59PM, Jan Kiszka wrote:
> This picks up the patches Ard send before in [1], including the
> "left-over" patches 6..8.
>
> As Ard suggested, I've taken updated patches 6 and 7 of him from [2]
> which address reviewer comments. Furthermore, I've changed patch 8 to
> factor out the Quark quirk logic from the overloaded
> efi_capsule_setup_info as requested by Matt and also applied Andy's
> suggestion to have a quirk dispatcher table with callbacks.
>
> Tested successfully on the IOT2040 - still without a working Galileo
> board.
>
> Jan
>
> [1] http://www.spinics.net/lists/linux-efi/msg11194.html
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=quark-capsule
>
> Cc: Matt Fleming <[email protected]>
>
> Ard Biesheuvel (2):
> efi/capsule-loader: Redirect calls to efi_capsule_setup_info via weak
> alias
> efi/capsule-loader: Use page addresses rather than struct page
> pointers
>
> Jan Kiszka (1):
> efi/capsule: Add support for Quark security header
>
> arch/x86/platform/efi/quirks.c | 137 ++++++++++++++++++++++++++++++++++
> drivers/firmware/efi/Kconfig | 9 +++
> drivers/firmware/efi/capsule-loader.c | 66 ++++++++--------
> drivers/firmware/efi/capsule.c | 7 +-
> include/linux/efi.h | 14 +++-
> 5 files changed, 197 insertions(+), 36 deletions(-)

OK, this looks like it's in good shape to me.

Ard, are we waiting for anything else before we queue this up for
v4.13?

2017-04-25 15:01:58

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] efi: add support for non-standard capsule headers

On 25 April 2017 at 16:00, Matt Fleming <[email protected]> wrote:
> On Wed, 19 Apr, at 08:32:59PM, Jan Kiszka wrote:
>> This picks up the patches Ard send before in [1], including the
>> "left-over" patches 6..8.
>>
>> As Ard suggested, I've taken updated patches 6 and 7 of him from [2]
>> which address reviewer comments. Furthermore, I've changed patch 8 to
>> factor out the Quark quirk logic from the overloaded
>> efi_capsule_setup_info as requested by Matt and also applied Andy's
>> suggestion to have a quirk dispatcher table with callbacks.
>>
>> Tested successfully on the IOT2040 - still without a working Galileo
>> board.
>>
>> Jan
>>
>> [1] http://www.spinics.net/lists/linux-efi/msg11194.html
>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=quark-capsule
>>
>> Cc: Matt Fleming <[email protected]>
>>
>> Ard Biesheuvel (2):
>> efi/capsule-loader: Redirect calls to efi_capsule_setup_info via weak
>> alias
>> efi/capsule-loader: Use page addresses rather than struct page
>> pointers
>>
>> Jan Kiszka (1):
>> efi/capsule: Add support for Quark security header
>>
>> arch/x86/platform/efi/quirks.c | 137 ++++++++++++++++++++++++++++++++++
>> drivers/firmware/efi/Kconfig | 9 +++
>> drivers/firmware/efi/capsule-loader.c | 66 ++++++++--------
>> drivers/firmware/efi/capsule.c | 7 +-
>> include/linux/efi.h | 14 +++-
>> 5 files changed, 197 insertions(+), 36 deletions(-)
>
> OK, this looks like it's in good shape to me.
>
> Ard, are we waiting for anything else before we queue this up for
> v4.13?

Ideally, we'd get someone to test it with compliant capsule headers,
but I'm happy to just queue it for v4.13, and get it in -next asap
(i.e., right after the merge window closes)

2017-04-25 15:07:31

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] efi: add support for non-standard capsule headers

On 25/04/17 16:01, Ard Biesheuvel wrote:
> On 25 April 2017 at 16:00, Matt Fleming <[email protected]> wrote:
>> On Wed, 19 Apr, at 08:32:59PM, Jan Kiszka wrote:
>>> This picks up the patches Ard send before in [1], including the
>>> "left-over" patches 6..8.
>>>
>>> As Ard suggested, I've taken updated patches 6 and 7 of him from [2]
>>> which address reviewer comments. Furthermore, I've changed patch 8 to
>>> factor out the Quark quirk logic from the overloaded
>>> efi_capsule_setup_info as requested by Matt and also applied Andy's
>>> suggestion to have a quirk dispatcher table with callbacks.
>>>
>>> Tested successfully on the IOT2040 - still without a working Galileo
>>> board.
>>>
>>> Jan
>>>
>>> [1] http://www.spinics.net/lists/linux-efi/msg11194.html
>>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=quark-capsule
>>>
>>> Cc: Matt Fleming <[email protected]>
>>>
>>> Ard Biesheuvel (2):
>>> efi/capsule-loader: Redirect calls to efi_capsule_setup_info via weak
>>> alias
>>> efi/capsule-loader: Use page addresses rather than struct page
>>> pointers
>>>
>>> Jan Kiszka (1):
>>> efi/capsule: Add support for Quark security header
>>>
>>> arch/x86/platform/efi/quirks.c | 137 ++++++++++++++++++++++++++++++++++
>>> drivers/firmware/efi/Kconfig | 9 +++
>>> drivers/firmware/efi/capsule-loader.c | 66 ++++++++--------
>>> drivers/firmware/efi/capsule.c | 7 +-
>>> include/linux/efi.h | 14 +++-
>>> 5 files changed, 197 insertions(+), 36 deletions(-)
>>
>> OK, this looks like it's in good shape to me.
>>
>> Ard, are we waiting for anything else before we queue this up for
>> v4.13?
>
> Ideally, we'd get someone to test it with compliant capsule headers,
> but I'm happy to just queue it for v4.13, and get it in -next asap
> (i.e., right after the merge window closes)
>

I'm OOO until Thursday however, I could test this out on a Galileo for
you with the CSH stripped out. As Jan has pointed out Galileo aka Quark
x1000 non-secure (not to be called insecure) will just skips past the
CSH anyway - so - we can give it a conformant capsule by stripping the
CSH junk off the top.

How about you queue it up and I'll let you know if there's a problem
Thursday?

--
bod

2017-04-25 15:08:23

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] efi: add support for non-standard capsule headers

On 25 April 2017 at 16:07, Bryan O'Donoghue
<[email protected]> wrote:
> On 25/04/17 16:01, Ard Biesheuvel wrote:
>> On 25 April 2017 at 16:00, Matt Fleming <[email protected]> wrote:
>>> On Wed, 19 Apr, at 08:32:59PM, Jan Kiszka wrote:
>>>> This picks up the patches Ard send before in [1], including the
>>>> "left-over" patches 6..8.
>>>>
>>>> As Ard suggested, I've taken updated patches 6 and 7 of him from [2]
>>>> which address reviewer comments. Furthermore, I've changed patch 8 to
>>>> factor out the Quark quirk logic from the overloaded
>>>> efi_capsule_setup_info as requested by Matt and also applied Andy's
>>>> suggestion to have a quirk dispatcher table with callbacks.
>>>>
>>>> Tested successfully on the IOT2040 - still without a working Galileo
>>>> board.
>>>>
>>>> Jan
>>>>
>>>> [1] http://www.spinics.net/lists/linux-efi/msg11194.html
>>>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=quark-capsule
>>>>
>>>> Cc: Matt Fleming <[email protected]>
>>>>
>>>> Ard Biesheuvel (2):
>>>> efi/capsule-loader: Redirect calls to efi_capsule_setup_info via weak
>>>> alias
>>>> efi/capsule-loader: Use page addresses rather than struct page
>>>> pointers
>>>>
>>>> Jan Kiszka (1):
>>>> efi/capsule: Add support for Quark security header
>>>>
>>>> arch/x86/platform/efi/quirks.c | 137 ++++++++++++++++++++++++++++++++++
>>>> drivers/firmware/efi/Kconfig | 9 +++
>>>> drivers/firmware/efi/capsule-loader.c | 66 ++++++++--------
>>>> drivers/firmware/efi/capsule.c | 7 +-
>>>> include/linux/efi.h | 14 +++-
>>>> 5 files changed, 197 insertions(+), 36 deletions(-)
>>>
>>> OK, this looks like it's in good shape to me.
>>>
>>> Ard, are we waiting for anything else before we queue this up for
>>> v4.13?
>>
>> Ideally, we'd get someone to test it with compliant capsule headers,
>> but I'm happy to just queue it for v4.13, and get it in -next asap
>> (i.e., right after the merge window closes)
>>
>
> I'm OOO until Thursday however, I could test this out on a Galileo for
> you with the CSH stripped out. As Jan has pointed out Galileo aka Quark
> x1000 non-secure (not to be called insecure) will just skips past the
> CSH anyway - so - we can give it a conformant capsule by stripping the
> CSH junk off the top.
>
> How about you queue it up and I'll let you know if there's a problem
> Thursday?
>

That would be highly appreciated, thanks.

2017-04-27 12:41:41

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] efi: add support for non-standard capsule headers

On 25/04/17 16:08, Ard Biesheuvel wrote:
> On 25 April 2017 at 16:07, Bryan O'Donoghue
>> I'm OOO until Thursday however, I could test this out on a Galileo for
>> you with the CSH stripped out. As Jan has pointed out Galileo aka Quark
>> x1000 non-secure (not to be called insecure) will just skips past the
>> CSH anyway - so - we can give it a conformant capsule by stripping the
>> CSH junk off the top.
>>
>> How about you queue it up and I'll let you know if there's a problem
>> Thursday?
>>
>
> That would be highly appreciated, thanks.
>

Looks good.

I stripped the header
dd skip=1024 bs=1 if=firmware.cap of=firmware-no-csh.cap

Downloaded the now conformat capsule and update/reflash succeeded.

---
bod


2017-04-27 12:42:44

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] efi: add support for non-standard capsule headers

On 27 April 2017 at 13:46, Bryan O'Donoghue
<[email protected]> wrote:
> On 25/04/17 16:08, Ard Biesheuvel wrote:
>>
>> On 25 April 2017 at 16:07, Bryan O'Donoghue
>>>
>>> I'm OOO until Thursday however, I could test this out on a Galileo for
>>> you with the CSH stripped out. As Jan has pointed out Galileo aka Quark
>>> x1000 non-secure (not to be called insecure) will just skips past the
>>> CSH anyway - so - we can give it a conformant capsule by stripping the
>>> CSH junk off the top.
>>>
>>> How about you queue it up and I'll let you know if there's a problem
>>> Thursday?
>>>
>>
>> That would be highly appreciated, thanks.
>>
>
> Looks good.
>
> I stripped the header
> dd skip=1024 bs=1 if=firmware.cap of=firmware-no-csh.cap
>
> Downloaded the now conformat capsule and update/reflash succeeded.
>

Thanks. I will go ahead and queue these patches for v4.13 with your
Tested-by added.