Re: [PATCH v1 1/5] console: add relocation hook

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 25/05/2025 3:15 pm, Marek Marczykowski-Górecki wrote:
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 25189541244d..3ef819a252e4 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1481,6 +1481,8 @@ void asmlinkage __init noreturn __start_xen(void)
>          highmem_start &= ~((1UL << L3_PAGETABLE_SHIFT) - 1);
>  #endif
>  
> +    console_init_pre_relocate();
> +
>      /*
>       * Iterate backwards over all superpage-aligned RAM regions.
>       *
> @@ -1606,6 +1608,12 @@ void asmlinkage __init noreturn __start_xen(void)
>      if ( !xen_phys_start )
>          panic("Not enough memory to relocate Xen\n");
>  
> +    /*
> +     * Notify console drivers about relocation, before reusing old Xen's
> +     * memory.
> +     */
> +    console_init_post_relocate();
> +

With reference to the next patch, there are printk()'s in this region
which want to work (in case something goes very wrong), so I don't think
setting dbc->suspended is the best approach.

In dbc_uart_init_pre_relocate(), you just wait for all transfers to
complete.  Can't this be merged with post_relocate(), at which point the
intermediate printk()'s will work too?  It will also remove a hook.

Meanwhile, we have things like:

    /* Cache {,compat_}gdt_l1e now that physically relocation is done. */
    this_cpu(gdt_l1e) =
        l1e_from_pfn(virt_to_mfn(boot_gdt), __PAGE_HYPERVISOR_RW);
    if ( IS_ENABLED(CONFIG_PV32) )
        this_cpu(compat_gdt_l1e) =
            l1e_from_pfn(virt_to_mfn(boot_compat_gdt), __PAGE_HYPERVISOR_RW);


in traps_init() which really doesn't belong here, but does belong in
some kind of general "relocation done" mechanism.

I wonder if we want a new type of initcall for this, allowing individual
areas of code to opt in with less boilerpate?

~Andrew