Introduce two utility functions to parse a kernel command line string according to the kernel code parsing rules in order to enable the caller to perform operations such as verifying whether certain argument=value combinations are present or retrieving an argument's value. Signed-off-by: Paulo de Rezende Pinatti <ppinatti(a)linux.ibm.com&gt; Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; --- src/libvirt_private.syms | 2 + src/util/virutil.c | 169 +++++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 17 ++++ tests/utiltest.c | 141 ++++++++++++++++++++++++++++++++ 4 files changed, 329 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a6af44fe1c..a206a943c5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3433,6 +3433,8 @@ virHostGetDRMRenderNode; virHostHasIOMMU; virIndexToDiskName; virIsDevMapperDevice; +virKernelCmdlineMatchParam; +virKernelCmdlineNextParam; virMemoryLimitIsSet; virMemoryLimitTruncate; virMemoryMaxValue; diff --git a/src/util/virutil.c b/src/util/virutil.c index fb46501142..749c9d7116 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1725,6 +1725,175 @@ virHostGetDRMRenderNode(void) return ret; } + +static const char *virKernelCmdlineSkipDbQuote(const char *cmdline, + bool *is_quoted) +{ + if (cmdline[0] == '"') { + *is_quoted = !(*is_quoted); + cmdline++; + } + return cmdline; +} + + +static size_t virKernelCmdlineSearchForward(const char *cmdline, + bool *is_quoted, + bool include_equal) +{ + size_t index; + + for (index = 0; cmdline[index]; index++) { + if ((!(*is_quoted) && g_ascii_isspace(cmdline[index])) || + (include_equal && cmdline[index] == '=')) + break; + virKernelCmdlineSkipDbQuote(cmdline + index, is_quoted); + } + return index; +} + + +static size_t virKernelCmdlineNextSpace(const char *cmdline, + bool *is_quoted) +{ + return virKernelCmdlineSearchForward(cmdline, is_quoted, false); +} + + +static size_t virKernelCmdlineNextSpaceOrEqual(const char *cmdline, + bool *is_quoted) +{ + return virKernelCmdlineSearchForward(cmdline, is_quoted, true); +} + + +static char* virKernelArgNormalize(const char *arg) +{ + return virStringReplace(arg, "_", "-"); +} + + +static char* virKernelCmdlineArgNormalize(const char *cmdline, size_t offset) +{ + g_autofree char *param = g_strndup((cmdline), offset); + + return virKernelArgNormalize(param); +} + + +/* + * Parse the kernel cmdline and store the next parameter in @param + * and the value of @param in @val which can be NULL if @param has + * no value. In addition returns the address right after @param=@value + * for possible further processing. + * + * @cmdline: kernel command line string to be checked for next parameter + * @param: pointer to hold retrieved parameter, will be NULL if none found + * @val: pointer to hold retrieved value of @param + * + * Returns a pointer to address right after @param=@val in the + * kernel command line, will point to the string's end (NULL) + * in case no next parameter is found + */ +const char *virKernelCmdlineNextParam(const char *cmdline, + char **param, + char **val) +{ + size_t offset; + bool is_quoted = false; + *param = NULL; + *val = NULL; + + virSkipSpaces(&cmdline); + cmdline = virKernelCmdlineSkipDbQuote(cmdline, &is_quoted); + offset = virKernelCmdlineNextSpaceOrEqual(cmdline, &is_quoted); + if (offset == 0) + return cmdline; + + *param = virKernelCmdlineArgNormalize(cmdline, offset); + cmdline = cmdline + offset; + /* param has no value */ + if (*cmdline != '=') + return cmdline; + + cmdline = virKernelCmdlineSkipDbQuote(++cmdline, &is_quoted); + offset = virKernelCmdlineNextSpace(cmdline, &is_quoted); + if (cmdline[offset-1] == '"') + *val = g_strndup(cmdline, offset-1); + else + *val = g_strndup(cmdline, offset); + + return cmdline + offset; +} + + +#define VIR_CMDLINE_STR_CMP(kernel_val, caller_val, flags) \ + (((flags & VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ) && \ + STREQ(kernel_val, caller_val)) || ((flags & VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX) && \ + STRPREFIX(kernel_val, caller_val))) + + +/* + * Try to match the provided kernel cmdline string with the provided @arg + * and the list @values of possible values according to the matching strategy + * defined in @flags. Possible options include: + * - VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX: do a substring comparison of values + * (uses size of value provided as input) + * - VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ: do a strict string comparison of values + * - VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY: first positive match satifies search + * (in case of multiple argument occurrences) + * - VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST: use the result of last argument occurence + * (in case of multiple argument occurrences) + * + * @cmdline: kernel command line string to be checked for @arg + * @arg: kernel command line argument + * @values: array of possible values to match @arg + * @len_values: size of array, it can be 0 meaning a match will be positive if the + * argument has no value. + * @flags: flag mask defining the strategy for matching and comparing + * + * Returns true if a match is found, false otherwise + */ +bool virKernelCmdlineMatchParam(const char *cmdline, + const char *arg, + const char **values, + size_t len_values, + virKernelCmdlineFlags flags) +{ + bool match = false; + size_t i; + const char *next = cmdline; + g_autofree char *norm_arg = virKernelArgNormalize(arg); + g_autofree char *kparam = NULL; + g_autofree char *kval = NULL; + + while (next[0] != '\0') { + VIR_FREE(kparam); + VIR_FREE(kval); + next = virKernelCmdlineNextParam(next, &kparam, &kval); + if (!kparam) + break; + if (STRNEQ(kparam, norm_arg)) + continue; + if (!kval) { + match = (len_values == 0) ? true : false; + } else { + match = false; + for (i = 0; i < len_values; i++) { + if (VIR_CMDLINE_STR_CMP(kval, values[i], flags)) { + match = true; + break; + } + } + } + if (match && (flags & VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY)) + break; + } + + return match; +} + + /* * Get a password from the console input stream. * The caller must free the returned password. diff --git a/src/util/virutil.h b/src/util/virutil.h index 49b4bf440f..7499b78153 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -147,6 +147,23 @@ bool virHostHasIOMMU(void); char *virHostGetDRMRenderNode(void) G_GNUC_NO_INLINE; +typedef enum { + VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX = 1, + VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ = 2, + VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY = 4, + VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST = 8, +} virKernelCmdlineFlags; + +const char *virKernelCmdlineNextParam(const char *cmdline, + char **param, + char **val); + +bool virKernelCmdlineMatchParam(const char *cmdline, + const char *arg, + const char **values, + size_t len_values, + virKernelCmdlineFlags flags); + /** * VIR_ASSIGN_IS_OVERFLOW: * @rvalue: value that is checked (evaluated twice) diff --git a/tests/utiltest.c b/tests/utiltest.c index 5ae04585cb..01fb8c89f5 100644 --- a/tests/utiltest.c +++ b/tests/utiltest.c @@ -254,6 +254,145 @@ testOverflowCheckMacro(const void *data G_GNUC_UNUSED) } +struct testKernelCmdlineNextParamData +{ + const char *cmdline; + const char *param; + const char *val; + const char *next; +}; + +static struct testKernelCmdlineNextParamData kEntries[] = { + { "arg1 arg2 arg3=val1", "arg1", NULL, " arg2 arg3=val1" }, + { "arg1=val1 arg2 arg3=val3 arg4", "arg1", "val1", " arg2 arg3=val3 arg4" }, + { "arg3=val3 ", "arg3", "val3", " " }, + { "arg3=val3", "arg3", "val3", "" }, + { "arg-3=val3 arg4", "arg-3", "val3", " arg4" }, + { "arg_3=val3 arg4", "arg-3", "val3", " arg4" }, + { " arg_3=val3 arg4", "arg-3", "val3", " arg4" }, + { " arg-3=val3 arg4", "arg-3", "val3", " arg4" }, + { "arg2=\"value with spaces\" arg3=val3", "arg2", "value with spaces", " arg3=val3" }, + { " arg2=\"value with spaces\" arg3=val3", "arg2", "value with spaces", " arg3=val3" }, + { " \"arg2=value with spaces\" arg3=val3", "arg2", "value with spaces", " arg3=val3" }, + { "arg2=\"val\"ue arg3", "arg2", "val\"ue", " arg3" }, + { " arg3\" escaped=val2\"", "arg3\" escaped", "val2", "" }, + { " arg2longer=someval arg2=val2 arg3 ", "arg2longer", "someval", " arg2=val2 arg3 " }, + { "=val1 arg2=val2", NULL, NULL, "=val1 arg2=val2" }, + { " ", NULL, NULL, "" }, + { "", NULL, NULL, "" }, +}; + +static int +testKernelCmdlineNextParam(const void *data G_GNUC_UNUSED) +{ + char *param = NULL; + char *val = NULL; + const char *next; + size_t i; + + for (i = 0; i < G_N_ELEMENTS(kEntries); ++i) { + VIR_FREE(param); + VIR_FREE(val); + + next = virKernelCmdlineNextParam(kEntries[i].cmdline, &param, &val); + + if (STRNEQ_NULLABLE(param, kEntries[i].param) || + STRNEQ_NULLABLE(val, kEntries[i].val) || + STRNEQ(next, kEntries[i].next)) { + VIR_TEST_DEBUG("\nKernel cmdline [%s]", kEntries[i].cmdline); + VIR_TEST_DEBUG("Expect param [%s]", kEntries[i].param); + VIR_TEST_DEBUG("Actual param [%s]", param); + VIR_TEST_DEBUG("Expect value [%s]", kEntries[i].val); + VIR_TEST_DEBUG("Actual value [%s]", val); + VIR_TEST_DEBUG("Expect next [%s]", kEntries[i].next); + VIR_TEST_DEBUG("Actual next [%s]", next); + + VIR_FREE(param); + VIR_FREE(val); + + return -1; + } + } + + VIR_FREE(param); + VIR_FREE(val); + + return 0; +} + + +struct testKernelCmdlineMatchData +{ + const char *cmdline; + const char *arg; + const char *values[2]; + virKernelCmdlineFlags flags; + bool result; +}; + +static struct testKernelCmdlineMatchData kMatchEntries[] = { + {"arg1 myarg=no arg2=val2 myarg=yes arg4=val4 myarg=no arg5", "myarg", {"1", "y"}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY | VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ, false }, + {"arg1 myarg=no arg2=val2 myarg=yes arg4=val4 myarg=no arg5", "myarg", {"on", "yes"}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY | VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ, true }, + {"arg1 myarg=no arg2=val2 myarg=yes arg4=val4 myarg=no arg5", "myarg", {"1", "y"}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY | VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX, true }, + {"arg1 myarg=no arg2=val2 myarg=yes arg4=val4 myarg=no arg5", "myarg", {"a", "b"}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY | VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX, false }, + {"arg1 myarg=no arg2=val2 myarg=yes arg4=val4 myarg=no arg5", "myarg", {"on", "yes"}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST | VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ, false}, + {"arg1 myarg=no arg2=val2 myarg=yes arg4=val4 myarg=no arg5", "myarg", {"1", "y"}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST | VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX, false}, + {"arg1 myarg=no arg2=val2 arg4=val4 myarg=yes arg5", "myarg", {"on", "yes"}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST | VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ, true }, + {"arg1 myarg=no arg2=val2 arg4=val4 myarg=yes arg5", "myarg", {"1", "y"}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST | VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX, true }, + {"arg1 myarg=no arg2=val2 arg4=val4 myarg arg5", "myarg", {NULL, NULL}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST, true }, + {"arg1 myarg arg2=val2 arg4=val4 myarg=yes arg5", "myarg", {NULL, NULL}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY, true }, + {"arg1 myarg arg2=val2 arg4=val4 myarg=yes arg5", "myarg", {NULL, NULL}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST, false }, + {"arg1 my-arg=no arg2=val2 arg4=val4 my_arg=yes arg5", "my-arg", {"on", "yes"}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST | VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ, true }, + {"arg1 my-arg=no arg2=val2 arg4=val4 my_arg=yes arg5 ", "my-arg", {"on", "yes"}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST | VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ, true }, + {"arg1 my-arg arg2=val2 arg4=val4 my_arg=yes arg5", "my_arg", {NULL, NULL}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY, true }, + {"arg1 my-arg arg2=val2 arg4=val4 my-arg=yes arg5", "my_arg", {NULL, NULL}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY, true }, + {"=val1 my-arg arg2=val2 arg4=val4 my-arg=yes arg5", "myarg", {NULL, NULL}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY, false }, + {"arg1 arg2=", "arg2", {"", ""}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST | VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ, true }, + {" ", "myarg", {NULL, NULL}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST, false }, + {"", "", {NULL, NULL}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST, false }, +}; + + +static int +testKernelCmdlineMatchParam(const void *data G_GNUC_UNUSED) +{ + bool result; + size_t i, lenValues; + + for (i = 0; i < G_N_ELEMENTS(kMatchEntries); ++i) { + if (kMatchEntries[i].values[0] == NULL) + lenValues = 0; + else + lenValues = G_N_ELEMENTS(kMatchEntries[i].values); + + result = virKernelCmdlineMatchParam(kMatchEntries[i].cmdline, + kMatchEntries[i].arg, + kMatchEntries[i].values, + lenValues, + kMatchEntries[i].flags); + + if (result != kMatchEntries[i].result) { + VIR_TEST_DEBUG("\nKernel cmdline [%s]", kMatchEntries[i].cmdline); + VIR_TEST_DEBUG("Kernel argument [%s]", kMatchEntries[i].arg); + VIR_TEST_DEBUG("Kernel values [%s] [%s]", kMatchEntries[i].values[0], + kMatchEntries[i].values[1]); + if (kMatchEntries[i].flags & VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX) + VIR_TEST_DEBUG("Flag [VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX]"); + if (kMatchEntries[i].flags & VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ) + VIR_TEST_DEBUG("Flag [VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ]"); + if (kMatchEntries[i].flags & VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY) + VIR_TEST_DEBUG("Flag [VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY]"); + if (kMatchEntries[i].flags & VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST) + VIR_TEST_DEBUG("Flag [VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST]"); + VIR_TEST_DEBUG("Expect result [%d]", kMatchEntries[i].result); + VIR_TEST_DEBUG("Actual result [%d]", result); + + return -1; + } + } + + return 0; +} static int @@ -277,6 +416,8 @@ mymain(void) DO_TEST(ParseVersionString); DO_TEST(RoundValueToPowerOfTwo); DO_TEST(OverflowCheckMacro); + DO_TEST(KernelCmdlineNextParam); + DO_TEST(KernelCmdlineMatchParam); return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.25.4

