> From: Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com]
> Sent: Monday, 11 January 2016 19:58
[...]
> > +static void write_board_setup(ARMCPU *cpu, const struct arm_boot_info
> *info)
>
> This is almost identical to Highbank, I'm guessing you are stubbing monitor
> firmware where you get away with nopping all the SMCs? Maybe we should
> split
> Highbanks version off to common code, and parameterise the few
> differences.
>
> write_board_setup_dummy_monitior(ARMCPU *cpu ..., uint32_t scr_flags);
>
> or something like it. Makes sense to be in arm/boot.c .
Actually, I added this only to make Linux happy (and yes, it was derived from
highbank). Without it, I was seeing complaints about:
Ignoring attempt to switch CPSR_A flag from non-secure world with SCR.AW bit
clear
Ignoring attempt to switch CPSR_F flag from non-secure world with SCR.FW bit
clear
I don't believe anything actually uses the SMC handler after boot. I think it's
just an architectural requirement to flush the change to non-secure mode.
I would prefer not to touch highbank, because I don't know how to test it. Is
it better to submit this patch without the board setup?
>
> > +{
> > + static const uint32_t board_setup[] = {
> > + /* MVBAR_ADDR: secure monitor vectors */
> > + 0xEAFFFFFE, /* (spin) */
> > + 0xEAFFFFFE, /* (spin) */
> > + 0xE1B0F00E, /* movs pc, lr ;SMC exception return */
> > + 0xEAFFFFFE, /* (spin) */
> > + 0xEAFFFFFE, /* (spin) */
> > + 0xEAFFFFFE, /* (spin) */
> > + 0xEAFFFFFE, /* (spin) */
> > + 0xEAFFFFFE, /* (spin) */
> > + /* BOARDSETUP_ADDR */
> > + 0xE3A00B01, /* mov r0, #0x400 ;MVBAR_ADDR */
> > + 0xEE0C0F30, /* mcr p15, 0, r0, c12, c0, 1 ;set MVBAR */
> > + 0xE3A00031, /* mov r0, #0x31 ;enable AW, FW, NS */
> > + 0xEE010F11, /* mcr p15, 0, r0, c1, c1, 0 ;write SCR */
>
> If combining with HB, could you do this as read-modify-write?
>
> > + 0xE1A0100E, /* mov r1, lr ;save LR across SMC
> > */
> > + 0xE1600070, /* smc #0 ;monitor call */
> > + 0xE1A0F001, /* mov pc, r1 ;return */
>
> I'm looking at the Highbank version which doesn't save lr across the SMC and
> wondering why it doesn't. Is this banked by CPU mode and do you get from
> already-in-monitor-mode? Or simply, that Highbank code may have a bug?
I think it's needed because I call the board setup blob on each core (from the
smpboot blob), but highbank doesn't. I found that I needed to do this to avoid
the above warnings on an SMP boot; I don't know why highbank doesn't.
[...]
> > + /* Allocate and map RAM */
> > + memory_region_allocate_system_memory(&s->ram,
> OBJECT(machine), "ram",
> > + machine->ram_size);
> > + memory_region_add_subregion_overlap(get_system_memory(), 0,
> &s->ram, 0);
>
> I thought the SoC handled this now? Why do you need to add to
> system_memory?
If I don't map it here, how do RAM accesses from the CPUs work?
Or are you saying that I should still do this, but do it in the SoC not the
board level?
[...]
> > +static void raspi2_machine_init(MachineClass *mc)
> > +{
> > + mc->desc = "Raspberry Pi 2";
> > + mc->init = raspi2_init;
> > + mc->block_default_type = IF_SD;
>
> > + mc->no_parallel = 1;
> > + mc->no_floppy = 1;
> > + mc->no_cdrom = 1;
>
> Curious, what do these do from a user-visible point of view? Maybe we
> should
> add them to more ARM boards as they certainly make sense.
I think they turn off some redundant stuff in the UI (e.g., there's no
View->Parallel menu option). I'm guessing they also disable the -cdrom and
-fd* options, but didn't test that.
Cheers,
Andrew
