Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code

[Peter Crosthwaite]

On Tue, Dec 17, 2013 at 8:31 PM, Peter Maydell <address@hidden> wrote:
> On 17 December 2013 01:24, Peter Crosthwaite
> <address@hidden> wrote:
>> On Tue, Dec 17, 2013 at 10:58 AM, Peter Maydell
>> <address@hidden> wrote:
>>> On 17 December 2013 00:52, Peter Crosthwaite
>>> <address@hidden> wrote:
>>>> On Fri, Dec 13, 2013 at 8:05 PM, Peter Maydell <address@hidden> wrote:
>>>>> On 13 December 2013 03:19, Peter Crosthwaite
>>>>> <address@hidden> wrote:
>>>>>> Why do we need blobs at all? Cant we just fix arm/boot to directly
>>>>>> setup the CPU state to the desired? Rather than complex blobs that
>>>>>> execute ARM instructions just manipulate the regs directly.
>>>>>
>>>>> We could in theory do this for the primary bootloader, but
>>>>> the secondary CPU blob has to have a loop in it so we
>>>>> can sit around waiting for the guest code running in the
>>>>> primary to tell us it's time to go:
>>>>>
>>>>>>> +static const ARMInsnFixup smpboot[] = {
>>>>>>> +    { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
>>>>>>> +    { 0xe59f0028 }, /* ldr r0, startaddr */
>>>>>>> +    { 0xe3a01001 }, /* mov r1, #1 */
>>>>>>> +    { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
>>>>>>> +    { 0xe3a010ff }, /* mov r1, #0xff */
>>>>>>> +    { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff 
>>>>>>> */
>>>>>>> +    { 0, FIXUP_DSB },   /* dsb */
>>>>>>> +    { 0xe320f003 }, /* wfi */
>>>>>>> +    { 0xe5901000 }, /* ldr     r1, [r0] */
>>>>>>> +    { 0xe1110001 }, /* tst     r1, r1 */
>>>>>>> +    { 0x0afffffb }, /* beq     <wfi> */
>>>>>>> +    { 0xe12fff11 }, /* bx      r1 */
>>>>>>> +    { 0, FIXUP_GIC_CPU_IF },
>>>>
>>>>
>>>> Reading up on Christopher's Kernel documentation link:
>>>>
>>>> Documentation/arm64/booting.txt
>>>> Documentation/arm/Booting
>>>>
>>>> I can't see any reference to GIC, where are these GICisms coming from?
>>>
>>> v7 secondary CPU boot protocol is platform specific,
>>> though most boards seem to do something vaguely
>>> vexpress like.
>>
>> So Zynq is very different. It just rewrites the vector table and
>> resets the secondarys from peripherals rst controller.
>
> Yeah. This is why boot.c supports letting the board provide its
> own secondary boot code string (used by exynos and highbank).
>
>> The kernel expects that we've set the
>>> GIC up so that the primary CPU can do an IPI to get
>>> the secondary out of the holding pen loop (that's the
>>> "wfi / ldr /tst / beq" sequence, which loops waiting
>>> for a CPU interrupt).
>>>
>>
>> It seems a bit board-specific and an obstacle to ultimately sanitizing
>> the embedded bootloaders across the architectures (I hope to one day
>> boot MB and ARM with one BL once I get the arch-isms out of the boot
>> flow).
>>
>> I have another problem while we are on the bootstrap discussion - In
>> Zynq we have some Linux specific bootstrap issues out in device land.
>> Our clock driver expects the bootloader to setup some state:
>>
>> https://github.com/Xilinx/meta-xilinx/blob/master/recipes-devtools/qemu/files/HACK_zynq_slcr_Bring_SLCR_out_of_reset_in_kernel_state.patch
>>
>> This seems similar to the Vexpress GIC requirement - peripherals
>> needing linux specific setup. Can we solve both problems here with a
>> bit or framework allowing devs to have an alternate Linux-specific
>> reset state?
>>
>> Then we can ditch on the machine code too :)
>
> I'm not convinced about this at all -- this would be letting the
> "I know about booting Linux" code spread out from boot.c
> where it currently is into every device. That sounds like a bad idea. -
>

So the "I know about boot linux" code already has to span multiple
architectures it's already reasonably 'spread out' just looking at the
zoo of Linux bootloaders. Its also a contributor to the much disliked
inconsistencies in boot flow between archs.

If we can outsource arch/board specific bringup to device land it
would allow consolidation of all the different Linux bootloaders. Or
at least across embedded - E.g. Microblaze, ARM, some of the board
level PPC bootloaders O.R. and the newly posted Moxie machine.

ARM_CPU / MICROBLAZE_CPU would be responsible for setting dtb reg ptr
/ kernel args, program entry point.
ARM_GIC would set itself up.
ZYNQ_SLCR would set itself up.

QOM interfaces can make this purely additive and unobtrusive. E.G.
interface TYPE_LINUX_RESET implements a reset hook that runs iff the
system is booting Linux.

Anyways,

we are not going to solve this today and at the end of the day, the
patch does make this much more self documenting and flexible for ARM,
so:

Reviewed-by: Peter Crosthwaite <address@hidden>

Regards,
Peter

> -- PMM
>