Re: [PATCH v1 1/3] x86/tdx: Check for TDX partitioning during early TDX init

[Jeremi Piotrowski]

On 29/11/2023 17:40, Borislav Petkov wrote:
> On Wed, Nov 22, 2023 at 06:19:20PM +0100, Jeremi Piotrowski wrote:
>> Which approach do you prefer?
> 
> I'm trying to figure out from the whole thread, what this guest is.

Wanted to clarify some things directly here. This type guest is supported
in the kernel already[1], so this whole series is the kind of attempt to
share more code that you advocated for in another email.

[1]: https://lore.kernel.org/lkml/20230824080712.30327-1-decui@xxxxxxxxxxxxx/#t

> 
> * A HyperV second-level guest

>From Hyper-V's point of view it's a TDX guest with privilege levels inside, not
second-level...

> 
> * of type TDX

...but Intel TDX calls these privilege levels L1 and L2 instead of VMPL0/VMPL1-3.

> 
> * Needs to defer cc_mask and page visibility bla...
>

The implementations in tdx_early_init() depend on TDX module calls (not avail)
and the correct calls are standard Hyper-V hypercalls (same as vTOM SNP guests).

> * needs to disable TDX module calls
> 
> * stub out tdx_accept_memory

This is actually a fix that for something that only works by accident right now
and I meant to post separately from the rest of the discussion.

If you look at arch/x86/include/asm/unaccepted_memory.h (below), it is used by both
CONFIG_INTEL_TDX_GUEST and CONFIG_AMD_MEM_ENCRYPT, but there is no tdx_accept_memory
implementation when CONFIG_INTEL_TDX_GUEST is not set. This is subtle and confusing,
the stub should be there.
 
static inline void arch_accept_memory(phys_addr_t start, phys_addr_t end)
{
        /* Platform-specific memory-acceptance call goes here */
        if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) {
                if (!tdx_accept_memory(start, end))
                        panic("TDX: Failed to accept memory\n");
        } else if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
                snp_accept_memory(start, end);
        } else {
                panic("Cannot accept memory: unknown platform\n");
        }
}

> 
> Anything else?
> 
> And my worry is that this is going to become a mess and your patches
> already show that it is going in that direction because you need to run
> the TDX side but still have *some* things done differently. Which is
> needed because this is a different type of guest, even if it is a TDX
> one.
> 
> Which reminds me, we have amd_cc_platform_vtom() which is a similar type
> of thing.
> 
> And the TDX side could do something similar and at least *try* to
> abstract away all that stuff.
> 
> Would it be nice? Of course not!
> 
> How can one model a virt zoo of at least a dozen guest types but still
> keep code sane... :-\
>