| Message ID | 148846789744.2349.167641684941925238.stgit@brijesh-build-machine (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On 02/03/2017 16:18, Brijesh Singh wrote: > +static int __sev_dbg_decrypt_page(struct kvm *kvm, unsigned long src, > + void *dst, int *error) > +{ > + inpages = sev_pin_memory(src, PAGE_SIZE, &npages); > + if (!inpages) { > + ret = -ENOMEM; > + goto err_1; > + } > + > + data->handle = sev_get_handle(kvm); > + data->dst_addr = __psp_pa(dst); > + data->src_addr = __sev_page_pa(inpages[0]); > + data->length = PAGE_SIZE; > + > + ret = sev_issue_cmd(kvm, SEV_CMD_DBG_DECRYPT, data, error); > + if (ret) > + printk(KERN_ERR "SEV: DEBUG_DECRYPT %d (%#010x)\n", > + ret, *error); > + sev_unpin_memory(inpages, npages); > +err_1: > + kfree(data); > + return ret; > +} > + > +static int sev_dbg_decrypt(struct kvm *kvm, struct kvm_sev_cmd *argp) > +{ > + void *data; > + int ret, offset, len; > + struct kvm_sev_dbg debug; > + > + if (!sev_guest(kvm)) > + return -ENOTTY; > + > + if (copy_from_user(&debug, (void *)argp->data, > + sizeof(struct kvm_sev_dbg))) > + return -EFAULT; > + /* > + * TODO: add support for decrypting length which crosses the > + * page boundary. > + */ > + offset = debug.src_addr & (PAGE_SIZE - 1); > + if (offset + debug.length > PAGE_SIZE) > + return -EINVAL; > + Please do add it, it doesn't seem very different from what you're doing in LAUNCH_UPDATE_DATA. There's no need for a separate __sev_dbg_decrypt_page function, you can just pin/unpin here and do a per-page loop as in LAUNCH_UPDATE_DATA. Paolo
On 03/16/2017 05:54 AM, Paolo Bonzini wrote: > > > On 02/03/2017 16:18, Brijesh Singh wrote: >> +static int __sev_dbg_decrypt_page(struct kvm *kvm, unsigned long src, >> + void *dst, int *error) >> +{ >> + inpages = sev_pin_memory(src, PAGE_SIZE, &npages); >> + if (!inpages) { >> + ret = -ENOMEM; >> + goto err_1; >> + } >> + >> + data->handle = sev_get_handle(kvm); >> + data->dst_addr = __psp_pa(dst); >> + data->src_addr = __sev_page_pa(inpages[0]); >> + data->length = PAGE_SIZE; >> + >> + ret = sev_issue_cmd(kvm, SEV_CMD_DBG_DECRYPT, data, error); >> + if (ret) >> + printk(KERN_ERR "SEV: DEBUG_DECRYPT %d (%#010x)\n", >> + ret, *error); >> + sev_unpin_memory(inpages, npages); >> +err_1: >> + kfree(data); >> + return ret; >> +} >> + >> +static int sev_dbg_decrypt(struct kvm *kvm, struct kvm_sev_cmd *argp) >> +{ >> + void *data; >> + int ret, offset, len; >> + struct kvm_sev_dbg debug; >> + >> + if (!sev_guest(kvm)) >> + return -ENOTTY; >> + >> + if (copy_from_user(&debug, (void *)argp->data, >> + sizeof(struct kvm_sev_dbg))) >> + return -EFAULT; >> + /* >> + * TODO: add support for decrypting length which crosses the >> + * page boundary. >> + */ >> + offset = debug.src_addr & (PAGE_SIZE - 1); >> + if (offset + debug.length > PAGE_SIZE) >> + return -EINVAL; >> + > > Please do add it, it doesn't seem very different from what you're doing > in LAUNCH_UPDATE_DATA. There's no need for a separate > __sev_dbg_decrypt_page function, you can just pin/unpin here and do a > per-page loop as in LAUNCH_UPDATE_DATA. > I can certainly add support to handle crossing the page boundary cases. Should we limit the size to prevent user passing arbitrary long length and we end up looping inside the kernel? I was thinking to limit to a PAGE_SIZE. ~ Brijesh
On 16/03/2017 19:41, Brijesh Singh wrote: >> >> Please do add it, it doesn't seem very different from what you're doing >> in LAUNCH_UPDATE_DATA. There's no need for a separate >> __sev_dbg_decrypt_page function, you can just pin/unpin here and do a >> per-page loop as in LAUNCH_UPDATE_DATA. > > I can certainly add support to handle crossing the page boundary cases. > Should we limit the size to prevent user passing arbitrary long length > and we end up looping inside the kernel? I was thinking to limit to a > PAGE_SIZE. I guess it depends on how it's used. PAGE_SIZE makes sense since you only know if a physical address is encrypted when you reach it from a visit of the page tables. Paolo
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 977aa22..ce8819a 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -5986,6 +5986,78 @@ static int sev_guest_status(struct kvm *kvm, struct kvm_sev_cmd *argp) return ret; } +static int __sev_dbg_decrypt_page(struct kvm *kvm, unsigned long src, + void *dst, int *error) +{ + int ret; + struct page **inpages; + struct sev_data_dbg *data; + unsigned long npages; + + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + inpages = sev_pin_memory(src, PAGE_SIZE, &npages); + if (!inpages) { + ret = -ENOMEM; + goto err_1; + } + + data->handle = sev_get_handle(kvm); + data->dst_addr = __psp_pa(dst); + data->src_addr = __sev_page_pa(inpages[0]); + data->length = PAGE_SIZE; + + ret = sev_issue_cmd(kvm, SEV_CMD_DBG_DECRYPT, data, error); + if (ret) + printk(KERN_ERR "SEV: DEBUG_DECRYPT %d (%#010x)\n", + ret, *error); + sev_unpin_memory(inpages, npages); +err_1: + kfree(data); + return ret; +} + +static int sev_dbg_decrypt(struct kvm *kvm, struct kvm_sev_cmd *argp) +{ + void *data; + int ret, offset, len; + struct kvm_sev_dbg debug; + + if (!sev_guest(kvm)) + return -ENOTTY; + + if (copy_from_user(&debug, (void *)argp->data, + sizeof(struct kvm_sev_dbg))) + return -EFAULT; + /* + * TODO: add support for decrypting length which crosses the + * page boundary. + */ + offset = debug.src_addr & (PAGE_SIZE - 1); + if (offset + debug.length > PAGE_SIZE) + return -EINVAL; + + data = (void *) get_zeroed_page(GFP_KERNEL); + if (!data) + return -ENOMEM; + + /* decrypt full page */ + ret = __sev_dbg_decrypt_page(kvm, debug.src_addr & PAGE_MASK, + data, &argp->error); + if (ret) + goto err_1; + + /* we have decrypted full page but copy request length */ + len = min_t(size_t, (PAGE_SIZE - offset), debug.length); + if (copy_to_user((uint8_t *)debug.dst_addr, data + offset, len)) + ret = -EFAULT; +err_1: + free_page((unsigned long)data); + return ret; +} + static int amd_memory_encryption_cmd(struct kvm *kvm, void __user *argp) { int r = -ENOTTY; @@ -6013,6 +6085,10 @@ static int amd_memory_encryption_cmd(struct kvm *kvm, void __user *argp) r = sev_guest_status(kvm, &sev_cmd); break; } + case KVM_SEV_DBG_DECRYPT: { + r = sev_dbg_decrypt(kvm, &sev_cmd); + break; + } default: break; }
The command is used to decrypt guest memory region for debug purposes. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- arch/x86/kvm/svm.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+)