Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42053 )
Change subject: sb/intel/bd82x6x: Factor out handling of ME fwcaps ......................................................................
sb/intel/bd82x6x: Factor out handling of ME fwcaps
Although we use different mechanisms to fetch the ME firmware capabilities depending on the PCH, we can still deduplicate most things.
Change-Id: Icdabf433ef53709b770373b2b246b6fd0f9d1d02 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/bd82x6x/me.c M src/southbridge/intel/bd82x6x/me.h M src/southbridge/intel/bd82x6x/me_8.x.c M src/southbridge/intel/bd82x6x/me_common.c 4 files changed, 61 insertions(+), 93 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/42053/1
diff --git a/src/southbridge/intel/bd82x6x/me.c b/src/southbridge/intel/bd82x6x/me.c index 38cdb48..036d10d 100644 --- a/src/southbridge/intel/bd82x6x/me.c +++ b/src/southbridge/intel/bd82x6x/me.c @@ -59,54 +59,15 @@ return 0; }
-static inline void print_cap(const char *name, int state) +/* Get and print ME Firmware Capabilities */ +static void me_print_fwcaps(void) { - printk(BIOS_DEBUG, "ME Capability: %-30s : %sabled\n", - name, state ? "en" : "dis"); -} + mefwcaps_sku cap;
-/* Get ME Firmware Capabilities */ -static int mkhi_get_fwcaps(void) -{ - u32 rule_id = 0; - struct me_fwcaps cap; - struct mkhi_header mkhi = { - .group_id = MKHI_GROUP_ID_FWCAPS, - .command = MKHI_FWCAPS_GET_RULE, - }; - struct mei_header mei = { - .is_complete = 1, - .host_address = MEI_HOST_ADDRESS, - .client_address = MEI_ADDRESS_MKHI, - .length = sizeof(mkhi) + sizeof(rule_id), - }; + if (mkhi_get_fwcaps(&cap)) + return;
- /* Send request and wait for response */ - if (mei_sendrecv(&mei, &mkhi, &rule_id, &cap, sizeof(cap)) < 0) { - printk(BIOS_ERR, "ME: GET FWCAPS message failed\n"); - return -1; - } - - print_cap("Full Network manageability", cap.caps_sku.full_net); - print_cap("Regular Network manageability", cap.caps_sku.std_net); - print_cap("Manageability", cap.caps_sku.manageability); - print_cap("Small business technology", cap.caps_sku.small_business); - print_cap("Level III manageability", cap.caps_sku.l3manageability); - print_cap("IntelR Anti-Theft (AT)", cap.caps_sku.intel_at); - print_cap("IntelR Capability Licensing Service (CLS)", - cap.caps_sku.intel_cls); - print_cap("IntelR Power Sharing Technology (MPC)", - cap.caps_sku.intel_mpc); - print_cap("ICC Over Clocking", cap.caps_sku.icc_over_clocking); - print_cap("Protected Audio Video Path (PAVP)", cap.caps_sku.pavp); - print_cap("IPV6", cap.caps_sku.ipv6); - print_cap("KVM Remote Control (KVM)", cap.caps_sku.kvm); - print_cap("Outbreak Containment Heuristic (OCH)", cap.caps_sku.och); - print_cap("Virtual LAN (VLAN)", cap.caps_sku.vlan); - print_cap("TLS", cap.caps_sku.tls); - print_cap("Wireless LAN (WLAN)", cap.caps_sku.wlan); - - return 0; + me_display_fwcaps(&cap); }
/* Determine the path that we should take based on ME status */ @@ -200,7 +161,7 @@ /* Print ME firmware version */ mkhi_get_fw_version(); /* Print ME firmware capabilities */ - mkhi_get_fwcaps(); + me_print_fwcaps(); }
/* diff --git a/src/southbridge/intel/bd82x6x/me.h b/src/southbridge/intel/bd82x6x/me.h index 29d5749..9ae8030 100644 --- a/src/southbridge/intel/bd82x6x/me.h +++ b/src/southbridge/intel/bd82x6x/me.h @@ -372,4 +372,7 @@ u8 reserved[3]; } __packed;
+void me_display_fwcaps(const mefwcaps_sku *const cap); +int mkhi_get_fwcaps(mefwcaps_sku *const cap); + #endif /* _INTEL_ME_H */ diff --git a/src/southbridge/intel/bd82x6x/me_8.x.c b/src/southbridge/intel/bd82x6x/me_8.x.c index 5659f7a..7eceb7c 100644 --- a/src/southbridge/intel/bd82x6x/me_8.x.c +++ b/src/southbridge/intel/bd82x6x/me_8.x.c @@ -28,12 +28,6 @@ #include <vendorcode/google/chromeos/gnvs.h> #endif
-static inline void print_cap(const char *name, int state) -{ - printk(BIOS_DEBUG, "ME Capability: %-41s : %sabled\n", - name, state ? " en" : "dis"); -} - static void me_print_fw_version(mbp_fw_version_name *vers_name) { if (!vers_name->major_version) { @@ -47,31 +41,6 @@ }
/* Get ME Firmware Capabilities */ -static int mkhi_get_fwcaps(mefwcaps_sku *cap) -{ - u32 rule_id = 0; - struct me_fwcaps cap_msg; - struct mkhi_header mkhi = { - .group_id = MKHI_GROUP_ID_FWCAPS, - .command = MKHI_FWCAPS_GET_RULE, - }; - struct mei_header mei = { - .is_complete = 1, - .host_address = MEI_HOST_ADDRESS, - .client_address = MEI_ADDRESS_MKHI, - .length = sizeof(mkhi) + sizeof(rule_id), - }; - - /* Send request and wait for response */ - if (mei_sendrecv(&mei, &mkhi, &rule_id, &cap_msg, sizeof(cap_msg)) < 0) { - printk(BIOS_ERR, "ME: GET FWCAPS message failed\n"); - return -1; - } - *cap = cap_msg.caps_sku; - return 0; -} - -/* Get ME Firmware Capabilities */ static void me_print_fwcaps(mbp_fw_caps *caps_section) { mefwcaps_sku *cap = &caps_section->fw_capabilities; @@ -81,22 +50,7 @@ return; }
- print_cap("Full Network manageability", cap->full_net); - print_cap("Regular Network manageability", cap->std_net); - print_cap("Manageability", cap->manageability); - print_cap("Small business technology", cap->small_business); - print_cap("Level III manageability", cap->l3manageability); - print_cap("IntelR Anti-Theft (AT)", cap->intel_at); - print_cap("IntelR Capability Licensing Service (CLS)", cap->intel_cls); - print_cap("IntelR Power Sharing Technology (MPC)", cap->intel_mpc); - print_cap("ICC Over Clocking", cap->icc_over_clocking); - print_cap("Protected Audio Video Path (PAVP)", cap->pavp); - print_cap("IPV6", cap->ipv6); - print_cap("KVM Remote Control (KVM)", cap->kvm); - print_cap("Outbreak Containment Heuristic (OCH)", cap->och); - print_cap("Virtual LAN (VLAN)", cap->vlan); - print_cap("TLS", cap->tls); - print_cap("Wireless LAN (WLAN)", cap->wlan); + me_display_fwcaps(cap); }
/* Determine the path that we should take based on ME status */ diff --git a/src/southbridge/intel/bd82x6x/me_common.c b/src/southbridge/intel/bd82x6x/me_common.c index 59c0b38..b6f97b4 100644 --- a/src/southbridge/intel/bd82x6x/me_common.c +++ b/src/southbridge/intel/bd82x6x/me_common.c @@ -30,6 +30,56 @@ return me_bios_path_values[path]; }
+static inline void print_cap(const char *const name, const bool state) +{ + printk(BIOS_DEBUG, "ME Capability: %-41s : %sabled\n", name, state ? " en" : "dis"); +} + +void me_display_fwcaps(const mefwcaps_sku *const cap) +{ + print_cap("Full Network manageability", cap->full_net); + print_cap("Regular Network manageability", cap->std_net); + print_cap("Manageability", cap->manageability); + print_cap("Small business technology", cap->small_business); + print_cap("Level III manageability", cap->l3manageability); + print_cap("IntelR Anti-Theft (AT)", cap->intel_at); + print_cap("IntelR Capability Licensing Service (CLS)", cap->intel_cls); + print_cap("IntelR Power Sharing Technology (MPC)", cap->intel_mpc); + print_cap("ICC Over Clocking", cap->icc_over_clocking); + print_cap("Protected Audio Video Path (PAVP)", cap->pavp); + print_cap("IPV6", cap->ipv6); + print_cap("KVM Remote Control (KVM)", cap->kvm); + print_cap("Outbreak Containment Heuristic (OCH)", cap->och); + print_cap("Virtual LAN (VLAN)", cap->vlan); + print_cap("TLS", cap->tls); + print_cap("Wireless LAN (WLAN)", cap->wlan); +} + +/* Get ME Firmware Capabilities */ +int mkhi_get_fwcaps(mefwcaps_sku *const cap) +{ + u32 rule_id = 0; + struct me_fwcaps cap_msg; + struct mkhi_header mkhi = { + .group_id = MKHI_GROUP_ID_FWCAPS, + .command = MKHI_FWCAPS_GET_RULE, + }; + struct mei_header mei = { + .is_complete = 1, + .host_address = MEI_HOST_ADDRESS, + .client_address = MEI_ADDRESS_MKHI, + .length = sizeof(mkhi) + sizeof(rule_id), + }; + + /* Send request and wait for response */ + if (mei_sendrecv(&mei, &mkhi, &rule_id, &cap_msg, sizeof(cap_msg)) < 0) { + printk(BIOS_ERR, "ME: GET FWCAPS message failed\n"); + return -1; + } + *cap = cap_msg.caps_sku; + return 0; +} + /* MMIO base address for MEI interface */ static u32 *mei_base_address;
Evgeny Zinoviev has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42053 )
Change subject: sb/intel/bd82x6x: Factor out handling of ME fwcaps ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42053/4/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/me_common.c:
https://review.coreboot.org/c/coreboot/+/42053/4/src/southbridge/intel/bd82x... PS4, Line 72: Maybe use tabs here, as everywhere else?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42053 )
Change subject: sb/intel/bd82x6x: Factor out handling of ME fwcaps ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42053/4/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/me_common.c:
https://review.coreboot.org/c/coreboot/+/42053/4/src/southbridge/intel/bd82x... PS4, Line 72:
Maybe use tabs here, as everywhere else?
I think I tried and it went way too far to the right. I can try again though
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42053
to look at the new patch set (#7).
Change subject: sb/intel/bd82x6x: Factor out handling of ME fwcaps ......................................................................
sb/intel/bd82x6x: Factor out handling of ME fwcaps
Although we use different mechanisms to fetch the ME firmware capabilities depending on the PCH, we can still deduplicate most things.
Tested on Asus P8Z77-V LX2, still boots.
Change-Id: Icdabf433ef53709b770373b2b246b6fd0f9d1d02 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/bd82x6x/me.c M src/southbridge/intel/bd82x6x/me.h M src/southbridge/intel/bd82x6x/me_8.x.c M src/southbridge/intel/bd82x6x/me_common.c 4 files changed, 61 insertions(+), 93 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/42053/7
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42053 )
Change subject: sb/intel/bd82x6x: Factor out handling of ME fwcaps ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42053/4/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/me_common.c:
https://review.coreboot.org/c/coreboot/+/42053/4/src/southbridge/intel/bd82x... PS4, Line 72:
I think I tried and it went way too far to the right. […]
Ah no, it originally used spaces. I'd say tabs are for things with many elements that vary wildly in length
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42053 )
Change subject: sb/intel/bd82x6x: Factor out handling of ME fwcaps ......................................................................
Patch Set 8: Code-Review+2
Hello build bot (Jenkins), Arthur Heymans, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42053
to look at the new patch set (#9).
Change subject: sb/intel/bd82x6x: Factor out printing of ME FW capabilities ......................................................................
sb/intel/bd82x6x: Factor out printing of ME FW capabilities
Fetching ME firmware capabilities differs between ME7 and ME8, but the data is equivalent. So, we can factor out printing the capabilities.
Change-Id: Icdabf433ef53709b770373b2b246b6fd0f9d1d02 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/bd82x6x/me.c M src/southbridge/intel/bd82x6x/me.h M src/southbridge/intel/bd82x6x/me_8.x.c M src/southbridge/intel/bd82x6x/me_common.c 4 files changed, 30 insertions(+), 47 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/42053/9
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42053 )
Change subject: sb/intel/bd82x6x: Factor out printing of ME FW capabilities ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42053/9/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/me.h:
https://review.coreboot.org/c/coreboot/+/42053/9/src/southbridge/intel/bd82x... PS9, Line 378: void intel_me_print_fwcaps(const mefwcaps_sku *const cap); need consistent spacing around '*' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/42053/9/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/me_common.c:
https://review.coreboot.org/c/coreboot/+/42053/9/src/southbridge/intel/bd82x... PS9, Line 39: void intel_me_print_fwcaps(const mefwcaps_sku *const cap) need consistent spacing around '*' (ctx:WxV)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42053 )
Change subject: sb/intel/bd82x6x: Factor out printing of ME FW capabilities ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42053/10/src/southbridge/intel/bd82... File src/southbridge/intel/bd82x6x/me.h:
https://review.coreboot.org/c/coreboot/+/42053/10/src/southbridge/intel/bd82... PS10, Line 377: void intel_me_print_fwcaps(const mefwcaps_sku *const cap); need consistent spacing around '*' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/42053/10/src/southbridge/intel/bd82... File src/southbridge/intel/bd82x6x/me_common.c:
https://review.coreboot.org/c/coreboot/+/42053/10/src/southbridge/intel/bd82... PS10, Line 39: void intel_me_print_fwcaps(const mefwcaps_sku *const cap) need consistent spacing around '*' (ctx:WxV)
Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/42053 )
Change subject: sb/intel/bd82x6x: Factor out printing of ME FW capabilities ......................................................................
Abandoned
Rotten
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
build bot (Jenkins) (Code Review)
-
Evgeny Zinoviev (Code Review)