diff mbox

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

Message ID 1493886884-7659-3-git-send-email-akshay.adiga@linux.vnet.ibm.com
State RFC
Headers show

Commit Message

Akshay Adiga May 4, 2017, 8:34 a.m. UTC
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@linux.vnet.ibm.com>
---
Changes in v1:
Change in commit message.

 hw/slw.c | 52 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 34 insertions(+), 18 deletions(-)

Comments

ppaidipe May 4, 2017, 10:44 a.m. UTC | #1
Hi Akshay

On 2017-05-04 14:04, Akshay Adiga 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@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)

Can you document this opal call OPAL_SLW_SET_REG in doc/opal-api.

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

This call will be called from linux during runtime.
So can this be PR_DEBUG or PR_TRACE?

>  	return OPAL_SUCCESS;
> 
>  }
Oliver O'Halloran May 8, 2017, 6:24 a.m. UTC | #2
On Thu, May 4, 2017 at 6:34 PM, Akshay Adiga
<akshay.adiga@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@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@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
diff mbox

Patch

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 ) {
+		if(!chip->homer_base) {
+			log_simple_error(&e_info(OPAL_RC_SLW_REG),
+					"SLW: HOMER base not set %x\n",
+					chip->id);
+			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++)  {
+			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);
 	return OPAL_SUCCESS;
 
 }