Re: [PATCH 4/6] tools: secure guest check on s390 in virt-host-validate

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/18/20 2:59 PM, Erik Skultety wrote:
On Mon, May 11, 2020 at 06:41:59PM +0200, Boris Fiuczynski wrote:
Add checking in virt-host-validate for secure guest support
on s390 for IBM Secure Execution.

Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
Tested-by: Viktor Mihajlovski <mihajlov@xxxxxxxxxxxxx>
Reviewed-by: Paulo de Rezende Pinatti <ppinatti@xxxxxxxxxxxxx>
Reviewed-by: Bjoern Walk <bwalk@xxxxxxxxxxxxx>
---
  tools/virt-host-validate-common.c | 58 +++++++++++++++++++++++++++++--
  tools/virt-host-validate-common.h |  4 +++
  tools/virt-host-validate-qemu.c   |  4 +++
  3 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c
index fbefbada96..dd73bd0dea 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -40,7 +40,8 @@ VIR_ENUM_IMPL(virHostValidateCPUFlag,
                VIR_HOST_VALIDATE_CPU_FLAG_LAST,
                "vmx",
                "svm",
-              "sie");
+              "sie",
+              "158");
static bool quiet; @@ -210,7 +211,8 @@ virBitmapPtr virHostValidateGetCPUFlags(void)
           * on the architecture, so check possible prefixes */
          if (!STRPREFIX(line, "flags") &&
              !STRPREFIX(line, "Features") &&
-            !STRPREFIX(line, "features"))
+            !STRPREFIX(line, "features") &&
+            !STRPREFIX(line, "facilities"))

I can't comment on the first 2 hunks as I don't have an appropriate s390 at
hand, so I can only trust you it's the correct way.

That is fine. We have given this some testing on s390.


              continue;
/* fgets() includes the trailing newline in the output buffer,
@@ -439,3 +441,55 @@ bool virHostKernelModuleIsLoaded(const char *module)
return ret;
  }
+
+
+int virHostValidateSecureGuests(const char *hvname,
+                                virHostValidateLevel level)
+{
+    virBitmapPtr flags;
+    bool hasFac158 = false;
+    virArch arch = virArchFromHost();
+    g_autofree char *cmdline = NULL;
+    static const char *kIBMValues[] = {"y", "Y", "1"};
+
+    flags = virHostValidateGetCPUFlags();
+
+    if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_FACILITY_158))
+        hasFac158 = true;
+
+    virBitmapFree(flags);
+
+    virHostMsgCheck(hvname, "%s", _("for secure guest support"));
+    if (ARCH_IS_S390(arch)) {

We don't need ^this check here, facility 158 won't be available on x86.

Since it is very true that the facility 158 won't be available on x86 you do not want to get the message "Hardware or firmware does not provide support for IBM Secure Execution" on x86. On the other hand you can be on s390 and not have the facility available and want to know see the message.


+        if (hasFac158) {

btw ^such checks should be adopted in QEMU caps code as well like, but we don't
have an appropriate helper at the moment.

+            if (!virFileIsDir("/sys/firmware/uv")) {
+                virHostMsgFail(level, "IBM Secure Execution not supported by "
+                                      "the currently used kernel");
+                return 0;
+            }
+            if (virFileReadValueString(&cmdline, "/proc/cmdline") < 0)
+                return -1;
+            if (virKernelCmdlineMatchParam(cmdline, "prot_virt", kIBMValues,
+                                           G_N_ELEMENTS(kIBMValues),
+                                           VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY |
+                                           VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX)) {
+                virHostMsgPass();
+                return 1;
+            } else {
+                virHostMsgFail(level,
+                               "IBM Secure Execution appears to be disabled "
+                               "in kernel. Add prot_virt=1 to kernel cmdline "
+                               "arguments");
+            }
+        } else {
+            virHostMsgFail(level, "Hardware or firmware does not provide "
+                                  "support for IBM Secure Execution");
+        }
+    } else {
+        virHostMsgFail(level,
+                       "Unknown if this platform has Secure Guest support");
+        return -1;
+    }
+
+    return 0;
+}
diff --git a/tools/virt-host-validate-common.h b/tools/virt-host-validate-common.h
index 8ae60a21de..44b5544a12 100644
--- a/tools/virt-host-validate-common.h
+++ b/tools/virt-host-validate-common.h
@@ -37,6 +37,7 @@ typedef enum {
      VIR_HOST_VALIDATE_CPU_FLAG_VMX = 0,
      VIR_HOST_VALIDATE_CPU_FLAG_SVM,
      VIR_HOST_VALIDATE_CPU_FLAG_SIE,
+    VIR_HOST_VALIDATE_CPU_FLAG_FACILITY_158,
VIR_HOST_VALIDATE_CPU_FLAG_LAST,
  } virHostValidateCPUFlag;
@@ -83,4 +84,7 @@ int virHostValidateCGroupControllers(const char *hvname,
  int virHostValidateIOMMU(const char *hvname,
                           virHostValidateLevel level);
+int virHostValidateSecureGuests(const char *hvname,
+                                virHostValidateLevel level);
+
  bool virHostKernelModuleIsLoaded(const char *module);
diff --git a/tools/virt-host-validate-qemu.c b/tools/virt-host-validate-qemu.c
index bd717a604e..ea7f172790 100644
--- a/tools/virt-host-validate-qemu.c
+++ b/tools/virt-host-validate-qemu.c
@@ -127,5 +127,9 @@ int virHostValidateQEMU(void)
                               VIR_HOST_VALIDATE_WARN) < 0)
          ret = -1;
+ if (virHostValidateSecureGuests("QEMU",
+                                    VIR_HOST_VALIDATE_WARN) < 0)
+        ret = -1;
+
      return ret;
  }
--
2.25.1




--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux