diff mbox series

[v2] RISC-V: fix sifive and thead section mismatches in errata

Message ID 20230429155247.12131-1-rdunlap@infradead.org
State New
Headers
Series [v2] RISC-V: fix sifive and thead section mismatches in errata |

Commit Message

Randy Dunlap April 29, 2023, 3:52 p.m. UTC 
  When CONFIG_MODULES is set, __init_or_module becomes <empty>, but when
CONFIG_MODULES is not set, __init_or_module becomes __init.
In the latter case, it causes section mismatch warnings:

WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> sifive_errata_patch_func (section: .init.text)
WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> thead_errata_patch_func (section: .init.text)

Fixes: bb3f89487fd9 ("RISC-V: hwprobe: Remove __init on probe_vendor_features()")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Albert Ou <aou@eecs.berkeley.edu>
Cc: linux-riscv@lists.infradead.org
Cc: Conor Dooley <conor@kernel.org>
Cc: Evan Green <evan@rivosinc.com>
---
v2: use corrected Fixes: commit info (thanks Conor)

 arch/riscv/errata/sifive/errata.c |    8 +++-----
 arch/riscv/errata/thead/errata.c  |    6 +++---
 2 files changed, 6 insertions(+), 8 deletions(-)

Comments

Evan Green April 29, 2023, 5:21 p.m. UTC | #1 
On Sat, Apr 29, 2023 at 8:52 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> When CONFIG_MODULES is set, __init_or_module becomes <empty>, but when
> CONFIG_MODULES is not set, __init_or_module becomes __init.
> In the latter case, it causes section mismatch warnings:
>
> WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> sifive_errata_patch_func (section: .init.text)
> WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> thead_errata_patch_func (section: .init.text)
>
> Fixes: bb3f89487fd9 ("RISC-V: hwprobe: Remove __init on probe_vendor_features()")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Albert Ou <aou@eecs.berkeley.edu>
> Cc: linux-riscv@lists.infradead.org
> Cc: Conor Dooley <conor@kernel.org>
> Cc: Evan Green <evan@rivosinc.com>
> ---

Thanks, Randy. I'm confused at how I didn't see that when I made the
original fix. I feel like repro of these section mismatch errors
depend on some other factor I'm not understanding. In any case:

Reviewed-by: Evan Green  <evan@rivosinc.com>
Conor Dooley April 29, 2023, 5:24 p.m. UTC | #2 
On Sat, Apr 29, 2023 at 10:21:39AM -0700, Evan Green wrote:
> On Sat, Apr 29, 2023 at 8:52 AM Randy Dunlap <rdunlap@infradead.org> wrote:
> >
> > When CONFIG_MODULES is set, __init_or_module becomes <empty>, but when
> > CONFIG_MODULES is not set, __init_or_module becomes __init.
> > In the latter case, it causes section mismatch warnings:
> >
> > WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> sifive_errata_patch_func (section: .init.text)
> > WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> thead_errata_patch_func (section: .init.text)
> >
> > Fixes: bb3f89487fd9 ("RISC-V: hwprobe: Remove __init on probe_vendor_features()")
> > Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> > Cc: Heiko Stuebner <heiko@sntech.de>
> > Cc: Paul Walmsley <paul.walmsley@sifive.com>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > Cc: Albert Ou <aou@eecs.berkeley.edu>
> > Cc: linux-riscv@lists.infradead.org
> > Cc: Conor Dooley <conor@kernel.org>
> > Cc: Evan Green <evan@rivosinc.com>
> > ---
> 
> Thanks, Randy. I'm confused at how I didn't see that when I made the
> original fix. I feel like repro of these section mismatch errors
> depend on some other factor I'm not understanding. In any case:

Perhaps you had a cut-down config that did not enable either of the
relevant ARCH_ options to get those errata compiled?

