* [PATCH] efi: permit calling efi_mem_reserve_persistent from atomic context
@ 2018-11-08 18:05 Ard Biesheuvel
2018-11-08 20:48 ` Qian Cai
2018-11-10 2:45 ` Qian Cai
0 siblings, 2 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2018-11-08 18:05 UTC (permalink / raw)
To: linux-efi
Cc: linux-arm-kernel, cai, linux-mm, linux-kernel, marc.zyngier,
will.deacon, Ard Biesheuvel
Currently, efi_mem_reserve_persistent() may not be called from atomic
context, since both the kmalloc() call and the memremap() call may
sleep.
The kmalloc() call is easy enough to fix, but the memremap() call
needs to be moved into an init hook since we cannot control the
memory allocation behavior of memremap() at the call site.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
drivers/firmware/efi/efi.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 249eb70691b0..cfc876e0b67b 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -963,36 +963,43 @@ bool efi_is_table_address(unsigned long phys_addr)
}
static DEFINE_SPINLOCK(efi_mem_reserve_persistent_lock);
+static struct linux_efi_memreserve *efi_memreserve_root __ro_after_init;
int efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
{
- struct linux_efi_memreserve *rsv, *parent;
+ struct linux_efi_memreserve *rsv;
- if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
+ if (!efi_memreserve_root)
return -ENODEV;
- rsv = kmalloc(sizeof(*rsv), GFP_KERNEL);
+ rsv = kmalloc(sizeof(*rsv), GFP_ATOMIC);
if (!rsv)
return -ENOMEM;
- parent = memremap(efi.mem_reserve, sizeof(*rsv), MEMREMAP_WB);
- if (!parent) {
- kfree(rsv);
- return -ENOMEM;
- }
-
rsv->base = addr;
rsv->size = size;
spin_lock(&efi_mem_reserve_persistent_lock);
- rsv->next = parent->next;
- parent->next = __pa(rsv);
+ rsv->next = efi_memreserve_root->next;
+ efi_memreserve_root->next = __pa(rsv);
spin_unlock(&efi_mem_reserve_persistent_lock);
- memunmap(parent);
+ return 0;
+}
+static int __init efi_memreserve_root_init(void)
+{
+ if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
+ return -ENODEV;
+
+ efi_memreserve_root = memremap(efi.mem_reserve,
+ sizeof(*efi_memreserve_root),
+ MEMREMAP_WB);
+ if (!efi_memreserve_root)
+ return -ENOMEM;
return 0;
}
+early_initcall(efi_memreserve_root_init);
#ifdef CONFIG_KEXEC
static int update_efi_random_seed(struct notifier_block *nb,
--
2.19.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] efi: permit calling efi_mem_reserve_persistent from atomic context
2018-11-08 18:05 [PATCH] efi: permit calling efi_mem_reserve_persistent from atomic context Ard Biesheuvel
@ 2018-11-08 20:48 ` Qian Cai
2018-11-10 2:45 ` Qian Cai
1 sibling, 0 replies; 6+ messages in thread
From: Qian Cai @ 2018-11-08 20:48 UTC (permalink / raw)
To: Ard Biesheuvel, linux-efi
Cc: linux-arm-kernel, linux-mm, linux-kernel, marc.zyngier, will.deacon
On Thu, 2018-11-08 at 19:05 +0100, Ard Biesheuvel wrote:
> Currently, efi_mem_reserve_persistent() may not be called from atomic
> context, since both the kmalloc() call and the memremap() call may
> sleep.
>
> The kmalloc() call is easy enough to fix, but the memremap() call
> needs to be moved into an init hook since we cannot control the
> memory allocation behavior of memremap() at the call site.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Tested-by: Qian Cai <cai@gmx.us>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] efi: permit calling efi_mem_reserve_persistent from atomic context
2018-11-08 18:05 [PATCH] efi: permit calling efi_mem_reserve_persistent from atomic context Ard Biesheuvel
2018-11-08 20:48 ` Qian Cai
@ 2018-11-10 2:45 ` Qian Cai
2018-11-12 2:45 ` Qian Cai
1 sibling, 1 reply; 6+ messages in thread
From: Qian Cai @ 2018-11-10 2:45 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-mm, linux-efi, will.deacon, linux-kernel, Ard Biesheuvel,
marc.zyngier, linux-arm-kernel
On 11/8/18 at 1:05 PM, Ard Biesheuvel wrote:
> Currently, efi_mem_reserve_persistent() may not be called from atomic
> context, since both the kmalloc() call and the memremap() call may
> sleep.
>
> The kmalloc() call is easy enough to fix, but the memremap() call
> needs to be moved into an init hook since we cannot control the
> memory allocation behavior of memremap() at the call site.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> drivers/firmware/efi/efi.c | 31 +++++++++++++++++++------------
> 1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 249eb70691b0..cfc876e0b67b 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -963,36 +963,43 @@ bool efi_is_table_address(unsigned long phys_addr)
> }
>
> static DEFINE_SPINLOCK(efi_mem_reserve_persistent_lock);
> +static struct linux_efi_memreserve *efi_memreserve_root __ro_after_init;
>
> int efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
> {
> - struct linux_efi_memreserve *rsv, *parent;
> + struct linux_efi_memreserve *rsv;
>
> - if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
> + if (!efi_memreserve_root)
> return -ENODEV;
>
> - rsv = kmalloc(sizeof(*rsv), GFP_KERNEL);
> + rsv = kmalloc(sizeof(*rsv), GFP_ATOMIC);
> if (!rsv)
> return -ENOMEM;
>
> - parent = memremap(efi.mem_reserve, sizeof(*rsv), MEMREMAP_WB);
> - if (!parent) {
> - kfree(rsv);
> - return -ENOMEM;
> - }
> -
> rsv->base = addr;
> rsv->size = size;
>
> spin_lock(&efi_mem_reserve_persistent_lock);
> - rsv->next = parent->next;
> - parent->next = __pa(rsv);
> + rsv->next = efi_memreserve_root->next;
> + efi_memreserve_root->next = __pa(rsv);
> spin_unlock(&efi_mem_reserve_persistent_lock);
>
> - memunmap(parent);
> + return 0;
> +}
>
> +static int __init efi_memreserve_root_init(void)
> +{
> + if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
> + return -ENODEV;
> +
> + efi_memreserve_root = memremap(efi.mem_reserve,
> + sizeof(*efi_memreserve_root),
> + MEMREMAP_WB);
> + if (!efi_memreserve_root)
> + return -ENOMEM;
> return 0;
> }
> +early_initcall(efi_memreserve_root_init);
>
> #ifdef CONFIG_KEXEC
> static int update_efi_random_seed(struct notifier_block *nb,
> --
> 2.19.1
BTW, I won’t be able to apply this patch on top of this series [1]. After applied that series, the original BUG sleep from atomic is gone as well as two other GIC warnings. Do you think a new patch is needed here?
[1] https://www.spinics.net/lists/arm-kernel/msg685751.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] efi: permit calling efi_mem_reserve_persistent from atomic context
2018-11-10 2:45 ` Qian Cai
@ 2018-11-12 2:45 ` Qian Cai
2018-11-12 8:32 ` Marc Zyngier
0 siblings, 1 reply; 6+ messages in thread
From: Qian Cai @ 2018-11-12 2:45 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-mm, linux-efi, will.deacon, linux kernel, Marc Zyngier,
linux-arm-kernel
> On Nov 9, 2018, at 9:45 PM, Qian Cai <cai@gmx.us> wrote:
>
>
> On 11/8/18 at 1:05 PM, Ard Biesheuvel wrote:
>
>> Currently, efi_mem_reserve_persistent() may not be called from atomic
>> context, since both the kmalloc() call and the memremap() call may
>> sleep.
>>
>> The kmalloc() call is easy enough to fix, but the memremap() call
>> needs to be moved into an init hook since we cannot control the
>> memory allocation behavior of memremap() at the call site.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> drivers/firmware/efi/efi.c | 31 +++++++++++++++++++------------
>> 1 file changed, 19 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
>> index 249eb70691b0..cfc876e0b67b 100644
>> --- a/drivers/firmware/efi/efi.c
>> +++ b/drivers/firmware/efi/efi.c
>> @@ -963,36 +963,43 @@ bool efi_is_table_address(unsigned long phys_addr)
>> }
>>
>> static DEFINE_SPINLOCK(efi_mem_reserve_persistent_lock);
>> +static struct linux_efi_memreserve *efi_memreserve_root __ro_after_init;
>>
>> int efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
>> {
>> - struct linux_efi_memreserve *rsv, *parent;
>> + struct linux_efi_memreserve *rsv;
>>
>> - if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
>> + if (!efi_memreserve_root)
>> return -ENODEV;
>>
>> - rsv = kmalloc(sizeof(*rsv), GFP_KERNEL);
>> + rsv = kmalloc(sizeof(*rsv), GFP_ATOMIC);
>> if (!rsv)
>> return -ENOMEM;
>>
>> - parent = memremap(efi.mem_reserve, sizeof(*rsv), MEMREMAP_WB);
>> - if (!parent) {
>> - kfree(rsv);
>> - return -ENOMEM;
>> - }
>> -
>> rsv->base = addr;
>> rsv->size = size;
>>
>> spin_lock(&efi_mem_reserve_persistent_lock);
>> - rsv->next = parent->next;
>> - parent->next = __pa(rsv);
>> + rsv->next = efi_memreserve_root->next;
>> + efi_memreserve_root->next = __pa(rsv);
>> spin_unlock(&efi_mem_reserve_persistent_lock);
>>
>> - memunmap(parent);
>> + return 0;
>> +}
>>
>> +static int __init efi_memreserve_root_init(void)
>> +{
>> + if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
>> + return -ENODEV;
>> +
>> + efi_memreserve_root = memremap(efi.mem_reserve,
>> + sizeof(*efi_memreserve_root),
>> + MEMREMAP_WB);
>> + if (!efi_memreserve_root)
>> + return -ENOMEM;
>> return 0;
>> }
>> +early_initcall(efi_memreserve_root_init);
>>
>> #ifdef CONFIG_KEXEC
>> static int update_efi_random_seed(struct notifier_block *nb,
>> --
>> 2.19.1
> BTW, I won’t be able to apply this patch on top of this series [1]. After applied that series, the original BUG sleep from atomic is gone as well as two other GIC warnings. Do you think a new patch is needed here?
>
> [1] https://www.spinics.net/lists/arm-kernel/msg685751.html
OK, I was able to apply this patch on top of latest mainline (ccda4af0f4b9)
which also include one patch (1/6) from the above series,
However, the efi-related patches from the series (4/6, 5/6, and 6/6) are no
longer able to be cleanly applied.
As the results, the above patch did fix the original BUG: sleep from atomic,
but it introduces 2 new warnings.
[ 0.000000] WARNING: CPU: 0 PID: 0 at drivers/irqchip/irq-gic-v3-its.c:1696 its_init+0x494/0x7e8
[ 0.000000] Modules linked in:
[ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.20.0-rc2+ #11
[ 0.000000] pstate: 00000085 (nzcv daIf -PAN -UAO)
[ 0.000000] pc : its_init+0x494/0x7e8
[ 0.000000] lr : its_init+0x490/0x7e8
[ 0.000000] sp : ffff20000a0c7a60
[ 0.000000] x29: ffff20000a0c7a60 x28: 1fffe40001418f6e
[ 0.000000] x27: ffff20000a1061a8 x26: ffff8016c0fb0000
[ 0.000000] x25: ffff20000a3d2300 x24: ffff20000c542000
[ 0.000000] x23: 00000016c0fb0000 x22: ffff20000a1061a8
[ 0.000000] x21: 1fffe40001418f5e x20: ffff20000c542fa0
[ 0.000000] x19: ffff2000091c1be0 x18: 000000000000003f
[ 0.000000] x17: 00000000001bb6ad x16: 0000000000000000
[ 0.000000] x15: 0000000000007fff x14: 6166306336314020
[ 0.000000] x13: 736e6f697463656c x12: ffff1002d81f8000
[ 0.000000] x11: 1ffff002d81f7fff x10: ffff1002d81f7fff
[ 0.000000] x9 : 0000000000000000 x8 : ffff8016c0fc0000
[ 0.000000] x7 : a2a2a2a2a2a2a2a2 x6 : ffff8016c0fbffff
[ 0.000000] x5 : ffff1002d81f8000 x4 : dfff200000000000
[ 0.000000] x3 : 000000000000003f x2 : 000000000000ffff
[ 0.000000] x1 : 0000000000010000 x0 : 00000000ffffffed
[ 0.000000] Call trace:
[ 0.000000] its_init+0x494/0x7e8
[ 0.000000] gic_init_bases+0x2c0/0x2e0
[ 0.000000] gic_acpi_init+0x180/0x2ac
[ 0.000000] acpi_match_madt+0x5c/0x98
[ 0.000000] acpi_table_parse_entries_array+0x1e8/0x2ec
[ 0.000000] acpi_table_parse_entries+0xd8/0x10c
[ 0.000000] acpi_table_parse_madt+0x44/0x54
[ 0.000000] __acpi_probe_device_table+0xa8/0x10c
[ 0.000000] irqchip_init+0x38/0x40
[ 0.000000] init_IRQ+0xa0/0xdc
[ 0.000000] start_kernel+0x3b8/0x5a8
[ 0.000000] irq event stamp: 0
[ 0.000000] hardirqs last enabled at (0): [<0000000000000000>] (null)
[ 0.000000] hardirqs last disabled at (0): [<0000000000000000>] (null)
[ 0.000000] softirqs last enabled at (0): [<0000000000000000>] (null)
[ 0.000000] softirqs last disabled at (0): [<0000000000000000>] (null)
[ 0.000000] ---[ end trace 598902d30712b79b ]---
[ 0.000000] GICv3: using LPI property table @0x00000016c0fb0000
[ 0.000000] ITS: Using DirectLPI for VPE invalidation
[ 0.000000] ITS: Enabling GICv4 support
[ 0.000000] WARNING: CPU: 0 PID: 0 at drivers/irqchip/irq-gic-v3-its.c:2096 its_cpu_init_lpis+0x400/0x428
[ 0.000000] Modules linked in:
[ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.20.0-rc2+ #11
[ 0.000000] pstate: 00000085 (nzcv daIf -PAN -UAO)
[ 0.000000] pc : its_cpu_init_lpis+0x400/0x428
[ 0.000000] lr : its_cpu_init_lpis+0x3fc/0x428
[ 0.000000] sp : ffff20000a0c7a30
[ 0.000000] x29: ffff20000a0c7a30 x28: ffff2000095d8000
[ 0.000000] x27: ffff20000a0f9848 x26: ffff20000c600000
[ 0.000000] x25: ffff2000095d85e0 x24: ffff20000c542000
[ 0.000000] x23: ffff20000a0c7ac0 x22: ffff7fe005b03f00
[ 0.000000] x21: ffff20000c542fa0 x20: 1fffe40001418f54
[ 0.000000] x19: 00000016c0fc0000 x18: 000000000000003f
[ 0.000000] x17: 0000000000000000 x16: ffff20000a10efc0
[ 0.000000] x15: 0000000000007fff x14: ffff20000992701c
[ 0.000000] x13: ffff20000991fafc x12: ffff040001418e0b
[ 0.000000] x11: 1fffe40001418e0a x10: ffff040001418e0a
[ 0.000000] x9 : dfff200000000000 x8 : dfff200000000000
[ 0.000000] x7 : dfff200000000000 x6 : 00000000f2f2f204
[ 0.000000] x5 : 00000000f1f1f1f1 x4 : dfff200000000000
[ 0.000000] x3 : 0000000000000010 x2 : 000000000000ffff
[ 0.000000] x1 : 0000000000010000 x0 : 00000000ffffffed
[ 0.000000] Call trace:
[ 0.000000] its_cpu_init_lpis+0x400/0x428
[ 0.000000] its_cpu_init+0x12c/0x290
[ 0.000000] gic_init_bases+0x2c4/0x2e0
[ 0.000000] gic_acpi_init+0x180/0x2ac
[ 0.000000] acpi_match_madt+0x5c/0x98
[ 0.000000] acpi_table_parse_entries_array+0x1e8/0x2ec
[ 0.000000] acpi_table_parse_entries+0xd8/0x10c
[ 0.000000] acpi_table_parse_madt+0x44/0x54
[ 0.000000] __acpi_probe_device_table+0xa8/0x10c
[ 0.000000] irqchip_init+0x38/0x40
[ 0.000000] init_IRQ+0xa0/0xdc
[ 0.000000] start_kernel+0x3b8/0x5a8
[ 0.000000] irq event stamp: 0
[ 0.000000] hardirqs last enabled at (0): [<0000000000000000>] (null)
[ 0.000000] hardirqs last disabled at (0): [<0000000000000000>] (null)
[ 0.000000] softirqs last enabled at (0): [<0000000000000000>] (null)
[ 0.000000] softirqs last disabled at (0): [<0000000000000000>] (null)
[ 0.000000] ---[ end trace 598902d30712b79c ]---
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] efi: permit calling efi_mem_reserve_persistent from atomic context
2018-11-12 2:45 ` Qian Cai
@ 2018-11-12 8:32 ` Marc Zyngier
2018-11-12 12:14 ` Qian Cai
0 siblings, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2018-11-12 8:32 UTC (permalink / raw)
To: Qian Cai
Cc: Ard Biesheuvel, linux-mm, linux-efi, will.deacon, linux kernel,
linux-arm-kernel
On Mon, 12 Nov 2018 02:45:48 +0000,
Qian Cai <cai@gmx.us> wrote:
>
>
>
> > On Nov 9, 2018, at 9:45 PM, Qian Cai <cai@gmx.us> wrote:
> >
> >
> > On 11/8/18 at 1:05 PM, Ard Biesheuvel wrote:
> >
> >> Currently, efi_mem_reserve_persistent() may not be called from atomic
> >> context, since both the kmalloc() call and the memremap() call may
> >> sleep.
> >>
> >> The kmalloc() call is easy enough to fix, but the memremap() call
> >> needs to be moved into an init hook since we cannot control the
> >> memory allocation behavior of memremap() at the call site.
> >>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >> drivers/firmware/efi/efi.c | 31 +++++++++++++++++++------------
> >> 1 file changed, 19 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> >> index 249eb70691b0..cfc876e0b67b 100644
> >> --- a/drivers/firmware/efi/efi.c
> >> +++ b/drivers/firmware/efi/efi.c
> >> @@ -963,36 +963,43 @@ bool efi_is_table_address(unsigned long phys_addr)
> >> }
> >>
> >> static DEFINE_SPINLOCK(efi_mem_reserve_persistent_lock);
> >> +static struct linux_efi_memreserve *efi_memreserve_root __ro_after_init;
> >>
> >> int efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
> >> {
> >> - struct linux_efi_memreserve *rsv, *parent;
> >> + struct linux_efi_memreserve *rsv;
> >>
> >> - if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
> >> + if (!efi_memreserve_root)
> >> return -ENODEV;
> >>
> >> - rsv = kmalloc(sizeof(*rsv), GFP_KERNEL);
> >> + rsv = kmalloc(sizeof(*rsv), GFP_ATOMIC);
> >> if (!rsv)
> >> return -ENOMEM;
> >>
> >> - parent = memremap(efi.mem_reserve, sizeof(*rsv), MEMREMAP_WB);
> >> - if (!parent) {
> >> - kfree(rsv);
> >> - return -ENOMEM;
> >> - }
> >> -
> >> rsv->base = addr;
> >> rsv->size = size;
> >>
> >> spin_lock(&efi_mem_reserve_persistent_lock);
> >> - rsv->next = parent->next;
> >> - parent->next = __pa(rsv);
> >> + rsv->next = efi_memreserve_root->next;
> >> + efi_memreserve_root->next = __pa(rsv);
> >> spin_unlock(&efi_mem_reserve_persistent_lock);
> >>
> >> - memunmap(parent);
> >> + return 0;
> >> +}
> >>
> >> +static int __init efi_memreserve_root_init(void)
> >> +{
> >> + if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
> >> + return -ENODEV;
> >> +
> >> + efi_memreserve_root = memremap(efi.mem_reserve,
> >> + sizeof(*efi_memreserve_root),
> >> + MEMREMAP_WB);
> >> + if (!efi_memreserve_root)
> >> + return -ENOMEM;
> >> return 0;
> >> }
> >> +early_initcall(efi_memreserve_root_init);
> >>
> >> #ifdef CONFIG_KEXEC
> >> static int update_efi_random_seed(struct notifier_block *nb,
> >> --
> >> 2.19.1
> > BTW, I won’t be able to apply this patch on top of this series [1]. After applied that series, the original BUG sleep from atomic is gone as well as two other GIC warnings. Do you think a new patch is needed here?
> >
> > [1] https://www.spinics.net/lists/arm-kernel/msg685751.html
> OK, I was able to apply this patch on top of latest mainline (ccda4af0f4b9)
> which also include one patch (1/6) from the above series,
>
> However, the efi-related patches from the series (4/6, 5/6, and 6/6) are no
> longer able to be cleanly applied.
>
> As the results, the above patch did fix the original BUG: sleep from atomic,
> but it introduces 2 new warnings.
[...]
These are the warnings you've already reported, aren't they? And we've
established that if you apply the whole series, everything work as
intended at least on the GIC side (the timer issue is a different
story altogether).
Or am I missing something?
Thanks,
M.
--
Jazz is not dead, it just smell funny.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] efi: permit calling efi_mem_reserve_persistent from atomic context
2018-11-12 8:32 ` Marc Zyngier
@ 2018-11-12 12:14 ` Qian Cai
0 siblings, 0 replies; 6+ messages in thread
From: Qian Cai @ 2018-11-12 12:14 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux kernel, Ard Biesheuvel, will.deacon, linux-mm,
linux-arm-kernel, linux-efi
On 11/12/18 at 3:32 AM, Marc Zyngier wrote:
> On Mon, 12 Nov 2018 02:45:48 +0000,
> Qian Cai <cai@gmx.us> wrote:
> >
> >
> >
> > > On Nov 9, 2018, at 9:45 PM, Qian Cai <cai@gmx.us> wrote:
> > >
> > >
> > > On 11/8/18 at 1:05 PM, Ard Biesheuvel wrote:
> > >
> > >> Currently, efi_mem_reserve_persistent() may not be called from atomic
> > >> context, since both the kmalloc() call and the memremap() call may
> > >> sleep.
> > >>
> > >> The kmalloc() call is easy enough to fix, but the memremap() call
> > >> needs to be moved into an init hook since we cannot control the
> > >> memory allocation behavior of memremap() at the call site.
> > >>
> > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > >> ---
> > >> drivers/firmware/efi/efi.c | 31 +++++++++++++++++++------------
> > >> 1 file changed, 19 insertions(+), 12 deletions(-)
> > >>
> > >> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> > >> index 249eb70691b0..cfc876e0b67b 100644
> > >> --- a/drivers/firmware/efi/efi.c
> > >> +++ b/drivers/firmware/efi/efi.c
> > >> @@ -963,36 +963,43 @@ bool efi_is_table_address(unsigned long phys_addr)
> > >> }
> > >>
> > >> static DEFINE_SPINLOCK(efi_mem_reserve_persistent_lock);
> > >> +static struct linux_efi_memreserve *efi_memreserve_root __ro_after_init;
> > >>
> > >> int efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
> > >> {
> > >> - struct linux_efi_memreserve *rsv, *parent;
> > >> + struct linux_efi_memreserve *rsv;
> > >>
> > >> - if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
> > >> + if (!efi_memreserve_root)
> > >> return -ENODEV;
> > >>
> > >> - rsv = kmalloc(sizeof(*rsv), GFP_KERNEL);
> > >> + rsv = kmalloc(sizeof(*rsv), GFP_ATOMIC);
> > >> if (!rsv)
> > >> return -ENOMEM;
> > >>
> > >> - parent = memremap(efi.mem_reserve, sizeof(*rsv), MEMREMAP_WB);
> > >> - if (!parent) {
> > >> - kfree(rsv);
> > >> - return -ENOMEM;
> > >> - }
> > >> -
> > >> rsv->base = addr;
> > >> rsv->size = size;
> > >>
> > >> spin_lock(&efi_mem_reserve_persistent_lock);
> > >> - rsv->next = parent->next;
> > >> - parent->next = __pa(rsv);
> > >> + rsv->next = efi_memreserve_root->next;
> > >> + efi_memreserve_root->next = __pa(rsv);
> > >> spin_unlock(&efi_mem_reserve_persistent_lock);
> > >>
> > >> - memunmap(parent);
> > >> + return 0;
> > >> +}
> > >>
> > >> +static int __init efi_memreserve_root_init(void)
> > >> +{
> > >> + if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
> > >> + return -ENODEV;
> > >> +
> > >> + efi_memreserve_root = memremap(efi.mem_reserve,
> > >> + sizeof(*efi_memreserve_root),
> > >> + MEMREMAP_WB);
> > >> + if (!efi_memreserve_root)
> > >> + return -ENOMEM;
> > >> return 0;
> > >> }
> > >> +early_initcall(efi_memreserve_root_init);
> > >>
> > >> #ifdef CONFIG_KEXEC
> > >> static int update_efi_random_seed(struct notifier_block *nb,
> > >> --
> > >> 2.19.1
> > > BTW, I won’t be able to apply this patch on top of this series [1]. After applied that series, the original BUG sleep from atomic is gone as well as two other GIC warnings. Do you think a new patch is needed here?
> > >
> > > [1] https://www.spinics.net/lists/arm-kernel/msg685751.html
> > OK, I was able to apply this patch on top of latest mainline (ccda4af0f4b9)
> > which also include one patch (1/6) from the above series,
> >
> > However, the efi-related patches from the series (4/6, 5/6, and 6/6) are no
> > longer able to be cleanly applied.
> >
> > As the results, the above patch did fix the original BUG: sleep from atomic,
> > but it introduces 2 new warnings.
>
> [...]
>
> These are the warnings you've already reported, aren't they? And we've
> established that if you apply the whole series, everything work as
> intended at least on the GIC side (the timer issue is a different
> story altogether).
>
> Or am I missing something?
The problem is that I am not able to apply the whole
series alone on top of the latest mainline (rc2) now to
verify it. Also, I won’t be able to apply the series and
this patch together on top of rc1. There are conflicts
between this patch and 4-6 of the series.
Originally, I said those GIC warnings are gone when
testing rc1 + the series, but I am not sure if it is just
dumb luck that also fix BUG: sleep from atomic. Make
sense?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-12 12:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-08 18:05 [PATCH] efi: permit calling efi_mem_reserve_persistent from atomic context Ard Biesheuvel
2018-11-08 20:48 ` Qian Cai
2018-11-10 2:45 ` Qian Cai
2018-11-12 2:45 ` Qian Cai
2018-11-12 8:32 ` Marc Zyngier
2018-11-12 12:14 ` Qian Cai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).