Re: [Qemu-devel] [RFC PATCH v1 10/22] sev: add SEV debug decrypt command

[Brijesh Singh]

On 09/13/2016 09:28 PM, Michael S. Tsirkin wrote:
> On Tue, Sep 13, 2016 at 10:48:27AM -0400, Brijesh Singh wrote:
> > The SEV DEBUG_DECRYPT command is used for decrypting a guest memory
> > for the debugging purposes. Note that debugging is permitting only
> > when guest policy allows it.
>
> When wouldn't you want to allow it?
> I don't see value in a "break debugging" feature.
A guest owner needs to provide the launch parameters before we launch a 
SEV guest, a typical input parameters looks like this.

[sev-launch]
        flags = "0"
        policy  = "0"
        dh_pub_qx = "0123456789abcdef0123456789abcdef"
        dh_pub_qy = "0123456789abcdef0123456789abcdef"
        nonce = "0123456789abcdef"
        vcpu_count = "1"
        vcpu_length = "30"
        vcpu_mask = "00ab"

One of the bit in policy field is "debugging", if this bit is set then 
hypervisor can use SEV commands to decrypt a guest memory otherwise 
hypervisor read will always get encrypted data. Also note that policy 
field is used by firmware when computing the measurement of a guest 
launch so any changes in policy by hypervisor will result in wrong 
measurement.

> > For more information see [1], section 7.1
> >
> > [1] http://support.amd.com/TechDocs/55766_SEV-KM%20API_Spec.pdf
>
> Please add comments documenting APIs. Spec links to figure out
> implementation is one thing, but you really can't require people
> to read specs just to figure out how to use an API.
Sure,  i will work towards creating a simple file in doc/ directory that 
will list of commands, usage and their parameters and provide the link 
to exact section.

> > The following KVM RFC patches defines and implements this command
> >
> > http://marc.info/?l=kvm&m=147190852423972&w=2
> > http://marc.info/?l=kvm&m=147191068524579&w=2
> >
> > Signed-off-by: Brijesh Singh <address@hidden>
> > ---
> >  include/sysemu/sev.h |   10 ++++++++++
> >  sev.c                |   23 +++++++++++++++++++++++
> >  2 files changed, 33 insertions(+)
> >
> > diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
> > index ab03c5d..5872c3e 100644
> > --- a/include/sysemu/sev.h
> > +++ b/include/sysemu/sev.h
> > @@ -55,4 +55,14 @@ int kvm_sev_guest_finish(void);
> >   */
> >  int kvm_sev_guest_measurement(uint8_t *measurement);
> >
> > +/**
> > + * kvm_sev_dbg_decrypt -  decrypt the guest memory for debugging purposes
> > + * @src - guest memory address
> > + * @dest - host memory address where the decrypted data should be copied
> > + * @length - length of memory region
> > + *
> > + * Returns: 0 on success and dest will contains the decrypted data
> > + */
> > +int kvm_sev_dbg_decrypt(uint8_t *dest, const uint8_t *src, uint32_t len);
> > +
> >  #endif
> > diff --git a/sev.c b/sev.c
> > index 055ed83..c7031d3 100644
> > --- a/sev.c
> > +++ b/sev.c
> > @@ -432,3 +432,26 @@ int kvm_sev_guest_measurement(uint8_t *out)
> >
> >      return 0;
> >  }
> > +
> > +int kvm_sev_dbg_decrypt(uint8_t *dst, const uint8_t *src, uint32_t len)
> > +{
> > +    int ret;
> > +    struct kvm_sev_dbg_decrypt decrypt;
> > +    struct kvm_sev_issue_cmd input;
> > +
> > +    decrypt.src_addr = (unsigned long)src;
> > +    decrypt.dst_addr = (unsigned long)dst;
> > +    decrypt.length = len;
> > +
> > +    input.cmd = KVM_SEV_DBG_DECRYPT;
> > +    input.opaque = (unsigned long)&decrypt;
> > +    ret = kvm_vm_ioctl(kvm_state, KVM_SEV_ISSUE_CMD, &input);
> > +    if (ret) {
> > +        fprintf(stderr, "SEV: dbg_decrypt failed ret=%d(%#010x)\n",
> > +                ret, input.ret_code);
> > +        return 1;
> > +    }
> > +
> > +    DPRINTF("SEV: DBG_DECRYPT dst %p src %p sz %d\n", dst, src, len);
> > +    return 0;
> > +}