[v8,3/3] ARM: Check if a CPU has gone offline

PSCIv0.2 adds a new function called AFFINITY_INFO, which
can be used to query if a specified CPU has actually gone
offline. Calling this function via cpu_kill ensures that
a CPU has quiesced after a call to cpu_die.

Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 arch/arm/kernel/psci_smp.c | 32 ++++++++++++++++++++++++++++++++
 include/uapi/linux/psci.h  |  5 +++++
 2 files changed, 37 insertions(+)

Hi Ashwin,

On 26 April 2014 01:18, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote:
> PSCIv0.2 adds a new function called AFFINITY_INFO, which
> can be used to query if a specified CPU has actually gone
> offline. Calling this function via cpu_kill ensures that
> a CPU has quiesced after a call to cpu_die.
>
> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  arch/arm/kernel/psci_smp.c | 32 ++++++++++++++++++++++++++++++++
>  include/uapi/linux/psci.h  |  5 +++++
>  2 files changed, 37 insertions(+)
>
> diff --git a/arch/arm/kernel/psci_smp.c b/arch/arm/kernel/psci_smp.c
> index 570a48c..94e230c 100644
> --- a/arch/arm/kernel/psci_smp.c
> +++ b/arch/arm/kernel/psci_smp.c
> @@ -16,6 +16,8 @@
>  #include <linux/init.h>
>  #include <linux/smp.h>
>  #include <linux/of.h>
> +#include <linux/delay.h>
> +#include <uapi/linux/psci.h>
>
>  #include <asm/psci.h>
>  #include <asm/smp_plat.h>
> @@ -66,6 +68,35 @@ void __ref psci_cpu_die(unsigned int cpu)
>         /* We should never return */
>         panic("psci: cpu %d failed to shutdown\n", cpu);
>  }
> +
> +int __ref psci_cpu_kill(unsigned int cpu)
> +{
> +       int err, i;
> +
> +       if (!psci_ops.affinity_info)
> +               return 1;
> +       /*
> +        * cpu_kill could race with cpu_die and we can
> +        * potentially end up declaring this cpu undead
> +        * while it is dying. So, try again a few times.
> +        */
> +
> +       for (i=0; i<10; i++) {
> +               err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);
> +
> +               if (err == PSCI_AFFINITY_INFO_RET_OFF)
> +                       return 1;
> +
> +               msleep(10);
> +               pr_info("Retrying again to check for CPU kill\n");
> +       }
> +
> +       pr_err("psci: Cannot kill CPU:%d, psci ret val: %d\n",
> +                       cpu, err);
> +       /* Make platform_cpu_kill() fail. */
> +       return 0;
> +}
> +
>  #endif
>
>  bool __init psci_smp_available(void)
> @@ -78,5 +109,6 @@ struct smp_operations __initdata psci_smp_ops = {
>         .smp_boot_secondary     = psci_boot_secondary,
>  #ifdef CONFIG_HOTPLUG_CPU
>         .cpu_die                = psci_cpu_die,
> +       .cpu_kill               = psci_cpu_kill,
>  #endif
>  };
> diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
> index a4136c3..857209b 100644
> --- a/include/uapi/linux/psci.h
> +++ b/include/uapi/linux/psci.h
> @@ -74,4 +74,9 @@
>  #define PSCI_RET_NOT_PRESENT                   -7
>  #define PSCI_RET_DISABLED                      -8
>
> +/* Return values from the PSCI_ID_AFFINITY_INFO Function. */
> +#define PSCI_AFFINITY_INFO_RET_ON                      0
> +#define PSCI_AFFINITY_INFO_RET_OFF                     1
> +#define PSCI_AFFINITY_INFO_RET_ON_PENDING      2

