[v1] Introduce support for confidential guest reset

[PATCH v1 15/28] i386/sev: add migration blockers only once

Ani Sinha posted 28 patches 1 month, 4 weeks ago

Diff against v2
Download mbox

Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, David Woodhouse <dwmw2@infradead.org>, Paul Durrant <paul@xen.org>, Bernhard Beschow <shentey@gmail.com>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@kernel.org>, Peter Maydell <peter.maydell@linaro.org>, Marcelo Tosatti <mtosatti@redhat.com>, Zhao Liu <zhao1.liu@intel.com>, Song Gao <gaosong@loongson.cn>, Huacai Chen <chenhuacai@kernel.org>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <arikalo@gmail.com>, Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Chinmay Rath <rathc@linux.ibm.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Thomas Huth <thuth@redhat.com>

There is a newer version of this series

Expand all Fold all

[PATCH v1 15/28] i386/sev: add migration blockers only once

Posted by Ani Sinha 1 month, 4 weeks ago

sev_launch_finish() and sev_snp_launch_finish() could be called multiple times
if the confidential guest is capable of being reset/rebooted. The migration
blockers should not be added multiple times, once per invocation. This change
makes sure that the migration blockers are added only one time and not every
time upon invocvation of launch_finish() calls.

Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
 target/i386/sev.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index fd2dada013..9a3f488b24 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -1409,6 +1409,7 @@ static void
 sev_launch_finish(SevCommonState *sev_common)
 {
     int ret, error;
+    static bool added_migration_blocker;
 
     trace_kvm_sev_launch_finish();
     ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_LAUNCH_FINISH, 0,
@@ -1421,10 +1422,13 @@ sev_launch_finish(SevCommonState *sev_common)
 
     sev_set_guest_state(sev_common, SEV_STATE_RUNNING);
 
-    /* add migration blocker */
-    error_setg(&sev_mig_blocker,
-               "SEV: Migration is not implemented");
-    migrate_add_blocker(&sev_mig_blocker, &error_fatal);
+    if (!added_migration_blocker) {
+        /* add migration blocker */
+        error_setg(&sev_mig_blocker,
+                   "SEV: Migration is not implemented");
+        migrate_add_blocker(&sev_mig_blocker, &error_fatal);
+        added_migration_blocker = true;
+    }
 }
 
 static int snp_launch_update_data(uint64_t gpa, void *hva, size_t len,
@@ -1608,6 +1612,7 @@ sev_snp_launch_finish(SevCommonState *sev_common)
 {
     int ret, error;
     Error *local_err = NULL;
+    static bool added_migration_blocker;
     OvmfSevMetadata *metadata;
     SevLaunchUpdateData *data;
     SevSnpGuestState *sev_snp = SEV_SNP_GUEST(sev_common);
@@ -1655,13 +1660,16 @@ sev_snp_launch_finish(SevCommonState *sev_common)
     kvm_mark_guest_state_protected();
     sev_set_guest_state(sev_common, SEV_STATE_RUNNING);
 
-    /* add migration blocker */
-    error_setg(&sev_mig_blocker,
-               "SEV-SNP: Migration is not implemented");
-    ret = migrate_add_blocker(&sev_mig_blocker, &local_err);
-    if (local_err) {
-        error_report_err(local_err);
-        exit(1);
+    if (!added_migration_blocker) {
+        /* add migration blocker */
+        error_setg(&sev_mig_blocker,
+                   "SEV-SNP: Migration is not implemented");
+        ret = migrate_add_blocker(&sev_mig_blocker, &local_err);
+        if (local_err) {
+            error_report_err(local_err);
+            exit(1);
+        }
+        added_migration_blocker = true;
     }
 }
 
-- 
2.42.0

Re: [PATCH v1 15/28] i386/sev: add migration blockers only once

Posted by Gerd Hoffmann 1 month, 3 weeks ago

On Fri, Dec 12, 2025 at 08:33:43PM +0530, Ani Sinha wrote:
> sev_launch_finish() and sev_snp_launch_finish() could be called multiple times
> if the confidential guest is capable of being reset/rebooted. The migration
> blockers should not be added multiple times, once per invocation. This change
> makes sure that the migration blockers are added only one time and not every
> time upon invocvation of launch_finish() calls.

> +    static bool added_migration_blocker;

> -    error_setg(&sev_mig_blocker,
> -               "SEV: Migration is not implemented");
> -    migrate_add_blocker(&sev_mig_blocker, &error_fatal);
> +    if (!added_migration_blocker) {
> +        /* add migration blocker */
> +        error_setg(&sev_mig_blocker,
> +                   "SEV: Migration is not implemented");
> +        migrate_add_blocker(&sev_mig_blocker, &error_fatal);
> +        added_migration_blocker = true;
> +    }

Maybe move this to another place which is called only once?  The
migration blocker should not be very sensitive to initialization
ordering, so I'd expect finding another place where you don't need
the added_migration_blocker tracker variable isn't too much of a
problem.

take care,
  Gerd

Re: [PATCH v1 15/28] i386/sev: add migration blockers only once

Posted by Ani Sinha 1 month, 3 weeks ago


> On 17 Dec 2025, at 5:14 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> On Fri, Dec 12, 2025 at 08:33:43PM +0530, Ani Sinha wrote:
>> sev_launch_finish() and sev_snp_launch_finish() could be called multiple times
>> if the confidential guest is capable of being reset/rebooted. The migration
>> blockers should not be added multiple times, once per invocation. This change
>> makes sure that the migration blockers are added only one time and not every
>> time upon invocvation of launch_finish() calls.
> 
>> +    static bool added_migration_blocker;
> 
>> -    error_setg(&sev_mig_blocker,
>> -               "SEV: Migration is not implemented");
>> -    migrate_add_blocker(&sev_mig_blocker, &error_fatal);
>> +    if (!added_migration_blocker) {
>> +        /* add migration blocker */
>> +        error_setg(&sev_mig_blocker,
>> +                   "SEV: Migration is not implemented");
>> +        migrate_add_blocker(&sev_mig_blocker, &error_fatal);
>> +        added_migration_blocker = true;
>> +    }
> 
> Maybe move this to another place which is called only once?  

Where do you suggest? sev_common_instance_init?

> The
> migration blocker should not be very sensitive to initialization
> ordering, so I'd expect finding another place where you don't need
> the added_migration_blocker tracker variable isn't too much of a
> problem.
> 
> take care,
>  Gerd
>