* [PATCH] riscv: force hart_lottery to put in .sdata section @ 2020-02-04 11:19 Zong Li 2020-02-04 11:39 ` Anup Patel 2020-02-10 6:42 ` Alex Ghiti 0 siblings, 2 replies; 6+ messages in thread From: Zong Li @ 2020-02-04 11:19 UTC (permalink / raw) To: paul.walmsley, palmer, aou, linux-riscv, linux-kernel; +Cc: Zong Li In PIC code model, the zero initialized data always be put in .bss section, so when building kernel as PIE, the hart_lottery won't present in small data section, and it causes more than one harts to get the lottery, because the main hart clears the content of .bss section immediately after it getting the lottery. Signed-off-by: Zong Li <zong.li@sifive.com> --- arch/riscv/kernel/setup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index 22b671dbbcf1..45c63dc06360 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -40,7 +40,7 @@ struct screen_info screen_info = { #endif /* The lucky hart to first increment this variable will boot the other cores */ -atomic_t hart_lottery; +atomic_t hart_lottery __section(.sdata); unsigned long boot_cpu_hartid; void __init parse_dtb(void) -- 2.25.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] riscv: force hart_lottery to put in .sdata section 2020-02-04 11:19 [PATCH] riscv: force hart_lottery to put in .sdata section Zong Li @ 2020-02-04 11:39 ` Anup Patel 2020-02-05 11:07 ` Zong Li 2020-02-10 6:42 ` Alex Ghiti 1 sibling, 1 reply; 6+ messages in thread From: Anup Patel @ 2020-02-04 11:39 UTC (permalink / raw) To: Zong Li Cc: linux-riscv, Albert Ou, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Paul Walmsley On Tue, Feb 4, 2020 at 4:49 PM Zong Li <zong.li@sifive.com> wrote: > > In PIC code model, the zero initialized data always be put in .bss > section, so when building kernel as PIE, the hart_lottery won't present > in small data section, and it causes more than one harts to get the > lottery, because the main hart clears the content of .bss section > immediately after it getting the lottery. > > Signed-off-by: Zong Li <zong.li@sifive.com> > --- > arch/riscv/kernel/setup.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > index 22b671dbbcf1..45c63dc06360 100644 > --- a/arch/riscv/kernel/setup.c > +++ b/arch/riscv/kernel/setup.c > @@ -40,7 +40,7 @@ struct screen_info screen_info = { > #endif > > /* The lucky hart to first increment this variable will boot the other cores */ > -atomic_t hart_lottery; > +atomic_t hart_lottery __section(.sdata); > unsigned long boot_cpu_hartid; > > void __init parse_dtb(void) > -- > 2.25.0 > > Looks good to me. Please ensure that it is tested with both RV32 and RV64. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] riscv: force hart_lottery to put in .sdata section 2020-02-04 11:39 ` Anup Patel @ 2020-02-05 11:07 ` Zong Li 2020-02-18 21:26 ` Palmer Dabbelt 0 siblings, 1 reply; 6+ messages in thread From: Zong Li @ 2020-02-05 11:07 UTC (permalink / raw) To: Anup Patel Cc: linux-riscv, Albert Ou, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Paul Walmsley On Tue, Feb 4, 2020 at 7:40 PM Anup Patel <anup@brainfault.org> wrote: > > On Tue, Feb 4, 2020 at 4:49 PM Zong Li <zong.li@sifive.com> wrote: > > > > In PIC code model, the zero initialized data always be put in .bss > > section, so when building kernel as PIE, the hart_lottery won't present > > in small data section, and it causes more than one harts to get the > > lottery, because the main hart clears the content of .bss section > > immediately after it getting the lottery. > > > > Signed-off-by: Zong Li <zong.li@sifive.com> > > --- > > arch/riscv/kernel/setup.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > > index 22b671dbbcf1..45c63dc06360 100644 > > --- a/arch/riscv/kernel/setup.c > > +++ b/arch/riscv/kernel/setup.c > > @@ -40,7 +40,7 @@ struct screen_info screen_info = { > > #endif > > > > /* The lucky hart to first increment this variable will boot the other cores */ > > -atomic_t hart_lottery; > > +atomic_t hart_lottery __section(.sdata); > > unsigned long boot_cpu_hartid; > > > > void __init parse_dtb(void) > > -- > > 2.25.0 > > > > > > Looks good to me. Please ensure that it is tested with both > RV32 and RV64. > I had tested it on RV32 and RV64, it works on both. > Reviewed-by: Anup Patel <anup@brainfault.org> > > Regards, > Anup ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] riscv: force hart_lottery to put in .sdata section 2020-02-05 11:07 ` Zong Li @ 2020-02-18 21:26 ` Palmer Dabbelt 2020-02-19 8:48 ` Zong Li 0 siblings, 1 reply; 6+ messages in thread From: Palmer Dabbelt @ 2020-02-18 21:26 UTC (permalink / raw) To: zong.li; +Cc: anup, linux-riscv, aou, linux-kernel, Paul Walmsley On Wed, 05 Feb 2020 03:07:52 PST (-0800), zong.li@sifive.com wrote: > On Tue, Feb 4, 2020 at 7:40 PM Anup Patel <anup@brainfault.org> wrote: >> >> On Tue, Feb 4, 2020 at 4:49 PM Zong Li <zong.li@sifive.com> wrote: >> > >> > In PIC code model, the zero initialized data always be put in .bss >> > section, so when building kernel as PIE, the hart_lottery won't present >> > in small data section, and it causes more than one harts to get the >> > lottery, because the main hart clears the content of .bss section >> > immediately after it getting the lottery. >> > >> > Signed-off-by: Zong Li <zong.li@sifive.com> >> > --- >> > arch/riscv/kernel/setup.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c >> > index 22b671dbbcf1..45c63dc06360 100644 >> > --- a/arch/riscv/kernel/setup.c >> > +++ b/arch/riscv/kernel/setup.c >> > @@ -40,7 +40,7 @@ struct screen_info screen_info = { >> > #endif >> > >> > /* The lucky hart to first increment this variable will boot the other cores */ >> > -atomic_t hart_lottery; >> > +atomic_t hart_lottery __section(.sdata); >> > unsigned long boot_cpu_hartid; >> > >> > void __init parse_dtb(void) >> > -- >> > 2.25.0 >> > >> > >> >> Looks good to me. Please ensure that it is tested with both >> RV32 and RV64. >> > > I had tested it on RV32 and RV64, it works on both. Can you be more specific about your setup? Before you patch, hart_lottery should be in .sbss, which we put inside .sdata. I'm a bit worried there's some other issue going on here that this is just masking. That said, putting sbss in sdata seems like a generally bad idea so I'd be in favor of getting rid of it. I've queued this up onto for-next, as even if it is fixing a bug related to the relocatable kernels we don't have support for that yet. I've also sent a patch that stops putting .sbss into .sdata. Thanks! >> Reviewed-by: Anup Patel <anup@brainfault.org> >> >> Regards, >> Anup ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] riscv: force hart_lottery to put in .sdata section 2020-02-18 21:26 ` Palmer Dabbelt @ 2020-02-19 8:48 ` Zong Li 0 siblings, 0 replies; 6+ messages in thread From: Zong Li @ 2020-02-19 8:48 UTC (permalink / raw) To: Palmer Dabbelt Cc: Anup Patel, linux-riscv, Albert Ou, linux-kernel@vger.kernel.org List, Paul Walmsley On Wed, Feb 19, 2020 at 5:26 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > On Wed, 05 Feb 2020 03:07:52 PST (-0800), zong.li@sifive.com wrote: > > On Tue, Feb 4, 2020 at 7:40 PM Anup Patel <anup@brainfault.org> wrote: > >> > >> On Tue, Feb 4, 2020 at 4:49 PM Zong Li <zong.li@sifive.com> wrote: > >> > > >> > In PIC code model, the zero initialized data always be put in .bss > >> > section, so when building kernel as PIE, the hart_lottery won't present > >> > in small data section, and it causes more than one harts to get the > >> > lottery, because the main hart clears the content of .bss section > >> > immediately after it getting the lottery. > >> > > >> > Signed-off-by: Zong Li <zong.li@sifive.com> > >> > --- > >> > arch/riscv/kernel/setup.c | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > >> > index 22b671dbbcf1..45c63dc06360 100644 > >> > --- a/arch/riscv/kernel/setup.c > >> > +++ b/arch/riscv/kernel/setup.c > >> > @@ -40,7 +40,7 @@ struct screen_info screen_info = { > >> > #endif > >> > > >> > /* The lucky hart to first increment this variable will boot the other cores */ > >> > -atomic_t hart_lottery; > >> > +atomic_t hart_lottery __section(.sdata); > >> > unsigned long boot_cpu_hartid; > >> > > >> > void __init parse_dtb(void) > >> > -- > >> > 2.25.0 > >> > > >> > > >> > >> Looks good to me. Please ensure that it is tested with both > >> RV32 and RV64. > >> > > > > I had tested it on RV32 and RV64, it works on both. > > Can you be more specific about your setup? Before you patch, hart_lottery I applied the Alex's CONFIG_RELOCATABLE patch (https://patchwork.kernel.org/patch/11349207/) on Linux 5.6-rc1, and selected the RELOCATABLE in Kconfig by manual, the firmware was using OpenSBI, then ran the Linux kernel on QEMU. (I ran the RV64 on sifive_u machine and RV32 on virt machine). > should be in .sbss, which we put inside .sdata. I'm a bit worried there's some > other issue going on here that this is just masking. That said, putting sbss > in sdata seems like a generally bad idea so I'd be in favor of getting rid of > it. It happened at the beginning, I would help to give a quick test the change you made. > > I've queued this up onto for-next, as even if it is fixing a bug related to the > relocatable kernels we don't have support for that yet. I've also sent a patch > that stops putting .sbss into .sdata. > > Thanks! > > >> Reviewed-by: Anup Patel <anup@brainfault.org> > >> > >> Regards, > >> Anup ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] riscv: force hart_lottery to put in .sdata section 2020-02-04 11:19 [PATCH] riscv: force hart_lottery to put in .sdata section Zong Li 2020-02-04 11:39 ` Anup Patel @ 2020-02-10 6:42 ` Alex Ghiti 1 sibling, 0 replies; 6+ messages in thread From: Alex Ghiti @ 2020-02-10 6:42 UTC (permalink / raw) To: Zong Li, paul.walmsley, palmer, aou, linux-riscv, linux-kernel Hi Zong, On 2/4/20 6:19 AM, Zong Li wrote: > In PIC code model, the zero initialized data always be put in .bss > section, so when building kernel as PIE, the hart_lottery won't present > in small data section, and it causes more than one harts to get the > lottery, because the main hart clears the content of .bss section > immediately after it getting the lottery. > > Signed-off-by: Zong Li <zong.li@sifive.com> > --- > arch/riscv/kernel/setup.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > index 22b671dbbcf1..45c63dc06360 100644 > --- a/arch/riscv/kernel/setup.c > +++ b/arch/riscv/kernel/setup.c > @@ -40,7 +40,7 @@ struct screen_info screen_info = { > #endif > > /* The lucky hart to first increment this variable will boot the other cores */ > -atomic_t hart_lottery; > +atomic_t hart_lottery __section(.sdata); > unsigned long boot_cpu_hartid; > > void __init parse_dtb(void) > Thanks for testing the relocatable patch in SMP and fixing this issue. It looks good to me too as I did not find any other variable being used before bss clearing so feel free to add: Reviewed-by: Alexandre Ghiti <alex@ghiti.fr> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-19 8:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-04 11:19 [PATCH] riscv: force hart_lottery to put in .sdata section Zong Li 2020-02-04 11:39 ` Anup Patel 2020-02-05 11:07 ` Zong Li 2020-02-18 21:26 ` Palmer Dabbelt 2020-02-19 8:48 ` Zong Li 2020-02-10 6:42 ` Alex Ghiti
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).