[PATCH 0/4] ARM: add PSCI host support

[PATCH 0/4] ARM: add PSCI host support

[Andre Przywara]

Xen did not make use of the host provided ARM PSCI (Power State
Coordination Interface) functionality so far, but relied on platform
specific SMP bringup functions.
This series adds support for PSCI on the host by reading the required
information from the DTB and invoking the appropriate handler when
bringing up each single CPU.
Since PSCI is defined for both ARM32 and ARM64, I put the code in a
file shared by both.
The ARM32 code was tested on Midway, but the ARM64 code was compile
tested only.

This approach seems to be the least intrusive, but one could also use
more of the current ARM64 code by copying the PSCI/spin-table
distinction code to a shared file and use that from both
architectures. However that seems more complicated.

Please take a look and complain ;-)

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>

Andre Przywara (4):
  arm: parse PSCI node from the host device-tree
  arm: add a function to invoke the PSCI handler and use it
  arm: dont give up on EAGAIN if PSCI is defined
  arm64: defer CPU initialization on ARM64 if PSCI is present

 xen/arch/arm/arm32/smpboot.c |  1 -
 xen/arch/arm/arm64/smpboot.c |  7 +++-
 xen/arch/arm/smpboot.c       | 89 +++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 88 insertions(+), 9 deletions(-)

-- 
1.7.12.1

[PATCH 1/4] arm: parse PSCI node from the host device-tree

[Andre Przywara]

The availability of a PSCI handler is advertised in the DTB.
Find and parse the node (described in the Linux device-tree binding)
and save the function number for bringing up a CPU for later usage.
We do some sanity checks, especially we deny using HVC as a callling
method, as it does not make much sense currently under Xen.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 xen/arch/arm/smpboot.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 6c90fa6..97bd414 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -89,6 +89,44 @@ smp_clear_cpu_maps (void)
     cpu_logical_map(0) = READ_SYSREG(MPIDR_EL1) & MPIDR_HWID_MASK;
 }
 
+uint32_t psci_host_cpu_on_nr;
+
+static int __init psci_host_init(void)
+{
+    struct dt_device_node *psci;
+    int ret;
+    const char *prop_str;
+
+    psci = dt_find_compatible_node(NULL, NULL, "arm,psci");
+    if ( !psci )
+        return 1;
+
+    ret = dt_property_read_string(psci, "method", &prop_str);
+    if ( ret )
+    {
+        printk("/psci node does not provide a method (%d)\n", ret);
+        return 2;
+    }
+
+    /* Since Xen runs in HYP all of the time, it does not make sense to
+     * let it call into HYP for PSCI handling, since the handler won't
+     * just be there. So bail out with an error if "smc" is not used.
+     */
+    if ( strcmp(prop_str, "smc") )
+    {
+        printk("/psci method must be smc, but is: \"%s\"\n", prop_str);
+        return 3;
+    }
+
+    if ( !dt_property_read_u32(psci, "cpu_on", &psci_host_cpu_on_nr) )
+    {
+        printk("/psci node is missing the \"cpu_on\" property\n");
+        return 4;
+    }
+
+    return 0;
+}
+
 /* Parse the device tree and build the logical map array containing
  * MPIDR values related to logical cpus
  * Code base on Linux arch/arm/kernel/devtree.c
@@ -107,6 +145,11 @@ void __init smp_init_cpus(void)
     bool_t bootcpu_valid = 0;
     int rc;
 
+    if ( psci_host_init() == 0 )
+    {
+        printk(XENLOG_INFO "Using PSCI for SMP bringup\n");
+    }
+
     if ( (rc = arch_smp_init()) < 0 )
     {
         printk(XENLOG_WARNING "SMP init failed (%d)\n"
-- 
1.7.12.1

Re: [PATCH 1/4] arm: parse PSCI node from the host device-tree

[Ian Campbell]

On Mon, 2013-11-25 at 13:02 +0100, Andre Przywara wrote:
> The availability of a PSCI handler is advertised in the DTB.
> Find and parse the node (described in the Linux device-tree binding)
> and save the function number for bringing up a CPU for later usage.
> We do some sanity checks, especially we deny using HVC as a callling

Too many l's in callling.

> method, as it does not make much sense currently under Xen.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>  xen/arch/arm/smpboot.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index 6c90fa6..97bd414 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -89,6 +89,44 @@ smp_clear_cpu_maps (void)
>      cpu_logical_map(0) = READ_SYSREG(MPIDR_EL1) & MPIDR_HWID_MASK;
>  }
>  
> +uint32_t psci_host_cpu_on_nr;

Can we make this static and have a global "psci_enable" flag to use
elsewhere please.

> +static int __init psci_host_init(void)

s/host// I think? Generally we have e.g. gic_init for the host and
vgic_init for the guest.

> +{
> +    struct dt_device_node *psci;
> +    int ret;
> +    const char *prop_str;
> +
> +    psci = dt_find_compatible_node(NULL, NULL, "arm,psci");
> +    if ( !psci )
> +        return 1;

You've made up some return codes here? Please can you return -Efoo
(whichever one(s) seem appropriate).

If you need to distinguish PSCI not present from PSCI present but buggy
then perhaps use ENODEV or something similarly unique+relevant for the
former case?

> +    ret = dt_property_read_string(psci, "method", &prop_str);
> +    if ( ret )
> +    {
> +        printk("/psci node does not provide a method (%d)\n", ret);
> +        return 2;
> +    }
> +
> +    /* Since Xen runs in HYP all of the time, it does not make sense to
> +     * let it call into HYP for PSCI handling, since the handler won't
> +     * just be there. So bail out with an error if "smc" is not used.
> +     */
> +    if ( strcmp(prop_str, "smc") )
> +    {
> +        printk("/psci method must be smc, but is: \"%s\"\n", prop_str);
> +        return 3;
> +    }
> +
> +    if ( !dt_property_read_u32(psci, "cpu_on", &psci_host_cpu_on_nr) )
> +    {
> +        printk("/psci node is missing the \"cpu_on\" property\n");
> +        return 4;
> +    }

