Re: [Xen-devel] [XEN/ARM PATCH v6 1/1] Add OdroidXU board (Exynos 5410)

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-09-08 at 10:26 -0700, Suriyan Ramasami wrote:
> On Mon, Sep 8, 2014 at 3:20 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> > On Thu, 2014-09-04 at 15:57 -0700, Suriyan Ramasami wrote:
> >>     XEN/ARM: Add support for the Odroid-XU board.
> >>
> >>     The Odroid-XU from hardkernel is an Exynos 5410 based board.
> >>     This patch adds support to the above said board.
> >
> > Please could you describe a bit more fully the refactoring you've done
> > and the new platform you've introduced. e.g. that we now have a general
> > exynos5 (which currently includes only the 4210) and that the previous
> > 5250 is a separate platform because of A, B, and C which differ from the
> > generic.
> >
> > Assuming there are no other changes to the patch please feel free to
> > just reply here with the updated commit text and I'll fold it in as I
> > commit.
> >
> OK, and here it is ...

[...]
Thanks.

> >> @@ -77,20 +125,127 @@ static int __init exynos5_smp_init(void)
> >>
> >>      printk("Set SYSRAM to %"PRIpaddr" (%p)\n",
> >>             __pa(init_secondary), init_secondary);
> >> -    writel(__pa(init_secondary), sysram);
> >> +    writel(__pa(init_secondary), sysram + 0x1c);
> >
> > I think this 01c offset is new, where does it come from? Was the old
> > code wrong (again, if this just needs a few words in the commit log I
> > can fold them in as I commit).
> >
> This 0x1c comes from the Non Secure memory area that u-boot SPL moves
> its code into. I believe this is the same for all exynos's (i believe
> they have this same code pattern in lowlevel_init.S. This is what
> u-boot copies over to start of IRAM_NS_BASE:
> [...]
> As can be seen, the secondary CPUs check _hotplug_addr for a non zero
> value on first being powered on, or after woken up from a wfe. This
> _hotplug_addr happens to be at offset +0x1c from NS_RAM_BASE.
> Linux mainline too has this hardcoded +0x1c.

So this is basically another difference between booting in S mode and NS
mode, which wasn't accounted for when we removed the
kicking/transitioning code from Xen's head.S?

I'm inclined to add something like this to the commit log:

        The existing SMP bringup code has been broken since 4557c2292854
        "xen: arm: rewrite start of day page table and cpu bring up"
        which moved the arndale CPU kick from secure world to non-secure
        world without updating it to match the new environment.
        Specifically the sysram address remained hardcoded to the S
        sysram address and not the NS sysram address, this is now
        correctly taken from DT. Secondly the offset within the sysram
        where the start address is written is 0x1c for NS bringup,
        rather than 0x0 as it is in S bringup.

Does that sound correct?

> >> +        { /*sentinel*/ },
> >> +    };
> >> +
> >> +    node = dt_find_matching_node(NULL, exynos_dt_pmu_matches);
> >> +    if ( !node )
> >> +    {
> >> +        dprintk(XENLOG_ERR, "samsung,exynos5XXX-pmu missing in DT\n");
> >
> > We have some 4XXX in the list too. Make it exynosXXXX? (likewise the
> > next message which was below but I've trimmed it by mistake).
> >
> I do not see any 4XXX in the pmu list that I have added.

I was being dumb and read exynos54xx as exynos4xxx, sorry.

> Please let me know if I should roll another patch with these comments
> and the minor 'X' change.

If you are happy with my proposed changelog additions then I can drop
the extra X and update the changelog as I commit, no need to resend.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel