On Tue, Oct 27, 2015 at 5:19 AM, Peter Maydell <peter.mayd...@linaro.org>
wrote:
> 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>
>
>
Thanks.
> >
> > 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.
> */
>
>
Will take verbatim.
> > 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".
>
>
Will fix.
Regards,
Peter
> > + * 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
>
