xen: arm: beginning the removal of mode_switch.S

xen: arm: beginning the removal of mode_switch.S

[Ian Campbell]

I did some hacking on boot-wrapper.git on the train to debconf and made
it support building a zImage container encapsulating Xen+Linux+initramfs
+fdt. Xen is optional so it can be used to boot natively too.

You can find the code in the multiplatform branch of
http://xenbits.xen.org/gitweb/?p=people/ianc/boot-wrapper.git

It has build time (Kconfig driven) options to support:
      * cubieboard2 (boots native ok, weird issue under Xen)
      * arndale (code taken from existing mode switch.S, untested)
      * vexpress and fastmodel (untested)

The code is pretty hacked up from the original (which only really
supported fastmodels, and had limited configurability) and it could
certainly be structured to be quite a bit cleaner (plus I think I got a
bit carried away with using Kconfig for everything). I'd rather have
some skanky hacked up code here than in Xen though, so I think this is
an acceptable level of hackedupness. 

At the moment it is sufficient to allow us to do away with the
enter_hyp_mode bits and the clock frequency, gic setup etc, along the
lines of the patch below.

It doesn't yet allow us to get rid of the kick_cpus stuff. My plan for
platforms which don't do the right thing here would be to add a
mechanism to use dtb /memreserve/ (and teach Xen about that construct)
to carve out a little bit of memory which secondary CPUs could safely be
left spinning in. The platform code would expose its normal interface
(e.g. SYS_FLAGS on vexpress and fastmodel), eventually maybe we'd do
PSCI too (which might let us skip reserving some memory since 2ndary
cpus would be in secure mode and could use the special ram regions
reserved for that)

I might have time for this on the train on the way home, but since my
cubieboard2 can't do SMP yet (even on native Linux, bringup looks
complex) I suppose that means I need to test and debug the fastmodel
support first...

As we add new platforms I think we should first push back on the vendors
to fix their firmware but when that turns out to not be possible we
should move to patching this code with platform hacks instead of adding
more stuff to mode_switch.S, IMO the only blocker to this is the
kick_cpu support.

What does everyone think?

8<------------

commit 487c0de6b1d541a9706ebd18c12e3981328a32af
Author: Ian Campbell <ian.campbell@citrix.com>
Date:   Tue Aug 13 18:20:21 2013 +0100

    xen: arm: Remove enter_hyp_mode from mode_switch.S
    
    bootwrapper.git can cope with this on both the platforms included here.
    
    mode_switch.S isn't quite dead yet -- need to handle kick_cpus.
    
    Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 1c9bdaf..1b75f65 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -168,10 +168,10 @@ boot_cpu:
         b     fail
 1:
         /* OK, we're in Secure state. */
-        PRINT("- Started in Secure state -\r\n- Entering Hyp mode -\r\n")
-        ldr   r0, =enter_hyp_mode    /* VA of function */
-        adr   lr, hyp                /* Set return address for call */
-        add   pc, r0, r10            /* Call PA of function */
+        PRINT("- Started in Secure state -\r\n" \
+              "- Xen must be entered in Hyp mode -\r\n" \
+              "- Try using bootwrapper? -\r\n")
+        b     fail
 
 hyp:
 
diff --git a/xen/arch/arm/arm32/mode_switch.S b/xen/arch/arm/arm32/mode_switch.S
index 3500eb0..89e80f1 100644
--- a/xen/arch/arm/arm32/mode_switch.S
+++ b/xen/arch/arm/arm32/mode_switch.S
@@ -68,91 +68,6 @@ kick_cpus_sgi:
         mov   pc, lr
 
 
