[Skiboot] [PATCH] SCOM restore for DARN and XIVE

[Oliver]

On Mon, Nov 13, 2017 at 5:04 PM, Stewart Smith
<stewart at linux.vnet.ibm.com> wrote:
> Akshay Adiga <akshay.adiga at linux.vnet.ibm.com> writes:
>> While waking up from stop11, we want NCU_DARN_BAR to have enable bit set.
>> Without this stop_api call, the value restored is without enable bit set.
>> We loose NCU_SPEC_BAR when the quad goes into stop11, stop_api will
>> restore while waking up from stop11.
>>
>> Signed-off-by: Akshay Adiga <akshay.adiga at linux.vnet.ibm.com>
>> ---
>>  hw/nx.c   | 12 +++++++++++-
>>  hw/xive.c |  8 ++++++++
>>  2 files changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/nx.c b/hw/nx.c
>> index 64ac793..8431c3c 100644
>> --- a/hw/nx.c
>> +++ b/hw/nx.c
>> @@ -24,6 +24,7 @@
>>  #include <chip.h>
>>  #include <xscom-p9-regs.h>
>>  #include <phys-map.h>
>> +#include <p9_stop_api.H>
>>
>>  extern void nx_p9_rng_init(void);
>>
>> @@ -31,7 +32,7 @@ void nx_p9_rng_init(void)
>>  {
>>       struct proc_chip *chip;
>>       struct cpu_thread *c;
>> -     uint64_t bar, tmp;
>> +     uint64_t bar, tmp, rc;
>>
>>       if (proc_gen != proc_gen_p9)
>>               return;
>> @@ -65,6 +66,15 @@ void nx_p9_rng_init(void)
>>                                               P9X_EX_NCU_DARN_BAR);
>>                       xscom_write(chip->id, addr,
>>                                   bar | P9X_EX_NCU_DARN_BAR_EN);
>> +                     rc = p9_stop_save_scom(( void *)chip->homer_base,
>> +                                     addr, bar | P9X_EX_NCU_DARN_BAR_EN,
>> +                                     P9_STOP_SCOM_REPLACE,
>> +                                     P9_STOP_SECTION_CORE_SCOM);
>> +                     if (rc) {
>> +                             prlog(PR_ERR,
>> +                                     " stopapi for DARN_BAR failed %lld",
>> +                                     rc);
>> +                     }
>
> Don't need {} for single line ifs

Is it really a single line if when it's spread across three?

>
> Also, this (and below) I think should just say p9_stop_api rather than
> 'stopapi' here and 'STOP_API' below.
>
> I also think the error handling here is incorrect.
>
> IF we can't get stop to save and restore the DARN_BAR then we either
> need to disable deep stop states or DARN.
>> @@ -3111,6 +3112,7 @@ static void xive_configure_ex_special_bar(struct xive *x, struct cpu_thread *c)
>>  {
>>       uint64_t xa, val;
>>       int64_t rc;
>> +     struct proc_chip *chip = get_chip(c->chip_id);
>>
>>       xive_cpu_dbg(c, "Setting up special BAR\n");
>>       xa = XSCOM_ADDR_P9_EX(pir_to_core_id(c->pir), P9X_EX_NCU_SPEC_BAR);
>> @@ -3123,6 +3125,12 @@ static void xive_configure_ex_special_bar(struct xive *x, struct cpu_thread *c)
>>               xive_cpu_err(c, "Failed to setup NCU_SPEC_BAR\n");
>>               /* XXXX  what do do now ? */
>>       }
>> +     rc = p9_stop_save_scom(( void *)chip->homer_base, xa, val,
>> +                             P9_STOP_SCOM_REPLACE, P9_STOP_SECTION_EQ_SCOM);
>> +     if (rc) {
>> +             xive_cpu_err(c, "STOP_API failed for NCU_SPEC_BAR rc=%lld\n",rc);
>> +
>> +     }
>>  }
>
> I think we need to handle the error case here too, although I'm not sure
> what options we have in this case...
>
> --
> Stewart Smith
> OPAL Architect, IBM.
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot