<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <tt>Hello Oliver, <br>
      <br>
      Thank you for your review.<br>
    </tt><br>
    <div class="moz-cite-prefix">On 11/10/19 6:44 AM, Oliver O'Halloran
      wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAOSf1CE16HHBatJqWx-bdeA0Jq9=ofQt86R_NGN9BFQZMxBowQ@mail.gmail.com">
      <pre class="moz-quote-pre" wrap="">On Thu, Oct 10, 2019 at 11:10 PM Pratik Rajesh Sampat
<a class="moz-txt-link-rfc2396E" href="mailto:psampat@linux.ibm.com"><psampat@linux.ibm.com></a> wrote:
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">
The commit makes the self save API available outside the firmware by defining
an OPAL wrapper.
This wrapper has a similar interface to that of self restore and expects the
cpu pir, SPR number, minus the value of that SPR to be passed in its
paramters and returns OPAL_SUCCESS on success.
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
Any need OPAL calls need to be documented in doc/opal-api/</pre>
    </blockquote>
    <pre>Sure thing, will do.
</pre>
    <blockquote type="cite"
cite="mid:CAOSf1CE16HHBatJqWx-bdeA0Jq9=ofQt86R_NGN9BFQZMxBowQ@mail.gmail.com">
      <pre class="moz-quote-pre" wrap="">

</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">
Signed-off-by: Pratik Rajesh Sampat <a class="moz-txt-link-rfc2396E" href="mailto:psampat@linux.ibm.com"><psampat@linux.ibm.com></a>
---
 hw/slw.c              | 88 +++++++++++++++++++++++++++++++++++++++++++
 include/opal-api.h    |  3 +-
 include/p9_stop_api.H | 16 ++++++++
 include/skiboot.h     |  3 ++
 libpore/p9_stop_api.C |  2 +-
 5 files changed, 110 insertions(+), 2 deletions(-)

diff --git a/hw/slw.c b/hw/slw.c
index bb88f0f1..b79aaab3 100644
--- a/hw/slw.c
+++ b/hw/slw.c
@@ -36,6 +36,42 @@ static bool slw_current_le = false;
 enum wakeup_engine_states wakeup_engine_state = WAKEUP_ENGINE_NOT_PRESENT;
 bool has_deep_states = false;

+/**
+ * The struct and SPR list is partially consistent with libpore/p9_stop_api.c
+ */
+/**
+ * @brief summarizes attributes associated with a SPR register.
+ */
+typedef struct
+{
+    uint32_t iv_sprId;
+    bool     iv_isThreadScope;
+    uint32_t iv_saveMaskPos;
+
+} StopSprReg_t;
+
+/**
+ * @brief a true in the table below means register is of scope thread
+ * whereas a false meanse register is of scope core.
+ * The number is the bit position on a uint32_t mask
+ */
+
+static const StopSprReg_t g_sprRegister[] =
+{
+       { P9_STOP_SPR_DAWR,      true,  1   },
+       { P9_STOP_SPR_HSPRG0,    true,  3   },
+       { P9_STOP_SPR_LDBAR,     true,  4,  },
+       { P9_STOP_SPR_LPCR,      true,  5   },
+       { P9_STOP_SPR_PSSCR,     true,  6   },
+       { P9_STOP_SPR_MSR,       true,  7   },
+       { P9_STOP_SPR_HRMOR,     false, 255 },
+       { P9_STOP_SPR_HID,       false, 21  },
+       { P9_STOP_SPR_HMEER,     false, 22  },
+       { P9_STOP_SPR_PMCR,      false, 23  },
+};
+
+static const uint32_t MAX_SPR_SUPPORTED        = ARRAY_SIZE(g_sprRegister);
+
 DEFINE_LOG_ENTRY(OPAL_RC_SLW_INIT, OPAL_PLATFORM_ERR_EVT, OPAL_SLW,
                 OPAL_PLATFORM_FIRMWARE, OPAL_PREDICTIVE_ERR_GENERAL,
                 OPAL_NA);
