2016-03-06 03:35:35

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel

On 17 February 2016 at 13:36, Matt Fleming <[email protected]> wrote:
> From: Ard Biesheuvel <[email protected]>
>
> A kernel built with support for a page size that is not supported by the
> hardware it runs on cannot boot to a state where it can inform the user
> about it.
>
> If we happen to be booting via UEFI, we can fail gracefully so check
> if the currently configured page size is supported by the hardware before
> entering the kernel proper. Note that UEFI mandates support for 4 KB pages,
> so in that case, no check is needed.
>
> Reviewed-by: Jeremy Linton <[email protected]>
> Tested-by: Suzuki K Poulose <[email protected]>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> Acked-by: Mark Rutland <[email protected]>
> Signed-off-by: Matt Fleming <[email protected]>

Hi Matt,

This patch turned up in -next with 'granule' replaced with 'granular',
both in the commit log and in the patch itself. The term 'granule' is
part of the idiom used by the ARM Architecture Reference Manual, and
so changing it silently to 'granular' is not entirely appropriate here
(although harmless in practice, obviously). In general, I would
appreciate it if in the future, such changes were not made silently
somewhere in the merge pipeline.

Thanks,
Ard.


> ---
> drivers/firmware/efi/libstub/arm64-stub.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> index 9e0342745e4f..aef04ad60e0d 100644
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -12,6 +12,26 @@
> #include <linux/efi.h>
> #include <asm/efi.h>
> #include <asm/sections.h>
> +#include <asm/sysreg.h>
> +
> +efi_status_t check_platform_features(efi_system_table_t *sys_table_arg)
> +{
> + u64 tg;
> +
> + /* UEFI mandates support for 4 KB granule, no need to check */
> + if (IS_ENABLED(CONFIG_ARM64_4K_PAGES))
> + return EFI_SUCCESS;
> +
> + tg = (read_cpuid(ID_AA64MMFR0_EL1) >> ID_AA64MMFR0_TGRAN_SHIFT) & 0xf;
> + if (tg != ID_AA64MMFR0_TGRAN_SUPPORTED) {
> + if (IS_ENABLED(CONFIG_ARM64_64K_PAGES))
> + pr_efi_err(sys_table_arg, "This 64 KB granule kernel is not supported by your CPU\n");
> + else
> + pr_efi_err(sys_table_arg, "This 16 KB granule kernel is not supported by your CPU\n");
> + return EFI_UNSUPPORTED;
> + }
> + return EFI_SUCCESS;
> +}
>
> efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> unsigned long *image_addr,
> --
> 2.6.2
>


2016-03-07 11:02:55

by Matt Fleming

[permalink] [raw]
Subject: Re: [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel

On Sun, 06 Mar, at 04:35:32AM, Ard Biesheuvel wrote:
>
> Hi Matt,
>
> This patch turned up in -next with 'granule' replaced with 'granular',
> both in the commit log and in the patch itself. The term 'granule' is
> part of the idiom used by the ARM Architecture Reference Manual, and
> so changing it silently to 'granular' is not entirely appropriate here
> (although harmless in practice, obviously). In general, I would
> appreciate it if in the future, such changes were not made silently
> somewhere in the merge pipeline.

Sorry about this Ard. I'll make sure this doesn't happen again in
future.

Ingo, is there any chance we can fixup this patch in-place and revert
back to the original wording before it gets to Linus' during the merge
window?

2016-03-07 11:05:54

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel

On 7 March 2016 at 18:02, Matt Fleming <[email protected]> wrote:
> On Sun, 06 Mar, at 04:35:32AM, Ard Biesheuvel wrote:
>>
>> Hi Matt,
>>
>> This patch turned up in -next with 'granule' replaced with 'granular',
>> both in the commit log and in the patch itself. The term 'granule' is
>> part of the idiom used by the ARM Architecture Reference Manual, and
>> so changing it silently to 'granular' is not entirely appropriate here
>> (although harmless in practice, obviously). In general, I would
>> appreciate it if in the future, such changes were not made silently
>> somewhere in the merge pipeline.
>
> Sorry about this Ard. I'll make sure this doesn't happen again in
> future.
>

Thanks

> Ingo, is there any chance we can fixup this patch in-place and revert
> back to the original wording before it gets to Linus' during the merge
> window?

To be honest, I don't care deeply about the commit log, as long as the
code changes are reverted.

Thanks,
Ard.