Re: [PATCH v4 07/12] efi: passing kexec necessary efi data via setup_data
On 12/05/13 at 09:52pm, Borislav Petkov wrote: > On Thu, Dec 05, 2013 at 08:56:02AM -0700, Toshi Kani wrote: > > The smbios in efi_setup_data is necessary for kexec to pass the physical > > address of the SMBIOS table from the 1st kernel to the 2nd kernel. > > > > The kernel boot sequence proceeds in the following order. Step 2 > > requires efi.smbios to be the physical address. However, Dave found > > that EFI system table has a virtual address of SMBIOS in step 1. Hence, > > he sets 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 > > Thanks Toshi, it is all clear now. > > @Dave: you might want to put Toshi's explanation in the commit message > of your patch so that we know *why* the change is needed. Ok, will do. 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Thu, Dec 05, 2013 at 08:56:02AM -0700, Toshi Kani wrote: > The smbios in efi_setup_data is necessary for kexec to pass the physical > address of the SMBIOS table from the 1st kernel to the 2nd kernel. > > The kernel boot sequence proceeds in the following order. Step 2 > requires efi.smbios to be the physical address. However, Dave found > that EFI system table has a virtual address of SMBIOS in step 1. Hence, > he sets 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 Thanks Toshi, it is all clear now. @Dave: you might want to put Toshi's explanation in the commit message of your patch so that we know *why* the change is needed. Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Thu, 2013-12-05 at 12:51 +0100, Borislav Petkov wrote: > On Thu, Dec 05, 2013 at 09:56:15AM +0800, Dave Young wrote: > > > The z420 firmware is based on some UEFI core that may be used by other > > > vendors as well. Since this handling is totally harmless (just > > > redundant), I'd suggest not to have a platform check on this handling. > > > > I have same worry as well, so I agree with you. > > Btw, why does kexec need the ->smbios table at all? I see it being > passed in efi_setup_data. Hi Boris, The smbios in efi_setup_data is necessary for kexec to pass the physical address of the SMBIOS table from the 1st kernel to the 2nd kernel. The kernel boot sequence proceeds in the following order. Step 2 requires efi.smbios to be the physical address. However, Dave found that EFI system table has a virtual address of SMBIOS in step 1. Hence, he sets 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 Thanks, -Toshi -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Thu, Dec 05, 2013 at 09:56:15AM +0800, Dave Young wrote: > > The z420 firmware is based on some UEFI core that may be used by other > > vendors as well. Since this handling is totally harmless (just > > redundant), I'd suggest not to have a platform check on this handling. > > I have same worry as well, so I agree with you. Btw, why does kexec need the ->smbios table at all? I see it being passed in efi_setup_data. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Thu, Dec 05, 2013 at 09:56:15AM +0800, Dave Young wrote: The z420 firmware is based on some UEFI core that may be used by other vendors as well. Since this handling is totally harmless (just redundant), I'd suggest not to have a platform check on this handling. I have same worry as well, so I agree with you. Btw, why does kexec need the -smbios table at all? I see it being passed in efi_setup_data. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Thu, 2013-12-05 at 12:51 +0100, Borislav Petkov wrote: On Thu, Dec 05, 2013 at 09:56:15AM +0800, Dave Young wrote: The z420 firmware is based on some UEFI core that may be used by other vendors as well. Since this handling is totally harmless (just redundant), I'd suggest not to have a platform check on this handling. I have same worry as well, so I agree with you. Btw, why does kexec need the -smbios table at all? I see it being passed in efi_setup_data. Hi Boris, The smbios in efi_setup_data is necessary for kexec to pass the physical address of the SMBIOS table from the 1st kernel to the 2nd kernel. The kernel boot sequence proceeds in the following order. Step 2 requires efi.smbios to be the physical address. However, Dave found that EFI system table has a virtual address of SMBIOS in step 1. Hence, he sets 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 Thanks, -Toshi -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Thu, Dec 05, 2013 at 08:56:02AM -0700, Toshi Kani wrote: The smbios in efi_setup_data is necessary for kexec to pass the physical address of the SMBIOS table from the 1st kernel to the 2nd kernel. The kernel boot sequence proceeds in the following order. Step 2 requires efi.smbios to be the physical address. However, Dave found that EFI system table has a virtual address of SMBIOS in step 1. Hence, he sets 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 Thanks Toshi, it is all clear now. @Dave: you might want to put Toshi's explanation in the commit message of your patch so that we know *why* the change is needed. Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On 12/05/13 at 09:52pm, Borislav Petkov wrote: On Thu, Dec 05, 2013 at 08:56:02AM -0700, Toshi Kani wrote: The smbios in efi_setup_data is necessary for kexec to pass the physical address of the SMBIOS table from the 1st kernel to the 2nd kernel. The kernel boot sequence proceeds in the following order. Step 2 requires efi.smbios to be the physical address. However, Dave found that EFI system table has a virtual address of SMBIOS in step 1. Hence, he sets 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 Thanks Toshi, it is all clear now. @Dave: you might want to put Toshi's explanation in the commit message of your patch so that we know *why* the change is needed. Ok, will do. 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On 12/04/13 at 09:43am, Toshi Kani wrote: > On Wed, 2013-12-04 at 10:46 +0800, Dave Young wrote: > > Hi, Toshi > > > > > Oh, I think I now understand what the issue was. The z420 firmware > > > updates the SMBIOS table address in the EFI system table to a virtual > > > address after calling EFI SetVirtualAddressMap. So, you are passing the > > > original physical address of the SMBIOS table from the 1st kernel to the > > > 2nd kernel to put it back to physical. Is that right? > > > > Right. > > Hi Dave, > > The z420 firmware is based on some UEFI core that may be used by other > vendors as well. Since this handling is totally harmless (just > redundant), I'd suggest not to have a platform check on this handling. I have same worry as well, so I agree with you. 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Wed, 2013-12-04 at 10:46 +0800, Dave Young wrote: > Hi, Toshi > > > Oh, I think I now understand what the issue was. The z420 firmware > > updates the SMBIOS table address in the EFI system table to a virtual > > address after calling EFI SetVirtualAddressMap. So, you are passing the > > original physical address of the SMBIOS table from the 1st kernel to the > > 2nd kernel to put it back to physical. Is that right? > > Right. Hi Dave, The z420 firmware is based on some UEFI core that may be used by other vendors as well. Since this handling is totally harmless (just redundant), I'd suggest not to have a platform check on this handling. Thanks, -Toshi -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Wed, 2013-12-04 at 10:46 +0800, Dave Young wrote: Hi, Toshi Oh, I think I now understand what the issue was. The z420 firmware updates the SMBIOS table address in the EFI system table to a virtual address after calling EFI SetVirtualAddressMap. So, you are passing the original physical address of the SMBIOS table from the 1st kernel to the 2nd kernel to put it back to physical. Is that right? Right. Hi Dave, The z420 firmware is based on some UEFI core that may be used by other vendors as well. Since this handling is totally harmless (just redundant), I'd suggest not to have a platform check on this handling. Thanks, -Toshi -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On 12/04/13 at 09:43am, Toshi Kani wrote: On Wed, 2013-12-04 at 10:46 +0800, Dave Young wrote: Hi, Toshi Oh, I think I now understand what the issue was. The z420 firmware updates the SMBIOS table address in the EFI system table to a virtual address after calling EFI SetVirtualAddressMap. So, you are passing the original physical address of the SMBIOS table from the 1st kernel to the 2nd kernel to put it back to physical. Is that right? Right. Hi Dave, The z420 firmware is based on some UEFI core that may be used by other vendors as well. Since this handling is totally harmless (just redundant), I'd suggest not to have a platform check on this handling. I have same worry as well, so I agree with you. 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
Hi, Toshi > Oh, I think I now understand what the issue was. The z420 firmware > updates the SMBIOS table address in the EFI system table to a virtual > address after calling EFI SetVirtualAddressMap. So, you are passing the > original physical address of the SMBIOS table from the 1st kernel to the > 2nd kernel to put it back to physical. Is that right? Right. 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Tue, 2013-12-03 at 09:56 +0800, Dave Young wrote: > On 12/02/13 at 06:31pm, Toshi Kani wrote: > > On Mon, 2013-12-02 at 15:33 -0700, Toshi Kani wrote: > > > On Fri, 2013-11-29 at 17:14 +0800, Dave Young wrote: > > > > On 11/27/13 at 03:07pm, Borislav Petkov wrote: > > > > > On Tue, Nov 26, 2013 at 01:57:52PM +0800, 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 it need another variable > > > > > > saving, > > > > > > > > > > Why the special handling? Does that mean, this is going to be the case > > > > > for other HP UEFI implementations too? > > > > > > > > I have only one HP machine for testing, Maybe Toshi can help to verify > > > > on other machines. Just comment out the function efi_reuse_config to see > > > > if kexec kernel panic. > > > > > > My system (HP prototype server) did not need the special handling in > > > efi_reuse_config(). I will check with firmware team to get more > > > information on this. > > > > I have one question. I think this special handling assumes that > > efi.smbios contains a virtual address on HP z420. So, how does the 1st > > kernel set a virtual address to efi.smbios in the first place? > > Hi, Toshi, > > It's originally a physical address, I think kernel does not change it. > As the fw_vendor, runtime etc. are changed to virt addr by firmware, so I > assume smbios on Hp z420 is also changed by firmware after becoming virtual > mode. Hi Dave, Oh, I think I now understand what the issue was. The z420 firmware updates the SMBIOS table address in the EFI system table to a virtual address after calling EFI SetVirtualAddressMap. So, you are passing the original physical address of the SMBIOS table from the 1st kernel to the 2nd kernel to put it back to physical. Is that right? Thanks, -Toshi -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Tue, 2013-12-03 at 09:56 +0800, Dave Young wrote: On 12/02/13 at 06:31pm, Toshi Kani wrote: On Mon, 2013-12-02 at 15:33 -0700, Toshi Kani wrote: On Fri, 2013-11-29 at 17:14 +0800, Dave Young wrote: On 11/27/13 at 03:07pm, Borislav Petkov wrote: On Tue, Nov 26, 2013 at 01:57:52PM +0800, 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 it need another variable saving, Why the special handling? Does that mean, this is going to be the case for other HP UEFI implementations too? I have only one HP machine for testing, Maybe Toshi can help to verify on other machines. Just comment out the function efi_reuse_config to see if kexec kernel panic. My system (HP prototype server) did not need the special handling in efi_reuse_config(). I will check with firmware team to get more information on this. I have one question. I think this special handling assumes that efi.smbios contains a virtual address on HP z420. So, how does the 1st kernel set a virtual address to efi.smbios in the first place? Hi, Toshi, It's originally a physical address, I think kernel does not change it. As the fw_vendor, runtime etc. are changed to virt addr by firmware, so I assume smbios on Hp z420 is also changed by firmware after becoming virtual mode. Hi Dave, Oh, I think I now understand what the issue was. The z420 firmware updates the SMBIOS table address in the EFI system table to a virtual address after calling EFI SetVirtualAddressMap. So, you are passing the original physical address of the SMBIOS table from the 1st kernel to the 2nd kernel to put it back to physical. Is that right? Thanks, -Toshi -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
Hi, Toshi Oh, I think I now understand what the issue was. The z420 firmware updates the SMBIOS table address in the EFI system table to a virtual address after calling EFI SetVirtualAddressMap. So, you are passing the original physical address of the SMBIOS table from the 1st kernel to the 2nd kernel to put it back to physical. Is that right? Right. 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On 12/02/13 at 06:31pm, Toshi Kani wrote: > On Mon, 2013-12-02 at 15:33 -0700, Toshi Kani wrote: > > On Fri, 2013-11-29 at 17:14 +0800, Dave Young wrote: > > > On 11/27/13 at 03:07pm, Borislav Petkov wrote: > > > > On Tue, Nov 26, 2013 at 01:57:52PM +0800, 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 it need another variable > > > > > saving, > > > > > > > > Why the special handling? Does that mean, this is going to be the case > > > > for other HP UEFI implementations too? > > > > > > I have only one HP machine for testing, Maybe Toshi can help to verify > > > on other machines. Just comment out the function efi_reuse_config to see > > > if kexec kernel panic. > > > > My system (HP prototype server) did not need the special handling in > > efi_reuse_config(). I will check with firmware team to get more > > information on this. > > I have one question. I think this special handling assumes that > efi.smbios contains a virtual address on HP z420. So, how does the 1st > kernel set a virtual address to efi.smbios in the first place? Hi, Toshi, It's originally a physical address, I think kernel does not change it. As the fw_vendor, runtime etc. are changed to virt addr by firmware, so I assume smbios on Hp z420 is also changed by firmware after becoming virtual mode. 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Mon, 2013-12-02 at 15:33 -0700, Toshi Kani wrote: > On Fri, 2013-11-29 at 17:14 +0800, Dave Young wrote: > > On 11/27/13 at 03:07pm, Borislav Petkov wrote: > > > On Tue, Nov 26, 2013 at 01:57:52PM +0800, 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 it need another variable > > > > saving, > > > > > > Why the special handling? Does that mean, this is going to be the case > > > for other HP UEFI implementations too? > > > > I have only one HP machine for testing, Maybe Toshi can help to verify > > on other machines. Just comment out the function efi_reuse_config to see > > if kexec kernel panic. > > My system (HP prototype server) did not need the special handling in > efi_reuse_config(). I will check with firmware team to get more > information on this. I have one question. I think this special handling assumes that efi.smbios contains a virtual address on HP z420. So, how does the 1st kernel set a virtual address to efi.smbios in the first place? Thanks, -Toshi -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Fri, 2013-11-29 at 17:14 +0800, Dave Young wrote: > On 11/27/13 at 03:07pm, Borislav Petkov wrote: > > On Tue, Nov 26, 2013 at 01:57:52PM +0800, 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 it need another variable > > > saving, > > > > Why the special handling? Does that mean, this is going to be the case > > for other HP UEFI implementations too? > > I have only one HP machine for testing, Maybe Toshi can help to verify > on other machines. Just comment out the function efi_reuse_config to see > if kexec kernel panic. My system (HP prototype server) did not need the special handling in efi_reuse_config(). I will check with firmware team to get more information on this. Thanks, -Toshi -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Mon, Dec 02, 2013 at 10:49:15AM +0800, Dave Young wrote: > All these setup_data passing, remapping etc. is mostly for kexec, > add a lot of contiditional #if #endif makes the code a mess. I would > prefer to not do this if you are not strongly object. Right, having efi_kexec.c could solve all that, see my other mail. > I'm confused how to connect this with DMI.. This HP platform - like any other platform out there - has specific DMI strings identifying it. Look for examples calling dmi_check_system(). > Ok, so I will leave this to you after this series settle down. Ok, /me puts it on the TODO list :) Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Mon, Dec 02, 2013 at 10:49:15AM +0800, Dave Young wrote: All these setup_data passing, remapping etc. is mostly for kexec, add a lot of contiditional #if #endif makes the code a mess. I would prefer to not do this if you are not strongly object. Right, having efi_kexec.c could solve all that, see my other mail. I'm confused how to connect this with DMI.. This HP platform - like any other platform out there - has specific DMI strings identifying it. Look for examples calling dmi_check_system(). Ok, so I will leave this to you after this series settle down. Ok, /me puts it on the TODO list :) Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Fri, 2013-11-29 at 17:14 +0800, Dave Young wrote: On 11/27/13 at 03:07pm, Borislav Petkov wrote: On Tue, Nov 26, 2013 at 01:57:52PM +0800, 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 it need another variable saving, Why the special handling? Does that mean, this is going to be the case for other HP UEFI implementations too? I have only one HP machine for testing, Maybe Toshi can help to verify on other machines. Just comment out the function efi_reuse_config to see if kexec kernel panic. My system (HP prototype server) did not need the special handling in efi_reuse_config(). I will check with firmware team to get more information on this. Thanks, -Toshi -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Mon, 2013-12-02 at 15:33 -0700, Toshi Kani wrote: On Fri, 2013-11-29 at 17:14 +0800, Dave Young wrote: On 11/27/13 at 03:07pm, Borislav Petkov wrote: On Tue, Nov 26, 2013 at 01:57:52PM +0800, 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 it need another variable saving, Why the special handling? Does that mean, this is going to be the case for other HP UEFI implementations too? I have only one HP machine for testing, Maybe Toshi can help to verify on other machines. Just comment out the function efi_reuse_config to see if kexec kernel panic. My system (HP prototype server) did not need the special handling in efi_reuse_config(). I will check with firmware team to get more information on this. I have one question. I think this special handling assumes that efi.smbios contains a virtual address on HP z420. So, how does the 1st kernel set a virtual address to efi.smbios in the first place? Thanks, -Toshi -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On 12/02/13 at 06:31pm, Toshi Kani wrote: On Mon, 2013-12-02 at 15:33 -0700, Toshi Kani wrote: On Fri, 2013-11-29 at 17:14 +0800, Dave Young wrote: On 11/27/13 at 03:07pm, Borislav Petkov wrote: On Tue, Nov 26, 2013 at 01:57:52PM +0800, 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 it need another variable saving, Why the special handling? Does that mean, this is going to be the case for other HP UEFI implementations too? I have only one HP machine for testing, Maybe Toshi can help to verify on other machines. Just comment out the function efi_reuse_config to see if kexec kernel panic. My system (HP prototype server) did not need the special handling in efi_reuse_config(). I will check with firmware team to get more information on this. I have one question. I think this special handling assumes that efi.smbios contains a virtual address on HP z420. So, how does the 1st kernel set a virtual address to efi.smbios in the first place? Hi, Toshi, It's originally a physical address, I think kernel does not change it. As the fw_vendor, runtime etc. are changed to virt addr by firmware, so I assume smbios on Hp z420 is also changed by firmware after becoming virtual mode. 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On 11/29/13 at 05:46pm, Borislav Petkov wrote: > On Fri, Nov 29, 2013 at 05:14:16PM +0800, Dave Young wrote: > > That's reserved for future extension use, who knows if we will need to > > pass other fields in the future. > > Hrrmmm, 8*64 = 64 Bytes?? And you can't change it later because of the > situation where machines might be using older kexec-tools? Right, compatibility is the reason. > > > > Static function - no need for "efi_" prefix. > > > > Ok. I'm not very satisfied with the function name, any better > > suggestion? > > > > reuse_config > > use_old_config_phys_addr > > update_config_entry_with_phys_addr > > Looks like we're applying quirks so apply_quirks()? Ok, it looks better, will use apply_quirks, thank you. > > And those kexec-specific quirks need to be behind CONFIG_KEXEC too, btw. All these setup_data passing, remapping etc. is mostly for kexec, add a lot of contiditional #if #endif makes the code a mess. I would prefer to not do this if you are not strongly object. > > > Since Matt suggest to extend the function for other possible field > > other than smbios, so I would like to move comment to the front > > of the function like below: > > /* > > * For kexec kernel there's some special config table entries which will be > > * converted to virtual addresses after entering virtual mode. In kexec > > kernel > > * we need the physical addresses instead, thus passing them via setup_data > > * and update the entries to physical addresses in this function. > > * > > * Currently only handles smbios which is necessary for HP z420. > > */ > > That's better. > > > Is there any idea in your mind how to add the code for HP only? > > DMI? I'm confused how to connect this with DMI.. > > > Yes, __map_region return an error code will be better. Will you send > > a patch for __map_region? or I can add one more patch along with next > > version. > > I was going to wait for your patches to settle down first so that I > don't disturb your work. > > But if you want to fix this, I certainly wouldn't want to stop you :) > but please do so in a prepatch before your series which addresses only > this issue. Ok, so I will leave this to you after this series settle down. 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On 11/29/13 at 05:46pm, Borislav Petkov wrote: On Fri, Nov 29, 2013 at 05:14:16PM +0800, Dave Young wrote: That's reserved for future extension use, who knows if we will need to pass other fields in the future. Hrrmmm, 8*64 = 64 Bytes?? And you can't change it later because of the situation where machines might be using older kexec-tools? Right, compatibility is the reason. Static function - no need for efi_ prefix. Ok. I'm not very satisfied with the function name, any better suggestion? reuse_config use_old_config_phys_addr update_config_entry_with_phys_addr Looks like we're applying quirks so apply_quirks()? Ok, it looks better, will use apply_quirks, thank you. And those kexec-specific quirks need to be behind CONFIG_KEXEC too, btw. All these setup_data passing, remapping etc. is mostly for kexec, add a lot of contiditional #if #endif makes the code a mess. I would prefer to not do this if you are not strongly object. Since Matt suggest to extend the function for other possible field other than smbios, so I would like to move comment to the front of the function like below: /* * For kexec kernel there's some special config table entries which will be * converted to virtual addresses after entering virtual mode. In kexec kernel * we need the physical addresses instead, thus passing them via setup_data * and update the entries to physical addresses in this function. * * Currently only handles smbios which is necessary for HP z420. */ That's better. Is there any idea in your mind how to add the code for HP only? DMI? I'm confused how to connect this with DMI.. Yes, __map_region return an error code will be better. Will you send a patch for __map_region? or I can add one more patch along with next version. I was going to wait for your patches to settle down first so that I don't disturb your work. But if you want to fix this, I certainly wouldn't want to stop you :) but please do so in a prepatch before your series which addresses only this issue. Ok, so I will leave this to you after this series settle down. 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Fri, Nov 29, 2013 at 05:14:16PM +0800, Dave Young wrote: > That's reserved for future extension use, who knows if we will need to > pass other fields in the future. Hrrmmm, 8*64 = 64 Bytes?? And you can't change it later because of the situation where machines might be using older kexec-tools? > > Static function - no need for "efi_" prefix. > > Ok. I'm not very satisfied with the function name, any better > suggestion? > > reuse_config > use_old_config_phys_addr > update_config_entry_with_phys_addr Looks like we're applying quirks so apply_quirks()? And those kexec-specific quirks need to be behind CONFIG_KEXEC too, btw. > Since Matt suggest to extend the function for other possible field > other than smbios, so I would like to move comment to the front > of the function like below: > /* > * For kexec kernel there's some special config table entries which will be > * converted to virtual addresses after entering virtual mode. In kexec kernel > * we need the physical addresses instead, thus passing them via setup_data > * and update the entries to physical addresses in this function. > * > * Currently only handles smbios which is necessary for HP z420. > */ That's better. > Is there any idea in your mind how to add the code for HP only? DMI? > Yes, __map_region return an error code will be better. Will you send > a patch for __map_region? or I can add one more patch along with next > version. I was going to wait for your patches to settle down first so that I don't disturb your work. But if you want to fix this, I certainly wouldn't want to stop you :) but please do so in a prepatch before your series which addresses only this issue. Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On 11/27/13 at 10:17am, Matt Fleming wrote:
> On Wed, 27 Nov, at 12:52:37PM, Dave Young wrote:
> > To make it more readable, I will change them like below:
> >
> > p = efi_runtime_map;
> > md = efi_setup->map;
> > for (i = 0; i < nr_efi_runtime_map; i++) {
> > [...]
> > md += 1;
> > }
>
> Actually, md++ is the canonical way to write this.
Ok.
>
> > >
> > > > + efi_map_region_fixed(md);
> > > > + size = md->num_pages << PAGE_SHIFT;
> > > > + end = md->phys_addr + size;
> > > > +
> > > > + systab = (u64) (unsigned long) efi_phys.systab;
> > > > + if (md->phys_addr <= systab && systab < end) {
> > > > + systab += md->virt_addr - md->phys_addr;
> > > > + efi.systab =
> > > > + (efi_system_table_t *) (unsigned long)
> > > > systab;
> > > > + }
> > > > + if (efi_runtime_map) {
> > > > + memcpy(p, md, memmap.desc_size);
> > > > + p += memmap.desc_size;
> > > > + }
> > >
> > > Is this if () needed? Is it possible to enter the loop and have
> > > 'efi_runtime_map' be NULL?
> >
> > Yes, it is needed. if efi_runtime_map kmalloc fails I only print error, do
> > not
> > return so kernel can still boot, just kexec on efi will not work that has
> > been
> > put in the error message.
>
> OK. On second thought, is there any way to turn the above code into a
> call to efi_save_runtime_map()? Because you've basically duplicated that
> code and I can definitely envisage the two code paths fragmenting over
> time, e.g. when someone makes changes to one but not the other.
OK, will consider to reuse the function.
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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On 11/27/13 at 03:07pm, Borislav Petkov wrote:
> On Tue, Nov 26, 2013 at 01:57:52PM +0800, 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 it need another variable
> > saving,
>
> Why the special handling? Does that mean, this is going to be the case
> for other HP UEFI implementations too?
I have only one HP machine for testing, Maybe Toshi can help to verify
on other machines. Just comment out the function efi_reuse_config to see
if kexec kernel panic.
>
> > it's the smbios physical address, the HP bios
> > also update the SMBIOS address after entering virtual mode
> > besides of the standard fw_vendor,runtime and config table.
> >
> > 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 esdata
> >
> > Signed-off-by: Dave Young
> > ---
> > arch/x86/include/asm/efi.h| 12 +++
> > arch/x86/include/uapi/asm/bootparam.h | 1 +
> > arch/x86/kernel/setup.c | 3 +
> > arch/x86/platform/efi/efi.c | 161
> > ++
> > 4 files changed, 160 insertions(+), 17 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> > index 9fbaeb2..73d5643 100644
> > --- a/arch/x86/include/asm/efi.h
> > +++ b/arch/x86/include/asm/efi.h
> > @@ -133,6 +133,18 @@ 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];
>
> What's that for?
That's reserved for future extension use, who knows if we will need
to pass other fields in the future.
>
> > + efi_memory_desc_t map[0];
> > +};
> > +
> > +extern void parse_efi_setup(u64 phys_addr, u32 data_len);
> > +extern struct efi_setup_data *esdata;
> > +
> > #ifdef CONFIG_EFI
> >
> > static inline bool efi_is_native(void)
>
> [ … ]
>
> > diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> > index c3a2aaa..fafeb40 100644
> > --- a/arch/x86/platform/efi/efi.c
> > +++ b/arch/x86/platform/efi/efi.c
> > @@ -504,8 +531,12 @@ static int __init efi_systab_init(void *phys)
> > }
> >
> > efi_systab.hdr = systab64->hdr;
> > - efi_systab.fw_vendor = systab64->fw_vendor;
> > - tmp |= systab64->fw_vendor;
> > +
> > + if (esdata)
> > + efi_systab.fw_vendor = (unsigned long)esdata->fw_vendor;
> > + else
> > + efi_systab.fw_vendor = systab64->fw_vendor;
>
> efi_systab.fw_vendor = esdata ? (unsigned long)esdata->fw_vendor
> : systab64->fw_vendor;
Ok, will update.
>
> > + tmp |= efi_systab.fw_vendor;
> > efi_systab.fw_revision = systab64->fw_revision;
> > efi_systab.con_in_handle = systab64->con_in_handle;
> > tmp |= systab64->con_in_handle;
> > @@ -519,13 +550,21 @@ 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;
> > + if (esdata)
> > + efi_systab.runtime =
> > + (void *)(unsigned long)esdata->runtime;
> > + else
> > + efi_systab.runtime =
> > + (void *)(unsigned long)systab64->runtime;
>
> Ditto. Which would take care of these linebreaks which are ugly.
Will do.
>
> > + 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;
> > + if (esdata)
> > + efi_systab.tables = (unsigned long)esdata->tables;
> > + else
> > + efi_systab.tables = systab64->tables;
>
> Ditto.
Will do
>
> > + tmp |= efi_systab.tables;
> >
> > early_iounmap(systab64, sizeof(*systab64));
> > #ifdef CONFIG_X86_32
> > @@ -631,6 +670,41 @@ static int __init efi_memmap_init(void)
> > return 0;
> > }
> >
> > +static int __init efi_reuse_config(u64 tables, int nr_tables)
Re: [PATCH v4 07/12] efi: passing kexec necessary efi data via setup_data
On 11/27/13 at 03:07pm, Borislav Petkov wrote:
On Tue, Nov 26, 2013 at 01:57:52PM +0800, 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 it need another variable
saving,
Why the special handling? Does that mean, this is going to be the case
for other HP UEFI implementations too?
I have only one HP machine for testing, Maybe Toshi can help to verify
on other machines. Just comment out the function efi_reuse_config to see
if kexec kernel panic.
it's the smbios physical address, the HP bios
also update the SMBIOS address after entering virtual mode
besides of the standard fw_vendor,runtime and config table.
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 esdata
Signed-off-by: Dave Young dyo...@redhat.com
---
arch/x86/include/asm/efi.h| 12 +++
arch/x86/include/uapi/asm/bootparam.h | 1 +
arch/x86/kernel/setup.c | 3 +
arch/x86/platform/efi/efi.c | 161
++
4 files changed, 160 insertions(+), 17 deletions(-)
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 9fbaeb2..73d5643 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -133,6 +133,18 @@ 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];
What's that for?
That's reserved for future extension use, who knows if we will need
to pass other fields in the future.
+ efi_memory_desc_t map[0];
+};
+
+extern void parse_efi_setup(u64 phys_addr, u32 data_len);
+extern struct efi_setup_data *esdata;
+
#ifdef CONFIG_EFI
static inline bool efi_is_native(void)
[ … ]
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index c3a2aaa..fafeb40 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -504,8 +531,12 @@ static int __init efi_systab_init(void *phys)
}
efi_systab.hdr = systab64-hdr;
- efi_systab.fw_vendor = systab64-fw_vendor;
- tmp |= systab64-fw_vendor;
+
+ if (esdata)
+ efi_systab.fw_vendor = (unsigned long)esdata-fw_vendor;
+ else
+ efi_systab.fw_vendor = systab64-fw_vendor;
efi_systab.fw_vendor = esdata ? (unsigned long)esdata-fw_vendor
: systab64-fw_vendor;
Ok, will update.
+ tmp |= efi_systab.fw_vendor;
efi_systab.fw_revision = systab64-fw_revision;
efi_systab.con_in_handle = systab64-con_in_handle;
tmp |= systab64-con_in_handle;
@@ -519,13 +550,21 @@ 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;
+ if (esdata)
+ efi_systab.runtime =
+ (void *)(unsigned long)esdata-runtime;
+ else
+ efi_systab.runtime =
+ (void *)(unsigned long)systab64-runtime;
Ditto. Which would take care of these linebreaks which are ugly.
Will do.
+ 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;
+ if (esdata)
+ efi_systab.tables = (unsigned long)esdata-tables;
+ else
+ efi_systab.tables = systab64-tables;
Ditto.
Will do
+ tmp |= efi_systab.tables;
early_iounmap(systab64, sizeof(*systab64));
#ifdef CONFIG_X86_32
@@ -631,6 +670,41 @@ static int __init efi_memmap_init(void)
return 0;
}
+static int __init efi_reuse_config(u64 tables, int nr_tables)
Static function - no need for efi_ prefix.
Ok. I'm not very satisfied with the function name, any better
suggestion?
reuse_config
use_old_config_phys_addr
update_config_entry_with_phys_addr
...
+{
+ void *p, *tablep;
Re: [PATCH v4 07/12] efi: passing kexec necessary efi data via setup_data
On 11/27/13 at 10:17am, Matt Fleming wrote:
On Wed, 27 Nov, at 12:52:37PM, Dave Young wrote:
To make it more readable, I will change them like below:
p = efi_runtime_map;
md = efi_setup-map;
for (i = 0; i nr_efi_runtime_map; i++) {
[...]
md += 1;
}
Actually, md++ is the canonical way to write this.
Ok.
+ efi_map_region_fixed(md);
+ size = md-num_pages PAGE_SHIFT;
+ end = md-phys_addr + size;
+
+ systab = (u64) (unsigned long) efi_phys.systab;
+ if (md-phys_addr = systab systab end) {
+ systab += md-virt_addr - md-phys_addr;
+ efi.systab =
+ (efi_system_table_t *) (unsigned long)
systab;
+ }
+ if (efi_runtime_map) {
+ memcpy(p, md, memmap.desc_size);
+ p += memmap.desc_size;
+ }
Is this if () needed? Is it possible to enter the loop and have
'efi_runtime_map' be NULL?
Yes, it is needed. if efi_runtime_map kmalloc fails I only print error, do
not
return so kernel can still boot, just kexec on efi will not work that has
been
put in the error message.
OK. On second thought, is there any way to turn the above code into a
call to efi_save_runtime_map()? Because you've basically duplicated that
code and I can definitely envisage the two code paths fragmenting over
time, e.g. when someone makes changes to one but not the other.
OK, will consider to reuse the function.
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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Fri, Nov 29, 2013 at 05:14:16PM +0800, Dave Young wrote: That's reserved for future extension use, who knows if we will need to pass other fields in the future. Hrrmmm, 8*64 = 64 Bytes?? And you can't change it later because of the situation where machines might be using older kexec-tools? Static function - no need for efi_ prefix. Ok. I'm not very satisfied with the function name, any better suggestion? reuse_config use_old_config_phys_addr update_config_entry_with_phys_addr Looks like we're applying quirks so apply_quirks()? And those kexec-specific quirks need to be behind CONFIG_KEXEC too, btw. Since Matt suggest to extend the function for other possible field other than smbios, so I would like to move comment to the front of the function like below: /* * For kexec kernel there's some special config table entries which will be * converted to virtual addresses after entering virtual mode. In kexec kernel * we need the physical addresses instead, thus passing them via setup_data * and update the entries to physical addresses in this function. * * Currently only handles smbios which is necessary for HP z420. */ That's better. Is there any idea in your mind how to add the code for HP only? DMI? Yes, __map_region return an error code will be better. Will you send a patch for __map_region? or I can add one more patch along with next version. I was going to wait for your patches to settle down first so that I don't disturb your work. But if you want to fix this, I certainly wouldn't want to stop you :) but please do so in a prepatch before your series which addresses only this issue. Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- 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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Tue, Nov 26, 2013 at 01:57:52PM +0800, 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 it need another variable
> saving,
Why the special handling? Does that mean, this is going to be the case
for other HP UEFI implementations too?
> it's the smbios physical address, the HP bios
> also update the SMBIOS address after entering virtual mode
> besides of the standard fw_vendor,runtime and config table.
>
> 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 esdata
>
> Signed-off-by: Dave Young
> ---
> arch/x86/include/asm/efi.h| 12 +++
> arch/x86/include/uapi/asm/bootparam.h | 1 +
> arch/x86/kernel/setup.c | 3 +
> arch/x86/platform/efi/efi.c | 161
> ++
> 4 files changed, 160 insertions(+), 17 deletions(-)
>
> diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> index 9fbaeb2..73d5643 100644
> --- a/arch/x86/include/asm/efi.h
> +++ b/arch/x86/include/asm/efi.h
> @@ -133,6 +133,18 @@ 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];
What's that for?
> + efi_memory_desc_t map[0];
> +};
> +
> +extern void parse_efi_setup(u64 phys_addr, u32 data_len);
> +extern struct efi_setup_data *esdata;
> +
> #ifdef CONFIG_EFI
>
> static inline bool efi_is_native(void)
[ … ]
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index c3a2aaa..fafeb40 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -504,8 +531,12 @@ static int __init efi_systab_init(void *phys)
> }
>
> efi_systab.hdr = systab64->hdr;
> - efi_systab.fw_vendor = systab64->fw_vendor;
> - tmp |= systab64->fw_vendor;
> +
> + if (esdata)
> + efi_systab.fw_vendor = (unsigned long)esdata->fw_vendor;
> + else
> + efi_systab.fw_vendor = systab64->fw_vendor;
efi_systab.fw_vendor = esdata ? (unsigned long)esdata->fw_vendor
: systab64->fw_vendor;
> + tmp |= efi_systab.fw_vendor;
> efi_systab.fw_revision = systab64->fw_revision;
> efi_systab.con_in_handle = systab64->con_in_handle;
> tmp |= systab64->con_in_handle;
> @@ -519,13 +550,21 @@ 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;
> + if (esdata)
> + efi_systab.runtime =
> + (void *)(unsigned long)esdata->runtime;
> + else
> + efi_systab.runtime =
> + (void *)(unsigned long)systab64->runtime;
Ditto. Which would take care of these linebreaks which are ugly.
> + 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;
> + if (esdata)
> + efi_systab.tables = (unsigned long)esdata->tables;
> + else
> + efi_systab.tables = systab64->tables;
Ditto.
> + tmp |= efi_systab.tables;
>
> early_iounmap(systab64, sizeof(*systab64));
> #ifdef CONFIG_X86_32
> @@ -631,6 +670,41 @@ static int __init efi_memmap_init(void)
> return 0;
> }
>
> +static int __init efi_reuse_config(u64 tables, int nr_tables)
Static function - no need for "efi_" prefix.
> +{
> + void *p, *tablep;
> + int i, sz;
> +
> + if (!efi_enabled(EFI_64BIT))
> + return 0;
> +
> + sz = sizeof(efi_config_table_64_t);
> +
> + p = tablep = early_memremap(tables, nr_tables * sz);
> + if (!p) {
> + pr_err("Could not map Configuration table!\n");
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < efi.systab->nr_tables; i++) {
> + efi_guid_t guid;
> +
> + guid =
Re: [PATCH v4 07/12] efi: passing kexec necessary efi data via setup_data
On Wed, 27 Nov, at 12:52:37PM, Dave Young wrote:
> To make it more readable, I will change them like below:
>
> p = efi_runtime_map;
> md = efi_setup->map;
> for (i = 0; i < nr_efi_runtime_map; i++) {
> [...]
> md += 1;
> }
Actually, md++ is the canonical way to write this.
> >
> > > + efi_map_region_fixed(md);
> > > + size = md->num_pages << PAGE_SHIFT;
> > > + end = md->phys_addr + size;
> > > +
> > > + systab = (u64) (unsigned long) efi_phys.systab;
> > > + if (md->phys_addr <= systab && systab < end) {
> > > + systab += md->virt_addr - md->phys_addr;
> > > + efi.systab =
> > > + (efi_system_table_t *) (unsigned long) systab;
> > > + }
> > > + if (efi_runtime_map) {
> > > + memcpy(p, md, memmap.desc_size);
> > > + p += memmap.desc_size;
> > > + }
> >
> > Is this if () needed? Is it possible to enter the loop and have
> > 'efi_runtime_map' be NULL?
>
> Yes, it is needed. if efi_runtime_map kmalloc fails I only print error, do not
> return so kernel can still boot, just kexec on efi will not work that has been
> put in the error message.
OK. On second thought, is there any way to turn the above code into a
call to efi_save_runtime_map()? Because you've basically duplicated that
code and I can definitely envisage the two code paths fragmenting over
time, e.g. when someone makes changes to one but not the other.
--
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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Wed, 27 Nov, at 12:52:37PM, Dave Young wrote:
To make it more readable, I will change them like below:
p = efi_runtime_map;
md = efi_setup-map;
for (i = 0; i nr_efi_runtime_map; i++) {
[...]
md += 1;
}
Actually, md++ is the canonical way to write this.
+ efi_map_region_fixed(md);
+ size = md-num_pages PAGE_SHIFT;
+ end = md-phys_addr + size;
+
+ systab = (u64) (unsigned long) efi_phys.systab;
+ if (md-phys_addr = systab systab end) {
+ systab += md-virt_addr - md-phys_addr;
+ efi.systab =
+ (efi_system_table_t *) (unsigned long) systab;
+ }
+ if (efi_runtime_map) {
+ memcpy(p, md, memmap.desc_size);
+ p += memmap.desc_size;
+ }
Is this if () needed? Is it possible to enter the loop and have
'efi_runtime_map' be NULL?
Yes, it is needed. if efi_runtime_map kmalloc fails I only print error, do not
return so kernel can still boot, just kexec on efi will not work that has been
put in the error message.
OK. On second thought, is there any way to turn the above code into a
call to efi_save_runtime_map()? Because you've basically duplicated that
code and I can definitely envisage the two code paths fragmenting over
time, e.g. when someone makes changes to one but not the other.
--
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 v4 07/12] efi: passing kexec necessary efi data via setup_data
On Tue, Nov 26, 2013 at 01:57:52PM +0800, 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 it need another variable
saving,
Why the special handling? Does that mean, this is going to be the case
for other HP UEFI implementations too?
it's the smbios physical address, the HP bios
also update the SMBIOS address after entering virtual mode
besides of the standard fw_vendor,runtime and config table.
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 esdata
Signed-off-by: Dave Young dyo...@redhat.com
---
arch/x86/include/asm/efi.h| 12 +++
arch/x86/include/uapi/asm/bootparam.h | 1 +
arch/x86/kernel/setup.c | 3 +
arch/x86/platform/efi/efi.c | 161
++
4 files changed, 160 insertions(+), 17 deletions(-)
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 9fbaeb2..73d5643 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -133,6 +133,18 @@ 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];
What's that for?
+ efi_memory_desc_t map[0];
+};
+
+extern void parse_efi_setup(u64 phys_addr, u32 data_len);
+extern struct efi_setup_data *esdata;
+
#ifdef CONFIG_EFI
static inline bool efi_is_native(void)
[ … ]
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index c3a2aaa..fafeb40 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -504,8 +531,12 @@ static int __init efi_systab_init(void *phys)
}
efi_systab.hdr = systab64-hdr;
- efi_systab.fw_vendor = systab64-fw_vendor;
- tmp |= systab64-fw_vendor;
+
+ if (esdata)
+ efi_systab.fw_vendor = (unsigned long)esdata-fw_vendor;
+ else
+ efi_systab.fw_vendor = systab64-fw_vendor;
efi_systab.fw_vendor = esdata ? (unsigned long)esdata-fw_vendor
: systab64-fw_vendor;
+ tmp |= efi_systab.fw_vendor;
efi_systab.fw_revision = systab64-fw_revision;
efi_systab.con_in_handle = systab64-con_in_handle;
tmp |= systab64-con_in_handle;
@@ -519,13 +550,21 @@ 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;
+ if (esdata)
+ efi_systab.runtime =
+ (void *)(unsigned long)esdata-runtime;
+ else
+ efi_systab.runtime =
+ (void *)(unsigned long)systab64-runtime;
Ditto. Which would take care of these linebreaks which are ugly.
+ 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;
+ if (esdata)
+ efi_systab.tables = (unsigned long)esdata-tables;
+ else
+ efi_systab.tables = systab64-tables;
Ditto.
+ tmp |= efi_systab.tables;
early_iounmap(systab64, sizeof(*systab64));
#ifdef CONFIG_X86_32
@@ -631,6 +670,41 @@ static int __init efi_memmap_init(void)
return 0;
}
+static int __init efi_reuse_config(u64 tables, int nr_tables)
Static function - no need for efi_ prefix.
+{
+ void *p, *tablep;
+ int i, sz;
+
+ if (!efi_enabled(EFI_64BIT))
+ return 0;
+
+ sz = sizeof(efi_config_table_64_t);
+
+ p = tablep = early_memremap(tables, nr_tables * sz);
+ if (!p) {
+ pr_err(Could not map Configuration table!\n);
+ return -ENOMEM;
+ }
+
+ for (i = 0; i efi.systab-nr_tables; i++) {
+ efi_guid_t guid;
+
+ guid = ((efi_config_table_64_t *)p)-guid;
+
+ /*
+ HP z420 workstation smbios will be convert to
+
Re: [PATCH v4 07/12] efi: passing kexec necessary efi data via setup_data
On 11/26/13 at 10:04pm, Matt Fleming wrote:
> On Tue, 26 Nov, at 01:57:52PM, 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 it need another variable
> > saving, it's the smbios physical address, the HP bios
> > also update the SMBIOS address after entering virtual mode
> > besides of the standard fw_vendor,runtime and config table.
> >
> > 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 esdata
> >
> > Signed-off-by: Dave Young
> > ---
> > arch/x86/include/asm/efi.h| 12 +++
> > arch/x86/include/uapi/asm/bootparam.h | 1 +
> > arch/x86/kernel/setup.c | 3 +
> > arch/x86/platform/efi/efi.c | 161
> > ++
> > 4 files changed, 160 insertions(+), 17 deletions(-)
>
> [...]
>
> > +void __init parse_efi_setup(u64 phys_addr, u32 data_len)
> > +{
> > + int size;
> > + struct setup_data *sdata;
> > + u64 esdata_phys;
> > +
> > + if (!efi_enabled(EFI_64BIT)) {
> > + pr_warn("skipping setup_data on EFI 32BIT!");
> > + return;
> > + }
>
> Hmm... this warning could be more informative. Perhaps something along
> the lines of,
>
> "SETUP_EFI not supported on 32-bit\n"
Will do
>
> because the reason we skip is because it isn't supported.
>
> > +
> > + sdata = early_memremap(phys_addr, data_len);
> > + if (!sdata)
> > + return;
> > +
> > + size = data_len - sizeof(struct setup_data);
> > +
> > + esdata_phys = phys_addr + sizeof(struct setup_data);
> > +
> > + nr_efi_runtime_map = (size - sizeof(struct efi_setup_data)) /
> > + sizeof(efi_memory_desc_t);
> > + early_iounmap(sdata, data_len);
> > +
> > + /* iounmap esdata in function efi_enter_virtual_mode */
> > + esdata = early_memremap(esdata_phys, size);
> > +}
> >
> > static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
> > {
> > @@ -504,8 +531,12 @@ static int __init efi_systab_init(void *phys)
> > }
> >
> > efi_systab.hdr = systab64->hdr;
> > - efi_systab.fw_vendor = systab64->fw_vendor;
> > - tmp |= systab64->fw_vendor;
> > +
> > + if (esdata)
>
> Could we name this something more explicit? 'efi_setup' perhaps?
Sure, just efi_setup_data is a bit long, will use efi_setup in next version.
>
> > @@ -631,6 +670,41 @@ static int __init efi_memmap_init(void)
> > return 0;
> > }
> >
> > +static int __init efi_reuse_config(u64 tables, int nr_tables)
> > +{
> > + void *p, *tablep;
> > + int i, sz;
> > +
> > + if (!efi_enabled(EFI_64BIT))
> > + return 0;
> > +
> > + sz = sizeof(efi_config_table_64_t);
> > +
> > + p = tablep = early_memremap(tables, nr_tables * sz);
> > + if (!p) {
> > + pr_err("Could not map Configuration table!\n");
> > + return -ENOMEM;
> > + }
> > +
> > + for (i = 0; i < efi.systab->nr_tables; i++) {
> > + efi_guid_t guid;
> > +
> > + guid = ((efi_config_table_64_t *)p)->guid;
> > +
> > + /*
> > + HP z420 workstation smbios will be convert to
> > + virtual address after enter virtual mode.
> > + Thus in case kexec/kdump the physical address
> > + will be passed in setup_data.
> > + */
>
> This isn't the correct multi-line comment format,
Will update
>
> /*
>* This is the preferred way to to write a multi-line
>* comment.
>*/
>
> > + if (!efi_guidcmp(guid, SMBIOS_TABLE_GUID))
> > + ((efi_config_table_64_t *)p)->table = esdata->smbios;
> > + p += sz;
> > + }
> > + early_iounmap(tablep, nr_tables * sz);
> > + return 0;
> > +}
> > +
> > void __init efi_init(void)
> > {
> > efi_char16_t *c16;
> > @@ -676,6 +750,9 @@ void __init efi_init(void)
> > efi.systab->hdr.revision >> 16,
> > efi.systab->hdr.revision & 0x, vendor);
> >
> > + if (esdata && esdata->smbios)
> > + efi_reuse_config(efi.systab->tables, efi.systab->nr_tables);
> > +
>
> Would it make sense to move the ->smbios check inside efi_reuse_config()
> in case we need to extend the number of tables in the future?
Good suggestion, will do.
>
> > if (efi_config_init(arch_tables))
> > return;
> >
> > @@ -886,6 +963,43 @@ ret:
> > }
> >
> > /*
> > + * map efi regions which was passed via setup_data
> > + * the virt_addr is a fixed addr which was used in
> > + * 1st kernel of kexec boot.
> >
Re: [PATCH v4 07/12] efi: passing kexec necessary efi data via setup_data
On Tue, 26 Nov, at 01:57:52PM, 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 it need another variable
> saving, it's the smbios physical address, the HP bios
> also update the SMBIOS address after entering virtual mode
> besides of the standard fw_vendor,runtime and config table.
>
> 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 esdata
>
> Signed-off-by: Dave Young
> ---
> arch/x86/include/asm/efi.h| 12 +++
> arch/x86/include/uapi/asm/bootparam.h | 1 +
> arch/x86/kernel/setup.c | 3 +
> arch/x86/platform/efi/efi.c | 161
> ++
> 4 files changed, 160 insertions(+), 17 deletions(-)
[...]
> +void __init parse_efi_setup(u64 phys_addr, u32 data_len)
> +{
> + int size;
> + struct setup_data *sdata;
> + u64 esdata_phys;
> +
> + if (!efi_enabled(EFI_64BIT)) {
> + pr_warn("skipping setup_data on EFI 32BIT!");
> + return;
> + }
Hmm... this warning could be more informative. Perhaps something along
the lines of,
"SETUP_EFI not supported on 32-bit\n"
because the reason we skip is because it isn't supported.
> +
> + sdata = early_memremap(phys_addr, data_len);
> + if (!sdata)
> + return;
> +
> + size = data_len - sizeof(struct setup_data);
> +
> + esdata_phys = phys_addr + sizeof(struct setup_data);
> +
> + nr_efi_runtime_map = (size - sizeof(struct efi_setup_data)) /
> + sizeof(efi_memory_desc_t);
> + early_iounmap(sdata, data_len);
> +
> + /* iounmap esdata in function efi_enter_virtual_mode */
> + esdata = early_memremap(esdata_phys, size);
> +}
>
> static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
> {
> @@ -504,8 +531,12 @@ static int __init efi_systab_init(void *phys)
> }
>
> efi_systab.hdr = systab64->hdr;
> - efi_systab.fw_vendor = systab64->fw_vendor;
> - tmp |= systab64->fw_vendor;
> +
> + if (esdata)
Could we name this something more explicit? 'efi_setup' perhaps?
> @@ -631,6 +670,41 @@ static int __init efi_memmap_init(void)
> return 0;
> }
>
> +static int __init efi_reuse_config(u64 tables, int nr_tables)
> +{
> + void *p, *tablep;
> + int i, sz;
> +
> + if (!efi_enabled(EFI_64BIT))
> + return 0;
> +
> + sz = sizeof(efi_config_table_64_t);
> +
> + p = tablep = early_memremap(tables, nr_tables * sz);
> + if (!p) {
> + pr_err("Could not map Configuration table!\n");
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < efi.systab->nr_tables; i++) {
> + efi_guid_t guid;
> +
> + guid = ((efi_config_table_64_t *)p)->guid;
> +
> + /*
> + HP z420 workstation smbios will be convert to
> + virtual address after enter virtual mode.
> + Thus in case kexec/kdump the physical address
> + will be passed in setup_data.
> + */
This isn't the correct multi-line comment format,
/*
* This is the preferred way to to write a multi-line
* comment.
*/
> + if (!efi_guidcmp(guid, SMBIOS_TABLE_GUID))
> + ((efi_config_table_64_t *)p)->table = esdata->smbios;
> + p += sz;
> + }
> + early_iounmap(tablep, nr_tables * sz);
> + return 0;
> +}
> +
> void __init efi_init(void)
> {
> efi_char16_t *c16;
> @@ -676,6 +750,9 @@ void __init efi_init(void)
> efi.systab->hdr.revision >> 16,
> efi.systab->hdr.revision & 0x, vendor);
>
> + if (esdata && esdata->smbios)
> + efi_reuse_config(efi.systab->tables, efi.systab->nr_tables);
> +
Would it make sense to move the ->smbios check inside efi_reuse_config()
in case we need to extend the number of tables in the future?
> if (efi_config_init(arch_tables))
> return;
>
> @@ -886,6 +963,43 @@ ret:
> }
>
> /*
> + * map efi regions which was passed via setup_data
> + * the virt_addr is a fixed addr which was used in
> + * 1st kernel of kexec boot.
> + */
> +static void __init efi_map_regions_fixed(void)
> +{
> + int i;
> + unsigned long size;
> + efi_memory_desc_t *md;
> + u64 end, systab;
> + void *p;
> +
> + efi_runtime_map = kzalloc(nr_efi_runtime_map * memmap.desc_size,
> + GFP_KERNEL);
> + if (!efi_runtime_map)
> + pr_err("Out
Re: [PATCH v4 07/12] efi: passing kexec necessary efi data via setup_data
On Tue, 26 Nov, at 01:57:52PM, 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 it need another variable
saving, it's the smbios physical address, the HP bios
also update the SMBIOS address after entering virtual mode
besides of the standard fw_vendor,runtime and config table.
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 esdata
Signed-off-by: Dave Young dyo...@redhat.com
---
arch/x86/include/asm/efi.h| 12 +++
arch/x86/include/uapi/asm/bootparam.h | 1 +
arch/x86/kernel/setup.c | 3 +
arch/x86/platform/efi/efi.c | 161
++
4 files changed, 160 insertions(+), 17 deletions(-)
[...]
+void __init parse_efi_setup(u64 phys_addr, u32 data_len)
+{
+ int size;
+ struct setup_data *sdata;
+ u64 esdata_phys;
+
+ if (!efi_enabled(EFI_64BIT)) {
+ pr_warn(skipping setup_data on EFI 32BIT!);
+ return;
+ }
Hmm... this warning could be more informative. Perhaps something along
the lines of,
SETUP_EFI not supported on 32-bit\n
because the reason we skip is because it isn't supported.
+
+ sdata = early_memremap(phys_addr, data_len);
+ if (!sdata)
+ return;
+
+ size = data_len - sizeof(struct setup_data);
+
+ esdata_phys = phys_addr + sizeof(struct setup_data);
+
+ nr_efi_runtime_map = (size - sizeof(struct efi_setup_data)) /
+ sizeof(efi_memory_desc_t);
+ early_iounmap(sdata, data_len);
+
+ /* iounmap esdata in function efi_enter_virtual_mode */
+ esdata = early_memremap(esdata_phys, size);
+}
static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
{
@@ -504,8 +531,12 @@ static int __init efi_systab_init(void *phys)
}
efi_systab.hdr = systab64-hdr;
- efi_systab.fw_vendor = systab64-fw_vendor;
- tmp |= systab64-fw_vendor;
+
+ if (esdata)
Could we name this something more explicit? 'efi_setup' perhaps?
@@ -631,6 +670,41 @@ static int __init efi_memmap_init(void)
return 0;
}
+static int __init efi_reuse_config(u64 tables, int nr_tables)
+{
+ void *p, *tablep;
+ int i, sz;
+
+ if (!efi_enabled(EFI_64BIT))
+ return 0;
+
+ sz = sizeof(efi_config_table_64_t);
+
+ p = tablep = early_memremap(tables, nr_tables * sz);
+ if (!p) {
+ pr_err(Could not map Configuration table!\n);
+ return -ENOMEM;
+ }
+
+ for (i = 0; i efi.systab-nr_tables; i++) {
+ efi_guid_t guid;
+
+ guid = ((efi_config_table_64_t *)p)-guid;
+
+ /*
+ HP z420 workstation smbios will be convert to
+ virtual address after enter virtual mode.
+ Thus in case kexec/kdump the physical address
+ will be passed in setup_data.
+ */
This isn't the correct multi-line comment format,
/*
* This is the preferred way to to write a multi-line
* comment.
*/
+ if (!efi_guidcmp(guid, SMBIOS_TABLE_GUID))
+ ((efi_config_table_64_t *)p)-table = esdata-smbios;
+ p += sz;
+ }
+ early_iounmap(tablep, nr_tables * sz);
+ return 0;
+}
+
void __init efi_init(void)
{
efi_char16_t *c16;
@@ -676,6 +750,9 @@ void __init efi_init(void)
efi.systab-hdr.revision 16,
efi.systab-hdr.revision 0x, vendor);
+ if (esdata esdata-smbios)
+ efi_reuse_config(efi.systab-tables, efi.systab-nr_tables);
+
Would it make sense to move the -smbios check inside efi_reuse_config()
in case we need to extend the number of tables in the future?
if (efi_config_init(arch_tables))
return;
@@ -886,6 +963,43 @@ ret:
}
/*
+ * map efi regions which was passed via setup_data
+ * the virt_addr is a fixed addr which was used in
+ * 1st kernel of kexec boot.
+ */
+static void __init efi_map_regions_fixed(void)
+{
+ int i;
+ unsigned long size;
+ efi_memory_desc_t *md;
+ u64 end, systab;
+ void *p;
+
+ efi_runtime_map = kzalloc(nr_efi_runtime_map * memmap.desc_size,
+ GFP_KERNEL);
+ if (!efi_runtime_map)
+ pr_err(Out of memory, EFI runtime on nested kexec
non-functional!\n);
+
+ for (i = 0, p = efi_runtime_map; i nr_efi_runtime_map; i++) {
+
Re: [PATCH v4 07/12] efi: passing kexec necessary efi data via setup_data
On 11/26/13 at 10:04pm, Matt Fleming wrote:
On Tue, 26 Nov, at 01:57:52PM, 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 it need another variable
saving, it's the smbios physical address, the HP bios
also update the SMBIOS address after entering virtual mode
besides of the standard fw_vendor,runtime and config table.
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 esdata
Signed-off-by: Dave Young dyo...@redhat.com
---
arch/x86/include/asm/efi.h| 12 +++
arch/x86/include/uapi/asm/bootparam.h | 1 +
arch/x86/kernel/setup.c | 3 +
arch/x86/platform/efi/efi.c | 161
++
4 files changed, 160 insertions(+), 17 deletions(-)
[...]
+void __init parse_efi_setup(u64 phys_addr, u32 data_len)
+{
+ int size;
+ struct setup_data *sdata;
+ u64 esdata_phys;
+
+ if (!efi_enabled(EFI_64BIT)) {
+ pr_warn(skipping setup_data on EFI 32BIT!);
+ return;
+ }
Hmm... this warning could be more informative. Perhaps something along
the lines of,
SETUP_EFI not supported on 32-bit\n
Will do
because the reason we skip is because it isn't supported.
+
+ sdata = early_memremap(phys_addr, data_len);
+ if (!sdata)
+ return;
+
+ size = data_len - sizeof(struct setup_data);
+
+ esdata_phys = phys_addr + sizeof(struct setup_data);
+
+ nr_efi_runtime_map = (size - sizeof(struct efi_setup_data)) /
+ sizeof(efi_memory_desc_t);
+ early_iounmap(sdata, data_len);
+
+ /* iounmap esdata in function efi_enter_virtual_mode */
+ esdata = early_memremap(esdata_phys, size);
+}
static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
{
@@ -504,8 +531,12 @@ static int __init efi_systab_init(void *phys)
}
efi_systab.hdr = systab64-hdr;
- efi_systab.fw_vendor = systab64-fw_vendor;
- tmp |= systab64-fw_vendor;
+
+ if (esdata)
Could we name this something more explicit? 'efi_setup' perhaps?
Sure, just efi_setup_data is a bit long, will use efi_setup in next version.
@@ -631,6 +670,41 @@ static int __init efi_memmap_init(void)
return 0;
}
+static int __init efi_reuse_config(u64 tables, int nr_tables)
+{
+ void *p, *tablep;
+ int i, sz;
+
+ if (!efi_enabled(EFI_64BIT))
+ return 0;
+
+ sz = sizeof(efi_config_table_64_t);
+
+ p = tablep = early_memremap(tables, nr_tables * sz);
+ if (!p) {
+ pr_err(Could not map Configuration table!\n);
+ return -ENOMEM;
+ }
+
+ for (i = 0; i efi.systab-nr_tables; i++) {
+ efi_guid_t guid;
+
+ guid = ((efi_config_table_64_t *)p)-guid;
+
+ /*
+ HP z420 workstation smbios will be convert to
+ virtual address after enter virtual mode.
+ Thus in case kexec/kdump the physical address
+ will be passed in setup_data.
+ */
This isn't the correct multi-line comment format,
Will update
/*
* This is the preferred way to to write a multi-line
* comment.
*/
+ if (!efi_guidcmp(guid, SMBIOS_TABLE_GUID))
+ ((efi_config_table_64_t *)p)-table = esdata-smbios;
+ p += sz;
+ }
+ early_iounmap(tablep, nr_tables * sz);
+ return 0;
+}
+
void __init efi_init(void)
{
efi_char16_t *c16;
@@ -676,6 +750,9 @@ void __init efi_init(void)
efi.systab-hdr.revision 16,
efi.systab-hdr.revision 0x, vendor);
+ if (esdata esdata-smbios)
+ efi_reuse_config(efi.systab-tables, efi.systab-nr_tables);
+
Would it make sense to move the -smbios check inside efi_reuse_config()
in case we need to extend the number of tables in the future?
Good suggestion, will do.
if (efi_config_init(arch_tables))
return;
@@ -886,6 +963,43 @@ ret:
}
/*
+ * map efi regions which was passed via setup_data
+ * the virt_addr is a fixed addr which was used in
+ * 1st kernel of kexec boot.
+ */
+static void __init efi_map_regions_fixed(void)
+{
+ int i;
+ unsigned long size;
+ efi_memory_desc_t *md;
+ u64 end, systab;
+ void *p;
+
+ efi_runtime_map = kzalloc(nr_efi_runtime_map * memmap.desc_size,
+ GFP_KERNEL);
[PATCH v4 07/12] efi: passing kexec necessary efi data via setup_data
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 it need another variable
saving, it's the smbios physical address, the HP bios
also update the SMBIOS address after entering virtual mode
besides of the standard fw_vendor,runtime and config table.
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 esdata
Signed-off-by: Dave Young
---
arch/x86/include/asm/efi.h| 12 +++
arch/x86/include/uapi/asm/bootparam.h | 1 +
arch/x86/kernel/setup.c | 3 +
arch/x86/platform/efi/efi.c | 161 ++
4 files changed, 160 insertions(+), 17 deletions(-)
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 9fbaeb2..73d5643 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -133,6 +133,18 @@ 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];
+};
+
+extern void parse_efi_setup(u64 phys_addr, u32 data_len);
+extern struct efi_setup_data *esdata;
+
#ifdef CONFIG_EFI
static inline bool efi_is_native(void)
diff --git a/arch/x86/include/uapi/asm/bootparam.h
b/arch/x86/include/uapi/asm/bootparam.h
index c15ddaf..ed6afc9 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -6,6 +6,7 @@
#define SETUP_E820_EXT 1
#define SETUP_DTB 2
#define SETUP_PCI 3
+#define SETUP_EFI 4
/* ram_size flags */
#define RAMDISK_IMAGE_START_MASK 0x07FF
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f0de629..ecf225e 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -447,6 +447,9 @@ static void __init parse_setup_data(void)
case SETUP_DTB:
add_dtb(pa_data);
break;
+ case SETUP_EFI:
+ parse_efi_setup(pa_data, data_len);
+ break;
default:
break;
}
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index c3a2aaa..fafeb40 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -78,6 +78,7 @@ static __initdata efi_config_table_type_t arch_tables[] = {
void *efi_runtime_map;
int nr_efi_runtime_map;
+struct efi_setup_data *esdata;
/*
* Returns 1 if 'facility' is enabled, 0 otherwise.
@@ -115,6 +116,32 @@ static int __init setup_storage_paranoia(char *arg)
}
early_param("efi_no_storage_paranoia", setup_storage_paranoia);
+void __init parse_efi_setup(u64 phys_addr, u32 data_len)
+{
+ int size;
+ struct setup_data *sdata;
+ u64 esdata_phys;
+
+ if (!efi_enabled(EFI_64BIT)) {
+ pr_warn("skipping setup_data on EFI 32BIT!");
+ return;
+ }
+
+ sdata = early_memremap(phys_addr, data_len);
+ if (!sdata)
+ return;
+
+ size = data_len - sizeof(struct setup_data);
+
+ esdata_phys = phys_addr + sizeof(struct setup_data);
+
+ nr_efi_runtime_map = (size - sizeof(struct efi_setup_data)) /
+ sizeof(efi_memory_desc_t);
+ early_iounmap(sdata, data_len);
+
+ /* iounmap esdata in function efi_enter_virtual_mode */
+ esdata = early_memremap(esdata_phys, size);
+}
static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
{
@@ -504,8 +531,12 @@ static int __init efi_systab_init(void *phys)
}
efi_systab.hdr = systab64->hdr;
- efi_systab.fw_vendor = systab64->fw_vendor;
- tmp |= systab64->fw_vendor;
+
+ if (esdata)
+ efi_systab.fw_vendor = (unsigned long)esdata->fw_vendor;
+ else
+ efi_systab.fw_vendor = systab64->fw_vendor;
+ tmp |= efi_systab.fw_vendor;
efi_systab.fw_revision = systab64->fw_revision;
efi_systab.con_in_handle = systab64->con_in_handle;
tmp |= systab64->con_in_handle;
@@ -519,13 +550,21 @@ 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
[PATCH v4 07/12] efi: passing kexec necessary efi data via setup_data
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 it need another variable
saving, it's the smbios physical address, the HP bios
also update the SMBIOS address after entering virtual mode
besides of the standard fw_vendor,runtime and config table.
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 esdata
Signed-off-by: Dave Young dyo...@redhat.com
---
arch/x86/include/asm/efi.h| 12 +++
arch/x86/include/uapi/asm/bootparam.h | 1 +
arch/x86/kernel/setup.c | 3 +
arch/x86/platform/efi/efi.c | 161 ++
4 files changed, 160 insertions(+), 17 deletions(-)
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 9fbaeb2..73d5643 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -133,6 +133,18 @@ 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];
+};
+
+extern void parse_efi_setup(u64 phys_addr, u32 data_len);
+extern struct efi_setup_data *esdata;
+
#ifdef CONFIG_EFI
static inline bool efi_is_native(void)
diff --git a/arch/x86/include/uapi/asm/bootparam.h
b/arch/x86/include/uapi/asm/bootparam.h
index c15ddaf..ed6afc9 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -6,6 +6,7 @@
#define SETUP_E820_EXT 1
#define SETUP_DTB 2
#define SETUP_PCI 3
+#define SETUP_EFI 4
/* ram_size flags */
#define RAMDISK_IMAGE_START_MASK 0x07FF
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f0de629..ecf225e 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -447,6 +447,9 @@ static void __init parse_setup_data(void)
case SETUP_DTB:
add_dtb(pa_data);
break;
+ case SETUP_EFI:
+ parse_efi_setup(pa_data, data_len);
+ break;
default:
break;
}
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index c3a2aaa..fafeb40 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -78,6 +78,7 @@ static __initdata efi_config_table_type_t arch_tables[] = {
void *efi_runtime_map;
int nr_efi_runtime_map;
+struct efi_setup_data *esdata;
/*
* Returns 1 if 'facility' is enabled, 0 otherwise.
@@ -115,6 +116,32 @@ static int __init setup_storage_paranoia(char *arg)
}
early_param(efi_no_storage_paranoia, setup_storage_paranoia);
+void __init parse_efi_setup(u64 phys_addr, u32 data_len)
+{
+ int size;
+ struct setup_data *sdata;
+ u64 esdata_phys;
+
+ if (!efi_enabled(EFI_64BIT)) {
+ pr_warn(skipping setup_data on EFI 32BIT!);
+ return;
+ }
+
+ sdata = early_memremap(phys_addr, data_len);
+ if (!sdata)
+ return;
+
+ size = data_len - sizeof(struct setup_data);
+
+ esdata_phys = phys_addr + sizeof(struct setup_data);
+
+ nr_efi_runtime_map = (size - sizeof(struct efi_setup_data)) /
+ sizeof(efi_memory_desc_t);
+ early_iounmap(sdata, data_len);
+
+ /* iounmap esdata in function efi_enter_virtual_mode */
+ esdata = early_memremap(esdata_phys, size);
+}
static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
{
@@ -504,8 +531,12 @@ static int __init efi_systab_init(void *phys)
}
efi_systab.hdr = systab64-hdr;
- efi_systab.fw_vendor = systab64-fw_vendor;
- tmp |= systab64-fw_vendor;
+
+ if (esdata)
+ efi_systab.fw_vendor = (unsigned long)esdata-fw_vendor;
+ else
+ efi_systab.fw_vendor = systab64-fw_vendor;
+ tmp |= efi_systab.fw_vendor;
efi_systab.fw_revision = systab64-fw_revision;
efi_systab.con_in_handle = systab64-con_in_handle;
tmp |= systab64-con_in_handle;
@@ -519,13 +550,21 @@ 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