@@ -1452,6 +1488,58 @@ int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val)

 opal_call(OPAL_SLW_SET_REG, opal_slw_set_reg, 3);

+int64_t opal_slw_self_save_reg(uint64_t cpu_pir, uint64_t sprn)
+{
+       struct cpu_thread * c = find_cpu_by_pir(cpu_pir);
+       struct proc_chip * chip;
+       int rc;
+       int index;
+       uint32_t save_reg_vector = 0;
+
+       if (!c) {
+               prerror("SLW: Unknown thread with pir %x\n", (u32) cpu_pir);
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
If you're going to print errors here then print them at PR_DEBUG or
higher. This function gets called once for every PIR in the system so
we'll get a few hundred lines printed to the console on every boot.
The existing SLW prints are spammy enough as-is...
</pre>
    </blockquote>
    <pre>Got it, I'll change the log level.
</pre>
    <blockquote type="cite"
cite="mid:CAOSf1CE16HHBatJqWx-bdeA0Jq9=ofQt86R_NGN9BFQZMxBowQ@mail.gmail.com">
      <pre class="moz-quote-pre" wrap="">
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">+               return OPAL_PARAMETER;
+       }
+
+       chip = get_chip(c->chip_id);
+       if (!chip) {
+               prerror("SLW: Unknown chip for thread with pir %x\n",
+                       (u32) cpu_pir);
+               return OPAL_PARAMETER;
+       }
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">+       if (proc_gen == proc_gen_p9 && has_deep_states) {
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
use an early-exit rather than stacking indentation levels.</pre>
    </blockquote>
    <pre>Will do.
</pre>
    <blockquote type="cite"
cite="mid:CAOSf1CE16HHBatJqWx-bdeA0Jq9=ofQt86R_NGN9BFQZMxBowQ@mail.gmail.com">
      <pre class="moz-quote-pre" wrap="">

</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">+               if (wakeup_engine_state != WAKEUP_ENGINE_PRESENT) {
+                       log_simple_error(&e_info(OPAL_RC_SLW_REG),
+                               "SLW: wakeup_engine in bad state=%d chip=%x\n",
+                               wakeup_engine_state, chip->id);
+                       return OPAL_INTERNAL_ERROR;
+               }
+               for (index = 0; index < MAX_SPR_SUPPORTED; ++index) {
+                       if (sprn == (CpuReg_t) g_sprRegister[index].iv_sprId) {
+                               save_reg_vector = PPC_BIT32(
+                                       g_sprRegister[index].iv_saveMaskPos);
+                               break;
+                       }
+               }
+               if (save_reg_vector == 0)
+                       return OPAL_INTERNAL_ERROR;
+               rc = p9_stop_save_cpureg_control((void *) chip->homer_base,
+                                                cpu_pir, save_reg_vector);
+
+               if (rc) {
+                       log_simple_error(&e_info(OPAL_RC_SLW_REG),
+                               "SLW: Failed to save vector %x for CPU %x\n",
+                               save_reg_vector, c->pir);
+                       return OPAL_INTERNAL_ERROR;
+               }
+               return OPAL_SUCCESS;
+       }
+       prerror("SLW: Does not support deep states\n");
+       return OPAL_UNSUPPORTED;
+}
+
+opal_call(OPAL_SLW_SELF_SAVE_REG, opal_slw_self_save_reg, 2);
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">don't put a newline between the function and opal_call()

