set_guest_policy was called from qigvm_handle_policy at the end of qigvm_process_file, after LAUNCH_START had already been issued for both SEV/SEV-ES and SEV-SNP. The guest was therefore launched with whatever policy was already configured (the command-line value, or the platform default if none was given) instead of the one requested by the IGVM file, quietly breaking attestation since the policy is part of the attestation report.
Move the call into qigvm_initialization_guest_policy, which runs while the initialization section is processed. Because that section is now handled during the pre-launch pass (see previous patch), the call happens before LAUNCH_START, so the policy is in effect for launch. Drop qigvm_handle_policy, whose only remaining job was that misplaced call. cgs_set_guest_policy no longer special-cases SEV_STATE_UNINIT: that guard used to skip the pre-processing pass so the policy was applied later, but forwarding it during pre-processing, before LAUNCH_START, is now precisely the point. The 'policy == 0 means unset' guard is dropped too, since the call site now only fires when the IGVM file actually provides a GUEST_POLICY header. The command line can also set a policy. It now takes precedence: if it differs from the IGVM one, print a warning and keep the command-line value instead of silently overriding it. Also reject a policy that doesn't fit in the 32-bit SEV/SEV-ES policy field instead of truncating it. Finally, use the get_guest_policy callback added by the previous patch to populate the SNP ID block's policy field. The command line sets its policy directly into the SEV/SNP guest's own struct, bypassing IGVM entirely, so ctx->sev_policy only ever reflects a GUEST_POLICY header and is 0 otherwise, causing SNP_LAUNCH_FINISH to reject the ID block whenever the file relies on a command-line policy. Reading the policy back from the platform instead always gets the value actually in effect, regardless of its source. ctx->sev_policy is now unused and removed. Link: https://gitlab.com/qemu-project/qemu/-/work_items/4189 Fixes: 915b47078d ("backends/igvm: Handle policy for SEV guests") Signed-off-by: Luigi Leonardi <[email protected]> --- backends/igvm.c | 27 +++++++++++---------------- include/system/igvm-internal.h | 3 --- target/i386/sev.c | 37 +++++++++++++++++++++++++------------ 3 files changed, 36 insertions(+), 31 deletions(-) diff --git a/backends/igvm.c b/backends/igvm.c index 521560822a..5360a2575b 100644 --- a/backends/igvm.c +++ b/backends/igvm.c @@ -778,8 +778,6 @@ static int qigvm_directive_snp_id_block(QIgvm *ctx, const uint8_t *header_data, ctx->id_block->version = IGVM_SEV_ID_BLOCK_VERSION; memcpy(ctx->id_block->ld, igvm_id->ld, sizeof(ctx->id_block->ld)); - ctx->id_block->policy = ctx->sev_policy; - ctx->id_auth->id_key_alg = igvm_id->id_key_algorithm; assert(sizeof(igvm_id->id_key_signature) <= sizeof(ctx->id_auth->id_block_sig)); @@ -808,6 +806,13 @@ static int qigvm_directive_snp_id_block(QIgvm *ctx, const uint8_t *header_data, 72); if (ctx->cgsc) { + uint64_t policy; + + if (ctx->cgsc->get_guest_policy(GUEST_POLICY_SEV, &policy, errp) < 0) { + return -1; + } + ctx->id_block->policy = policy; + return ctx->cgsc->set_id_block(ctx->id_block, sizeof(struct sev_id_block), ctx->id_auth, @@ -867,7 +872,10 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx, (const IGVM_VHS_GUEST_POLICY *)header_data; if (guest->compatibility_mask & ctx->compatibility_mask) { - ctx->sev_policy = guest->policy; + if (ctx->cgsc) { + return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV, + guest->policy, errp); + } } return 0; } @@ -968,15 +976,6 @@ 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) { - return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV, ctx->sev_policy, - errp); - } - return 0; -} - IgvmHandle qigvm_file_init(char *filename, Error **errp) { IgvmHandle igvm; @@ -1102,10 +1101,6 @@ int qigvm_process_file(IgvmCfg *cfg, MachineState *machine_state, */ 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) { diff --git a/include/system/igvm-internal.h b/include/system/igvm-internal.h index 9e9fa1d9af..041f77586a 100644 --- a/include/system/igvm-internal.h +++ b/include/system/igvm-internal.h @@ -64,9 +64,6 @@ struct QIgvm { 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; uint64_t region_start; diff --git a/target/i386/sev.c b/target/i386/sev.c index f11fdb6590..11072b00dc 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -128,6 +128,8 @@ struct SevCommonState { bool kernel_hashes; uint64_t sev_features; uint64_t supported_sev_features; + /* whether the guest policy was explicitly set on the command line */ + bool policy_set; /* runtime state */ uint8_t api_major; @@ -2729,10 +2731,6 @@ static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type, uint64_t policy, 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, "SEV: Invalid guest policy type provided for SEV: %d", @@ -2740,18 +2738,31 @@ static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type, return -1; } - /* do not reset existing policy if policy was not set in IGVM */ - if (policy == 0) { - return 0; - } - if (sev_snp_enabled()) { - SevSnpGuestState *sev_snp_guest = - SEV_SNP_GUEST(MACHINE(qdev_get_machine())->cgs); + SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(sev_common); + + if (sev_common->policy_set && + sev_snp_guest->kvm_start_conf.policy != policy) { + warn_report_once("SNP: policy mismatch between IGVM and CLI, " + "keeping the command-line policy"); + return 0; + } sev_snp_guest->kvm_start_conf.policy = policy; } else { - SevGuestState *sev_guest = SEV_GUEST(MACHINE(qdev_get_machine())->cgs); + SevGuestState *sev_guest = SEV_GUEST(sev_common); + + if (sev_common->policy_set && sev_guest->policy != policy) { + warn_report_once("SEV: policy mismatch between IGVM and CLI, " + "keeping the command-line policy"); + return 0; + } + + if (policy > UINT32_MAX) { + error_setg(errp, "SEV: policy 0x%" PRIx64 " does not fit in the " + "32-bit SEV/SEV-ES guest policy field", policy); + return -1; + } sev_guest->policy = policy; } @@ -3034,6 +3045,7 @@ sev_guest_set_policy(Object *obj, Visitor *v, const char *name, if (!visit_type_uint32(v, name, &SEV_GUEST(obj)->policy, errp)) { return; } + SEV_COMMON(obj)->policy_set = true; } static void @@ -3091,6 +3103,7 @@ sev_snp_guest_set_policy(Object *obj, Visitor *v, const char *name, errp)) { return; } + SEV_COMMON(obj)->policy_set = true; } static char * -- 2.55.0
