[GIT PULL] EFI changes for v3.16

[GIT PULL] EFI changes for v3.16

[Matt Fleming]

Hi folks,

Please pull the following changes for v3.16. I'd like to get these in
early so that they've got plenty of time to bake in linux-next. In
particular, the ARM folks have had a hard time getting the generic EFI
cleanups/improvements picked up via other trees.

Obviously by taking these through tip we're creating a dependency with
whatever tree the ARM patches live in. Do we need to inform Stephen
Rothwell explicitly of this dependency? How should we handle things when
the v3.16 merge window opens?

The following changes since commit c9eaa447e77efe77b7fa4c953bd62de8297fd6c5:

  Linux 3.15-rc1 (2014-04-13 14:18:35 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git tags/efi-next

for you to fetch changes up to e33655a386ed3b26ad36fb97a47ebb1c2ca1e928:

  efivars: Add compatibility code for compat tasks (2014-04-17 13:53:53 +0100)

----------------------------------------------------------------
 * Generic EFI cleanups to help with the pending ARM/ARM64 EFI boot stub
   by reducing code duplication - Roy Franz, Leif Lindholm, Mark Salter

 * Swap to a single efi_call* implementation to reduce code duplication

 * Perform full FPU context saving/restoring where needed before making
   calls into the firmware - Ricardo Neri

 * efivars compat support to allow 32-bit userland to access the EFI
   variables via the sysfs efivars interface when running with a 64-bit
   kernel

----------------------------------------------------------------
H. Peter Anvin (1):
      efi: x86: Handle arbitrary Unicode characters

Leif Lindholm (1):
      efi: efi-stub-helper cleanup

Mark Salter (1):
      efi: create memory map iteration helper

Matt Fleming (7):
      x86/efi: Delete most of the efi_call* macros
      x86, fpu: Extend the use of static_cpu_has_safe
      efivars: Use local variables instead of a pointer dereference
      efivars: Check size of user object
      efivars: Stop passing a struct argument to efivar_validate()
      efivars: Refactor sanity checking code into separate function
      efivars: Add compatibility code for compat tasks

Ricardo Neri (3):
      x86/efi: Implement a __efi_call_virt macro
      x86/efi: Save and restore FPU context around efi_calls (x86_64)
      x86/efi: Save and restore FPU context around efi_calls (i386)

Roy Franz (2):
      efi: Add shared printk wrapper for consistent prefixing
      efi: Add get_dram_base() helper function

 arch/x86/boot/compressed/eboot.c       |   3 +-
 arch/x86/boot/compressed/head_64.S     |   2 +-
 arch/x86/include/asm/efi.h             | 100 ++++++-----------
 arch/x86/include/asm/fpu-internal.h    |  10 +-
 arch/x86/platform/efi/efi.c            |  48 ++++-----
 arch/x86/platform/efi/efi_stub_64.S    |  81 +-------------
 arch/x86/platform/uv/bios_uv.c         |   2 +-
 drivers/firmware/efi/efi-stub-helper.c | 144 ++++++++++++++++++-------
 drivers/firmware/efi/efivars.c         | 192 +++++++++++++++++++++++++++------
 drivers/firmware/efi/vars.c            |  30 +++---
 include/linux/efi.h                    |  12 ++-
 11 files changed, 361 insertions(+), 263 deletions(-)

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [GIT PULL] EFI changes for v3.16

[H. Peter Anvin]

On 04/19/2014 03:06 AM, Matt Fleming wrote:
> Hi folks,
> 
> Please pull the following changes for v3.16. I'd like to get these in
> early so that they've got plenty of time to bake in linux-next. In
> particular, the ARM folks have had a hard time getting the generic EFI
> cleanups/improvements picked up via other trees.
> 
> Obviously by taking these through tip we're creating a dependency with
> whatever tree the ARM patches live in. Do we need to inform Stephen
> Rothwell explicitly of this dependency? How should we handle things when
> the v3.16 merge window opens?
> 

It doesn't hurt to inform Stephen, although I think it will simply fall
out automatically since he uses git to merge and git will recognize the
graph.

During the merge window, it means they should not push their patches
until Linus has accepted the precondition patches from the tip tree.
Since Ingo and I try to push most of the tip tree as early as possible
in the merge window, this is usually not a problem.

	-hpa

[GIT PULL] EFI changes for v3.16

[Matt Fleming]

Folks, please queue the following change for v3.16 from Borislav that
uses the more strict kernel_fpu_{begin,end}() instead of the __*
verisons that won't catch buggy use in interrupt context.

The following changes since commit e33655a386ed3b26ad36fb97a47ebb1c2ca1e928:

  efivars: Add compatibility code for compat tasks (2014-04-17 13:53:53 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git tags/efi-next

for you to fetch changes up to baa916f39b50ad91661534652110df40396acda0:

  x86/efi: Check for unsafe dealing with FPU state in irq ctxt (2014-05-03 06:39:25 +0100)

----------------------------------------------------------------
 * Use the more strict FPU handling functions before invoking EFI
   services to catch misuse in irq context - Borislav Petkov

----------------------------------------------------------------
Borislav Petkov (1):
      x86/efi: Check for unsafe dealing with FPU state in irq ctxt

 arch/x86/include/asm/efi.h | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [GIT PULL] EFI changes for v3.16

[Matt Fleming]

On Sat, 03 May, at 02:04:47PM, Matt Fleming wrote:
> Folks, please queue the following change for v3.16 from Borislav that
> uses the more strict kernel_fpu_{begin,end}() instead of the __*
> verisons that won't catch buggy use in interrupt context.
> 
> The following changes since commit e33655a386ed3b26ad36fb97a47ebb1c2ca1e928:
> 
>   efivars: Add compatibility code for compat tasks (2014-04-17 13:53:53 +0100)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git tags/efi-next
> 
> for you to fetch changes up to baa916f39b50ad91661534652110df40396acda0:
> 
>   x86/efi: Check for unsafe dealing with FPU state in irq ctxt (2014-05-03 06:39:25 +0100)
> 
> ----------------------------------------------------------------
>  * Use the more strict FPU handling functions before invoking EFI
>    services to catch misuse in irq context - Borislav Petkov

Ping?

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [GIT PULL] EFI changes for v3.16

[H. Peter Anvin]

On 05/19/2014 03:51 AM, Matt Fleming wrote:
> On Sat, 03 May, at 02:04:47PM, Matt Fleming wrote:
>> Folks, please queue the following change for v3.16 from Borislav that
>> uses the more strict kernel_fpu_{begin,end}() instead of the __*
>> verisons that won't catch buggy use in interrupt context.
>>
>> The following changes since commit e33655a386ed3b26ad36fb97a47ebb1c2ca1e928:
>>
>>   efivars: Add compatibility code for compat tasks (2014-04-17 13:53:53 +0100)
>>
>> are available in the git repository at:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git tags/efi-next
>>
>> for you to fetch changes up to baa916f39b50ad91661534652110df40396acda0:
>>
>>   x86/efi: Check for unsafe dealing with FPU state in irq ctxt (2014-05-03 06:39:25 +0100)
>>
>> ----------------------------------------------------------------
>>  * Use the more strict FPU handling functions before invoking EFI
>>    services to catch misuse in irq context - Borislav Petkov
> 
> Ping?
> 

>     efi_call can happen in an irq context (pstore) and there we really need
>     to make sure we're not scribbling over FPU state while we've interrupted
>     a thread or kernel mode with a live FPU state. Therefore, use the
>     kernel_fpu_begin/end() variants which do that check.

How on earth does this solve anything?  The only thing we add here is a
WARN_ON_ONCE()... but the above text already tells us we have a problem.

It seems, rather, that we need to figure out how to deal with a pstore
in this case.  There are a few possibilities:

1. We could keep an XSAVE buffer area around for this particular use.
   I am *assuming* we don't let more than one CPU into EFI, because I
   cannot for my life imagine that this is safe in typical CPUs.

2. Drop the pstore on the floor if !irq_fpu_usable().

3. Allow the pstore, then die (on the assumption that we're dead
   anyway.)

Comments?

	-hpa

Re: [GIT PULL] EFI changes for v3.16

[Borislav Petkov]

On Mon, May 19, 2014 at 03:47:31PM -0700, H. Peter Anvin wrote:
> >     efi_call can happen in an irq context (pstore) and there we really need
> >     to make sure we're not scribbling over FPU state while we've interrupted
> >     a thread or kernel mode with a live FPU state. Therefore, use the
> >     kernel_fpu_begin/end() variants which do that check.
> 
> How on earth does this solve anything?  The only thing we add here is a
> WARN_ON_ONCE()... but the above text already tells us we have a problem.
> 
> It seems, rather, that we need to figure out how to deal with a pstore
> in this case.  There are a few possibilities:
> 
> 1. We could keep an XSAVE buffer area around for this particular use.
>    I am *assuming* we don't let more than one CPU into EFI, because I
>    cannot for my life imagine that this is safe in typical CPUs.
> 
> 2. Drop the pstore on the floor if !irq_fpu_usable().
> 
> 3. Allow the pstore, then die (on the assumption that we're dead
>    anyway.)
> 
> Comments?

The question is, why can't that pstore mumbo jumbo go and do its dance
in !irq context?

And how useful is the whole deal really, btw? I wanted to use it for
saving oopses into it, for example, but Tony said its write speed is
horribly low for that.

So why do we even bother with this thing and do the dance in irq context
for it? Is it worth it at all?

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [GIT PULL] EFI changes for v3.16

[H. Peter Anvin]

On 05/19/2014 04:10 PM, Borislav Petkov wrote:
> 
> The question is, why can't that pstore mumbo jumbo go and do its dance
> in !irq context?
> 
> And how useful is the whole deal really, btw? I wanted to use it for
> saving oopses into it, for example, but Tony said its write speed is
> horribly low for that.
> 
> So why do we even bother with this thing and do the dance in irq context
> for it? Is it worth it at all?
> 

I thought the whole point of allowing pstore in IRQ context was to write
oopsen (panics?)

	-hpa

Re: [GIT PULL] EFI changes for v3.16

[Borislav Petkov]

On Mon, May 19, 2014 at 05:17:01PM -0700, H. Peter Anvin wrote:
> I thought the whole point of allowing pstore in IRQ context was to
> write oopsen (panics?)

Right, so Tony meant last time we were talking about it that nvram is
too slow for collecting the whole dmesg into it (could mess up timing
and such - although, come to think of it, we do have CONFIG_PRINTK_TIME
so we can sort them if they're out of order...)

So what I'm questioning is whether a single oops/panic can be written
out to nvram reliably too?

Because if not, then the problem we're trying to solve is moot and
beside the point - maybe we should not use EFI as a pstore backend in
irq context at all?

I'm just putting it on the table so that we have talked about it too,
before devising a solution.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [GIT PULL] EFI changes for v3.16

[Matt Fleming]

On Mon, 19 May, at 03:47:31PM, H. Peter Anvin wrote:
> 
> How on earth does this solve anything?  The only thing we add here is a
> WARN_ON_ONCE()... but the above text already tells us we have a problem.
> 
> It seems, rather, that we need to figure out how to deal with a pstore
> in this case.  There are a few possibilities:
> 
> 1. We could keep an XSAVE buffer area around for this particular use.
>    I am *assuming* we don't let more than one CPU into EFI, because I
>    cannot for my life imagine that this is safe in typical CPUs.
 
Correct. This is actually prohibited by the spec, so we have a lock to
enforce it.

> 2. Drop the pstore on the floor if !irq_fpu_usable().
> 
> 3. Allow the pstore, then die (on the assumption that we're dead
>    anyway.)

Personally, I'd prefer 2, but I'm open to suggestions.

-- 
Matt Fleming, Intel Open Source Technology Center