| Message ID | 20181108144511.12678-4-colin.king@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Remove modprobe, replace with module system calls | expand |
On 2018-11-08 10:45 p.m., Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Make the msr library helper now use the new module loading > functions. This involves adding an extra fw parameter which means > msr releated tests need modifying too. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/bios/mtrr/mtrr.c | 7 ++++--- > src/cpu/msr/msr.c | 14 ++++++++------ > src/cpu/nx/nx.c | 2 +- > src/cpu/virt/virt_svm.c | 6 +++--- > src/cpu/virt/virt_vmx.c | 6 +++--- > src/lib/include/fwts_cpu.h | 2 +- > src/lib/src/fwts_cpu.c | 22 ++++++++++++++++------ > 7 files changed, 36 insertions(+), 23 deletions(-) > > diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c > index 5dfb4a60..4948b55a 100644 > --- a/src/bios/mtrr/mtrr.c > +++ b/src/bios/mtrr/mtrr.c > @@ -182,8 +182,9 @@ static int get_mtrrs(void) > return FWTS_OK; > } > > -static int get_default_mtrr(void) { > - if (fwts_cpu_readmsr(0, MTRR_DEF_TYPE_MSR, &mtrr_default) == FWTS_OK) { > +static int get_default_mtrr(fwts_framework *fw) > +{ > + if (fwts_cpu_readmsr(fw, 0, MTRR_DEF_TYPE_MSR, &mtrr_default) == FWTS_OK) { > switch (mtrr_default & 0xFF) { > case 0: > mtrr_default = UNCACHED; > @@ -408,7 +409,7 @@ static int validate_iomem(fwts_framework *fw) > if ((file = fopen("/proc/iomem", "r")) == NULL) > return FWTS_ERROR; > > - if (get_default_mtrr() != FWTS_OK) > + if (get_default_mtrr(fw) != FWTS_OK) > mtrr_default = UNKNOWN; > > while (!feof(file)) { > diff --git a/src/cpu/msr/msr.c b/src/cpu/msr/msr.c > index 70d89650..a9dfd40c 100644 > --- a/src/cpu/msr/msr.c > +++ b/src/cpu/msr/msr.c > @@ -77,7 +77,9 @@ static int msr_deinit(fwts_framework *fw) > return FWTS_OK; > } > > -static int msr_consistent(const uint32_t msr, > +static int msr_consistent( > + fwts_framework *fw, > + const uint32_t msr, > const int shift, > const uint64_t mask, > uint64_t *const vals, > @@ -90,7 +92,7 @@ static int msr_consistent(const uint32_t msr, > > for (cpu = 0; cpu < ncpus; cpu++) { > uint64_t val; > - if (fwts_cpu_readmsr(cpu, msr, &val) != FWTS_OK) { > + if (fwts_cpu_readmsr(fw, cpu, msr, &val) != FWTS_OK) { > return FWTS_ERROR; > } > val >>= shift; > @@ -130,7 +132,7 @@ static int msr_consistent_check(fwts_framework *fw, > free(vals); > return FWTS_ERROR; > } > - if (msr_consistent(msr, shift, mask, > + if (msr_consistent(fw, msr, shift, mask, > vals, &inconsistent_count, inconsistent) != FWTS_OK) { > free(inconsistent); > free(vals); > @@ -197,7 +199,7 @@ static int msr_smrr(fwts_framework *fw) > uint64_t val; > > if (intel_cpu) { > - if (fwts_cpu_readmsr(0, 0xfe, &val) != FWTS_OK) { > + if (fwts_cpu_readmsr(fw, 0, 0xfe, &val) != FWTS_OK) { > fwts_skipped(fw, "Cannot read MSR 0xfe."); > return FWTS_ERROR; > } > @@ -210,7 +212,7 @@ static int msr_smrr(fwts_framework *fw) > msr_consistent_check(fw, LOG_LEVEL_HIGH, "SMRR_PHYSMASK", 0x1f3, 12, 0xfffff, NULL); > msr_consistent_check(fw, LOG_LEVEL_HIGH, "SMRR_VALID", 0x1f3, 11, 0x1, NULL); > > - if (fwts_cpu_readmsr(0, 0x1f2, &val) == FWTS_OK) { > + if (fwts_cpu_readmsr(fw, 0, 0x1f2, &val) == FWTS_OK) { > uint64_t physbase = val & 0xfffff000; > uint64_t type = val & 7; > if ((physbase & 0xfff) != 0) > @@ -221,7 +223,7 @@ static int msr_smrr(fwts_framework *fw) > fwts_failed(fw, LOG_LEVEL_HIGH, "MSRSMRR_TYPE", > "SMRR: SMRR_TYPE is 0x%" PRIx64 ", should be 0x6 (Write-Back).", type); > } > - if (fwts_cpu_readmsr(0, 0x1f3, &val) == FWTS_OK) { > + if (fwts_cpu_readmsr(fw, 0, 0x1f3, &val) == FWTS_OK) { > uint64_t physmask = val & 0xfffff000; > uint64_t valid = (val >> 11) & 1; > > diff --git a/src/cpu/nx/nx.c b/src/cpu/nx/nx.c > index 66a7b6f1..5caf0c6c 100644 > --- a/src/cpu/nx/nx.c > +++ b/src/cpu/nx/nx.c > @@ -169,7 +169,7 @@ static int nx_test3(fwts_framework *fw) > fwts_cpu_free_info(fwts_nx_cpuinfo); > return FWTS_OK; > } > - if (fwts_cpu_readmsr(i, 0x1a0, &val) != FWTS_OK) { > + if (fwts_cpu_readmsr(fw, i, 0x1a0, &val) != FWTS_OK) { > fwts_log_error(fw, "Cannot read msr 0x1a0 on CPU%d", i); > fwts_cpu_free_info(fwts_nx_cpuinfo); > return FWTS_ERROR; > diff --git a/src/cpu/virt/virt_svm.c b/src/cpu/virt/virt_svm.c > index 8cf85305..59ebb83f 100644 > --- a/src/cpu/virt/virt_svm.c > +++ b/src/cpu/virt/virt_svm.c > @@ -52,14 +52,14 @@ static int can_lock_with_msr(void) > return (fwts_virt_cpuinfo->x86 & 0x10); > } > > -static int vt_locked_by_bios(void) > +static int vt_locked_by_bios(fwts_framework *fw) > { > uint64_t msr; > > if (!can_lock_with_msr()) > return 0; > > - if (fwts_cpu_readmsr(0, MSR_FEATURE_CONTROL, &msr)) > + if (fwts_cpu_readmsr(fw, 0, MSR_FEATURE_CONTROL, &msr)) > return -1; > > return ((msr & 0x1000) == 0x1000); /* SVM capable but locked by bios*/ > @@ -72,7 +72,7 @@ void virt_check_svm(fwts_framework *fw) > if (!cpu_has_svm()) > fwts_skipped(fw, "Processor does not support Virtualization extensions, won't test BIOS configuration, skipping test."); > else { > - int ret = vt_locked_by_bios(); > + int ret = vt_locked_by_bios(fw); > switch (ret) { > case 0: > fwts_passed(fw, "Virtualization extensions supported and enabled by BIOS."); > diff --git a/src/cpu/virt/virt_vmx.c b/src/cpu/virt/virt_vmx.c > index 307424e7..c4cfb6ab 100644 > --- a/src/cpu/virt/virt_vmx.c > +++ b/src/cpu/virt/virt_vmx.c > @@ -55,11 +55,11 @@ static int cpu_has_vmx(void) > return (strstr(fwts_virt_cpuinfo->flags, "vmx") != NULL); > } > > -static int vt_locked_by_bios(void) > +static int vt_locked_by_bios(fwts_framework *fw) > { > uint64_t msr; > > - if (fwts_cpu_readmsr(0, MSR_FEATURE_CONTROL, &msr)) > + if (fwts_cpu_readmsr(fw, 0, MSR_FEATURE_CONTROL, &msr)) > return -1; > > return (msr & 5) == 1; /* VT capable but locked by bios*/ > @@ -72,7 +72,7 @@ void virt_check_vmx(fwts_framework *fw) > if (!cpu_has_vmx()) > fwts_skipped(fw, "Processor does not support Virtualization extensions, won't test BIOS configuration, skipping test."); > else { > - int ret = vt_locked_by_bios(); > + int ret = vt_locked_by_bios(fw); > switch (ret) { > case 0: > fwts_passed(fw, "Virtualization extensions supported and enabled by BIOS."); > diff --git a/src/lib/include/fwts_cpu.h b/src/lib/include/fwts_cpu.h > index 5aa92984..4192e571 100644 > --- a/src/lib/include/fwts_cpu.h > +++ b/src/lib/include/fwts_cpu.h > @@ -57,7 +57,7 @@ typedef struct cpu_benchmark_result { > uint64_t cycles; > } fwts_cpu_benchmark_result; > > -int fwts_cpu_readmsr(const int cpu, const uint32_t reg, uint64_t *val); > +int fwts_cpu_readmsr(fwts_framework *fw, const int cpu, const uint32_t reg, uint64_t *val); > > int fwts_cpu_is_Intel(bool *is_intel); > int fwts_cpu_is_AMD(bool *is_amd); > diff --git a/src/lib/src/fwts_cpu.c b/src/lib/src/fwts_cpu.c > index 18ff7823..a3c1638a 100644 > --- a/src/lib/src/fwts_cpu.c > +++ b/src/lib/src/fwts_cpu.c > @@ -53,7 +53,11 @@ static pid_t *fwts_cpu_pids; > * fwts_cpu_readmsr() > * Read a given msr on a specificied CPU > */ > -int fwts_cpu_readmsr(const int cpu, const uint32_t reg, uint64_t *val) > +int fwts_cpu_readmsr( > + fwts_framework *fw, > + const int cpu, > + const uint32_t reg, > + uint64_t *val) > { > char buffer[PATH_MAX]; > uint64_t value = 0; > @@ -62,12 +66,18 @@ int fwts_cpu_readmsr(const int cpu, const uint32_t reg, uint64_t *val) > > snprintf(buffer, sizeof(buffer), "/dev/cpu/%d/msr", cpu); > if ((fd = open(buffer, O_RDONLY)) < 0) { > - /* Hrm, msr not there, so force modprobe msr and see what happens */ > - pid_t pid; > - if (fwts_pipe_open_ro("modprobe msr", &pid, &fd) < 0) > - return FWTS_ERROR; > - fwts_pipe_close(fd, pid); > + bool loaded = false; > > + /* > + * msr not there, so force a load of the msr > + * module and retry > + */ > + if (fwts_module_load(fw, "msr") != FWTS_OK) > + return FWTS_ERROR; > + if (fwts_module_loaded(fw, "msr", &loaded) != FWTS_OK) > + return FWTS_ERROR; > + if (!loaded) > + return FWTS_ERROR; > if ((fd = open(buffer, O_RDONLY)) < 0) > return FWTS_ERROR; /* Really failed */ > } > Acked-by: Alex Hung <alex.hung@canonical.com>
On 11/8/18 10:45 PM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Make the msr library helper now use the new module loading > functions. This involves adding an extra fw parameter which means > msr releated tests need modifying too. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/bios/mtrr/mtrr.c | 7 ++++--- > src/cpu/msr/msr.c | 14 ++++++++------ > src/cpu/nx/nx.c | 2 +- > src/cpu/virt/virt_svm.c | 6 +++--- > src/cpu/virt/virt_vmx.c | 6 +++--- > src/lib/include/fwts_cpu.h | 2 +- > src/lib/src/fwts_cpu.c | 22 ++++++++++++++++------ > 7 files changed, 36 insertions(+), 23 deletions(-) > > diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c > index 5dfb4a60..4948b55a 100644 > --- a/src/bios/mtrr/mtrr.c > +++ b/src/bios/mtrr/mtrr.c > @@ -182,8 +182,9 @@ static int get_mtrrs(void) > return FWTS_OK; > } > > -static int get_default_mtrr(void) { > - if (fwts_cpu_readmsr(0, MTRR_DEF_TYPE_MSR, &mtrr_default) == FWTS_OK) { > +static int get_default_mtrr(fwts_framework *fw) > +{ > + if (fwts_cpu_readmsr(fw, 0, MTRR_DEF_TYPE_MSR, &mtrr_default) == FWTS_OK) { > switch (mtrr_default & 0xFF) { > case 0: > mtrr_default = UNCACHED; > @@ -408,7 +409,7 @@ static int validate_iomem(fwts_framework *fw) > if ((file = fopen("/proc/iomem", "r")) == NULL) > return FWTS_ERROR; > > - if (get_default_mtrr() != FWTS_OK) > + if (get_default_mtrr(fw) != FWTS_OK) > mtrr_default = UNKNOWN; > > while (!feof(file)) { > diff --git a/src/cpu/msr/msr.c b/src/cpu/msr/msr.c > index 70d89650..a9dfd40c 100644 > --- a/src/cpu/msr/msr.c > +++ b/src/cpu/msr/msr.c > @@ -77,7 +77,9 @@ static int msr_deinit(fwts_framework *fw) > return FWTS_OK; > } > > -static int msr_consistent(const uint32_t msr, > +static int msr_consistent( > + fwts_framework *fw, > + const uint32_t msr, > const int shift, > const uint64_t mask, > uint64_t *const vals, > @@ -90,7 +92,7 @@ static int msr_consistent(const uint32_t msr, > > for (cpu = 0; cpu < ncpus; cpu++) { > uint64_t val; > - if (fwts_cpu_readmsr(cpu, msr, &val) != FWTS_OK) { > + if (fwts_cpu_readmsr(fw, cpu, msr, &val) != FWTS_OK) { > return FWTS_ERROR; > } > val >>= shift; > @@ -130,7 +132,7 @@ static int msr_consistent_check(fwts_framework *fw, > free(vals); > return FWTS_ERROR; > } > - if (msr_consistent(msr, shift, mask, > + if (msr_consistent(fw, msr, shift, mask, > vals, &inconsistent_count, inconsistent) != FWTS_OK) { > free(inconsistent); > free(vals); > @@ -197,7 +199,7 @@ static int msr_smrr(fwts_framework *fw) > uint64_t val; > > if (intel_cpu) { > - if (fwts_cpu_readmsr(0, 0xfe, &val) != FWTS_OK) { > + if (fwts_cpu_readmsr(fw, 0, 0xfe, &val) != FWTS_OK) { > fwts_skipped(fw, "Cannot read MSR 0xfe."); > return FWTS_ERROR; > } > @@ -210,7 +212,7 @@ static int msr_smrr(fwts_framework *fw) > msr_consistent_check(fw, LOG_LEVEL_HIGH, "SMRR_PHYSMASK", 0x1f3, 12, 0xfffff, NULL); > msr_consistent_check(fw, LOG_LEVEL_HIGH, "SMRR_VALID", 0x1f3, 11, 0x1, NULL); > > - if (fwts_cpu_readmsr(0, 0x1f2, &val) == FWTS_OK) { > + if (fwts_cpu_readmsr(fw, 0, 0x1f2, &val) == FWTS_OK) { > uint64_t physbase = val & 0xfffff000; > uint64_t type = val & 7; > if ((physbase & 0xfff) != 0) > @@ -221,7 +223,7 @@ static int msr_smrr(fwts_framework *fw) > fwts_failed(fw, LOG_LEVEL_HIGH, "MSRSMRR_TYPE", > "SMRR: SMRR_TYPE is 0x%" PRIx64 ", should be 0x6 (Write-Back).", type); > } > - if (fwts_cpu_readmsr(0, 0x1f3, &val) == FWTS_OK) { > + if (fwts_cpu_readmsr(fw, 0, 0x1f3, &val) == FWTS_OK) { > uint64_t physmask = val & 0xfffff000; > uint64_t valid = (val >> 11) & 1; > > diff --git a/src/cpu/nx/nx.c b/src/cpu/nx/nx.c > index 66a7b6f1..5caf0c6c 100644 > --- a/src/cpu/nx/nx.c > +++ b/src/cpu/nx/nx.c > @@ -169,7 +169,7 @@ static int nx_test3(fwts_framework *fw) > fwts_cpu_free_info(fwts_nx_cpuinfo); > return FWTS_OK; > } > - if (fwts_cpu_readmsr(i, 0x1a0, &val) != FWTS_OK) { > + if (fwts_cpu_readmsr(fw, i, 0x1a0, &val) != FWTS_OK) { > fwts_log_error(fw, "Cannot read msr 0x1a0 on CPU%d", i); > fwts_cpu_free_info(fwts_nx_cpuinfo); > return FWTS_ERROR; > diff --git a/src/cpu/virt/virt_svm.c b/src/cpu/virt/virt_svm.c > index 8cf85305..59ebb83f 100644 > --- a/src/cpu/virt/virt_svm.c > +++ b/src/cpu/virt/virt_svm.c > @@ -52,14 +52,14 @@ static int can_lock_with_msr(void) > return (fwts_virt_cpuinfo->x86 & 0x10); > } > > -static int vt_locked_by_bios(void) > +static int vt_locked_by_bios(fwts_framework *fw) > { > uint64_t msr; > > if (!can_lock_with_msr()) > return 0; > > - if (fwts_cpu_readmsr(0, MSR_FEATURE_CONTROL, &msr)) > + if (fwts_cpu_readmsr(fw, 0, MSR_FEATURE_CONTROL, &msr)) > return -1; > > return ((msr & 0x1000) == 0x1000); /* SVM capable but locked by bios*/ > @@ -72,7 +72,7 @@ void virt_check_svm(fwts_framework *fw) > if (!cpu_has_svm()) > fwts_skipped(fw, "Processor does not support Virtualization extensions, won't test BIOS configuration, skipping test."); > else { > - int ret = vt_locked_by_bios(); > + int ret = vt_locked_by_bios(fw); > switch (ret) { > case 0: > fwts_passed(fw, "Virtualization extensions supported and enabled by BIOS."); > diff --git a/src/cpu/virt/virt_vmx.c b/src/cpu/virt/virt_vmx.c > index 307424e7..c4cfb6ab 100644 > --- a/src/cpu/virt/virt_vmx.c > +++ b/src/cpu/virt/virt_vmx.c > @@ -55,11 +55,11 @@ static int cpu_has_vmx(void) > return (strstr(fwts_virt_cpuinfo->flags, "vmx") != NULL); > } > > -static int vt_locked_by_bios(void) > +static int vt_locked_by_bios(fwts_framework *fw) > { > uint64_t msr; > > - if (fwts_cpu_readmsr(0, MSR_FEATURE_CONTROL, &msr)) > + if (fwts_cpu_readmsr(fw, 0, MSR_FEATURE_CONTROL, &msr)) > return -1; > > return (msr & 5) == 1; /* VT capable but locked by bios*/ > @@ -72,7 +72,7 @@ void virt_check_vmx(fwts_framework *fw) > if (!cpu_has_vmx()) > fwts_skipped(fw, "Processor does not support Virtualization extensions, won't test BIOS configuration, skipping test."); > else { > - int ret = vt_locked_by_bios(); > + int ret = vt_locked_by_bios(fw); > switch (ret) { > case 0: > fwts_passed(fw, "Virtualization extensions supported and enabled by BIOS."); > diff --git a/src/lib/include/fwts_cpu.h b/src/lib/include/fwts_cpu.h > index 5aa92984..4192e571 100644 > --- a/src/lib/include/fwts_cpu.h > +++ b/src/lib/include/fwts_cpu.h > @@ -57,7 +57,7 @@ typedef struct cpu_benchmark_result { > uint64_t cycles; > } fwts_cpu_benchmark_result; > > -int fwts_cpu_readmsr(const int cpu, const uint32_t reg, uint64_t *val); > +int fwts_cpu_readmsr(fwts_framework *fw, const int cpu, const uint32_t reg, uint64_t *val); > > int fwts_cpu_is_Intel(bool *is_intel); > int fwts_cpu_is_AMD(bool *is_amd); > diff --git a/src/lib/src/fwts_cpu.c b/src/lib/src/fwts_cpu.c > index 18ff7823..a3c1638a 100644 > --- a/src/lib/src/fwts_cpu.c > +++ b/src/lib/src/fwts_cpu.c > @@ -53,7 +53,11 @@ static pid_t *fwts_cpu_pids; > * fwts_cpu_readmsr() > * Read a given msr on a specificied CPU > */ > -int fwts_cpu_readmsr(const int cpu, const uint32_t reg, uint64_t *val) > +int fwts_cpu_readmsr( > + fwts_framework *fw, > + const int cpu, > + const uint32_t reg, > + uint64_t *val) > { > char buffer[PATH_MAX]; > uint64_t value = 0; > @@ -62,12 +66,18 @@ int fwts_cpu_readmsr(const int cpu, const uint32_t reg, uint64_t *val) > > snprintf(buffer, sizeof(buffer), "/dev/cpu/%d/msr", cpu); > if ((fd = open(buffer, O_RDONLY)) < 0) { > - /* Hrm, msr not there, so force modprobe msr and see what happens */ > - pid_t pid; > - if (fwts_pipe_open_ro("modprobe msr", &pid, &fd) < 0) > - return FWTS_ERROR; > - fwts_pipe_close(fd, pid); > + bool loaded = false; > > + /* > + * msr not there, so force a load of the msr > + * module and retry > + */ > + if (fwts_module_load(fw, "msr") != FWTS_OK) > + return FWTS_ERROR; > + if (fwts_module_loaded(fw, "msr", &loaded) != FWTS_OK) > + return FWTS_ERROR; > + if (!loaded) > + return FWTS_ERROR; > if ((fd = open(buffer, O_RDONLY)) < 0) > return FWTS_ERROR; /* Really failed */ > } Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c index 5dfb4a60..4948b55a 100644 --- a/src/bios/mtrr/mtrr.c +++ b/src/bios/mtrr/mtrr.c @@ -182,8 +182,9 @@ static int get_mtrrs(void) return FWTS_OK; } -static int get_default_mtrr(void) { - if (fwts_cpu_readmsr(0, MTRR_DEF_TYPE_MSR, &mtrr_default) == FWTS_OK) { +static int get_default_mtrr(fwts_framework *fw) +{ + if (fwts_cpu_readmsr(fw, 0, MTRR_DEF_TYPE_MSR, &mtrr_default) == FWTS_OK) { switch (mtrr_default & 0xFF) { case 0: mtrr_default = UNCACHED; @@ -408,7 +409,7 @@ static int validate_iomem(fwts_framework *fw) if ((file = fopen("/proc/iomem", "r")) == NULL) return FWTS_ERROR; - if (get_default_mtrr() != FWTS_OK) + if (get_default_mtrr(fw) != FWTS_OK) mtrr_default = UNKNOWN; while (!feof(file)) { diff --git a/src/cpu/msr/msr.c b/src/cpu/msr/msr.c index 70d89650..a9dfd40c 100644 --- a/src/cpu/msr/msr.c +++ b/src/cpu/msr/msr.c @@ -77,7 +77,9 @@ static int msr_deinit(fwts_framework *fw) return FWTS_OK; } -static int msr_consistent(const uint32_t msr, +static int msr_consistent( + fwts_framework *fw, + const uint32_t msr, const int shift, const uint64_t mask, uint64_t *const vals, @@ -90,7 +92,7 @@ static int msr_consistent(const uint32_t msr, for (cpu = 0; cpu < ncpus; cpu++) { uint64_t val; - if (fwts_cpu_readmsr(cpu, msr, &val) != FWTS_OK) { + if (fwts_cpu_readmsr(fw, cpu, msr, &val) != FWTS_OK) { return FWTS_ERROR; } val >>= shift; @@ -130,7 +132,7 @@ static int msr_consistent_check(fwts_framework *fw, free(vals); return FWTS_ERROR; } - if (msr_consistent(msr, shift, mask, + if (msr_consistent(fw, msr, shift, mask, vals, &inconsistent_count, inconsistent) != FWTS_OK) { free(inconsistent); free(vals); @@ -197,7 +199,7 @@ static int msr_smrr(fwts_framework *fw) uint64_t val; if (intel_cpu) { - if (fwts_cpu_readmsr(0, 0xfe, &val) != FWTS_OK) { + if (fwts_cpu_readmsr(fw, 0, 0xfe, &val) != FWTS_OK) { fwts_skipped(fw, "Cannot read MSR 0xfe."); return FWTS_ERROR; } @@ -210,7 +212,7 @@ static int msr_smrr(fwts_framework *fw) msr_consistent_check(fw, LOG_LEVEL_HIGH, "SMRR_PHYSMASK", 0x1f3, 12, 0xfffff, NULL); msr_consistent_check(fw, LOG_LEVEL_HIGH, "SMRR_VALID", 0x1f3, 11, 0x1, NULL); - if (fwts_cpu_readmsr(0, 0x1f2, &val) == FWTS_OK) { + if (fwts_cpu_readmsr(fw, 0, 0x1f2, &val) == FWTS_OK) { uint64_t physbase = val & 0xfffff000; uint64_t type = val & 7; if ((physbase & 0xfff) != 0) @@ -221,7 +223,7 @@ static int msr_smrr(fwts_framework *fw) fwts_failed(fw, LOG_LEVEL_HIGH, "MSRSMRR_TYPE", "SMRR: SMRR_TYPE is 0x%" PRIx64 ", should be 0x6 (Write-Back).", type); } - if (fwts_cpu_readmsr(0, 0x1f3, &val) == FWTS_OK) { + if (fwts_cpu_readmsr(fw, 0, 0x1f3, &val) == FWTS_OK) { uint64_t physmask = val & 0xfffff000; uint64_t valid = (val >> 11) & 1; diff --git a/src/cpu/nx/nx.c b/src/cpu/nx/nx.c index 66a7b6f1..5caf0c6c 100644 --- a/src/cpu/nx/nx.c +++ b/src/cpu/nx/nx.c @@ -169,7 +169,7 @@ static int nx_test3(fwts_framework *fw) fwts_cpu_free_info(fwts_nx_cpuinfo); return FWTS_OK; } - if (fwts_cpu_readmsr(i, 0x1a0, &val) != FWTS_OK) { + if (fwts_cpu_readmsr(fw, i, 0x1a0, &val) != FWTS_OK) { fwts_log_error(fw, "Cannot read msr 0x1a0 on CPU%d", i); fwts_cpu_free_info(fwts_nx_cpuinfo); return FWTS_ERROR; diff --git a/src/cpu/virt/virt_svm.c b/src/cpu/virt/virt_svm.c index 8cf85305..59ebb83f 100644 --- a/src/cpu/virt/virt_svm.c +++ b/src/cpu/virt/virt_svm.c @@ -52,14 +52,14 @@ static int can_lock_with_msr(void) return (fwts_virt_cpuinfo->x86 & 0x10); } -static int vt_locked_by_bios(void) +static int vt_locked_by_bios(fwts_framework *fw) { uint64_t msr; if (!can_lock_with_msr()) return 0; - if (fwts_cpu_readmsr(0, MSR_FEATURE_CONTROL, &msr)) + if (fwts_cpu_readmsr(fw, 0, MSR_FEATURE_CONTROL, &msr)) return -1; return ((msr & 0x1000) == 0x1000); /* SVM capable but locked by bios*/ @@ -72,7 +72,7 @@ void virt_check_svm(fwts_framework *fw) if (!cpu_has_svm()) fwts_skipped(fw, "Processor does not support Virtualization extensions, won't test BIOS configuration, skipping test."); else { - int ret = vt_locked_by_bios(); + int ret = vt_locked_by_bios(fw); switch (ret) { case 0: fwts_passed(fw, "Virtualization extensions supported and enabled by BIOS."); diff --git a/src/cpu/virt/virt_vmx.c b/src/cpu/virt/virt_vmx.c index 307424e7..c4cfb6ab 100644 --- a/src/cpu/virt/virt_vmx.c +++ b/src/cpu/virt/virt_vmx.c @@ -55,11 +55,11 @@ static int cpu_has_vmx(void) return (strstr(fwts_virt_cpuinfo->flags, "vmx") != NULL); } -static int vt_locked_by_bios(void) +static int vt_locked_by_bios(fwts_framework *fw) { uint64_t msr; - if (fwts_cpu_readmsr(0, MSR_FEATURE_CONTROL, &msr)) + if (fwts_cpu_readmsr(fw, 0, MSR_FEATURE_CONTROL, &msr)) return -1; return (msr & 5) == 1; /* VT capable but locked by bios*/ @@ -72,7 +72,7 @@ void virt_check_vmx(fwts_framework *fw) if (!cpu_has_vmx()) fwts_skipped(fw, "Processor does not support Virtualization extensions, won't test BIOS configuration, skipping test."); else { - int ret = vt_locked_by_bios(); + int ret = vt_locked_by_bios(fw); switch (ret) { case 0: fwts_passed(fw, "Virtualization extensions supported and enabled by BIOS."); diff --git a/src/lib/include/fwts_cpu.h b/src/lib/include/fwts_cpu.h index 5aa92984..4192e571 100644 --- a/src/lib/include/fwts_cpu.h +++ b/src/lib/include/fwts_cpu.h @@ -57,7 +57,7 @@ typedef struct cpu_benchmark_result { uint64_t cycles; } fwts_cpu_benchmark_result; -int fwts_cpu_readmsr(const int cpu, const uint32_t reg, uint64_t *val); +int fwts_cpu_readmsr(fwts_framework *fw, const int cpu, const uint32_t reg, uint64_t *val); int fwts_cpu_is_Intel(bool *is_intel); int fwts_cpu_is_AMD(bool *is_amd); diff --git a/src/lib/src/fwts_cpu.c b/src/lib/src/fwts_cpu.c index 18ff7823..a3c1638a 100644 --- a/src/lib/src/fwts_cpu.c +++ b/src/lib/src/fwts_cpu.c @@ -53,7 +53,11 @@ static pid_t *fwts_cpu_pids; * fwts_cpu_readmsr() * Read a given msr on a specificied CPU */ -int fwts_cpu_readmsr(const int cpu, const uint32_t reg, uint64_t *val) +int fwts_cpu_readmsr( + fwts_framework *fw, + const int cpu, + const uint32_t reg, + uint64_t *val) { char buffer[PATH_MAX]; uint64_t value = 0; @@ -62,12 +66,18 @@ int fwts_cpu_readmsr(const int cpu, const uint32_t reg, uint64_t *val) snprintf(buffer, sizeof(buffer), "/dev/cpu/%d/msr", cpu); if ((fd = open(buffer, O_RDONLY)) < 0) { - /* Hrm, msr not there, so force modprobe msr and see what happens */ - pid_t pid; - if (fwts_pipe_open_ro("modprobe msr", &pid, &fd) < 0) - return FWTS_ERROR; - fwts_pipe_close(fd, pid); + bool loaded = false; + /* + * msr not there, so force a load of the msr + * module and retry + */ + if (fwts_module_load(fw, "msr") != FWTS_OK) + return FWTS_ERROR; + if (fwts_module_loaded(fw, "msr", &loaded) != FWTS_OK) + return FWTS_ERROR; + if (!loaded) + return FWTS_ERROR; if ((fd = open(buffer, O_RDONLY)) < 0) return FWTS_ERROR; /* Really failed */ }