Re: [PATCH v7 030/102] KVM: TDX: Do TDX specific vcpu initialization

Isaku Yamahata
2022-07-12T20:35:49Z

On Fri, Jul 08, 2022 at 10:14:43AM +0800,
Yuan Yao <yuan.yao@linux.intel.com> wrote:

> On Mon, Jun 27, 2022 at 02:53:22PM -0700, isaku.yamahata@intel.com wrote:
> > From: Sean Christopherson <sean.j.christopherson@intel.com>
> >
> > TD guest vcpu need to be configured before ready to run which requests
> > addtional information from Device model (e.g. qemu), one 64bit value is
> > passed to vcpu's RCX as an initial value. Repurpose KVM_MEMORY_ENCRYPT_OP
> > to vcpu-scope and add new sub-commands KVM_TDX_INIT_VCPU under it for such
> > additional vcpu configuration.
> >
> > Add callback for kvm vCPU-scoped operations of KVM_MEMORY_ENCRYPT_OP and
> > add a new subcommand, KVM_TDX_INIT_VCPU, for further vcpu initialization.
> >
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> > ---
> > arch/x86/include/asm/kvm-x86-ops.h | 1 +
> > arch/x86/include/asm/kvm_host.h | 1 +
> > arch/x86/include/uapi/asm/kvm.h | 1 +
> > arch/x86/kvm/vmx/main.c | 9 +++++++
> > arch/x86/kvm/vmx/tdx.c | 36 +++++++++++++++++++++++++++
> > arch/x86/kvm/vmx/tdx.h | 4 +++
> > arch/x86/kvm/vmx/x86_ops.h | 2 ++
> > arch/x86/kvm/x86.c | 6 +++++
> > tools/arch/x86/include/uapi/asm/kvm.h | 1 +
> > 9 files changed, 61 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> > index 3677a5015a4f..32a6df784ea6 100644
> > --- a/arch/x86/include/asm/kvm-x86-ops.h
> > +++ b/arch/x86/include/asm/kvm-x86-ops.h
> > @@ -119,6 +119,7 @@ KVM_X86_OP(leave_smm)
> > KVM_X86_OP(enable_smi_window)
> > KVM_X86_OP_OPTIONAL(dev_mem_enc_ioctl)
> > KVM_X86_OP_OPTIONAL(mem_enc_ioctl)
> > +KVM_X86_OP_OPTIONAL(vcpu_mem_enc_ioctl)
> > KVM_X86_OP_OPTIONAL(mem_enc_register_region)
> > KVM_X86_OP_OPTIONAL(mem_enc_unregister_region)
> > KVM_X86_OP_OPTIONAL(vm_copy_enc_context_from)
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 81638987cdb9..e5d4e5b60fdc 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1595,6 +1595,7 @@ struct kvm_x86_ops {
> >
> > int (*dev_mem_enc_ioctl)(void __user *argp);
> > int (*mem_enc_ioctl)(struct kvm *kvm, void __user *argp);
> > + int (*vcpu_mem_enc_ioctl)(struct kvm_vcpu *vcpu, void __user *argp);
> > int (*mem_enc_register_region)(struct kvm *kvm, struct kvm_enc_region *argp);
> > int (*mem_enc_unregister_region)(struct kvm *kvm, struct kvm_enc_region *argp);
> > int (*vm_copy_enc_context_from)(struct kvm *kvm, unsigned int source_fd);
> > diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> > index f89774ccd4ae..399c28b2f4f5 100644
> > --- a/arch/x86/include/uapi/asm/kvm.h
> > +++ b/arch/x86/include/uapi/asm/kvm.h
> > @@ -538,6 +538,7 @@ struct kvm_pmu_event_filter {
> > enum kvm_tdx_cmd_id {
> > KVM_TDX_CAPABILITIES = 0,
> > KVM_TDX_INIT_VM,
> > + KVM_TDX_INIT_VCPU,
> >
> > KVM_TDX_CMD_NR_MAX,
> > };
> > diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
> > index 4f4ed4ad65a7..ce12cc8276ef 100644
> > --- a/arch/x86/kvm/vmx/main.c
> > +++ b/arch/x86/kvm/vmx/main.c
> > @@ -113,6 +113,14 @@ static int vt_mem_enc_ioctl(struct kvm *kvm, void __user *argp)
> > return tdx_vm_ioctl(kvm, argp);
> > }
> >
> > +static int vt_vcpu_mem_enc_ioctl(struct kvm_vcpu *vcpu, void __user *argp)
> > +{
> > + if (!is_td_vcpu(vcpu))
> > + return -EINVAL;
> > +
> > + return tdx_vcpu_ioctl(vcpu, argp);
> > +}
> > +
> > struct kvm_x86_ops vt_x86_ops __initdata = {
> > .name = "kvm_intel",
> >
> > @@ -255,6 +263,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = {
> >
> > .dev_mem_enc_ioctl = tdx_dev_ioctl,
> > .mem_enc_ioctl = vt_mem_enc_ioctl,
> > + .vcpu_mem_enc_ioctl = vt_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 d9fe3f6463c3..2772775457b0 100644
> > --- a/arch/x86/kvm/vmx/tdx.c
> > +++ b/arch/x86/kvm/vmx/tdx.c
> > @@ -83,6 +83,11 @@ static inline bool is_hkid_assigned(struct kvm_tdx *kvm_tdx)
> > return kvm_tdx->hkid > 0;
> > }
> >
> > +static inline bool is_td_finalized(struct kvm_tdx *kvm_tdx)
> > +{
> > + return kvm_tdx->finalized;
> > +}
> > +
> > static void tdx_clear_page(unsigned long page)
> > {
> > const void *zero_page = (const void *) __va(page_to_phys(ZERO_PAGE(0)));
> > @@ -805,6 +810,37 @@ int tdx_vm_ioctl(struct kvm *kvm, void __user *argp)
> > return r;
> > }
> >
> > +int tdx_vcpu_ioctl(struct kvm_vcpu *vcpu, void __user *argp)
> > +{
> > + struct kvm_tdx *kvm_tdx = to_kvm_tdx(vcpu->kvm);
> > + struct vcpu_tdx *tdx = to_tdx(vcpu);
> > + struct kvm_tdx_cmd cmd;
> > + u64 err;
> > +
> > + if (tdx->initialized)
>
> Minor: How about "tdx_vcpu->initialized" ? there's
> "is_td_initialized()" below, the "tdx" here may lead guys to treat it
> as whole td vm until they confirmed it's type again.

I think you man tdx->vcpu_initialized. If so, makes sense. I'll rename it.
--
Isaku Yamahata <isaku.yamahata@gmail.com>