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

[Suriyan Ramasami <suriyan.r@xxxxxxxxx>]

On Tue, Sep 9, 2014 at 2:15 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> 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?
>
Sounds good to me!

>> >> +        { /*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.
>
That should be wonderful! Thank you so much, Ian.
> Ian.
>

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