-/* Get up a CPU into Hyp mode.  Clobbers r0-r3.
- *
- * r5: Machine ID
- * r12: CPU number
- *
- * This code is specific to the VE model/Arndale, and not intended to be used
- * on production systems.  As such it's a bit hackier than the main
- * boot code in head.S.  In future it will be replaced by better
- * integration with the bootloader/firmware so that Xen always starts
- * in Hyp mode.
- * Clobber r0 - r4 */
-
-.globl enter_hyp_mode
-enter_hyp_mode:
-        mov   r3, lr                 /* Put return address in non-banked reg */
-        cpsid aif, #0x16             /* Enter Monitor mode */
-        mrc   CP32(r0, SCR)
-        orr   r0, r0, #0x100         /* Set HCE */
-        orr   r0, r0, #0xb1          /* Set SCD, AW, FW and NS */
-        bic   r0, r0, #0xe           /* Clear EA, FIQ and IRQ */
-        mcr   CP32(r0, SCR)
-
-        ldr   r2, =MACH_TYPE_SMDK5250   /* r4 := Arndale machine ID */
-        /* By default load Arndale defaults values */
-        ldr   r0, =EXYNOS5_TIMER_FREQUENCY  /* r0 := timer's frequency */
-        ldr   r1, =EXYNOS5_GIC_BASE_ADDRESS /* r1 := GIC base address */
-        /* If it's not the Arndale machine ID, load VE values */
-        teq   r5, r2
-        ldrne r0, =V2M_TIMER_FREQUENCY
-        ldrne r1, =V2M_GIC_BASE_ADDRESS
-
-        /* Ugly: the system timer's frequency register is only
-         * programmable in Secure state.  Since we don't know where its
-         * memory-mapped control registers live, we can't find out the
-         * right frequency. */
-        mcr   CP32(r0, CNTFRQ)
-
-        mrc   CP32(r0,NSACR)
-        ldr   r4, =0x3fff            /* Allow access to all co-processors in NS mode */
-        orr   r0, r0, r4
-        orr   r0, r0, #(1<<18)       /* CA7/CA15: Allow access to ACTLR.SMP in NS mode */
-        mcr   CP32(r0, NSACR)
-
-        add   r0, r1, #GIC_DR_OFFSET
-        /* Disable the GIC distributor, on the boot CPU only */
-        mov   r4, #0
-        teq   r12, #0                /* Is this the boot CPU? */
-        streq r4, [r0]
-        /* Continuing ugliness: Set up the GIC so NS state owns interrupts,
-         * The first 32 interrupts (SGIs & PPIs) must be configured on all
-         * CPUs while the remainder are SPIs and only need to be done one, on
-         * the boot CPU. */
-        add   r0, r0, #0x80          /* GICD_IGROUP0 */
-        mov   r2, #0xffffffff        /* All interrupts to group 1 */
-        str   r2, [r0]               /* Interrupts  0-31 (SGI & PPI) */
-        teq   r12, #0                /* Boot CPU? */
-        bne   skip_spis              /* Don't route SPIs on secondary CPUs */
-
-        add   r4, r1, #GIC_DR_OFFSET
-        ldr   r4, [r4, #4]            /* r4 := Interrupt Controller Type Reg */
-        and   r4, r4, #GICD_TYPE_LINES /* r4 := number of SPIs */
-1:      teq   r4, #0
-        beq   skip_spis
-        add   r0, r0, #4             /* Go to the new group */
-        str   r2, [r0]               /* Update the group */
-        sub  r4, r4, #1
-        b     1b
-skip_spis:
-        /* Disable the GIC CPU interface on all processors */
-        add   r0, r1, #GIC_CR_OFFSET
-        mov   r1, #0
-        str   r1, [r0]
-        /* Must drop priority mask below 0x80 before entering NS state */
-        ldr   r1, =0xff
-        str   r1, [r0, #0x4]         /* -> GICC_PMR */
-        /* Reset a few config registers */
-        mov   r0, #0
-        mcr   CP32(r0, FCSEIDR)
-        mcr   CP32(r0, CONTEXTIDR)
-
-        mrs   r0, cpsr               /* Copy the CPSR */
-        add   r0, r0, #0x4           /* 0x16 (Monitor) -> 0x1a (Hyp) */
-        msr   spsr_cxsf, r0          /* into the SPSR */
-        movs  pc, r3                 /* Exception-return into Hyp mode */
-
 /*
  * Local variables:
  * mode: ASM
diff --git a/xen/include/asm-arm/platforms/exynos5.h b/xen/include/asm-arm/platforms/exynos5.h
index ee5bdfa..4c414b3 100644
--- a/xen/include/asm-arm/platforms/exynos5.h
+++ b/xen/include/asm-arm/platforms/exynos5.h
@@ -20,9 +20,6 @@
 /* GIC Base Address */
 #define EXYNOS5_GIC_BASE_ADDRESS    0x10480000
 
-/* Timer's frequency */
-#define EXYNOS5_TIMER_FREQUENCY     (24 * 1000 * 1000) /* 24 MHz */
-
 /* Arndale machine ID */
 #define MACH_TYPE_SMDK5250          3774
 
diff --git a/xen/include/asm-arm/platforms/vexpress.h b/xen/include/asm-arm/platforms/vexpress.h
index 982a293..1ffafee 100644
--- a/xen/include/asm-arm/platforms/vexpress.h
+++ b/xen/include/asm-arm/platforms/vexpress.h
@@ -38,9 +38,6 @@ int vexpress_syscfg(int write, int function, int device, uint32_t *data);
 /* GIC base address */
 #define V2M_GIC_BASE_ADDRESS        0x2c000000
 
-/* Timer's frequency */
-#define V2M_TIMER_FREQUENCY         0x5f5e100 /* 100 Mhz */
-
 #endif /* __ASSEMBLY__ */
 
 #endif /* __ASM_ARM_PLATFORMS_VEXPRESS_H */

Re: xen: arm: beginning the removal of mode_switch.S

[Tim Deegan]

At 12:51 +0100 on 15 Aug (1376571083), Ian Campbell wrote:
> At the moment it is sufficient to allow us to do away with the
> enter_hyp_mode bits and the clock frequency, gic setup etc, along the
> lines of the patch below.
> 
> It doesn't yet allow us to get rid of the kick_cpus stuff. My plan for
> platforms which don't do the right thing here would be to add a
> mechanism to use dtb /memreserve/ (and teach Xen about that construct)
> to carve out a little bit of memory which secondary CPUs could safely be
> left spinning in. The platform code would expose its normal interface
> (e.g. SYS_FLAGS on vexpress and fastmodel), eventually maybe we'd do
> PSCI too (which might let us skip reserving some memory since 2ndary
> cpus would be in secure mode and could use the special ram regions
> reserved for that)

Sorry, I'm not quite clear -- do you plan to have this bootwrapper
handle the spinning CPUs, with a semihosting callback from Xen to
release them?  That sounds pretty good.

> As we add new platforms I think we should first push back on the vendors
> to fix their firmware but when that turns out to not be possible we
> should move to patching this code with platform hacks instead of adding
> more stuff to mode_switch.S, IMO the only blocker to this is the
> kick_cpu support.
> 
> What does everyone think?

I certainly like the look of the xen-side patch :)

The bootloader looks broadly good to me, including all the kconfig
stuff.  Do you think we'd maintain that in xen.org or upstream?

Cheers,

Tim.

Re: xen: arm: beginning the removal of mode_switch.S

[Ian Campbell]

On Thu, 2013-08-15 at 14:46 +0100, Tim Deegan wrote:
> At 12:51 +0100 on 15 Aug (1376571083), Ian Campbell wrote:
> > At the moment it is sufficient to allow us to do away with the
> > enter_hyp_mode bits and the clock frequency, gic setup etc, along the
> > lines of the patch below.
> > 
> > It doesn't yet allow us to get rid of the kick_cpus stuff. My plan for
> > platforms which don't do the right thing here would be to add a
> > mechanism to use dtb /memreserve/ (and teach Xen about that construct)
> > to carve out a little bit of memory which secondary CPUs could safely be
> > left spinning in. The platform code would expose its normal interface
> > (e.g. SYS_FLAGS on vexpress and fastmodel), eventually maybe we'd do
> > PSCI too (which might let us skip reserving some memory since 2ndary
> > cpus would be in secure mode and could use the special ram regions
> > reserved for that)
> 
> Sorry, I'm not quite clear -- do you plan to have this bootwrapper
> handle the spinning CPUs, with a semihosting callback from Xen to
> release them?  That sounds pretty good.

Yes to spinning, no to semihosting (that's model specific).

On vexpress native this would be handled by writing the start paddr to
SYS_FLAGS and doing a "sev", which will wake up the secondaries (which
are spinning doing wfe) and cause them to jump to that address, so I
would expect the wrapper to implement the same but spinning in NS HYP
mode instead -- in fact the code is there already but I haven't tried it
(bet its broken).

Other platforms have similar mechanisms and I would expect the
bootwrapper to do something similar and mimic whatever the mechanism is
in NS HYP mode.

Thinking about PSCI a bit more, it is likely that if the platform
implements PSCI then it will do it correctly and bring up secondaries in
HYP mode. Probably there would be no possibility of boot-wrapper using
monitor mode anyway just by the nature of PSCI, so we'd be between a
rock and a hard place. I think it'll just work though with no
bootwrapper needed.

> The bootloader looks broadly good to me, including all the kconfig
> stuff.  Do you think we'd maintain that in xen.org or upstream?

I'm not sure, I'm happy to keep it as a local Xen.org hack and when I've
spoken to the upstream in the past they considered it a hack too and
seemed hppy with the idea of us forking. I think they only really care
about models anyway since they don't need to paper over issues on native
so much. Or maybe KVM to care about this. I'll sound them out...

Ian.

Re: xen: arm: beginning the removal of mode_switch.S

[Tim Deegan]

At 15:07 +0100 on 15 Aug (1376579221), Ian Campbell wrote:
> On Thu, 2013-08-15 at 14:46 +0100, Tim Deegan wrote:
> > At 12:51 +0100 on 15 Aug (1376571083), Ian Campbell wrote:
> > > At the moment it is sufficient to allow us to do away with the
> > > enter_hyp_mode bits and the clock frequency, gic setup etc, along the
> > > lines of the patch below.
> > > 
> > > It doesn't yet allow us to get rid of the kick_cpus stuff. My plan for
> > > platforms which don't do the right thing here would be to add a
> > > mechanism to use dtb /memreserve/ (and teach Xen about that construct)
> > > to carve out a little bit of memory which secondary CPUs could safely be
> > > left spinning in. The platform code would expose its normal interface
> > > (e.g. SYS_FLAGS on vexpress and fastmodel), eventually maybe we'd do
> > > PSCI too (which might let us skip reserving some memory since 2ndary
> > > cpus would be in secure mode and could use the special ram regions
> > > reserved for that)
> > 
> > Sorry, I'm not quite clear -- do you plan to have this bootwrapper
> > handle the spinning CPUs, with a semihosting callback from Xen to
> > release them?  That sounds pretty good.
> 
> Yes to spinning, no to semihosting (that's model specific).
> 
> On vexpress native this would be handled by writing the start paddr to
> SYS_FLAGS and doing a "sev", which will wake up the secondaries (which
> are spinning doing wfe) and cause them to jump to that address, so I
> would expect the wrapper to implement the same but spinning in NS HYP
> mode instead -- in fact the code is there already but I haven't tried it
> (bet its broken).
> 
> Other platforms have similar mechanisms and I would expect the
> bootwrapper to do something similar and mimic whatever the mechanism is
> in NS HYP mode.

Ah, OK.  And the release code is in C in xen so it can be a
model-specific routine that knows how the bootwrapper has set things up?

Tim.

Re: xen: arm: beginning the removal of mode_switch.S

[Julien Grall]

Adding Andre.

On 08/15/2013 12:51 PM, Ian Campbell wrote:
> I did some hacking on boot-wrapper.git on the train to debconf and made
> it support building a zImage container encapsulating Xen+Linux+initramfs
> +fdt. Xen is optional so it can be used to boot natively too.
> 
> You can find the code in the multiplatform branch of
> http://xenbits.xen.org/gitweb/?p=people/ianc/boot-wrapper.git
> 
> It has build time (Kconfig driven) options to support:
>       * cubieboard2 (boots native ok, weird issue under Xen)
>       * arndale (code taken from existing mode switch.S, untested)
>       * vexpress and fastmodel (untested)
> 
> The code is pretty hacked up from the original (which only really
> supported fastmodels, and had limited configurability) and it could
> certainly be structured to be quite a bit cleaner (plus I think I got a
> bit carried away with using Kconfig for everything). I'd rather have
> some skanky hacked up code here than in Xen though, so I think this is
> an acceptable level of hackedupness. 
> 
> At the moment it is sufficient to allow us to do away with the
> enter_hyp_mode bits and the clock frequency, gic setup etc, along the
> lines of the patch below.
> 
> It doesn't yet allow us to get rid of the kick_cpus stuff. My plan for
> platforms which don't do the right thing here would be to add a
> mechanism to use dtb /memreserve/ (and teach Xen about that construct)
> to carve out a little bit of memory which secondary CPUs could safely be
> left spinning in. The platform code would expose its normal interface
> (e.g. SYS_FLAGS on vexpress and fastmodel), eventually maybe we'd do
> PSCI too (which might let us skip reserving some memory since 2ndary
> cpus would be in secure mode and could use the special ram regions
> reserved for that)
> 
> I might have time for this on the train on the way home, but since my
> cubieboard2 can't do SMP yet (even on native Linux, bringup looks
> complex) I suppose that means I need to test and debug the fastmodel
> support first...
> 
> As we add new platforms I think we should first push back on the vendors
> to fix their firmware but when that turns out to not be possible we
> should move to patching this code with platform hacks instead of adding
> more stuff to mode_switch.S, IMO the only blocker to this is the
> kick_cpu support.
> 
> What does everyone think?

I'm not sure it's related... does this patch series
(https://lists.cs.columbia.edu/pipermail/kvmarm/2013-April/005581.html)
can avoid the bootwrapper code?

-- 
Julien Grall

Re: xen: arm: beginning the removal of mode_switch.S

[Ian Campbell]

On Thu, 2013-08-15 at 17:53 +0100, Tim Deegan wrote:
> At 15:07 +0100 on 15 Aug (1376579221), Ian Campbell wrote:
> > On Thu, 2013-08-15 at 14:46 +0100, Tim Deegan wrote:
> > > At 12:51 +0100 on 15 Aug (1376571083), Ian Campbell wrote:
> > > > At the moment it is sufficient to allow us to do away with the
> > > > enter_hyp_mode bits and the clock frequency, gic setup etc, along the
> > > > lines of the patch below.
> > > > 
> > > > It doesn't yet allow us to get rid of the kick_cpus stuff. My plan for
> > > > platforms which don't do the right thing here would be to add a
> > > > mechanism to use dtb /memreserve/ (and teach Xen about that construct)
> > > > to carve out a little bit of memory which secondary CPUs could safely be
> > > > left spinning in. The platform code would expose its normal interface
> > > > (e.g. SYS_FLAGS on vexpress and fastmodel), eventually maybe we'd do
> > > > PSCI too (which might let us skip reserving some memory since 2ndary
> > > > cpus would be in secure mode and could use the special ram regions
> > > > reserved for that)
> > > 
> > > Sorry, I'm not quite clear -- do you plan to have this bootwrapper
> > > handle the spinning CPUs, with a semihosting callback from Xen to
> > > release them?  That sounds pretty good.
> > 
> > Yes to spinning, no to semihosting (that's model specific).
> > 
> > On vexpress native this would be handled by writing the start paddr to
> > SYS_FLAGS and doing a "sev", which will wake up the secondaries (which
> > are spinning doing wfe) and cause them to jump to that address, so I
> > would expect the wrapper to implement the same but spinning in NS HYP
> > mode instead -- in fact the code is there already but I haven't tried it
> > (bet its broken).
> > 
> > Other platforms have similar mechanisms and I would expect the
> > bootwrapper to do something similar and mimic whatever the mechanism is
> > in NS HYP mode.
> 
> Ah, OK.  And the release code is in C in xen so it can be a
> model-specific routine that knows how the bootwrapper has set things up?

Yes, it would be a platform hook called from around __cpu_up.

My hope is that bootwrapper can mimic the underlying firmwares behaviour
but in non-secure instead of secure mode, so that the bringup code is
the same, and if the firmware is fixed then Xen doesn't need to change
or be aware of bootwrapper vs. non-bootwrapper etc.

Ian

Re: xen: arm: beginning the removal of mode_switch.S

[Ian Campbell]

On Thu, 2013-08-15 at 18:05 +0100, Julien Grall wrote:
> Adding Andre.


I think Andre's platform should be able to avoid bootwrapper completely,
they do sensible things with cpu bringup, boot in NS HYP mode with
everything setup sensibly etc. This is the ideal situation of course,
with bootwrapper just being a last resort type thing.

> On 08/15/2013 12:51 PM, Ian Campbell wrote:
> > I did some hacking on boot-wrapper.git on the train to debconf and made
> > it support building a zImage container encapsulating Xen+Linux+initramfs
> > +fdt. Xen is optional so it can be used to boot natively too.
> > 
> > You can find the code in the multiplatform branch of
> > http://xenbits.xen.org/gitweb/?p=people/ianc/boot-wrapper.git
> > 
> > It has build time (Kconfig driven) options to support:
> >       * cubieboard2 (boots native ok, weird issue under Xen)
> >       * arndale (code taken from existing mode switch.S, untested)
> >       * vexpress and fastmodel (untested)
> > 
> > The code is pretty hacked up from the original (which only really
> > supported fastmodels, and had limited configurability) and it could
> > certainly be structured to be quite a bit cleaner (plus I think I got a
> > bit carried away with using Kconfig for everything). I'd rather have
> > some skanky hacked up code here than in Xen though, so I think this is
> > an acceptable level of hackedupness. 
> > 
> > At the moment it is sufficient to allow us to do away with the
> > enter_hyp_mode bits and the clock frequency, gic setup etc, along the
> > lines of the patch below.
> > 
> > It doesn't yet allow us to get rid of the kick_cpus stuff. My plan for
> > platforms which don't do the right thing here would be to add a
> > mechanism to use dtb /memreserve/ (and teach Xen about that construct)
> > to carve out a little bit of memory which secondary CPUs could safely be
> > left spinning in. The platform code would expose its normal interface
> > (e.g. SYS_FLAGS on vexpress and fastmodel), eventually maybe we'd do
> > PSCI too (which might let us skip reserving some memory since 2ndary
> > cpus would be in secure mode and could use the special ram regions
> > reserved for that)
> > 
> > I might have time for this on the train on the way home, but since my
> > cubieboard2 can't do SMP yet (even on native Linux, bringup looks
> > complex) I suppose that means I need to test and debug the fastmodel
> > support first...
> > 
> > As we add new platforms I think we should first push back on the vendors
> > to fix their firmware but when that turns out to not be possible we
> > should move to patching this code with platform hacks instead of adding
> > more stuff to mode_switch.S, IMO the only blocker to this is the
> > kick_cpu support.
> > 
> > What does everyone think?
> 
> I'm not sure it's related... does this patch series
> (https://lists.cs.columbia.edu/pipermail/kvmarm/2013-April/005581.html)
> can avoid the bootwrapper code?

Yes, in cases where users can update u-boot or where vendors are
motivated to ship a system which works properly. Bootwrapper is only a
workaround for cases where this isn't possible

It is my intention that bootwrapper become a thing which you expect to
have to use with Xen always -- we should always strive to make the
firmware Just Work and only fallback to bootwrapper  where that isn't
possible for some reason.

Ian.

Re: xen: arm: beginning the removal of mode_switch.S

[Andre Przywara]

On 08/15/2013 07:05 PM, Julien Grall wrote:
> Adding Andre.

Thanks, Julien.
CCing Christoffer for KVM point of view and boot-wrapper.git.
Comments below.

> On 08/15/2013 12:51 PM, Ian Campbell wrote:
>> I did some hacking on boot-wrapper.git on the train to debconf and made
>> it support building a zImage container encapsulating Xen+Linux+initramfs
>> +fdt. Xen is optional so it can be used to boot natively too.
>>
>> You can find the code in the multiplatform branch of
>> http://xenbits.xen.org/gitweb/?p=people/ianc/boot-wrapper.git
>>
>> It has build time (Kconfig driven) options to support:
>>        * cubieboard2 (boots native ok, weird issue under Xen)
>>        * arndale (code taken from existing mode switch.S, untested)
>>        * vexpress and fastmodel (untested)
>>
>> The code is pretty hacked up from the original (which only really
>> supported fastmodels, and had limited configurability) and it could
>> certainly be structured to be quite a bit cleaner (plus I think I got a
>> bit carried away with using Kconfig for everything). I'd rather have
>> some skanky hacked up code here than in Xen though, so I think this is
>> an acceptable level of hackedupness.
>>
>> At the moment it is sufficient to allow us to do away with the
>> enter_hyp_mode bits and the clock frequency, gic setup etc, along the
>> lines of the patch below.
>>
>> It doesn't yet allow us to get rid of the kick_cpus stuff. My plan for
>> platforms which don't do the right thing here would be to add a
>> mechanism to use dtb /memreserve/ (and teach Xen about that construct)
>> to carve out a little bit of memory which secondary CPUs could safely be
>> left spinning in. The platform code would expose its normal interface
>> (e.g. SYS_FLAGS on vexpress and fastmodel), eventually maybe we'd do
>> PSCI too (which might let us skip reserving some memory since 2ndary
>> cpus would be in secure mode and could use the special ram regions
>> reserved for that)
>>
>> I might have time for this on the train on the way home, but since my
>> cubieboard2 can't do SMP yet (even on native Linux, bringup looks
>> complex) I suppose that means I need to test and debug the fastmodel
>> support first...
>>
>> As we add new platforms I think we should first push back on the vendors
>> to fix their firmware but when that turns out to not be possible we
>> should move to patching this code with platform hacks instead of adding
>> more stuff to mode_switch.S, IMO the only blocker to this is the
>> kick_cpu support.
>>
>> What does everyone think?

Ian,

honestly I would not pursue any kind of secure mode / SMP / HYP mode 
switching inside Xen or a Xen related boot wrapper. This is actually 
task of the board's firmware, or bootloader if firmware does not do it 
for one or another reason.
Naturally it should be the firmware's responsibility of doing this (like 
Calxeda does), if not there is now an "almost merged"(TM) u-boot 
implementation by me to solve at least the HYP mode / SMP switching:
http://lists.denx.de/pipermail/u-boot/2013-August/160501.html
This currently supports VExpress, I have Arndale support almost ready 
(still untested). Other boards should be easy to add as this was a 
design criteria of the patch set.

My next work item (next week's time frame) is to get some basic 
multiboot support into u-boot and Xen/ARM to allow the user to tell the 
bootloader which bits to load.

This is the approach Linaro agreed upon for Xen in July.

Alternatively I could teach u-boot to write the addresses in the DTB 
meeting your Xen code.

Wouldn't that solve all the problems you try to address with boot-wrapper?
Since Linux/KVM requires the kernel to be entered in HYP mode anyway, 
Xen should be on the same page as them, so a joint effort would make 
sense here, right?

Honestly I'd like to do away with Xen's mode_switch.S as soon as 
possible. I tried to simply add Calxeda's native SMP kicking in there 
and failed, since the code actually reads:
If r1 isn't the Arndale machine ID, assume Versatile Express :-(

We could either require PSCI support for boards supporting Xen or add 
platform specific SMP ops (boldly copying bits from Linux here).

> I'm not sure it's related... does this patch series
> (https://lists.cs.columbia.edu/pipermail/kvmarm/2013-April/005581.html)
> can avoid the bootwrapper code?

That's the plan, but please see the v4 version posted last Friday (URL 
above).

Thanks,
Andre.

Re: xen: arm: beginning the removal of mode_switch.S

[Ian Campbell]

On Thu, 2013-08-15 at 22:55 +0200, Andre Przywara wrote:
> honestly I would not pursue any kind of secure mode / SMP / HYP mode 
> switching inside Xen or a Xen related boot wrapper. This is actually 
> task of the board's firmware, or bootloader if firmware does not do it 
> for one or another reason.
> Naturally it should be the firmware's responsibility of doing this (like 
> Calxeda does), if not there is now an "almost merged"(TM) u-boot 
> implementation by me to solve at least the HYP mode / SMP switching:
> http://lists.denx.de/pipermail/u-boot/2013-August/160501.html
> This currently supports VExpress, I have Arndale support almost ready 
> (still untested). Other boards should be easy to add as this was a 
> design criteria of the patch set.

That's great and as I've said a few times in this thread this is what we
should be aiming for, bootwrapper is not intended to replace or compete
with those patches, they are the right answer for sure.

> My next work item (next week's time frame) is to get some basic 
> multiboot support into u-boot and Xen/ARM to allow the user to tell the 
> bootloader which bits to load.
> 
> This is the approach Linaro agreed upon for Xen in July.

> Alternatively I could teach u-boot to write the addresses in the DTB 
> meeting your Xen code.
> 
> Wouldn't that solve all the problems you try to address with boot-wrapper?

In an ideal world where vendors were responsive to bug reports and cared
enough to fix the virt use case, and where users feel confident enough
to replace the bootloader on the system etc etc then yes it absolutely
solves the problem and is exactly what people should be doing.

bootwrapper is there for the cases where the vendor doesn't care (about
virt, about Xen, etc), or people are unwilling to upgrade u-boot
(perhaps lack of JTAG makes bricking the system too much of a risk,
think random consumer devices), or maybe the vendor u-boot is too old or
too diverged from mainline to allow the patches to be applied without a
lot of development work. bootwrapper is a hack intended to allow people
to still run Xen on those platforms, it is not supposed to be
alternative to doing it properly.

I will happily delete platform code from bootwrapper the moment that a
fixed firmware/bootloader which can be deployed on a platform in a way
which end users can cope with is available. And I will certainly be
encouraging people to figure out how to do that rather than adding
platform support to bootwrapper.

> Since Linux/KVM requires the kernel to be entered in HYP mode anyway, 
> Xen should be on the same page as them, so a joint effort would make 
> sense here, right?

Yes. Xen's requirement is exactly the same as KVMs, we must be entered
in NS HYP mode. Bootwrapper is there to make this true in cases where it
otherwise is not and the firmware/bootloader cannot be fixed.

It's very good that you've fixed (or are fixing) the arndale and
vexpress stuff and have just done The Right Thing on Calxeda platforms
already.

With bootwrapper I'm more concerned about all the random cheap hardware
coming out of China (like the Allwinner derived stuff) which it might be
desirable for community members etc to run Xen on.

> Honestly I'd like to do away with Xen's mode_switch.S as soon as 
> possible.

Me too.

>  I tried to simply add Calxeda's native SMP kicking in there 
> and failed, since the code actually reads:
> If r1 isn't the Arndale machine ID, assume Versatile Express :-(

Right, it sucks. It's a hack and must die.

> We could either require PSCI support for boards supporting Xen or add 
> platform specific SMP ops (boldly copying bits from Linux here).

Yes, this is the plan.

Ian.

Re: xen: arm: beginning the removal of mode_switch.S

[Julien Grall]

On 08/15/2013 09:51 PM, Ian Campbell wrote:
> On Thu, 2013-08-15 at 18:05 +0100, Julien Grall wrote:
>> Adding Andre.
> 
> 
> I think Andre's platform should be able to avoid bootwrapper completely,
> they do sensible things with cpu bringup, boot in NS HYP mode with
> everything setup sensibly etc. This is the ideal situation of course,
> with bootwrapper just being a last resort type thing.
> 
>> On 08/15/2013 12:51 PM, Ian Campbell wrote:
>>> I did some hacking on boot-wrapper.git on the train to debconf and made
>>> it support building a zImage container encapsulating Xen+Linux+initramfs
>>> +fdt. Xen is optional so it can be used to boot natively too.
>>>
>>> You can find the code in the multiplatform branch of
>>> http://xenbits.xen.org/gitweb/?p=people/ianc/boot-wrapper.git
>>>
>>> It has build time (Kconfig driven) options to support:
>>>       * cubieboard2 (boots native ok, weird issue under Xen)
>>>       * arndale (code taken from existing mode switch.S, untested)
>>>       * vexpress and fastmodel (untested)
>>>
>>> The code is pretty hacked up from the original (which only really
>>> supported fastmodels, and had limited configurability) and it could
>>> certainly be structured to be quite a bit cleaner (plus I think I got a
>>> bit carried away with using Kconfig for everything). I'd rather have
>>> some skanky hacked up code here than in Xen though, so I think this is
>>> an acceptable level of hackedupness. 
>>>
>>> At the moment it is sufficient to allow us to do away with the
>>> enter_hyp_mode bits and the clock frequency, gic setup etc, along the
>>> lines of the patch below.
>>>
>>> It doesn't yet allow us to get rid of the kick_cpus stuff. My plan for
>>> platforms which don't do the right thing here would be to add a
>>> mechanism to use dtb /memreserve/ (and teach Xen about that construct)
>>> to carve out a little bit of memory which secondary CPUs could safely be
>>> left spinning in. The platform code would expose its normal interface
>>> (e.g. SYS_FLAGS on vexpress and fastmodel), eventually maybe we'd do
>>> PSCI too (which might let us skip reserving some memory since 2ndary
>>> cpus would be in secure mode and could use the special ram regions
>>> reserved for that)
>>>
>>> I might have time for this on the train on the way home, but since my
>>> cubieboard2 can't do SMP yet (even on native Linux, bringup looks
>>> complex) I suppose that means I need to test and debug the fastmodel
>>> support first...
>>>
>>> As we add new platforms I think we should first push back on the vendors
>>> to fix their firmware but when that turns out to not be possible we
>>> should move to patching this code with platform hacks instead of adding
>>> more stuff to mode_switch.S, IMO the only blocker to this is the
>>> kick_cpu support.
>>>
>>> What does everyone think?
>>
>> I'm not sure it's related... does this patch series
>> (https://lists.cs.columbia.edu/pipermail/kvmarm/2013-April/005581.html)
>> can avoid the bootwrapper code?
> 
> Yes, in cases where users can update u-boot or where vendors are
> motivated to ship a system which works properly. Bootwrapper is only a
> workaround for cases where this isn't possible
> 
> It is my intention that bootwrapper become a thing which you expect to
> have to use with Xen always -- we should always strive to make the
> firmware Just Work and only fallback to bootwrapper  where that isn't
> possible for some reason.

Thanks for your answer !
The bootwrapper can be avoid for the Arndale. I have noticed that if
kick cpus is moved later, the secondary cpu will boot in HYP mode.

Otherwise I will give a try to the bootwrapper on the Versatile Express.

Cheers,


-- 
Julien Grall

Re: xen: arm: beginning the removal of mode_switch.S

[Ian Campbell]

On Fri, 2013-08-16 at 11:12 +0100, Julien Grall wrote:
> On 08/15/2013 09:51 PM, Ian Campbell wrote:
> > On Thu, 2013-08-15 at 18:05 +0100, Julien Grall wrote:
> >> Adding Andre.
> > 
> > 
> > I think Andre's platform should be able to avoid bootwrapper completely,
> > they do sensible things with cpu bringup, boot in NS HYP mode with
> > everything setup sensibly etc. This is the ideal situation of course,
> > with bootwrapper just being a last resort type thing.
> > 
> >> On 08/15/2013 12:51 PM, Ian Campbell wrote:
> >>> I did some hacking on boot-wrapper.git on the train to debconf and made
> >>> it support building a zImage container encapsulating Xen+Linux+initramfs
> >>> +fdt. Xen is optional so it can be used to boot natively too.
> >>>
> >>> You can find the code in the multiplatform branch of
> >>> http://xenbits.xen.org/gitweb/?p=people/ianc/boot-wrapper.git
> >>>
> >>> It has build time (Kconfig driven) options to support:
> >>>       * cubieboard2 (boots native ok, weird issue under Xen)
> >>>       * arndale (code taken from existing mode switch.S, untested)
> >>>       * vexpress and fastmodel (untested)
> >>>
> >>> The code is pretty hacked up from the original (which only really
> >>> supported fastmodels, and had limited configurability) and it could
> >>> certainly be structured to be quite a bit cleaner (plus I think I got a
> >>> bit carried away with using Kconfig for everything). I'd rather have
> >>> some skanky hacked up code here than in Xen though, so I think this is
> >>> an acceptable level of hackedupness. 
> >>>
> >>> At the moment it is sufficient to allow us to do away with the
> >>> enter_hyp_mode bits and the clock frequency, gic setup etc, along the
> >>> lines of the patch below.
> >>>
> >>> It doesn't yet allow us to get rid of the kick_cpus stuff. My plan for
> >>> platforms which don't do the right thing here would be to add a
> >>> mechanism to use dtb /memreserve/ (and teach Xen about that construct)
> >>> to carve out a little bit of memory which secondary CPUs could safely be
> >>> left spinning in. The platform code would expose its normal interface
> >>> (e.g. SYS_FLAGS on vexpress and fastmodel), eventually maybe we'd do
> >>> PSCI too (which might let us skip reserving some memory since 2ndary
> >>> cpus would be in secure mode and could use the special ram regions
> >>> reserved for that)
> >>>
> >>> I might have time for this on the train on the way home, but since my
> >>> cubieboard2 can't do SMP yet (even on native Linux, bringup looks
> >>> complex) I suppose that means I need to test and debug the fastmodel
> >>> support first...
> >>>
> >>> As we add new platforms I think we should first push back on the vendors
> >>> to fix their firmware but when that turns out to not be possible we
> >>> should move to patching this code with platform hacks instead of adding
> >>> more stuff to mode_switch.S, IMO the only blocker to this is the
> >>> kick_cpu support.
> >>>
> >>> What does everyone think?
> >>
> >> I'm not sure it's related... does this patch series
> >> (https://lists.cs.columbia.edu/pipermail/kvmarm/2013-April/005581.html)
> >> can avoid the bootwrapper code?
> > 
> > Yes, in cases where users can update u-boot or where vendors are
> > motivated to ship a system which works properly. Bootwrapper is only a
> > workaround for cases where this isn't possible
> > 
> > It is my intention that bootwrapper become a thing which you expect to
> > have to use with Xen always -- we should always strive to make the
> > firmware Just Work and only fallback to bootwrapper  where that isn't
> > possible for some reason.
> 
> Thanks for your answer !
> The bootwrapper can be avoid for the Arndale. I have noticed that if
> kick cpus is moved later, the secondary cpu will boot in HYP mode.

Excellent, so you are going to send a patch to nuke the relevant bits of
mode_switch.S?

> Otherwise I will give a try to the bootwrapper on the Versatile Express.

Andre seemed to suggest he had fixed (or was fixing) vexpress's u-boot
too, which only really leaves the models. I'd love to be able to run
u-boot there too simply for consistency...

I'd be happy if bootwrapper died before it even left the starting
line ;-) So long as we can nuke mode_switch.S...

Ian.

Re: xen: arm: beginning the removal of mode_switch.S

[Andre Przywara]

On 08/16/2013 05:04 PM, Ian Campbell wrote:
> On Fri, 2013-08-16 at 11:12 +0100, Julien Grall wrote:
>> On 08/15/2013 09:51 PM, Ian Campbell wrote:
>>> On Thu, 2013-08-15 at 18:05 +0100, Julien Grall wrote:
>>>> Adding Andre.
>>>
>>>
>>> I think Andre's platform should be able to avoid bootwrapper completely,
>>> they do sensible things with cpu bringup, boot in NS HYP mode with
>>> everything setup sensibly etc. This is the ideal situation of course,
>>> with bootwrapper just being a last resort type thing.
>>>
>>>> On 08/15/2013 12:51 PM, Ian Campbell wrote:
>>>>> I did some hacking on boot-wrapper.git on the train to debconf and made
>>>>> it support building a zImage container encapsulating Xen+Linux+initramfs
>>>>> +fdt. Xen is optional so it can be used to boot natively too.
>>>>>
>>>>> You can find the code in the multiplatform branch of
>>>>> http://xenbits.xen.org/gitweb/?p=people/ianc/boot-wrapper.git
>>>>>
>>>>> It has build time (Kconfig driven) options to support:
>>>>>        * cubieboard2 (boots native ok, weird issue under Xen)
>>>>>        * arndale (code taken from existing mode switch.S, untested)
>>>>>        * vexpress and fastmodel (untested)
>>>>>
>>>>> The code is pretty hacked up from the original (which only really
>>>>> supported fastmodels, and had limited configurability) and it could
>>>>> certainly be structured to be quite a bit cleaner (plus I think I got a
>>>>> bit carried away with using Kconfig for everything). I'd rather have
>>>>> some skanky hacked up code here than in Xen though, so I think this is
>>>>> an acceptable level of hackedupness.
>>>>>
>>>>> At the moment it is sufficient to allow us to do away with the
>>>>> enter_hyp_mode bits and the clock frequency, gic setup etc, along the
>>>>> lines of the patch below.
>>>>>
>>>>> It doesn't yet allow us to get rid of the kick_cpus stuff. My plan for
>>>>> platforms which don't do the right thing here would be to add a
>>>>> mechanism to use dtb /memreserve/ (and teach Xen about that construct)
>>>>> to carve out a little bit of memory which secondary CPUs could safely be
>>>>> left spinning in. The platform code would expose its normal interface
>>>>> (e.g. SYS_FLAGS on vexpress and fastmodel), eventually maybe we'd do
>>>>> PSCI too (which might let us skip reserving some memory since 2ndary
>>>>> cpus would be in secure mode and could use the special ram regions
>>>>> reserved for that)
>>>>>
>>>>> I might have time for this on the train on the way home, but since my
>>>>> cubieboard2 can't do SMP yet (even on native Linux, bringup looks
>>>>> complex) I suppose that means I need to test and debug the fastmodel
>>>>> support first...
>>>>>
>>>>> As we add new platforms I think we should first push back on the vendors
>>>>> to fix their firmware but when that turns out to not be possible we
>>>>> should move to patching this code with platform hacks instead of adding
>>>>> more stuff to mode_switch.S, IMO the only blocker to this is the
>>>>> kick_cpu support.
>>>>>
>>>>> What does everyone think?
>>>>
>>>> I'm not sure it's related... does this patch series
>>>> (https://lists.cs.columbia.edu/pipermail/kvmarm/2013-April/005581.html)
>>>> can avoid the bootwrapper code?
>>>
>>> Yes, in cases where users can update u-boot or where vendors are
>>> motivated to ship a system which works properly. Bootwrapper is only a
>>> workaround for cases where this isn't possible
>>>
>>> It is my intention that bootwrapper become a thing which you expect to
>>> have to use with Xen always -- we should always strive to make the
>>> firmware Just Work and only fallback to bootwrapper  where that isn't
>>> possible for some reason.
>>
>> Thanks for your answer !
>> The bootwrapper can be avoid for the Arndale. I have noticed that if
>> kick cpus is moved later, the secondary cpu will boot in HYP mode.
>
> Excellent, so you are going to send a patch to nuke the relevant bits of
> mode_switch.S?
>
>> Otherwise I will give a try to the bootwrapper on the Versatile Express.
>
> Andre seemed to suggest he had fixed (or was fixing) vexpress's u-boot
> too, which only really leaves the models. I'd love to be able to run
> u-boot there too simply for consistency...

I managed to start Xen on the VExpress via my HYP-u-boot before. Only 
today I couldn't reproduce it with the latest bits...
Will see if I find the culprit.

> I'd be happy if bootwrapper died before it even left the starting
> line ;-) So long as we can nuke mode_switch.S...

