From: Borislav Petkov <bp@alien8.de>
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>,
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>,
Tom Lendacky <thomas.lendacky@amd.com>,
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>,
Dave Hansen <dave.hansen@intel.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: [PATCHv8 06/14] efi/x86: Implement support for unaccepted memory
Date: Tue, 3 Jan 2023 15:20:55 +0100 [thread overview]
Message-ID: <Y7Q5x7qV/Cmc/p8s@zn.tnic> (raw)
In-Reply-To: <20221207014933.8435-7-kirill.shutemov@linux.intel.com>
On Wed, Dec 07, 2022 at 04:49:25AM +0300, Kirill A. Shutemov wrote:
> The implementation requires some basic helpers in boot stub. They
> provided by linux/ includes in the main kernel image, but is not present
> in boot stub. Create copy of required functionality in the boot stub.
Leftover paragraph from a previous version. Can be removed.
...
> +/*
> + * The accepted memory bitmap only works at PMD_SIZE granularity. This
> + * function takes unaligned start/end addresses and either:
s/This function takes/Take/
> + * 1. Accepts the memory immediately and in its entirety
> + * 2. Accepts unaligned parts, and marks *some* aligned part unaccepted
> + *
> + * The function will never reach the bitmap_set() with zero bits to set.
> + */
> +void process_unaccepted_memory(struct boot_params *params, u64 start, u64 end)
> +{
> + /*
> + * Ensure that at least one bit will be set in the bitmap by
> + * immediately accepting all regions under 2*PMD_SIZE. This is
> + * imprecise and may immediately accept some areas that could
> + * have been represented in the bitmap. But, results in simpler
> + * code below
> + *
> + * Consider case like this:
> + *
> + * | 4k | 2044k | 2048k |
> + * ^ 0x0 ^ 2MB ^ 4MB
> + *
> + * Only the first 4k has been accepted. The 0MB->2MB region can not be
> + * represented in the bitmap. The 2MB->4MB region can be represented in
> + * the bitmap. But, the 0MB->4MB region is <2*PMD_SIZE and will be
> + * immediately accepted in its entirety.
> + */
> + if (end - start < 2 * PMD_SIZE) {
> + __accept_memory(start, end);
> + return;
> + }
> +
> + /*
> + * No matter how the start and end are aligned, at least one unaccepted
> + * PMD_SIZE area will remain to be marked in the bitmap.
> + */
> +
> + /* Immediately accept a <PMD_SIZE piece at the start: */
> + if (start & ~PMD_MASK) {
> + __accept_memory(start, round_up(start, PMD_SIZE));
> + start = round_up(start, PMD_SIZE);
> + }
> +
> + /* Immediately accept a <PMD_SIZE piece at the end: */
> + if (end & ~PMD_MASK) {
> + __accept_memory(round_down(end, PMD_SIZE), end);
> + end = round_down(end, PMD_SIZE);
> + }
> +
> + /*
> + * 'start' and 'end' are now both PMD-aligned.
> + * Record the range as being unaccepted:
> + */
> + bitmap_set((unsigned long *)params->unaccepted_memory,
> + start / PMD_SIZE, (end - start) / PMD_SIZE);
> +}
...
> diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
> index 6787ed8dfacf..8aa8adf0bcb5 100644
> --- a/drivers/firmware/efi/Kconfig
> +++ b/drivers/firmware/efi/Kconfig
> @@ -314,6 +314,20 @@ config EFI_COCO_SECRET
> virt/coco/efi_secret module to access the secrets, which in turn
> allows userspace programs to access the injected secrets.
>
> +config UNACCEPTED_MEMORY
> + bool
> + depends on EFI_STUB
This still doesn't make a whole lotta sense. If I do "make menuconfig" I don't
see the help text because that bool doesn't have a string prompt. So who is that
help text for?
Then, in the last patch you have
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -888,6 +888,8 @@ config INTEL_TDX_GUEST
select ARCH_HAS_CC_PLATFORM
select X86_MEM_ENCRYPT
select X86_MCE
+ select UNACCEPTED_MEMORY
+ select EFI_STUB
I guess you want to select UNACCEPTED_MEMORY only.
And I've already mentioned this whole mess:
https://lore.kernel.org/r/Yt%2BnOeLMqRxjObbx@zn.tnic
Please incorporate all review comments before sending a new version of
your patch.
Ignoring review feedback is a very unfriendly thing to do:
- if you agree with the feedback, you work it in in the next revision
- if you don't agree, you *say* *why* you don't
> + help
> + Some Virtual Machine platforms, such as Intel TDX, require
> + some memory to be "accepted" by the guest before it can be used.
> + This mechanism helps prevent malicious hosts from making changes
> + to guest memory.
> +
> + UEFI specification v2.9 introduced EFI_UNACCEPTED_MEMORY memory type.
> +
> + This option adds support for unaccepted memory and makes such memory
> + usable by the kernel.
...
> +static efi_status_t allocate_unaccepted_bitmap(struct boot_params *params,
> + __u32 nr_desc,
> + struct efi_boot_memmap *map)
> +{
> + unsigned long *mem = NULL;
> + u64 size, max_addr = 0;
> + efi_status_t status;
> + bool found = false;
> + int i;
> +
> + /* Check if there's any unaccepted memory and find the max address */
> + for (i = 0; i < nr_desc; i++) {
> + efi_memory_desc_t *d;
> + unsigned long m = (unsigned long)map->map;
> +
> + d = efi_early_memdesc_ptr(m, map->desc_size, i);
> + if (d->type == EFI_UNACCEPTED_MEMORY)
> + found = true;
> + if (d->phys_addr + d->num_pages * PAGE_SIZE > max_addr)
> + max_addr = d->phys_addr + d->num_pages * PAGE_SIZE;
> + }
> +
> + if (!found) {
> + params->unaccepted_memory = 0;
> + return EFI_SUCCESS;
> + }
> +
> + /*
> + * If unaccepted memory is present, allocate a bitmap to track what
> + * memory has to be accepted before access.
> + *
> + * One bit in the bitmap represents 2MiB in the address space:
> + * A 4k bitmap can track 64GiB of 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.
> + *
> + * TODO: handle situation if params->unaccepted_memory is already set.
> + * It's required to deal with kexec.
A TODO in a patch basically says this patch is not ready to go anywhere.
IOW, you need to handle that kexec case here gracefully. Even if you refuse to
boot a kexec-ed kernel because it cannot support handing in the bitmap from the
first kernel, yadda yadda...
> + *
> + * The bitmap will be populated in setup_e820() according to the memory
> + * map after efi_exit_boot_services().
> + */
> + size = DIV_ROUND_UP(max_addr, PMD_SIZE * BITS_PER_BYTE);
> + status = efi_allocate_pages(size, (unsigned long *)&mem, ULONG_MAX);
> + if (status == EFI_SUCCESS) {
> + memset(mem, 0, size);
> + params->unaccepted_memory = (unsigned long)mem;
> + }
> +
> + return status;
> +}
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
next prev parent reply other threads:[~2023-01-03 14:21 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-07 1:49 [PATCHv8 00/14] mm, x86/cc: Implement support for unaccepted memory Kirill A. Shutemov
2022-12-07 1:49 ` [PATCHv8 01/14] x86/boot: Centralize __pa()/__va() definitions Kirill A. Shutemov
2022-12-07 1:49 ` [PATCHv8 02/14] mm: Add support for unaccepted memory Kirill A. Shutemov
2022-12-09 18:10 ` Vlastimil Babka
2022-12-09 19:26 ` Kirill A. Shutemov
2022-12-09 22:23 ` Vlastimil Babka
2022-12-24 16:46 ` Kirill A. Shutemov
2023-01-12 11:59 ` Vlastimil Babka
2022-12-26 12:23 ` Borislav Petkov
2022-12-27 3:18 ` Kirill A. Shutemov
2023-01-16 13:04 ` Mel Gorman
2022-12-07 1:49 ` [PATCHv8 03/14] mm: Report unaccepted memory in meminfo Kirill A. Shutemov
2022-12-07 1:49 ` [PATCHv8 04/14] efi/x86: Get full memory map in allocate_e820() Kirill A. Shutemov
2022-12-07 1:49 ` [PATCHv8 05/14] x86/boot: Add infrastructure required for unaccepted memory support Kirill A. Shutemov
2023-01-03 13:52 ` Borislav Petkov
2022-12-07 1:49 ` [PATCHv8 06/14] efi/x86: Implement support for unaccepted memory Kirill A. Shutemov
2023-01-03 14:20 ` Borislav Petkov [this message]
2023-03-25 0:51 ` Kirill A. Shutemov
2022-12-07 1:49 ` [PATCHv8 07/14] x86/boot/compressed: Handle " Kirill A. Shutemov
2022-12-07 1:49 ` [PATCHv8 08/14] x86/mm: Reserve unaccepted memory bitmap Kirill A. Shutemov
2022-12-07 1:49 ` [PATCHv8 09/14] x86/mm: Provide helpers for unaccepted memory Kirill A. Shutemov
2022-12-07 1:49 ` [PATCHv8 10/14] x86/mm: Avoid load_unaligned_zeropad() stepping into " Kirill A. Shutemov
2022-12-07 1:49 ` [PATCHv8 11/14] x86: Disable kexec if system has " Kirill A. Shutemov
2022-12-07 1:49 ` [PATCHv8 12/14] x86/tdx: Make _tdx_hypercall() and __tdx_module_call() available in boot stub Kirill A. Shutemov
2022-12-07 1:49 ` [PATCHv8 13/14] x86/tdx: Refactor try_accept_one() Kirill A. Shutemov
2022-12-07 1:49 ` [PATCHv8 14/14] x86/tdx: Add unaccepted memory support Kirill A. Shutemov
2022-12-08 15:29 ` [PATCH v6 0/5] Provide SEV-SNP support for unaccepted memory Tom Lendacky
2022-12-08 15:29 ` [PATCH v6 1/5] x86/sev: Fix calculation of end address based on number of pages Tom Lendacky
2022-12-08 15:29 ` [PATCH v6 2/5] x86/sev: Put PSC struct on the stack in prep for unaccepted memory support Tom Lendacky
2022-12-08 15:29 ` [PATCH v6 3/5] x86/sev: Allow for use of the early boot GHCB for PSC requests Tom Lendacky
2022-12-08 15:29 ` [PATCH v6 4/5] x86/sev: Use large PSC requests if applicable Tom Lendacky
2022-12-08 15:29 ` [PATCH v6 5/5] x86/sev: Add SNP-specific unaccepted memory support Tom Lendacky
2022-12-08 22:12 ` Kirill A. Shutemov
2022-12-09 14:18 ` Tom Lendacky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y7Q5x7qV/Cmc/p8s@zn.tnic \
--to=bp@alien8.de \
--cc=aarcange@redhat.com \
--cc=ak@linux.intel.com \
--cc=akpm@linux-foundation.org \
--cc=ardb@kernel.org \
--cc=dave.hansen@intel.com \
--cc=david@redhat.com \
--cc=dfaggioli@suse.com \
--cc=jroedel@suse.de \
--cc=khalid.elmously@canonical.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-coco@lists.linux.dev \
--cc=linux-efi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=luto@kernel.org \
--cc=marcelo.cerri@canonical.com \
--cc=mgorman@techsingularity.net \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.com \
--cc=peterz@infradead.org \
--cc=philip.cox@canonical.com \
--cc=rientjes@google.com \
--cc=rppt@kernel.org \
--cc=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=seanjc@google.com \
--cc=tglx@linutronix.de \
--cc=thomas.lendacky@amd.com \
--cc=tim.gardner@canonical.com \
--cc=vbabka@suse.cz \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).