On Thu, Sep 07, 2023 at 01:49:14PM +0300, Kirill A. Shutemov wrote:
> On Wed, Sep 06, 2023 at 11:17:12AM +0200, Ard Biesheuvel wrote:
> > On Fri, 18 Aug 2023 at 17:16, Kirill A. Shutemov
> > <kirill.shutemov@xxxxxxxxxxxxxxx> wrote:
> > >
> > > On Thu, Aug 17, 2023 at 10:25:56PM +0200, Ard Biesheuvel wrote:
> > > > On Wed, 16 Aug 2023 at 23:24, Kirill A. Shutemov
> > > > <kirill.shutemov@xxxxxxxxxxxxxxx> wrote:
> > > > >
> > > > > e820__end_of_ram_pfn() is used to calculate max_pfn which, among other
> > > > > things, guides where direct mapping ends. Any memory above max_pfn is
> > > > > not going to be present in the direct mapping.
> > > > >
> > > > > e820__end_of_ram_pfn() finds the end of the ram based on the highest
> > > > > E820_TYPE_RAM range. But it doesn't includes E820_TYPE_ACPI ranges into
> > > > > calculation.
> > > > >
> > > > > Despite the name, E820_TYPE_ACPI covers not only ACPI data, but also EFI
> > > > > tables and might be required by kernel to function properly.
> > > > >
> > > > > Usually the problem is hidden because there is some E820_TYPE_RAM memory
> > > > > above E820_TYPE_ACPI. But crashkernel only presents pre-allocated crash
> > > > > memory as E820_TYPE_RAM on boot. If the preallocated range is small, it
> > > > > can fit under the last E820_TYPE_ACPI range.
> > > > >
> > > > > Modify e820__end_of_ram_pfn() and e820__end_of_low_ram_pfn() to cover
> > > > > E820_TYPE_ACPI memory.
> > > > >
> > > > > The problem was discovered during debugging kexec for TDX guest. TDX
> > > > > guest uses E820_TYPE_ACPI to store the unaccepted memory bitmap and pass
> > > > > it between the kernels on kexec.
> > > > >
> > > > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> > > >
> > > > No objections to this, but we might also simply drop E820_TYPE_ACPI
> > > > altogether: it is only used for EFI_ACPI_RECLAIM_MEMORY, which is
> > > > memory that can be used by the OS as ordinary RAM if it is not
> > > > interested in the contents (or has already consumed them). So this
> > > > could arguably be classified as E820_TYPE_RAM too.
> > >
> > > Hm. I'm not sure about this. E820_TYPE_ACPI also get tracked as
> > > IORES_DESC_ACPI_TABLES resource and get passed to the next kernel on
> > > kexec, regardless if it is crash kernel or not. I'm not sure we would not
> > > break anything.
> > >
> >
> > Yeah, you're right. So this patch is necessary in any case.
> >
> > Do we also need the EFI side patch then?
>
> Yes, we need it to get it mapped into the crashkernel direct mapping.
Ughh. The patch alone causes crash as EFI_ACPI_RELACLAIM_MEMORY is not
mapped into direct mapping during memory init.
The patch below fixes the issue.
I should have noticed it before, but I had essentially the same patch in
my tree for a different reason. Sorry for that :/
Please apply the patch below.
>From b5d1faf9e515195c58cc5e34132284894fca17f2 Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>
Date: Mon, 14 Aug 2023 19:12:47 +0300
Subject: [PATCH] efi/unaccepted: Make sure unaccepted table is mapped in
crashkernel case
Unaccepted table is now allocated from EFI_ACPI_RELACLAIM_MEMORY. It
translates into E820_TYPE_ACPI, which is not added to memblock and
therefore not mapped in the direct mapping.
It causes crash on the first touch of the table.
Use memblock_add() to make sure that the table is mapped in direct
mapping.
Align the range to the nearest page boarders. Ranges smaller than page
size are not going to be mapped.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
Fixes: e7761d827e99 ("efi/unaccepted: Use ACPI reclaim memory for unaccepted memory table")
---
drivers/firmware/efi/efi.c | 32 +++++++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 1599f1176842..4f409652b3c6 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -623,6 +623,34 @@ static __init int match_config_table(const efi_guid_t *guid,
return 0;
}
+/**
+ * reserve_unaccepted - Map and reserve unaccepted configuration table
+ * @unaccepted: Pointer to unaccepted memory table
+ *
+ * memblock_add() makes sure that the table is mapped in direct mapping. During
+ * normal boot it happens automatically because the table is allocated from
+ * usable memory. But during crashkernel boot only memory specifically
+ * reserved for crash scenario is mapped. memblock_add() forces the table to be
+ * mapped in crashkernel case.
+ *
+ * Align the range to the nearest page boarders. Ranges smaller than page size
+ * are not going to be mapped.
+ *
+ * memblock_reserve() makes sure that future allocations will not touch the
+ * table.
+ */
+
+static __init void reserve_unaccepted(struct efi_unaccepted_memory *unaccepted)
+{
+ phys_addr_t start, size;
+
+ start = PAGE_ALIGN_DOWN(efi.unaccepted);
+ size = PAGE_ALIGN(sizeof(*unaccepted) + unaccepted->size);
+
+ memblock_add(start, size);
+ memblock_reserve(start, size);
+}
+
int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
int count,
const efi_config_table_type_t *arch_tables)
@@ -751,11 +779,9 @@ int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
unaccepted = early_memremap(efi.unaccepted, sizeof(*unaccepted));
if (unaccepted) {
- unsigned long size;
if (unaccepted->version == 1) {
- size = sizeof(*unaccepted) + unaccepted->size;
- memblock_reserve(efi.unaccepted, size);
+ reserve_unaccepted(unaccepted);
} else {
efi.unaccepted = EFI_INVALID_TABLE_ADDR;
}
--
Kiryl Shutsemau / Kirill A. Shutemov
