[libvirt] [PATCH] virt-host-validate: warn if kvm_hv is not loaded for POWER hosts

Daniel Henrique Barboza posted 1 patch 4 years, 4 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20191212211150.2750564-1-danielhb413@gmail.com
tools/virt-host-validate-common.c | 136 ++++++++++++++++++++++++++++++
tools/virt-host-validate-common.h |   2 +
tools/virt-host-validate-qemu.c   |   6 ++
3 files changed, 144 insertions(+)
[libvirt] [PATCH] virt-host-validate: warn if kvm_hv is not loaded for POWER hosts
Posted by Daniel Henrique Barboza 4 years, 4 months ago
POWER hosts does not implement CPU virtualization extensions like
x86 or s390x. Instead, all bare-metal POWER hosts are considered
to be virtualization ready.

For POWER, the validation is done by checking the virtualization
kernel modules, kvm_hv and kvm_pr, to see if they are either not
installed or not loaded in the host. If the KVM modules aren't
present, we should not just warn but fail to validate.

This patch implements this support. If kvm_hv is not installed,
which can be determined by 'modinfo' returning not-zero return
code, fail the verification. If kvm_hv is installed but not
loaded, show a warning. The exception are POWER8 hosts, which can
work with kvm_pr. In its case, ACK the use of kvm_pr if kvm_hv
is not loaded/present.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 tools/virt-host-validate-common.c | 136 ++++++++++++++++++++++++++++++
 tools/virt-host-validate-common.h |   2 +
 tools/virt-host-validate-qemu.c   |   6 ++
 3 files changed, 144 insertions(+)

diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c
index bce0f14917..e6d7986758 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -411,3 +411,139 @@ int virHostValidateIOMMU(const char *hvname,
     virHostMsgPass();
     return 0;
 }
+
+
+static bool virHostCPUIsPower8(void)
+{
+   FILE *fp;
+   bool ret = false;
+
+    if (!(fp = fopen("/proc/cpuinfo", "r")))
+        return false;
+
+    do {
+        char line[1024];
+
+        if (!fgets(line, sizeof(line), fp))
+            break;
+
+        /* Looks for the 'model name' line. This is more common for
+         * Intel /proc/cpuinfo formats, but let's account for it
+         * too. */
+        if (STRPREFIX(line, "model name")) {
+            if (strstr(line, "POWER8"))
+                ret = true;
+            break;
+        }
+
+        /* Looks for the 'cpu:' line which is more commonly present
+         * in /proc/cpuinfo Power systems. To ensure this is not
+         * 'cpu id' or any other cpu attribute, peek at the next char
+         * after the first whitespace. A tab, whitespace or ':'
+         * indicates we're on the right line */
+        if (STRPREFIX(line, "cpu") &&
+            (line[3] == '\t' || line[3] == ':' || line[3] == ' ')) {
+             if (strstr(line, "POWER8"))
+                ret = true;
+            break;
+        }
+
+    } while (1);
+
+    VIR_FORCE_FCLOSE(fp);
+
+    return ret;
+}
+
+
+static bool virHostKernelModuleExists(const char *module)
+{
+    g_autofree char *cmd = g_strdup_printf("modinfo %s", module);
+    g_autofree char *stdout = NULL;
+    g_autofree char *stderr = NULL;
+    g_autoptr(GError) err = NULL;
+    int errStatus;
+
+    if (g_spawn_command_line_sync(cmd, &stdout, &stderr, &errStatus, &err))
+        return true;
+
+    return false;
+}
+
+
+static bool virHostKernelModuleIsLoaded(const char *module)
+{
+    FILE *fp;
+    bool ret = false;
+
+    if (!(fp = fopen("/proc/modules", "r")))
+        return false;
+
+    do {
+        char line[1024];
+
+        if (!fgets(line, sizeof(line), fp))
+            break;
+
+        if (STRPREFIX(line, module)) {
+            ret = true;
+            break;
+        }
+
+    } while (1);
+
+    VIR_FORCE_FCLOSE(fp);
+
+    return ret;
+}
+
+
+int virHostValidatePowerPCModules(void)
+{
+    bool kvm_pr_exists = virHostKernelModuleExists("kvm_pr");
+    bool kvm_pr_loaded = kvm_pr_exists && virHostKernelModuleIsLoaded("kvm_pr");
+    bool kvm_hv_exists = virHostKernelModuleExists("kvm_hv");
+    bool kvm_hv_loaded = kvm_hv_exists && virHostKernelModuleIsLoaded("kvm_hv");
+    bool hostIsP8 = virHostCPUIsPower8();
+
+    virHostMsgCheck("QEMU", "%s", _("for PowerPC KVM modules loaded"));
+
+    /* No Power KVM virtualization modules present on the host. */
+    if (!kvm_hv_exists && !kvm_pr_exists) {
+        virHostMsgFail(VIR_HOST_VALIDATE_FAIL,
+                           _("No kvm_hv or kvm_pr module present in "
+                             "the host"));
+        return -1;
+    }
+
+    /* Bail out for all non-Power8 CPUs if kvm_hv is not present. */
+    if (!kvm_hv_exists && !hostIsP8) {
+        virHostMsgFail(VIR_HOST_VALIDATE_FAIL,
+                       _("No kvm_hv module present in the host"));
+        return -1;
+    }
+
+    /* Power8 CPUs virtualization works with any of kvm_hv and kvm_pr.
+     * Issue a warning if none are loaded. */
+    if (hostIsP8) {
+        if (!kvm_hv_loaded && !kvm_pr_loaded) {
+            virHostMsgFail(VIR_HOST_VALIDATE_WARN,
+                           _("Load kvm_hv or kvm_pr module "
+                             "for better performance"));
+            return 0;
+        }
+
+        virHostMsgPass();
+        return 0;
+    }
+
+    /* For non-Power8 hosts, show a warning if kvm_hv is not loaded. */
+    if (!kvm_hv_loaded) {
+        virHostMsgFail(VIR_HOST_VALIDATE_WARN,
+                      _("Load kvm_hv for better performance"));
+        return 0;
+    }
+
+    virHostMsgPass();
+    return 0;
+}
diff --git a/tools/virt-host-validate-common.h b/tools/virt-host-validate-common.h
index 1b7e93e520..7a2933c8fd 100644
--- a/tools/virt-host-validate-common.h
+++ b/tools/virt-host-validate-common.h
@@ -83,3 +83,5 @@ int virHostValidateCGroupControllers(const char *hvname,
 
 int virHostValidateIOMMU(const char *hvname,
                          virHostValidateLevel level);
+
+int virHostValidatePowerPCModules(void);
\ No newline at end of file
diff --git a/tools/virt-host-validate-qemu.c b/tools/virt-host-validate-qemu.c
index ff3c1f0231..8753c6a31d 100644
--- a/tools/virt-host-validate-qemu.c
+++ b/tools/virt-host-validate-qemu.c
@@ -57,6 +57,12 @@ int virHostValidateQEMU(void)
         if (virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_SIE))
             hasHwVirt = true;
         break;
+    case VIR_ARCH_PPC64:
+    case VIR_ARCH_PPC64LE:
+        hasVirtFlag = true;
+        if (virHostValidatePowerPCModules() == 0)
+            hasHwVirt = true;
+        break;
     default:
         hasHwVirt = false;
     }
-- 
2.23.0


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virt-host-validate: warn if kvm_hv is not loaded for POWER hosts
Posted by Cole Robinson 4 years, 4 months ago
On 12/12/19 4:11 PM, Daniel Henrique Barboza wrote:
> POWER hosts does not implement CPU virtualization extensions like
> x86 or s390x. Instead, all bare-metal POWER hosts are considered
> to be virtualization ready.
> 
> For POWER, the validation is done by checking the virtualization
> kernel modules, kvm_hv and kvm_pr, to see if they are either not
> installed or not loaded in the host. If the KVM modules aren't
> present, we should not just warn but fail to validate.
> 
> This patch implements this support. If kvm_hv is not installed,
> which can be determined by 'modinfo' returning not-zero return
> code, fail the verification. If kvm_hv is installed but not
> loaded, show a warning. The exception are POWER8 hosts, which can
> work with kvm_pr. In its case, ACK the use of kvm_pr if kvm_hv
> is not loaded/present.

For x86, we check for /dev/kvm being available and usable. This side
steps whether kvm is a module or not, in theory it could be compiled
into the kernel. Is there anything in /dev we can check for power8?

I don't follow the reasoning for for why the module is installed vs
loaded matters for FAIL vs WARN. Can you expand on that a bit more?

Rather than parsing /proc/modinfo, can we check for /sys/module/$modname
instead, or something under that directory?

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virt-host-validate: warn if kvm_hv is not loaded for POWER hosts
Posted by Daniel Henrique Barboza 4 years, 1 month ago
Hey Cole. I took my sweet time but I got some answers here:

On 12/17/19 2:58 PM, Cole Robinson wrote:
> On 12/12/19 4:11 PM, Daniel Henrique Barboza wrote:
>> POWER hosts does not implement CPU virtualization extensions like
>> x86 or s390x. Instead, all bare-metal POWER hosts are considered
>> to be virtualization ready.
>>
>> For POWER, the validation is done by checking the virtualization
>> kernel modules, kvm_hv and kvm_pr, to see if they are either not
>> installed or not loaded in the host. If the KVM modules aren't
>> present, we should not just warn but fail to validate.
>>
>> This patch implements this support. If kvm_hv is not installed,
>> which can be determined by 'modinfo' returning not-zero return
>> code, fail the verification. If kvm_hv is installed but not
>> loaded, show a warning. The exception are POWER8 hosts, which can
>> work with kvm_pr. In its case, ACK the use of kvm_pr if kvm_hv
>> is not loaded/present.
> 
> For x86, we check for /dev/kvm being available and usable. This side
> steps whether kvm is a module or not, in theory it could be compiled
> into the kernel. Is there anything in /dev we can check for power8?

The device /dev/kvm exists in Power even without the KVM module loaded.
This is why we must check for kvm being loaded instead of relying in the
/dev/kvm device being present.

About KVM being compiled into the kernel, this is not possible for ppc64
at all - it must be a module. And in a quick check here it seems to be
case for KVM_INTEL as well - you can choose either 'm' or 'N' in the
config. You can't built it in the kernel.


> 
> I don't follow the reasoning for for why the module is installed vs
> loaded matters for FAIL vs WARN. Can you expand on that a bit more?
> 
> Rather than parsing /proc/modinfo, can we check for /sys/module/$modname
> instead, or something under that directory?


I'll drop the verification I was doing with modinfo to check for the
module existence. Instead I'll just check for the 'kvm_hv' module being
loaded and fire a WARN in case it isn't, regardless of being a Power 8
that can work with kvm_pr. This can be done by checking /proc/modules.

kvm_pr is too slow for most usages and, now that PowerPC has nested
support with kvm_hv, kvm_pr is now even more niche. The average user will want
to use kvm_hv instead.




Thanks,


DHB


> 
> - Cole
> 

Re: [libvirt] [PATCH] virt-host-validate: warn if kvm_hv is not loaded for POWER hosts
Posted by Daniel Henrique Barboza 4 years, 4 months ago

On 12/17/19 2:58 PM, Cole Robinson wrote:
> On 12/12/19 4:11 PM, Daniel Henrique Barboza wrote:
>> POWER hosts does not implement CPU virtualization extensions like
>> x86 or s390x. Instead, all bare-metal POWER hosts are considered
>> to be virtualization ready.
>>
>> For POWER, the validation is done by checking the virtualization
>> kernel modules, kvm_hv and kvm_pr, to see if they are either not
>> installed or not loaded in the host. If the KVM modules aren't
>> present, we should not just warn but fail to validate.
>>
>> This patch implements this support. If kvm_hv is not installed,
>> which can be determined by 'modinfo' returning not-zero return
>> code, fail the verification. If kvm_hv is installed but not
>> loaded, show a warning. The exception are POWER8 hosts, which can
>> work with kvm_pr. In its case, ACK the use of kvm_pr if kvm_hv
>> is not loaded/present.
> 
> For x86, we check for /dev/kvm being available and usable. This side
> steps whether kvm is a module or not, in theory it could be compiled
> into the kernel. Is there anything in /dev we can check for power8?

I don't know. I'll investigate.

> 
> I don't follow the reasoning for for why the module is installed vs
> loaded matters for FAIL vs WARN. Can you expand on that a bit more?

The reasoning is that the module not present (i.e. not compiled
in the kernel) is not easy to solve, while the module not loaded is
a matter of loading either kvm_pr or kvm_hv.

The nonexistence of both kvm_pr and kvm_hv is something I haven't seen
in the field and I'm somewhat theorizing about it. I wouldn't oppose to
simply check for the modules being loaded and use WARN.


> 
> Rather than parsing /proc/modinfo, can we check for /sys/module/$modname
> instead, or something under that directory?


Just checked. If the existence of say /sys/module/kvm_hv is an indication
that the module kvm_hv is loaded, then I see no issues with checking for
this instead of parsing modinfo.




Thanks,


DHB

> 
> - Cole
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list