target/arm/arm-powerctl: Fix psci info return values

[Qemu-devel] [PATCH] [RFT] target/arm/arm-powerctl: Fix psci info return values

Posted by Andrew Jones 7 years, 1 month ago

The power state spec section 5.1.5 AFFINITY_INFO defines the
affinity info return values as

  0 ON
  1 OFF
  2 ON_PENDING

I grepped QEMU for power_state to ensure that no assumptions
of OFF=0 were being made.

Signed-off-by: Andrew Jones <drjones@redhat.com>

---

I found this with the yet to be committed kvm-unit-tests test
https://lists.cs.columbia.edu/pipermail/kvmarm/2017-February/023820.html

I've added the RFT because I didn't bother to confirm Linux is still
happy (although I can't see why it wouldn't be happier).

 target/arm/cpu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 25ceaabb5ded..a8aabce7ddd5 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -536,8 +536,8 @@ typedef void ARMELChangeHook(ARMCPU *cpu, void *opaque);
 /* These values map onto the return values for
  * QEMU_PSCI_0_2_FN_AFFINITY_INFO */
 typedef enum ARMPSCIState {
-    PSCI_OFF = 0,
-    PSCI_ON = 1,
+    PSCI_ON = 0,
+    PSCI_OFF = 1,
     PSCI_ON_PENDING = 2
 } ARMPSCIState;
 
-- 
2.9.3

Re: [Qemu-devel] [PATCH] [RFT] target/arm/arm-powerctl: Fix psci info return values

Posted by Peter Maydell 7 years, 1 month ago

On 3 March 2017 at 12:32, Andrew Jones <drjones@redhat.com> wrote:
> The power state spec section 5.1.5 AFFINITY_INFO defines the
> affinity info return values as
>
>   0 ON
>   1 OFF
>   2 ON_PENDING
>
> I grepped QEMU for power_state to ensure that no assumptions
> of OFF=0 were being made.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
>
> ---
>
> I found this with the yet to be committed kvm-unit-tests test
> https://lists.cs.columbia.edu/pipermail/kvmarm/2017-February/023820.html
>
> I've added the RFT because I didn't bother to confirm Linux is still
> happy (although I can't see why it wouldn't be happier).

Nice catch. Without this patch:

root@genericarmv8:~# echo 0 > /sys/devices/system/cpu/cpu1/online
[  112.797621] CPU1: shutdown
[  112.822447] psci: Retrying again to check for CPU kill
[  112.841306] psci: Retrying again to check for CPU kill
[  112.861303] psci: Retrying again to check for CPU kill
[  112.881346] psci: Retrying again to check for CPU kill
[  112.901302] psci: Retrying again to check for CPU kill
[  112.921301] psci: Retrying again to check for CPU kill
[  112.941303] psci: Retrying again to check for CPU kill
[  112.961302] psci: Retrying again to check for CPU kill
[  112.981350] psci: Retrying again to check for CPU kill
[  113.001368] psci: Retrying again to check for CPU kill
[  113.001789] psci: CPU1 may not have shut down cleanly
(AFFINITY_INFO reports 0)
[  113.002429] CPU1 may not have shut down cleanly: -110

With the patch:
root@genericarmv8:~# echo 0 > /sys/devices/system/cpu/cpu1/online
[   83.303563] CPU1: shutdown
[   83.308003] psci: CPU1 killed.

Applied to target-arm.next for 2.9.

thanks
-- PMM