[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <dd9ceec3-2ae4-c468-0a6a-de14965b65a2@linux.intel.com>
Date: Thu, 14 Oct 2021 20:03:05 -0700
From: Sathyanarayanan Kuppuswamy
<sathyanarayanan.kuppuswamy@...ux.intel.com>
To: Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
x86@...nel.org, Paolo Bonzini <pbonzini@...hat.com>,
David Hildenbrand <david@...hat.com>,
Andrea Arcangeli <aarcange@...hat.com>,
Josh Poimboeuf <jpoimboe@...hat.com>,
Juergen Gross <jgross@...e.com>, Deep Shah <sdeep@...are.com>,
VMware Inc <pv-drivers@...are.com>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
Wanpeng Li <wanpengli@...cent.com>,
Jim Mattson <jmattson@...gle.com>,
Joerg Roedel <joro@...tes.org>
Cc: Peter H Anvin <hpa@...or.com>, Dave Hansen <dave.hansen@...el.com>,
Tony Luck <tony.luck@...el.com>,
Dan Williams <dan.j.williams@...el.com>,
Andi Kleen <ak@...ux.intel.com>,
Kirill Shutemov <kirill.shutemov@...ux.intel.com>,
Sean Christopherson <seanjc@...gle.com>,
Kuppuswamy Sathyanarayanan <knsathya@...nel.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v10 08/11] x86/tdx: Wire up KVM hypercalls
On 10/14/21 3:21 AM, Thomas Gleixner wrote:
> On Fri, Oct 08 2021 at 22:37, Kuppuswamy Sathyanarayanan wrote:
>> From: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
>>
>> #ifdef CONFIG_KVM_GUEST
>> @@ -32,6 +34,10 @@ static inline bool kvm_check_and_clear_guest_paused(void)
>> static inline long kvm_hypercall0(unsigned int nr)
>> {
>> long ret;
>> +
>> + if (cc_platform_has(CC_ATTR_GUEST_TDX))
>> + return tdx_kvm_hypercall(nr, 0, 0, 0, 0);
> So if TDX is not enabled in Kconfig this cannot be optimized out unless
> CC_PLATFORM is disabled as well. But what's worse is that every
> hypercall needs to call into cc_platform_has().
>
> None of the hypercalls is used before the early TDX detection. So we can
> simply use
>
> if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
>
> here, right? Then you add X86_FEATURE_TDX_GUEST to the disabled feature
> bits correctly and all of the above is solved.
>
> Hmm?
Make sense. Since this will only be used after tdx_early_init() call,
and X86_FEATURE_TDX_GUEST is also set in that call, we can just use
cpu_feature_enabled(X86_FEATURE_TDX_GUEST) as you have mentioned.
I will fix this in next version.
>
>> +#if defined(CONFIG_KVM_GUEST) && defined(CONFIG_INTEL_TDX_GUEST)
>> +static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
>> + unsigned long p2, unsigned long p3,
>> + unsigned long p4)
>> +{
>> + struct tdx_hypercall_output out;
>> + u64 err;
>> +
>> + err = __tdx_hypercall(TDX_HYPERCALL_VENDOR_KVM, nr, p1, p2,
>> + p3, p4, &out);
>> +
>> + /*
>> + * Non zero return value means buggy TDX module (which is fatal).
>> + * So use BUG_ON() to panic.
>> + */
>> + BUG_ON(err);
>> +
>> + return out.r10;
>> +}
> Can we make that a proper exported function (instead of
> tdx_kvm_hypercall) so we don't end up with the very same code inlined
> all over the place?
Initially it was an exported function. But we made it inline in tdx.h
to simplify the implementation. But if exported function is preferred,
I will fix it in next version.
>
> Thanks,
>
> tglx
>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Powered by blists - more mailing lists