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" (&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?
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" (&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
[PATCH v1] xen: add late init call in start_xen
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.
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
---
xen/arch/arm/setup.c | 2 ++
xen/arch/arm/xen.lds.S | 2 ++
xen/arch/x86/setup.c | 3 +++
xen/arch/x86/xen.lds.S | 2 ++
xen/common/kernel.c| 9 -
xen/include/xen/init.h | 3 +++
6 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 85ff956..332a207 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -1063,6 +1063,8 @@ void __init start_xen(unsigned long boot_phys_offset,
/* Hide UART from DOM0 if we're using it */
serial_endboot();
+do_late_initcalls();
+
if ( (rc = xsm_set_system_active()) != 0 )
panic("xsm: unable to switch to SYSTEM_ACTIVE privilege: %d\n", rc);
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 1e986e2..215e2c3 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -163,6 +163,8 @@ SECTIONS
__presmp_initcall_end = .;
*(.initcall1.init)
__initcall_end = .;
+ *(.initcalllate.init)
+ __late_initcall_end = .;
. = ALIGN(4);
__alt_instructions = .;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index f08b07b..d59298b 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1952,6 +1952,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
setup_io_bitmap(dom0);
+do_late_initcalls();
+
if ( bsp_delay_spec_ctrl )
{
info->spec_ctrl_flags &= ~SCF_use_shadow;
@@ -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");
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 8930e14..c90c7b0 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -250,6 +250,8 @@ SECTIONS
__presmp_initcall_end = .;
*(.initcall1.init)
__initcall_end = .;
+ *(.initcalllate.init)
+ __late_initcall_end = .;
*(.init.data)
*(.init.data.rel)
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index f8134d3..5a3d037 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -369,7 +369,7 @@ void add_taint(unsigned int flag)
}
extern const initcall_t __initcall_start[], __presmp_initcall_end[],
-__initcall_end[];
+__initcall_end[], __late_initcall_end[];
void __init do_presmp_initcalls(void)
{
@@ -385,6 +385,13 @@ void __init do_initcalls(void)
(*call)();
}
+void __init do_late_initcalls(void)
+{
+const initcall_t *call;
+for ( call = __initcall_end; call < __late_initcall_end; call++ )
+(*call)();
+}
+
#ifdef CONFIG_HYPFS
static unsigned int __read_mostly major_version;
static unsigned int __read_mostly minor_version;
diff --git a/xen/include/xen/init.h b/xen/include/xen/init.h
index 0af0e23..48210ee 100644
--- a/xen/include/xen/init.h
+++ b/xen/include/xen/init.h
@@ -68,11 +68,14 @@ typedef void (*exitcall_t)(void);
const static initcall_t __initcall_##fn __init_call("presmp") = fn
#define __initcall(fn) \
const static initcall_t __initcall_##fn __init_call("1") = fn
+#define __late_initcall(fn) \
+const static initcall_t __ini
