Re: [PATCHv3 08/11] arm64: Check for selected granule support
On 15 October 2015 at 17:11, Mark Rutland wrote:
> On Thu, Oct 15, 2015 at 09:47:53AM -0500, Jeremy Linton wrote:
>> On 10/15/2015 06:25 AM, Suzuki K. Poulose wrote:
>> >+/*
>> >+ * Check to see if the CPU supports the requested pagesize
>> >+ */
>> >+asm volatile("mrs %0, ID_AA64MMFR0_EL1" : "=r" (aa64mmfr0_el1));
>> >+aa64mmfr0_el1 >>= ID_AA64MMFR0_TGRAN_SHIFT;
>> >+if ((aa64mmfr0_el1 & 0xf) != ID_AA64MMFR0_TGRAN_SUPPORTED) {
>> >+pr_efi_err(sys_table_arg, PAGE_SIZE_STR" granule not supported
>> >by the CPU\n");
>> >+return EFI_UNSUPPORTED;
>> >+}
>>
>>
>> This is definitely an improvement over my original hack job.
>>
>> I would like to add, that I actually think this should be in a new
>> function "check_kernel_compatibility" (or whatever) that is called
>> before handle_kernel_image.
>
> To bikeshed, perhaps efi_arch_check_system?
>
Yes, that makes sense. But before we add such a function, we should
move all the stub C code to libstub where it will be subject to the
new check against R_AARCH64_ABSxx relocations (which we cannot support
in the stub).
>> That is because I don't really think it belongs in
>> handle_kernel_image which is focused on relocation. Plus, if you add
>> another function, you can avoid the "Failed to relocate kernel"
>> error that comes out following the granule not supported message.
>> Further, checks like this in the future will have a place to live.
>
> I agree.
>
> There are some other diagnostic utilities I'd like to add to the stub
> (e.g. dumping the memory map and ID registers) that would help with
> diagnosing boot issues. I started on those at Connect, but realised I
> needed to first implement half of printf for those to be useful.
>
Yes, printf() is sorely lacking in that context. But note that the
memory map can already be retrieved from the UEFI shell via the
'memmap' command.
>> Of course you will then need a matching stubbed out function for the
>> normal arm kernel as well.
>
> I'm sure there are similar things we'll want to check for 32-bit (e.g.
> LPAE support), but a stub should be fine for now.
>
ARM support is not merged yet, but it is good to keep it in mind.
--
Ard.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 08/11] arm64: Check for selected granule support
On 15 October 2015 at 14:58, Suzuki K. Poulose wrote:
> On 15/10/15 13:37, Mark Rutland wrote:
>>
>> On Thu, Oct 15, 2015 at 12:25:33PM +0100, Suzuki K. Poulose wrote:
>>>
>>> On Thu, Oct 15, 2015 at 11:45:15AM +0100, Mark Rutland wrote:
On Wed, Oct 14, 2015 at 04:13:47PM -0500, Jeremy Linton wrote:
>
> On 10/14/2015 06:20 AM, Suzuki K. Poulose wrote:
>
>
>
>>>
>>> 8>
>>>
>>> Author: Suzuki K. Poulose
>>> Date: Wed Oct 14 11:25:16 2015 +0100
>>>
>>> arm64: Check for selected granule support
>>>
>>> Ensure that the selected page size is supported by the CPU(s). If it
>>> isn't
>>> park the CPU. A check is added to the EFI stub to detect if the boot
>>> CPU
>>> supports the page size, failing which, we fail the boot gracefully,
>>> with
>>> an error message.
>>>
>>> Signed-off-by: Suzuki K. Poulose
>>> [ Added a check to EFI stub ]
>>> Signed-off-by: Jeremy Linton
>>
>>
>> Your sign-off should be last, given you are taking resposibility for
>> Jeremy's patch.
>
>
> OK, I was a bit confused about how it should look like. I kept it on top
> so that I could add [ ] for Jeremy's contribution.
>
>>
>> However, I would prefer that the EFI stub addition were a separate/later
>> patch.
>
>
> OK, that makes sense.
>
>>
>>> diff --git a/arch/arm64/include/asm/sysreg.h
>>> b/arch/arm64/include/asm/sysreg.h
>>> index a7f3d4b..72d814c 100644
>>> --- a/arch/arm64/include/asm/sysreg.h
>>> +++ b/arch/arm64/include/asm/sysreg.h
>
>
>>> +#define ID_AA64MMFR0_TGRAN4_NI 0xf
>>> +#define ID_AA64MMFR0_TGRAN4_ON 0x0
>>> +#define ID_AA64MMFR0_TGRAN64_NI0xf
>>> +#define ID_AA64MMFR0_TGRAN64_ON0x0
>>> +#define ID_AA64MMFR0_TGRAN16_NI0x0
>>> +#define ID_AA64MMFR0_TGRAN16_ON0x1
>>
>>
>> I still don't like "ON" here -- I thought these would also be changed
>> s/ON/SUPPORTED/.
>
>
> I know and I expected that. I have "_ON" in my 'CPU feature' series, which
> will/can be reused here. Hence kept it _ON. I can change it everywhere to
> _SUPPORTED, since I may need to spin another version for that.
>
>
>>> #include
>>> #include
>>> +#include
>>> #include
>>
>>
>> Nit: include order.
>
>
> OK
>
>>
>>>
>>> +#if defined(CONFIG_ARM64_4K_PAGES)
>>> +#define PAGE_SIZE_STR "4K"
>>> +#elif defined(CONFIG_ARM64_64K_PAGES)
>>> +#define PAGE_SIZE_STR "64K"
>>> +#endif
>>> +
4k can be dropped (since UEFI support implies support for 4k pages)
16k is missing here
>>> efi_status_t __init handle_kernel_image(efi_system_table_t
>>> *sys_table_arg,
>>> unsigned long *image_addr,
>>> unsigned long *image_size,
>>> @@ -25,6 +32,17 @@ efi_status_t __init
>>> handle_kernel_image(efi_system_table_t *sys_table_arg,
>>> unsigned long kernel_size, kernel_memsize = 0;
>>> unsigned long nr_pages;
>>> void *old_image_addr = (void *)*image_addr;
>>> + u64 aa64mmfr0_el1;
>>> +
>>> + /*
>>> +* Check to see if the CPU supports the requested pagesize
>>> +*/
... so here you would need a #ifndef CONFIG_ARM_4K_PAGES as well.
>>> + asm volatile("mrs %0, ID_AA64MMFR0_EL1" : "=r" (aa64mmfr0_el1));
>>
>>
>> Can we not use read_cpuid() or similar here?
>
>
> Yes, I will try that out. I didn't want to include additional header-files
> in efi-stub.c.
>
>
>>> + aa64mmfr0_el1 >>= ID_AA64MMFR0_TGRAN_SHIFT;
>>
>>
>> ... and can we not do the shift and mask in one go?
>>
>
>>> + if ((aa64mmfr0_el1 & 0xf) != ID_AA64MMFR0_TGRAN_SUPPORTED) {
>>> + pr_efi_err(sys_table_arg, PAGE_SIZE_STR" granule not
>>> supported by the CPU\n");
>>
>>
>> Nit: space before the first quote, please.
>
>
> Will do
>
> Thanks
> Suzuki
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 08/11] arm64: Check for selected granule support
On Thu, Oct 15, 2015 at 09:47:53AM -0500, Jeremy Linton wrote:
> On 10/15/2015 06:25 AM, Suzuki K. Poulose wrote:
> >+/*
> >+ * Check to see if the CPU supports the requested pagesize
> >+ */
> >+asm volatile("mrs %0, ID_AA64MMFR0_EL1" : "=r" (aa64mmfr0_el1));
> >+aa64mmfr0_el1 >>= ID_AA64MMFR0_TGRAN_SHIFT;
> >+if ((aa64mmfr0_el1 & 0xf) != ID_AA64MMFR0_TGRAN_SUPPORTED) {
> >+pr_efi_err(sys_table_arg, PAGE_SIZE_STR" granule not supported
> >by the CPU\n");
> >+return EFI_UNSUPPORTED;
> >+}
>
>
> This is definitely an improvement over my original hack job.
>
> I would like to add, that I actually think this should be in a new
> function "check_kernel_compatibility" (or whatever) that is called
> before handle_kernel_image.
To bikeshed, perhaps efi_arch_check_system?
> That is because I don't really think it belongs in
> handle_kernel_image which is focused on relocation. Plus, if you add
> another function, you can avoid the "Failed to relocate kernel"
> error that comes out following the granule not supported message.
> Further, checks like this in the future will have a place to live.
I agree.
There are some other diagnostic utilities I'd like to add to the stub
(e.g. dumping the memory map and ID registers) that would help with
diagnosing boot issues. I started on those at Connect, but realised I
needed to first implement half of printf for those to be useful.
> Of course you will then need a matching stubbed out function for the
> normal arm kernel as well.
I'm sure there are similar things we'll want to check for 32-bit (e.g.
LPAE support), but a stub should be fine for now.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 08/11] arm64: Check for selected granule support
On 15/10/15 15:47, Jeremy Linton wrote:
On 10/15/2015 06:25 AM, Suzuki K. Poulose wrote:
+/*
+ * Check to see if the CPU supports the requested pagesize
+ */
+asm volatile("mrs %0, ID_AA64MMFR0_EL1" : "=r" (aa64mmfr0_el1));
+aa64mmfr0_el1 >>= ID_AA64MMFR0_TGRAN_SHIFT;
+if ((aa64mmfr0_el1 & 0xf) != ID_AA64MMFR0_TGRAN_SUPPORTED) {
+pr_efi_err(sys_table_arg, PAGE_SIZE_STR" granule not supported by the
CPU\n");
+return EFI_UNSUPPORTED;
+}
This is definitely an improvement over my original hack job.
I would like to add, that I actually think this should be in a new function
"check_kernel_compatibility" (or whatever) that is called before
handle_kernel_image.
That is because I don't really think it belongs in handle_kernel_image which is
focused on relocation. Plus, if you add another function, you can avoid the
"Failed to relocate kernel" error that comes out following the granule not
supported message.
Further, checks like this in the future will have a place to live.
Of course you will then need a matching stubbed out function for the normal arm
kernel as well.
OK, I will drop it from this series then and can be worked as a separate patch.
Thanks
Suzuki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 08/11] arm64: Check for selected granule support
On 10/15/2015 06:25 AM, Suzuki K. Poulose wrote:
+ /*
+* Check to see if the CPU supports the requested pagesize
+*/
+ asm volatile("mrs %0, ID_AA64MMFR0_EL1" : "=r" (aa64mmfr0_el1));
+ aa64mmfr0_el1 >>= ID_AA64MMFR0_TGRAN_SHIFT;
+ if ((aa64mmfr0_el1 & 0xf) != ID_AA64MMFR0_TGRAN_SUPPORTED) {
+ pr_efi_err(sys_table_arg, PAGE_SIZE_STR" granule not supported by
the CPU\n");
+ return EFI_UNSUPPORTED;
+ }
This is definitely an improvement over my original hack job.
I would like to add, that I actually think this should be in a new
function "check_kernel_compatibility" (or whatever) that is called
before handle_kernel_image.
That is because I don't really think it belongs in handle_kernel_image
which is focused on relocation. Plus, if you add another function, you
can avoid the "Failed to relocate kernel" error that comes out following
the granule not supported message. Further, checks like this in the
future will have a place to live.
Of course you will then need a matching stubbed out function for the
normal arm kernel as well.
Thanks,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 08/11] arm64: Check for selected granule support
On 15/10/15 13:37, Mark Rutland wrote:
On Thu, Oct 15, 2015 at 12:25:33PM +0100, Suzuki K. Poulose wrote:
On Thu, Oct 15, 2015 at 11:45:15AM +0100, Mark Rutland wrote:
On Wed, Oct 14, 2015 at 04:13:47PM -0500, Jeremy Linton wrote:
On 10/14/2015 06:20 AM, Suzuki K. Poulose wrote:
8>
Author: Suzuki K. Poulose
Date: Wed Oct 14 11:25:16 2015 +0100
arm64: Check for selected granule support
Ensure that the selected page size is supported by the CPU(s). If it isn't
park the CPU. A check is added to the EFI stub to detect if the boot CPU
supports the page size, failing which, we fail the boot gracefully, with
an error message.
Signed-off-by: Suzuki K. Poulose
[ Added a check to EFI stub ]
Signed-off-by: Jeremy Linton
Your sign-off should be last, given you are taking resposibility for
Jeremy's patch.
OK, I was a bit confused about how it should look like. I kept it on top
so that I could add [ ] for Jeremy's contribution.
However, I would prefer that the EFI stub addition were a separate/later
patch.
OK, that makes sense.
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index a7f3d4b..72d814c 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
+#define ID_AA64MMFR0_TGRAN4_NI 0xf
+#define ID_AA64MMFR0_TGRAN4_ON 0x0
+#define ID_AA64MMFR0_TGRAN64_NI0xf
+#define ID_AA64MMFR0_TGRAN64_ON0x0
+#define ID_AA64MMFR0_TGRAN16_NI0x0
+#define ID_AA64MMFR0_TGRAN16_ON0x1
I still don't like "ON" here -- I thought these would also be changed
s/ON/SUPPORTED/.
I know and I expected that. I have "_ON" in my 'CPU feature' series, which
will/can be reused here. Hence kept it _ON. I can change it everywhere to
_SUPPORTED, since I may need to spin another version for that.
#include
#include
+#include
#include
Nit: include order.
OK
+#if defined(CONFIG_ARM64_4K_PAGES)
+#define PAGE_SIZE_STR "4K"
+#elif defined(CONFIG_ARM64_64K_PAGES)
+#define PAGE_SIZE_STR "64K"
+#endif
+
efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table_arg,
unsigned long *image_addr,
unsigned long *image_size,
@@ -25,6 +32,17 @@ efi_status_t __init handle_kernel_image(efi_system_table_t
*sys_table_arg,
unsigned long kernel_size, kernel_memsize = 0;
unsigned long nr_pages;
void *old_image_addr = (void *)*image_addr;
+ u64 aa64mmfr0_el1;
+
+ /*
+* Check to see if the CPU supports the requested pagesize
+*/
+ asm volatile("mrs %0, ID_AA64MMFR0_EL1" : "=r" (aa64mmfr0_el1));
Can we not use read_cpuid() or similar here?
Yes, I will try that out. I didn't want to include additional header-files
in efi-stub.c.
+ aa64mmfr0_el1 >>= ID_AA64MMFR0_TGRAN_SHIFT;
... and can we not do the shift and mask in one go?
+ if ((aa64mmfr0_el1 & 0xf) != ID_AA64MMFR0_TGRAN_SUPPORTED) {
+ pr_efi_err(sys_table_arg, PAGE_SIZE_STR" granule not supported by
the CPU\n");
Nit: space before the first quote, please.
Will do
Thanks
Suzuki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 08/11] arm64: Check for selected granule support
On Thu, Oct 15, 2015 at 12:25:33PM +0100, Suzuki K. Poulose wrote:
> On Thu, Oct 15, 2015 at 11:45:15AM +0100, Mark Rutland wrote:
> > On Wed, Oct 14, 2015 at 04:13:47PM -0500, Jeremy Linton wrote:
> > > On 10/14/2015 06:20 AM, Suzuki K. Poulose wrote:
> > >
> > > >+ * Checks if the selected granule size is supported by the CPU.
> > > >+ * If it doesn't park the CPU
> > >
> > > The problem is when you park the boot CPU.
> > >
> > > I think for EFI there is a slightly better error mechanism. This
> > > tweak will print an error and return to the EFI boot manager rather
> > > than hanging the machine without any notification. Now it prints:
> > >
> > > EFI stub: Booting Linux Kernel...
> > > EFI stub: ERROR: 16K granule not supported by this machine
> > > EFI stub: ERROR: Failed to relocate kernel
> > > FS4:\>
> >
> > Neat. We should definitely have checks like this in the stub.
> >
> > However, we still need checks in head.S, given !EFI systems, SMP, and
> > kexec, so this is a complementary mechanism.
>
> Indeed. I meant to add the above check. The updated patch looks like :
>
> 8>
>
> Author: Suzuki K. Poulose
> Date: Wed Oct 14 11:25:16 2015 +0100
>
> arm64: Check for selected granule support
>
> Ensure that the selected page size is supported by the CPU(s). If it isn't
> park the CPU. A check is added to the EFI stub to detect if the boot CPU
> supports the page size, failing which, we fail the boot gracefully, with
> an error message.
>
> Signed-off-by: Suzuki K. Poulose
> [ Added a check to EFI stub ]
> Signed-off-by: Jeremy Linton
Your sign-off should be last, given you are taking resposibility for
Jeremy's patch.
However, I would prefer that the EFI stub addition were a separate/later
patch.
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index a7f3d4b..72d814c 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -44,6 +44,26 @@
> #define SET_PSTATE_PAN(x) __inst_arm(0xd500 | REG_PSTATE_PAN_IMM |\
>(!!x)<<8 | 0x1f)
>
> +
> +#define ID_AA64MMFR0_TGRAN4_SHIFT28
> +#define ID_AA64MMFR0_TGRAN64_SHIFT 24
> +#define ID_AA64MMFR0_TGRAN16_SHIFT 20
> +
> +#define ID_AA64MMFR0_TGRAN4_NI 0xf
> +#define ID_AA64MMFR0_TGRAN4_ON 0x0
> +#define ID_AA64MMFR0_TGRAN64_NI 0xf
> +#define ID_AA64MMFR0_TGRAN64_ON 0x0
> +#define ID_AA64MMFR0_TGRAN16_NI 0x0
> +#define ID_AA64MMFR0_TGRAN16_ON 0x1
I still don't like "ON" here -- I thought these would also be changed
s/ON/SUPPORTED/.
> +
> +#if defined(CONFIG_ARM64_4K_PAGES)
> +#define ID_AA64MMFR0_TGRAN_SHIFT ID_AA64MMFR0_TGRAN4_SHIFT
> +#define ID_AA64MMFR0_TGRAN_SUPPORTED ID_AA64MMFR0_TGRAN4_ON
> +#else
> +#define ID_AA64MMFR0_TGRAN_SHIFT ID_AA64MMFR0_TGRAN64_SHIFT
> +#define ID_AA64MMFR0_TGRAN_SUPPORTED ID_AA64MMFR0_TGRAN64_ON
> +#endif
> +
> #ifdef __ASSEMBLY__
>
> .irp
> num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30
> diff --git a/arch/arm64/kernel/efi-stub.c b/arch/arm64/kernel/efi-stub.c
> index 816120e..77d7de1 100644
> --- a/arch/arm64/kernel/efi-stub.c
> +++ b/arch/arm64/kernel/efi-stub.c
> @@ -11,8 +11,15 @@
> */
> #include
> #include
> +#include
> #include
Nit: include order.
>
> +#if defined(CONFIG_ARM64_4K_PAGES)
> +#define PAGE_SIZE_STR"4K"
> +#elif defined(CONFIG_ARM64_64K_PAGES)
> +#define PAGE_SIZE_STR"64K"
> +#endif
> +
> efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table_arg,
> unsigned long *image_addr,
> unsigned long *image_size,
> @@ -25,6 +32,17 @@ efi_status_t __init handle_kernel_image(efi_system_table_t
> *sys_table_arg,
> unsigned long kernel_size, kernel_memsize = 0;
> unsigned long nr_pages;
> void *old_image_addr = (void *)*image_addr;
> + u64 aa64mmfr0_el1;
> +
> + /*
> + * Check to see if the CPU supports the requested pagesize
> + */
> + asm volatile("mrs %0, ID_AA64MMFR0_EL1" : "=r" (aa64mmfr0_el1));
Can we not use read_cpuid() or similar here?
> + aa64mmfr0_el1 >>= ID_AA64MMFR0_TGRAN_SHIFT;
... and can we not do the shift and mask in one go?
> + if ((aa64mmfr0_el1 & 0xf) != ID_AA64MMFR0_TGRAN_SUPPORTED) {
> + pr_efi_err(sys_table_arg, PAGE_SIZE_STR" granule not supported
> by the CPU\n");
Nit: space before the first quote, please.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 08/11] arm64: Check for selected granule support
On Thu, Oct 15, 2015 at 11:45:15AM +0100, Mark Rutland wrote:
> On Wed, Oct 14, 2015 at 04:13:47PM -0500, Jeremy Linton wrote:
> > On 10/14/2015 06:20 AM, Suzuki K. Poulose wrote:
> >
> > >+ * Checks if the selected granule size is supported by the CPU.
> > >+ * If it doesn't park the CPU
> >
> > The problem is when you park the boot CPU.
> >
> > I think for EFI there is a slightly better error mechanism. This
> > tweak will print an error and return to the EFI boot manager rather
> > than hanging the machine without any notification. Now it prints:
> >
> > EFI stub: Booting Linux Kernel...
> > EFI stub: ERROR: 16K granule not supported by this machine
> > EFI stub: ERROR: Failed to relocate kernel
> > FS4:\>
>
> Neat. We should definitely have checks like this in the stub.
>
> However, we still need checks in head.S, given !EFI systems, SMP, and
> kexec, so this is a complementary mechanism.
Indeed. I meant to add the above check. The updated patch looks like :
8>
Author: Suzuki K. Poulose
Date: Wed Oct 14 11:25:16 2015 +0100
arm64: Check for selected granule support
Ensure that the selected page size is supported by the CPU(s). If it isn't
park the CPU. A check is added to the EFI stub to detect if the boot CPU
supports the page size, failing which, we fail the boot gracefully, with
an error message.
Signed-off-by: Suzuki K. Poulose
[ Added a check to EFI stub ]
Signed-off-by: Jeremy Linton
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index a7f3d4b..72d814c 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -44,6 +44,26 @@
#define SET_PSTATE_PAN(x) __inst_arm(0xd500 | REG_PSTATE_PAN_IMM |\
(!!x)<<8 | 0x1f)
+
+#define ID_AA64MMFR0_TGRAN4_SHIFT 28
+#define ID_AA64MMFR0_TGRAN64_SHIFT 24
+#define ID_AA64MMFR0_TGRAN16_SHIFT 20
+
+#define ID_AA64MMFR0_TGRAN4_NI 0xf
+#define ID_AA64MMFR0_TGRAN4_ON 0x0
+#define ID_AA64MMFR0_TGRAN64_NI0xf
+#define ID_AA64MMFR0_TGRAN64_ON0x0
+#define ID_AA64MMFR0_TGRAN16_NI0x0
+#define ID_AA64MMFR0_TGRAN16_ON0x1
+
+#if defined(CONFIG_ARM64_4K_PAGES)
+#define ID_AA64MMFR0_TGRAN_SHIFT ID_AA64MMFR0_TGRAN4_SHIFT
+#define ID_AA64MMFR0_TGRAN_SUPPORTED ID_AA64MMFR0_TGRAN4_ON
+#else
+#define ID_AA64MMFR0_TGRAN_SHIFT ID_AA64MMFR0_TGRAN64_SHIFT
+#define ID_AA64MMFR0_TGRAN_SUPPORTED ID_AA64MMFR0_TGRAN64_ON
+#endif
+
#ifdef __ASSEMBLY__
.irp
num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30
diff --git a/arch/arm64/kernel/efi-stub.c b/arch/arm64/kernel/efi-stub.c
index 816120e..77d7de1 100644
--- a/arch/arm64/kernel/efi-stub.c
+++ b/arch/arm64/kernel/efi-stub.c
@@ -11,8 +11,15 @@
*/
#include
#include
+#include
#include
+#if defined(CONFIG_ARM64_4K_PAGES)
+#define PAGE_SIZE_STR "4K"
+#elif defined(CONFIG_ARM64_64K_PAGES)
+#define PAGE_SIZE_STR "64K"
+#endif
+
efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table_arg,
unsigned long *image_addr,
unsigned long *image_size,
@@ -25,6 +32,17 @@ efi_status_t __init handle_kernel_image(efi_system_table_t
*sys_table_arg,
unsigned long kernel_size, kernel_memsize = 0;
unsigned long nr_pages;
void *old_image_addr = (void *)*image_addr;
+ u64 aa64mmfr0_el1;
+
+ /*
+* Check to see if the CPU supports the requested pagesize
+*/
+ asm volatile("mrs %0, ID_AA64MMFR0_EL1" : "=r" (aa64mmfr0_el1));
+ aa64mmfr0_el1 >>= ID_AA64MMFR0_TGRAN_SHIFT;
+ if ((aa64mmfr0_el1 & 0xf) != ID_AA64MMFR0_TGRAN_SUPPORTED) {
+ pr_efi_err(sys_table_arg, PAGE_SIZE_STR" granule not supported
by the CPU\n");
+ return EFI_UNSUPPORTED;
+ }
/* Relocate the image, if required. */
kernel_size = _edata - _text;
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 7ace955..514c1cc 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -31,10 +31,11 @@
#include
#include
#include
-#include
#include
#include
#include
+#include
+#include
#include
#define __PHYS_OFFSET (KERNEL_START - TEXT_OFFSET)
@@ -613,10 +614,17 @@ ENDPROC(__secondary_switched)
* x0 = SCTLR_EL1 value for turning on the MMU.
* x27 = *virtual* address to jump to upon completion
*
- * other registers depend on the function called upon completion
+ * Other registers depend on the function called upon completion.
+ *
+ * Checks if the selected granule size is supported by the CPU.
+ * If it isn't, park the CPU
*/
.section".idmap.text", "ax"
__enable_mmu:
+ mrs x1, ID_AA64MMFR0_EL1
+ ubfxx2, x1, #ID_AA64MMFR0_TGRAN_SHIFT, 4
+ cmp x2, #ID_AA64MMFR0_TGRAN_SUPPO
Re: [PATCHv3 08/11] arm64: Check for selected granule support
On Wed, Oct 14, 2015 at 04:13:47PM -0500, Jeremy Linton wrote:
> On 10/14/2015 06:20 AM, Suzuki K. Poulose wrote:
>
> >+ * Checks if the selected granule size is supported by the CPU.
> >+ * If it doesn't park the CPU
>
> The problem is when you park the boot CPU.
>
> I think for EFI there is a slightly better error mechanism. This
> tweak will print an error and return to the EFI boot manager rather
> than hanging the machine without any notification. Now it prints:
>
> EFI stub: Booting Linux Kernel...
> EFI stub: ERROR: 16K granule not supported by this machine
> EFI stub: ERROR: Failed to relocate kernel
> FS4:\>
Neat. We should definitely have checks like this in the stub.
However, we still need checks in head.S, given !EFI systems, SMP, and
kexec, so this is a complementary mechanism.
Thanks,
Mark.
> Signed-off-by: Jeremy Linton
> ---
> arch/arm64/kernel/efi-stub.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm64/kernel/efi-stub.c b/arch/arm64/kernel/efi-stub.c
> index 816120e..90fb868 100644
> --- a/arch/arm64/kernel/efi-stub.c
> +++ b/arch/arm64/kernel/efi-stub.c
> @@ -25,6 +25,20 @@ efi_status_t __init
> handle_kernel_image(efi_system_table_t *sys_table_arg,
> unsigned long kernel_size, kernel_memsize = 0;
> unsigned long nr_pages;
> void *old_image_addr = (void *)*image_addr;
> + u32 aa64mmfr0_el1;
> +
> +#ifdef CONFIG_ARM64_16K_PAGES
> + /*
> +* check to see if this kernel image is
> +* compatible with the current system
> +*/
> + asm volatile("mrs %0, ID_AA64MMFR0_EL1" : "=r" (aa64mmfr0_el1));
> + aa64mmfr0_el1 >>= ID_AA64MMFR0_TGRAN16_SHIFT;
> + if ((aa64mmfr0_el1 & ID_AA64MMFR0_TGRAN4_ON) == 0) {
> + pr_efi_err(sys_table_arg, "16K granule not supported
> by this machine\n");
> + return EFI_UNSUPPORTED;
> + }
> +#endif
>
> /* Relocate the image, if required. */
> kernel_size = _edata - _text;
> --
> 2.4.3
>
>
>
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 08/11] arm64: Check for selected granule support
> >>+#define id_aa64mmfr0_tgran_shift ID_AA64MMFR0_TGRAN64_SHIFT > >>+#define id_aa64mmfr0_tgran_on ID_AA64MMFR0_TGRAN64_ON > >>+ > >>+#else > >>+ > >>+#define id_aa64mmfr0_tgran_shift ID_AA64MMFR0_TGRAN4_SHIFT > >>+#define id_aa64mmfr0_tgran_on ID_AA64MMFR0_TGRAN4_ON > > > >Any reason for not using upper-case names for the macros? > > Nothing in particular. I had them in upper-case in the previous version, > changed it here ;) for absolutely no reason. I could switch it back. Please do! > >Given they're local you could just call them TGRAN_SHIFT and TRGRAN_ON > >to make the asm slightly nicer. > > Given Jeremy's suggestion to add something to the EFI stub, I will retain > the original definition with all upper-case and define it somewhere in > a header so that we can reuse it. Ok, that's also fine by me. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 08/11] arm64: Check for selected granule support
On 14/10/15 22:13, Jeremy Linton wrote:
On 10/14/2015 06:20 AM, Suzuki K. Poulose wrote:
+ * Checks if the selected granule size is supported by the CPU.
+ * If it doesn't park the CPU
The problem is when you park the boot CPU.
I think for EFI there is a slightly better error mechanism. This tweak will
print an
error and return to the EFI boot manager rather than hanging the machine
without any
notification. Now it prints:
EFI stub: Booting Linux Kernel...
EFI stub: ERROR: 16K granule not supported by this machine
EFI stub: ERROR: Failed to relocate kernel
FS4:\>
Nice ! I will pick this up.
Signed-off-by: Jeremy Linton
---
arch/arm64/kernel/efi-stub.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/kernel/efi-stub.c b/arch/arm64/kernel/efi-stub.c
index 816120e..90fb868 100644
--- a/arch/arm64/kernel/efi-stub.c
+++ b/arch/arm64/kernel/efi-stub.c
@@ -25,6 +25,20 @@ efi_status_t __init handle_kernel_image(efi_system_table_t
*sys_table_arg,
unsigned long kernel_size, kernel_memsize = 0;
unsigned long nr_pages;
void *old_image_addr = (void *)*image_addr;
+ u32 aa64mmfr0_el1;
+
+#ifdef CONFIG_ARM64_16K_PAGES
I would prefer to have it on for all page sizes and not just 16K, to be on a
safer side
+ /*
+* check to see if this kernel image is
+* compatible with the current system
+*/
+ asm volatile("mrs %0, ID_AA64MMFR0_EL1" : "=r" (aa64mmfr0_el1));
+ aa64mmfr0_el1 >>= ID_AA64MMFR0_TGRAN16_SHIFT;
+ if ((aa64mmfr0_el1 & ID_AA64MMFR0_TGRAN4_ON) == 0) {
+ pr_efi_err(sys_table_arg, "16K granule not supported by this
machine\n");
+ return EFI_UNSUPPORTED;
+ }
+#endif
Thanks
Suzuki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 08/11] arm64: Check for selected granule support
On 14/10/15 18:24, Mark Rutland wrote: - * other registers depend on the function called upon completion + * Other registers depend on the function called upon completion. + * + * Checks if the selected granule size is supported by the CPU. + * If it doesn't park the CPU Nit: "If it isn't, park the CPU." OK */ +#ifdefined(CONFIG_ARM64_64K_PAGES) + +#define id_aa64mmfr0_tgran_shift ID_AA64MMFR0_TGRAN64_SHIFT +#define id_aa64mmfr0_tgran_on ID_AA64MMFR0_TGRAN64_ON + +#else + +#define id_aa64mmfr0_tgran_shift ID_AA64MMFR0_TGRAN4_SHIFT +#define id_aa64mmfr0_tgran_on ID_AA64MMFR0_TGRAN4_ON Any reason for not using upper-case names for the macros? Nothing in particular. I had them in upper-case in the previous version, changed it here ;) for absolutely no reason. I could switch it back. Given they're local you could just call them TGRAN_SHIFT and TRGRAN_ON to make the asm slightly nicer. Given Jeremy's suggestion to add something to the EFI stub, I will retain the original definition with all upper-case and define it somewhere in a header so that we can reuse it. + +__no_granule_support: + wfe + b __no_granule_support +ENDPROC(__no_granule_support) Other than the above, this loogs fine to me. In future it would be nice if we could somehow signal that these dead CPUs are trapped in the kernel -- we should have some kind of canary mechanism for that. That needn't block this patch, though. Yes, we should. Thanks for the review Suzuki -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 08/11] arm64: Check for selected granule support
On 10/14/2015 06:20 AM, Suzuki K. Poulose wrote:
+ * Checks if the selected granule size is supported by the CPU.
+ * If it doesn't park the CPU
The problem is when you park the boot CPU.
I think for EFI there is a slightly better error mechanism. This tweak
will print an error and return to the EFI boot manager rather than
hanging the machine without any notification. Now it prints:
EFI stub: Booting Linux Kernel...
EFI stub: ERROR: 16K granule not supported by this machine
EFI stub: ERROR: Failed to relocate kernel
FS4:\>
Signed-off-by: Jeremy Linton
---
arch/arm64/kernel/efi-stub.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/kernel/efi-stub.c b/arch/arm64/kernel/efi-stub.c
index 816120e..90fb868 100644
--- a/arch/arm64/kernel/efi-stub.c
+++ b/arch/arm64/kernel/efi-stub.c
@@ -25,6 +25,20 @@ efi_status_t __init
handle_kernel_image(efi_system_table_t *sys_table_arg,
unsigned long kernel_size, kernel_memsize = 0;
unsigned long nr_pages;
void *old_image_addr = (void *)*image_addr;
+ u32 aa64mmfr0_el1;
+
+#ifdef CONFIG_ARM64_16K_PAGES
+ /*
+* check to see if this kernel image is
+* compatible with the current system
+*/
+ asm volatile("mrs %0, ID_AA64MMFR0_EL1" : "=r" (aa64mmfr0_el1));
+ aa64mmfr0_el1 >>= ID_AA64MMFR0_TGRAN16_SHIFT;
+ if ((aa64mmfr0_el1 & ID_AA64MMFR0_TGRAN4_ON) == 0) {
+ pr_efi_err(sys_table_arg, "16K granule not supported by
this machine\n");
+ return EFI_UNSUPPORTED;
+ }
+#endif
/* Relocate the image, if required. */
kernel_size = _edata - _text;
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 08/11] arm64: Check for selected granule support
On Wed, Oct 14, 2015 at 06:24:18PM +0100, Mark Rutland wrote: > > @@ -613,10 +614,28 @@ ENDPROC(__secondary_switched) > > * x0 = SCTLR_EL1 value for turning on the MMU. > > * x27 = *virtual* address to jump to upon completion > > * > > - * other registers depend on the function called upon completion > > + * Other registers depend on the function called upon completion. > > + * > > + * Checks if the selected granule size is supported by the CPU. > > + * If it doesn't park the CPU > > Nit: "If it isn't, park the CPU." > > > */ > > +#ifdefined(CONFIG_ARM64_64K_PAGES) > > + > > +#define id_aa64mmfr0_tgran_shift ID_AA64MMFR0_TGRAN64_SHIFT > > +#define id_aa64mmfr0_tgran_on ID_AA64MMFR0_TGRAN64_ON > > + > > +#else > > + > > +#define id_aa64mmfr0_tgran_shift ID_AA64MMFR0_TGRAN4_SHIFT > > +#define id_aa64mmfr0_tgran_on ID_AA64MMFR0_TGRAN4_ON > > Any reason for not using upper-case names for the macros? > > Given they're local you could just call them TGRAN_SHIFT and TRGRAN_ON > to make the asm slightly nicer. Actually, even better, s/TGRAN_ON/TGRAN_SUPPORTED/ Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3 08/11] arm64: Check for selected granule support
> @@ -613,10 +614,28 @@ ENDPROC(__secondary_switched) > * x0 = SCTLR_EL1 value for turning on the MMU. > * x27 = *virtual* address to jump to upon completion > * > - * other registers depend on the function called upon completion > + * Other registers depend on the function called upon completion. > + * > + * Checks if the selected granule size is supported by the CPU. > + * If it doesn't park the CPU Nit: "If it isn't, park the CPU." > */ > +#if defined(CONFIG_ARM64_64K_PAGES) > + > +#define id_aa64mmfr0_tgran_shift ID_AA64MMFR0_TGRAN64_SHIFT > +#define id_aa64mmfr0_tgran_onID_AA64MMFR0_TGRAN64_ON > + > +#else > + > +#define id_aa64mmfr0_tgran_shift ID_AA64MMFR0_TGRAN4_SHIFT > +#define id_aa64mmfr0_tgran_onID_AA64MMFR0_TGRAN4_ON Any reason for not using upper-case names for the macros? Given they're local you could just call them TGRAN_SHIFT and TRGRAN_ON to make the asm slightly nicer. > + > +#endif > .section".idmap.text", "ax" > __enable_mmu: > + mrs x1, ID_AA64MMFR0_EL1 > + ubfxx2, x1, #id_aa64mmfr0_tgran_shift, 4 > + cmp x2, #id_aa64mmfr0_tgran_on > + b.ne__no_granule_support > ldr x5, =vectors > msr vbar_el1, x5 > msr ttbr0_el1, x25 // load TTBR0 > @@ -634,3 +653,8 @@ __enable_mmu: > isb > br x27 > ENDPROC(__enable_mmu) > + > +__no_granule_support: > + wfe > + b __no_granule_support > +ENDPROC(__no_granule_support) Other than the above, this loogs fine to me. In future it would be nice if we could somehow signal that these dead CPUs are trapped in the kernel -- we should have some kind of canary mechanism for that. That needn't block this patch, though. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv3 08/11] arm64: Check for selected granule support
Ensure that the selected page size is supported by the CPU(s). Cc: Mark Rutland Cc: Catalin Marinas Cc: Will Deacon Signed-off-by: Suzuki K. Poulose Reviewed-by: Ard Biesheuvel Tested-by: Ard Biesheuvel --- arch/arm64/include/asm/sysreg.h | 12 arch/arm64/kernel/head.S| 28 ++-- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index a7f3d4b..1f07cc5 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -44,6 +44,18 @@ #define SET_PSTATE_PAN(x) __inst_arm(0xd500 | REG_PSTATE_PAN_IMM |\ (!!x)<<8 | 0x1f) + +#define ID_AA64MMFR0_TGRAN4_SHIFT 28 +#define ID_AA64MMFR0_TGRAN64_SHIFT 24 +#define ID_AA64MMFR0_TGRAN16_SHIFT 20 + +#define ID_AA64MMFR0_TGRAN4_NI 0xf +#define ID_AA64MMFR0_TGRAN4_ON 0x0 +#define ID_AA64MMFR0_TGRAN64_NI0xf +#define ID_AA64MMFR0_TGRAN64_ON0x0 +#define ID_AA64MMFR0_TGRAN16_NI0x0 +#define ID_AA64MMFR0_TGRAN16_ON0x1 + #ifdef __ASSEMBLY__ .irp num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30 diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index 7ace955..b6aa9e0 100644 --- a/arch/arm64/kernel/head.S +++ b/arch/arm64/kernel/head.S @@ -31,10 +31,11 @@ #include #include #include -#include #include #include #include +#include +#include #include #define __PHYS_OFFSET (KERNEL_START - TEXT_OFFSET) @@ -613,10 +614,28 @@ ENDPROC(__secondary_switched) * x0 = SCTLR_EL1 value for turning on the MMU. * x27 = *virtual* address to jump to upon completion * - * other registers depend on the function called upon completion + * Other registers depend on the function called upon completion. + * + * Checks if the selected granule size is supported by the CPU. + * If it doesn't park the CPU */ +#ifdefined(CONFIG_ARM64_64K_PAGES) + +#define id_aa64mmfr0_tgran_shift ID_AA64MMFR0_TGRAN64_SHIFT +#define id_aa64mmfr0_tgran_on ID_AA64MMFR0_TGRAN64_ON + +#else + +#define id_aa64mmfr0_tgran_shift ID_AA64MMFR0_TGRAN4_SHIFT +#define id_aa64mmfr0_tgran_on ID_AA64MMFR0_TGRAN4_ON + +#endif .section".idmap.text", "ax" __enable_mmu: + mrs x1, ID_AA64MMFR0_EL1 + ubfxx2, x1, #id_aa64mmfr0_tgran_shift, 4 + cmp x2, #id_aa64mmfr0_tgran_on + b.ne__no_granule_support ldr x5, =vectors msr vbar_el1, x5 msr ttbr0_el1, x25 // load TTBR0 @@ -634,3 +653,8 @@ __enable_mmu: isb br x27 ENDPROC(__enable_mmu) + +__no_granule_support: + wfe + b __no_granule_support +ENDPROC(__no_granule_support) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
