Re: [PATCH 07/27] kexec_file: Disable at runtime if securelevel has been set

[Mimi Zohar]

On Thu, 2017-10-26 at 10:17 -0400, Mimi Zohar wrote:
> On Thu, 2017-10-26 at 15:42 +0800, joeyli wrote:
> > Hi Mimi,
> > 
> > Thank you for reviewing.
> > 
> > On Mon, Oct 23, 2017 at 11:54:43AM -0400, Mimi Zohar wrote:
> > > On Thu, 2017-10-19 at 15:51 +0100, David Howells wrote:
> > > > From: Chun-Yi Lee 
> > > > 
> > > > When KEXEC_VERIFY_SIG is not enabled, kernel should not loads image
> > > > through kexec_file systemcall if securelevel has been set.
> > > 
> > > The patch title and description needs to be updated to refer to
> > > lockdown, not securelevel.
> > > 
> > > As previously mentioned the last time these patches were posted, this
> > > leaves out testing to see if the integrity subsystem is enabled.
> > > 
> > > Commit 503ceaef8e2e "ima: define a set of appraisal rules requiring
> > > file signatures" was upstreamed.  An additional patch could force
> > > these rules to be added to the custom policy, if lockdown is enabled.
> > >  This and other patches in this series could then check to see if
> > > is_ima_appraise_enabled() is true.
> > > 
> > > Mimi
> > >
> > 
> > I have updated the patch title and description, and I also added
> > is_ima_appraise_enabled() as the following. Is it good to you?
> 
> Yes, that works.  Thanks!  Remember is_ima_appraise_enabled() is
> dependent on the "ima: require secure_boot rules in lockdown mode"
> patch - http://kernsec.org/pipermail/linux-security-module-archive/201
> 7-October/003910.html.
> 
> The IMA "secure_boot" policy can be specified on the boot command line
> as ima_policy="secure_boot".  It requires kernel modules, firmware,
> kexec kernel image and the IMA custom policy to be signed.  In
> lockdown mode, these rules are enabled by default and added to the
> custom policy.
> 
> > On the other hand, I am not good on IMA. I have traced the code path
> > in kimage_file_prepare_segments(). Looks that the READING_KEXEC_IMAGE
> > doesn't show in selinux_kernel_read_file(). Where is the exact code
> > in IMA for checking the signature when loading crash kernel file?
> 
> kernel_read_file_from_fd() calls the security_kernel_read_file() and
> security_kernel_post_read_file() hooks, which call ima_read_file() and
> ima_post_read_file() respectively.

Hm, with "lockdown" enabled on the boot command line, I'm now able to
do the kexec load, but not the unload.  :/   After the kexec load with
the "--reuse-cmdline" option, the system reboots, but isn't in
"lockdown" mode.

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] EFI fixes

[Ingo Molnar]

Linus,

Please pull the latest efi-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 
efi-urgent-for-linus

   # HEAD: 38fb6652229c2149e8694d57db442878fdf8a1bd efi/libstub/arm: Don't 
randomize runtime regions when CONFIG_HIBERNATION=y

Two fixes: an ARM fix for KASLR interaction with hibernation, plus an efi_test 
crash fix.

 Thanks,

Ingo

-->
Ard Biesheuvel (1):
  efi/libstub/arm: Don't randomize runtime regions when CONFIG_HIBERNATION=y

Dan Carpenter (1):
  efi/efi_test: Prevent an Oops in efi_runtime_query_capsulecaps()


 drivers/firmware/efi/libstub/arm-stub.c | 3 ++-
 drivers/firmware/efi/test/efi_test.c| 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/arm-stub.c 