ACK!

Andre.

Re: xen: arm: beginning the removal of mode_switch.S

[Julien Grall]

On 08/16/2013 04:04 PM, Ian Campbell wrote:
> On Fri, 2013-08-16 at 11:12 +0100, Julien Grall wrote:
>> On 08/15/2013 09:51 PM, Ian Campbell wrote:
>>> On Thu, 2013-08-15 at 18:05 +0100, Julien Grall wrote:
>>>> Adding Andre.
>>>
>>>
>>> I think Andre's platform should be able to avoid bootwrapper completely,
>>> they do sensible things with cpu bringup, boot in NS HYP mode with
>>> everything setup sensibly etc. This is the ideal situation of course,
>>> with bootwrapper just being a last resort type thing.
>>>
>>>> On 08/15/2013 12:51 PM, Ian Campbell wrote:
>>>>> I did some hacking on boot-wrapper.git on the train to debconf and made
>>>>> it support building a zImage container encapsulating Xen+Linux+initramfs
>>>>> +fdt. Xen is optional so it can be used to boot natively too.
>>>>>
>>>>> You can find the code in the multiplatform branch of
>>>>> http://xenbits.xen.org/gitweb/?p=people/ianc/boot-wrapper.git
>>>>>
>>>>> It has build time (Kconfig driven) options to support:
>>>>>       * cubieboard2 (boots native ok, weird issue under Xen)
>>>>>       * arndale (code taken from existing mode switch.S, untested)
>>>>>       * vexpress and fastmodel (untested)
>>>>>
>>>>> The code is pretty hacked up from the original (which only really
>>>>> supported fastmodels, and had limited configurability) and it could
>>>>> certainly be structured to be quite a bit cleaner (plus I think I got a
>>>>> bit carried away with using Kconfig for everything). I'd rather have
>>>>> some skanky hacked up code here than in Xen though, so I think this is
>>>>> an acceptable level of hackedupness. 
>>>>>
>>>>> At the moment it is sufficient to allow us to do away with the
>>>>> enter_hyp_mode bits and the clock frequency, gic setup etc, along the
>>>>> lines of the patch below.
>>>>>
>>>>> It doesn't yet allow us to get rid of the kick_cpus stuff. My plan for
>>>>> platforms which don't do the right thing here would be to add a
>>>>> mechanism to use dtb /memreserve/ (and teach Xen about that construct)
>>>>> to carve out a little bit of memory which secondary CPUs could safely be
>>>>> left spinning in. The platform code would expose its normal interface
>>>>> (e.g. SYS_FLAGS on vexpress and fastmodel), eventually maybe we'd do
>>>>> PSCI too (which might let us skip reserving some memory since 2ndary
>>>>> cpus would be in secure mode and could use the special ram regions
>>>>> reserved for that)
>>>>>
>>>>> I might have time for this on the train on the way home, but since my
>>>>> cubieboard2 can't do SMP yet (even on native Linux, bringup looks
>>>>> complex) I suppose that means I need to test and debug the fastmodel
>>>>> support first...
>>>>>
>>>>> As we add new platforms I think we should first push back on the vendors
>>>>> to fix their firmware but when that turns out to not be possible we
>>>>> should move to patching this code with platform hacks instead of adding
>>>>> more stuff to mode_switch.S, IMO the only blocker to this is the
>>>>> kick_cpu support.
>>>>>
>>>>> What does everyone think?
>>>>
>>>> I'm not sure it's related... does this patch series
>>>> (https://lists.cs.columbia.edu/pipermail/kvmarm/2013-April/005581.html)
>>>> can avoid the bootwrapper code?
>>>
>>> Yes, in cases where users can update u-boot or where vendors are
>>> motivated to ship a system which works properly. Bootwrapper is only a
>>> workaround for cases where this isn't possible
>>>
>>> It is my intention that bootwrapper become a thing which you expect to
>>> have to use with Xen always -- we should always strive to make the
>>> firmware Just Work and only fallback to bootwrapper  where that isn't
>>> possible for some reason.
>>
>> Thanks for your answer !
>> The bootwrapper can be avoid for the Arndale. I have noticed that if
>> kick cpus is moved later, the secondary cpu will boot in HYP mode.
> 
> Excellent, so you are going to send a patch to nuke the relevant bits of
> mode_switch.S?

Unfortunately I haven't a patch for this part. My change was a quick
hack to try to move Arndale kick cpus in C code. But I had some issue
with the page table. I don't remember what was the exact problem and I
lost my hack :/.

-- 
Julien Grall

Re: xen: arm: beginning the removal of mode_switch.S

[Christoffer Dall]

On Thu, Aug 15, 2013 at 10:55:35PM +0200, Andre Przywara wrote:
> On 08/15/2013 07:05 PM, Julien Grall wrote:
> >Adding Andre.
> 
> Thanks, Julien.
> CCing Christoffer for KVM point of view and boot-wrapper.git.
> Comments below.
> 
> >On 08/15/2013 12:51 PM, Ian Campbell wrote:
> >>I did some hacking on boot-wrapper.git on the train to debconf and made
> >>it support building a zImage container encapsulating Xen+Linux+initramfs
> >>+fdt. Xen is optional so it can be used to boot natively too.
> >>
> >>You can find the code in the multiplatform branch of
> >>http://xenbits.xen.org/gitweb/?p=people/ianc/boot-wrapper.git
> >>
> >>It has build time (Kconfig driven) options to support:
> >>       * cubieboard2 (boots native ok, weird issue under Xen)
> >>       * arndale (code taken from existing mode switch.S, untested)
> >>       * vexpress and fastmodel (untested)
> >>
> >>The code is pretty hacked up from the original (which only really
> >>supported fastmodels, and had limited configurability) and it could
> >>certainly be structured to be quite a bit cleaner (plus I think I got a
> >>bit carried away with using Kconfig for everything). I'd rather have
> >>some skanky hacked up code here than in Xen though, so I think this is
> >>an acceptable level of hackedupness.
> >>
> >>At the moment it is sufficient to allow us to do away with the
> >>enter_hyp_mode bits and the clock frequency, gic setup etc, along the
> >>lines of the patch below.
> >>
> >>It doesn't yet allow us to get rid of the kick_cpus stuff. My plan for
> >>platforms which don't do the right thing here would be to add a
> >>mechanism to use dtb /memreserve/ (and teach Xen about that construct)
> >>to carve out a little bit of memory which secondary CPUs could safely be
> >>left spinning in. The platform code would expose its normal interface
> >>(e.g. SYS_FLAGS on vexpress and fastmodel), eventually maybe we'd do
> >>PSCI too (which might let us skip reserving some memory since 2ndary
> >>cpus would be in secure mode and could use the special ram regions
> >>reserved for that)
> >>
> >>I might have time for this on the train on the way home, but since my
> >>cubieboard2 can't do SMP yet (even on native Linux, bringup looks
> >>complex) I suppose that means I need to test and debug the fastmodel
> >>support first...
> >>
> >>As we add new platforms I think we should first push back on the vendors
> >>to fix their firmware but when that turns out to not be possible we
> >>should move to patching this code with platform hacks instead of adding
> >>more stuff to mode_switch.S, IMO the only blocker to this is the
> >>kick_cpu support.
> >>
> >>What does everyone think?
> 
> Ian,
> 
> honestly I would not pursue any kind of secure mode / SMP / HYP mode
> switching inside Xen or a Xen related boot wrapper. This is actually
> task of the board's firmware, or bootloader if firmware does not do
> it for one or another reason.
> Naturally it should be the firmware's responsibility of doing this
> (like Calxeda does), if not there is now an "almost merged"(TM)
> u-boot implementation by me to solve at least the HYP mode / SMP
> switching:
> http://lists.denx.de/pipermail/u-boot/2013-August/160501.html
> This currently supports VExpress, I have Arndale support almost
> ready (still untested). Other boards should be easy to add as this
> was a design criteria of the patch set.
> 
> My next work item (next week's time frame) is to get some basic
> multiboot support into u-boot and Xen/ARM to allow the user to tell
> the bootloader which bits to load.
> 
> This is the approach Linaro agreed upon for Xen in July.
> 
> Alternatively I could teach u-boot to write the addresses in the DTB
> meeting your Xen code.
> 
> Wouldn't that solve all the problems you try to address with boot-wrapper?
> Since Linux/KVM requires the kernel to be entered in HYP mode
> anyway, Xen should be on the same page as them, so a joint effort
> would make sense here, right?
> 
> Honestly I'd like to do away with Xen's mode_switch.S as soon as
> possible. I tried to simply add Calxeda's native SMP kicking in
> there and failed, since the code actually reads:
> If r1 isn't the Arndale machine ID, assume Versatile Express :-(
> 
> We could either require PSCI support for boards supporting Xen or
> add platform specific SMP ops (boldly copying bits from Linux here).
> 
> >I'm not sure it's related... does this patch series
> >(https://lists.cs.columbia.edu/pipermail/kvmarm/2013-April/005581.html)
> >can avoid the bootwrapper code?
> 
> That's the plan, but please see the v4 version posted last Friday
> (URL above).

FWIW, I second Andre's position; the boot-wrapper was simply a hack
before we had any proper bootloader support (like U-boot) on development
boards like the versatile express and it has been (ab)used for models as
well.  I'm not sure if boot-wrapper could be completely abandoned at
this point or if it is still needed for some cases (models, big.little,
...).  But surely anything we can reasonably do to leverage existing
U-boot work and do things the proper way will set the right example for
users and other implementors.

The latter perspective is especially important for Linaro as the whole
point of the organization is to work upstream and make things work
across many platforms.

-Christoffer

Re: xen: arm: beginning the removal of mode_switch.S

[Ian Campbell]

On Mon, 2013-08-19 at 10:46 -0700, Christoffer Dall wrote:
> FWIW, I second Andre's position; the boot-wrapper was simply a hack
> before we had any proper bootloader support (like U-boot) on development
> boards like the versatile express and it has been (ab)used for models as
> well.  I'm not sure if boot-wrapper could be completely abandoned at
> this point or if it is still needed for some cases (models, big.little,
> ...).  But surely anything we can reasonably do to leverage existing
> U-boot work and do things the proper way will set the right example for
> users and other implementors.

I absolutely agree, as I said to Andre the use of bootwrapper should be
an absolute last resort. I'd be more than happy to reject bootwrapper
patches for platforms which do this stuff properly. Or if I get told
"platform A has been fixed, here is how to run Xen on it without
bootwrapper" then I will delete support for platform A in a microsecond.
If this turns out to be all the platforms then even better.

> The latter perspective is especially important for Linaro as the whole
> point of the organization is to work upstream and make things work
> across many platforms.

I'm not really worried about the Linaro Supported platforms, you guys
are clearly doing the right things and getting fixed u-boot etc into
upstreams and as I've said this is my preference for supporting Xen too.

However I would like Xen to support a wider variety of platforms than
that, in particular the super el-cheapo boards like the cubieboard2,
popular hobbyist platforms like the pandaboard and random consumer
hardware (e.g. the MELE android set top boxes have Cortex-A7s in them).
Having something easily available for reasonably low prices is important
for the hobbyist/non-commercially supported developer segment IMHO. 

The vendors of those sort of boards don't always work so closely with
upstreams, so getting a u-boot which does hyp mode, or even one where
Andre's patches can be backported is not necessarily always going to be
possible. Likewise many of these systems don't have easy access to JTAG
or some fallback when reflashing u-boot goes wrong, or the hobbyists
don't have JTAG hardware etc. Under those circumstances people are
understandably reticent to reflash their bootloader.

cubieboard2 is not really a good example here, since the linux-sunxi
community have taken up the baton and upstream u-boot support is pretty
good and the hardware is pretty much unbrickable due to the FEL mode
fallback.

Perhaps the panda5 is a better example -- I don't know how good upstream
u-boot, but AFAIK the platform is no longer supported by the vendor.
Chen, a hobbyist, was posting patches to add more stuff to
mode_switch.S. I don't think it is reasonable to ask him to go and get
panda5 support into upstream u-boot. So I started hacking on bootwrapper
mainly to give him somewhere he could put his boot time hacks which was
outside the Xen source tree and which prevented the addition of new
hacks Xen's mode_switch.S.

Ultimately what I really care about is deleting mode_switch.S from Xen,
it is a hack with no place in the Xen tree. bootwrapper is also a hack
but it's not in the Xen tree.

Ian.

Re: xen: arm: beginning the removal of mode_switch.S

[Ian Campbell]

On Fri, 2013-08-16 at 16:44 +0100, Julien Grall wrote:
> Unfortunately I haven't a patch for this part. My change was a quick
> hack to try to move Arndale kick cpus in C code. But I had some issue
> with the page table. I don't remember what was the exact problem and I
> lost my hack :/.

Sadly I think removing kick_cpus is going to require at least some
reworking of the boot time pages tables, for at least secondary CPUs.
Note that I still think we should do it ;-)

I think currently we have:

        BOOT CPU			SECONDARY CPU
        
        startup				held in firmware
        
        call platform kick cpu.
        
        				wfe loop 1, checking smp_up_cpu for our CPU
        
        
*** At this point all CPUs are running Xen code, at the original 
*** physical load address and with paging disabled
        
        ensure we are in hyp mode
        (mode_switch.S called here, 
         needs to go!)
        
        zero bss
        
        call proc specific cpu_init
        
        setup MMU (HMAIR, HTCR, HSCTLR)
        
        HTTBR = boot_pgtable
        
        build page tables in boot_pgtable,
        contains regular vaddr mapping of
        Xen at 2M and a 1:1 mapping of the
        physical load address. This might
        play badly if the load address was 2M?
        
        enable paging
        
*** At this point the boot CPU is running on the boot_pgtable at the 
*** original physical load address on an identity mapping.
*** Secondary CPUs are still spinning at the start with paging disabled.
        
        jump to Xen virtual mapping at 2M
        
*** At this point the boot CPU is running on the boot_pgtable at the 
*** original physical load address on the normal vaddr (text at 2M).
*** Secondary CPUs are still spinning at the original paddr with paging
*** disabled.
        
        load boot stack pointer
        
        branch to start_xen
        
        setup_pagetables
          copies xen image, including
          boot_pgtable to final physical
          location
        
          remove 1:1 mapping from copy, 
          leave in the original
        
          switch to relocated page tables
        
*** At this point the boot CPU is now running on the relocated 
*** boot_pgtable at the normal vaddr.
*** Secondary CPUs are still spinning at the original paddr with paging
*** disabled.
    (at this point we have something of a bug, which is that we rely on 
     luck to not overwrite the original physical address...)

        ... other setup
        
        make_cpus_ready:
          for each cpu (0..max_cpus):
            set smp_up_cpu at orig paddr.
            sev
            wait for ready_cpus to change
        
        				exits wfe loop 1
        
        				ensure in hyp mode (mode_switch.S, must die)
        
        				proc specific cpu init
        
        				setup MMU (HMAIR, HTCR, HSCTLR)
        
        				HTTBR = boot_pgtable (original paddr)
        
        				enable paging
        
*** At this point the boot CPU is now running on the relocated 
*** boot_pgtable at the normal vaddr.
*** Secondary CPUs are now running on the boot_pgtable at the original 
*** load address, on the 1:1 mapping original used by the boot CPU. 

        				jump to 2MB text mapping
        
*** At this point the boot CPU is now running on the relocated 
*** boot_pgtable at the normal vaddr.
*** Secondary CPUs are now running on the boot_pgtable at the original
*** load address, on the 1:1 mapping original used by the boot CPU. 

        				switch from boot_pgtable to boot_ttbr
        
*** At this point all CPUs are now running on the relocated 
*** pgtables at the normal vaddr.

        				increment ready_cpus
        
        				wfe loop 1, checking smp_up_cpu for our CPU
        				this time smp_up_cpu at relocated address
        
        for each secondary:
          __cpu_up:
            write cpu to smp_up_cpu
        
            sev
        
            wait for cpu to come up
        
        				wakes up
        
        				calls start_secondary
        
        				marks cpu online
        
        				enters idle loop
        
            cpu comes on line
        
        build and start dom0.
        
        idle loop
        
*** we are done

So this is all pretty complex (not to mention hard to describe in ASCII)
and in lockstep, the secondary cpus wait twice once on the original
smp_cpu_up and then again on the relocated version. There is a subtle
reliance on the 1:1 mapping being retained in the original copy of the
page tables.

I think the original wait is actually a workaround for lack of firmware
on the fastmodels, and should be implemented by either the firmware or
bootwrapper.

The second wait is something of a consequence of this and is because the
secondary CPUs start on the original paddr. They should instead be told
to start directly on the relocated copy of Xen. This gets rid of both
the second wait and the make_cpus_ready call I think.

Ultimately it should be possible for the boot cpu to bootstrap itself
all the way to "for each secondary: __cpu_up" while the secondaries are
still spinning in the firmware. __cpu_up should instruct (using the
platforms existing firmware interface, SYSREG or PSCI etc) the
secondaries to start at a paddr within the relocated image (removing the
bug with crossing our fingers and hoping the original is still there).
Secondary CPUs would then come all the way up to idling without needing
to wait for anything.

However there is a slight wrinkle which is that the relocated page
tables do not contain the 1:1 mapping needed to enable paging on the
secondaries.

I think the right solution here is to have a special set of page tables
which are used only during bring up (and eventually for pCPU hotplug
etc) which contain the necessary 1:1 mapping + 2M mapping of Xen.

Ian.

Re: xen: arm: beginning the removal of mode_switch.S

[Julien Grall]

On 08/20/2013 03:11 PM, Ian Campbell wrote:
> So this is all pretty complex (not to mention hard to describe in ASCII)
> and in lockstep, the secondary cpus wait twice once on the original
> smp_cpu_up and then again on the relocated version. There is a subtle
> reliance on the 1:1 mapping being retained in the original copy of the
> page tables.

Thanks for this ASCII!

> I think the original wait is actually a workaround for lack of firmware
> on the fastmodels, and should be implemented by either the firmware or
> bootwrapper.

BTW, this wait is an issue when the boot CPU ID is not equal to 0.

I gave a quick try to move kick cpus after the HYP mode switch in
assembly and I'm unable to boot secondary cpus on the Versatile Express.
I guess, on the VE secondary cpus can only be "kick" in secure mode.

Cheers,

-- 
Julien Grall

Re: xen: arm: beginning the removal of mode_switch.S

[Andre Przywara]

On 08/21/2013 02:36 PM, Julien Grall wrote:
> On 08/20/2013 03:11 PM, Ian Campbell wrote:
>> So this is all pretty complex (not to mention hard to describe in ASCII)
>> and in lockstep, the secondary cpus wait twice once on the original
>> smp_cpu_up and then again on the relocated version. There is a subtle
>> reliance on the 1:1 mapping being retained in the original copy of the
>> page tables.
>
> Thanks for this ASCII!
>
>> I think the original wait is actually a workaround for lack of firmware
>> on the fastmodels, and should be implemented by either the firmware or
>> bootwrapper.
>
> BTW, this wait is an issue when the boot CPU ID is not equal to 0.
>
> I gave a quick try to move kick cpus after the HYP mode switch in
> assembly and I'm unable to boot secondary cpus on the Versatile Express.
> I guess, on the VE secondary cpus can only be "kick" in secure mode.

Right, but I think this is not specific to the VE, but ARM in general.
If I remember correctly the reason was that you cannot send IPIs from an 
non-secure GIC CPU interface to a secure one. So you have to enable the 
CPU interfaces of the others cores for non-secure operation first, which 
involves waking them up.
In my U-Boot code I switch to HYP mode on the BSP at the very end, after 
having switched the secondary cores to HYP before.

Regards,
Andre.

Re: xen: arm: beginning the removal of mode_switch.S

[Ian Campbell]

On Wed, 2013-08-21 at 14:42 +0200, Andre Przywara wrote:
> On 08/21/2013 02:36 PM, Julien Grall wrote:
> > On 08/20/2013 03:11 PM, Ian Campbell wrote:
> >> So this is all pretty complex (not to mention hard to describe in ASCII)
> >> and in lockstep, the secondary cpus wait twice once on the original
> >> smp_cpu_up and then again on the relocated version. There is a subtle
> >> reliance on the 1:1 mapping being retained in the original copy of the
> >> page tables.
> >
> > Thanks for this ASCII!
> >
> >> I think the original wait is actually a workaround for lack of firmware
> >> on the fastmodels, and should be implemented by either the firmware or
> >> bootwrapper.
> >
> > BTW, this wait is an issue when the boot CPU ID is not equal to 0.
> >
> > I gave a quick try to move kick cpus after the HYP mode switch in
> > assembly and I'm unable to boot secondary cpus on the Versatile Express.
> > I guess, on the VE secondary cpus can only be "kick" in secure mode.
> 
> Right, but I think this is not specific to the VE, but ARM in general.
> If I remember correctly the reason was that you cannot send IPIs from an 
> non-secure GIC CPU interface to a secure one. So you have to enable the 
> CPU interfaces of the others cores for non-secure operation first, which 
> involves waking them up.
> In my U-Boot code I switch to HYP mode on the BSP at the very end, after 
> having switched the secondary cores to HYP before.

Yes, this is IMHO the right thing to do, firmware which launches the
boot cpu in a mode which cannot kick the other cpus is buggy.

Firmware which launches the boot CPU in NS mode but leaves secondary
CPUs in S world, without leaving some sort of channel (e.g PSCI) to wake
them is irrevocably buggy unfortunately, we cannot support those systems
unless the firmware can be updated.

At least if the firmware launches everything in secure world then u-boot
can fix it.

Ian.