[PATCH 3/3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[mark.rutland@arm.com (Mark Rutland)]

On Tue, Mar 17, 2015 at 10:11:14AM +0000, Ard Biesheuvel wrote:
> According to the arm64 boot protocol, registers x1 to x3 should be
> zero upon kernel entry, and non-zero values are reserved for future
> use. This future use is going to be problematic if we never enforce
> the current rules, so start enforcing them now, by emitting a warning
> if non-zero values are detected.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/kernel/head.S  |  8 ++++++++
>  arch/arm64/kernel/setup.c | 13 +++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 1651c0fd50e6..fe5354eae069 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -231,6 +231,10 @@ section_table:
>  #endif
>  
>  ENTRY(stext)
> +	adr	x8, boot_regs			// record the contents of
> +	stp	x0, x1, [x8]			// x0 .. x3 at kernel entry
> +	stp	x2, x3, [x8, #16]

These accesses will be non-cacheable, but the caches could have clean
entries for this PA range.

Without an invalidate between the writes and reads, we might see stale
values (the other option is to preserve the registers across enabling
the MMU).

> +
>  	mov	x21, x0				// x21=FDT
>  	bl	el2_setup			// Drop to EL1, w20=cpu_boot_mode
>  	adrp	x24, KERNEL_START - TEXT_OFFSET	// x24=PHYS_OFFSET
> @@ -251,6 +255,10 @@ ENTRY(stext)
>  	b	__cpu_setup			// initialise processor
>  ENDPROC(stext)
>  
> +	.align	3
> +ENTRY(boot_regs)
> +	.skip	4 * 8				// x0 .. x3

Can't we can drop this here...

> +
>  /*
>   * Determine validity of the x21 FDT pointer.
>   * The dtb must be 8-byte aligned and live in the first 512M of memory.
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 6c5fb5aff325..2b81d0a907ce 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -532,3 +532,16 @@ const struct seq_operations cpuinfo_op = {
>  	.stop	= c_stop,
>  	.show	= c_show
>  };

... and here have

u64 boot_regs[4];

Using adrp + lo12 to address the symbol?

> +
> +static int verify_boot_protocol(void)
> +{
> +	extern u64 boot_regs[];
> +
> +	if (boot_regs[1] || boot_regs[2] || boot_regs[3]) {
> +		pr_err("WARNING: boot protocol violation detected (x1 == %llx, x2 == %llx, x3 == %llx)\n",
> +			boot_regs[1], boot_regs[2], boot_regs[3]);
> +		pr_err("WARNING: your bootloader may fail to load newer kernels\n");
> +	}
> +	return 0;
> +}
> +late_initcall(verify_boot_protocol);

I think it would be nicer to plumb this into setup_arch rather than an
initcall. That way the warning will be at a consistent, prominent
location in the log.

Thanks,
Mark.