Re: [RFC 1/3] powerpc/powernv: Interface to define support and preference for a SPR
Hello Ram,
Thank you for your reviewing the patches.
+/* Interface for the stop state supported and preference */
+#define SELF_RESTORE_TYPE0
+#define SELF_SAVE_TYPE 1
+
+#define NR_PREFERENCES2
+#define PREFERENCE_SHIFT 8
+#define PREFERENCE_MASK 0xff
+
+#define UNSUPPORTED 0x0
+#define SELF_RESTORE_STRICT 0x01
+#define SELF_SAVE_STRICT0x10
+
+/*
+ * Bitmask defining the kind of preferences available.
+ * Note : The higher to lower preference is from LSB to MSB, with a shift of
+ * 8 bits.
A minor comment.
Is there a reason why shift is 8? Shift of 4 must be sufficient,
and a mask of '0xf' should do. And SELF_SAVE_STRICT can be 0x2.
Yes, you're right! We could do away with using fewer bits here.
+/* Caching the lpcr & ptcr support to use later */
+static bool is_lpcr_self_save;
+static bool is_ptcr_self_save;
I understand why you need to track the status of PTCR register.
But its not clear, why LPCR register's save status need to be tracked?
Normally it does not but LPCR was previously unsupported in self-restore
and the kernel saved and restored its value in context. Now that we have
support for saving LPCR automatically I believe we leverage it and
make sure the kernel does not do redundant work.
+
+struct preferred_sprs {
+ u64 spr;
+ u32 preferred_mode;
+ u32 supported_mode;
+};
+
+struct preferred_sprs preferred_sprs[] = {
+ {
+ .spr = SPRN_HSPRG0,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = SPRN_LPCR,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = SPRN_PTCR,
+ .preferred_mode = PREFER_SAVE_RESTORE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = SPRN_HMEER,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = SPRN_HID0,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = P9_STOP_SPR_MSR,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = P9_STOP_SPR_PSSCR,
+ .preferred_mode = PREFER_SAVE_RESTORE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = SPRN_HID1,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = SPRN_HID4,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = SPRN_HID5,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ }
+};
What determines the list of registers tracked in this table?
.snip..
This list is of the SPRs of all the registers that the kernel is interested in
at wakeup. It has been refactored out as a list from what the kernel used
previously in the kernel.
Re: [RFC 1/3] powerpc/powernv: Interface to define support and preference for a SPR
On Wed, Dec 04, 2019 at 03:02:53PM +0530, Pratik Rajesh Sampat wrote:
> Define a bitmask interface to determine support for the Self Restore,
> Self Save or both.
>
> Also define an interface to determine the preference of that SPR to
> be strictly saved or restored or encapsulated with an order of preference.
>
> The preference bitmask is shown as below:
>
> |... | 2nd pref | 1st pref |
>
> MSB LSB
>
> The preference from higher to lower is from LSB to MSB with a shift of 8
> bits.
> Example:
> Prefer self save first, if not available then prefer self
> restore
> The preference mask for this scenario will be seen as below.
> ((SELF_RESTORE_STRICT << PREFERENCE_SHIFT) | SELF_SAVE_STRICT)
> -
> |... | Self restore | Self save |
> -
> MSB LSB
>
> Finally, declare a list of preferred SPRs which encapsulate the bitmaks
> for preferred and supported with defaults of both being set to support
> legacy firmware.
>
> This commit also implements using the above interface and retains the
> legacy functionality of self restore.
>
> Signed-off-by: Pratik Rajesh Sampat
> ---
> arch/powerpc/platforms/powernv/idle.c | 325 +-
> 1 file changed, 269 insertions(+), 56 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/idle.c
> b/arch/powerpc/platforms/powernv/idle.c
> index 78599bca66c2..d38b8b6dcbce 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -32,9 +32,106 @@
> #define P9_STOP_SPR_MSR 2000
> #define P9_STOP_SPR_PSSCR 855
>
> +/* Interface for the stop state supported and preference */
> +#define SELF_RESTORE_TYPE0
> +#define SELF_SAVE_TYPE 1
> +
> +#define NR_PREFERENCES2
> +#define PREFERENCE_SHIFT 8
> +#define PREFERENCE_MASK 0xff
> +
> +#define UNSUPPORTED 0x0
> +#define SELF_RESTORE_STRICT 0x01
> +#define SELF_SAVE_STRICT0x10
> +
> +/*
> + * Bitmask defining the kind of preferences available.
> + * Note : The higher to lower preference is from LSB to MSB, with a shift of
> + * 8 bits.
A minor comment.
Is there a reason why shift is 8? Shift of 4 must be sufficient,
and a mask of '0xf' should do. And SELF_SAVE_STRICT can be 0x2.
> + *
> + * || 2nd pref | 1st pref |
> + *
> + * MSB LSB
> + */
> +/* Prefer Restore if available, otherwise unsupported */
> +#define PREFER_SELF_RESTORE_ONLY SELF_RESTORE_STRICT
> +/* Prefer Save if available, otherwise unsupported */
> +#define PREFER_SELF_SAVE_ONLYSELF_SAVE_STRICT
> +/* Prefer Restore when available, otherwise prefer Save */
> +#define PREFER_RESTORE_SAVE ((SELF_SAVE_STRICT << \
> + PREFERENCE_SHIFT)\
> + | SELF_RESTORE_STRICT)
> +/* Prefer Save when available, otherwise prefer Restore*/
> +#define PREFER_SAVE_RESTORE ((SELF_RESTORE_STRICT <<\
> + PREFERENCE_SHIFT)\
> + | SELF_SAVE_STRICT)
> static u32 supported_cpuidle_states;
> struct pnv_idle_states_t *pnv_idle_states;
> int nr_pnv_idle_states;
> +/* Caching the lpcr & ptcr support to use later */
> +static bool is_lpcr_self_save;
> +static bool is_ptcr_self_save;
I understand why you need to track the status of PTCR register.
But its not clear, why LPCR register's save status need to be tracked?
> +
> +struct preferred_sprs {
> + u64 spr;
> + u32 preferred_mode;
> + u32 supported_mode;
> +};
> +
> +struct preferred_sprs preferred_sprs[] = {
> + {
> + .spr = SPRN_HSPRG0,
> + .preferred_mode = PREFER_RESTORE_SAVE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = SPRN_LPCR,
> + .preferred_mode = PREFER_RESTORE_SAVE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = SPRN_PTCR,
> + .preferred_mode = PREFER_SAVE_RESTORE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = SPRN_HMEER,
> + .preferred_mode = PREFER_RESTORE_SAVE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = SPRN_HID0,
> + .preferred_mode = PREFER_RESTORE_SAVE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = P9_STOP_SPR_MSR,
> + .preferred_mode = PREFER_RESTORE_SAVE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = P9_STOP_SPR_PSSCR,
> + .preferred_mode = PREFER_SAVE_RESTORE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> +
[RFC 1/3] powerpc/powernv: Interface to define support and preference for a SPR
Define a bitmask interface to determine support for the Self Restore,
Self Save or both.
Also define an interface to determine the preference of that SPR to
be strictly saved or restored or encapsulated with an order of preference.
The preference bitmask is shown as below:
|... | 2nd pref | 1st pref |
MSB LSB
The preference from higher to lower is from LSB to MSB with a shift of 8
bits.
Example:
Prefer self save first, if not available then prefer self
restore
The preference mask for this scenario will be seen as below.
((SELF_RESTORE_STRICT << PREFERENCE_SHIFT) | SELF_SAVE_STRICT)
-
|... | Self restore | Self save |
-
MSB LSB
Finally, declare a list of preferred SPRs which encapsulate the bitmaks
for preferred and supported with defaults of both being set to support
legacy firmware.
This commit also implements using the above interface and retains the
legacy functionality of self restore.
Signed-off-by: Pratik Rajesh Sampat
---
arch/powerpc/platforms/powernv/idle.c | 325 +-
1 file changed, 269 insertions(+), 56 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/idle.c
b/arch/powerpc/platforms/powernv/idle.c
index 78599bca66c2..d38b8b6dcbce 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -32,9 +32,106 @@
#define P9_STOP_SPR_MSR 2000
#define P9_STOP_SPR_PSSCR 855
+/* Interface for the stop state supported and preference */
+#define SELF_RESTORE_TYPE0
+#define SELF_SAVE_TYPE 1
+
+#define NR_PREFERENCES2
+#define PREFERENCE_SHIFT 8
+#define PREFERENCE_MASK 0xff
+
+#define UNSUPPORTED 0x0
+#define SELF_RESTORE_STRICT 0x01
+#define SELF_SAVE_STRICT0x10
+
+/*
+ * Bitmask defining the kind of preferences available.
+ * Note : The higher to lower preference is from LSB to MSB, with a shift of
+ * 8 bits.
+ *
+ * || 2nd pref | 1st pref |
+ *
+ * MSB LSB
+ */
+/* Prefer Restore if available, otherwise unsupported */
+#define PREFER_SELF_RESTORE_ONLY SELF_RESTORE_STRICT
+/* Prefer Save if available, otherwise unsupported */
+#define PREFER_SELF_SAVE_ONLY SELF_SAVE_STRICT
+/* Prefer Restore when available, otherwise prefer Save */
+#define PREFER_RESTORE_SAVE((SELF_SAVE_STRICT << \
+ PREFERENCE_SHIFT)\
+ | SELF_RESTORE_STRICT)
+/* Prefer Save when available, otherwise prefer Restore*/
+#define PREFER_SAVE_RESTORE((SELF_RESTORE_STRICT <<\
+ PREFERENCE_SHIFT)\
+ | SELF_SAVE_STRICT)
static u32 supported_cpuidle_states;
struct pnv_idle_states_t *pnv_idle_states;
int nr_pnv_idle_states;
+/* Caching the lpcr & ptcr support to use later */
+static bool is_lpcr_self_save;
+static bool is_ptcr_self_save;
+
+struct preferred_sprs {
+ u64 spr;
+ u32 preferred_mode;
+ u32 supported_mode;
+};
+
+struct preferred_sprs preferred_sprs[] = {
+ {
+ .spr = SPRN_HSPRG0,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = SPRN_LPCR,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = SPRN_PTCR,
+ .preferred_mode = PREFER_SAVE_RESTORE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = SPRN_HMEER,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = SPRN_HID0,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = P9_STOP_SPR_MSR,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = P9_STOP_SPR_PSSCR,
+ .preferred_mode = PREFER_SAVE_RESTORE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = SPRN_HID1,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = SPRN_HID4,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ },
+ {
+ .spr = SPRN_HID5,
+ .preferred_mode = PREFER_RESTORE_SAVE,
+ .supported_mode = SELF_RESTORE_STRICT,
+ }
+};
+
+const int nr_preferred_sprs = ARRAY_SIZE(preferred_sprs);
