Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: ie3994620:Fastmail
Date:   Wed, 17 May 2023 02:22:04 +0300
From:   "Kirill A. Shutemov" <kirill@shutemov.name>
To:     Tom Lendacky <thomas.lendacky@amd.com>
Cc:     "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        Borislav Petkov <bp@alien8.de>,
        Andy Lutomirski <luto@kernel.org>,
        Dave Hansen <dave.hansen@intel.com>,
        Sean Christopherson <seanjc@google.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Joerg Roedel <jroedel@suse.de>,
        Ard Biesheuvel <ardb@kernel.org>,
        Andi Kleen <ak@linux.intel.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        David Rientjes <rientjes@google.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Thomas Gleixner <tglx@linutronix.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Ingo Molnar <mingo@redhat.com>,
        Dario Faggioli <dfaggioli@suse.com>,
        Mike Rapoport <rppt@kernel.org>,
        David Hildenbrand <david@redhat.com>,
        Mel Gorman <mgorman@techsingularity.net>,
        marcelo.cerri@canonical.com, tim.gardner@canonical.com,
        khalid.elmously@canonical.com, philip.cox@canonical.com,
        aarcange@redhat.com, peterx@redhat.com, x86@kernel.org,
        linux-mm@kvack.org, linux-coco@lists.linux.dev,
        linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCHv11 0/9] mm, x86/cc, efi: Implement support for unaccepted
 memory
Message-ID: <20230516232204.3k53vh6pdnimdrhi@box.shutemov.name>
References: <20230513220418.19357-1-kirill.shutemov@linux.intel.com>
 <aa35a445-f410-b11f-bf21-3e482647faae@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <aa35a445-f410-b11f-bf21-3e482647faae@amd.com>
Precedence: bulk

On Tue, May 16, 2023 at 05:41:55PM -0500, Tom Lendacky wrote:
> On 5/13/23 17:04, Kirill A. Shutemov wrote:
> > UEFI Specification version 2.9 introduces the concept of memory
> > acceptance: some Virtual Machine platforms, such as Intel TDX or AMD
> > SEV-SNP, requiring memory to be accepted before it can be used by the
> > guest. Accepting happens via a protocol specific for the Virtual
> > Machine platform.
> > 
> > Accepting memory is costly and it makes VMM allocate memory for the
> > accepted guest physical address range. It's better to postpone memory
> > acceptance until memory is needed. It lowers boot time and reduces
> > memory overhead.
> > 
> > The kernel needs to know what memory has been accepted. Firmware
> > communicates this information via memory map: a new memory type --
> > EFI_UNACCEPTED_MEMORY -- indicates such memory.
> > 
> > Range-based tracking works fine for firmware, but it gets bulky for
> > the kernel: e820 has to be modified on every page acceptance. It leads
> > to table fragmentation, but there's a limited number of entries in the
> > e820 table
> > 
> > Another option is to mark such memory as usable in e820 and track if the
> > range has been accepted in a bitmap. One bit in the bitmap represents
> > 2MiB in the address space: one 4k page is enough to track 64GiB or
> > physical address space.
> > 
> > In the worst-case scenario -- a huge hole in the middle of the
> > address space -- It needs 256MiB to handle 4PiB of the address
> > space.
> > 
> > Any unaccepted memory that is not aligned to 2M gets accepted upfront.
> > 
> > The approach lowers boot time substantially. Boot to shell is ~2.5x
> > faster for 4G TDX VM and ~4x faster for 64G.
> > 
> > TDX-specific code isolated from the core of unaccepted memory support. It
> > supposed to help to plug-in different implementation of unaccepted memory
> > such as SEV-SNP.
> > 
> > -- Fragmentation study --
> > 
> > Vlastimil and Mel were concern about effect of unaccepted memory on
> > fragmentation prevention measures in page allocator. I tried to evaluate
> > it, but it is tricky. As suggested I tried to run multiple parallel kernel
> > builds and follow how often kmem:mm_page_alloc_extfrag gets hit.
> > 
> > See results in the v9 of the patchset[1][2]
> > 
> > [1] https://lore.kernel.org/all/20230330114956.20342-1-kirill.shutemov@linux.intel.com
> > [2] https://lore.kernel.org/all/20230416191940.ex7ao43pmrjhru2p@box.shutemov.name
> > 
> > --
> > 
> > The tree can be found here:
> > 
> > https://github.com/intel/tdx.git guest-unaccepted-memory
> 
> I get some failures when building without TDX support selected in my
> kernel config after adding unaccepted memory support for SNP:
> 
>   In file included from arch/x86/boot/compressed/../../coco/tdx/tdx-shared.c:1,
>                    from arch/x86/boot/compressed/tdx-shared.c:2:
>   ./arch/x86/include/asm/tdx.h: In function ‘tdx_kvm_hypercall’:
>   ./arch/x86/include/asm/tdx.h:72:17: error: ‘ENODEV’ undeclared (first use in this function)
>      72 |         return -ENODEV;
>         |                 ^~~~~~
>   ./arch/x86/include/asm/tdx.h:72:17: note: each undeclared identifier is reported only once for each function it appears in
> 
> Adding an include for linux/errno.h gets past that error, but then
> I get the following:
> 
>   ld: arch/x86/boot/compressed/tdx-shared.o: in function `tdx_enc_status_changed_phys':
>   tdx-shared.c:(.text+0x42): undefined reference to `__tdx_hypercall'
>   ld: tdx-shared.c:(.text+0x7f): undefined reference to `__tdx_module_call'
>   ld: tdx-shared.c:(.text+0xce): undefined reference to `__tdx_module_call'
>   ld: tdx-shared.c:(.text+0x13b): undefined reference to `__tdx_module_call'
>   ld: tdx-shared.c:(.text+0x153): undefined reference to `cc_mkdec'
>   ld: tdx-shared.c:(.text+0x15d): undefined reference to `cc_mkdec'
>   ld: tdx-shared.c:(.text+0x18e): undefined reference to `__tdx_hypercall'
>   ld: arch/x86/boot/compressed/vmlinux: hidden symbol `__tdx_hypercall' isn't defined
>   ld: final link failed: bad value
> 
> So it looks like arch/x86/boot/compressed/tdx-shared.c is being
> built, while arch/x86/boot/compressed/tdx.c isn't.

Right. I think this should help:

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 78f67e0a2666..b13a58021086 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -106,8 +106,8 @@ ifdef CONFIG_X86_64
 endif

 vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o
-vmlinux-objs-$(CONFIG_INTEL_TDX_GUEST) += $(obj)/tdx.o $(obj)/tdcall.o
-vmlinux-objs-$(CONFIG_UNACCEPTED_MEMORY) += $(obj)/mem.o $(obj)/tdx-shared.o
+vmlinux-objs-$(CONFIG_INTEL_TDX_GUEST) += $(obj)/tdx.o $(obj)/tdcall.o $(obj)/tdx-shared.o
+vmlinux-objs-$(CONFIG_UNACCEPTED_MEMORY) += $(obj)/mem.o

 vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi.o
 vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_mixed.o

> After setting TDX in the kernel config, I can build successfully, but
> I'm running into an error when trying to accept memory during
> decompression.
> 
> In drivers/firmware/efi/libstub/unaccepted_memory.c, I can see that the
> unaccepted_table is allocated, but when accept_memory() is invoked the
> table address is now zero. I thought maybe it had to do with bss, but even
> putting it in the .data section didn't help. I'll keep digging, but if you
> have any ideas, that would be great.

Not right away. But maybe seeing your side of enabling would help.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov