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... :-\ >
