| Message ID | 20150211060941.2970.58191.stgit@localhost.localdomain |
|---|---|
| State | Changes Requested |
| Headers | show |
Neelesh Gupta <neelegup@linux.vnet.ibm.com> writes: > From: Jeremy Kerr <jk@ozlabs.org> > > Signed-off-by: Jeremy Kerr <jk@ozlabs.org> > Signed-off-by: Neelesh Gupta <neelegup@linux.vnet.ibm.com> > --- > include/opal.h | 41 ++++++++++++++++++++++++++++++++++++++++- > include/xscom.h | 8 ++++++++ > 2 files changed, 48 insertions(+), 1 deletion(-) Add something to doc/opal-api/opal-messages.txt ? > +enum opal_prd_msg_type { > + OPAL_PRD_MSG_TYPE_INIT = 0, /* RT --> FW */ RT? > +struct opal_prd_msg { > + uint8_t type; > + uint8_t pad[3]; > + uint32_t token; Do we want a version in here? I'm thinking of future when we have either: a) more types b) some types have extended information. Please clearly document how OS should behave in the event of unknown types or newer version. > + union { > + struct { > + uint32_t version; > + uint64_t ipoll; > + } init; > + struct { > + uint64_t proc; > + uint64_t ipoll_status; > + uint64_t ipoll_mask; > + } attn; > + struct { > + uint64_t proc; > + uint64_t ipoll_ack; > + } attn_ack; > + struct { > + uint64_t chip; > + } occ_error; > + struct { > + uint64_t chip; > + } occ_reset; > + }; > +}; seeing as this is an opal_msg, can we also have a BUILD_ASSERT() of it being < sizeof(struct opal_msg) ? Actually, recent discoveries suggest we should actually BUILD_ASSERT() on it being <= 72 as well.
On Mon, 2015-02-16 at 15:42 +1100, Stewart Smith wrote: > Neelesh Gupta <neelegup@linux.vnet.ibm.com> writes: > > From: Jeremy Kerr <jk@ozlabs.org> > > > > Signed-off-by: Jeremy Kerr <jk@ozlabs.org> > > Signed-off-by: Neelesh Gupta <neelegup@linux.vnet.ibm.com> > > --- > > include/opal.h | 41 ++++++++++++++++++++++++++++++++++++++++- > > include/xscom.h | 8 ++++++++ > > 2 files changed, 48 insertions(+), 1 deletion(-) > > Add something to doc/opal-api/opal-messages.txt ? > > > +enum opal_prd_msg_type { > > + OPAL_PRD_MSG_TYPE_INIT = 0, /* RT --> FW */ > > RT? > > > +struct opal_prd_msg { > > + uint8_t type; > > + uint8_t pad[3]; > > + uint32_t token; > > Do we want a version in here? > > I'm thinking of future when we have either: > a) more types > b) some types have extended information. > > Please clearly document how OS should behave in the event of unknown > types or newer version. We just make a new message in that case, though we could use pad as flags ... > > + union { > > + struct { > > + uint32_t version; > > + uint64_t ipoll; > > + } init; > > + struct { > > + uint64_t proc; > > + uint64_t ipoll_status; > > + uint64_t ipoll_mask; > > + } attn; > > + struct { > > + uint64_t proc; > > + uint64_t ipoll_ack; > > + } attn_ack; > > + struct { > > + uint64_t chip; > > + } occ_error; > > + struct { > > + uint64_t chip; > > + } occ_reset; > > + }; > > +}; > > seeing as this is an opal_msg, can we also have a BUILD_ASSERT() of it > being < sizeof(struct opal_msg) ? Ack. > Actually, recent discoveries suggest we should actually BUILD_ASSERT() > on it being <= 72 as well. Ben.
Hi Stewart, >>> +struct opal_prd_msg { >>> + uint8_t type; >>> + uint8_t pad[3]; >>> + uint32_t token; >> >> Do we want a version in here? >> >> I'm thinking of future when we have either: >> a) more types >> b) some types have extended information. >> >> Please clearly document how OS should behave in the event of unknown >> types or newer version. > > We just make a new message in that case, though we could use pad as > flags ... We have a version field in the init message; this'll be the first message that the "client" sends. > >>> + union { >>> + struct { >>> + uint32_t version; >>> + uint64_t ipoll; >>> + } init; >>> + struct { >>> + uint64_t proc; >>> + uint64_t ipoll_status; >>> + uint64_t ipoll_mask; >>> + } attn; >>> + struct { >>> + uint64_t proc; >>> + uint64_t ipoll_ack; >>> + } attn_ack; >>> + struct { >>> + uint64_t chip; >>> + } occ_error; >>> + struct { >>> + uint64_t chip; >>> + } occ_reset; >>> + }; >>> +}; >> >> seeing as this is an opal_msg, can we also have a BUILD_ASSERT() of it >> being < sizeof(struct opal_msg) ? > > Ack. Already have one in the new version of 2/2 :) This change also uses the __be32/__be64 types, for endian annotation. Cheers, Jeremy
Jeremy Kerr <jk@ozlabs.org> writes: > Hi Stewart, > >>>> +struct opal_prd_msg { >>>> + uint8_t type; >>>> + uint8_t pad[3]; >>>> + uint32_t token; >>> >>> Do we want a version in here? >>> >>> I'm thinking of future when we have either: >>> a) more types >>> b) some types have extended information. >>> >>> Please clearly document how OS should behave in the event of unknown >>> types or newer version. >> >> We just make a new message in that case, though we could use pad as >> flags ... > > We have a version field in the init message; this'll be the first > message that the "client" sends. Ahh, excellent! So in the event of bumping the version, it's the initial handshake that works out the versions of messages back and forth. We probably want to document that so I don't ask the same question next time :) (and after I've trained everyone to ask all the questions for each opal.h patch :) >>> seeing as this is an opal_msg, can we also have a BUILD_ASSERT() of it >>> being < sizeof(struct opal_msg) ? >> >> Ack. > > Already have one in the new version of 2/2 :) > > This change also uses the __be32/__be64 types, for endian annotation. Excellent!
diff --git a/include/opal.h b/include/opal.h index 6c9b13a..f9f502f 100644 --- a/include/opal.h +++ b/include/opal.h @@ -154,7 +154,8 @@ #define OPAL_IPMI_SEND 107 #define OPAL_IPMI_RECV 108 #define OPAL_I2C_REQUEST 109 -#define OPAL_LAST 109 +#define OPAL_PRD_MSG 110 +#define OPAL_LAST 110 #ifndef __ASSEMBLY__ @@ -398,6 +399,7 @@ enum OpalMessageType { OPAL_MSG_SHUTDOWN, OPAL_MSG_HMI_EVT, OPAL_MSG_DPO, + OPAL_MSG_PRD, OPAL_MSG_TYPE_MAX, }; @@ -754,6 +756,43 @@ typedef struct oppanel_line { uint64_t line_len; } oppanel_line_t; +enum opal_prd_msg_type { + OPAL_PRD_MSG_TYPE_INIT = 0, /* RT --> FW */ + OPAL_PRD_MSG_TYPE_FINI, /* RT --> FW */ + OPAL_PRD_MSG_TYPE_ATTN, /* RT <-- FW */ + OPAL_PRD_MSG_TYPE_ATTN_ACK, /* RT --> FW */ + OPAL_PRD_MSG_TYPE_OCC_ERROR, /* RT <-- FW */ + OPAL_PRD_MSG_TYPE_OCC_RESET, /* RT <-- FW */ + OPAL_PRD_MSG_TYPE_MAX, +}; + +struct opal_prd_msg { + uint8_t type; + uint8_t pad[3]; + uint32_t token; + union { + struct { + uint32_t version; + uint64_t ipoll; + } init; + struct { + uint64_t proc; + uint64_t ipoll_status; + uint64_t ipoll_mask; + } attn; + struct { + uint64_t proc; + uint64_t ipoll_ack; + } attn_ack; + struct { + uint64_t chip; + } occ_error; + struct { + uint64_t chip; + } occ_reset; + }; +}; + /* * SG entries used for code update * diff --git a/include/xscom.h b/include/xscom.h index 5eed85f..970840b 100644 --- a/include/xscom.h +++ b/include/xscom.h @@ -154,6 +154,14 @@ #define EX_PM_IDLE_ST_HIST_PM_STATE_MASK PPC_BITMASK(0, 2) #define EX_PM_IDLE_ST_HIST_PM_STATE_LSH PPC_BITLSHIFT(2) +/* PRD registers */ +#define PRD_IPOLL_MASK_REG 0x01020013 +#define PRD_ERROR_STATUS 0x01020014 +#define PRD_IPOLL_XSTOP PPC_BIT(0) /* Xstop for host/core/millicode */ +#define PRD_IPOLL_RECOV PPC_BIT(1) /* Recoverable */ +#define PRD_IPOLL_SPEC_ATTN PPC_BIT(2) /* Special attention */ +#define PRD_IPOLL_HOST_ATTN PPC_BIT(3) /* Host attention */ +#define PRD_IPOLL_MASK PPC_BITMASK(0, 3) /* * Error handling: