Re: [PATCH v6 0/3] powerpc/powernv: Introduce interface for self-restore support

From: Pratik Sampat
Date: Tue Apr 14 2020 - 08:10:55 EST


Hello Gautham

On 14/04/20 12:41 pm, Gautham R Shenoy wrote:
Hello Pratik,

On Thu, Mar 26, 2020 at 12:40:31PM +0530, Pratik Rajesh Sampat wrote:
v5: https://lkml.org/lkml/2020/3/17/944
Changelog
v5-->v6
1. Updated background, motivation and illuminated potential design
choices
2. Re-organization of patch-set
a. Split introducing preference for optimization from 1/1 to patch 3/3
b. Merge introducing self-save API and parsing the device-tree
c. Introduce a supported mode called KERNEL_SAVE_RESTORE which
outlines and makes kernel supported SPRs for save-restore more
explicit

[..snip..]

Presenting the design choices in front of us:

Design-Choice 1:
----------------
A simple implementation is to just replace self-restore calls with
self-save as it is direct super-set.

Pros:
A simple design, quick to implement


Cons:
* Breaks backward compatibility. Self-restore has historically been
supported in the firmware and an old firmware running on an new
kernel will be incompatible and deep stop states will be cut.
* Furthermore, critical SPRs which need to be restored
before 0x100 vector like HID0 are not supported by self-save.

Design-Choice 2:
----------------
Advertise both self-restore and self-save from OPAL including the set
of registers that each support. The kernel can then choose which API
to go with.
For the sake of simplicity, in case both modes are supported for an
SPR by default self-save would be called for it.

Pros:
* Backwards compatible

Cons:
Overhead in parsing device tree with the SPR list

Possible optimization with Approach2:
-------------------------------------
There are SPRs whose values don't tend to change over time and invoking
self-save on them, where the values are gotten each time may turn out to
be inefficient. In that case calling a self-restore where passing the
value makes more sense as, if the value is same, the memory location
is not updated.
SPRs that dont change are as follows:
SPRN_HSPRG0,
SPRN_LPCR,
SPRN_PTCR,
SPRN_HMEER,
SPRN_HID0,
We can just pick self-save wherever available and fallback to
self-restore when self-save support is not avaiable for any SPR.
The optimization that you mention here can be revisited if the
additional latency due to self-save becomes observable (Note that both
stop4 and stop5 have wakeup latency between 200-500us).

Yes, Patch2 introduces picking self-save when available and using
self-restore as fallback.
Sure. The optimization of preference does certainly add an additional
layer of complexity. As you mentioned, in the case the latency really
begins to hurt us we can revisit this discussion.

Thanks
Pratik

The values of PSSCR and MSR change at runtime and hence, the kernel
cannot determine during boot time what their values will be before
entering a particular deep-stop state.

Therefore, a preference based interface is introduced for choosing
between self-save or self-restore between for each SPR.
The per-SPR preference is only a refinement of
approach 2 purely for performance reasons. It can be dropped if the
complexity is not deemed worth the returns.

Patches Organization
====================
Design Choice 2 has been chosen as an implementation to demonstrate in
the patch series.

Patch1:
Devises an interface which lists all the interested SPRs, along with
highlighting the support of mode.
It is an isomorphic patch to replicate the functionality of the older
self-restore firmware for the new interface

Patch2:
Introduces the self-save API and leverages upon the struct interface to
add another supported mode in the mix of saving and restoring. It also
enforces that in case both modes are supported self-save is chosen over
self-restore

The commit also parses the device-tree and populate support for
self-save and self-restore in the supported mask

Patch3:
Introduce an optimization to allow preference to choose between one more
over the one when both both modes are supported. This optimization can
allow for better performance for the SPRs that don't change in value and
hence self-restore is a better alternative, and in cases when it is
known for values to change self-save is more convenient.

Pratik Rajesh Sampat (3):
powerpc/powernv: Introduce interface for self-restore support
powerpc/powernv: Introduce support and parsing for self-save API
powerpc/powernv: Preference optimization for SPRs with constant values

.../bindings/powerpc/opal/power-mgt.txt | 18 +
arch/powerpc/include/asm/opal-api.h | 3 +-
arch/powerpc/include/asm/opal.h | 1 +
arch/powerpc/platforms/powernv/idle.c | 385 +++++++++++++++---
arch/powerpc/platforms/powernv/opal-call.c | 1 +
5 files changed, 351 insertions(+), 57 deletions(-)

--
2.17.1