[PATCH 1/2] efilinux: Allocate boot_params before parsing initrd

2014-06-18 Thread Yinghai Lu
We need to allocate boot_params early and clear it before copy
setup_header to it.

So kernel will not call sanitize_boot_params() to overwrite new
added parameter like ext_ramdisk_image, ext_ramdisk_size...

We should modify boot_params later instead of touch temp buf too early.

Signed-off-by: Yinghai Lu ying...@kernel.org

---
 loaders/bzimage/bzimage.c |   65 --
 1 file changed, 34 insertions(+), 31 deletions(-)

Index: efilinux/loaders/bzimage/bzimage.c
===
--- efilinux.orig/loaders/bzimage/bzimage.c
+++ efilinux/loaders/bzimage/bzimage.c
@@ -50,21 +50,22 @@ struct initrd {
struct file *file;
 };
 
-static void parse_initrd(EFI_LOADED_IMAGE *image, struct boot_params *buf, 
char *cmdline)
+static void parse_initrd(EFI_LOADED_IMAGE *image,
+struct boot_params *boot_params, char *cmdline)
 {
EFI_PHYSICAL_ADDRESS addr;
struct initrd *initrds;
int nr_initrds;
EFI_STATUS err;
-   UINT64 size;
+   UINT64 size = 0;
char *initrd;
int i, j;
 
/*
 * Has there been an initrd specified on the cmdline?
 */
-   buf-hdr.ramdisk_start = 0;
-   buf-hdr.ramdisk_len = 0;
+   boot_params-hdr.ramdisk_start = 0;
+   boot_params-hdr.ramdisk_len = 0;
 
initrd = cmdline;
for (nr_initrds = 0; *initrd; nr_initrds++) {
@@ -96,6 +97,7 @@ static void parse_initrd(EFI_LOADED_IMAG
struct initrd *rd = initrds[i];
struct file *rdfile;
char *o, *p;
+   UINT64 sz;
 
initrd = strstr(initrd, initrd=);
if (!initrd)
@@ -116,26 +118,26 @@ static void parse_initrd(EFI_LOADED_IMAG
if (err != EFI_SUCCESS)
goto close_handles;
 
-   file_size(rdfile, size);
+   file_size(rdfile, sz);
 
-   rd-size = size;
+   rd-size = sz;
rd-file = rdfile;
 
-   buf-hdr.ramdisk_len += size;
+   size += sz;
}
 
-   size = buf-hdr.ramdisk_len;
err = emalloc(size, 0x1000, addr);
if (err != EFI_SUCCESS)
goto close_handles;
 
-   if ((UINTN)addr  buf-hdr.ramdisk_max) {
+   if ((UINTN)addr  boot_params-hdr.ramdisk_max) {
Print(Lramdisk address is too high!\n);
efree(addr, size);
goto close_handles;
}
 
-   buf-hdr.ramdisk_start = (UINT32)(UINTN)addr;
+   boot_params-hdr.ramdisk_start = (UINT32)(UINTN)addr;
+   boot_params-hdr.ramdisk_len = (UINT32)size;
 
for (j = 0; j  nr_initrds; j++) {
struct initrd *rd = initrds[j];
@@ -268,9 +270,6 @@ load_kernel(EFI_HANDLE image, CHAR16 *na
init_size = size * 3;
}
 
-   /* Don't need an allocated ID, we're a prototype */
-   buf-hdr.loader_id = 0x1;
-
/*
 * The kernel expects cmdline to be allocated pretty low,
 * Documentation/x86/boot.txt says,
@@ -287,11 +286,26 @@ load_kernel(EFI_HANDLE image, CHAR16 *na
cmdline = (char *)(UINTN)addr;
memcpy(cmdline, _cmdline, strlen(_cmdline) + 1);
 
-   parse_initrd(info, buf, cmdline);
+   addr = 0x3fff;
+   err = allocate_pages(AllocateMaxAddress, EfiLoaderData,
+EFI_SIZE_TO_PAGES(16384), addr);
+   if (err != EFI_SUCCESS)
+   goto out;
+
+   boot_params = (struct boot_params *)(UINTN)addr;
 
-   buf-hdr.cmd_line_ptr = (UINT32)(UINTN)cmdline;
+   memset((void *)boot_params, 0x0, 16384);
+
+   /* Copy setup_header to boot_params */
+   memcpy((char *)boot_params-hdr, (char *)buf-hdr,
+sizeof(struct setup_header));
 
-   memset((char *)buf-screen_info, 0x0, sizeof(buf-screen_info));
+   /* Don't need an allocated ID, we're a prototype */
+   boot_params-hdr.loader_id = 0x1;
+
+   boot_params-hdr.cmd_line_ptr = (UINT32)(UINTN)cmdline;
+
+   parse_initrd(info, boot_params, cmdline);
 
addr = pref_address;
err = allocate_pages(AllocateAddress, EfiLoaderData,
@@ -301,7 +315,8 @@ load_kernel(EFI_HANDLE image, CHAR16 *na
 * We failed to allocate the preferred address, so
 * just allocate some memory and hope for the best.
 */
-   err = emalloc(init_size, buf-hdr.kernel_alignment, addr);
+   err = emalloc(init_size, boot_params-hdr.kernel_alignment,
+addr);
if (err != EFI_SUCCESS)
goto out;
}
@@ -315,26 +330,14 @@ load_kernel(EFI_HANDLE image, CHAR16 *na
if (err != EFI_SUCCESS)
goto out;
 
-   addr = 0x3fff;
-   err = allocate_pages(AllocateMaxAddress, EfiLoaderData,
-

[PATCH 2/2] efilinux: set ext_ramdisk_* for huge initrd

2014-06-18 Thread Yinghai Lu
We could load it high if it is more than 2G when kernel support
LOAD_ABOVE_4G.

Signed-off-by: Yinghai Lu ying...@kernel.org

---
 loaders/bzimage/bzimage.c |9 -
 loaders/bzimage/bzimage.h |8 ++--
 2 files changed, 14 insertions(+), 3 deletions(-)

Index: efilinux/loaders/bzimage/bzimage.c
===
--- efilinux.orig/loaders/bzimage/bzimage.c
+++ efilinux/loaders/bzimage/bzimage.c
@@ -130,7 +130,9 @@ static void parse_initrd(EFI_LOADED_IMAG
if (err != EFI_SUCCESS)
goto close_handles;
 
-   if ((UINTN)addr  boot_params-hdr.ramdisk_max) {
+   if ((boot_params-hdr.version  0x20c ||
+!(boot_params-hdr.xloadflags  (11))) 
+   (UINTN)addr  boot_params-hdr.ramdisk_max) {
Print(Lramdisk address is too high!\n);
efree(addr, size);
goto close_handles;
@@ -138,6 +140,11 @@ static void parse_initrd(EFI_LOADED_IMAG
 
boot_params-hdr.ramdisk_start = (UINT32)(UINTN)addr;
boot_params-hdr.ramdisk_len = (UINT32)size;
+   if (boot_params-hdr.version = 0x20c 
+   (boot_params-hdr.xloadflags  (11))) {
+   boot_params-ext_ramdisk_image = (UINT64)(UINTN)addr  32;
+   boot_params-ext_ramdisk_size = size  32;
+   }
 
for (j = 0; j  nr_initrds; j++) {
struct initrd *rd = initrds[j];
Index: efilinux/loaders/bzimage/bzimage.h
===
--- efilinux.orig/loaders/bzimage/bzimage.h
+++ efilinux/loaders/bzimage/bzimage.h
@@ -69,7 +69,8 @@ struct setup_header {
UINT32 ramdisk_max;/* Highest legal initrd address */
UINT32 kernel_alignment; /* Physical addr alignment required for kernel 
*/
UINT8 relocatable_kernel; /* Whether kernel is relocatable or not */
-   UINT8 _pad2[3];
+   UINT8 min_alignment;
+   UINT16 xloadflags;
UINT32 cmdline_size;
UINT32 hardware_subarch;
UINT64 hardware_subarch_data;
@@ -148,7 +149,10 @@ struct boot_params {
UINT8 hd1_info[16];
UINT8 sys_desc_table[0x10];
UINT8 olpc_ofw_header[0x10];
-   UINT8 _pad4[128];
+   UINT32 ext_ramdisk_image;
+   UINT32 ext_ramdisk_size;
+   UINT32 ext_cmd_line_ptr;
+   UINT8 _pad4[116];
UINT8 edid_info[0x80];
struct efi_info efi_info;
UINT32 alt_mem_k;
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] pstore: Fix an overflow on 32-bit builds.

2014-06-18 Thread Matt Fleming
On Mon, 09 Jun, at 01:24:43PM, David Rientjes wrote:
 On Mon, 9 Jun 2014, Andrzej Zaborowski wrote:
 
  [resend]
  In generic_id the long int timestamp is multiplied by 10 and needs
  an explicit cast to u64.
  
  Without that the id in the resulting pstore filename is wrong and
  userspace may have problems parsing it, but more importantly files in
  pstore can never be deleted and may fill the EFI flash (brick device?).
  This happens because when generic pstore code wants to delete a file,
  it passes the id to the EFI backend which reinterpretes it and a wrong
  variable name is attempted to be deleted.  There's no error message but
  after remounting pstore, deleted files would reappear.

It shouldn't be possible to brick devices because the efi-pstore code
still goes through efivar_entry_set_safe() whic has the necessary
checks. Please let me know if you've witnessed any fallout from this bug
other than being unable to delete files.

 This fixes commit fdeadb43fdf1 (efi-pstore: Make efi-pstore return a 
 unique id) that went into stable, so I'm not sure if this should go into 
 stable as well.
 
I think it should go to stable too. Tony?

 You probably had to resend this because you didn't email any of the 
 maintainers (fixed).  Use scripts/get_maintainer.pl to figure out who to 
 email about a patch.

Thanks for triaging this David.

Unless anyone speaks up I'm going to throw this into the EFI tree with
David's Acked-by.

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] efi/arm64: efistub: remove local copy of linux_banner

2014-06-18 Thread Matt Fleming
On Fri, 13 Jun, at 01:11:51PM, Ard Biesheuvel wrote:
 The shared efistub code for ARM and arm64 contains a local copy of 
 linux_banner,
 allowing it to be referenced from separate executables such as the ARM
 decompressor. However, this introduces a dependency on generated header files,
 causing unnecessary rebuilds of the stub itself and, in case of arm64, vmlinux
 which contains it.
 
 On arm64, the copy is not actually needed since we can reference the original
 symbol directly, and as it turns out, there may be better ways to deal with 
 this
 for ARM as well, so let's remove it from the shared code. If it still needs to
 be reintroduced for ARM later, it should live under arch/arm anyway and not in
 shared code.

I remember making some similar arguments when this patch was first
introduced. From looking at my notes, the patch rationale was based on
some DT binding discussion where people wanted an explicit way to
identify the kernel they were booting and everyone agreed upon this
string - I don't know which discussion, I don't have that data.

I'm more than happy to delete this from the shared code, I never wanted
it to go in in the first place. But could someone please give me some
details as to why this change is safe and isn't going to break
ARM/ARM64? How does this affect DT bindings?

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] efi: fix pointer type errors in fdt_uefi_find_params()

2014-06-18 Thread Matt Fleming
On Fri, 13 Jun, at 04:54:59PM, Ard Biesheuvel wrote:
 Fix two instances of pointer type errors, a harmless one where a const void*
 value is assigned to a non-const void* variable, and a not-so-harmless one 
 where
 we pass a pointer to unsigned long where a pointer to int is expected.
 
 Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
 ---
  drivers/firmware/efi/efi.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
 index cd36deb619fa..fe737832a882 100644
 --- a/drivers/firmware/efi/efi.c
 +++ b/drivers/firmware/efi/efi.c
 @@ -353,8 +353,8 @@ static int __init fdt_find_uefi_params(unsigned long 
 node, const char *uname,
  int depth, void *data)
  {
   struct param_info *info = data;
 - void *prop, *dest;
 - unsigned long len;
 + void const *prop, *dest;
 + int len;
   u64 val;
   int i;

const void *?

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] efi: fix pointer type errors in fdt_uefi_find_params()

2014-06-18 Thread Ard Biesheuvel
On 18 June 2014 11:02, Matt Fleming m...@console-pimps.org wrote:
 On Fri, 13 Jun, at 04:54:59PM, Ard Biesheuvel wrote:
 Fix two instances of pointer type errors, a harmless one where a const void*
 value is assigned to a non-const void* variable, and a not-so-harmless one 
 where
 we pass a pointer to unsigned long where a pointer to int is expected.

 Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
 ---
  drivers/firmware/efi/efi.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
 index cd36deb619fa..fe737832a882 100644
 --- a/drivers/firmware/efi/efi.c
 +++ b/drivers/firmware/efi/efi.c
 @@ -353,8 +353,8 @@ static int __init fdt_find_uefi_params(unsigned long 
 node, const char *uname,
  int depth, void *data)
  {
   struct param_info *info = data;
 - void *prop, *dest;
 - unsigned long len;
 + void const *prop, *dest;
 + int len;
   u64 val;
   int i;

 const void *?


You say potato, I say po-tah-to?
But seriously, whichever you prefer ...

Regards,
Ard.
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] efi/arm64: efistub: remove local copy of linux_banner

2014-06-18 Thread Ard Biesheuvel
On 18 June 2014 10:50, Matt Fleming m...@console-pimps.org wrote:
 On Fri, 13 Jun, at 01:11:51PM, Ard Biesheuvel wrote:
 The shared efistub code for ARM and arm64 contains a local copy of 
 linux_banner,
 allowing it to be referenced from separate executables such as the ARM
 decompressor. However, this introduces a dependency on generated header 
 files,
 causing unnecessary rebuilds of the stub itself and, in case of arm64, 
 vmlinux
 which contains it.

 On arm64, the copy is not actually needed since we can reference the original
 symbol directly, and as it turns out, there may be better ways to deal with 
 this
 for ARM as well, so let's remove it from the shared code. If it still needs 
 to
 be reintroduced for ARM later, it should live under arch/arm anyway and not 
 in
 shared code.

 I remember making some similar arguments when this patch was first
 introduced. From looking at my notes, the patch rationale was based on
 some DT binding discussion where people wanted an explicit way to
 identify the kernel they were booting and everyone agreed upon this
 string - I don't know which discussion, I don't have that data.

 I'm more than happy to delete this from the shared code, I never wanted
 it to go in in the first place. But could someone please give me some
 details as to why this change is safe and isn't going to break
 ARM/ARM64? How does this affect DT bindings?


This just removes the duplicate definition of linux_banner, *not* the
reference to it a couple of lines down. IOW, the DT bindings and
everything behind it are not affected at all by this patch.
The reason it works fine for arm64 is that its stub is part of the
kernel proper, so the reference will bind to the global definition
instead.

What will be affected is 32-bit ARM, as its stub is not part of the
kernel proper. In this case, we will propose an alternative [and
better] way of allowing the stub to reference linux_banner, i.e.,
without having to redefine it at the C level. We will take this into
account when we propose the next round of efistub patches for ARM.

Regards,
Ard.
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] efi/arm64: efistub: remove local copy of linux_banner

2014-06-18 Thread Matt Fleming
On Wed, 18 Jun, at 11:12:28AM, Ard Biesheuvel wrote:
 
 This just removes the duplicate definition of linux_banner, *not* the
 reference to it a couple of lines down. IOW, the DT bindings and
 everything behind it are not affected at all by this patch.
 The reason it works fine for arm64 is that its stub is part of the
 kernel proper, so the reference will bind to the global definition
 instead.
 
Aha, thanks.

 What will be affected is 32-bit ARM, as its stub is not part of the
 kernel proper. In this case, we will propose an alternative [and
 better] way of allowing the stub to reference linux_banner, i.e.,
 without having to redefine it at the C level. We will take this into
 account when we propose the next round of efistub patches for ARM.

No complaints from me.

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] efi: fix pointer type errors in fdt_uefi_find_params()

2014-06-18 Thread Matt Fleming
On Wed, 18 Jun, at 11:06:09AM, Ard Biesheuvel wrote:
 
 You say potato, I say po-tah-to?
 But seriously, whichever you prefer ...

$ git grep const void | wc -l
4441

$ git grep void const | wc -l
50

I say potato, you say tasty carbohydrate ball

But yeah, it's not a big deal and I can fix this up when applying.

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] efi: fix pointer type errors in fdt_uefi_find_params()

2014-06-18 Thread Ard Biesheuvel
On 18 June 2014 11:28, Matt Fleming m...@console-pimps.org wrote:
 On Wed, 18 Jun, at 11:06:09AM, Ard Biesheuvel wrote:

 You say potato, I say po-tah-to?
 But seriously, whichever you prefer ...

 $ git grep const void | wc -l
 4441

 $ git grep void const | wc -l
 50

 I say potato, you say tasty carbohydrate ball

 But yeah, it's not a big deal and I can fix this up when applying.


Actually, it appears I messed up the subject line too:
s/fdt_uefi_find_params/fdt_find_uefi_params/

Cheers,
Ard.
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 1/7] efi: Use early_mem*() instead of early_io*()

2014-06-18 Thread Matt Fleming
(Pulling in Mark Salter for early_ioremap() knowledge)

On Fri, 13 Jun, at 07:00:17PM, Daniel Kiper wrote:
 Use early_mem*() instead of early_io*() because all mapped EFI regions
 are true RAM not I/O regions. Additionally, I/O family calls do not work
 correctly under Xen in our case. AIUI, early_io*() maps/unmaps real machine
 addresses. However, all artificial EFI structures created under Xen live
 in dom0 memory and should be mapped/unmapped using early_mem*() family
 calls which map domain memory.
 
 Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
 ---
  arch/x86/platform/efi/efi.c |   42 +-
  drivers/firmware/efi/efi.c  |4 ++--
  2 files changed, 23 insertions(+), 23 deletions(-)
 
 diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
 index 87fc96b..dd1e351 100644
 --- a/arch/x86/platform/efi/efi.c
 +++ b/arch/x86/platform/efi/efi.c
 @@ -427,7 +427,7 @@ void __init efi_unmap_memmap(void)
  {
   clear_bit(EFI_MEMMAP, efi.flags);
   if (memmap.map) {
 - early_iounmap(memmap.map, memmap.nr_map * memmap.desc_size);
 + early_memunmap(memmap.map, memmap.nr_map * memmap.desc_size);
   memmap.map = NULL;
   }
  }
 @@ -467,12 +467,12 @@ static int __init efi_systab_init(void *phys)
   if (!data)
   return -ENOMEM;
   }
 - systab64 = early_ioremap((unsigned long)phys,
 -  sizeof(*systab64));
 + systab64 = early_memremap((unsigned long)phys,
 + sizeof(*systab64));

Please don't misalign the arguments :-( This makes the diff harder to
read when all you're doing is changing the function call. You're
indentation isn't an improvement.
 
As Matthew pointed out we may also need to access EFI mapped flash
devices.

Now, the only difference between early_memremap() and early_ioremap(),
at least on x86, is PAGE_KERNEL vs. PAGE_KERNEL_IO, where PAGE_KERNEL_IO
has the additional _PAGE_BIT_IOMAP bit set in the pte. But that's a
software bit for x86.

So, even though this change should be harmless, it's not clear to me why
this change is needed. You say I/O family calls do not work correctly
under Xen in our case, but could you provide some more explanation as
to why they don't work correctly?

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 1/7] efi: Use early_mem*() instead of early_io*()

2014-06-18 Thread David Vrabel
On 18/06/14 13:17, Matt Fleming wrote:
 (Pulling in Mark Salter for early_ioremap() knowledge)
 
 On Fri, 13 Jun, at 07:00:17PM, Daniel Kiper wrote:
 Use early_mem*() instead of early_io*() because all mapped EFI regions
 are true RAM not I/O regions. Additionally, I/O family calls do not work
 correctly under Xen in our case. AIUI, early_io*() maps/unmaps real machine
 addresses. However, all artificial EFI structures created under Xen live
 in dom0 memory and should be mapped/unmapped using early_mem*() family
 calls which map domain memory.

 Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
 ---
  arch/x86/platform/efi/efi.c |   42 
 +-
  drivers/firmware/efi/efi.c  |4 ++--
  2 files changed, 23 insertions(+), 23 deletions(-)

 diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
 index 87fc96b..dd1e351 100644
 --- a/arch/x86/platform/efi/efi.c
 +++ b/arch/x86/platform/efi/efi.c
 @@ -427,7 +427,7 @@ void __init efi_unmap_memmap(void)
  {
  clear_bit(EFI_MEMMAP, efi.flags);
  if (memmap.map) {
 -early_iounmap(memmap.map, memmap.nr_map * memmap.desc_size);
 +early_memunmap(memmap.map, memmap.nr_map * memmap.desc_size);
  memmap.map = NULL;
  }
  }
 @@ -467,12 +467,12 @@ static int __init efi_systab_init(void *phys)
  if (!data)
  return -ENOMEM;
  }
 -systab64 = early_ioremap((unsigned long)phys,
 - sizeof(*systab64));
 +systab64 = early_memremap((unsigned long)phys,
 +sizeof(*systab64));
 
 Please don't misalign the arguments :-( This makes the diff harder to
 read when all you're doing is changing the function call. You're
 indentation isn't an improvement.
  
 As Matthew pointed out we may also need to access EFI mapped flash
 devices.
 
 Now, the only difference between early_memremap() and early_ioremap(),
 at least on x86, is PAGE_KERNEL vs. PAGE_KERNEL_IO, where PAGE_KERNEL_IO
 has the additional _PAGE_BIT_IOMAP bit set in the pte. But that's a
 software bit for x86.

_PAGE_BIT_IOMAP is supposed to be going away...

 So, even though this change should be harmless, it's not clear to me why
 this change is needed. You say I/O family calls do not work correctly
 under Xen in our case, but could you provide some more explanation as
 to why they don't work correctly?

_PAGE_BIT_IOMAP causes a Xen PV guest to skip the PFN to MFN conversion
when building the PTE.  Using it for RAM will attempt to map the wrong
machine frame.

David
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 1/7] efi: Use early_mem*() instead of early_io*()

2014-06-18 Thread Daniel Kiper
On Wed, Jun 18, 2014 at 01:17:09PM +0100, Matt Fleming wrote:
 (Pulling in Mark Salter for early_ioremap() knowledge)

 On Fri, 13 Jun, at 07:00:17PM, Daniel Kiper wrote:
  Use early_mem*() instead of early_io*() because all mapped EFI regions
  are true RAM not I/O regions. Additionally, I/O family calls do not work
  correctly under Xen in our case. AIUI, early_io*() maps/unmaps real machine
  addresses. However, all artificial EFI structures created under Xen live
  in dom0 memory and should be mapped/unmapped using early_mem*() family
  calls which map domain memory.
 
  Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
  ---
   arch/x86/platform/efi/efi.c |   42 
  +-
   drivers/firmware/efi/efi.c  |4 ++--
   2 files changed, 23 insertions(+), 23 deletions(-)
 
  diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
  index 87fc96b..dd1e351 100644
  --- a/arch/x86/platform/efi/efi.c
  +++ b/arch/x86/platform/efi/efi.c
  @@ -427,7 +427,7 @@ void __init efi_unmap_memmap(void)
   {
  clear_bit(EFI_MEMMAP, efi.flags);
  if (memmap.map) {
  -   early_iounmap(memmap.map, memmap.nr_map * memmap.desc_size);
  +   early_memunmap(memmap.map, memmap.nr_map * memmap.desc_size);
  memmap.map = NULL;
  }
   }
  @@ -467,12 +467,12 @@ static int __init efi_systab_init(void *phys)
  if (!data)
  return -ENOMEM;
  }
  -   systab64 = early_ioremap((unsigned long)phys,
  -sizeof(*systab64));
  +   systab64 = early_memremap((unsigned long)phys,
  +   sizeof(*systab64));

 Please don't misalign the arguments :-( This makes the diff harder to
 read when all you're doing is changing the function call. You're
 indentation isn't an improvement.

I think that it improves readability a bit but if you wish I will not
do that in the future.

 As Matthew pointed out we may also need to access EFI mapped flash
 devices.

Right, but I think it does not change a lot in that case. Flash
is simply slower type of memory used as permanent storage.
Am I missing something?

 Now, the only difference between early_memremap() and early_ioremap(),
 at least on x86, is PAGE_KERNEL vs. PAGE_KERNEL_IO, where PAGE_KERNEL_IO
 has the additional _PAGE_BIT_IOMAP bit set in the pte. But that's a
 software bit for x86.

 So, even though this change should be harmless, it's not clear to me why
 this change is needed. You say I/O family calls do not work correctly
 under Xen in our case, but could you provide some more explanation as
 to why they don't work correctly?

As I saw David provided better explanation. If you wish I could include
it in commit message.

Daniel
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 2/7] efi: Introduce EFI_NO_DIRECT flag

2014-06-18 Thread Matt Fleming
On Fri, 13 Jun, at 07:00:18PM, Daniel Kiper wrote:
 Introduce EFI_NO_DIRECT flag. If it is set then kernel runs
 on EFI platform but it has not direct control on EFI stuff
 like EFI runtime, tables, structures, etc. If not this means
 that Linux Kernel has direct access to EFI infrastructure
 and everything runs as usual.
 
 This functionality is used in Xen dom0 because hypervisor
 has full control on EFI stuff and all calls from dom0 to
 EFI must be requested via special hypercall which in turn
 executes relevant EFI code in behalf of dom0.
 
 v5 - suggestions/fixes:
- rename EFI_DIRECT to EFI_NO_DIRECT
  (suggested by David Vrabel),
- limit EFI_NO_DIRECT usage
  (suggested by Jan Beulich and Matt Fleming),
- improve commit message
  (suggested by David Vrabel).
 
 Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
 ---
  arch/x86/platform/efi/efi.c |   27 +--
  drivers/firmware/efi/efi.c  |   22 +-
  include/linux/efi.h |3 ++-
  3 files changed, 36 insertions(+), 16 deletions(-)

[...]

 @@ -617,13 +620,16 @@ static int __init efi_runtime_init(void)
* address of several of the EFI runtime functions, needed to
* set the firmware into virtual mode.
*/
 - if (efi_enabled(EFI_64BIT))
 - rv = efi_runtime_init64();
 - else
 - rv = efi_runtime_init32();
  
 - if (rv)
 - return rv;
 + if (!efi_enabled(EFI_NO_DIRECT)) {
 + if (efi_enabled(EFI_64BIT))
 + rv = efi_runtime_init64();
 + else
 + rv = efi_runtime_init32();
 +
 + if (rv)
 + return rv;
 + }
  
   set_bit(EFI_RUNTIME_SERVICES, efi.flags);
  

This could do with some comments to explain why you want to set
EFI_RUNTIME_SERVICES even though you're skipping efi_runtime_init*(),
e.g. that for Xen things are already mapped.

I'm not likely to remember the rationale for this in 6 months time, and
anyone else hacking on this code that isn't part of this thread also may
not realise at first glance. Comments would go a long way to fixing
that.

 @@ -1220,6 +1232,9 @@ u64 efi_mem_attributes(unsigned long phys_addr)
   efi_memory_desc_t *md;
   void *p;
  
 + if (!efi_enabled(EFI_MEMMAP))
 + return 0;
 +
   for (p = memmap.map; p  memmap.map_end; p += memmap.desc_size) {
   md = p;
   if ((md-phys_addr = phys_addr) 

This should be a separate patch, please.

 diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
 index 023937a..8bb1075 100644
 --- a/drivers/firmware/efi/efi.c
 +++ b/drivers/firmware/efi/efi.c
 @@ -104,16 +104,20 @@ static struct attribute *efi_subsys_attrs[] = {
  static umode_t efi_attr_is_visible(struct kobject *kobj,
  struct attribute *attr, int n)
  {
 - umode_t mode = attr-mode;
 -
 - if (attr == efi_attr_fw_vendor.attr)
 - return (efi.fw_vendor == EFI_INVALID_TABLE_ADDR) ? 0 : mode;
 - else if (attr == efi_attr_runtime.attr)
 - return (efi.runtime == EFI_INVALID_TABLE_ADDR) ? 0 : mode;
 - else if (attr == efi_attr_config_table.attr)
 - return (efi.config_table == EFI_INVALID_TABLE_ADDR) ? 0 : mode;
 + if (attr == efi_attr_fw_vendor.attr) {
 + if (efi_enabled(EFI_NO_DIRECT) ||
 + efi.fw_vendor == EFI_INVALID_TABLE_ADDR)
 + return 0;
 + } else if (attr == efi_attr_runtime.attr) {
 + if (efi_enabled(EFI_NO_DIRECT) ||
 + efi.runtime == EFI_INVALID_TABLE_ADDR)
 + return 0;
 + } else if (attr == efi_attr_config_table.attr) {
 + if (efi.config_table == EFI_INVALID_TABLE_ADDR)
 + return 0;
 + }
  
 - return mode;
 + return attr-mode;
  }
  
Why don't you want to export efi.fw_vendor, etc? Rationale please.

  static struct attribute_group efi_subsys_attr_group = {
 diff --git a/include/linux/efi.h b/include/linux/efi.h
 index 41bbf8b..e917c4a 100644
 --- a/include/linux/efi.h
 +++ b/include/linux/efi.h
 @@ -916,7 +916,8 @@ extern int __init efi_setup_pcdp_console(char *);
  #define EFI_RUNTIME_SERVICES 3   /* Can we use runtime services? */
  #define EFI_MEMMAP   4   /* Can we use EFI memory map? */
  #define EFI_64BIT5   /* Is the firmware 64-bit? */
 -#define EFI_ARCH_1   6   /* First arch-specific bit */
 +#define EFI_NO_DIRECT6   /* Can we access EFI directly? 
 */
 +#define EFI_ARCH_1   7   /* First arch-specific bit */

I like David's suggestion of using EFI_PARAVIRT.

Why the bit shuffling? Are you trying to keep the non-arch bits
together? That does make sense, and I can't help but feel that
EFI_ARCH_1 should probably be bit 31 so we can subtract 1 for each new
arch bit so we don't have to 

Re: [PATCH v5 5/7] arch/x86: Replace plain strings with constants

2014-06-18 Thread Matt Fleming
On Fri, 13 Jun, at 07:00:21PM, Daniel Kiper wrote:
 v5 - suggestions/fixes:
- do not change indentation
  (suggested by Matt Fleming).
 
 Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
 ---
  arch/x86/kernel/setup.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

Looks good, but it's customary to have at least one line after the patch
title. Even something as trivial as,

  We've got constants, so let's use them instead of hard-coded values.

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 6/7] arch/x86: Remove redundant set_bit() call

2014-06-18 Thread Matt Fleming
On Fri, 13 Jun, at 07:00:22PM, Daniel Kiper wrote:
 Remove redundant set_bit(EFI_SYSTEM_TABLES, efi.flags) call.
 It is executed earlier in efi_systab_init().
 
 Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
 ---
  arch/x86/platform/efi/efi.c |2 --
  1 file changed, 2 deletions(-)

Looks good!

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 2/7] efi: Introduce EFI_NO_DIRECT flag

2014-06-18 Thread Jan Beulich
 On 18.06.14 at 15:52, m...@console-pimps.org wrote:
 EFI_PARAVIRT will be usable by architectures other than x86, correct? If
 your intention is for it only ever to be used by x86, then it should
 probably be EFI_ARCH_2.

I would expect ARM, once it gets UEFI support on the Xen side, to
be able to handle most of this identically to x86. Which raises the
question whether most of the new Xen-specific code (in one of the
other patches) wouldn't better live under drivers/xen/.

Jan

--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 6/7] arch/x86: Remove redundant set_bit() call

2014-06-18 Thread Konrad Rzeszutek Wilk
On Wed, Jun 18, 2014 at 02:56:37PM +0100, Matt Fleming wrote:
 On Fri, 13 Jun, at 07:00:22PM, Daniel Kiper wrote:
  Remove redundant set_bit(EFI_SYSTEM_TABLES, efi.flags) call.
  It is executed earlier in efi_systab_init().
  
  Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
  ---
   arch/x86/platform/efi/efi.c |2 --
   1 file changed, 2 deletions(-)
 
 Looks good!

Is that an Acked-by or Reviewed-by ? :-)
 
 -- 
 Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 1/7] efi: Use early_mem*() instead of early_io*()

2014-06-18 Thread Mark Salter
On Wed, 2014-06-18 at 13:17 +0100, Matt Fleming wrote:
 Now, the only difference between early_memremap() and early_ioremap(),
 at least on x86, is PAGE_KERNEL vs. PAGE_KERNEL_IO, where PAGE_KERNEL_IO
 has the additional _PAGE_BIT_IOMAP bit set in the pte. But that's a
 software bit for x86.

I can't comment on the x86 MMU details, but using early_memremap() will
get rid of some sparse warnings in the current code.


--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 7/7] arch/x86: Comment out efi_set_rtc_mmss()

