Morgan Jang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40286 )
Change subject: drivers/ipmi: Implement "IPMI add SEL" function ......................................................................
drivers/ipmi: Implement "IPMI add SEL" function
Implemented for functions need to add SEL, the information of SEL can be clear when adding SEL.
TEST=Check if SEL is add in BMC
Change-Id: I38f3acb958d12c196d33d34fd5cfa0b784f403b7 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/drivers/ipmi/ipmi_kcs.h M src/drivers/ipmi/ipmi_ops.c M src/drivers/ipmi/ipmi_ops.h 3 files changed, 82 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/40286/1
diff --git a/src/drivers/ipmi/ipmi_kcs.h b/src/drivers/ipmi/ipmi_kcs.h index d511edf..44f668d 100644 --- a/src/drivers/ipmi/ipmi_kcs.h +++ b/src/drivers/ipmi/ipmi_kcs.h @@ -21,6 +21,7 @@ #define IPMI_NETFN_FIRMWARE 0x08 #define IPMI_NETFN_STORAGE 0x0a #define IPMI_READ_FRU_DATA 0x11 +#define IPMI_ADD_SEL_ENTRY 0x44 #define IPMI_NETFN_TRANSPORT 0x0c
#define IPMI_CMD_ACPI_POWERON 0x06 diff --git a/src/drivers/ipmi/ipmi_ops.c b/src/drivers/ipmi/ipmi_ops.c index 25bf077..6d6e562 100644 --- a/src/drivers/ipmi/ipmi_ops.c +++ b/src/drivers/ipmi/ipmi_ops.c @@ -118,3 +118,44 @@ memcpy(uuid, rsp.data, 16); return CB_SUCCESS; } + +enum cb_err ipmi_add_sel(const int port, struct sel_event_record *sel) +{ + int ret; + struct ipmi_add_sel_rsp rsp; + unsigned char data[16]; + + if (sel == NULL) { + printk(BIOS_ERR, "%s failed, null pointer parameter\n", + __func__); + return CB_ERR; + } + + // system event record + if (sel->record_type == 0x2) { + memcpy(data, sel, 16); + } + + // OEM timestamped + if (sel->record_type >= 0xC0 && sel->record_type <= 0xDF) { + memcpy(data, sel, 3); + memcpy(data + 3, &sel->sel_type.oem_ts_type, 13); + } + + // OEM non-timestamped + if (sel->record_type >= 0xE0) { + memcpy(data, sel, 3); + memcpy(data + 3, &sel->sel_type.oem_nots_type, 13); + } + + ret = ipmi_kcs_message(port, IPMI_NETFN_STORAGE, 0x0, + IPMI_ADD_SEL_ENTRY, data, + 16, (unsigned char *) &rsp, sizeof(rsp)); + + if (ret < sizeof(struct ipmi_rsp) || rsp.resp.completion_code) { + printk(BIOS_ERR, "IPMI: %s command failed (ret=%d resp=0x%x)\n", + __func__, ret, rsp.resp.completion_code); + return CB_ERR; + } + return CB_SUCCESS; +} diff --git a/src/drivers/ipmi/ipmi_ops.h b/src/drivers/ipmi/ipmi_ops.h index f889595..5774c58 100644 --- a/src/drivers/ipmi/ipmi_ops.h +++ b/src/drivers/ipmi/ipmi_ops.h @@ -50,6 +50,42 @@ uint8_t data[CONFIG_IPMI_FRU_SINGLE_RW_SZ]; } __packed;
+struct standard_spec_sel_rec{ + uint32_t timestamp; + uint16_t gen_id; + uint8_t evm_rev; + uint8_t sensor_type; + uint8_t sensor_num; + uint8_t event_dir_type; + uint8_t event_data[3]; +}; + +struct oem_ts_spec_sel_rec{ + uint32_t timestamp; + uint8_t manf_id[3]; + uint8_t oem_defined[6]; +}; + +struct oem_nots_spec_sel_rec{ + uint8_t oem_defined[13]; +}; + +/* SEL Event Record */ +struct sel_event_record { + uint16_t record_id; + uint8_t record_type; + union{ + struct standard_spec_sel_rec standard_type; + struct oem_ts_spec_sel_rec oem_ts_type; + struct oem_nots_spec_sel_rec oem_nots_type; + } sel_type; +} __packed; + +struct ipmi_add_sel_rsp { + struct ipmi_rsp resp; + uint16_t record_id; +} __packed; + /* Platform Management FRU Information Storage Definition Spec. */ #define PRODUCT_MAN_TYPE_LEN_OFFSET 3 #define BOARD_MAN_TYPE_LEN_OFFSET 6 @@ -123,4 +159,8 @@ /* Read a particular FRU inventory area into fru_info_str. */ void read_fru_one_area(const int port, uint8_t id, uint16_t offset, struct fru_info_str *fru_info_str, enum fru_area fru_area); + +/* Add a SEL record entry, returns CB_SUCCESS on success and CB_ERR + * if an error occurred */ +enum cb_err ipmi_add_sel(const int port, struct sel_event_record *sel); #endif
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40286 )
Change subject: drivers/ipmi: Implement "IPMI add SEL" function ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/40286/1/src/drivers/ipmi/ipmi_ops.h File src/drivers/ipmi/ipmi_ops.h:
https://review.coreboot.org/c/coreboot/+/40286/1/src/drivers/ipmi/ipmi_ops.h... PS1, Line 53: struct standard_spec_sel_rec{ missing space after struct definition
https://review.coreboot.org/c/coreboot/+/40286/1/src/drivers/ipmi/ipmi_ops.h... PS1, Line 63: struct oem_ts_spec_sel_rec{ missing space after struct definition
https://review.coreboot.org/c/coreboot/+/40286/1/src/drivers/ipmi/ipmi_ops.h... PS1, Line 69: struct oem_nots_spec_sel_rec{ missing space after struct definition
https://review.coreboot.org/c/coreboot/+/40286/1/src/drivers/ipmi/ipmi_ops.c File src/drivers/ipmi/ipmi_ops.c:
https://review.coreboot.org/c/coreboot/+/40286/1/src/drivers/ipmi/ipmi_ops.c... PS1, Line 135: if (sel->record_type == 0x2) { braces {} are not necessary for single statement blocks
Hello build bot (Jenkins), Jonathan Zhang, Johnny Lin, David Hendricks, Jingle Hsu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40286
to look at the new patch set (#2).
Change subject: drivers/ipmi: Implement "IPMI add SEL" function ......................................................................
drivers/ipmi: Implement "IPMI add SEL" function
Implemented for functions need to add SEL, the information of SEL can be clear when adding SEL.
TEST=Check if SEL is add in BMC
Change-Id: I38f3acb958d12c196d33d34fd5cfa0b784f403b7 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/drivers/ipmi/ipmi_kcs.h M src/drivers/ipmi/ipmi_ops.c M src/drivers/ipmi/ipmi_ops.h 3 files changed, 81 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/40286/2
Hello build bot (Jenkins), Jonathan Zhang, Johnny Lin, David Hendricks, Jingle Hsu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40286
to look at the new patch set (#3).
Change subject: drivers/ipmi: Implement "IPMI add SEL" function ......................................................................
drivers/ipmi: Implement "IPMI add SEL" function
Implemented for functions need to add SEL, the information of SEL can be clear when adding SEL.
TEST=Check if SEL is add in BMC
Change-Id: I38f3acb958d12c196d33d34fd5cfa0b784f403b7 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/drivers/ipmi/ipmi_kcs.h M src/drivers/ipmi/ipmi_ops.c M src/drivers/ipmi/ipmi_ops.h 3 files changed, 81 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/40286/3
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40286 )
Change subject: drivers/ipmi: Implement "IPMI add SEL" function ......................................................................
Patch Set 3:
(9 comments)
https://review.coreboot.org/c/coreboot/+/40286/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40286/3//COMMIT_MSG@8 PS3, Line 8: What is *IPMI add SEL*?
https://review.coreboot.org/c/coreboot/+/40286/3//COMMIT_MSG@9 PS3, Line 9: need needed?
https://review.coreboot.org/c/coreboot/+/40286/3//COMMIT_MSG@9 PS3, Line 9: clear cleared?
https://review.coreboot.org/c/coreboot/+/40286/3//COMMIT_MSG@11 PS3, Line 11: TEST=Check if SEL is add in BMC How did you check that?
https://review.coreboot.org/c/coreboot/+/40286/3/src/drivers/ipmi/ipmi_ops.h File src/drivers/ipmi/ipmi_ops.h:
https://review.coreboot.org/c/coreboot/+/40286/3/src/drivers/ipmi/ipmi_ops.h... PS3, Line 56: uint8_t evm_rev; Please use a space instead of a tab.
https://review.coreboot.org/c/coreboot/+/40286/3/src/drivers/ipmi/ipmi_ops.h... PS3, Line 76: uint8_t record_type; Ditto.
https://review.coreboot.org/c/coreboot/+/40286/3/src/drivers/ipmi/ipmi_ops.c File src/drivers/ipmi/ipmi_ops.c:
https://review.coreboot.org/c/coreboot/+/40286/3/src/drivers/ipmi/ipmi_ops.c... PS3, Line 129: printk(BIOS_ERR, "%s failed, null pointer parameter\n", Please reword the error message. BIOS_ERROR messages should be user understandable.
https://review.coreboot.org/c/coreboot/+/40286/3/src/drivers/ipmi/ipmi_ops.c... PS3, Line 130: __func__); Fits into 96 characters per line.
https://review.coreboot.org/c/coreboot/+/40286/3/src/drivers/ipmi/ipmi_ops.c... PS3, Line 138: timestamped timestamp?
Hello build bot (Jenkins), Andrey Petrov, Jonathan Zhang, Johnny Lin, David Hendricks, Jingle Hsu, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40286
to look at the new patch set (#4).
Change subject: drivers/ipmi: Implement the function for logging system event into BMC ......................................................................
drivers/ipmi: Implement the function for logging system event into BMC
Implemented for functions that need to log system event into BMC, the information of system event can be specific.
TEST=Use ipmitool to check if SEL is added into BMC.
Change-Id: I38f3acb958d12c196d33d34fd5cfa0b784f403b7 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/drivers/ipmi/ipmi_kcs.h M src/drivers/ipmi/ipmi_ops.c M src/drivers/ipmi/ipmi_ops.h 3 files changed, 80 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/40286/4
Morgan Jang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40286 )
Change subject: drivers/ipmi: Implement the function for logging system event into BMC ......................................................................
Patch Set 4:
(13 comments)
Patch Set 3:
(9 comments)
https://review.coreboot.org/c/coreboot/+/40286/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40286/3//COMMIT_MSG@8 PS3, Line 8:
What is *IPMI add SEL*?
Done
https://review.coreboot.org/c/coreboot/+/40286/3//COMMIT_MSG@9 PS3, Line 9: clear
cleared?
Done
https://review.coreboot.org/c/coreboot/+/40286/3//COMMIT_MSG@9 PS3, Line 9: need
needed?
Done
https://review.coreboot.org/c/coreboot/+/40286/3//COMMIT_MSG@11 PS3, Line 11: TEST=Check if SEL is add in BMC
How did you check that?
Done
https://review.coreboot.org/c/coreboot/+/40286/3/src/drivers/ipmi/ipmi_ops.h File src/drivers/ipmi/ipmi_ops.h:
https://review.coreboot.org/c/coreboot/+/40286/3/src/drivers/ipmi/ipmi_ops.h... PS3, Line 56: uint8_t evm_rev;
Please use a space instead of a tab.
Done
https://review.coreboot.org/c/coreboot/+/40286/3/src/drivers/ipmi/ipmi_ops.h... PS3, Line 76: uint8_t record_type;
Ditto.
Done
https://review.coreboot.org/c/coreboot/+/40286/1/src/drivers/ipmi/ipmi_ops.h File src/drivers/ipmi/ipmi_ops.h:
https://review.coreboot.org/c/coreboot/+/40286/1/src/drivers/ipmi/ipmi_ops.h... PS1, Line 53: struct standard_spec_sel_rec{
missing space after struct definition
Done
https://review.coreboot.org/c/coreboot/+/40286/1/src/drivers/ipmi/ipmi_ops.h... PS1, Line 63: struct oem_ts_spec_sel_rec{
missing space after struct definition
Done
https://review.coreboot.org/c/coreboot/+/40286/1/src/drivers/ipmi/ipmi_ops.h... PS1, Line 69: struct oem_nots_spec_sel_rec{
missing space after struct definition
Done
https://review.coreboot.org/c/coreboot/+/40286/1/src/drivers/ipmi/ipmi_ops.c File src/drivers/ipmi/ipmi_ops.c:
https://review.coreboot.org/c/coreboot/+/40286/1/src/drivers/ipmi/ipmi_ops.c... PS1, Line 135: if (sel->record_type == 0x2) {
braces {} are not necessary for single statement blocks
Done
https://review.coreboot.org/c/coreboot/+/40286/3/src/drivers/ipmi/ipmi_ops.c File src/drivers/ipmi/ipmi_ops.c:
https://review.coreboot.org/c/coreboot/+/40286/3/src/drivers/ipmi/ipmi_ops.c... PS3, Line 129: printk(BIOS_ERR, "%s failed, null pointer parameter\n",
Please reword the error message. BIOS_ERROR messages should be user understandable.
Done
https://review.coreboot.org/c/coreboot/+/40286/3/src/drivers/ipmi/ipmi_ops.c... PS3, Line 130: __func__);
Fits into 96 characters per line.
Done
https://review.coreboot.org/c/coreboot/+/40286/3/src/drivers/ipmi/ipmi_ops.c... PS3, Line 138: timestamped
timestamp?
Done
Andrey Petrov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40286 )
Change subject: drivers/ipmi: Implement the function for logging system event into BMC ......................................................................
Patch Set 4:
(1 comment)
thanks !
https://review.coreboot.org/c/coreboot/+/40286/4/src/drivers/ipmi/ipmi_ops.c File src/drivers/ipmi/ipmi_ops.c:
https://review.coreboot.org/c/coreboot/+/40286/4/src/drivers/ipmi/ipmi_ops.c... PS4, Line 126: unsigned char data[16]; would it be possible to define type for this data? this way we can avoid all the memcpy and hardcoded lengths, and just can initialize data structure in one go, i.e
stuct blah data = { .a = x, .b = y, c = z } and so on.
Hello build bot (Jenkins), Andrey Petrov, Jonathan Zhang, Johnny Lin, David Hendricks, Jingle Hsu, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40286
to look at the new patch set (#5).
Change subject: drivers/ipmi: Implement the function for logging system event into BMC ......................................................................
drivers/ipmi: Implement the function for logging system event into BMC
Implemented for functions that need to log system event into BMC, the information of system event can be specific.
TEST=Use ipmitool to check if SEL is added into BMC.
Change-Id: I38f3acb958d12c196d33d34fd5cfa0b784f403b7 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/drivers/ipmi/ipmi_kcs.h M src/drivers/ipmi/ipmi_ops.c M src/drivers/ipmi/ipmi_ops.h 3 files changed, 63 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/40286/5
Morgan Jang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40286 )
Change subject: drivers/ipmi: Implement the function for logging system event into BMC ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40286/4/src/drivers/ipmi/ipmi_ops.c File src/drivers/ipmi/ipmi_ops.c:
https://review.coreboot.org/c/coreboot/+/40286/4/src/drivers/ipmi/ipmi_ops.c... PS4, Line 126: unsigned char data[16];
would it be possible to define type for this data? […]
Because I union three event types, just make sure to fill the correct event type, the sel does not need to be converted, I have verified this way, it worked.
Morgan Jang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40286 )
Change subject: drivers/ipmi: Implement the function for logging system event into BMC ......................................................................
Patch Set 5:
(1 comment)
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40286/4/src/drivers/ipmi/ipmi_ops.c File src/drivers/ipmi/ipmi_ops.c:
https://review.coreboot.org/c/coreboot/+/40286/4/src/drivers/ipmi/ipmi_ops.c... PS4, Line 126: unsigned char data[16];
Because I union three event types, just make sure to fill the correct event type, the sel does not n […]
Done
Andrey Petrov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40286 )
Change subject: drivers/ipmi: Implement the function for logging system event into BMC ......................................................................
Patch Set 5: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40286 )
Change subject: drivers/ipmi: Implement the function for logging system event into BMC ......................................................................
Patch Set 5: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/40286/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40286/5//COMMIT_MSG@9 PS5, Line 9: Implemented for functions that need to log system event into BMC, the information of system event can be specific. Please wrap the line after 72 characters.
https://review.coreboot.org/c/coreboot/+/40286/5//COMMIT_MSG@9 PS5, Line 9: event events
https://review.coreboot.org/c/coreboot/+/40286/5//COMMIT_MSG@11 PS5, Line 11: TEST=Use ipmitool to check if SEL is added into BMC. Paste the exact command?
Hello build bot (Jenkins), Andrey Petrov, Jonathan Zhang, Johnny Lin, Paul Menzel, David Hendricks, Jingle Hsu, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40286
to look at the new patch set (#6).
Change subject: drivers/ipmi: Implement the function for logging system event into BMC ......................................................................
drivers/ipmi: Implement the function for logging system event into BMC
Implemented for functions that need to log system events into BMC, the information of system event can be specific.
TEST=Use ipmitool and execute "ipmitool sel list" command to check if SEL is added into BMC.
Change-Id: I38f3acb958d12c196d33d34fd5cfa0b784f403b7 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/drivers/ipmi/ipmi_kcs.h M src/drivers/ipmi/ipmi_ops.c M src/drivers/ipmi/ipmi_ops.h 3 files changed, 63 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/40286/6
Hello build bot (Jenkins), Andrey Petrov, Jonathan Zhang, Johnny Lin, Paul Menzel, David Hendricks, Jingle Hsu, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40286
to look at the new patch set (#7).
Change subject: drivers/ipmi: Implement the function for logging system events into BMC ......................................................................
drivers/ipmi: Implement the function for logging system events into BMC
Implemented for functions that need to log system events into BMC, the information of system events can be specific.
TEST=Use ipmitool and execute "ipmitool sel list" command to check if SEL is added into BMC.
Change-Id: I38f3acb958d12c196d33d34fd5cfa0b784f403b7 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/drivers/ipmi/ipmi_kcs.h M src/drivers/ipmi/ipmi_ops.c M src/drivers/ipmi/ipmi_ops.h 3 files changed, 63 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/40286/7
Morgan Jang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40286 )
Change subject: drivers/ipmi: Implement the function for logging system events into BMC ......................................................................
Patch Set 7:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40286/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40286/5//COMMIT_MSG@9 PS5, Line 9: Implemented for functions that need to log system event into BMC, the information of system event can be specific.
Please wrap the line after 72 characters.
Done
https://review.coreboot.org/c/coreboot/+/40286/5//COMMIT_MSG@9 PS5, Line 9: event
events
Done
https://review.coreboot.org/c/coreboot/+/40286/5//COMMIT_MSG@11 PS5, Line 11: TEST=Use ipmitool to check if SEL is added into BMC.
Paste the exact command?
Done
Andrey Petrov has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40286 )
Change subject: drivers/ipmi: Implement the function for logging system events into BMC ......................................................................
drivers/ipmi: Implement the function for logging system events into BMC
Implemented for functions that need to log system events into BMC, the information of system events can be specific.
TEST=Use ipmitool and execute "ipmitool sel list" command to check if SEL is added into BMC.
Change-Id: I38f3acb958d12c196d33d34fd5cfa0b784f403b7 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/40286 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Andrey Petrov andrey.petrov@gmail.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net --- M src/drivers/ipmi/ipmi_kcs.h M src/drivers/ipmi/ipmi_ops.c M src/drivers/ipmi/ipmi_ops.h 3 files changed, 63 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Andrey Petrov: Looks good to me, approved
diff --git a/src/drivers/ipmi/ipmi_kcs.h b/src/drivers/ipmi/ipmi_kcs.h index d511edf..44f668d 100644 --- a/src/drivers/ipmi/ipmi_kcs.h +++ b/src/drivers/ipmi/ipmi_kcs.h @@ -21,6 +21,7 @@ #define IPMI_NETFN_FIRMWARE 0x08 #define IPMI_NETFN_STORAGE 0x0a #define IPMI_READ_FRU_DATA 0x11 +#define IPMI_ADD_SEL_ENTRY 0x44 #define IPMI_NETFN_TRANSPORT 0x0c
#define IPMI_CMD_ACPI_POWERON 0x06 diff --git a/src/drivers/ipmi/ipmi_ops.c b/src/drivers/ipmi/ipmi_ops.c index 25bf077..2a52ba0 100644 --- a/src/drivers/ipmi/ipmi_ops.c +++ b/src/drivers/ipmi/ipmi_ops.c @@ -118,3 +118,25 @@ memcpy(uuid, rsp.data, 16); return CB_SUCCESS; } + +enum cb_err ipmi_add_sel(const int port, struct sel_event_record *sel) +{ + int ret; + struct ipmi_add_sel_rsp rsp; + + if (sel == NULL) { + printk(BIOS_ERR, "%s failed, system evnt log is not present.\n", __func__); + return CB_ERR; + } + + ret = ipmi_kcs_message(port, IPMI_NETFN_STORAGE, 0x0, + IPMI_ADD_SEL_ENTRY, (const unsigned char *) sel, + 16, (unsigned char *) &rsp, sizeof(rsp)); + + if (ret < sizeof(struct ipmi_rsp) || rsp.resp.completion_code) { + printk(BIOS_ERR, "IPMI: %s command failed (ret=%d resp=0x%x)\n", + __func__, ret, rsp.resp.completion_code); + return CB_ERR; + } + return CB_SUCCESS; +} diff --git a/src/drivers/ipmi/ipmi_ops.h b/src/drivers/ipmi/ipmi_ops.h index f889595..e6e6b77 100644 --- a/src/drivers/ipmi/ipmi_ops.h +++ b/src/drivers/ipmi/ipmi_ops.h @@ -50,6 +50,42 @@ uint8_t data[CONFIG_IPMI_FRU_SINGLE_RW_SZ]; } __packed;
+struct standard_spec_sel_rec { + uint32_t timestamp; + uint16_t gen_id; + uint8_t evm_rev; + uint8_t sensor_type; + uint8_t sensor_num; + uint8_t event_dir_type; + uint8_t event_data[3]; +}; + +struct oem_ts_spec_sel_rec { + uint32_t timestamp; + uint8_t manf_id[3]; + uint8_t oem_defined[6]; +}; + +struct oem_nots_spec_sel_rec { + uint8_t oem_defined[13]; +}; + +/* SEL Event Record */ +struct sel_event_record { + uint16_t record_id; + uint8_t record_type; + union{ + struct standard_spec_sel_rec standard_type; + struct oem_ts_spec_sel_rec oem_ts_type; + struct oem_nots_spec_sel_rec oem_nots_type; + } sel_type; +} __packed; + +struct ipmi_add_sel_rsp { + struct ipmi_rsp resp; + uint16_t record_id; +} __packed; + /* Platform Management FRU Information Storage Definition Spec. */ #define PRODUCT_MAN_TYPE_LEN_OFFSET 3 #define BOARD_MAN_TYPE_LEN_OFFSET 6 @@ -123,4 +159,8 @@ /* Read a particular FRU inventory area into fru_info_str. */ void read_fru_one_area(const int port, uint8_t id, uint16_t offset, struct fru_info_str *fru_info_str, enum fru_area fru_area); + +/* Add a SEL record entry, returns CB_SUCCESS on success and CB_ERR + * if an error occurred */ +enum cb_err ipmi_add_sel(const int port, struct sel_event_record *sel); #endif
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40286 )
Change subject: drivers/ipmi: Implement the function for logging system events into BMC ......................................................................
Patch Set 8:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/2245 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2244 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2243
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Andrey Petrov (Code Review)
-
build bot (Jenkins) (Code Review)
-
Morgan Jang (Code Review)
-
Paul Menzel (Code Review)