On Fri, May 29, 2020 at 12:10:03PM +0200, Paulo de Rezende Pinatti wrote: minor nitpick: we can call ^this simply ...SkipQuote, we don't need to be so specific about it being a double quotation mark, do we? Hmm, what if instead we tried to find and return the index of the '=' character but iterated all the way until the next applicable (i.e. taking quotation into account) space and saved that end of arg/arg=val parameter into **res. The caller of this function would then continue directly from *res with the next arg/arg=val parameter. We could then call this something like virKernelCmdlineFindEqual and return -1 if there is no '=' sign, indicating that it's a standalone parameter with no value. If we implement what I suggested above for virKernelCmdlineSearchForward, we won't need 2 wrappers for the same thing differentiating only in whether we do or do not need to search for the '=' sign as well. See below for comments why we don't need this wrapper. if you get the index of the equal sign, but iterate all the way until the end of the arg/arg=val parameter, you can use index and do: *param = g_strndup(cmdline, equal_index); If we used something like char **res (taking it as a parameter to this function) to represent the rest of the unparsed cmdline, then it should be NULL in this case, so the check could remain with a slight adjustment (I haven't implemented it, but I do hope it should work :)) ^This normalization should be done in virKernelCmdlineMatchParam instead. That way, you won't need a wrapper over virKernelArgNormalize. ^This check could then be dropped along with moving the cursor in the cmdline string. If we do the above, we should be able to ditch ^this second loop searching for the next space. ^Here you'd have to do arithmetics using the *res variable instead. We'd simply return *res; I know you used it in the following patches to match against the accepted values, but do we really need to match with a prefix, I'd be fine with simple stirng equality matching in all cases and not mix the matching strategy for both values and kernel arguments. ^This should be default used in all cases IMO unless the prefix matching provides us with a considerable performance gain. is "sticky searching" some terminus technicus? If not, we probably should name this FLAGS_MATCH_FIRST and FLAGS_MATCH_LAST respectively. ^These last two variables can be moved into the while loop, you won't need the explicit VIR_FREEs then. Insert a blank line in between all these "ifs" for better readability (I know the coding guideline we have doesn't mention it). You'd do the normalization of the parsed arg value here. ^Is this even valid for the kernel itself? Looking at [1], they clearly don't allow escaped \" in the arg/value. [1] https://github.com/torvalds/linux/blob/db54615e21419c3cb4d699a0b0aa16cc44... I appreciate the thorough unit testing :) Erik

On 11/06/20 10:09, Erik Skultety wrote: Changed. Will do. No, actually other parameters might use equality matching, this is the case with 'mem_encrypt' (see arch/x86/mm/mem_encrypt_identity.c -> sme_enable -> strncmp at line 575) which was being checked for SEV in the first version of this series. I think we are fine with the current approach as we allow the caller to choose which matching strategy to use by specifying the appropriate flag. I suppose one can also see that way :) I will thus use SEARCH_FIRST as requested. -- Best regards, Paulo de Rezende Pinatti

Implement secure guest check for AMD SEV (Secure Encrypted Virtualization) in order to invalidate the qemu capabilities cache in case the availability of the feature changed. For AMD SEV the verification consists of: - checking if /sys/module/kvm_amd/parameters/sev contains the value '1': meaning SEV is enabled in the host kernel; - checking if /dev/sev exists Signed-off-by: Paulo de Rezende Pinatti <ppinatti(a)linux.ibm.com&gt; Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com&gt; --- src/qemu/qemu_capabilities.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index cbc577353b..0d19d4adff 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4702,6 +4702,24 @@ virQEMUCapsKVMSupportsSecureGuestS390(void) } +/* + * Check whether AMD Secure Encrypted Virtualization (x86) is enabled + */ +static bool +virQEMUCapsKVMSupportsSecureGuestAMD(void) +{ + g_autofree char *modValue = NULL; + + if (virFileReadValueString(&modValue, "/sys/module/kvm_amd/parameters/sev") < 0) + return false; + if (modValue[0] != '1') + return false; + if (virFileExists(QEMU_DEV_SEV)) + return true; + return false; +} + + /* * Check whether the secure guest functionality is enabled. * See the specific architecture function for details on the verifications made. @@ -4713,6 +4731,8 @@ virQEMUCapsKVMSupportsSecureGuest(void) if (ARCH_IS_S390(arch)) return virQEMUCapsKVMSupportsSecureGuestS390(); + if (ARCH_IS_X86(arch)) + return virQEMUCapsKVMSupportsSecureGuestAMD(); return false; } -- 2.25.4

From: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; 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..8ead68798f 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")) 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", "on", "ON", "oN", "On", "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)) { + if (hasFac158) { + 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.4

From: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; Add checking in virt-host-validate for secure guest support on x86 for AMD Secure Encrypted Virtualization. Signed-off-by: Boris Fiuczynski <fiuczy(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 | 29 +++++++++++++++++++++++++++-- tools/virt-host-validate-common.h | 1 + 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index 8ead68798f..de007f2c43 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -41,7 +41,8 @@ VIR_ENUM_IMPL(virHostValidateCPUFlag, "vmx", "svm", "sie", - "158"); + "158", + "sev"); static bool quiet; @@ -447,15 +448,18 @@ int virHostValidateSecureGuests(const char *hvname, virHostValidateLevel level) { virBitmapPtr flags; - bool hasFac158 = false; + bool hasFac158 = false, hasAMDSev = false; virArch arch = virArchFromHost(); g_autofree char *cmdline = NULL; static const char *kIBMValues[] = {"y", "Y", "on", "ON", "oN", "On", "1"}; + g_autofree char *mod_value = NULL; flags = virHostValidateGetCPUFlags(); if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_FACILITY_158)) hasFac158 = true; + else if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_SEV)) + hasAMDSev = true; virBitmapFree(flags); @@ -485,6 +489,27 @@ int virHostValidateSecureGuests(const char *hvname, virHostMsgFail(level, "Hardware or firmware does not provide " "support for IBM Secure Execution"); } + } else if (hasAMDSev) { + if (virFileReadValueString(&mod_value, "/sys/module/kvm_amd/parameters/sev") < 0) { + virHostMsgFail(level, "AMD Secure Encrypted Virtualization not " + "supported by the currently used kernel"); + return 0; + } + if (mod_value[0] != '1') { + virHostMsgFail(level, + "AMD Secure Encrypted Virtualization appears to be " + "disabled in kernel. Add mem_encrypt=on " + "kvm_amd.sev=1 to kernel cmdline arguments"); + return 0; + } + if (virFileExists("/dev/sev")) { + virHostMsgPass(); + return 1; + } else { + virHostMsgFail(level, + "AMD Secure Encrypted Virtualization appears to be " + "disabled in firemare."); + } } else { virHostMsgFail(level, "Unknown if this platform has Secure Guest support"); diff --git a/tools/virt-host-validate-common.h b/tools/virt-host-validate-common.h index 44b5544a12..3df5ea0c7e 100644 --- a/tools/virt-host-validate-common.h +++ b/tools/virt-host-validate-common.h @@ -38,6 +38,7 @@ typedef enum { VIR_HOST_VALIDATE_CPU_FLAG_SVM, VIR_HOST_VALIDATE_CPU_FLAG_SIE, VIR_HOST_VALIDATE_CPU_FLAG_FACILITY_158, + VIR_HOST_VALIDATE_CPU_FLAG_SEV, VIR_HOST_VALIDATE_CPU_FLAG_LAST, } virHostValidateCPUFlag; -- 2.25.4

From: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; Update document with changes in qemu capability caching and the added secure guest support checking for AMD SEV in virt-host-validate. Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; --- docs/kbase/launch_security_sev.rst | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/kbase/launch_security_sev.rst b/docs/kbase/launch_security_sev.rst index 65f258587d..19b978481a 100644 --- a/docs/kbase/launch_security_sev.rst +++ b/docs/kbase/launch_security_sev.rst @@ -30,8 +30,11 @@ Enabling SEV on the host ======================== Before VMs can make use of the SEV feature you need to make sure your -AMD CPU does support SEV. You can check whether SEV is among the CPU -flags with: +AMD CPU does support SEV. You can run ``libvirt-host-validate`` +(libvirt >= 6.5.0) to check if your host supports secure guests or you +can follow the manual checks below. + +You can manually check whether SEV is among the CPU flags with: :: @@ -109,7 +112,7 @@ following: </features> </domainCapabilities> -Note that if libvirt was already installed and libvirtd running before +Note that if libvirt (<6.5.0) was already installed and libvirtd running before enabling SEV in the kernel followed by the host reboot you need to force libvirtd to re-probe both the host and QEMU capabilities. First stop libvirtd: -- 2.25.4

From: Viktor Mihajlovski <mihajlov(a)linux.ibm.com&gt; Protected virtualization/IBM Secure Execution for Linux protects guest memory and state from the host. Add some basic information about technology and a brief guide on setting up secure guests with libvirt. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.ibm.com&gt; Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; Reviewed-by: Paulo de Rezende Pinatti <ppinatti(a)linux.ibm.com&gt; --- docs/kbase.html.in | 3 + docs/kbase/s390_protected_virt.rst | 189 +++++++++++++++++++++++++++++ 2 files changed, 192 insertions(+) create mode 100644 docs/kbase/s390_protected_virt.rst diff --git a/docs/kbase.html.in b/docs/kbase.html.in index c586e0f676..241a212fa9 100644 --- a/docs/kbase.html.in +++ b/docs/kbase.html.in @@ -14,6 +14,9 @@ <dt><a href="kbase/secureusage.html">Secure usage</a></dt> <dd>Secure usage of the libvirt APIs</dd> + <dt><a href="kbase/s390_protected_virt.html">Protected virtualization on s390</a></dt> + <dd>Running secure s390 guests with IBM Secure Execution</dd> + <dt><a href="kbase/launch_security_sev.html">Launch security</a></dt> <dd>Securely launching VMs with AMD SEV</dd> diff --git a/docs/kbase/s390_protected_virt.rst b/docs/kbase/s390_protected_virt.rst new file mode 100644 index 0000000000..f38d16d743 --- /dev/null +++ b/docs/kbase/s390_protected_virt.rst @@ -0,0 +1,189 @@ +================================ +Protected Virtualization on s390 +================================ + +.. contents:: + +Overview +======== + +Protected virtualization, also known as IBM Secure Execution is a +hardware-based privacy protection technology for s390x (IBM Z). +It allows to execute virtual machines such that the host system +has no access to a VM's state and memory contents. + +Unlike other similar technologies, the memory of a running guest +is not encrypted but protected by hardware access controls, which +may only be manipulated by trusted system firmware, called +ultravisor. + +For the cases where the host needs access to guest memory (e.g. for +paging), it can request pages to be exported to it. The exported page +will be encrypted with a unique key for the running guest by the +ultravisor. The ultravisor also computes an integrity value for +the page, and stores it in a special table, together with the page +index and a counter. This way it can verify the integrity of +the page content upon re-import into the guest. + +In other cases it may be necessary for a guest to grant the host access +to dedicated memory regions (e.g. for I/O). The guest can request +that the ultravisor removes the memory protection from individual +pages, so that they can be shared with the host. Likewise, the +guest can undo the sharing. + +A secure guest will initially start in a regular non-protected VM. +The start-up is controlled by a small bootstrap program loaded +into memory together with encrypted operating system components and +a control structure (the PV header). +The operating system components (e.g. Linux kernel, initial RAM +file system, kernel parameters) are encrypted and integrity +protected. The component encryption keys and integrity values are +stored in the PV header. +The PV header is wrapped with a public key belonging to a specific +system (in fact it can be wrapped with multiple such keys). The +matching private key is only accessible by trusted hardware and +firmware in that specific system. +Consequently, such a secure guest boot image can only be run on the +systems it has been prepared for. Its contents can't be decrypted +without access to the private key and it can't be modified as +it is integrity protected. + +Host Requirements +================= + +IBM Secure Execution for Linux has some hardware and firmware +requirements. The system hardware must be an IBM z15 (or newer), +or an IBM LinuxONE III (or newer). + +It is also necessary that the IBM Secure Execution feature is +enabled for that system. With libvirt >= 6.5.0 you can run +``libvirt-host--validate`` or otherwise check for facility '158', e.g. + +:: + + $ grep facilities /proc/cpuinfo | grep 158 + +The kernel must include the protected virtualization support +which can be verified by checking for the presence of directory +``/sys/firmware/uv``. It will only be present when both the +hardware and the kernel support are available. + +Finally, the host operating system must donate some memory to +the ultravisor needed to store memory security information. +This is achieved by specifying the following kernel command +line parameter to the host boot configuration + +:: + + prot_virt=1 + + +Guest Requirements +================== + +Guest Boot +---------- + +To start a guest in protected virtualization secure mode, the +boot image must have been prepared first with the program +``genprotimg`` using the correct public key for this host. +``genprotimg`` is part of the package ``s390-tools``, or +``s390-utils``, depending on the Linux distribution being used. +It can also be found at +`<https://github.com/ibm-s390-tools/s390-tools/tree/master/genprotimg>`_ + +The guests have to be configured to use the host CPU model, which +must contain the ``unpack`` facility indicating ultravisor guest support. + +With the following command it's possible to check whether the host +CPU model satisfies the requirement + +:: + + $ virsh domcapabilities | grep unpack + +which should return + +:: + + <feature policy='require' name='unpack'/> + +Note that on hosts with libvirt < 6.5.0 if the check fails despite +the host system actually supporting protected virtualization guests, +this can be caused by a stale libvirt capabilities cache. +To recover, run the following commands + +:: + + $ systemctl stop libvirtd + $ rm /var/cache/libvirt/qemu/capabilities/*.xml + $ systemctl start libvirtd + + +Guest I/O +--------- + +Protected virtualization guests support I/O using virtio devices. +As the virtio data structures of secure guests are not accessible +by the host, it is necessary to use shared memory ('bounce buffers'). + +To enable virtio devices to use shared buffers, it is necessary +to configure them with platform_iommu enabled. This can done by adding +``iommu='on'`` to the driver element of a virtio device definition in the +guest's XML, e.g. + +:: + + <interface type='network'> + <source network='default'/> + <model type='virtio'/> + <driver name='vhost' iommu='on'/> + </interface> + +It is mandatory to define all virtio bus devices in this way to +prevent the host from attempting to access protected memory. +Ballooning will not work and is fenced by QEMU. It should be +disabled by specifying + +:: + + <memballoon model='none'/> + +Finally, the guest Linux must be instructed to allocate I/O +buffers using memory shared between host and guest using SWIOTLB. +This is done by adding ``swiotlb=nnn`` to the guest's kernel command +line string, where ``nnn`` stands for the number of statically +allocated 2K entries. A commonly used value for swiotlb is 262144. + +Example guest definition +======================== + +Minimal domain XML for a protected virtualization guest, essentially +it's mostly about the ``iommu`` property + +:: + + <domain type='kvm'> + <name>protected</name> + <memory unit='KiB'>2048000</memory> + <currentMemory unit='KiB'>2048000</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='s390x'>hvm</type> + </os> + <cpu mode='host-model'/> + <devices> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none' io='native' iommu='on'> + <source file='/var/lib/libvirt/images/protected.qcow2'/> + <target dev='vda' bus='virtio'/> + </disk> + <interface type='network'> + <driver iommu='on'/> + <source network='default'/> + <model type='virtio'/> + </interface> + <console type='pty'/> + <memballoon model='none'/> + </devices> + </domain> -- 2.25.4