On Wed, Aug 07, 2024 at 07:33:26PM +0200, Thomas Gleixner wrote:
> On Tue, Aug 06 2024 at 15:12, Yunhong Jiang wrote:
> > +static void __init hv_reserve_real_mode(void)
> > +{
> > + phys_addr_t mem;
> > + size_t size = real_mode_size_needed();
> > +
> > + /*
> > + * We only need the memory to be <4GB since the 64-bit trampoline goes
> > + * down to 32-bit mode.
> > + */
> > + mem = memblock_phys_alloc_range(size, PAGE_SIZE, 0, SZ_4G);
> > + if (!mem)
> > + panic("No sub-4G memory is available for the trampoline\n");
> > + set_real_mode_mem(mem);
> > +}
>
> We really don't need another copy of reserve_real_mode(). See uncompiled
> patch below. It does not panic when the allocation fails, but why do you
> want to panic in that case? If it's not there then the system boots with
> a single CPU, so what.
I created a separated patch on my v2 patch set with "Originally-by:" tag
following suggestion at
https://lore.kernel.org/lkml/20240711081317.GD4587@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
Please let me know if that's not the appropriate solution.
>
> > void __init hv_vtl_init_platform(void)
> > {
> > pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
> >
> > if (wakeup_mailbox_addr) {
> > x86_platform.hyper.is_private_mmio = hv_is_private_mmio_tdx;
> > + x86_platform.realmode_reserve = hv_reserve_real_mode;
> > } else {
> > x86_platform.realmode_reserve = x86_init_noop;
> > x86_platform.realmode_init = x86_init_noop;
> > @@ -259,7 +276,8 @@ int __init hv_vtl_early_init(void)
> > panic("XSAVE has to be disabled as it is not supported by this module.\n"
> > "Please add 'noxsave' to the kernel command line.\n");
> >
> > - real_mode_header = &hv_vtl_real_mode_header;
> > + if (!wakeup_mailbox_addr)
> > + real_mode_header = &hv_vtl_real_mode_header;
>
> Why is that not suffient to be done in hv_vtl_init_platform() inside the
> condition which clears x86_platform.realmode_reserve/init?
>
> x86_platform.realmode_init() is invoked from an early initcall while
> hv_vtl_init_platform() is called during early boot.
I created a separated patch for this change on my v2 patch set, since it's a
a separated logic change. I added a Suggested-by: tag to the patch, hope it's
ok.
Thanks
--jyh
>
> Thanks,
>
> tglx
> ---
> --- a/arch/x86/include/asm/x86_init.h
> +++ b/arch/x86/include/asm/x86_init.h
> @@ -31,12 +31,18 @@ struct x86_init_mpparse {
> * platform
> * @memory_setup: platform specific memory setup
> * @dmi_setup: platform specific DMI setup
> + * @realmode_limit: platform specific address limit for the realmode trampoline
> + * (default 1M)
> + * @reserve_bios: platform specific address limit for reserving the BIOS area
> + * (default 1M)
> */
> struct x86_init_resources {
> void (*probe_roms)(void);
> void (*reserve_resources)(void);
> char *(*memory_setup)(void);
> void (*dmi_setup)(void);
> + unsigned long realmode_limit;
> + unsigned long reserve_bios;
> };
>
> /**
> --- a/arch/x86/kernel/x86_init.c
> +++ b/arch/x86/kernel/x86_init.c
> @@ -8,6 +8,7 @@
> #include <linux/ioport.h>
> #include <linux/export.h>
> #include <linux/pci.h>
> +#include <linux/sizes.h>
>
> #include <asm/acpi.h>
> #include <asm/bios_ebda.h>
> @@ -68,6 +69,8 @@ struct x86_init_ops x86_init __initdata
> .reserve_resources = reserve_standard_io_resources,
> .memory_setup = e820__memory_setup_default,
> .dmi_setup = dmi_setup,
> + .realmode_limit = SZ_1M,
> + .reserve_bios = SZ_1M,
> },
>
> .mpparse = {
> --- a/arch/x86/realmode/init.c
> +++ b/arch/x86/realmode/init.c
> @@ -45,7 +45,7 @@ void load_trampoline_pgtable(void)
>
> void __init reserve_real_mode(void)
> {
> - phys_addr_t mem;
> + phys_addr_t mem, limit = x86_init.resources.realmode_limit;
> size_t size = real_mode_size_needed();
>
> if (!size)
> @@ -54,17 +54,15 @@ void __init reserve_real_mode(void)
> WARN_ON(slab_is_available());
>
> /* Has to be under 1M so we can execute real-mode AP code. */
> - mem = memblock_phys_alloc_range(size, PAGE_SIZE, 0, 1<<20);
> + mem = memblock_phys_alloc_range(size, PAGE_SIZE, 0, limit);
> if (!mem)
> - pr_info("No sub-1M memory is available for the trampoline\n");
> + pr_info("No memory below %lluM for the real-mode trampoline\n", limit >> 20);
> else
> set_real_mode_mem(mem);
>
> - /*
> - * Unconditionally reserve the entire first 1M, see comment in
> - * setup_arch().
> - */
> - memblock_reserve(0, SZ_1M);
> + /* Reserve the entire first 1M, if enabled. See comment in setup_arch(). */
> + if (x86_init.resources.reserve_bios)
> + memblock_reserve(0, x86_init.resources.reserve_bios);
> }
>
> static void __init sme_sev_setup_real_mode(struct trampoline_header *th)