2014-06-18 Thread Matt Fleming
On Fri, 13 Jun, at 07:00:23PM, Daniel Kiper wrote:
 efi_set_rtc_mmss() is never used to set RTC due to bugs found
 on many EFI platforms. It is set directly by mach_set_rtc_mmss().
 
 Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
 ---
  arch/x86/platform/efi/efi.c |6 ++
  1 file changed, 6 insertions(+)
 
 diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
 index deb4f05..bd3e080 100644
 --- a/arch/x86/platform/efi/efi.c
 +++ b/arch/x86/platform/efi/efi.c
 @@ -244,6 +244,11 @@ static efi_status_t __init 
 phys_efi_set_virtual_address_map(
   return status;
  }
  
 +#if 0
 +/*
 + * efi_set_rtc_mmss() is never used to set RTC due to bugs found
 + * on many EFI platforms. It is set directly by mach_set_rtc_mmss().
 + */
  int efi_set_rtc_mmss(const struct timespec *now)
  {
   unsigned long nowtime = now-tv_sec;
 @@ -279,6 +284,7 @@ int efi_set_rtc_mmss(const struct timespec *now)
   }
   return 0;
  }
 +#endif
  
  void efi_get_time(struct timespec *now)
  {

As others have said in this thread - just delete this sucker.

I did make an attempt to get rid of all the time functions recently, but
chickened out at the last minute because we were nearing the merge
window,

  http://article.gmane.org/gmane.linux.kernel.janitors/30082

So yes, please go ahead and delete this function.

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 6/7] arch/x86: Remove redundant set_bit() call

