Re: [PATCH v2 0/8] ccp: KVM: SVM: Use stack for SEV command buffers
On 07/04/21 20:00, Tom Lendacky wrote: For the series: Acked-by: Tom Lendacky Shall I take this as a request (or permission, whatever :)) to merge it through the KVM tree? Paolo
Re: [PATCH v2 5/8] crypto: ccp: Use the stack for small SEV command buffers
On 07/04/21 00:49, Sean Christopherson wrote:
For commands with small input/output buffers, use the local stack to
"allocate" the structures used to communicate with the PSP. Now that
__sev_do_cmd_locked() gracefully handles vmalloc'd buffers, there's no
reason to avoid using the stack, e.g. CONFIG_VMAP_STACK=y will just work.
Signed-off-by: Sean Christopherson
Squashing this in (inspired by Christophe's review, though not quite
matching his suggestion).
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 0f5644a3b138..246b281b6376 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -408,12 +408,11 @@ static int sev_ioctl_do_pek_csr(struct sev_issue_cmd
*argp, bool writable)
if (copy_from_user(&input, (void __user *)argp->data, sizeof(input)))
return -EFAULT;
+ memset(&data, 0, sizeof(data));
+
/* userspace wants to query CSR length */
- if (!input.address || !input.length) {
- data.address = 0;
- data.len = 0;
+ if (!input.address || !input.length)
goto cmd;
- }
/* allocate a physically contiguous buffer to store the CSR blob */
input_address = (void __user *)input.address;
Re: [PATCH v2 7/8] crypto: ccp: Use the stack and common buffer for INIT command
On 07/04/21 07:20, Christophe Leroy wrote:
+ struct sev_data_init data;
struct sev_data_init data = {0, 0, 0, 0};
Having to count the number of items is suboptimal. The alternative
could be {} (which however is technically not standard C), {0} (a bit
mysterious, but it works) and memset. I kept the latter to avoid
touching the submitter's patch too much.
Paolo
Re: [PATCH v2 8/8] KVM: SVM: Allocate SEV command structures on local stack
On 07/04/21 19:34, Borislav Petkov wrote:
On Wed, Apr 07, 2021 at 05:05:07PM +, Sean Christopherson wrote:
I used memset() to defer initialization until after the various sanity
checks,
I'd actually vote for that too - I don't like doing stuff which is not
going to be used. I.e., don't change what you have.
It's three votes for that then. :)
Sean, I squashed in this change
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index ec4c01807272..a4d0ca8c4710 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1039,21 +1039,14 @@ static int sev_get_attestation_report(struct kvm *kvm,
struct kvm_sev_cmd *argp)
static int sev_send_cancel(struct kvm *kvm, struct kvm_sev_cmd *argp)
{
struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
- struct sev_data_send_cancel *data;
+ struct sev_data_send_cancel data;
int ret;
if (!sev_guest(kvm))
return -ENOTTY;
- data = kzalloc(sizeof(*data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- data->handle = sev->handle;
- ret = sev_issue_cmd(kvm, SEV_CMD_SEND_CANCEL, data, &argp->error);
-
- kfree(data);
- return ret;
+ data.handle = sev->handle;
+ return sev_issue_cmd(kvm, SEV_CMD_SEND_CANCEL, &data, &argp->error);
}
int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
to handle SV_CMD_SEND_CANCEL which I merged before this series.
Paolo
Re: [PATCH v2 0/8] ccp: KVM: SVM: Use stack for SEV command buffers
On 07/04/21 00:49, Sean Christopherson wrote:
This series teaches __sev_do_cmd_locked() to gracefully handle vmalloc'd
command buffers by copying _all_ incoming data pointers to an internal
buffer before sending the command to the PSP. The SEV driver and KVM are
then converted to use the stack for all command buffers.
Tested everything except sev_ioctl_do_pek_import(), I don't know anywhere
near enough about the PSP to give it the right input.
v2:
- Rebase to kvm/queue, commit f96be2deac9b ("KVM: x86: Support KVM VMs
sharing SEV context").
- Unconditionally copy @data to the internal buffer. [Christophe, Brijesh]
- Allocate a full page for the buffer. [Brijesh]
- Drop one set of the "!"s. [Christophe]
- Use virt_addr_valid() instead of is_vmalloc_addr() for the temporary
patch (definitely feel free to drop the patch if it's not worth
backporting). [Christophe]
- s/intput/input/. [Tom]
- Add a patch to free "sev" if init fails. This is not strictly
necessary (I think; I suck horribly when it comes to the driver
framework). But it felt wrong to not free cmd_buf on failure, and
even more wrong to free cmd_buf but not sev.
v1:
- https://lkml.kernel.org/r/20210402233702.3291792-1-sea...@google.com
Sean Christopherson (8):
crypto: ccp: Free SEV device if SEV init fails
crypto: ccp: Detect and reject "invalid" addresses destined for PSP
crypto: ccp: Reject SEV commands with mismatching command buffer
crypto: ccp: Play nice with vmalloc'd memory for SEV command structs
crypto: ccp: Use the stack for small SEV command buffers
crypto: ccp: Use the stack and common buffer for status commands
crypto: ccp: Use the stack and common buffer for INIT command
KVM: SVM: Allocate SEV command structures on local stack
arch/x86/kvm/svm/sev.c | 262 +--
drivers/crypto/ccp/sev-dev.c | 197 +-
drivers/crypto/ccp/sev-dev.h | 4 +-
3 files changed, 196 insertions(+), 267 deletions(-)
Queued, thanks.
Paolo
Re: [RFC PATCH v2 12/32] x86: Add early boot support when running with SEV active
On 09/03/2017 15:07, Borislav Petkov wrote:
> + /* Check if running under a hypervisor */
> + eax = 0x4000;
> + ecx = 0;
> + native_cpuid(&eax, &ebx, &ecx, &edx);
This is not how you check if running under a hypervisor; you should
check the HYPERVISOR bit, i.e. bit 31 of cpuid(1).ecx. This in turn
tells you if leaf 0x4000 is valid.
That said, the main issue with this function is that it hardcodes the
behavior for KVM. It is possible that another hypervisor defines its
0x4001 leaf in such a way that KVM_FEATURE_SEV has a different meaning.
Instead, AMD should define a "well-known" bit in its own space (i.e.
0x80xx) that is only used by hypervisors that support SEV. This is
similar to how Intel defined one bit in leaf 1 to say "is leaf
0x4000 valid".
Thanks,
Paolo
> + if (eax > 0x4000) {
> + eax = 0x4001;
> + ecx = 0;
> + native_cpuid(&eax, &ebx, &ecx, &edx);
> + if (!(eax & BIT(KVM_FEATURE_SEV)))
> + goto out;
> +
> + eax = 0x801f;
> + ecx = 0;
> + native_cpuid(&eax, &ebx, &ecx, &edx);
> + if (!(eax & 1))
> + goto out;
> +
> + sme_me_mask = 1UL << (ebx & 0x3f);
> + sev_enabled = 1;
> +
> + goto out;
> + }
> +
Re: [RFC PATCH v2 23/32] kvm: introduce KVM_MEMORY_ENCRYPT_OP ioctl
On 02/03/2017 16:17, Brijesh Singh wrote:
> If hardware supports encrypting then KVM_MEMORY_ENCRYPT_OP ioctl can
> be used by qemu to issue platform specific memory encryption commands.
>
> Signed-off-by: Brijesh Singh
> ---
> arch/x86/include/asm/kvm_host.h |2 ++
> arch/x86/kvm/x86.c | 12
> include/uapi/linux/kvm.h|2 ++
> 3 files changed, 16 insertions(+)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index bff1f15..62651ad 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1033,6 +1033,8 @@ struct kvm_x86_ops {
> void (*cancel_hv_timer)(struct kvm_vcpu *vcpu);
>
> void (*setup_mce)(struct kvm_vcpu *vcpu);
> +
> + int (*memory_encryption_op)(struct kvm *kvm, void __user *argp);
> };
>
> struct kvm_arch_async_pf {
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2099df8..6a737e9 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3926,6 +3926,14 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> return r;
> }
>
> +static int kvm_vm_ioctl_memory_encryption_op(struct kvm *kvm, void __user
> *argp)
> +{
> + if (kvm_x86_ops->memory_encryption_op)
> + return kvm_x86_ops->memory_encryption_op(kvm, argp);
> +
> + return -ENOTTY;
> +}
> +
> long kvm_arch_vm_ioctl(struct file *filp,
> unsigned int ioctl, unsigned long arg)
> {
> @@ -4189,6 +4197,10 @@ long kvm_arch_vm_ioctl(struct file *filp,
> r = kvm_vm_ioctl_enable_cap(kvm, &cap);
> break;
> }
> + case KVM_MEMORY_ENCRYPT_OP: {
> + r = kvm_vm_ioctl_memory_encryption_op(kvm, argp);
> + break;
> + }
> default:
> r = kvm_vm_ioctl_assigned_device(kvm, ioctl, arg);
> }
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index cac48ed..fef7d83 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1281,6 +1281,8 @@ struct kvm_s390_ucas_mapping {
> #define KVM_S390_GET_IRQ_STATE _IOW(KVMIO, 0xb6, struct
> kvm_s390_irq_state)
> /* Available with KVM_CAP_X86_SMM */
> #define KVM_SMI _IO(KVMIO, 0xb7)
> +/* Memory Encryption Commands */
> +#define KVM_MEMORY_ENCRYPT_OP _IOWR(KVMIO, 0xb8, unsigned long)
>
> #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
> #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
>
Reviewed-by: Paolo Bonzini
Re: [RFC PATCH v2 24/32] kvm: x86: prepare for SEV guest management API support
On 02/03/2017 16:17, Brijesh Singh wrote:
> ASID management:
> - Reserve asid range for SEV guest, SEV asid range is obtained through
>CPUID Fn8000_001f[ECX]. A non-SEV guest can use any asid outside the SEV
>asid range.
How is backwards compatibility handled?
> - SEV guest must have asid value within asid range obtained through CPUID.
> - SEV guest must have the same asid for all vcpu's. A TLB flush is required
>if different vcpu for the same ASID is to be run on the same host CPU.
[...]
> +
> + /* which host cpu was used for running this vcpu */
> + bool last_cpuid;
Should be unsigned int.
>
> + /* Assign the asid allocated for this SEV guest */
> + svm->vmcb->control.asid = asid;
> +
> + /* Flush guest TLB:
> + * - when different VMCB for the same ASID is to be run on the
> + * same host CPU
> + * or
> + * - this VMCB was executed on different host cpu in previous VMRUNs.
> + */
> + if (sd->sev_vmcbs[asid] != (void *)svm->vmcb ||
Why the cast?
> + svm->last_cpuid != cpu)
> + svm->vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ALL_ASID;
If there is a match, you don't need to do anything else (neither reset
the asid, nor mark it as dirty, nor update the fields), so:
if (sd->sev_vmcbs[asid] == svm->vmcb &&
svm->last_cpuid == cpu)
return;
svm->last_cpuid = cpu;
sd->sev_vmcbs[asid] = svm->vmcb;
svm->vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ALL_ASID;
svm->vmcb->control.asid = asid;
mark_dirty(svm->vmcb, VMCB_ASID);
(plus comments ;)).
Also, why not TLB_CONTROL_FLUSH_ASID if possible?
> + svm->last_cpuid = cpu;
> + sd->sev_vmcbs[asid] = (void *)svm->vmcb;
> +
> + mark_dirty(svm->vmcb, VMCB_ASID);
[...]
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index fef7d83..9df37a2 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1284,6 +1284,104 @@ struct kvm_s390_ucas_mapping {
> /* Memory Encryption Commands */
> #define KVM_MEMORY_ENCRYPT_OP _IOWR(KVMIO, 0xb8, unsigned long)
>
> +/* Secure Encrypted Virtualization mode */
> +enum sev_cmd_id {
Please add documentation in Documentation/virtual/kvm/memory_encrypt.txt.
Paolo
Re: [RFC PATCH v2 32/32] x86: kvm: Pin the guest memory when SEV is active
On 02/03/2017 16:18, Brijesh Singh wrote:
> The SEV memory encryption engine uses a tweak such that two identical
> plaintexts at different location will have a different ciphertexts.
> So swapping or moving ciphertexts of two pages will not result in
> plaintexts being swapped. Relocating (or migrating) a physical backing pages
> for SEV guest will require some additional steps. The current SEV key
> management spec [1] does not provide commands to swap or migrate (move)
> ciphertexts. For now we pin the memory allocated for the SEV guest. In
> future when SEV key management spec provides the commands to support the
> page migration we can update the KVM code to remove the pinning logical
> without making any changes into userspace (qemu).
>
> The patch pins userspace memory when a new slot is created and unpin the
> memory when slot is removed.
>
> [1] http://support.amd.com/TechDocs/55766_SEV-KM%20API_Spec.pdf
This is not enough, because memory can be hidden temporarily from the
guest and remapped later. Think of a PCI BAR that is backed by RAM, or
also SMRAM. The pinning must be kept even in that case.
You need to add a pair of KVM_MEMORY_ENCRYPT_OPs (one that doesn't map
to a PSP operation), such as KVM_REGISTER/UNREGISTER_ENCRYPTED_RAM. In
QEMU you can use a RAMBlockNotifier to invoke the ioctls.
Paolo
> Signed-off-by: Brijesh Singh
> ---
> arch/x86/include/asm/kvm_host.h |6 +++
> arch/x86/kvm/svm.c | 93
> +++
> arch/x86/kvm/x86.c |3 +
> 3 files changed, 102 insertions(+)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index fcc4710..9dc59f0 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -723,6 +723,7 @@ struct kvm_sev_info {
> unsigned int handle;/* firmware handle */
> unsigned int asid; /* asid for this guest */
> int sev_fd; /* SEV device fd */
> + struct list_head pinned_memory_slot;
> };
>
> struct kvm_arch {
> @@ -1043,6 +1044,11 @@ struct kvm_x86_ops {
> void (*setup_mce)(struct kvm_vcpu *vcpu);
>
> int (*memory_encryption_op)(struct kvm *kvm, void __user *argp);
> +
> + void (*prepare_memory_region)(struct kvm *kvm,
> + struct kvm_memory_slot *memslot,
> + const struct kvm_userspace_memory_region *mem,
> + enum kvm_mr_change change);
> };
>
> struct kvm_arch_async_pf {
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 13996d6..ab973f9 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -498,12 +498,21 @@ static inline bool gif_set(struct vcpu_svm *svm)
> }
>
> /* Secure Encrypted Virtualization */
> +struct kvm_sev_pinned_memory_slot {
> + struct list_head list;
> + unsigned long npages;
> + struct page **pages;
> + unsigned long userspace_addr;
> + short id;
> +};
> +
> static unsigned int max_sev_asid;
> static unsigned long *sev_asid_bitmap;
> static void sev_deactivate_handle(struct kvm *kvm);
> static void sev_decommission_handle(struct kvm *kvm);
> static int sev_asid_new(void);
> static void sev_asid_free(int asid);
> +static void sev_unpin_memory(struct page **pages, unsigned long npages);
> #define __sev_page_pa(x) ((page_to_pfn(x) << PAGE_SHIFT) | sme_me_mask)
>
> static bool kvm_sev_enabled(void)
> @@ -1544,9 +1553,25 @@ static inline int avic_free_vm_id(int id)
>
> static void sev_vm_destroy(struct kvm *kvm)
> {
> + struct list_head *pos, *q;
> + struct kvm_sev_pinned_memory_slot *pinned_slot;
> + struct list_head *head = &kvm->arch.sev_info.pinned_memory_slot;
> +
> if (!sev_guest(kvm))
> return;
>
> + /* if guest memory is pinned then unpin it now */
> + if (!list_empty(head)) {
> + list_for_each_safe(pos, q, head) {
> + pinned_slot = list_entry(pos,
> + struct kvm_sev_pinned_memory_slot, list);
> + sev_unpin_memory(pinned_slot->pages,
> + pinned_slot->npages);
> + list_del(pos);
> + kfree(pinned_slot);
> + }
> + }
> +
> /* release the firmware resources */
> sev_deactivate_handle(kvm);
> sev_decommission_handle(kvm);
> @@ -5663,6 +5688,8 @@ static int sev_pre_start(struct kvm *kvm, int *asid)
> }
> *asid = ret;
> ret = 0;
> +
> + INIT_LIST_HEAD(&kvm->arch.sev_info.pinned_memory_slot);
> }
>
> return ret;
> @@ -6189,6 +6216,71 @@ static int sev_launch_measure(struct kvm *kvm, struct
> kvm_sev_cmd *argp)
> return ret;
> }
>
> +static struct kvm_sev_pinned_memory_slot *sev_find_pinned_memory_slot(
> + struct kvm *kvm, struct kvm_memory_slot *slot)
> +{
> + struct kvm_sev_pinned_memory_slot *i;
> +
Re: [RFC PATCH v2 26/32] kvm: svm: Add support for SEV LAUNCH_UPDATE_DATA command
On 02/03/2017 16:17, Brijesh Singh wrote:
> +static struct page **sev_pin_memory(unsigned long uaddr, unsigned long ulen,
> + unsigned long *n)
> +{
> + struct page **pages;
> + int first, last;
> + unsigned long npages, pinned;
> +
> + /* Get number of pages */
> + first = (uaddr & PAGE_MASK) >> PAGE_SHIFT;
> + last = ((uaddr + ulen - 1) & PAGE_MASK) >> PAGE_SHIFT;
> + npages = (last - first + 1);
> +
> + pages = kzalloc(npages * sizeof(struct page *), GFP_KERNEL);
> + if (!pages)
> + return NULL;
> +
> + /* pin the user virtual address */
> + down_read(¤t->mm->mmap_sem);
> + pinned = get_user_pages_fast(uaddr, npages, 1, pages);
> + up_read(¤t->mm->mmap_sem);
get_user_pages_fast, like get_user_pages_unlocked, must be called
without mmap_sem held.
> + if (pinned != npages) {
> + printk(KERN_ERR "SEV: failed to pin %ld pages (got %ld)\n",
> + npages, pinned);
> + goto err;
> + }
> +
> + *n = npages;
> + return pages;
> +err:
> + if (pinned > 0)
> + release_pages(pages, pinned, 0);
> + kfree(pages);
> +
> + return NULL;
> +}
>
> + /* the array of pages returned by get_user_pages() is a page-aligned
> + * memory. Since the user buffer is probably not page-aligned, we need
> + * to calculate the offset within a page for first update entry.
> + */
> + offset = uaddr & (PAGE_SIZE - 1);
> + len = min_t(size_t, (PAGE_SIZE - offset), ulen);
> + ulen -= len;
> +
> + /* update first page -
> + * special care need to be taken for the first page because we might
> + * be dealing with offset within the page
> + */
No need to special case the first page; just set "offset = 0" inside the
loop after the first iteration.
Paolo
> + data->handle = sev_get_handle(kvm);
> + data->length = len;
> + data->address = __sev_page_pa(inpages[0]) + offset;
> + ret = sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE_DATA,
> + data, &argp->error);
> + if (ret)
> + goto err_3;
> +
> + /* update remaining pages */
> + for (i = 1; i < nr_pages; i++) {
> +
> + len = min_t(size_t, PAGE_SIZE, ulen);
> + ulen -= len;
> + data->length = len;
> + data->address = __sev_page_pa(inpages[i]);
> + ret = sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE_DATA,
> + data, &argp->error);
> + if (ret)
> + goto err_3;
> + }
Re: [RFC PATCH v2 29/32] kvm: svm: Add support for SEV DEBUG_DECRYPT command
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
Re: [RFC PATCH v2 30/32] kvm: svm: Add support for SEV DEBUG_ENCRYPT command
On 02/03/2017 16:18, Brijesh Singh wrote:
> + data = (void *) get_zeroed_page(GFP_KERNEL);
The page does not need to be zeroed, does it?
> +
> + if ((len & 15) || (dst_addr & 15)) {
> + /* if destination address and length are not 16-byte
> + * aligned then:
> + * a) decrypt destination page into temporary buffer
> + * b) copy source data into temporary buffer at correct offset
> + * c) encrypt temporary buffer
> + */
> + ret = __sev_dbg_decrypt_page(kvm, dst_addr, data, &argp->error);
Ah, I see now you're using this function here for read-modify-write.
data is already pinned here, so even if you keep the function it makes
sense to push pinning out of __sev_dbg_decrypt_page and into
sev_dbg_decrypt.
> + if (ret)
> + goto err_3;
> + d_off = dst_addr & (PAGE_SIZE - 1);
> +
> + if (copy_from_user(data + d_off,
> + (uint8_t *)debug.src_addr, len)) {
> + ret = -EFAULT;
> + goto err_3;
> + }
> +
> + encrypt->length = PAGE_SIZE;
Why decrypt/re-encrypt all the page instead of just the 16 byte area
around the [dst_addr, dst_addr+len) range?
> + encrypt->src_addr = __psp_pa(data);
> + encrypt->dst_addr = __sev_page_pa(inpages[0]);
> + } else {
> + if (copy_from_user(data, (uint8_t *)debug.src_addr, len)) {
> + ret = -EFAULT;
> + goto err_3;
> + }
Do you need copy_from_user, or can you just pin/unpin memory as for
DEBUG_DECRYPT?
Paolo
> + d_off = dst_addr & (PAGE_SIZE - 1);
> + encrypt->length = len;
> + encrypt->src_addr = __psp_pa(data);
> + encrypt->dst_addr = __sev_page_pa(inpages[0]);
> + encrypt->dst_addr += d_off;
> + }
> +
> + encrypt->handle = sev_get_handle(kvm);
> + ret = sev_issue_cmd(kvm, SEV_CMD_DBG_ENCRYPT, encrypt, &argp->error);
Re: [RFC PATCH v2 16/32] x86: kvm: Provide support to create Guest and HV shared per-CPU variables
On 02/03/2017 16:15, Brijesh Singh wrote:
> Some KVM specific MSR's (steal-time, asyncpf, avic_eio) allocates per-CPU
> variable at compile time and share its physical address with hypervisor.
> It presents a challege when SEV is active in guest OS. When SEV is active,
> guest memory is encrypted with guest key and hypervisor will no longer able
> to modify the guest memory. When SEV is active, we need to clear the
> encryption attribute of shared physical addresses so that both guest and
> hypervisor can access the data.
>
> To solve this problem, I have tried these three options:
>
> 1) Convert the static per-CPU to dynamic per-CPU allocation. When SEV is
> detected then clear the encryption attribute. But while doing so I found
> that per-CPU dynamic allocator was not ready when kvm_guest_cpu_init was
> called.
>
> 2) Since the encryption attributes works on PAGE_SIZE hence add some extra
> padding to 'struct kvm-steal-time' to make it PAGE_SIZE and then at runtime
> clear the encryption attribute of the full PAGE. The downside of this was
> now we need to modify structure which may break the compatibility.
>
> 3) Define a new per-CPU section (.data..percpu.hv_shared) which will be
> used to hold the compile time shared per-CPU variables. When SEV is
> detected we map this section with encryption attribute cleared.
>
> This patch implements #3. It introduces a new DEFINE_PER_CPU_HV_SHAHRED
> macro to create a compile time per-CPU variable. When SEV is detected we
> map the per-CPU variable as decrypted (i.e with encryption attribute cleared).
>
> Signed-off-by: Brijesh Singh
Looks good to me.
Paolo
> ---
> arch/x86/kernel/kvm.c | 43
> +++--
> include/asm-generic/vmlinux.lds.h |3 +++
> include/linux/percpu-defs.h |9
> 3 files changed, 48 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 099fcba..706a08e 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -75,8 +75,8 @@ static int parse_no_kvmclock_vsyscall(char *arg)
>
> early_param("no-kvmclock-vsyscall", parse_no_kvmclock_vsyscall);
>
> -static DEFINE_PER_CPU(struct kvm_vcpu_pv_apf_data, apf_reason) __aligned(64);
> -static DEFINE_PER_CPU(struct kvm_steal_time, steal_time) __aligned(64);
> +static DEFINE_PER_CPU_HV_SHARED(struct kvm_vcpu_pv_apf_data, apf_reason)
> __aligned(64);
> +static DEFINE_PER_CPU_HV_SHARED(struct kvm_steal_time, steal_time)
> __aligned(64);
> static int has_steal_clock = 0;
>
> /*
> @@ -290,6 +290,22 @@ static void __init paravirt_ops_setup(void)
> #endif
> }
>
> +static int kvm_map_percpu_hv_shared(void *addr, unsigned long size)
> +{
> + /* When SEV is active, the percpu static variables initialized
> + * in data section will contain the encrypted data so we first
> + * need to decrypt it and then map it as decrypted.
> + */
> + if (sev_active()) {
> + unsigned long pa = slow_virt_to_phys(addr);
> +
> + sme_early_decrypt(pa, size);
> + return early_set_memory_decrypted(addr, size);
> + }
> +
> + return 0;
> +}
> +
> static void kvm_register_steal_time(void)
> {
> int cpu = smp_processor_id();
> @@ -298,12 +314,17 @@ static void kvm_register_steal_time(void)
> if (!has_steal_clock)
> return;
>
> + if (kvm_map_percpu_hv_shared(st, sizeof(*st))) {
> + pr_err("kvm-stealtime: failed to map hv_shared percpu\n");
> + return;
> + }
> +
> wrmsrl(MSR_KVM_STEAL_TIME, (slow_virt_to_phys(st) | KVM_MSR_ENABLED));
> pr_info("kvm-stealtime: cpu %d, msr %llx\n",
> cpu, (unsigned long long) slow_virt_to_phys(st));
> }
>
> -static DEFINE_PER_CPU(unsigned long, kvm_apic_eoi) = KVM_PV_EOI_DISABLED;
> +static DEFINE_PER_CPU_HV_SHARED(unsigned long, kvm_apic_eoi) =
> KVM_PV_EOI_DISABLED;
>
> static notrace void kvm_guest_apic_eoi_write(u32 reg, u32 val)
> {
> @@ -327,25 +348,33 @@ static void kvm_guest_cpu_init(void)
> if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF) && kvmapf) {
> u64 pa = slow_virt_to_phys(this_cpu_ptr(&apf_reason));
>
> + if (kvm_map_percpu_hv_shared(this_cpu_ptr(&apf_reason),
> + sizeof(struct kvm_vcpu_pv_apf_data)))
> + goto skip_asyncpf;
> #ifdef CONFIG_PREEMPT
> pa |= KVM_ASYNC_PF_SEND_ALWAYS;
> #endif
> wrmsrl(MSR_KVM_ASYNC_PF_EN, pa | KVM_ASYNC_PF_ENABLED);
> __this_cpu_write(apf_reason.enabled, 1);
> - printk(KERN_INFO"KVM setup async PF for cpu %d\n",
> -smp_processor_id());
> + printk(KERN_INFO"KVM setup async PF for cpu %d msr %llx\n",
> +smp_processor_id(), pa);
> }
> -
> +skip_asyncpf:
> if (kvm_para_has_feature(KVM_FEATURE_PV_EOI)) {
> unsigned long pa;
>
Re: [RFC PATCH v2 14/32] x86: mm: Provide support to use memblock when spliting large pages
On 02/03/2017 16:15, Brijesh Singh wrote:
>
> __split_large_page(struct cpa_data *cpa, pte_t *kpte, unsigned long address,
> -struct page *base)
> + pte_t *pbase, unsigned long new_pfn)
> {
> - pte_t *pbase = (pte_t *)page_address(base);
Just one comment and I'll reply to Boris, I think you can compute pbase
with pfn_to_kaddr, and avoid adding a new argument.
>*/
> - __set_pmd_pte(kpte, address, mk_pte(base, __pgprot(_KERNPG_TABLE)));
> + __set_pmd_pte(kpte, address,
> + native_make_pte((new_pfn << PAGE_SHIFT) + _KERNPG_TABLE));
And this probably is better written as:
__set_pmd_pte(kpte, address, pfn_pte(new_pfn, __pgprot(_KERNPG_TABLE));
Paolo
Re: [RFC PATCH v2 14/32] x86: mm: Provide support to use memblock when spliting large pages
On 10/03/2017 23:41, Brijesh Singh wrote:
>> Maybe there's a reason this fires:
>>
>> WARNING: modpost: Found 2 section mismatch(es).
>> To see full details build your kernel with:
>> 'make CONFIG_DEBUG_SECTION_MISMATCH=y'
>>
>> WARNING: vmlinux.o(.text+0x48edc): Section mismatch in reference from
>> the function __change_page_attr() to the function
>> .init.text:memblock_alloc()
>> The function __change_page_attr() references
>> the function __init memblock_alloc().
>> This is often because __change_page_attr lacks a __init
>> annotation or the annotation of memblock_alloc is wrong.
>>
>> WARNING: vmlinux.o(.text+0x491d1): Section mismatch in reference from
>> the function __change_page_attr() to the function
>> .meminit.text:memblock_free()
>> The function __change_page_attr() references
>> the function __meminit memblock_free().
>> This is often because __change_page_attr lacks a __meminit
>> annotation or the annotation of memblock_free is wrong.
>>
>> But maybe Paolo might have an even better idea...
>
> I am sure he will have better idea :)
Not sure if it's better or worse, but an alternative idea is to turn
__change_page_attr and __change_page_attr_set_clr inside out, so that:
1) the alloc_pages/__free_page happens in __change_page_attr_set_clr;
2) __change_page_attr_set_clr overall does not beocome more complex.
Then you can introduce __early_change_page_attr_set_clr and/or
early_kernel_map_pages_in_pgd, for use in your next patches. They use
the memblock allocator instead of alloc/free_page
The attached patch is compile-tested only and almost certainly has some
thinko in it. But it even skims a few lines from the code so the idea
might have some merit.
Paolo
diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 28d42130243c..953c8e697562 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -490,11 +490,12 @@ static void __set_pmd_pte(pte_t *kpte, unsigned long address, pte_t pte)
}
static int
-try_preserve_large_page(pte_t *kpte, unsigned long address,
+try_preserve_large_page(pte_t **p_kpte, unsigned long address,
struct cpa_data *cpa)
{
unsigned long nextpage_addr, numpages, pmask, psize, addr, pfn, old_pfn;
- pte_t new_pte, old_pte, *tmp;
+ pte_t *kpte = *p_kpte;
+ pte_t new_pte, old_pte;
pgprot_t old_prot, new_prot, req_prot;
int i, do_split = 1;
enum pg_level level;
@@ -507,8 +508,8 @@ try_preserve_large_page(pte_t *kpte, unsigned long address,
* Check for races, another CPU might have split this page
* up already:
*/
- tmp = _lookup_address_cpa(cpa, address, &level);
- if (tmp != kpte)
+ *p_kpte = _lookup_address_cpa(cpa, address, &level);
+ if (*p_kpte != kpte)
goto out_unlock;
switch (level) {
@@ -634,17 +635,18 @@ __split_large_page(struct cpa_data *cpa, pte_t *kpte, unsigned long address,
unsigned int i, level;
pte_t *tmp;
pgprot_t ref_prot;
+ int retry = 1;
+ if (!debug_pagealloc_enabled())
+ spin_lock(&cpa_lock);
spin_lock(&pgd_lock);
/*
* Check for races, another CPU might have split this page
* up for us already:
*/
tmp = _lookup_address_cpa(cpa, address, &level);
- if (tmp != kpte) {
- spin_unlock(&pgd_lock);
- return 1;
- }
+ if (tmp != kpte)
+ goto out;
paravirt_alloc_pte(&init_mm, page_to_pfn(base));
@@ -671,10 +673,11 @@ __split_large_page(struct cpa_data *cpa, pte_t *kpte, unsigned long address,
break;
default:
- spin_unlock(&pgd_lock);
- return 1;
+ goto out;
}
+ retry = 0;
+
/*
* Set the GLOBAL flags only if the PRESENT flag is set
* otherwise pmd/pte_present will return true even on a non
@@ -718,28 +721,34 @@ __split_large_page(struct cpa_data *cpa, pte_t *kpte, unsigned long address,
* going on.
*/
__flush_tlb_all();
- spin_unlock(&pgd_lock);
- return 0;
-}
-
-static int split_large_page(struct cpa_data *cpa, pte_t *kpte,
- unsigned long address)
-{
- struct page *base;
+out:
+ spin_unlock(&pgd_lock);
+ /*
+ * Do a global flush tlb after splitting the large page
+ * and before we do the actual change page attribute in the PTE.
+ *
+ * With out this, we violate the TLB application note, that says
+ * "The TLBs may contain both ordinary and large-page
+ * translations for a 4-KByte range of linear addresses. This
+ * may occur if software modifies the paging structures so that
+ * the page size used for the address range changes. If the two
+ * translations differ with respect to page frame or attributes
+ * (e.g., permissions), processor behavior is undefined and may
+ * be implementation-specific."
+ *
+ * We do this global tlb flush inside the cpa_lock, so that we
+ * don't allow any other cpu, with stale tlb entries change the
+ * page attribute in parallel, that also falls into the
+ * just split large page entry.
+ */
+ if (!retry)
+ flush_tlb_all();
if (!debug_pagealloc_enabled())
spin_unlock(&cpa_lock);
- base = alloc_pages(GFP_KERNEL | __GFP_NOTRACK, 0);
- if (!debug_pagealloc_enabled())
-
Re: [RFC PATCH v2 29/32] kvm: svm: Add support for SEV DEBUG_DECRYPT command
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
Re: [RFC PATCH v2 16/32] x86: kvm: Provide support to create Guest and HV shared per-CPU variables
On 28/03/2017 20:39, Borislav Petkov wrote: >> 2) Since the encryption attributes works on PAGE_SIZE hence add some extra >> padding to 'struct kvm-steal-time' to make it PAGE_SIZE and then at runtime >> clear the encryption attribute of the full PAGE. The downside of this was >> now we need to modify structure which may break the compatibility. > From SEV-ES whitepaper: > > "To facilitate this communication, the SEV-ES architecture defines > a Guest Hypervisor Communication Block (GHCB). The GHCB resides in > page of shared memory so it is accessible to both the guest VM and the > hypervisor." > > So this is kinda begging to be implemented with a shared page between > guest and host. And then put steal-time, ... etc in there too. Provided > there's enough room in the single page for the GHCB *and* our stuff. The GHCB would have to be allocated much earlier, possibly even by firmware depending on how things will be designed. I think it's premature to consider SEV-ES requirements. Paolo
Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
On 22/09/2016 16:35, Borislav Petkov wrote: >> > @@ -230,6 +230,10 @@ int __init efi_setup_page_tables(unsigned long >> > pa_memmap, unsigned num_pages) >> >efi_scratch.efi_pgt = (pgd_t *)__sme_pa(efi_pgd); >> >pgd = efi_pgd; >> > >> > + flags = _PAGE_NX | _PAGE_RW; >> > + if (sev_active) >> > + flags |= _PAGE_ENC; > So this is confusing me. There's this patch which says EFI data is > accessed in the clear: > > https://lkml.kernel.org/r/2016083738.29880.6909.st...@tlendack-t1.amdoffice.net > > but now here it is encrypted when SEV is enabled. > > Do you mean, it is encrypted here because we're in the guest kernel? I suspect this patch is untested, and also wrong. :) The main difference between the SME and SEV encryption, from the point of view of the kernel, is that real-mode always writes unencrypted in SME and always writes encrypted in SEV. But UEFI can run in 64-bit mode and learn about the C bit, so EFI boot data should be unprotected in SEV guests. Because the firmware volume is written to high memory in encrypted form, and because the PEI phase runs in 32-bit mode, the firmware code will be encrypted; on the other hand, data that is placed in low memory for the kernel can be unencrypted, thus limiting differences between SME and SEV. Important: I don't know what you guys are doing for SEV and Windows guests, but if you are doing something I would really appreciate doing things in the open. If Linux and Windows end up doing different things with EFI boot data, ACPI tables, etc. it will be a huge pain. On the other hand, if we can enjoy being first, that's great. In fact, I have suggested in the QEMU list that SEV guests should always use UEFI; because BIOS runs in real-mode or 32-bit non-paging protected mode, BIOS must always write encrypted data, which becomes painful in the kernel. And regarding the above "important" point, all I know is that Microsoft for sure will be happy to restrict SEV to UEFI guests. :) There are still some differences, mostly around the real mode trampoline executed by the kernel, but they should be much smaller. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
On 22/09/2016 16:59, Borislav Petkov wrote: > On Thu, Sep 22, 2016 at 04:45:51PM +0200, Paolo Bonzini wrote: >> The main difference between the SME and SEV encryption, from the point >> of view of the kernel, is that real-mode always writes unencrypted in >> SME and always writes encrypted in SEV. But UEFI can run in 64-bit mode >> and learn about the C bit, so EFI boot data should be unprotected in SEV >> guests. > > Actually, it is different: you can start fully encrypted in SME, see: > > https://lkml.kernel.org/r/2016083539.29880.96739.st...@tlendack-t1.amdoffice.net > > The last paragraph alludes to a certain transparent mode where you're > already encrypted and only certain pieces like EFI is not encrypted. Which paragraph? >> Because the firmware volume is written to high memory in encrypted >> form, and because the PEI phase runs in 32-bit mode, the firmware >> code will be encrypted; on the other hand, data that is placed in low >> memory for the kernel can be unencrypted, thus limiting differences >> between SME and SEV. > > When you run fully encrypted, you still need to access EFI tables in the > clear. That's why I'm confused about this patch here. I might be wrong, but I don't think this patch was tested with OVMF or Duet. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
On 22/09/2016 19:07, Borislav Petkov wrote: >> Which paragraph? > "Linux relies on BIOS to set this bit if BIOS has determined that the > reduction in the physical address space as a result of enabling memory > encryption..." > > Basically, you can enable SME in the BIOS and you're all set. That's not how I read it. I just figured that the BIOS has some magic things high in the physical address space and if you reduce the physical address space the BIOS (which is called from e.g. EFI runtime services) would have problems with that. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
On 22/09/2016 19:46, Tom Lendacky wrote: >> > Do you mean, it is encrypted here because we're in the guest kernel? > Yes, the idea is that the SEV guest will be running encrypted from the > start, including the BIOS/UEFI, and so all of the EFI related data will > be encrypted. Unless this is part of some spec, it's easier if things are the same in SME and SEV. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
On 22/09/2016 20:47, Tom Lendacky wrote: > > Because the firmware volume is written to high memory in encrypted form, > > and because the PEI phase runs in 32-bit mode, the firmware code will be > > encrypted; on the other hand, data that is placed in low memory for the > > kernel can be unencrypted, thus limiting differences between SME and SEV. > > I like the idea of limiting the differences but it would leave the EFI > data and ACPI tables exposed and able to be manipulated. Hmm, that makes sense. So I guess this has to stay, and Borislav's proposal doesn't fly either. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
single_task_running() vs. preemption warnings (was Re: [PATCH] kvm: fix preemption warnings in kvm_vcpu_block)
On 17/09/2015 18:27, Dominik Dingel wrote:
> + preempt_disable();
> + solo = single_task_running();
> + preempt_enable();
> +
> cur = ktime_get();
> - } while (single_task_running() && ktime_before(cur, stop));
That's the obvious way to fix it, but the TOCTTOU problem (which was in
the buggy code too) is obvious too. :) And the only other user of
single_task_running() in drivers/crypto/mcryptd.c has the same issue.
In fact, because of the way the function is used ("maybe I can do a
little bit of work before going to sleep") it will likely be called many
times in a loop. This in turn means that:
- any wrong result due to a concurrent process migration would be
rectified very soon
- preempt_disable()/preempt_enable() can actually be just as expensive
or more expensive than single_task_running() itself.
Therefore, I wonder if single_task_running() should just use
raw_smp_processor_id(). At least the TOCTTOU issue can be clearly
documented in the function comment, instead of being hidden behind each
of the callers.
Thanks,
Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] KVM/SVM: add support for SEV attestation command
On 04/01/21 16:17, Brijesh Singh wrote:
The SEV FW version >= 0.23 added a new command that can be used to query
the attestation report containing the SHA-256 digest of the guest memory
encrypted through the KVM_SEV_LAUNCH_UPDATE_{DATA, VMSA} commands and
sign the report with the Platform Endorsement Key (PEK).
See the SEV FW API spec section 6.8 for more details.
Note there already exist a command (KVM_SEV_LAUNCH_MEASURE) that can be
used to get the SHA-256 digest. The main difference between the
KVM_SEV_LAUNCH_MEASURE and KVM_SEV_ATTESTATION_REPORT is that the latter
can be called while the guest is running and the measurement value is
signed with PEK.
Cc: James Bottomley
Cc: Tom Lendacky
Cc: David Rientjes
Cc: Paolo Bonzini
Cc: Sean Christopherson
Cc: Borislav Petkov
Cc: John Allen
Cc: Herbert Xu
Cc: linux-crypto@vger.kernel.org
Reviewed-by: Tom Lendacky
Acked-by: David Rientjes
Tested-by: James Bottomley
Signed-off-by: Brijesh Singh
---
v2:
* Fix documentation typo
.../virt/kvm/amd-memory-encryption.rst| 21 ++
arch/x86/kvm/svm/sev.c| 71 +++
drivers/crypto/ccp/sev-dev.c | 1 +
include/linux/psp-sev.h | 17 +
include/uapi/linux/kvm.h | 8 +++
5 files changed, 118 insertions(+)
diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst
b/Documentation/virt/kvm/amd-memory-encryption.rst
index 09a8f2a34e39..469a6308765b 100644
--- a/Documentation/virt/kvm/amd-memory-encryption.rst
+++ b/Documentation/virt/kvm/amd-memory-encryption.rst
@@ -263,6 +263,27 @@ Returns: 0 on success, -negative on error
__u32 trans_len;
};
+10. KVM_SEV_GET_ATTESTATION_REPORT
+--
+
+The KVM_SEV_GET_ATTESTATION_REPORT command can be used by the hypervisor to
query the attestation
+report containing the SHA-256 digest of the guest memory and VMSA passed
through the KVM_SEV_LAUNCH
+commands and signed with the PEK. The digest returned by the command should
match the digest
+used by the guest owner with the KVM_SEV_LAUNCH_MEASURE.
+
+Parameters (in): struct kvm_sev_attestation
+
+Returns: 0 on success, -negative on error
+
+::
+
+struct kvm_sev_attestation_report {
+__u8 mnonce[16];/* A random mnonce that will be placed
in the report */
+
+__u64 uaddr;/* userspace address where the report
should be copied */
+__u32 len;
+};
+
References
==
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 566f4d18185b..c4d3ee6be362 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -927,6 +927,74 @@ static int sev_launch_secret(struct kvm *kvm, struct
kvm_sev_cmd *argp)
return ret;
}
+static int sev_get_attestation_report(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+ void __user *report = (void __user *)(uintptr_t)argp->data;
+ struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+ struct sev_data_attestation_report *data;
+ struct kvm_sev_attestation_report params;
+ void __user *p;
+ void *blob = NULL;
+ int ret;
+
+ if (!sev_guest(kvm))
+ return -ENOTTY;
+
+ if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data,
sizeof(params)))
+ return -EFAULT;
+
+ data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
+ if (!data)
+ return -ENOMEM;
+
+ /* User wants to query the blob length */
+ if (!params.len)
+ goto cmd;
+
+ p = (void __user *)(uintptr_t)params.uaddr;
+ if (p) {
+ if (params.len > SEV_FW_BLOB_MAX_SIZE) {
+ ret = -EINVAL;
+ goto e_free;
+ }
+
+ ret = -ENOMEM;
+ blob = kmalloc(params.len, GFP_KERNEL);
+ if (!blob)
+ goto e_free;
+
+ data->address = __psp_pa(blob);
+ data->len = params.len;
+ memcpy(data->mnonce, params.mnonce, sizeof(params.mnonce));
+ }
+cmd:
+ data->handle = sev->handle;
+ ret = sev_issue_cmd(kvm, SEV_CMD_ATTESTATION_REPORT, data,
&argp->error);
+ /*
+* If we query the session length, FW responded with expected data.
+*/
+ if (!params.len)
+ goto done;
+
+ if (ret)
+ goto e_free_blob;
+
+ if (blob) {
+ if (copy_to_user(p, blob, params.len))
+ ret = -EFAULT;
+ }
+
+done:
+ params.len = data->len;
+ if (copy_to_user(report, ¶ms, sizeof(params)))
+ ret = -EFAULT;
+e_free_blob:
+ kfree(blob);
+e_free:
+ kfree(data);
+ return ret;
+}
+
int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
{
struct kvm_sev_cmd sev_
Re: [Part2 PATCH v9 00/38] x86: Secure Encrypted Virtualization (AMD)
On 05/12/2017 02:04, Brijesh Singh wrote: > This part of Secure Encrypted Virtualization (SEV) patch series focuses on KVM > changes required to create and manage SEV guests. > > SEV is an extension to the AMD-V architecture which supports running encrypted > virtual machine (VMs) under the control of a hypervisor. Encrypted VMs have > their > pages (code and data) secured such that only the guest itself has access to > unencrypted version. Each encrypted VM is associated with a unique encryption > key; > if its data is accessed to a different entity using a different key the > encrypted > guest's data will be incorrectly decrypted, leading to unintelligible data. > This security model ensures that hypervisor will no longer able to inspect or > alter any guest code or data. > > The key management of this feature is handled by a separate processor known as > the AMD Secure Processor (AMD-SP) which is present on AMD SOCs. The SEV Key > Management Specification (see below) provides a set of commands which can be > used by hypervisor to load virtual machine keys through the AMD-SP driver. > > The patch series adds a new ioctl in KVM driver (KVM_MEMORY_ENCRYPT_OP). The > ioctl will be used by qemu to issue SEV guest-specific commands defined in Key > Management Specification. Hi Brijesh, I have a couple comments: 1) how is MSR_AMD64_SEV's value passed to the guest, and where is it in the manual? 2) ECX should be 0 in the guest's 0x8000_001f leaf, because we don't support nested SEV guests. Likewise, EAX bit 2 should be 0 since you don't emulate the page flush MSR. Both can be fixed on top (and I can do the second myself of course), so there should be no need for a v10. But MSR_AMD64_SEV is leaving me quite puzzled. Thanks, Paolo > The following links provide additional details: > > AMD Memory Encryption white paper: > http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf > > AMD64 Architecture Programmer's Manual: > http://support.amd.com/TechDocs/24593.pdf > SME is section 7.10 > SEV is section 15.34 > > SEV Key Management: > http://support.amd.com/TechDocs/55766_SEV-KM API_Specification.pdf > > KVM Forum Presentation: > http://www.linux-kvm.org/images/7/74/02x08A-Thomas_Lendacky-AMDs_Virtualizatoin_Memory_Encryption_Technology.pdf > > SEV Guest BIOS support: > SEV support has been add to EDKII/OVMF BIOS > https://github.com/tianocore/edk2 > > -- > > The series applies on kvm/next commit : 4fbd8d194f06 (Linux 4.15-rc1) > > Complete tree is available at: > repo: https://github.com/codomania/kvm.git > branch: sev-v9-p2 > > TODO: > * Add SEV guest migration command support > > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: "H. Peter Anvin" > Cc: Paolo Bonzini > Cc: "Radim KrÄmář" > Cc: Joerg Roedel > Cc: Borislav Petkov > Cc: Tom Lendacky > Cc: Herbert Xu > Cc: David S. Miller > Cc: Gary Hook > Cc: x...@kernel.org > Cc: k...@vger.kernel.org > Cc: linux-ker...@vger.kernel.org > Cc: linux-crypto@vger.kernel.org > > Changes since v8: > * Rebase the series to kvm/next branch > * Update SEV asid allocation to limit the ASID between SEV_MIN_ASID to > SEV_MAX_ASID >(EPYC BIOS provide option to change the SEV_MIN_ASID -- which can be used > to >limit the number of SEV-enable guest) > > Changes since v7: > * Rebase the series to kvm/next branch > * move the FW error enum definition in include/uapi/linux/psp-sev.h so that >both userspace and kernel can share it. > * (ccp) drop cmd_buf arg from sev_platform_init() > * (ccp) apply some cleanup/fixup from Boris > * (ccp) add some comments in FACTORY_RESET command handling > * (kvm) some fixup/cleanup from Boris > * (kvm) acquire the kvm->lock when modifying the sev->regions_list > > Changes since v6: > * (ccp): Extend psp_device structure to track the FW INIT and SHUTDOWN > states. > * (ccp): Init and Uninit SEV FW during module load/unload > * (ccp): Avoid repeated k*alloc() for init and status command buffer > * (kvm): Rework DBG command to fix the compilation warning seen with gcc7.x > * (kvm): Convert the SEV doc in rst format > > Changes since v5: > * split the PSP driver support into multiple patches > * multiple improvements from Boris > * remove mem_enc_enabled() ops > > Changes since v4: > * Fixes to address kbuild robot errors > * Add 'sev' module params to allow enable/disable SEV feature > * Update documentation > * Multiple fixes to address v4 feedbacks > * Some coding style changes to address checkpatch reports > > Changes sin
Re: [Part2 PATCH v9 00/38] x86: Secure Encrypted Virtualization (AMD)
On 05/12/2017 02:04, Brijesh Singh wrote: > This part of Secure Encrypted Virtualization (SEV) patch series focuses on KVM > changes required to create and manage SEV guests. > > SEV is an extension to the AMD-V architecture which supports running encrypted > virtual machine (VMs) under the control of a hypervisor. Encrypted VMs have > their > pages (code and data) secured such that only the guest itself has access to > unencrypted version. Each encrypted VM is associated with a unique encryption > key; > if its data is accessed to a different entity using a different key the > encrypted > guest's data will be incorrectly decrypted, leading to unintelligible data. > This security model ensures that hypervisor will no longer able to inspect or > alter any guest code or data. > > The key management of this feature is handled by a separate processor known as > the AMD Secure Processor (AMD-SP) which is present on AMD SOCs. The SEV Key > Management Specification (see below) provides a set of commands which can be > used by hypervisor to load virtual machine keys through the AMD-SP driver. > > The patch series adds a new ioctl in KVM driver (KVM_MEMORY_ENCRYPT_OP). The > ioctl will be used by qemu to issue SEV guest-specific commands defined in Key > Management Specification. > > The following links provide additional details: > > AMD Memory Encryption white paper: > http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf > > AMD64 Architecture Programmer's Manual: > http://support.amd.com/TechDocs/24593.pdf > SME is section 7.10 > SEV is section 15.34 > > SEV Key Management: > http://support.amd.com/TechDocs/55766_SEV-KM API_Specification.pdf > > KVM Forum Presentation: > http://www.linux-kvm.org/images/7/74/02x08A-Thomas_Lendacky-AMDs_Virtualizatoin_Memory_Encryption_Technology.pdf > > SEV Guest BIOS support: > SEV support has been add to EDKII/OVMF BIOS > https://github.com/tianocore/edk2 Merged! Thanks, Paolo
Re: [PATCH 0/4] KVM: SVM: kbuild test robot warning fixes
On 15/01/2018 14:32, Brijesh Singh wrote: > The patch series fixes the warnings reported by kbuild test robot > after SEV patches. Additionally, during testing I found that LAUNCH_SECRET > command was broken and patch series contains the fix for it. > > The patch series applies on kvm/queue branch. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav Petkov > Cc: Herbert Xu > Cc: Gary Hook > Cc: Tom Lendacky > Cc: Joerg Roedel > Cc: linux-crypto@vger.kernel.org > Cc: linux-ker...@vger.kernel.org > > Brijesh Singh (4): > crypto: ccp: Fix sparse, use plain integer as NULL pointer > KVM: SVM: Fix sparse: incorrect type in argument 1 (different base > types) > include: psp-sev: Capitalize invalid length enum > KVM: SVM: Fix SEV LAUNCH_SECRET command > > arch/x86/kvm/svm.c | 14 ++ > drivers/crypto/ccp/psp-dev.c | 8 > include/uapi/linux/psp-sev.h | 2 +- > 3 files changed, 15 insertions(+), 9 deletions(-) > Applying patches 1 and 3 from this series, thanks. Paolo
Re: Wrong system clock vs X.509 date specifiers
Il 25/09/2012 17:35, David Howells ha scritto: > Alan Cox wrote: > >> > Generate a certificate that is valid from a few minutes before the >> > wallclock time. It's a certificate policy question not a kernel hackery >> > one. > That doesn't seem to be possible with openssl req. What would you recommend? Disgusting, but: add an LD_PRELOAD library that returns a time well in the past. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
