| Message ID | 20170912053059.18702-1-hegdevasant@linux.vnet.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] prd: Enable error logging via firmware_request interface | expand |
Hi Vasant, > In P9 HBRT sends error logs to FSP via firmware_request interface. > This patch adds support to parse error log and send it to FSP. Looks good to me. One comment though: > diff --git a/include/prd-fw-msg.h b/include/prd-fw-msg.h > index c00405d..19f355a 100644 > --- a/include/prd-fw-msg.h > +++ b/include/prd-fw-msg.h > @@ -26,10 +26,24 @@ > enum { > PRD_FW_MSG_TYPE_REQ_NOP = 0, > PRD_FW_MSG_TYPE_RESP_NOP = 1, > + PRD_FW_MSG_TYPE_RESP_GENERIC = 2, > + PRD_FW_MSG_TYPE_REQ_HCODE_UPDATE = 3, > + PRD_FW_MSG_TYPE_HBRT_FSP = 4, > + PRD_FW_MSG_TYPE_ERROR_LOG = 5, > }; > > struct prd_fw_msg { > __be64 type; > + union { > + struct { > + __be64 status; > + } generic_resp; > + struct { > + __be32 plid; > + __be32 size; > + char data[]; > + } __packed errorlog; > + }; > }; > > #define PRD_FW_MSG_BASE_SIZE sizeof(__be64) - would it make sense to add the generic_resp (and TYPE_GENERIC_RESP) as a separate change? it's not really specific to the error log implementation here. Either way: Acked-by: Jeremy Kerr <jk@ozlabs.org> Cheers, Jeremy
On 10/17/2017 01:11 PM, Jeremy Kerr wrote: > Hi Vasant, > >> In P9 HBRT sends error logs to FSP via firmware_request interface. >> This patch adds support to parse error log and send it to FSP. > > Looks good to me. One comment though: > >> diff --git a/include/prd-fw-msg.h b/include/prd-fw-msg.h >> index c00405d..19f355a 100644 >> --- a/include/prd-fw-msg.h >> +++ b/include/prd-fw-msg.h >> @@ -26,10 +26,24 @@ >> enum { >> PRD_FW_MSG_TYPE_REQ_NOP = 0, >> PRD_FW_MSG_TYPE_RESP_NOP = 1, >> + PRD_FW_MSG_TYPE_RESP_GENERIC = 2, >> + PRD_FW_MSG_TYPE_REQ_HCODE_UPDATE = 3, >> + PRD_FW_MSG_TYPE_HBRT_FSP = 4, >> + PRD_FW_MSG_TYPE_ERROR_LOG = 5, >> }; >> >> struct prd_fw_msg { >> __be64 type; >> + union { >> + struct { >> + __be64 status; >> + } generic_resp; >> + struct { >> + __be32 plid; >> + __be32 size; >> + char data[]; >> + } __packed errorlog; >> + }; >> }; >> >> #define PRD_FW_MSG_BASE_SIZE sizeof(__be64) > > - would it make sense to add the generic_resp (and TYPE_GENERIC_RESP) as > a separate change? it's not really specific to the error log > implementation here. Yeah. May be it makes sense to split the patch. Let me send v3. -Vasant
diff --git a/core/hostservices.c b/core/hostservices.c index d1f6fda..dd8cae2 100644 --- a/core/hostservices.c +++ b/core/hostservices.c @@ -329,7 +329,7 @@ static void hservice_start_elog_send(void) goto again; } -static int hservice_send_error_log(uint32_t plid, uint32_t dsize, void *data) +int hservice_send_error_log(uint32_t plid, uint32_t dsize, void *data) { struct hbrt_elog_ent *ent; void *abuf; diff --git a/hw/prd.c b/hw/prd.c index d076c19..4db92eb 100644 --- a/hw/prd.c +++ b/hw/prd.c @@ -23,6 +23,7 @@ #include <fsp.h> #include <mem_region.h> #include <prd-fw-msg.h> +#include <hostservices.h> enum events { EVENT_ATTN = 1 << 0, @@ -375,6 +376,18 @@ static int prd_msg_handle_firmware_req(struct opal_prd_msg *msg) prd_msg->hdr.size = cpu_to_be16(sizeof(*prd_msg)); rc = 0; break; + case PRD_FW_MSG_TYPE_ERROR_LOG: + rc = hservice_send_error_log(fw_req->errorlog.plid, + fw_req->errorlog.size, + fw_req->errorlog.data); + /* Return generic response to HBRT */ + fw_resp->type = cpu_to_be64(PRD_FW_MSG_TYPE_RESP_GENERIC); + fw_resp->generic_resp.status = cpu_to_be64(rc); + prd_msg->fw_resp.len = cpu_to_be64(PRD_FW_MSG_BASE_SIZE + + sizeof(fw_resp->generic_resp)); + prd_msg->hdr.size = cpu_to_be16(sizeof(*prd_msg)); + rc = 0; + break; default: rc = -ENOSYS; } diff --git a/include/hostservices.h b/include/hostservices.h index d6bb3e3..62ef04b 100644 --- a/include/hostservices.h +++ b/include/hostservices.h @@ -38,5 +38,6 @@ void host_services_occ_base_setup(void); int find_master_and_slave_occ(uint64_t **master, uint64_t **slave, int *nr_masters, int *nr_slaves); +int hservice_send_error_log(uint32_t plid, uint32_t dsize, void *data); #endif /* __HOSTSERVICES_H */ diff --git a/include/prd-fw-msg.h b/include/prd-fw-msg.h index c00405d..19f355a 100644 --- a/include/prd-fw-msg.h +++ b/include/prd-fw-msg.h @@ -26,10 +26,24 @@ enum { PRD_FW_MSG_TYPE_REQ_NOP = 0, PRD_FW_MSG_TYPE_RESP_NOP = 1, + PRD_FW_MSG_TYPE_RESP_GENERIC = 2, + PRD_FW_MSG_TYPE_REQ_HCODE_UPDATE = 3, + PRD_FW_MSG_TYPE_HBRT_FSP = 4, + PRD_FW_MSG_TYPE_ERROR_LOG = 5, }; struct prd_fw_msg { __be64 type; + union { + struct { + __be64 status; + } generic_resp; + struct { + __be32 plid; + __be32 size; + char data[]; + } __packed errorlog; + }; }; #define PRD_FW_MSG_BASE_SIZE sizeof(__be64)
In P9 HBRT sends error logs to FSP via firmware_request interface. This patch adds support to parse error log and send it to FSP. CC: Jeremy Kerr <jk@ozlabs.org> CC: Daniel M Crowell <dcrowell@us.ibm.com> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> --- Changes in v2: Changes PRD_FW_MSG_TYPE_ERROR_LOG from 4 to 5 - Thanks Dan -Vasant core/hostservices.c | 2 +- hw/prd.c | 13 +++++++++++++ include/hostservices.h | 1 + include/prd-fw-msg.h | 14 ++++++++++++++ 4 files changed, 29 insertions(+), 1 deletion(-)