linux-kernel - Re: [PATCH v8 04/11] x86/tdx: Add Intel ARCH support to cc_platform

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5fa4df70-fab6-fd7a-06cf-cb71e77ac4d5@linux.intel.com>
Date:   Wed, 6 Oct 2021 11:14:31 -0700
From:   "Kuppuswamy, Sathyanarayanan" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
To:     Borislav Petkov <bp@...en8.de>,
        Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, x86@...nel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        David Hildenbrand <david@...hat.com>,
        Andrea Arcangeli <aarcange@...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>, 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 v8 04/11] x86/tdx: Add Intel ARCH support to
 cc_platform_has()



On 10/6/21 11:02 AM, Borislav Petkov wrote:
> On Tue, Oct 05, 2021 at 02:16:11PM -0700, Josh Poimboeuf wrote:
>> I assume this needs a rebase on -tip since cc_platform.c already has an
>> empty version of this function (and it's static so it doesn't need to be
>> declared in a header).
> 
> Yes:
> 
> arch/x86/kernel/cc_platform.c:16:28: error: static declaration of ‘intel_cc_platform_has’ follows non-static declaration
>     16 | static bool __maybe_unused intel_cc_platform_has(enum cc_attr attr)
>        |                            ^~~~~~~~~~~~~~~~~~~~~
> In file included from ./include/linux/mem_encrypt.h:17,
>                   from arch/x86/kernel/cc_platform.c:12:
> ./arch/x86/include/asm/mem_encrypt.h:105:6: note: previous declaration of ‘intel_cc_platform_has’ was here
>    105 | bool intel_cc_platform_has(enum cc_attr attr);
>        |      ^~~~~~~~~~~~~~~~~~~~~
> make[2]: *** [scripts/Makefile.build:277: arch/x86/kernel/cc_platform.o] Error 1
> make[1]: *** [scripts/Makefile.build:540: arch/x86/kernel] Error 2
> make: *** [Makefile:1868: arch/x86] Error 2
> make: *** Waiting for unfinished jobs....
> 
> I had already started that function there - please add all TDX logic in
> cc_platform.c.
> 
> When you do your next set, you can use tip/master as a base. This should
> be used for all x86 patchsets anyway.

Yes. I have already rebased my patches on top of tip branch. Next version
will not have this issue.


> 
>>> +	/**
>>> +	 * @CC_ATTR_GUEST_TDX: Trusted Domain Extension Support
>>> +	 *
>>> +	 * The platform/OS is running as a TDX guest/virtual machine.
>>> +	 *
>>> +	 * Examples include Intel TDX.
>>> +	 */
>>> +	CC_ATTR_GUEST_TDX,
>>
>> Examples of TDX include TDX? :-)
> 
> Yeah, so whether we should be naming the actual conf. computing
> implementation came up during the cc_platform_has() review and looking
> forward in this patchset:
> 
> +       if (cc_platform_has(CC_ATTR_GUEST_TDX))
> +               return tdx_kvm_hypercall(nr, 0, 0, 0, 0);
> 
> you really need to test for TDX because you're doing a TDX-specific
> hypercall.
> 
> Which brings me back to the fastpath use of is_idx_guest(): this
> looks to me like a fastpath use - dunno how often one needs to do TDX
> hypercalls so I can imagine that for this, the is_tdx_guest() check
> should use a static branch.
> 
> But only with numbers to show the need for it.

Compared to TDX hypercall, additional function call should not take much
time. IMO, we don't need fast path for hypercalls.

Sean/Andi - Any comments?

> 
> Thx.
> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer