| Message ID | 20180129174132.108925-19-brijesh.singh@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Brijesh Singh (brijesh.singh@amd.com) wrote: > During machine creation we encrypted the guest bios image, the > LAUNCH_MEASURE command can be used to retrieve the measurement of > the encrypted memory region. Emit the SEV_MEASUREMENT event so that > libvirt can grab the measurement value as soon as we are done with > creating the encrypted machine. Can you ust clarify what happens if the libvirt has disconnected and reconnected to qemu and so didn't see the event? Can the reconnecting libvirt query it and find out it's ready/not ready yet? Dave > Cc: Daniel P. Berrange <berrange@redhat.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: kvm@vger.kernel.org > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > accel/kvm/sev.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++ > accel/kvm/trace-events | 1 + > include/sysemu/sev.h | 1 + > 3 files changed, 60 insertions(+) > > diff --git a/accel/kvm/sev.c b/accel/kvm/sev.c > index 1f757df725df..b78cf3144b1d 100644 > --- a/accel/kvm/sev.c > +++ b/accel/kvm/sev.c > @@ -19,11 +19,13 @@ > #include "sysemu/sev.h" > #include "sysemu/sysemu.h" > #include "trace.h" > +#include "qapi-event.h" > > #define DEFAULT_GUEST_POLICY 0x1 /* disable debug */ > #define DEFAULT_SEV_DEVICE "/dev/sev" > > static int sev_fd; > +static SEVState *sev_state; > > #define SEV_FW_MAX_ERROR 0x17 > > @@ -418,6 +420,59 @@ err: > return ret; > } > > +static void > +sev_launch_get_measure(Notifier *notifier, void *unused) > +{ > + int ret, error; > + guchar *data; > + SEVState *s = sev_state; > + struct kvm_sev_launch_measure *measurement; > + > + measurement = g_malloc0(sizeof(*measurement)); > + if (!measurement) { > + return; > + } > + > + /* query the measurement blob length */ > + ret = sev_ioctl(KVM_SEV_LAUNCH_MEASURE, measurement, &error); > + if (!measurement->len) { > + error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'", > + __func__, ret, error, fw_error_to_str(errno)); > + goto free_measurement; > + } > + > + data = g_malloc(measurement->len); > + if (s->measurement) { > + goto free_data; > + } > + > + measurement->uaddr = (unsigned long)data; > + > + /* get the measurement blob */ > + ret = sev_ioctl(KVM_SEV_LAUNCH_MEASURE, measurement, &error); > + if (ret) { > + error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'", > + __func__, ret, error, fw_error_to_str(errno)); > + goto free_data; > + } > + > + sev_set_guest_state(SEV_STATE_SECRET); > + > + /* encode the measurement value and emit the event */ > + s->measurement = g_base64_encode(data, measurement->len); > + trace_kvm_sev_launch_measurement(s->measurement); > + qapi_event_send_sev_measurement(s->measurement, &error_abort); > + > +free_data: > + g_free(data); > +free_measurement: > + g_free(measurement); > +} > + > +static Notifier sev_machine_done_notify = { > + .notify = sev_launch_get_measure, > +}; > + > void * > sev_guest_init(const char *id) > { > @@ -461,6 +516,9 @@ sev_guest_init(const char *id) > } > > ram_block_notifier_add(&sev_ram_notifier); > + qemu_add_machine_init_done_notifier(&sev_machine_done_notify); > + > + sev_state = s; > > return s; > err: > diff --git a/accel/kvm/trace-events b/accel/kvm/trace-events > index c55546f36a25..51df5113ad07 100644 > --- a/accel/kvm/trace-events > +++ b/accel/kvm/trace-events > @@ -20,3 +20,4 @@ kvm_memcrypt_unregister_region(void *addr, size_t len) "addr %p len 0x%lu" > kvm_sev_change_state(char *old, char *new) "%s -> %s" > kvm_sev_launch_start(int policy, void *session, void *pdh) "policy 0x%x session %p pdh %p" > kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%" PRIu64 > +kvm_sev_launch_measurement(const char *value) "data %s" > diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h > index 839800efdbbf..572120c865ea 100644 > --- a/include/sysemu/sev.h > +++ b/include/sysemu/sev.h > @@ -63,6 +63,7 @@ typedef enum { > > struct SEVState { > QSevGuestInfo *sev_info; > + gchar *measurement; > }; > > typedef struct SEVState SEVState; > -- > 2.9.5 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 1/30/18 2:08 PM, Dr. David Alan Gilbert wrote: > * Brijesh Singh (brijesh.singh@amd.com) wrote: >> During machine creation we encrypted the guest bios image, the >> LAUNCH_MEASURE command can be used to retrieve the measurement of >> the encrypted memory region. Emit the SEV_MEASUREMENT event so that >> libvirt can grab the measurement value as soon as we are done with >> creating the encrypted machine. > Can you ust clarify what happens if the libvirt has disconnected and > reconnected to qemu and so didn't see the event? Can the reconnecting > libvirt query it and find out it's ready/not ready yet? Dave, I have not looked into details between libvirt and qemu interaction to comment how and when the events will be delivered. Recently, one of my colleague was implementing libvirt interface for the SEV guest and ran into somewhat a similar challenge and posted question on libvirt mailing list [1]. In previous discussion on qemu mailing list, we agreed to implement SEV MEASUREMENT event which can be seen by libvirt. That's what this patch is doing. But during the libvirt implementation it seems that qemu monitor silently drops all the events before it get the first qmp_capabilities command. At a quick glance it seems on reconnect, libvirt issues qmp_capabilities command and any event issued before the qmp_capabilities command will never to delivered to libvirt. we are looking for help from libvirt/qemu monitor experts on how we solve this problem. Our goal is to provide the measurement to libvirt before libvirt issues "continue" command. Since event can't be seen by libvirt before it resumes the guest hence I was wondering if we should we should drop the SEV measurement event and consider adding a new QMP command to query the SEV measurement. [1] https://www.redhat.com/archives/libvir-list/2018-January/msg00602.html > Dave > >> Cc: Daniel P. Berrange <berrange@redhat.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: kvm@vger.kernel.org >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >> --- >> accel/kvm/sev.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> accel/kvm/trace-events | 1 + >> include/sysemu/sev.h | 1 + >> 3 files changed, 60 insertions(+) >> >> diff --git a/accel/kvm/sev.c b/accel/kvm/sev.c >> index 1f757df725df..b78cf3144b1d 100644 >> --- a/accel/kvm/sev.c >> +++ b/accel/kvm/sev.c >> @@ -19,11 +19,13 @@ >> #include "sysemu/sev.h" >> #include "sysemu/sysemu.h" >> #include "trace.h" >> +#include "qapi-event.h" >> >> #define DEFAULT_GUEST_POLICY 0x1 /* disable debug */ >> #define DEFAULT_SEV_DEVICE "/dev/sev" >> >> static int sev_fd; >> +static SEVState *sev_state; >> >> #define SEV_FW_MAX_ERROR 0x17 >> >> @@ -418,6 +420,59 @@ err: >> return ret; >> } >> >> +static void >> +sev_launch_get_measure(Notifier *notifier, void *unused) >> +{ >> + int ret, error; >> + guchar *data; >> + SEVState *s = sev_state; >> + struct kvm_sev_launch_measure *measurement; >> + >> + measurement = g_malloc0(sizeof(*measurement)); >> + if (!measurement) { >> + return; >> + } >> + >> + /* query the measurement blob length */ >> + ret = sev_ioctl(KVM_SEV_LAUNCH_MEASURE, measurement, &error); >> + if (!measurement->len) { >> + error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'", >> + __func__, ret, error, fw_error_to_str(errno)); >> + goto free_measurement; >> + } >> + >> + data = g_malloc(measurement->len); >> + if (s->measurement) { >> + goto free_data; >> + } >> + >> + measurement->uaddr = (unsigned long)data; >> + >> + /* get the measurement blob */ >> + ret = sev_ioctl(KVM_SEV_LAUNCH_MEASURE, measurement, &error); >> + if (ret) { >> + error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'", >> + __func__, ret, error, fw_error_to_str(errno)); >> + goto free_data; >> + } >> + >> + sev_set_guest_state(SEV_STATE_SECRET); >> + >> + /* encode the measurement value and emit the event */ >> + s->measurement = g_base64_encode(data, measurement->len); >> + trace_kvm_sev_launch_measurement(s->measurement); >> + qapi_event_send_sev_measurement(s->measurement, &error_abort); >> + >> +free_data: >> + g_free(data); >> +free_measurement: >> + g_free(measurement); >> +} >> + >> +static Notifier sev_machine_done_notify = { >> + .notify = sev_launch_get_measure, >> +}; >> + >> void * >> sev_guest_init(const char *id) >> { >> @@ -461,6 +516,9 @@ sev_guest_init(const char *id) >> } >> >> ram_block_notifier_add(&sev_ram_notifier); >> + qemu_add_machine_init_done_notifier(&sev_machine_done_notify); >> + >> + sev_state = s; >> >> return s; >> err: >> diff --git a/accel/kvm/trace-events b/accel/kvm/trace-events >> index c55546f36a25..51df5113ad07 100644 >> --- a/accel/kvm/trace-events >> +++ b/accel/kvm/trace-events >> @@ -20,3 +20,4 @@ kvm_memcrypt_unregister_region(void *addr, size_t len) "addr %p len 0x%lu" >> kvm_sev_change_state(char *old, char *new) "%s -> %s" >> kvm_sev_launch_start(int policy, void *session, void *pdh) "policy 0x%x session %p pdh %p" >> kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%" PRIu64 >> +kvm_sev_launch_measurement(const char *value) "data %s" >> diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h >> index 839800efdbbf..572120c865ea 100644 >> --- a/include/sysemu/sev.h >> +++ b/include/sysemu/sev.h >> @@ -63,6 +63,7 @@ typedef enum { >> >> struct SEVState { >> QSevGuestInfo *sev_info; >> + gchar *measurement; >> }; >> >> typedef struct SEVState SEVState; >> -- >> 2.9.5 >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
* Brijesh Singh (brijesh.singh@amd.com) wrote: > > > On 1/30/18 2:08 PM, Dr. David Alan Gilbert wrote: > > * Brijesh Singh (brijesh.singh@amd.com) wrote: > >> During machine creation we encrypted the guest bios image, the > >> LAUNCH_MEASURE command can be used to retrieve the measurement of > >> the encrypted memory region. Emit the SEV_MEASUREMENT event so that > >> libvirt can grab the measurement value as soon as we are done with > >> creating the encrypted machine. > > Can you ust clarify what happens if the libvirt has disconnected and > > reconnected to qemu and so didn't see the event? Can the reconnecting > > libvirt query it and find out it's ready/not ready yet? > > Dave, > > I have not looked into details between libvirt and qemu interaction to > comment how and when the events will be delivered. Recently, one of my > colleague was implementing libvirt interface for the SEV guest and ran > into somewhat a similar challenge and posted question on libvirt mailing > list [1]. > > In previous discussion on qemu mailing list, we agreed to implement SEV > MEASUREMENT event which can be seen by libvirt. That's what this patch > is doing. > > But during the libvirt implementation it seems that qemu monitor > silently drops all the events before it get the first qmp_capabilities > command. At a quick glance it seems on reconnect, libvirt issues > qmp_capabilities command and any event issued before the > qmp_capabilities command will never to delivered to libvirt. we are > looking for help from libvirt/qemu monitor experts on how we solve this > problem. Our goal is to provide the measurement to libvirt before > libvirt issues "continue" command. Since event can't be seen by libvirt > before it resumes the guest hence I was wondering if we should we should > drop the SEV measurement event and consider adding a new QMP command to > query the SEV measurement. Yep, I'll leave it to the libvirt contacts for the best way they'd like to see that, as Eric says there's nothing wrong with having both the command and event if useful. Also keep in mind coping with a guest that crashes early or that measurement never arrives. Dave > [1] https://www.redhat.com/archives/libvir-list/2018-January/msg00602.html > > > Dave > > > >> Cc: Daniel P. Berrange <berrange@redhat.com> > >> Cc: Paolo Bonzini <pbonzini@redhat.com> > >> Cc: kvm@vger.kernel.org > >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > >> --- > >> accel/kvm/sev.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++ > >> accel/kvm/trace-events | 1 + > >> include/sysemu/sev.h | 1 + > >> 3 files changed, 60 insertions(+) > >> > >> diff --git a/accel/kvm/sev.c b/accel/kvm/sev.c > >> index 1f757df725df..b78cf3144b1d 100644 > >> --- a/accel/kvm/sev.c > >> +++ b/accel/kvm/sev.c > >> @@ -19,11 +19,13 @@ > >> #include "sysemu/sev.h" > >> #include "sysemu/sysemu.h" > >> #include "trace.h" > >> +#include "qapi-event.h" > >> > >> #define DEFAULT_GUEST_POLICY 0x1 /* disable debug */ > >> #define DEFAULT_SEV_DEVICE "/dev/sev" > >> > >> static int sev_fd; > >> +static SEVState *sev_state; > >> > >> #define SEV_FW_MAX_ERROR 0x17 > >> > >> @@ -418,6 +420,59 @@ err: > >> return ret; > >> } > >> > >> +static void > >> +sev_launch_get_measure(Notifier *notifier, void *unused) > >> +{ > >> + int ret, error; > >> + guchar *data; > >> + SEVState *s = sev_state; > >> + struct kvm_sev_launch_measure *measurement; > >> + > >> + measurement = g_malloc0(sizeof(*measurement)); > >> + if (!measurement) { > >> + return; > >> + } > >> + > >> + /* query the measurement blob length */ > >> + ret = sev_ioctl(KVM_SEV_LAUNCH_MEASURE, measurement, &error); > >> + if (!measurement->len) { > >> + error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'", > >> + __func__, ret, error, fw_error_to_str(errno)); > >> + goto free_measurement; > >> + } > >> + > >> + data = g_malloc(measurement->len); > >> + if (s->measurement) { > >> + goto free_data; > >> + } > >> + > >> + measurement->uaddr = (unsigned long)data; > >> + > >> + /* get the measurement blob */ > >> + ret = sev_ioctl(KVM_SEV_LAUNCH_MEASURE, measurement, &error); > >> + if (ret) { > >> + error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'", > >> + __func__, ret, error, fw_error_to_str(errno)); > >> + goto free_data; > >> + } > >> + > >> + sev_set_guest_state(SEV_STATE_SECRET); > >> + > >> + /* encode the measurement value and emit the event */ > >> + s->measurement = g_base64_encode(data, measurement->len); > >> + trace_kvm_sev_launch_measurement(s->measurement); > >> + qapi_event_send_sev_measurement(s->measurement, &error_abort); > >> + > >> +free_data: > >> + g_free(data); > >> +free_measurement: > >> + g_free(measurement); > >> +} > >> + > >> +static Notifier sev_machine_done_notify = { > >> + .notify = sev_launch_get_measure, > >> +}; > >> + > >> void * > >> sev_guest_init(const char *id) > >> { > >> @@ -461,6 +516,9 @@ sev_guest_init(const char *id) > >> } > >> > >> ram_block_notifier_add(&sev_ram_notifier); > >> + qemu_add_machine_init_done_notifier(&sev_machine_done_notify); > >> + > >> + sev_state = s; > >> > >> return s; > >> err: > >> diff --git a/accel/kvm/trace-events b/accel/kvm/trace-events > >> index c55546f36a25..51df5113ad07 100644 > >> --- a/accel/kvm/trace-events > >> +++ b/accel/kvm/trace-events > >> @@ -20,3 +20,4 @@ kvm_memcrypt_unregister_region(void *addr, size_t len) "addr %p len 0x%lu" > >> kvm_sev_change_state(char *old, char *new) "%s -> %s" > >> kvm_sev_launch_start(int policy, void *session, void *pdh) "policy 0x%x session %p pdh %p" > >> kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%" PRIu64 > >> +kvm_sev_launch_measurement(const char *value) "data %s" > >> diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h > >> index 839800efdbbf..572120c865ea 100644 > >> --- a/include/sysemu/sev.h > >> +++ b/include/sysemu/sev.h > >> @@ -63,6 +63,7 @@ typedef enum { > >> > >> struct SEVState { > >> QSevGuestInfo *sev_info; > >> + gchar *measurement; > >> }; > >> > >> typedef struct SEVState SEVState; > >> -- > >> 2.9.5 > >> > > -- > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 02/01/2018 11:27 AM, Dr. David Alan Gilbert wrote: > * Brijesh Singh (brijesh.singh@amd.com) wrote: >> >> >> On 1/30/18 2:08 PM, Dr. David Alan Gilbert wrote: >>> * Brijesh Singh (brijesh.singh@amd.com) wrote: >>>> During machine creation we encrypted the guest bios image, the >>>> LAUNCH_MEASURE command can be used to retrieve the measurement of >>>> the encrypted memory region. Emit the SEV_MEASUREMENT event so that >>>> libvirt can grab the measurement value as soon as we are done with >>>> creating the encrypted machine. >>> Can you ust clarify what happens if the libvirt has disconnected and >>> reconnected to qemu and so didn't see the event? Can the reconnecting >>> libvirt query it and find out it's ready/not ready yet? >> >> Dave, >> >> I have not looked into details between libvirt and qemu interaction to >> comment how and when the events will be delivered. Recently, one of my >> colleague was implementing libvirt interface for the SEV guest and ran >> into somewhat a similar challenge and posted question on libvirt mailing >> list [1]. >> >> In previous discussion on qemu mailing list, we agreed to implement SEV >> MEASUREMENT event which can be seen by libvirt. That's what this patch >> is doing. >> >> But during the libvirt implementation it seems that qemu monitor >> silently drops all the events before it get the first qmp_capabilities >> command. At a quick glance it seems on reconnect, libvirt issues >> qmp_capabilities command and any event issued before the >> qmp_capabilities command will never to delivered to libvirt. we are >> looking for help from libvirt/qemu monitor experts on how we solve this >> problem. Our goal is to provide the measurement to libvirt before >> libvirt issues "continue" command. Since event can't be seen by libvirt >> before it resumes the guest hence I was wondering if we should we should >> drop the SEV measurement event and consider adding a new QMP command to >> query the SEV measurement. > > Yep, I'll leave it to the libvirt contacts for the best way they'd like > to see that, as Eric says there's nothing wrong with having both the > command and event if useful. Also keep in mind coping with a guest that > crashes early or that measurement never arrives. > Yep, lets see what libvirt experts say about it. Hi Daniel, Do you have any recommendation on whether we should consider adding a new QMP to retrieve the measurement or we do event or both? Please note that the launch measurement is generate only once for the lifetime of the guest. The measurement will be available after qmeu encrypts the guest bios during the machine initialization time. -Brijesh
On Fri, Feb 02, 2018 at 09:11:41AM -0600, Brijesh Singh wrote: > > > On 02/01/2018 11:27 AM, Dr. David Alan Gilbert wrote: > > * Brijesh Singh (brijesh.singh@amd.com) wrote: > > > > > > > > > On 1/30/18 2:08 PM, Dr. David Alan Gilbert wrote: > > > > * Brijesh Singh (brijesh.singh@amd.com) wrote: > > > > > During machine creation we encrypted the guest bios image, the > > > > > LAUNCH_MEASURE command can be used to retrieve the measurement of > > > > > the encrypted memory region. Emit the SEV_MEASUREMENT event so that > > > > > libvirt can grab the measurement value as soon as we are done with > > > > > creating the encrypted machine. > > > > Can you ust clarify what happens if the libvirt has disconnected and > > > > reconnected to qemu and so didn't see the event? Can the reconnecting > > > > libvirt query it and find out it's ready/not ready yet? > > > > > > Dave, > > > > > > I have not looked into details between libvirt and qemu interaction to > > > comment how and when the events will be delivered. Recently, one of my > > > colleague was implementing libvirt interface for the SEV guest and ran > > > into somewhat a similar challenge and posted question on libvirt mailing > > > list [1]. > > > > > > In previous discussion on qemu mailing list, we agreed to implement SEV > > > MEASUREMENT event which can be seen by libvirt. That's what this patch > > > is doing. > > > > > > But during the libvirt implementation it seems that qemu monitor > > > silently drops all the events before it get the first qmp_capabilities > > > command. At a quick glance it seems on reconnect, libvirt issues > > > qmp_capabilities command and any event issued before the > > > qmp_capabilities command will never to delivered to libvirt. we are > > > looking for help from libvirt/qemu monitor experts on how we solve this > > > problem. Our goal is to provide the measurement to libvirt before > > > libvirt issues "continue" command. Since event can't be seen by libvirt > > > before it resumes the guest hence I was wondering if we should we should > > > drop the SEV measurement event and consider adding a new QMP command to > > > query the SEV measurement. > > > > Yep, I'll leave it to the libvirt contacts for the best way they'd like > > to see that, as Eric says there's nothing wrong with having both the > > command and event if useful. Also keep in mind coping with a guest that > > crashes early or that measurement never arrives. > > > > Yep, lets see what libvirt experts say about it. > > Hi Daniel, > > Do you have any recommendation on whether we should consider adding a new > QMP to retrieve the measurement or we do event or both? Please note that the > launch measurement is generate only once for the lifetime of the guest. The > measurement will be available after qmeu encrypts the guest bios during the > machine initialization time. IIUC, the measurement event is only required during the initial QEMU startup sequence. Once the guest CPUs are running this info is not needed any more. If libvirtd crashes/restarts in the middle of QEMU startup sequence it is game over from libvirt's POV. Libvirtd won't try to carry on starting that guest when it restarts. So I don't think there's a compelling need for a command to query the measurement from libvirt's POV, the event is fine. That all said, I think it might be useful to have a command to query the SEV measurement purely as a debugging aid, if some admin / support person wants to get hold of this info for some reason... Regards, Daniel
On Fri, Feb 2, 2018 at 9:16 AM, Daniel P. Berrangé <berrange@redhat.com> wrote: > On Fri, Feb 02, 2018 at 09:11:41AM -0600, Brijesh Singh wrote: > > > > > > On 02/01/2018 11:27 AM, Dr. David Alan Gilbert wrote: > > > * Brijesh Singh (brijesh.singh@amd.com) wrote: > > > > > > > > > > > > On 1/30/18 2:08 PM, Dr. David Alan Gilbert wrote: > > > > > * Brijesh Singh (brijesh.singh@amd.com) wrote: > > > > > > During machine creation we encrypted the guest bios image, the > > > > > > LAUNCH_MEASURE command can be used to retrieve the measurement of > > > > > > the encrypted memory region. Emit the SEV_MEASUREMENT event so > that > > > > > > libvirt can grab the measurement value as soon as we are done > with > > > > > > creating the encrypted machine. > > > > > Can you ust clarify what happens if the libvirt has disconnected > and > > > > > reconnected to qemu and so didn't see the event? Can the > reconnecting > > > > > libvirt query it and find out it's ready/not ready yet? > > > > > > > > Dave, > > > > > > > > I have not looked into details between libvirt and qemu interaction > to > > > > comment how and when the events will be delivered. Recently, one of > my > > > > colleague was implementing libvirt interface for the SEV guest and > ran > > > > into somewhat a similar challenge and posted question on libvirt > mailing > > > > list [1]. > > > > > > > > In previous discussion on qemu mailing list, we agreed to implement > SEV > > > > MEASUREMENT event which can be seen by libvirt. That's what this > patch > > > > is doing. > > > > > > > > But during the libvirt implementation it seems that qemu monitor > > > > silently drops all the events before it get the first > qmp_capabilities > > > > command. At a quick glance it seems on reconnect, libvirt issues > > > > qmp_capabilities command and any event issued before the > > > > qmp_capabilities command will never to delivered to libvirt. we are > > > > looking for help from libvirt/qemu monitor experts on how we solve > this > > > > problem. Our goal is to provide the measurement to libvirt before > > > > libvirt issues "continue" command. Since event can't be seen by > libvirt > > > > before it resumes the guest hence I was wondering if we should we > should > > > > drop the SEV measurement event and consider adding a new QMP command > to > > > > query the SEV measurement. > > > > > > Yep, I'll leave it to the libvirt contacts for the best way they'd like > > > to see that, as Eric says there's nothing wrong with having both the > > > command and event if useful. Also keep in mind coping with a guest > that > > > crashes early or that measurement never arrives. > > > > > > > Yep, lets see what libvirt experts say about it. > > > > Hi Daniel, > > > > Do you have any recommendation on whether we should consider adding a new > > QMP to retrieve the measurement or we do event or both? Please note that > the > > launch measurement is generate only once for the lifetime of the guest. > The > > measurement will be available after qmeu encrypts the guest bios during > the > > machine initialization time. > > IIUC, the measurement event is only required during the initial QEMU > startup sequence. Once the guest CPUs are running this info is not needed > any more. > That is correct. > > If libvirtd crashes/restarts in the middle of QEMU startup sequence it is > game over from libvirt's POV. Libvirtd won't try to carry on starting that > guest when it restarts. So I don't think there's a compelling need for a > command to query the measurement from libvirt's POV, the event is fine. > > Sorry for the late response, somehow this email was never delievered to my AMD account hence I was not able to see the response on-time. Anyway, the main issue is event emitted before "qmp_capabilities" are dropped silently , see [1] . In our case, the measurement event is emitted during the machine creation time, libvirt was still waiting to reconnect the monitor when event was emitted hence libvirt never gets the event. Because of this, I am not able to see any strong reason to emit the event. Do you see any issue with libvirt issuing a 'query-sev-launch-measure' to get the measurement on reconnect ? [1] https://github.com/qemu/qemu/blob/master/monitor.c#L418 That all said, I think it might be useful to have a command to query the > SEV measurement purely as a debugging aid, if some admin / support person > wants to get hold of this info for some reason... > > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/ > dberrange :| > |: https://libvirt.org -o- > https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/ > dberrange :| > >
On Thu, Feb 08, 2018 at 10:17:28AM -0600, Brijesh Singh wrote: > > Anyway, the main issue is event emitted before "qmp_capabilities" are > dropped silently , see [1] . In our case, the measurement event is emitted > during the machine creation time, libvirt was still waiting to reconnect > the monitor when event was emitted hence libvirt never gets the event. > Because of this, I am not able to see any strong reason to emit the event. > Do you see any issue with libvirt issuing a 'query-sev-launch-measure' to > get the measurement on reconnect ? Hmm, yes, I see so there can be a race between SEV emitting the event and libvirtd making the initial monitor connection. In that case, we definitely to have a QMP command to let libvirt explicitly query it. Regards, Daniel
diff --git a/accel/kvm/sev.c b/accel/kvm/sev.c index 1f757df725df..b78cf3144b1d 100644 --- a/accel/kvm/sev.c +++ b/accel/kvm/sev.c @@ -19,11 +19,13 @@ #include "sysemu/sev.h" #include "sysemu/sysemu.h" #include "trace.h" +#include "qapi-event.h" #define DEFAULT_GUEST_POLICY 0x1 /* disable debug */ #define DEFAULT_SEV_DEVICE "/dev/sev" static int sev_fd; +static SEVState *sev_state; #define SEV_FW_MAX_ERROR 0x17 @@ -418,6 +420,59 @@ err: return ret; } +static void +sev_launch_get_measure(Notifier *notifier, void *unused) +{ + int ret, error; + guchar *data; + SEVState *s = sev_state; + struct kvm_sev_launch_measure *measurement; + + measurement = g_malloc0(sizeof(*measurement)); + if (!measurement) { + return; + } + + /* query the measurement blob length */ + ret = sev_ioctl(KVM_SEV_LAUNCH_MEASURE, measurement, &error); + if (!measurement->len) { + error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'", + __func__, ret, error, fw_error_to_str(errno)); + goto free_measurement; + } + + data = g_malloc(measurement->len); + if (s->measurement) { + goto free_data; + } + + measurement->uaddr = (unsigned long)data; + + /* get the measurement blob */ + ret = sev_ioctl(KVM_SEV_LAUNCH_MEASURE, measurement, &error); + if (ret) { + error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'", + __func__, ret, error, fw_error_to_str(errno)); + goto free_data; + } + + sev_set_guest_state(SEV_STATE_SECRET); + + /* encode the measurement value and emit the event */ + s->measurement = g_base64_encode(data, measurement->len); + trace_kvm_sev_launch_measurement(s->measurement); + qapi_event_send_sev_measurement(s->measurement, &error_abort); + +free_data: + g_free(data); +free_measurement: + g_free(measurement); +} + +static Notifier sev_machine_done_notify = { + .notify = sev_launch_get_measure, +}; + void * sev_guest_init(const char *id) { @@ -461,6 +516,9 @@ sev_guest_init(const char *id) } ram_block_notifier_add(&sev_ram_notifier); + qemu_add_machine_init_done_notifier(&sev_machine_done_notify); + + sev_state = s; return s; err: diff --git a/accel/kvm/trace-events b/accel/kvm/trace-events index c55546f36a25..51df5113ad07 100644 --- a/accel/kvm/trace-events +++ b/accel/kvm/trace-events @@ -20,3 +20,4 @@ kvm_memcrypt_unregister_region(void *addr, size_t len) "addr %p len 0x%lu" kvm_sev_change_state(char *old, char *new) "%s -> %s" kvm_sev_launch_start(int policy, void *session, void *pdh) "policy 0x%x session %p pdh %p" kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%" PRIu64 +kvm_sev_launch_measurement(const char *value) "data %s" diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h index 839800efdbbf..572120c865ea 100644 --- a/include/sysemu/sev.h +++ b/include/sysemu/sev.h @@ -63,6 +63,7 @@ typedef enum { struct SEVState { QSevGuestInfo *sev_info; + gchar *measurement; }; typedef struct SEVState SEVState;
During machine creation we encrypted the guest bios image, the LAUNCH_MEASURE command can be used to retrieve the measurement of the encrypted memory region. Emit the SEV_MEASUREMENT event so that libvirt can grab the measurement value as soon as we are done with creating the encrypted machine. Cc: Daniel P. Berrange <berrange@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: kvm@vger.kernel.org Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- accel/kvm/sev.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++ accel/kvm/trace-events | 1 + include/sysemu/sev.h | 1 + 3 files changed, 60 insertions(+)