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

[Tom Lendacky <thomas.lendacky@amd.com>]

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@linux.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)