Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-29 Thread Dave Young
On 12/29/13 at 01:05pm, Matt Fleming wrote:
> On Wed, 25 Dec, at 11:32:05AM, Dave Young wrote:
> > 
> > Matt, if you want to remove the map[0] please fold below one line
> > patch? I have no strong opinion.
> > 
> > Index: linux/arch/x86/include/asm/efi.h
> > ===
> > --- linux.orig/arch/x86/include/asm/efi.h
> > +++ linux/arch/x86/include/asm/efi.h
> > @@ -139,7 +139,6 @@ struct efi_setup_data {
> > u64 tables;
> > u64 smbios;
> > u64 reserved[8];
> > -   efi_memory_desc_t map[0];
> >  };
> >  
> >  extern u64 efi_setup;
> 
> Thanks Dave, I will fold this in. I would suggest updating your
> kexec-tools patches so that the two structures are in sync.

Sure, since it is removed in kernel I will update kexec-tools.

Thanks
Dave
> 
> -- 
> Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-29 Thread Matt Fleming
On Wed, 25 Dec, at 11:32:05AM, Dave Young wrote:
> 
> Matt, if you want to remove the map[0] please fold below one line
> patch? I have no strong opinion.
> 
> Index: linux/arch/x86/include/asm/efi.h
> ===
> --- linux.orig/arch/x86/include/asm/efi.h
> +++ linux/arch/x86/include/asm/efi.h
> @@ -139,7 +139,6 @@ struct efi_setup_data {
>   u64 tables;
>   u64 smbios;
>   u64 reserved[8];
> - efi_memory_desc_t map[0];
>  };
>  
>  extern u64 efi_setup;

Thanks Dave, I will fold this in. I would suggest updating your
kexec-tools patches so that the two structures are in sync.

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


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-29 Thread Matt Fleming
On Wed, 25 Dec, at 11:32:05AM, Dave Young wrote:
 
 Matt, if you want to remove the map[0] please fold below one line
 patch? I have no strong opinion.
 
 Index: linux/arch/x86/include/asm/efi.h
 ===
 --- linux.orig/arch/x86/include/asm/efi.h
 +++ linux/arch/x86/include/asm/efi.h
 @@ -139,7 +139,6 @@ struct efi_setup_data {
   u64 tables;
   u64 smbios;
   u64 reserved[8];
 - efi_memory_desc_t map[0];
  };
  
  extern u64 efi_setup;

Thanks Dave, I will fold this in. I would suggest updating your
kexec-tools patches so that the two structures are in sync.

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


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-29 Thread Dave Young
On 12/29/13 at 01:05pm, Matt Fleming wrote:
 On Wed, 25 Dec, at 11:32:05AM, Dave Young wrote:
  
  Matt, if you want to remove the map[0] please fold below one line
  patch? I have no strong opinion.
  
  Index: linux/arch/x86/include/asm/efi.h
  ===
  --- linux.orig/arch/x86/include/asm/efi.h
  +++ linux/arch/x86/include/asm/efi.h
  @@ -139,7 +139,6 @@ struct efi_setup_data {
  u64 tables;
  u64 smbios;
  u64 reserved[8];
  -   efi_memory_desc_t map[0];
   };
   
   extern u64 efi_setup;
 
 Thanks Dave, I will fold this in. I would suggest updating your
 kexec-tools patches so that the two structures are in sync.