I have already added this return values in v10 patchset for PSCI v0.2
emulation for KVM ARM/ARM64.
(http://www.spinics.net/lists/kvm-arm/msg09062.html)

Please rebase your patch against that.

> +
>  #endif /* _UAPI_LINUX_PSCI_H */
> --
> 1.8.3.2
>

Regards,
Anup

Hello,

On 28 April 2014 02:18, Anup Patel <anup.patel@linaro.org> wrote:
>> --- a/include/uapi/linux/psci.h
>> +++ b/include/uapi/linux/psci.h
>> @@ -74,4 +74,9 @@
>>  #define PSCI_RET_NOT_PRESENT                   -7
>>  #define PSCI_RET_DISABLED                      -8
>>
>> +/* Return values from the PSCI_ID_AFFINITY_INFO Function. */
>> +#define PSCI_AFFINITY_INFO_RET_ON                      0
>> +#define PSCI_AFFINITY_INFO_RET_OFF                     1
>> +#define PSCI_AFFINITY_INFO_RET_ON_PENDING      2
>
> I have already added this return values in v10 patchset for PSCI v0.2
> emulation for KVM ARM/ARM64.
> (http://www.spinics.net/lists/kvm-arm/msg09062.html)
>
> Please rebase your patch against that.


Ok will do.

Whats the merge plan for these patches? We both share a common header
but the code that uses it would seemingly go up via different trees.
armv8 (Catalin) and KVM (Christoffer)?


Cheers,
Ashwin

On 28 April 2014 14:16, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote:
> Hello,
>
> On 28 April 2014 02:18, Anup Patel <anup.patel@linaro.org> wrote:
>>> --- a/include/uapi/linux/psci.h
>>> +++ b/include/uapi/linux/psci.h
>>> @@ -74,4 +74,9 @@
>>>  #define PSCI_RET_NOT_PRESENT                   -7
>>>  #define PSCI_RET_DISABLED                      -8
>>>
>>> +/* Return values from the PSCI_ID_AFFINITY_INFO Function. */
>>> +#define PSCI_AFFINITY_INFO_RET_ON                      0
>>> +#define PSCI_AFFINITY_INFO_RET_OFF                     1
>>> +#define PSCI_AFFINITY_INFO_RET_ON_PENDING      2
>>
>> I have already added this return values in v10 patchset for PSCI v0.2
>> emulation for KVM ARM/ARM64.
>> (http://www.spinics.net/lists/kvm-arm/msg09062.html)
>>
>> Please rebase your patch against that.
>
>
> Ok will do.
>
> Whats the merge plan for these patches? We both share a common header
> but the code that uses it would seemingly go up via different trees.
> armv8 (Catalin) and KVM (Christoffer)?
>
Are they inter-dependent?  I thought your patches depended on Anup's -
not the other way around?

If my assumption is correct, I think we can make sure to send the pull
request to kvm/next early and ask Catalin to pull in kvm/next before
pulling your changes.

I'm cc'ing both Catalin and Paolo here for their advice.

Thanks,
-Christoffer

On 28 April 2014 08:23, Christoffer Dall <christoffer.dall@linaro.org> wrote:

>>
>> Whats the merge plan for these patches? We both share a common header
>> but the code that uses it would seemingly go up via different trees.
>> armv8 (Catalin) and KVM (Christoffer)?
>>
> Are they inter-dependent?  I thought your patches depended on Anup's -
> not the other way around?
>
> If my assumption is correct, I think we can make sure to send the pull
> request to kvm/next early and ask Catalin to pull in kvm/next before
> pulling your changes.


Right. My patches depend on the header (uapi/psci.h) introduced by
Anups patchset.

Cheers,
Ashwin

Il 28/04/2014 14:23, Christoffer Dall ha scritto:
> Are they inter-dependent?  I thought your patches depended on Anup's -
> not the other way around?
>
> If my assumption is correct, I think we can make sure to send the pull
> request to kvm/next early and ask Catalin to pull in kvm/next before
> pulling your changes.
>
> I'm cc'ing both Catalin and Paolo here for their advice.

Is Anup's patchset already on a branch?  If you send me a pointer to it 
I can look at that branch history and see what's best.

Paolo

On Mon, Apr 28, 2014 at 03:03:23PM +0200, Paolo Bonzini wrote:
> Il 28/04/2014 14:23, Christoffer Dall ha scritto:
> >Are they inter-dependent?  I thought your patches depended on Anup's -
> >not the other way around?
> >
> >If my assumption is correct, I think we can make sure to send the pull
> >request to kvm/next early and ask Catalin to pull in kvm/next before
> >pulling your changes.
> >
> >I'm cc'ing both Catalin and Paolo here for their advice.
> 
> Is Anup's patchset already on a branch?  If you send me a pointer to
> it I can look at that branch history and see what's best.
> 
I didn't merge it yet - need to make one final pass over it (likely
later tonight).

But this is the series:
https://lists.cs.columbia.edu/pipermail/kvmarm/2014-April/009061.html

I believe Ashwin's patch set depends only on "ARM/ARM64: KVM: Add common
header for PSCI related defines".

Thanks,
-Christoffer

Il 28/04/2014 15:06, Christoffer Dall ha scritto:
>> >
>> > Is Anup's patchset already on a branch?  If you send me a pointer to
>> > it I can look at that branch history and see what's best.
>> >
> I didn't merge it yet - need to make one final pass over it (likely
> later tonight).
>
> But this is the series:
> https://lists.cs.columbia.edu/pipermail/kvmarm/2014-April/009061.html

Even better. :)

1) Catalin can apply "Add common header for PSCI related defines", or 
Ashwin can resend his series with the patch at the beginning.  Duplicate 
commits are fine, especially for seldom-modified files where they do not 
cause conflicts.

2) Send Anup's patchset as a pull request to both me and Catalin, 
relative to v3.16-rc1, both of us can apply it and Ashwin's series can 
be commit on top.

Paolo

On 28 April 2014 15:48, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 28/04/2014 15:06, Christoffer Dall ha scritto:
>
>>> >
>>> > Is Anup's patchset already on a branch?  If you send me a pointer to
>>> > it I can look at that branch history and see what's best.
>>> >
>>
>> I didn't merge it yet - need to make one final pass over it (likely
>> later tonight).
>>
>> But this is the series:
>> https://lists.cs.columbia.edu/pipermail/kvmarm/2014-April/009061.html
>
>
> Even better. :)
>
> 1) Catalin can apply "Add common header for PSCI related defines", or Ashwin
> can resend his series with the patch at the beginning.  Duplicate commits
> are fine, especially for seldom-modified files where they do not cause
> conflicts.
>
> 2) Send Anup's patchset as a pull request to both me and Catalin, relative
> to v3.16-rc1, both of us can apply it and Ashwin's series can be commit on
> top.
>
(2) sounds cleaner to me, we'll do that

thanks,
-Christoffer