Re: [PATCH v10 2/7] x86/boot: Copy kstrtoull() to compressed period
On Tue, Nov 06, 2018 at 08:13:03PM +0100, Borislav Petkov wrote: >On Mon, Oct 22, 2018 at 05:37:15PM +0800, Chao Fan wrote: >> kstrtoull() lives in 'uncompressed' period, used to >> convert a string to an unsigned long long. >> Copy to 'compressed' so that we can use it to >> convert the memory address from sting to unsigned > >sting? oops, typo, string. > >> long long in 'compressed' period. >> >> Signed-off-by: Chao Fan >> --- >> arch/x86/boot/compressed/misc.c | 88 + >> arch/x86/boot/compressed/misc.h | 4 ++ >> 2 files changed, 92 insertions(+) > >Why do you need to copy things? > >You can link that file into compressed/ as lib/kstrtox.c is a library or >include it similar to what arch/x86/boot/compressed/cmdline.c does. > >Still better than copying the code. I will have a try, thanks for your suggestion. > >> diff --git a/arch/x86/boot/compressed/misc.h >> b/arch/x86/boot/compressed/misc.h >> index 008fdc47a29c..40378408d980 100644 >> --- a/arch/x86/boot/compressed/misc.h >> +++ b/arch/x86/boot/compressed/misc.h >> @@ -63,6 +63,10 @@ static inline void debug_puthex(const char *s) >> >> #endif >> >> +#if (defined CONFIG_RANDOMIZE_BASE) && (defined CONFIG_RANDOMIZE_BASE) > >CONFIG_RANDOMIZE_BASE twice huh? Once not enough? Sorry for that, the second should be CONFIG_MEMORY_HOTREMOVE. Thanks, Chao Fan > >:-) > >-- >Regards/Gruss, >Boris. > >Good mailing practices for 400: avoid top-posting and trim the reply. > >
Re: [PATCH v10 1/7] x86/boot: Introduce cmdline_find_option_arg()to detect if option=arg in cmdline
On Tue, Nov 06, 2018 at 01:22:53PM +0100, Borislav Petkov wrote:
>On Mon, Oct 22, 2018 at 05:37:14PM +0800, Chao Fan wrote:
>> Now, there are cmdline_find_option() and cmdline_find_option_bool() in
>> cmdline.c. Sometimes, when detecting such as whether 'acpi=off' is
>> in cmdline, we need to cmdline_find_option() first, then compare
>> the argument. Now splite the operation as a independent function.
>> Introduce a new function cmdline_find_option_arg() to detect whether
>> option is in command line and the value is arg.
>
>For all future commit messages you write:
>
>Use passive tone in your commit message: no "we", etc.
Got it.
>
>Also, pls read section "2) Describe your changes" in
>Documentation/process/submitting-patches.rst.
OK.
>
>> Signed-off-by: Chao Fan
>> ---
>> arch/x86/boot/compressed/cmdline.c | 15 +++
>> arch/x86/boot/compressed/misc.h| 1 +
>> 2 files changed, 16 insertions(+)
>>
>> diff --git a/arch/x86/boot/compressed/cmdline.c
>> b/arch/x86/boot/compressed/cmdline.c
>> index af6cda0b7900..61118c69feb8 100644
>> --- a/arch/x86/boot/compressed/cmdline.c
>> +++ b/arch/x86/boot/compressed/cmdline.c
>> @@ -1,5 +1,7 @@
>> // SPDX-License-Identifier: GPL-2.0
>> #include "misc.h"
>> +#define STATIC
>> +#include
>>
>> #if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE || CONFIG_X86_5LEVEL
>>
>> @@ -30,5 +32,18 @@ int cmdline_find_option_bool(const char *option)
>> {
>> return __cmdline_find_option_bool(get_cmd_line_ptr(), option);
>> }
>> +bool cmdline_find_option_arg(const char *option, const char *arg, int
>> argsize)
>> +{
>> +char *buffer = malloc(argsize+1);
>> +bool find = false;
>> +int ret;
>> +
>> +ret = cmdline_find_option(option, buffer, argsize+1);
>> +if (ret == argsize && !strncmp(buffer, arg, argsize))
>> +find = true;
>> +
>> +free(buffer);
>> +return find;
>> +}
>
>I don't think such wrapper is needed. Simply calling
>cmdline_find_option() and then examining the buffer - like other call
>sites do - is perfectly fine.
I will change it.
Thanks,
Chao Fan
>
>Thx.
>
>--
>Regards/Gruss,
>Boris.
>
>Good mailing practices for 400: avoid top-posting and trim the reply.
>
>
Re: [PATCH v8 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory
On Tue, Nov 06, 2018 at 10:07:31PM +0800, Baoquan He wrote: >On 11/06/18 at 01:10pm, Borislav Petkov wrote: >> > I have another idea to solve this issue. Adding a SRAT parsing code >> > to arch/x86/mm/kaslr.c. It is useful for both EFI and BIOS and >> > also we don't need a new kernel parameter... >> > Dose the idea make sense? >> >> The more automatic stuff we do and we don't have to involve the user, >> the better. >> >> However, lemme look at Chao's current patchset first - we should not go >> nuts by putting SRAT parsing everywhere :) > >arch/x86/mm/ident_map.c is a good example, it's shared between >arch/x86/boot/compressed and arch/x86/mm/init_64.c Thanks to Baoquan, I think we can try this idea. How about you, Masa? Thanks, Chao Fan > >
Re: [PATCH 2/4] efi/arm: defer persistent reservations until after paging_init()
On Tue, Nov 06, 2018 at 09:06:56PM +0100, Ard Biesheuvel wrote: > On 6 November 2018 at 20:08, Russell King - ARM Linux > wrote: > > On Tue, Nov 06, 2018 at 08:02:58PM +0100, Ard Biesheuvel wrote: > >> On 6 November 2018 at 12:37, Ard Biesheuvel > >> wrote: > >> > The new memory EFI reservation feature we introduced to allow memory > >> > reservations to persist across kexec may trigger an unbounded number > >> > of calls to memblock_reserve(). The memblock subsystem can deal with > >> > this fine, but not before memblock resizing is enabled, which we can > >> > only do after paging_init(), when the memory we reallocate the array > >> > into is actually mapped. > >> > > >> > So break out the memreserve table processing into a separate function > >> > and call if after paging_init() on both arm64 and ARM. > >> > > >> > Cc: Russell King > >> > Signed-off-by: Ard Biesheuvel > >> > >> Russell, > >> > >> If you are ok with this patch, may I have your ack please? I would > >> like to send it out before the end of the week. > > > > You're not going to get a quick answer to this, because it needs me to > > investigate what the effect of this change actually is by code review. > > I can't guarantee when I'll get around to that. > > > > Fair enough. > > I will drop the ARM hunk for now then, and we'll fix ARM when you have > more time. I don't see how you can do that - you're dropping the processing of reserved areas out of the efi_config_parse_tables() path, so that won't happen any more on ARM if you don't apply the ARM hunk. So what I see at the moment is that efi_config_parse_tables() is called prior to paging_init() - and prior to our memblock initialisation, and you're proposing to move the processing that marks blocks as reserved immediately after paging_init(), and to use the normal memremap() interface to map stuff. I'm not convinced this will work - the memory allocators have not been setup at this point, so using memremap() will try to allocate page tables and potentially fail. If the normal allocators are setup, it's way too late to be calling memblock_reserve() - the memory will have been passed over to the page allocator according to which memblocks are available but not reserved. The normal page allocator is setup by mem_init(), which happens way after setup_arch() has returned. So, I don't see how your patch can be correct at the moment. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: [PATCH v8 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory
On Tue, Nov 06, 2018 at 09:45:11PM +0100, Borislav Petkov wrote:
> On Tue, Nov 06, 2018 at 02:36:38PM -0500, Masayoshi Mizuma wrote:
> > Yes, I think I can see what you are saying. However, I'm not sure how
> > I use the setup_data in legacy BIOS environment.
>
> What is "legacy BIOS environment" and what does that have to do with
> setup_data?
My proposed patch [1] is useful only for EFI environment. The patch
allocates a setup_date structure by efi_call_early() and store the
KASLR data into the memory area.
+static void setup_kaslr(struct boot_params *params)
+{
+ struct setup_data *kaslr_data = NULL;
+ struct setup_data *data;
+ unsigned long size;
+ efi_status_t status;
+
+ size = sizeof(struct setup_data) + sizeof(unsigned long long);
+
+ status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
+ size, (void **)_data);
I'm not sure how I allocate such memory on no EFI environment.
Am I missing something...?
[1] https://lkml.kernel.org/r/20181022154204.kagmdb55jtoez4ca@gabell
Thanks,
Masa
Re: [PATCH 0/4] arm/efi: fix memblock reallocation crash due to persistent reservations
On Tue, Nov 06, 2018 at 10:39:47PM +0100, Ard Biesheuvel wrote: > On 6 November 2018 at 22:34, Will Deacon wrote: > > On Tue, Nov 06, 2018 at 12:37:28PM +0100, Ard Biesheuvel wrote: > >> This series addresses the kexec/kdump crash on arm64 system with many CPUs > >> that was reported by Bhupesh. > >> > >> Patches #1 and #2 fix the actual crash. Patches #3 and #4 optimize the > >> EFI persistent memreserve infrastructure so that fewer memblock > >> reservations > >> are required. > > > > I acked the arm64 part and patches 3 and 4 look good afaict, so: > > > > Acked-by: Will Deacon > > > > for those as well. > > > > The only thing I was wondering is whether or not exhausting the page-sized > > array in the first list entry is rare enough that we could just realloc the > > thing and copy instead of chaining together new pages. That said, without > > seeing the code it's hard to tell whether you save much complexity with such > > a scheme so I'll leave it up to you. > > > > Well, the problem is that the page-sized array may have been allocated > by a previous kernel, and so it may be memblock_reserve()d already, in > which case reallocating does not actually free up the memory in a > useful way. > > Also, in the unlikely event that we race and allocate two new pages at > the same time, the current scheme will not break (and we will > ultimately fill up all the slots in both pages before allocating even > more pages). This will become a lot trickier if we do reallocation. Ah crap, yeah, the concurrency angle makes that really awful. Thanks. > So if the current approach looks correct to you, then I'd prefer to > stick with it. > > Do you agree with applying #3 and #4 as fixes? If Patch 1 alone is sufficient to solve the issue for Bhupesh, then I don't think we have a need to treat 3 and 4 as fixes (and therefore we can avoid having to backport them to stable kernels). Will
Re: [PATCH 0/4] arm/efi: fix memblock reallocation crash due to persistent reservations
On 6 November 2018 at 22:34, Will Deacon wrote: > Hi Ard, > > On Tue, Nov 06, 2018 at 12:37:28PM +0100, Ard Biesheuvel wrote: >> This series addresses the kexec/kdump crash on arm64 system with many CPUs >> that was reported by Bhupesh. >> >> Patches #1 and #2 fix the actual crash. Patches #3 and #4 optimize the >> EFI persistent memreserve infrastructure so that fewer memblock reservations >> are required. > > I acked the arm64 part and patches 3 and 4 look good afaict, so: > > Acked-by: Will Deacon > > for those as well. > > The only thing I was wondering is whether or not exhausting the page-sized > array in the first list entry is rare enough that we could just realloc the > thing and copy instead of chaining together new pages. That said, without > seeing the code it's hard to tell whether you save much complexity with such > a scheme so I'll leave it up to you. > Well, the problem is that the page-sized array may have been allocated by a previous kernel, and so it may be memblock_reserve()d already, in which case reallocating does not actually free up the memory in a useful way. Also, in the unlikely event that we race and allocate two new pages at the same time, the current scheme will not break (and we will ultimately fill up all the slots in both pages before allocating even more pages). This will become a lot trickier if we do reallocation. So if the current approach looks correct to you, then I'd prefer to stick with it. Do you agree with applying #3 and #4 as fixes?
Re: [PATCH 0/4] arm/efi: fix memblock reallocation crash due to persistent reservations
Hi Ard, On Tue, Nov 06, 2018 at 12:37:28PM +0100, Ard Biesheuvel wrote: > This series addresses the kexec/kdump crash on arm64 system with many CPUs > that was reported by Bhupesh. > > Patches #1 and #2 fix the actual crash. Patches #3 and #4 optimize the > EFI persistent memreserve infrastructure so that fewer memblock reservations > are required. I acked the arm64 part and patches 3 and 4 look good afaict, so: Acked-by: Will Deacon for those as well. The only thing I was wondering is whether or not exhausting the page-sized array in the first list entry is rare enough that we could just realloc the thing and copy instead of chaining together new pages. That said, without seeing the code it's hard to tell whether you save much complexity with such a scheme so I'll leave it up to you. Will
Re: [PATCH 1/4] arm64: memblock: don't permit memblock resizing until linear mapping is up
On Tue, Nov 06, 2018 at 12:37:29PM +0100, Ard Biesheuvel wrote: > Bhupesh reports that having numerous memblock reservations at early > boot may result in the following crash: > > Unable to handle kernel paging request at virtual address 80003ffe > ... > Call trace: >__memcpy+0x110/0x180 >memblock_add_range+0x134/0x2e8 >memblock_reserve+0x70/0xb8 >memblock_alloc_base_nid+0x6c/0x88 >__memblock_alloc_base+0x3c/0x4c >memblock_alloc_base+0x28/0x4c >memblock_alloc+0x2c/0x38 >early_pgtable_alloc+0x20/0xb0 >paging_init+0x28/0x7f8 > > This is caused by the fact that we permit memblock resizing before the > linear mapping is up, and so the memblock_reserved() array is moved > into memory that is not mapped yet. > > So let's ensure that this crash can no longer occur, by deferring to > call to memblock_allow_resize() to after the linear mapping has been > created. > > Reported-by: Bhupesh Sharma > Signed-off-by: Ard Biesheuvel > --- > arch/arm64/mm/init.c | 2 -- > arch/arm64/mm/mmu.c | 2 ++ > 2 files changed, 2 insertions(+), 2 deletions(-) Thanks for posting this so quickly. Acked-by: Will Deacon Bhupesh -- please can you give this series a spin and confirm that it fixes the problem you were seeing? Thanks, Will
Re: [PATCH v8 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory
On Tue, Nov 06, 2018 at 02:36:38PM -0500, Masayoshi Mizuma wrote: > Yes, I think I can see what you are saying. However, I'm not sure how > I use the setup_data in legacy BIOS environment. What is "legacy BIOS environment" and what does that have to do with setup_data? -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH 2/4] efi/arm: defer persistent reservations until after paging_init()
On 6 November 2018 at 20:08, Russell King - ARM Linux wrote: > On Tue, Nov 06, 2018 at 08:02:58PM +0100, Ard Biesheuvel wrote: >> On 6 November 2018 at 12:37, Ard Biesheuvel >> wrote: >> > The new memory EFI reservation feature we introduced to allow memory >> > reservations to persist across kexec may trigger an unbounded number >> > of calls to memblock_reserve(). The memblock subsystem can deal with >> > this fine, but not before memblock resizing is enabled, which we can >> > only do after paging_init(), when the memory we reallocate the array >> > into is actually mapped. >> > >> > So break out the memreserve table processing into a separate function >> > and call if after paging_init() on both arm64 and ARM. >> > >> > Cc: Russell King >> > Signed-off-by: Ard Biesheuvel >> >> Russell, >> >> If you are ok with this patch, may I have your ack please? I would >> like to send it out before the end of the week. > > You're not going to get a quick answer to this, because it needs me to > investigate what the effect of this change actually is by code review. > I can't guarantee when I'll get around to that. > Fair enough. I will drop the ARM hunk for now then, and we'll fix ARM when you have more time. Thanks,
Re: [PATCH 0/4] arm/efi: fix memblock reallocation crash due to persistent reservations
On Tue, 06 Nov 2018 19:01:51 +, Ard Biesheuvel wrote: > > On 6 November 2018 at 19:27, Marc Zyngier wrote: > > Hi Ard, > > > > On 06/11/18 11:37, Ard Biesheuvel wrote: > >> This series addresses the kexec/kdump crash on arm64 system with many CPUs > >> that was reported by Bhupesh. > >> > >> Patches #1 and #2 fix the actual crash. Patches #3 and #4 optimize the > >> EFI persistent memreserve infrastructure so that fewer memblock > >> reservations > >> are required. > >> > >> Ard Biesheuvel (4): > >> arm64: memblock: don't permit memblock resizing until linear mapping > >> is up > >> efi/arm: defer persistent reservations until after paging_init() > >> efi: permit multiple entries in persistent memreserve data structure > >> efi: reduce the amount of memblock reservations for persistent > >> allocations > >> > >> arch/arm/kernel/setup.c | 1 + > >> arch/arm64/kernel/setup.c | 1 + > >> arch/arm64/mm/init.c| 2 - > >> arch/arm64/mm/mmu.c | 2 + > >> drivers/firmware/efi/efi.c | 59 ++-- > >> drivers/firmware/efi/libstub/arm-stub.c | 2 +- > >> include/linux/efi.h | 23 +++- > >> 7 files changed, 68 insertions(+), 22 deletions(-) > > > > I've just given these patches a go a a TX2 box (one of the 224 CPU > > ones...), and kexec worked just fine (v4.20-rc1 vanilla didn't manage to > > kexec on this box). > > > > There seem to be some additional userspace patches that are required for > > the ACPI tables not to be corrupted in the secondary kernel, but that's > > an orthogonal issue. > > > > Feel free to add > > > > Tested-by: Marc Zyngier > > > > Thanks Marc. > > Any thoughts on whether patches #3 and #4 should be included as fixes? > I'm leaning towards yes. They certainly massively reduce the number of list entries, and probably help reducing the overhead. I'd be inclined to merge them as well. Thanks, M. -- Jazz is not dead, it just smell funny.
Re: [PATCH v8 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory
On Tue, Nov 06, 2018 at 07:45:19PM +0100, Borislav Petkov wrote: > Ok, having swapped the whole thing back into my brain, forget what I > said earlier today. > > Didn't we talk about passing info with setup_data to the later kernel > stage? You even had a patch: > > https://lkml.kernel.org/r/20181022154204.kagmdb55jtoez4ca@gabell > > So what is that "idea" again about adding SRAT parsing code to > arch/x86/mm/kaslr.c?!?! > > The intent of passing info with setup_data is to *avoid* parsing SRAT > yet another time and duplicating that code one more time. > > IOW: > > * The first place that needs SRAT parsing, does the parsing - i.e., the > compressed stage. > > * Later stages get information passed to them with setup_data. No second > parsing. > > Ok? Yes, I think I can see what you are saying. However, I'm not sure how I use the setup_data in legacy BIOS environment. So, I said the another idea which adding the SRAT parsing code to arch/x86/mm/kaslr.c as well. Yes, as you said, that is not so good... I would appreciate if you could help to use setup_data or something to pass the information to later kernel stage in BIOS environment. Thanks, Masa
Re: [PATCH v10 2/7] x86/boot: Copy kstrtoull() to compressed period
On Mon, Oct 22, 2018 at 05:37:15PM +0800, Chao Fan wrote: > kstrtoull() lives in 'uncompressed' period, used to > convert a string to an unsigned long long. > Copy to 'compressed' so that we can use it to > convert the memory address from sting to unsigned sting? > long long in 'compressed' period. > > Signed-off-by: Chao Fan > --- > arch/x86/boot/compressed/misc.c | 88 + > arch/x86/boot/compressed/misc.h | 4 ++ > 2 files changed, 92 insertions(+) Why do you need to copy things? You can link that file into compressed/ as lib/kstrtox.c is a library or include it similar to what arch/x86/boot/compressed/cmdline.c does. Still better than copying the code. > diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h > index 008fdc47a29c..40378408d980 100644 > --- a/arch/x86/boot/compressed/misc.h > +++ b/arch/x86/boot/compressed/misc.h > @@ -63,6 +63,10 @@ static inline void debug_puthex(const char *s) > > #endif > > +#if (defined CONFIG_RANDOMIZE_BASE) && (defined CONFIG_RANDOMIZE_BASE) CONFIG_RANDOMIZE_BASE twice huh? Once not enough? :-) -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH 0/4] arm/efi: fix memblock reallocation crash due to persistent reservations
On 6 November 2018 at 19:27, Marc Zyngier wrote: > Hi Ard, > > On 06/11/18 11:37, Ard Biesheuvel wrote: >> This series addresses the kexec/kdump crash on arm64 system with many CPUs >> that was reported by Bhupesh. >> >> Patches #1 and #2 fix the actual crash. Patches #3 and #4 optimize the >> EFI persistent memreserve infrastructure so that fewer memblock reservations >> are required. >> >> Ard Biesheuvel (4): >> arm64: memblock: don't permit memblock resizing until linear mapping >> is up >> efi/arm: defer persistent reservations until after paging_init() >> efi: permit multiple entries in persistent memreserve data structure >> efi: reduce the amount of memblock reservations for persistent >> allocations >> >> arch/arm/kernel/setup.c | 1 + >> arch/arm64/kernel/setup.c | 1 + >> arch/arm64/mm/init.c| 2 - >> arch/arm64/mm/mmu.c | 2 + >> drivers/firmware/efi/efi.c | 59 ++-- >> drivers/firmware/efi/libstub/arm-stub.c | 2 +- >> include/linux/efi.h | 23 +++- >> 7 files changed, 68 insertions(+), 22 deletions(-) > > I've just given these patches a go a a TX2 box (one of the 224 CPU > ones...), and kexec worked just fine (v4.20-rc1 vanilla didn't manage to > kexec on this box). > > There seem to be some additional userspace patches that are required for > the ACPI tables not to be corrupted in the secondary kernel, but that's > an orthogonal issue. > > Feel free to add > > Tested-by: Marc Zyngier > Thanks Marc. Any thoughts on whether patches #3 and #4 should be included as fixes? I'm leaning towards yes.
Re: [PATCH 2/4] efi/arm: defer persistent reservations until after paging_init()
On 6 November 2018 at 12:37, Ard Biesheuvel wrote:
> The new memory EFI reservation feature we introduced to allow memory
> reservations to persist across kexec may trigger an unbounded number
> of calls to memblock_reserve(). The memblock subsystem can deal with
> this fine, but not before memblock resizing is enabled, which we can
> only do after paging_init(), when the memory we reallocate the array
> into is actually mapped.
>
> So break out the memreserve table processing into a separate function
> and call if after paging_init() on both arm64 and ARM.
>
> Cc: Russell King
> Signed-off-by: Ard Biesheuvel
Russell,
If you are ok with this patch, may I have your ack please? I would
like to send it out before the end of the week.
Thanks,
> ---
> arch/arm/kernel/setup.c| 1 +
> arch/arm64/kernel/setup.c | 1 +
> drivers/firmware/efi/efi.c | 8 ++--
> include/linux/efi.h| 7 +++
> 4 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index ac7e08886863..e99f12eaf390 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -1117,6 +1117,7 @@ void __init setup_arch(char **cmdline_p)
> early_ioremap_reset();
>
> paging_init(mdesc);
> + efi_apply_persistent_mem_reservations();
> request_standard_resources(mdesc);
>
> if (mdesc->restart)
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 953e316521fc..f4fc1e0544b7 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -313,6 +313,7 @@ void __init setup_arch(char **cmdline_p)
> arm64_memblock_init();
>
> paging_init();
> + efi_apply_persistent_mem_reservations();
>
> acpi_table_upgrade();
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 249eb70691b0..55e4ea20bdc3 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -592,7 +592,11 @@ int __init efi_config_parse_tables(void *config_tables,
> int count, int sz,
>
> early_memunmap(tbl, sizeof(*tbl));
> }
> + return 0;
> +}
>
> +int __init efi_apply_persistent_mem_reservations(void)
> +{
> if (efi.mem_reserve != EFI_INVALID_TABLE_ADDR) {
> unsigned long prsv = efi.mem_reserve;
>
> @@ -602,7 +606,7 @@ int __init efi_config_parse_tables(void *config_tables,
> int count, int sz,
> /* reserve the entry itself */
> memblock_reserve(prsv, sizeof(*rsv));
>
> - rsv = early_memremap(prsv, sizeof(*rsv));
> + rsv = memremap(prsv, sizeof(*rsv), MEMREMAP_WB);
> if (rsv == NULL) {
> pr_err("Could not map UEFI memreserve
> entry!\n");
> return -ENOMEM;
> @@ -612,7 +616,7 @@ int __init efi_config_parse_tables(void *config_tables,
> int count, int sz,
> memblock_reserve(rsv->base, rsv->size);
>
> prsv = rsv->next;
> - early_memunmap(rsv, sizeof(*rsv));
> + memunmap(rsv);
> }
> }
>
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 845174e113ce..100ce4a4aff6 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1167,6 +1167,8 @@ static inline bool efi_enabled(int feature)
> extern void efi_reboot(enum reboot_mode reboot_mode, const char *__unused);
>
> extern bool efi_is_table_address(unsigned long phys_addr);
> +
> +extern int efi_apply_persistent_mem_reservations(void);
> #else
> static inline bool efi_enabled(int feature)
> {
> @@ -1185,6 +1187,11 @@ static inline bool efi_is_table_address(unsigned long
> phys_addr)
> {
> return false;
> }
> +
> +static inline int efi_apply_persistent_mem_reservations(void)
> +{
> + return 0;
> +}
> #endif
>
> extern int efi_status_to_err(efi_status_t status);
> --
> 2.19.1
>
Re: [PATCH 2/4] efi/arm: defer persistent reservations until after paging_init()
On Tue, Nov 06, 2018 at 08:02:58PM +0100, Ard Biesheuvel wrote: > On 6 November 2018 at 12:37, Ard Biesheuvel wrote: > > The new memory EFI reservation feature we introduced to allow memory > > reservations to persist across kexec may trigger an unbounded number > > of calls to memblock_reserve(). The memblock subsystem can deal with > > this fine, but not before memblock resizing is enabled, which we can > > only do after paging_init(), when the memory we reallocate the array > > into is actually mapped. > > > > So break out the memreserve table processing into a separate function > > and call if after paging_init() on both arm64 and ARM. > > > > Cc: Russell King > > Signed-off-by: Ard Biesheuvel > > Russell, > > If you are ok with this patch, may I have your ack please? I would > like to send it out before the end of the week. You're not going to get a quick answer to this, because it needs me to investigate what the effect of this change actually is by code review. I can't guarantee when I'll get around to that. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: [PATCH v8 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory
On Thu, Oct 25, 2018 at 09:40:51AM -0400, Masayoshi Mizuma wrote: > I have another idea to solve this issue. Adding a SRAT parsing code > to arch/x86/mm/kaslr.c. It is useful for both EFI and BIOS and > also we don't need a new kernel parameter... > Dose the idea make sense? Ok, having swapped the whole thing back into my brain, forget what I said earlier today. Didn't we talk about passing info with setup_data to the later kernel stage? You even had a patch: https://lkml.kernel.org/r/20181022154204.kagmdb55jtoez4ca@gabell So what is that "idea" again about adding SRAT parsing code to arch/x86/mm/kaslr.c?!?! The intent of passing info with setup_data is to *avoid* parsing SRAT yet another time and duplicating that code one more time. IOW: * The first place that needs SRAT parsing, does the parsing - i.e., the compressed stage. * Later stages get information passed to them with setup_data. No second parsing. Ok? -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH 0/4] arm/efi: fix memblock reallocation crash due to persistent reservations
Hi Ard, On 06/11/18 11:37, Ard Biesheuvel wrote: > This series addresses the kexec/kdump crash on arm64 system with many CPUs > that was reported by Bhupesh. > > Patches #1 and #2 fix the actual crash. Patches #3 and #4 optimize the > EFI persistent memreserve infrastructure so that fewer memblock reservations > are required. > > Ard Biesheuvel (4): > arm64: memblock: don't permit memblock resizing until linear mapping > is up > efi/arm: defer persistent reservations until after paging_init() > efi: permit multiple entries in persistent memreserve data structure > efi: reduce the amount of memblock reservations for persistent > allocations > > arch/arm/kernel/setup.c | 1 + > arch/arm64/kernel/setup.c | 1 + > arch/arm64/mm/init.c| 2 - > arch/arm64/mm/mmu.c | 2 + > drivers/firmware/efi/efi.c | 59 ++-- > drivers/firmware/efi/libstub/arm-stub.c | 2 +- > include/linux/efi.h | 23 +++- > 7 files changed, 68 insertions(+), 22 deletions(-) I've just given these patches a go a a TX2 box (one of the 224 CPU ones...), and kexec worked just fine (v4.20-rc1 vanilla didn't manage to kexec on this box). There seem to be some additional userspace patches that are required for the ACPI tables not to be corrupted in the secondary kernel, but that's an orthogonal issue. Feel free to add Tested-by: Marc Zyngier Thanks, M. -- Jazz is not dead. It just smells funny...
Re: [PATCH 1/4] EFI stub: remove -fdata-sections
On 6 November 2018 at 15:21, Russell King - ARM Linux wrote: > On Tue, Nov 06, 2018 at 03:11:18PM +0100, Ard Biesheuvel wrote: >> On 6 November 2018 at 14:40, Russell King wrote: >> > Remove -fdata-sections from the EFI stub build as this causes problems >> > for the ARM decompressor code. >> > >> Just out of curiosity: what kind of problems? > > I'm afraid I don't remember - these patches are March 2018. I suspect > it was a result of a randconfig build failing to link, as I don't have > any 32-bit ARM configs that have the EFI stub enabled. > > In any case, this will break: > > # > # ARM discards the .data section because it disallows r/w data in the > # decompressor. So move our .data to .data.efistub, which is preserved > # explicitly by the decompressor linker script. > # > STUBCOPY_FLAGS-$(CONFIG_ARM)+= --rename-section .data=.data.efistub > > because with -fdata-sections enabled, there is no longer a single .data > section, but multiple .data.* sections. Hence, the rename no longer > works, and all the EFI stub data ends up being discarded by the > decompressor link stage. > Ah, of course, that must be it.
Re: [PATCH 1/4] EFI stub: remove -fdata-sections
On Tue, Nov 06, 2018 at 03:11:18PM +0100, Ard Biesheuvel wrote: > On 6 November 2018 at 14:40, Russell King wrote: > > Remove -fdata-sections from the EFI stub build as this causes problems > > for the ARM decompressor code. > > > Just out of curiosity: what kind of problems? I'm afraid I don't remember - these patches are March 2018. I suspect it was a result of a randconfig build failing to link, as I don't have any 32-bit ARM configs that have the EFI stub enabled. In any case, this will break: # # ARM discards the .data section because it disallows r/w data in the # decompressor. So move our .data to .data.efistub, which is preserved # explicitly by the decompressor linker script. # STUBCOPY_FLAGS-$(CONFIG_ARM)+= --rename-section .data=.data.efistub because with -fdata-sections enabled, there is no longer a single .data section, but multiple .data.* sections. Hence, the rename no longer works, and all the EFI stub data ends up being discarded by the decompressor link stage. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: [PATCH] efi/libstub: Disable some warnings for x86{,_64}
On 6 November 2018 at 14:15, Sedat Dilek wrote:
> On Mon, Nov 5, 2018 at 1:52 PM Ard Biesheuvel
> wrote:
>>
>> On 24 October 2018 at 09:22, Sedat Dilek wrote:
>> > On Tue, Oct 23, 2018 at 7:53 PM Nathan Chancellor
>> > wrote:
>> >>
>> >> On Fri, Oct 12, 2018 at 06:03:49PM -0700, Nathan Chancellor wrote:
>> >> > When building the kernel with Clang, some disabled warnings appear
>> >> > because this Makefile overrides KBUILD_CFLAGS for x86{,_64}. Add them to
>> >> > this list so that the build is clean again.
>> >> >
>> >> > -Wpointer-sign was disabled for the whole kernel before the beginning
>> >> > of git history.
>> >> >
>> >> > -Waddress-of-packed-member was disabled for the whole kernel in
>> >> > commit bfb38988c51e ("kbuild: clang: Disable 'address-of-packed-member'
>> >> > warning") and for x86/boot/compressed in commit 20c6c1890455 ("x86/boot:
>> >> > Disable the address-of-packed-member compiler warning").
>> >> >
>> >> > -Wgnu was disabled for the whole kernel in commit 61163efae020 ("kbuild:
>> >> > LLVMLinux: Add Kbuild support for building kernel with Clang") and for
>> >> > x86/boot/compressed in commit 6c3b56b19730 ("x86/boot: Disable Clang
>> >> > warnings about GNU extensions").
>> >> >
>> >> > Link: https://github.com/ClangBuiltLinux/linux/issues/112
>> >> > Signed-off-by: Nathan Chancellor
>> >> > ---
>> >> >
>> >> > Nick expressed concern that this Makefile is overwriting KBUILD_CFLAGS
>> >> > and suggested potentially rewriting the x86 portion of this Makefile to
>> >> > behave like the arm/arm64 one where problematic flags are filtered out.
>> >> > While that comes to fruition, it would be nice for this folder to behave
>> >> > like the rest of the kernel when it comes to this warnings so that the
>> >> > build is cleaner, thus this patch.
>> >> >
>> >> > drivers/firmware/efi/libstub/Makefile | 5 -
>> >> > 1 file changed, 4 insertions(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/drivers/firmware/efi/libstub/Makefile
>> >> > b/drivers/firmware/efi/libstub/Makefile
>> >> > index c51627660dbb..d9845099635e 100644
>> >> > --- a/drivers/firmware/efi/libstub/Makefile
>> >> > +++ b/drivers/firmware/efi/libstub/Makefile
>> >> > @@ -9,7 +9,10 @@ cflags-$(CONFIG_X86_32) := -march=i386
>> >> > cflags-$(CONFIG_X86_64) := -mcmodel=small
>> >> > cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \
>> >> > -fPIC -fno-strict-aliasing
>> >> > -mno-red-zone \
>> >> > --mno-mmx -mno-sse -fshort-wchar
>> >> > +-mno-mmx -mno-sse -fshort-wchar \
>> >> > +-Wno-pointer-sign \
>> >> > +$(call cc-disable-warning,
>> >> > address-of-packed-member) \
>> >> > +$(call cc-disable-warning, gnu)
>> >> >
>> >> > # arm64 uses the full KBUILD_CFLAGS so it's necessary to explicitly
>> >> > # disable the stackleak plugin
>> >> > --
>> >> > 2.19.1
>> >> >
>> >>
>> >> + Sedat for review/testing.
>> >
>> > Thanks Nathan.
>> >
>> > Reviewed-by: Sedat Dilek
>> > Tested-by: Sedat Dilek
>> > [ Reported-by: Sedat Dilek ]
>> > [ See duplicate "arch/x86/include/asm/processor.h: Warning
>> > -Waddress-of-packed-member "
>> > ]
>> >
>>
>> Queued in efi/next
>>
>
> Thanks Ard.
> This means efi-for-4.21 / efi-for-5.0?
>
Yes
Re: [PATCH 1/4] EFI stub: remove -fdata-sections
On 6 November 2018 at 14:40, Russell King wrote: > Remove -fdata-sections from the EFI stub build as this causes problems > for the ARM decompressor code. > Just out of curiosity: what kind of problems? In any case: it doesn't make sense to obsess about .data size optimizations here, so Acked-by: Ard Biesheuvel > Signed-off-by: Russell King > --- > drivers/firmware/efi/libstub/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/firmware/efi/libstub/Makefile > b/drivers/firmware/efi/libstub/Makefile > index c51627660dbb..98133d0577ab 100644 > --- a/drivers/firmware/efi/libstub/Makefile > +++ b/drivers/firmware/efi/libstub/Makefile > @@ -15,7 +15,7 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 > \ > # disable the stackleak plugin > cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) -fpie \ >$(DISABLE_STACKLEAK_PLUGIN) > -cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ > +cflags-$(CONFIG_ARM) := $(filter-out -pg > -fdata-sections,$(KBUILD_CFLAGS)) \ >-fno-builtin -fpic \ >$(call cc-option,-mno-single-pic-base) > > -- > 2.7.4 >
Re: [PATCH v8 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory
On 11/06/18 at 01:10pm, Borislav Petkov wrote: > > I have another idea to solve this issue. Adding a SRAT parsing code > > to arch/x86/mm/kaslr.c. It is useful for both EFI and BIOS and > > also we don't need a new kernel parameter... > > Dose the idea make sense? > > The more automatic stuff we do and we don't have to involve the user, > the better. > > However, lemme look at Chao's current patchset first - we should not go > nuts by putting SRAT parsing everywhere :) arch/x86/mm/ident_map.c is a good example, it's shared between arch/x86/boot/compressed and arch/x86/mm/init_64.c
[PATCH 1/4] EFI stub: remove -fdata-sections
Remove -fdata-sections from the EFI stub build as this causes problems for the ARM decompressor code. Signed-off-by: Russell King --- drivers/firmware/efi/libstub/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index c51627660dbb..98133d0577ab 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -15,7 +15,7 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \ # disable the stackleak plugin cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) -fpie \ $(DISABLE_STACKLEAK_PLUGIN) -cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ +cflags-$(CONFIG_ARM) := $(filter-out -pg -fdata-sections,$(KBUILD_CFLAGS)) \ -fno-builtin -fpic \ $(call cc-option,-mno-single-pic-base) -- 2.7.4
[PATCH 0/4] Enable deadcode elimination at link time
This series enables elimination of dead code from the kernel at link time. The changes are mostly ARM specific, except for one change in EFI as the EFI stub must not be compiled with -fdata-sections. arch/arm/Kconfig | 1 + arch/arm/boot/compressed/vmlinux.lds.S | 2 +- arch/arm/kernel/vmlinux.lds.S | 10 +- arch/arm/kernel/vmlinux.lds.h | 18 +- drivers/firmware/efi/libstub/Makefile | 2 +- 5 files changed, 17 insertions(+), 16 deletions(-) -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: [PATCH] efi/libstub: Disable some warnings for x86{,_64}
On Mon, Nov 5, 2018 at 1:52 PM Ard Biesheuvel wrote:
>
> On 24 October 2018 at 09:22, Sedat Dilek wrote:
> > On Tue, Oct 23, 2018 at 7:53 PM Nathan Chancellor
> > wrote:
> >>
> >> On Fri, Oct 12, 2018 at 06:03:49PM -0700, Nathan Chancellor wrote:
> >> > When building the kernel with Clang, some disabled warnings appear
> >> > because this Makefile overrides KBUILD_CFLAGS for x86{,_64}. Add them to
> >> > this list so that the build is clean again.
> >> >
> >> > -Wpointer-sign was disabled for the whole kernel before the beginning
> >> > of git history.
> >> >
> >> > -Waddress-of-packed-member was disabled for the whole kernel in
> >> > commit bfb38988c51e ("kbuild: clang: Disable 'address-of-packed-member'
> >> > warning") and for x86/boot/compressed in commit 20c6c1890455 ("x86/boot:
> >> > Disable the address-of-packed-member compiler warning").
> >> >
> >> > -Wgnu was disabled for the whole kernel in commit 61163efae020 ("kbuild:
> >> > LLVMLinux: Add Kbuild support for building kernel with Clang") and for
> >> > x86/boot/compressed in commit 6c3b56b19730 ("x86/boot: Disable Clang
> >> > warnings about GNU extensions").
> >> >
> >> > Link: https://github.com/ClangBuiltLinux/linux/issues/112
> >> > Signed-off-by: Nathan Chancellor
> >> > ---
> >> >
> >> > Nick expressed concern that this Makefile is overwriting KBUILD_CFLAGS
> >> > and suggested potentially rewriting the x86 portion of this Makefile to
> >> > behave like the arm/arm64 one where problematic flags are filtered out.
> >> > While that comes to fruition, it would be nice for this folder to behave
> >> > like the rest of the kernel when it comes to this warnings so that the
> >> > build is cleaner, thus this patch.
> >> >
> >> > drivers/firmware/efi/libstub/Makefile | 5 -
> >> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/firmware/efi/libstub/Makefile
> >> > b/drivers/firmware/efi/libstub/Makefile
> >> > index c51627660dbb..d9845099635e 100644
> >> > --- a/drivers/firmware/efi/libstub/Makefile
> >> > +++ b/drivers/firmware/efi/libstub/Makefile
> >> > @@ -9,7 +9,10 @@ cflags-$(CONFIG_X86_32) := -march=i386
> >> > cflags-$(CONFIG_X86_64) := -mcmodel=small
> >> > cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -O2 \
> >> > -fPIC -fno-strict-aliasing
> >> > -mno-red-zone \
> >> > --mno-mmx -mno-sse -fshort-wchar
> >> > +-mno-mmx -mno-sse -fshort-wchar \
> >> > +-Wno-pointer-sign \
> >> > +$(call cc-disable-warning,
> >> > address-of-packed-member) \
> >> > +$(call cc-disable-warning, gnu)
> >> >
> >> > # arm64 uses the full KBUILD_CFLAGS so it's necessary to explicitly
> >> > # disable the stackleak plugin
> >> > --
> >> > 2.19.1
> >> >
> >>
> >> + Sedat for review/testing.
> >
> > Thanks Nathan.
> >
> > Reviewed-by: Sedat Dilek
> > Tested-by: Sedat Dilek
> > [ Reported-by: Sedat Dilek ]
> > [ See duplicate "arch/x86/include/asm/processor.h: Warning
> > -Waddress-of-packed-member "
> > ]
> >
>
> Queued in efi/next
>
Thanks Ard.
This means efi-for-4.21 / efi-for-5.0?
- Sedat -
Re: [PATCH v10 1/7] x86/boot: Introduce cmdline_find_option_arg()to detect if option=arg in cmdline
On Mon, Oct 22, 2018 at 05:37:14PM +0800, Chao Fan wrote:
> Now, there are cmdline_find_option() and cmdline_find_option_bool() in
> cmdline.c. Sometimes, when detecting such as whether 'acpi=off' is
> in cmdline, we need to cmdline_find_option() first, then compare
> the argument. Now splite the operation as a independent function.
> Introduce a new function cmdline_find_option_arg() to detect whether
> option is in command line and the value is arg.
For all future commit messages you write:
Use passive tone in your commit message: no "we", etc.
Also, pls read section "2) Describe your changes" in
Documentation/process/submitting-patches.rst.
> Signed-off-by: Chao Fan
> ---
> arch/x86/boot/compressed/cmdline.c | 15 +++
> arch/x86/boot/compressed/misc.h| 1 +
> 2 files changed, 16 insertions(+)
>
> diff --git a/arch/x86/boot/compressed/cmdline.c
> b/arch/x86/boot/compressed/cmdline.c
> index af6cda0b7900..61118c69feb8 100644
> --- a/arch/x86/boot/compressed/cmdline.c
> +++ b/arch/x86/boot/compressed/cmdline.c
> @@ -1,5 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> #include "misc.h"
> +#define STATIC
> +#include
>
> #if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE || CONFIG_X86_5LEVEL
>
> @@ -30,5 +32,18 @@ int cmdline_find_option_bool(const char *option)
> {
> return __cmdline_find_option_bool(get_cmd_line_ptr(), option);
> }
> +bool cmdline_find_option_arg(const char *option, const char *arg, int
> argsize)
> +{
> + char *buffer = malloc(argsize+1);
> + bool find = false;
> + int ret;
> +
> + ret = cmdline_find_option(option, buffer, argsize+1);
> + if (ret == argsize && !strncmp(buffer, arg, argsize))
> + find = true;
> +
> + free(buffer);
> + return find;
> +}
I don't think such wrapper is needed. Simply calling
cmdline_find_option() and then examining the buffer - like other call
sites do - is perfectly fine.
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH v8 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory
On Thu, Oct 25, 2018 at 09:40:51AM -0400, Masayoshi Mizuma wrote: > My actual use case is for EFI boxes, however, I think it's better to useful > for legacy BIOS as well because memory hot-plug affinity in SRAT and KASLR > are available on legacy BIOS. > Actually, we can create such environment in qemu. Ah, right, qemu. :) > I have another idea to solve this issue. Adding a SRAT parsing code > to arch/x86/mm/kaslr.c. It is useful for both EFI and BIOS and > also we don't need a new kernel parameter... > Dose the idea make sense? The more automatic stuff we do and we don't have to involve the user, the better. However, lemme look at Chao's current patchset first - we should not go nuts by putting SRAT parsing everywhere :) -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
[PATCH 4/4] efi: reduce the amount of memblock reservations for persistent allocations
The current implementation of efi_mem_reserve_persistent() is rather
naive, in the sense that for each invocation, it creates a separate
linked list entry to describe the reservation. Since the linked list
entries themselves need to persist across subsequent kexec reboots,
every reservation created this way results in two memblock_reserve()
calls at the next boot.
On arm64 systems with 100s of CPUs, this may result in a excessive
number of memblock reservations, and needless fragmentation.
So instead, make use of the newly updated struct linux_efi_memreserve
layout to put multiple reservations into a single linked list entry.
This should get rid of the numerous tiny memblock reservations, and
effectively cut the total number of reservations in half on arm64
systems with many CPUs.
Signed-off-by: Ard Biesheuvel
---
drivers/firmware/efi/efi.c | 29 ++--
include/linux/efi.h| 3 ++
2 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 85d2ec532816..2ca5a59f7568 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -978,22 +978,35 @@ static DEFINE_SPINLOCK(efi_mem_reserve_persistent_lock);
int efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
{
struct linux_efi_memreserve *rsv, *parent;
- int rsvsize = EFI_MEMRESERVE_SIZE(1);
+ unsigned long prsv;
+ int index;
if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
return -ENODEV;
- rsv = kmalloc(rsvsize, GFP_KERNEL);
- if (!rsv)
- return -ENOMEM;
-
parent = memremap(efi.mem_reserve, sizeof(*rsv), MEMREMAP_WB);
- if (!parent) {
- kfree(rsv);
+ if (!parent)
return -ENOMEM;
+
+ /* first try to find a slot in an existing linked list entry */
+ for (prsv = parent->next; prsv; prsv = rsv->next) {
+ rsv = __va(prsv);
+ index = atomic_fetch_add_unless(>count, 1, rsv->size);
+ if (index < rsv->size) {
+ rsv->entry[index].base = addr;
+ rsv->entry[index].size = size;
+
+ memunmap(parent);
+ return 0;
+ }
}
- rsv->size = 1;
+ /* no slot found - allocate a new linked list entry */
+ rsv = (struct linux_efi_memreserve *)__get_free_page(GFP_KERNEL);
+ if (!rsv)
+ return -ENOMEM;
+
+ rsv->size = EFI_MEMRESERVE_COUNT(PAGE_SIZE);
atomic_set(>count, 1);
rsv->entry[0].base = addr;
rsv->entry[0].size = size;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index dfce82b2ca8a..1a1a081f7244 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1727,4 +1727,7 @@ struct linux_efi_memreserve {
#define EFI_MEMRESERVE_SIZE(count) (sizeof(struct linux_efi_memreserve) + \
(count) * sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
+#define EFI_MEMRESERVE_COUNT(size) (((size) - sizeof(struct
linux_efi_memreserve)) \
+ / sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
+
#endif /* _LINUX_EFI_H */
--
2.19.1
[PATCH 2/4] efi/arm: defer persistent reservations until after paging_init()
The new memory EFI reservation feature we introduced to allow memory
reservations to persist across kexec may trigger an unbounded number
of calls to memblock_reserve(). The memblock subsystem can deal with
this fine, but not before memblock resizing is enabled, which we can
only do after paging_init(), when the memory we reallocate the array
into is actually mapped.
So break out the memreserve table processing into a separate function
and call if after paging_init() on both arm64 and ARM.
Cc: Russell King
Signed-off-by: Ard Biesheuvel
---
arch/arm/kernel/setup.c| 1 +
arch/arm64/kernel/setup.c | 1 +
drivers/firmware/efi/efi.c | 8 ++--
include/linux/efi.h| 7 +++
4 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index ac7e08886863..e99f12eaf390 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1117,6 +1117,7 @@ void __init setup_arch(char **cmdline_p)
early_ioremap_reset();
paging_init(mdesc);
+ efi_apply_persistent_mem_reservations();
request_standard_resources(mdesc);
if (mdesc->restart)
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 953e316521fc..f4fc1e0544b7 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -313,6 +313,7 @@ void __init setup_arch(char **cmdline_p)
arm64_memblock_init();
paging_init();
+ efi_apply_persistent_mem_reservations();
acpi_table_upgrade();
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 249eb70691b0..55e4ea20bdc3 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -592,7 +592,11 @@ int __init efi_config_parse_tables(void *config_tables,
int count, int sz,
early_memunmap(tbl, sizeof(*tbl));
}
+ return 0;
+}
+int __init efi_apply_persistent_mem_reservations(void)
+{
if (efi.mem_reserve != EFI_INVALID_TABLE_ADDR) {
unsigned long prsv = efi.mem_reserve;
@@ -602,7 +606,7 @@ int __init efi_config_parse_tables(void *config_tables, int
count, int sz,
/* reserve the entry itself */
memblock_reserve(prsv, sizeof(*rsv));
- rsv = early_memremap(prsv, sizeof(*rsv));
+ rsv = memremap(prsv, sizeof(*rsv), MEMREMAP_WB);
if (rsv == NULL) {
pr_err("Could not map UEFI memreserve
entry!\n");
return -ENOMEM;
@@ -612,7 +616,7 @@ int __init efi_config_parse_tables(void *config_tables, int
count, int sz,
memblock_reserve(rsv->base, rsv->size);
prsv = rsv->next;
- early_memunmap(rsv, sizeof(*rsv));
+ memunmap(rsv);
}
}
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 845174e113ce..100ce4a4aff6 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1167,6 +1167,8 @@ static inline bool efi_enabled(int feature)
extern void efi_reboot(enum reboot_mode reboot_mode, const char *__unused);
extern bool efi_is_table_address(unsigned long phys_addr);
+
+extern int efi_apply_persistent_mem_reservations(void);
#else
static inline bool efi_enabled(int feature)
{
@@ -1185,6 +1187,11 @@ static inline bool efi_is_table_address(unsigned long
phys_addr)
{
return false;
}
+
+static inline int efi_apply_persistent_mem_reservations(void)
+{
+ return 0;
+}
#endif
extern int efi_status_to_err(efi_status_t status);
--
2.19.1
[PATCH 1/4] arm64: memblock: don't permit memblock resizing until linear mapping is up
Bhupesh reports that having numerous memblock reservations at early boot may result in the following crash: Unable to handle kernel paging request at virtual address 80003ffe ... Call trace: __memcpy+0x110/0x180 memblock_add_range+0x134/0x2e8 memblock_reserve+0x70/0xb8 memblock_alloc_base_nid+0x6c/0x88 __memblock_alloc_base+0x3c/0x4c memblock_alloc_base+0x28/0x4c memblock_alloc+0x2c/0x38 early_pgtable_alloc+0x20/0xb0 paging_init+0x28/0x7f8 This is caused by the fact that we permit memblock resizing before the linear mapping is up, and so the memblock_reserved() array is moved into memory that is not mapped yet. So let's ensure that this crash can no longer occur, by deferring to call to memblock_allow_resize() to after the linear mapping has been created. Reported-by: Bhupesh Sharma Signed-off-by: Ard Biesheuvel --- arch/arm64/mm/init.c | 2 -- arch/arm64/mm/mmu.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index 9d9582cac6c4..9b432d9fcada 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -483,8 +483,6 @@ void __init arm64_memblock_init(void) high_memory = __va(memblock_end_of_DRAM() - 1) + 1; dma_contiguous_reserve(arm64_dma_phys_limit); - - memblock_allow_resize(); } void __init bootmem_init(void) diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index 394b8d554def..d1d6601b385d 100644 --- a/arch/arm64/mm/mmu.c +++ b/arch/arm64/mm/mmu.c @@ -659,6 +659,8 @@ void __init paging_init(void) memblock_free(__pa_symbol(init_pg_dir), __pa_symbol(init_pg_end) - __pa_symbol(init_pg_dir)); + + memblock_allow_resize(); } /* -- 2.19.1
[PATCH 3/4] efi: permit multiple entries in persistent memreserve data structure
In preparation of updating efi_mem_reserve_persistent() to cause less
fragmentation when dealing with many persistent reservations, update
the struct definition and the code that handles it currently so it
can describe an arbitrary number of reservations using a single linked
list entry. The actual optimization will be implemented in a subsequent
patch.
Signed-off-by: Ard Biesheuvel
---
drivers/firmware/efi/efi.c | 30 +---
drivers/firmware/efi/libstub/arm-stub.c | 2 +-
include/linux/efi.h | 13 +++--
3 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 55e4ea20bdc3..85d2ec532816 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -602,18 +602,25 @@ int __init efi_apply_persistent_mem_reservations(void)
while (prsv) {
struct linux_efi_memreserve *rsv;
+ u8 *p;
+ int i;
- /* reserve the entry itself */
- memblock_reserve(prsv, sizeof(*rsv));
-
- rsv = memremap(prsv, sizeof(*rsv), MEMREMAP_WB);
- if (rsv == NULL) {
+ p = memremap(ALIGN_DOWN(prsv, PAGE_SIZE), PAGE_SIZE,
+MEMREMAP_WB);
+ if (p == NULL) {
pr_err("Could not map UEFI memreserve
entry!\n");
return -ENOMEM;
}
- if (rsv->size)
- memblock_reserve(rsv->base, rsv->size);
+ rsv = (void *)(p + prsv % PAGE_SIZE);
+
+ /* reserve the entry itself */
+ memblock_reserve(prsv, EFI_MEMRESERVE_SIZE(rsv->size));
+
+ for (i = 0; i < atomic_read(>count); i++) {
+ memblock_reserve(rsv->entry[i].base,
+rsv->entry[i].size);
+ }
prsv = rsv->next;
memunmap(rsv);
@@ -971,11 +978,12 @@ static DEFINE_SPINLOCK(efi_mem_reserve_persistent_lock);
int efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
{
struct linux_efi_memreserve *rsv, *parent;
+ int rsvsize = EFI_MEMRESERVE_SIZE(1);
if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
return -ENODEV;
- rsv = kmalloc(sizeof(*rsv), GFP_KERNEL);
+ rsv = kmalloc(rsvsize, GFP_KERNEL);
if (!rsv)
return -ENOMEM;
@@ -985,8 +993,10 @@ int efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
return -ENOMEM;
}
- rsv->base = addr;
- rsv->size = size;
+ rsv->size = 1;
+ atomic_set(>count, 1);
+ rsv->entry[0].base = addr;
+ rsv->entry[0].size = size;
spin_lock(_mem_reserve_persistent_lock);
rsv->next = parent->next;
diff --git a/drivers/firmware/efi/libstub/arm-stub.c
b/drivers/firmware/efi/libstub/arm-stub.c
index 30ac0c975f8a..5bcfa08e8bb1 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -83,8 +83,8 @@ void install_memreserve_table(efi_system_table_t
*sys_table_arg)
}
rsv->next = 0;
- rsv->base = 0;
rsv->size = 0;
+ atomic_set(>count, 0);
status = efi_call_early(install_configuration_table,
_table_guid,
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 100ce4a4aff6..dfce82b2ca8a 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1715,9 +1715,16 @@ extern struct efi_runtime_work efi_rts_work;
extern struct workqueue_struct *efi_rts_wq;
struct linux_efi_memreserve {
- phys_addr_t next;
- phys_addr_t base;
- phys_addr_t size;
+ int size; // allocated size of the array
+ atomic_tcount; // number of entries used
+ phys_addr_t next; // pa of next struct instance
+ struct {
+ phys_addr_t base;
+ phys_addr_t size;
+ } entry[0];
};
+#define EFI_MEMRESERVE_SIZE(count) (sizeof(struct linux_efi_memreserve) + \
+ (count) * sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
+
#endif /* _LINUX_EFI_H */
--
2.19.1
[PATCH 0/4] arm/efi: fix memblock reallocation crash due to persistent reservations
This series addresses the kexec/kdump crash on arm64 system with many CPUs that was reported by Bhupesh. Patches #1 and #2 fix the actual crash. Patches #3 and #4 optimize the EFI persistent memreserve infrastructure so that fewer memblock reservations are required. Ard Biesheuvel (4): arm64: memblock: don't permit memblock resizing until linear mapping is up efi/arm: defer persistent reservations until after paging_init() efi: permit multiple entries in persistent memreserve data structure efi: reduce the amount of memblock reservations for persistent allocations arch/arm/kernel/setup.c | 1 + arch/arm64/kernel/setup.c | 1 + arch/arm64/mm/init.c| 2 - arch/arm64/mm/mmu.c | 2 + drivers/firmware/efi/efi.c | 59 ++-- drivers/firmware/efi/libstub/arm-stub.c | 2 +- include/linux/efi.h | 23 +++- 7 files changed, 68 insertions(+), 22 deletions(-) -- 2.19.1
Re: [Bug Report] kdump crashes after latest EFI memblock changes on arm64 machines with large number of CPUs
On 6 November 2018 at 02:30, Will Deacon wrote: > On Fri, Nov 02, 2018 at 02:44:10AM +0530, Bhupesh Sharma wrote: >> With the latest EFI changes for memblock reservation across kdump >> kernel from Ard (Commit 71e0940d52e107748b270213a01d3b1546657d74 >> ["efi: honour memory reservations passed via a linux specific config >> table"]), we hit a panic while trying to boot the kdump kernel on >> machines which have large number of CPUs. >> >> I have a arm64 board which has 224 CPUS: >> # lscpu >> <..snip..> >> CPU(s): 224 >> On-line CPU(s) list: 0-223 >> <..snip..> >> >> Here are the crash logs in the kdump kernel on this machine: >> >> [0.00] Unable to handle kernel paging request at virtual >> address 80003ffe >> val)nt EL), IL ata abort info: >> [0.or: Oops: 96inted 4.18.0+ #3 >> [0.00] pstate: 20400089 (nzCv daIf +PAN -UAO) >> [0.00] pc : __memcpy+0x110/0x180 >> [0.00] lr : memblock_double_array+0x240/0x348 >> [0.00] sp : 092efc80 x28: bffe >> [0.00] x27: 1800 x26: 09d59000 >> [0.00] x25: 80003ffe x24: >> [0.00] x23: 0001 x22: 09d594e8 >> [0.00] x21: 09d594f4 x20: 093c7268 >> [0.00] x19: 0c00 x18: 0010 >> [0.00] x17: x16: >> [0.00] x15: 3: 000fc18d x12: 0008 >> [0.00] x11: 0018 x10: ddab9e18 >> [0.00] x9 : 0008 x8 : 02c1 >> [0.00] x7 : 91b9 x6 : 80003ffe >> [0.00] x5 : 0001 x4 : >> [0.00] x3 : x2 : 0b80 >> [0.00] x1 : 09d59540 x0 : 80003ffe >> [0.00] Process swapper) >> [0.00] Call trace: >> [0.00] __memcpy+0x110/0x180 >> [0.00] memblock_add_range+0x134/0x2e8 >> [0.00] memblock_reserve+0x70/0xb8 >> [0.00] memblock_alloc_base_nid+0x6c/0x88 >> [0.00] __memblock_alloc_base+0x3c/0x4c >> [0.00] memblock_alloc_base+0x28/0x4c >> [0.00] memblock_alloc+0x2c/0x38 >> [0.00] early_pgtable_alloc+0x20/0xb0 > > Hmm, so this seems to be the crux of the issue: early_pgtable_alloc() relies > on memblock to allocate page-table memory, but this can be called before the > linear mapping is up and running (or even as part of creating the linear > mapping itself!) so the use of __va in memblock_double_array() actually > returns an unmapped address. > OK, so this means we are calling memblock_allow_resize() too early in any case > So I guess we either need to implement early_pgtable_alloc() some other way > (how?) or get memblock_double_array() to use a fixmap if it's called too > early (yuck). Alternatively, would it be possible to postpone processing of > the EFI mem_reserve entries until after we've created the linear mapping? > We could move this until after paging_init(), I suppose. I'll cook something up. Bhupesh: any comments?