b/drivers/firmware/efi/libstub/arm-stub.c
index 1cb2d1c070c3..a94601d5939e 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -238,7 +238,8 @@ unsigned long efi_entry(void *handle, efi_system_table_t 
*sys_table,
 
efi_random_get_seed(sys_table);
 
-   if (!nokaslr()) {
+   /* hibernation expects the runtime regions to stay in the same place */
+   if (!IS_ENABLED(CONFIG_HIBERNATION) && !nokaslr()) {
/*
 * Randomize the base of the UEFI runtime services region.
 * Preserve the 2 MB alignment of the region by taking a
diff --git a/drivers/firmware/efi/test/efi_test.c 
b/drivers/firmware/efi/test/efi_test.c
index 08129b7b80ab..41c48a1e8baa 100644
--- a/drivers/firmware/efi/test/efi_test.c
+++ b/drivers/firmware/efi/test/efi_test.c
@@ -593,6 +593,9 @@ static long efi_runtime_query_capsulecaps(unsigned long arg)
if (copy_from_user(, qcaps_user, sizeof(qcaps)))
return -EFAULT;
 
+   if (qcaps.capsule_count == ULONG_MAX)
+   return -EINVAL;
+
capsules = kcalloc(qcaps.capsule_count + 1,
   sizeof(efi_capsule_header_t), GFP_KERNEL);
if (!capsules)
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

[Mimi Zohar]

On Thu, 2017-10-19 at 15:50 +0100, David Howells wrote:
> If the kernel is locked down, require that all modules have valid
> signatures that we can verify.
> 
> Signed-off-by: David Howells 
> ---
> 
>  kernel/module.c |3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index de66ec825992..3d9a3270c179 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2781,7 +2781,8 @@ static int module_sig_check(struct load_info *info, int 
> flags)
>   }
> 
>   /* Not having a signature is only an error if we're strict. */
> - if (err == -ENOKEY && !sig_enforce)
> + if (err == -ENOKEY && !sig_enforce &&
> + !kernel_is_locked_down("Loading of unsigned modules"))
 
This kernel_is_locked_down() check is being called for both the
original and new module_load syscalls.  We need to be able
differentiate them.  This is fine for the original syscall, but for
the new syscall we would need an additional IMA check -
!is_ima_appraise_enabled().

Mimi
 
>   err = 0;
> 
>   return err;

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Matthias Brugger]

On 10/27/2017 05:28 PM, Russell King - ARM Linux wrote:
> On Fri, Oct 27, 2017 at 05:12:58PM +0200, Matthias Brugger wrote:
>> Hi Ard,
>>
>> On 10/22/2017 04:14 PM, Ard Biesheuvel wrote:
>>> ARM shares its EFI stub implementation with arm64, which has some
>>> special handling in the virtual remapping code to
>>> a) make sure that we can map everything even if the OS executes
>>>with 64k page size, and
>>> b) make sure that adjacent regions with the same attributes are not
>>>reordered or moved apart in memory.
>>>
>>> The latter is a workaround for a 'feature' that was shortly recommended
>>> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
>>> it broke many OS installers, including non-Linux ones, and it was never
>>> widely implemented for ARM systems. Before implementing b), the arm64
>>> code simply rounded up all regions to 64 KB granularity, but given that
>>> that results in moving adjacent regions apart, it had to be refined when
>>> b) was implemented.
>>>
>>> The adjacency check requires a sort() pass, due to the fact that the
>>> UEFI spec does not mandate any ordering, and the inclusion of the
>>> lib/sort.c code into the ARM EFI stub is causing some trouble with
>>> the decompressor build due to the fact that its EXPORT_SYMBOL() call
>>> triggers the creation of ksymtab/kcrctab sections.
>>>
>>> So let's simply do away with the adjacency check for ARM, and simply put
>>> all UEFI runtime regions together if they have the same memory attributes.
>>> This is guaranteed to work, given that ARM only supports 4 KB pages,
>>> and allows us to remove the sort() call entirely.
>>>
>>> Signed-off-by: Ard Biesheuvel 
>>> ---
>>>  drivers/firmware/efi/libstub/Makefile   | 6 +++---
>>>  drivers/firmware/efi/libstub/arm-stub.c | 7 +--
>>>  2 files changed, 8 insertions(+), 5 deletions(-)
>>>
>>
>> I thought I already provided my tag, but just in case:
>> This fixes the boot problems I had on the bananapi-r2.
>>
>> Tested-by: Matthias Brugger 
> 
> Please test my "fixes" branch - that has all three patches merged.
> 
> We don't have linux-next running at the moment, so folk need to be
> manually fetching the appropriate git trees for testing.
> 

I gave your branch a shot, feel free to add my:

Tested-by: Matthias Brugger 

Especially for commit
4c2b35b71ea3 ARM: verify size of zImage

