On 25 October 2015 at 23:13, Peter Crosthwaite
<crosthwaitepe...@gmail.com> wrote:
> Add an API for boards to inject their own preboot software (or
> firmware) seqeuence.
>
> The software then returns to bootloader via the link register. This
> allows boards to do their own little bits of firmware setup without
> needed to replace the bootloader completely (which is the requirement
> for existing firmware support).
>
> The blob is loaded by a callback if and only if doing a linux boot
> (similar to the existing write_secondary support).
>
> Signed-off-by: Peter Crosthwaite <crosthwaite.pe...@gmail.com>
> ---
> Changed since RFC:
> Load blob via firmware.
> Remove un-needed 0-word in bootloader sequence.
> Remove "blob", just use "board setup" consistently
> Remove boolean for (just use a pointer NULL check on write_board_setup)
> Adjust comment about functionality of primary bootloader
Couple of comment tweaks, but otherwise
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
>
> hw/arm/boot.c | 17 ++++++++++++++++-
> include/hw/arm/arm.h | 10 ++++++++++
> 2 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 2a151e2..5640989 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -31,6 +31,7 @@ typedef enum {
> FIXUP_NONE = 0, /* do nothing */
> FIXUP_TERMINATOR, /* end of insns */
> FIXUP_BOARDID, /* overwrite with board ID number */
> + FIXUP_BOARD_SETUP, /* overwrite with board specific setup code address
> */
> FIXUP_ARGPTR, /* overwrite with pointer to kernel args */
> FIXUP_ENTRYPOINT, /* overwrite with kernel entry point */
> FIXUP_GIC_CPU_IF, /* overwrite with GIC CPU interface address */
> @@ -58,8 +59,14 @@ static const ARMInsnFixup bootloader_aarch64[] = {
> { 0, FIXUP_TERMINATOR }
> };
>
> -/* The worlds second smallest bootloader. Set r0-r2, then jump to kernel.
> */
> +/* The worlds second smallest bootloader. Call the board-setup code, Set
> r0-r2,
> + * then jump to kernel.
> + */
While we're editing this comment we might as well fix the grammar
error and remove the claim about size (since we're gradually increasing
it); a note about the fact the first part is not always used is
probably helpful too:
/* A very small bootloader: call the board-setup code (if needed),
* set r0-r2, then jump to the kernel.
* If we're not calling boot setup code then we don't copy across
* the first BOOTLOADER_NO_BOARD_SETUP_OFFSET insns in this array.
*/
> static const ARMInsnFixup bootloader[] = {
> + { 0xe28fe008 }, /* add lr, pc, #8 */
> + { 0xe51ff004 }, /* ldr pc, [pc, #-4] */
> + { 0, FIXUP_BOARD_SETUP },
> +#define BOOTLOADER_NO_BOARD_SETUP_OFFSET 3
> { 0xe3a00000 }, /* mov r0, #0 */
> { 0xe59f1004 }, /* ldr r1, [pc, #4] */
> { 0xe59f2004 }, /* ldr r2, [pc, #4] */
> @@ -131,6 +138,7 @@ static void write_bootloader(const char *name, hwaddr
> addr,
> case FIXUP_NONE:
> break;
> case FIXUP_BOARDID:
> + case FIXUP_BOARD_SETUP:
> case FIXUP_ARGPTR:
> case FIXUP_ENTRYPOINT:
> case FIXUP_GIC_CPU_IF:
> @@ -640,6 +648,9 @@ static void arm_load_kernel_notify(Notifier *notifier,
> void *data)
> elf_machine = EM_AARCH64;
> } else {
> primary_loader = bootloader;
> + if (!info->write_board_setup) {
> + primary_loader += BOOTLOADER_NO_BOARD_SETUP_OFFSET;
> + }
> kernel_load_offset = KERNEL_LOAD_ADDR;
> elf_machine = EM_ARM;
> }
> @@ -745,6 +756,7 @@ static void arm_load_kernel_notify(Notifier *notifier,
> void *data)
> info->initrd_size = initrd_size;
>
> fixupcontext[FIXUP_BOARDID] = info->board_id;
> + fixupcontext[FIXUP_BOARD_SETUP] = info->board_setup_addr;
>
> /* for device tree boot, we pass the DTB directly in r2. Otherwise
> * we point to the kernel args.
> @@ -793,6 +805,9 @@ static void arm_load_kernel_notify(Notifier *notifier,
> void *data)
> if (info->nb_cpus > 1) {
> info->write_secondary_boot(cpu, info);
> }
> + if (info->write_board_setup) {
> + info->write_board_setup(cpu, info);
> + }
>
> /* Notify devices which need to fake up firmware initialization
> * that we're doing a direct kernel boot.
> diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
> index 4dcd4f9..128a54e 100644
> --- a/include/hw/arm/arm.h
> +++ b/include/hw/arm/arm.h
> @@ -87,6 +87,16 @@ struct arm_boot_info {
> * -pflash. It also implies that fw_cfg_find() will succeed.
> */
> bool firmware_loaded;
> +
> + /* Address at which board specific loader/setup code exists. If enabled,
> + * this code-blob will run before anything else. It must return to the
> + * caller via the link register. There is no stack setup. Enabled by
"set up".
> + * defining write_board_setup, which is responsible for loading the blob
> + * to the specified address.
> + */
> + hwaddr board_setup_addr;
> + void (*write_board_setup)(ARMCPU *cpu,
> + const struct arm_boot_info *info);
> };
>
> /**
> --
> 1.9.1
thanks
-- PMM
