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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-09-10 at 20:35 -0700, Suriyan Ramasami wrote:
> On Wed, Sep 10, 2014 at 6:03 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> > Hi Suriyan,
> >
> > On 10/09/14 17:51, Suriyan Ramasami wrote:
> >>
> >> On Wed, Sep 10, 2014 at 3:21 PM, Julien Grall <julien.grall@xxxxxxxxxx>
> >> wrote:
> >>>
> >>> On 08/09/14 10:26, Suriyan Ramasami wrote:
> >>>>
> >>>>
> >>>> 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.
> >>>
> >>>
> >>>
> >>> You half read the Linux code... This offset is only add when there is a
> >>> secure firmware (detected by "samsung,secure-firmware" node in the device
> >>> tree).
> >>>
> >>> On the Arndale the node doesn't exist.
> >>>
> >> Thanks for digging there Julien. Previously, I must have gone through
> >> the linux code with only exynos5410 in mind. Nonetheless, looks like I
> >> have left out the code which handles the arndale and possibly 5420 and
> >> the 5800 (if they do not have "samsung,secure-firmware" defined). But
> >> on the other hand, I do see arndale-octa has the "secure-firmware"
> >> entry which is a 5420 (so does the 5800). This adds to the confusion.
> >> This is from looking at the linux-3.16.y source.
> >>
> >> Nonetheless, I think to handle arndale for now, I should add the
> >> "samsung,secure-firmware" logic in the code which will then use
> >> sysram_base_addr instead without any offset.
> >>
> >> So, how do I go about this? Should I roll out another patch with these
> >> cumulative changes and also add the BUG_ON change?
> >
> >
> > I think you could avoid to check the "samsung,secure-firmware" logic by only
> > replicate arch/arm/mach-exynos/platsmp.c to Xen.
> >
> 
> Hi Julien,
>    I think "samsung,secure-firmware" is what differentiates the code
> path in platsmp.c, from what I can tell. Correct me if I am wrong,
> please. This is from the smp_init's perspective in XEN.
> 
> The code in linux is:
>                 /*
>                  * Try to set boot address using firmware first
>                  * and fall back to boot register if it fails.
>                  */
>                 ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, 
> boot_addr);
>                 if (ret && ret != -ENOSYS)
>                         goto fail;
>                 if (ret == -ENOSYS) {
>                         void __iomem *boot_reg = cpu_boot_reg(phys_cpu);
> 
>                         if (IS_ERR(boot_reg)) {
>                                 ret = PTR_ERR(boot_reg);
>                                 goto fail;
>                         }
>                         __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
>                 }
> 
> call_firmware_op(set_cpu_boot_addr, ...) will return -ENOSYS (which
> then uses the alternative code path of using cpu_boot_reg(), which
> uses sysram_base_addr), only if register_firmware_ops() is not called
> with &exynos_firmware_ops, as per the code in mach-exynos/firmware.c.
> Furthermore, it is not registered in exynos_firmware_init() only if
> "samsung,secure-firmware" is not found in the DT.
> 
> I am just wondering, if you had something else in mind which can be
> achieved without depending on "samsung,secure-firmware". Please let me
> know the alternative.

I think what you suggest sounds plausible. For me the following fixes
boot on my arndale (I mistakenly thought that it worked earlier but I
was testing the wrong thing). If the presence of samsung,secure-firmware
is the way to distinguish the two cases then lets go that way.

Since arndale is used in automated tests I'm inclined towards applying
this patch until a solution is found to detect the Odroid case.

Ian.

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

From 24f74cbb981689b37d6bf83978c628a9fbabe246 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@xxxxxxxxxx>
Date: Thu, 11 Sep 2014 13:55:08 +0100
Subject: [PATCH] xen: arm: fix boot on arndale.

The differences between Arndale and the Odoid-XU are more interesting
than first though, which results in 0bf8ddecb4df "xen/arm: Add
support for the Odroid-XU board." breaking boot on arndale.

Revert back to arndale compatible behaviour while we sort this out.
Specifically we must (counterintuitively) use the regular (!ns)
sysram and the correct offset is 0x0 and 0x1c.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
---
 xen/arch/arm/platforms/exynos5.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
index bc9ae15..22a38f0 100644
--- a/xen/arch/arm/platforms/exynos5.c
+++ b/xen/arch/arm/platforms/exynos5.c
@@ -99,7 +99,7 @@ static int __init exynos5_smp_init(void)
     u64 size;
     int rc;
 
-    node = dt_find_compatible_node(NULL, NULL, "samsung,exynos4210-sysram-ns");
+    node = dt_find_compatible_node(NULL, NULL, "samsung,exynos4210-sysram");
     if ( !node )
     {
         dprintk(XENLOG_ERR, "samsung,exynos4210-sysram-ns missing in DT\n");
@@ -125,7 +125,7 @@ static int __init exynos5_smp_init(void)
 
     printk("Set SYSRAM to %"PRIpaddr" (%p)\n",
            __pa(init_secondary), init_secondary);
-    writel(__pa(init_secondary), sysram + 0x1c);
+    writel(__pa(init_secondary), sysram);
 
     iounmap(sysram);
 
-- 
1.7.10.4




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