Regards,
Matthias
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Matthias Brugger]

On 10/27/2017 05:28 PM, Russell King - ARM Linux wrote:
> On Fri, Oct 27, 2017 at 05:12:58PM +0200, Matthias Brugger wrote:
>> Hi Ard,
>>
>> On 10/22/2017 04:14 PM, Ard Biesheuvel wrote:
>>> ARM shares its EFI stub implementation with arm64, which has some
>>> special handling in the virtual remapping code to
>>> a) make sure that we can map everything even if the OS executes
>>>with 64k page size, and
>>> b) make sure that adjacent regions with the same attributes are not
>>>reordered or moved apart in memory.
>>>
>>> The latter is a workaround for a 'feature' that was shortly recommended
>>> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
>>> it broke many OS installers, including non-Linux ones, and it was never
>>> widely implemented for ARM systems. Before implementing b), the arm64
>>> code simply rounded up all regions to 64 KB granularity, but given that
>>> that results in moving adjacent regions apart, it had to be refined when
>>> b) was implemented.
>>>
>>> The adjacency check requires a sort() pass, due to the fact that the
>>> UEFI spec does not mandate any ordering, and the inclusion of the
>>> lib/sort.c code into the ARM EFI stub is causing some trouble with
>>> the decompressor build due to the fact that its EXPORT_SYMBOL() call
>>> triggers the creation of ksymtab/kcrctab sections.
>>>
>>> So let's simply do away with the adjacency check for ARM, and simply put
>>> all UEFI runtime regions together if they have the same memory attributes.
>>> This is guaranteed to work, given that ARM only supports 4 KB pages,
>>> and allows us to remove the sort() call entirely.
>>>
>>> Signed-off-by: Ard Biesheuvel 
>>> ---
>>>  drivers/firmware/efi/libstub/Makefile   | 6 +++---
>>>  drivers/firmware/efi/libstub/arm-stub.c | 7 +--
>>>  2 files changed, 8 insertions(+), 5 deletions(-)
>>>
>>
>> I thought I already provided my tag, but just in case:
>> This fixes the boot problems I had on the bananapi-r2.
>>
>> Tested-by: Matthias Brugger 
> 
> Please test my "fixes" branch - that has all three patches merged.
> 
> We don't have linux-next running at the moment, so folk need to be
> manually fetching the appropriate git trees for testing.
> 

I gave your branch a shot, feel free to add my:

Tested-by: Matthias Brugger 

Especially for commit
4c2b35b71ea3 ARM: verify size of zImage

Regards,
Matthias
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Russell King - ARM Linux]

On Fri, Oct 27, 2017 at 05:12:58PM +0200, Matthias Brugger wrote:
> Hi Ard,
> 
> On 10/22/2017 04:14 PM, Ard Biesheuvel wrote:
> > ARM shares its EFI stub implementation with arm64, which has some
> > special handling in the virtual remapping code to
> > a) make sure that we can map everything even if the OS executes
> >with 64k page size, and
> > b) make sure that adjacent regions with the same attributes are not
> >reordered or moved apart in memory.
> > 
> > The latter is a workaround for a 'feature' that was shortly recommended
> > by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> > it broke many OS installers, including non-Linux ones, and it was never
> > widely implemented for ARM systems. Before implementing b), the arm64
> > code simply rounded up all regions to 64 KB granularity, but given that
> > that results in moving adjacent regions apart, it had to be refined when
> > b) was implemented.
> > 
> > The adjacency check requires a sort() pass, due to the fact that the
> > UEFI spec does not mandate any ordering, and the inclusion of the
> > lib/sort.c code into the ARM EFI stub is causing some trouble with
> > the decompressor build due to the fact that its EXPORT_SYMBOL() call
> > triggers the creation of ksymtab/kcrctab sections.
> > 
> > So let's simply do away with the adjacency check for ARM, and simply put
> > all UEFI runtime regions together if they have the same memory attributes.
> > This is guaranteed to work, given that ARM only supports 4 KB pages,
> > and allows us to remove the sort() call entirely.
> > 
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  drivers/firmware/efi/libstub/Makefile   | 6 +++---
> >  drivers/firmware/efi/libstub/arm-stub.c | 7 +--
> >  2 files changed, 8 insertions(+), 5 deletions(-)
> > 
> 
> I thought I already provided my tag, but just in case:
> This fixes the boot problems I had on the bananapi-r2.
> 
> Tested-by: Matthias Brugger 