2014-06-18 Thread Matt Fleming
On Wed, 18 Jun, at 10:01:01AM, Konrad Rzeszutek Wilk wrote:
 On Wed, Jun 18, 2014 at 02:56:37PM +0100, Matt Fleming wrote:
  On Fri, 13 Jun, at 07:00:22PM, Daniel Kiper wrote:
   Remove redundant set_bit(EFI_SYSTEM_TABLES, efi.flags) call.
   It is executed earlier in efi_systab_init().
   
   Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
   ---
arch/x86/platform/efi/efi.c |2 --
1 file changed, 2 deletions(-)
  
  Looks good!
 
 Is that an Acked-by or Reviewed-by ? :-)

Since I assumed this series would be going through my tree, I didn't
think it mattered.

Reviewed-by: Matt Fleming matt.flem...@intel.com

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 0/7] xen: Add EFI support

2014-06-18 Thread Matt Fleming
On Fri, 13 Jun, at 07:00:16PM, Daniel Kiper wrote:
 Hey,
 
 This patch series adds EFI support for Xen dom0 guests.
 It is based on Jan Beulich and Tang Liang work. I was
 trying to take into account all previous comments,
 however, if I missed something sorry for that.
 
 Daniel
 
  arch/x86/kernel/setup.c  |4 +-
  arch/x86/platform/efi/efi.c  |   77 ++---
  arch/x86/xen/enlighten.c |   24 ++
  drivers/firmware/efi/efi.c   |   26 +++---
  drivers/xen/Kconfig  |4 +
  drivers/xen/Makefile |3 +
  drivers/xen/efi.c|  374 
 +++
  include/linux/efi.h  |3 +-
  include/xen/interface/platform.h |  123 +++
  9 files changed, 595 insertions(+), 43 deletions(-)

