| Message ID | 20170724200303.12197-11-brijesh.singh@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 24, 2017 at 03:02:47PM -0500, Brijesh Singh wrote: > If hardware support memory encryption then KVM_MEMORY_REGISTER_RAM and supports > KVM_MEMORY_UNREGISTER_RAM ioctl's can be used by userspace to register/ > unregister the guest memory regions which may contains the encrypted "... which may contain encrypted data ... " > data (e.g guest RAM, PCI BAR, SMRAM etc). > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > arch/x86/include/asm/kvm_host.h | 4 ++++ > arch/x86/kvm/x86.c | 36 ++++++++++++++++++++++++++++++++++++ > include/uapi/linux/kvm.h | 9 +++++++++ > 3 files changed, 49 insertions(+) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 99a0e11..4295f82 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1059,6 +1059,10 @@ struct kvm_x86_ops { > void (*setup_mce)(struct kvm_vcpu *vcpu); > > int (*memory_encryption_op)(struct kvm *kvm, void __user *argp); > + int (*memory_encryption_register_ram)(struct kvm *kvm, > + struct kvm_memory_encrypt_ram *ram); > + int (*memory_encryption_unregister_ram)(struct kvm *kvm, > + struct kvm_memory_encrypt_ram *ram); > }; You can shorten those prefixes to "mem_enc" or so and struct kvm_memory_encrypt_ram to struct enc_region - which is exactly what it is - an encrypted memory region descriptor - and then fit each function on a single line. > > struct kvm_arch_async_pf { > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index c9d3ff5..8febdb5 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3982,6 +3982,24 @@ static int kvm_vm_ioctl_memory_encryption_op(struct kvm *kvm, void __user *argp) > return -ENOTTY; > } > > +static int kvm_vm_ioctl_mem_encrypt_register_ram(struct kvm *kvm, > + struct kvm_memory_encrypt_ram *ram) > +{ > + if (kvm_x86_ops->memory_encryption_register_ram) > + return kvm_x86_ops->memory_encryption_register_ram(kvm, ram); > + > + return -ENOTTY; > +} > + > +static int kvm_vm_ioctl_mem_encrypt_unregister_ram(struct kvm *kvm, > + struct kvm_memory_encrypt_ram *ram) > +{ > + if (kvm_x86_ops->memory_encryption_unregister_ram) > + return kvm_x86_ops->memory_encryption_unregister_ram(kvm, ram); > + > + return -ENOTTY; > +} > + > long kvm_arch_vm_ioctl(struct file *filp, > unsigned int ioctl, unsigned long arg) > { > @@ -4246,6 +4264,24 @@ long kvm_arch_vm_ioctl(struct file *filp, > r = kvm_vm_ioctl_memory_encryption_op(kvm, argp); > break; > } > + case KVM_MEMORY_ENCRYPT_REGISTER_RAM: { > + struct kvm_memory_encrypt_ram ram; > + > + r = -EFAULT; > + if (copy_from_user(&ram, argp, sizeof(ram))) > + goto out; > + r = kvm_vm_ioctl_mem_encrypt_register_ram(kvm, &ram); > + break; > + } > + case KVM_MEMORY_ENCRYPT_UNREGISTER_RAM: { > + struct kvm_memory_encrypt_ram ram; > + > + r = -EFAULT; > + if (copy_from_user(&ram, argp, sizeof(ram))) As earlier, those ioctl functions need to check the user-supplied values. > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index ab3b711..6074065 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -1357,6 +1357,15 @@ struct kvm_s390_ucas_mapping { > #define KVM_S390_SET_CMMA_BITS _IOW(KVMIO, 0xb9, struct kvm_s390_cmma_log) > /* Memory Encryption Commands */ > #define KVM_MEMORY_ENCRYPT_OP _IOWR(KVMIO, 0xba, unsigned long) > +#define KVM_MEMORY_ENCRYPT_REGISTER_RAM _IOR(KVMIO, 0xbb, \ > + struct kvm_memory_encrypt_ram) > +#define KVM_MEMORY_ENCRYPT_UNREGISTER_RAM _IOR(KVMIO, 0xbc, \ > + struct kvm_memory_encrypt_ram) As with KVM_MEMORY_ENCRYPT_OP, those two need to be in the KVM API document.
On 09/12/2017 03:29 PM, Borislav Petkov wrote: ... >> + int (*memory_encryption_unregister_ram)(struct kvm *kvm, >> + struct kvm_memory_encrypt_ram *ram); >> }; > > You can shorten those prefixes to "mem_enc" or so and struct > kvm_memory_encrypt_ram to struct enc_region - which is exactly what it > is - an encrypted memory region descriptor - and then fit each function > on a single line. > Sure, I can do that. In one of the feedback Paolo recommended KVM_MEMORY_ENCRYPT_* ioctl name hence I tried to stick with the same name for structure. I am flexible to use 'struct enc_region' but I personally prefer to keep "mem" somewhere in the structure naming to indicate its for *memory* encryption -- maybe struct kvm_mem_enc_region. ... >> + struct kvm_memory_encrypt_ram) > > As with KVM_MEMORY_ENCRYPT_OP, those two need to be in the KVM API document. > Yes, I missed updating the Documentation/virtual/kvm/api.txt for these new ioctls. I will update it.
On Tue, Sep 12, 2017 at 03:50:55PM -0500, Brijesh Singh wrote: > Sure, I can do that. In one of the feedback Paolo recommended > KVM_MEMORY_ENCRYPT_* ioctl name hence I tried to stick with the same name > for structure. I am flexible to use 'struct enc_region' but I personally > prefer to keep "mem" somewhere in the structure naming to indicate its for > *memory* encryption -- maybe struct kvm_mem_enc_region. I'd drop the "kvm_" prefix: struct mem_enc_region; But I sense we're bikeshedding here and I'd leave it up to you guys. My only intent was to shorten those so that they fit on a single line. In general, I like using small struct names which do not look like sentences and which give you code that reads quickly, at a glance. But this is just me.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 99a0e11..4295f82 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1059,6 +1059,10 @@ struct kvm_x86_ops { void (*setup_mce)(struct kvm_vcpu *vcpu); int (*memory_encryption_op)(struct kvm *kvm, void __user *argp); + int (*memory_encryption_register_ram)(struct kvm *kvm, + struct kvm_memory_encrypt_ram *ram); + int (*memory_encryption_unregister_ram)(struct kvm *kvm, + struct kvm_memory_encrypt_ram *ram); }; struct kvm_arch_async_pf { diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c9d3ff5..8febdb5 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3982,6 +3982,24 @@ static int kvm_vm_ioctl_memory_encryption_op(struct kvm *kvm, void __user *argp) return -ENOTTY; } +static int kvm_vm_ioctl_mem_encrypt_register_ram(struct kvm *kvm, + struct kvm_memory_encrypt_ram *ram) +{ + if (kvm_x86_ops->memory_encryption_register_ram) + return kvm_x86_ops->memory_encryption_register_ram(kvm, ram); + + return -ENOTTY; +} + +static int kvm_vm_ioctl_mem_encrypt_unregister_ram(struct kvm *kvm, + struct kvm_memory_encrypt_ram *ram) +{ + if (kvm_x86_ops->memory_encryption_unregister_ram) + return kvm_x86_ops->memory_encryption_unregister_ram(kvm, ram); + + return -ENOTTY; +} + long kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) { @@ -4246,6 +4264,24 @@ long kvm_arch_vm_ioctl(struct file *filp, r = kvm_vm_ioctl_memory_encryption_op(kvm, argp); break; } + case KVM_MEMORY_ENCRYPT_REGISTER_RAM: { + struct kvm_memory_encrypt_ram ram; + + r = -EFAULT; + if (copy_from_user(&ram, argp, sizeof(ram))) + goto out; + r = kvm_vm_ioctl_mem_encrypt_register_ram(kvm, &ram); + break; + } + case KVM_MEMORY_ENCRYPT_UNREGISTER_RAM: { + struct kvm_memory_encrypt_ram ram; + + r = -EFAULT; + if (copy_from_user(&ram, argp, sizeof(ram))) + goto out; + r = kvm_vm_ioctl_mem_encrypt_unregister_ram(kvm, &ram); + break; + } default: r = -ENOTTY; } diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index ab3b711..6074065 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -1357,6 +1357,15 @@ struct kvm_s390_ucas_mapping { #define KVM_S390_SET_CMMA_BITS _IOW(KVMIO, 0xb9, struct kvm_s390_cmma_log) /* Memory Encryption Commands */ #define KVM_MEMORY_ENCRYPT_OP _IOWR(KVMIO, 0xba, unsigned long) +#define KVM_MEMORY_ENCRYPT_REGISTER_RAM _IOR(KVMIO, 0xbb, \ + struct kvm_memory_encrypt_ram) +#define KVM_MEMORY_ENCRYPT_UNREGISTER_RAM _IOR(KVMIO, 0xbc, \ + struct kvm_memory_encrypt_ram) + +struct kvm_memory_encrypt_ram { + __u64 address; + __u64 size; +}; #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0) #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
If hardware support memory encryption then KVM_MEMORY_REGISTER_RAM and KVM_MEMORY_UNREGISTER_RAM ioctl's can be used by userspace to register/ unregister the guest memory regions which may contains the encrypted data (e.g guest RAM, PCI BAR, SMRAM etc). Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- arch/x86/include/asm/kvm_host.h | 4 ++++ arch/x86/kvm/x86.c | 36 ++++++++++++++++++++++++++++++++++++ include/uapi/linux/kvm.h | 9 +++++++++ 3 files changed, 49 insertions(+)