Re: [PATCH v7 1/2] efi: export efi_capsule_supported() function symbol

Re: [PATCH v7 1/2] efi: export efi_capsule_supported() function symbol

[Borislav Petkov]

On Tue, Oct 06, 2015 at 04:15:54AM +0800, Kweh, Hock Leong wrote:
> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> 
> This patch export efi_capsule_supported() function symbol for capsule
> kernel module to use.
> 
> Cc: Matt Fleming <matt.fleming@intel.com>
> Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
> ---
>  drivers/firmware/efi/capsule.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
> index d8cd75c0..738d437 100644
> --- a/drivers/firmware/efi/capsule.c
> +++ b/drivers/firmware/efi/capsule.c
> @@ -101,6 +101,7 @@ out:
>  	kfree(capsule);
>  	return rv;
>  }
> +EXPORT_SYMBOL_GPL(efi_capsule_supported);

So this one is still a separate patch.

If you're going to ignore review comments, maybe I should stop wasting
my time reviewing your stuff...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

RE: [PATCH v7 1/2] efi: export efi_capsule_supported() function symbol

[Kweh, Hock Leong]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 641 bytes --]

> -----Original Message-----
> From: Borislav Petkov [mailto:bp@alien8.de]
> Sent: Monday, October 05, 2015 9:14 PM
> 
> So this one is still a separate patch.
> 
> If you're going to ignore review comments, maybe I should stop wasting my
> time reviewing your stuff...
> 
> --
> Regards/Gruss,
>     Boris.

Already follow what you have suggested, put a note under --- line:
https://lkml.org/lkml/2015/10/5/230 (at line 25 - 27)

Thanks for the review comments.


Regards,
Wilson
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[PATCH v7 0/2] Enable capsule loader interface for efi firmware updating

[Kweh, Hock Leong]

From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>

Dear maintainers & communities,

This patchset is created on top of Matt's patchset:
1.)https://lkml.org/lkml/2014/10/7/390
"[PATCH 1/2] efi: Move efi_status_to_err() to efi.h"
2.)https://lkml.org/lkml/2014/10/7/391
"[PATCH 2/2] efi: Capsule update support"

It expose a misc char interface for user to upload the capsule binary and
calling efi_capsule_update() API to pass the binary to EFI firmware.

The steps to update efi firmware are:
1.) cat firmware.cap > /dev/efi_capsule_loader
2.) reboot

Any failed upload error message will be returned while doing "cat" through
Write() function call.

Tested the code with Intel Quark Galileo platform.

Thanks.

---
NOTE:
If Matt agrees with this design, [PATCH v7 1/1] will be squash into his
[PATCH 2/2]: https://lkml.org/lkml/2014/10/7/391 for submitting

changelog v7:
* add successful message printed in dmesg
* shorten the code in efi_capsule_write() by splitting out
  efi_capsule_setup_info() & efi_capsule_submit_update() functions
* design added capability to support multiple file open action
* re-write those comments by following standard format
* design added the "uncomplete" error return through flush() file operation

changelog v6:
* clean up on error handling for better code flow and review
* clean up on pr_err() for critical error only
* design taking care writing block that below PAGE_SIZE
* once error has occurred, design will return -EIO until file close
* document design expectations/scenarios in the code
* change the dynamic allocation cap_info struct to statically allocated

changelog v5:
* changed to new design without leveraging firmware_class API
* use misc_char device interface instead of sysfs
* error return through file Write() function call


Kweh, Hock Leong (2):
  efi: export efi_capsule_supported() function symbol
  efi: a misc char interface for user to update efi firmware

 drivers/firmware/efi/Kconfig              |   10
 drivers/firmware/efi/Makefile             |    1
 drivers/firmware/efi/capsule.c            |    1
 drivers/firmware/efi/efi-capsule-loader.c |  336 +++++++++++++++++++++++++++++
 4 files changed, 348 insertions(+)
 create mode 100644 drivers/firmware/efi/efi-capsule-loader.c

--
1.7.9.5

[PATCH v7 1/2] efi: export efi_capsule_supported() function symbol

[Kweh, Hock Leong]

From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>

This patch export efi_capsule_supported() function symbol for capsule
kernel module to use.