FWIW I'm much happier with this version than v4. There's nothing that
jumps out at me as being obviously wrong. Just a few minor cleanups
needed.

Which tree is this intended to go through? I'm more than happy to take
it through the EFI tree, particularly since it touches
include/linux/efi.h and drivers/firmware/efi/efi.c which is the core EFI
interface for all platforms.

But if it's going through the Xen tree or something, I can supply my
Acked-by's (once the aforementioned minor cleanups are taken care of).

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] x86, eboot: Support initrd loaded above 4G

2014-06-18 Thread Matt Fleming
On Sat, 14 Jun, at 12:23:41PM, Yinghai Lu wrote:
 For boot efi kernel directly without bootloader.
 If the kernel support XLF_CAN_BE_LOADED_ABOVE_4G, we should
 not limit initrd under hdr-initrd_add_max.
 
 Signed-off-by: Yinghai Lu ying...@kernel.org
 
 ---
  arch/x86/boot/compressed/eboot.c |   14 +++---
  1 file changed, 11 insertions(+), 3 deletions(-)

Applied, thanks!

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 2/7] efi: Introduce EFI_NO_DIRECT flag

2014-06-18 Thread Stefano Stabellini
On Wed, 18 Jun 2014, Jan Beulich wrote:
  On 18.06.14 at 15:52, m...@console-pimps.org wrote:
  EFI_PARAVIRT will be usable by architectures other than x86, correct? If
  your intention is for it only ever to be used by x86, then it should
  probably be EFI_ARCH_2.
 
 I would expect ARM, once it gets UEFI support on the Xen side, to
 be able to handle most of this identically to x86. Which raises the
 question whether most of the new Xen-specific code (in one of the
 other patches) wouldn't better live under drivers/xen/.

