Re: [PATCH 1/6] util: introduce a parser for kernel cmdline arguments
On 15/05/20 16:19, Erik Skultety wrote:
On Mon, May 11, 2020 at 06:41:56PM +0200, Boris Fiuczynski wrote:
From: Paulo de Rezende Pinatti
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
Reviewed-by: Boris Fiuczynski
---
src/libvirt_private.syms | 2 +
src/util/virutil.c | 173 +++
src/util/virutil.h | 18
tests/utiltest.c | 130 +
4 files changed, 323 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 935ef7303b..25b22a3e3f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3428,6 +3428,8 @@ virHostGetDRMRenderNode;
virHostHasIOMMU;
virIndexToDiskName;
virIsDevMapperDevice;
+virKernelCmdlineGetValue;
+virKernelCmdlineMatchParam;
virMemoryLimitIsSet;
virMemoryLimitTruncate;
virMemoryMaxValue;
diff --git a/src/util/virutil.c b/src/util/virutil.c
index fb46501142..264aae8d01 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1725,6 +1725,179 @@ virHostGetDRMRenderNode(void)
return ret;
}
+
+#define VIR_IS_CMDLINE_SPACE(value) \
+(g_ascii_isspace(value) || (unsigned char) value == 160)
Hmm, we're not checking the non-breaking space anywhere else in the code, so I
think we'd be fine not checking it here either, so plain g_ascii_isspace()
would suffice in the code
+
+
+static bool virKernelArgsAreEqual(const char *arg1,
+ const char *arg2,
+ size_t n)
+{
+size_t i;
+
+for (i = 0; i < n; i++) {
+if ((arg1[i] == '-' && arg2[i] == '_') ||
+(arg1[i] == '_' && arg2[i] == '-') || (arg1[i] == arg2[i])) {
Because '-' and '_' are equal in the parameter syntax, rather than introducing
another string equality function just because of this, we should normalize the
inputs by replacing occurrences of one with the other and then simply do STREQ
at the appropriate spot in the code
+if (arg1[i] == '\0')
+return true;
+continue;
+}
+return false;
+}
+return true;
+}
+
+
+/*
+ * Parse the kernel cmdline and store the value of @arg in @val
+ * which can be NULL if @arg has no value or if it's not found.
+ * In addition, store in @next the address right after
+ * @arg=@value for possible further processing.
+ *
+ * @arg: kernel command line argument
+ * @cmdline: kernel command line string to be checked for @arg
+ * @val: pointer to hold retrieved value of @arg
+ * @next: pointer to hold address right after @arg=@val, will
+ * point to the string's end (NULL) in case @arg is not found
+ *
+ * Returns 0 if @arg is present in @cmdline, 1 otherwise
+ */
+int virKernelCmdlineGetValue(const char *arg,
+ const char *cmdline,
+ char **val,
+ const char **next)
+{
+size_t i = 0, param_i;
in this specific case I think that naming the iteration variable param_i is
more confusing than actually settling down with something like "offset"
especially when you're doing arithmetics with it.
+size_t arg_len = strlen(arg);
+bool is_quoted, match;
1 declaration/definition per line please
+
+*next = cmdline;
+*val = NULL;
+
+while (cmdline[i] != '\0') {
+/* remove leading spaces */
+while (VIR_IS_CMDLINE_SPACE(cmdline[i]))
+i++;
For ^this, we already have a primitive virSkipSpaces()
+if (cmdline[i] == '"') {
+i++;
+is_quoted = true;
+} else {
+is_quoted = false;
+}
+for (param_i = i; cmdline[param_i]; param_i++) {
+if ((VIR_IS_CMDLINE_SPACE(cmdline[param_i]) && !is_quoted) ||
+cmdline[param_i] == '=')
+break;
+if (cmdline[param_i] == '"')
+is_quoted = !is_quoted;
+}
+if (param_i-i == arg_len && virKernelArgsAreEqual(cmdline+i, arg,
arg_len))
We don't use Yoda conditions, so ^this should be arg_len == param_i - i
+match = true;
+else
+match = false;
+/* arg has no value */
+if (cmdline[param_i] != '=') {
+if (match) {
+*next = cmdline+param_i;
please use a space in between the operands and the operator
+return 0;
+}
+i = param_i;
+continue;
+}
+param_i++;
+if (cmdline[param_i] == '\0')
+break;
+
+if (cmdline[param_i] == '"') {
+param_i++;
+is_quoted = !is_quoted;
+
[PATCH v2 4/7] tools: secure guest check on s390 in virt-host-validate
From: Boris Fiuczynski
Add checking in virt-host-validate for secure guest support
on s390 for IBM Secure Execution.
Signed-off-by: Boris Fiuczynski
Tested-by: Viktor Mihajlovski
Reviewed-by: Paulo de Rezende Pinatti
Reviewed-by: Bjoern Walk
---
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(, "/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
[PATCH v2 2/7] qemu: check if s390 secure guest support is enabled
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
Signed-off-by: Boris Fiuczynski
Tested-by: Viktor Mihajlovski
Reviewed-by: Bjoern Walk
---
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..cbc577353b 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(, "\n");
+if (qemuCaps->kvmSupportsSecureGuest)
+virBufferAddLit(, "\n");
+
virBufferAdjustIndent(, -2);
virBufferAddLit(, "\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(, "/proc/cmdline") < 0)
+return false;
+if (virKernelCmdlineMatchParam(cmdline, "prot_virt", kValues,
+ G_N_ELEMENTS(kValues),
+ VIR_KERNEL_CMDLINE_FLAGS_SEARCH_STICKY |
+ 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.25.4
[PATCH v2 5/7] tools: secure guest check for AMD in virt-host-validate
From: Boris Fiuczynski
Add checking in virt-host-validate for secure guest support
on x86 for AMD Secure Encrypted Virtualization.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Paulo de Rezende Pinatti
Reviewed-by: Bjoern Walk
---
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(_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
[PATCH v2 1/7] util: introduce a parser for kernel cmdline arguments
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
Signed-off-by: Boris Fiuczynski
---
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 = virKernelCmdlineSkipDbQuote(cmdline, _quoted);
+offset = virKernelCmdlineNextSpaceOrEqual(cmdline, _quoted);
+if (offset == 0)
+return cmdline;
+
+*param = virKernelCmdlineArgNormalize(cmdline, offset);
+cmdline = cmdline + offset;
+/* param has no value */
+if (*cmdline != '=')
+return cmdline;
+
+cmdline = virKernelCmdlineSkipDbQuote(++cmdline, _quoted);
+offset = virKernelCmdlineNextSpace(cmdline, _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 st
[PATCH v2 7/7] docs: Describe protected virtualization guest setup
From: Viktor Mihajlovski 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 Signed-off-by: Boris Fiuczynski Reviewed-by: Paulo de Rezende Pinatti --- 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 @@ Secure usage Secure usage of the libvirt APIs +Protected virtualization on s390 +Running secure s390 guests with IBM Secure Execution + Launch security Securely launching VMs with AMD SEV diff --git a/docs/kbase/s390_protected_virt.rst b/docs/kbase/s390_protected_virt.rst new file mode 100644 index 00..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 i
[PATCH v2 3/7] qemu: check if AMD secure guest support is enabled
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
Signed-off-by: Boris Fiuczynski
Reviewed-by: Bjoern Walk
---
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(,
"/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
[PATCH v2 6/7] docs: update AMD launch secure description
From: Boris Fiuczynski 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 --- 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: -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
[PATCH v2 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 v2: [Patch 1] Reworked kernel cmdline parser into a parameter based processing. [Patch 2] Added missing value "on" to kvalue list [Patch 3] Changed AMD SEV support check to module parameter is set and /dev/sev exists. Moved doc changes to a new standalone patch 6. [Patch 4] Added missing value "on" to kvalue list [Patch 5] Changed AMD SEV support check to align with patch 3. Moved doc changes to a new standalone patch 6. [Patch 6] Summarized AMD SEV doc changes from patches 3 and 5. Adjusted libvirt version number [Patch 7 (v1: Patch 6)] Adjusted libvirt version number link to v1: https://www.redhat.com/archives/libvir-list/2020-May/msg00416.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 | 169 ++ src/util/virutil.h | 17 +++ tests/utiltest.c | 141 + tools/virt-host-validate-common.c | 83 - tools/virt-host-validate-common.h | 5 + tools/virt-host-validate-qemu.c| 4 + 11 files changed, 693 insertions(+), 5 deletions(-) create mode 100644 docs/kbase/s390_protected_virt.rst -- 2.25.4
Re: [PATCH] qemu: do not add model when actual iface type is hostdev
On 17/06/20 06:01, Laine Stump wrote:
On 6/16/20 10:32 AM, Paulo de Rezende Pinatti wrote:
No default model should be added to the interface
entry at post parse when its actual network type is hostdev
as doing so might cause a mismatch between the interface
definition and its actual device type.
Have you encountered a real problem from this? (I have, and have been
thinking about the issue for awhile, but only late at night when I'm not
near my keyboard to do something about it. I'm just wondering what
problem you've had :-))
Yes I have. I first defined a passthrough network out of a PF and then
created the domain with an interface definition 'network=passthrough/>' and no model (basically the process outlined in
https://wiki.libvirt.org/page/Networking#Assignment_from_a_pool_of_SRIOV_VFs_in_a_libvirt_%3Cnetwork%3E_definition)
, but after saving it libvirt automatically added
(that's the default model on S390) which led to a qemu error when trying
to start the vm (see below).
The reason it's been on my mind is this: The default model is rtl8139.
When libvirt is auto-assigning PCI addresses to devices at parse time,
it decides whether to assign a network device to a conventional PCI or
PCI Express slot based on the model, and rtl8139 is conventional PCI. So
if you have where the network is a pool of
hostdevs, and if you don't assign a "fake" model like "virtio" or
"e1000e", then the hostdev device (which is 100% certainly a PCIe
device) will be assigned to a conventional PCI slot. That works, but
is "sub-optimal" :-)
For me it didn't work at all because model virtio on S390 leads to a
'devno' property being specified which of course is wrong for this type
of device (i.e. '-device
vfio-pci,host=0100:00:08.2,id=hostdev0,devno=fe.0.0006').
I think we will still need to add a bit to
qemuDomainDeviceCalculatePCIConnectFlags() in order to get the right
type of slot set, but this is a good start.
Reviewed-by: Laine Stump
Thanks, and congratulations on your first libvirt patch!
Thanks! :)
---
src/qemu/qemu_domain.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2dad823a86..33ce0ad992 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5831,6 +5831,7 @@
qemuDomainDeviceNetDefPostParse(virDomainNetDefPtr net,
virQEMUCapsPtr qemuCaps)
{
if (net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
+ virDomainNetResolveActualType(net) !=
VIR_DOMAIN_NET_TYPE_HOSTDEV &&
!virDomainNetGetModelString(net))
net->model = qemuDomainDefaultNetModel(def, qemuCaps);
--
Best regards,
Paulo de Rezende Pinatti
Re: [PATCH] qemu: auto-assign hostdev interface devices to PCIe
On 17/06/20 23:17, Laine Stump wrote:
[...]
(Note to Paulo - I realize this doesn't describe exactly what happens
on s390, since the default interface model in that case is "virtio"
rather than "rtl8139", and this patch should actually cause no
behavior change on S390. I'm Cc'ing you since you're the author of the
other patch I mention in the commit message, and also so you can try
running your same tests with this patch added, and verify that it
really doesn't break anything for S390.)
Hi Laine,
Thanks for the note, I ran the same tests with this patch and can
confirm it still works for S390.
src/qemu/qemu_domain_address.c | 8
1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 07431343ed..05cf251cd5 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -734,6 +734,14 @@
qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
if (net->model == VIR_DOMAIN_NET_MODEL_E1000E)
return pcieFlags;
+/* the only time model can be "unknown" is for type='hostdev'
+ * or for type='network' where the network is a pool of
+ * hostdev devices. These will always be pcie on the host, and
+ * should be pcie in the guest if it supports pcie.
+ */
+if (net->model == VIR_DOMAIN_NET_MODEL_UNKNOWN)
+return pcieFlags;
+
return pciFlags;
}
--
Best regards,
Paulo de Rezende Pinatti
Re: [PATCH] qemu: do not add model when actual iface type is hostdev
Sorry, forgot to include my signed-off-by, so:
On 16/06/20 16:32, Paulo de Rezende Pinatti wrote:
No default model should be added to the interface
entry at post parse when its actual network type is hostdev
as doing so might cause a mismatch between the interface
definition and its actual device type.
---
src/qemu/qemu_domain.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2dad823a86..33ce0ad992 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5831,6 +5831,7 @@ qemuDomainDeviceNetDefPostParse(virDomainNetDefPtr net,
virQEMUCapsPtr qemuCaps)
{
if (net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
+virDomainNetResolveActualType(net) != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
!virDomainNetGetModelString(net))
net->model = qemuDomainDefaultNetModel(def, qemuCaps);
Signed-off-by: Paulo de Rezende Pinatti
[PATCH] qemu: do not add model when actual iface type is hostdev
No default model should be added to the interface
entry at post parse when its actual network type is hostdev
as doing so might cause a mismatch between the interface
definition and its actual device type.
---
src/qemu/qemu_domain.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2dad823a86..33ce0ad992 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5831,6 +5831,7 @@ qemuDomainDeviceNetDefPostParse(virDomainNetDefPtr net,
virQEMUCapsPtr qemuCaps)
{
if (net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
+virDomainNetResolveActualType(net) != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
!virDomainNetGetModelString(net))
net->model = qemuDomainDefaultNetModel(def, qemuCaps);
--
2.26.2
[PATCH v3 2/7] qemu: check if s390 secure guest support is enabled
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
Signed-off-by: Boris Fiuczynski
Tested-by: Viktor Mihajlovski
Reviewed-by: Bjoern Walk
---
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(, "\n");
+if (qemuCaps->kvmSupportsSecureGuest)
+virBufferAddLit(, "\n");
+
virBufferAdjustIndent(, -2);
virBufferAddLit(, "\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(, "/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
[PATCH v3 6/7] docs: update AMD launch secure description
From: Boris Fiuczynski 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 Reviewed-by: Erik Skultety --- 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: -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
[PATCH v3 1/7] util: introduce a parser for kernel cmdline arguments
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
Signed-off-by: Boris Fiuczynski
---
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, _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 = virKernelCmdlineSkipQuote(cmdline, _quoted);
+equal_index = virKernelCmdlineFindEqual(cmdline, is_quoted, );
+
+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_KERNE
[PATCH v3 4/7] tools: secure guest check on s390 in virt-host-validate
From: Boris Fiuczynski
Add checking in virt-host-validate for secure guest support
on s390 for IBM Secure Execution.
Signed-off-by: Boris Fiuczynski
Tested-by: Viktor Mihajlovski
Reviewed-by: Paulo de Rezende Pinatti
Reviewed-by: Bjoern Walk
Reviewed-by: Erik Skultety
---
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(, "/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
[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
[PATCH v3 3/7] qemu: check if AMD secure guest support is enabled
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
Signed-off-by: Boris Fiuczynski
Reviewed-by: Bjoern Walk
Reviewed-by: Erik Skultety
---
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(,
"/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
[PATCH v3 7/7] docs: Describe protected virtualization guest setup
From: Viktor Mihajlovski 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 Signed-off-by: Boris Fiuczynski Reviewed-by: Paulo de Rezende Pinatti Reviewed-by: Erik Skultety --- 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 @@ Secure usage Secure usage of the libvirt APIs +Protected virtualization on s390 +Running secure s390 guests with IBM Secure Execution + Launch security Securely launching VMs with AMD SEV diff --git a/docs/kbase/s390_protected_virt.rst b/docs/kbase/s390_protected_virt.rst new file mode 100644 index 00..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
[PATCH v3 5/7] tools: secure guest check for AMD in virt-host-validate
From: Boris Fiuczynski
Add checking in virt-host-validate for secure guest support
on x86 for AMD Secure Encrypted Virtualization.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Paulo de Rezende Pinatti
Reviewed-by: Bjoern Walk
Reviewed-by: Erik Skultety
---
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(_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
Re: [PATCH v3 1/7] util: introduce a parser for kernel cmdline arguments
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
Signed-off-by: Boris Fiuczynski
Reviewed-by: Erik Skultety
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, , );
@@ -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
Re: [PATCH v3 1/7] util: introduce a parser for kernel cmdline arguments
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
Signed-off-by: Boris Fiuczynski
Reviewed-by: Erik Skultety
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, , );
@@ -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
Re: [PATCH v2 1/7] util: introduce a parser for kernel cmdline arguments
On 10/06/20 15:55, Paulo de Rezende Pinatti wrote:
+ { " arg3\" escaped=val2\"", "arg3\"
escaped", "val2", "" },
^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/db54615e21419c3cb4d699a0b0aa16cc44d0e9da/lib/cmdline.c
I guess the word "escaped" in this test is a bit misleading; it's
actually escaping the blank space, not the quote itself. This is valid
for the kernel. In order to assure our parsing results would match those
of the kernel I executed the code in cmdline.c in a standalone file to
generate the reference values for the test.
I'll change "arg3\" escaped" to "arg3\" with spaces" to clearly state
the intention here.
I forgot to mention that while double checking the reference values I
had to add a missing trailing double quote to the expected result value
in order to match what the kernel produces for this case. So this is
actually "val2\"".
--
Best regards,
Paulo de Rezende Pinatti
Re: [PATCH v2 1/7] util: introduce a parser for kernel cmdline arguments
On 08/06/20 16:35, Erik Skultety wrote:
On Fri, May 29, 2020 at 12:10:03PM +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
Signed-off-by: Boris Fiuczynski
---
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,
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?
Sure, changed to SkipQuote.
+ 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)
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.
Yes, we can do that. It would look like this:
size_t index;
size_t equal_index = 0;
for (index = 0; cmdline[index]; index++) {
if (!(is_quoted) && g_ascii_isspace(cmdline[index]))
break;
if (equal_index == 0 && cmdline[index] == '=') {
equal_index = index;
continue;
}
virKernelCmdlineSkipQuote(cmdline + index, _quoted);
}
*res = cmdline + index;
return equal_index;
I found it more convenient to use 0 rather than -1 so that we can also
handle the case when the argument itself starts with the equal sign.
+{
+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);
+}
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.
Yes, wrappers are gone.
+
+
+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);
See below for comments why we don't need this wrapper.
This wrapper is also gone.
+}
+
+
+/*
+ * 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 aft
Re: [PATCH v2 0/7] Add Security Guest doc and check for capabilities cache validation
Ping for reviews. On 29/05/20 12:10, 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. Changes in v2: [Patch 1] Reworked kernel cmdline parser into a parameter based processing. [Patch 2] Added missing value "on" to kvalue list [Patch 3] Changed AMD SEV support check to module parameter is set and /dev/sev exists. Moved doc changes to a new standalone patch 6. [Patch 4] Added missing value "on" to kvalue list [Patch 5] Changed AMD SEV support check to align with patch 3. Moved doc changes to a new standalone patch 6. [Patch 6] Summarized AMD SEV doc changes from patches 3 and 5. Adjusted libvirt version number [Patch 7 (v1: Patch 6)] Adjusted libvirt version number link to v1: https://www.redhat.com/archives/libvir-list/2020-May/msg00416.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 | 169 ++ src/util/virutil.h | 17 +++ tests/utiltest.c | 141 + tools/virt-host-validate-common.c | 83 - tools/virt-host-validate-common.h | 5 + tools/virt-host-validate-qemu.c| 4 + 11 files changed, 693 insertions(+), 5 deletions(-) create mode 100644 docs/kbase/s390_protected_virt.rst -- Best regards, Paulo de Rezende Pinatti
Re: [PATCH v2 1/7] util: introduce a parser for kernel cmdline arguments
On 11/06/20 10:09, Erik Skultety wrote:
On Wed, Jun 10, 2020 at 03:55:14PM +0200, Paulo de Rezende Pinatti wrote:
On 08/06/20 16:35, Erik Skultety wrote:
On Fri, May 29, 2020 at 12:10:03PM +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
Signed-off-by: Boris Fiuczynski
---
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,
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?
Sure, changed to SkipQuote.
+ 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)
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.
Yes, we can do that. It would look like this:
size_t index;
nitpick: ^this is a standard iteration variable, so naming it "i" might look
more "expected"
Changed.
size_t equal_index = 0;
for (index = 0; cmdline[index]; index++) {
if (!(is_quoted) && g_ascii_isspace(cmdline[index]))
break;
if (equal_index == 0 && cmdline[index] == '=') {
equal_index = index;
continue;
}
virKernelCmdlineSkipQuote(cmdline + index, _quoted);
}
*res = cmdline + index;
return equal_index;
I found it more convenient to use 0 rather than -1 so that we can also
handle the case when the argument itself starts with the equal sign.
Yes, that's fine, but please provide a doc string for this function mentioning
this.
Will do.
...
So the function would look like this now:
virSkipSpaces();
cmdline = virKernelCmdlineSkipQuote(cmdline, _quoted);
equal_index = virKernelCmdlineFindEqual(cmdline, is_quoted, );
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;
There's still a bit of handling required due to the kernel's quoting rules.
I took the liberty of using 'next' in place of 'res' as I think it conveys
better its purpose. Let me know if that looks good to you.
That is fine, looks good.
+}
+
+
+#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_
[PATCH] doc: fix name of file containing max number of VFs
Signed-off-by: Paulo de Rezende Pinatti --- docs/formatnode.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index e4328fedbe..8a51c4da80 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -141,7 +141,7 @@ In this case this device is an SRIOV PF, and the capability element will have a list of address subelements, one for each VF on this PF. If the host system -supports reporting it (via the "sriov_maxvfs" file in the +supports reporting it (via the "sriov_totalvfs" file in the device's sysfs directory) the capability element will also have an attribute named maxCount which is the maximum number of SRIOV VFs supported by this device, which -- 2.26.2
[RFC] Dynamic creation of VFs in a network definition containing an SRIOV device
Context: Libvirt can already detect the active VFs of an SRIOV PF device specified in a network definition and automatically assign these VFs to guests via an entry referring to that network in the domain definition. This functionality, however, depends on the system administrator having activated in advance the desired number of VFs outside of libvirt (either manually or through system scripts). It would be more convenient if the VFs activation could also be managed inside libvirt so that the whole management of the VF pool is done exclusively by libvirt and in only one place (the network definition) rather than spread in different components of the system. Proposal: We can extend the existing network definition by adding a new tag as a child of the tag in order to allow the user to specify how many VFs they wish to have activated for the corresponding SRIOV device when the network is started. That would look like the following: sriov-pool At xml definition time nothing gets changed on the system, as it is today. When the network is started with 'virth net-start sriov-pool' then libvirt will activate the desired number of VFs as specified in the tag of the network definition. The operation might require resetting 'sriov_numvfs' to zero first in case the number of VFs currently active differs from the desired value. In order to avoid the situation where the user tries to start the network when a VF is already assigned to a running guest, the implementation will have to ensure all existing VFs of the target PF are not in use, otherwise VFs would be inadvertently hot-unplugged from guests upon network start. In such cases, trying to start the network will then result in an error. Stopping the network with 'virsh net-destroy' will cause all VFs to be removed. Similarly to when starting the network, the implementation will also need to verify for running guests in order to prevent inadvertent hot-unplugging. Is the functionality proposed above desirable? -- Thanks and best regards, Paulo de Rezende Pinatti