http://www.spinics.net/lists/devicetree/msg05348.html updates the
bindings for PSCI 0.2. I suppose Midway must only support 0.1?

I'd be OK with only supporting 0.1 right now, but it would be useful to
comment either in the code or in the commit message.

(nb: I'm not sure of v4 was the final version of that series)

> +
> +    return 0;
> +}
> +
>  /* Parse the device tree and build the logical map array containing
>   * MPIDR values related to logical cpus
>   * Code base on Linux arch/arm/kernel/devtree.c
> @@ -107,6 +145,11 @@ void __init smp_init_cpus(void)
>      bool_t bootcpu_valid = 0;
>      int rc;
>  
> +    if ( psci_host_init() == 0 )
> +    {
> +        printk(XENLOG_INFO "Using PSCI for SMP bringup\n");
> +    }
> +
>      if ( (rc = arch_smp_init()) < 0 )

arch_smp_init is empty on both platforms. arm32 has a comment "TODO:
PSCI" ;-)

I think we can nuke this function while we are here, since it's only
purpose was as a PSCI placehoder.

Ian.

Re: [PATCH 1/4] arm: parse PSCI node from the host device-tree

[Ian Campbell]

On Tue, 2013-11-26 at 11:12 +0000, Ian Campbell wrote:
> On Mon, 2013-11-25 at 13:02 +0100, Andre Przywara wrote:
> > The availability of a PSCI handler is advertised in the DTB.
> > Find and parse the node (described in the Linux device-tree binding)
> > and save the function number for bringing up a CPU for later usage.
> > We do some sanity checks, especially we deny using HVC as a callling
> 
> Too many l's in callling.
> 
> > method, as it does not make much sense currently under Xen.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> > ---
> >  xen/arch/arm/smpboot.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> > 
> > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> > index 6c90fa6..97bd414 100644
> > --- a/xen/arch/arm/smpboot.c
> > +++ b/xen/arch/arm/smpboot.c
> > @@ -89,6 +89,44 @@ smp_clear_cpu_maps (void)
> >      cpu_logical_map(0) = READ_SYSREG(MPIDR_EL1) & MPIDR_HWID_MASK;
> >  }
> >  
> > +uint32_t psci_host_cpu_on_nr;
> 
> Can we make this static and have a global "psci_enable" flag to use
> elsewhere please.

I think it will have become clear in my comments on patch #0/#2 but also
can we have this in a separate psci.c please.

It would be useful to have a no-psci command line option too, please.

Ian.

Re: [PATCH 1/4] arm: parse PSCI node from the host device-tree

[Andre Przywara]

On 11/26/2013 12:12 PM, Ian Campbell wrote:
> On Mon, 2013-11-25 at 13:02 +0100, Andre Przywara wrote:
>> +
>> +    if ( !dt_property_read_u32(psci, "cpu_on", &psci_host_cpu_on_nr) )
>> +    {
>> +        printk("/psci node is missing the \"cpu_on\" property\n");
>> +        return 4;
>> +    }
>
> http://www.spinics.net/lists/devicetree/msg05348.html updates the
> bindings for PSCI 0.2. I suppose Midway must only support 0.1?
>
> I'd be OK with only supporting 0.1 right now, but it would be useful to
> comment either in the code or in the commit message.
>
> (nb: I'm not sure of v4 was the final version of that series)

If I look at http://www.spinics.net/lists/devicetree/msg12293.html
I would keep it as it is - at least until the binding changes. Hopefully 
there will be a compatibility fallback if cpu_on should change to 
cpu_on-{32,64}. I will talk to Rob about this and will later send a fix 
if that is needed.
Midway provides an implementation conforming to PSCI 0.2, however the 
device tree binding is still the one from the current kernel git HEAD.

>
>> +
>> +    return 0;
>> +}
>> +
>>   /* Parse the device tree and build the logical map array containing
>>    * MPIDR values related to logical cpus
>>    * Code base on Linux arch/arm/kernel/devtree.c
>> @@ -107,6 +145,11 @@ void __init smp_init_cpus(void)
>>       bool_t bootcpu_valid = 0;
>>       int rc;
>>
>> +    if ( psci_host_init() == 0 )
>> +    {
>> +        printk(XENLOG_INFO "Using PSCI for SMP bringup\n");
>> +    }
>> +
>>       if ( (rc = arch_smp_init()) < 0 )
>
> arch_smp_init is empty on both platforms. arm32 has a comment "TODO:
> PSCI" ;-)
>
> I think we can nuke this function while we are here, since it's only
> purpose was as a PSCI placehoder.
>

But arch_smp_init() calls platform_smp_init(), which is not empty for 
the Exynos, VExpress and OMAP5 (it contains the native SMP bringup for 
these platforms).
So shall I skip the superfluous arch_smp_init() and call 
platform_smp_init() directly from smpboot.c?
Or keep it as it is and we clean it up later?

Thanks,
Andre.

[PATCH 2/4] arm: add a function to invoke the PSCI handler and use it

[Andre Przywara]

The PSCI handler is invoked via a secure monitor call with the
arguments defined in registers [1]. Copy the function from the
Linux code and adjust it to work on both ARM32 and ARM64.
Later use that function instead of the generic GIC SEV kick to
actually bring up the secondary CPUs.

[1]: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0022b/index.html

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 xen/arch/arm/arm32/smpboot.c |  1 -
 xen/arch/arm/smpboot.c       | 39 +++++++++++++++++++++++++++++++++++----
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/arm32/smpboot.c b/xen/arch/arm/arm32/smpboot.c
index 88fe8fb..fcf653f 100644
--- a/xen/arch/arm/arm32/smpboot.c
+++ b/xen/arch/arm/arm32/smpboot.c
@@ -10,7 +10,6 @@ int __init arch_smp_init(void)
 
 int __init arch_cpu_init(int cpu, struct dt_device_node *dn)
 {
-    /* TODO handle PSCI init */
     return 0;
 }
 
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 97bd414..44326d8 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -89,6 +89,29 @@ smp_clear_cpu_maps (void)
     cpu_logical_map(0) = READ_SYSREG(MPIDR_EL1) & MPIDR_HWID_MASK;
 }
 
