All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: "Kuppuswamy, Sathyanarayanan"  <sathyanarayanan.kuppuswamy@linux.intel.com>
To: Borislav Petkov <bp@alien8.de>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>, Peter H Anvin <hpa@zytor.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Tony Luck <tony.luck@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Andi Kleen <ak@linux.intel.com>,
	Kirill Shutemov <kirill.shutemov@linux.intel.com>,
	Sean Christopherson <seanjc@google.com>,
	Kuppuswamy Sathyanarayanan <knsathya@kernel.org>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 04/12] x86/x86: Add early_is_tdx_guest() interface
Date: Fri, 18 Jun 2021 12:14:16 -0700	[thread overview]
Message-ID: <76e177b6-7666-bc8f-a00a-c6206578d566@linux.intel.com> (raw)
In-Reply-To: <YMuAvP7bqwHVSCw8@zn.tnic>



On 6/17/21 10:05 AM, Borislav Petkov wrote:
> On Sat, Jun 12, 2021 at 02:04:45PM -0700, Kuppuswamy Sathyanarayanan wrote:
> 
>> Subject: Re: [PATCH v2 04/12] x86/x86: Add early_is_tdx_guest() interface
> 
> Subject prefix should be "x86/tdx:" ofc.

I will fix this in next version.

> 
>> diff --git a/arch/x86/boot/compressed/tdx.c b/arch/x86/boot/compressed/tdx.c
>> new file mode 100644
>> index 000000000000..ddfa4a6d1939
>> --- /dev/null
>> +++ b/arch/x86/boot/compressed/tdx.c
>> @@ -0,0 +1,28 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * tdx.c - Early boot code for TDX
>> + */
>> +
>> +#include <asm/tdx.h>
> 
> Please no kernel proper includes in the decompressor stage - that thing
> is an include hell mess and needs cleaning up. Use cpuid_count() from
> arch/x86/boot/cpuflags.c by exporting it properly and add the other
> defines here instead of using kernel proper facilities.
> 
> I know, I know, there is other misuse but it has to stop.

I will re-use cpuid_count() from cpuflags.c. But it is missing definition
for cpuid_eax(0). So, I will have to define a new function to handle it.
Hope its alright with you.

> 
>> +static int __ro_after_init tdx_guest = -1;
>> +
>> +static inline bool native_cpuid_has_tdx_guest(void)
> 
> Why is this function prefixed with "native_"?
> 
>> +{
>> +	u32 eax = TDX_CPUID_LEAF_ID, sig[3] = {0};
>> +
>> +	if (native_cpuid_eax(0) < TDX_CPUID_LEAF_ID)
>> +		return false;
>> +
>> +	native_cpuid(&eax, &sig[0], &sig[1], &sig[2]);
>> +
>> +	return !memcmp("IntelTDX    ", sig, 12);
>> +}
>> +
>> +bool early_is_tdx_guest(void)
> 
> So I guess this is going to be used somewhere, because I don't see it.
> Or is it going away in favor of prot_guest_has(PR_GUEST_TDX) ?

It is used for handling TDX specific I/O support in decompressor code.

You can find its usage in https://lore.kernel.org/patchwork/patch/1444186/

May be I should move this patch next to that patch for easy reading.

> 
> This is the problem with sending new versions of patches as a reply to
> the old ones in the patchset: I get confused. Why don't you wait until
> the whole thing has been reviewed and then send a new revision which has
> all the changes and I can find my way in the context?
> 
> And with the amount of changes so far, you should probably send a new
> revision of the initial support set now-ish instead of me reviewing this
> one 'til the end.

I will send the new series with review fixes. Also, since this patch is only
used by I/O support code, I will move this patch to another TDX series which
adds TDX I/O support.

> 
> Thx.
> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

  reply	other threads:[~2021-06-18 19:14 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-02  2:21 [PATCH v1 00/11] Add TDX Guest Support (Initial support) Kuppuswamy Sathyanarayanan
2021-06-02  2:21 ` [PATCH v1 01/11] x86/paravirt: Move halt paravirt calls under CONFIG_PARAVIRT Kuppuswamy Sathyanarayanan
2021-06-02  2:21 ` [PATCH v1 02/11] x86/tdx: Introduce INTEL_TDX_GUEST config option Kuppuswamy Sathyanarayanan
2021-06-02  2:21 ` [PATCH v1 03/11] x86/cpufeatures: Add TDX Guest CPU feature Kuppuswamy Sathyanarayanan
2021-06-07 14:32   ` Tom Lendacky
2021-06-07 16:59     ` Kuppuswamy, Sathyanarayanan
2021-06-10 12:28   ` Borislav Petkov
2021-06-10 14:28     ` Kuppuswamy, Sathyanarayanan
2021-06-10 14:29     ` Kirill A. Shutemov
2021-06-10 14:35       ` Borislav Petkov
2021-06-10 14:41         ` Kirill A. Shutemov
2021-06-10 15:56           ` Borislav Petkov
2021-06-12 21:02             ` [PATCH v2 03/12] " Kuppuswamy Sathyanarayanan
2021-06-16  9:52               ` Borislav Petkov
2021-06-16 16:57                 ` Kuppuswamy, Sathyanarayanan
2021-06-02  2:21 ` [PATCH v1 04/11] x86/x86: Add is_tdx_guest() interface Kuppuswamy Sathyanarayanan
2021-06-10 19:59   ` Borislav Petkov
2021-06-10 21:01     ` Kuppuswamy, Sathyanarayanan
2021-06-10 21:07       ` Borislav Petkov
2021-06-12 21:04         ` [PATCH v2 04/12] x86/x86: Add early_is_tdx_guest() interface Kuppuswamy Sathyanarayanan
2021-06-17 17:05           ` Borislav Petkov
2021-06-18 19:14             ` Kuppuswamy, Sathyanarayanan [this message]
2021-06-02  2:21 ` [PATCH v1 05/11] x86/tdx: Add __tdx_module_call() and __tdx_hypercall() helper functions Kuppuswamy Sathyanarayanan
2021-06-14  8:47   ` Borislav Petkov
2021-06-14 19:45     ` Kuppuswamy, Sathyanarayanan
2021-06-14 20:11       ` Borislav Petkov
2021-06-14 21:37         ` Kuppuswamy, Sathyanarayanan
2021-06-02  2:21 ` [PATCH v1 06/11] x86/tdx: Get TD execution environment information via TDINFO Kuppuswamy Sathyanarayanan
2021-06-02  2:21 ` [PATCH v1 07/11] x86/traps: Add #VE support for TDX guest Kuppuswamy Sathyanarayanan
2021-06-02  2:21 ` [PATCH v1 08/11] x86/tdx: Add HLT " Kuppuswamy Sathyanarayanan
2021-06-02  2:21 ` [PATCH v1 09/11] x86/tdx: Wire up KVM hypercalls Kuppuswamy Sathyanarayanan
2021-06-12 21:08   ` [PATCH v2 10/12] " Kuppuswamy Sathyanarayanan
2021-06-02  2:21 ` [PATCH v1 10/11] x86/tdx: Add MSR support for TDX guest Kuppuswamy Sathyanarayanan
2021-06-02  2:21 ` [PATCH v1 11/11] x86/tdx: Handle CPUID via #VE Kuppuswamy Sathyanarayanan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=76e177b6-7666-bc8f-a00a-c6206578d566@linux.intel.com \
    --to=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=ak@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=hpa@zytor.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=knsathya@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.