Re: [PATCH v16 08/11] x86, arm64: Add ARCH_WANT_RESERVE_CRASH_KERNEL config
On 2021/12/9 1:09, Catalin Marinas wrote:
> On Tue, Nov 23, 2021 at 08:46:43PM +0800, Zhen Lei wrote:
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index 26b8ed11639da46..19256aa924c3b2c 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -24,6 +24,9 @@ config KEXEC_ELF
>> config HAVE_IMA_KEXEC
>> bool
>>
>> +config ARCH_WANT_RESERVE_CRASH_KERNEL
>> +bool
>> +
>> config SET_FS
>> bool
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index c4207cf9bb17ffb..4b99efa36da3793 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -95,6 +95,7 @@ config ARM64
>> select ARCH_WANT_FRAME_POINTERS
>> select ARCH_WANT_HUGE_PMD_SHARE if ARM64_4K_PAGES || (ARM64_16K_PAGES
>> && !ARM64_VA_BITS_36)
>> select ARCH_WANT_LD_ORPHAN_WARN
>> +select ARCH_WANT_RESERVE_CRASH_KERNEL if KEXEC_CORE
>> select ARCH_WANTS_NO_INSTR
>> select ARCH_HAS_UBSAN_SANITIZE_ALL
>> select ARM_AMBA
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 7399327d1eff79d..528034b4276ecf8 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -12,6 +12,7 @@ config X86_32
>> depends on !64BIT
>> # Options that are inherently 32-bit kernel only:
>> select ARCH_WANT_IPC_PARSE_VERSION
>> +select ARCH_WANT_RESERVE_CRASH_KERNEL if KEXEC_CORE
>> select CLKSRC_I8253
>> select CLONE_BACKWARDS
>> select GENERIC_VDSO_32
>> @@ -28,6 +29,7 @@ config X86_64
>> select ARCH_HAS_GIGANTIC_PAGE
>> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
>> select ARCH_USE_CMPXCHG_LOCKREF
>> +select ARCH_WANT_RESERVE_CRASH_KERNEL if KEXEC_CORE
>> select HAVE_ARCH_SOFT_DIRTY
>> select MODULES_USE_ELF_RELA
>> select NEED_DMA_MAP_STATE
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> index 4dc2643fcbccf99..b23cfc0ca8905fd 100644
>> --- a/kernel/crash_core.c
>> +++ b/kernel/crash_core.c
>> @@ -321,9 +321,7 @@ int __init parse_crashkernel_low(char *cmdline,
>> * - Crashkernel reservation --
>> */
>>
>> -#ifdef CONFIG_KEXEC_CORE
>> -
>> -#if defined(CONFIG_X86) || defined(CONFIG_ARM64)
>> +#ifdef CONFIG_ARCH_WANT_RESERVE_CRASH_KERNEL
>> static int __init reserve_crashkernel_low(void)
>> {
>> #ifdef CONFIG_64BIT
>> @@ -451,8 +449,7 @@ void __init reserve_crashkernel(void)
>> crashk_res.start = crash_base;
>> crashk_res.end = crash_base + crash_size - 1;
>> }
>> -#endif
>> -#endif /* CONFIG_KEXEC_CORE */
>> +#endif /* CONFIG_ARCH_WANT_RESERVE_CRASH_KERNEL */
>
> Nitpick mostly but it may simplify the patches if the x86, arch/Kconfig
> and crash_core.c changes here could be moved to patch 5. The remaining
> select for arm64 should be moved to patch 7 and drop the #if change in
> that patch.
>
> This way we can keep the x86 patches on a separate branch.
That's a good suggestion. I will do it.
>
> Thanks.
>
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v16 10/11] of: fdt: Add memory for devices by DT property "linux,usable-memory-range"
On 2021/12/1 10:55, Leizhen (ThunderTown) wrote:
>>> + }
>>>
>>> - memblock_cap_memory_range(cap_mem_addr, cap_mem_size);
>>> + memblock_cap_memory_range(rgn[0].base, rgn[0].size);
>>> + for (i = 1; i < MAX_USABLE_RANGES && rgn[i].size; i++)
>> s/ &&/,/
Hi Rob:
I want to keep "&&" unchanged, do you mind? I'm going to post an
updated version tomorrow, hopefully the last.
> Hi Rob:
>
> The comma operator may not be suitable for logical judgment. The logical
> judgment
> before commas (,) is ignored.
>
> Here's my test：
>
> C code：
> int main()
> {
> int i, j;
>
> printf("&&:\n");
> for (i = 0, j = 0; i < 2 && j < 3; i++, j++)
> printf("i=%d, j=%d\n", i, j);
>
> printf("\ncomma:\n");
> for (i = 0, j = 0; i < 2, j < 3; i++, j++)//(i < 2） before comma
> is ignored
> printf("i=%d, j=%d\n", i, j);
>
> return 0;
> }
>
> Output：
> &&:
> i=0, j=0
> i=1, j=1
>
> comma:
> i=0, j=0
> i=1, j=1
> i=2, j=2
>
>
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 0/6] KEXEC_SIG with appended signature
On 11/25/21 13:02, Michal Suchanek wrote: Hello, Hi Michael, This is resend of the KEXEC_SIG patchset. The first patch is new because it'a a cleanup that does not require any change to the module verification code. The second patch is the only one that is intended to change any functionality. The rest only deduplicates code but I did not receive any review on that part so I don't know if it's desirable as implemented. The first two patches can be applied separately without the rest. Patch 2 fails to apply on v5.16-rc4. Can you please also include git tree/branch while posting the patches ? Secondly, I see that you add the powerpc support in Patch 2 and then modify it again in Patch 5 after cleanup. Why not add the support for powerpc after the clean up ? This will reduce some rework and also probably simplify patches. Thanks & Regards, - Nayna ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v16 00/11] support reserving crashkernel above 4G on arm64 kdump
On Tue, Nov 23, 2021 at 08:46:35PM +0800, Zhen Lei wrote: > Chen Zhou (10): > x86: kdump: replace the hard-coded alignment with macro CRASH_ALIGN > x86: kdump: make the lower bound of crash kernel reservation > consistent > x86: kdump: use macro CRASH_ADDR_LOW_MAX in functions > reserve_crashkernel() > x86: kdump: move xen_pv_domain() check and insert_resource() to > setup_arch() > x86: kdump: move reserve_crashkernel[_low]() into crash_core.c > arm64: kdump: introduce some macros for crash kernel reservation > arm64: kdump: reimplement crashkernel=X > x86, arm64: Add ARCH_WANT_RESERVE_CRASH_KERNEL config > of: fdt: Add memory for devices by DT property > "linux,usable-memory-range" > kdump: update Documentation about crashkernel > > Zhen Lei (1): > of: fdt: Aggregate the processing of "linux,usable-memory-range" Apart from a minor comment I made on patch 8 and some comments from Rob that need addressing, the rest looks fine to me. Ingo stated in the past that he's happy to ack the x86 changes as long as there's no functional change (and that's the case AFAICT). Ingo, does your conditional ack still stand? In terms of merging, I'm happy to take it all through the arm64 tree with acks from the x86 maintainers. Alternatively, with the change I mentioned for patch 8, the first 5 patches could be queued via the tip tree on a stable branch and I can base the rest of the arm64 on top. Thomas, Ingo, Peter, any preference? Thanks. -- Catalin ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v16 08/11] x86, arm64: Add ARCH_WANT_RESERVE_CRASH_KERNEL config
On Tue, Nov 23, 2021 at 08:46:43PM +0800, Zhen Lei wrote:
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 26b8ed11639da46..19256aa924c3b2c 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -24,6 +24,9 @@ config KEXEC_ELF
> config HAVE_IMA_KEXEC
> bool
>
> +config ARCH_WANT_RESERVE_CRASH_KERNEL
> + bool
> +
> config SET_FS
> bool
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index c4207cf9bb17ffb..4b99efa36da3793 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -95,6 +95,7 @@ config ARM64
> select ARCH_WANT_FRAME_POINTERS
> select ARCH_WANT_HUGE_PMD_SHARE if ARM64_4K_PAGES || (ARM64_16K_PAGES
> && !ARM64_VA_BITS_36)
> select ARCH_WANT_LD_ORPHAN_WARN
> + select ARCH_WANT_RESERVE_CRASH_KERNEL if KEXEC_CORE
> select ARCH_WANTS_NO_INSTR
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> select ARM_AMBA
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 7399327d1eff79d..528034b4276ecf8 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -12,6 +12,7 @@ config X86_32
> depends on !64BIT
> # Options that are inherently 32-bit kernel only:
> select ARCH_WANT_IPC_PARSE_VERSION
> + select ARCH_WANT_RESERVE_CRASH_KERNEL if KEXEC_CORE
> select CLKSRC_I8253
> select CLONE_BACKWARDS
> select GENERIC_VDSO_32
> @@ -28,6 +29,7 @@ config X86_64
> select ARCH_HAS_GIGANTIC_PAGE
> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> select ARCH_USE_CMPXCHG_LOCKREF
> + select ARCH_WANT_RESERVE_CRASH_KERNEL if KEXEC_CORE
> select HAVE_ARCH_SOFT_DIRTY
> select MODULES_USE_ELF_RELA
> select NEED_DMA_MAP_STATE
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 4dc2643fcbccf99..b23cfc0ca8905fd 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -321,9 +321,7 @@ int __init parse_crashkernel_low(char *cmdline,
> * - Crashkernel reservation --
> */
>
> -#ifdef CONFIG_KEXEC_CORE
> -
> -#if defined(CONFIG_X86) || defined(CONFIG_ARM64)
> +#ifdef CONFIG_ARCH_WANT_RESERVE_CRASH_KERNEL
> static int __init reserve_crashkernel_low(void)
> {
> #ifdef CONFIG_64BIT
> @@ -451,8 +449,7 @@ void __init reserve_crashkernel(void)
> crashk_res.start = crash_base;
> crashk_res.end = crash_base + crash_size - 1;
> }
> -#endif
> -#endif /* CONFIG_KEXEC_CORE */
> +#endif /* CONFIG_ARCH_WANT_RESERVE_CRASH_KERNEL */
Nitpick mostly but it may simplify the patches if the x86, arch/Kconfig
and crash_core.c changes here could be moved to patch 5. The remaining
select for arm64 should be moved to patch 7 and drop the #if change in
that patch.
This way we can keep the x86 patches on a separate branch.
Thanks.
--
Catalin
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC v2 4/6] crash hp: generic crash hotplug support infrastructure
> +#if defined(CONFIG_MEMORY_HOTPLUG)
> +static int crash_memhp_notifier(struct notifier_block *nb,
> + unsigned long val, void *v)
> +{
> + struct memory_notify *mhp = v;
> + unsigned long start, end;
> +
> + start = mhp->start_pfn << PAGE_SHIFT;
> + end = ((mhp->start_pfn + mhp->nr_pages) << PAGE_SHIFT) - 1;
> +
> + switch (val) {
> + case MEM_GOING_ONLINE:
> + crash_hotplug_handler(KEXEC_CRASH_HP_ADD_MEMORY,
> + start, end-start);
> + break;
> +
> + case MEM_OFFLINE:
> + case MEM_CANCEL_ONLINE:
> + crash_hotplug_handler(KEXEC_CRASH_HP_REMOVE_MEMORY,
> + start, end-start);
Any reason you don't handle this after the effects completely, meaning
MEM_ONLINE and MEM_OFFLINE?
--
Thanks,
David / dhildenb
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2] kernel/crash_core: suppress unknown crashkernel parameter warning
When booting with crashkernel= on the kernel command line a warning
similar to
[0.038294] Kernel command line: ro console=ttyS0 crashkernel=256M
[0.038353] Unknown kernel command line parameters "crashkernel=256M", will
be passed to user space.
is printed. This comes from crashkernel= being parsed independent from
the kernel parameter handling mechanism. So the code in init/main.c
doesn't know that crashkernel= is a valid kernel parameter and prints
this incorrect warning. Suppress the warning by adding a dummy
early_param handler for crashkernel=.
Fixes: 86d1919a4fb0 ("init: print out unknown kernel parameters")
Signed-off-by: Philipp Rudo
Acked-by: Baoquan He
---
v2:
- improve commit message
- add Acked-by from Baoquan
---
kernel/crash_core.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index eb53f5ec62c9..256cf6db573c 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -6,6 +6,7 @@
#include
#include
+#include
#include
#include
@@ -295,6 +296,16 @@ int __init parse_crashkernel_low(char *cmdline,
"crashkernel=", suffix_tbl[SUFFIX_LOW]);
}
+/*
+ * Add a dummy early_param handler to mark crashkernel= as a known command line
+ * parameter and suppress incorrect warnings in init/main.c.
+ */
+static int __init parse_crashkernel_dummy(char *arg)
+{
+ return 0;
+}
+early_param("crashkernel", parse_crashkernel_dummy);
+
Elf_Word *append_elf_note(Elf_Word *buf, char *name, unsigned int type,
void *data, size_t data_len)
{
--
2.31.1
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 1/1] s390: handle R_390_PLT32DBL reloc entries in machine_apply_elf_rel()
Starting with gcc 11.3, the C compiler will generate PLT-relative function calls even if they are local and do not require it. Later on during linking, the linker will replace all PLT-relative calls to local functions with PC-relative ones. Unfortunately, the purgatory code of kexec/kdump is not being linked as a regular executable or shared library would have been, and therefore, all PLT-relative addresses remain in the generated purgatory object code unresolved. This leads to the situation where the purgatory code is being executed during kdump with all PLT-relative addresses unresolved. And this results in endless loops within the purgatory code. Furthermore, the clang C compiler has always behaved like described above and this commit should fix the purgatory code built with the latter. Because the purgatory code is no regular executable or shared library, contains only calls to local functions and has no PLT, all R_390_PLT32DBL relocation entries can be resolved just like a R_390_PC32DBL one. * https://refspecs.linuxfoundation.org/ELF/zSeries/lzsabi0_zSeries/x1633.html#AEN1699 Relocation entries of purgatory code generated with gcc 11.3 $ readelf -r purgatory/purgatory.o Relocation section '.rela.text' at offset 0x6e8 contains 27 entries: Offset Info Type Sym. ValueSym. Name + Addend 000c 00030013 R_390_PC32DBL .data + 2 001a 00100014 R_390_PLT32DBL sha256_starts + 2 0030 00110014 R_390_PLT32DBL sha256_update + 2 0046 00120014 R_390_PLT32DBL sha256_finish + 2 0050 00030013 R_390_PC32DBL .data + 102 005a 00130014 R_390_PLT32DBL memcmp + 2 ... 0118 00160014 R_390_PLT32DBL setup_arch + 2 011e 00030013 R_390_PC32DBL .data + 2 012c 000f0014 R_390_PLT32DBL verify_sha256_digest + 2 0142 00170014 R_390_PLT32DBL post_verification[...] + 2 Relocation entries of purgatory code generated with gcc 11.2 $ readelf -r purgatory/purgatory.o Relocation section '.rela.text' at offset 0x6e8 contains 27 entries: Offset Info Type Sym. ValueSym. Name + Addend 000e 00030013 R_390_PC32DBL .data + 2 001c 00100013 R_390_PC32DBL sha256_starts + 2 0036 00110013 R_390_PC32DBL sha256_update + 2 0048 00120013 R_390_PC32DBL sha256_finish + 2 0052 00030013 R_390_PC32DBL .data + 102 005c 00130013 R_390_PC32DBL memcmp + 2 ... 011a 00160013 R_390_PC32DBL setup_arch + 2 0120 00030013 R_390_PC32DBL .data + 122 0130 000f0013 R_390_PC32DBL verify_sha256_digest + 2 0146 00170013 R_390_PC32DBL post_verification[...] + 2 Corresponding s390 kernel discussion: * https://lore.kernel.org/linux-s390/20211208105801.188140-1-egore...@linux.ibm.com/T/#u Signed-off-by: Alexander Egorenkov Reported-by: Tao Liu Suggested-by: Philipp Rudo --- kexec/arch/s390/kexec-elf-rel-s390.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kexec/arch/s390/kexec-elf-rel-s390.c b/kexec/arch/s390/kexec-elf-rel-s390.c index a5e1b7345578..91ba86a9991d 100644 --- a/kexec/arch/s390/kexec-elf-rel-s390.c +++ b/kexec/arch/s390/kexec-elf-rel-s390.c @@ -56,6 +56,7 @@ void machine_apply_elf_rel(struct mem_ehdr *UNUSED(ehdr), case R_390_PC16:/* PC relative 16 bit. */ case R_390_PC16DBL: /* PC relative 16 bit shifted by 1. */ case R_390_PC32DBL: /* PC relative 32 bit shifted by 1. */ + case R_390_PLT32DBL:/* 32 bit PC rel. PLT shifted by 1. */ case R_390_PC32:/* PC relative 32 bit. */ case R_390_PC64:/* PC relative 64 bit. */ val -= address; @@ -63,7 +64,7 @@ void machine_apply_elf_rel(struct mem_ehdr *UNUSED(ehdr), *(unsigned short *) loc = val; else if (r_type == R_390_PC16DBL) *(unsigned short *) loc = val >> 1; - else if (r_type == R_390_PC32DBL) + else if (r_type == R_390_PC32DBL || r_type == R_390_PLT32DBL) *(unsigned int *) loc = val >> 1; else if (r_type == R_390_PC32) *(unsigned int *) loc = val; -- 2.31.1 ___ kexec mailing list kexec@lists.infradead.org
Re: [PATCH v2 0/6] KEXEC_SIG with appended signature
Hi Michal, On Tue, 7 Dec 2021 18:32:21 +0100 Michal Suchánek wrote: > On Tue, Dec 07, 2021 at 05:10:14PM +0100, Philipp Rudo wrote: > > Hi Michal, > > > > i finally had the time to take a closer look at the series. Except for > > the nit in patch 4 and my personal preference in patch 6 the code looks > > good to me. > > > > What I don't like are the commit messages on the first commits. In my > > opinion they are so short that they are almost useless. For example in > > patch 2 there is absolutely no explanation why you can simply copy the > > s390 over to ppc. > > They use the same signature format. I suppose I can add a note saying > that. The note is what I was asking for. For me the commit message is an important piece of documentation for other developers (or yourself in a year). That's why in my opinion it's important to describe _why_ you do something in it as you cannot get the _why_ by reading the code. > > Or in patch 3 you are silently changing the error > > code in kexec from EKEYREJECT to ENODATA. So I would appreciate it if > > Not sure what I should do about this. The different implementations use > different random error codes, and when they are unified the error code > clearly changes for one or the other. My complaint wasn't that you change the return code. There's no way to avoid choosing one over the other. It's again that you don't document the change in the commit message for others. > Does anything depend on a particular error code returned? Not that I know of. At least in the kexec-tools ENODATA and EKEYREJECT are handled the same way. Thanks Philipp > Thanks > > Michal > > > you could improve them a little. > > > > Thanks > > Philipp > > > > On Thu, 25 Nov 2021 19:02:38 +0100 > > Michal Suchanek wrote: > > > > > Hello, > > > > > > This is resend of the KEXEC_SIG patchset. > > > > > > The first patch is new because it'a a cleanup that does not require any > > > change to the module verification code. > > > > > > The second patch is the only one that is intended to change any > > > functionality. > > > > > > The rest only deduplicates code but I did not receive any review on that > > > part so I don't know if it's desirable as implemented. > > > > > > The first two patches can be applied separately without the rest. > > > > > > Thanks > > > > > > Michal > > > > > > Michal Suchanek (6): > > > s390/kexec_file: Don't opencode appended signature check. > > > powerpc/kexec_file: Add KEXEC_SIG support. > > > kexec_file: Don't opencode appended signature verification. > > > module: strip the signature marker in the verification function. > > > module: Use key_being_used_for for log messages in > > > verify_appended_signature > > > module: Move duplicate mod_check_sig users code to mod_parse_sig > > > > > > arch/powerpc/Kconfig | 11 + > > > arch/powerpc/kexec/elf_64.c | 14 ++ > > > arch/s390/kernel/machine_kexec_file.c| 42 ++ > > > crypto/asymmetric_keys/asymmetric_type.c | 1 + > > > include/linux/module_signature.h | 1 + > > > include/linux/verification.h | 4 ++ > > > kernel/module-internal.h | 2 - > > > kernel/module.c | 12 +++-- > > > kernel/module_signature.c| 56 +++- > > > kernel/module_signing.c | 33 +++--- > > > security/integrity/ima/ima_modsig.c | 22 ++ > > > 11 files changed, 113 insertions(+), 85 deletions(-) > > > > > > ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
