Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1246537 > unrolled thread

[PATCHv3 08/11] arm64: Check for selected granule support

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCHv3 08/11] arm64: Check for selected granule support "Suzuki K. Poulose" <suzuki.poulose@arm.com> - 2015-10-14 13:30 +0200
    Re: [PATCHv3 08/11] arm64: Check for selected granule support Mark Rutland <mark.rutland@arm.com> - 2015-10-14 19:30 +0200
      Re: [PATCHv3 08/11] arm64: Check for selected granule support Mark Rutland <mark.rutland@arm.com> - 2015-10-14 19:40 +0200
      Re: [PATCHv3 08/11] arm64: Check for selected granule support "Suzuki K. Poulose" <Suzuki.Poulose@arm.com> - 2015-10-15 11:50 +0200
        Re: [PATCHv3 08/11] arm64: Check for selected granule support Mark Rutland <mark.rutland@arm.com> - 2015-10-15 12:40 +0200
    Re: [PATCHv3 08/11] arm64: Check for selected granule support Jeremy Linton <jeremy.linton@arm.com> - 2015-10-14 23:20 +0200
      Re: [PATCHv3 08/11] arm64: Check for selected granule support "Suzuki K. Poulose" <Suzuki.Poulose@arm.com> - 2015-10-15 11:50 +0200
      Re: [PATCHv3 08/11] arm64: Check for selected granule support Mark Rutland <mark.rutland@arm.com> - 2015-10-15 12:50 +0200
        Re: [PATCHv3 08/11] arm64: Check for selected granule support "Suzuki K. Poulose" <Suzuki.Poulose@arm.com> - 2015-10-15 13:30 +0200
          Re: [PATCHv3 08/11] arm64: Check for selected granule support Mark Rutland <mark.rutland@arm.com> - 2015-10-15 14:40 +0200
            Re: [PATCHv3 08/11] arm64: Check for selected granule support "Suzuki K. Poulose" <Suzuki.Poulose@arm.com> - 2015-10-15 15:00 +0200
              Re: [PATCHv3 08/11] arm64: Check for selected granule support Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2015-10-16 10:10 +0200
          Re: [PATCHv3 08/11] arm64: Check for selected granule support Jeremy Linton <jeremy.linton@arm.com> - 2015-10-15 16:50 +0200
            Re: [PATCHv3 08/11] arm64: Check for selected granule support "Suzuki K. Poulose" <Suzuki.Poulose@arm.com> - 2015-10-15 17:10 +0200
            Re: [PATCHv3 08/11] arm64: Check for selected granule support Mark Rutland <mark.rutland@arm.com> - 2015-10-15 17:20 +0200
              Re: [PATCHv3 08/11] arm64: Check for selected granule support Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2015-10-16 10:20 +0200

#1246537 — [PATCHv3 08/11] arm64: Check for selected granule support

Ensure that the selected page size is supported by the
CPU(s).

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 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(0xd5000000 | 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_NI		0xf
+#define ID_AA64MMFR0_TGRAN64_ON		0x0
+#define ID_AA64MMFR0_TGRAN16_NI		0x0
+#define ID_AA64MMFR0_TGRAN16_ON		0x1
+
 #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 <asm/cputype.h>
 #include <asm/kernel-pgtable.h>
 #include <asm/memory.h>
-#include <asm/thread_info.h>
 #include <asm/pgtable-hwdef.h>
 #include <asm/pgtable.h>
 #include <asm/page.h>
+#include <asm/sysreg.h>
+#include <asm/thread_info.h>
 #include <asm/virt.h>
 
 #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
  */
+#if	defined(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
+	ubfx	x2, 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1247018

> @@ -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_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.

> +
> +#endif
>  	.section	".idmap.text", "ax"
>  __enable_mmu:
> +	mrs	x1, ID_AA64MMFR0_EL1
> +	ubfx	x2, 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247029

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."
> 
> >   */
> > +#if	defined(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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247622

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

>
>>    */
>> +#if	defined(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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247652

> >>+#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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247194

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 <jeremy.linton@arm.com>
---
  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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247623

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 <jeremy.linton@arm.com>
> ---
>   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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247664

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 <jeremy.linton@arm.com>
> ---
>  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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247701

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 <suzuki.poulose@arm.com>
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 <suzuki.poulose@arm.com>
    [ Added a check to EFI stub ]
    Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

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(0xd5000000 | 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_NI		0xf
+#define ID_AA64MMFR0_TGRAN64_ON		0x0
+#define ID_AA64MMFR0_TGRAN16_NI		0x0
+#define ID_AA64MMFR0_TGRAN16_ON		0x1
+
+#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 <linux/efi.h>
 #include <asm/efi.h>
+#include <asm/sysreg.h>
 #include <asm/sections.h>
 
+#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 <asm/cputype.h>
 #include <asm/kernel-pgtable.h>
 #include <asm/memory.h>
-#include <asm/thread_info.h>
 #include <asm/pgtable-hwdef.h>
 #include <asm/pgtable.h>
 #include <asm/page.h>
+#include <asm/sysreg.h>
+#include <asm/thread_info.h>
 #include <asm/virt.h>
 
 #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
+	ubfx	x2, x1, #ID_AA64MMFR0_TGRAN_SHIFT, 4
+	cmp	x2, #ID_AA64MMFR0_TGRAN_SUPPORTED
+	b.ne	__no_granule_support
 	ldr	x5, =vectors
 	msr	vbar_el1, x5
 	msr	ttbr0_el1, x25			// load TTBR0
@@ -634,3 +642,8 @@ __enable_mmu:
 	isb
 	br	x27
 ENDPROC(__enable_mmu)
+
+__no_granule_support:
+	wfe
+	b __no_granule_support
+ENDPROC(__no_granule_support)

----8<----
> 
> Thanks,
> Mark.
> 
> > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> > ---
> >  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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247761

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 <suzuki.poulose@arm.com>
> 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 <suzuki.poulose@arm.com>
>     [ Added a check to EFI stub ]
>     Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

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(0xd5000000 | 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_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 <linux/efi.h>
>  #include <asm/efi.h>
> +#include <asm/sysreg.h>
>  #include <asm/sections.h>

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247775

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 <suzuki.poulose@arm.com>
>> 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 <suzuki.poulose@arm.com>
>>      [ Added a check to EFI stub ]
>>      Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>
> 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_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/.

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 <linux/efi.h>
>>   #include <asm/efi.h>
>> +#include <asm/sysreg.h>
>>   #include <asm/sections.h>
>
> 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248444

On 15 October 2015 at 14:58, Suzuki K. Poulose <Suzuki.Poulose@arm.com> 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 <suzuki.poulose@arm.com>
>>> 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 <suzuki.poulose@arm.com>
>>>      [ Added a check to EFI stub ]
>>>      Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>
>>
>> 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_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/.
>
>
> 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 <linux/efi.h>
>>>   #include <asm/efi.h>
>>> +#include <asm/sysreg.h>
>>>   #include <asm/sections.h>
>>
>>
>> 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247857

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247877

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247881

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248455

On 15 October 2015 at 17:11, Mark Rutland <mark.rutland@arm.com> 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web