Message ID | 2d65bd56a7ab8c0776f5c6b7c8481dd45ad96794.1659854790.git.isaku.yamahata@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | KVM TDX basic feature support | expand |
On 2022/8/8 6:01, isaku.yamahata@intel.com wrote: > From: Isaku Yamahata <isaku.yamahata@intel.com> > > Add a place holder function for TDX specific VM-scoped ioctl as mem_enc_op. > TDX specific sub-commands will be added to retrieve/pass TDX specific > parameters. > > KVM_MEMORY_ENCRYPT_OP was introduced for VM-scoped operations specific for > guest state-protected VM. It defined subcommands for technology-specific > operations under KVM_MEMORY_ENCRYPT_OP. Despite its name, the subcommands > are not limited to memory encryption, but various technology-specific > operations are defined. It's natural to repurpose KVM_MEMORY_ENCRYPT_OP > for TDX specific operations and define subcommands. > > TDX requires VM-scoped TDX-specific operations for device model, for > example, qemu. Getting system-wide parameters, TDX-specific VM > initialization. > > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> > --- > arch/x86/kvm/vmx/main.c | 9 +++++++++ > arch/x86/kvm/vmx/tdx.c | 26 ++++++++++++++++++++++++++ > arch/x86/kvm/vmx/x86_ops.h | 4 ++++ > 3 files changed, 39 insertions(+) > > diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c > index 7b497ed1f21c..067f5de56c53 100644 > --- a/arch/x86/kvm/vmx/main.c > +++ b/arch/x86/kvm/vmx/main.c > @@ -73,6 +73,14 @@ static void vt_vm_free(struct kvm *kvm) > return tdx_vm_free(kvm); > } > > +static int vt_mem_enc_ioctl(struct kvm *kvm, void __user *argp) > +{ > + if (!is_td(kvm)) > + return -ENOTTY; > + > + return tdx_vm_ioctl(kvm, argp); > +} > + > struct kvm_x86_ops vt_x86_ops __initdata = { > .name = "kvm_intel", > > @@ -214,6 +222,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = { > .vcpu_deliver_sipi_vector = kvm_vcpu_deliver_sipi_vector, > > .dev_mem_enc_ioctl = tdx_dev_ioctl, > + .mem_enc_ioctl = vt_mem_enc_ioctl, suggeust to align the interafce/function name style with the scop. patch 21 and 27 have the scope in interafce names(dev / vcpu), may be clearer to useĀ vcpu_mem_enc_ioctl? > }; > > struct kvm_x86_init_ops vt_init_ops __initdata = { > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c > index 16c6570dbe52..d3b9f653da4b 100644 > --- a/arch/x86/kvm/vmx/tdx.c > +++ b/arch/x86/kvm/vmx/tdx.c > @@ -424,6 +424,32 @@ int tdx_dev_ioctl(void __user *argp) > return 0; > } > > +int tdx_vm_ioctl(struct kvm *kvm, void __user *argp) > +{ > + struct kvm_tdx_cmd tdx_cmd; > + int r; > + > + if (copy_from_user(&tdx_cmd, argp, sizeof(struct kvm_tdx_cmd))) > + return -EFAULT; > + if (tdx_cmd.error || tdx_cmd.unused) > + return -EINVAL; > + > + mutex_lock(&kvm->lock); > + > + switch (tdx_cmd.id) { > + default: > + r = -EINVAL; > + goto out; > + } > + > + if (copy_to_user(argp, &tdx_cmd, sizeof(struct kvm_tdx_cmd))) > + r = -EFAULT; > + > +out: > + mutex_unlock(&kvm->lock); > + return r; > +} > + > int __init tdx_module_setup(void) > { > const struct tdsysinfo_struct *tdsysinfo; > diff --git a/arch/x86/kvm/vmx/x86_ops.h b/arch/x86/kvm/vmx/x86_ops.h > index 02490515d190..f0fe40c7ac34 100644 > --- a/arch/x86/kvm/vmx/x86_ops.h > +++ b/arch/x86/kvm/vmx/x86_ops.h > @@ -137,6 +137,8 @@ int tdx_dev_ioctl(void __user *argp); > int tdx_vm_init(struct kvm *kvm); > void tdx_mmu_release_hkid(struct kvm *kvm); > void tdx_vm_free(struct kvm *kvm); > + > +int tdx_vm_ioctl(struct kvm *kvm, void __user *argp); > #else > static inline int tdx_hardware_setup(struct kvm_x86_ops *x86_ops) { return 0; } > static inline bool tdx_is_vm_type_supported(unsigned long type) { return false; } > @@ -147,6 +149,8 @@ static inline int tdx_vm_init(struct kvm *kvm) { return -EOPNOTSUPP; } > static inline void tdx_mmu_release_hkid(struct kvm *kvm) {} > static inline void tdx_flush_shadow_all_private(struct kvm *kvm) {} > static inline void tdx_vm_free(struct kvm *kvm) {} > + > +static inline int tdx_vm_ioctl(struct kvm *kvm, void __user *argp) { return -EOPNOTSUPP; } > #endif > > #endif /* __KVM_X86_VMX_X86_OPS_H */
On Mon, Aug 29, 2022 at 12:07:55PM +0800, Binbin Wu <binbin.wu@linux.intel.com> wrote: > > diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c > > index 7b497ed1f21c..067f5de56c53 100644 > > --- a/arch/x86/kvm/vmx/main.c > > +++ b/arch/x86/kvm/vmx/main.c > > @@ -73,6 +73,14 @@ static void vt_vm_free(struct kvm *kvm) > > return tdx_vm_free(kvm); > > } > > +static int vt_mem_enc_ioctl(struct kvm *kvm, void __user *argp) > > +{ > > + if (!is_td(kvm)) > > + return -ENOTTY; > > + > > + return tdx_vm_ioctl(kvm, argp); > > +} > > + > > struct kvm_x86_ops vt_x86_ops __initdata = { > > .name = "kvm_intel", > > @@ -214,6 +222,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = { > > .vcpu_deliver_sipi_vector = kvm_vcpu_deliver_sipi_vector, > > .dev_mem_enc_ioctl = tdx_dev_ioctl, > > + .mem_enc_ioctl = vt_mem_enc_ioctl, > > suggeust to align the interafce/function name style with the scop. > > patch 21 and 27 have the scope in interafce names(dev / vcpu), may be > clearer to useĀ vcpu_mem_enc_ioctl? This is a matter of preference. I intentionally chose to drop mem_enc part because actually KVM_MEMORY_ENCRYPT_OP is abused for TDX or SEV(-SNP) specific operation. Subcommand for KVM_MEMORY_ENCRYPT_OP is not directly related to memory encryption. It should be KVM_CONFIDENTIAL_OP or something. Unfortunately it's too late to rename it.
On Sun, Aug 07, 2022 at 03:01:07PM -0700, isaku.yamahata@intel.com wrote: > From: Isaku Yamahata <isaku.yamahata@intel.com> > > Add a place holder function for TDX specific VM-scoped ioctl as mem_enc_op. > TDX specific sub-commands will be added to retrieve/pass TDX specific > parameters. > > KVM_MEMORY_ENCRYPT_OP was introduced for VM-scoped operations specific for > guest state-protected VM. It defined subcommands for technology-specific > operations under KVM_MEMORY_ENCRYPT_OP. Despite its name, the subcommands > are not limited to memory encryption, but various technology-specific > operations are defined. It's natural to repurpose KVM_MEMORY_ENCRYPT_OP > for TDX specific operations and define subcommands. > > TDX requires VM-scoped TDX-specific operations for device model, for > example, qemu. Getting system-wide parameters, TDX-specific VM > initialization. > > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> > --- > arch/x86/kvm/vmx/main.c | 9 +++++++++ > arch/x86/kvm/vmx/tdx.c | 26 ++++++++++++++++++++++++++ > arch/x86/kvm/vmx/x86_ops.h | 4 ++++ > 3 files changed, 39 insertions(+) > > diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c > index 7b497ed1f21c..067f5de56c53 100644 > --- a/arch/x86/kvm/vmx/main.c > +++ b/arch/x86/kvm/vmx/main.c > @@ -73,6 +73,14 @@ static void vt_vm_free(struct kvm *kvm) > return tdx_vm_free(kvm); > } > > +static int vt_mem_enc_ioctl(struct kvm *kvm, void __user *argp) > +{ > + if (!is_td(kvm)) > + return -ENOTTY; > + > + return tdx_vm_ioctl(kvm, argp); > +} > + > struct kvm_x86_ops vt_x86_ops __initdata = { > .name = "kvm_intel", > > @@ -214,6 +222,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = { > .vcpu_deliver_sipi_vector = kvm_vcpu_deliver_sipi_vector, > > .dev_mem_enc_ioctl = tdx_dev_ioctl, > + .mem_enc_ioctl = vt_mem_enc_ioctl, > }; > > struct kvm_x86_init_ops vt_init_ops __initdata = { > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c > index 16c6570dbe52..d3b9f653da4b 100644 > --- a/arch/x86/kvm/vmx/tdx.c > +++ b/arch/x86/kvm/vmx/tdx.c > @@ -424,6 +424,32 @@ int tdx_dev_ioctl(void __user *argp) > return 0; > } > > +int tdx_vm_ioctl(struct kvm *kvm, void __user *argp) > +{ > + struct kvm_tdx_cmd tdx_cmd; > + int r; > + > + if (copy_from_user(&tdx_cmd, argp, sizeof(struct kvm_tdx_cmd))) Minor: sizeof(tdx_cmd), escape from type change & better readability. > + return -EFAULT; > + if (tdx_cmd.error || tdx_cmd.unused) > + return -EINVAL; > + > + mutex_lock(&kvm->lock); > + > + switch (tdx_cmd.id) { > + default: > + r = -EINVAL; > + goto out; > + } > + > + if (copy_to_user(argp, &tdx_cmd, sizeof(struct kvm_tdx_cmd))) Ditto > + r = -EFAULT; > + > +out: > + mutex_unlock(&kvm->lock); > + return r; > +} > + > int __init tdx_module_setup(void) > { > const struct tdsysinfo_struct *tdsysinfo; > diff --git a/arch/x86/kvm/vmx/x86_ops.h b/arch/x86/kvm/vmx/x86_ops.h > index 02490515d190..f0fe40c7ac34 100644 > --- a/arch/x86/kvm/vmx/x86_ops.h > +++ b/arch/x86/kvm/vmx/x86_ops.h > @@ -137,6 +137,8 @@ int tdx_dev_ioctl(void __user *argp); > int tdx_vm_init(struct kvm *kvm); > void tdx_mmu_release_hkid(struct kvm *kvm); > void tdx_vm_free(struct kvm *kvm); > + > +int tdx_vm_ioctl(struct kvm *kvm, void __user *argp); > #else > static inline int tdx_hardware_setup(struct kvm_x86_ops *x86_ops) { return 0; } > static inline bool tdx_is_vm_type_supported(unsigned long type) { return false; } > @@ -147,6 +149,8 @@ static inline int tdx_vm_init(struct kvm *kvm) { return -EOPNOTSUPP; } > static inline void tdx_mmu_release_hkid(struct kvm *kvm) {} > static inline void tdx_flush_shadow_all_private(struct kvm *kvm) {} > static inline void tdx_vm_free(struct kvm *kvm) {} > + > +static inline int tdx_vm_ioctl(struct kvm *kvm, void __user *argp) { return -EOPNOTSUPP; } > #endif > > #endif /* __KVM_X86_VMX_X86_OPS_H */ > -- > 2.25.1 >
diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c index 7b497ed1f21c..067f5de56c53 100644 --- a/arch/x86/kvm/vmx/main.c +++ b/arch/x86/kvm/vmx/main.c @@ -73,6 +73,14 @@ static void vt_vm_free(struct kvm *kvm) return tdx_vm_free(kvm); } +static int vt_mem_enc_ioctl(struct kvm *kvm, void __user *argp) +{ + if (!is_td(kvm)) + return -ENOTTY; + + return tdx_vm_ioctl(kvm, argp); +} + struct kvm_x86_ops vt_x86_ops __initdata = { .name = "kvm_intel", @@ -214,6 +222,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = { .vcpu_deliver_sipi_vector = kvm_vcpu_deliver_sipi_vector, .dev_mem_enc_ioctl = tdx_dev_ioctl, + .mem_enc_ioctl = vt_mem_enc_ioctl, }; struct kvm_x86_init_ops vt_init_ops __initdata = { diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c index 16c6570dbe52..d3b9f653da4b 100644 --- a/arch/x86/kvm/vmx/tdx.c +++ b/arch/x86/kvm/vmx/tdx.c @@ -424,6 +424,32 @@ int tdx_dev_ioctl(void __user *argp) return 0; } +int tdx_vm_ioctl(struct kvm *kvm, void __user *argp) +{ + struct kvm_tdx_cmd tdx_cmd; + int r; + + if (copy_from_user(&tdx_cmd, argp, sizeof(struct kvm_tdx_cmd))) + return -EFAULT; + if (tdx_cmd.error || tdx_cmd.unused) + return -EINVAL; + + mutex_lock(&kvm->lock); + + switch (tdx_cmd.id) { + default: + r = -EINVAL; + goto out; + } + + if (copy_to_user(argp, &tdx_cmd, sizeof(struct kvm_tdx_cmd))) + r = -EFAULT; + +out: + mutex_unlock(&kvm->lock); + return r; +} + int __init tdx_module_setup(void) { const struct tdsysinfo_struct *tdsysinfo; diff --git a/arch/x86/kvm/vmx/x86_ops.h b/arch/x86/kvm/vmx/x86_ops.h index 02490515d190..f0fe40c7ac34 100644 --- a/arch/x86/kvm/vmx/x86_ops.h +++ b/arch/x86/kvm/vmx/x86_ops.h @@ -137,6 +137,8 @@ int tdx_dev_ioctl(void __user *argp); int tdx_vm_init(struct kvm *kvm); void tdx_mmu_release_hkid(struct kvm *kvm); void tdx_vm_free(struct kvm *kvm); + +int tdx_vm_ioctl(struct kvm *kvm, void __user *argp); #else static inline int tdx_hardware_setup(struct kvm_x86_ops *x86_ops) { return 0; } static inline bool tdx_is_vm_type_supported(unsigned long type) { return false; } @@ -147,6 +149,8 @@ static inline int tdx_vm_init(struct kvm *kvm) { return -EOPNOTSUPP; } static inline void tdx_mmu_release_hkid(struct kvm *kvm) {} static inline void tdx_flush_shadow_all_private(struct kvm *kvm) {} static inline void tdx_vm_free(struct kvm *kvm) {} + +static inline int tdx_vm_ioctl(struct kvm *kvm, void __user *argp) { return -EOPNOTSUPP; } #endif #endif /* __KVM_X86_VMX_X86_OPS_H */