Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-20 02: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); > +} > > 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. Done now in my newer patches. Some refactoring in the core is still required, but the Quark logic will be active only on x86 and fully executed only on Quark CPUs. > > 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. No idea what went wrong back then, but I was able to reflash a Gen2 multiple times these days with the original 1.1.0 and 1.0.4 capsules Intel ships for that board - without removing the CSH. IOW, we can finally use capsule support on the Galileo. Patches to come soon. Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-20 02: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); > +} > > 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. Done now in my newer patches. Some refactoring in the core is still required, but the Quark logic will be active only on x86 and fully executed only on Quark CPUs. > > 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. No idea what went wrong back then, but I was able to reflash a Gen2 multiple times these days with the original 1.1.0 and 1.0.4 capsules Intel ships for that board - without removing the CSH. IOW, we can finally use capsule support on the Galileo. Patches to come soon. Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Wed, Mar 1, 2017 at 4:02 PM, Bryan O'Donoghuewrote: >>> So, summarize, you state that >>> 1. CONFIG_SMP=y and >>> 2. CONFIG_M686=y and >>> 3. Kernel works on Quark >>> >>> Is it correct? >> Logically yes. It's a very long time since I looked in detail. No harm >> in checking it out though. >> >> I'll compile up the above kernel this evening (GMT) and verify. > CONFIG_SMP=y - no difference - like I say it's PF# on lock prefix >instructions, not SMP=y that's the problem here > CONFIG_M686=y (doesnt' boot) > CONFIG_M586TSC=y does boot > So yes M686 is not bootable on this part. My point to you about having a > custom kernel though still stands, you shouldn't have to compile a quark > specific kernel - just a 586TSC kernel with Quark support. ... which is not default. That's my point. > For example CentOS is bootable on Quark. Because someone there took care of it. (I think being i586 compatible binaries as well) -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Wed, Mar 1, 2017 at 4:02 PM, Bryan O'Donoghue wrote: >>> So, summarize, you state that >>> 1. CONFIG_SMP=y and >>> 2. CONFIG_M686=y and >>> 3. Kernel works on Quark >>> >>> Is it correct? >> Logically yes. It's a very long time since I looked in detail. No harm >> in checking it out though. >> >> I'll compile up the above kernel this evening (GMT) and verify. > CONFIG_SMP=y - no difference - like I say it's PF# on lock prefix >instructions, not SMP=y that's the problem here > CONFIG_M686=y (doesnt' boot) > CONFIG_M586TSC=y does boot > So yes M686 is not bootable on this part. My point to you about having a > custom kernel though still stands, you shouldn't have to compile a quark > specific kernel - just a 586TSC kernel with Quark support. ... which is not default. That's my point. > For example CentOS is bootable on Quark. Because someone there took care of it. (I think being i586 compatible binaries as well) -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 28/02/17 17:42, Bryan O'Donoghue wrote: On 28/02/17 17:18, Andy Shevchenko wrote: On Tue, Feb 28, 2017 at 6:52 PM, Bryan O'Donoghue A kernel compiled like this make menuconfig ARCH=i386 I hope you care that it is equivalent to make menuconfig ARCH=i686 make bzImage -j 8 will run just fine on Quark x1000 I do it regularly. CPUID ought to (and does) inform the runtime kernel of what to do re: MSRs etc. We won't execute xmm/mmx, we won't touch 686 specific MSRs etc, etc. It is i*6*86 code still. So, summarize, you state that 1. CONFIG_SMP=y and 2. CONFIG_M686=y and 3. Kernel works on Quark Is it correct? Logically yes. It's a very long time since I looked in detail. No harm in checking it out though. I'll compile up the above kernel this evening (GMT) and verify. CONFIG_SMP=y - no difference - like I say it's PF# on lock prefix instructions, not SMP=y that's the problem here CONFIG_M686=y (doesnt' boot) CONFIG_M586TSC=y does boot So yes M686 is not bootable on this part. My point to you about having a custom kernel though still stands, you shouldn't have to compile a quark specific kernel - just a 586TSC kernel with Quark support. For example CentOS is bootable on Quark. --- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 28/02/17 17:42, Bryan O'Donoghue wrote: On 28/02/17 17:18, Andy Shevchenko wrote: On Tue, Feb 28, 2017 at 6:52 PM, Bryan O'Donoghue A kernel compiled like this make menuconfig ARCH=i386 I hope you care that it is equivalent to make menuconfig ARCH=i686 make bzImage -j 8 will run just fine on Quark x1000 I do it regularly. CPUID ought to (and does) inform the runtime kernel of what to do re: MSRs etc. We won't execute xmm/mmx, we won't touch 686 specific MSRs etc, etc. It is i*6*86 code still. So, summarize, you state that 1. CONFIG_SMP=y and 2. CONFIG_M686=y and 3. Kernel works on Quark Is it correct? Logically yes. It's a very long time since I looked in detail. No harm in checking it out though. I'll compile up the above kernel this evening (GMT) and verify. CONFIG_SMP=y - no difference - like I say it's PF# on lock prefix instructions, not SMP=y that's the problem here CONFIG_M686=y (doesnt' boot) CONFIG_M586TSC=y does boot So yes M686 is not bootable on this part. My point to you about having a custom kernel though still stands, you shouldn't have to compile a quark specific kernel - just a 586TSC kernel with Quark support. For example CentOS is bootable on Quark. --- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 28/02/17 17:18, Andy Shevchenko wrote: > On Tue, Feb 28, 2017 at 6:52 PM, Bryan O'Donoghue >> A kernel compiled like this >> >> make menuconfig ARCH=i386 > > I hope you care that it is equivalent to > > make menuconfig ARCH=i686 > >> make bzImage -j 8 >> >> will run just fine on Quark x1000 I do it regularly. CPUID ought to (and >> does) inform the runtime kernel of what to do re: MSRs etc. >> >> We won't execute xmm/mmx, we won't touch 686 specific MSRs etc, etc. > > It is i*6*86 code still. > > So, summarize, you state that > 1. CONFIG_SMP=y and > 2. CONFIG_M686=y and > 3. Kernel works on Quark > > Is it correct? Logically yes. It's a very long time since I looked in detail. No harm in checking it out though. I'll compile up the above kernel this evening (GMT) and verify. -- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 28/02/17 17:18, Andy Shevchenko wrote: > On Tue, Feb 28, 2017 at 6:52 PM, Bryan O'Donoghue >> A kernel compiled like this >> >> make menuconfig ARCH=i386 > > I hope you care that it is equivalent to > > make menuconfig ARCH=i686 > >> make bzImage -j 8 >> >> will run just fine on Quark x1000 I do it regularly. CPUID ought to (and >> does) inform the runtime kernel of what to do re: MSRs etc. >> >> We won't execute xmm/mmx, we won't touch 686 specific MSRs etc, etc. > > It is i*6*86 code still. > > So, summarize, you state that > 1. CONFIG_SMP=y and > 2. CONFIG_M686=y and > 3. Kernel works on Quark > > Is it correct? Logically yes. It's a very long time since I looked in detail. No harm in checking it out though. I'll compile up the above kernel this evening (GMT) and verify. -- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 28/02/17 15:27, Andy Shevchenko wrote: > On Tue, Feb 28, 2017 at 5:07 PM, Bryan O'Donoghue >wrote: >> On 28/02/17 13:36, Andy Shevchenko wrote: >>> On Tue, Feb 28, 2017 at 3:35 PM, Andy Shevchenko >>> wrote: On Tue, Feb 28, 2017 at 3:25 PM, Ard Biesheuvel wrote: > On 28 February 2017 at 12:29, Matt Fleming > wrote: >> On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: > As I said before, I'd be ok with it if we select it compile time, > i.e., no runtime logic that infers whether we are running on such a > system or not, and no carrying both implementations in all kernels > that have capsule loading built in. Actually it most likely that Quark kernel (kernel compiled to be run on Quark) will be ever used on the rest of (modern) x86 since it's 486+ architecture (kernel has specific option for it, 586TSC). >>> >>> + it's UP only! >>> So, we might just be dependent or chosen by Quark. >>> >> >> Still though the current ia32 kernel runs on Quark and all other ia32 >> systems. > > How come? Quark has a silicon bug (SMP kernel might oops) and it is > not even i586! sorry if this is a bit long in advance... You mean a lock prefixed pagefault. For reference Quark should be considered CONFIG_M586TSC=y (or better) i.e. it's 586 ISA with a TSC added on. I've been meaning to do a write up about this, since I've spent some time with a debugger doing an analysis of the fault. Basically any operation that is lock-prefixed that also page-faults pushes the address of the _previous_ instruction (not the instruction that faulted) onto the PF# stack. Which sucks. Obviously then when you IRET you will execute the previous instruction again - on return to user-space - as opposed to the instruction you faulted on. So it's nothing to do with SMP per se, except that the lock prefix was added to drive the #lock signal on future SMP versions of the part (that never happened). We discussed this @ the time Dave Jones did commit d4e1a0af1d3a88cdfc8c954d3005eb8745ec518d Author: Dave Jones Date: Tue Oct 28 13:57:53 2014 -0400 x86: Don't enable F00F workaround on Intel Quark processors ... and we agreed to have a good look at lock prefixed instructions in the kernel. On !SMP builds there is (or was at the time anyway) alot of LOCK_PREFIX looking like this #ifdef CONFIG_SMP #define LOCK_PREFIX_HERE \ ".pushsection .smp_locks,\"a\"\n" \ ".balign 4\n" \ ".long 671f - .\n" /* offset */ \ ".popsection\n" \ "671:" #define LOCK_PREFIX LOCK_PREFIX_HERE "\n\tlock; " #else /* ! CONFIG_SMP */ #define LOCK_PREFIX_HERE "" #define LOCK_PREFIX "" #endif Which meant that !SMP was safe. It was probably overkill though because kernel code shouldn't PF anyway. User-space is another story. This image faults reliably in rpc-stad and sshd - because like I say - lock-prefixed page faults push the previous (not the current) instruction onto the pagefault stack. https://sourceforge.net/projects/galileodebian/ anyway... !SMP ia32 builds should be fine and we have never actually seen _kernel_ code die on SMP builds ... presumably (demonstrably) because lock prefixed operations in the kernel don't PF#. >> It would be a pity/shame to make this feature dependent on >> compiling a Quark specific kernel, after all its only a header on a >> capsule as opposed to a large hardware-level architectural divergence. >> > >> I'd still like us to try for a low-fat hook that would a big fat ia32 >> kernel just work without having to force a user compile up a >> Quark-specific kernel. > > Can you elaborate how to run i686 kernel (which is default for x86 > 32-bit AFAIK) on Quark? A kernel compiled like this make menuconfig ARCH=i386 make bzImage -j 8 will run just fine on Quark x1000 I do it regularly. CPUID ought to (and does) inform the runtime kernel of what to do re: MSRs etc. We won't execute xmm/mmx, we won't touch 686 specific MSRs etc, etc. -- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 28/02/17 15:27, Andy Shevchenko wrote: > On Tue, Feb 28, 2017 at 5:07 PM, Bryan O'Donoghue > wrote: >> On 28/02/17 13:36, Andy Shevchenko wrote: >>> On Tue, Feb 28, 2017 at 3:35 PM, Andy Shevchenko >>> wrote: On Tue, Feb 28, 2017 at 3:25 PM, Ard Biesheuvel wrote: > On 28 February 2017 at 12:29, Matt Fleming > wrote: >> On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: > As I said before, I'd be ok with it if we select it compile time, > i.e., no runtime logic that infers whether we are running on such a > system or not, and no carrying both implementations in all kernels > that have capsule loading built in. Actually it most likely that Quark kernel (kernel compiled to be run on Quark) will be ever used on the rest of (modern) x86 since it's 486+ architecture (kernel has specific option for it, 586TSC). >>> >>> + it's UP only! >>> So, we might just be dependent or chosen by Quark. >>> >> >> Still though the current ia32 kernel runs on Quark and all other ia32 >> systems. > > How come? Quark has a silicon bug (SMP kernel might oops) and it is > not even i586! sorry if this is a bit long in advance... You mean a lock prefixed pagefault. For reference Quark should be considered CONFIG_M586TSC=y (or better) i.e. it's 586 ISA with a TSC added on. I've been meaning to do a write up about this, since I've spent some time with a debugger doing an analysis of the fault. Basically any operation that is lock-prefixed that also page-faults pushes the address of the _previous_ instruction (not the instruction that faulted) onto the PF# stack. Which sucks. Obviously then when you IRET you will execute the previous instruction again - on return to user-space - as opposed to the instruction you faulted on. So it's nothing to do with SMP per se, except that the lock prefix was added to drive the #lock signal on future SMP versions of the part (that never happened). We discussed this @ the time Dave Jones did commit d4e1a0af1d3a88cdfc8c954d3005eb8745ec518d Author: Dave Jones Date: Tue Oct 28 13:57:53 2014 -0400 x86: Don't enable F00F workaround on Intel Quark processors ... and we agreed to have a good look at lock prefixed instructions in the kernel. On !SMP builds there is (or was at the time anyway) alot of LOCK_PREFIX looking like this #ifdef CONFIG_SMP #define LOCK_PREFIX_HERE \ ".pushsection .smp_locks,\"a\"\n" \ ".balign 4\n" \ ".long 671f - .\n" /* offset */ \ ".popsection\n" \ "671:" #define LOCK_PREFIX LOCK_PREFIX_HERE "\n\tlock; " #else /* ! CONFIG_SMP */ #define LOCK_PREFIX_HERE "" #define LOCK_PREFIX "" #endif Which meant that !SMP was safe. It was probably overkill though because kernel code shouldn't PF anyway. User-space is another story. This image faults reliably in rpc-stad and sshd - because like I say - lock-prefixed page faults push the previous (not the current) instruction onto the pagefault stack. https://sourceforge.net/projects/galileodebian/ anyway... !SMP ia32 builds should be fine and we have never actually seen _kernel_ code die on SMP builds ... presumably (demonstrably) because lock prefixed operations in the kernel don't PF#. >> It would be a pity/shame to make this feature dependent on >> compiling a Quark specific kernel, after all its only a header on a >> capsule as opposed to a large hardware-level architectural divergence. >> > >> I'd still like us to try for a low-fat hook that would a big fat ia32 >> kernel just work without having to force a user compile up a >> Quark-specific kernel. > > Can you elaborate how to run i686 kernel (which is default for x86 > 32-bit AFAIK) on Quark? A kernel compiled like this make menuconfig ARCH=i386 make bzImage -j 8 will run just fine on Quark x1000 I do it regularly. CPUID ought to (and does) inform the runtime kernel of what to do re: MSRs etc. We won't execute xmm/mmx, we won't touch 686 specific MSRs etc, etc. -- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Tue, Feb 28, 2017 at 6:52 PM, Bryan O'Donoghuewrote: > On 28/02/17 15:27, Andy Shevchenko wrote: >> On Tue, Feb 28, 2017 at 5:07 PM, Bryan O'Donoghue >> wrote: >>> On 28/02/17 13:36, Andy Shevchenko wrote: On Tue, Feb 28, 2017 at 3:35 PM, Andy Shevchenko wrote: > On Tue, Feb 28, 2017 at 3:25 PM, Ard Biesheuvel > wrote: >> On 28 February 2017 at 12:29, Matt Fleming >> wrote: >>> On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: > >> As I said before, I'd be ok with it if we select it compile time, >> i.e., no runtime logic that infers whether we are running on such a >> system or not, and no carrying both implementations in all kernels >> that have capsule loading built in. > > Actually it most likely that Quark kernel (kernel compiled to be run > on Quark) will be ever used on the rest of (modern) x86 since it's > 486+ architecture (kernel has specific option for it, 586TSC). + it's UP only! > So, we might just be dependent or chosen by Quark. >>> >>> Still though the current ia32 kernel runs on Quark and all other ia32 >>> systems. >> >> How come? Quark has a silicon bug (SMP kernel might oops) and it is >> not even i586! > > sorry if this is a bit long in advance... > > You mean a lock prefixed pagefault. > > For reference Quark should be considered CONFIG_M586TSC=y (or better) > i.e. it's 586 ISA with a TSC added on. So, if it would be CONFIG_M686=y then? This is default for x86 32-bit kernels. > > I've been meaning to do a write up about this, since I've spent some > time with a debugger doing an analysis of the fault. Basically any > operation that is lock-prefixed that also page-faults pushes the address > of the _previous_ instruction (not the instruction that faulted) onto > the PF# stack. Which sucks. > > Obviously then when you IRET you will execute the previous instruction > again - on return to user-space - as opposed to the instruction you > faulted on. > > So it's nothing to do with SMP per se, except that the lock prefix was > added to drive the #lock signal on future SMP versions of the part (that > never happened). We discussed this @ the time Dave Jones did > > commit d4e1a0af1d3a88cdfc8c954d3005eb8745ec518d > Author: Dave Jones > Date: Tue Oct 28 13:57:53 2014 -0400 > > x86: Don't enable F00F workaround on Intel Quark processors > > ... and we agreed to have a good look at lock prefixed instructions in > the kernel. On !SMP builds there is (or was at the time anyway) alot of > LOCK_PREFIX looking like this > > #ifdef CONFIG_SMP > #define LOCK_PREFIX_HERE \ > ".pushsection .smp_locks,\"a\"\n" \ > ".balign 4\n" \ > ".long 671f - .\n" /* offset */ \ > ".popsection\n" \ > "671:" > > #define LOCK_PREFIX LOCK_PREFIX_HERE "\n\tlock; " > > #else /* ! CONFIG_SMP */ > #define LOCK_PREFIX_HERE "" > #define LOCK_PREFIX "" > #endif > > Which meant that !SMP was safe. It was probably overkill though because > kernel code shouldn't PF anyway. So, would it work if CONFIG_SMP=y ? (This is default for x86 32-bit kernels) > !SMP ia32 builds should be fine and we have never actually seen _kernel_ > code die on SMP builds ... presumably (demonstrably) because lock > prefixed operations in the kernel don't PF#. Which doesn't guarantee that it will not oops at some circumstances. >>> It would be a pity/shame to make this feature dependent on >>> compiling a Quark specific kernel, after all its only a header on a >>> capsule as opposed to a large hardware-level architectural divergence. >>> >> >>> I'd still like us to try for a low-fat hook that would a big fat ia32 >>> kernel just work without having to force a user compile up a >>> Quark-specific kernel. >> >> Can you elaborate how to run i686 kernel (which is default for x86 >> 32-bit AFAIK) on Quark? > > A kernel compiled like this > > make menuconfig ARCH=i386 I hope you care that it is equivalent to make menuconfig ARCH=i686 > make bzImage -j 8 > > will run just fine on Quark x1000 I do it regularly. CPUID ought to (and > does) inform the runtime kernel of what to do re: MSRs etc. > > We won't execute xmm/mmx, we won't touch 686 specific MSRs etc, etc. It is i*6*86 code still. So, summarize, you state that 1. CONFIG_SMP=y and 2. CONFIG_M686=y and 3. Kernel works on Quark Is it correct? If 1 or 2 is not correct it means we can *not* use the same kernel on Quark. Unfortunately. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Tue, Feb 28, 2017 at 6:52 PM, Bryan O'Donoghue wrote: > On 28/02/17 15:27, Andy Shevchenko wrote: >> On Tue, Feb 28, 2017 at 5:07 PM, Bryan O'Donoghue >> wrote: >>> On 28/02/17 13:36, Andy Shevchenko wrote: On Tue, Feb 28, 2017 at 3:35 PM, Andy Shevchenko wrote: > On Tue, Feb 28, 2017 at 3:25 PM, Ard Biesheuvel > wrote: >> On 28 February 2017 at 12:29, Matt Fleming >> wrote: >>> On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: > >> As I said before, I'd be ok with it if we select it compile time, >> i.e., no runtime logic that infers whether we are running on such a >> system or not, and no carrying both implementations in all kernels >> that have capsule loading built in. > > Actually it most likely that Quark kernel (kernel compiled to be run > on Quark) will be ever used on the rest of (modern) x86 since it's > 486+ architecture (kernel has specific option for it, 586TSC). + it's UP only! > So, we might just be dependent or chosen by Quark. >>> >>> Still though the current ia32 kernel runs on Quark and all other ia32 >>> systems. >> >> How come? Quark has a silicon bug (SMP kernel might oops) and it is >> not even i586! > > sorry if this is a bit long in advance... > > You mean a lock prefixed pagefault. > > For reference Quark should be considered CONFIG_M586TSC=y (or better) > i.e. it's 586 ISA with a TSC added on. So, if it would be CONFIG_M686=y then? This is default for x86 32-bit kernels. > > I've been meaning to do a write up about this, since I've spent some > time with a debugger doing an analysis of the fault. Basically any > operation that is lock-prefixed that also page-faults pushes the address > of the _previous_ instruction (not the instruction that faulted) onto > the PF# stack. Which sucks. > > Obviously then when you IRET you will execute the previous instruction > again - on return to user-space - as opposed to the instruction you > faulted on. > > So it's nothing to do with SMP per se, except that the lock prefix was > added to drive the #lock signal on future SMP versions of the part (that > never happened). We discussed this @ the time Dave Jones did > > commit d4e1a0af1d3a88cdfc8c954d3005eb8745ec518d > Author: Dave Jones > Date: Tue Oct 28 13:57:53 2014 -0400 > > x86: Don't enable F00F workaround on Intel Quark processors > > ... and we agreed to have a good look at lock prefixed instructions in > the kernel. On !SMP builds there is (or was at the time anyway) alot of > LOCK_PREFIX looking like this > > #ifdef CONFIG_SMP > #define LOCK_PREFIX_HERE \ > ".pushsection .smp_locks,\"a\"\n" \ > ".balign 4\n" \ > ".long 671f - .\n" /* offset */ \ > ".popsection\n" \ > "671:" > > #define LOCK_PREFIX LOCK_PREFIX_HERE "\n\tlock; " > > #else /* ! CONFIG_SMP */ > #define LOCK_PREFIX_HERE "" > #define LOCK_PREFIX "" > #endif > > Which meant that !SMP was safe. It was probably overkill though because > kernel code shouldn't PF anyway. So, would it work if CONFIG_SMP=y ? (This is default for x86 32-bit kernels) > !SMP ia32 builds should be fine and we have never actually seen _kernel_ > code die on SMP builds ... presumably (demonstrably) because lock > prefixed operations in the kernel don't PF#. Which doesn't guarantee that it will not oops at some circumstances. >>> It would be a pity/shame to make this feature dependent on >>> compiling a Quark specific kernel, after all its only a header on a >>> capsule as opposed to a large hardware-level architectural divergence. >>> >> >>> I'd still like us to try for a low-fat hook that would a big fat ia32 >>> kernel just work without having to force a user compile up a >>> Quark-specific kernel. >> >> Can you elaborate how to run i686 kernel (which is default for x86 >> 32-bit AFAIK) on Quark? > > A kernel compiled like this > > make menuconfig ARCH=i386 I hope you care that it is equivalent to make menuconfig ARCH=i686 > make bzImage -j 8 > > will run just fine on Quark x1000 I do it regularly. CPUID ought to (and > does) inform the runtime kernel of what to do re: MSRs etc. > > We won't execute xmm/mmx, we won't touch 686 specific MSRs etc, etc. It is i*6*86 code still. So, summarize, you state that 1. CONFIG_SMP=y and 2. CONFIG_M686=y and 3. Kernel works on Quark Is it correct? If 1 or 2 is not correct it means we can *not* use the same kernel on Quark. Unfortunately. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Tue, Feb 28, 2017 at 5:07 PM, Bryan O'Donoghuewrote: > On 28/02/17 13:36, Andy Shevchenko wrote: >> On Tue, Feb 28, 2017 at 3:35 PM, Andy Shevchenko >> wrote: >>> On Tue, Feb 28, 2017 at 3:25 PM, Ard Biesheuvel >>> wrote: On 28 February 2017 at 12:29, Matt Fleming wrote: > On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: >>> As I said before, I'd be ok with it if we select it compile time, i.e., no runtime logic that infers whether we are running on such a system or not, and no carrying both implementations in all kernels that have capsule loading built in. >>> >>> Actually it most likely that Quark kernel (kernel compiled to be run >>> on Quark) will be ever used on the rest of (modern) x86 since it's >>> 486+ architecture (kernel has specific option for it, 586TSC). >> >> + it's UP only! >> >>> So, we might just be dependent or chosen by Quark. >> > > Still though the current ia32 kernel runs on Quark and all other ia32 > systems. How come? Quark has a silicon bug (SMP kernel might oops) and it is not even i586! > It would be a pity/shame to make this feature dependent on > compiling a Quark specific kernel, after all its only a header on a > capsule as opposed to a large hardware-level architectural divergence. > > I'd still like us to try for a low-fat hook that would a big fat ia32 > kernel just work without having to force a user compile up a > Quark-specific kernel. Can you elaborate how to run i686 kernel (which is default for x86 32-bit AFAIK) on Quark? -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Tue, Feb 28, 2017 at 5:07 PM, Bryan O'Donoghue wrote: > On 28/02/17 13:36, Andy Shevchenko wrote: >> On Tue, Feb 28, 2017 at 3:35 PM, Andy Shevchenko >> wrote: >>> On Tue, Feb 28, 2017 at 3:25 PM, Ard Biesheuvel >>> wrote: On 28 February 2017 at 12:29, Matt Fleming wrote: > On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: >>> As I said before, I'd be ok with it if we select it compile time, i.e., no runtime logic that infers whether we are running on such a system or not, and no carrying both implementations in all kernels that have capsule loading built in. >>> >>> Actually it most likely that Quark kernel (kernel compiled to be run >>> on Quark) will be ever used on the rest of (modern) x86 since it's >>> 486+ architecture (kernel has specific option for it, 586TSC). >> >> + it's UP only! >> >>> So, we might just be dependent or chosen by Quark. >> > > Still though the current ia32 kernel runs on Quark and all other ia32 > systems. How come? Quark has a silicon bug (SMP kernel might oops) and it is not even i586! > It would be a pity/shame to make this feature dependent on > compiling a Quark specific kernel, after all its only a header on a > capsule as opposed to a large hardware-level architectural divergence. > > I'd still like us to try for a low-fat hook that would a big fat ia32 > kernel just work without having to force a user compile up a > Quark-specific kernel. Can you elaborate how to run i686 kernel (which is default for x86 32-bit AFAIK) on Quark? -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 28/02/17 13:36, Andy Shevchenko wrote: > On Tue, Feb 28, 2017 at 3:35 PM, Andy Shevchenko >wrote: >> On Tue, Feb 28, 2017 at 3:25 PM, Ard Biesheuvel >> wrote: >>> On 28 February 2017 at 12:29, Matt Fleming wrote: On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: >> >>> As I said before, I'd be ok with it if we select it compile time, >>> i.e., no runtime logic that infers whether we are running on such a >>> system or not, and no carrying both implementations in all kernels >>> that have capsule loading built in. >> >> Actually it most likely that Quark kernel (kernel compiled to be run >> on Quark) will be ever used on the rest of (modern) x86 since it's >> 486+ architecture (kernel has specific option for it, 586TSC). > > + it's UP only! > >> So, we might just be dependent or chosen by Quark. > Still though the current ia32 kernel runs on Quark and all other ia32 systems. It would be a pity/shame to make this feature dependent on compiling a Quark specific kernel, after all its only a header on a capsule as opposed to a large hardware-level architectural divergence. I'd still like us to try for a low-fat hook that would a big fat ia32 kernel just work without having to force a user compile up a Quark-specific kernel. -- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 28/02/17 13:36, Andy Shevchenko wrote: > On Tue, Feb 28, 2017 at 3:35 PM, Andy Shevchenko > wrote: >> On Tue, Feb 28, 2017 at 3:25 PM, Ard Biesheuvel >> wrote: >>> On 28 February 2017 at 12:29, Matt Fleming wrote: On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: >> >>> As I said before, I'd be ok with it if we select it compile time, >>> i.e., no runtime logic that infers whether we are running on such a >>> system or not, and no carrying both implementations in all kernels >>> that have capsule loading built in. >> >> Actually it most likely that Quark kernel (kernel compiled to be run >> on Quark) will be ever used on the rest of (modern) x86 since it's >> 486+ architecture (kernel has specific option for it, 586TSC). > > + it's UP only! > >> So, we might just be dependent or chosen by Quark. > Still though the current ia32 kernel runs on Quark and all other ia32 systems. It would be a pity/shame to make this feature dependent on compiling a Quark specific kernel, after all its only a header on a capsule as opposed to a large hardware-level architectural divergence. I'd still like us to try for a low-fat hook that would a big fat ia32 kernel just work without having to force a user compile up a Quark-specific kernel. -- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 28/02/17 15:07, Bryan O'Donoghue wrote: > a big fat ia32 *allow a full fat.. -- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 28/02/17 15:07, Bryan O'Donoghue wrote: > a big fat ia32 *allow a full fat.. -- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Tue, Feb 28, 2017 at 3:35 PM, Andy Shevchenkowrote: > On Tue, Feb 28, 2017 at 3:25 PM, Ard Biesheuvel > wrote: >> On 28 February 2017 at 12:29, Matt Fleming wrote: >>> On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: > >> As I said before, I'd be ok with it if we select it compile time, >> i.e., no runtime logic that infers whether we are running on such a >> system or not, and no carrying both implementations in all kernels >> that have capsule loading built in. > > Actually it most likely that Quark kernel (kernel compiled to be run > on Quark) will be ever used on the rest of (modern) x86 since it's > 486+ architecture (kernel has specific option for it, 586TSC). + it's UP only! > So, we might just be dependent or chosen by Quark. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Tue, Feb 28, 2017 at 3:35 PM, Andy Shevchenko wrote: > On Tue, Feb 28, 2017 at 3:25 PM, Ard Biesheuvel > wrote: >> On 28 February 2017 at 12:29, Matt Fleming wrote: >>> On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: > >> As I said before, I'd be ok with it if we select it compile time, >> i.e., no runtime logic that infers whether we are running on such a >> system or not, and no carrying both implementations in all kernels >> that have capsule loading built in. > > Actually it most likely that Quark kernel (kernel compiled to be run > on Quark) will be ever used on the rest of (modern) x86 since it's > 486+ architecture (kernel has specific option for it, 586TSC). + it's UP only! > So, we might just be dependent or chosen by Quark. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Tue, Feb 28, 2017 at 3:25 PM, Ard Biesheuvelwrote: > On 28 February 2017 at 12:29, Matt Fleming wrote: >> On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: > As I said before, I'd be ok with it if we select it compile time, > i.e., no runtime logic that infers whether we are running on such a > system or not, and no carrying both implementations in all kernels > that have capsule loading built in. Actually it most likely that Quark kernel (kernel compiled to be run on Quark) will be ever used on the rest of (modern) x86 since it's 486+ architecture (kernel has specific option for it, 586TSC). So, we might just be dependent or chosen by Quark. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Tue, Feb 28, 2017 at 3:25 PM, Ard Biesheuvel wrote: > On 28 February 2017 at 12:29, Matt Fleming wrote: >> On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: > As I said before, I'd be ok with it if we select it compile time, > i.e., no runtime logic that infers whether we are running on such a > system or not, and no carrying both implementations in all kernels > that have capsule loading built in. Actually it most likely that Quark kernel (kernel compiled to be run on Quark) will be ever used on the rest of (modern) x86 since it's 486+ architecture (kernel has specific option for it, 586TSC). So, we might just be dependent or chosen by Quark. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 28 February 2017 at 12:29, Matt Flemingwrote: > On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: >> >> From you POV, does this exclude upstream quirk support for already >> shipped devices? > > It would need to be an extremely small, well-contained change, that > had no chance of disrupting other users of the capsule interface and > where I had a good feeling that supporting it wouldn't turn into a > maintenance nightmare (mountains of DMI strings or new platforms > coming to market that used it). > > That's a tall order, and I'm pretty skeptical. Still, I'll never say > never. Plus Ard would need convincing to give his ACK too. > > P.S. Has anyone actually investigated what would be required to fix > the firmware to be able to extract the CSH if it was contained inside > a capsule? As I said before, I'd be ok with it if we select it compile time, i.e., no runtime logic that infers whether we are running on such a system or not, and no carrying both implementations in all kernels that have capsule loading built in. But I do realise that it increases the validation space for Matt, given that he does the testing on the x86 side. For the ARM side of things, the Kconfig option would simply not be settable. So I am going to let Matt have the final word on this.
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 28 February 2017 at 12:29, Matt Fleming wrote: > On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: >> >> From you POV, does this exclude upstream quirk support for already >> shipped devices? > > It would need to be an extremely small, well-contained change, that > had no chance of disrupting other users of the capsule interface and > where I had a good feeling that supporting it wouldn't turn into a > maintenance nightmare (mountains of DMI strings or new platforms > coming to market that used it). > > That's a tall order, and I'm pretty skeptical. Still, I'll never say > never. Plus Ard would need convincing to give his ACK too. > > P.S. Has anyone actually investigated what would be required to fix > the firmware to be able to extract the CSH if it was contained inside > a capsule? As I said before, I'd be ok with it if we select it compile time, i.e., no runtime logic that infers whether we are running on such a system or not, and no carrying both implementations in all kernels that have capsule loading built in. But I do realise that it increases the validation space for Matt, given that he does the testing on the x86 side. For the ARM side of things, the Kconfig option would simply not be settable. So I am going to let Matt have the final word on this.
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: > > From you POV, does this exclude upstream quirk support for already > shipped devices? It would need to be an extremely small, well-contained change, that had no chance of disrupting other users of the capsule interface and where I had a good feeling that supporting it wouldn't turn into a maintenance nightmare (mountains of DMI strings or new platforms coming to market that used it). That's a tall order, and I'm pretty skeptical. Still, I'll never say never. Plus Ard would need convincing to give his ACK too. P.S. Has anyone actually investigated what would be required to fix the firmware to be able to extract the CSH if it was contained inside a capsule?
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Tue, 28 Feb, at 01:20:25PM, Jan Kiszka wrote: > > From you POV, does this exclude upstream quirk support for already > shipped devices? It would need to be an extremely small, well-contained change, that had no chance of disrupting other users of the capsule interface and where I had a good feeling that supporting it wouldn't turn into a maintenance nightmare (mountains of DMI strings or new platforms coming to market that used it). That's a tall order, and I'm pretty skeptical. Still, I'll never say never. Plus Ard would need convincing to give his ACK too. P.S. Has anyone actually investigated what would be required to fix the firmware to be able to extract the CSH if it was contained inside a capsule?
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-28 13:12, Matt Fleming wrote: > On Fri, 17 Feb, at 10:24:41AM, Jan Kiszka wrote: >> >> I just can re-express my frustration that this essential step hasn't >> been started years ago by whoever designed the extension. Then I bet >> there would have been constructive feedback on the interface BEFORE its >> ugliness spread to broader use. >> >> Or is there a technical need, in general or on Quark, to have the >> signature header right before the standard capsule *for the handover* to >> the firmware? I mean, I would naively put it into another capsule and >> prepend that to the core so that the existing UEFI API can palate it >> transparently and cleanly. > > I'm fairly sure this was my first thought when we discussed this > originally, some years ago now. > > The whole CSH concept is, frankly, stupid. It makes a mockery of > everything the capsule interface was designed to be. > > I have long been holding out in hope that someone would patch the > firmware to work around this CSH requirement, something along the > lines of the double wrapping Jan mentions above. It's not like the > Quark is the only platform that wants to verify capsules. > > But to my knowledge, that hasn't happened. > > Nevertheless my answer is still the same - someone needs to go and > update the Quark firmware source to work with the generic capsule > mechanism. > >From you POV, does this exclude upstream quirk support for already shipped devices? Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-28 13:12, Matt Fleming wrote: > On Fri, 17 Feb, at 10:24:41AM, Jan Kiszka wrote: >> >> I just can re-express my frustration that this essential step hasn't >> been started years ago by whoever designed the extension. Then I bet >> there would have been constructive feedback on the interface BEFORE its >> ugliness spread to broader use. >> >> Or is there a technical need, in general or on Quark, to have the >> signature header right before the standard capsule *for the handover* to >> the firmware? I mean, I would naively put it into another capsule and >> prepend that to the core so that the existing UEFI API can palate it >> transparently and cleanly. > > I'm fairly sure this was my first thought when we discussed this > originally, some years ago now. > > The whole CSH concept is, frankly, stupid. It makes a mockery of > everything the capsule interface was designed to be. > > I have long been holding out in hope that someone would patch the > firmware to work around this CSH requirement, something along the > lines of the double wrapping Jan mentions above. It's not like the > Quark is the only platform that wants to verify capsules. > > But to my knowledge, that hasn't happened. > > Nevertheless my answer is still the same - someone needs to go and > update the Quark firmware source to work with the generic capsule > mechanism. > >From you POV, does this exclude upstream quirk support for already shipped devices? Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Fri, 17 Feb, at 10:24:41AM, Jan Kiszka wrote: > > I just can re-express my frustration that this essential step hasn't > been started years ago by whoever designed the extension. Then I bet > there would have been constructive feedback on the interface BEFORE its > ugliness spread to broader use. > > Or is there a technical need, in general or on Quark, to have the > signature header right before the standard capsule *for the handover* to > the firmware? I mean, I would naively put it into another capsule and > prepend that to the core so that the existing UEFI API can palate it > transparently and cleanly. I'm fairly sure this was my first thought when we discussed this originally, some years ago now. The whole CSH concept is, frankly, stupid. It makes a mockery of everything the capsule interface was designed to be. I have long been holding out in hope that someone would patch the firmware to work around this CSH requirement, something along the lines of the double wrapping Jan mentions above. It's not like the Quark is the only platform that wants to verify capsules. But to my knowledge, that hasn't happened. Nevertheless my answer is still the same - someone needs to go and update the Quark firmware source to work with the generic capsule mechanism.
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Fri, 17 Feb, at 10:24:41AM, Jan Kiszka wrote: > > I just can re-express my frustration that this essential step hasn't > been started years ago by whoever designed the extension. Then I bet > there would have been constructive feedback on the interface BEFORE its > ugliness spread to broader use. > > Or is there a technical need, in general or on Quark, to have the > signature header right before the standard capsule *for the handover* to > the firmware? I mean, I would naively put it into another capsule and > prepend that to the core so that the existing UEFI API can palate it > transparently and cleanly. I'm fairly sure this was my first thought when we discussed this originally, some years ago now. The whole CSH concept is, frankly, stupid. It makes a mockery of everything the capsule interface was designed to be. I have long been holding out in hope that someone would patch the firmware to work around this CSH requirement, something along the lines of the double wrapping Jan mentions above. It's not like the Quark is the only platform that wants to verify capsules. But to my knowledge, that hasn't happened. Nevertheless my answer is still the same - someone needs to go and update the Quark firmware source to work with the generic capsule mechanism.
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
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
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
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
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
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); +} 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. --- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
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); +} 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. --- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-18 13:48, Ard Biesheuvel wrote: > On 16 February 2017 at 07:29, Jan Kiszka <jan.kis...@siemens.com> wrote: >> On 2017-02-16 04:00, Kweh, Hock Leong wrote: >>>> -Original Message- >>>> From: Jan Kiszka [mailto:jan.kis...@siemens.com] >>>> Sent: Thursday, February 16, 2017 3:00 AM >>>> To: Andy Shevchenko <andy.shevche...@gmail.com> >>>> Cc: Matt Fleming <m...@codeblueprint.co.uk>; Ard Biesheuvel >>>> <ard.biesheu...@linaro.org>; linux-...@vger.kernel.org; Linux Kernel >>>> Mailing >>>> List <linux-kernel@vger.kernel.org>; Borislav Petkov <b...@alien8.de>; >>>> Kweh, >>>> Hock Leong <hock.leong.k...@intel.com>; Bryan O'Donoghue >>>> <pure.lo...@nexus-software.ie> >>>> Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed >>>> Quark >>>> images >>>> >>>> On 2017-02-15 19:50, Jan Kiszka wrote: >>>>> On 2017-02-15 19:46, Andy Shevchenko wrote: >>>>>> On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka <jan.kis...@siemens.com> >>>> wrote: >>>>>>> See patch 2 for the background. >>>>>>> >>>>>>> Series has been tested on the Galileo Gen2, to exclude regressions, >>>>>>> with a firmware.cap without security header and the SIMATIC IOT2040 >>>>>>> which requires the header because of its mandatory secure boot. >>>>>> >>>>>> Briefly looking to the code it looks like a real hack. >>>>>> Sorry, but it would be carefully (re-)designed. >>>>> >>>>> The interface that the firmware provides us? That should have been >>>>> done differently, I agree, but I'm not too much into those firmware >>>>> details, specifically when it comes to signatures. >>>>> >>>>> The Linux code was designed around that suboptimal situation. If there >>>>> are better ideas, I'm all ears. >>>>> >>>> >>>> Expanding CC's as requested by Andy. >>>> >>>> Jan >>>> >>> >>> Hi Jan, >>> >>> While I upstreaming the capsule loader patches, I did work with maintainer >>> Matt and look into this security header created for Quark. Eventually both >>> of us agreed that this will not be upstream to mainline as it is really a >>> Quark >>> specific implementation. >> >> This is ... [swallowing down a lengthy rant about Quark upstreaming] >> unfortunate given that Intel hands out firmware and BSPs to their >> customers without further explanations on this "minor detail". >> >> I have no idea what other integrators of the X102x did with that, but my >> customer has now thousands and thousands of devices in the field with >> firmware that works exactly this way. Only for that feature, we will now >> have to provide a non-upstream kernel in order to keep the installed >> devices updatable. Or create and maintain a different mechanism. Beautiful. >> > > OK, so you shipped thousands and thousands of devices with mainline > kernels and never tested the capsule update feature, which now turns > out to require modifications to support the non-UEFI compliant > firmware on these devices. We are shipping an open platform. The users can download a reference image with Yocto-Linux that comes with a Yocto kernel plus some enabling patches. One of them is currently a forward-port of the original Intel capsule loader driver that does a similar thing like these patches and therefore works fine (of course firmware update has been tested before the release). But in order to overcome the dependencies on Yocto kernels as well our own patch queue, we are in the process of upstreaming necessary changes (and upstream cleanups as well, some are already merged). In the end, our users should have the possibility to chose mainline or Yocto or some other kernel flavour without having the need for additional BSP patches. That will ensure long-term support for the hardware, software-wise. Users already asked us if they will eventually be stuck with a patch queue and, thus, an outdated kernel like it is a sad standard in this domain. But that is not our plan. Yes, in an ideal world, this discussion had happened earlier and prevented at least our deployment of the non-standard firmware. But the world is not always working ideally. > > I'm sorry, but that puts it firmly in the 'not our problem' category, > simply because I refuse to believe that you would seriously
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-18 13:48, Ard Biesheuvel wrote: > On 16 February 2017 at 07:29, Jan Kiszka wrote: >> On 2017-02-16 04:00, Kweh, Hock Leong wrote: >>>> -Original Message- >>>> From: Jan Kiszka [mailto:jan.kis...@siemens.com] >>>> Sent: Thursday, February 16, 2017 3:00 AM >>>> To: Andy Shevchenko >>>> Cc: Matt Fleming ; Ard Biesheuvel >>>> ; linux-...@vger.kernel.org; Linux Kernel >>>> Mailing >>>> List ; Borislav Petkov ; >>>> Kweh, >>>> Hock Leong ; Bryan O'Donoghue >>>> >>>> Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed >>>> Quark >>>> images >>>> >>>> On 2017-02-15 19:50, Jan Kiszka wrote: >>>>> On 2017-02-15 19:46, Andy Shevchenko wrote: >>>>>> On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka >>>> wrote: >>>>>>> See patch 2 for the background. >>>>>>> >>>>>>> Series has been tested on the Galileo Gen2, to exclude regressions, >>>>>>> with a firmware.cap without security header and the SIMATIC IOT2040 >>>>>>> which requires the header because of its mandatory secure boot. >>>>>> >>>>>> Briefly looking to the code it looks like a real hack. >>>>>> Sorry, but it would be carefully (re-)designed. >>>>> >>>>> The interface that the firmware provides us? That should have been >>>>> done differently, I agree, but I'm not too much into those firmware >>>>> details, specifically when it comes to signatures. >>>>> >>>>> The Linux code was designed around that suboptimal situation. If there >>>>> are better ideas, I'm all ears. >>>>> >>>> >>>> Expanding CC's as requested by Andy. >>>> >>>> Jan >>>> >>> >>> Hi Jan, >>> >>> While I upstreaming the capsule loader patches, I did work with maintainer >>> Matt and look into this security header created for Quark. Eventually both >>> of us agreed that this will not be upstream to mainline as it is really a >>> Quark >>> specific implementation. >> >> This is ... [swallowing down a lengthy rant about Quark upstreaming] >> unfortunate given that Intel hands out firmware and BSPs to their >> customers without further explanations on this "minor detail". >> >> I have no idea what other integrators of the X102x did with that, but my >> customer has now thousands and thousands of devices in the field with >> firmware that works exactly this way. Only for that feature, we will now >> have to provide a non-upstream kernel in order to keep the installed >> devices updatable. Or create and maintain a different mechanism. Beautiful. >> > > OK, so you shipped thousands and thousands of devices with mainline > kernels and never tested the capsule update feature, which now turns > out to require modifications to support the non-UEFI compliant > firmware on these devices. We are shipping an open platform. The users can download a reference image with Yocto-Linux that comes with a Yocto kernel plus some enabling patches. One of them is currently a forward-port of the original Intel capsule loader driver that does a similar thing like these patches and therefore works fine (of course firmware update has been tested before the release). But in order to overcome the dependencies on Yocto kernels as well our own patch queue, we are in the process of upstreaming necessary changes (and upstream cleanups as well, some are already merged). In the end, our users should have the possibility to chose mainline or Yocto or some other kernel flavour without having the need for additional BSP patches. That will ensure long-term support for the hardware, software-wise. Users already asked us if they will eventually be stuck with a patch queue and, thus, an outdated kernel like it is a sad standard in this domain. But that is not our plan. Yes, in an ideal world, this discussion had happened earlier and prevented at least our deployment of the non-standard firmware. But the world is not always working ideally. > > I'm sorry, but that puts it firmly in the 'not our problem' category, > simply because I refuse to believe that you would seriously consider > performing this kind of firmware update on that many devices in the > field if you never tested it in development. I would recommend reading my email completely (see below) to understand who I was targeting. > > So while I fully agree that &g
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 16 February 2017 at 07:29, Jan Kiszka <jan.kis...@siemens.com> wrote: > On 2017-02-16 04:00, Kweh, Hock Leong wrote: >>> -Original Message- >>> From: Jan Kiszka [mailto:jan.kis...@siemens.com] >>> Sent: Thursday, February 16, 2017 3:00 AM >>> To: Andy Shevchenko <andy.shevche...@gmail.com> >>> Cc: Matt Fleming <m...@codeblueprint.co.uk>; Ard Biesheuvel >>> <ard.biesheu...@linaro.org>; linux-...@vger.kernel.org; Linux Kernel Mailing >>> List <linux-kernel@vger.kernel.org>; Borislav Petkov <b...@alien8.de>; Kweh, >>> Hock Leong <hock.leong.k...@intel.com>; Bryan O'Donoghue >>> <pure.lo...@nexus-software.ie> >>> Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark >>> images >>> >>> On 2017-02-15 19:50, Jan Kiszka wrote: >>>> On 2017-02-15 19:46, Andy Shevchenko wrote: >>>>> On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka <jan.kis...@siemens.com> >>> wrote: >>>>>> See patch 2 for the background. >>>>>> >>>>>> Series has been tested on the Galileo Gen2, to exclude regressions, >>>>>> with a firmware.cap without security header and the SIMATIC IOT2040 >>>>>> which requires the header because of its mandatory secure boot. >>>>> >>>>> Briefly looking to the code it looks like a real hack. >>>>> Sorry, but it would be carefully (re-)designed. >>>> >>>> The interface that the firmware provides us? That should have been >>>> done differently, I agree, but I'm not too much into those firmware >>>> details, specifically when it comes to signatures. >>>> >>>> The Linux code was designed around that suboptimal situation. If there >>>> are better ideas, I'm all ears. >>>> >>> >>> Expanding CC's as requested by Andy. >>> >>> Jan >>> >> >> Hi Jan, >> >> While I upstreaming the capsule loader patches, I did work with maintainer >> Matt and look into this security header created for Quark. Eventually both >> of us agreed that this will not be upstream to mainline as it is really a >> Quark >> specific implementation. > > This is ... [swallowing down a lengthy rant about Quark upstreaming] > unfortunate given that Intel hands out firmware and BSPs to their > customers without further explanations on this "minor detail". > > I have no idea what other integrators of the X102x did with that, but my > customer has now thousands and thousands of devices in the field with > firmware that works exactly this way. Only for that feature, we will now > have to provide a non-upstream kernel in order to keep the installed > devices updatable. Or create and maintain a different mechanism. Beautiful. > OK, so you shipped thousands and thousands of devices with mainline kernels and never tested the capsule update feature, which now turns out to require modifications to support the non-UEFI compliant firmware on these devices. I'm sorry, but that puts it firmly in the 'not our problem' category, simply because I refuse to believe that you would seriously consider performing this kind of firmware update on that many devices in the field if you never tested it in development. So while I fully agree that a) it is quite unfortunate that Intel, which has such a dominant presence in all aspects of UEFI and PI standardization, ships a non-compliant BSP, and b) it is useful to be able to sign capsules, I think we should push back on random, unstandardized signature headers. The argument that Quark is the only working implementation of capsule updates, and so we should support it, does not hold. First of all, arm64 servers are shipping with working capsule update based on the current kernel implementation, but what is currently shipping is not really the point for mainline imo, but what is intended to be shipped with the next kernel release. 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. Thanks, Ard. >> >> The proper implementation may require to work with UEFI community >> to expand its capsule spec to support signed binary. >> > > Are you working on this? How is this solved on other platforms that > require signatures? No one tried that yet? In any case, this sounds like > a lengthy, way too late considered process that will not solve our issue > in the foreseeable future. > > Don't get me wrong, I'm not intending to push this into the kernel > because "What the heck?!" was my first reaction as well once I found out > how this update interface is actually working. But maybe you can bring > this topic up on your side as well so that we come to an upstreamable > solution in all affected projects. > > Thanks, > Jan > > PS: @Daniel, another example for your presentation. ;) > > -- > Siemens AG, Corporate Technology, CT RDA ITP SES-DE > Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 16 February 2017 at 07:29, Jan Kiszka wrote: > On 2017-02-16 04:00, Kweh, Hock Leong wrote: >>> -Original Message- >>> From: Jan Kiszka [mailto:jan.kis...@siemens.com] >>> Sent: Thursday, February 16, 2017 3:00 AM >>> To: Andy Shevchenko >>> Cc: Matt Fleming ; Ard Biesheuvel >>> ; linux-...@vger.kernel.org; Linux Kernel Mailing >>> List ; Borislav Petkov ; Kweh, >>> Hock Leong ; Bryan O'Donoghue >>> >>> Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark >>> images >>> >>> On 2017-02-15 19:50, Jan Kiszka wrote: >>>> On 2017-02-15 19:46, Andy Shevchenko wrote: >>>>> On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka >>> wrote: >>>>>> See patch 2 for the background. >>>>>> >>>>>> Series has been tested on the Galileo Gen2, to exclude regressions, >>>>>> with a firmware.cap without security header and the SIMATIC IOT2040 >>>>>> which requires the header because of its mandatory secure boot. >>>>> >>>>> Briefly looking to the code it looks like a real hack. >>>>> Sorry, but it would be carefully (re-)designed. >>>> >>>> The interface that the firmware provides us? That should have been >>>> done differently, I agree, but I'm not too much into those firmware >>>> details, specifically when it comes to signatures. >>>> >>>> The Linux code was designed around that suboptimal situation. If there >>>> are better ideas, I'm all ears. >>>> >>> >>> Expanding CC's as requested by Andy. >>> >>> Jan >>> >> >> Hi Jan, >> >> While I upstreaming the capsule loader patches, I did work with maintainer >> Matt and look into this security header created for Quark. Eventually both >> of us agreed that this will not be upstream to mainline as it is really a >> Quark >> specific implementation. > > This is ... [swallowing down a lengthy rant about Quark upstreaming] > unfortunate given that Intel hands out firmware and BSPs to their > customers without further explanations on this "minor detail". > > I have no idea what other integrators of the X102x did with that, but my > customer has now thousands and thousands of devices in the field with > firmware that works exactly this way. Only for that feature, we will now > have to provide a non-upstream kernel in order to keep the installed > devices updatable. Or create and maintain a different mechanism. Beautiful. > OK, so you shipped thousands and thousands of devices with mainline kernels and never tested the capsule update feature, which now turns out to require modifications to support the non-UEFI compliant firmware on these devices. I'm sorry, but that puts it firmly in the 'not our problem' category, simply because I refuse to believe that you would seriously consider performing this kind of firmware update on that many devices in the field if you never tested it in development. So while I fully agree that a) it is quite unfortunate that Intel, which has such a dominant presence in all aspects of UEFI and PI standardization, ships a non-compliant BSP, and b) it is useful to be able to sign capsules, I think we should push back on random, unstandardized signature headers. The argument that Quark is the only working implementation of capsule updates, and so we should support it, does not hold. First of all, arm64 servers are shipping with working capsule update based on the current kernel implementation, but what is currently shipping is not really the point for mainline imo, but what is intended to be shipped with the next kernel release. 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. Thanks, Ard. >> >> The proper implementation may require to work with UEFI community >> to expand its capsule spec to support signed binary. >> > > Are you working on this? How is this solved on other platforms that > require signatures? No one tried that yet? In any case, this sounds like > a lengthy, way too late considered process that will not solve our issue > in the foreseeable future. > > Don't get me wrong, I'm not intending to push this into the kernel > because "What the heck?!" was my first reaction as well once I found out > how this update interface is actually working. But maybe you can bring > this topic up on your side as well so that we come to an upstreamable > solution in all affected projects. > > Thanks, > Jan > > PS: @Daniel, another example for your presentation. ;) > > -- > Siemens AG, Corporate Technology, CT RDA ITP SES-DE > Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 17/02/17 10:14, Jan Kiszka wrote: > On 2017-02-17 10:51, Bryan O'Donoghue wrote: >> On 17/02/17 08:23, Kweh, Hock Leong wrote: >>> And to have UEFI expand >>> it capsule support and take in signed binary would be a more secured way. >>> So, influencing UEFI community to have such support would be the right >>> move throughout the discussion. That is my summary. >> >> CSH stands for "Clanton Secure Header" - Clanton being the internal >> code-name for Quark X1000 prior to release. >> >> There is no chance the UEFI standard (which can be used on ARM and >> potentially other architectures) will accept a SoC specific >> route-of-trust prepended header. >> >> Sure some kind of binary signed headers might become part of the >> standard eventually but, definitely _not_ a CSH. >> >> The fact is CSH exists in the real-world and a UEFI firmware supports >> accepting the CSH/UEFI-capsule pair for updating itself. >> >> I think a far more practical solution is to accommodate the defacto >> implementation (the only ? current implementation). To me it defies >> reason to have Quark X1000 be the only system (that I know of) capable >> of doing a capsule update - have capsule code in the kernel - but _not_ >> support the header prepended to that capsule that the Quark >> firmware/bootrom require. >> >> Right now the capsule code is dead code on Quark x1000. Let's do the >> right thing and make it usable. I fully support having a >> separate/parallel conversation with the UEFI body but, I'd be amazed if >> the "Clanton Secure Header" made it into the standard... >> > > To be precise, CSH is only required on X102x. The X100x SoCs, those are > also found on the Galileo Gen2 maker board, do not support secure boot > and do not use the header. The CSH is supported on the non-secure SoCs - the BSP on Gen1 certainly did. https://downloadcenter.intel.com/download/23197/Intel-Quark-SoC-X1000-Board-Support-Package-BSP- - > QuarkPlatformPkg/Platform/Dxe/PlatformInit/DxeCapsuleSecurity.c CreateCapsuleBufferForWriting() Looks like it will tolerate a lack of CSH but, obviously that's no solution for the secure boot parts. > IIRC, there used to be an eval system with > the X1020 as well, but I think it's no longer available. > > Interestingly, the capsule file found in Intel's Galileo firmware update > package [1] contains the CSH header. But I only succeeded flashing it on > a Gen2 by removing the header first. Hmm - the out of the box firmware will accept capsules from the website. Gen1 and Gen2 should be the same in that respect - if you use the BSP kernel. You should validate the out-of-the-box capsule update - with the kernel on SPI flash works with the CSH in place. Next step then is to get it working on tip-of-tree. I have one Gen1 left (which I won't be experimenting with) - and I think one Gen2 (which I don't especially mind blowing up). Let's take the time to validate (or repudiate) 1. Out of the box BSP works with CSH capsules in place 2. New tip-of-tree addition supports CSH capsules and review. Stripping the CSH shouldn't be necessary (and breaks the secure boot parts anyway). -- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 17/02/17 10:14, Jan Kiszka wrote: > On 2017-02-17 10:51, Bryan O'Donoghue wrote: >> On 17/02/17 08:23, Kweh, Hock Leong wrote: >>> And to have UEFI expand >>> it capsule support and take in signed binary would be a more secured way. >>> So, influencing UEFI community to have such support would be the right >>> move throughout the discussion. That is my summary. >> >> CSH stands for "Clanton Secure Header" - Clanton being the internal >> code-name for Quark X1000 prior to release. >> >> There is no chance the UEFI standard (which can be used on ARM and >> potentially other architectures) will accept a SoC specific >> route-of-trust prepended header. >> >> Sure some kind of binary signed headers might become part of the >> standard eventually but, definitely _not_ a CSH. >> >> The fact is CSH exists in the real-world and a UEFI firmware supports >> accepting the CSH/UEFI-capsule pair for updating itself. >> >> I think a far more practical solution is to accommodate the defacto >> implementation (the only ? current implementation). To me it defies >> reason to have Quark X1000 be the only system (that I know of) capable >> of doing a capsule update - have capsule code in the kernel - but _not_ >> support the header prepended to that capsule that the Quark >> firmware/bootrom require. >> >> Right now the capsule code is dead code on Quark x1000. Let's do the >> right thing and make it usable. I fully support having a >> separate/parallel conversation with the UEFI body but, I'd be amazed if >> the "Clanton Secure Header" made it into the standard... >> > > To be precise, CSH is only required on X102x. The X100x SoCs, those are > also found on the Galileo Gen2 maker board, do not support secure boot > and do not use the header. The CSH is supported on the non-secure SoCs - the BSP on Gen1 certainly did. https://downloadcenter.intel.com/download/23197/Intel-Quark-SoC-X1000-Board-Support-Package-BSP- - > QuarkPlatformPkg/Platform/Dxe/PlatformInit/DxeCapsuleSecurity.c CreateCapsuleBufferForWriting() Looks like it will tolerate a lack of CSH but, obviously that's no solution for the secure boot parts. > IIRC, there used to be an eval system with > the X1020 as well, but I think it's no longer available. > > Interestingly, the capsule file found in Intel's Galileo firmware update > package [1] contains the CSH header. But I only succeeded flashing it on > a Gen2 by removing the header first. Hmm - the out of the box firmware will accept capsules from the website. Gen1 and Gen2 should be the same in that respect - if you use the BSP kernel. You should validate the out-of-the-box capsule update - with the kernel on SPI flash works with the CSH in place. Next step then is to get it working on tip-of-tree. I have one Gen1 left (which I won't be experimenting with) - and I think one Gen2 (which I don't especially mind blowing up). Let's take the time to validate (or repudiate) 1. Out of the box BSP works with CSH capsules in place 2. New tip-of-tree addition supports CSH capsules and review. Stripping the CSH shouldn't be necessary (and breaks the secure boot parts anyway). -- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-17 10:51, Bryan O'Donoghue wrote: > On 17/02/17 08:23, Kweh, Hock Leong wrote: >> And to have UEFI expand >> it capsule support and take in signed binary would be a more secured way. >> So, influencing UEFI community to have such support would be the right >> move throughout the discussion. That is my summary. > > CSH stands for "Clanton Secure Header" - Clanton being the internal > code-name for Quark X1000 prior to release. > > There is no chance the UEFI standard (which can be used on ARM and > potentially other architectures) will accept a SoC specific > route-of-trust prepended header. > > Sure some kind of binary signed headers might become part of the > standard eventually but, definitely _not_ a CSH. > > The fact is CSH exists in the real-world and a UEFI firmware supports > accepting the CSH/UEFI-capsule pair for updating itself. > > I think a far more practical solution is to accommodate the defacto > implementation (the only ? current implementation). To me it defies > reason to have Quark X1000 be the only system (that I know of) capable > of doing a capsule update - have capsule code in the kernel - but _not_ > support the header prepended to that capsule that the Quark > firmware/bootrom require. > > Right now the capsule code is dead code on Quark x1000. Let's do the > right thing and make it usable. I fully support having a > separate/parallel conversation with the UEFI body but, I'd be amazed if > the "Clanton Secure Header" made it into the standard... > To be precise, CSH is only required on X102x. The X100x SoCs, those are also found on the Galileo Gen2 maker board, do not support secure boot and do not use the header. IIRC, there used to be an eval system with the X1020 as well, but I think it's no longer available. Interestingly, the capsule file found in Intel's Galileo firmware update package [1] contains the CSH header. But I only succeeded flashing it on a Gen2 by removing the header first. Jan [1] https://downloadcenter.intel.com/download/26417/Intel-Galileo-Firmware-Updater-and-Drivers?product=83137 -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-17 10:51, Bryan O'Donoghue wrote: > On 17/02/17 08:23, Kweh, Hock Leong wrote: >> And to have UEFI expand >> it capsule support and take in signed binary would be a more secured way. >> So, influencing UEFI community to have such support would be the right >> move throughout the discussion. That is my summary. > > CSH stands for "Clanton Secure Header" - Clanton being the internal > code-name for Quark X1000 prior to release. > > There is no chance the UEFI standard (which can be used on ARM and > potentially other architectures) will accept a SoC specific > route-of-trust prepended header. > > Sure some kind of binary signed headers might become part of the > standard eventually but, definitely _not_ a CSH. > > The fact is CSH exists in the real-world and a UEFI firmware supports > accepting the CSH/UEFI-capsule pair for updating itself. > > I think a far more practical solution is to accommodate the defacto > implementation (the only ? current implementation). To me it defies > reason to have Quark X1000 be the only system (that I know of) capable > of doing a capsule update - have capsule code in the kernel - but _not_ > support the header prepended to that capsule that the Quark > firmware/bootrom require. > > Right now the capsule code is dead code on Quark x1000. Let's do the > right thing and make it usable. I fully support having a > separate/parallel conversation with the UEFI body but, I'd be amazed if > the "Clanton Secure Header" made it into the standard... > To be precise, CSH is only required on X102x. The X100x SoCs, those are also found on the Galileo Gen2 maker board, do not support secure boot and do not use the header. IIRC, there used to be an eval system with the X1020 as well, but I think it's no longer available. Interestingly, the capsule file found in Intel's Galileo firmware update package [1] contains the CSH header. But I only succeeded flashing it on a Gen2 by removing the header first. Jan [1] https://downloadcenter.intel.com/download/26417/Intel-Galileo-Firmware-Updater-and-Drivers?product=83137 -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 17/02/17 08:23, Kweh, Hock Leong wrote: > And to have UEFI expand > it capsule support and take in signed binary would be a more secured way. > So, influencing UEFI community to have such support would be the right > move throughout the discussion. That is my summary. CSH stands for "Clanton Secure Header" - Clanton being the internal code-name for Quark X1000 prior to release. There is no chance the UEFI standard (which can be used on ARM and potentially other architectures) will accept a SoC specific route-of-trust prepended header. Sure some kind of binary signed headers might become part of the standard eventually but, definitely _not_ a CSH. The fact is CSH exists in the real-world and a UEFI firmware supports accepting the CSH/UEFI-capsule pair for updating itself. I think a far more practical solution is to accommodate the defacto implementation (the only ? current implementation). To me it defies reason to have Quark X1000 be the only system (that I know of) capable of doing a capsule update - have capsule code in the kernel - but _not_ support the header prepended to that capsule that the Quark firmware/bootrom require. Right now the capsule code is dead code on Quark x1000. Let's do the right thing and make it usable. I fully support having a separate/parallel conversation with the UEFI body but, I'd be amazed if the "Clanton Secure Header" made it into the standard... -- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 17/02/17 08:23, Kweh, Hock Leong wrote: > And to have UEFI expand > it capsule support and take in signed binary would be a more secured way. > So, influencing UEFI community to have such support would be the right > move throughout the discussion. That is my summary. CSH stands for "Clanton Secure Header" - Clanton being the internal code-name for Quark X1000 prior to release. There is no chance the UEFI standard (which can be used on ARM and potentially other architectures) will accept a SoC specific route-of-trust prepended header. Sure some kind of binary signed headers might become part of the standard eventually but, definitely _not_ a CSH. The fact is CSH exists in the real-world and a UEFI firmware supports accepting the CSH/UEFI-capsule pair for updating itself. I think a far more practical solution is to accommodate the defacto implementation (the only ? current implementation). To me it defies reason to have Quark X1000 be the only system (that I know of) capable of doing a capsule update - have capsule code in the kernel - but _not_ support the header prepended to that capsule that the Quark firmware/bootrom require. Right now the capsule code is dead code on Quark x1000. Let's do the right thing and make it usable. I fully support having a separate/parallel conversation with the UEFI body but, I'd be amazed if the "Clanton Secure Header" made it into the standard... -- bod
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-17 09:23, Kweh, Hock Leong wrote: >> -Original Message- >> From: Bryan O'Donoghue [mailto:pure.lo...@nexus-software.ie] >> Sent: Friday, February 17, 2017 8:54 AM >> To: Kweh, Hock Leong <hock.leong.k...@intel.com>; Jan Kiszka >> <jan.kis...@siemens.com>; Andy Shevchenko <andy.shevche...@gmail.com> >> Cc: Matt Fleming <m...@codeblueprint.co.uk>; Ard Biesheuvel >> <ard.biesheu...@linaro.org>; linux-...@vger.kernel.org; Linux Kernel Mailing >> List <linux-kernel@vger.kernel.org>; Borislav Petkov <b...@alien8.de> >> Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark >> images >> >> >> >> On 16/02/17 03:00, Kweh, Hock Leong wrote: >>>> -Original Message- >>>> From: Jan Kiszka [mailto:jan.kis...@siemens.com] >>>> Sent: Thursday, February 16, 2017 3:00 AM >>>> To: Andy Shevchenko <andy.shevche...@gmail.com> >>>> Cc: Matt Fleming <m...@codeblueprint.co.uk>; Ard Biesheuvel >>>> <ard.biesheu...@linaro.org>; linux-...@vger.kernel.org; Linux Kernel >>>> Mailing List <linux-kernel@vger.kernel.org>; Borislav Petkov >>>> <b...@alien8.de>; Kweh, Hock Leong <hock.leong.k...@intel.com>; Bryan >>>> O'Donoghue <pure.lo...@nexus-software.ie> >>>> Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support >>>> signed Quark images >>>> >>>> On 2017-02-15 19:50, Jan Kiszka wrote: >>>>> On 2017-02-15 19:46, Andy Shevchenko wrote: >>>>>> On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka >>>>>> <jan.kis...@siemens.com> >>>> wrote: >>>>>>> See patch 2 for the background. >>>>>>> >>>>>>> Series has been tested on the Galileo Gen2, to exclude >>>>>>> regressions, with a firmware.cap without security header and the >>>>>>> SIMATIC IOT2040 which requires the header because of its mandatory >> secure boot. >>>>>> >>>>>> Briefly looking to the code it looks like a real hack. >>>>>> Sorry, but it would be carefully (re-)designed. >>>>> >>>>> The interface that the firmware provides us? That should have been >>>>> done differently, I agree, but I'm not too much into those firmware >>>>> details, specifically when it comes to signatures. >>>>> >>>>> The Linux code was designed around that suboptimal situation. If >>>>> there are better ideas, I'm all ears. >>>>> >>>> >>>> Expanding CC's as requested by Andy. >>>> >>>> Jan >>>> >>> >>> Hi Jan, >>> >>> While I upstreaming the capsule loader patches, I did work with >>> maintainer Matt and look into this security header created for Quark. >>> Eventually both of us agreed that this will not be upstream to >>> mainline as it is really a Quark specific implementation. >> >> What's the logic of that ? >> >> It should be possible to provide a hook (or a custom function). >> >>> >>> The proper implementation may require to work with UEFI community to >>> expand its capsule spec to support signed binary. >> >> Are you volunteering to do that with - getting the CSH into the UEFI spec ? >> >> If not then we should have a method to load/ignore a capsule including the >> CSH, >> if so then we should have a realistic timeline laid out for getting that >> spec work >> done. >> >> Hint: I don't believe integrating the CSH into the UEFI standard will >> happen... >> > > Hi Jan & Bryan, > > Please don't get me wrong. I am not rejecting the patch submission. > I am just sharing a summary for the discussion that I had had it a year back > with maintainer Matt for this topic. From the discussion, we did mention > what would be the proper handling to this case. And to have UEFI expand Do you happen to have a reference to the part of the discussions that deal with the CSH topic? > it capsule support and take in signed binary would be a more secured way. > So, influencing UEFI community to have such support would be the right > move throughout the discussion. That is my summary. > > Of course, influencing the UEFI community would be the longer path. > I think it is worth for try to bring the topic up here again to see if Matt > would reconsider it. I just can re-express my frustration that this essential step hasn't been started years ago by whoever designed the extension. Then I bet there would have been constructive feedback on the interface BEFORE its ugliness spread to broader use. Or is there a technical need, in general or on Quark, to have the signature header right before the standard capsule *for the handover* to the firmware? I mean, I would naively put it into another capsule and prepend that to the core so that the existing UEFI API can palate it transparently and cleanly. Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-17 09:23, Kweh, Hock Leong wrote: >> -Original Message- >> From: Bryan O'Donoghue [mailto:pure.lo...@nexus-software.ie] >> Sent: Friday, February 17, 2017 8:54 AM >> To: Kweh, Hock Leong ; Jan Kiszka >> ; Andy Shevchenko >> Cc: Matt Fleming ; Ard Biesheuvel >> ; linux-...@vger.kernel.org; Linux Kernel Mailing >> List ; Borislav Petkov >> Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark >> images >> >> >> >> On 16/02/17 03:00, Kweh, Hock Leong wrote: >>>> -Original Message- >>>> From: Jan Kiszka [mailto:jan.kis...@siemens.com] >>>> Sent: Thursday, February 16, 2017 3:00 AM >>>> To: Andy Shevchenko >>>> Cc: Matt Fleming ; Ard Biesheuvel >>>> ; linux-...@vger.kernel.org; Linux Kernel >>>> Mailing List ; Borislav Petkov >>>> ; Kweh, Hock Leong ; Bryan >>>> O'Donoghue >>>> Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support >>>> signed Quark images >>>> >>>> On 2017-02-15 19:50, Jan Kiszka wrote: >>>>> On 2017-02-15 19:46, Andy Shevchenko wrote: >>>>>> On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka >>>>>> >>>> wrote: >>>>>>> See patch 2 for the background. >>>>>>> >>>>>>> Series has been tested on the Galileo Gen2, to exclude >>>>>>> regressions, with a firmware.cap without security header and the >>>>>>> SIMATIC IOT2040 which requires the header because of its mandatory >> secure boot. >>>>>> >>>>>> Briefly looking to the code it looks like a real hack. >>>>>> Sorry, but it would be carefully (re-)designed. >>>>> >>>>> The interface that the firmware provides us? That should have been >>>>> done differently, I agree, but I'm not too much into those firmware >>>>> details, specifically when it comes to signatures. >>>>> >>>>> The Linux code was designed around that suboptimal situation. If >>>>> there are better ideas, I'm all ears. >>>>> >>>> >>>> Expanding CC's as requested by Andy. >>>> >>>> Jan >>>> >>> >>> Hi Jan, >>> >>> While I upstreaming the capsule loader patches, I did work with >>> maintainer Matt and look into this security header created for Quark. >>> Eventually both of us agreed that this will not be upstream to >>> mainline as it is really a Quark specific implementation. >> >> What's the logic of that ? >> >> It should be possible to provide a hook (or a custom function). >> >>> >>> The proper implementation may require to work with UEFI community to >>> expand its capsule spec to support signed binary. >> >> Are you volunteering to do that with - getting the CSH into the UEFI spec ? >> >> If not then we should have a method to load/ignore a capsule including the >> CSH, >> if so then we should have a realistic timeline laid out for getting that >> spec work >> done. >> >> Hint: I don't believe integrating the CSH into the UEFI standard will >> happen... >> > > Hi Jan & Bryan, > > Please don't get me wrong. I am not rejecting the patch submission. > I am just sharing a summary for the discussion that I had had it a year back > with maintainer Matt for this topic. From the discussion, we did mention > what would be the proper handling to this case. And to have UEFI expand Do you happen to have a reference to the part of the discussions that deal with the CSH topic? > it capsule support and take in signed binary would be a more secured way. > So, influencing UEFI community to have such support would be the right > move throughout the discussion. That is my summary. > > Of course, influencing the UEFI community would be the longer path. > I think it is worth for try to bring the topic up here again to see if Matt > would reconsider it. I just can re-express my frustration that this essential step hasn't been started years ago by whoever designed the extension. Then I bet there would have been constructive feedback on the interface BEFORE its ugliness spread to broader use. Or is there a technical need, in general or on Quark, to have the signature header right before the standard capsule *for the handover* to the firmware? I mean, I would naively put it into another capsule and prepend that to the core so that the existing UEFI API can palate it transparently and cleanly. Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
RE: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
> -Original Message- > From: Bryan O'Donoghue [mailto:pure.lo...@nexus-software.ie] > Sent: Friday, February 17, 2017 8:54 AM > To: Kweh, Hock Leong <hock.leong.k...@intel.com>; Jan Kiszka > <jan.kis...@siemens.com>; Andy Shevchenko <andy.shevche...@gmail.com> > Cc: Matt Fleming <m...@codeblueprint.co.uk>; Ard Biesheuvel > <ard.biesheu...@linaro.org>; linux-...@vger.kernel.org; Linux Kernel Mailing > List <linux-kernel@vger.kernel.org>; Borislav Petkov <b...@alien8.de> > Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark > images > > > > On 16/02/17 03:00, Kweh, Hock Leong wrote: > >> -Original Message- > >> From: Jan Kiszka [mailto:jan.kis...@siemens.com] > >> Sent: Thursday, February 16, 2017 3:00 AM > >> To: Andy Shevchenko <andy.shevche...@gmail.com> > >> Cc: Matt Fleming <m...@codeblueprint.co.uk>; Ard Biesheuvel > >> <ard.biesheu...@linaro.org>; linux-...@vger.kernel.org; Linux Kernel > >> Mailing List <linux-kernel@vger.kernel.org>; Borislav Petkov > >> <b...@alien8.de>; Kweh, Hock Leong <hock.leong.k...@intel.com>; Bryan > >> O'Donoghue <pure.lo...@nexus-software.ie> > >> Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support > >> signed Quark images > >> > >> On 2017-02-15 19:50, Jan Kiszka wrote: > >>> On 2017-02-15 19:46, Andy Shevchenko wrote: > >>>> On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka > >>>> <jan.kis...@siemens.com> > >> wrote: > >>>>> See patch 2 for the background. > >>>>> > >>>>> Series has been tested on the Galileo Gen2, to exclude > >>>>> regressions, with a firmware.cap without security header and the > >>>>> SIMATIC IOT2040 which requires the header because of its mandatory > secure boot. > >>>> > >>>> Briefly looking to the code it looks like a real hack. > >>>> Sorry, but it would be carefully (re-)designed. > >>> > >>> The interface that the firmware provides us? That should have been > >>> done differently, I agree, but I'm not too much into those firmware > >>> details, specifically when it comes to signatures. > >>> > >>> The Linux code was designed around that suboptimal situation. If > >>> there are better ideas, I'm all ears. > >>> > >> > >> Expanding CC's as requested by Andy. > >> > >> Jan > >> > > > > Hi Jan, > > > > While I upstreaming the capsule loader patches, I did work with > > maintainer Matt and look into this security header created for Quark. > > Eventually both of us agreed that this will not be upstream to > > mainline as it is really a Quark specific implementation. > > What's the logic of that ? > > It should be possible to provide a hook (or a custom function). > > > > > The proper implementation may require to work with UEFI community to > > expand its capsule spec to support signed binary. > > Are you volunteering to do that with - getting the CSH into the UEFI spec ? > > If not then we should have a method to load/ignore a capsule including the > CSH, > if so then we should have a realistic timeline laid out for getting that spec > work > done. > > Hint: I don't believe integrating the CSH into the UEFI standard will > happen... > Hi Jan & Bryan, Please don't get me wrong. I am not rejecting the patch submission. I am just sharing a summary for the discussion that I had had it a year back with maintainer Matt for this topic. From the discussion, we did mention what would be the proper handling to this case. And to have UEFI expand it capsule support and take in signed binary would be a more secured way. So, influencing UEFI community to have such support would be the right move throughout the discussion. That is my summary. Of course, influencing the UEFI community would be the longer path. I think it is worth for try to bring the topic up here again to see if Matt would reconsider it. Regards, Wilson
RE: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
> -Original Message- > From: Bryan O'Donoghue [mailto:pure.lo...@nexus-software.ie] > Sent: Friday, February 17, 2017 8:54 AM > To: Kweh, Hock Leong ; Jan Kiszka > ; Andy Shevchenko > Cc: Matt Fleming ; Ard Biesheuvel > ; linux-...@vger.kernel.org; Linux Kernel Mailing > List ; Borislav Petkov > Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark > images > > > > On 16/02/17 03:00, Kweh, Hock Leong wrote: > >> -Original Message- > >> From: Jan Kiszka [mailto:jan.kis...@siemens.com] > >> Sent: Thursday, February 16, 2017 3:00 AM > >> To: Andy Shevchenko > >> Cc: Matt Fleming ; Ard Biesheuvel > >> ; linux-...@vger.kernel.org; Linux Kernel > >> Mailing List ; Borislav Petkov > >> ; Kweh, Hock Leong ; Bryan > >> O'Donoghue > >> Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support > >> signed Quark images > >> > >> On 2017-02-15 19:50, Jan Kiszka wrote: > >>> On 2017-02-15 19:46, Andy Shevchenko wrote: > >>>> On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka > >>>> > >> wrote: > >>>>> See patch 2 for the background. > >>>>> > >>>>> Series has been tested on the Galileo Gen2, to exclude > >>>>> regressions, with a firmware.cap without security header and the > >>>>> SIMATIC IOT2040 which requires the header because of its mandatory > secure boot. > >>>> > >>>> Briefly looking to the code it looks like a real hack. > >>>> Sorry, but it would be carefully (re-)designed. > >>> > >>> The interface that the firmware provides us? That should have been > >>> done differently, I agree, but I'm not too much into those firmware > >>> details, specifically when it comes to signatures. > >>> > >>> The Linux code was designed around that suboptimal situation. If > >>> there are better ideas, I'm all ears. > >>> > >> > >> Expanding CC's as requested by Andy. > >> > >> Jan > >> > > > > Hi Jan, > > > > While I upstreaming the capsule loader patches, I did work with > > maintainer Matt and look into this security header created for Quark. > > Eventually both of us agreed that this will not be upstream to > > mainline as it is really a Quark specific implementation. > > What's the logic of that ? > > It should be possible to provide a hook (or a custom function). > > > > > The proper implementation may require to work with UEFI community to > > expand its capsule spec to support signed binary. > > Are you volunteering to do that with - getting the CSH into the UEFI spec ? > > If not then we should have a method to load/ignore a capsule including the > CSH, > if so then we should have a realistic timeline laid out for getting that spec > work > done. > > Hint: I don't believe integrating the CSH into the UEFI standard will > happen... > Hi Jan & Bryan, Please don't get me wrong. I am not rejecting the patch submission. I am just sharing a summary for the discussion that I had had it a year back with maintainer Matt for this topic. From the discussion, we did mention what would be the proper handling to this case. And to have UEFI expand it capsule support and take in signed binary would be a more secured way. So, influencing UEFI community to have such support would be the right move throughout the discussion. That is my summary. Of course, influencing the UEFI community would be the longer path. I think it is worth for try to bring the topic up here again to see if Matt would reconsider it. Regards, Wilson
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 16/02/17 03:00, Kweh, Hock Leong wrote: -Original Message- From: Jan Kiszka [mailto:jan.kis...@siemens.com] Sent: Thursday, February 16, 2017 3:00 AM To: Andy Shevchenko <andy.shevche...@gmail.com> Cc: Matt Fleming <m...@codeblueprint.co.uk>; Ard Biesheuvel <ard.biesheu...@linaro.org>; linux-...@vger.kernel.org; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; Borislav Petkov <b...@alien8.de>; Kweh, Hock Leong <hock.leong.k...@intel.com>; Bryan O'Donoghue <pure.lo...@nexus-software.ie> Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images On 2017-02-15 19:50, Jan Kiszka wrote: On 2017-02-15 19:46, Andy Shevchenko wrote: On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka <jan.kis...@siemens.com> wrote: See patch 2 for the background. Series has been tested on the Galileo Gen2, to exclude regressions, with a firmware.cap without security header and the SIMATIC IOT2040 which requires the header because of its mandatory secure boot. Briefly looking to the code it looks like a real hack. Sorry, but it would be carefully (re-)designed. The interface that the firmware provides us? That should have been done differently, I agree, but I'm not too much into those firmware details, specifically when it comes to signatures. The Linux code was designed around that suboptimal situation. If there are better ideas, I'm all ears. Expanding CC's as requested by Andy. Jan Hi Jan, While I upstreaming the capsule loader patches, I did work with maintainer Matt and look into this security header created for Quark. Eventually both of us agreed that this will not be upstream to mainline as it is really a Quark specific implementation. What's the logic of that ? It should be possible to provide a hook (or a custom function). The proper implementation may require to work with UEFI community to expand its capsule spec to support signed binary. Are you volunteering to do that with - getting the CSH into the UEFI spec ? If not then we should have a method to load/ignore a capsule including the CSH, if so then we should have a realistic timeline laid out for getting that spec work done. Hint: I don't believe integrating the CSH into the UEFI standard will happen... Regards, Wilson
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 16/02/17 03:00, Kweh, Hock Leong wrote: -Original Message- From: Jan Kiszka [mailto:jan.kis...@siemens.com] Sent: Thursday, February 16, 2017 3:00 AM To: Andy Shevchenko Cc: Matt Fleming ; Ard Biesheuvel ; linux-...@vger.kernel.org; Linux Kernel Mailing List ; Borislav Petkov ; Kweh, Hock Leong ; Bryan O'Donoghue Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images On 2017-02-15 19:50, Jan Kiszka wrote: On 2017-02-15 19:46, Andy Shevchenko wrote: On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka wrote: See patch 2 for the background. Series has been tested on the Galileo Gen2, to exclude regressions, with a firmware.cap without security header and the SIMATIC IOT2040 which requires the header because of its mandatory secure boot. Briefly looking to the code it looks like a real hack. Sorry, but it would be carefully (re-)designed. The interface that the firmware provides us? That should have been done differently, I agree, but I'm not too much into those firmware details, specifically when it comes to signatures. The Linux code was designed around that suboptimal situation. If there are better ideas, I'm all ears. Expanding CC's as requested by Andy. Jan Hi Jan, While I upstreaming the capsule loader patches, I did work with maintainer Matt and look into this security header created for Quark. Eventually both of us agreed that this will not be upstream to mainline as it is really a Quark specific implementation. What's the logic of that ? It should be possible to provide a hook (or a custom function). The proper implementation may require to work with UEFI community to expand its capsule spec to support signed binary. Are you volunteering to do that with - getting the CSH into the UEFI spec ? If not then we should have a method to load/ignore a capsule including the CSH, if so then we should have a realistic timeline laid out for getting that spec work done. Hint: I don't believe integrating the CSH into the UEFI standard will happen... Regards, Wilson
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-16 04:00, Kweh, Hock Leong wrote: >> -Original Message- >> From: Jan Kiszka [mailto:jan.kis...@siemens.com] >> Sent: Thursday, February 16, 2017 3:00 AM >> To: Andy Shevchenko <andy.shevche...@gmail.com> >> Cc: Matt Fleming <m...@codeblueprint.co.uk>; Ard Biesheuvel >> <ard.biesheu...@linaro.org>; linux-...@vger.kernel.org; Linux Kernel Mailing >> List <linux-kernel@vger.kernel.org>; Borislav Petkov <b...@alien8.de>; Kweh, >> Hock Leong <hock.leong.k...@intel.com>; Bryan O'Donoghue >> <pure.lo...@nexus-software.ie> >> Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark >> images >> >> On 2017-02-15 19:50, Jan Kiszka wrote: >>> On 2017-02-15 19:46, Andy Shevchenko wrote: >>>> On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka <jan.kis...@siemens.com> >> wrote: >>>>> See patch 2 for the background. >>>>> >>>>> Series has been tested on the Galileo Gen2, to exclude regressions, >>>>> with a firmware.cap without security header and the SIMATIC IOT2040 >>>>> which requires the header because of its mandatory secure boot. >>>> >>>> Briefly looking to the code it looks like a real hack. >>>> Sorry, but it would be carefully (re-)designed. >>> >>> The interface that the firmware provides us? That should have been >>> done differently, I agree, but I'm not too much into those firmware >>> details, specifically when it comes to signatures. >>> >>> The Linux code was designed around that suboptimal situation. If there >>> are better ideas, I'm all ears. >>> >> >> Expanding CC's as requested by Andy. >> >> Jan >> > > Hi Jan, > > While I upstreaming the capsule loader patches, I did work with maintainer > Matt and look into this security header created for Quark. Eventually both > of us agreed that this will not be upstream to mainline as it is really a > Quark > specific implementation. This is ... [swallowing down a lengthy rant about Quark upstreaming] unfortunate given that Intel hands out firmware and BSPs to their customers without further explanations on this "minor detail". I have no idea what other integrators of the X102x did with that, but my customer has now thousands and thousands of devices in the field with firmware that works exactly this way. Only for that feature, we will now have to provide a non-upstream kernel in order to keep the installed devices updatable. Or create and maintain a different mechanism. Beautiful. > > The proper implementation may require to work with UEFI community > to expand its capsule spec to support signed binary. > Are you working on this? How is this solved on other platforms that require signatures? No one tried that yet? In any case, this sounds like a lengthy, way too late considered process that will not solve our issue in the foreseeable future. Don't get me wrong, I'm not intending to push this into the kernel because "What the heck?!" was my first reaction as well once I found out how this update interface is actually working. But maybe you can bring this topic up on your side as well so that we come to an upstreamable solution in all affected projects. Thanks, Jan PS: @Daniel, another example for your presentation. ;) -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-16 04:00, Kweh, Hock Leong wrote: >> -Original Message- >> From: Jan Kiszka [mailto:jan.kis...@siemens.com] >> Sent: Thursday, February 16, 2017 3:00 AM >> To: Andy Shevchenko >> Cc: Matt Fleming ; Ard Biesheuvel >> ; linux-...@vger.kernel.org; Linux Kernel Mailing >> List ; Borislav Petkov ; Kweh, >> Hock Leong ; Bryan O'Donoghue >> >> Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark >> images >> >> On 2017-02-15 19:50, Jan Kiszka wrote: >>> On 2017-02-15 19:46, Andy Shevchenko wrote: >>>> On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka >> wrote: >>>>> See patch 2 for the background. >>>>> >>>>> Series has been tested on the Galileo Gen2, to exclude regressions, >>>>> with a firmware.cap without security header and the SIMATIC IOT2040 >>>>> which requires the header because of its mandatory secure boot. >>>> >>>> Briefly looking to the code it looks like a real hack. >>>> Sorry, but it would be carefully (re-)designed. >>> >>> The interface that the firmware provides us? That should have been >>> done differently, I agree, but I'm not too much into those firmware >>> details, specifically when it comes to signatures. >>> >>> The Linux code was designed around that suboptimal situation. If there >>> are better ideas, I'm all ears. >>> >> >> Expanding CC's as requested by Andy. >> >> Jan >> > > Hi Jan, > > While I upstreaming the capsule loader patches, I did work with maintainer > Matt and look into this security header created for Quark. Eventually both > of us agreed that this will not be upstream to mainline as it is really a > Quark > specific implementation. This is ... [swallowing down a lengthy rant about Quark upstreaming] unfortunate given that Intel hands out firmware and BSPs to their customers without further explanations on this "minor detail". I have no idea what other integrators of the X102x did with that, but my customer has now thousands and thousands of devices in the field with firmware that works exactly this way. Only for that feature, we will now have to provide a non-upstream kernel in order to keep the installed devices updatable. Or create and maintain a different mechanism. Beautiful. > > The proper implementation may require to work with UEFI community > to expand its capsule spec to support signed binary. > Are you working on this? How is this solved on other platforms that require signatures? No one tried that yet? In any case, this sounds like a lengthy, way too late considered process that will not solve our issue in the foreseeable future. Don't get me wrong, I'm not intending to push this into the kernel because "What the heck?!" was my first reaction as well once I found out how this update interface is actually working. But maybe you can bring this topic up on your side as well so that we come to an upstreamable solution in all affected projects. Thanks, Jan PS: @Daniel, another example for your presentation. ;) -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
RE: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
> -Original Message- > From: Jan Kiszka [mailto:jan.kis...@siemens.com] > Sent: Thursday, February 16, 2017 3:00 AM > To: Andy Shevchenko <andy.shevche...@gmail.com> > Cc: Matt Fleming <m...@codeblueprint.co.uk>; Ard Biesheuvel > <ard.biesheu...@linaro.org>; linux-...@vger.kernel.org; Linux Kernel Mailing > List <linux-kernel@vger.kernel.org>; Borislav Petkov <b...@alien8.de>; Kweh, > Hock Leong <hock.leong.k...@intel.com>; Bryan O'Donoghue > <pure.lo...@nexus-software.ie> > Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark > images > > On 2017-02-15 19:50, Jan Kiszka wrote: > > On 2017-02-15 19:46, Andy Shevchenko wrote: > >> On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka <jan.kis...@siemens.com> > wrote: > >>> See patch 2 for the background. > >>> > >>> Series has been tested on the Galileo Gen2, to exclude regressions, > >>> with a firmware.cap without security header and the SIMATIC IOT2040 > >>> which requires the header because of its mandatory secure boot. > >> > >> Briefly looking to the code it looks like a real hack. > >> Sorry, but it would be carefully (re-)designed. > > > > The interface that the firmware provides us? That should have been > > done differently, I agree, but I'm not too much into those firmware > > details, specifically when it comes to signatures. > > > > The Linux code was designed around that suboptimal situation. If there > > are better ideas, I'm all ears. > > > > Expanding CC's as requested by Andy. > > Jan > Hi Jan, While I upstreaming the capsule loader patches, I did work with maintainer Matt and look into this security header created for Quark. Eventually both of us agreed that this will not be upstream to mainline as it is really a Quark specific implementation. The proper implementation may require to work with UEFI community to expand its capsule spec to support signed binary. Regards, Wilson
RE: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
> -Original Message- > From: Jan Kiszka [mailto:jan.kis...@siemens.com] > Sent: Thursday, February 16, 2017 3:00 AM > To: Andy Shevchenko > Cc: Matt Fleming ; Ard Biesheuvel > ; linux-...@vger.kernel.org; Linux Kernel Mailing > List ; Borislav Petkov ; Kweh, > Hock Leong ; Bryan O'Donoghue > > Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark > images > > On 2017-02-15 19:50, Jan Kiszka wrote: > > On 2017-02-15 19:46, Andy Shevchenko wrote: > >> On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka > wrote: > >>> See patch 2 for the background. > >>> > >>> Series has been tested on the Galileo Gen2, to exclude regressions, > >>> with a firmware.cap without security header and the SIMATIC IOT2040 > >>> which requires the header because of its mandatory secure boot. > >> > >> Briefly looking to the code it looks like a real hack. > >> Sorry, but it would be carefully (re-)designed. > > > > The interface that the firmware provides us? That should have been > > done differently, I agree, but I'm not too much into those firmware > > details, specifically when it comes to signatures. > > > > The Linux code was designed around that suboptimal situation. If there > > are better ideas, I'm all ears. > > > > Expanding CC's as requested by Andy. > > Jan > Hi Jan, While I upstreaming the capsule loader patches, I did work with maintainer Matt and look into this security header created for Quark. Eventually both of us agreed that this will not be upstream to mainline as it is really a Quark specific implementation. The proper implementation may require to work with UEFI community to expand its capsule spec to support signed binary. Regards, Wilson
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-15 19:50, Jan Kiszka wrote: > On 2017-02-15 19:46, Andy Shevchenko wrote: >> On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszkawrote: >>> See patch 2 for the background. >>> >>> Series has been tested on the Galileo Gen2, to exclude regressions, with >>> a firmware.cap without security header and the SIMATIC IOT2040 which >>> requires the header because of its mandatory secure boot. >> >> Briefly looking to the code it looks like a real hack. >> Sorry, but it would be carefully (re-)designed. > > The interface that the firmware provides us? That should have been done > differently, I agree, but I'm not too much into those firmware details, > specifically when it comes to signatures. > > The Linux code was designed around that suboptimal situation. If there > are better ideas, I'm all ears. > Expanding CC's as requested by Andy. Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-15 19:50, Jan Kiszka wrote: > On 2017-02-15 19:46, Andy Shevchenko wrote: >> On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka wrote: >>> See patch 2 for the background. >>> >>> Series has been tested on the Galileo Gen2, to exclude regressions, with >>> a firmware.cap without security header and the SIMATIC IOT2040 which >>> requires the header because of its mandatory secure boot. >> >> Briefly looking to the code it looks like a real hack. >> Sorry, but it would be carefully (re-)designed. > > The interface that the firmware provides us? That should have been done > differently, I agree, but I'm not too much into those firmware details, > specifically when it comes to signatures. > > The Linux code was designed around that suboptimal situation. If there > are better ideas, I'm all ears. > Expanding CC's as requested by Andy. Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-15 19:46, Andy Shevchenko wrote: > On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszkawrote: >> See patch 2 for the background. >> >> Series has been tested on the Galileo Gen2, to exclude regressions, with >> a firmware.cap without security header and the SIMATIC IOT2040 which >> requires the header because of its mandatory secure boot. > > Briefly looking to the code it looks like a real hack. > Sorry, but it would be carefully (re-)designed. The interface that the firmware provides us? That should have been done differently, I agree, but I'm not too much into those firmware details, specifically when it comes to signatures. The Linux code was designed around that suboptimal situation. If there are better ideas, I'm all ears. Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-15 19:46, Andy Shevchenko wrote: > On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka wrote: >> See patch 2 for the background. >> >> Series has been tested on the Galileo Gen2, to exclude regressions, with >> a firmware.cap without security header and the SIMATIC IOT2040 which >> requires the header because of its mandatory secure boot. > > Briefly looking to the code it looks like a real hack. > Sorry, but it would be carefully (re-)designed. The interface that the firmware provides us? That should have been done differently, I agree, but I'm not too much into those firmware details, specifically when it comes to signatures. The Linux code was designed around that suboptimal situation. If there are better ideas, I'm all ears. Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-15 19:17, Ard Biesheuvel wrote: > On 15 February 2017 at 18:14, Jan Kiszkawrote: >> See patch 2 for the background. >> >> Series has been tested on the Galileo Gen2, to exclude regressions, with >> a firmware.cap without security header and the SIMATIC IOT2040 which >> requires the header because of its mandatory secure boot. >> > > Hello Jan, > > What is a Quark? Is it in the UEFI spec? http://ark.intel.com/products/79084/Intel-Quark-SoC-X1000-16K-Cache-400-MHz I didn't find any obvious reference to this format in the UEFI spec. This might be specific to the Quark UEFI EDK2 that Intel ships (it's not in upstream edk2) and that was used as foundation for the IOT2000 series. The capsule driver that Intel includes in their Galileo BSP does something similar (I don't have a browsable reference to that at hand, sorry, must be in this nice package https://downloadcenter.intel.com/download/24702/Intel-Galileo-Board-GPL-Compliance-files-1-0-4?product=83137). Jan > >> Jan Kiszka (2): >> efi/capsule: Prepare for loading images with security header >> efi/capsule: Add support for Quark security header >> >> drivers/firmware/efi/capsule-loader.c | 73 >> ++- >> drivers/firmware/efi/capsule.c| 19 +++-- >> include/linux/efi.h | 2 +- >> 3 files changed, 79 insertions(+), 15 deletions(-) >> >> -- >> 2.1.4 >> -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 2017-02-15 19:17, Ard Biesheuvel wrote: > On 15 February 2017 at 18:14, Jan Kiszka wrote: >> See patch 2 for the background. >> >> Series has been tested on the Galileo Gen2, to exclude regressions, with >> a firmware.cap without security header and the SIMATIC IOT2040 which >> requires the header because of its mandatory secure boot. >> > > Hello Jan, > > What is a Quark? Is it in the UEFI spec? http://ark.intel.com/products/79084/Intel-Quark-SoC-X1000-16K-Cache-400-MHz I didn't find any obvious reference to this format in the UEFI spec. This might be specific to the Quark UEFI EDK2 that Intel ships (it's not in upstream edk2) and that was used as foundation for the IOT2000 series. The capsule driver that Intel includes in their Galileo BSP does something similar (I don't have a browsable reference to that at hand, sorry, must be in this nice package https://downloadcenter.intel.com/download/24702/Intel-Galileo-Board-GPL-Compliance-files-1-0-4?product=83137). Jan > >> Jan Kiszka (2): >> efi/capsule: Prepare for loading images with security header >> efi/capsule: Add support for Quark security header >> >> drivers/firmware/efi/capsule-loader.c | 73 >> ++- >> drivers/firmware/efi/capsule.c| 19 +++-- >> include/linux/efi.h | 2 +- >> 3 files changed, 79 insertions(+), 15 deletions(-) >> >> -- >> 2.1.4 >> -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszkawrote: > See patch 2 for the background. > > Series has been tested on the Galileo Gen2, to exclude regressions, with > a firmware.cap without security header and the SIMATIC IOT2040 which > requires the header because of its mandatory secure boot. Briefly looking to the code it looks like a real hack. Sorry, but it would be carefully (re-)designed. Just my 2 cents. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka wrote: > See patch 2 for the background. > > Series has been tested on the Galileo Gen2, to exclude regressions, with > a firmware.cap without security header and the SIMATIC IOT2040 which > requires the header because of its mandatory secure boot. Briefly looking to the code it looks like a real hack. Sorry, but it would be carefully (re-)designed. Just my 2 cents. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszkawrote: > See patch 2 for the background. > > Series has been tested on the Galileo Gen2, to exclude regressions, with > a firmware.cap without security header and the SIMATIC IOT2040 which > requires the header because of its mandatory secure boot. > > Jan Based on discussions in [1], please, Cc your further messages regarding this topic to Borislav, Bryan, Hock Leong. [1] http://lists-archives.com/linux-kernel/28494369-enable-capsule-loader-interface-for-efi-firmware-updating.html > > Jan Kiszka (2): > efi/capsule: Prepare for loading images with security header > efi/capsule: Add support for Quark security header > > drivers/firmware/efi/capsule-loader.c | 73 > ++- > drivers/firmware/efi/capsule.c| 19 +++-- > include/linux/efi.h | 2 +- > 3 files changed, 79 insertions(+), 15 deletions(-) > > -- > 2.1.4 > -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On Wed, Feb 15, 2017 at 8:14 PM, Jan Kiszka wrote: > See patch 2 for the background. > > Series has been tested on the Galileo Gen2, to exclude regressions, with > a firmware.cap without security header and the SIMATIC IOT2040 which > requires the header because of its mandatory secure boot. > > Jan Based on discussions in [1], please, Cc your further messages regarding this topic to Borislav, Bryan, Hock Leong. [1] http://lists-archives.com/linux-kernel/28494369-enable-capsule-loader-interface-for-efi-firmware-updating.html > > Jan Kiszka (2): > efi/capsule: Prepare for loading images with security header > efi/capsule: Add support for Quark security header > > drivers/firmware/efi/capsule-loader.c | 73 > ++- > drivers/firmware/efi/capsule.c| 19 +++-- > include/linux/efi.h | 2 +- > 3 files changed, 79 insertions(+), 15 deletions(-) > > -- > 2.1.4 > -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 15 February 2017 at 18:14, Jan Kiszkawrote: > See patch 2 for the background. > > Series has been tested on the Galileo Gen2, to exclude regressions, with > a firmware.cap without security header and the SIMATIC IOT2040 which > requires the header because of its mandatory secure boot. > Hello Jan, What is a Quark? Is it in the UEFI spec? > Jan Kiszka (2): > efi/capsule: Prepare for loading images with security header > efi/capsule: Add support for Quark security header > > drivers/firmware/efi/capsule-loader.c | 73 > ++- > drivers/firmware/efi/capsule.c| 19 +++-- > include/linux/efi.h | 2 +- > 3 files changed, 79 insertions(+), 15 deletions(-) > > -- > 2.1.4 >
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images
On 15 February 2017 at 18:14, Jan Kiszka wrote: > See patch 2 for the background. > > Series has been tested on the Galileo Gen2, to exclude regressions, with > a firmware.cap without security header and the SIMATIC IOT2040 which > requires the header because of its mandatory secure boot. > Hello Jan, What is a Quark? Is it in the UEFI spec? > Jan Kiszka (2): > efi/capsule: Prepare for loading images with security header > efi/capsule: Add support for Quark security header > > drivers/firmware/efi/capsule-loader.c | 73 > ++- > drivers/firmware/efi/capsule.c| 19 +++-- > include/linux/efi.h | 2 +- > 3 files changed, 79 insertions(+), 15 deletions(-) > > -- > 2.1.4 >