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

[Boris Fiuczynski]

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

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