lkml.org 
[lkml]   [2021]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v1 03/11] x86/cpufeatures: Add TDX Guest CPU feature
    From
    Date
    Hi,

    On 6/10/21 5:28 AM, Borislav Petkov wrote:
    > On Tue, Jun 01, 2021 at 07:21:28PM -0700, Kuppuswamy Sathyanarayanan wrote:
    >> Add CPU feature detection for Trusted Domain Extensions support.
    >> TDX feature adds capabilities to keep guest register state and
    >> memory isolated from hypervisor.
    >>
    >> For TDX guest platforms, executing CPUID(0x21, 0) will return
    >
    > I'm assuming that 0 is ECX?

    Correct. I will clarify it in commit log.

    >
    >> following values in EAX, EBX, ECX and EDX.
    >>
    >> EAX: Maximum sub-leaf number: 0
    >> EBX/EDX/ECX: Vendor string:
    >>
    >> EBX = "Inte"
    >> EDX = "lTDX"
    >> ECX = " "
    >>
    >> So when above condition is true, set X86_FEATURE_TDX_GUEST
    >> feature cap bit
    > ^
    > Fullstop.

    Will fix it in next version.

    >
    >> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
    >> index ac37830ae941..dddc3a27cc8a 100644
    >> --- a/arch/x86/include/asm/cpufeatures.h
    >> +++ b/arch/x86/include/asm/cpufeatures.h
    >> @@ -238,6 +238,7 @@
    >> #define X86_FEATURE_VMW_VMMCALL ( 8*32+19) /* "" VMware prefers VMMCALL hypercall instruction */
    >> #define X86_FEATURE_PVUNLOCK ( 8*32+20) /* "" PV unlock function */
    >> #define X86_FEATURE_VCPUPREEMPT ( 8*32+21) /* "" PV vcpu_is_preempted function */
    >> +#define X86_FEATURE_TDX_GUEST ( 8*32+22) /* Trusted Domain Extensions Guest */
    >
    > What's the name of the feature bit? "TDX guest"? Why not only
    > X86_FEATURE_TDX and then you can have "tdx" in cpuinfo?

    Make sense. I can change it to "TDX". I guess we will not a feature bit
    for "tdx host"

    >
    >>
    >> /* Intel-defined CPU features, CPUID level 0x00000007:0 (EBX), word 9 */
    >> #define X86_FEATURE_FSGSBASE ( 9*32+ 0) /* RDFSBASE, WRFSBASE, RDGSBASE, WRGSBASE instructions*/
    >> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
    >> new file mode 100644
    >> index 000000000000..679500e807f3
    >> --- /dev/null
    >> +++ b/arch/x86/include/asm/tdx.h
    >> @@ -0,0 +1,20 @@
    >> +/* SPDX-License-Identifier: GPL-2.0 */
    >> +/* Copyright (C) 2020 Intel Corporation */
    >> +#ifndef _ASM_X86_TDX_H
    >> +#define _ASM_X86_TDX_H
    >> +
    >> +#define TDX_CPUID_LEAF_ID 0x21
    >> +
    >> +#ifdef CONFIG_INTEL_TDX_GUEST
    >> +
    >> +#include <asm/cpufeature.h>
    >
    > As before - put the include at the top.

    I will change it.

    >
    >> +void __init tdx_early_init(void);
    >> +
    >> +#else // !CONFIG_INTEL_TDX_GUEST
    >
    > No need for that // - comment

    I will remove it in next version.

    >
    >> diff --git a/arch/x86/kernel/tdx.c b/arch/x86/kernel/tdx.c
    >> new file mode 100644
    >> index 000000000000..5b14b72e41c5
    >> --- /dev/null
    >> +++ b/arch/x86/kernel/tdx.c
    >> @@ -0,0 +1,30 @@
    >> +// SPDX-License-Identifier: GPL-2.0
    >> +/* Copyright (C) 2020 Intel Corporation */
    >> +
    >> +#include <asm/tdx.h>
    >> +
    >> +static inline bool cpuid_has_tdx_guest(void)
    >> +{
    >> + u32 eax, signature[3];
    >
    > Shorten that array name to "sig" so that you don't have to break the
    > cpuid_count() line below.

    I will fix it in next version.

    >
    >> +
    >> + if (cpuid_eax(0) < TDX_CPUID_LEAF_ID)
    >> + return false;
    >> +
    >> + cpuid_count(TDX_CPUID_LEAF_ID, 0, &eax, &signature[0],
    >> + &signature[1], &signature[2]);
    >> +
    >> + if (memcmp("IntelTDX ", signature, 12))
    >> + return false;
    >> +
    >> + return true;
    >
    > or simply:
    >
    > return !memcmp(...

    Got it. I will change it in next version.

    >
    >> +}
    >> +
    >> +void __init tdx_early_init(void)
    >> +{
    >> + if (!cpuid_has_tdx_guest())
    >> + return;
    >> +
    >> + setup_force_cpu_cap(X86_FEATURE_TDX_GUEST);
    >> +
    >> + pr_info("TDX guest is initialized\n");
    >
    > Use pr_fmt in this file:
    >
    > #undef pr_fmt
    > #define pr_fmt(fmt) "x86/tdx: " fmt
    >
    > and then do
    >
    > pr_info("Guest initialized\n");

    Patch titled "x86/tdx: Add __tdx_module_call() and __tdx_hypercall() helper functions" add
    it. It makes sense to move it to this patch.

    >
    > Thx.
    >

    --
    Sathyanarayanan Kuppuswamy
    Linux Kernel Developer

    \
     
     \ /
      Last update: 2021-06-10 16:28    [W:3.624 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site