I was thinking the same thing.

However this patch series doesn't add much code outside
drivers/xen/efi.c and include/xen/interface/platform.h.
I think it wouldn't be fair to ask Daniel to refactor the efi code
currently under arch/x86 to an arch-independent location.
Whoever comes in later and adds EFI Xen support for ARM can do that.
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 0/7] xen: Add EFI support

2014-06-18 Thread Konrad Rzeszutek Wilk
On Wed, Jun 18, 2014 at 03:15:41PM +0100, Matt Fleming wrote:
 On Fri, 13 Jun, at 07:00:16PM, Daniel Kiper wrote:
  Hey,
  
  This patch series adds EFI support for Xen dom0 guests.
  It is based on Jan Beulich and Tang Liang work. I was
  trying to take into account all previous comments,
  however, if I missed something sorry for that.
  
  Daniel
  
   arch/x86/kernel/setup.c  |4 +-
   arch/x86/platform/efi/efi.c  |   77 ++---
   arch/x86/xen/enlighten.c |   24 ++
   drivers/firmware/efi/efi.c   |   26 +++---
   drivers/xen/Kconfig  |4 +
   drivers/xen/Makefile |3 +
   drivers/xen/efi.c|  374 
  +++
   include/linux/efi.h  |3 +-
   include/xen/interface/platform.h |  123 +++
   9 files changed, 595 insertions(+), 43 deletions(-)
 
 FWIW I'm much happier with this version than v4. There's nothing that
 jumps out at me as being obviously wrong. Just a few minor cleanups
 needed.

