[M] Change in coreboot[master]: include/cpu/msr.h: transform into an union

[Arthur Heymans (Code Review)]

Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/68572 )

Change subject: include/cpu/msr.h: transform into an union
......................................................................

include/cpu/msr.h: transform into an union

This makes it easier to get the content of an msr into a full 64bit
variable.

Change-Id: I1b026cd3807fd68d805051a74b3d31fcde1c5626
Signed-off-by: Arthur Heymans 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/68572
Reviewed-by: Raul Rangel 
Tested-by: build bot (Jenkins) 
Reviewed-by: Angel Pons 
---
M src/cpu/x86/mtrr/earlymtrr.c
M src/include/cpu/x86/msr.h
M src/include/cpu/x86/msr_access.h
M src/soc/intel/common/block/sgx/sgx.c
4 files changed, 43 insertions(+), 36 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Raul Rangel: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/src/cpu/x86/mtrr/earlymtrr.c b/src/cpu/x86/mtrr/earlymtrr.c
index aa430d5..b6cc737 100644
--- a/src/cpu/x86/mtrr/earlymtrr.c
+++ b/src/cpu/x86/mtrr/earlymtrr.c
@@ -55,7 +55,7 @@

void clear_all_var_mtrr(void)
 {
-	msr_t mtrr = {0, 0};
+	msr_t mtrr = { .raw = 0 };
    int vcnt;
    int i;

diff --git a/src/include/cpu/x86/msr.h b/src/include/cpu/x86/msr.h
index f4e3985..33eb457 100644
--- a/src/include/cpu/x86/msr.h
+++ b/src/include/cpu/x86/msr.h
@@ -285,7 +285,7 @@
 static inline uint64_t msr_read(unsigned int reg)
 {
    msr_t msr = rdmsr(reg);
-	return (((uint64_t)msr.hi << 32) | msr.lo);
+	return msr.raw;
 }

/**
@@ -296,10 +296,7 @@
  */
 static inline void msr_write(unsigned int reg, uint64_t value)
 {
-	msr_t msr = {
-		.lo = (unsigned int)value,
-		.hi = (unsigned int)(value >> 32)
-	};
+	msr_t msr = { .raw = value };
    wrmsr(reg, msr);
 }

@@ -315,10 +312,8 @@
    msr_t msr;

msr = rdmsr(reg);
-	msr.lo &= (unsigned int)~unset;
-	msr.hi &= (unsigned int)~(unset >> 32);
-	msr.lo |= (unsigned int)set;
-	msr.hi |= (unsigned int)(set >> 32);
+	msr.raw &= ~unset;
+	msr.raw |= set;
    wrmsr(reg, msr);
 }

diff --git a/src/include/cpu/x86/msr_access.h b/src/include/cpu/x86/msr_access.h
index a7f72fd..33029cc 100644
--- a/src/include/cpu/x86/msr_access.h
+++ b/src/include/cpu/x86/msr_access.h
@@ -6,10 +6,14 @@
 #ifndef __ASSEMBLER__
 #include <types.h>

-typedef struct msr_struct {
-	unsigned int lo;
-	unsigned int hi;
+typedef union msr_union {
+	struct {
+		unsigned int lo;
+		unsigned int hi;
+	};
+	uint64_t raw;
 } msr_t;
+_Static_assert(sizeof(msr_t) == sizeof(uint64_t), "Incorrect size for msr_t");

#if CONFIG(SOC_SETS_MSRS)
 msr_t soc_msr_read(unsigned int index);
diff --git a/src/soc/intel/common/block/sgx/sgx.c b/src/soc/intel/common/block/sgx/sgx.c
index 3008911..88fe174 100644
--- a/src/soc/intel/common/block/sgx/sgx.c
+++ b/src/soc/intel/common/block/sgx/sgx.c
@@ -24,14 +24,7 @@

void prmrr_core_configure(void)
 {
-	union {
-		uint64_t data64;
-		struct {
-			uint32_t lo;
-			uint32_t hi;
-		} data32;
-	} prmrr_base, prmrr_mask;
-	msr_t msr;
+	msr_t prmrr_base, prmrr_mask;

/*
     * Software Developer's Manual Volume 4:
@@ -45,38 +38,36 @@
    	return;

/* PRMRR_PHYS_MASK is in scope "Core" */
-	msr = rdmsr(MSR_PRMRR_PHYS_MASK);
+	prmrr_mask = rdmsr(MSR_PRMRR_PHYS_MASK);
    /* If it is locked don't attempt to write PRMRR MSRs. */
-	if (msr.lo & PRMRR_PHYS_MASK_LOCK)
+	if (prmrr_mask.lo & PRMRR_PHYS_MASK_LOCK)
    	return;

/* PRMRR base and mask are read from the UNCORE PRMRR MSRs
     * that are already set in FSP-M. */
-	if (soc_get_uncore_prmmr_base_and_mask(&prmrr_base.data64,
-						&prmrr_mask.data64) < 0) {
+	if (soc_get_uncore_prmmr_base_and_mask(&prmrr_base.raw,
+						&prmrr_mask.raw) < 0) {
    	printk(BIOS_ERR, "SGX: Failed to get PRMRR base and mask\n");
    	return;
    }

-	if (!prmrr_base.data32.lo) {
+	if (!prmrr_base.lo) {
    	printk(BIOS_ERR, "SGX Error: Uncore PRMRR is not set!\n");
    	return;
    }

-	printk(BIOS_INFO, "SGX: prmrr_base = 0x%llx\n", prmrr_base.data64);
-	printk(BIOS_INFO, "SGX: prmrr_mask = 0x%llx\n", prmrr_mask.data64);
+	printk(BIOS_INFO, "SGX: prmrr_base = 0x%llx\n", prmrr_base.raw);
+	printk(BIOS_INFO, "SGX: prmrr_mask = 0x%llx\n", prmrr_mask.raw);

/* Program core PRMRR MSRs.
     * - Set cache writeback mem attrib in PRMRR base MSR
     * - Clear the valid bit in PRMRR mask MSR
     * - Lock PRMRR MASK MSR */
-	prmrr_base.data32.lo |= MTRR_TYPE_WRBACK;
-	wrmsr(MSR_PRMRR_PHYS_BASE, (msr_t) {.lo = prmrr_base.data32.lo,
-					.hi = prmrr_base.data32.hi});
-	prmrr_mask.data32.lo &= ~PRMRR_PHYS_MASK_VALID;
-	prmrr_mask.data32.lo |= PRMRR_PHYS_MASK_LOCK;
-	wrmsr(MSR_PRMRR_PHYS_MASK, (msr_t) {.lo = prmrr_mask.data32.lo,
-					.hi = prmrr_mask.data32.hi});
+	prmrr_base.lo |= MTRR_TYPE_WRBACK;
+	wrmsr(MSR_PRMRR_PHYS_BASE, prmrr_base);
+	prmrr_mask.lo &= ~PRMRR_PHYS_MASK_VALID;
+	prmrr_mask.lo |= PRMRR_PHYS_MASK_LOCK;
+	wrmsr(MSR_PRMRR_PHYS_MASK, prmrr_mask);
 }

static int is_prmrr_set(void)
@@ -133,7 +124,7 @@
 {
    /* TODO - the Owner Epoch update mechanism is not determined yet,
     * for PoC just write '0's to the MSRs. */
-	msr_t msr = {0, 0};
+	msr_t msr = { .raw = 0 };

/* SGX_OWNEREPOCH is in scope "Package" */
    wrmsr(MSR_SGX_OWNEREPOCH0, msr);