Sure, since it is removed in kernel I will update kexec-tools.

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


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-24 Thread Dave Young
On 12/25/13 at 11:12am, Dave Young wrote:
> On 12/23/13 at 08:07am, Matt Fleming wrote:
> > On Mon, 23 Dec, at 10:09:58AM, Dave Young wrote:
> > > On 12/21/13 at 04:06pm, Matt Fleming wrote:
> > > > On Fri, 20 Dec, at 06:02:19PM, Dave Young wrote:
> > > > > @@ -133,6 +133,19 @@ extern void efi_sync_low_kernel_mappings(void);
> > > > >  extern void efi_setup_page_tables(void);
> > > > >  extern void __init old_map_region(efi_memory_desc_t *md);
> > > > >  
> > > > > +struct efi_setup_data {
> > > > > + u64 fw_vendor;
> > > > > + u64 runtime;
> > > > > + u64 tables;
> > > > > + u64 smbios;
> > > > > + u64 reserved[8];
> > > > > + efi_memory_desc_t map[0];
> > > > > +};
> > > > 
> > > > [...]
> > > > 
> > > > > +static void get_nr_runtime_map(void)
> > > > > +{
> > > > > + if (!efi_setup)
> > > > > + return;
> > > > > +
> > > > > + nr_efi_runtime_map = (efi_data_len - sizeof(struct 
> > > > > efi_setup_data)) /
> > > > > +  sizeof(efi_memory_desc_t);
> > > > > +}
> > > > 
> > > > Do we actually need the 'map' entry in efi_setup_data now that you're
> > > > passing it via efi_info (which is much better approach!)? Also, we don't
> > > > need the global nr_efi_runtime_map or efi_runtime_map variables now,
> > > > right?
> > > 
> > > The map is still necessary because we need store the map somewhere and 
> > > pass
> > > the physicall address to kexec kernel. Passing them in setup_data is the
> > > only better way currently...
> > > 
> > > In efi_info there's only an entry for the map physical address, the 
> > > original
> > > map area is not valid any more.
> > 
> > Where do you dereference efi_setup_data.map* in the kexec kernel?
> 
> The field itself is not dereferenced in kernel but it's still used in 
> userspace
> code for collecting and saving runtime maps. Since I still append the map at 
> the
> end of efi_setup_data so I think keep the map[0] is ok. What's your opinion?

Matt, if you want to remove the map[0] please fold below one line patch? I have 
no
strong opinion.

Index: linux/arch/x86/include/asm/efi.h
===
--- linux.orig/arch/x86/include/asm/efi.h
+++ linux/arch/x86/include/asm/efi.h
@@ -139,7 +139,6 @@ struct efi_setup_data {
u64 tables;
u64 smbios;
u64 reserved[8];
-   efi_memory_desc_t map[0];
 };
 
 extern u64 efi_setup;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-24 Thread Dave Young
On 12/23/13 at 08:07am, Matt Fleming wrote:
> On Mon, 23 Dec, at 10:09:58AM, Dave Young wrote:
> > On 12/21/13 at 04:06pm, Matt Fleming wrote:
> > > On Fri, 20 Dec, at 06:02:19PM, Dave Young wrote:
> > > > @@ -133,6 +133,19 @@ extern void efi_sync_low_kernel_mappings(void);
> > > >  extern void efi_setup_page_tables(void);
> > > >  extern void __init old_map_region(efi_memory_desc_t *md);
> > > >  
> > > > +struct efi_setup_data {
> > > > +   u64 fw_vendor;
> > > > +   u64 runtime;
> > > > +   u64 tables;
> > > > +   u64 smbios;
> > > > +   u64 reserved[8];
> > > > +   efi_memory_desc_t map[0];
> > > > +};
> > > 
> > > [...]
> > > 
> > > > +static void get_nr_runtime_map(void)
> > > > +{
> > > > +   if (!efi_setup)
> > > > +   return;
> > > > +
> > > > +   nr_efi_runtime_map = (efi_data_len - sizeof(struct 
> > > > efi_setup_data)) /
> > > > +sizeof(efi_memory_desc_t);
> > > > +}
> > > 
> > > Do we actually need the 'map' entry in efi_setup_data now that you're
> > > passing it via efi_info (which is much better approach!)? Also, we don't
> > > need the global nr_efi_runtime_map or efi_runtime_map variables now,
> > > right?
> > 
> > The map is still necessary because we need store the map somewhere and pass
> > the physicall address to kexec kernel. Passing them in setup_data is the
> > only better way currently...
> > 
> > In efi_info there's only an entry for the map physical address, the original
> > map area is not valid any more.
> 
> Where do you dereference efi_setup_data.map* in the kexec kernel?

The field itself is not dereferenced in kernel but it's still used in userspace
code for collecting and saving runtime maps. Since I still append the map at the
end of efi_setup_data so I think keep the map[0] is ok. What's your opinion?

