* [PATCH] efi: Free existing memory map before installing new memory map
@ 2018-06-26 2:41 Sai Praneeth Prakhya
2018-06-26 3:15 ` kbuild test robot
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Sai Praneeth Prakhya @ 2018-06-26 2:41 UTC (permalink / raw)
To: linux-efi, linux-kernel
Cc: Sai Praneeth, Lee Chun-Yi, Borislav Petkov, Dave Young,
Laszlo Ersek, Bhupesh Sharma, Ricardo Neri, Ravi Shankar,
Matt Fleming, Ard Biesheuvel
From: Sai Praneeth <sai.praneeth.prakhya@intel.com>
efi_memmap_install(), unmaps the existing memory map and installs the
new memory map but doesn't free the memory allocated to the existing
memory map. Fortunately, the details about the existing memory map are
stored in efi.memmap. Hence, use them to free the memory.
Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Lee Chun-Yi <jlee@suse.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Young <dyoung@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Ricardo Neri <ricardo.neri@intel.com>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Note: Patch based on efi tree @https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
drivers/firmware/efi/memmap.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c
index 678e85704054..68b27b14fe94 100644
--- a/drivers/firmware/efi/memmap.c
+++ b/drivers/firmware/efi/memmap.c
@@ -229,6 +229,9 @@ int __init efi_memmap_install(phys_addr_t addr, unsigned int nr_map)
efi_memmap_unmap();
+ /* Free the memory allocated to the existing memory map */
+ efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, efi.memmap.late);
+
data.phys_map = addr;
data.size = efi.memmap.desc_size * nr_map;
data.desc_version = efi.memmap.desc_version;
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] efi: Free existing memory map before installing new memory map 2018-06-26 2:41 [PATCH] efi: Free existing memory map before installing new memory map Sai Praneeth Prakhya @ 2018-06-26 3:15 ` kbuild test robot 2018-06-26 3:15 ` kbuild test robot 2018-06-26 9:38 ` Ard Biesheuvel 2 siblings, 0 replies; 14+ messages in thread From: kbuild test robot @ 2018-06-26 3:15 UTC (permalink / raw) Cc: kbuild-all, linux-efi, linux-kernel, Sai Praneeth, Lee Chun-Yi, Borislav Petkov, Dave Young, Laszlo Ersek, Bhupesh Sharma, Ricardo Neri, Ravi Shankar, Matt Fleming, Ard Biesheuvel [-- Attachment #1: Type: text/plain, Size: 2330 bytes --] Hi Sai, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.18-rc2 next-20180625] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Sai-Praneeth-Prakhya/efi-Free-existing-memory-map-before-installing-new-memory-map/20180626-104301 config: x86_64-allyesconfig (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/firmware//efi/memmap.c: In function 'efi_memmap_install': >> drivers/firmware//efi/memmap.c:199:2: error: implicit declaration of function 'efi_memmap_free'; did you mean 'vmemmap_free'? [-Werror=implicit-function-declaration] efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, efi.memmap.late); ^~~~~~~~~~~~~~~ vmemmap_free cc1: some warnings being treated as errors vim +199 drivers/firmware//efi/memmap.c 180 181 /** 182 * efi_memmap_install - Install a new EFI memory map in efi.memmap 183 * @addr: Physical address of the memory map 184 * @nr_map: Number of entries in the memory map 185 * 186 * Unlike efi_memmap_init_*(), this function does not allow the caller 187 * to switch from early to late mappings. It simply uses the existing 188 * mapping function and installs the new memmap. 189 * 190 * Returns zero on success, a negative error code on failure. 191 */ 192 int __init efi_memmap_install(phys_addr_t addr, unsigned int nr_map) 193 { 194 struct efi_memory_map_data data; 195 196 efi_memmap_unmap(); 197 198 /* Free the memory allocated to the existing memory map */ > 199 efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, efi.memmap.late); 200 201 data.phys_map = addr; 202 data.size = efi.memmap.desc_size * nr_map; 203 data.desc_version = efi.memmap.desc_version; 204 data.desc_size = efi.memmap.desc_size; 205 206 return __efi_memmap_init(&data, efi.memmap.late); 207 } 208 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 63997 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] efi: Free existing memory map before installing new memory map @ 2018-06-26 3:15 ` kbuild test robot 0 siblings, 0 replies; 14+ messages in thread From: kbuild test robot @ 2018-06-26 3:15 UTC (permalink / raw) To: Sai Praneeth Prakhya Cc: kbuild-all, linux-efi, linux-kernel, Sai Praneeth, Lee Chun-Yi, Borislav Petkov, Dave Young, Laszlo Ersek, Bhupesh Sharma, Ricardo Neri, Ravi Shankar, Matt Fleming, Ard Biesheuvel [-- Attachment #1: Type: text/plain, Size: 2330 bytes --] Hi Sai, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.18-rc2 next-20180625] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Sai-Praneeth-Prakhya/efi-Free-existing-memory-map-before-installing-new-memory-map/20180626-104301 config: x86_64-allyesconfig (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/firmware//efi/memmap.c: In function 'efi_memmap_install': >> drivers/firmware//efi/memmap.c:199:2: error: implicit declaration of function 'efi_memmap_free'; did you mean 'vmemmap_free'? [-Werror=implicit-function-declaration] efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, efi.memmap.late); ^~~~~~~~~~~~~~~ vmemmap_free cc1: some warnings being treated as errors vim +199 drivers/firmware//efi/memmap.c 180 181 /** 182 * efi_memmap_install - Install a new EFI memory map in efi.memmap 183 * @addr: Physical address of the memory map 184 * @nr_map: Number of entries in the memory map 185 * 186 * Unlike efi_memmap_init_*(), this function does not allow the caller 187 * to switch from early to late mappings. It simply uses the existing 188 * mapping function and installs the new memmap. 189 * 190 * Returns zero on success, a negative error code on failure. 191 */ 192 int __init efi_memmap_install(phys_addr_t addr, unsigned int nr_map) 193 { 194 struct efi_memory_map_data data; 195 196 efi_memmap_unmap(); 197 198 /* Free the memory allocated to the existing memory map */ > 199 efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, efi.memmap.late); 200 201 data.phys_map = addr; 202 data.size = efi.memmap.desc_size * nr_map; 203 data.desc_version = efi.memmap.desc_version; 204 data.desc_size = efi.memmap.desc_size; 205 206 return __efi_memmap_init(&data, efi.memmap.late); 207 } 208 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 63997 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] efi: Free existing memory map before installing new memory map 2018-06-26 2:41 [PATCH] efi: Free existing memory map before installing new memory map Sai Praneeth Prakhya @ 2018-06-26 3:15 ` kbuild test robot 2018-06-26 3:15 ` kbuild test robot 2018-06-26 9:38 ` Ard Biesheuvel 2 siblings, 0 replies; 14+ messages in thread From: kbuild test robot @ 2018-06-26 3:15 UTC (permalink / raw) Cc: kbuild-all, linux-efi, linux-kernel, Sai Praneeth, Lee Chun-Yi, Borislav Petkov, Dave Young, Laszlo Ersek, Bhupesh Sharma, Ricardo Neri, Ravi Shankar, Matt Fleming, Ard Biesheuvel [-- Attachment #1: Type: text/plain, Size: 2343 bytes --] Hi Sai, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.18-rc2 next-20180625] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Sai-Praneeth-Prakhya/efi-Free-existing-memory-map-before-installing-new-memory-map/20180626-104301 config: x86_64-randconfig-x003-201825 (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/firmware/efi/memmap.c: In function 'efi_memmap_install': >> drivers/firmware/efi/memmap.c:199:2: error: implicit declaration of function 'efi_memmap_free'; did you mean 'efi_memmap_walk'? [-Werror=implicit-function-declaration] efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, efi.memmap.late); ^~~~~~~~~~~~~~~ efi_memmap_walk cc1: some warnings being treated as errors vim +199 drivers/firmware/efi/memmap.c 180 181 /** 182 * efi_memmap_install - Install a new EFI memory map in efi.memmap 183 * @addr: Physical address of the memory map 184 * @nr_map: Number of entries in the memory map 185 * 186 * Unlike efi_memmap_init_*(), this function does not allow the caller 187 * to switch from early to late mappings. It simply uses the existing 188 * mapping function and installs the new memmap. 189 * 190 * Returns zero on success, a negative error code on failure. 191 */ 192 int __init efi_memmap_install(phys_addr_t addr, unsigned int nr_map) 193 { 194 struct efi_memory_map_data data; 195 196 efi_memmap_unmap(); 197 198 /* Free the memory allocated to the existing memory map */ > 199 efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, efi.memmap.late); 200 201 data.phys_map = addr; 202 data.size = efi.memmap.desc_size * nr_map; 203 data.desc_version = efi.memmap.desc_version; 204 data.desc_size = efi.memmap.desc_size; 205 206 return __efi_memmap_init(&data, efi.memmap.late); 207 } 208 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 32495 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] efi: Free existing memory map before installing new memory map @ 2018-06-26 3:15 ` kbuild test robot 0 siblings, 0 replies; 14+ messages in thread From: kbuild test robot @ 2018-06-26 3:15 UTC (permalink / raw) To: Sai Praneeth Prakhya Cc: kbuild-all, linux-efi, linux-kernel, Sai Praneeth, Lee Chun-Yi, Borislav Petkov, Dave Young, Laszlo Ersek, Bhupesh Sharma, Ricardo Neri, Ravi Shankar, Matt Fleming, Ard Biesheuvel [-- Attachment #1: Type: text/plain, Size: 2343 bytes --] Hi Sai, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.18-rc2 next-20180625] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Sai-Praneeth-Prakhya/efi-Free-existing-memory-map-before-installing-new-memory-map/20180626-104301 config: x86_64-randconfig-x003-201825 (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/firmware/efi/memmap.c: In function 'efi_memmap_install': >> drivers/firmware/efi/memmap.c:199:2: error: implicit declaration of function 'efi_memmap_free'; did you mean 'efi_memmap_walk'? [-Werror=implicit-function-declaration] efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, efi.memmap.late); ^~~~~~~~~~~~~~~ efi_memmap_walk cc1: some warnings being treated as errors vim +199 drivers/firmware/efi/memmap.c 180 181 /** 182 * efi_memmap_install - Install a new EFI memory map in efi.memmap 183 * @addr: Physical address of the memory map 184 * @nr_map: Number of entries in the memory map 185 * 186 * Unlike efi_memmap_init_*(), this function does not allow the caller 187 * to switch from early to late mappings. It simply uses the existing 188 * mapping function and installs the new memmap. 189 * 190 * Returns zero on success, a negative error code on failure. 191 */ 192 int __init efi_memmap_install(phys_addr_t addr, unsigned int nr_map) 193 { 194 struct efi_memory_map_data data; 195 196 efi_memmap_unmap(); 197 198 /* Free the memory allocated to the existing memory map */ > 199 efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, efi.memmap.late); 200 201 data.phys_map = addr; 202 data.size = efi.memmap.desc_size * nr_map; 203 data.desc_version = efi.memmap.desc_version; 204 data.desc_size = efi.memmap.desc_size; 205 206 return __efi_memmap_init(&data, efi.memmap.late); 207 } 208 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 32495 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] efi: Free existing memory map before installing new memory map 2018-06-26 3:15 ` kbuild test robot (?) @ 2018-06-26 7:18 ` Prakhya, Sai Praneeth 2018-06-27 6:02 ` [kbuild-all] " Ye Xiaolong -1 siblings, 1 reply; 14+ messages in thread From: Prakhya, Sai Praneeth @ 2018-06-26 7:18 UTC (permalink / raw) To: lkp Cc: kbuild-all@01.org, linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org, Lee Chun-Yi, Borislav Petkov, Dave Young, Laszlo Ersek, Bhupesh Sharma, Neri, Ricardo, Shankar, Ravi V, Matt Fleming, Ard Biesheuvel > > Hi Sai, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on linus/master] > [also build test ERROR on v4.18-rc2 next-20180625] [if your patch is applied to > the wrong git tree, please drop us a note to help improve the system] Since efi_memmap_free() is a recently introduced API [1], please note that the patch is based on efi tree @https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git [1] https://lkml.org/lkml/2018/6/22/39 Regards, Sai ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kbuild-all] [PATCH] efi: Free existing memory map before installing new memory map 2018-06-26 7:18 ` Prakhya, Sai Praneeth @ 2018-06-27 6:02 ` Ye Xiaolong 2018-06-27 6:09 ` Ard Biesheuvel 2018-06-27 6:32 ` Prakhya, Sai Praneeth 0 siblings, 2 replies; 14+ messages in thread From: Ye Xiaolong @ 2018-06-27 6:02 UTC (permalink / raw) To: Prakhya, Sai Praneeth Cc: lkp, Shankar, Ravi V, linux-efi@vger.kernel.org, Ard Biesheuvel, Matt Fleming, Laszlo Ersek, Bhupesh Sharma, linux-kernel@vger.kernel.org, Neri, Ricardo, Lee Chun-Yi, Borislav Petkov, kbuild-all@01.org, Dave Young Hi, On 06/26, Prakhya, Sai Praneeth wrote: >> >> Hi Sai, >> >> Thank you for the patch! Yet something to improve: >> >> [auto build test ERROR on linus/master] >> [also build test ERROR on v4.18-rc2 next-20180625] [if your patch is applied to >> the wrong git tree, please drop us a note to help improve the system] > >Since efi_memmap_free() is a recently introduced API [1], please note that the patch >is based on efi tree @https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git I noticed the official efi tree recored in MAINTAINERS is git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git, are they identical? EXTENSIBLE FIRMWARE INTERFACE (EFI) M: Matt Fleming <matt@codeblueprint.co.uk> L: linux-efi@vger.kernel.org T: git git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git S: Maintained F: Documentation/efi-stub.txt F: arch/ia64/kernel/efi.c F: arch/x86/boot/compressed/eboot.[ch] F: arch/x86/include/asm/efi.h F: arch/x86/platform/efi/* F: drivers/firmware/efi/* F: include/linux/efi*.h Thanks, Xiaolong > >[1] https://lkml.org/lkml/2018/6/22/39 > >Regards, >Sai >_______________________________________________ >kbuild-all mailing list >kbuild-all@lists.01.org >https://lists.01.org/mailman/listinfo/kbuild-all ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kbuild-all] [PATCH] efi: Free existing memory map before installing new memory map 2018-06-27 6:02 ` [kbuild-all] " Ye Xiaolong @ 2018-06-27 6:09 ` Ard Biesheuvel 2018-06-27 6:29 ` Ye Xiaolong 2018-06-27 6:32 ` Prakhya, Sai Praneeth 1 sibling, 1 reply; 14+ messages in thread From: Ard Biesheuvel @ 2018-06-27 6:09 UTC (permalink / raw) To: Ye Xiaolong Cc: Prakhya, Sai Praneeth, lkp, Shankar, Ravi V, linux-efi@vger.kernel.org, Matt Fleming, Laszlo Ersek, Bhupesh Sharma, linux-kernel@vger.kernel.org, Neri, Ricardo, Lee Chun-Yi, Borislav Petkov, kbuild-all@01.org, Dave Young On 27 June 2018 at 08:02, Ye Xiaolong <xiaolong.ye@intel.com> wrote: > Hi, > > On 06/26, Prakhya, Sai Praneeth wrote: >>> >>> Hi Sai, >>> >>> Thank you for the patch! Yet something to improve: >>> >>> [auto build test ERROR on linus/master] >>> [also build test ERROR on v4.18-rc2 next-20180625] [if your patch is applied to >>> the wrong git tree, please drop us a note to help improve the system] >> >>Since efi_memmap_free() is a recently introduced API [1], please note that the patch >>is based on efi tree @https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git > > I noticed the official efi tree recored in MAINTAINERS is git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git, > are they identical? > Which version of MAINTAINERS did you look at? > EXTENSIBLE FIRMWARE INTERFACE (EFI) > M: Matt Fleming <matt@codeblueprint.co.uk> > L: linux-efi@vger.kernel.org > T: git git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git > S: Maintained > F: Documentation/efi-stub.txt > F: arch/ia64/kernel/efi.c > F: arch/x86/boot/compressed/eboot.[ch] > F: arch/x86/include/asm/efi.h > F: arch/x86/platform/efi/* > F: drivers/firmware/efi/* > F: include/linux/efi*.h > > > Thanks, > Xiaolong > >> >>[1] https://lkml.org/lkml/2018/6/22/39 >> >>Regards, >>Sai >>_______________________________________________ >>kbuild-all mailing list >>kbuild-all@lists.01.org >>https://lists.01.org/mailman/listinfo/kbuild-all ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kbuild-all] [PATCH] efi: Free existing memory map before installing new memory map 2018-06-27 6:09 ` Ard Biesheuvel @ 2018-06-27 6:29 ` Ye Xiaolong 0 siblings, 0 replies; 14+ messages in thread From: Ye Xiaolong @ 2018-06-27 6:29 UTC (permalink / raw) To: Ard Biesheuvel Cc: Prakhya, Sai Praneeth, lkp, Shankar, Ravi V, linux-efi@vger.kernel.org, Matt Fleming, Laszlo Ersek, Bhupesh Sharma, linux-kernel@vger.kernel.org, Neri, Ricardo, Lee Chun-Yi, Borislav Petkov, kbuild-all@01.org, Dave Young On 06/27, Ard Biesheuvel wrote: >On 27 June 2018 at 08:02, Ye Xiaolong <xiaolong.ye@intel.com> wrote: >> Hi, >> >> On 06/26, Prakhya, Sai Praneeth wrote: >>>> >>>> Hi Sai, >>>> >>>> Thank you for the patch! Yet something to improve: >>>> >>>> [auto build test ERROR on linus/master] >>>> [also build test ERROR on v4.18-rc2 next-20180625] [if your patch is applied to >>>> the wrong git tree, please drop us a note to help improve the system] >>> >>>Since efi_memmap_free() is a recently introduced API [1], please note that the patch >>>is based on efi tree @https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git >> >> I noticed the official efi tree recored in MAINTAINERS is git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git, >> are they identical? >> > >Which version of MAINTAINERS did you look at? Oops, I just checked a old version. I'll add above efi.git tree to 0day's repo pool. Thanks, Xiaolong > >> EXTENSIBLE FIRMWARE INTERFACE (EFI) >> M: Matt Fleming <matt@codeblueprint.co.uk> >> L: linux-efi@vger.kernel.org >> T: git git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git >> S: Maintained >> F: Documentation/efi-stub.txt >> F: arch/ia64/kernel/efi.c >> F: arch/x86/boot/compressed/eboot.[ch] >> F: arch/x86/include/asm/efi.h >> F: arch/x86/platform/efi/* >> F: drivers/firmware/efi/* >> F: include/linux/efi*.h >> >> >> Thanks, >> Xiaolong >> >>> >>>[1] https://lkml.org/lkml/2018/6/22/39 >>> >>>Regards, >>>Sai >>>_______________________________________________ >>>kbuild-all mailing list >>>kbuild-all@lists.01.org >>>https://lists.01.org/mailman/listinfo/kbuild-all ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [kbuild-all] [PATCH] efi: Free existing memory map before installing new memory map 2018-06-27 6:02 ` [kbuild-all] " Ye Xiaolong 2018-06-27 6:09 ` Ard Biesheuvel @ 2018-06-27 6:32 ` Prakhya, Sai Praneeth 1 sibling, 0 replies; 14+ messages in thread From: Prakhya, Sai Praneeth @ 2018-06-27 6:32 UTC (permalink / raw) To: Ye, Xiaolong Cc: lkp, Shankar, Ravi V, linux-efi@vger.kernel.org, Ard Biesheuvel, Matt Fleming, Laszlo Ersek, Bhupesh Sharma, linux-kernel@vger.kernel.org, Neri, Ricardo, Lee Chun-Yi, Borislav Petkov, kbuild-all@01.org, Dave Young > >Since efi_memmap_free() is a recently introduced API [1], please note > >that the patch is based on efi tree > >@https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git > > I noticed the official efi tree recored in MAINTAINERS is > git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git, > are they identical? > > EXTENSIBLE FIRMWARE INTERFACE (EFI) > M: Matt Fleming <matt@codeblueprint.co.uk> > L: linux-efi@vger.kernel.org > T: git git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git > S: Maintained > F: Documentation/efi-stub.txt > F: arch/ia64/kernel/efi.c > F: arch/x86/boot/compressed/eboot.[ch] > F: arch/x86/include/asm/efi.h > F: arch/x86/platform/efi/* > F: drivers/firmware/efi/* > F: include/linux/efi*.h Hi Xiaolong, Ard replaced Matt as EFI maintainer. Please see updated MAINTAINERS file for more details. https://elixir.bootlin.com/linux/v4.18-rc2/source/MAINTAINERS EXTENSIBLE FIRMWARE INTERFACE (EFI) M: Ard Biesheuvel <ard.biesheuvel@linaro.org> L: linux-efi@vger.kernel.org T: git git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git S: Maintained F: Documentation/efi-stub.txt F: arch/*/kernel/efi.c F: arch/x86/boot/compressed/eboot.[ch] F: arch/*/include/asm/efi.h F: arch/x86/platform/efi/ F: drivers/firmware/efi/ F: include/linux/efi*.h F: arch/arm/boot/compressed/efi-header.S F: arch/arm64/kernel/efi-entry.S Regards, Sai ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] efi: Free existing memory map before installing new memory map 2018-06-26 2:41 [PATCH] efi: Free existing memory map before installing new memory map Sai Praneeth Prakhya 2018-06-26 3:15 ` kbuild test robot 2018-06-26 3:15 ` kbuild test robot @ 2018-06-26 9:38 ` Ard Biesheuvel 2018-06-27 4:51 ` Prakhya, Sai Praneeth 2 siblings, 1 reply; 14+ messages in thread From: Ard Biesheuvel @ 2018-06-26 9:38 UTC (permalink / raw) To: Sai Praneeth Prakhya Cc: linux-efi, Linux Kernel Mailing List, Lee Chun-Yi, Borislav Petkov, Dave Young, Laszlo Ersek, Bhupesh Sharma, Ricardo Neri, Ravi Shankar, Matt Fleming On 26 June 2018 at 04:41, Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com> wrote: > From: Sai Praneeth <sai.praneeth.prakhya@intel.com> > > efi_memmap_install(), unmaps the existing memory map and installs the > new memory map but doesn't free the memory allocated to the existing > memory map. Fortunately, the details about the existing memory map are > stored in efi.memmap. Hence, use them to free the memory. > > Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com> > Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Lee Chun-Yi <jlee@suse.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Dave Young <dyoung@redhat.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Bhupesh Sharma <bhsharma@redhat.com> > Cc: Ricardo Neri <ricardo.neri@intel.com> > Cc: Ravi Shankar <ravi.v.shankar@intel.com> > Cc: Matt Fleming <matt@codeblueprint.co.uk> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > > Note: Patch based on efi tree @https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git > > drivers/firmware/efi/memmap.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c > index 678e85704054..68b27b14fe94 100644 > --- a/drivers/firmware/efi/memmap.c > +++ b/drivers/firmware/efi/memmap.c > @@ -229,6 +229,9 @@ int __init efi_memmap_install(phys_addr_t addr, unsigned int nr_map) > > efi_memmap_unmap(); > > + /* Free the memory allocated to the existing memory map */ > + efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, efi.memmap.late); > + > data.phys_map = addr; > data.size = efi.memmap.desc_size * nr_map; > data.desc_version = efi.memmap.desc_version; > -- > 2.7.4 > If only it were so simple :-) At this point, efi.memmap.phys_map could point to memory that was allocated early, allocated late or simply passed to the OS at boot time by the stub (in which case it was memblock_reserve()d but not memblock_alloc()d, and it should not be freed) So only allocations made with efi_memmap_alloc() should be freed here. I'm not sure /how/ we should keep track of that: perhaps it is simply a matter of replacing the boolean 'late' with an enum that describes where the memory came from that phys_map points to. ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] efi: Free existing memory map before installing new memory map 2018-06-26 9:38 ` Ard Biesheuvel @ 2018-06-27 4:51 ` Prakhya, Sai Praneeth 2018-06-27 7:01 ` Ard Biesheuvel 0 siblings, 1 reply; 14+ messages in thread From: Prakhya, Sai Praneeth @ 2018-06-27 4:51 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-efi, Linux Kernel Mailing List, Lee Chun-Yi, Borislav Petkov, Dave Young, Laszlo Ersek, Bhupesh Sharma, Neri, Ricardo, Shankar, Ravi V, Matt Fleming > > + /* Free the memory allocated to the existing memory map */ > > + efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, > > + efi.memmap.late); > > + > > data.phys_map = addr; > > data.size = efi.memmap.desc_size * nr_map; > > data.desc_version = efi.memmap.desc_version; > > -- > > 2.7.4 > > > > If only it were so simple :-) > > At this point, efi.memmap.phys_map could point to memory that was allocated > early, allocated late or simply passed to the OS at boot time by the stub (in > which case it was memblock_reserve()d but not memblock_alloc()d, and it > should not be freed) > Yes, completely agree that there could be three types of allocations for memmap. I thought, efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, efi.memmap.late); should work because the previous type of allocation should have been recorded in efi.memmap.late. But, now I see this will fail for memblock_reserved() memory because it will be mistaken to memblock_alloced() (I assumed both are almost similar :(). > So only allocations made with efi_memmap_alloc() should be freed here. Makes sense and I think that also means efi_memmap_free() should be called from function that called efi_memmap_alloc() and not efi_memmap_install(). > I'm not sure /how/ we should keep track of that: perhaps it is simply a matter of > replacing the boolean 'late' with an enum that describes where the memory > came from that phys_map points to. I did try changing boolean late to enum and it seems to be working fine. I will do more testing/clean up and will submit a patch for review. Also, could you please clarify if there is any specific reason why memory allocated using memblock_reserve() shouldn't be freed. I mean, not with memblock_free() but I think we could make it _available_ using free_bootmem() (or something similar, please correct me if this is not the right API). If we allocate and install a new memory map (as in case with efi_fake_memmap()), I think we should free the memory used by memory map originally passed by EFI stub, because, at any point of time there should only be one active memory map. If we don't free the original memory map passed by EFI stub, we will be having two and hence will be leaking memory. Regards, Sai ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] efi: Free existing memory map before installing new memory map 2018-06-27 4:51 ` Prakhya, Sai Praneeth @ 2018-06-27 7:01 ` Ard Biesheuvel 2018-06-27 7:28 ` Prakhya, Sai Praneeth 0 siblings, 1 reply; 14+ messages in thread From: Ard Biesheuvel @ 2018-06-27 7:01 UTC (permalink / raw) To: Prakhya, Sai Praneeth Cc: linux-efi, Linux Kernel Mailing List, Lee Chun-Yi, Borislav Petkov, Dave Young, Laszlo Ersek, Bhupesh Sharma, Neri, Ricardo, Shankar, Ravi V, Matt Fleming On 27 June 2018 at 06:51, Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> wrote: >> > + /* Free the memory allocated to the existing memory map */ >> > + efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, >> > + efi.memmap.late); >> > + >> > data.phys_map = addr; >> > data.size = efi.memmap.desc_size * nr_map; >> > data.desc_version = efi.memmap.desc_version; >> > -- >> > 2.7.4 >> > >> >> If only it were so simple :-) >> >> At this point, efi.memmap.phys_map could point to memory that was allocated >> early, allocated late or simply passed to the OS at boot time by the stub (in >> which case it was memblock_reserve()d but not memblock_alloc()d, and it >> should not be freed) >> > > Yes, completely agree that there could be three types of allocations for memmap. > I thought, > efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, efi.memmap.late); > > should work because the previous type of allocation should have been recorded in efi.memmap.late. > But, now I see this will fail for memblock_reserved() memory because it will be mistaken to > memblock_alloced() (I assumed both are almost similar :(). > >> So only allocations made with efi_memmap_alloc() should be freed here. > > Makes sense and I think that also means efi_memmap_free() should be called from function > that called efi_memmap_alloc() and not efi_memmap_install(). > >> I'm not sure /how/ we should keep track of that: perhaps it is simply a matter of >> replacing the boolean 'late' with an enum that describes where the memory >> came from that phys_map points to. > > I did try changing boolean late to enum and it seems to be working fine. I will do more > testing/clean up and will submit a patch for review. > > Also, could you please clarify if there is any specific reason why memory allocated > using memblock_reserve() shouldn't be freed. I mean, not with memblock_free() but I > think we could make it _available_ using free_bootmem() (or something similar, please > correct me if this is not the right API). On arm64, the memory map is provided to the core kernel by the stub, and after kexec, a pointer to the same memory map will be passed to the next kernel. So the kernel does not 'own' that allocation, and it should not free it or overwrite it. > If we allocate and install a new memory map (as > in case with efi_fake_memmap()), I think we should free the memory used by memory map > originally passed by EFI stub, because, at any point of time there should only be one active > memory map. If we don't free the original memory map passed by EFI stub, we will be having > two and hence will be leaking memory. > > Regards, > Sai ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] efi: Free existing memory map before installing new memory map 2018-06-27 7:01 ` Ard Biesheuvel @ 2018-06-27 7:28 ` Prakhya, Sai Praneeth 0 siblings, 0 replies; 14+ messages in thread From: Prakhya, Sai Praneeth @ 2018-06-27 7:28 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-efi, Linux Kernel Mailing List, Lee Chun-Yi, Borislav Petkov, Dave Young, Laszlo Ersek, Bhupesh Sharma, Neri, Ricardo, Shankar, Ravi V, Matt Fleming > > Also, could you please clarify if there is any specific reason why > > memory allocated using memblock_reserve() shouldn't be freed. I mean, > > not with memblock_free() but I think we could make it _available_ > > using free_bootmem() (or something similar, please correct me if this is not > the right API). > > On arm64, the memory map is provided to the core kernel by the stub, and after > kexec, a pointer to the same memory map will be passed to the next kernel. So > the kernel does not 'own' that allocation, and it should not free it or overwrite > it. Thanks for the reply. It confirms that the issue is only on x86 systems. I see that arm64 doesn't call efi_memmap_alloc() and hence there is no concept of allocating memory for new memory map and installing it (so no memory leak). Regards, Sai ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-06-27 7:28 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-06-26 2:41 [PATCH] efi: Free existing memory map before installing new memory map Sai Praneeth Prakhya 2018-06-26 3:15 ` kbuild test robot 2018-06-26 3:15 ` kbuild test robot 2018-06-26 3:15 ` kbuild test robot 2018-06-26 3:15 ` kbuild test robot 2018-06-26 7:18 ` Prakhya, Sai Praneeth 2018-06-27 6:02 ` [kbuild-all] " Ye Xiaolong 2018-06-27 6:09 ` Ard Biesheuvel 2018-06-27 6:29 ` Ye Xiaolong 2018-06-27 6:32 ` Prakhya, Sai Praneeth 2018-06-26 9:38 ` Ard Biesheuvel 2018-06-27 4:51 ` Prakhya, Sai Praneeth 2018-06-27 7:01 ` Ard Biesheuvel 2018-06-27 7:28 ` Prakhya, Sai Praneeth
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.