Re: [PATCH 2/3] x86/boot: Drop move_memory() and use memcpy() directly

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.12.2021 11:53, Andrew Cooper wrote:
> @@ -1243,7 +1196,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>               * data until after we have switched to the relocated pagetables!
>               */
>              barrier();
> -            move_memory(e, XEN_IMG_OFFSET, _end - _start, 1);
> +            memcpy(__va(__pa(_start)), _start, _end - _start);
>  
>              /* Walk idle_pg_table, relocating non-leaf entries. */
>              pl4e = __va(__pa(idle_pg_table));
> @@ -1300,8 +1253,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>                     "1" (__va(__pa(cpu0_stack))), "2" (STACK_SIZE / 8)
>                  : "memory" );
>  
> -            bootstrap_map(NULL);
> -
>              printk("New Xen image base address: %#lx\n", xen_phys_start);
>          }

This bootstrap_map() must have been dead code already before, except
for the "keep" argument above needlessly having got passed as 1? Afaict
passing 1 was pointless without using the function's return value.

> @@ -1325,9 +1276,10 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>                   (headroom ||
>                    ((end - size) >> PAGE_SHIFT) > mod[j].mod_start) )
>              {
> -                move_memory(end - size + headroom,
> -                            (uint64_t)mod[j].mod_start << PAGE_SHIFT,
> -                            mod[j].mod_end, 0);
> +                memcpy(__va(end - size + headroom),
> +                       __va((uint64_t)mod[j].mod_start << PAGE_SHIFT),
> +                       mod[j].mod_end);

I'm not convinced this can be memcpy() - consider_modules() specifically
allows for the current module's source and destination areas to overlap.
See also the comment ahead of its invocation a few lines up from here.

I'm also not convinced we have the source range (fully) direct-mapped at
this point. Only full superpages have been mapped so far, and only those
for the current or higher address E820 entries (plus of course the pre-
built mappings of the space below 1Gb [PREBUILT_MAP_LIMIT]) located
below 4Gb.

As to the 2nd argument - if this can indeed be converted in the first
place, may I suggest to also switch to using pfn_to_paddr()?

Jan