x86_energy_perf_policy fails with Input/output error in a VM

x86_energy_perf_policy fails with Input/output error in a VM

[Ondřej Lysoněk]

Hi,

I've encountered an issue with x86_energy_perf_policy. If I run it on a
machine that I'm told is a qemu-kvm virtual machine running inside a
privileged container, I get the following error:

x86_energy_perf_policy: /dev/cpu/0/msr offset 0x1ad read failed: Input/output error

I get the same error in a Digital Ocean droplet, so that might be a
similar environment.

I created the following patch which is intended to give a more
user-friendly message. It's based on a patch for turbostat from Prarit
Bhargava that was posted some time ago. The patch is "[v2] turbostat:
Running on virtual machine is not supported" [1].

Given my limited knowledge of the topic, I can't say with confidence
that this is the right solution, though (that's why this is not an
official patch submission). Also, I'm not sure what the convention with
exit codes is in this tool. Also, instead of the error message, perhaps
the tool should just not print anything in this case, which is how it
behaves in a "regular" VM?

[1] https://patchwork.kernel.org/patch/9868587/

Thanks.

Ondřej Lysoněk

diff --git a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
index 3fe1eed900d4..ff6c6661f075 100644
--- a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
+++ b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
@@ -622,6 +622,57 @@ void cmdline(int argc, char **argv)
 	}
 }
 
+/*
+ * Open a file, and exit on failure
+ */
+FILE *fopen_or_die(const char *path, const char *mode)
+{
+	FILE *filep = fopen(path, "r");
+
+	if (!filep)
+		err(1, "%s: open failed", path);
+	return filep;
+}
+
+void err_on_hypervisor(void)
+{
+	FILE *cpuinfo;
+	char *flags, *hypervisor;
+	char *buffer;
+
+	/* On VMs /proc/cpuinfo contains a "flags" entry for hypervisor */
+	cpuinfo = fopen_or_die("/proc/cpuinfo", "ro");
+
+	buffer = malloc(4096);
+	if (!buffer) {
+		fclose(cpuinfo);
+		err(-ENOMEM, "buffer malloc fail");
+	}
+
+	if (!fread(buffer, 1024, 1, cpuinfo)) {
+		fclose(cpuinfo);
+		free(buffer);
+		err(1, "Reading /proc/cpuinfo failed");
+	}
+
+	flags = strstr(buffer, "flags");
+	rewind(cpuinfo);
+	fseek(cpuinfo, flags - buffer, SEEK_SET);
+	if (!fgets(buffer, 4096, cpuinfo)) {
+		fclose(cpuinfo);
+		free(buffer);
+		err(1, "Reading /proc/cpuinfo failed");
+	}
+	fclose(cpuinfo);
+
+	hypervisor = strstr(buffer, "hypervisor");
+
+	free(buffer);
+
+	if (hypervisor)
+		err(-1,
+		    "not supported on this virtual machine");
+}
 
 int get_msr(int cpu, int offset, unsigned long long *msr)
 {
@@ -635,8 +686,10 @@ int get_msr(int cpu, int offset, unsigned long long *msr)
 		err(-1, "%s open failed, try chown or chmod +r /dev/cpu/*/msr, or run as root", pathname);
 
 	retval = pread(fd, msr, sizeof(*msr), offset);
-	if (retval != sizeof(*msr))
+	if (retval != sizeof(*msr)) {
+		err_on_hypervisor();
 		err(-1, "%s offset 0x%llx read failed", pathname, (unsigned long long)offset);
+	}
 
 	if (debug > 1)
 		fprintf(stderr, "get_msr(cpu%d, 0x%X, 0x%llX)\n", cpu, offset, *msr);
@@ -1086,18 +1139,6 @@ int update_cpu_msrs(int cpu)
 	return 0;
 }
 
