Re: [PATCH v2 02/31] arm64: Kernel booting and initialisation

[Santosh Shilimkar]

On Tuesday 14 August 2012 11:22 PM, Catalin Marinas wrote:
> The patch adds the kernel booting and the initial setup code.
> Documentation/arm64/booting.txt describes the booting protocol on the
> AArch64 Linux kernel. This is subject to change following the work on
> boot standardisation, ACPI.
>
> Signed-off-by: Will Deacon<will.deacon@xxxxxxx>
> Signed-off-by: Catalin Marinas<catalin.marinas@xxxxxxx>
> ---
>   Documentation/arm64/booting.txt |  141 +++++++++++
>   arch/arm64/include/asm/setup.h  |   26 ++
>   arch/arm64/kernel/head.S        |  521 +++++++++++++++++++++++++++++++++++++++
>   arch/arm64/kernel/setup.c       |  357 +++++++++++++++++++++++++++
>   4 files changed, 1045 insertions(+), 0 deletions(-)
>   create mode 100644 Documentation/arm64/booting.txt
>   create mode 100644 arch/arm64/include/asm/setup.h
>   create mode 100644 arch/arm64/kernel/head.S
>   create mode 100644 arch/arm64/kernel/setup.c
>
> diff --git a/Documentation/arm64/booting.txt b/Documentation/arm64/booting.txt
> new file mode 100644
> index 0000000..3197820
> --- /dev/null
> +++ b/Documentation/arm64/booting.txt

[...]

> +
> +The boot loader is expected to enter the kernel on each CPU in the
> +following manner:
> +
> +- The primary CPU must jump directly to the first instruction of the
> +  kernel image.  The device tree blob passed by this CPU must contain
> +  for each CPU node:
> +
> +    1. An 'enable-method' property. Currently, the only supported value
> +       for this field is the string "spin-table".
> +
> +    2. A 'cpu-release-addr' property identifying a 64-bit,
> +       zero-initialised memory location.
> +
> +  It is expected that the bootloader will generate these device tree
> +  properties and insert them into the blob prior to kernel entry.
> +
> +- Any secondary CPUs must spin outside of the kernel in a reserved area
> +  of memory (communicated to the kernel by a /memreserve/ region in the
> +  device tree) polling their cpu-release-addr location, which must be
> +  contained in the reserved region.  A wfe instruction may be inserted
> +  to reduce the overhead of the busy-loop and a sev will be issued by
> +  the primary CPU.  When a read of the location pointed to by the
> +  cpu-release-addr returns a non-zero value, the CPU must jump directly
> +  to this value.

So you expect all the secondary CPUs to be in wakeup state and probably
looping in WFE for a signal from kernel to boot. There is one issue
with this requirement though. For large CPU system, you need to reset
all the CPUs and hit this waiting loop. This will lead to large inrush
current need at bootup which may be not be supported. To avoid this
issue, secondary CPUs are kept in OFF state and then they are woken
up from kernel one by one whenever they need to be brought into the
system. This requirement should be considered.


> diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h
> new file mode 100644
> index 0000000..d766493
> --- /dev/null
> +++ b/arch/arm64/include/asm/setup.h
> @@ -0,0 +1,26 @@
> +/*
> + * Based on arch/arm/include/asm/setup.h
> + *
> + * Copyright (C) 1997-1999 Russell King
> + * Copyright (C) 2012 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see<http://www.gnu.org/licenses/>.
> + */
> +#ifndef __ASM_SETUP_H
> +#define __ASM_SETUP_H
> +
> +#include<linux/types.h>
> +
> +#define COMMAND_LINE_SIZE 1024
> +
> +#endif
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> new file mode 100644
> index 0000000..34ccdc0
> --- /dev/null
> +++ b/arch/arm64/kernel/head.S
> @@ -0,0 +1,521 @@
> +/*
> + * Low-level CPU initialisation
> + * Based on arch/arm/kernel/head.S
> + *
> + * Copyright (C) 1994-2002 Russell King
> + * Copyright (C) 2003-2012 ARM Ltd.
> + * Authors:	Catalin Marinas<catalin.marinas@xxxxxxx>
> + *		Will Deacon<will.deacon@xxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see<http://www.gnu.org/licenses/>.
> + */
> +

[..]

> +
> +	/*
> +	 * DO NOT MODIFY. Image header expected by Linux boot-loaders.
> +	 */
> +	b	stext				// branch to kernel start, magic
> +	.long	0				// reserved
> +	.quad	TEXT_OFFSET			// Image load offset from start of RAM
> +	.quad	0				// reserved
> +	.quad	0				// reserved
> +

Minor nit. Avoid C++ commenting style "//"  here and rest of the patch.

Regards
santosh

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