Re: [PATCH 0/2 RESEND v7] add reserved e820 ranges to the kdump kernel e820 table
在 2018年11月27日 02:54, Dave Hansen 写道: > On 11/23/18 9:12 PM, Lianbo Jiang wrote: >> These patches add the new I/O resource descriptor 'IORES_DESC_RESERVED' >> for the iomem resources search interfaces, and in order to make it still >> work after the new descriptor is added, these codes originally related >> to 'IORES_DESC_NONE' have been updated. > > This is rather anemic "0/" text. Could you please include some more > background in here? The 2/2 patch is pretty good in this regard, but it > needs to be here, too. > Thanks for your comment. Originally, this patch added the e820 reserved ranges by accurately comparing a string. It only modifies fewer code paths. Please refer to patch v6. https://lore.kernel.org/lkml/20181114072926.13312-2-liji...@redhat.com/ Because the string comparison is fragile and error-prone, this patch used the solution that adds a new descriptor 'IORES_DESC_RESERVED'. Please refer to this link: https://lore.kernel.org/lkml/20181120192935.gk2...@zn.tnic/ When passing the e820 reserved ranges to the second kernel, why does it need to compare strings accurately or add a new descriptor 'IORES_DESC_RESERVED'? -The reason is that it can not exactly match the e820 reserved resource ranges when walking through iomem resources with the descriptor 'IORES_DESC_NONE'. Thanks. Lianbo
Re: [PATCH 1/2 RESEND v7] resource: add the new I/O resource descriptor 'IORES_DESC_RESERVED'
On 11/23/18 9:12 PM, Lianbo Jiang wrote:
> The upstream kernel can not accurately add the e820 reserved type to> kdump
> krenel e820 table.
^ kernel
> Kdump uses walk_iomem_res_desc() to iterate io resources, then adds
> the matched resource ranges to the e820 table for kdump kernel. But,
> when convert the e820 type to the iores descriptor, several e820
> types are converted to 'IORES_DESC_NONE' in this function e820_type
> _to_iores_desc(). So the walk_iomem_res_desc() will get the redundant
> types(such as E820_TYPE_RAM/E820_TYPE_UNUSABLE/E820_TYPE_KERN) when
> walk through io resources with the descriptor 'IORES_DESC_NONE'.
Let me see if I understand.
When doing kexec, the old kernel makes a new e820 table for the new
kernel. The old kernel constructs the new e820 table from
'iomem_resource'. But, when creating the 'iomem_resource' tree,
reserved areas like E820_TYPE_RESERVED are not properly passed through.
This causes problems like described in the next patch.
> This patch adds the new I/O resource descriptor 'IORES_DESC_RESERVED'
> for the iomem resources search interfaces. It is helpful to exactly
> match the reserved resource ranges when walking through iomem resources.
It's more than that, though. You're specifically storing the reserved
area(s) when we see them come through the firmware.
> Furthermore, in order to make it still work after the new descriptor
> is added, these codes originally related to 'IORES_DESC_NONE' have
> been updated.
It would be nice to explain why they needed to be updated and what
breaks if they are not.
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 5378d10f1d31..91b6112e7489 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -83,7 +83,14 @@ static bool __ioremap_check_ram(struct resource *res)
>
> static int __ioremap_check_desc_other(struct resource *res)
> {
> - return (res->desc != IORES_DESC_NONE);
> + /*
> + * The E820_TYPE_RESERVED was converted to the IORES_DESC_NONE
> + * before the new IORES_DESC_RESERVED is added, so it contained
> + * the e820 reserved type. In order to make it still work for
> + * SEV, here keep it the same as before.
> + */
> + return ((res->desc != IORES_DESC_NONE) ||
> + (res->desc != IORES_DESC_RESERVED));
> }
After reading the changelog and the comment, I still have no idea why
this hunk is here. Could you try to explain a bit more?
Re: Did You Receive My Last Mail?
Hello, My name is ms. Reem Al-Hashimi. The UAE minister of state for international cooparation. I got your contact from an email database from your country. I have a financial transaction i would like to discuss with you. Please reply to reem2...@daum.net, for more details if you are interested. Regards, Ms. Reem Al-Hashimi
Re: [PATCH 0/2 RESEND v7] add reserved e820 ranges to the kdump kernel e820 table
On 11/23/18 9:12 PM, Lianbo Jiang wrote: > These patches add the new I/O resource descriptor 'IORES_DESC_RESERVED' > for the iomem resources search interfaces, and in order to make it still > work after the new descriptor is added, these codes originally related > to 'IORES_DESC_NONE' have been updated. This is rather anemic "0/" text. Could you please include some more background in here? The 2/2 patch is pretty good in this regard, but it needs to be here, too.
Re: [PATCH 0/2 RESEND v7] add reserved e820 ranges to the kdump kernel e820 table
On Mon, Nov 26, 2018 at 09:44:46AM -0800, Dave Hansen wrote: > On 11/23/18 9:12 PM, Lianbo Jiang wrote: > > These patches add the new I/O resource descriptor 'IORES_DESC_RESERVED' > > for the iomem resources search interfaces, and in order to make it still > > work after the new descriptor is added, these codes originally related > > to 'IORES_DESC_NONE' have been updated. > > I'm having a really hard time figuring out what problem these patches solve. > > What end-user visible behavior does this set change? Yeah, that was a long process of digging out the "why". I'd recommend reading the previous threads and, more specifically: https://lkml.kernel.org/r/cabhmzuuscs3juzusm5y6eyjk6weo7mjj5-eakgvbw0qee%2b3...@mail.gmail.com in short: we need to export e820 reserved ranges to the second kernel so that it can talk to PCI devices properly. (And I'm wondering how it did work until now...) -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH 0/2 RESEND v7] add reserved e820 ranges to the kdump kernel e820 table
On 11/23/18 9:12 PM, Lianbo Jiang wrote: > These patches add the new I/O resource descriptor 'IORES_DESC_RESERVED' > for the iomem resources search interfaces, and in order to make it still > work after the new descriptor is added, these codes originally related > to 'IORES_DESC_NONE' have been updated. I'm having a really hard time figuring out what problem these patches solve. What end-user visible behavior does this set change?
EFI-pstore, sleeping from back context after fault
So I triggered a bug in x86 code. First the "okay" backtrace:
|BUG: GPF in non-whitelisted uaccess (non-canonical address?)
|general protection fault: [#1] PREEMPT SMP NOPTI
|CPU: 26 PID: 2236 Comm: sig-xstate-bum Not tainted 4.20.0-rc3 #45
|RIP: 0010:__fpu__restore_sig+0x1c1/0x540
|Call Trace:
| fpu__restore_sig+0x28/0x40
| restore_sigcontext+0x13a/0x180
| __ia32_sys_rt_sigreturn+0xae/0x100
| do_syscall_64+0x4f/0x100
| entry_SYSCALL_64_after_hwframe+0x44/0xa9
|RIP: 0033:0x7f9b06aea227
|---[ end trace a45ac23b593e9ab0 ]---
and now the not okay part:
|BUG: sleeping function called from invalid context at
kernel/sched/completion.c:99
|in_atomic(): 1, irqs_disabled(): 1, pid: 2236, name: sig-xstate-bum
|Preemption disabled at:
|[] pstore_dump+0x72/0x330
|CPU: 26 PID: 2236 Comm: sig-xstate-bum Tainted: G D 4.20.0-rc3
#45
|Call Trace:
| dump_stack+0x4f/0x6a
| ___might_sleep.cold.91+0xd3/0xe4
| __might_sleep+0x50/0x90
| wait_for_completion+0x32/0x130
| virt_efi_query_variable_info+0x14e/0x160
| efi_query_variable_store+0x51/0x1a0
| efivar_entry_set_safe+0xa3/0x1b0
| efi_pstore_write+0x109/0x140
| pstore_dump+0x11c/0x330
| kmsg_dump+0xa4/0xd0
| oops_exit+0x22/0x30
| oops_end+0x67/0xd0
| die+0x41/0x4a
| do_general_protection+0xc1/0x150
| general_protection+0x1e/0x30
|RIP: 0010:__fpu__restore_sig+0x1c1/0x540
| fpu__restore_sig+0x28/0x40
| restore_sigcontext+0x13a/0x180
| __ia32_sys_rt_sigreturn+0xae/0x100
| do_syscall_64+0x4f/0x100
| entry_SYSCALL_64_after_hwframe+0x44/0xa9
|RIP: 0033:0x7f9b06aea227
… [ moar backtrace ]
This looks like it comes from commit 21b3ddd39fee ("efi: Don't use
spinlocks for efi vars") because it replaced a spin_lock() with
down_interruptible() in this case. In this case, since pstore_dump()
holds psinfo->buf_lock with disabled interrupts we can't block…
I have this as a workaround:
diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
index 9336ffdf6e2c..d4dcfe46eb2e 100644
--- a/drivers/firmware/efi/vars.c
+++ b/drivers/firmware/efi/vars.c
@@ -755,7 +755,9 @@ int efivar_entry_set_safe(efi_char16_t *name, efi_guid_t
vendor, u32 attributes,
return -EINTR;
}
- status = check_var_size(attributes, size + ucs2_strsize(name, 1024));
+ status = ops->query_variable_store(attributes,
+ size + ucs2_strsize(name, 1024),
+ block);
if (status != EFI_SUCCESS) {
up(&efivars_lock);
return -ENOSPC;
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index b821054ca3ed..9aa27a2c8d36 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -127,10 +127,10 @@ static const char *get_reason_str(enum kmsg_dump_reason
reason)
bool pstore_cannot_block_path(enum kmsg_dump_reason reason)
{
/*
-* In case of NMI path, pstore shouldn't be blocked
-* regardless of reason.
+* In case of disabled preemption / interrupts we can't block on any
+* lock regardless of reason.
*/
- if (in_nmi())
+ if (in_atomic() || irqs_disabled())
return true;
switch (reason) {
Sebastian
[PATCH 4.4 19/70] efi/libstub/arm64: Set -fpie when building the EFI stub
4.4-stable review patch. If anyone has any objections, please let me know. -- From: Ard Biesheuvel commit 91ee5b21ee026c49e4e7483de69b55b8b47042be upstream. Clang may emit absolute symbol references when building in non-PIC mode, even when using the default 'small' code model, which is already mostly position independent to begin with, due to its use of adrp/add pairs that have a relative range of +/- 4 GB. The remedy is to pass the -fpie flag, which can be done safely now that the code has been updated to avoid GOT indirections (which may be emitted due to the compiler assuming that the PIC/PIE code may end up in a shared library that is subject to ELF symbol preemption) Passing -fpie when building code that needs to execute at an a priori unknown offset is arguably an improvement in any case, and given that the recent visibility changes allow the PIC build to pass with GCC as well, let's add -fpie for all arm64 builds rather than only for Clang. Tested-by: Matthias Kaehlcke Signed-off-by: Ard Biesheuvel Cc: Linus Torvalds Cc: Matt Fleming Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: linux-efi@vger.kernel.org Link: http://lkml.kernel.org/r/20170818194947.19347-5-ard.biesheu...@linaro.org Signed-off-by: Ingo Molnar Signed-off-by: Nathan Chancellor Signed-off-by: Greg Kroah-Hartman --- drivers/firmware/efi/libstub/Makefile |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -10,7 +10,7 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__K -fPIC -fno-strict-aliasing -mno-red-zone \ -mno-mmx -mno-sse -DDISABLE_BRANCH_PROFILING -cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) +cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS)) -fpie cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \ -fno-builtin -fpic -mno-single-pic-base
[PATCH 4.4 18/70] efi/libstub/arm64: Force hidden visibility for section markers
4.4-stable review patch. If anyone has any objections, please let me know. -- From: Ard Biesheuvel commit 0426a4e68f18d75515414361de9e3e1445d2644e upstream. To prevent the compiler from emitting absolute references to the section markers when running in PIC mode, override the visibility to 'hidden' for all contents of asm/sections.h Tested-by: Matthias Kaehlcke Signed-off-by: Ard Biesheuvel Cc: Linus Torvalds Cc: Matt Fleming Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: linux-efi@vger.kernel.org Link: http://lkml.kernel.org/r/20170818194947.19347-4-ard.biesheu...@linaro.org Signed-off-by: Ingo Molnar [nc: Fix conflict due to lack of commit 42b55734030c1 in linux-4.4.y] Signed-off-by: Nathan Chancellor Signed-off-by: Greg Kroah-Hartman --- drivers/firmware/efi/libstub/arm64-stub.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) --- a/drivers/firmware/efi/libstub/arm64-stub.c +++ b/drivers/firmware/efi/libstub/arm64-stub.c @@ -9,9 +9,17 @@ * published by the Free Software Foundation. * */ + +/* + * To prevent the compiler from emitting GOT-indirected (and thus absolute) + * references to the section markers, override their visibility as 'hidden' + */ +#pragma GCC visibility push(hidden) +#include +#pragma GCC visibility pop + #include #include -#include efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table_arg, unsigned long *image_addr,
