Nico Huber submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
hwaccess_x86_msr: rename msr function to msr_xxx

This eliminates the need to redefine the rdmsr and wrmsr symbols,
resulting in more understandable code. The common prefix clarify the
relation between the functions.

Change-Id: Ie5ad54d198312578e0a1ee719eec67b37d2bf6a4
Signed-off-by: Thomas Heijligen <thomas.heijligen@secunet.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/62851
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
---
M board_enable.c
M chipset_enable.c
M hwaccess_x86_msr.c
M hwaccess_x86_msr.h
4 files changed, 64 insertions(+), 61 deletions(-)

diff --git a/board_enable.c b/board_enable.c
index 9e4ab93..d740806 100644
--- a/board_enable.c
+++ b/board_enable.c
@@ -1310,10 +1310,10 @@
unsigned long boot_loc;

/* Geode only has a single core */
- if (setup_cpu_msr(0))
+ if (msr_setup(0))
return -1;

- msr = rdmsr(DBE6x_MSR_DIVIL_BALL_OPTS);
+ msr = msr_read(DBE6x_MSR_DIVIL_BALL_OPTS);

if ((msr.lo & (DBE6x_BOOT_OP_LATCHED)) ==
(DBE6x_BOOT_LOC_FWHUB << DBE6x_BOOT_OP_LATCHED_SHIFT))
@@ -1325,9 +1325,9 @@
msr.lo |= ((boot_loc << DBE6x_PRI_BOOT_LOC_SHIFT) |
(boot_loc << DBE6x_SEC_BOOT_LOC_SHIFT));

- wrmsr(DBE6x_MSR_DIVIL_BALL_OPTS, msr);
+ msr_write(DBE6x_MSR_DIVIL_BALL_OPTS, msr);

- cleanup_cpu_msr();
+ msr_cleanup();

return 0;
}
diff --git a/chipset_enable.c b/chipset_enable.c
index b050f64..27ab126 100644
--- a/chipset_enable.c
+++ b/chipset_enable.c
@@ -1264,21 +1264,21 @@
msr_t msr;

/* Geode only has a single core */
- if (setup_cpu_msr(0))
+ if (msr_setup(0))
return -1;

- msr = rdmsr(MSR_RCONF_DEFAULT);
+ msr = msr_read(MSR_RCONF_DEFAULT);
if ((msr.hi >> 24) != 0x22) {
msr.hi &= 0xfbffffff;
- wrmsr(MSR_RCONF_DEFAULT, msr);
+ msr_write(MSR_RCONF_DEFAULT, msr);
}

- msr = rdmsr(MSR_NORF_CTL);
+ msr = msr_read(MSR_NORF_CTL);
/* Raise WE_CS3 bit. */
msr.lo |= 0x08;
- wrmsr(MSR_NORF_CTL, msr);
+ msr_write(MSR_NORF_CTL, msr);

- cleanup_cpu_msr();
+ msr_cleanup();

#undef MSR_RCONF_DEFAULT
#undef MSR_NORF_CTL
diff --git a/hwaccess_x86_msr.c b/hwaccess_x86_msr.c
index 79cf2f8..1fbfa9e 100644
--- a/hwaccess_x86_msr.c
+++ b/hwaccess_x86_msr.c
@@ -41,7 +41,7 @@

static int fd_msr = -1;

-msr_t rdmsr(int addr)
+msr_t msr_read(int addr)
{
uint32_t buf[2];
msr_t msr = { 0xffffffff, 0xffffffff };
@@ -68,7 +68,7 @@
return msr;
}

-int wrmsr(int addr, msr_t msr)
+int msr_write(int addr, msr_t msr)
{
uint32_t buf[2];
buf[0] = msr.lo;
@@ -93,7 +93,7 @@
return 0;
}

-int setup_cpu_msr(int cpu)
+int msr_setup(int cpu)
{
char msrfilename[64] = { 0 };
snprintf(msrfilename, sizeof(msrfilename), "/dev/cpu/%d/msr", cpu);
@@ -114,7 +114,7 @@
return 0;
}

