[v5,23/23] sev: add migration blocker

Message ID 20171206200346.116537-24-brijesh.singh@amd.com
State New
Headers show
Series
  • x86: Secure Encrypted Virtualization (AMD)
Related show

Commit Message

Brijesh Singh Dec. 6, 2017, 8:03 p.m.
SEV guest migration is not yet implemented yet.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 accel/kvm/sev.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Dr. David Alan Gilbert Dec. 7, 2017, 11:03 a.m. | #1
* Brijesh Singh (brijesh.singh@amd.com) wrote:
> SEV guest migration is not yet implemented yet.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  accel/kvm/sev.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/accel/kvm/sev.c b/accel/kvm/sev.c
> index 3edfb5b08416..10647645eacd 100644
> --- a/accel/kvm/sev.c
> +++ b/accel/kvm/sev.c
> @@ -19,6 +19,7 @@
>  #include "sysemu/sev.h"
>  #include "sysemu/sysemu.h"
>  #include "qapi-event.h"
> +#include "migration/blocker.h"
>  
>  #define DEFAULT_GUEST_POLICY    0x1 /* disable debug */
>  #define DEFAULT_SEV_DEVICE      "/dev/sev"
> @@ -36,6 +37,7 @@
>  static int sev_fd;
>  static SEVState *sev_state;
>  static MemoryRegionRAMReadWriteOps  sev_ops;
> +static Error *sev_mig_blocker;
>  
>  #define SEV_FW_MAX_ERROR      0x17
>  
> @@ -460,6 +462,7 @@ static void
>  sev_launch_finish(SEVState *s)
>  {
>      int ret, error;
> +    Error *local_err = NULL;
>  
>      ret = sev_ioctl(KVM_SEV_LAUNCH_FINISH, 0, &error);
>      if (ret) {
> @@ -470,6 +473,16 @@ sev_launch_finish(SEVState *s)
>  
>      s->cur_state = SEV_STATE_RUNNING;
>      DPRINTF("SEV: LAUNCH_FINISH\n");

(from a previous patch)
Please use the tracing facility rather than new DPRINTF's
if possible - if you've not used it before, then
--enable-trace-backends=log   is the easy way to get going
and you can turn on and off the stuff you're interested in
tracing at run time without having to rebuild.

> +
> +    /* add migration blocker */
> +    error_setg(&sev_mig_blocker,
> +               "SEV: Migration is not implemented");
> +    ret = migrate_add_blocker(sev_mig_blocker, &local_err);
> +    if (local_err) {
> +        error_report_err(local_err);
> +        error_free(sev_mig_blocker);
> +        exit(1);
> +    }

Yep, reasonable:


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

>  }
>  
>  static void
> -- 
> 2.9.5
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Peter Maydell Dec. 7, 2017, 11:10 a.m. | #2
On 7 December 2017 at 11:03, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> * Brijesh Singh (brijesh.singh@amd.com) wrote:
>> SEV guest migration is not yet implemented yet.

Is there at least a plan for how migration of a guest with
encrypted memory would be implemented?

thanks
-- PMM
Dr. David Alan Gilbert Dec. 7, 2017, 11:27 a.m. | #3
* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 7 December 2017 at 11:03, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > * Brijesh Singh (brijesh.singh@amd.com) wrote:
> >> SEV guest migration is not yet implemented yet.
> 
> Is there at least a plan for how migration of a guest with
> encrypted memory would be implemented?

It's something I've discussed with Brijesh and co and I've read
through the spec and it does look doable; I understand wanting to
get the basics of SEV going first though.

Dave

> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Brijesh Singh Dec. 7, 2017, 9:25 p.m. | #4
On 12/07/2017 05:27 AM, Dr. David Alan Gilbert wrote:
> * Peter Maydell (peter.maydell@linaro.org) wrote:
>> On 7 December 2017 at 11:03, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
>>> * Brijesh Singh (brijesh.singh@amd.com) wrote:
>>>> SEV guest migration is not yet implemented yet.
>>
>> Is there at least a plan for how migration of a guest with
>> encrypted memory would be implemented?
> 
> It's something I've discussed with Brijesh and co and I've read
> through the spec and it does look doable; I understand wanting to
> get the basics of SEV going first though.
> 

Yes this is something I have in plans. Both the spec and firmware 
supports encrypted guest migration. Lets get the basics enabled first.

thanks!

-Brijesh
Brijesh Singh Dec. 7, 2017, 10:50 p.m. | #5
On 12/07/2017 05:03 AM, Dr. David Alan Gilbert wrote:
.......

>>   
>>   #define SEV_FW_MAX_ERROR      0x17
>>   
>> @@ -460,6 +462,7 @@ static void
>>   sev_launch_finish(SEVState *s)
>>   {
>>       int ret, error;
>> +    Error *local_err = NULL;
>>   
>>       ret = sev_ioctl(KVM_SEV_LAUNCH_FINISH, 0, &error);
>>       if (ret) {
>> @@ -470,6 +473,16 @@ sev_launch_finish(SEVState *s)
>>   
>>       s->cur_state = SEV_STATE_RUNNING;
>>       DPRINTF("SEV: LAUNCH_FINISH\n");
> 
> (from a previous patch)
> Please use the tracing facility rather than new DPRINTF's
> if possible - if you've not used it before, then
> --enable-trace-backends=log   is the easy way to get going
> and you can turn on and off the stuff you're interested in
> tracing at run time without having to rebuild.


Thanks for review, I will look into converting those DPRINTF's in trace 
logging.

Patch

diff --git a/accel/kvm/sev.c b/accel/kvm/sev.c
index 3edfb5b08416..10647645eacd 100644
--- a/accel/kvm/sev.c
+++ b/accel/kvm/sev.c
@@ -19,6 +19,7 @@ 
 #include "sysemu/sev.h"
 #include "sysemu/sysemu.h"
 #include "qapi-event.h"
+#include "migration/blocker.h"
 
 #define DEFAULT_GUEST_POLICY    0x1 /* disable debug */
 #define DEFAULT_SEV_DEVICE      "/dev/sev"
@@ -36,6 +37,7 @@ 
 static int sev_fd;
 static SEVState *sev_state;
 static MemoryRegionRAMReadWriteOps  sev_ops;
+static Error *sev_mig_blocker;
 
 #define SEV_FW_MAX_ERROR      0x17
 
@@ -460,6 +462,7 @@  static void
 sev_launch_finish(SEVState *s)
 {
     int ret, error;
+    Error *local_err = NULL;
 
     ret = sev_ioctl(KVM_SEV_LAUNCH_FINISH, 0, &error);
     if (ret) {
@@ -470,6 +473,16 @@  sev_launch_finish(SEVState *s)
 
     s->cur_state = SEV_STATE_RUNNING;
     DPRINTF("SEV: LAUNCH_FINISH\n");
+
+    /* add migration blocker */
+    error_setg(&sev_mig_blocker,
+               "SEV: Migration is not implemented");
+    ret = migrate_add_blocker(sev_mig_blocker, &local_err);
+    if (local_err) {
+        error_report_err(local_err);
+        error_free(sev_mig_blocker);
+        exit(1);
+    }
 }
 
 static void