Re: [PATCH v6 0/3] powerpc/powernv: Introduce interface for self-restore support
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
Re: [PATCH v6 0/3] powerpc/powernv: Introduce interface for self-restore support
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). > > 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 >
[PATCH v6 0/3] powerpc/powernv: Introduce interface for self-restore support
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 Background == The power management framework on POWER systems include core idle states that lose context. Deep idle states namely "winkle" on POWER8 and "stop4" and "stop5" on POWER9 can be entered by a CPU to save different levels of power, as a consequence of which all the hypervisor resources such as SPRs and SCOMs are lost. For most SPRs, saving and restoration of content for SPRs and SCOMs is handled by the hypervisor kernel prior to entering an post exit from an idle state respectively. However, there is a small set of critical SPRs and XSCOMs that are expected to contain sane values even before the control is transferred to the hypervisor kernel at system reset vector. For this purpose, microcode firmware provides a mechanism to restore values on certain SPRs. The communication mechanism between the hypervisor kernel and the microcode is a standard interface called sleep-winkle-engine (SLW) on Power8 and Stop-API on Power9 which is abstracted by OPAL calls from the hypervisor kernel. The Stop-API provides an interface known as the self-restore API, to which the SPR number and a predefined value to be restored on wake-up from a deep stop state is supplied. Motivation to introduce a new Stop-API == The self-restore API expects not just the SPR number but also the value with which the SPR is restored. This is good for those SPRs such as HSPRG0 whose values do not change at runtime, since for them, the kernel can invoke the self-restore API at boot time once the values of these SPRs are determined. However, there are use-cases where-in the value to be saved cannot be known or cannot be updated in the layer it currently is. The shortcomings and the new use-cases which cannot be served by the existing self-restore API, serves as motivation for a new API: Shortcoming1: In a special wakeup scenario, SPRs such as PSSCR, whose values can change at runtime, are compelled to make the self-restore API call every time before entering a deep-idle state rendering it to be prohibitively expensive Shortcoming2: The value of LPCR is dynamic based on if the CPU is entered a stop state during cpu idle versus cpu hotplug. Today, an additional self-restore call is made before entering CPU-Hotplug to clear the PECE1 bit in stop-API so that if we are woken up by a special wakeup on an offlined CPU, we go back to stop with the the bit cleared. There is a overhead of an extra call New Use-case: - In the case where the hypervisor is running on an ultravisor environment, the boot time is too late in the cycle to make the self-restore API calls, as these cannot be invoked from an non-secure context anymore To address these shortcomings, the firmware provides another API known as the self-save API. The self-save API only takes the SPR number as a parameter and will ensure that on wakeup from a deep-stop state the SPR is restored with the value that it contained prior to entering the deep-stop. Contrast between self-save and self-restore APIs Before entering deep idle |---| > | HCODE A | | |---| |-|| | CPU || |-|| | |---| |>| HCODE B | On waking up |---| from deep idle When a self-restore API is invoked, the HCODE inserts instructions into "HCODE B" region of the above figure to restore the content of the SPR to the said value. The "HCODE B" region gets executed soon after the CPU wakes up from a deep idle state, thus executing the inserted instructions, thereby restoring the contents of the SPRs to the required values. When a self-save API is invoked, the HCODE inserts instructions into the "HCODE A" region of the above figure to save the content of the SPR into some location in memory. It also inserts instructions into the "HCODE B" region to restore the content of the SPR to the corresponding value saved in the memory by the instructions in "HCODE A" region. Thus, in contrast with self-restore, the self-save API *does not* need a value to be passed to it, since it ensures that the value of SPR before entering deep stop is saved, and subsequently the same value is restored. Self-save and self-restore are complementary features since, self-restore c
