[Skiboot] [RESEND PATCH v2 2/4] Self save API integration
psampat at linux.ibm.com
Wed Feb 5 17:13:27 AEDT 2020
Thanks for the review.
>> +.. OPAL_SLW_SELF_SAVE_REG:
>> +.. code-block:: c
>> + #define OPAL_SLW_SELF_SAVE_REG 176
>> + int64_t opal_slw_self_save_reg(uint64_t cpu_pir, uint64_t sprn);
> https://open-power.github.io/skiboot/doc/opal-api/opal-slw-set-reg-100.html is the existing interface for making something set a register on resume from deep sleep states. The function signature looks good, but it's probably useful to add cross-references in the documentation so you can find each of the functions easily enough.
> int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val);
Sure thing, I can add a cross reference for the functions owing to their similarities.
>> +``uint64_t cpu_pir``
>> + This parameter specifies the pir of the cpu for which the call is
>> being made.
>> +``uint64_t sprn``
>> + This parameter specifies the spr number as mentioned in
>> p9_stop_api.H for
>> + Power9 and p8_pore_table_gen_api.H for Power8.
> It might be good to list things currently and how things would change in the future. Should an OS optimistically try everything it wishes for and fall back?
Right. I could put up the list of currently supported registers in the documentation as well.
Not really, the OS does get the list of the supported registers via the device tree and only for those the self-save will be tried for in the OS.
>> + If spr restore is not supported by pore engine.
> just the SPR being passed in? or all of them?
> How should a caller use it? Try for each SPR, have a fallback if OPAL_UNSUPPORTED
The OPAL_UNSUPPORTED is just for the SPR passed in and not for all of them.
As you correctly pointed out, the caller should try the call for each interested SPR and must have a fallback in case the call fails.
I can make sure I mention this in the documentation?
More information about the Skiboot