On Thu, Aug 29, 2024 at 02:27:56PM +0800, Yan Zhao wrote:
> On Mon, Aug 12, 2024 at 03:48:09PM -0700, Rick Edgecombe wrote:
> > From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> >
> ...
> > +static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
> > +{
...
> > + kvm_tdx->tsc_offset = td_tdcs_exec_read64(kvm_tdx, TD_TDCS_EXEC_TSC_OFFSET);
> > + kvm_tdx->attributes = td_params->attributes;
> > + kvm_tdx->xfam = td_params->xfam;
> > +
> > + if (td_params->exec_controls & TDX_EXEC_CONTROL_MAX_GPAW)
> > + kvm->arch.gfn_direct_bits = gpa_to_gfn(BIT_ULL(51));
> > + else
> > + kvm->arch.gfn_direct_bits = gpa_to_gfn(BIT_ULL(47));
> > +
> Could we introduce a initialized field in struct kvm_tdx and set it true
> here? e.g
> + kvm_tdx->initialized = true;
>
> Then reject vCPU creation in tdx_vcpu_create() before KVM_TDX_INIT_VM is
> executed successfully? e.g.
>
> @@ -584,6 +589,9 @@ int tdx_vcpu_create(struct kvm_vcpu *vcpu)
> struct kvm_tdx *kvm_tdx = to_kvm_tdx(vcpu->kvm);
> struct vcpu_tdx *tdx = to_tdx(vcpu);
>
> + if (!kvm_tdx->initialized)
> + return -EIO;
> +
> /* TDX only supports x2APIC, which requires an in-kernel local APIC. */
> if (!vcpu->arch.apic)
> return -EINVAL;
>
> Allowing vCPU creation only after TD is initialized can prevent unexpected
> userspace access to uninitialized TD primitives.
Makes sense to check for initialized TD before allowing other calls. Maybe
the check is needed in other places too in additoin to the tdx_vcpu_create().
How about just a function to check for one or more of the already existing
initialized struct kvm_tdx values?
Regards,
Tony
