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(-)
From c30d5022529200b3a11f397acc2b494a5df26ae1 Mon Sep 17 00:00:00 2001
Message-Id: <cover.1658998872.git.boyoun.park@samsung.com>
From: Boyoun Park <boyoun.park@samsung.com>
To: xen-devel@lists.xenproject.org
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien@xen.org>
Cc: Bertrand Marquis <bertrand.marquis@arm.com>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Wei Liu <wl@xen.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Date: Thu, 28 Jul 2022 18:01:12 +0900
Subject: [PATCH v1] Add late_initcall for start_xen
Hello,
This patch added late_initcall to deal with
some init functions which should be called
after other init functions.
If this patch is merged,
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.
Boyoun Park (1):
xen: add late init call in start_xen
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(-)
--
2.7.4
From c30d5022529200b3a11f397acc2b494a5df26ae1 Mon Sep 17 00:00:00 2001
Message-Id: <c30d5022529200b3a11f397acc2b494a5df26ae1.1658998872.git.boyoun.park@samsung.com>
In-Reply-To: <cover.1658998872.git.boyoun.park@samsung.com>
References: <cover.1658998872.git.boyoun.park@samsung.com>
From: Boyoun Park <boyoun.park@samsung.com>
To: xen-devel@lists.xenproject.org
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien@xen.org>
Cc: Bertrand Marquis <bertrand.marquis@arm.com>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Wei Liu <wl@xen.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
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@samsung.com>
---
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 __initcall_##fn __init_call("late") = fn
#define __exitcall(fn) \
static exitcall_t __exitcall_##fn __exit_call = fn
void do_presmp_initcalls(void);
void do_initcalls(void);
+void do_late_initcalls(void);
#endif /* __ASSEMBLY__ */
--
2.7.4
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@samsung.com>
>
> To: xen-devel@lists.xenproject.org
>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
>
> Cc: Julien Grall <julien@xen.org>
>
> Cc: Bertrand Marquis <bertrand.marquis@arm.com>
>
> Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Cc: George Dunlap <george.dunlap@citrix.com>
>
> Cc: Jan Beulich <jbeulich@suse.com>
>
> Cc: Wei Liu <wl@xen.org>
>
> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
>
> 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@samsung.com>
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
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 <boyoun.park@samsung.com> > > 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 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 <boyoun.park@samsung.com>
> >
> > 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
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 <boyoun.park@samsung.com> > > > > > > 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
Thank you for your reply. I will seriously consider contributing
platform specific drivers. Currently, I make a sample usage by applying this
to a function in start_xen. I think functions in start_xen could be reduced
for readability and understandability.
These patches are just one of my suggestion. So feel free to reject it.
On Sat, 30 Jul 2022, Stefano Stabellini wrote:
> 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:
>>>>> 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@samsung.com>
>>>>
>>>> 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
From: Boyoun Park <boyoun.park@samsung.com>
Date: Tue, 15 Mar 2022 12:57:59 +0900
Subject: [PATCH v2 1/2] 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@samsung.com>
---
xen/arch/arm/setup.c | 2 ++
xen/arch/arm/xen.lds.S | 2 ++
xen/arch/x86/setup.c | 2 ++
xen/arch/x86/xen.lds.S | 2 ++
xen/common/kernel.c | 9 ++++++++-
xen/include/xen/init.h | 3 +++
6 files changed, 19 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..5dc6654 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;
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 __initcall_##fn __init_call("late") = fn
#define __exitcall(fn) \
static exitcall_t __exitcall_##fn __exit_call = fn
void do_presmp_initcalls(void);
void do_initcalls(void);
+void do_late_initcalls(void);
#endif /* __ASSEMBLY__ */
--
2.7.4
Boyoun ParkFrom 5fd58ac8679189a878609e45b4bce1e78dee4848 Mon Sep 17 00:00:00 2001
Message-Id: <5fd58ac8679189a878609e45b4bce1e78dee4848.1659492818.git.boyoun.park@samsung.com>
From: Boyoun Park <boyoun.park@samsung.com>
Date: Tue, 15 Mar 2022 12:57:59 +0900
Subject: [PATCH v2 1/2] 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@samsung.com>
---
xen/arch/arm/setup.c | 2 ++
xen/arch/arm/xen.lds.S | 2 ++
xen/arch/x86/setup.c | 2 ++
xen/arch/x86/xen.lds.S | 2 ++
xen/common/kernel.c | 9 ++++++++-
xen/include/xen/init.h | 3 +++
6 files changed, 19 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..5dc6654 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;
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 __initcall_##fn __init_call("late") = fn
#define __exitcall(fn) \
static exitcall_t __exitcall_##fn __exit_call = fn
void do_presmp_initcalls(void);
void do_initcalls(void);
+void do_late_initcalls(void);
#endif /* __ASSEMBLY__ */
--
2.7.4
Hi Boyoun, On 03/08/2022 03:40, Boyoun Park wrote: > Thank you for your reply. I will seriously consider contributing > platform specific drivers. Currently, I make a sample usage by applying this > to a function in start_xen. I think functions in start_xen could be reduced > for readability and understandability. > These patches are just one of my suggestion. So feel free to reject it. Please avoid top-posting. Also, I would suggest to read through [1] to check how patch series should be submitted. For instance... > > On Sat, 30 Jul 2022, Stefano Stabellini wrote: >> 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: >>>>>> 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@samsung.com> >>>>> >>>>> 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 > > From: Boyoun Park <boyoun.park@samsung.com> > Date: Tue, 15 Mar 2022 12:57:59 +0900 > Subject: [PATCH v2 1/2] xen: add late init call in start_xen A new version of a series is usually sent as a separate thread rather than in-reply to the previous version. Furthermore, as there is 2 patches, you need to provide a cover letter (even it doesn't contain much). Lastly (process wise), please provide a list of changes made in v2. I tend to prefer per-patch changelog, but one in the cover letter would also work. > > This patch added late_initcall section in init.data. > The late initcall would be called after initcall > in the start_xen function. I think this is a bit too vague. AFAIU, you want late_initcall() to happen *after* all the domains have been created but *before* they are unpaused. Is that correct? From the previous discussion, I saw you said you have drivers needing to call initlate. Could you briefly explain why they can't be called in initcall? > > Some initializing works on priority should be run > in do_initcalls and other non-prioritized works > would be run in do_late_initcalls. IIUC, you are saying that do_late_initcalls() was introduced for prioritization purpose. But then, there are also a difference in behavior (initcalls happens before creating the domains whereas late happens after). Therefore, if the priority is the only reasons, then I think we should introduce priority within the 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@samsung.com> > --- > xen/arch/arm/setup.c | 2 ++ > xen/arch/arm/xen.lds.S | 2 ++ > xen/arch/x86/setup.c | 2 ++ > xen/arch/x86/xen.lds.S | 2 ++ > xen/common/kernel.c | 9 ++++++++- > xen/include/xen/init.h | 3 +++ > 6 files changed, 19 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(); > + [...] > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index f08b07b..5dc6654 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(); > + It would be preferable if the call is done roughly at the same place on all architecture. So if it easier for a developer to know when this will be called (e.g. just after serial_endboot()). If you need to call the function at the different place, then I think this ought to be explained. Cheers, [1] https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches -- Julien Grall
On Wed, 3 Aug 2022, Julien Grall wrote: > Hi Boyoun, > > On 03/08/2022 03:40, Boyoun Park wrote: >> From: Boyoun Park <boyoun.park@samsung.com> >> Date: Tue, 15 Mar 2022 12:57:59 +0900 >> Subject: [PATCH v2 1/2] 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. > >I think this is a bit too vague. AFAIU, you want late_initcall() to >happen *after* all the domains have been created but *before* they are >unpaused. Is that correct? > > From the previous discussion, I saw you said you have drivers needing >to call initlate. Could you briefly explain why they can't be called in >initcall? > >> >> Some initializing works on priority should be run >> in do_initcalls and other non-prioritized works >> would be run in do_late_initcalls. > >IIUC, you are saying that do_late_initcalls() was introduced for >prioritization purpose. But then, there are also a difference in >behavior (initcalls happens before creating the domains whereas late >happens after). > >Therefore, if the priority is the only reasons, then I think we should >introduce priority within the initcalls. When I made the patch for the first time, there was a problem related to memory access from some of my drivers with original initcall which is not the problem in Xen mainline. But it is resolved now, so for my case, it seems that the location when it is called is more important now as you said. Most of my drivers are domain-specific so they are called in late boot time after creating domains. Similar to priority within the initcalls, I think it could be subdivided according to the functions' own purposes such as arch_initcall, device_initcall, and others in Linux Kernel. >> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c >> index f08b07b..5dc6654 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(); >> + > >It would be preferable if the call is done roughly at the same place on >all architecture. So if it easier for a developer to know when this will >be called (e.g. just after serial_endboot()). > >If you need to call the function at the different place, then I think >this ought to be explained. I understand your comments. The reason of the location should be explained. I also checked Jan's review. All of the reviews including yours are reasonable. It would be better to add this function with my specific drivers after removing lots of unclean codes. Thank you for all of your replies. Although it is my first time to send patches to open-source project, it helps a lot to understand what kinds of codes are appropriate to be contributed. I will make new thread if I can improve the ideas and patches. With gratitude, Boyoun Park
From: Boyoun Park <boyoun.park@samsung.com>
Date: Wed, 3 Aug 2022 10:31:55 +0900
Subject: [PATCH v2 2/2] xen: remove init_constructors out of start_xen
This patch removed init_constructors from start_xen
by using __late_initcall.
It can be applied to other init functions in start_xen
so that only main init functions are included in there.
To use __late_initcall, the format of a function should
be changed according to initcall. Thus, the return type
of init_constructors function is changed in this patch.
Change-Id: Ife13484d346cff15983aacbfefde21d508f4690a
Signed-off-by: Boyoun Park <boyoun.park@samsung.com>
---
xen/arch/arm/setup.c | 2 --
xen/arch/x86/setup.c | 2 --
xen/include/xen/lib.h | 2 +-
xen/lib/ctors.c | 6 +++++-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 332a207..6c88b31 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -1056,8 +1056,6 @@ void __init start_xen(unsigned long boot_phys_offset,
init_trace_bufs();
- init_constructors();
-
console_endboot();
/* Hide UART from DOM0 if we're using it */
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 5dc6654..603100c 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1941,8 +1941,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
init_trace_bufs();
- init_constructors();
-
console_endboot();
/* Hide UART from DOM0 if we're using it */
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 05ee1e1..8e08c29 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -211,7 +211,7 @@ extern void add_taint(unsigned int taint);
struct cpu_user_regs;
void cf_check dump_execstate(struct cpu_user_regs *);
-void init_constructors(void);
+int init_constructors(void);
/*
* bsearch - binary search an array of elements
diff --git a/xen/lib/ctors.c b/xen/lib/ctors.c
index 5bdc591..b54144b 100644
--- a/xen/lib/ctors.c
+++ b/xen/lib/ctors.c
@@ -4,7 +4,7 @@
typedef void (*ctor_func_t)(void);
extern const ctor_func_t __ctors_start[], __ctors_end[];
-void __init init_constructors(void)
+int __init init_constructors(void)
{
const ctor_func_t *f;
for ( f = __ctors_start; f < __ctors_end; ++f )
@@ -12,8 +12,12 @@ void __init init_constructors(void)
/* Putting this here seems as good (or bad) as any other place. */
BUILD_BUG_ON(sizeof(size_t) != sizeof(ssize_t));
+
+ return 0;
}
+__late_initcall(init_constructors);
+
/*
* Local variables:
* mode: C
--
2.7.4From 825a304d45c80b816b30fcbc81e1d3dab4f8c81a Mon Sep 17 00:00:00 2001
Message-Id: <825a304d45c80b816b30fcbc81e1d3dab4f8c81a.1659492818.git.boyoun.park@samsung.com>
In-Reply-To: <5fd58ac8679189a878609e45b4bce1e78dee4848.1659492818.git.boyoun.park@samsung.com>
References: <5fd58ac8679189a878609e45b4bce1e78dee4848.1659492818.git.boyoun.park@samsung.com>
From: Boyoun Park <boyoun.park@samsung.com>
Date: Wed, 3 Aug 2022 10:31:55 +0900
Subject: [PATCH v2 2/2] xen: remove init_constructors out of start_xen
This patch removed init_constructors from start_xen
by using __late_initcall.
It can be applied to other init functions in start_xen
so that only main init functions are included in there.
To use __late_initcall, the format of a function should
be changed according to initcall. Thus, the return type
of init_constructors function is changed in this patch.
Change-Id: Ife13484d346cff15983aacbfefde21d508f4690a
Signed-off-by: Boyoun Park <boyoun.park@samsung.com>
---
xen/arch/arm/setup.c | 2 --
xen/arch/x86/setup.c | 2 --
xen/include/xen/lib.h | 2 +-
xen/lib/ctors.c | 6 +++++-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 332a207..6c88b31 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -1056,8 +1056,6 @@ void __init start_xen(unsigned long boot_phys_offset,
init_trace_bufs();
- init_constructors();
-
console_endboot();
/* Hide UART from DOM0 if we're using it */
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 5dc6654..603100c 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1941,8 +1941,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
init_trace_bufs();
- init_constructors();
-
console_endboot();
/* Hide UART from DOM0 if we're using it */
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 05ee1e1..8e08c29 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -211,7 +211,7 @@ extern void add_taint(unsigned int taint);
struct cpu_user_regs;
void cf_check dump_execstate(struct cpu_user_regs *);
-void init_constructors(void);
+int init_constructors(void);
/*
* bsearch - binary search an array of elements
diff --git a/xen/lib/ctors.c b/xen/lib/ctors.c
index 5bdc591..b54144b 100644
--- a/xen/lib/ctors.c
+++ b/xen/lib/ctors.c
@@ -4,7 +4,7 @@
typedef void (*ctor_func_t)(void);
extern const ctor_func_t __ctors_start[], __ctors_end[];
-void __init init_constructors(void)
+int __init init_constructors(void)
{
const ctor_func_t *f;
for ( f = __ctors_start; f < __ctors_end; ++f )
@@ -12,8 +12,12 @@ void __init init_constructors(void)
/* Putting this here seems as good (or bad) as any other place. */
BUILD_BUG_ON(sizeof(size_t) != sizeof(ssize_t));
+
+ return 0;
}
+__late_initcall(init_constructors);
+
/*
* Local variables:
* mode: C
--
2.7.4
On 03.08.2022 04:44, Boyoun Park wrote: > From: Boyoun Park <boyoun.park@samsung.com> > Date: Wed, 3 Aug 2022 10:31:55 +0900 > Subject: [PATCH v2 2/2] xen: remove init_constructors out of start_xen > > This patch removed init_constructors from start_xen > by using __late_initcall. > It can be applied to other init functions in start_xen > so that only main init functions are included in there. Only if the relative order in which they're called doesn't matter. In fact ... > To use __late_initcall, the format of a function should > be changed according to initcall. Thus, the return type > of init_constructors function is changed in this patch. ... you're changing the point in time when the constructors are called. This _may_ be fine, but the reason(s) why it is would need supplying in the description. > Change-Id: Ife13484d346cff15983aacbfefde21d508f4690a Please omit such. > --- a/xen/include/xen/lib.h > +++ b/xen/include/xen/lib.h > @@ -211,7 +211,7 @@ extern void add_taint(unsigned int taint); > struct cpu_user_regs; > void cf_check dump_execstate(struct cpu_user_regs *); > > -void init_constructors(void); > +int init_constructors(void); This wants to be removed, with ... > --- a/xen/lib/ctors.c > +++ b/xen/lib/ctors.c > @@ -4,7 +4,7 @@ > typedef void (*ctor_func_t)(void); > extern const ctor_func_t __ctors_start[], __ctors_end[]; > > -void __init init_constructors(void) > +int __init init_constructors(void) ... static added here. > @@ -12,8 +12,12 @@ void __init init_constructors(void) > > /* Putting this here seems as good (or bad) as any other place. */ > BUILD_BUG_ON(sizeof(size_t) != sizeof(ssize_t)); > + > + return 0; > } > > +__late_initcall(init_constructors); > + > /* > * Local variables: > * mode: C We prefer to not leave a blank line between the function referenced by __initcall et al and the __initcall() itself. Jan
© 2016 - 2024 Red Hat, Inc.