Cc: Matt Fleming <matt.fleming@intel.com>
Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
---
 drivers/firmware/efi/capsule.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
index d8cd75c0..738d437 100644
--- a/drivers/firmware/efi/capsule.c
+++ b/drivers/firmware/efi/capsule.c
@@ -101,6 +101,7 @@ out:
 	kfree(capsule);
 	return rv;
 }
+EXPORT_SYMBOL_GPL(efi_capsule_supported);
 
 /**
  * efi_capsule_update - send a capsule to the firmware
-- 
1.7.9.5

[PATCH v7 2/2] efi: a misc char interface for user to update efi firmware

[Kweh, Hock Leong]

From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>

Introducing a kernel module to expose capsule loader interface
(misc char device file note) for user to upload capsule binaries.

Example method to load the capsule binary:
cat firmware.bin > /dev/efi_capsule_loader

Cc: Matt Fleming <matt.fleming@intel.com>
Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
---
 drivers/firmware/efi/Kconfig              |   10 +
 drivers/firmware/efi/Makefile             |    1 +
 drivers/firmware/efi/efi-capsule-loader.c |  336 +++++++++++++++++++++++++++++
 3 files changed, 347 insertions(+)
 create mode 100644 drivers/firmware/efi/efi-capsule-loader.c

diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index f712d47..0be8ee3 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -60,6 +60,16 @@ config EFI_RUNTIME_WRAPPERS
 config EFI_ARMSTUB
 	bool
 
+config EFI_CAPSULE_LOADER
+	tristate "EFI capsule loader"
+	depends on EFI
+	help
+	  This option exposes a loader interface "/dev/efi_capsule_loader" for
+	  user to load EFI capsule binary and update the EFI firmware through
+	  system reboot.
+
+	  If unsure, say N.
+
 endmenu
 
 config UEFI_CPER
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 698846e..5ab031a 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_UEFI_CPER)			+= cper.o
 obj-$(CONFIG_EFI_RUNTIME_MAP)		+= runtime-map.o
 obj-$(CONFIG_EFI_RUNTIME_WRAPPERS)	+= runtime-wrappers.o
 obj-$(CONFIG_EFI_STUB)			+= libstub/
+obj-$(CONFIG_EFI_CAPSULE_LOADER)	+= efi-capsule-loader.o
diff --git a/drivers/firmware/efi/efi-capsule-loader.c b/drivers/firmware/efi/efi-capsule-loader.c
new file mode 100644
index 0000000..9c91658
--- /dev/null
+++ b/drivers/firmware/efi/efi-capsule-loader.c
@@ -0,0 +1,336 @@
+/*
+ * EFI capsule loader driver.
+ *
+ * Copyright 2015 Intel Corporation
+ *
+ * This file is part of the Linux kernel, and is made available under
+ * the terms of the GNU General Public License version 2.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/miscdevice.h>
+#include <linux/highmem.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/efi.h>
+
+#define DEV_NAME "efi_capsule_loader"
+#define UPLOAD_DONE -1
+#define ERR_OCCURED -2
+
+struct capsule_info {
+	char		header_obtained;
+	int		reset_type;
+	long		index;
+	unsigned long	count;
+	unsigned long	total_size;
+	struct page	**pages;
+	unsigned long	page_count_remain;
+};
+
+static DEFINE_MUTEX(capsule_loader_lock);
+
+/**
+ * efi_free_all_buff_pages - free the current & all previous allocated buffer
+ *			     pages
+ * @file: file pointer
+ * @kbuff_page: the current execution allocated buffer page's pointer which has
+ *		not yet assigned to pages[] array
+ *
+ *	The input param can be NULL if the current execution has not allocated
+ *	any buffer page.
+ **/
+static void efi_free_all_buff_pages(struct file *file, struct page *kbuff_page)
+{
+	struct capsule_info *cap_info = file->private_data;
+
+	if (kbuff_page)
+		__free_page(kbuff_page);
+
+	if (cap_info->index > 0)
+		while (cap_info->index > 0)
+			__free_page(cap_info->pages[--cap_info->index]);
+
+	if (cap_info->header_obtained)
+		kfree(cap_info->pages);
+
+	/* indicate to the next that error had occurred */
+	cap_info->index = ERR_OCCURED;
+}
+
+/**
+ * efi_capsule_setup_info - to obtain the efi capsule header in the binary and
+ *			    setup capsule_info structure
+ * @file: file pointer
+ * @kbuff: a mapped 1st page buffer pointer
+ **/
+static ssize_t efi_capsule_setup_info(struct file *file, void *kbuff)
+{
+	int ret = 0;
+	struct capsule_info *cap_info = file->private_data;
+	efi_capsule_header_t *cap_hdr = kbuff;
+	size_t pages_needed = ALIGN(cap_hdr->imagesize, PAGE_SIZE) >>
+				    PAGE_SHIFT;
+
+	if (pages_needed == 0) {
+		pr_err("%s: pages count invalid\n", __func__);
+		return -EINVAL;
+	}
+
+	/* check if the capsule binary supported */
+	mutex_lock(&capsule_loader_lock);
+	ret = efi_capsule_supported(cap_hdr->guid, cap_hdr->flags,
+				    cap_hdr->imagesize, &cap_info->reset_type);
+	mutex_unlock(&capsule_loader_lock);
+	if (ret) {
+		pr_err("%s: efi_capsule_supported() failed\n", __func__);
+		return ret;
+	}
+
+	cap_info->total_size = cap_hdr->imagesize;
+	cap_info->pages = kmalloc_array(pages_needed, sizeof(void *),
+					GFP_KERNEL);
+	if (!cap_info->pages) {
+		pr_debug("%s: kmalloc_array() failed\n", __func__);
+		return -ENOMEM;
+	}
+
+	cap_info->header_obtained = 1;
+	return ret;
+}
+
+/**
+ * efi_capsule_submit_update - invoke the efi_capsule_update API once binary
+ *			       upload done
+ * @file: file pointer
+ **/
+static ssize_t efi_capsule_submit_update(struct file *file)
+{
+	int ret = 0;
+	struct capsule_info *cap_info = file->private_data;
+	void *cap_hdr_temp;
+
+	cap_hdr_temp = kmap(cap_info->pages[0]);
+	if (!cap_hdr_temp) {
+		pr_debug("%s: kmap() failed\n", __func__);
+		return -EFAULT;
+	}
+
+	mutex_lock(&capsule_loader_lock);
+	ret = efi_capsule_update(cap_hdr_temp, cap_info->pages);
+	mutex_unlock(&capsule_loader_lock);
+	kunmap(cap_info->pages[0]);
+	if (ret) {
+		pr_err("%s: efi_capsule_update() failed\n", __func__);
+		return ret;
+	}
+
+	/* indicate capsule binary uploading is done */
+	cap_info->index = UPLOAD_DONE;
+	pr_info("%s: Successfully upload capsule file with reboot type '%s'\n",
+		__func__, !cap_info->reset_type ? "RESET_COLD" :
+		cap_info->reset_type == 1 ? "RESET_WARM" :
+		"RESET_SHUTDOWN");
+	return ret;
+}
+
+/**
+ * efi_capsule_write - store the capsule binary and pass it to
+ *		       efi_capsule_update() API in capsule.c
+ * @file: file pointer
+ * @buff: buffer pointer
+ * @count: number of bytes in @buff
+ * @offp: not use
+ *
+ *	Expectation:
+ *	- User space tool should start at the beginning of capsule binary and
+ *	  pass data in sequential.
+ *	- User should close and re-open this file note in order to upload more
+ *	  capsules.
+ *	- Any error occurred, user should close the file and restart the
+ *	  operation for the next try otherwise -EIO will be returned until the
+ *	  file is closed.
+ *	- EFI capsule header must be located at the beginning of capsule binary
+ *	  file and passed in as a single block.
+ **/
+static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
+				 size_t count, loff_t *offp)
+{
+	int ret = 0;
+	struct capsule_info *cap_info = file->private_data;
+	struct page *kbuff_page = NULL;
+	void *kbuff;
+	size_t write_byte;
+
+	if (count == 0)
+		return 0;
+
+	/* return error while error had occurred or capsule uploading is done */
+	if (cap_info->index < 0)
+		return -EIO;
+
+	/* only alloc a new page when previous page is full */
+	if (!cap_info->page_count_remain) {
+		kbuff_page = alloc_page(GFP_KERNEL);
+		if (!kbuff_page) {
+			pr_debug("%s: alloc_page() failed\n", __func__);
+			ret = -ENOMEM;
+			goto failed;
+		}
+		cap_info->page_count_remain = PAGE_SIZE;
+	} else {
+		kbuff_page = cap_info->pages[--cap_info->index];
+	}
+
+	kbuff = kmap(kbuff_page);
+	if (!kbuff) {
+		pr_debug("%s: kmap() failed\n", __func__);
+		ret = -EFAULT;
+		goto failed;
+	}
+	kbuff += PAGE_SIZE - cap_info->page_count_remain;
+
+	/* copy capsule binary data from user space to kernel space buffer */
+	write_byte = min_t(size_t, count, cap_info->page_count_remain);
+	if (copy_from_user(kbuff, buff, write_byte)) {
+		pr_debug("%s: copy_from_user() failed\n", __func__);
+		kunmap(kbuff_page);
+		ret = -EFAULT;
+		goto failed;
+	}
+	cap_info->page_count_remain -= write_byte;
+
+	/* setup capsule binary info structure */
+	if (cap_info->header_obtained == 0 && cap_info->index == 0) {
+		ret = efi_capsule_setup_info(file, kbuff);
+		if (ret) {
+			kunmap(kbuff_page);
+			goto failed;
+		}
+	}
+
+	cap_info->pages[cap_info->index++] = kbuff_page;
+	cap_info->count += write_byte;
+	kunmap(kbuff_page);
+	kbuff_page = NULL;
+
+	/* submit the full binary to efi_capsule_update() API */
+	if (cap_info->count >= cap_info->total_size) {
+		ret = efi_capsule_submit_update(file);
+		if (ret)
+			goto failed;
+	}
+
+	return write_byte;
+
+failed:
+	efi_free_all_buff_pages(file, kbuff_page);
+	return ret;
+}
+
+/**
+ * efi_capsule_flush - called by file close or file flush
+ * @file: file pointer
+ * @id: not use
+ *
+ *	If capsule being uploaded partially, calling this function will be
+ *	treated as uploading canceled and will free up those completed buffer
+ *	pages and then return -ECANCELED.
+ **/
+static int efi_capsule_flush(struct file *file, fl_owner_t id)
+{
+	int ret = 0;
+	struct capsule_info *cap_info = file->private_data;
+
+	if (cap_info->index > 0) {
+		pr_err("%s: capsule upload not complete\n", __func__);
+		efi_free_all_buff_pages(file, NULL);
+		ret = -ECANCELED;
+	}
+
+	return ret;
+}
+
+/**
+ * efi_capsule_release - called by file close
+ * @inode: not use
+ * @file: file pointer
+ *
+ *	We would not free the successful submitted buffer pages here due to
+ *	efi update require system reboot with data maintained.
+ **/
+static int efi_capsule_release(struct inode *inode, struct file *file)
+{
+	kfree(file->private_data);
+	file->private_data = NULL;
+	return 0;
+}
+
+/**
+ * efi_capsule_open - called by file open
+ * @inode: not use
+ * @file: file pointer
+ *
+ *	Will allocate each capsule_info memory for each file open call.
+ *	This provided the capability to support multiple file open feature
+ *	where user is not needed to wait for others to finish in order to
+ *	upload their capsule binary.
+ **/
+static int efi_capsule_open(struct inode *inode, struct file *file)
+{
+	struct capsule_info *cap_info;
+
+	cap_info = kzalloc(sizeof(*cap_info), GFP_KERNEL);
+	if (!cap_info) {
+		pr_debug("%s: kzalloc() failed\n", __func__);
+		return -ENOMEM;
+	}
+	file->private_data = cap_info;
+
+	return 0;
+}
+
+static const struct file_operations efi_capsule_fops = {
+	.owner = THIS_MODULE,
+	.open = efi_capsule_open,
+	.write = efi_capsule_write,
+	.flush = efi_capsule_flush,
+	.release = efi_capsule_release,
+	.llseek = no_llseek,
+};
+
+static struct miscdevice efi_capsule_misc = {
+	.minor = MISC_DYNAMIC_MINOR,
+	.name = DEV_NAME,
+	.fops = &efi_capsule_fops,
+};
+
+static int __init efi_capsule_loader_init(void)
+{
+	int ret;
+
+	mutex_init(&capsule_loader_lock);
+
+	ret = misc_register(&efi_capsule_misc);
+	if (ret) {
+		pr_err("%s: Failed to register misc char file note\n",
+		       __func__);
+		mutex_destroy(&capsule_loader_lock);
+	}
+
+	return ret;
+}
+module_init(efi_capsule_loader_init);
+
+static void __exit efi_capsule_loader_exit(void)
+{
+	misc_deregister(&efi_capsule_misc);
+	mutex_destroy(&capsule_loader_lock);
+}
+module_exit(efi_capsule_loader_exit);
+
+MODULE_DESCRIPTION("EFI capsule firmware binary loader");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

