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

[Isaku Yamahata <isaku.yamahata@gmail.com>]

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>