-void cleanup_cpu_msr(void)
+void msr_cleanup(void)
{
if (fd_msr == -1) {
msg_pinfo("No MSR initialized.\n");
@@ -127,14 +127,19 @@
fd_msr = -1;
}
#elif defined(__OpenBSD__) && defined (__i386__) /* This does only work for certain AMD Geode LX systems see amdmsr(4). */
+#include <stdlib.h>
#include <stdint.h>
#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <unistd.h>
#include <sys/ioctl.h>
#include <machine/amdmsr.h>

static int fd_msr = -1;

-msr_t rdmsr(int addr)
+msr_t msr_read(int addr)
{
struct amdmsr_req args;

@@ -154,7 +159,7 @@
return msr;
}

-int wrmsr(int addr, msr_t msr)
+int msr_write(int addr, msr_t msr)
{
struct amdmsr_req args;

@@ -170,7 +175,7 @@
return 0;
}

-int setup_cpu_msr(int cpu)
+int msr_setup(int cpu)
{
char msrfilename[64] = { 0 };
snprintf(msrfilename, sizeof(msrfilename), "/dev/amdmsr");
@@ -190,7 +195,7 @@
return 0;
}

-void cleanup_cpu_msr(void)
+void msr_cleanup(void)
{
if (fd_msr == -1) {
msg_pinfo("No MSR initialized.\n");
@@ -222,7 +227,7 @@

static int fd_msr = -1;

-msr_t rdmsr(int addr)
+msr_t msr_read(int addr)
{
cpu_msr_args_t args;

@@ -242,7 +247,7 @@
return msr;
}

-int wrmsr(int addr, msr_t msr)
+int msr_write(int addr, msr_t msr)
{
cpu_msr_args_t args;

@@ -258,7 +263,7 @@
return 0;
}

-int setup_cpu_msr(int cpu)
+int msr_setup(int cpu)
{
char msrfilename[64] = { 0 };
snprintf(msrfilename, sizeof(msrfilename), "/dev/cpu%d", cpu);
@@ -279,7 +284,7 @@
return 0;
}

-void cleanup_cpu_msr(void)
+void msr_cleanup(void)
{
if (fd_msr == -1) {
msg_pinfo("No MSR initialized.\n");
@@ -293,19 +298,41 @@
}

#elif defined(__MACH__) && defined(__APPLE__)
-/* rdmsr() and wrmsr() are provided by DirectHW which needs neither setup nor cleanup. */
-int setup_cpu_msr(int cpu)
+/*
+ * DirectHW has identical, but conflicting typedef for msr_t. We redefine msr_t
+ * to directhw_msr_t for DirectHW.
+ * rdmsr() and wrmsr() are provided by DirectHW and need neither setup nor cleanup.
+ */
+#define msr_t directhw_msr_t
+#include <DirectHW/DirectHW.h>
+#undef msr_t
+
+msr_t msr_read(int addr)
+{
+ directhw_msr_t msr;
+ msr = rdmsr(addr);
+ return (msr_t){msr.hi, msr.lo};
+}
+
+int msr_write(int addr, msr_t msr)
+{
+ return wrmsr(addr, (directhw_msr_t){msr.hi, msr.lo});
+}
+
+int msr_setup(int cpu)
{
// Always succeed for now
return 0;
}

-void cleanup_cpu_msr(void)
+void msr_cleanup(void)
{
// Nothing, yet.
}
#elif defined(__LIBPAYLOAD__)
-msr_t libpayload_rdmsr(int addr)
+#include <arch/msr.h>
+
+msr_t msr_read(int addr)
{
msr_t msr;
unsigned long long val = _rdmsr(addr);
@@ -314,41 +341,41 @@
return msr;
}

-int libpayload_wrmsr(int addr, msr_t msr)
+int msr_write(int addr, msr_t msr)
{
_wrmsr(addr, msr.lo | ((unsigned long long)msr.hi << 32));
return 0;
}

-int setup_cpu_msr(int cpu)
+int msr_setup(int cpu)
{
return 0;
}

-void cleanup_cpu_msr(void)
+void msr_cleanup(void)
{
}
#else
/* default MSR implementation */
-msr_t rdmsr(int addr)
+msr_t msr_read(int addr)
{
msr_t ret = { 0xffffffff, 0xffffffff };

return ret;
}

-int wrmsr(int addr, msr_t msr)
+int msr_write(int addr, msr_t msr)
{
return -1;
}

-int setup_cpu_msr(int cpu)
+int msr_setup(int cpu)
{
msg_pinfo("No MSR support for your OS yet.\n");
return -1;
}

-void cleanup_cpu_msr(void)
+void msr_cleanup(void)
{
// Nothing, yet.
}
diff --git a/hwaccess_x86_msr.h b/hwaccess_x86_msr.h
index 4229b3b..eda007e 100644
--- a/hwaccess_x86_msr.h
+++ b/hwaccess_x86_msr.h
@@ -18,35 +18,11 @@

#include <stdint.h>

-#if !(defined(__MACH__) && defined(__APPLE__)) && !defined(__FreeBSD__) && !defined(__FreeBSD_kernel__) && !defined(__DragonFly__) && !defined(__LIBPAYLOAD__)
typedef struct { uint32_t hi, lo; } msr_t;
-msr_t rdmsr(int addr);
-int wrmsr(int addr, msr_t msr);
-#endif

-#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__)
-/* FreeBSD already has conflicting definitions for wrmsr/rdmsr. */
-#undef rdmsr
-#undef wrmsr
-#define rdmsr freebsd_rdmsr
-#define wrmsr freebsd_wrmsr
-typedef struct { uint32_t hi, lo; } msr_t;
-msr_t freebsd_rdmsr(int addr);
-int freebsd_wrmsr(int addr, msr_t msr);
-#endif
-
-#if defined(__LIBPAYLOAD__)
-#include <arch/io.h>
-#include <arch/msr.h>
-typedef struct { uint32_t hi, lo; } msr_t;
-msr_t libpayload_rdmsr(int addr);
-int libpayload_wrmsr(int addr, msr_t msr);
-#undef rdmsr
-#define rdmsr libpayload_rdmsr
-#define wrmsr libpayload_wrmsr
-#endif
-
-int setup_cpu_msr(int cpu);
-void cleanup_cpu_msr(void);
+msr_t msr_read(int addr);
+int msr_write(int addr, msr_t msr);
+int msr_setup(int cpu);
+void msr_cleanup(void);

#endif /* __HWACCESS_X86_MSR_H__ */
\ No newline at end of file

4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one.

To view, visit change 62851. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie5ad54d198312578e0a1ee719eec67b37d2bf6a4
Gerrit-Change-Number: 62851
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Heijligen <src@posteo.de>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@mailbox.org>
Gerrit-MessageType: merged