Thanks for changing the fixes tag Randy,
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.
Evan Green April 29, 2023, 5:48 p.m. UTC | #3 
On Sat, Apr 29, 2023 at 10:24 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Sat, Apr 29, 2023 at 10:21:39AM -0700, Evan Green wrote:
> > On Sat, Apr 29, 2023 at 8:52 AM Randy Dunlap <rdunlap@infradead.org> wrote:
> > >
> > > When CONFIG_MODULES is set, __init_or_module becomes <empty>, but when
> > > CONFIG_MODULES is not set, __init_or_module becomes __init.
> > > In the latter case, it causes section mismatch warnings:
> > >
> > > WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> sifive_errata_patch_func (section: .init.text)
> > > WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> thead_errata_patch_func (section: .init.text)
> > >
> > > Fixes: bb3f89487fd9 ("RISC-V: hwprobe: Remove __init on probe_vendor_features()")
> > > Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> > > Cc: Heiko Stuebner <heiko@sntech.de>
> > > Cc: Paul Walmsley <paul.walmsley@sifive.com>
> > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > Cc: Albert Ou <aou@eecs.berkeley.edu>
> > > Cc: linux-riscv@lists.infradead.org
> > > Cc: Conor Dooley <conor@kernel.org>
> > > Cc: Evan Green <evan@rivosinc.com>
> > > ---
> >
> > Thanks, Randy. I'm confused at how I didn't see that when I made the
> > original fix. I feel like repro of these section mismatch errors
> > depend on some other factor I'm not understanding. In any case:
>
> Perhaps you had a cut-down config that did not enable either of the
> relevant ARCH_ options to get those errata compiled?

It was weird, my original "fix" (the one listed in this Fixes tag) was
needed because while the hwprobe series was clean on Palmer's branch,
it generated a "section mismatch" on linux-next. As noted here, it was
only with !CONFIG_MODULES, so I explicitly remember generating that
config and checking it on linux-next to generate this "fix", and it
came out clean. It's like the robots are getting smarter.
-Evan
Randy Dunlap April 29, 2023, 5:58 p.m. UTC | #4 
On 4/29/23 10:48, Evan Green wrote:
> On Sat, Apr 29, 2023 at 10:24 AM Conor Dooley <conor@kernel.org> wrote:
>>
>> On Sat, Apr 29, 2023 at 10:21:39AM -0700, Evan Green wrote:
>>> On Sat, Apr 29, 2023 at 8:52 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>>>>
>>>> When CONFIG_MODULES is set, __init_or_module becomes <empty>, but when
>>>> CONFIG_MODULES is not set, __init_or_module becomes __init.
>>>> In the latter case, it causes section mismatch warnings:
>>>>
>>>> WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> sifive_errata_patch_func (section: .init.text)
>>>> WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> thead_errata_patch_func (section: .init.text)
>>>>
>>>> Fixes: bb3f89487fd9 ("RISC-V: hwprobe: Remove __init on probe_vendor_features()")
>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>>> Cc: Heiko Stuebner <heiko@sntech.de>
>>>> Cc: Paul Walmsley <paul.walmsley@sifive.com>
>>>> Cc: Palmer Dabbelt <palmer@dabbelt.com>
>>>> Cc: Albert Ou <aou@eecs.berkeley.edu>
>>>> Cc: linux-riscv@lists.infradead.org
>>>> Cc: Conor Dooley <conor@kernel.org>
>>>> Cc: Evan Green <evan@rivosinc.com>
>>>> ---
>>>
>>> Thanks, Randy. I'm confused at how I didn't see that when I made the
>>> original fix. I feel like repro of these section mismatch errors
>>> depend on some other factor I'm not understanding. In any case:
>>
>> Perhaps you had a cut-down config that did not enable either of the
>> relevant ARCH_ options to get those errata compiled?
> 
> It was weird, my original "fix" (the one listed in this Fixes tag) was
> needed because while the hwprobe series was clean on Palmer's branch,
> it generated a "section mismatch" on linux-next. As noted here, it was
> only with !CONFIG_MODULES, so I explicitly remember generating that
> config and checking it on linux-next to generate this "fix", and it
> came out clean. It's like the robots are getting smarter.
> -Evan