Thanks
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-24 Thread Dave Young
On 12/23/13 at 08:07am, Matt Fleming wrote:
 On Mon, 23 Dec, at 10:09:58AM, Dave Young wrote:
  On 12/21/13 at 04:06pm, Matt Fleming wrote:
   On Fri, 20 Dec, at 06:02:19PM, Dave Young wrote:
@@ -133,6 +133,19 @@ extern void efi_sync_low_kernel_mappings(void);
 extern void efi_setup_page_tables(void);
 extern void __init old_map_region(efi_memory_desc_t *md);
 
+struct efi_setup_data {
+   u64 fw_vendor;
+   u64 runtime;
+   u64 tables;
+   u64 smbios;
+   u64 reserved[8];
+   efi_memory_desc_t map[0];
+};
   
   [...]
   
+static void get_nr_runtime_map(void)
+{
+   if (!efi_setup)
+   return;
+
+   nr_efi_runtime_map = (efi_data_len - sizeof(struct 
efi_setup_data)) /
+sizeof(efi_memory_desc_t);
+}
   
   Do we actually need the 'map' entry in efi_setup_data now that you're
   passing it via efi_info (which is much better approach!)? Also, we don't
   need the global nr_efi_runtime_map or efi_runtime_map variables now,
   right?
  
  The map is still necessary because we need store the map somewhere and pass
  the physicall address to kexec kernel. Passing them in setup_data is the
  only better way currently...
  
  In efi_info there's only an entry for the map physical address, the original
  map area is not valid any more.
 
 Where do you dereference efi_setup_data.map* in the kexec kernel?

The field itself is not dereferenced in kernel but it's still used in userspace
code for collecting and saving runtime maps. Since I still append the map at the
end of efi_setup_data so I think keep the map[0] is ok. What's your opinion?

Thanks
Dave
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-24 Thread Dave Young
On 12/25/13 at 11:12am, Dave Young wrote:
 On 12/23/13 at 08:07am, Matt Fleming wrote:
  On Mon, 23 Dec, at 10:09:58AM, Dave Young wrote:
   On 12/21/13 at 04:06pm, Matt Fleming wrote:
On Fri, 20 Dec, at 06:02:19PM, Dave Young wrote:
 @@ -133,6 +133,19 @@ extern void efi_sync_low_kernel_mappings(void);
  extern void efi_setup_page_tables(void);
  extern void __init old_map_region(efi_memory_desc_t *md);
  
 +struct efi_setup_data {
 + u64 fw_vendor;
 + u64 runtime;
 + u64 tables;
 + u64 smbios;
 + u64 reserved[8];
 + efi_memory_desc_t map[0];
 +};

[...]

 +static void get_nr_runtime_map(void)
 +{
 + if (!efi_setup)
 + return;
 +
 + nr_efi_runtime_map = (efi_data_len - sizeof(struct 
 efi_setup_data)) /
 +  sizeof(efi_memory_desc_t);
 +}

Do we actually need the 'map' entry in efi_setup_data now that you're
passing it via efi_info (which is much better approach!)? Also, we don't
need the global nr_efi_runtime_map or efi_runtime_map variables now,
right?
   
   The map is still necessary because we need store the map somewhere and 
   pass
   the physicall address to kexec kernel. Passing them in setup_data is the
   only better way currently...
   
   In efi_info there's only an entry for the map physical address, the 
   original
   map area is not valid any more.
  
  Where do you dereference efi_setup_data.map* in the kexec kernel?
 
 The field itself is not dereferenced in kernel but it's still used in 
 userspace
 code for collecting and saving runtime maps. Since I still append the map at 
 the
 end of efi_setup_data so I think keep the map[0] is ok. What's your opinion?

Matt, if you want to remove the map[0] please fold below one line patch? I have 
no
strong opinion.

Index: linux/arch/x86/include/asm/efi.h
===
--- linux.orig/arch/x86/include/asm/efi.h
+++ linux/arch/x86/include/asm/efi.h
@@ -139,7 +139,6 @@ struct efi_setup_data {
u64 tables;
u64 smbios;
u64 reserved[8];
-   efi_memory_desc_t map[0];
 };
 
 extern u64 efi_setup;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-23 Thread Matt Fleming