Re: [PATCH v7 1/2] efi: export efi_capsule_supported() function symbol

[Bryan O'Donoghue]

On 05/10/15 16:19, Kweh, Hock Leong wrote:
>> -----Original Message-----
>> From: Borislav Petkov [mailto:bp@alien8.de]
>> Sent: Monday, October 05, 2015 9:14 PM
>>
>> So this one is still a separate patch.
>>
>> If you're going to ignore review comments, maybe I should stop wasting my
>> time reviewing your stuff...
>>
>> --
>> Regards/Gruss,
>>      Boris.
>
> Already follow what you have suggested, put a note under --- line:
> https://lkml.org/lkml/2015/10/5/230 (at line 25 - 27)
>
> Thanks for the review comments.

Wilson - trying to test this out on a Galileo Gen2 - which branch are 
you doing this against ?

I can apply the first patch you're proposing to squash your commit into

https://lkml.org/lkml/diff/2014/10/7/390/1

but then trying to apply the first in your series on top of that patch I get

deckard@aineko:~/Development/linux$ git apply 
../patches/capsule_wilson/1_2.eml
../patches/capsule_wilson/1_2.eml:72: trailing whitespace.
EXPORT_SYMBOL_GPL(efi_capsule_supported);
error: drivers/firmware/efi/capsule.c: No such file or directory

https://kernel.googlesource.com/pub/scm/linux/kernel/git/mfleming/efi/+/capsule/drivers/firmware/efi/capsule.c 


??

If so - then why not use the interface here ?
https://kernel.googlesource.com/pub/scm/linux/kernel/git/mfleming/efi/+/capsule

(Sorry I know I'm coming to this thread late)

Aside from that, I'm curious which types of capsules you've used here 
too - does it include the MFH header ? Keep in mind the initial firmware 
that shipped with Galileo will depend on that MFH being present.

http://download.intel.com/support/processors/quark/sb/quark_securebootprm_330234_001.pdf 
- Section A1 - table 7 ?

So if we boot a 4.x kernel with that initial firmware version 0.75 if 
memory serves - it's important that the capsule.c code handles the MFH.

--
BOD

RE: [PATCH v7 1/2] efi: export efi_capsule_supported() function symbol

[Kweh, Hock Leong]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3493 bytes --]

> -----Original Message-----
> From: Bryan O'Donoghue [mailto:pure.logic@nexus-software.ie]
> Sent: Tuesday, October 06, 2015 5:27 AM
> 
> Wilson - trying to test this out on a Galileo Gen2 - which branch are you doing
> this against ?
> 
> I can apply the first patch you're proposing to squash your commit into
> 
> https://lkml.org/lkml/diff/2014/10/7/390/1
> 
> but then trying to apply the first in your series on top of that patch I get
> 
> deckard@aineko:~/Development/linux$ git
> apply ../patches/capsule_wilson/1_2.eml
> ../patches/capsule_wilson/1_2.eml:72: trailing whitespace.
> EXPORT_SYMBOL_GPL(efi_capsule_supported);
> error: drivers/firmware/efi/capsule.c: No such file or directory
> 
> https://kernel.googlesource.com/pub/scm/linux/kernel/git/mfleming/efi/+/
> capsule/drivers/firmware/efi/capsule.c
> 
> 
> ??

If you are applying Matt's patch https://lkml.org/lkml/diff/2014/10/7/390/1 which
had been created 1 year ago to mainline vanilla kernel (Linux 4.3-rc4), you are not
able to direct patch in due to the Makefile error below:

~/MyWorks/linux_mainline$ git apply .git/rebase-apply/0001 --reject 
Checking patch arch/x86/kernel/reboot.c...
Hunk #1 succeeded at 527 (offset 11 lines).
Checking patch drivers/firmware/efi/Makefile...
error: while searching for:
#
# Makefile for linux kernel
#
obj-$(CONFIG_EFI)                       += efi.o vars.o reboot.o
obj-$(CONFIG_EFI_VARS)                  += efivars.o
obj-$(CONFIG_EFI_VARS_PSTORE)           += efi-pstore.o
obj-$(CONFIG_UEFI_CPER)                 += cper.o
 
error: patch failed: drivers/firmware/efi/Makefile:1
Checking patch drivers/firmware/efi/capsule.c...
Checking patch drivers/firmware/efi/reboot.c...
Checking patch include/linux/efi.h...
Hunk #1 succeeded at 122 (offset 3 lines).
Hunk #2 succeeded at 983 (offset 23 lines).
Hunk #3 succeeded at 1235 (offset 23 lines).
Hunk #4 succeeded at 1317 (offset 23 lines).
Applied patch arch/x86/kernel/reboot.c cleanly.
Applying patch drivers/firmware/efi/Makefile with 1 rejects...
Rejected hunk #1.
Applied patch drivers/firmware/efi/capsule.c cleanly.
Applied patch drivers/firmware/efi/reboot.c cleanly.
Applied patch include/linux/efi.h cleanly.

You should resolve the Makefile error and then git add 5 files below:
- arch/x86/kernel/reboot.c
- drivers/firmware/efi/Makefile
- drivers/firmware/efi/reboot.c
- include/linux/efi.h
- drivers/firmware/efi/capsule.c

then you are able to patch in my patchset.

> 
> If so - then why not use the interface here ?
> https://kernel.googlesource.com/pub/scm/linux/kernel/git/mfleming/efi/+/
> capsule
> 
> (Sorry I know I'm coming to this thread late)
> 
> Aside from that, I'm curious which types of capsules you've used here too -
> does it include the MFH header ? Keep in mind the initial firmware that
> shipped with Galileo will depend on that MFH being present.
> 
> http://download.intel.com/support/processors/quark/sb/quark_secureboot
> prm_330234_001.pdf
> - Section A1 - table 7 ?
> 
> So if we boot a 4.x kernel with that initial firmware version 0.75 if memory
> serves - it's important that the capsule.c code handles the MFH.
> 

Already got agreement with Matt that Quark Security Header patch will not
be upstream to mainline as it is not a standard header. So Intel will carry this
patch ourselves.


Thanks & Regards,
Wilson
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v7 1/2] efi: export efi_capsule_supported() function symbol

[Bryan O'Donoghue]

On 06/10/15 11:53, Kweh, Hock Leong wrote:
>> -----Original Message-----
>> From: Bryan O'Donoghue [mailto:pure.logic@nexus-software.ie]
>> Sent: Tuesday, October 06, 2015 5:27 AM
>>
>> Wilson - trying to test this out on a Galileo Gen2 - which branch are you doing
>> this against ?
>>
>> I can apply the first patch you're proposing to squash your commit into
>>
>> https://lkml.org/lkml/diff/2014/10/7/390/1
>>
>> but then trying to apply the first in your series on top of that patch I get
>>
>> deckard@aineko:~/Development/linux$ git
>> apply ../patches/capsule_wilson/1_2.eml
>> ../patches/capsule_wilson/1_2.eml:72: trailing whitespace.
>> EXPORT_SYMBOL_GPL(efi_capsule_supported);
>> error: drivers/firmware/efi/capsule.c: No such file or directory
>>
>> https://kernel.googlesource.com/pub/scm/linux/kernel/git/mfleming/efi/+/
>> capsule/drivers/firmware/efi/capsule.c
>>
>>
>> ??
>
> If you are applying Matt's patch https://lkml.org/lkml/diff/2014/10/7/390/1 which
> had been created 1 year ago to mainline vanilla kernel (Linux 4.3-rc4), you are not
> able to direct patch in due to the Makefile error below:
>
> ~/MyWorks/linux_mainline$ git apply .git/rebase-apply/0001 --reject
> Checking patch arch/x86/kernel/reboot.c...
> Hunk #1 succeeded at 527 (offset 11 lines).
> Checking patch drivers/firmware/efi/Makefile...
> error: while searching for:
> #
> # Makefile for linux kernel
> #
> obj-$(CONFIG_EFI)                       += efi.o vars.o reboot.o
> obj-$(CONFIG_EFI_VARS)                  += efivars.o
> obj-$(CONFIG_EFI_VARS_PSTORE)           += efi-pstore.o
> obj-$(CONFIG_UEFI_CPER)                 += cper.o
>
> error: patch failed: drivers/firmware/efi/Makefile:1
> Checking patch drivers/firmware/efi/capsule.c...
> Checking patch drivers/firmware/efi/reboot.c...
> Checking patch include/linux/efi.h...
> Hunk #1 succeeded at 122 (offset 3 lines).
> Hunk #2 succeeded at 983 (offset 23 lines).
> Hunk #3 succeeded at 1235 (offset 23 lines).
> Hunk #4 succeeded at 1317 (offset 23 lines).
> Applied patch arch/x86/kernel/reboot.c cleanly.
> Applying patch drivers/firmware/efi/Makefile with 1 rejects...
> Rejected hunk #1.
> Applied patch drivers/firmware/efi/capsule.c cleanly.
> Applied patch drivers/firmware/efi/reboot.c cleanly.
> Applied patch include/linux/efi.h cleanly.
>
> You should resolve the Makefile error and then git add 5 files below:
> - arch/x86/kernel/reboot.c
> - drivers/firmware/efi/Makefile
> - drivers/firmware/efi/reboot.c
> - include/linux/efi.h
> - drivers/firmware/efi/capsule.c
>
> then you are able to patch in my patchset.
>
>>
>> If so - then why not use the interface here ?
>> https://kernel.googlesource.com/pub/scm/linux/kernel/git/mfleming/efi/+/
>> capsule
>>
>> (Sorry I know I'm coming to this thread late)
>>
>> Aside from that, I'm curious which types of capsules you've used here too -
>> does it include the MFH header ? Keep in mind the initial firmware that
>> shipped with Galileo will depend on that MFH being present.
>>
>> http://download.intel.com/support/processors/quark/sb/quark_secureboot
>> prm_330234_001.pdf
>> - Section A1 - table 7 ?
>>
>> So if we boot a 4.x kernel with that initial firmware version 0.75 if memory
>> serves - it's important that the capsule.c code handles the MFH.
>>
>
> Already got agreement with Matt that Quark Security Header patch will not
> be upstream to mainline as it is not a standard header. So Intel will carry this
> patch ourselves.

Right... so what sort of capsule are you testing with ?

RE: [PATCH v7 1/2] efi: export efi_capsule_supported() function symbol

[Kweh, Hock Leong]

> -----Original Message-----
> From: Bryan O'Donoghue [mailto:pure.logic@nexus-software.ie]
> Sent: Tuesday, October 06, 2015 10:54 PM
> >>
> >> Aside from that, I'm curious which types of capsules you've used here
> >> too - does it include the MFH header ? Keep in mind the initial
> >> firmware that shipped with Galileo will depend on that MFH being
> present.
> >>
> >>
> http://download.intel.com/support/processors/quark/sb/quark_secureboo
> >> t
> >> prm_330234_001.pdf
> >> - Section A1 - table 7 ?
> >>
> >> So if we boot a 4.x kernel with that initial firmware version 0.75 if
> >> memory serves - it's important that the capsule.c code handles the MFH.
> >>
> >
> > Already got agreement with Matt that Quark Security Header patch will
> > not be upstream to mainline as it is not a standard header. So Intel
> > will carry this patch ourselves.
> 
> Right... so what sort of capsule are you testing with ?

I am testing on Intel Galileo Gen 1 with bios version v0.7.5, v0.8.0, v1.0.1 & v1.0.2.

Thanks & Regards,
Wilson

Re: [PATCH v7 1/2] efi: export efi_capsule_supported() function symbol

[Bryan O'Donoghue]

On 07/10/15 03:01, Kweh, Hock Leong wrote:
>> -----Original Message-----
>> From: Bryan O'Donoghue [mailto:pure.logic@nexus-software.ie]
>> Sent: Tuesday, October 06, 2015 10:54 PM
>>>>
>>>> Aside from that, I'm curious which types of capsules you've used here
>>>> too - does it include the MFH header ? Keep in mind the initial
>>>> firmware that shipped with Galileo will depend on that MFH being
>> present.
>>>>
>>>>
>> http://download.intel.com/support/processors/quark/sb/quark_secureboo
>>>> t
>>>> prm_330234_001.pdf
>>>> - Section A1 - table 7 ?
>>>>
>>>> So if we boot a 4.x kernel with that initial firmware version 0.75 if
>>>> memory serves - it's important that the capsule.c code handles the MFH.
>>>>
>>>
>>> Already got agreement with Matt that Quark Security Header patch will
>>> not be upstream to mainline as it is not a standard header. So Intel
>>> will carry this patch ourselves.
>>
>> Right... so what sort of capsule are you testing with ?
>
> I am testing on Intel Galileo Gen 1 with bios version v0.7.5, v0.8.0, v1.0.1 & v1.0.2.
>
> Thanks & Regards,
> Wilson
>

Hmm.

That's pretty confusing.

The 0.75 BIOS requires the MFH as far as I remember. If the capsule you 
are using doesn't have the MFH - how is this working ?

Re: [PATCH v7 1/2] efi: export efi_capsule_supported() function symbol

[Matt Fleming]

On Mon, 05 Oct, at 03:13:50PM, Borislav Petkov wrote:
> On Tue, Oct 06, 2015 at 04:15:54AM +0800, Kweh, Hock Leong wrote:
> > From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> > 
> > This patch export efi_capsule_supported() function symbol for capsule
> > kernel module to use.
> > 
> > Cc: Matt Fleming <matt.fleming@intel.com>
> > Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
> > ---
> >  drivers/firmware/efi/capsule.c |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
> > index d8cd75c0..738d437 100644
> > --- a/drivers/firmware/efi/capsule.c
> > +++ b/drivers/firmware/efi/capsule.c
> > @@ -101,6 +101,7 @@ out:
> >  	kfree(capsule);
> >  	return rv;
> >  }
> > +EXPORT_SYMBOL_GPL(efi_capsule_supported);
> 
> So this one is still a separate patch.
> 
> If you're going to ignore review comments, maybe I should stop wasting
> my time reviewing your stuff...

I agree that it makes sense to fold this patch into your PATCH 2,
because then we know why we need the above symbol to be exported.

-- 
Matt Fleming, Intel Open Source Technology Center

RE: [PATCH v7 1/2] efi: export efi_capsule_supported() function symbol

[Kweh, Hock Leong]

> -----Original Message-----
> From: Matt Fleming [mailto:matt@console-pimps.org]
> Sent: Sunday, October 11, 2015 6:02 AM
> 
> I agree that it makes sense to fold this patch into your PATCH 2, because then
> we know why we need the above symbol to be exported.
> 

Okay, I will squash that into my patch and re-submit v9. Before that,
are there any comments to my v8 patchset?

Thanks & Regards,
Wilson

Re: [PATCH v7 1/2] efi: export efi_capsule_supported() function symbol

[Matt Fleming]

On Sun, 11 Oct, at 02:28:30PM, Kweh Hock Leong wrote:
> > -----Original Message-----
> > From: Matt Fleming [mailto:matt@console-pimps.org]
> > Sent: Sunday, October 11, 2015 6:02 AM
> > 
> > I agree that it makes sense to fold this patch into your PATCH 2, because then
> > we know why we need the above symbol to be exported.
> > 
> 
> Okay, I will squash that into my patch and re-submit v9. Before that,
> are there any comments to my v8 patchset?

Yeah, I've got some comments on your v8. Please wait until I send them
before mailing the next version of this patch series.

-- 
Matt Fleming, Intel Open Source Technology Center