Re: [PATCH v1] xen: add late init call in start_xen

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.07.2022 11:22, Boyoun Park wrote:
> Hello,
> 
> This patch added late_initcall to deal with
> 
> some init functions which should be called
> 
> after other init functions in start_xen.
> 
> If this patch is added,
> 
> then the original initcall in xen will be treated
> 
> as early_initcall and the late_initcall
> 
> which is added by this patch will be
> 
> called sequentially.
> 
> I cannot send patches through git send-email
> 
> due to some security issues in my work.
> 
> So intead, I just send the patches manually.

Which is fine, but you want to configure your mail client such that it
doesn't mangle the patch. Albeit I see that to cover for that at least
you've also attached both the patch and a cover letter. For a single
patch a cover letter can normally be omitted, but if you don't then
even if you're sending without "git send-email" you will want to send
both as separate mails, with the patch being a reply to the cover
letter thread root.

> Sorry for the inconvenience.
> 
> I made this patch during using xen for a project.
> 
> And I want to share it and ask for review.
> 
> Boyoun Park.
> 
> From: Boyoun Park <boyoun.park@xxxxxxxxxxx>
> 
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> 
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> 
> Cc: Julien Grall <julien@xxxxxxx>
> 
> Cc: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> 
> Cc: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> 
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> Cc: George Dunlap <george.dunlap@xxxxxxxxxx>
> 
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Cc: Wei Liu <wl@xxxxxxx>
> 
> Cc: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
> 
> Date: Tue, 15 Mar 2022 12:57:59 +0900
> 
> Subject: [PATCH v1] xen: add late init call in start_xen
> 
> This patch added late_initcall section in init.data.
> 
> The late initcall would be called after initcall
> 
> in the start_xen function.
> 
> Some initializing works on priority should be run
> 
> in do_initcalls and other non-prioritized works
> 
> would be run in do_late_initcalls.
> 
> To call some functions by late_initcall,
> 
> then it is possible by using
> 
> __late_initcall(/*Function Name*/);
> 
> Signed-off-by: Boyoun Park <boyoun.park@xxxxxxxxxxx>

So I could imagine this patch to be in a series where a subsequent
patch then adds an actual use of the new functionality. Without
that what you're proposing is to add dead code.

> @@ -1964,6 +1966,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> 
>       bsp_info = get_cpu_info_from_stack((unsigned long)bsp_stack);
> 
>       *bsp_info = *info;
> 
> +    /* Jump to the 1:1 virtual mappings of cpu0_stack. */
> 
>       asm volatile ("mov %[stk], %%rsp; jmp %c[fn]" ::
> 
>                     [stk] "g" (&bsp_info->guest_cpu_user_regs),
> 
>                     [fn] "i" (reinit_bsp_stack) : "memory");

How does this addition of a comment relate to the purpose of the
patch?

Jan