+#ifdef CONFIG_ARM_32
+#define REG_PREFIX "r"
+#else
+#define REG_PREFIX "x"
+#endif
+
+static noinline int __invoke_psci_fn_smc(u32 function_id, u32 arg0, u32 arg1,
+                                         u32 arg2)
+{
+    asm volatile(
+        __asmeq("%0", REG_PREFIX"0")
+        __asmeq("%1", REG_PREFIX"1")
+        __asmeq("%2", REG_PREFIX"2")
+        __asmeq("%3", REG_PREFIX"3")
+        "smc #0"
+        : "+r" (function_id)
+        : "r" (arg0), "r" (arg1), "r" (arg2));
+
+    return function_id;
+}
+
+#undef REG_PREFIX
+
 uint32_t psci_host_cpu_on_nr;
 
 static int __init psci_host_init(void)
@@ -393,10 +416,18 @@ int __cpu_up(unsigned int cpu)
         return rc;
     }
 
-    /* We don't know the GIC ID of the CPU until it has woken up, so just signal
-     * everyone and rely on our own smp_up_cpu gate to ensure only the one we
-     * want gets through. */
-    send_SGI_allbutself(GIC_SGI_EVENT_CHECK);
+    if ( psci_host_cpu_on_nr != 0 )
+    {
+        /* If the DTB provided a PSCI node, use this for kicking the CPUs */
+        __invoke_psci_fn_smc(
+            psci_host_cpu_on_nr, cpu, __pa(init_secondary), 0);
+    } else
+    {
+        /* We don't know the GIC ID of the CPU until it has woken up, so just
+         * signal everyone and rely on our own smp_up_cpu gate to ensure only
+         * the one we want gets through. */
+        send_SGI_allbutself(GIC_SGI_EVENT_CHECK);
+    }
 
     while ( !cpu_online(cpu) )
     {
-- 
1.7.12.1

Re: [PATCH 2/4] arm: add a function to invoke the PSCI handler and use it

[Ian Campbell]

On Mon, 2013-11-25 at 13:02 +0100, Andre Przywara wrote:
> The PSCI handler is invoked via a secure monitor call with the
> arguments defined in registers [1]. Copy the function from the
> Linux code and adjust it to work on both ARM32 and ARM64.
> Later use that function instead of the generic GIC SEV kick to
> actually bring up the secondary CPUs.
> 
> [1]: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0022b/index.html
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>  xen/arch/arm/arm32/smpboot.c |  1 -
>  xen/arch/arm/smpboot.c       | 39 +++++++++++++++++++++++++++++++++++----
>  2 files changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/smpboot.c b/xen/arch/arm/arm32/smpboot.c
> index 88fe8fb..fcf653f 100644
> --- a/xen/arch/arm/arm32/smpboot.c
> +++ b/xen/arch/arm/arm32/smpboot.c
> @@ -10,7 +10,6 @@ int __init arch_smp_init(void)
>  
>  int __init arch_cpu_init(int cpu, struct dt_device_node *dn)
>  {
> -    /* TODO handle PSCI init */
>      return 0;
>  }
>  
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index 97bd414..44326d8 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -89,6 +89,29 @@ smp_clear_cpu_maps (void)
>      cpu_logical_map(0) = READ_SYSREG(MPIDR_EL1) & MPIDR_HWID_MASK;
>  }
>  
> +#ifdef CONFIG_ARM_32
> +#define REG_PREFIX "r"
> +#else
> +#define REG_PREFIX "x"
> +#endif
> +
> +static noinline int __invoke_psci_fn_smc(u32 function_id, u32 arg0, u32 arg1,
> +                                         u32 arg2)

Please can you put this in psci.c and provide wrappers e.g.
psci_cpu_up(). THese can return some suitable errno if PSCI isn't
enabled. Or we could add a psci_enabled() call

Why noinline?

> +{
> +    asm volatile(
> +        __asmeq("%0", REG_PREFIX"0")
> +        __asmeq("%1", REG_PREFIX"1")
> +        __asmeq("%2", REG_PREFIX"2")
> +        __asmeq("%3", REG_PREFIX"3")
> +        "smc #0"
> +        : "+r" (function_id)
> +        : "r" (arg0), "r" (arg1), "r" (arg2));
> +
> +    return function_id;
> +}
> +
> +#undef REG_PREFIX
> +
>  uint32_t psci_host_cpu_on_nr;
>  
>  static int __init psci_host_init(void)
> @@ -393,10 +416,18 @@ int __cpu_up(unsigned int cpu)
>          return rc;
>      }
>  
> -    /* We don't know the GIC ID of the CPU until it has woken up, so just signal
> -     * everyone and rely on our own smp_up_cpu gate to ensure only the one we
> -     * want gets through. */
> -    send_SGI_allbutself(GIC_SGI_EVENT_CHECK);
> +    if ( psci_host_cpu_on_nr != 0 )

We could go for a set of smp function pointers initialised by
psci_init() but I think for now 
	if (psci_cpu_up(cpu, __pa(...)) < 0)
	{
	    /* No PSCI, send a manual ... blah. We don't know the GIC
             * ID, etc etc
             */
            send_SGI...(...)
	}
would be OK? Or could use a psci_enable() call, I have slightly less
preference for that.

