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

["Ondřej Lysoněk" <olysonek@redhat.com>]

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;
--