Fantastic!
 
 Which tree is this intended to go through? I'm more than happy to take
 it through the EFI tree, particularly since it touches
 include/linux/efi.h and drivers/firmware/efi/efi.c which is the core EFI
 interface for all platforms.

Your tree is perfect. I presume you do some automatic regression testing
when you have accumulated a tons of patch - so if any of them cause havoc
we can catch them before the merge window and fix them.

 
 But if it's going through the Xen tree or something, I can supply my
 Acked-by's (once the aforementioned minor cleanups are taken care of).

Nah, lets do your tree. And either Boris, me or David will supply
the Acked-by on the drivers/xen* and arch/x86/xen/* so all the
right tags are in place.

Thanks again!
 
 -- 
 Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] arm64: support reboot and power off via EFI runtime

2014-06-18 Thread Mark Salter
On Wed, 2014-06-18 at 09:09 -0500, Rob Herring wrote:
 On Tue, Jun 17, 2014 at 11:45 AM, Mark Salter msal...@redhat.com wrote:
  Add handlers for arm_pm_resestart and pm_power_off which use EFI
 
 typo.
 
  runtime services ResetSystem call to perform the functions. These
  handlers are only installed if no handler currently exists. This
  allows PSCI to take priority over EFI for these functions.
 
  Signed-off-by: Mark Salter msal...@redhat.com
  ---
   arch/arm64/kernel/efi.c | 40 
 
 Where's the arm32 version? Surely this could be shared with all
 arches. pm_power_off is at least architecturally independent. We
 should do the same for restart/reboot. I'm not saying do that now, but
 at least put it in the right place for that to happen.
 
 Rob

Good point. So the right place would be drivers/firmware/efi I think.
Add a CONFIG_EFI_RESET with default off. I suppose we could also
go ahead and s/arm_pm_restart/pm_restart/ kernel-wide or have an
ifdef in the code (or depends on ARM64 || ARM in Kconfig) until we
do so.

[Matt Fleming CC'd]

 
   1 file changed, 40 insertions(+)
 
  diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
  index 14db1f6..e8c0476 100644
  --- a/arch/arm64/kernel/efi.c
  +++ b/arch/arm64/kernel/efi.c
  @@ -19,11 +19,14 @@
   #include linux/of_fdt.h
   #include linux/sched.h
   #include linux/slab.h
  +#include linux/reboot.h
  +#include linux/pm.h
 
   #include asm/cacheflush.h
   #include asm/efi.h
   #include asm/tlbflush.h
   #include asm/mmu_context.h
  +#include asm/system_misc.h
 
   struct efi_memory_map memmap;
 
  @@ -467,3 +470,40 @@ static int __init arm64_enter_virtual_mode(void)
  return 0;
   }
   early_initcall(arm64_enter_virtual_mode);
  +
  +static void efi_restart(enum reboot_mode reboot_mode, const char *cmd)
  +{
  +   int efi_mode;
  +
  +   switch (reboot_mode) {
  +   case REBOOT_WARM:
  +   case REBOOT_SOFT:
  +   efi_mode = EFI_RESET_WARM;
  +   break;
  +   default:
  +   efi_mode = EFI_RESET_COLD;
  +   break;
  +   }
  +   efi.reset_system(efi_mode, 0, 0, NULL);
  +}
  +
  +static void efi_power_off(void)
  +{
  +   efi.reset_system(EFI_RESET_SHUTDOWN, 0, 0, NULL);
  +}
  +
  +static int __init setup_efi_reset(void)
  +{
  +   if (efi_enabled(EFI_RUNTIME_SERVICES)) {
  +   /*
  +* If something (psci, etc) hasn't already registered
  +* a handler, use EFI.
  +*/
  +   if (arm_pm_restart == NULL)
  +   arm_pm_restart = efi_restart;
  +   if (pm_power_off == NULL)
  +   pm_power_off = efi_power_off;
  +   }
  +   return 0;
  +}
  +pure_initcall(setup_efi_reset);
  --
  1.9.0
 
 
  ___
  linux-arm-kernel mailing list
  linux-arm-ker...@lists.infradead.org
  http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 0/7] xen: Add EFI support

2014-06-18 Thread Matt Fleming
On Wed, 18 Jun, at 10:31:43AM, Konrad Rzeszutek Wilk wrote:
 
 Your tree is perfect. I presume you do some automatic regression testing
 when you have accumulated a tons of patch - so if any of them cause havoc
 we can catch them before the merge window and fix them.
 
