[PATCH v3 0/7] Add Security Guest doc and check for capabilities cache validation
This series introduces the concept of a 'Secure Guest' feature which covers on s390 IBM Secure Execution and on x86 AMD Secure Encrypted Virtualization. Besides adding documentation for IBM Secure Execution it also adds checks during validation of the qemu capabilities cache. These checks per architecture can be performed for IBM Secure Execution on s390 and AMD Secure Encrypted Virtualization on AMD x86 CPUs (both checks implemented in this series). For s390 the verification consists of: - checking if /sys/firmware/uv is available: meaning the HW facility is available and the host OS supports it; - checking if the kernel cmdline contains 'prot_virt=1': meaning the host OS wants to use the feature. For AMD Secure Encrypted Virtualization 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 Whenever the availability of the feature does not match the secure guest flag in the cache then libvirt will re-build it in order to pick up the new set of capabilities available. Additionally, this series adds the same aforementioned checks to the virt-host-validate tool to facilitate the manual verification process for users. Changes in v3: [Patch 1] Reworked auxiliary functions to eliminate unnecessary wrappers Moved arg normalization to MatchParam function Replaced macro VIR_CMDLINE_STR_CMP by the simpler function virKernelCmdlineStrCmp Renamed function SkipDbQuote to SkipQuote Renamed flag SEARCH_STICKY to SEARCH_FIRST Reworked some input values in unit test for better test coverage [Patches 4, 5] Added empty lines between if statements link to v2: https://www.redhat.com/archives/libvir-list/2020-May/msg01175.html Boris Fiuczynski (3): tools: secure guest check on s390 in virt-host-validate tools: secure guest check for AMD in virt-host-validate docs: update AMD launch secure description Paulo de Rezende Pinatti (3): util: introduce a parser for kernel cmdline arguments qemu: check if s390 secure guest support is enabled qemu: check if AMD secure guest support is enabled Viktor Mihajlovski (1): docs: Describe protected virtualization guest setup docs/kbase.html.in | 3 + docs/kbase/launch_security_sev.rst | 9 +- docs/kbase/s390_protected_virt.rst | 189 +++++++++++++++++++++++++++++ src/libvirt_private.syms | 2 + src/qemu/qemu_capabilities.c | 76 ++++++++++++ src/util/virutil.c | 188 ++++++++++++++++++++++++++++ src/util/virutil.h | 17 +++ tests/utiltest.c | 144 ++++++++++++++++++++++ tools/virt-host-validate-common.c | 88 +++++++++++++- tools/virt-host-validate-common.h | 5 + tools/virt-host-validate-qemu.c | 4 + 11 files changed, 720 insertions(+), 5 deletions(-) create mode 100644 docs/kbase/s390_protected_virt.rst -- 2.26.2
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@linux.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/libvirt_private.syms | 2 + src/util/virutil.c | 188 +++++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 17 ++++ tests/utiltest.c | 144 ++++++++++++++++++++++++++++++ 4 files changed, 351 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..b7ea643e4a 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1725,6 +1725,194 @@ virHostGetDRMRenderNode(void) return ret; } + +static const char *virKernelCmdlineSkipQuote(const char *cmdline, + bool *is_quoted) +{ + if (cmdline[0] == '"') { + *is_quoted = !(*is_quoted); + cmdline++; + } + return cmdline; +} + + +/* + * Iterate over the provided kernel command line string while honoring + * the kernel quoting rules and returns the index of the equal sign + * separating argument and value. + * + * @cmdline: target kernel command line string + * @is_quoted: indicates whether the string begins with quotes + * @res: pointer to the position immediately after the parsed parameter, + * can be used in subsequent calls to process further parameters until + * the end of the string. + * + * Returns 0 for the cases where no equal sign is found or the argument + * itself begins with the equal sign (both cases indicating that the + * argument has no value). Otherwise, returns the index of the equal + * sign in the string. + */ +static size_t virKernelCmdlineFindEqual(const char *cmdline, + bool is_quoted, + const char **res) +{ + size_t i; + size_t equal_index = 0; + + for (i = 0; cmdline[i]; i++) { + if (!(is_quoted) && g_ascii_isspace(cmdline[i])) + break; + if (equal_index == 0 && cmdline[i] == '=') { + equal_index = i; + continue; + } + virKernelCmdlineSkipQuote(cmdline + i, &is_quoted); + } + *res = cmdline + i; + return equal_index; +} + + +static char* virKernelArgNormalize(const char *arg) +{ + return virStringReplace(arg, "_", "-"); +} + + +/* + * 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) +{ + const char *next; + int equal_index; + bool is_quoted = false; + *param = NULL; + *val = NULL; + + virSkipSpaces(&cmdline); + cmdline = virKernelCmdlineSkipQuote(cmdline, &is_quoted); + equal_index = virKernelCmdlineFindEqual(cmdline, is_quoted, &next); + + if (next == cmdline) + return next; + + /* param has no value */ + if (equal_index == 0) { + if (is_quoted && next[-1] == '"') + *param = g_strndup(cmdline, next - cmdline - 1); + else + *param = g_strndup(cmdline, next - cmdline); + return next; + } + + *param = g_strndup(cmdline, equal_index); + + if (cmdline[equal_index + 1] == '"') { + is_quoted = true; + equal_index++; + } + + if (is_quoted && next[-1] == '"') + *val = g_strndup(cmdline + equal_index + 1, + next - cmdline - equal_index - 2); + else + *val = g_strndup(cmdline + equal_index + 1, + next - cmdline - equal_index - 1); + return next; +} + + +static bool virKernelCmdlineStrCmp(const char *kernel_val, + const char *caller_val, + virKernelCmdlineFlags flags) +{ + if (flags & VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX) + return STRPREFIX(kernel_val, caller_val); + return STREQ(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, + * this is the default if no CMP flag was specified + * - VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST: first match satisfies search, no matter + * the order (in case of multiple argument occurrences) + * - VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST: use the result of last argument occurrence + * (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 *arg_norm = virKernelArgNormalize(arg); + + while (next[0] != '\0') { + g_autofree char *kparam = NULL; + g_autofree char *kparam_norm = NULL; + g_autofree char *kval = NULL; + next = virKernelCmdlineNextParam(next, &kparam, &kval); + + if (!kparam) + break; + kparam_norm = virKernelArgNormalize(kparam); + + if (STRNEQ(kparam_norm, arg_norm)) + continue; + + if (!kval) { + match = (len_values == 0) ? true : false; + } else { + match = false; + for (i = 0; i < len_values; i++) { + if (virKernelCmdlineStrCmp(kval, values[i], flags)) { + match = true; + break; + } + } + } + + if (match && (flags & VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST)) + 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..b1f6cfdfe4 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_FIRST = 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..e6c1bb1050 100644 --- a/tests/utiltest.c +++ b/tests/utiltest.c @@ -254,6 +254,148 @@ 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" }, + { "arg1=sub1=val1,sub2=val2 arg3=val3 arg4", "arg1", "sub1=val1,sub2=val2", " 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" }, + { "arg2=\"value with space\" arg3=val3", "arg2", "value with space", " arg3=val3" }, + { " arg2=\"value with space\" arg3=val3", "arg2", "value with space", " arg3=val3" }, + { " \"arg2=value with space\" arg3=val3", "arg2", "value with space", " arg3=val3" }, + { "arg2=\"val\"ue arg3", "arg2", "val\"ue", " arg3" }, + { "arg2=value\" long\" arg3", "arg2", "value\" long\"", " arg3" }, + { " \"arg2 with space=value with space\" arg3", "arg2 with space", "value with space", " arg3" }, + { " arg2\" with space=val2\" arg3", "arg2\" with space", "val2\"", " arg3" }, + { " arg2longer=someval\" long\" arg2=val2", "arg2longer", "someval\" long\"", " arg2=val2" }, + { "=val1 arg2=val2", "=val1", NULL, " 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, ¶m, &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_FIRST | 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_FIRST | 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_FIRST | 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_FIRST | 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_FIRST, 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, 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_FIRST, true }, + {"arg1 my-arg arg2=val2 arg4=val4 my-arg=yes arg5", "my_arg", {NULL, NULL}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST, true }, + {"=arg1 my-arg arg2=val2 arg4=val4 my-arg=yes arg5", "my_arg", {NULL, NULL}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST, true }, + {"my-arg =arg1 arg2=val2 arg4=val4 my-arg=yes arg5", "=arg1", {NULL, NULL}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST, true }, + {"arg1 arg2=val2 myarg=sub1=val1 arg5", "myarg", {"sub1=val1", NULL}, VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST, true }, + {"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_FIRST) + VIR_TEST_DEBUG("Flag [VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST]"); + 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 +419,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.26.2
On Mon, Jun 15, 2020 at 10:28:06AM +0200, Paulo de Rezende Pinatti wrote:
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@linux.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com> The code is fine, I just spotted last minute codestyle nitpick that I didn't pay attention to previously, so if you're okay with the snippet below, I'll squash it in before merging: diff --git a/src/util/virutil.c b/src/util/virutil.c index 30df707b8e..b7ea643e4a 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1737,18 +1737,17 @@ static const char *virKernelCmdlineSkipQuote(const char *cmdline, } -/** - * virKernelCmdlineFindEqual: +/* + * Iterate over the provided kernel command line string while honoring + * the kernel quoting rules and returns the index of the equal sign + * separating argument and value. + * * @cmdline: target kernel command line string * @is_quoted: indicates whether the string begins with quotes * @res: pointer to the position immediately after the parsed parameter, * can be used in subsequent calls to process further parameters until * the end of the string. * - * Iterate over the provided kernel command line string while honoring - * the kernel quoting rules and returns the index of the equal sign - * separating argument and value. - * * Returns 0 for the cases where no equal sign is found or the argument * itself begins with the equal sign (both cases indicating that the * argument has no value). Otherwise, returns the index of the equal @@ -1781,17 +1780,16 @@ static char* virKernelArgNormalize(const char *arg) } -/** - * virKernelCmdlineNextParam: - * @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 - * +/* * 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 @@ -1849,19 +1847,25 @@ static bool virKernelCmdlineStrCmp(const char *kernel_val, } -/** - * virKernelCmdlineMatchParam: +/* + * 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, + * this is the default if no CMP flag was specified + * - VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST: first match satisfies search, no matter + * the order (in case of multiple argument occurrences) + * - VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST: use the result of last argument occurrence + * (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: bitwise-OR of virKernelCmdlineFlags - * - * 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. - * + * @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 */ @@ -1880,12 +1884,10 @@ bool virKernelCmdlineMatchParam(const char *cmdline, g_autofree char *kparam = NULL; g_autofree char *kparam_norm = NULL; g_autofree char *kval = NULL; - next = virKernelCmdlineNextParam(next, &kparam, &kval); if (!kparam) break; - kparam_norm = virKernelArgNormalize(kparam); if (STRNEQ(kparam_norm, arg_norm)) diff --git a/src/util/virutil.h b/src/util/virutil.h index 0b9ef5b850..b1f6cfdfe4 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -147,12 +147,11 @@ bool virHostHasIOMMU(void); char *virHostGetDRMRenderNode(void) G_GNUC_NO_INLINE; -/* Kernel cmdline match and comparison strategy for arg=value pairs */ typedef enum { - VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX = 1, /* substring comparison of argument values */ - VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ = 2, /* strict string comparison of argument values */ - VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST = 4, /* match first occurrence of pattern */ - VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST = 8, /* match last occurrence of pattern */ + VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX = 1, + VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ = 2, + VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST = 4, + VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST = 8, } virKernelCmdlineFlags; const char *virKernelCmdlineNextParam(const char *cmdline, diff --git a/tests/utiltest.c b/tests/utiltest.c index 2bff7859dc..e6c1bb1050 100644 --- a/tests/utiltest.c +++ b/tests/utiltest.c @@ -286,12 +286,14 @@ static struct testKernelCmdlineNextParamData kEntries[] = { 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) { - g_autofree char * param = NULL; - g_autofree char * val = NULL; + VIR_FREE(param); + VIR_FREE(val); next = virKernelCmdlineNextParam(kEntries[i].cmdline, ¶m, &val); @@ -306,10 +308,16 @@ testKernelCmdlineNextParam(const void *data G_GNUC_UNUSED) 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; }
On 15/06/20 16:16, Erik Skultety wrote:
On Mon, Jun 15, 2020 at 10:28:06AM +0200, Paulo de Rezende Pinatti wrote:
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@linux.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The code is fine, I just spotted last minute codestyle nitpick that I didn't pay attention to previously, so if you're okay with the snippet below, I'll squash it in before merging:
I just think the description of the flag SEARCH_FIRST might not be clear enough without the "no matter the order" text, as it might suggest it just checks the first occurrence of the parameter and stops there (as opposed to the flag SEARCH_LAST which checks only the last occurrence). I suggest using "match any occurrence of pattern" to avoid confusion. Everything else (including the changes in the other patches) looks good to me.
-/* Kernel cmdline match and comparison strategy for arg=value pairs */ typedef enum { - VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX = 1, /* substring comparison of argument values > - VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ = 2, /* strict string comparison of argument values */ - VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST = 4, /* match first occurrence of pattern */ - VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST = 8, /* match last occurrence of pattern */ + VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX = 1, + VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ = 2, + VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST = 4, + VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST = 8, } virKernelCmdlineFlags;
const char *virKernelCmdlineNextParam(const char *cmdline, diff --git a/tests/utiltest.c b/tests/utiltest.c index 2bff7859dc..e6c1bb1050 100644 --- a/tests/utiltest.c +++ b/tests/utiltest.c @@ -286,12 +286,14 @@ static struct testKernelCmdlineNextParamData kEntries[] = { 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) { - g_autofree char * param = NULL; - g_autofree char * val = NULL; + VIR_FREE(param); + VIR_FREE(val);
next = virKernelCmdlineNextParam(kEntries[i].cmdline, ¶m, &val);
@@ -306,10 +308,16 @@ testKernelCmdlineNextParam(const void *data G_GNUC_UNUSED) 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; }
-- Best regards, Paulo de Rezende Pinatti
On Mon, Jun 15, 2020 at 04:49:10PM +0200, Paulo de Rezende Pinatti wrote:
On 15/06/20 16:16, Erik Skultety wrote:
On Mon, Jun 15, 2020 at 10:28:06AM +0200, Paulo de Rezende Pinatti wrote:
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@linux.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The code is fine, I just spotted last minute codestyle nitpick that I didn't pay attention to previously, so if you're okay with the snippet below, I'll squash it in before merging:
I just think the description of the flag SEARCH_FIRST might not be clear enough without the "no matter the order" text, as it might suggest it just checks the first occurrence of the parameter and stops there (as opposed to the flag SEARCH_LAST which checks only the last occurrence). I suggest using "match any occurrence of pattern" to avoid confusion.
Oh, that would not indeed reflect the reality, but I think we need something more verbose then, something along these lines (along with an example): for SEARCH_FIRST: "look for any occurrence of the argument with the expected value (this should be used when an argument set to certain value overrides all the other occurrences, e.g. when matching value 'on', arg='off arg='on' arg='off' would match with this flag)" and for SEARCH_LAST: "look for the last occurrence of argument with the expected value (this should be used when the last occurrence of the argument overrides all the other ones, e.g. when matching value 'on', arg='on' arg='off' would not match with this flag, but arg='off' arg='on' would)" Does that sound better? PS: sorry for the reverse diff I sent, wrong order when doing the diff :/ Erik
Everything else (including the changes in the other patches) looks good to me.
-/* Kernel cmdline match and comparison strategy for arg=value pairs */ typedef enum { - VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX = 1, /* substring comparison of argument values > - VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ = 2, /* strict string comparison of argument values */ - VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST = 4, /* match first occurrence of pattern */ - VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST = 8, /* match last occurrence of pattern */ + VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX = 1, + VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ = 2, + VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST = 4, + VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST = 8, } virKernelCmdlineFlags;
const char *virKernelCmdlineNextParam(const char *cmdline, diff --git a/tests/utiltest.c b/tests/utiltest.c index 2bff7859dc..e6c1bb1050 100644 --- a/tests/utiltest.c +++ b/tests/utiltest.c @@ -286,12 +286,14 @@ static struct testKernelCmdlineNextParamData kEntries[] = { 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) { - g_autofree char * param = NULL; - g_autofree char * val = NULL; + VIR_FREE(param); + VIR_FREE(val);
next = virKernelCmdlineNextParam(kEntries[i].cmdline, ¶m, &val);
@@ -306,10 +308,16 @@ testKernelCmdlineNextParam(const void *data G_GNUC_UNUSED) 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; }
-- Best regards,
Paulo de Rezende Pinatti
On 15/06/20 17:46, Erik Skultety wrote:
On Mon, Jun 15, 2020 at 04:49:10PM +0200, Paulo de Rezende Pinatti wrote:
On 15/06/20 16:16, Erik Skultety wrote:
On Mon, Jun 15, 2020 at 10:28:06AM +0200, Paulo de Rezende Pinatti wrote:
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@linux.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The code is fine, I just spotted last minute codestyle nitpick that I didn't pay attention to previously, so if you're okay with the snippet below, I'll squash it in before merging:
I just think the description of the flag SEARCH_FIRST might not be clear enough without the "no matter the order" text, as it might suggest it just checks the first occurrence of the parameter and stops there (as opposed to the flag SEARCH_LAST which checks only the last occurrence). I suggest using "match any occurrence of pattern" to avoid confusion.
Oh, that would not indeed reflect the reality, but I think we need something more verbose then, something along these lines (along with an example):
for SEARCH_FIRST: "look for any occurrence of the argument with the expected value (this should be used when an argument set to certain value overrides all the other occurrences, e.g. when matching value 'on', arg='off arg='on' arg='off' would match with this flag)"
and for SEARCH_LAST: "look for the last occurrence of argument with the expected value (this should be used when the last occurrence of the argument overrides all the other ones, e.g. when matching value 'on', arg='on' arg='off' would not match with this flag, but arg='off' arg='on' would)"
Does that sound better?
That sounds great :)
PS: sorry for the reverse diff I sent, wrong order when doing the diff :/
No worries :)
Erik
Everything else (including the changes in the other patches) looks good to me.
-/* Kernel cmdline match and comparison strategy for arg=value pairs */ typedef enum { - VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX = 1, /* substring comparison of argument values > - VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ = 2, /* strict string comparison of argument values */ - VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST = 4, /* match first occurrence of pattern */ - VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST = 8, /* match last occurrence of pattern */ + VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX = 1, + VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ = 2, + VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST = 4, + VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST = 8, } virKernelCmdlineFlags;
const char *virKernelCmdlineNextParam(const char *cmdline, diff --git a/tests/utiltest.c b/tests/utiltest.c index 2bff7859dc..e6c1bb1050 100644 --- a/tests/utiltest.c +++ b/tests/utiltest.c @@ -286,12 +286,14 @@ static struct testKernelCmdlineNextParamData kEntries[] = { 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) { - g_autofree char * param = NULL; - g_autofree char * val = NULL; + VIR_FREE(param); + VIR_FREE(val);
next = virKernelCmdlineNextParam(kEntries[i].cmdline, ¶m, &val);
@@ -306,10 +308,16 @@ testKernelCmdlineNextParam(const void *data G_GNUC_UNUSED) 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; }
-- Best regards,
Paulo de Rezende Pinatti
-- Best regards, Paulo de Rezende Pinatti
This patch introduces a common function to verify if the availability of the so-called Secure Guest feature on the host has changed in order to invalidate the qemu capabilities cache. It can be used as an entry point for verification on different architectures. For s390 the verification consists of: - checking if /sys/firmware/uv is available: meaning the HW facility is available and the host OS supports it; - checking if the kernel cmdline contains 'prot_virt=1': meaning the host OS wants to use the feature. Whenever the availability of the feature does not match the secure guest flag in the cache then libvirt will re-build it in order to pick up the new set of capabilities available. Signed-off-by: Paulo de Rezende Pinatti <ppinatti@linux.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Tested-by: Viktor Mihajlovski <mihajlov@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/qemu/qemu_capabilities.c | 56 ++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f12769635a..1b90682113 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -23,6 +23,7 @@ #include "qemu_capabilities.h" #include "viralloc.h" +#include "virarch.h" #include "vircrypto.h" #include "virlog.h" #include "virerror.h" @@ -657,6 +658,7 @@ struct _virQEMUCaps { virObject parent; bool kvmSupportsNesting; + bool kvmSupportsSecureGuest; char *binary; time_t ctime; @@ -1901,6 +1903,7 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) ret->invalidation = qemuCaps->invalidation; ret->kvmSupportsNesting = qemuCaps->kvmSupportsNesting; + ret->kvmSupportsSecureGuest = qemuCaps->kvmSupportsSecureGuest; ret->ctime = qemuCaps->ctime; @@ -4396,6 +4399,9 @@ virQEMUCapsLoadCache(virArch hostArch, if (virXPathBoolean("boolean(./kvmSupportsNesting)", ctxt) > 0) qemuCaps->kvmSupportsNesting = true; + if (virXPathBoolean("boolean(./kvmSupportsSecureGuest)", ctxt) > 0) + qemuCaps->kvmSupportsSecureGuest = true; + ret = 0; cleanup: VIR_FREE(str); @@ -4633,6 +4639,9 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps) if (qemuCaps->kvmSupportsNesting) virBufferAddLit(&buf, "<kvmSupportsNesting/>\n"); + if (qemuCaps->kvmSupportsSecureGuest) + virBufferAddLit(&buf, "<kvmSupportsSecureGuest/>\n"); + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</qemuCaps>\n"); @@ -4670,6 +4679,44 @@ virQEMUCapsSaveFile(void *data, } +/* + * Check whether IBM Secure Execution (S390) is enabled + */ +static bool +virQEMUCapsKVMSupportsSecureGuestS390(void) +{ + + g_autofree char *cmdline = NULL; + static const char *kValues[] = {"y", "Y", "on", "ON", "oN", "On", "1"}; + + if (!virFileIsDir("/sys/firmware/uv")) + return false; + if (virFileReadValueString(&cmdline, "/proc/cmdline") < 0) + return false; + if (virKernelCmdlineMatchParam(cmdline, "prot_virt", kValues, + G_N_ELEMENTS(kValues), + VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST | + VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX)) + return true; + return false; +} + + +/* + * Check whether the secure guest functionality is enabled. + * See the specific architecture function for details on the verifications made. + */ +static bool +virQEMUCapsKVMSupportsSecureGuest(void) +{ + virArch arch = virArchFromHost(); + + if (ARCH_IS_S390(arch)) + return virQEMUCapsKVMSupportsSecureGuestS390(); + return false; +} + + /* Check the kernel module parameters 'nested' file to determine if enabled * * Intel: 'kvm_intel' uses 'Y' @@ -4857,6 +4904,13 @@ virQEMUCapsIsValid(void *data, qemuCaps->binary, qemuCaps->kvmSupportsNesting); return false; } + + if (virQEMUCapsKVMSupportsSecureGuest() != qemuCaps->kvmSupportsSecureGuest) { + VIR_DEBUG("Outdated capabilities for '%s': kvm kernel secure guest " + "value changed from %d", + qemuCaps->binary, qemuCaps->kvmSupportsSecureGuest); + return false; + } } return true; @@ -5349,6 +5403,8 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, qemuCaps->kernelVersion = g_strdup(kernelVersion); qemuCaps->kvmSupportsNesting = virQEMUCapsKVMSupportsNesting(); + + qemuCaps->kvmSupportsSecureGuest = virQEMUCapsKVMSupportsSecureGuest(); } return qemuCaps; -- 2.26.2
On Mon, Jun 15, 2020 at 10:28:07AM +0200, Paulo de Rezende Pinatti wrote:
This patch introduces a common function to verify if the availability of the so-called Secure Guest feature on the host has changed in order to invalidate the qemu capabilities cache. It can be used as an entry point for verification on different architectures.
For s390 the verification consists of: - checking if /sys/firmware/uv is available: meaning the HW facility is available and the host OS supports it; - checking if the kernel cmdline contains 'prot_virt=1': meaning the host OS wants to use the feature.
Whenever the availability of the feature does not match the secure guest flag in the cache then libvirt will re-build it in order to pick up the new set of capabilities available.
Signed-off-by: Paulo de Rezende Pinatti <ppinatti@linux.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Tested-by: Viktor Mihajlovski <mihajlov@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> ---
Reviewed-by: Erik Skultety <eskultet@redhat.com> I'll squash the following in: diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0bade7e71b..54835f12a6 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4699,12 +4699,8 @@ virQEMUCapsKVMSupportsSecureGuestS390(void) if (!virFileIsDir("/sys/firmware/uv")) return false; - if (virFileReadValueString(&cmdline, "/proc/cmdline") < 0) return false; - - /* we're prefix matching rather than equality matching here, because kernel - * would treat even something like prot_virt='yFOO' as enabled */ if (virKernelCmdlineMatchParam(cmdline, "prot_virt", kValues, G_N_ELEMENTS(kValues), VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST |
On 6/15/20 4:17 PM, Erik Skultety wrote:
On Mon, Jun 15, 2020 at 10:28:07AM +0200, Paulo de Rezende Pinatti wrote:
This patch introduces a common function to verify if the availability of the so-called Secure Guest feature on the host has changed in order to invalidate the qemu capabilities cache. It can be used as an entry point for verification on different architectures.
For s390 the verification consists of: - checking if /sys/firmware/uv is available: meaning the HW facility is available and the host OS supports it; - checking if the kernel cmdline contains 'prot_virt=1': meaning the host OS wants to use the feature.
Whenever the availability of the feature does not match the secure guest flag in the cache then libvirt will re-build it in order to pick up the new set of capabilities available.
Signed-off-by: Paulo de Rezende Pinatti <ppinatti@linux.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Tested-by: Viktor Mihajlovski <mihajlov@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> ---
Reviewed-by: Erik Skultety <eskultet@redhat.com>
I'll squash the following in:
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0bade7e71b..54835f12a6 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4699,12 +4699,8 @@ virQEMUCapsKVMSupportsSecureGuestS390(void)
if (!virFileIsDir("/sys/firmware/uv")) return false; - if (virFileReadValueString(&cmdline, "/proc/cmdline") < 0) return false; - - /* we're prefix matching rather than equality matching here, because kernel - * would treat even something like prot_virt='yFOO' as enabled */ if (virKernelCmdlineMatchParam(cmdline, "prot_virt", kValues, G_N_ELEMENTS(kValues), VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST |
Did you miss adding new lines before the last " return false;" lines in virQEMUCapsKVMSupportsSecureGuestS390 and virQEMUCapsKVMSupportsSecureGuest ? Besides that question I am fine with your micro fixups. -- 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
On Mon, Jun 15, 2020 at 04:49:30PM +0200, Boris Fiuczynski wrote:
On 6/15/20 4:17 PM, Erik Skultety wrote:
On Mon, Jun 15, 2020 at 10:28:07AM +0200, Paulo de Rezende Pinatti wrote:
This patch introduces a common function to verify if the availability of the so-called Secure Guest feature on the host has changed in order to invalidate the qemu capabilities cache. It can be used as an entry point for verification on different architectures.
For s390 the verification consists of: - checking if /sys/firmware/uv is available: meaning the HW facility is available and the host OS supports it; - checking if the kernel cmdline contains 'prot_virt=1': meaning the host OS wants to use the feature.
Whenever the availability of the feature does not match the secure guest flag in the cache then libvirt will re-build it in order to pick up the new set of capabilities available.
Signed-off-by: Paulo de Rezende Pinatti <ppinatti@linux.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Tested-by: Viktor Mihajlovski <mihajlov@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> ---
Reviewed-by: Erik Skultety <eskultet@redhat.com>
I'll squash the following in:
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0bade7e71b..54835f12a6 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4699,12 +4699,8 @@ virQEMUCapsKVMSupportsSecureGuestS390(void)
if (!virFileIsDir("/sys/firmware/uv")) return false; - if (virFileReadValueString(&cmdline, "/proc/cmdline") < 0) return false; - - /* we're prefix matching rather than equality matching here, because kernel - * would treat even something like prot_virt='yFOO' as enabled */ if (virKernelCmdlineMatchParam(cmdline, "prot_virt", kValues, G_N_ELEMENTS(kValues), VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST |
Did you miss adding new lines before the last " return false;" lines in virQEMUCapsKVMSupportsSecureGuestS390 and virQEMUCapsKVMSupportsSecureGuest ?
Good catch :). Again, sorry for the reverse diff. Erik
On 6/15/20 5:51 PM, Erik Skultety wrote:
On Mon, Jun 15, 2020 at 04:49:30PM +0200, Boris Fiuczynski wrote:
On 6/15/20 4:17 PM, Erik Skultety wrote:
On Mon, Jun 15, 2020 at 10:28:07AM +0200, Paulo de Rezende Pinatti wrote:
This patch introduces a common function to verify if the availability of the so-called Secure Guest feature on the host has changed in order to invalidate the qemu capabilities cache. It can be used as an entry point for verification on different architectures.
For s390 the verification consists of: - checking if /sys/firmware/uv is available: meaning the HW facility is available and the host OS supports it; - checking if the kernel cmdline contains 'prot_virt=1': meaning the host OS wants to use the feature.
Whenever the availability of the feature does not match the secure guest flag in the cache then libvirt will re-build it in order to pick up the new set of capabilities available.
Signed-off-by: Paulo de Rezende Pinatti <ppinatti@linux.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Tested-by: Viktor Mihajlovski <mihajlov@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> ---
Reviewed-by: Erik Skultety <eskultet@redhat.com>
I'll squash the following in:
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0bade7e71b..54835f12a6 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4699,12 +4699,8 @@ virQEMUCapsKVMSupportsSecureGuestS390(void)
if (!virFileIsDir("/sys/firmware/uv")) return false; - if (virFileReadValueString(&cmdline, "/proc/cmdline") < 0) return false; - - /* we're prefix matching rather than equality matching here, because kernel - * would treat even something like prot_virt='yFOO' as enabled */ if (virKernelCmdlineMatchParam(cmdline, "prot_virt", kValues, G_N_ELEMENTS(kValues), VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST |
Did you miss adding new lines before the last " return false;" lines in virQEMUCapsKVMSupportsSecureGuestS390 and virQEMUCapsKVMSupportsSecureGuest ?
Good catch :). Again, sorry for the reverse diff.
Erik
Erik, thanks for your review, your micro fixups and pushing the series. -- 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
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@linux.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- 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 1b90682113..60df5b2f7f 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.26.2
On Mon, Jun 15, 2020 at 10:28:08AM +0200, Paulo de Rezende Pinatti wrote:
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@linux.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
Again, tiny codestyle fixup: diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index eaa7741c33..3959b92069 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4724,13 +4724,10 @@ virQEMUCapsKVMSupportsSecureGuestAMD(void) 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; } @@ -4746,10 +4743,8 @@ virQEMUCapsKVMSupportsSecureGuest(void) if (ARCH_IS_S390(arch)) return virQEMUCapsKVMSupportsSecureGuestS390(); - if (ARCH_IS_X86(arch)) return virQEMUCapsKVMSupportsSecureGuestAMD(); - return false; }
From: Boris Fiuczynski <fiuczy@linux.ibm.com> Add checking in virt-host-validate for secure guest support on s390 for IBM Secure Execution. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Tested-by: Viktor Mihajlovski <mihajlov@linux.ibm.com> Reviewed-by: Paulo de Rezende Pinatti <ppinatti@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- tools/virt-host-validate-common.c | 60 +++++++++++++++++++++++++++++-- tools/virt-host-validate-common.h | 4 +++ tools/virt-host-validate-qemu.c | 4 +++ 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index fbefbada96..6e03235ceb 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,57 @@ 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_FIRST | + 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.26.2
On Mon, Jun 15, 2020 at 10:28:09AM +0200, Paulo de Rezende Pinatti wrote:
From: Boris Fiuczynski <fiuczy@linux.ibm.com>
Add checking in virt-host-validate for secure guest support on s390 for IBM Secure Execution.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Tested-by: Viktor Mihajlovski <mihajlov@linux.ibm.com> Reviewed-by: Paulo de Rezende Pinatti <ppinatti@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> ---
My RB still stands, again small enhancement: diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index a279e9cc19..6e03235ceb 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -471,10 +471,6 @@ int virHostValidateSecureGuests(const char *hvname, if (virFileReadValueString(&cmdline, "/proc/cmdline") < 0) return -1; - /* we're prefix matching rather than equality matching here, because - * kernel would treat even something like prot_virt='yFOO' as - * enabled - */ if (virKernelCmdlineMatchParam(cmdline, "prot_virt", kIBMValues, G_N_ELEMENTS(kIBMValues), VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST |
On 6/15/20 4:19 PM, Erik Skultety wrote:
On Mon, Jun 15, 2020 at 10:28:09AM +0200, Paulo de Rezende Pinatti wrote:
From: Boris Fiuczynski <fiuczy@linux.ibm.com>
Add checking in virt-host-validate for secure guest support on s390 for IBM Secure Execution.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Tested-by: Viktor Mihajlovski <mihajlov@linux.ibm.com> Reviewed-by: Paulo de Rezende Pinatti <ppinatti@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> ---
My RB still stands, again small enhancement:
diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index a279e9cc19..6e03235ceb 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -471,10 +471,6 @@ int virHostValidateSecureGuests(const char *hvname, if (virFileReadValueString(&cmdline, "/proc/cmdline") < 0) return -1;
- /* we're prefix matching rather than equality matching here, because - * kernel would treat even something like prot_virt='yFOO' as - * enabled - */ if (virKernelCmdlineMatchParam(cmdline, "prot_virt", kIBMValues, G_N_ELEMENTS(kIBMValues), VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST |
Thanks Erik, the "enhancement" is fine with me. :-) -- 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
From: Boris Fiuczynski <fiuczy@linux.ibm.com> Add checking in virt-host-validate for secure guest support on x86 for AMD Secure Encrypted Virtualization. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Paulo de Rezende Pinatti <ppinatti@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- tools/virt-host-validate-common.c | 30 +++++++++++++++++++++++++++++- tools/virt-host-validate-common.h | 1 + 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index 6e03235ceb..0e5932c17e 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; @@ -448,14 +449,18 @@ int virHostValidateSecureGuests(const char *hvname, { virBitmapPtr flags; bool hasFac158 = false; + bool 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); @@ -487,6 +492,29 @@ 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.26.2
On Mon, Jun 15, 2020 at 10:28:10AM +0200, Paulo de Rezende Pinatti wrote:
From: Boris Fiuczynski <fiuczy@linux.ibm.com>
Add checking in virt-host-validate for secure guest support on x86 for AMD Secure Encrypted Virtualization.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Paulo de Rezende Pinatti <ppinatti@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- RB still stands, I just noticed that we require users to set mem_encrypt=on for SEV which we know is not mandatory, so I dropped that bit, we can recommend mem_encrypt somewhere else in the docs or kbase.
diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index f68c9c7c96..f05252439e 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -506,8 +506,8 @@ int virHostValidateSecureGuests(const char *hvname, if (mod_value[0] != '1') { virHostMsgFail(level, "AMD Secure Encrypted Virtualization appears to be " - "disabled in kernel. Add kvm_amd.sev=1 " - "to the kernel cmdline arguments"); + "disabled in kernel. Add mem_encrypt=on " + "kvm_amd.sev=1 to kernel cmdline arguments"); return 0; }
On 6/15/20 4:21 PM, Erik Skultety wrote:
On Mon, Jun 15, 2020 at 10:28:10AM +0200, Paulo de Rezende Pinatti wrote:
From: Boris Fiuczynski <fiuczy@linux.ibm.com>
Add checking in virt-host-validate for secure guest support on x86 for AMD Secure Encrypted Virtualization.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Paulo de Rezende Pinatti <ppinatti@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- RB still stands, I just noticed that we require users to set mem_encrypt=on for SEV which we know is not mandatory, so I dropped that bit, we can recommend mem_encrypt somewhere else in the docs or kbase.
diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index f68c9c7c96..f05252439e 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -506,8 +506,8 @@ int virHostValidateSecureGuests(const char *hvname, if (mod_value[0] != '1') { virHostMsgFail(level, "AMD Secure Encrypted Virtualization appears to be " - "disabled in kernel. Add kvm_amd.sev=1 " - "to the kernel cmdline arguments"); + "disabled in kernel. Add mem_encrypt=on " + "kvm_amd.sev=1 to kernel cmdline arguments"); return 0; }
Erik, I agree to the change which was an oversight in my changes for the adjusted AMD checks. Thanks for catching it. -- 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
From: Boris Fiuczynski <fiuczy@linux.ibm.com> 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@linux.ibm.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- 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.26.2
From: Viktor Mihajlovski <mihajlov@linux.ibm.com> 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@linux.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Paulo de Rezende Pinatti <ppinatti@linux.ibm.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- 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.26.2
On Mon, Jun 15, 2020 at 10:28:05AM +0200, Paulo de Rezende Pinatti wrote:
This series introduces the concept of a 'Secure Guest' feature which covers on s390 IBM Secure Execution and on x86 AMD Secure Encrypted Virtualization.
Besides adding documentation for IBM Secure Execution it also adds checks during validation of the qemu capabilities cache. These checks per architecture can be performed for IBM Secure Execution on s390 and AMD Secure Encrypted Virtualization on AMD x86 CPUs (both checks implemented in this series).
For s390 the verification consists of: - checking if /sys/firmware/uv is available: meaning the HW facility is available and the host OS supports it; - checking if the kernel cmdline contains 'prot_virt=1': meaning the host OS wants to use the feature.
For AMD Secure Encrypted Virtualization 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
Whenever the availability of the feature does not match the secure guest flag in the cache then libvirt will re-build it in order to pick up the new set of capabilities available.
Additionally, this series adds the same aforementioned checks to the virt-host-validate tool to facilitate the manual verification process for users.
ACK to the series, let me know whether you agree with the micro fixups I attached to the individual patch review and I'll squash them before pushing. Thanks for bearing with me, Erik
participants (3)
-
Boris Fiuczynski -
Erik Skultety -
Paulo de Rezende Pinatti