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

[Andre Przywara <andre.przywara@linaro.org>]

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(-)
>>
>
>