Re: [PATCH v6 2/3] powerpc/powernv: Introduce support and parsing for self-save API

2020-04-14 Thread Pratik Sampat

Hello Gautham,

[..snip..]


+
+   if (curr_spr.supported_mode & FIRMWARE_SELF_SAVE) {
+   rc = opal_slw_self_save_reg(pir,
+   curr_spr.spr);
+   if (rc != 0)
+   return rc;
+   switch (curr_spr.spr) {
+   case SPRN_LPCR:
+   is_lpcr_self_save = true;

Could you consider converting is_lpcr_self_save and is_ptcr_self_save
into static_keys ? For reference see commit
14c73bd344da("powerpc/vcpu: Assume dedicated processors as
non-preempt")


Sure, using static keys is cleaner.

I'll also address the other nits mentioned in the e-mail earlier.

Thanks for the review.
Pratik



Re: [PATCH v6 2/3] powerpc/powernv: Introduce support and parsing for self-save API

2020-04-14 Thread Gautham R Shenoy
On Thu, Mar 26, 2020 at 12:40:33PM +0530, Pratik Rajesh Sampat wrote:
> This commit introduces and leverages the Self save API. The difference
> between self-save and self-restore is that the value to be saved for the
> SPR does not need to be passed to the call.
> 
> Add the new Self Save OPAL API call in the list of OPAL calls.
> Implement the self saving of the SPRs based on the support populated.
> This commit imposes the self-save over self-restore in case both are
> supported for a particular SPR.

s/imposes/prefers.

> 
> Along with support for self-save, kernel supported save restore is also
> populated in the list. This property is only populated for those SPRs
> which encapsulate support from the kernel and hav ethe possibility to
^
have the

> garner support from a firmware mode too.


What you mean here is that some of the SPRs which are currently being
saved and restored by the kernel can be leverage the firmware
self-save support when available.

In particular, this is *required* for PTCR in an Ultravisor mode, as
the Hypervisor Kernel is not allowed to restore PTCR while running in
an Ultravisor environment.




> 
> In addition, the commit also parses the device tree for nodes self-save,
> self-restore and populate support for the preferred SPRs based on what
> was advertised by the device tree.
> 
> In the case a SPR is supported by the firmware self-save, self-restore
> and kernel save restore then the preference of execution is also in the
> same order as above.
> 
> Signed-off-by: Pratik Rajesh Sampat 

Some more comments below.

> ---
>  .../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 | 131 +-
>  arch/powerpc/platforms/powernv/opal-call.c|   1 +
>  5 files changed, 146 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt 
> b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> index 9d619e955576..5fb03c6d7de9 100644
> --- a/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> +++ b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> @@ -116,3 +116,21 @@ otherwise. The length of all the property arrays must be 
> the same.
>   which of the fields of the PMICR are set in the corresponding
>   entries in ibm,cpu-idle-state-pmicr. This is an optional
>   property on POWER8 and is absent on POWER9.
> +
> +- self-restore:
> + Array of unsigned 64-bit values containing a property for sprn-mask
> + with each bit indicating the index of the supported SPR for the
> + functionality. This is an optional property for both Power8 and Power9
> +
> +- self-save:
> +  Array of unsigned 64-bit values containing a property for sprn-mask
> +  with each bit indicating the index of the supported SPR for the
> +  functionality. This is an optional property for both Power8 and Power9
> +
> +Example of arrangement of self-restore and self-save arrays:
> +For instance if PSSCR is supported, the value is 0x357 = 855.
> +Since the array is of 64 bit values, the index of the array is determined by
> +855 / 64 = 13th element. Within that index, the bit number is determined by
> +855 % 64 = 23rd bit.
> +This means that if the 23rd bit in array[13] is set, then that SPR is 
> supported
> +by the corresponding self-save or self-restore API.
> diff --git a/arch/powerpc/include/asm/opal-api.h 
> b/arch/powerpc/include/asm/opal-api.h
> index c1f25a760eb1..1b6e1a68d431 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -214,7 +214,8 @@
>  #define OPAL_SECVAR_GET  176
>  #define OPAL_SECVAR_GET_NEXT 177
>  #define OPAL_SECVAR_ENQUEUE_UPDATE   178
> -#define OPAL_LAST178
> +#define OPAL_SLW_SELF_SAVE_REG   181
> +#define OPAL_LAST181
> 
>  #define QUIESCE_HOLD 1 /* Spin all calls at entry */
>  #define QUIESCE_REJECT   2 /* Fail all calls with 
> OPAL_BUSY */
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 9986ac34b8e2..a370b0e8d899 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -204,6 +204,7 @@ int64_t opal_handle_hmi2(__be64 *out_flags);
>  int64_t opal_register_dump_region(uint32_t id, uint64_t start, uint64_t end);
>  int64_t opal_unregister_dump_region(uint32_t id);
>  int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val);
> +int64_t opal_slw_self_save_reg(uint64_t cpu_pir, uint64_t sprn);
>  int64_t opal_config_cpu_idle_state(uint64_t state, uint64_t flag);
>  int64_t opal_pci_set_phb_cxl_mode(uint64_t phb_id, uint64_t

[PATCH v6 2/3] powerpc/powernv: Introduce support and parsing for self-save API

2020-03-26 Thread Pratik Rajesh Sampat
This commit introduces and leverages the Self save API. The difference
between self-save and self-restore is that the value to be saved for the
SPR does not need to be passed to the call.

Add the new Self Save OPAL API call in the list of OPAL calls.
Implement the self saving of the SPRs based on the support populated.
This commit imposes the self-save over self-restore in case both are
supported for a particular SPR.

Along with support for self-save, kernel supported save restore is also
populated in the list. This property is only populated for those SPRs
which encapsulate support from the kernel and hav ethe possibility to
garner support from a firmware mode too.

In addition, the commit also parses the device tree for nodes self-save,
self-restore and populate support for the preferred SPRs based on what
was advertised by the device tree.

In the case a SPR is supported by the firmware self-save, self-restore
and kernel save restore then the preference of execution is also in the
same order as above.

Signed-off-by: Pratik Rajesh Sampat 
---
 .../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 | 131 +-
 arch/powerpc/platforms/powernv/opal-call.c|   1 +
 5 files changed, 146 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt 
b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
index 9d619e955576..5fb03c6d7de9 100644
--- a/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
+++ b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
@@ -116,3 +116,21 @@ otherwise. The length of all the property arrays must be 
the same.
which of the fields of the PMICR are set in the corresponding
entries in ibm,cpu-idle-state-pmicr. This is an optional
property on POWER8 and is absent on POWER9.
+
+- self-restore:
+ Array of unsigned 64-bit values containing a property for sprn-mask
+ with each bit indicating the index of the supported SPR for the
+ functionality. This is an optional property for both Power8 and Power9
+
+- self-save:
+  Array of unsigned 64-bit values containing a property for sprn-mask
+  with each bit indicating the index of the supported SPR for the
+  functionality. This is an optional property for both Power8 and Power9
+
+Example of arrangement of self-restore and self-save arrays:
+For instance if PSSCR is supported, the value is 0x357 = 855.
+Since the array is of 64 bit values, the index of the array is determined by
+855 / 64 = 13th element. Within that index, the bit number is determined by
+855 % 64 = 23rd bit.
+This means that if the 23rd bit in array[13] is set, then that SPR is supported
+by the corresponding self-save or self-restore API.
diff --git a/arch/powerpc/include/asm/opal-api.h 
b/arch/powerpc/include/asm/opal-api.h
index c1f25a760eb1..1b6e1a68d431 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -214,7 +214,8 @@
 #define OPAL_SECVAR_GET176
 #define OPAL_SECVAR_GET_NEXT   177
 #define OPAL_SECVAR_ENQUEUE_UPDATE 178
-#define OPAL_LAST  178
+#define OPAL_SLW_SELF_SAVE_REG 181
+#define OPAL_LAST  181
 
 #define QUIESCE_HOLD   1 /* Spin all calls at entry */
 #define QUIESCE_REJECT 2 /* Fail all calls with OPAL_BUSY */
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 9986ac34b8e2..a370b0e8d899 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -204,6 +204,7 @@ int64_t opal_handle_hmi2(__be64 *out_flags);
 int64_t opal_register_dump_region(uint32_t id, uint64_t start, uint64_t end);
 int64_t opal_unregister_dump_region(uint32_t id);
 int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val);
+int64_t opal_slw_self_save_reg(uint64_t cpu_pir, uint64_t sprn);
 int64_t opal_config_cpu_idle_state(uint64_t state, uint64_t flag);
 int64_t opal_pci_set_phb_cxl_mode(uint64_t phb_id, uint64_t mode, uint64_t 
pe_number);
 int64_t opal_pci_get_pbcq_tunnel_bar(uint64_t phb_id, uint64_t *addr);
diff --git a/arch/powerpc/platforms/powernv/idle.c 
b/arch/powerpc/platforms/powernv/idle.c
index 858ceb86394d..e77b31621081 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -35,13 +35,20 @@
 /*
  * Type of support for each SPR
  * FIRMWARE_RESTORE: firmware restoration supported: calls self-restore OPAL 
API
+ * FIRMWARE_SELF_SAVE: firmware save and restore: calls self-save OPAL API
+ * KERNEL_SAVE_RESTORE: kernel handles the saving and restoring of SPR
  */
 #define UNSUPPORTED   0x0
 #define FIRMWARE_RESTORE  0x1
+#define FIRMWARE_SELF_SAVE0x2
+#define KERNEL_SAVE_RES