I observed the problem in 8 out of 20 randconfig builds,
using linux-next 20230427.
Evan Green April 29, 2023, 7:06 p.m. UTC | #5 
On Sat, Apr 29, 2023 at 10:58 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
>
>
> On 4/29/23 10:48, Evan Green wrote:
> > On Sat, Apr 29, 2023 at 10:24 AM Conor Dooley <conor@kernel.org> wrote:
> >>
> >> On Sat, Apr 29, 2023 at 10:21:39AM -0700, Evan Green wrote:
> >>> On Sat, Apr 29, 2023 at 8:52 AM Randy Dunlap <rdunlap@infradead.org> wrote:
> >>>>
> >>>> When CONFIG_MODULES is set, __init_or_module becomes <empty>, but when
> >>>> CONFIG_MODULES is not set, __init_or_module becomes __init.
> >>>> In the latter case, it causes section mismatch warnings:
> >>>>
> >>>> WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> sifive_errata_patch_func (section: .init.text)
> >>>> WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> thead_errata_patch_func (section: .init.text)
> >>>>
> >>>> Fixes: bb3f89487fd9 ("RISC-V: hwprobe: Remove __init on probe_vendor_features()")
> >>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> >>>> Cc: Heiko Stuebner <heiko@sntech.de>
> >>>> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> >>>> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> >>>> Cc: Albert Ou <aou@eecs.berkeley.edu>
> >>>> Cc: linux-riscv@lists.infradead.org
> >>>> Cc: Conor Dooley <conor@kernel.org>
> >>>> Cc: Evan Green <evan@rivosinc.com>
> >>>> ---
> >>>
> >>> Thanks, Randy. I'm confused at how I didn't see that when I made the
> >>> original fix. I feel like repro of these section mismatch errors
> >>> depend on some other factor I'm not understanding. In any case:
> >>
> >> Perhaps you had a cut-down config that did not enable either of the
> >> relevant ARCH_ options to get those errata compiled?
> >
> > It was weird, my original "fix" (the one listed in this Fixes tag) was
> > needed because while the hwprobe series was clean on Palmer's branch,
> > it generated a "section mismatch" on linux-next. As noted here, it was
> > only with !CONFIG_MODULES, so I explicitly remember generating that
> > config and checking it on linux-next to generate this "fix", and it
> > came out clean. It's like the robots are getting smarter.
> > -Evan
>
> I observed the problem in 8 out of 20 randconfig builds,
> using linux-next 20230427.

Oh interesting, so not the stock k210_nommu defconfig. That makes me
feel a little better at least.
-Evan
Conor Dooley April 29, 2023, 7:11 p.m. UTC | #6 
On Sat, Apr 29, 2023 at 12:06:19PM -0700, Evan Green wrote:
> On Sat, Apr 29, 2023 at 10:58 AM Randy Dunlap <rdunlap@infradead.org> wrote:
> >
> >
> >
> > On 4/29/23 10:48, Evan Green wrote:
> > > On Sat, Apr 29, 2023 at 10:24 AM Conor Dooley <conor@kernel.org> wrote:
> > >>
> > >> On Sat, Apr 29, 2023 at 10:21:39AM -0700, Evan Green wrote:
> > >>> On Sat, Apr 29, 2023 at 8:52 AM Randy Dunlap <rdunlap@infradead.org> wrote:
> > >>>>
> > >>>> When CONFIG_MODULES is set, __init_or_module becomes <empty>, but when
> > >>>> CONFIG_MODULES is not set, __init_or_module becomes __init.
> > >>>> In the latter case, it causes section mismatch warnings:
> > >>>>
> > >>>> WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> sifive_errata_patch_func (section: .init.text)
> > >>>> WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> thead_errata_patch_func (section: .init.text)
> > >>>>
> > >>>> Fixes: bb3f89487fd9 ("RISC-V: hwprobe: Remove __init on probe_vendor_features()")
> > >>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> > >>>> Cc: Heiko Stuebner <heiko@sntech.de>
> > >>>> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> > >>>> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > >>>> Cc: Albert Ou <aou@eecs.berkeley.edu>
> > >>>> Cc: linux-riscv@lists.infradead.org
> > >>>> Cc: Conor Dooley <conor@kernel.org>
> > >>>> Cc: Evan Green <evan@rivosinc.com>
> > >>>> ---
> > >>>
> > >>> Thanks, Randy. I'm confused at how I didn't see that when I made the
> > >>> original fix. I feel like repro of these section mismatch errors
> > >>> depend on some other factor I'm not understanding. In any case:
> > >>
> > >> Perhaps you had a cut-down config that did not enable either of the
> > >> relevant ARCH_ options to get those errata compiled?
> > >
> > > It was weird, my original "fix" (the one listed in this Fixes tag) was
> > > needed because while the hwprobe series was clean on Palmer's branch,
> > > it generated a "section mismatch" on linux-next. As noted here, it was
> > > only with !CONFIG_MODULES, so I explicitly remember generating that
> > > config and checking it on linux-next to generate this "fix", and it
> > > came out clean. It's like the robots are getting smarter.
> > > -Evan
> >
> > I observed the problem in 8 out of 20 randconfig builds,
> > using linux-next 20230427.
> 
> Oh interesting, so not the stock k210_nommu defconfig. That makes me
> feel a little better at least.