> +    {
> +        /* If the DTB provided a PSCI node, use this for kicking the CPUs */
> +        __invoke_psci_fn_smc(
> +            psci_host_cpu_on_nr, cpu, __pa(init_secondary), 0);
> +    } else
> +    {
> +        /* We don't know the GIC ID of the CPU until it has woken up, so just
> +         * signal everyone and rely on our own smp_up_cpu gate to ensure only
> +         * the one we want gets through. */
> +        send_SGI_allbutself(GIC_SGI_EVENT_CHECK);
> +    }
>  
>      while ( !cpu_online(cpu) )
>      {

Re: [PATCH 2/4] arm: add a function to invoke the PSCI handler and use it

[Andre Przywara]

On 11/26/2013 12:18 PM, Ian Campbell wrote:
> On Mon, 2013-11-25 at 13:02 +0100, Andre Przywara wrote:
>> The PSCI handler is invoked via a secure monitor call with the
>> arguments defined in registers [1]. Copy the function from the
>> Linux code and adjust it to work on both ARM32 and ARM64.
>> Later use that function instead of the generic GIC SEV kick to
>> actually bring up the secondary CPUs.
>>
>> [1]: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0022b/index.html
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> ---
>>   xen/arch/arm/arm32/smpboot.c |  1 -
>>   xen/arch/arm/smpboot.c       | 39 +++++++++++++++++++++++++++++++++++----
>>   2 files changed, 35 insertions(+), 5 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm32/smpboot.c b/xen/arch/arm/arm32/smpboot.c
>> index 88fe8fb..fcf653f 100644
>> --- a/xen/arch/arm/arm32/smpboot.c
>> +++ b/xen/arch/arm/arm32/smpboot.c
>> @@ -10,7 +10,6 @@ int __init arch_smp_init(void)
>>
>>   int __init arch_cpu_init(int cpu, struct dt_device_node *dn)
>>   {
>> -    /* TODO handle PSCI init */
>>       return 0;
>>   }
>>
>> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
>> index 97bd414..44326d8 100644
>> --- a/xen/arch/arm/smpboot.c
>> +++ b/xen/arch/arm/smpboot.c
>> @@ -89,6 +89,29 @@ smp_clear_cpu_maps (void)
>>       cpu_logical_map(0) = READ_SYSREG(MPIDR_EL1) & MPIDR_HWID_MASK;
>>   }
>>
>> +#ifdef CONFIG_ARM_32
>> +#define REG_PREFIX "r"
>> +#else
>> +#define REG_PREFIX "x"
>> +#endif
>> +
>> +static noinline int __invoke_psci_fn_smc(u32 function_id, u32 arg0, u32 arg1,
>> +                                         u32 arg2)
>
> Please can you put this in psci.c and provide wrappers e.g.
> psci_cpu_up(). THese can return some suitable errno if PSCI isn't
> enabled. Or we could add a psci_enabled() call
>
> Why noinline?

This was copied from Linux and I thought it was there for a reason, so I 
kept it. But looking closer this was to make sure Linux can use function 
pointers later (to abstract hvc and smc calling), so we can remove this 
for Xen.

Regards,
Andre.

[PATCH 3/4] arm: dont give up on EAGAIN if PSCI is defined

[Andre Przywara]

Currently the platforms define an empty, zero-returning cpu_up
function to state that they don't need any special treatment beside
the GIC SEV kick to come up.
Allow platforms which only provide PSCI to not define a platform
specific cpu_up() function at all. For this we need to handle the
EAGAIN error code that the platform returns in this case and ignore
that if a PSCI node was found in the DTB.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 xen/arch/arm/smpboot.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 44326d8..14774c5 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -412,8 +412,11 @@ int __cpu_up(unsigned int cpu)
 
     if ( rc < 0 )
     {
-        printk("Failed to bring up CPU%d\n", cpu);
-        return rc;
+        if ( rc != -EAGAIN || psci_host_cpu_on_nr == 0 )
+        {
+            printk("Failed to bring up CPU%d\n", cpu);
+            return rc;
+        }
     }
 
     if ( psci_host_cpu_on_nr != 0 )
-- 
1.7.12.1

Re: [PATCH 3/4] arm: dont give up on EAGAIN if PSCI is defined

[Ian Campbell]

On Mon, 2013-11-25 at 13:02 +0100, Andre Przywara wrote:
> Currently the platforms define an empty, zero-returning cpu_up
> function to state that they don't need any special treatment beside
> the GIC SEV kick to come up.
> Allow platforms which only provide PSCI to not define a platform
> specific cpu_up() function at all. For this we need to handle the
> EAGAIN error code that the platform returns in this case and ignore
> that if a PSCI node was found in the DTB.

I think we should only call the arch hook if PSCI is not enabled.

Either that or we should only try PSCI if there is no arch hook.

I think probably the former.

I think the call to arch_cpu_up can be moved inside the PSCI conditional
which follows in the patch context e.g. just before the SGI kick.

> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>  xen/arch/arm/smpboot.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index 44326d8..14774c5 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -412,8 +412,11 @@ int __cpu_up(unsigned int cpu)
>  
>      if ( rc < 0 )
>      {
> -        printk("Failed to bring up CPU%d\n", cpu);
> -        return rc;
> +        if ( rc != -EAGAIN || psci_host_cpu_on_nr == 0 )
> +        {
> +            printk("Failed to bring up CPU%d\n", cpu);
> +            return rc;
> +        }
>      }
>  
>      if ( psci_host_cpu_on_nr != 0 )

[PATCH 4/4] arm64: defer CPU initialization on ARM64 if PSCI is present

[Andre Przywara]

Since PSCI is now handled in ARM generic smpboot code, we can remove
the PSCI todo from the ARM64 specific code and defer the actual PSCI
handling.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 xen/arch/arm/arm64/smpboot.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c
index 8239590..715c44e 100644
--- a/xen/arch/arm/arm64/smpboot.c
+++ b/xen/arch/arm/arm64/smpboot.c
@@ -61,8 +61,11 @@ int __init arch_cpu_init(int cpu, struct dt_device_node *dn)
 
     if ( !strcmp(enable_method, "spin-table") )
         smp_spin_table_init(cpu, dn);
