| Message ID | 20220429094744.72855-4-clombard@linux.vnet.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Complete PLDM responder and enable PLDM support | expand |
On 29/04/2022 11:47, Christophe Lombard wrote: > PLDM Event Messages are sent as PLDM request messages to the Event Receiver > using the PlatformEventMessage command. > > The Event Receiver acknowledges receiving the PLDM Event Message in the > response to this command. > > Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com> > --- > core/pldm/pldm-platform-requests.c | 10 +++ > core/pldm/pldm-responder.c | 108 +++++++++++++++++++++++++++++ > core/pldm/pldm.h | 1 + > 3 files changed, 119 insertions(+) > > diff --git a/core/pldm/pldm-platform-requests.c b/core/pldm/pldm-platform-requests.c > index 5b0bdac4..c3baad54 100644 > --- a/core/pldm/pldm-platform-requests.c > +++ b/core/pldm/pldm-platform-requests.c > @@ -419,6 +419,16 @@ int pldm_platform_init(void) > return OPAL_SUCCESS; > } > > +int plmd_platform_reload_pdrs(void) The version of this function I'm reviewing has been updated. The new version has a memory leak: struct pldm_pdrs *pdrs = zalloc(sizeof(struct pldm_pdrs)); pdrs is never freed as it is re-allocated each time we are notified of an PLDM_PDR_REPOSITORY_CHG_EVENT event > +{ > + if (pdr_ready) { > + pldm_pdr_destroy(repo); > + > + return pdrs_init(); > + } else > + return OPAL_HARDWARE; > +} > + > void pldm_platform_exit(void) > { > if (pdr_ready) > diff --git a/core/pldm/pldm-responder.c b/core/pldm/pldm-responder.c > index 232447e2..5eb0c3f3 100644 > --- a/core/pldm/pldm-responder.c > +++ b/core/pldm/pldm-responder.c > @@ -8,6 +8,7 @@ > #include <opal.h> > #include <stdio.h> > #include <string.h> > +#include <pldm/ibm/libpldm/platform_oem_ibm.h> > #include <pldm/libpldm/platform.h> > #include <pldm/libpldm/utils.h> > #include "pldm.h" > @@ -461,6 +462,112 @@ static struct pldm_cmd pldm_platform_set_event_receiver = { > .handler = platform_set_event_receiver_handler, > }; > > +struct event_message_req { > + uint8_t format_version; > + uint8_t tid; > + uint8_t event_class; > + size_t event_data_offset; > +}; As mentioned in previous patch, that is another example of a duplicated structure with libpldm (struct pldm_platform_event_message_req). > + > +/* > + * PlatformEventMessage (0x10) > + * PLDM Event Messages are sent as PLDM request messages to the Event > + * Receiver using the PlatformEventMessage command. > + */ > +static int platform_event_message(const struct pldm_rx_data *req) > +{ > + char response_msg[PKT_SIZE(struct pldm_platform_event_message_resp)]; > + struct pldm_bios_attribute_update_event_req *request; > + struct event_message_req message_req; > + uint8_t *bios_attribute_handles; > + uint8_t cc = PLDM_SUCCESS; > + int rc, i; > + > + /* decode PlatformEventMessage request data */ > + rc = decode_platform_event_message_req( > + req->msg, > + sizeof(struct event_message_req), > + &message_req.format_version, > + &message_req.tid, > + &message_req.event_class, > + &message_req.event_data_offset); > + if (rc) { > + prlog(PR_ERR, "Failed to decode PlatformEventMessage request, rc = %d\n", rc); > + pldm_cc_resp(req, req->hdrinf.pldm_type, > + req->hdrinf.command, PLDM_ERROR); > + return OPAL_INTERNAL_ERROR; > + } > + > + prlog(PR_DEBUG, "%s - format_version: %d, " > + "tid: %d " > + "event_class: %d " > + "event_data_offset: 0x%lx\n", > + __func__, > + message_req.format_version, > + message_req.tid, > + message_req.event_class, > + message_req.event_data_offset); > + > + /* we don't support any other event than the PDR Repo Changed event */ > + if ((message_req.event_class != PLDM_PDR_REPOSITORY_CHG_EVENT) && > + (message_req.event_class != PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE)) { > + prlog(PR_ERR, "%s - Invalid event class %d in platform event handler\n", > + __func__, message_req.event_class); > + cc = PLDM_ERROR; > + } > + > + rc = encode_platform_event_message_resp( > + req->hdrinf.instance, > + cc, > + PLDM_EVENT_NO_LOGGING, > + (struct pldm_msg *)response_msg); > + if (rc != PLDM_SUCCESS) { > + prlog(PR_ERR, "Encode PlatformEventMessage Error, rc: %d\n", rc); > + pldm_cc_resp(req, req->hdrinf.pldm_type, > + req->hdrinf.command, PLDM_ERROR); > + return OPAL_PARAMETER; > + } > + > + /* send PLDM message over MCTP */ > + rc = pldm_send(req->source_eid, response_msg, sizeof(response_msg)); > + if (rc) { > + prlog(PR_ERR, "Failed to send PlatformEventMessage response, rc = %d\n", rc); > + return OPAL_HARDWARE; > + } > + > + /* invoke the appropriate callback handler */ > + if (message_req.event_class == PLDM_PDR_REPOSITORY_CHG_EVENT) > + return plmd_platform_reload_pdrs(); > + > + /* When the attribute value changes for any BIOS attribute, then > + * PlatformEventMessage command with OEM event type > + * PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE is send to > + * host with the list of BIOS attribute handles. > + */ > + if (message_req.event_class == PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE) { > + request = (struct pldm_bios_attribute_update_event_req *)req->msg->payload; > + bios_attribute_handles = (uint8_t *)request->bios_attribute_handles; > + > + prlog(PR_DEBUG, "%s - OEM_EVENT_BIOS_ATTRIBUTE_UPDATE, handles: %d\n", > + __func__, request->num_handles); > + > + /* list of BIOS attribute handles */ > + for (i = 0; i < request->num_handles; i++) { > + prlog(PR_DEBUG, "%s - OEM_EVENT_BIOS_ATTRIBUTE_UPDATE: handle(%d): %d\n", > + __func__, i, *bios_attribute_handles); > + bios_attribute_handles += sizeof(uint16_t); > + } > + } So anything about PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE is just printed here for debug? We don't seem to be doing anything with it Fred > + > + return OPAL_SUCCESS; > +} > + > +static struct pldm_cmd pldm_platform_event_message = { > + .name = "PLDM_PLATFORM_EVENT_MESSAGE", > + .pldm_cmd_id = PLDM_PLATFORM_EVENT_MESSAGE, > + .handler = platform_event_message, > +}; > + > int pldm_rx_handle_request(struct pldm_rx_data *rx) > { > const struct pldm_type *t; > @@ -504,6 +611,7 @@ int pldm_mctp_responder_init(void) > /* Register platform commands we'll respond to - DSP0248 */ > pldm_add_type(&pldm_platform_type); > pldm_add_cmd(&pldm_platform_type, &pldm_platform_set_event_receiver); > + pldm_add_cmd(&pldm_platform_type, &pldm_platform_event_message); > > return OPAL_SUCCESS; > } > diff --git a/core/pldm/pldm.h b/core/pldm/pldm.h > index 5470f64a..938a554c 100644 > --- a/core/pldm/pldm.h > +++ b/core/pldm/pldm.h > @@ -80,6 +80,7 @@ int pldm_bios_init(void); > > int pldm_base_get_tid_req(void); > > +int plmd_platform_reload_pdrs(void); > int pldm_platform_init(void); > void pldm_platform_exit(void); >
Le 26/04/2023 à 15:19, Frederic Barrat a écrit : > > > On 29/04/2022 11:47, Christophe Lombard wrote: >> PLDM Event Messages are sent as PLDM request messages to the Event >> Receiver >> using the PlatformEventMessage command. >> >> The Event Receiver acknowledges receiving the PLDM Event Message in the >> response to this command. >> >> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com> >> --- >> core/pldm/pldm-platform-requests.c | 10 +++ >> core/pldm/pldm-responder.c | 108 +++++++++++++++++++++++++++++ >> core/pldm/pldm.h | 1 + >> 3 files changed, 119 insertions(+) >> >> diff --git a/core/pldm/pldm-platform-requests.c >> b/core/pldm/pldm-platform-requests.c >> index 5b0bdac4..c3baad54 100644 >> --- a/core/pldm/pldm-platform-requests.c >> +++ b/core/pldm/pldm-platform-requests.c >> @@ -419,6 +419,16 @@ int pldm_platform_init(void) >> return OPAL_SUCCESS; >> } >> +int plmd_platform_reload_pdrs(void) > > > The version of this function I'm reviewing has been updated. The new > version has a memory leak: > struct pldm_pdrs *pdrs = zalloc(sizeof(struct pldm_pdrs)); > pdrs is never freed as it is re-allocated each time we are notified of > an PLDM_PDR_REPOSITORY_CHG_EVENT event > > >> +{ >> + if (pdr_ready) { >> + pldm_pdr_destroy(repo); >> + >> + return pdrs_init(); >> + } else >> + return OPAL_HARDWARE; >> +} >> + >> void pldm_platform_exit(void) >> { >> if (pdr_ready) >> diff --git a/core/pldm/pldm-responder.c b/core/pldm/pldm-responder.c >> index 232447e2..5eb0c3f3 100644 >> --- a/core/pldm/pldm-responder.c >> +++ b/core/pldm/pldm-responder.c >> @@ -8,6 +8,7 @@ >> #include <opal.h> >> #include <stdio.h> >> #include <string.h> >> +#include <pldm/ibm/libpldm/platform_oem_ibm.h> >> #include <pldm/libpldm/platform.h> >> #include <pldm/libpldm/utils.h> >> #include "pldm.h" >> @@ -461,6 +462,112 @@ static struct pldm_cmd >> pldm_platform_set_event_receiver = { >> .handler = platform_set_event_receiver_handler, >> }; >> +struct event_message_req { >> + uint8_t format_version; >> + uint8_t tid; >> + uint8_t event_class; >> + size_t event_data_offset; >> +}; > > > As mentioned in previous patch, that is another example of a > duplicated structure with libpldm (struct > pldm_platform_event_message_req). > > >> + >> +/* >> + * PlatformEventMessage (0x10) >> + * PLDM Event Messages are sent as PLDM request messages to the Event >> + * Receiver using the PlatformEventMessage command. >> + */ >> +static int platform_event_message(const struct pldm_rx_data *req) >> +{ >> + char response_msg[PKT_SIZE(struct >> pldm_platform_event_message_resp)]; >> + struct pldm_bios_attribute_update_event_req *request; >> + struct event_message_req message_req; >> + uint8_t *bios_attribute_handles; >> + uint8_t cc = PLDM_SUCCESS; >> + int rc, i; >> + >> + /* decode PlatformEventMessage request data */ >> + rc = decode_platform_event_message_req( >> + req->msg, >> + sizeof(struct event_message_req), >> + &message_req.format_version, >> + &message_req.tid, >> + &message_req.event_class, >> + &message_req.event_data_offset); >> + if (rc) { >> + prlog(PR_ERR, "Failed to decode PlatformEventMessage >> request, rc = %d\n", rc); >> + pldm_cc_resp(req, req->hdrinf.pldm_type, >> + req->hdrinf.command, PLDM_ERROR); >> + return OPAL_INTERNAL_ERROR; >> + } >> + >> + prlog(PR_DEBUG, "%s - format_version: %d, " >> + "tid: %d " >> + "event_class: %d " >> + "event_data_offset: 0x%lx\n", >> + __func__, >> + message_req.format_version, >> + message_req.tid, >> + message_req.event_class, >> + message_req.event_data_offset); >> + >> + /* we don't support any other event than the PDR Repo Changed >> event */ >> + if ((message_req.event_class != PLDM_PDR_REPOSITORY_CHG_EVENT) && >> + (message_req.event_class != >> PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE)) { >> + prlog(PR_ERR, "%s - Invalid event class %d in platform event >> handler\n", >> + __func__, message_req.event_class); >> + cc = PLDM_ERROR; >> + } >> + >> + rc = encode_platform_event_message_resp( >> + req->hdrinf.instance, >> + cc, >> + PLDM_EVENT_NO_LOGGING, >> + (struct pldm_msg *)response_msg); >> + if (rc != PLDM_SUCCESS) { >> + prlog(PR_ERR, "Encode PlatformEventMessage Error, rc: %d\n", >> rc); >> + pldm_cc_resp(req, req->hdrinf.pldm_type, >> + req->hdrinf.command, PLDM_ERROR); >> + return OPAL_PARAMETER; >> + } >> + >> + /* send PLDM message over MCTP */ >> + rc = pldm_send(req->source_eid, response_msg, >> sizeof(response_msg)); >> + if (rc) { >> + prlog(PR_ERR, "Failed to send PlatformEventMessage response, >> rc = %d\n", rc); >> + return OPAL_HARDWARE; >> + } >> + >> + /* invoke the appropriate callback handler */ >> + if (message_req.event_class == PLDM_PDR_REPOSITORY_CHG_EVENT) >> + return plmd_platform_reload_pdrs(); >> + >> + /* When the attribute value changes for any BIOS attribute, then >> + * PlatformEventMessage command with OEM event type >> + * PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE is send to >> + * host with the list of BIOS attribute handles. >> + */ >> + if (message_req.event_class == >> PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE) { >> + request = (struct pldm_bios_attribute_update_event_req >> *)req->msg->payload; >> + bios_attribute_handles = (uint8_t >> *)request->bios_attribute_handles; >> + >> + prlog(PR_DEBUG, "%s - OEM_EVENT_BIOS_ATTRIBUTE_UPDATE, >> handles: %d\n", >> + __func__, request->num_handles); >> + >> + /* list of BIOS attribute handles */ >> + for (i = 0; i < request->num_handles; i++) { >> + prlog(PR_DEBUG, "%s - OEM_EVENT_BIOS_ATTRIBUTE_UPDATE: >> handle(%d): %d\n", >> + __func__, i, *bios_attribute_handles); >> + bios_attribute_handles += sizeof(uint16_t); >> + } >> + } > > > So anything about PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE is > just printed here for debug? We don't seem to be doing anything with it > Correct, for the time being, nothing is done, just a debug message. > Fred > > >> + >> + return OPAL_SUCCESS; >> +} >> + >> +static struct pldm_cmd pldm_platform_event_message = { >> + .name = "PLDM_PLATFORM_EVENT_MESSAGE", >> + .pldm_cmd_id = PLDM_PLATFORM_EVENT_MESSAGE, >> + .handler = platform_event_message, >> +}; >> + >> int pldm_rx_handle_request(struct pldm_rx_data *rx) >> { >> const struct pldm_type *t; >> @@ -504,6 +611,7 @@ int pldm_mctp_responder_init(void) >> /* Register platform commands we'll respond to - DSP0248 */ >> pldm_add_type(&pldm_platform_type); >> pldm_add_cmd(&pldm_platform_type, >> &pldm_platform_set_event_receiver); >> + pldm_add_cmd(&pldm_platform_type, &pldm_platform_event_message); >> return OPAL_SUCCESS; >> } >> diff --git a/core/pldm/pldm.h b/core/pldm/pldm.h >> index 5470f64a..938a554c 100644 >> --- a/core/pldm/pldm.h >> +++ b/core/pldm/pldm.h >> @@ -80,6 +80,7 @@ int pldm_bios_init(void); >> int pldm_base_get_tid_req(void); >> +int plmd_platform_reload_pdrs(void); >> int pldm_platform_init(void); >> void pldm_platform_exit(void);
diff --git a/core/pldm/pldm-platform-requests.c b/core/pldm/pldm-platform-requests.c index 5b0bdac4..c3baad54 100644 --- a/core/pldm/pldm-platform-requests.c +++ b/core/pldm/pldm-platform-requests.c @@ -419,6 +419,16 @@ int pldm_platform_init(void) return OPAL_SUCCESS; } +int plmd_platform_reload_pdrs(void) +{ + if (pdr_ready) { + pldm_pdr_destroy(repo); + + return pdrs_init(); + } else + return OPAL_HARDWARE; +} + void pldm_platform_exit(void) { if (pdr_ready) diff --git a/core/pldm/pldm-responder.c b/core/pldm/pldm-responder.c index 232447e2..5eb0c3f3 100644 --- a/core/pldm/pldm-responder.c +++ b/core/pldm/pldm-responder.c @@ -8,6 +8,7 @@ #include <opal.h> #include <stdio.h> #include <string.h> +#include <pldm/ibm/libpldm/platform_oem_ibm.h> #include <pldm/libpldm/platform.h> #include <pldm/libpldm/utils.h> #include "pldm.h" @@ -461,6 +462,112 @@ static struct pldm_cmd pldm_platform_set_event_receiver = { .handler = platform_set_event_receiver_handler, }; +struct event_message_req { + uint8_t format_version; + uint8_t tid; + uint8_t event_class; + size_t event_data_offset; +}; + +/* + * PlatformEventMessage (0x10) + * PLDM Event Messages are sent as PLDM request messages to the Event + * Receiver using the PlatformEventMessage command. + */ +static int platform_event_message(const struct pldm_rx_data *req) +{ + char response_msg[PKT_SIZE(struct pldm_platform_event_message_resp)]; + struct pldm_bios_attribute_update_event_req *request; + struct event_message_req message_req; + uint8_t *bios_attribute_handles; + uint8_t cc = PLDM_SUCCESS; + int rc, i; + + /* decode PlatformEventMessage request data */ + rc = decode_platform_event_message_req( + req->msg, + sizeof(struct event_message_req), + &message_req.format_version, + &message_req.tid, + &message_req.event_class, + &message_req.event_data_offset); + if (rc) { + prlog(PR_ERR, "Failed to decode PlatformEventMessage request, rc = %d\n", rc); + pldm_cc_resp(req, req->hdrinf.pldm_type, + req->hdrinf.command, PLDM_ERROR); + return OPAL_INTERNAL_ERROR; + } + + prlog(PR_DEBUG, "%s - format_version: %d, " + "tid: %d " + "event_class: %d " + "event_data_offset: 0x%lx\n", + __func__, + message_req.format_version, + message_req.tid, + message_req.event_class, + message_req.event_data_offset); + + /* we don't support any other event than the PDR Repo Changed event */ + if ((message_req.event_class != PLDM_PDR_REPOSITORY_CHG_EVENT) && + (message_req.event_class != PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE)) { + prlog(PR_ERR, "%s - Invalid event class %d in platform event handler\n", + __func__, message_req.event_class); + cc = PLDM_ERROR; + } + + rc = encode_platform_event_message_resp( + req->hdrinf.instance, + cc, + PLDM_EVENT_NO_LOGGING, + (struct pldm_msg *)response_msg); + if (rc != PLDM_SUCCESS) { + prlog(PR_ERR, "Encode PlatformEventMessage Error, rc: %d\n", rc); + pldm_cc_resp(req, req->hdrinf.pldm_type, + req->hdrinf.command, PLDM_ERROR); + return OPAL_PARAMETER; + } + + /* send PLDM message over MCTP */ + rc = pldm_send(req->source_eid, response_msg, sizeof(response_msg)); + if (rc) { + prlog(PR_ERR, "Failed to send PlatformEventMessage response, rc = %d\n", rc); + return OPAL_HARDWARE; + } + + /* invoke the appropriate callback handler */ + if (message_req.event_class == PLDM_PDR_REPOSITORY_CHG_EVENT) + return plmd_platform_reload_pdrs(); + + /* When the attribute value changes for any BIOS attribute, then + * PlatformEventMessage command with OEM event type + * PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE is send to + * host with the list of BIOS attribute handles. + */ + if (message_req.event_class == PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE) { + request = (struct pldm_bios_attribute_update_event_req *)req->msg->payload; + bios_attribute_handles = (uint8_t *)request->bios_attribute_handles; + + prlog(PR_DEBUG, "%s - OEM_EVENT_BIOS_ATTRIBUTE_UPDATE, handles: %d\n", + __func__, request->num_handles); + + /* list of BIOS attribute handles */ + for (i = 0; i < request->num_handles; i++) { + prlog(PR_DEBUG, "%s - OEM_EVENT_BIOS_ATTRIBUTE_UPDATE: handle(%d): %d\n", + __func__, i, *bios_attribute_handles); + bios_attribute_handles += sizeof(uint16_t); + } + } + + return OPAL_SUCCESS; +} + +static struct pldm_cmd pldm_platform_event_message = { + .name = "PLDM_PLATFORM_EVENT_MESSAGE", + .pldm_cmd_id = PLDM_PLATFORM_EVENT_MESSAGE, + .handler = platform_event_message, +}; + int pldm_rx_handle_request(struct pldm_rx_data *rx) { const struct pldm_type *t; @@ -504,6 +611,7 @@ int pldm_mctp_responder_init(void) /* Register platform commands we'll respond to - DSP0248 */ pldm_add_type(&pldm_platform_type); pldm_add_cmd(&pldm_platform_type, &pldm_platform_set_event_receiver); + pldm_add_cmd(&pldm_platform_type, &pldm_platform_event_message); return OPAL_SUCCESS; } diff --git a/core/pldm/pldm.h b/core/pldm/pldm.h index 5470f64a..938a554c 100644 --- a/core/pldm/pldm.h +++ b/core/pldm/pldm.h @@ -80,6 +80,7 @@ int pldm_bios_init(void); int pldm_base_get_tid_req(void); +int plmd_platform_reload_pdrs(void); int pldm_platform_init(void); void pldm_platform_exit(void);
PLDM Event Messages are sent as PLDM request messages to the Event Receiver using the PlatformEventMessage command. The Event Receiver acknowledges receiving the PLDM Event Message in the response to this command. Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com> --- core/pldm/pldm-platform-requests.c | 10 +++ core/pldm/pldm-responder.c | 108 +++++++++++++++++++++++++++++ core/pldm/pldm.h | 1 + 3 files changed, 119 insertions(+)