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

[Erik Skultety]

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 at linux.ibm.com>
> Tested-by: Viktor Mihajlovski <mihajlov at linux.ibm.com>
> Reviewed-by: Paulo de Rezende Pinatti <ppinatti at linux.ibm.com>
> Reviewed-by: Bjoern Walk <bwalk at linux.ibm.com>
> ---
>  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
> 

-- 
Erik Skultety