Ohh man, that's a pretty bad config to try use (if that's your default)
for build testing stuff. The k210_mmu defconfig doesn't enable anything
other than SOC_CANAAN.
I could reproduce Randy's issue on defconfig w/ CONFIG_MODULES disabled.
Evan Green May 1, 2023, 3:26 p.m. UTC | #7 
On Sat, Apr 29, 2023 at 12:11 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Sat, Apr 29, 2023 at 12:06:19PM -0700, Evan Green wrote:
> > On Sat, Apr 29, 2023 at 10:58 AM Randy Dunlap <rdunlap@infradead.org> wrote:
> > >
> > >
> > >
> > > On 4/29/23 10:48, Evan Green wrote:
> > > > On Sat, Apr 29, 2023 at 10:24 AM Conor Dooley <conor@kernel.org> wrote:
> > > >>
> > > >> On Sat, Apr 29, 2023 at 10:21:39AM -0700, Evan Green wrote:
> > > >>> On Sat, Apr 29, 2023 at 8:52 AM Randy Dunlap <rdunlap@infradead.org> wrote:
> > > >>>>
> > > >>>> When CONFIG_MODULES is set, __init_or_module becomes <empty>, but when
> > > >>>> CONFIG_MODULES is not set, __init_or_module becomes __init.
> > > >>>> In the latter case, it causes section mismatch warnings:
> > > >>>>
> > > >>>> WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> sifive_errata_patch_func (section: .init.text)
> > > >>>> WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> thead_errata_patch_func (section: .init.text)
> > > >>>>
> > > >>>> Fixes: bb3f89487fd9 ("RISC-V: hwprobe: Remove __init on probe_vendor_features()")
> > > >>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> > > >>>> Cc: Heiko Stuebner <heiko@sntech.de>
> > > >>>> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> > > >>>> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > >>>> Cc: Albert Ou <aou@eecs.berkeley.edu>
> > > >>>> Cc: linux-riscv@lists.infradead.org
> > > >>>> Cc: Conor Dooley <conor@kernel.org>
> > > >>>> Cc: Evan Green <evan@rivosinc.com>
> > > >>>> ---
> > > >>>
> > > >>> Thanks, Randy. I'm confused at how I didn't see that when I made the
> > > >>> original fix. I feel like repro of these section mismatch errors
> > > >>> depend on some other factor I'm not understanding. In any case:
> > > >>
> > > >> Perhaps you had a cut-down config that did not enable either of the
> > > >> relevant ARCH_ options to get those errata compiled?
> > > >
> > > > It was weird, my original "fix" (the one listed in this Fixes tag) was
> > > > needed because while the hwprobe series was clean on Palmer's branch,
> > > > it generated a "section mismatch" on linux-next. As noted here, it was
> > > > only with !CONFIG_MODULES, so I explicitly remember generating that
> > > > config and checking it on linux-next to generate this "fix", and it
> > > > came out clean. It's like the robots are getting smarter.
> > > > -Evan
> > >
> > > I observed the problem in 8 out of 20 randconfig builds,
> > > using linux-next 20230427.
> >
> > Oh interesting, so not the stock k210_nommu defconfig. That makes me
> > feel a little better at least.
>
> Ohh man, that's a pretty bad config to try use (if that's your default)
> for build testing stuff. The k210_mmu defconfig doesn't enable anything
> other than SOC_CANAAN.
> I could reproduce Randy's issue on defconfig w/ CONFIG_MODULES disabled.