</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">+
 void slw_init(void)
 {
        struct proc_chip *chip;
diff --git a/include/opal-api.h b/include/opal-api.h
index 782666dd..53ce23e0 100644
--- a/include/opal-api.h
+++ b/include/opal-api.h
@@ -219,7 +219,8 @@
 #define OPAL_XIVE_GET_VP_STATE                 170 /* Get NVT state */
 #define OPAL_NPU_MEM_ALLOC                     171
 #define OPAL_NPU_MEM_RELEASE                   172
-#define OPAL_LAST                              172
+#define OPAL_SLW_SELF_SAVE_REG                 173
+#define OPAL_LAST                              173
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
This needs to be updated since the MPIPL calls are using these numbers.
</pre>
    </blockquote>
    <pre>Thanks, will do.
</pre>
    <blockquote type="cite"
cite="mid:CAOSf1CE16HHBatJqWx-bdeA0Jq9=ofQt86R_NGN9BFQZMxBowQ@mail.gmail.com">
      <pre class="moz-quote-pre" wrap="">
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap=""> #define QUIESCE_HOLD                   1 /* Spin all calls at entry */
 #define QUIESCE_REJECT                 2 /* Fail all calls with OPAL_BUSY */
diff --git a/include/p9_stop_api.H b/include/p9_stop_api.H
index 79abd000..e9962c37 100644
--- a/include/p9_stop_api.H
+++ b/include/p9_stop_api.H
@@ -34,6 +34,8 @@
 ///
 /// @file   p9_stop_api.H
 /// @brief  describes STOP API which  create/manipulate STOP image.
+///         This header need not be consistent, however is a subset of the
+///         libpore/p9_stop_api.H counterpart
 ///
 // *HWP HW Owner    :  Greg Still <a class="moz-txt-link-rfc2396E" href="mailto:stillgs@us.ibm.com"><stillgs@us.ibm.com></a>
 // *HWP FW Owner    :  Prem Shanker Jha <a class="moz-txt-link-rfc2396E" href="mailto:premjha2@in.ibm.com"><premjha2@in.ibm.com></a>
@@ -155,6 +157,20 @@ StopReturnCode_t p9_stop_save_scom( void* const   i_pImage,
                                     const ScomOperation_t i_operation,
                                     const ScomSection_t i_section );

+/**
+ * @brief       Facilitates self save and restore of a list of SPRs of a thread.
+ * @param[in]   i_pImage        points to the start of HOMER image of P9 chip.
+ * @param[in]   i_pir           PIR associated with thread
+ * @param[in]   i_saveRegVector bit vector representing SPRs that needs to be restored.
+ * @return      STOP_SAVE_SUCCESS if API succeeds, error code otherwise.
+ * @note        SPR save vector is a bit vector. For each SPR supported,
+ *              there is an associated bit position in the bit vector.Refer
+ *              to definition of SprBitPositionList_t to determine bit position
+ *              associated with a particular SPR.
+ */
+StopReturnCode_t
+p9_stop_save_cpureg_control( void* i_pImage, const uint64_t i_pir,
+                             const uint32_t  i_saveRegVector );
 #ifdef __cplusplus
 } // extern "C"
 };  // namespace stopImageSection ends
diff --git a/include/skiboot.h b/include/skiboot.h
index 6cac1cfd..1aa8bf7c 100644
--- a/include/skiboot.h
+++ b/include/skiboot.h
@@ -299,6 +299,9 @@ extern void xive_late_init(void);
 /* SLW reinit function for switching core settings */
 extern int64_t slw_reinit(uint64_t flags);

+/* Self save SPR before entering the stop state */
+extern int64_t opal_slw_self_save_reg(uint64_t cpu_pir, uint64_t sprn);
+
 /* Patch SPR in SLW image */
 extern int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val);

diff --git a/libpore/p9_stop_api.C b/libpore/p9_stop_api.C
index f41086b4..d231d1eb 100644
--- a/libpore/p9_stop_api.C
+++ b/libpore/p9_stop_api.C
@@ -1401,7 +1401,7 @@ STATIC StopReturnCode_t updateSelfSaveEntry( uint32_t* i_pSaveReg, uint16_t i_sp

 //-----------------------------------------------------------------------------

-StopReturnCode_t p9_stop_save_cpureg_control(  void* i_pImage,
+StopReturnCode_t p9_stop_save_cpureg_control(  void* const i_pImage,
         const uint64_t i_pir,
         const uint32_t i_saveRegVector )
 {
--
2.21.0

</pre>
      </blockquote>
    </blockquote>
    <br>
  </body>
</html>