> On 3 Sep 2026, at 4:32 PM, Luigi Leonardi <[email protected]> wrote:
>
> On Thu, Sep 03, 2026 at 03:33:30PM +0530, Ani Sinha wrote:
>>
>>
>>> On 3 Sep 2026, at 2:31 PM, Luigi Leonardi <[email protected]> wrote:
>>>
>>> Hi Ani,
>>>
>>> On Thu, Sep 03, 2026 at 02:16:30PM +0530, Ani Sinha wrote:
>>>>
>>>>
>>>>> On 1 Sep 2026, at 3:39 PM, Luigi Leonardi <[email protected]> wrote:
>>>>>
>>>>> The guest policy carried in the IGVM guest-policy initialization header
>>>>> was parsed into QIgvm but never forwarded to the confidential guest
>>>>> platform: the previous callback ran at the end of qigvm_process_file,
>>>>> after LAUNCH_START had already been issued, so writing the policy had
>>>>> no effect.
>>>>>
>>>>> Add a set_guest_policy callback and invoke it from the guest-policy
>>>>> initialization handler, so the policy reaches the platform before
>>>>> LAUNCH_START.
>>>>>
>>>>> The guest policy can also be set on the command line. As it is part of
>>>>> the attestation report, silently overriding it would cause attestation
>>>>> to fail, so return an error if the command-line value differs from the
>>>>> one supplied by the IGVM file.
>>>>>
>>>>> 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/confidential-guest-support.c | 9 ++++++++
>>>>> backends/igvm.c | 4 ++++
>>>>> target/i386/sev.c | 39
>>>>> +++++++++++++++++++++++++++++++++++
>>>>> 3 files changed, 52 insertions(+)
>>>>>
>>>>> diff --git a/backends/confidential-guest-support.c
>>>>> b/backends/confidential-guest-support.c
>>>>> index a0b36d2da5..c0d15b4a76 100644
>>>>> --- a/backends/confidential-guest-support.c
>>>>> +++ b/backends/confidential-guest-support.c
>>>>> @@ -38,6 +38,14 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr,
>>>>> uint64_t len,
>>>>> return -1;
>>>>> }
>>>>>
>>>>> +static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
>>>>> + uint64_t policy, Error **errp)
>>>>> +{
>>>>> + error_setg(errp,
>>>>> + "Setting guest policy is not supported for this
>>>>> platform");
>>>>> + return -1;
>>>>> +}
>>>>> +
>>>>> static int set_id_block(void *id_block, uint32_t id_block_size,
>>>>> void *id_auth, uint32_t id_auth_size,
>>>>> Error **errp)
>>>>> @@ -62,6 +70,7 @@ static void
>>>>> confidential_guest_support_class_init(ObjectClass *oc,
>>>>> ConfidentialGuestSupportClass *cgsc =
>>>>> CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
>>>>> cgsc->check_support = check_support;
>>>>> cgsc->set_guest_state = set_guest_state;
>>>>> + cgsc->set_guest_policy = set_guest_policy;
>>>>> cgsc->set_id_block = set_id_block;
>>>>> cgsc->get_mem_map_entry = get_mem_map_entry;
>>>>> }
>>>>> diff --git a/backends/igvm.c b/backends/igvm.c
>>>>> index 6545382546..5131ee7829 100644
>>>>> --- a/backends/igvm.c
>>>>> +++ b/backends/igvm.c
>>>>> @@ -868,6 +868,10 @@ static int qigvm_initialization_guest_policy(QIgvm
>>>>> *ctx,
>>>>>
>>>>> 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;
>>>>> }
>>>>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>>>>> index c76cdba8d2..533ea4b54e 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;
>>>>> @@ -2723,6 +2725,40 @@ static int cgs_get_mem_map_entry(int index,
>>>>> return 0;
>>>>> }
>>>>>
>>>>> +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 (policy_type != GUEST_POLICY_SEV) {
>>>>> + error_setg(errp, "SEV: Invalid guest policy type provided for
>>>>> SEV: %d",
>>>>> + policy_type);
>>>>> + return -1;
>>>>> + }
>>>>> +
>>>>> + if (sev_snp_enabled()) {
>>>>> + SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(sev_common);
>>>>> +
>>>>> + if (sev_common->policy_set &&
>>>>> + sev_snp_guest->kvm_start_conf.policy != policy) {
>>>>> + error_setg(errp, "SNP: policy mismatch between IGVM and
>>>>> CLI");
>>>>> + return -1;
>>>>> + }
>>>>> +
>>>>> + sev_snp_guest->kvm_start_conf.policy = policy;
>>>
>>> Thanks for you review!
>>>
>>>>
>>>> I had fixed a bug initially here where we need to check if the policy
>>>> passed is not 0. I am not sure if that fix is still needed here.
>>>> Please test this scenario:
>>>> a) Generate an IGVM file with policy set to 0.
>>>
>>> 0 is not a valid sev-snp guest policy: bit 17 is reserved and must be 1.
>>> It's a valid sev/sev-es policy though.
>>>
>>>> b) Start a confidential SEV-SNP guest with policy set in command line and
>>>> with this IGVM.
>>>> I think with your patch this will fail as the policies won’t match.
>>>
>>> Correct: this was suggested by Gerd, as this is something unexpected. I
>>> think it would break launch measurement.
>>
>> Yes I think the right thing to do is that if the policy is correctly set by
>> IGVM, override the one set in the cli with the one in IGVM. Otherwise ignore
>> IGVM policy.
>>
>
> mmh why? I might be missing something, but for now I'm not really convinced:
> if we have a policy set in IGVM we should use that one.
That is what I said. “Override the one set in cli with the one in IGVM”. The
exception is that if the policy in igvm is wrong (say set to 0 for sev-snp
case). In that case, ignore igvm one.
> Overriding it using the
> CLI, may cause measurement failure. If we have an IGVM image with a wrong
> policy
> set, then the problem should be fixed there.
>
> @Gerd @Stefano WDYT?
>
> Luigi