-/*
- * Open a file, and exit on failure
- */
-FILE *fopen_or_die(const char *path, const char *mode)
-{
-	FILE *filep = fopen(path, "r");
-
-	if (!filep)
-		err(1, "%s: open failed", path);
-	return filep;
-}
-
 unsigned int get_pkg_num(int cpu)
 {
 	FILE *fp;
-- 

RE: x86_energy_perf_policy fails with Input/output error in a VM

[Brown, Len]

Thanks for the note,

I agree that is unfriendly how the tool tells the user that it is not possible for it to run in a VM guest.
If people are running into that, and we can make it more graceful, we should.

Is parsing /proc/cpuinfo a universal/reliable way to detect this situation?

RE: x86_energy_perf_policy fails with Input/output error in a VM

[Ondřej Lysoněk]

Hi,

"Brown, Len" <len.brown@intel.com> writes:

> Thanks for the note,
>
> I agree that is unfriendly how the tool tells the user that it is not possible for it to run in a VM guest.
> If people are running into that, and we can make it more graceful, we should.

My use case is being able to differentiate why x86_energy_perf_policy
failed in programs that use it, namely Tuned [1].

>
> Is parsing /proc/cpuinfo a universal/reliable way to detect this situation?

From what I've read it seems that it's possible to create a VM that does
not have 'hypervisor' in CPU flags. However I don't think this is a
problem for us - false negatives in the detection will just preserve the
current behaviour.

Regarding the reverse case, to get the 'hypervisor' CPU flag on bare
metal, you'd have to change the CPU microcode so that the CPUID
instruction returns different values, as far as I understand it.

Also, the kernel uses the 'hypervisor' flag (X86_FEATURE_HYPERVISOR)
internally in a number of places to detect a virtual machine. E.g.
arch/x86/events/core.c:270 or drivers/acpi/processor_idle.c:509 as of
commit 9420e8ade4353a6710.

However, perhaps it would be good to change the patch so that it prints
the original msr reading error in cases where /proc/cpuinfo is unavailable.

I've fixed up the patch a bit and changed it so that it searches only
for whole words '\<hypervisor\>' on lines beginning with
'flags\t\t'. This should make it more reliable.

Consider the patch
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>

[1] https://github.com/redhat-performance/tuned/blob/master/tuned/plugins/plugin_cpu.py#L96

Ondrej Lysonek


diff --git a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
index 3fe1eed900d4..29e0afbb7b4f 100644
--- a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
+++ b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
@@ -622,6 +622,77 @@ void cmdline(int argc, char **argv)
 	}
 }
 