Please test my "fixes" branch - that has all three patches merged.

We don't have linux-next running at the moment, so folk need to be
manually fetching the appropriate git trees for testing.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Matthias Brugger]

Hi Ard,

On 10/22/2017 04:14 PM, Ard Biesheuvel wrote:
> ARM shares its EFI stub implementation with arm64, which has some
> special handling in the virtual remapping code to
> a) make sure that we can map everything even if the OS executes
>with 64k page size, and
> b) make sure that adjacent regions with the same attributes are not
>reordered or moved apart in memory.
> 
> The latter is a workaround for a 'feature' that was shortly recommended
> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> it broke many OS installers, including non-Linux ones, and it was never
> widely implemented for ARM systems. Before implementing b), the arm64
> code simply rounded up all regions to 64 KB granularity, but given that
> that results in moving adjacent regions apart, it had to be refined when
> b) was implemented.
> 
> The adjacency check requires a sort() pass, due to the fact that the
> UEFI spec does not mandate any ordering, and the inclusion of the
> lib/sort.c code into the ARM EFI stub is causing some trouble with
> the decompressor build due to the fact that its EXPORT_SYMBOL() call
> triggers the creation of ksymtab/kcrctab sections.
> 
> So let's simply do away with the adjacency check for ARM, and simply put
> all UEFI runtime regions together if they have the same memory attributes.
> This is guaranteed to work, given that ARM only supports 4 KB pages,
> and allows us to remove the sort() call entirely.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  drivers/firmware/efi/libstub/Makefile   | 6 +++---
>  drivers/firmware/efi/libstub/arm-stub.c | 7 +--
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 

I thought I already provided my tag, but just in case:
This fixes the boot problems I had on the bananapi-r2.

Tested-by: Matthias Brugger 

> diff --git a/drivers/firmware/efi/libstub/Makefile 
> b/drivers/firmware/efi/libstub/Makefile
> index dedf9bde44db..f3e8431565ea 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -33,13 +33,14 @@ lib-y := efi-stub-helper.o 
> gop.o secureboot.o
>  lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
>  
>  # include the stub's generic dependencies from lib/ when building for 
> ARM/arm64
> -arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c 
> sort.c
> +arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> +arm-deps-$(CONFIG_ARM64) += sort.c
>  
>  $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
>   $(call if_changed_rule,cc_o_c)
>  
>  lib-$(CONFIG_EFI_ARMSTUB)+= arm-stub.o fdt.o string.o random.o \
> -$(patsubst %.c,lib-%.o,$(arm-deps))
> +$(patsubst %.c,lib-%.o,$(arm-deps-y))
>  
>  lib-$(CONFIG_ARM)+= arm32-stub.o
>  lib-$(CONFIG_ARM64)  += arm64-stub.o
> @@ -90,5 +91,4 @@ quiet_cmd_stubcopy = STUBCPY $@
>  # explicitly by the decompressor linker script.
>  #
>  STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
> -STUBCOPY_RM-$(CONFIG_ARM)+= -R ___ksymtab+sort -R ___kcrctab+sort
>  STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c 
> b/drivers/firmware/efi/libstub/arm-stub.c
> index 1cb2d1c070c3..3061e4057483 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -349,7 +349,9 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, 
> unsigned long map_size,
>* The easiest way to find adjacent regions is to sort the memory map
>* before traversing it.
>*/
> - sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc, NULL);
> + if (IS_ENABLED(CONFIG_ARM64))
> + sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc,
> +  NULL);
>  
>   for (l = 0; l < map_size; l += desc_size, prev = in) {
>   u64 paddr, size;
> @@ -366,7 +368,8 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, 
> unsigned long map_size,
>* a 4k page size kernel to kexec a 64k page size kernel and
>* vice versa.
>*/
> - if (!regions_are_adjacent(prev, in) ||
> + if ((IS_ENABLED(CONFIG_ARM64) &&
> +  !regions_are_adjacent(prev, in)) ||
>   !regions_have_compatible_memory_type_attrs(prev, in)) {
>  
>   paddr = round_down(in->phys_addr, SZ_64K);
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Gregory CLEMENT]

Hi Ard,
 
 On dim., oct. 22 2017, Ard Biesheuvel  wrote:

> ARM shares its EFI stub implementation with arm64, which has some
> special handling in the virtual remapping code to
> a) make sure that we can map everything even if the OS executes
>with 64k page size, and
> b) make sure that adjacent regions with the same attributes are not
>reordered or moved apart in memory.
>
> The latter is a workaround for a 'feature' that was shortly recommended
> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> it broke many OS installers, including non-Linux ones, and it was never
> widely implemented for ARM systems. Before implementing b), the arm64
> code simply rounded up all regions to 64 KB granularity, but given that
> that results in moving adjacent regions apart, it had to be refined when
> b) was implemented.
>
> The adjacency check requires a sort() pass, due to the fact that the
> UEFI spec does not mandate any ordering, and the inclusion of the
> lib/sort.c code into the ARM EFI stub is causing some trouble with
> the decompressor build due to the fact that its EXPORT_SYMBOL() call
> triggers the creation of ksymtab/kcrctab sections.
>
> So let's simply do away with the adjacency check for ARM, and simply put
> all UEFI runtime regions together if they have the same memory attributes.
> This is guaranteed to work, given that ARM only supports 4 KB pages,
> and allows us to remove the sort() call entirely.
>
> Signed-off-by: Ard Biesheuvel 