On Mon, 23 Dec, at 10:09:58AM, Dave Young wrote:
> On 12/21/13 at 04:06pm, Matt Fleming wrote:
> > On Fri, 20 Dec, at 06:02:19PM, Dave Young wrote:
> > > @@ -133,6 +133,19 @@ extern void efi_sync_low_kernel_mappings(void);
> > >  extern void efi_setup_page_tables(void);
> > >  extern void __init old_map_region(efi_memory_desc_t *md);
> > >  
> > > +struct efi_setup_data {
> > > + u64 fw_vendor;
> > > + u64 runtime;
> > > + u64 tables;
> > > + u64 smbios;
> > > + u64 reserved[8];
> > > + efi_memory_desc_t map[0];
> > > +};
> > 
> > [...]
> > 
> > > +static void get_nr_runtime_map(void)
> > > +{
> > > + if (!efi_setup)
> > > + return;
> > > +
> > > + nr_efi_runtime_map = (efi_data_len - sizeof(struct efi_setup_data)) /
> > > +  sizeof(efi_memory_desc_t);
> > > +}
> > 
> > Do we actually need the 'map' entry in efi_setup_data now that you're
> > passing it via efi_info (which is much better approach!)? Also, we don't
> > need the global nr_efi_runtime_map or efi_runtime_map variables now,
> > right?
> 
> The map is still necessary because we need store the map somewhere and pass
> the physicall address to kexec kernel. Passing them in setup_data is the
> only better way currently...
> 
> In efi_info there's only an entry for the map physical address, the original
> map area is not valid any more.

Where do you dereference efi_setup_data.map* in the kexec kernel?

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


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-23 Thread Matt Fleming
On Mon, 23 Dec, at 10:09:58AM, Dave Young wrote:
 On 12/21/13 at 04:06pm, Matt Fleming wrote:
  On Fri, 20 Dec, at 06:02:19PM, Dave Young wrote:
   @@ -133,6 +133,19 @@ extern void efi_sync_low_kernel_mappings(void);
extern void efi_setup_page_tables(void);
extern void __init old_map_region(efi_memory_desc_t *md);

   +struct efi_setup_data {
   + u64 fw_vendor;
   + u64 runtime;
   + u64 tables;
   + u64 smbios;
   + u64 reserved[8];
   + efi_memory_desc_t map[0];
   +};
  
  [...]
  
   +static void get_nr_runtime_map(void)
   +{
   + if (!efi_setup)
   + return;
   +
   + nr_efi_runtime_map = (efi_data_len - sizeof(struct efi_setup_data)) /
   +  sizeof(efi_memory_desc_t);
   +}
  
  Do we actually need the 'map' entry in efi_setup_data now that you're
  passing it via efi_info (which is much better approach!)? Also, we don't
  need the global nr_efi_runtime_map or efi_runtime_map variables now,
  right?
 
 The map is still necessary because we need store the map somewhere and pass
 the physicall address to kexec kernel. Passing them in setup_data is the
 only better way currently...
 
 In efi_info there's only an entry for the map physical address, the original
 map area is not valid any more.

Where do you dereference efi_setup_data.map* in the kexec kernel?

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


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-22 Thread Dave Young
> > Do we actually need the 'map' entry in efi_setup_data now that you're
> > passing it via efi_info (which is much better approach!)? Also, we don't
> > need the global nr_efi_runtime_map or efi_runtime_map variables now,
> > right?
> 
> The map is still necessary because we need store the map somewhere and pass
> the physicall address to kexec kernel. Passing them in setup_data is the
> only better way currently...
> 
> In efi_info there's only an entry for the map physical address, the original
> map area is not valid any more.

For kexec kernel it might be ok to use the old map area, but for kdump kernel
we can only use the memory which is early reserved with crashkernel= cmdline.

