ACK: [PATCH 3/3] lib/fwts_cpu: use new use the new module loading helper functions
ivanhu
ivan.hu at canonical.com
Tue Nov 13 07:52:46 UTC 2018
On 11/8/18 10:45 PM, Colin King wrote:
> From: Colin Ian King <colin.king at 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 at 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 at canonical.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/fwts-devel/attachments/20181113/09df069d/attachment-0001.sig>
More information about the fwts-devel
mailing list