Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59622 )
Change subject: sb/intel/lynxpoint: Use unions for ME PCI registers ......................................................................
sb/intel/lynxpoint: Use unions for ME PCI registers
Wrap bitfield structs in unions to reduce pointer usage.
Change-Id: I8ac901211beb0ef24dff926b1a06004a99e68bda Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/lynxpoint/early_me.c M src/southbridge/intel/lynxpoint/me.c M src/southbridge/intel/lynxpoint/me.h M src/southbridge/intel/lynxpoint/me_status.c 4 files changed, 87 insertions(+), 103 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/59622/1
diff --git a/src/southbridge/intel/lynxpoint/early_me.c b/src/southbridge/intel/lynxpoint/early_me.c index e5ce3f5..9ab00cd 100644 --- a/src/southbridge/intel/lynxpoint/early_me.c +++ b/src/southbridge/intel/lynxpoint/early_me.c @@ -20,26 +20,10 @@ [ME_HFS_ACK_CONTINUE] = "Continue to boot" };
-static inline void pci_read_dword_ptr(void *ptr, int offset) -{ - u32 dword = pci_read_config32(PCH_ME_DEV, offset); - memcpy(ptr, &dword, sizeof(dword)); -} - -static inline void pci_write_dword_ptr(void *ptr, int offset) -{ - u32 dword = 0; - memcpy(&dword, ptr, sizeof(dword)); - pci_write_config32(PCH_ME_DEV, offset, dword); -} - void intel_early_me_status(void) { - struct me_hfs hfs; - struct me_hfs2 hfs2; - - pci_read_dword_ptr(&hfs, PCI_ME_HFS); - pci_read_dword_ptr(&hfs2, PCI_ME_HFS2); + union me_hfs hfs = { .raw = pci_read_config32(PCH_ME_DEV, PCI_ME_HFS) }; + union me_hfs2 hfs2 = { .raw = pci_read_config32(PCH_ME_DEV, PCI_ME_HFS2) };
intel_me_status(&hfs, &hfs2); } @@ -47,15 +31,15 @@ int intel_early_me_init(void) { int count; - struct me_uma uma; - struct me_hfs hfs; + union me_uma uma; + union me_hfs hfs;
printk(BIOS_INFO, "Intel ME early init\n");
/* Wait for ME UMA SIZE VALID bit to be set */ /* FIXME: ME9 BGW indicates a 5 sec poll timeout. */ for (count = ME_RETRY; count > 0; --count) { - pci_read_dword_ptr(&uma, PCI_ME_UMA); + uma.raw = pci_read_config32(PCH_ME_DEV, PCI_ME_UMA); if (uma.valid) break; udelay(ME_DELAY); @@ -66,7 +50,7 @@ }
/* Check for valid firmware */ - pci_read_dword_ptr(&hfs, PCI_ME_HFS); + hfs.raw = pci_read_config32(PCH_ME_DEV, PCI_ME_HFS); if (hfs.fpt_bad) { printk(BIOS_WARNING, "WARNING: ME has bad firmware\n"); return -1; @@ -78,9 +62,8 @@
int intel_early_me_uma_size(void) { - struct me_uma uma; + union me_uma uma = { .raw = pci_read_config32(PCH_ME_DEV, PCI_ME_UMA) };
- pci_read_dword_ptr(&uma, PCI_ME_UMA); if (uma.valid) { printk(BIOS_DEBUG, "ME: Requested %uMB UMA\n", uma.size); return uma.size; @@ -108,8 +91,8 @@ u8 reset; int count; u32 mebase_l, mebase_h; - struct me_hfs hfs; - struct me_did did = { + union me_hfs hfs; + union me_did did = { .init_done = ME_INIT_DONE, .status = status }; @@ -123,7 +106,7 @@ printk(BIOS_DEBUG, "ME: Sending Init Done with status: %d, " "UMA base: 0x%04x\n", status, did.uma_base);
- pci_write_dword_ptr(&did, PCI_ME_H_GS); + pci_write_config32(PCH_ME_DEV, PCI_ME_H_GS, did.raw);
/* * The ME firmware does not respond with an ACK when NOMEM or ERROR @@ -134,7 +117,7 @@
/* Must wait for ME acknowledgement */ for (count = ME_RETRY; count > 0; --count) { - pci_read_dword_ptr(&hfs, PCI_ME_HFS); + hfs.raw = pci_read_config32(PCH_ME_DEV, PCI_ME_HFS); if (hfs.bios_msg_ack) break; udelay(ME_DELAY); diff --git a/src/southbridge/intel/lynxpoint/me.c b/src/southbridge/intel/lynxpoint/me.c index 9196fb3..a1625e8 100644 --- a/src/southbridge/intel/lynxpoint/me.c +++ b/src/southbridge/intel/lynxpoint/me.c @@ -92,13 +92,6 @@ mei_dump(ptr, dword, offset, "WRITE"); }
-static inline void pci_read_dword_ptr(struct device *dev, void *ptr, int offset) -{ - u32 dword = pci_read_config32(dev, offset); - memcpy(ptr, &dword, sizeof(dword)); - mei_dump(ptr, dword, offset, "PCI READ"); -} - static inline void read_host_csr(struct mei_csr *csr) { mei_read_dword_ptr(csr, MEI_H_CSR); @@ -434,11 +427,11 @@ static void intel_me_mbp_clear(struct device *dev) { int count; - struct me_hfs2 hfs2; + union me_hfs2 hfs2;
/* Wait for the mbp_cleared indicator */ for (count = ME_RETRY; count > 0; --count) { - pci_read_dword_ptr(dev, &hfs2, PCI_ME_HFS2); + hfs2.raw = pci_read_config32(dev, PCI_ME_HFS2); if (hfs2.mbp_cleared) break; udelay(ME_DELAY); @@ -539,7 +532,7 @@
void intel_me_finalize(struct device *dev) { - struct me_hfs hfs; + union me_hfs hfs; u32 reg32;
reg32 = pci_read_config32(dev, PCI_BASE_ADDRESS_0); @@ -553,8 +546,7 @@ intel_me_mbp_clear(dev);
/* Make sure ME is in a mode that expects EOP */ - reg32 = pci_read_config32(dev, PCI_ME_HFS); - memcpy(&hfs, ®32, sizeof(u32)); + hfs.raw = pci_read_config32(dev, PCI_ME_HFS);
/* Abort and leave device alone if not normal mode */ if (hfs.fpt_bad || @@ -599,11 +591,8 @@ static enum me_bios_path intel_me_path(struct device *dev) { enum me_bios_path path = ME_DISABLE_BIOS_PATH; - struct me_hfs hfs; - struct me_hfs2 hfs2; - - pci_read_dword_ptr(dev, &hfs, PCI_ME_HFS); - pci_read_dword_ptr(dev, &hfs2, PCI_ME_HFS2); + union me_hfs hfs = { .raw = pci_read_config32(dev, PCI_ME_HFS) }; + union me_hfs2 hfs2 = { .raw = pci_read_config32(dev, PCI_ME_HFS2) };
/* Check and dump status */ intel_me_status(&hfs, &hfs2); @@ -693,11 +682,10 @@ /* Read the Extend register hash of ME firmware */ static int intel_me_extend_valid(struct device *dev) { - struct me_heres status; + union me_heres status = { .raw = pci_read_config32(dev, PCI_ME_HERES) }; u32 extend[8] = {0}; int i, count = 0;
- pci_read_dword_ptr(dev, &status, PCI_ME_HERES); if (!status.extend_feature_present) { printk(BIOS_ERR, "ME: Extend Feature not present\n"); return -1; @@ -762,12 +750,10 @@ struct mbp_header mbp_hdr; u32 me2host_pending; struct mei_csr host; - struct me_hfs2 hfs2; + union me_hfs2 hfs2 = { .raw = pci_read_config32(dev, PCI_ME_HFS2) }; struct mbp_payload *mbp; int i;
- pci_read_dword_ptr(dev, &hfs2, PCI_ME_HFS2); - if (!hfs2.mbp_rdy) { printk(BIOS_ERR, "ME: MBP not ready\n"); goto mbp_failure; diff --git a/src/southbridge/intel/lynxpoint/me.h b/src/southbridge/intel/lynxpoint/me.h index cf797f3..6b072f9 100644 --- a/src/southbridge/intel/lynxpoint/me.h +++ b/src/southbridge/intel/lynxpoint/me.h @@ -50,31 +50,37 @@ #define ME_HFS_ACK_GBL_RESET 6 #define ME_HFS_ACK_CONTINUE 7
-struct me_hfs { - u32 working_state: 4; - u32 mfg_mode: 1; - u32 fpt_bad: 1; - u32 operation_state: 3; - u32 fw_init_complete: 1; - u32 ft_bup_ld_flr: 1; - u32 update_in_progress: 1; - u32 error_code: 4; - u32 operation_mode: 4; - u32 reserved: 4; - u32 boot_options_present: 1; - u32 ack_data: 3; - u32 bios_msg_ack: 4; -} __packed; +union me_hfs { + struct __packed { + u32 working_state: 4; + u32 mfg_mode: 1; + u32 fpt_bad: 1; + u32 operation_state: 3; + u32 fw_init_complete: 1; + u32 ft_bup_ld_flr: 1; + u32 update_in_progress: 1; + u32 error_code: 4; + u32 operation_mode: 4; + u32 reserved: 4; + u32 boot_options_present: 1; + u32 ack_data: 3; + u32 bios_msg_ack: 4; + }; + u32 raw; +};
#define PCI_ME_UMA 0x44
-struct me_uma { - u32 size: 6; - u32 reserved_1: 10; - u32 valid: 1; - u32 reserved_0: 14; - u32 set_to_one: 1; -} __packed; +union me_uma { + struct __packed { + u32 size: 6; + u32 reserved_1: 10; + u32 valid: 1; + u32 reserved_0: 14; + u32 set_to_one: 1; + }; + u32 raw; +};
#define PCI_ME_H_GS 0x4c #define ME_INIT_DONE 1 @@ -83,13 +89,16 @@ #define ME_INIT_STATUS_ERROR 2 #define ME_INIT_STATUS_SUCCESS_OTHER 3 /* SEE ME9 BWG */
-struct me_did { - u32 uma_base: 16; - u32 reserved: 7; - u32 rapid_start: 1; - u32 status: 4; - u32 init_done: 4; -} __packed; +union me_did { + struct __packed { + u32 uma_base: 16; + u32 reserved: 7; + u32 rapid_start: 1; + u32 status: 4; + u32 init_done: 4; + }; + u32 raw; +};
/* * Apparently the GMES register is renamed to HFS2 (or HFSTS2 according @@ -165,22 +174,25 @@ #define ME_HFS2_PMEVENT_PWR_CYCLE_RESET_MOFF 0xb #define ME_HFS2_PMEVENT_SXMX_SXMOFF 0xc
-struct me_hfs2 { - u32 bist_in_progress: 1; - u32 reserved1: 2; - u32 invoke_mebx: 1; - u32 cpu_replaced_sts: 1; - u32 mbp_rdy: 1; - u32 mfs_failure: 1; - u32 warm_reset_request: 1; - u32 cpu_replaced_valid: 1; - u32 reserved2: 4; - u32 mbp_cleared: 1; - u32 reserved3: 2; - u32 current_state: 8; - u32 current_pmevent: 4; - u32 progress_code: 4; -} __packed; +union me_hfs2 { + struct __packed { + u32 bist_in_progress: 1; + u32 reserved1: 2; + u32 invoke_mebx: 1; + u32 cpu_replaced_sts: 1; + u32 mbp_rdy: 1; + u32 mfs_failure: 1; + u32 warm_reset_request: 1; + u32 cpu_replaced_valid: 1; + u32 reserved2: 4; + u32 mbp_cleared: 1; + u32 reserved3: 2; + u32 current_state: 8; + u32 current_pmevent: 4; + u32 progress_code: 4; + }; + u32 raw; +};
#define PCI_ME_H_GS2 0x70 #define PCI_ME_MBP_GIVE_UP 0x01 @@ -190,12 +202,15 @@ #define PCI_ME_EXT_SHA256 0x02 #define PCI_ME_HER(x) (0xc0+(4*(x)))
-struct me_heres { - u32 extend_reg_algorithm: 4; - u32 reserved: 26; - u32 extend_feature_present: 1; - u32 extend_reg_valid: 1; -} __packed; +union me_heres { + struct __packed { + u32 extend_reg_algorithm: 4; + u32 reserved: 26; + u32 extend_feature_present: 1; + u32 extend_reg_valid: 1; + }; + u32 raw; +};
/* * Management Engine MEI registers @@ -313,7 +328,7 @@ };
/* Defined in me_status.c for both romstage and ramstage */ -void intel_me_status(struct me_hfs *hfs, struct me_hfs2 *hfs2); +void intel_me_status(union me_hfs *hfs, union me_hfs2 *hfs2);
void intel_early_me_status(void); int intel_early_me_init(void); diff --git a/src/southbridge/intel/lynxpoint/me_status.c b/src/southbridge/intel/lynxpoint/me_status.c index fb4490f..3f4d9bc 100644 --- a/src/southbridge/intel/lynxpoint/me_status.c +++ b/src/southbridge/intel/lynxpoint/me_status.c @@ -123,7 +123,7 @@ [ME_HFS2_STATE_POLICY_VSCC_NO_MATCH] = "Required VSCC values for flash parts do not match", };
-void intel_me_status(struct me_hfs *hfs, struct me_hfs2 *hfs2) +void intel_me_status(union me_hfs *hfs, union me_hfs2 *hfs2) { if (CONFIG_DEFAULT_CONSOLE_LOGLEVEL < BIOS_DEBUG) return;
-
Angel Pons (Code Review)