+/*
+ * Open a file, and exit on failure
+ */
+FILE *fopen_or_die(const char *path, const char *mode)
+{
+	FILE *filep = fopen(path, "r");
+
+	if (!filep)
+		err(1, "%s: open failed", path);
+	return filep;
+}
+
+void err_on_hypervisor(void)
+{
+	FILE *cpuinfo;
+	char *buffer;
+	char *flags;
+	char *start_pos, *stop_pos;
+	const char *err_msg = NULL;
+	const char *hypervisor = " hypervisor";
+
+	/* On VMs, /proc/cpuinfo contains a "hypervisor" flags entry */
+	cpuinfo = fopen_or_die("/proc/cpuinfo", "ro");
+
+	buffer = malloc(4096);
+	if (!buffer) {
+		err_msg = "buffer malloc failed";
+		goto close_file;
+	}
+
+	if (!fread(buffer, 1024, 1, cpuinfo)) {
+		err_msg = "Reading /proc/cpuinfo failed";
+		goto free_mem;
+	}
+
+	flags = strstr(buffer, "flags\t\t");
+	if (!flags || (flags > buffer && *(flags - 1) != '\n'))
+		goto free_mem;
+
+	if (fseek(cpuinfo, flags - buffer, SEEK_SET)) {
+		err_msg = "fseek on /proc/cpuinfo failed";
+		goto free_mem;
+	}
+	if (!fgets(buffer, 4096, cpuinfo)) {
+		err_msg = "Reading /proc/cpuinfo failed";
+		goto free_mem;
+	}
+
+	start_pos = buffer;
+	while (1) {
+		start_pos = strstr(start_pos, hypervisor);
+		stop_pos = start_pos + strlen(hypervisor);
+		if (!start_pos || (*stop_pos == ' ' ||
+				   *stop_pos == '\n' ||
+				   *stop_pos == '\0'))
+			break;
+		start_pos = stop_pos;
+	}
+
+	if (start_pos) {
+		err_msg = "not supported on this virtual machine";
+	}
+
+free_mem:
+	free(buffer);
+close_file:
+	fclose(cpuinfo);
+
+	if (err_msg)
+		err(1, err_msg);
+}
 
 int get_msr(int cpu, int offset, unsigned long long *msr)
 {
@@ -635,8 +706,10 @@ int get_msr(int cpu, int offset, unsigned long long *msr)
 		err(-1, "%s open failed, try chown or chmod +r /dev/cpu/*/msr, or run as root", pathname);
 
 	retval = pread(fd, msr, sizeof(*msr), offset);
-	if (retval != sizeof(*msr))
+	if (retval != sizeof(*msr)) {
+		err_on_hypervisor();
 		err(-1, "%s offset 0x%llx read failed", pathname, (unsigned long long)offset);
+	}
 
 	if (debug > 1)
 		fprintf(stderr, "get_msr(cpu%d, 0x%X, 0x%llX)\n", cpu, offset, *msr);
@@ -1086,18 +1159,6 @@ int update_cpu_msrs(int cpu)
 	return 0;
 }
 