Yeah, I run regression tests to catch breakage to EFI platforms, usually
as soon as I apply them.

 Nah, lets do your tree. And either Boris, me or David will supply
 the Acked-by on the drivers/xen* and arch/x86/xen/* so all the
 right tags are in place.
 
Sounds like a plan.

 Thanks again!

My pleasure.

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 1/7] efi: Use early_mem*() instead of early_io*()

2014-06-18 Thread Matt Fleming
On Wed, 18 Jun, at 02:59:57PM, Daniel Kiper wrote:
 
 I think that it improves readability a bit but if you wish I will not
 do that in the future.
 
That would be much appreciated.

  As Matthew pointed out we may also need to access EFI mapped flash
  devices.
 
 Right, but I think it does not change a lot in that case. Flash
 is simply slower type of memory used as permanent storage.
 Am I missing something?
 
No you're not missing anything, I hit send too quickly while typing my
reply so the above isn't a complete thought. I don't think this
distinction between mapped flash and RAM matters in this case.

 As I saw David provided better explanation. If you wish I could include
 it in commit message.

Yeah, I think that would be very helpful, thanks!

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 2/7] efi: Introduce EFI_NO_DIRECT flag

2014-06-18 Thread Daniel Kiper
On Wed, Jun 18, 2014 at 03:30:25PM +0100, Stefano Stabellini wrote:
 On Wed, 18 Jun 2014, Jan Beulich wrote:
   On 18.06.14 at 15:52, m...@console-pimps.org wrote:
   EFI_PARAVIRT will be usable by architectures other than x86, correct? If
   your intention is for it only ever to be used by x86, then it should
   probably be EFI_ARCH_2.
 
  I would expect ARM, once it gets UEFI support on the Xen side, to
  be able to handle most of this identically to x86. Which raises the
  question whether most of the new Xen-specific code (in one of the
  other patches) wouldn't better live under drivers/xen/.

 I was thinking the same thing.

 However this patch series doesn't add much code outside
 drivers/xen/efi.c and include/xen/interface/platform.h.
 I think it wouldn't be fair to ask Daniel to refactor the efi code
 currently under arch/x86 to an arch-independent location.
 Whoever comes in later and adds EFI Xen support for ARM can do that.

I was doing those EFI patches with ARM support in mind from the beginning.
That is why I moved everything, which was possible, to arch independent
place. There are only some simple init calls from arch/x86. Just a few
required lines. So I think that it will be quite easy to use this EFI
code on ARM platform.

Daniel
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 2/7] efi: Introduce EFI_NO_DIRECT flag

2014-06-18 Thread Matt Fleming
On Wed, 18 Jun, at 03:30:25PM, Stefano Stabellini wrote:
 
 I was thinking the same thing.
 
 However this patch series doesn't add much code outside
 drivers/xen/efi.c and include/xen/interface/platform.h.
 I think it wouldn't be fair to ask Daniel to refactor the efi code
 currently under arch/x86 to an arch-independent location.
 Whoever comes in later and adds EFI Xen support for ARM can do that.

I agree that we shouldn't necessarily build tons of arch-independent
infrastructure when there's only one user, but it is wise to be careful
about the design decisions being made if you know there's another user
coming around the corner.

What we don't want to happen, is for the designs that we have now to
become set in stone such that they can never be changed, to the
deteriment of code duplication and maintenance.

I don't think that's happening in this patch series, but it's definitely
smart discussing this stuff upfront.

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] arm64: support reboot and power off via EFI runtime

2014-06-18 Thread Matt Fleming
On Wed, 18 Jun, at 10:41:47AM, Mark Salter wrote:
 On Wed, 2014-06-18 at 09:09 -0500, Rob Herring wrote:
  On Tue, Jun 17, 2014 at 11:45 AM, Mark Salter msal...@redhat.com wrote:
   Add handlers for arm_pm_resestart and pm_power_off which use EFI
  
  typo.
  
   runtime services ResetSystem call to perform the functions. These
   handlers are only installed if no handler currently exists. This
   allows PSCI to take priority over EFI for these functions.
  
   Signed-off-by: Mark Salter msal...@redhat.com
   ---
arch/arm64/kernel/efi.c | 40 
  
  Where's the arm32 version? Surely this could be shared with all
  arches. pm_power_off is at least architecturally independent. We
  should do the same for restart/reboot. I'm not saying do that now, but
  at least put it in the right place for that to happen.
  
  Rob
 
 Good point. So the right place would be drivers/firmware/efi I think.
 Add a CONFIG_EFI_RESET with default off. I suppose we could also
 go ahead and s/arm_pm_restart/pm_restart/ kernel-wide or have an
 ifdef in the code (or depends on ARM64 || ARM in Kconfig) until we
 do so.
 
 [Matt Fleming CC'd]
 
Heh, this is timely. I wrote some patches last week to use
EfiResetSystem() for ACPI Hardware Reduced platforms,

  http://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=efi/reboot

so it would definitely be good if we could try and solve both of our
problems simultaneously.

Let me post that patch series to kickoff the discussion.

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 13/13] kexec: Support kexec/kdump on EFI systems

2014-06-18 Thread Borislav Petkov
Addomg linux-efi to CC for the efi bits. Please CC it on your next
submission.

Thanks.

On Tue, Jun 03, 2014 at 09:07:02AM -0400, Vivek Goyal wrote:
 This patch does two thigns. It passes EFI run time mappings to second
 kernel in bootparams efi_info. Second kernel parse this info and create
 new mappings in second kernel. That means mappings in first and second
 kernel will be same. This paves the way to enable EFI in kexec kernel.
 
 This patch also prepares and passes EFI setup data through bootparams.
 This contains bunch of information about various tables and their
 addresses.
 
 These information gathering and passing has been written along the lines
 of what current kexec-tools is doing to make kexec work with UEFI.
 
 Signed-off-by: Vivek Goyal vgo...@redhat.com
 ---
  arch/x86/include/asm/kexec.h   |  4 +-
  arch/x86/kernel/kexec-bzimage.c| 40 
  arch/x86/kernel/machine_kexec.c| 93 
 --
  drivers/firmware/efi/runtime-map.c | 21 +
  include/linux/efi.h| 19 
  5 files changed, 163 insertions(+), 14 deletions(-)
 
 diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
 index 4cbe5f7..d8461cf 100644
 --- a/arch/x86/include/asm/kexec.h
 +++ b/arch/x86/include/asm/kexec.h
 @@ -214,7 +214,9 @@ extern int kexec_setup_cmdline(struct kimage *image,
   unsigned long cmdline_offset, char *cmdline,
   unsigned long cmdline_len);
  extern int kexec_setup_boot_parameters(struct kimage *image,
 - struct boot_params *params);
 + struct boot_params *params, unsigned long params_load_addr,
 + unsigned int efi_map_offset, unsigned int efi_map_sz,
 + unsigned int efi_setup_data_offset);
  
  
  #endif /* __ASSEMBLY__ */
 diff --git a/arch/x86/kernel/kexec-bzimage.c b/arch/x86/kernel/kexec-bzimage.c
 index 8e762d3..55716e1 100644
 --- a/arch/x86/kernel/kexec-bzimage.c
 +++ b/arch/x86/kernel/kexec-bzimage.c
 @@ -15,10 +15,12 @@
  #include linux/kexec.h
  #include linux/kernel.h
  #include linux/mm.h
 +#include linux/efi.h
  
  #include asm/bootparam.h
  #include asm/setup.h
  #include asm/crash.h
 +#include asm/efi.h
  
  #define MAX_ELFCOREHDR_STR_LEN   30  /* elfcorehdr=0x64bit-value */
  
 @@ -106,7 +108,7 @@ void *bzImage64_load(struct kimage *image, char *kernel,
  
   struct setup_header *header;
   int setup_sects, kern16_size, ret = 0;
 - unsigned long setup_header_size, params_cmdline_sz;
 + unsigned long setup_header_size, params_cmdline_sz, params_misc_sz;
   struct boot_params *params;
   unsigned long bootparam_load_addr, kernel_load_addr, initrd_load_addr;
   unsigned long purgatory_load_addr;
 @@ -116,6 +118,7 @@ void *bzImage64_load(struct kimage *image, char *kernel,
   struct kexec_entry64_regs regs64;
   void *stack;
   unsigned int setup_hdr_offset = offsetof(struct boot_params, hdr);
 + unsigned int efi_map_offset, efi_map_sz, efi_setup_data_offset;
  
   header = (struct setup_header *)(kernel + setup_hdr_offset);
   setup_sects = header-setup_sects;
 @@ -168,28 +171,47 @@ void *bzImage64_load(struct kimage *image, char *kernel,
  
   pr_debug(Loaded purgatory at 0x%lx\n, purgatory_load_addr);
  
 - /* Load Bootparams and cmdline */
 +
 + /*
 +  * Load Bootparams and cmdline and space for efi stuff.
 +  *
 +  * Allocate memory together for multiple data structures so
 +  * that they all can go in single area/segment and we don't
 +  * have to create separate segment for each. Keeps things
 +  * little bit simple
 +  */
 + efi_map_sz = get_efi_runtime_map_size();
 + efi_map_sz = ALIGN(efi_map_sz, 16);
 +
   params_cmdline_sz = sizeof(struct boot_params) + cmdline_len +
   MAX_ELFCOREHDR_STR_LEN;
 - params = kzalloc(params_cmdline_sz, GFP_KERNEL);
 + params_cmdline_sz = ALIGN(params_cmdline_sz, 16);
 + params_misc_sz = params_cmdline_sz + efi_map_sz +
 + sizeof(struct setup_data) +
 + sizeof(struct efi_setup_data);
 +
 + params = kzalloc(params_misc_sz, GFP_KERNEL);
   if (!params) {
   ret = -ENOMEM;
   goto out_free_loader_data;
   }
  
 + efi_map_offset = params_cmdline_sz;
 + efi_setup_data_offset = efi_map_offset + efi_map_sz;
 +
   /* Copy setup header onto bootparams. Documentation/x86/boot.txt */
   setup_header_size = 0x0202 + kernel[0x0201] - setup_hdr_offset;
  
   /* Is there a limit on setup header size? */
   memcpy(params-hdr, (kernel + setup_hdr_offset), setup_header_size);
  
 - ret = kexec_add_buffer(image, (char *)params, params_cmdline_sz,
 -params_cmdline_sz, 16, MIN_BOOTPARAM_ADDR,
 + ret = kexec_add_buffer(image, (char *)params, params_misc_sz,
 +

Re: [PATCH v5 2/7] efi: Introduce EFI_NO_DIRECT flag

2014-06-18 Thread Daniel Kiper
On Wed, Jun 18, 2014 at 02:52:29PM +0100, Matt Fleming wrote:
 On Fri, 13 Jun, at 07:00:18PM, Daniel Kiper wrote:
  Introduce EFI_NO_DIRECT flag. If it is set then kernel runs
  on EFI platform but it has not direct control on EFI stuff
  like EFI runtime, tables, structures, etc. If not this means
  that Linux Kernel has direct access to EFI infrastructure
  and everything runs as usual.
 
  This functionality is used in Xen dom0 because hypervisor
  has full control on EFI stuff and all calls from dom0 to
  EFI must be requested via special hypercall which in turn
  executes relevant EFI code in behalf of dom0.
 
  v5 - suggestions/fixes:
 - rename EFI_DIRECT to EFI_NO_DIRECT
   (suggested by David Vrabel),
 - limit EFI_NO_DIRECT usage
   (suggested by Jan Beulich and Matt Fleming),
 - improve commit message
   (suggested by David Vrabel).
 
  Signed-off-by: Daniel Kiper daniel.ki...@oracle.com
  ---
   arch/x86/platform/efi/efi.c |   27 +--
   drivers/firmware/efi/efi.c  |   22 +-
   include/linux/efi.h |3 ++-
   3 files changed, 36 insertions(+), 16 deletions(-)

 [...]

  @@ -617,13 +620,16 @@ static int __init efi_runtime_init(void)
   * address of several of the EFI runtime functions, needed to
   * set the firmware into virtual mode.
   */
  -   if (efi_enabled(EFI_64BIT))
  -   rv = efi_runtime_init64();
  -   else
  -   rv = efi_runtime_init32();
 
  -   if (rv)
  -   return rv;
  +   if (!efi_enabled(EFI_NO_DIRECT)) {
  +   if (efi_enabled(EFI_64BIT))
  +   rv = efi_runtime_init64();
  +   else
  +   rv = efi_runtime_init32();
  +
  +   if (rv)
  +   return rv;
  +   }
 
  set_bit(EFI_RUNTIME_SERVICES, efi.flags);
 

 This could do with some comments to explain why you want to set
 EFI_RUNTIME_SERVICES even though you're skipping efi_runtime_init*(),
 e.g. that for Xen things are already mapped.

 I'm not likely to remember the rationale for this in 6 months time, and
 anyone else hacking on this code that isn't part of this thread also may
 not realise at first glance. Comments would go a long way to fixing
 that.

OK, I will add relevant comment here.

  @@ -1220,6 +1232,9 @@ u64 efi_mem_attributes(unsigned long phys_addr)
  efi_memory_desc_t *md;
  void *p;
 
  +   if (!efi_enabled(EFI_MEMMAP))
  +   return 0;
  +
  for (p = memmap.map; p  memmap.map_end; p += memmap.desc_size) {
  md = p;
  if ((md-phys_addr = phys_addr) 

 This should be a separate patch, please.

OK. I was not sure about that.

  diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
  index 023937a..8bb1075 100644
  --- a/drivers/firmware/efi/efi.c
  +++ b/drivers/firmware/efi/efi.c
  @@ -104,16 +104,20 @@ static struct attribute *efi_subsys_attrs[] = {
   static umode_t efi_attr_is_visible(struct kobject *kobj,
 struct attribute *attr, int n)
   {
  -   umode_t mode = attr-mode;
  -
  -   if (attr == efi_attr_fw_vendor.attr)
  -   return (efi.fw_vendor == EFI_INVALID_TABLE_ADDR) ? 0 : mode;
  -   else if (attr == efi_attr_runtime.attr)
  -   return (efi.runtime == EFI_INVALID_TABLE_ADDR) ? 0 : mode;
  -   else if (attr == efi_attr_config_table.attr)
  -   return (efi.config_table == EFI_INVALID_TABLE_ADDR) ? 0 : mode;
  +   if (attr == efi_attr_fw_vendor.attr) {
  +   if (efi_enabled(EFI_NO_DIRECT) ||
  +   efi.fw_vendor == EFI_INVALID_TABLE_ADDR)
  +   return 0;
  +   } else if (attr == efi_attr_runtime.attr) {
  +   if (efi_enabled(EFI_NO_DIRECT) ||
  +   efi.runtime == EFI_INVALID_TABLE_ADDR)
  +   return 0;
  +   } else if (attr == efi_attr_config_table.attr) {
  +   if (efi.config_table == EFI_INVALID_TABLE_ADDR)
  +   return 0;
  +   }
 
  -   return mode;
  +   return attr-mode;
   }

 Why don't you want to export efi.fw_vendor, etc? Rationale please.

I am exporting real addresses (machine addresses) of things which
I am able to get. Stuff which was created artificially and lives
in dom0 address space or does not exist are not exported.

   static struct attribute_group efi_subsys_attr_group = {
  diff --git a/include/linux/efi.h b/include/linux/efi.h
  index 41bbf8b..e917c4a 100644
  --- a/include/linux/efi.h
  +++ b/include/linux/efi.h
  @@ -916,7 +916,8 @@ extern int __init efi_setup_pcdp_console(char *);
   #define EFI_RUNTIME_SERVICES   3   /* Can we use runtime services? 
  */
   #define EFI_MEMMAP 4   /* Can we use EFI memory map? */
   #define EFI_64BIT  5   /* Is the firmware 64-bit? */
  -#define EFI_ARCH_1 6   /* First arch-specific bit */
  +#define EFI_NO_DIRECT  6   /* Can we access EFI directly? 
  */
  +#define