On Wed, Jun 29, 2022 at 3:14 AM Isaku Yamahata <isaku.yamahata@xxxxxxxxx> wrote: > > On Tue, Jun 14, 2022 at 11:15:00AM -0700, > Sagi Shahar <sagis@xxxxxxxxxx> wrote: > > > On Thu, May 5, 2022 at 11:16 AM <isaku.yamahata@xxxxxxxxx> wrote: > > > > > > From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx> > > > > > > Wire up TDX PV CPUID hypercall to the KVM backend function. > > > > > > Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx> > > > --- > > > arch/x86/kvm/vmx/tdx.c | 22 ++++++++++++++++++++++ > > > 1 file changed, 22 insertions(+) > > > > > > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c > > > index 9c712f661a7c..c7cdfee397ec 100644 > > > --- a/arch/x86/kvm/vmx/tdx.c > > > +++ b/arch/x86/kvm/vmx/tdx.c > > > @@ -946,12 +946,34 @@ static int tdx_emulate_vmcall(struct kvm_vcpu *vcpu) > > > return 1; > > > } > > > > > > +static int tdx_emulate_cpuid(struct kvm_vcpu *vcpu) > > > +{ > > > + u32 eax, ebx, ecx, edx; > > > + > > > + /* EAX and ECX for cpuid is stored in R12 and R13. */ > > > + eax = tdvmcall_a0_read(vcpu); > > > + ecx = tdvmcall_a1_read(vcpu); > > > + > > > + kvm_cpuid(vcpu, &eax, &ebx, &ecx, &edx, true); > > > > According to the GHCI spec section 3.6 > > (TDG.VP.VMCALL<Instruction.CPUID>) we should return > > VMCALL_INVALID_OPERAND if an invalid CPUID is requested. > > > > kvm_cpuid already returns false in this case so we should use that > > return value to set the tdvmcall return code in case of invalid leaf. > > Based on CPUID instruction, cpuid results in #UD when lock prefix is used or > earlier CPU that doesn't support cpuid instruction. > So I'm not sure what CPUID input result in INVALID_OPERAND error. > Does the following make sense for you? > > --- a/arch/x86/kvm/vmx/tdx.c > +++ b/arch/x86/kvm/vmx/tdx.c > @@ -1347,7 +1347,7 @@ static int tdx_emulate_cpuid(struct kvm_vcpu *vcpu) > eax = tdvmcall_a0_read(vcpu); > ecx = tdvmcall_a1_read(vcpu); > > - kvm_cpuid(vcpu, &eax, &ebx, &ecx, &edx, true); > + kvm_cpuid(vcpu, &eax, &ebx, &ecx, &edx, false); > > tdvmcall_a0_write(vcpu, eax); > tdvmcall_a1_write(vcpu, ebx); > > thanks, If any CPUID request is considered valid, then perhaps the spec itself needs to be updated. Right now it clearly states that TDG.VP.VMCALL_INVALID_OPERAND is returned if "Invalid CPUID requested" which I understood as a non-existing leaf. But if you say that a non-existing leaf is still a valid CPUID request than I'm not sure what "Invalid CPUID requested" means in the spec itself. > > -- > Isaku Yamahata <isaku.yamahata@xxxxxxxxx>