That's the one that caught me before, so I remembered it as being
"different". I'll try what you describe above next time I'm hunting
for section mismatches.
-Evan
Conor Dooley May 1, 2023, 3:37 p.m. UTC | #8 
On Mon, May 01, 2023 at 08:26:19AM -0700, Evan Green wrote:
> On Sat, Apr 29, 2023 at 12:11 PM Conor Dooley <conor@kernel.org> wrote:

> > Ohh man, that's a pretty bad config to try use (if that's your default)
> > for build testing stuff. The k210_mmu defconfig doesn't enable anything
> > other than SOC_CANAAN.
> > I could reproduce Randy's issue on defconfig w/ CONFIG_MODULES disabled.
> 
> That's the one that caught me before, so I remembered it as being
> "different". I'll try what you describe above next time I'm hunting
> for section mismatches.

By nature of being nommu with lots of stuff disabled, it is a good niche
config to test - the nommu stuff mostly gets forgotten about..
On the other hand, it does skip the errata handling stuff which is what
caught us out here.

Cheers,
Conor.
Palmer Dabbelt May 1, 2023, 10:44 p.m. UTC | #9 
On Sat, 29 Apr 2023 08:52:47 -0700, Randy Dunlap wrote:
> When CONFIG_MODULES is set, __init_or_module becomes <empty>, but when
> CONFIG_MODULES is not set, __init_or_module becomes __init.
> In the latter case, it causes section mismatch warnings:
> 
> WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> sifive_errata_patch_func (section: .init.text)
> WARNING: modpost: vmlinux.o: section mismatch in reference: riscv_fill_cpu_mfr_info (section: .text) -> thead_errata_patch_func (section: .init.text)
> 
> [...]

Applied, thanks!

[1/1] RISC-V: fix sifive and thead section mismatches in errata
      https://git.kernel.org/palmer/c/a2a58b5ca124

Best regards,
diff mbox series

Patch

diff -- a/arch/riscv/errata/sifive/errata.c b/arch/riscv/errata/sifive/errata.c
--- a/arch/riscv/errata/sifive/errata.c
+++ b/arch/riscv/errata/sifive/errata.c
@@ -82,11 +82,9 @@  static void __init_or_module warn_miss_e
 	pr_warn("----------------------------------------------------------------\n");
 }
 
-void __init_or_module sifive_errata_patch_func(struct alt_entry *begin,
-					       struct alt_entry *end,
-					       unsigned long archid,
-					       unsigned long impid,
-					       unsigned int stage)
+void sifive_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
+			      unsigned long archid, unsigned long impid,
+			      unsigned int stage)
 {
 	struct alt_entry *alt;
 	u32 cpu_req_errata;
diff -- a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
--- a/arch/riscv/errata/thead/errata.c
+++ b/arch/riscv/errata/thead/errata.c
@@ -83,9 +83,9 @@  static u32 thead_errata_probe(unsigned i
 	return cpu_req_errata;
 }
 
-void __init_or_module thead_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
-					      unsigned long archid, unsigned long impid,
-					      unsigned int stage)
+void thead_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
+			     unsigned long archid, unsigned long impid,
+			     unsigned int stage)
 {
 	struct alt_entry *alt;
 	u32 cpu_req_errata = thead_errata_probe(stage, archid, impid);