diff mbox

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

Message ID 1444076155-19295-2-git-send-email-hock.leong.kweh@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kweh Hock Leong Oct. 5, 2015, 8:15 p.m. UTC
From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>

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

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

Comments

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

So this one is still a separate patch.

If you're going to ignore review comments, maybe I should stop wasting
my time reviewing your stuff...
Kweh Hock Leong Oct. 5, 2015, 3:19 p.m. UTC | #2
> -----Original Message-----

> From: Borislav Petkov [mailto:bp@alien8.de]

> Sent: Monday, October 05, 2015 9:14 PM

> 

> So this one is still a separate patch.

> 

> If you're going to ignore review comments, maybe I should stop wasting my

> time reviewing your stuff...

> 

> --

> Regards/Gruss,

>     Boris.


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

Thanks for the review comments.


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

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

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

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

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

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

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


??

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

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

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

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

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

--
BOD


--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kweh Hock Leong Oct. 6, 2015, 10:53 a.m. UTC | #4
> -----Original Message-----

> From: Bryan O'Donoghue [mailto:pure.logic@nexus-software.ie]

> Sent: Tuesday, October 06, 2015 5:27 AM

> 

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

> this against ?

> 

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

> 

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

> 

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

> 

> deckard@aineko:~/Development/linux$ git

> apply ../patches/capsule_wilson/1_2.eml

> ../patches/capsule_wilson/1_2.eml:72: trailing whitespace.

> EXPORT_SYMBOL_GPL(efi_capsule_supported);

> error: drivers/firmware/efi/capsule.c: No such file or directory

> 

> https://kernel.googlesource.com/pub/scm/linux/kernel/git/mfleming/efi/+/

> capsule/drivers/firmware/efi/capsule.c

> 

> 

> ??


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

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

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

then you are able to patch in my patchset.

> 

> If so - then why not use the interface here ?

> https://kernel.googlesource.com/pub/scm/linux/kernel/git/mfleming/efi/+/

> capsule

> 

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

> 

> Aside from that, I'm curious which types of capsules you've used here too -

> does it include the MFH header ? Keep in mind the initial firmware that

> shipped with Galileo will depend on that MFH being present.

> 

> http://download.intel.com/support/processors/quark/sb/quark_secureboot

> prm_330234_001.pdf

> - Section A1 - table 7 ?

> 

> So if we boot a 4.x kernel with that initial firmware version 0.75 if memory

> serves - it's important that the capsule.c code handles the MFH.

> 


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


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

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

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

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

Hmm.

That's pretty confusing.

The 0.75 BIOS requires the MFH as far as I remember. If the capsule you 
are using doesn't have the MFH - how is this working ?
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matt Fleming Oct. 10, 2015, 10:02 p.m. UTC | #8
On Mon, 05 Oct, at 03:13:50PM, Borislav Petkov wrote:
> On Tue, Oct 06, 2015 at 04:15:54AM +0800, Kweh, Hock Leong wrote:
> > From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> > 
> > This patch export efi_capsule_supported() function symbol for capsule
> > kernel module to use.
> > 
> > Cc: Matt Fleming <matt.fleming@intel.com>
> > Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
> > ---
> >  drivers/firmware/efi/capsule.c |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
> > index d8cd75c0..738d437 100644
> > --- a/drivers/firmware/efi/capsule.c
> > +++ b/drivers/firmware/efi/capsule.c
> > @@ -101,6 +101,7 @@ out:
> >  	kfree(capsule);
> >  	return rv;
> >  }
> > +EXPORT_SYMBOL_GPL(efi_capsule_supported);
> 
> So this one is still a separate patch.
> 
> If you're going to ignore review comments, maybe I should stop wasting
> my time reviewing your stuff...

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

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

Thanks & Regards,
Wilson

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matt Fleming Oct. 11, 2015, 7:03 p.m. UTC | #10
On Sun, 11 Oct, at 02:28:30PM, Kweh Hock Leong wrote:
> > -----Original Message-----
> > From: Matt Fleming [mailto:matt@console-pimps.org]
> > Sent: Sunday, October 11, 2015 6:02 AM
> > 
> > I agree that it makes sense to fold this patch into your PATCH 2, because then
> > we know why we need the above symbol to be exported.
> > 
> 
> Okay, I will squash that into my patch and re-submit v9. Before that,
> are there any comments to my v8 patchset?

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

Patch

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