Re: [RFC 0/3] New idle device-tree format and support for versioned stop state

2019-08-27 Thread Nicholas Piggin
Abhishek Goel's on August 23, 2019 5:09 pm:
> Background
> --
> 
> Previously if a older kernel runs on a newer firmware, it may enable
> all available states irrespective of its capability of handling it.
> Consider a case that some stop state has a bug, we end up disabling all
> the stop states. This patch introduces selective control to solve this
> problem.
> 
> Previous version of these patches can be found at:
> https://lkml.org/lkml/2018/10/11/544
> These patch however also had patches for support of opal save-restore
> which now I am decoupling and will take them seperately.
> I have posted the corresponding skiboot patches for this kernel patchset
> here : https://patchwork.ozlabs.org/cover/1144587/
> 
> What's new?
> 
> 
> Add stop states under ibm,idle-states in addition to the current array
> based device tree properties.
> 
> New device tree format adds a compatible flag which has version
> corresponding to every state, so that only kernel which has the capability
> to handle the version of stop state will enable it. Drawback of the array
> based dt node is that versioning of idle states is not possible.
> 
> Older kernel will still see stop0 and stop0_lite in older format and we
> will deprecate it after some time.
> 
> Consider a case that stop4 has a bug. We take the following steps to
> mitigate the problem.
> 
> 1) Change compatible string for stop4 in OPAL to "stop4,v2" from
> "stop4,v1", i.e. basicallly bump up the previous version and ship the
> new firmware.
> 
> 2) The kernel will ignore stop4 as it won't be able to recognize this
> new version. Kernel will also ignore all the deeper states because its
> possible that a cpu have requested for a deeper state but was never able
> to enter into it. But we will still have shallower states that are there
> before stop 4. This, thus prevents from completely disabling stop states.
> 
> Linux kernel can now look at the version string and decide if it has the
> ability to handle that idle state. Henceforth, if kernel does not know
> about a version, it will skip that state and all the deeper state.
> 
> Once when the workaround are implemented into the kernel, we can bump up
> the known version in kernel for that state, so that support can be
> enabled once again in kernel.
> 
> New Device-tree :
> 
> Final output
>power-mgt {
> ...
>  ibm,enabled-stop-levels = <0xec00>;
>  ibm,cpu-idle-state-psscr-mask = <0x0 0x3003ff 0x0 0x3003ff>;
>  ibm,cpu-idle-state-latencies-ns = <0x3e8 0x7d0>;
>  ibm,cpu-idle-state-psscr = <0x0 0x330 0x0 0x300330>;
>  ibm,cpu-idle-state-flags = <0x10 0x101000>;
>  ibm,cpu-idle-state-residency-ns = <0x2710 0x4e20>;
>  ibm,idle-states {
>  stop4 {
>  flags = <0x207000>;
>  compatible = "stop4,v1",
>  psscr-mask = <0x0 0x3003ff>;
>  latency-ns = <0x186a0>;
>  residency-ns = <0x989680>;
>  psscr = <0x0 0x300374>;
>...
>   };
> ...
> stop11 {
>  ...
>  compatible = "stop11,v1",
>  ...
>   };
>  };

I'm not sure about this. I think the real problem we have is that the
OPAL stop API is not actually implementation independent. Because we
*can* selectively disable stop states in the firmware if there is a
hardware/firmware problem with them.

So we need a way to rev ISA/Book4/OPAL stuff so the kernel won't try
to use it if it's incapable.

We have that today in dt-cpu-ftrs. POWER9 advertises the "idle-stop"
feature. An incompatible implementation could advertise idle-stop-v4.

That won't allow individual states to remain back compatible, but if 
there is a new implementation which changes any behaviour it needs to be 
incopmatible, even if Linux happens to not rely on said behaviour yet.  
So I don't think we can keep any of them (except stop0 lite) compatible, 
so I don't see a real downside (we'll have to discuss this offline).

Thanks,
Nick


[RFC 0/3] New idle device-tree format and support for versioned stop state

2019-08-23 Thread Abhishek Goel
Background
--

Previously if a older kernel runs on a newer firmware, it may enable
all available states irrespective of its capability of handling it.
Consider a case that some stop state has a bug, we end up disabling all
the stop states. This patch introduces selective control to solve this
problem.

Previous version of these patches can be found at:
https://lkml.org/lkml/2018/10/11/544
These patch however also had patches for support of opal save-restore
which now I am decoupling and will take them seperately.
I have posted the corresponding skiboot patches for this kernel patchset
here : https://patchwork.ozlabs.org/cover/1144587/

What's new?


Add stop states under ibm,idle-states in addition to the current array
based device tree properties.

New device tree format adds a compatible flag which has version
corresponding to every state, so that only kernel which has the capability
to handle the version of stop state will enable it. Drawback of the array
based dt node is that versioning of idle states is not possible.

Older kernel will still see stop0 and stop0_lite in older format and we
will deprecate it after some time.

Consider a case that stop4 has a bug. We take the following steps to
mitigate the problem.

1) Change compatible string for stop4 in OPAL to "stop4,v2" from
"stop4,v1", i.e. basicallly bump up the previous version and ship the
new firmware.

2) The kernel will ignore stop4 as it won't be able to recognize this
new version. Kernel will also ignore all the deeper states because its
possible that a cpu have requested for a deeper state but was never able
to enter into it. But we will still have shallower states that are there
before stop 4. This, thus prevents from completely disabling stop states.

Linux kernel can now look at the version string and decide if it has the
ability to handle that idle state. Henceforth, if kernel does not know
about a version, it will skip that state and all the deeper state.

Once when the workaround are implemented into the kernel, we can bump up
the known version in kernel for that state, so that support can be
enabled once again in kernel.

New Device-tree :

Final output
   power-mgt {
...
 ibm,enabled-stop-levels = <0xec00>;
 ibm,cpu-idle-state-psscr-mask = <0x0 0x3003ff 0x0 0x3003ff>;
 ibm,cpu-idle-state-latencies-ns = <0x3e8 0x7d0>;
 ibm,cpu-idle-state-psscr = <0x0 0x330 0x0 0x300330>;
 ibm,cpu-idle-state-flags = <0x10 0x101000>;
 ibm,cpu-idle-state-residency-ns = <0x2710 0x4e20>;
 ibm,idle-states {
 stop4 {
 flags = <0x207000>;
 compatible = "stop4,v1",
 psscr-mask = <0x0 0x3003ff>;
 latency-ns = <0x186a0>;
 residency-ns = <0x989680>;
 psscr = <0x0 0x300374>;
 ...
  };
...
stop11 {
 ...
 compatible = "stop11,v1",
 ...
  };
 };


Abhishek Goel (3):
  cpuidle/powernv : Pass state pointer instead of values to stop loop
  cpuidle/powernv: Add support for versioned stop states
  cpuidle/powernv : Add flags to identify stop state type

 arch/powerpc/include/asm/cpuidle.h|   8 +-
 arch/powerpc/include/asm/opal-api.h   |   7 +
 arch/powerpc/include/asm/processor.h  |   5 +-
 arch/powerpc/platforms/powernv/idle.c | 371 +-
 drivers/cpuidle/cpuidle-powernv.c |  81 +++---
 5 files changed, 363 insertions(+), 109 deletions(-)

-- 
2.17.1