I finally managed to test this patch and it fixed my booting issue. It
was just this single patch onto v4.14-rc6.

Tested-by: Gregory CLEMENT 

Thanks,

Gregory


> ---
>  drivers/firmware/efi/libstub/Makefile   | 6 +++---
>  drivers/firmware/efi/libstub/arm-stub.c | 7 +--
>  2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile 
> b/drivers/firmware/efi/libstub/Makefile
> index dedf9bde44db..f3e8431565ea 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -33,13 +33,14 @@ lib-y := efi-stub-helper.o 
> gop.o secureboot.o
>  lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
>  
>  # include the stub's generic dependencies from lib/ when building for 
> ARM/arm64
> -arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c 
> sort.c
> +arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> +arm-deps-$(CONFIG_ARM64) += sort.c
>  
>  $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
>   $(call if_changed_rule,cc_o_c)
>  
>  lib-$(CONFIG_EFI_ARMSTUB)+= arm-stub.o fdt.o string.o random.o \
> -$(patsubst %.c,lib-%.o,$(arm-deps))
> +$(patsubst %.c,lib-%.o,$(arm-deps-y))
>  
>  lib-$(CONFIG_ARM)+= arm32-stub.o
>  lib-$(CONFIG_ARM64)  += arm64-stub.o
> @@ -90,5 +91,4 @@ quiet_cmd_stubcopy = STUBCPY $@
>  # explicitly by the decompressor linker script.
>  #
>  STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
> -STUBCOPY_RM-$(CONFIG_ARM)+= -R ___ksymtab+sort -R ___kcrctab+sort
>  STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c 
> b/drivers/firmware/efi/libstub/arm-stub.c
> index 1cb2d1c070c3..3061e4057483 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -349,7 +349,9 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, 
> unsigned long map_size,
>* The easiest way to find adjacent regions is to sort the memory map
>* before traversing it.
>*/
> - sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc, NULL);
> + if (IS_ENABLED(CONFIG_ARM64))
> + sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc,
> +  NULL);
>  
>   for (l = 0; l < map_size; l += desc_size, prev = in) {
>   u64 paddr, size;
> @@ -366,7 +368,8 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, 
> unsigned long map_size,
>* a 4k page size kernel to kexec a 64k page size kernel and
>* vice versa.
>*/
> - if (!regions_are_adjacent(prev, in) ||
> + if ((IS_ENABLED(CONFIG_ARM64) &&
> +  !regions_are_adjacent(prev, in)) ||
>   !regions_have_compatible_memory_type_attrs(prev, in)) {
>  
>   paddr = round_down(in->phys_addr, SZ_64K);
> -- 
> 2.11.0
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Multiple Acer laptops hang on ACPI poweroff

[Daniel Drake]

On Fri, Oct 27, 2017 at 3:57 PM, Rafael J. Wysocki  wrote:
>> Testing shutdown on Acer Aspire ES1-732 (Intel Apollo Lake N4200) on
>> Linux 4.14-rc6, this issue is still present.
>>
>> The FADT has:
>>
>> [0ACh 0172  12]   PM1A Control Block : [Generic Address Structure]
>> [0ACh 0172   1] Space ID : 01 [SystemIO]
>> [0ADh 0173   1]Bit Width : 10
>> [0AEh 0174   1]   Bit Offset : 00
>> [0AFh 0175   1] Encoded Access Width : 02 [Word Access:16]
>> [0B0h 0176   8]  Address : 0404
>>
>> Full ACPI tables dump:
>> https://gist.github.com/dsd/ed80d9fdd32f99e310002b2492cd6e1b
>>
>> We have tested that writing bit 13 of port 0404 under Windows 10
>> (using an app called RW everything) results in an immediate and
>> successful power down. However, writing the same bit under Linux just
>> makes the system hang.
>>
>> I am not really familiar with the guts of x86 systems. When the OS
>> writes to this port, which component of the system receives that
>> request and acts accordingly? Is it handled by the BIOS? Or an EC, or
>> ...? With more background here we may be able to approach the relevant
>> component vendor and ask for help.
>
> Writes to the PM1A register may go straight to the PMC or trigger an
> SMM trap.  In both cases the platform takes over.

Platform means BIOS/firmware? (i.e. Insyde in this case)
Or you are referring to the Intel SoC?

> Is Apollo Lake the only platform affected or are there any other?

All 3 affected product families are Apollo Lake

Thanks
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Multiple Acer laptops hang on ACPI poweroff

[Rafael J. Wysocki]

On Fri, Oct 27, 2017 at 8:35 AM, Daniel Drake  wrote:
> On Sat, Mar 4, 2017 at 4:15 AM, Daniel Drake  wrote:
>> Some particular Acer/Packard Bell machines hang during shutdown.
>> The system completely hangs while doing bit operations for turning on SLP_EN
>> bit in ACPI PM1A control address and Sleep Control Register. Thus the
>> normal acpi_power_off path can never complete the shutdown process.
>>
>> We have found a workaround to force these systems to use EFI for poweroff,
>> included below, but I wonder if anything better can be done.

Maybe it can, but I need to investigate.

>> It is especially not ideal because the system hangs the same way when going
>> into suspend and we don't have a workaround for that.

It may just not do S3 at all if it shipped with Windows 10.
Suspend-to-idle is the only way to suspend such platforms.

> Testing shutdown on Acer Aspire ES1-732 (Intel Apollo Lake N4200) on
> Linux 4.14-rc6, this issue is still present.
>
> The FADT has:
>
> [0ACh 0172  12]   PM1A Control Block : [Generic Address Structure]
> [0ACh 0172   1] Space ID : 01 [SystemIO]
> [0ADh 0173   1]Bit Width : 10
> [0AEh 0174   1]   Bit Offset : 00
> [0AFh 0175   1] Encoded Access Width : 02 [Word Access:16]
> [0B0h 0176   8]  Address : 0404
>
> Full ACPI tables dump:
> https://gist.github.com/dsd/ed80d9fdd32f99e310002b2492cd6e1b
>
> We have tested that writing bit 13 of port 0404 under Windows 10
> (using an app called RW everything) results in an immediate and
> successful power down. However, writing the same bit under Linux just
> makes the system hang.
>
> I am not really familiar with the guts of x86 systems. When the OS
> writes to this port, which component of the system receives that
> request and acts accordingly? Is it handled by the BIOS? Or an EC, or
> ...? With more background here we may be able to approach the relevant
> component vendor and ask for help.

Writes to the PM1A register may go straight to the PMC or trigger an
SMM trap.  In both cases the platform takes over.

Is Apollo Lake the only platform affected or are there any other?

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Multiple Acer laptops hang on ACPI poweroff

[Daniel Drake]

On Sat, Mar 4, 2017 at 4:15 AM, Daniel Drake  wrote:
> Some particular Acer/Packard Bell machines hang during shutdown.
> The system completely hangs while doing bit operations for turning on SLP_EN
> bit in ACPI PM1A control address and Sleep Control Register. Thus the
> normal acpi_power_off path can never complete the shutdown process.
>
> We have found a workaround to force these systems to use EFI for poweroff,
> included below, but I wonder if anything better can be done. It is especially
> not ideal because the system hangs the same way when going into suspend and
> we don't have a workaround for that.

Testing shutdown on Acer Aspire ES1-732 (Intel Apollo Lake N4200) on
Linux 4.14-rc6, this issue is still present.

The FADT has:

[0ACh 0172  12]   PM1A Control Block : [Generic Address Structure]
[0ACh 0172   1] Space ID : 01 [SystemIO]
[0ADh 0173   1]Bit Width : 10
[0AEh 0174   1]   Bit Offset : 00
[0AFh 0175   1] Encoded Access Width : 02 [Word Access:16]
[0B0h 0176   8]  Address : 0404

Full ACPI tables dump:
https://gist.github.com/dsd/ed80d9fdd32f99e310002b2492cd6e1b

We have tested that writing bit 13 of port 0404 under Windows 10
(using an app called RW everything) results in an immediate and
successful power down. However, writing the same bit under Linux just
makes the system hang.

I am not really familiar with the guts of x86 systems. When the OS
writes to this port, which component of the system receives that
request and acts accordingly? Is it handled by the BIOS? Or an EC, or
...? With more background here we may be able to approach the relevant
component vendor and ask for help.

Searching the web for "pm1a control 0404" I can see at
least a handful of systems with the exact same address here; could
just be a coincidence, or is there some kind of standardization?

Any other debugging suggestions would be very welcome.

Thanks
Daniel


> ---
>  drivers/firmware/efi/reboot.c | 42 +-
>  1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/reboot.c b/drivers/firmware/efi/reboot.c
> index 62ead9b..6d1496d 100644
> --- a/drivers/firmware/efi/reboot.c
> +++ b/drivers/firmware/efi/reboot.c
> @@ -4,6 +4,7 @@
>   */
>  #include 
>  #include 
> +#include 
>
>  int efi_reboot_quirk_mode = -1;
>
> @@ -43,6 +44,45 @@ void efi_reboot(enum reboot_mode reboot_mode, const char 
> *__unused)
> efi.reset_system(efi_mode, EFI_SUCCESS, 0, NULL);
>  }
>
> +static const struct dmi_system_id force_efi_poweroff[] = {
> +{
> +.ident = "Packard Bell Easynote ENLG81AP",
> +.matches = {
> +DMI_MATCH(DMI_SYS_VENDOR, "Packard Bell"),
> +DMI_MATCH(DMI_PRODUCT_NAME, "Easynote ENLG81AP"),
> +},
> +},
> +{
> +.ident = "Packard Bell Easynote ENTE69AP",
> +.matches = {
> +DMI_MATCH(DMI_SYS_VENDOR, "Packard Bell"),
> +DMI_MATCH(DMI_PRODUCT_NAME, "Easynote ENTE69AP"),
> +},
> +},
> +{
> +.ident = "Acer Aspire ES1-533",
> +.matches = {
> +DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
> +DMI_MATCH(DMI_PRODUCT_NAME, "Aspire ES1-533"),
> +},
> +},
> +{
> +.ident = "Acer Aspire ES1-732",
> +.matches = {
> +DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
> +DMI_MATCH(DMI_PRODUCT_NAME, "Aspire ES1-732"),
> +},
> +},
> +{}
> +};
> +
> +bool efi_poweroff_forced(void)
> +{
> +   if (dmi_check_system(force_efi_poweroff))
> +   return true;
> +   return false;
> +}
> +
>  bool __weak efi_poweroff_required(void)
>  {
> return false;
> @@ -58,7 +98,7 @@ static int __init efi_shutdown_init(void)
> if (!efi_enabled(EFI_RUNTIME_SERVICES))
> return -ENODEV;
>
> -   if (efi_poweroff_required())
> +   if (efi_poweroff_required() || efi_poweroff_forced())
> pm_power_off = efi_power_off;
>
> return 0;
> --
> 2.9.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html