-    /* TODO: method "psci" */
-    else
+    else if ( !strcmp(enable_method, "psci") )
+    {
+        /* PSCI code is handled somewhere else */
+        return -EAGAIN;
+    } else
     {
         printk("CPU%d has unknown enable method \"%s\"\n", cpu, enable_method);
         return -EINVAL;
-- 
1.7.12.1

Re: [PATCH 4/4] arm64: defer CPU initialization on ARM64 if PSCI is present

[Ian Campbell]

On Mon, 2013-11-25 at 13:02 +0100, Andre Przywara wrote:
> Since PSCI is now handled in ARM generic smpboot code, we can remove
> the PSCI todo from the ARM64 specific code and defer the actual PSCI
> handling.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>  xen/arch/arm/arm64/smpboot.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c
> index 8239590..715c44e 100644
> --- a/xen/arch/arm/arm64/smpboot.c
> +++ b/xen/arch/arm/arm64/smpboot.c
> @@ -61,8 +61,11 @@ int __init arch_cpu_init(int cpu, struct dt_device_node *dn)
>  
>      if ( !strcmp(enable_method, "spin-table") )
>          smp_spin_table_init(cpu, dn);
> -    /* TODO: method "psci" */
> -    else
> +    else if ( !strcmp(enable_method, "psci") )
> +    {
> +        /* PSCI code is handled somewhere else */
> +        return -EAGAIN;

I think this should check for the presence of PSCI and either return an
error or success (on the basis that nothing needs doing).

Ian.

Re: [PATCH 0/4] ARM: add PSCI host support

[George Dunlap]

On Mon, Nov 25, 2013 at 12:02 PM, Andre Przywara
<andre.przywara@linaro.org> wrote:
> Xen did not make use of the host provided ARM PSCI (Power State
> Coordination Interface) functionality so far, but relied on platform
> specific SMP bringup functions.
> This series adds support for PSCI on the host by reading the required
> information from the DTB and invoking the appropriate handler when
> bringing up each single CPU.
> Since PSCI is defined for both ARM32 and ARM64, I put the code in a
> file shared by both.
> The ARM32 code was tested on Midway, but the ARM64 code was compile
> tested only.
>
> This approach seems to be the least intrusive, but one could also use
> more of the current ARM64 code by copying the PSCI/spin-table
> distinction code to a shared file and use that from both
> architectures. However that seems more complicated.
>
> Please take a look and complain ;-)
>
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>

Ian, do you agree that this is too late for 4.4?

 -George

Re: [PATCH 0/4] ARM: add PSCI host support

[Ian Campbell]

On Mon, 2013-11-25 at 13:00 +0000, George Dunlap wrote:
> On Mon, Nov 25, 2013 at 12:02 PM, Andre Przywara
> <andre.przywara@linaro.org> wrote:
> > Xen did not make use of the host provided ARM PSCI (Power State
> > Coordination Interface) functionality so far, but relied on platform
> > specific SMP bringup functions.
> > This series adds support for PSCI on the host by reading the required
> > information from the DTB and invoking the appropriate handler when
> > bringing up each single CPU.
> > Since PSCI is defined for both ARM32 and ARM64, I put the code in a
> > file shared by both.
> > The ARM32 code was tested on Midway, but the ARM64 code was compile
> > tested only.
> >
> > This approach seems to be the least intrusive, but one could also use
> > more of the current ARM64 code by copying the PSCI/spin-table
> > distinction code to a shared file and use that from both
> > architectures. However that seems more complicated.
> >
> > Please take a look and complain ;-)
> >
> > Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> 
> Ian, do you agree that this is too late for 4.4?

I'm in two minds. On the one hand none of the existing platforms
currently require this functionality, so it has clearly not been
necessary up to now.

On the other hand it plays into the strategy of allowing people to
trivially support their platform, and since it is a standard way to do
power control on ARM (albeit quite new and so far uptake is not huge) I
think it is expected that many new platforms will use it.

Of our current platforms Midway can optionally use PSCI (we have
"native" code at the minute) and sunxi is going to need it whenever SMP
is enabled (patches to u-boot are circulating now).

I'm inclined towards punting on this for 4.4.0 but be open to the idea
of adding it in 4.4.1 if it turns out to be something that people are
needing in practice..

An alternative could be requiring for 4.4 that the platform code
explicitly call into/request PSCI for 4.4 and only move to automatically
using it in the absence of the platform code saying otherwise for 4.5.
This has the advantage of being zero risk, but the downside of not being
very well tested (we could enable it for Midway, with the attendant
increase in risk).

Ian.

Re: [PATCH 0/4] ARM: add PSCI host support

[Andre Przywara]

On 11/25/2013 03:03 PM, Ian Campbell wrote:
> On Mon, 2013-11-25 at 13:00 +0000, George Dunlap wrote:
>> On Mon, Nov 25, 2013 at 12:02 PM, Andre Przywara
>> <andre.przywara@linaro.org> wrote:
>>> Xen did not make use of the host provided ARM PSCI (Power State
>>> Coordination Interface) functionality so far, but relied on platform
>>> specific SMP bringup functions.
>>> This series adds support for PSCI on the host by reading the required
>>> information from the DTB and invoking the appropriate handler when
>>> bringing up each single CPU.
>>> Since PSCI is defined for both ARM32 and ARM64, I put the code in a
>>> file shared by both.
>>> The ARM32 code was tested on Midway, but the ARM64 code was compile
>>> tested only.
>>>
>>> This approach seems to be the least intrusive, but one could also use
>>> more of the current ARM64 code by copying the PSCI/spin-table
>>> distinction code to a shared file and use that from both
>>> architectures. However that seems more complicated.
>>>
>>> Please take a look and complain ;-)
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>>
>> Ian, do you agree that this is too late for 4.4?
>
> I'm in two minds. On the one hand none of the existing platforms
> currently require this functionality, so it has clearly not been
> necessary up to now.
>
> On the other hand it plays into the strategy of allowing people to
> trivially support their platform, and since it is a standard way to do
> power control on ARM (albeit quite new and so far uptake is not huge) I
> think it is expected that many new platforms will use it.
>
> Of our current platforms Midway can optionally use PSCI (we have
> "native" code at the minute)

but which is not upstream yet, right?
So if you are considering dropping PSCI for 4.4, I'd like to know so 
that I can ack Julien's "native" SMP patch.
I hope at least this patch can make it for 4.4?

> and sunxi is going to need it whenever SMP
> is enabled (patches to u-boot are circulating now).

That is a good point. We would get sunxi SMP support basically for free, 
also the timing for this is independent of any Xen release cycle.

> I'm inclined towards punting on this for 4.4.0 but be open to the idea
> of adding it in 4.4.1 if it turns out to be something that people are
> needing in practice.

4.4.1 sounds OK for me. But it would be nice to have the native SMP 
support then in 4.4.0.

> An alternative could be requiring for 4.4 that the platform code
> explicitly call into/request PSCI for 4.4 and only move to automatically
> using it in the absence of the platform code saying otherwise for 4.5.

So you are thinking about a change in the priorities? The Linux kernel 
prefers PSCI over a native method, which is how I modeled the Xen patch 
also. This has the advantage of having control in the DTB, so if PSCI 
fails in Xen, one could do "fdt rm /psci" in u-boot to get the old 
behavior back.

> This has the advantage of being zero risk, but the downside of not being
> very well tested (we could enable it for Midway, with the attendant
> increase in risk).

So are you concerned about one of the existing platforms breaking SMP as 
soon as it gets PSCI support? One could change the patch to only use 
PSCI if platform_cpu_up() does _not_ return an explicit "ignore PSCI" 
value, if that helps.

Regards,
Andre.

Re: [PATCH 0/4] ARM: add PSCI host support

[Ian Campbell]

On Mon, 2013-11-25 at 15:21 +0100, Andre Przywara wrote:
> On 11/25/2013 03:03 PM, Ian Campbell wrote:
> > On Mon, 2013-11-25 at 13:00 +0000, George Dunlap wrote:
> >> On Mon, Nov 25, 2013 at 12:02 PM, Andre Przywara
> >> <andre.przywara@linaro.org> wrote:
> >>> Xen did not make use of the host provided ARM PSCI (Power State
> >>> Coordination Interface) functionality so far, but relied on platform
> >>> specific SMP bringup functions.
> >>> This series adds support for PSCI on the host by reading the required
> >>> information from the DTB and invoking the appropriate handler when
> >>> bringing up each single CPU.
> >>> Since PSCI is defined for both ARM32 and ARM64, I put the code in a
> >>> file shared by both.
> >>> The ARM32 code was tested on Midway, but the ARM64 code was compile
> >>> tested only.
> >>>
> >>> This approach seems to be the least intrusive, but one could also use
> >>> more of the current ARM64 code by copying the PSCI/spin-table
> >>> distinction code to a shared file and use that from both
> >>> architectures. However that seems more complicated.
> >>>
> >>> Please take a look and complain ;-)
> >>>
> >>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> >>
> >> Ian, do you agree that this is too late for 4.4?
> >
> > I'm in two minds. On the one hand none of the existing platforms
> > currently require this functionality, so it has clearly not been
> > necessary up to now.
> >
> > On the other hand it plays into the strategy of allowing people to
> > trivially support their platform, and since it is a standard way to do
> > power control on ARM (albeit quite new and so far uptake is not huge) I
> > think it is expected that many new platforms will use it.
> >
> > Of our current platforms Midway can optionally use PSCI (we have
> > "native" code at the minute)
> 
> but which is not upstream yet, right?

Oh right, I forgot it was still waiting for an Ack from you and thought
I'd committed it when I had not.

> So if you are considering dropping PSCI for 4.4, I'd like to know so 
> that I can ack Julien's "native" SMP patch.
> I hope at least this patch can make it for 4.4?

Yes, one or the other should definitely go in for 4.4. It changes the
argument for the PSCI stuff a bit too, since we can now enable midway
and make it easier for other platforms at the same time.

[...]
> > An alternative could be requiring for 4.4 that the platform code
> > explicitly call into/request PSCI for 4.4 and only move to automatically
> > using it in the absence of the platform code saying otherwise for 4.5.
> 
> So you are thinking about a change in the priorities?

I was only suggesting as a way to mitigate risk for 4.4 -- long term we
should certainly do as Linux does and prefer PSCI. (I confess I wasn't
sure how this manifests in Linux, if its at odds with what I wrote
then ...oops)

>  The Linux kernel 
> prefers PSCI over a native method, which is how I modeled the Xen patch 
> also. This has the advantage of having control in the DTB, so if PSCI 
> fails in Xen, one could do "fdt rm /psci" in u-boot to get the old 
> behavior back.
> 
> > This has the advantage of being zero risk, but the downside of not being
> > very well tested (we could enable it for Midway, with the attendant
> > increase in risk).
> 
> So are you concerned about one of the existing platforms breaking SMP as 
> soon as it gets PSCI support? One could change the patch to only use 
> PSCI if platform_cpu_up() does _not_ return an explicit "ignore PSCI" 
> value, if that helps.

I'm addressing George's concerns as release manager about the risk of
taking any sort of PSCI patches at this stage.

Ian.

Re: [PATCH 0/4] ARM: add PSCI host support

[Andre Przywara]

On 11/25/2013 03:50 PM, Ian Campbell wrote:
> On Mon, 2013-11-25 at 15:21 +0100, Andre Przywara wrote:
>> On 11/25/2013 03:03 PM, Ian Campbell wrote:
>>> On Mon, 2013-11-25 at 13:00 +0000, George Dunlap wrote:
>>>> On Mon, Nov 25, 2013 at 12:02 PM, Andre Przywara
>>>> <andre.przywara@linaro.org> wrote:
>>>>> Xen did not make use of the host provided ARM PSCI (Power State
>>>>> Coordination Interface) functionality so far, but relied on platform
>>>>> specific SMP bringup functions.
>>>>> This series adds support for PSCI on the host by reading the required
>>>>> information from the DTB and invoking the appropriate handler when
>>>>> bringing up each single CPU.
>>>>> Since PSCI is defined for both ARM32 and ARM64, I put the code in a
>>>>> file shared by both.
>>>>> The ARM32 code was tested on Midway, but the ARM64 code was compile
>>>>> tested only.
>>>>>
>>>>> This approach seems to be the least intrusive, but one could also use
>>>>> more of the current ARM64 code by copying the PSCI/spin-table
>>>>> distinction code to a shared file and use that from both
>>>>> architectures. However that seems more complicated.
>>>>>
>>>>> Please take a look and complain ;-)
>>>>>
>>>>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>>>>
>>>> Ian, do you agree that this is too late for 4.4?
>>>
>>> I'm in two minds. On the one hand none of the existing platforms
>>> currently require this functionality, so it has clearly not been
>>> necessary up to now.
>>>
>>> On the other hand it plays into the strategy of allowing people to
>>> trivially support their platform, and since it is a standard way to do
>>> power control on ARM (albeit quite new and so far uptake is not huge) I
>>> think it is expected that many new platforms will use it.
>>>
>>> Of our current platforms Midway can optionally use PSCI (we have
>>> "native" code at the minute)
>>
>> but which is not upstream yet, right?
>
> Oh right, I forgot it was still waiting for an Ack from you and thought
> I'd committed it when I had not.

