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

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(a)linux.ibm.com&gt; > Tested-by: Viktor Mihajlovski <mihajlov(a)linux.ibm.com&gt; > Reviewed-by: Paulo de Rezende Pinatti <ppinatti(a)linux.ibm.com&gt; > Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com&gt; > --- > 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.

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

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