linux-kernel - Re: [PATCHv13 9/9] x86/tdx: Add unaccepted memory support

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]
Date:   Fri, 2 Jun 2023 09:26:45 -0500
From:   Tom Lendacky <thomas.lendacky@....com>
To:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Borislav Petkov <bp@...en8.de>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...el.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Joerg Roedel <jroedel@...e.de>,
        Ard Biesheuvel <ardb@...nel.org>
Cc:     Andi Kleen <ak@...ux.intel.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        David Rientjes <rientjes@...gle.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Ingo Molnar <mingo@...hat.com>,
        Dario Faggioli <dfaggioli@...e.com>,
        Mike Rapoport <rppt@...nel.org>,
        David Hildenbrand <david@...hat.com>,
        Mel Gorman <mgorman@...hsingularity.net>,
        marcelo.cerri@...onical.com, tim.gardner@...onical.com,
        khalid.elmously@...onical.com, philip.cox@...onical.com,
        aarcange@...hat.com, peterx@...hat.com, x86@...nel.org,
        linux-mm@...ck.org, linux-coco@...ts.linux.dev,
        linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv13 9/9] x86/tdx: Add unaccepted memory support

On 6/2/23 08:22, Tom Lendacky wrote:
> On 6/1/23 13:25, Kirill A. Shutemov wrote:
>> Hookup TDX-specific code to accept memory.
>>
>> Accepting the memory is done with ACCEPT_PAGE module call on every page
>> in the range. MAP_GPA hypercall is not required as the unaccepted memory
>> is considered private already.
>>
>> Extract the part of tdx_enc_status_changed() that does memory acceptance
>> in a new helper. Move the helper tdx-shared.c. It is going to be used by
>> both main kernel and decompressor.
>>
>> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
>> ---
>>   arch/x86/Kconfig                         |  2 +
>>   arch/x86/boot/compressed/Makefile        |  2 +-
>>   arch/x86/boot/compressed/error.c         | 19 +++++++
>>   arch/x86/boot/compressed/error.h         |  1 +
>>   arch/x86/boot/compressed/mem.c           | 35 +++++++++++-
>>   arch/x86/boot/compressed/tdx-shared.c    |  2 +
>>   arch/x86/coco/tdx/Makefile               |  2 +-
>>   arch/x86/coco/tdx/tdx-shared.c           | 71 ++++++++++++++++++++++++
>>   arch/x86/coco/tdx/tdx.c                  | 70 +----------------------
>>   arch/x86/include/asm/shared/tdx.h        |  2 +
>>   arch/x86/include/asm/unaccepted_memory.h | 24 ++++++++
>>   11 files changed, 160 insertions(+), 70 deletions(-)
>>   create mode 100644 arch/x86/boot/compressed/tdx-shared.c
>>   create mode 100644 arch/x86/coco/tdx/tdx-shared.c
>>   create mode 100644 arch/x86/include/asm/unaccepted_memory.h
>>

>> diff --git a/arch/x86/boot/compressed/mem.c 
>> b/arch/x86/boot/compressed/mem.c
>> index 4ecf26576a77..d2b6948a7801 100644
>> --- a/arch/x86/boot/compressed/mem.c
>> +++ b/arch/x86/boot/compressed/mem.c
>> @@ -2,11 +2,44 @@
>>   #include "error.h"
>>   #include "misc.h"
>> +#include "tdx.h"
>> +#include <asm/shared/tdx.h>
>> +
>> +/*
>> + * accept_memory() and process_unaccepted_memory() called from EFI stub 
>> which
>> + * runs before decompresser and its early_tdx_detect().
>> + *
>> + * Enumerate TDX directly from the early users.
>> + */
>> +static bool early_is_tdx_guest(void)
>> +{
>> +    static bool once;
>> +    static bool is_tdx;
>> +
>> +    if (!IS_ENABLED(CONFIG_INTEL_TDX_GUEST))
>> +        return false;
>> +
>> +    if (!once) {
>> +        u32 eax, sig[3];
>> +
>> +        cpuid_count(TDX_CPUID_LEAF_ID, 0, &eax,
>> +                &sig[0], &sig[2],  &sig[1]);
>> +        is_tdx = !memcmp(TDX_IDENT, sig, sizeof(sig));
>> +        once = true;
>> +    }
>> +
>> +    return is_tdx;
>> +}
>>   void arch_accept_memory(phys_addr_t start, phys_addr_t end)
>>   {
>>       /* Platform-specific memory-acceptance call goes here */
>> -    error("Cannot accept memory");
>> +    if (early_is_tdx_guest()) {
>> +        if (tdx_accept_memory(start, end))
>> +            return;
>> +    }
>> +
>> +    error("Cannot accept memory: unknown platform\n");
> 
> So this is a change in this version. If tdx_accept_memory() fails, you'll 
> report unknown platform. Wouldn't it be better to have an error message 
> that indicates a failure in the accept path?
> 

Maybe you can keep it similar to the v12 version with just a new error 
message, something like:

	if (early_is_tdx_guest()) {
		if (!tdx_accept_memory(start, end))
			error("TDX error accepting memory\n");
	} else {
		error("Cannot accept memory: unknown platform\n");
	}

And similar in arch/x86/include/asm/unaccepted_memory.h.

Thanks,
Tom

> Thanks,
> Tom
> 
>>   }
>>   void init_unaccepted_memory(void)