IGVM files can contain an initial VMSA that should be applied to each
vcpu as part of the initial guest state. The sev_features flags are
provided as part of the VMSA structure. However, KVM only allows
sev_features to be set during initialization and not as the guest is
being prepared for launch.
This patch queries KVM for the supported set of sev_features flags and
processes the VP context entries in the IGVM file during kvm_init to
determine any sev_features flags set in the IGVM file. These are then
provided in the call to KVM_SEV_INIT2 to ensure the guest state
matches that specified in the IGVM file.
The igvm process() function is modified to allow a partial processing
of the file during initialization, with on the IGVM_VHT_VP_CONTEXT
fields being processed. This means the function is called twice,
firstly to extract the sev_features then secondly to actually
configure the guest.
Signed-off-by: Roy Hopkins <roy.hopkins@randomman.co.uk>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Stefano Garzarella <sgarzare@redhat.com>
Acked-by: Gerd Hoffman <kraxel@redhat.com>
Tested-by: Stefano Garzarella <sgarzare@redhat.com>
---
backends/igvm.c | 17 +++-
backends/igvm.h | 2 +-
hw/i386/pc_piix.c | 2 +-
hw/i386/pc_q35.c | 2 +-
include/system/igvm-cfg.h | 5 +-
target/i386/sev.c | 161 +++++++++++++++++++++++++++++++++-----
6 files changed, 163 insertions(+), 26 deletions(-)
diff --git a/backends/igvm.c b/backends/igvm.c
index 034f504716..9853ba941e 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -869,7 +869,7 @@ static IgvmHandle qigvm_file_init(char *filename, Error **errp)
}
int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
- Error **errp)
+ bool onlyVpContext, Error **errp)
{
int32_t header_count;
QIgvmParameterData *parameter;
@@ -913,11 +913,22 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
ctx.current_header_index++) {
IgvmVariableHeaderType type = igvm_get_header_type(
ctx.file, IGVM_HEADER_SECTION_DIRECTIVE, ctx.current_header_index);
- if (qigvm_handler(&ctx, type, errp) < 0) {
- goto cleanup_parameters;
+ if (!onlyVpContext || (type == IGVM_VHT_VP_CONTEXT)) {
+ if (qigvm_handler(&ctx, type, errp) < 0) {
+ goto cleanup_parameters;
+ }
}
}
+ /*
+ * If only processing the VP context then we don't need to process
+ * any more of the file.
+ */
+ if (onlyVpContext) {
+ retval = 0;
+ goto cleanup_parameters;
+ }
+
header_count =
igvm_header_count(ctx.file, IGVM_HEADER_SECTION_INITIALIZATION);
if (header_count < 0) {
diff --git a/backends/igvm.h b/backends/igvm.h
index db02ea9165..a4abab043a 100644
--- a/backends/igvm.h
+++ b/backends/igvm.h
@@ -17,6 +17,6 @@
#include "qapi/error.h"
int qigvm_process_file(IgvmCfg *igvm, ConfidentialGuestSupport *cgs,
- Error **errp);
+ bool onlyVpContext, Error **errp);
#endif
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 3184ea1b37..a3285fbc64 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -371,7 +371,7 @@ static void pc_init1(MachineState *machine, const char *pci_type)
/* Apply guest state from IGVM if supplied */
if (x86ms->igvm) {
if (IGVM_CFG_GET_CLASS(x86ms->igvm)
- ->process(x86ms->igvm, machine->cgs, &error_fatal) < 0) {
+ ->process(x86ms->igvm, machine->cgs, false, &error_fatal) < 0) {
g_assert_not_reached();
}
}
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 6990e1c669..cf871cfdad 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -330,7 +330,7 @@ static void pc_q35_init(MachineState *machine)
/* Apply guest state from IGVM if supplied */
if (x86ms->igvm) {
if (IGVM_CFG_GET_CLASS(x86ms->igvm)
- ->process(x86ms->igvm, machine->cgs, &error_fatal) < 0) {
+ ->process(x86ms->igvm, machine->cgs, false, &error_fatal) < 0) {
g_assert_not_reached();
}
}
diff --git a/include/system/igvm-cfg.h b/include/system/igvm-cfg.h
index 321b3196f0..944f23a814 100644
--- a/include/system/igvm-cfg.h
+++ b/include/system/igvm-cfg.h
@@ -31,11 +31,14 @@ typedef struct IgvmCfgClass {
/*
* If an IGVM filename has been specified then process the IGVM file.
* Performs a no-op if no filename has been specified.
+ * If onlyVpContext is true then only the IGVM_VHT_VP_CONTEXT entries
+ * in the IGVM file will be processed, allowing information about the
+ * CPU state to be determined before processing the entire file.
*
* Returns 0 for ok and -1 on error.
*/
int (*process)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
- Error **errp);
+ bool onlyVpContext, Error **errp);
} IgvmCfgClass;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 2798fe1c38..df55a25a67 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -118,6 +118,8 @@ struct SevCommonState {
uint32_t cbitpos;
uint32_t reduced_phys_bits;
bool kernel_hashes;
+ uint64_t sev_features;
+ uint64_t supported_sev_features;
/* runtime state */
uint8_t api_major;
@@ -485,7 +487,40 @@ static void sev_apply_cpu_context(CPUState *cpu)
}
}
-static int check_vmsa_supported(hwaddr gpa, const struct sev_es_save_area *vmsa,
+static int check_sev_features(SevCommonState *sev_common, uint64_t sev_features,
+ Error **errp)
+{
+ /*
+ * Ensure SEV_FEATURES is configured for correct SEV hardware and that
+ * the requested features are supported. If SEV-SNP is enabled then
+ * that feature must be enabled, otherwise it must be cleared.
+ */
+ if (sev_snp_enabled() && !(sev_features & SVM_SEV_FEAT_SNP_ACTIVE)) {
+ error_setg(
+ errp,
+ "%s: SEV_SNP is enabled but is not enabled in VMSA sev_features",
+ __func__);
+ return -1;
+ } else if (!sev_snp_enabled() &&
+ (sev_features & SVM_SEV_FEAT_SNP_ACTIVE)) {
+ error_setg(
+ errp,
+ "%s: SEV_SNP is not enabled but is enabled in VMSA sev_features",
+ __func__);
+ return -1;
+ }
+ if (sev_features & ~sev_common->supported_sev_features) {
+ error_setg(errp,
+ "%s: VMSA contains unsupported sev_features: %lX, "
+ "supported features: %lX",
+ __func__, sev_features, sev_common->supported_sev_features);
+ return -1;
+ }
+ return 0;
+}
+
+static int check_vmsa_supported(SevCommonState *sev_common, hwaddr gpa,
+ const struct sev_es_save_area *vmsa,
Error **errp)
{
struct sev_es_save_area vmsa_check;
@@ -551,24 +586,10 @@ static int check_vmsa_supported(hwaddr gpa, const struct sev_es_save_area *vmsa,
vmsa_check.x87_fcw = 0;
vmsa_check.mxcsr = 0;
- if (sev_snp_enabled()) {
- if (vmsa_check.sev_features != SVM_SEV_FEAT_SNP_ACTIVE) {
- error_setg(errp,
- "%s: sev_features in the VMSA contains an unsupported "
- "value. For SEV-SNP, sev_features must be set to %x.",
- __func__, SVM_SEV_FEAT_SNP_ACTIVE);
- return -1;
- }
- vmsa_check.sev_features = 0;
- } else {
- if (vmsa_check.sev_features != 0) {
- error_setg(errp,
- "%s: sev_features in the VMSA contains an unsupported "
- "value. For SEV-ES and SEV, sev_features must be "
- "set to 0.", __func__);
- return -1;
- }
+ if (check_sev_features(sev_common, vmsa_check.sev_features, errp) < 0) {
+ return -1;
}
+ vmsa_check.sev_features = 0;
if (!buffer_is_zero(&vmsa_check, sizeof(vmsa_check))) {
error_setg(errp,
@@ -1722,6 +1743,39 @@ static int sev_snp_kvm_type(X86ConfidentialGuest *cg)
return KVM_X86_SNP_VM;
}
+static int sev_init_supported_features(ConfidentialGuestSupport *cgs,
+ SevCommonState *sev_common, Error **errp)
+{
+ X86ConfidentialGuestClass *x86_klass =
+ X86_CONFIDENTIAL_GUEST_GET_CLASS(cgs);
+ /*
+ * Older kernels do not support query or setting of sev_features. In this
+ * case the set of supported features must be zero to match the settings
+ * in the kernel.
+ */
+ if (x86_klass->kvm_type(X86_CONFIDENTIAL_GUEST(sev_common)) ==
+ KVM_X86_DEFAULT_VM) {
+ sev_common->supported_sev_features = 0;
+ return 0;
+ }
+
+ /* Query KVM for the supported set of sev_features */
+ struct kvm_device_attr attr = {
+ .group = KVM_X86_GRP_SEV,
+ .attr = KVM_X86_SEV_VMSA_FEATURES,
+ .addr = (unsigned long)&sev_common->supported_sev_features,
+ };
+ if (kvm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr) < 0) {
+ error_setg(errp, "%s: failed to query supported sev_features",
+ __func__);
+ return -1;
+ }
+ if (sev_snp_enabled()) {
+ sev_common->supported_sev_features |= SVM_SEV_FEAT_SNP_ACTIVE;
+ }
+ return 0;
+}
+
static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
{
char *devname;
@@ -1802,6 +1856,10 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
}
}
+ if (sev_init_supported_features(cgs, sev_common, errp) < 0) {
+ return -1;
+ }
+
trace_kvm_sev_init();
switch (x86_klass->kvm_type(X86_CONFIDENTIAL_GUEST(sev_common))) {
case KVM_X86_DEFAULT_VM:
@@ -1813,6 +1871,40 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
case KVM_X86_SEV_ES_VM:
case KVM_X86_SNP_VM: {
struct kvm_sev_init args = { 0 };
+ MachineState *machine = MACHINE(qdev_get_machine());
+ X86MachineState *x86machine = X86_MACHINE(qdev_get_machine());
+
+ /*
+ * If configuration is provided via an IGVM file then the IGVM file
+ * might contain configuration of the initial vcpu context. For SEV
+ * the vcpu context includes the sev_features which should be applied
+ * to the vcpu.
+ *
+ * KVM does not synchronize sev_features from CPU state. Instead it
+ * requires sev_features to be provided as part of this initialization
+ * call which is subsequently automatically applied to the VMSA of
+ * each vcpu.
+ *
+ * The IGVM file is normally processed after initialization. Therefore
+ * we need to pre-process it here to extract sev_features in order to
+ * provide it to KVM_SEV_INIT2. Each cgs_* function that is called by
+ * the IGVM processor detects this pre-process by observing the state
+ * as SEV_STATE_UNINIT.
+ */
+ if (x86machine->igvm) {
+ if (IGVM_CFG_GET_CLASS(x86machine->igvm)
+ ->process(x86machine->igvm, machine->cgs, true, errp) ==
+ -1) {
+ return -1;
+ }
+ /*
+ * KVM maintains a bitmask of allowed sev_features. This does not
+ * include SVM_SEV_FEAT_SNP_ACTIVE which is set accordingly by KVM
+ * itself. Therefore we need to clear this flag.
+ */
+ args.vmsa_features = sev_common->sev_features &
+ ~SVM_SEV_FEAT_SNP_ACTIVE;
+ }
ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_INIT2, &args, &fw_error);
break;
@@ -2417,6 +2509,24 @@ static int cgs_set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
SevCommonStateClass *klass = SEV_COMMON_GET_CLASS(sev_common);
+ if (sev_common->state == SEV_STATE_UNINIT) {
+ /* Pre-processing of IGVM file called from sev_common_kvm_init() */
+ if ((cpu_index == 0) && (memory_type == CGS_PAGE_TYPE_VMSA)) {
+ const struct sev_es_save_area *sa =
+ (const struct sev_es_save_area *)ptr;
+ if (len < sizeof(*sa)) {
+ error_setg(errp, "%s: invalid VMSA length encountered",
+ __func__);
+ return -1;
+ }
+ if (check_sev_features(sev_common, sa->sev_features, errp) < 0) {
+ return -1;
+ }
+ sev_common->sev_features = sa->sev_features;
+ }
+ return 0;
+ }
+
if (!sev_enabled()) {
error_setg(errp, "%s: attempt to configure guest memory, but SEV "
"is not enabled", __func__);
@@ -2436,7 +2546,8 @@ static int cgs_set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
__func__);
return -1;
}
- if (check_vmsa_supported(gpa, (const struct sev_es_save_area *)ptr,
+ if (check_vmsa_supported(sev_common, gpa,
+ (const struct sev_es_save_area *)ptr,
errp) < 0) {
return -1;
}
@@ -2493,6 +2604,12 @@ static int cgs_get_mem_map_entry(int index,
struct e820_entry *table;
int num_entries;
+ SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
+ if (sev_common->state == SEV_STATE_UNINIT) {
+ /* Pre-processing of IGVM file called from sev_common_kvm_init() */
+ return 1;
+ }
+
num_entries = e820_get_table(&table);
if ((index < 0) || (index >= num_entries)) {
return 1;
@@ -2524,6 +2641,12 @@ static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
uint32_t policy_data1_size, void *policy_data2,
uint32_t policy_data2_size, Error **errp)
{
+ SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
+ if (sev_common->state == SEV_STATE_UNINIT) {
+ /* Pre-processing of IGVM file called from sev_common_kvm_init() */
+ return 0;
+ }
+
if (policy_type != GUEST_POLICY_SEV) {
error_setg(errp, "%s: Invalid guest policy type provided for SEV: %d",
__func__, policy_type);
--
2.43.0
On 13/06/2025 16:32, Roy Hopkins wrote:
> IGVM files can contain an initial VMSA that should be applied to each
> vcpu as part of the initial guest state. The sev_features flags are
> provided as part of the VMSA structure. However, KVM only allows
> sev_features to be set during initialization and not as the guest is
> being prepared for launch.
>
> This patch queries KVM for the supported set of sev_features flags and
> processes the VP context entries in the IGVM file during kvm_init to
> determine any sev_features flags set in the IGVM file. These are then
> provided in the call to KVM_SEV_INIT2 to ensure the guest state
> matches that specified in the IGVM file.
>
> The igvm process() function is modified to allow a partial processing
> of the file during initialization, with on the IGVM_VHT_VP_CONTEXT
s/with on/with only/
> fields being processed. This means the function is called twice,
> firstly to extract the sev_features then secondly to actually
> configure the guest.
>
> Signed-off-by: Roy Hopkins <roy.hopkins@randomman.co.uk>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Stefano Garzarella <sgarzare@redhat.com>
> Acked-by: Gerd Hoffman <kraxel@redhat.com>
> Tested-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Liam Merwick <liam.merwick@oracle.com>
> ---
> backends/igvm.c | 17 +++-
> backends/igvm.h | 2 +-
> hw/i386/pc_piix.c | 2 +-
> hw/i386/pc_q35.c | 2 +-
> include/system/igvm-cfg.h | 5 +-
> target/i386/sev.c | 161 +++++++++++++++++++++++++++++++++-----
> 6 files changed, 163 insertions(+), 26 deletions(-)
>
> diff --git a/backends/igvm.c b/backends/igvm.c
> index 034f504716..9853ba941e 100644
> --- a/backends/igvm.c
> +++ b/backends/igvm.c
> @@ -869,7 +869,7 @@ static IgvmHandle qigvm_file_init(char *filename, Error **errp)
> }
>
> int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
> - Error **errp)
> + bool onlyVpContext, Error **errp)
> {
> int32_t header_count;
> QIgvmParameterData *parameter;
> @@ -913,11 +913,22 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
> ctx.current_header_index++) {
> IgvmVariableHeaderType type = igvm_get_header_type(
> ctx.file, IGVM_HEADER_SECTION_DIRECTIVE, ctx.current_header_index);
> - if (qigvm_handler(&ctx, type, errp) < 0) {
> - goto cleanup_parameters;
> + if (!onlyVpContext || (type == IGVM_VHT_VP_CONTEXT)) {
> + if (qigvm_handler(&ctx, type, errp) < 0) {
> + goto cleanup_parameters;
> + }
> }
> }
>
> + /*
> + * If only processing the VP context then we don't need to process
> + * any more of the file.
> + */
> + if (onlyVpContext) {
> + retval = 0;
> + goto cleanup_parameters;
> + }
> +
> header_count =
> igvm_header_count(ctx.file, IGVM_HEADER_SECTION_INITIALIZATION);
> if (header_count < 0) {
> diff --git a/backends/igvm.h b/backends/igvm.h
> index db02ea9165..a4abab043a 100644
> --- a/backends/igvm.h
> +++ b/backends/igvm.h
> @@ -17,6 +17,6 @@
> #include "qapi/error.h"
>
> int qigvm_process_file(IgvmCfg *igvm, ConfidentialGuestSupport *cgs,
> - Error **errp);
> + bool onlyVpContext, Error **errp);
>
> #endif
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 3184ea1b37..a3285fbc64 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -371,7 +371,7 @@ static void pc_init1(MachineState *machine, const char *pci_type)
> /* Apply guest state from IGVM if supplied */
> if (x86ms->igvm) {
> if (IGVM_CFG_GET_CLASS(x86ms->igvm)
> - ->process(x86ms->igvm, machine->cgs, &error_fatal) < 0) {
> + ->process(x86ms->igvm, machine->cgs, false, &error_fatal) < 0) {
> g_assert_not_reached();
> }
> }
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 6990e1c669..cf871cfdad 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -330,7 +330,7 @@ static void pc_q35_init(MachineState *machine)
> /* Apply guest state from IGVM if supplied */
> if (x86ms->igvm) {
> if (IGVM_CFG_GET_CLASS(x86ms->igvm)
> - ->process(x86ms->igvm, machine->cgs, &error_fatal) < 0) {
> + ->process(x86ms->igvm, machine->cgs, false, &error_fatal) < 0) {
> g_assert_not_reached();
> }
> }
> diff --git a/include/system/igvm-cfg.h b/include/system/igvm-cfg.h
> index 321b3196f0..944f23a814 100644
> --- a/include/system/igvm-cfg.h
> +++ b/include/system/igvm-cfg.h
> @@ -31,11 +31,14 @@ typedef struct IgvmCfgClass {
> /*
> * If an IGVM filename has been specified then process the IGVM file.
> * Performs a no-op if no filename has been specified.
> + * If onlyVpContext is true then only the IGVM_VHT_VP_CONTEXT entries
> + * in the IGVM file will be processed, allowing information about the
> + * CPU state to be determined before processing the entire file.
> *
> * Returns 0 for ok and -1 on error.
> */
> int (*process)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
> - Error **errp);
> + bool onlyVpContext, Error **errp);
>
> } IgvmCfgClass;
>
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 2798fe1c38..df55a25a67 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -118,6 +118,8 @@ struct SevCommonState {
> uint32_t cbitpos;
> uint32_t reduced_phys_bits;
> bool kernel_hashes;
> + uint64_t sev_features;
> + uint64_t supported_sev_features;
>
> /* runtime state */
> uint8_t api_major;
> @@ -485,7 +487,40 @@ static void sev_apply_cpu_context(CPUState *cpu)
> }
> }
>
> -static int check_vmsa_supported(hwaddr gpa, const struct sev_es_save_area *vmsa,
> +static int check_sev_features(SevCommonState *sev_common, uint64_t sev_features,
> + Error **errp)
> +{
> + /*
> + * Ensure SEV_FEATURES is configured for correct SEV hardware and that
> + * the requested features are supported. If SEV-SNP is enabled then
> + * that feature must be enabled, otherwise it must be cleared.
> + */
> + if (sev_snp_enabled() && !(sev_features & SVM_SEV_FEAT_SNP_ACTIVE)) {
> + error_setg(
> + errp,
> + "%s: SEV_SNP is enabled but is not enabled in VMSA sev_features",
> + __func__);
> + return -1;
> + } else if (!sev_snp_enabled() &&
> + (sev_features & SVM_SEV_FEAT_SNP_ACTIVE)) {
> + error_setg(
> + errp,
> + "%s: SEV_SNP is not enabled but is enabled in VMSA sev_features",
> + __func__);
> + return -1;
> + }
> + if (sev_features & ~sev_common->supported_sev_features) {
> + error_setg(errp,
> + "%s: VMSA contains unsupported sev_features: %lX, "
> + "supported features: %lX",
> + __func__, sev_features, sev_common->supported_sev_features);
> + return -1;
> + }
> + return 0;
> +}
> +
> +static int check_vmsa_supported(SevCommonState *sev_common, hwaddr gpa,
> + const struct sev_es_save_area *vmsa,
> Error **errp)
> {
> struct sev_es_save_area vmsa_check;
> @@ -551,24 +586,10 @@ static int check_vmsa_supported(hwaddr gpa, const struct sev_es_save_area *vmsa,
> vmsa_check.x87_fcw = 0;
> vmsa_check.mxcsr = 0;
>
> - if (sev_snp_enabled()) {
> - if (vmsa_check.sev_features != SVM_SEV_FEAT_SNP_ACTIVE) {
> - error_setg(errp,
> - "%s: sev_features in the VMSA contains an unsupported "
> - "value. For SEV-SNP, sev_features must be set to %x.",
> - __func__, SVM_SEV_FEAT_SNP_ACTIVE);
> - return -1;
> - }
> - vmsa_check.sev_features = 0;
> - } else {
> - if (vmsa_check.sev_features != 0) {
> - error_setg(errp,
> - "%s: sev_features in the VMSA contains an unsupported "
> - "value. For SEV-ES and SEV, sev_features must be "
> - "set to 0.", __func__);
> - return -1;
> - }
> + if (check_sev_features(sev_common, vmsa_check.sev_features, errp) < 0) {
> + return -1;
> }
> + vmsa_check.sev_features = 0;
>
> if (!buffer_is_zero(&vmsa_check, sizeof(vmsa_check))) {
> error_setg(errp,
> @@ -1722,6 +1743,39 @@ static int sev_snp_kvm_type(X86ConfidentialGuest *cg)
> return KVM_X86_SNP_VM;
> }
>
> +static int sev_init_supported_features(ConfidentialGuestSupport *cgs,
> + SevCommonState *sev_common, Error **errp)
> +{
> + X86ConfidentialGuestClass *x86_klass =
> + X86_CONFIDENTIAL_GUEST_GET_CLASS(cgs);
> + /*
> + * Older kernels do not support query or setting of sev_features. In this
> + * case the set of supported features must be zero to match the settings
> + * in the kernel.
> + */
> + if (x86_klass->kvm_type(X86_CONFIDENTIAL_GUEST(sev_common)) ==
> + KVM_X86_DEFAULT_VM) {
> + sev_common->supported_sev_features = 0;
> + return 0;
> + }
> +
> + /* Query KVM for the supported set of sev_features */
> + struct kvm_device_attr attr = {
> + .group = KVM_X86_GRP_SEV,
> + .attr = KVM_X86_SEV_VMSA_FEATURES,
> + .addr = (unsigned long)&sev_common->supported_sev_features,
> + };
> + if (kvm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr) < 0) {
> + error_setg(errp, "%s: failed to query supported sev_features",
> + __func__);
> + return -1;
> + }
> + if (sev_snp_enabled()) {
> + sev_common->supported_sev_features |= SVM_SEV_FEAT_SNP_ACTIVE;
> + }
> + return 0;
> +}
> +
> static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
> {
> char *devname;
> @@ -1802,6 +1856,10 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
> }
> }
>
> + if (sev_init_supported_features(cgs, sev_common, errp) < 0) {
> + return -1;
> + }
> +
> trace_kvm_sev_init();
> switch (x86_klass->kvm_type(X86_CONFIDENTIAL_GUEST(sev_common))) {
> case KVM_X86_DEFAULT_VM:
> @@ -1813,6 +1871,40 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
> case KVM_X86_SEV_ES_VM:
> case KVM_X86_SNP_VM: {
> struct kvm_sev_init args = { 0 };
> + MachineState *machine = MACHINE(qdev_get_machine());
> + X86MachineState *x86machine = X86_MACHINE(qdev_get_machine());
> +
> + /*
> + * If configuration is provided via an IGVM file then the IGVM file
> + * might contain configuration of the initial vcpu context. For SEV
> + * the vcpu context includes the sev_features which should be applied
> + * to the vcpu.
> + *
> + * KVM does not synchronize sev_features from CPU state. Instead it
> + * requires sev_features to be provided as part of this initialization
> + * call which is subsequently automatically applied to the VMSA of
> + * each vcpu.
> + *
> + * The IGVM file is normally processed after initialization. Therefore
> + * we need to pre-process it here to extract sev_features in order to
> + * provide it to KVM_SEV_INIT2. Each cgs_* function that is called by
> + * the IGVM processor detects this pre-process by observing the state
> + * as SEV_STATE_UNINIT.
> + */
> + if (x86machine->igvm) {
> + if (IGVM_CFG_GET_CLASS(x86machine->igvm)
> + ->process(x86machine->igvm, machine->cgs, true, errp) ==
> + -1) {
> + return -1;
> + }
> + /*
> + * KVM maintains a bitmask of allowed sev_features. This does not
> + * include SVM_SEV_FEAT_SNP_ACTIVE which is set accordingly by KVM
> + * itself. Therefore we need to clear this flag.
> + */
> + args.vmsa_features = sev_common->sev_features &
> + ~SVM_SEV_FEAT_SNP_ACTIVE;
> + }
>
> ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_INIT2, &args, &fw_error);
> break;
> @@ -2417,6 +2509,24 @@ static int cgs_set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
> SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
> SevCommonStateClass *klass = SEV_COMMON_GET_CLASS(sev_common);
>
> + if (sev_common->state == SEV_STATE_UNINIT) {
> + /* Pre-processing of IGVM file called from sev_common_kvm_init() */
> + if ((cpu_index == 0) && (memory_type == CGS_PAGE_TYPE_VMSA)) {
> + const struct sev_es_save_area *sa =
> + (const struct sev_es_save_area *)ptr;
> + if (len < sizeof(*sa)) {
> + error_setg(errp, "%s: invalid VMSA length encountered",
> + __func__);
> + return -1;
> + }
> + if (check_sev_features(sev_common, sa->sev_features, errp) < 0) {
> + return -1;
> + }
> + sev_common->sev_features = sa->sev_features;
> + }
> + return 0;
> + }
> +
> if (!sev_enabled()) {
> error_setg(errp, "%s: attempt to configure guest memory, but SEV "
> "is not enabled", __func__);
> @@ -2436,7 +2546,8 @@ static int cgs_set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
> __func__);
> return -1;
> }
> - if (check_vmsa_supported(gpa, (const struct sev_es_save_area *)ptr,
> + if (check_vmsa_supported(sev_common, gpa,
> + (const struct sev_es_save_area *)ptr,
> errp) < 0) {
> return -1;
> }
> @@ -2493,6 +2604,12 @@ static int cgs_get_mem_map_entry(int index,
> struct e820_entry *table;
> int num_entries;
>
> + SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
> + if (sev_common->state == SEV_STATE_UNINIT) {
> + /* Pre-processing of IGVM file called from sev_common_kvm_init() */
> + return 1;
> + }
> +
> num_entries = e820_get_table(&table);
> if ((index < 0) || (index >= num_entries)) {
> return 1;
> @@ -2524,6 +2641,12 @@ static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
> uint32_t policy_data1_size, void *policy_data2,
> uint32_t policy_data2_size, Error **errp)
> {
> + SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
> + if (sev_common->state == SEV_STATE_UNINIT) {
> + /* Pre-processing of IGVM file called from sev_common_kvm_init() */
> + return 0;
> + }
> +
> if (policy_type != GUEST_POLICY_SEV) {
> error_setg(errp, "%s: Invalid guest policy type provided for SEV: %d",
> __func__, policy_type);
> On 13/06/2025 16:22, Roy Hopkins wrote:
> > Adds a handler for the guest policy initialization IGVM section and
> > builds an SEV policy based on this information and the ID block
> > directive if present. The policy is applied using by calling
> > 'set_guest_policy()' on the ConfidentialGuestSupport object.
> >
> > Signed-off-by: Roy Hopkins <roy.hopkins@randomman.co.uk>
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > Acked-by: Stefano Garzarella <sgarzare@redhat.com>
> > Acked-by: Gerd Hoffman <kraxel@redhat.com>
> > ---
> > backends/igvm.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 138 insertions(+)
> >
> > diff --git a/backends/igvm.c b/backends/igvm.c
> > index ebdb4594d1..034f504716 100644
> > --- a/backends/igvm.c
> > +++ b/backends/igvm.c
> > @@ -27,6 +27,33 @@ typedef struct QIgvmParameterData {
> > uint32_t index;
> > } QIgvmParameterData;
> >
> > +/*
> > + * Some directives are specific to particular confidential computing platforms.
> > + * Define required types for each of those platforms here.
> > + */
> > +
> > +/* SEV/SEV-ES/SEV-SNP */
>
>
> Could mention that the following are in Chapter 8 of
> "SEV Secure Nested Paging Firmware ABI Specification"
>
Good suggestion. I'll add it.
>
> > +struct QEMU_PACKED sev_id_block {
> > + uint8_t ld[48];
> > + uint8_t family_id[16];
> > + uint8_t image_id[16];
> > + uint32_t version;
> > + uint32_t guest_svn;
> > + uint64_t policy;
> > +};
> > +
> > +struct QEMU_PACKED sev_id_authentication {
> > + uint32_t id_key_alg;
> > + uint32_t auth_key_algo;
> > + uint8_t reserved[56];
> > + uint8_t id_block_sig[512];
> > + uint8_t id_key[1028];
> > + uint8_t reserved2[60];
> > + uint8_t id_key_sig[512];
> > + uint8_t author_key[1028];
> > + uint8_t reserved3[892];
> > +};
> > +
>
> In the next patch (#15), sev_snp_id_authentication is in target/i386/sev.h
> Should they be all together? (and sev_snp_id_authentication seems
> identical to sev_id_authentication - do we we need both structs?)
>
That does make sense and I hoped to do that, but the two files should not really
depend on each other in their current locations. Perhaps we can look at moving
the SEV definitions into a common location in a future patch?
> > /*
> > * QIgvm contains the information required during processing
> > * of a single IGVM file.
> > @@ -38,6 +65,17 @@ typedef struct QIgvm {
> > uint32_t compatibility_mask;
> > unsigned current_header_index;
> > QTAILQ_HEAD(, QIgvmParameterData) parameter_data;
> > + IgvmPlatformType platform_type;
> > +
> > + /*
> > + * SEV-SNP platforms can contain an ID block and authentication
> > + * that should be verified by the guest.
> > + */
> > + struct sev_id_block *id_block;
> > + struct sev_id_authentication *id_auth;
> > +
> > + /* Define the guest policy for SEV guests */
> > + uint64_t sev_policy;
> >
> > /* These variables keep track of contiguous page regions */
> > IGVM_VHS_PAGE_DATA region_prev_page_data;
> > @@ -67,6 +105,11 @@ static int qigvm_directive_environment_info(QIgvm *ctx,
> > static int qigvm_directive_required_memory(QIgvm *ctx,
> > const uint8_t *header_data,
> > Error **errp);
> > +static int qigvm_directive_snp_id_block(QIgvm *ctx, const uint8_t *header_data,
> > + Error **errp);
> > +static int qigvm_initialization_guest_policy(QIgvm *ctx,
> > + const uint8_t *header_data,
> > + Error **errp);
> >
> > struct QIGVMHandler {
> > uint32_t type;
> > @@ -91,6 +134,10 @@ static struct QIGVMHandler handlers[] = {
> > qigvm_directive_environment_info },
> > { IGVM_VHT_REQUIRED_MEMORY, IGVM_HEADER_SECTION_DIRECTIVE,
> > qigvm_directive_required_memory },
> > + { IGVM_VHT_SNP_ID_BLOCK, IGVM_HEADER_SECTION_DIRECTIVE,
> > + qigvm_directive_snp_id_block },
> > + { IGVM_VHT_GUEST_POLICY, IGVM_HEADER_SECTION_INITIALIZATION,
> > + qigvm_initialization_guest_policy },
> > };
> >
> > static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp)
> > @@ -632,6 +679,70 @@ static int qigvm_directive_required_memory(QIgvm *ctx,
> > return 0;
> > }
> >
> > +static int qigvm_directive_snp_id_block(QIgvm *ctx, const uint8_t *header_data,
> > + Error **errp)
> > +{
> > + const IGVM_VHS_SNP_ID_BLOCK *igvm_id =
> > + (const IGVM_VHS_SNP_ID_BLOCK *)header_data;
> > +
> > + if (!(igvm_id->compatibility_mask & ctx->compatibility_mask)) {
> > + return 0;
> > + }
> > +
> > + if (ctx->id_block) {
> > + error_setg(errp, "IGVM: Multiple ID blocks encountered "
> > + "in IGVM file.");
>
> Error string should be on one line to make it easier to search/grep for.
This, and the other formatting comments are this way due to the formatting
requirements in the project. checkpatch.pl fails with an error if I try
to stretch these lines out.
>
> > + return -1;
> > + }
> > + ctx->id_block = g_new0(struct sev_id_block, 1);
> > + ctx->id_auth = g_new0(struct sev_id_authentication, 1);
> > +
> > + memcpy(ctx->id_block->family_id, igvm_id->family_id,
> > + sizeof(ctx->id_block->family_id));
> > + memcpy(ctx->id_block->image_id, igvm_id->image_id,
> > + sizeof(ctx->id_block->image_id));
> > + ctx->id_block->guest_svn = igvm_id->guest_svn;
> > + ctx->id_block->version = 1;
>
> Worth a #define for version?
>
Yes. Done.
> > + memcpy(ctx->id_block->ld, igvm_id->ld, sizeof(ctx->id_block->ld));
> > +
> > + ctx->id_auth->id_key_alg = igvm_id->id_key_algorithm;
> > + memcpy(ctx->id_auth->id_block_sig, &igvm_id->id_key_signature,
> > + sizeof(igvm_id->id_key_signature));
> > +
>
> Should the sizeof be ctx->id_auth->id_block_sig (the dest) ?
>
> > + ctx->id_auth->auth_key_algo = igvm_id->author_key_algorithm;
> > + memcpy(ctx->id_auth->id_key_sig, &igvm_id->author_key_signature,
> > + sizeof(igvm_id->author_key_signature));
> > +
>
> Should the sizeof be ctx->id_auth->id_key_sig ?
>
Unfortunately the two structures are different sizes. I've added an assert
for this and the other one below to catch any future errors.
> > + /*
> > + * SEV and IGVM public key structure population are slightly different.
> > + * See SEV Secure Nested Paging Firmware ABI Specification, Chapter 10.
> > + */
> > + *((uint32_t *)ctx->id_auth->id_key) = igvm_id->id_public_key.curve;
> > + memcpy(&ctx->id_auth->id_key[4], &igvm_id->id_public_key.qx, 72);
> > + memcpy(&ctx->id_auth->id_key[76], &igvm_id->id_public_key.qy, 72);
> > +
> > + *((uint32_t *)ctx->id_auth->author_key) =
> > + igvm_id->author_public_key.curve;
> > + memcpy(&ctx->id_auth->author_key[4], &igvm_id->author_public_key.qx,
> > + 72);
> > + memcpy(&ctx->id_auth->author_key[76], &igvm_id->author_public_key.qy,
> > + 72);
> > +
>
> For both memcpy calls, they could fit on one line and be more readable.
>
> > + return 0;
> > +}
> > +
> > +static int qigvm_initialization_guest_policy(QIgvm *ctx,
> > + const uint8_t *header_data, Error **errp)
> > +{
> > + const IGVM_VHS_GUEST_POLICY *guest =
> > + (const IGVM_VHS_GUEST_POLICY *)header_data;
> > +
> > + if (guest->compatibility_mask & ctx->compatibility_mask) {
> > + ctx->sev_policy = guest->policy;
> > + }
> > + return 0;
> > +}
> > +
> > static int qigvm_supported_platform_compat_mask(QIgvm *ctx, Error **errp)
> > {
> > int32_t header_count;
> > @@ -701,12 +812,16 @@ static int qigvm_supported_platform_compat_mask(QIgvm *ctx, Error **errp)
> > /* Choose the strongest supported isolation technology */
> > if (compatibility_mask_sev_snp != 0) {
> > ctx->compatibility_mask = compatibility_mask_sev_snp;
> > + ctx->platform_type = IGVM_PLATFORM_TYPE_SEV_SNP;
> > } else if (compatibility_mask_sev_es != 0) {
> > ctx->compatibility_mask = compatibility_mask_sev_es;
> > + ctx->platform_type = IGVM_PLATFORM_TYPE_SEV_ES;
> > } else if (compatibility_mask_sev != 0) {
> > ctx->compatibility_mask = compatibility_mask_sev;
> > + ctx->platform_type = IGVM_PLATFORM_TYPE_SEV;
> > } else if (compatibility_mask != 0) {
> > ctx->compatibility_mask = compatibility_mask;
> > + ctx->platform_type = IGVM_PLATFORM_TYPE_NATIVE;
> > } else {
> > error_setg(
> > errp,
> > @@ -716,6 +831,23 @@ static int qigvm_supported_platform_compat_mask(QIgvm *ctx, Error **errp)
> > return 0;
> > }
> >
> > +static int qigvm_handle_policy(QIgvm *ctx, Error **errp)
> > +{
> > + if (ctx->platform_type == IGVM_PLATFORM_TYPE_SEV_SNP) {
> > + int id_block_len = 0;
> > + int id_auth_len = 0;
> > + if (ctx->id_block) {
> > + ctx->id_block->policy = ctx->sev_policy;
> > + id_block_len = sizeof(struct sev_id_block);
> > + id_auth_len = sizeof(struct sev_id_authentication);
> > + }
> > + return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV, ctx->sev_policy,
> > + ctx->id_block, id_block_len,
> > + ctx->id_auth, id_auth_len, errp);
> > + }
> > + return 0;
> > +}
> > +
> > static IgvmHandle qigvm_file_init(char *filename, Error **errp)
> > {
> > IgvmHandle igvm;
> > @@ -814,12 +946,18 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
> > */
> > retval = qigvm_process_mem_page(&ctx, NULL, errp);
> >
> > + if (retval == 0) {
> > + retval = qigvm_handle_policy(&ctx, errp);
> > + }
> > +
> > cleanup_parameters:
> > QTAILQ_FOREACH(parameter, &ctx.parameter_data, next)
> > {
> > g_free(parameter->data);
> > parameter->data = NULL;
> > }
> > + g_free(ctx.id_block);
> > + g_free(ctx.id_auth);
> >
> > cleanup:
> > igvm_free(ctx.file);
© 2016 - 2025 Red Hat, Inc.