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

[Akshay Adiga]

I have added errror handling code and some other fixes and reposted.

https://patchwork.ozlabs.org/patch/838382/
https://patchwork.ozlabs.org/patch/838383/

On 11/13/2017 11:36 AM, Oliver wrote:
> 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
>