I deliberately held back my ACK: on this one to give PSCI a chance, 
since it turned out to be easier than I thought.
Technically I am OK with Julien's patch, so I can ACK it as well if you 
like.

>> So if you are considering dropping PSCI for 4.4, I'd like to know so
>> that I can ack Julien's "native" SMP patch.
>> I hope at least this patch can make it for 4.4?
>
> Yes, one or the other should definitely go in for 4.4. It changes the
> argument for the PSCI stuff a bit too, since we can now enable midway
> and make it easier for other platforms at the same time.

That was my thinking. But I see both George's and your point with a 
release manager's hat on, so I am OK with whatever you decide.

Thanks for caring!
Andre

> [...]
>>> An alternative could be requiring for 4.4 that the platform code
>>> explicitly call into/request PSCI for 4.4 and only move to automatically
>>> using it in the absence of the platform code saying otherwise for 4.5.
>>
>> So you are thinking about a change in the priorities?
>
> I was only suggesting as a way to mitigate risk for 4.4 -- long term we
> should certainly do as Linux does and prefer PSCI. (I confess I wasn't
> sure how this manifests in Linux, if its at odds with what I wrote
> then ...oops)
>>   The Linux kernel
>> prefers PSCI over a native method, which is how I modeled the Xen patch
>> also. This has the advantage of having control in the DTB, so if PSCI
>> fails in Xen, one could do "fdt rm /psci" in u-boot to get the old
>> behavior back.
>>
>>> This has the advantage of being zero risk, but the downside of not being
>>> very well tested (we could enable it for Midway, with the attendant
>>> increase in risk).
>>
>> So are you concerned about one of the existing platforms breaking SMP as
>> soon as it gets PSCI support? One could change the patch to only use
>> PSCI if platform_cpu_up() does _not_ return an explicit "ignore PSCI"
>> value, if that helps.
>
> I'm addressing George's concerns as release manager about the risk of
> taking any sort of PSCI patches at this stage.
>
> Ian.
>

Re: [PATCH 0/4] ARM: add PSCI host support

[George Dunlap]

On 11/25/2013 02:03 PM, Ian Campbell wrote:
> On Mon, 2013-11-25 at 13:00 +0000, George Dunlap wrote:
>> On Mon, Nov 25, 2013 at 12:02 PM, Andre Przywara
>> <andre.przywara@linaro.org> wrote:
>>> Xen did not make use of the host provided ARM PSCI (Power State
>>> Coordination Interface) functionality so far, but relied on platform
>>> specific SMP bringup functions.
>>> This series adds support for PSCI on the host by reading the required
>>> information from the DTB and invoking the appropriate handler when
>>> bringing up each single CPU.
>>> Since PSCI is defined for both ARM32 and ARM64, I put the code in a
>>> file shared by both.
>>> The ARM32 code was tested on Midway, but the ARM64 code was compile
>>> tested only.
>>>
>>> This approach seems to be the least intrusive, but one could also use
>>> more of the current ARM64 code by copying the PSCI/spin-table
>>> distinction code to a shared file and use that from both
>>> architectures. However that seems more complicated.
>>>
>>> Please take a look and complain ;-)
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> Ian, do you agree that this is too late for 4.4?
> I'm in two minds. On the one hand none of the existing platforms
> currently require this functionality, so it has clearly not been
> necessary up to now.
>
> On the other hand it plays into the strategy of allowing people to
> trivially support their platform, and since it is a standard way to do
> power control on ARM (albeit quite new and so far uptake is not huge) I
> think it is expected that many new platforms will use it.
>
> Of our current platforms Midway can optionally use PSCI (we have
> "native" code at the minute) and sunxi is going to need it whenever SMP
> is enabled (patches to u-boot are circulating now).
>
> I'm inclined towards punting on this for 4.4.0 but be open to the idea
> of adding it in 4.4.1 if it turns out to be something that people are
> needing in practice..

At some point every cost/benefits analysis comes down to a judgement 
call; in cases where the release coordinator doesn't have the experience 
to make the call themselves, their job should be to help set the 
criteria, ask questions, and clarify the thinking.

Both arguments -- "We should risk including this because it will enable 
other platforms, in particular sunxi", and "We should wait until 4.4.1", 
sound reasonable to me.  So I think I'll have to leave it up to you to 
judge which is a better bet at this point.  :-)

  -George

Re: [PATCH 0/4] ARM: add PSCI host support

[Ian Campbell]

On Mon, 2013-11-25 at 16:35 +0000, George Dunlap wrote:
> At some point every cost/benefits analysis comes down to a judgement 
> call; in cases where the release coordinator doesn't have the experience 
> to make the call themselves, their job should be to help set the 
> criteria, ask questions, and clarify the thinking.
> 
> Both arguments -- "We should risk including this because it will enable 
> other platforms, in particular sunxi", and "We should wait until 4.4.1", 
> sound reasonable to me.  So I think I'll have to leave it up to you to 
> judge which is a better bet at this point.  :-)

I think given that I was mistaken about the native Midway SMP support
being in we should take either that patch or this PSCI series and since
PSCI also enables other platforms I think it should be this one.

Ian.

Re: [PATCH 0/4] ARM: add PSCI host support

[Ian Campbell]

On Mon, 2013-11-25 at 13:02 +0100, Andre Przywara wrote:
> Xen did not make use of the host provided ARM PSCI (Power State
> Coordination Interface) functionality so far, but relied on platform
> specific SMP bringup functions.
> This series adds support for PSCI on the host by reading the required
> information from the DTB and invoking the appropriate handler when
> bringing up each single CPU.
> Since PSCI is defined for both ARM32 and ARM64, I put the code in a
> file shared by both.
> The ARM32 code was tested on Midway, but the ARM64 code was compile
> tested only.
> 
> This approach seems to be the least intrusive, but one could also use
> more of the current ARM64 code by copying the PSCI/spin-table
> distinction code to a shared file and use that from both
> architectures. However that seems more complicated.

