Re: [Qemu-devel] [PATCH for-2.5 v1 3/4] arm: highbank: Implement PSCI and dummy monitor

[Peter Maydell]

On 27 October 2015 at 15:11, Peter Crosthwaite
<address@hidden> wrote:
>
>
> On Tue, Oct 27, 2015 at 5:26 AM, Peter Maydell <address@hidden>
> wrote:
>>
>> On 25 October 2015 at 23:13, Peter Crosthwaite
>> <address@hidden> wrote:

>> > +static void hb_write_board_setup(ARMCPU *cpu,
>> > +                                 const struct arm_boot_info *info)
>> > +{
>> > +    int n;
>> > +    uint32_t board_setup_blob[] = {
>> > +        /* Reset */
>> > +        0xe320f000, /* nop */
>> > +        0xe320f000, /* nop */
>> > +        /* smc */
>>
>> It would probably be a good idea to actually have a full vector
>> table here, even if the remaining entries just do "jump off
>> to an infinite-loop error location", so that if the guest is
>> buggy and causes an exception in early bootup before it's got
>> its own vector table set up then we do something noticeable
>
>
> What could we to that is noticeable to the guest?

I just meant "noticeable when you're debugging", like
a tight loop at a very low location in memory.

>> rather than just leaping back into the start of the kernel
>> boot sequence.
>>
>> > +        0xe10f0000, /* mrs r0, CPSR */
>> > +        0xe200001f, /* and r0, r0, #0x1f - mask off mode bits */
>> > +        0xe3500016, /* cmp r0, #0x16 - are we in monitor mode? */
>> > +        /* if (!monitor_mode) { */
>> > +            0x11600070, /* smcne - go to monitor mode */
>> > +            0x112fff1e, /* bxne lr - return to caller */
>> > +        /* } */
>>
>> This seems a bit weird. At the reset entry point we know we're
>> not in monitor mode (will be secure SVC). At the SMC entry point
>> (and indeed every other entry point for MVBAR) we know we are
>> in monitor mode. So why have a runtime check?
>>
>
> It is handling both the initial setup and nopping at the same time. It could
> be done split but this allowed a solutions without a jump table, which is
> hard to maintain in machine code.

You already have to deal with both entry points, that's why you
have a bunch of nops in your code right now.

>>
>> > +        /* do setup from monitor mode */
>> > +        0xe3a00000 + BOARD_SETUP_ADDR, /* mov r0, #BOARD_SETUP_ADDR */
>>
>> This is an unfortunate choice of location for the monitor
>> vector table, because it turns out to be zero, which will
>> also be the initial default for the non-monitor vector table.
>>
>
> Linux can manage this

Yeah, it can, because in general Linux (unless buggy) won't take
any kind of exception until after it's got its own vector table
set up somewhere (usually after MMU init). But it's confusing for
people who are fiddling with Linux's early boot code, and then
they tend to come to us to debug it for them :-)

> so I went this was for the simplicity of an all-in-one
> table. Otherwise I need to have two tables. One to trap the first SMC to 0x8
> which then sets up MVBAR, then another table for the runtime nop.

This confuses me. Surely we should set up MVBAR on initial reset entry
(ie at the start of this little blob of boot specific code),
and SMC is always a runtime nop?

...oh, I see, this is because we start our boot in NS SVC,
because we think we're booting a kernel, which is rather at
cross-purposes with a firmware-ish blob. I'd thought that
we would be entering from reset in Secure SVC, in which case
you don't need to do anything to set up MVBAR.

That's actually a problem for this general scheme of doing
things, because if you don't happen to have spare space at
address 0 to put a vector table for your board-blob then
you have no way to set MVBAR to wherever you do happen to
have free space. Maybe we need to have the board-blob
run in Secure mode (and then drop down to NS either in
the board blob or in the generic bootloader after it
returns from the board blob) ?

thanks
-- PMM