[Skiboot] [RFC v1 2/4] SLW: Add opal_slw_set_reg support for power9

[Oliver O'Halloran]

On Thu, May 4, 2017 at 6:34 PM, Akshay Adiga
<akshay.adiga at linux.vnet.ibm.com> wrote:
> This OPAL call is made from Linux to OPAL to configure valuse in
> various SPRs after wakeup from a deep idle state.
>
> Signed-off-by: Akshay Adiga <akshay.adiga at linux.vnet.ibm.com>
> ---
> Changes in v1:
> Change in commit message.
>
>  hw/slw.c | 52 ++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/hw/slw.c b/hw/slw.c
> index 6503fa7..e4ab2c2 100644
> --- a/hw/slw.c
> +++ b/hw/slw.c
> @@ -1324,33 +1324,49 @@ int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val)
>         assert(c);
>         chip = get_chip(c->chip_id);
>         assert(chip);
> -       image = (void *) chip->slw_base;
>
> -       /* Check of the SPR is supported by libpore */
> -       for ( i=0; i < SLW_SPR_REGS_SIZE ; i++)  {
> -               if (sprn == SLW_SPR_REGS[i].value)  {
> -                       spr_is_supported = 1;
> -                       break;
> +
> +       if (chip->type == PROC_CHIP_P9_NIMBUS ||
> +                       chip->type == PROC_CHIP_P9_CUMULUS ) {

please use the proc_gen variable for testing what generation of
processor that you're running on.

> +               if(!chip->homer_base) {
> +                       log_simple_error(&e_info(OPAL_RC_SLW_REG),
> +                                       "SLW: HOMER base not set %x\n",
> +                                       chip->id);

Not your fault, but why are we generating error logs here? It seems
like overkill.

> +                       return OPAL_INTERNAL_ERROR;
>                 }
> -       }
> -       if (!spr_is_supported) {
> -               log_simple_error(&e_info(OPAL_RC_SLW_REG),
> -                       "SLW: Trying to set unsupported spr for CPU %x\n",
> -                       c->pir);
> -               return OPAL_UNSUPPORTED;
> -       }
> +               rc = p9_stop_save_cpureg((void *) chip->homer_base,
> +                                         sprn, val, cpu_pir);
> +
> +       } else { /* Assuming its P8 */
>
> -       rc = p8_pore_gen_cpureg_fixed(image, P8_SLW_MODEBUILD_SRAM, sprn,
> -                                               val, cpu_get_core_index(c),
> +               /* Check of the SPR is supported by libpore */
> +               for ( i=0; i < SLW_SPR_REGS_SIZE ; i++)  {

While you're moving things around can you fix the whitespace around i = 0 ;)

> +                       if (sprn == SLW_SPR_REGS[i].value)  {
> +                               spr_is_supported = 1;
> +                               break;
> +                       }
> +               }
> +               if (!spr_is_supported) {
> +                       log_simple_error(&e_info(OPAL_RC_SLW_REG),
> +                       "SLW: Trying to set unsupported spr for CPU %x\n",
> +                               c->pir);
> +                       return OPAL_UNSUPPORTED;
> +               }
> +               image = (void *) chip->slw_base;
> +               rc = p8_pore_gen_cpureg_fixed(image, P8_SLW_MODEBUILD_SRAM,
> +                                               sprn, val,
> +                                               cpu_get_core_index(c),
>                                                 cpu_get_thread_index(c));
> +       }
>
>         if (rc) {
>                 log_simple_error(&e_info(OPAL_RC_SLW_REG),
> -                       "SLW: Failed to set spr for CPU %x\n",
> -                       c->pir);
> +                       "SLW: Failed to set spr %llx for CPU %x\n",
> +                       sprn, c->pir);
>                 return OPAL_INTERNAL_ERROR;
>         }
> -
> +       prlog(PR_NOTICE, "SLW: restore spr:0x%llx on c:0x%x with 0x%llx\n",
> +                                                       sprn, c->pir, val);

+1 for PR_NOTICE being too high a log level. This should be PR_TRACE
at a minimum and considering this needs to be called for every core
(thread?) in the system INSANE might be more appropriate...

>         return OPAL_SUCCESS;
>
>  }
> --
> 2.5.5
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot