Re: [PATCH v6 4/4] Advertise the self-save and self-restore attributes in the device tree

[Gautham R Shenoy <ego@linux.vnet.ibm.com>]

Hello Pratik,

On Thu, Mar 26, 2020 at 12:39:17PM +0530, Pratik Rajesh Sampat wrote:
> Support for self save and self restore interface is advertised in the
> device tree, along with the list of SPRs it supports for each.
> 
> The Special Purpose Register identification is encoded in a 2048 bitmask
> structure, where each bit signifies the identification key of that SPR
> which is consistent with that of the POWER architecture set for that
> register.
> 
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> ---
>  .../ibm,opal/power-mgt/self-restore.rst       |  27 ++++
>  .../ibm,opal/power-mgt/self-save.rst          |  27 ++++
>  hw/slw.c                                      | 116 ++++++++++++++++++
>  include/skiboot.h                             |   1 +
>  4 files changed, 171 insertions(+)
>  create mode 100644 doc/device-tree/ibm,opal/power-mgt/self-restore.rst
>  create mode 100644 doc/device-tree/ibm,opal/power-mgt/self-save.rst
> 
> diff --git a/doc/device-tree/ibm,opal/power-mgt/self-restore.rst b/doc/device-tree/ibm,opal/power-mgt/self-restore.rst
> new file mode 100644
> index 00000000..2a2269f7
> --- /dev/null
> +++ b/doc/device-tree/ibm,opal/power-mgt/self-restore.rst
> @@ -0,0 +1,27 @@
> +ibm,opal/power-mgt/self-restore device tree entries
> +===================================================
> +
> +This node exports the bitmask representing the special purpose registers that
> +the self-restore API currently supports.
> +
> +Example:
> +
> +.. code-block:: dts
> +
> +  self-restore {
> +        sprn-bitmask = <0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x42010000 0x0 0x0
> +                        0x20000 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0
> +                        0x0 0x0 0x100000 0x900000 0x0 0x0 0x530000 0x0 0x0 0x0
> +                        0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0
> +                        0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0
> +                        0x0 0x10000>;
> +        phandle = <0x1c7>;
> +  };
> +
> +sprn-bitmask
> +------------
> +
> +This property is a bitmask of of all the existing SPRs and if the SPR is
> +supported, the corresponding bit of the SPR number is set to 1.
> +The representation of the bits are left-right, i.e the MSB of the first
> +doubleword represants the 0th bit.
> diff --git a/doc/device-tree/ibm,opal/power-mgt/self-save.rst b/doc/device-tree/ibm,opal/power-mgt/self-save.rst
> new file mode 100644
> index 00000000..c367720e
> --- /dev/null
> +++ b/doc/device-tree/ibm,opal/power-mgt/self-save.rst
> @@ -0,0 +1,27 @@
> +ibm,opal/power-mgt/self-save device tree entries
> +===================================================
> +
> +This node exports the bitmask representing the special purpose registers that
> +the self-save API currently supports.
> +
> +Example:
> +
> +.. code-block:: dts
> +
> +  self-save {
> +        sprn-bitmask = <0x0 0x0 0x0 0x0 0x100000 0x0 0x0 0x0 0x42010000 0x0 0x0
> +                        0x20000 0x0 0x0 0x0 0x10000 0x0 0x0 0x0 0x0 0x0 0x0 0x0
> +                        0x0 0x0 0x0 0x100000 0x840000 0x0 0x0 0x0 0x0 0x0 0x0
> +                        0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0
> +                        0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0
> +                        0x0 0x10000>;
> +        phandle = <0x1c8>;
> +  };
> +
> +sprn-bitmask
> +------------
> +
> +This property is a bitmask of of all the existing SPRs and if the SPR is
> +supported, the corresponding bit of the SPR number is set to 1.
> +The representation of the bits are left-right, i.e the MSB of the first
> +doubleword represants the 0th bit.
> diff --git a/hw/slw.c b/hw/slw.c
> index 6a09cc2c..9d1fe2c5 100644
> --- a/hw/slw.c
> +++ b/hw/slw.c
> @@ -29,6 +29,7 @@
>  #include <sbe_xip_image.h>
> 
>  static uint32_t slw_saved_reset[0x100];
> +#define SPR_BITMAP_LENGTH	2048
> 
>  static bool slw_current_le = false;
> 
> @@ -750,6 +751,119 @@ static void slw_late_init_p9(struct proc_chip *chip)
>  	}
>  }
> 
> +/* Add device tree properties to determine self-save | restore */
> +void add_cpu_self_save_restore_properties(void)
> +{
> +	struct dt_node *self_restore, *self_save, *power_mgt;
> +	uint64_t *self_save_mask, *self_restore_mask;
> +	bool self_save_supported = true;
> +	uint64_t compVector = -1;
> +	struct proc_chip *chip;
> +	int i, rc;
> +
> +	const uint64_t self_restore_regs[] = {
> +		P8_SPR_HRMOR,
> +		P8_SPR_HMEER,
> +		P8_SPR_PMICR,
> +		P8_SPR_PMCR,
> +		P8_SPR_HID0,
> +		P8_SPR_HID1,
> +		P8_SPR_HID4,
> +		P8_SPR_HID5,
> +		P8_SPR_HSPRG0,
> +		P8_SPR_LPCR,
> +		P8_MSR_MSR
> +	};
> +
> +	const uint64_t self_save_regs[] = {
> +		P9_STOP_SPR_DAWR,
> +		P9_STOP_SPR_HSPRG0,
> +		P9_STOP_SPR_LDBAR,
> +		P9_STOP_SPR_LPCR,
> +		P9_STOP_SPR_PSSCR,
> +		P9_STOP_SPR_MSR,
> +		P9_STOP_SPR_HRMOR,
> +		P9_STOP_SPR_HMEER,
> +		P9_STOP_SPR_PMCR,
> +		P9_STOP_SPR_PTCR
> +	};
> +
> +	chip = next_chip(NULL);
> +	assert(chip);
> +	rc = proc_stop_api_discover_capability((void *) chip->homer_base,
> +					       &compVector);
> +	if (rc == STOP_SAVE_ARG_INVALID_IMG) {
> +		prlog(PR_DEBUG, "HOMER BASE INVALID\n");
> +		return;
> +	} else if (rc == STOP_SAVE_API_IMG_INCOMPATIBLE) {
> +		prlog(PR_DEBUG, "STOP API running incompatible versions\n");
> +		if ((compVector & SELF_RESTORE_VER_MISMATCH) == 0) {
> +			prlog(PR_DEBUG, "Self-save API unsupported\n");
> +			self_save_supported = false;
> +		}
> +	}
> +
> +	power_mgt = dt_find_by_path(dt_root, "/ibm,opal/power-mgt");
> +	if (!power_mgt) {
> +		prerror("OCC: dt node /ibm,opal/power-mgt not found\n");

The prerror/warning in this function should be prefixed with "SLW"
instead of "OCC" here. This function has nothing to do with OCC.


> +		return;
> +	}
> +
> +	self_restore = dt_new(power_mgt, "self-restore");
> +	if (!self_restore) {
> +		prerror("OCC: Failed to create self restore node");

Ditto.

> +		return;
> +	}
> +
> +	self_restore_mask = zalloc(SPR_BITMAP_LENGTH / 8);
> +	if (!self_restore_mask)
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(self_restore_regs); i++) {
> +		int bitmask_idx = self_restore_regs[i] / 64;
> +		uint64_t bitmask_pos = self_restore_regs[i] % 64;
> +		self_restore_mask[bitmask_idx] |= 1ul << bitmask_pos;
> +	}
> +
> +	for (i = 0; i < (SPR_BITMAP_LENGTH / 64); i++) {
> +		self_restore_mask[i] = cpu_to_be64(self_restore_mask[i]);
> +	}
> +
> +	dt_add_property(self_restore, "sprn-bitmask", self_restore_mask,
> +			SPR_BITMAP_LENGTH / 8);
> +	dt_add_property_string(self_restore, "compatible",
> +			       "ibm,opal-self-restore");
> +	free(self_restore_mask);
> +
> +	if (proc_gen != proc_gen_p9 || !self_save_supported)

