Re: [PATCH v1] xen: add late init call in start_xen
On Fri, 29 Jul 2022, Boyoun Park wrote: > I really appreciate all the comments and reviews. > I understand that it is hard to add this patch without any usage. > > On Fri, 29 Jul 2022, Stefano Stabellini: > > On Thu, 28 Jul 2022, Jan Beulich wrote: > > > 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. > > > > Yeah. Boyoun, if you only have a couple of patches then it is fine to > > send them manually. Otherwise, if you have many patches, you can send > > them in attachment as tarball and I'll send them all to xen-devel using > > git-send-email for you (of course keeping you as original author and > > retaining all Signed-off-bys). > > You're awesome. Thanks you so much. I'm still requesting approvals to use git > send-email. > I'll let you know if I have to send many patches wihout git send-email. > > > > > 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 > > > > > > 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. > > > > Yeah, I think it would be cool to have late initcalls but it makes sense > > to add them if we have someone that makes use of them. > > I totally agree with your comments. Some drivers that I'm developing now and > use this function are specific to my hardware environment. > Thus, it seems difficult to arrange them in the near future. > I will update patches if I can suggest an actual use. I totally understand custom setups and non-upstreamable configurations and I have certainly some of them myself. However, I just wanted to let you know that we are fine with accepting platform specific drivers in Xen where it makes sense, for instance: - Renesas IPMMU-VMSA found in R-Car Gen3/Gen4 SoCs (an IOMMU driver) xen/drivers/passthrough/arm/ipmmu-vmsa.c - Xilinx EEMI firmware interface "mediator" in Xen (power management) xen/arch/arm/platforms/xilinx-zynqmp-eemi.c Cheers, Stefano
Re: [PATCH v1] xen: add late init call in start_xen
I really appreciate all the comments and reviews.
I understand that it is hard to add this patch without any usage.
On Fri, 29 Jul 2022, Stefano Stabellini:
> On Thu, 28 Jul 2022, Jan Beulich wrote:
> > 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.
>
> Yeah. Boyoun, if you only have a couple of patches then it is fine to
> send them manually. Otherwise, if you have many patches, you can send
> them in attachment as tarball and I'll send them all to xen-devel using
> git-send-email for you (of course keeping you as original author and
> retaining all Signed-off-bys).
You're awesome. Thanks you so much. I'm still requesting approvals to use git
send-email.
I'll let you know if I have to send many patches wihout git send-email.
> > > 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
> >
> > 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.
>
> Yeah, I think it would be cool to have late initcalls but it makes sense
> to add them if we have someone that makes use of them.
I totally agree with your comments. Some drivers that I'm developing now and
use this function are specific to my hardware environment.
Thus, it seems difficult to arrange them in the near future.
I will update patches if I can suggest an actual use.
> > > @@ -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" (_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?
It seems that this is unintentionally added while updating a commit base.
I'm so sorry for not checking thoroughly.
I will check carefully before sending next patches.
Boyoun Park
Re: [PATCH v1] xen: add late init call in start_xen
Hi Boyoun, Thanks for your interest in Xen and thanks for your contribution! On Thu, 28 Jul 2022, Jan Beulich wrote: > 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. Yeah. Boyoun, if you only have a couple of patches then it is fine to send them manually. Otherwise, if you have many patches, you can send them in attachment as tarball and I'll send them all to xen-devel using git-send-email for you (of course keeping you as original author and retaining all Signed-off-bys). > > 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 > > 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. Yeah, I think it would be cool to have late initcalls but it makes sense to add them if we have someone that makes use of them.
Re: [PATCH v1] xen: add late init call in start_xen
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
>
> To: xen-devel@lists.xenproject.org
>
> Cc: Stefano Stabellini
>
> Cc: Julien Grall
>
> Cc: Bertrand Marquis
>
> Cc: Volodymyr Babchuk
>
> Cc: Andrew Cooper
>
> Cc: George Dunlap
>
> Cc: Jan Beulich
>
> Cc: Wei Liu
>
> Cc: "Roger Pau Monné"
>
> 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
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" (_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