-/*
- * Open a file, and exit on failure
- */
-FILE *fopen_or_die(const char *path, const char *mode)
-{
-	FILE *filep = fopen(path, "r");
-
-	if (!filep)
-		err(1, "%s: open failed", path);
-	return filep;
-}
-
 unsigned int get_pkg_num(int cpu)
 {
 	FILE *fp;
-- 

Re: x86_energy_perf_policy fails with Input/output error in a VM

[Len Brown]

Applied.

thanks!
-Len

On Wed, Apr 1, 2020 at 12:51 PM Ondřej Lysoněk <olysonek@redhat.com> wrote:
>
> Hi,
>
> "Brown, Len" <len.brown@intel.com> writes:
>
> > Thanks for the note,
> >
> > I agree that is unfriendly how the tool tells the user that it is not possible for it to run in a VM guest.
> > If people are running into that, and we can make it more graceful, we should.
>
> My use case is being able to differentiate why x86_energy_perf_policy
> failed in programs that use it, namely Tuned [1].
>
> >
> > Is parsing /proc/cpuinfo a universal/reliable way to detect this situation?
>
> From what I've read it seems that it's possible to create a VM that does
> not have 'hypervisor' in CPU flags. However I don't think this is a
> problem for us - false negatives in the detection will just preserve the
> current behaviour.
>
> Regarding the reverse case, to get the 'hypervisor' CPU flag on bare
> metal, you'd have to change the CPU microcode so that the CPUID
> instruction returns different values, as far as I understand it.
>
> Also, the kernel uses the 'hypervisor' flag (X86_FEATURE_HYPERVISOR)
> internally in a number of places to detect a virtual machine. E.g.
> arch/x86/events/core.c:270 or drivers/acpi/processor_idle.c:509 as of
> commit 9420e8ade4353a6710.
>
> However, perhaps it would be good to change the patch so that it prints
> the original msr reading error in cases where /proc/cpuinfo is unavailable.
>
> I've fixed up the patch a bit and changed it so that it searches only
> for whole words '\<hypervisor\>' on lines beginning with
> 'flags\t\t'. This should make it more reliable.
>
> Consider the patch
> Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
>
> [1] https://github.com/redhat-performance/tuned/blob/master/tuned/plugins/plugin_cpu.py#L96
>
> Ondrej Lysonek
>
>
> diff --git a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
> index 3fe1eed900d4..29e0afbb7b4f 100644
> --- a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
> +++ b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
> @@ -622,6 +622,77 @@ void cmdline(int argc, char **argv)
>         }
>  }
>
> +/*
> + * Open a file, and exit on failure
> + */
> +FILE *fopen_or_die(const char *path, const char *mode)
> +{
> +       FILE *filep = fopen(path, "r");
> +
> +       if (!filep)
> +               err(1, "%s: open failed", path);
> +       return filep;
> +}
> +
> +void err_on_hypervisor(void)
> +{
> +       FILE *cpuinfo;
> +       char *buffer;
> +       char *flags;
> +       char *start_pos, *stop_pos;
> +       const char *err_msg = NULL;
> +       const char *hypervisor = " hypervisor";
> +
> +       /* On VMs, /proc/cpuinfo contains a "hypervisor" flags entry */
> +       cpuinfo = fopen_or_die("/proc/cpuinfo", "ro");
> +
> +       buffer = malloc(4096);
> +       if (!buffer) {
> +               err_msg = "buffer malloc failed";
> +               goto close_file;
> +       }
> +
> +       if (!fread(buffer, 1024, 1, cpuinfo)) {
> +               err_msg = "Reading /proc/cpuinfo failed";
> +               goto free_mem;
> +       }
> +
> +       flags = strstr(buffer, "flags\t\t");
> +       if (!flags || (flags > buffer && *(flags - 1) != '\n'))
> +               goto free_mem;
> +
> +       if (fseek(cpuinfo, flags - buffer, SEEK_SET)) {
> +               err_msg = "fseek on /proc/cpuinfo failed";
> +               goto free_mem;
> +       }
> +       if (!fgets(buffer, 4096, cpuinfo)) {
> +               err_msg = "Reading /proc/cpuinfo failed";
> +               goto free_mem;
> +       }
> +
> +       start_pos = buffer;
> +       while (1) {
> +               start_pos = strstr(start_pos, hypervisor);
> +               stop_pos = start_pos + strlen(hypervisor);
> +               if (!start_pos || (*stop_pos == ' ' ||
> +                                  *stop_pos == '\n' ||
> +                                  *stop_pos == '\0'))
> +                       break;
> +               start_pos = stop_pos;
> +       }
> +
> +       if (start_pos) {
> +               err_msg = "not supported on this virtual machine";
> +       }
> +
> +free_mem:
> +       free(buffer);
> +close_file:
> +       fclose(cpuinfo);
> +
> +       if (err_msg)
> +               err(1, err_msg);
> +}
>
>  int get_msr(int cpu, int offset, unsigned long long *msr)
>  {
> @@ -635,8 +706,10 @@ int get_msr(int cpu, int offset, unsigned long long *msr)
>                 err(-1, "%s open failed, try chown or chmod +r /dev/cpu/*/msr, or run as root", pathname);
>
>         retval = pread(fd, msr, sizeof(*msr), offset);
> -       if (retval != sizeof(*msr))
> +       if (retval != sizeof(*msr)) {
> +               err_on_hypervisor();
>                 err(-1, "%s offset 0x%llx read failed", pathname, (unsigned long long)offset);
> +       }
>
>         if (debug > 1)
>                 fprintf(stderr, "get_msr(cpu%d, 0x%X, 0x%llX)\n", cpu, offset, *msr);
> @@ -1086,18 +1159,6 @@ int update_cpu_msrs(int cpu)
>         return 0;
>  }
>
> -/*
> - * Open a file, and exit on failure
> - */
> -FILE *fopen_or_die(const char *path, const char *mode)
> -{
> -       FILE *filep = fopen(path, "r");
> -
> -       if (!filep)
> -               err(1, "%s: open failed", path);
> -       return filep;
> -}
> -
>  unsigned int get_pkg_num(int cpu)
>  {
>         FILE *fp;
> --
>


-- 
Len Brown, Intel Open Source Technology Center