We need a 
prlog(PR_INFO, "SLW: self-save not supported on this platform");

?

> +		return;


> +
> +	self_save = dt_new(power_mgt, "self-save");
> +	if (!self_save) {
> +		prerror("OCC: Failed to create self save node");
> +		return;
> +	}
> +
> +	self_save_mask = zalloc(SPR_BITMAP_LENGTH / 8);
> +	if (!self_save_mask)
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(self_save_regs); i++) {
> +		int bitmask_idx = self_save_regs[i] / 64;
> +		uint64_t bitmask_pos = self_save_regs[i] % 64;
> +		self_save_mask[bitmask_idx] |= 1ul << bitmask_pos;
> +	}
> +
> +	for (i = 0; i < (SPR_BITMAP_LENGTH / 64); i++) {
> +		self_save_mask[i] = cpu_to_be64(self_save_mask[i]);
> +	}
> +
> +	dt_add_property(self_save, "sprn-bitmask", self_save_mask,
> +			SPR_BITMAP_LENGTH / 8);
> +	dt_add_property_string(self_save, "compatible", "ibm,opal-self-save");
> +	free(self_save_mask);
> +}
> +
>  /* Add device tree properties to describe idle states */
>  void add_cpu_idle_state_properties(void)
>  {
> @@ -1563,4 +1677,6 @@ void slw_init(void)
>  		}
>  	}
>  	add_cpu_idle_state_properties();
> +	if (has_deep_states)
> +		add_cpu_self_save_restore_properties();
>  }
> diff --git a/include/skiboot.h b/include/skiboot.h
> index 9ced240e..d3631dea 100644
> --- a/include/skiboot.h
> +++ b/include/skiboot.h
> @@ -209,6 +209,7 @@ extern void early_uart_init(void);
>  extern void homer_init(void);
>  extern void slw_init(void);
>  extern void add_cpu_idle_state_properties(void);
> +extern void add_cpu_self_save_restore_properties(void);
>  extern void lpc_rtc_init(void);

Apart from these minor quibbles,

Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>

> 
>  /* flash support */
> -- 
> 2.24.1
>