I don't think that is needed (since armv7 spintable vs armv8 spintable
mechanisms are a bit different). But I would like to see the psci code
in a separate psci.c. It's not much code right now but it will likely
grow.

I'll comment a bit more on the individual patches shortly.

Thanks.

Ian.
> 
> Please take a look and complain ;-)
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> 
> Andre Przywara (4):
>   arm: parse PSCI node from the host device-tree
>   arm: add a function to invoke the PSCI handler and use it
>   arm: dont give up on EAGAIN if PSCI is defined
>   arm64: defer CPU initialization on ARM64 if PSCI is present
> 
>  xen/arch/arm/arm32/smpboot.c |  1 -
>  xen/arch/arm/arm64/smpboot.c |  7 +++-
>  xen/arch/arm/smpboot.c       | 89 +++++++++++++++++++++++++++++++++++++++++---
>  3 files changed, 88 insertions(+), 9 deletions(-)
> 

Re: [PATCH 0/4] ARM: add PSCI host support

[Andre Przywara]

On 11/26/2013 12:05 PM, Ian Campbell wrote:
> On Mon, 2013-11-25 at 13:02 +0100, Andre Przywara wrote:
>> Xen did not make use of the host provided ARM PSCI (Power State
>> Coordination Interface) functionality so far, but relied on platform
>> specific SMP bringup functions.
>> This series adds support for PSCI on the host by reading the required
>> information from the DTB and invoking the appropriate handler when
>> bringing up each single CPU.
>> Since PSCI is defined for both ARM32 and ARM64, I put the code in a
>> file shared by both.
>> The ARM32 code was tested on Midway, but the ARM64 code was compile
>> tested only.
>>
>> This approach seems to be the least intrusive, but one could also use
>> more of the current ARM64 code by copying the PSCI/spin-table
>> distinction code to a shared file and use that from both
>> architectures. However that seems more complicated.

Ian,

thanks for the review and for the willingness to take the patches for 
4.4 still. Will address the comments ASAP.

> I don't think that is needed (since armv7 spintable vs armv8 spintable
> mechanisms are a bit different). But I would like to see the psci code
> in a separate psci.c. It's not much code right now but it will likely
> grow.

But there is already a xen/arch/arm/psci.c file, which holds the two 
functions to bring up/take down vCPUs. Not much in here, I could easily 
add my code there, but the scope of those two is quite different and I 
would find it strange to have both host and guest PSCI functions in one 
file.
That was one reason to not create a new file, the other was that I found 
smpboot.c an obvious place to keep functions for SMP bringup in.

So should I use psci_host.c? Or keep it in smpboot.c (at least for the 
time being)?

And beside that there is quite some other (only guest related) code in 
Xen which simply uses *psci* in identifiers and filenames (like 
asm/psci.h), that's why I used the psci_host prefix to tell them apart.

Regards,
Andre.

>
> I'll comment a bit more on the individual patches shortly.
>
> Thanks.
>
> Ian.
>>
>> Please take a look and complain ;-)
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>>
>> Andre Przywara (4):
>>    arm: parse PSCI node from the host device-tree
>>    arm: add a function to invoke the PSCI handler and use it
>>    arm: dont give up on EAGAIN if PSCI is defined
>>    arm64: defer CPU initialization on ARM64 if PSCI is present
>>
>>   xen/arch/arm/arm32/smpboot.c |  1 -
>>   xen/arch/arm/arm64/smpboot.c |  7 +++-
>>   xen/arch/arm/smpboot.c       | 89 +++++++++++++++++++++++++++++++++++++++++---
>>   3 files changed, 88 insertions(+), 9 deletions(-)
>>
>
>

Re: [PATCH 0/4] ARM: add PSCI host support

[Ian Campbell]

On Wed, 2013-11-27 at 14:45 +0100, Andre Przywara wrote:
> On 11/26/2013 12:05 PM, Ian Campbell wrote:
> > On Mon, 2013-11-25 at 13:02 +0100, Andre Przywara wrote:
> >> Xen did not make use of the host provided ARM PSCI (Power State
> >> Coordination Interface) functionality so far, but relied on platform
> >> specific SMP bringup functions.
> >> This series adds support for PSCI on the host by reading the required
> >> information from the DTB and invoking the appropriate handler when
> >> bringing up each single CPU.
> >> Since PSCI is defined for both ARM32 and ARM64, I put the code in a
> >> file shared by both.
> >> The ARM32 code was tested on Midway, but the ARM64 code was compile
> >> tested only.
> >>
> >> This approach seems to be the least intrusive, but one could also use
> >> more of the current ARM64 code by copying the PSCI/spin-table
> >> distinction code to a shared file and use that from both
> >> architectures. However that seems more complicated.
> 
> Ian,
> 
> thanks for the review and for the willingness to take the patches for 
> 4.4 still. Will address the comments ASAP.
> 
> > I don't think that is needed (since armv7 spintable vs armv8 spintable
> > mechanisms are a bit different). But I would like to see the psci code
> > in a separate psci.c. It's not much code right now but it will likely
> > grow.
> 
> But there is already a xen/arch/arm/psci.c file, which holds the two 
> functions to bring up/take down vCPUs.

Ah, sorry, I didn't know about this.

This file should be called vpscsi.c IMHO with the non-v version for the
host stuff. Do you mind renaming as you go?

[...]
> And beside that there is quite some other (only guest related) code in 
> Xen which simply uses *psci* in identifiers and filenames (like 
> asm/psci.h), that's why I used the psci_host prefix to tell them apart.

Urk. I think the #defines are common since they are part of the spec and
the do_pscsi could stand being do_vpscsi. Probably no need for vpsci.h
here.

I seem to be unable to type PSCI without typing PSCSI instead today,
sorry...

Ian.