Re: [PATCH v11 023/113] KVM: TDX: allocate/free TDX vcpu structure

[Huang, Kai]

On Tue, 2023-02-28 at 03:06 -0800, Isaku Yamahata wrote:
> > > +	if (!e)
> > > +		return -ENOMEM;
> > > +	*e  = (struct kvm_cpuid_entry2) {
> > > +		.function = 1,	/* Features for X2APIC */
> > > +		.index = 0,
> > > +		.eax = 0,
> > > +		.ebx = 0,
> > > +		.ecx = 1ULL << 21,	/* X2APIC */
> > > +		.edx = 0,
> > > +	};
> > > +	vcpu->arch.cpuid_entries = e;
> > > +	vcpu->arch.cpuid_nent = 1;
> > 
> > As mentioned above, why doing it here? Won't be this be overwritten later in
> > KVM_SET_CPUID2?
> 
> Yes, user space VMM can overwrite cpuid[0x1] and APIC base MSR.  But it
> doesn't
> matter because it's a bug of user space VMM. user space VMM has to keep the
> consistency of cpuid and MSRs.
> Because TDX module virtualizes cpuid[0x1].x2apic to fixed 1, KVM value doesn't
> matter after vcpu creation.
> Because KVM virtualizes APIC base as read only to guest, cpuid[0x1].x2apic
> doesn't matter after vcpu creation as long as user space VMM keeps KVM APIC
> BASE
> value.
> 

Contrary, can we depend on userspace VMM to set x2APIC in CPUID, but not do this
in KVM?  If userspace doesn't do it, we treat it as userspace's bug.

Plus, userspace anyway needs to set x2APIC in CPUID regardless whether you have
done above here, correct?

I don't see the point of doing above in KVM because you are neither enforcing
anything in KVM, nor you are reducing effort of userspace.