Message ID | 20240924160512.4138879-1-dwmw2@infradead.org |
---|---|
State | New |
Headers | show |
Series | [v4,1/6] firmware/psci: Add definitions for PSCI v1.3 specification | expand |
Hi David, > On 24 Sep 2024, at 16:05, David Woodhouse <dwmw2@infradead.org> wrote: > > From: David Woodhouse <dwmw@amazon.co.uk> > > The v1.3 PSCI spec (https://developer.arm.com/documentation/den0022) adds > SYSTEM_OFF2, CLEAN_INV_MEMREGION and CLEAN_INV_MEMREGION_ATTRIBUTES > functions. Add definitions for them and their parameters, along with the > new TIMEOUT, RATE_LIMITED and BUSY error values. > DEN0022F REL superseded DEN0022F ALP1 which doesn’t describe CLEAN_INV_MEMREGION or CLEAN_INV_MEMREGION_ATTRIBUTES. Defining those at another time shouldn’t be a blocker for the rest of this patchset. > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > include/uapi/linux/psci.h | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h > index 42a40ad3fb62..082ed689fdaf 100644 > --- a/include/uapi/linux/psci.h > +++ b/include/uapi/linux/psci.h > @@ -59,6 +59,8 @@ > #define PSCI_1_1_FN_SYSTEM_RESET2 PSCI_0_2_FN(18) > #define PSCI_1_1_FN_MEM_PROTECT PSCI_0_2_FN(19) > #define PSCI_1_1_FN_MEM_PROTECT_CHECK_RANGE PSCI_0_2_FN(20) > +#define PSCI_1_3_FN_SYSTEM_OFF2 PSCI_0_2_FN(21) > +#define PSCI_1_3_FN_CLEAN_INV_MEMREGION_ATTRIBUTES PSCI_0_2_FN(23) > > #define PSCI_1_0_FN64_CPU_DEFAULT_SUSPEND PSCI_0_2_FN64(12) > #define PSCI_1_0_FN64_NODE_HW_STATE PSCI_0_2_FN64(13) > @@ -68,6 +70,8 @@ > > #define PSCI_1_1_FN64_SYSTEM_RESET2 PSCI_0_2_FN64(18) > #define PSCI_1_1_FN64_MEM_PROTECT_CHECK_RANGE PSCI_0_2_FN64(20) > +#define PSCI_1_3_FN64_SYSTEM_OFF2 PSCI_0_2_FN64(21) > +#define PSCI_1_3_FN64_CLEAN_INV_MEMREGION PSCI_0_2_FN64(22) > > /* PSCI v0.2 power state encoding for CPU_SUSPEND function */ > #define PSCI_0_2_POWER_STATE_ID_MASK 0xffff > @@ -100,6 +104,19 @@ > #define PSCI_1_1_RESET_TYPE_SYSTEM_WARM_RESET 0 > #define PSCI_1_1_RESET_TYPE_VENDOR_START 0x80000000U > > +/* PSCI v1.3 hibernate type for SYSTEM_OFF2 */ > +#define PSCI_1_3_HIBERNATE_TYPE_OFF 0 Should it be 1 as hibernate type? > + > +/* PSCI v1.3 flags for CLEAN_INV_MEMREGION */ > +#define PSCI_1_3_CLEAN_INV_MEMREGION_FLAG_DRY_RUN BIT(0) > + > +/* PSCI v1.3 attributes for CLEAN_INV_MEMREGION_ATTRIBUTES */ > +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_OP_TYPE 0 > +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_CPU_RDVZ 1 > +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_LATENCY 2 > +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_RATE_LIMIT 3 > +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_TIMEOUT 4 > + > /* PSCI version decoding (independent of PSCI version) */ > #define PSCI_VERSION_MAJOR_SHIFT 16 > #define PSCI_VERSION_MINOR_MASK \ > @@ -133,5 +150,8 @@ > #define PSCI_RET_NOT_PRESENT -7 > #define PSCI_RET_DISABLED -8 > #define PSCI_RET_INVALID_ADDRESS -9 > +#define PSCI_RET_TIMEOUT -10 > +#define PSCI_RET_RATE_LIMITED -11 > +#define PSCI_RET_BUSY -12 > Thanks Miguel > #endif /* _UAPI_LINUX_PSCI_H */ > -- > 2.44.0 > >
On Thu, 2024-09-26 at 09:56 +0000, Miguel Luis wrote: > > > +/* PSCI v1.3 hibernate type for SYSTEM_OFF2 */ > > +#define PSCI_1_3_HIBERNATE_TYPE_OFF 0 > > Should it be 1 as hibernate type? It is in discovery, as BIT(PSCI_1_3_HIBERNATE_TYPE_OFF) == 1<<0 == 1. But using a bitmask was only supposed to be for the discovery with PSCI_FEATURES, as that has to advertise all the available hibernation types. The actual SYSTEM_OFF2 call was supposed to just take the numeric value as an argument, since obviously *that* one isn't a bitmask. Except... I see that now the spec has finally been updated, it seems to say that 0x1 is the value to pass to the SYSTEM_OFF2 call for HIBERNATE_OFF, not 0x0. Which doesn't seem to make much sense, and I don't recall it being what we discussed. Souvik, what happened there? My understanding was that for each supported hibernation type #n, for which HIBERERNATE_OFF is zero), the PSCI_FEATURES query would include the bit (1<<n) to indicate that it is supported, and then the actual SYSTEM_OFF2 call parameter would be (n) itself, precisely as implemented here. But the spec now seems to say that HIBERNATE_OFF is advertised as (1<<0) in PSCI_FEATURES, but invoked with the value (1). Is it too late to fix? If it isn't just a thinko, what is the intent in the current spec? If we have new hibernate types such that #define PSCI_1_3_HIBERNATE_TYPE_OFF 0 #define PSCI_1_3_HIBERNATE_TYPE_FOO 1 #define PSCI_1_3_HIBERNATE_TYPE_BAR 2 It seems obvious that the PSCI_FEATURES response will contain (1<<0), (1<<1) and (1<<2) for them respectively, but what is supposed to be passed to the actual SYSTEM_OFF2 call? Is it always just going to be (PSCI_1_3_HIBERNATE_TYPE_xxx + 1)? I think we should just fix §5.1.10 to report that 0x0 is HIBERNATE_OFF, yes?
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h index 42a40ad3fb62..082ed689fdaf 100644 --- a/include/uapi/linux/psci.h +++ b/include/uapi/linux/psci.h @@ -59,6 +59,8 @@ #define PSCI_1_1_FN_SYSTEM_RESET2 PSCI_0_2_FN(18) #define PSCI_1_1_FN_MEM_PROTECT PSCI_0_2_FN(19) #define PSCI_1_1_FN_MEM_PROTECT_CHECK_RANGE PSCI_0_2_FN(20) +#define PSCI_1_3_FN_SYSTEM_OFF2 PSCI_0_2_FN(21) +#define PSCI_1_3_FN_CLEAN_INV_MEMREGION_ATTRIBUTES PSCI_0_2_FN(23) #define PSCI_1_0_FN64_CPU_DEFAULT_SUSPEND PSCI_0_2_FN64(12) #define PSCI_1_0_FN64_NODE_HW_STATE PSCI_0_2_FN64(13) @@ -68,6 +70,8 @@ #define PSCI_1_1_FN64_SYSTEM_RESET2 PSCI_0_2_FN64(18) #define PSCI_1_1_FN64_MEM_PROTECT_CHECK_RANGE PSCI_0_2_FN64(20) +#define PSCI_1_3_FN64_SYSTEM_OFF2 PSCI_0_2_FN64(21) +#define PSCI_1_3_FN64_CLEAN_INV_MEMREGION PSCI_0_2_FN64(22) /* PSCI v0.2 power state encoding for CPU_SUSPEND function */ #define PSCI_0_2_POWER_STATE_ID_MASK 0xffff @@ -100,6 +104,19 @@ #define PSCI_1_1_RESET_TYPE_SYSTEM_WARM_RESET 0 #define PSCI_1_1_RESET_TYPE_VENDOR_START 0x80000000U +/* PSCI v1.3 hibernate type for SYSTEM_OFF2 */ +#define PSCI_1_3_HIBERNATE_TYPE_OFF 0 + +/* PSCI v1.3 flags for CLEAN_INV_MEMREGION */ +#define PSCI_1_3_CLEAN_INV_MEMREGION_FLAG_DRY_RUN BIT(0) + +/* PSCI v1.3 attributes for CLEAN_INV_MEMREGION_ATTRIBUTES */ +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_OP_TYPE 0 +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_CPU_RDVZ 1 +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_LATENCY 2 +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_RATE_LIMIT 3 +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_TIMEOUT 4 + /* PSCI version decoding (independent of PSCI version) */ #define PSCI_VERSION_MAJOR_SHIFT 16 #define PSCI_VERSION_MINOR_MASK \ @@ -133,5 +150,8 @@ #define PSCI_RET_NOT_PRESENT -7 #define PSCI_RET_DISABLED -8 #define PSCI_RET_INVALID_ADDRESS -9 +#define PSCI_RET_TIMEOUT -10 +#define PSCI_RET_RATE_LIMITED -11 +#define PSCI_RET_BUSY -12 #endif /* _UAPI_LINUX_PSCI_H */