Thanks
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-22 Thread Dave Young
On 12/21/13 at 04:06pm, Matt Fleming wrote:
> On Fri, 20 Dec, at 06:02:19PM, Dave Young wrote:
> > @@ -133,6 +133,19 @@ extern void efi_sync_low_kernel_mappings(void);
> >  extern void efi_setup_page_tables(void);
> >  extern void __init old_map_region(efi_memory_desc_t *md);
> >  
> > +struct efi_setup_data {
> > +   u64 fw_vendor;
> > +   u64 runtime;
> > +   u64 tables;
> > +   u64 smbios;
> > +   u64 reserved[8];
> > +   efi_memory_desc_t map[0];
> > +};
> 
> [...]
> 
> > +static void get_nr_runtime_map(void)
> > +{
> > +   if (!efi_setup)
> > +   return;
> > +
> > +   nr_efi_runtime_map = (efi_data_len - sizeof(struct efi_setup_data)) /
> > +sizeof(efi_memory_desc_t);
> > +}
> 
> Do we actually need the 'map' entry in efi_setup_data now that you're
> passing it via efi_info (which is much better approach!)? Also, we don't
> need the global nr_efi_runtime_map or efi_runtime_map variables now,
> right?

The map is still necessary because we need store the map somewhere and pass
the physicall address to kexec kernel. Passing them in setup_data is the
only better way currently...

In efi_info there's only an entry for the map physical address, the original
map area is not valid any more.

Thanks
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-22 Thread Dave Young
> > +   if (data)
> > +   early_memunmap(data, sizeof(*data));
> >  #ifdef CONFIG_X86_32
> > if (tmp >> 32) {
> > pr_err("EFI data located above 4GB, disabling EFI.\n");
> 
> This isn't correct, and means we now won't trigger this pr_err() if
> booting a CONFIG_X86_32 kernel with EFI_64BIT and where the ->fw_vendor,
> ->runtime or ->tables pointer is above 4GB - you've mixed up systab and
> systab64. I've fixed this up like so when applying this patch,

You are right, I missed this 32bit tmp value, thanks for taking care of this.

Thanks
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-22 Thread Dave Young
  +   if (data)
  +   early_memunmap(data, sizeof(*data));
   #ifdef CONFIG_X86_32
  if (tmp  32) {
  pr_err(EFI data located above 4GB, disabling EFI.\n);
 
 This isn't correct, and means we now won't trigger this pr_err() if
 booting a CONFIG_X86_32 kernel with EFI_64BIT and where the -fw_vendor,
 -runtime or -tables pointer is above 4GB - you've mixed up systab and
 systab64. I've fixed this up like so when applying this patch,

You are right, I missed this 32bit tmp value, thanks for taking care of this.

Thanks
Dave
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-22 Thread Dave Young
On 12/21/13 at 04:06pm, Matt Fleming wrote:
 On Fri, 20 Dec, at 06:02:19PM, Dave Young wrote:
  @@ -133,6 +133,19 @@ extern void efi_sync_low_kernel_mappings(void);
   extern void efi_setup_page_tables(void);
   extern void __init old_map_region(efi_memory_desc_t *md);
   
  +struct efi_setup_data {
  +   u64 fw_vendor;
  +   u64 runtime;
  +   u64 tables;
  +   u64 smbios;
  +   u64 reserved[8];
  +   efi_memory_desc_t map[0];
  +};
 
 [...]
 
  +static void get_nr_runtime_map(void)
  +{
  +   if (!efi_setup)
  +   return;
  +
  +   nr_efi_runtime_map = (efi_data_len - sizeof(struct efi_setup_data)) /
  +sizeof(efi_memory_desc_t);
  +}
 
 Do we actually need the 'map' entry in efi_setup_data now that you're
 passing it via efi_info (which is much better approach!)? Also, we don't
 need the global nr_efi_runtime_map or efi_runtime_map variables now,
 right?

The map is still necessary because we need store the map somewhere and pass
the physicall address to kexec kernel. Passing them in setup_data is the
only better way currently...

In efi_info there's only an entry for the map physical address, the original
map area is not valid any more.

Thanks
Dave
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-22 Thread Dave Young
  Do we actually need the 'map' entry in efi_setup_data now that you're
  passing it via efi_info (which is much better approach!)? Also, we don't
  need the global nr_efi_runtime_map or efi_runtime_map variables now,
  right?
 
 The map is still necessary because we need store the map somewhere and pass
 the physicall address to kexec kernel. Passing them in setup_data is the
 only better way currently...
 
 In efi_info there's only an entry for the map physical address, the original
 map area is not valid any more.

For kexec kernel it might be ok to use the old map area, but for kdump kernel
we can only use the memory which is early reserved with crashkernel= cmdline.

Thanks
Dave
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-21 Thread Matt Fleming
On Fri, 20 Dec, at 06:02:19PM, Dave Young wrote:
> @@ -133,6 +133,19 @@ extern void efi_sync_low_kernel_mappings(void);
>  extern void efi_setup_page_tables(void);
>  extern void __init old_map_region(efi_memory_desc_t *md);
>  
> +struct efi_setup_data {
> + u64 fw_vendor;
> + u64 runtime;
> + u64 tables;
> + u64 smbios;
> + u64 reserved[8];
> + efi_memory_desc_t map[0];
> +};

[...]

> +static void get_nr_runtime_map(void)
> +{
> + if (!efi_setup)
> + return;
> +
> + nr_efi_runtime_map = (efi_data_len - sizeof(struct efi_setup_data)) /
> +  sizeof(efi_memory_desc_t);
> +}

Do we actually need the 'map' entry in efi_setup_data now that you're
passing it via efi_info (which is much better approach!)? Also, we don't
need the global nr_efi_runtime_map or efi_runtime_map variables now,
right?

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


Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-21 Thread Matt Fleming
On Fri, 20 Dec, at 06:02:19PM, Dave Young wrote:
> Add a new setup_data type SETUP_EFI for kexec use.
> Passing the saved fw_vendor, runtime, config tables and efi runtime mappings.
> 
> When entering virtual mode, directly mapping the efi runtime ragions which
> we passed in previously. And skip the step to call SetVirtualAddressMap.
> 
> Specially for HP z420 workstation we need save the smbios physical address.
> The kernel boot sequence proceeds in the following order.  Step 2
> requires efi.smbios to be the physical address.  However, I found that on
> HP z420 EFI system table has a virtual address of SMBIOS in step 1.  Hence,
> we need set it back to the physical address with the smbios in
> efi_setup_data.  (When it is still the physical address, it simply sets
> the same value.)
> 
> 1. efi_init() - Set efi.smbios from EFI system table
> 2. dmi_scan_machine() - Temporary map efi.smbios to access SMBIOS table
> 3. efi_enter_virtual_mode() - Map EFI ranges
> 
> Tested on ovmf+qemu, lenovo thinkpad, a dell laptop and an
> HP z420 workstation.
> 
> v2: refresh based on previous patch changes, code cleanup.
> v3: use ioremap instead of phys_to_virt for efi_setup
> v5: improve some code structure per comments from Matt
> Boris: improve code structure, spell fix, etc.
> Improve changelog from Toshi.
> change the variable efi_setup to the physical address of efi setup_data
> instead of the ioremapped virt address
> v6: Boris: Documentation fixes
>move parse_efi_setup to efi_$(BITS).c
>simplify parse_efi_setup logic
> Matt: check return value of efi_reuse_config
> v7: cleanup efi_map_regions and efi_map_regions_fixed
> remove useless return at the end of efi_enter_virtual_mode
> 
> Signed-off-by: Dave Young 
> ---
>  arch/x86/include/asm/efi.h|  13 +++
>  arch/x86/include/uapi/asm/bootparam.h |   1 +
>  arch/x86/kernel/setup.c   |   3 +
>  arch/x86/platform/efi/efi.c   | 159 
> --
>  arch/x86/platform/efi/efi_32.c|   1 +
>  arch/x86/platform/efi/efi_64.c|   6 ++
>  6 files changed, 158 insertions(+), 25 deletions(-)

[...]

>   efi_systab.hdr = systab64->hdr;
> - efi_systab.fw_vendor = systab64->fw_vendor;
> - tmp |= systab64->fw_vendor;
> +
> + efi_systab.fw_vendor = data ? (unsigned long)data->fw_vendor :
> +   systab64->fw_vendor;
> + tmp |= efi_systab.fw_vendor;

[... snip ...]

> @@ -519,15 +530,20 @@ static int __init efi_systab_init(void *phys)
>   tmp |= systab64->stderr_handle;
>   efi_systab.stderr = systab64->stderr;
>   tmp |= systab64->stderr;
> - efi_systab.runtime = (void *)(unsigned long)systab64->runtime;
> - tmp |= systab64->runtime;
> + efi_systab.runtime = data ?
> +  (void *)(unsigned long)data->runtime :
> +  (void *)(unsigned long)systab64->runtime;
> + tmp |= (unsigned long)efi_systab.runtime;
>   efi_systab.boottime = (void *)(unsigned long)systab64->boottime;
>   tmp |= systab64->boottime;
>   efi_systab.nr_tables = systab64->nr_tables;
> - efi_systab.tables = systab64->tables;
> - tmp |= systab64->tables;
> + efi_systab.tables = data ? (unsigned long)data->tables :
> +systab64->tables;
> + tmp |= efi_systab.tables;
>  
>   early_memunmap(systab64, sizeof(*systab64));
> + if (data)
> + early_memunmap(data, sizeof(*data));
>  #ifdef CONFIG_X86_32
>   if (tmp >> 32) {
>   pr_err("EFI data located above 4GB, disabling EFI.\n");

This isn't correct, and means we now won't trigger this pr_err() if
booting a CONFIG_X86_32 kernel with EFI_64BIT and where the ->fw_vendor,
->runtime or ->tables pointer is above 4GB - you've mixed up systab and
systab64. I've fixed this up like so when applying this patch,

---

@@ -494,18 +495,27 @@ static int __init efi_systab_init(void *phys)
 {
if (efi_enabled(EFI_64BIT)) {
efi_system_table_64_t *systab64;
+   struct efi_setup_data *data = NULL;
u64 tmp = 0;
 
+   if (efi_setup) {
+   data = early_memremap(efi_setup, sizeof(*data));
+   if (!data)
+   return -ENOMEM;
+   }
systab64 = early_ioremap((unsigned long)phys,
 sizeof(*systab64));
if (systab64 == NULL) {
pr_err("Couldn't map the system table!\n");
+   if (data)
+   early_iounmap(data, sizeof(*data));
return -ENOMEM;
 

Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-21 Thread Matt Fleming
On Fri, 20 Dec, at 06:02:19PM, Dave Young wrote:
 Add a new setup_data type SETUP_EFI for kexec use.
 Passing the saved fw_vendor, runtime, config tables and efi runtime mappings.
 
 When entering virtual mode, directly mapping the efi runtime ragions which
 we passed in previously. And skip the step to call SetVirtualAddressMap.
 
 Specially for HP z420 workstation we need save the smbios physical address.
 The kernel boot sequence proceeds in the following order.  Step 2
 requires efi.smbios to be the physical address.  However, I found that on
 HP z420 EFI system table has a virtual address of SMBIOS in step 1.  Hence,
 we need set it back to the physical address with the smbios in
 efi_setup_data.  (When it is still the physical address, it simply sets
 the same value.)
 
 1. efi_init() - Set efi.smbios from EFI system table
 2. dmi_scan_machine() - Temporary map efi.smbios to access SMBIOS table
 3. efi_enter_virtual_mode() - Map EFI ranges
 
 Tested on ovmf+qemu, lenovo thinkpad, a dell laptop and an
 HP z420 workstation.
 
 v2: refresh based on previous patch changes, code cleanup.
 v3: use ioremap instead of phys_to_virt for efi_setup
 v5: improve some code structure per comments from Matt
 Boris: improve code structure, spell fix, etc.
 Improve changelog from Toshi.
 change the variable efi_setup to the physical address of efi setup_data
 instead of the ioremapped virt address
 v6: Boris: Documentation fixes
move parse_efi_setup to efi_$(BITS).c
simplify parse_efi_setup logic
 Matt: check return value of efi_reuse_config
 v7: cleanup efi_map_regions and efi_map_regions_fixed
 remove useless return at the end of efi_enter_virtual_mode
 
 Signed-off-by: Dave Young dyo...@redhat.com
 ---
  arch/x86/include/asm/efi.h|  13 +++
  arch/x86/include/uapi/asm/bootparam.h |   1 +
  arch/x86/kernel/setup.c   |   3 +
  arch/x86/platform/efi/efi.c   | 159 
 --
  arch/x86/platform/efi/efi_32.c|   1 +
  arch/x86/platform/efi/efi_64.c|   6 ++
  6 files changed, 158 insertions(+), 25 deletions(-)

[...]

   efi_systab.hdr = systab64-hdr;
 - efi_systab.fw_vendor = systab64-fw_vendor;
 - tmp |= systab64-fw_vendor;
 +
 + efi_systab.fw_vendor = data ? (unsigned long)data-fw_vendor :
 +   systab64-fw_vendor;
 + tmp |= efi_systab.fw_vendor;

[... snip ...]

 @@ -519,15 +530,20 @@ static int __init efi_systab_init(void *phys)
   tmp |= systab64-stderr_handle;
   efi_systab.stderr = systab64-stderr;
   tmp |= systab64-stderr;
 - efi_systab.runtime = (void *)(unsigned long)systab64-runtime;
 - tmp |= systab64-runtime;
 + efi_systab.runtime = data ?
 +  (void *)(unsigned long)data-runtime :
 +  (void *)(unsigned long)systab64-runtime;
 + tmp |= (unsigned long)efi_systab.runtime;
   efi_systab.boottime = (void *)(unsigned long)systab64-boottime;
   tmp |= systab64-boottime;
   efi_systab.nr_tables = systab64-nr_tables;
 - efi_systab.tables = systab64-tables;
 - tmp |= systab64-tables;
 + efi_systab.tables = data ? (unsigned long)data-tables :
 +systab64-tables;
 + tmp |= efi_systab.tables;
  
   early_memunmap(systab64, sizeof(*systab64));
 + if (data)
 + early_memunmap(data, sizeof(*data));
  #ifdef CONFIG_X86_32
   if (tmp  32) {
   pr_err(EFI data located above 4GB, disabling EFI.\n);

This isn't correct, and means we now won't trigger this pr_err() if
booting a CONFIG_X86_32 kernel with EFI_64BIT and where the -fw_vendor,
-runtime or -tables pointer is above 4GB - you've mixed up systab and
systab64. I've fixed this up like so when applying this patch,

---

@@ -494,18 +495,27 @@ static int __init efi_systab_init(void *phys)
 {
if (efi_enabled(EFI_64BIT)) {
efi_system_table_64_t *systab64;
+   struct efi_setup_data *data = NULL;
u64 tmp = 0;
 
+   if (efi_setup) {
+   data = early_memremap(efi_setup, sizeof(*data));
+   if (!data)
+   return -ENOMEM;
+   }
systab64 = early_ioremap((unsigned long)phys,
 sizeof(*systab64));
if (systab64 == NULL) {
pr_err(Couldn't map the system table!\n);
+   if (data)
+   early_iounmap(data, sizeof(*data));
return -ENOMEM;
}
 
efi_systab.hdr = systab64-hdr;
-   

Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data

2013-12-21 Thread Matt Fleming
On Fri, 20 Dec, at 06:02:19PM, Dave Young wrote:
 @@ -133,6 +133,19 @@ extern void efi_sync_low_kernel_mappings(void);
  extern void efi_setup_page_tables(void);
  extern void __init old_map_region(efi_memory_desc_t *md);
  
 +struct efi_setup_data {
 + u64 fw_vendor;
 + u64 runtime;
 + u64 tables;
 + u64 smbios;
 + u64 reserved[8];
 + efi_memory_desc_t map[0];
 +};

[...]

 +static void get_nr_runtime_map(void)
 +{
 + if (!efi_setup)
 + return;
 +
 + nr_efi_runtime_map = (efi_data_len - sizeof(struct efi_setup_data)) /
 +  sizeof(efi_memory_desc_t);
 +}

Do we actually need the 'map' entry in efi_setup_data now that you're
passing it via efi_info (which is much better approach!)? Also, we don't
need the global nr_efi_runtime_map or efi_runtime_map variables now,
right?

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