Re: [PATCH] x86/mm: Make e820_end_ram_pfn() cover E820_TYPE_ACPI ranges

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 18 Sept 2023 at 14:13, Kirill A. Shutemov
<kirill.shutemov@xxxxxxxxxxxxxxx> wrote:
>
> On Mon, Sep 18, 2023 at 09:01:27AM +0200, Ard Biesheuvel wrote:
> > On Sun, 17 Sept 2023 at 19:06, Kirill A. Shutemov
> > <kirill.shutemov@xxxxxxxxxxxxxxx> wrote:
> > >
> > > 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.
> > >
> >
> > It would be good if this boot path could be covered by lkp@ as well -
> > currently, you are the only person testing this manually. The same
> > applies to SEV-SNP by the way - there is zero coverage except for the
> > manual testing that Boris or Tom Lendacky might do.
>
> It made me remember that I forgot attribute the finding:
>
> Reported-by: "Hongyu Ning" <hongyu.ning@xxxxxxxxx>
>

OK, I will add that next time I update the branch.

> Hongyu tests linux-next periodically, but I agree we need to get LKP
> working.
>
> I will try to get LKP going for TDX guest.
>

Yes, please. We are likely to run into such issues again if we don't
have test coverage for this functionality.

Thanks,
Ard.



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux