Re: [PATCH v7 09/12] efi: passing kexec necessary efi data via setup_data
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
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
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
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
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
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
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
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
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
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
> > 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
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
> > + 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
+ 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
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
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
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
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
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
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/