* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Mark Rutland @ 2022-01-27 12:27 UTC (permalink / raw)
To: Sven Schnelle
Cc: keescook, linux-kernel, Steven Rostedt, Sachin Sant, Yinan Liu,
linuxppc-dev, ardb
In-Reply-To: <yt9dy231gzae.fsf@linux.ibm.com>
On Thu, Jan 27, 2022 at 01:04:41PM +0100, Sven Schnelle wrote:
> Mark Rutland <mark.rutland@arm.com> writes:
>
> >> Isn't x86 relocatable in some configurations (e.g. for KASLR)?
> >>
> >> I can't see how the sort works for those cases, because the mcount_loc entries
> >> are absolute, and either:
> >>
> >> * The sorted entries will get overwritten by the unsorted relocation entries,
> >> and won't be sorted.
> >>
> >> * The sorted entries won't get overwritten, but then the absolute address will
> >> be wrong since they hadn't been relocated.
> >>
> >> How does that work?
>
> From what i've seen when looking into this ftrace sort problem x86 has a
> a relocation tool, which is run before final linking: arch/x86/tools/relocs.c
> This tools converts all the required relocations to three types:
>
> - 32 bit relocations
> - 64 bit relocations
> - inverse 32 bit relocations
>
> These are added to the end of the image.
>
> The decompressor then iterates over that array, and just adds/subtracts
> the KASLR offset - see arch/x86/boot/compressed/misc.c, handle_relocations()
>
> So IMHO x86 never uses 'real' relocations during boot, and just
> adds/subtracts. That's why the order stays the same, and the compile
> time sort works.
Ah, so those non-ELF relocations for the mcount_loc table just mean "apply the
KASLR offset here", which is equivalent for all entries.
That makes sense, thanks!
Mark.
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Steven Rostedt @ 2022-01-27 12:46 UTC (permalink / raw)
To: Mark Rutland
Cc: keescook, linux-kernel, Sven Schnelle, Sachin Sant, Yinan Liu,
linuxppc-dev, ardb
In-Reply-To: <YfKPmFJ2MGsem4VB@FVFF77S0Q05N>
On Thu, 27 Jan 2022 12:27:04 +0000
Mark Rutland <mark.rutland@arm.com> wrote:
> Ah, so those non-ELF relocations for the mcount_loc table just mean "apply the
> KASLR offset here", which is equivalent for all entries.
>
> That makes sense, thanks!
And this is why we were having such a hard time understanding each other ;-)
I started a new project called "shelf", which is a shell interface to
read ELF files (Shelf on a ELF!).
It uses my ccli library:
https://github.com/rostedt/libccli
and can be found here:
https://github.com/rostedt/shelf
Build and install the latest libccli and then build this with just
"make".
$ shelf vmlinux
and then you can see what is stored in the mcount location:
shelf> dump symbol __start_mcount_loc - __stop_mcount_loc
I plan on adding more to include the REL and RELA sections and show how
they affect symbols and such.
Feel free to contribute too ;-)
-- Steve
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Mark Rutland @ 2022-01-27 12:59 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Kees Cook, Linux Kernel Mailing List, Steven Rostedt, Sachin Sant,
Yinan Liu, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <CAMj1kXGsmK9pBmgwmMEr302eCHtL=cqs4jqM_jOiK-bLO2gQog@mail.gmail.com>
On Thu, Jan 27, 2022 at 01:22:17PM +0100, Ard Biesheuvel wrote:
> On Thu, 27 Jan 2022 at 13:20, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Thu, Jan 27, 2022 at 01:03:34PM +0100, Ard Biesheuvel wrote:
> >
> > > These architectures use place-relative extables for the same reason:
> > > place relative references are resolved at build time rather than at
> > > runtime during relocation, making a build time sort feasible.
> > >
> > > arch/alpha/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > arch/arm64/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > arch/ia64/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > arch/parisc/include/asm/uaccess.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > arch/powerpc/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > arch/riscv/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > arch/s390/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > arch/x86/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > >
> > > Note that the swap routine becomes something like the below, given
> > > that the relative references need to be fixed up after the entry
> > > changes place in the sorted list.
> > >
> > > static void swap_ex(void *a, void *b, int size)
> > > {
> > > struct exception_table_entry *x = a, *y = b, tmp;
> > > int delta = b - a;
> > >
> > > tmp = *x;
> > > x->insn = y->insn + delta;
> > > y->insn = tmp.insn - delta;
> > > ...
> > > }
> > >
> > > As a bonus, the resulting footprint of the table in the image is
> > > reduced by 8x, given that every 8 byte pointer has an accompanying 24
> > > byte RELA record, so we go from 32 bytes to 4 bytes for every call to
> > > __gnu_mcount_mc.
> >
> > Absolutely -- it'd be great if we could do that for the callsite locations; the
> > difficulty is that the entries are generated by the compiler itself, so we'd
> > either need some build/link time processing to convert each absolute 64-bit
> > value to a relative 32-bit offset, or new compiler options to generate those as
> > relative offsets from the outset.
>
> Don't we use scripts/recordmcount.pl for that?
Not quite -- we could adjust it to do so, but today it doesn't consider
existing mcount_loc entries, and just generates new ones where the compiler has
generated calls to mcount, which it finds by scanning the instructions in the
binary. Today it is not used:
* On arm64 when we default to using `-fpatchable-function-entry=N`. That makes
the compiler insert 2 NOPs in the function prologue, and log the location of
that NOP sled to a section called. `__patchable_function_entries`.
We need the compiler to do that since we can't reliably identify 2 NOPs in a
function prologue as being intended to be a patch site, as e.g. there could
be notrace functions where the compiler had to insert NOPs for alignment of a
subsequent brnach or similar.
* On architectures with `-nop-mcount`. On these, it's necessary to use
`-mrecord-mcount` to have the compiler log the patch-site, for the same
reason as with `-fpatchable-function-entry`.
* On architectures with `-mrecord-mcount` generally, since this avoids the
overhead of scanning each object.
* On x86 when objtool is used.
Thanks,
Mark.
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Ard Biesheuvel @ 2022-01-27 13:07 UTC (permalink / raw)
To: Mark Rutland
Cc: Kees Cook, Linux Kernel Mailing List, Steven Rostedt, Sachin Sant,
Yinan Liu, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <YfKXM0wBfTh0V8+L@FVFF77S0Q05N>
On Thu, 27 Jan 2022 at 13:59, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Thu, Jan 27, 2022 at 01:22:17PM +0100, Ard Biesheuvel wrote:
> > On Thu, 27 Jan 2022 at 13:20, Mark Rutland <mark.rutland@arm.com> wrote:
> > > On Thu, Jan 27, 2022 at 01:03:34PM +0100, Ard Biesheuvel wrote:
> > >
> > > > These architectures use place-relative extables for the same reason:
> > > > place relative references are resolved at build time rather than at
> > > > runtime during relocation, making a build time sort feasible.
> > > >
> > > > arch/alpha/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > arch/arm64/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > arch/ia64/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > arch/parisc/include/asm/uaccess.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > arch/powerpc/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > arch/riscv/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > arch/s390/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > arch/x86/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > >
> > > > Note that the swap routine becomes something like the below, given
> > > > that the relative references need to be fixed up after the entry
> > > > changes place in the sorted list.
> > > >
> > > > static void swap_ex(void *a, void *b, int size)
> > > > {
> > > > struct exception_table_entry *x = a, *y = b, tmp;
> > > > int delta = b - a;
> > > >
> > > > tmp = *x;
> > > > x->insn = y->insn + delta;
> > > > y->insn = tmp.insn - delta;
> > > > ...
> > > > }
> > > >
> > > > As a bonus, the resulting footprint of the table in the image is
> > > > reduced by 8x, given that every 8 byte pointer has an accompanying 24
> > > > byte RELA record, so we go from 32 bytes to 4 bytes for every call to
> > > > __gnu_mcount_mc.
> > >
> > > Absolutely -- it'd be great if we could do that for the callsite locations; the
> > > difficulty is that the entries are generated by the compiler itself, so we'd
> > > either need some build/link time processing to convert each absolute 64-bit
> > > value to a relative 32-bit offset, or new compiler options to generate those as
> > > relative offsets from the outset.
> >
> > Don't we use scripts/recordmcount.pl for that?
>
> Not quite -- we could adjust it to do so, but today it doesn't consider
> existing mcount_loc entries, and just generates new ones where the compiler has
> generated calls to mcount, which it finds by scanning the instructions in the
> binary. Today it is not used:
>
> * On arm64 when we default to using `-fpatchable-function-entry=N`. That makes
> the compiler insert 2 NOPs in the function prologue, and log the location of
> that NOP sled to a section called. `__patchable_function_entries`.
>
> We need the compiler to do that since we can't reliably identify 2 NOPs in a
> function prologue as being intended to be a patch site, as e.g. there could
> be notrace functions where the compiler had to insert NOPs for alignment of a
> subsequent brnach or similar.
>
> * On architectures with `-nop-mcount`. On these, it's necessary to use
> `-mrecord-mcount` to have the compiler log the patch-site, for the same
> reason as with `-fpatchable-function-entry`.
>
> * On architectures with `-mrecord-mcount` generally, since this avoids the
> overhead of scanning each object.
>
> * On x86 when objtool is used.
>
Right.
I suppose that on arm64, we can work around this by passing
--apply-dynamic-relocs to the linker, so that all R_AARCH64_RELATIVE
targets are prepopulated with the link time value of the respective
addresses. It does cause some bloat, which is why we disable that
today, but we could make that dependent on ftrace being enabled.
I do wonder how much over head we accumulate, though, by having all
these relocations, but I suppose that is the situation today in any
case.
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Mark Rutland @ 2022-01-27 13:08 UTC (permalink / raw)
To: Steven Rostedt
Cc: keescook, linux-kernel, Sven Schnelle, Sachin Sant, Yinan Liu,
linuxppc-dev, ardb
In-Reply-To: <20220127074601.41a3773d@rorschach.local.home>
On Thu, Jan 27, 2022 at 07:46:01AM -0500, Steven Rostedt wrote:
> On Thu, 27 Jan 2022 12:27:04 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
>
> > Ah, so those non-ELF relocations for the mcount_loc table just mean "apply the
> > KASLR offset here", which is equivalent for all entries.
> >
> > That makes sense, thanks!
>
> And this is why we were having such a hard time understanding each other ;-)
;)
With that in mind, I think that we understand that the build-time sort works
for:
* arch/x86, becuase the non-ELF relocations for mcount_loc happen to be
equivalent.
* arch/arm, because there's no dynamic relocaiton and the mcount_loc entries
have been finalized prior to sorting.
... but doesn't work for anyone else (including arm64) because the ELF
relocations are not equivalent, and need special care that is not yet
implemented.
So we should have arm and x86 opt-in, but for now everyone else (including
arm64, powerpc, s390) should be left with the prior behaviour with the runtime
sort only (in case the build-time sort breaks anything, as I mentioned in my
other mail).
Does that sound about right?
In future we might be able to do something much smarter (e.g. adding some
preprocessing to use relative entries).
I'll take a look at shelf. :)
Thanks,
Mark.
> I started a new project called "shelf", which is a shell interface to
> read ELF files (Shelf on a ELF!).
>
> It uses my ccli library:
>
> https://github.com/rostedt/libccli
>
> and can be found here:
>
> https://github.com/rostedt/shelf
>
> Build and install the latest libccli and then build this with just
> "make".
>
> $ shelf vmlinux
>
> and then you can see what is stored in the mcount location:
>
> shelf> dump symbol __start_mcount_loc - __stop_mcount_loc
>
> I plan on adding more to include the REL and RELA sections and show how
> they affect symbols and such.
>
> Feel free to contribute too ;-)
>
> -- Steve
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Mark Rutland @ 2022-01-27 13:24 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Kees Cook, Linux Kernel Mailing List, Steven Rostedt, Sachin Sant,
Yinan Liu, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <CAMj1kXF3Fg2O=5prQnUWeNFeCqojP1z2zDXMwxFdcNtWLfL7Vg@mail.gmail.com>
On Thu, Jan 27, 2022 at 02:07:03PM +0100, Ard Biesheuvel wrote:
> On Thu, 27 Jan 2022 at 13:59, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Thu, Jan 27, 2022 at 01:22:17PM +0100, Ard Biesheuvel wrote:
> > > On Thu, 27 Jan 2022 at 13:20, Mark Rutland <mark.rutland@arm.com> wrote:
> > > > On Thu, Jan 27, 2022 at 01:03:34PM +0100, Ard Biesheuvel wrote:
> > > >
> > > > > These architectures use place-relative extables for the same reason:
> > > > > place relative references are resolved at build time rather than at
> > > > > runtime during relocation, making a build time sort feasible.
> > > > >
> > > > > arch/alpha/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > arch/arm64/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > arch/ia64/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > arch/parisc/include/asm/uaccess.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > arch/powerpc/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > arch/riscv/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > arch/s390/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > arch/x86/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > >
> > > > > Note that the swap routine becomes something like the below, given
> > > > > that the relative references need to be fixed up after the entry
> > > > > changes place in the sorted list.
> > > > >
> > > > > static void swap_ex(void *a, void *b, int size)
> > > > > {
> > > > > struct exception_table_entry *x = a, *y = b, tmp;
> > > > > int delta = b - a;
> > > > >
> > > > > tmp = *x;
> > > > > x->insn = y->insn + delta;
> > > > > y->insn = tmp.insn - delta;
> > > > > ...
> > > > > }
> > > > >
> > > > > As a bonus, the resulting footprint of the table in the image is
> > > > > reduced by 8x, given that every 8 byte pointer has an accompanying 24
> > > > > byte RELA record, so we go from 32 bytes to 4 bytes for every call to
> > > > > __gnu_mcount_mc.
> > > >
> > > > Absolutely -- it'd be great if we could do that for the callsite locations; the
> > > > difficulty is that the entries are generated by the compiler itself, so we'd
> > > > either need some build/link time processing to convert each absolute 64-bit
> > > > value to a relative 32-bit offset, or new compiler options to generate those as
> > > > relative offsets from the outset.
> > >
> > > Don't we use scripts/recordmcount.pl for that?
> >
> > Not quite -- we could adjust it to do so, but today it doesn't consider
> > existing mcount_loc entries, and just generates new ones where the compiler has
> > generated calls to mcount, which it finds by scanning the instructions in the
> > binary. Today it is not used:
> >
> > * On arm64 when we default to using `-fpatchable-function-entry=N`. That makes
> > the compiler insert 2 NOPs in the function prologue, and log the location of
> > that NOP sled to a section called. `__patchable_function_entries`.
> >
> > We need the compiler to do that since we can't reliably identify 2 NOPs in a
> > function prologue as being intended to be a patch site, as e.g. there could
> > be notrace functions where the compiler had to insert NOPs for alignment of a
> > subsequent brnach or similar.
> >
> > * On architectures with `-nop-mcount`. On these, it's necessary to use
> > `-mrecord-mcount` to have the compiler log the patch-site, for the same
> > reason as with `-fpatchable-function-entry`.
> >
> > * On architectures with `-mrecord-mcount` generally, since this avoids the
> > overhead of scanning each object.
> >
> > * On x86 when objtool is used.
> >
>
> Right.
>
> I suppose that on arm64, we can work around this by passing
> --apply-dynamic-relocs to the linker, so that all R_AARCH64_RELATIVE
> targets are prepopulated with the link time value of the respective
> addresses. It does cause some bloat, which is why we disable that
> today, but we could make that dependent on ftrace being enabled.
We'd also need to teach the build-time sort to update the relocations, unless
you mean to also change the boot-time reloc code to RMW with the offset?
I think for right now the best thing is to disable the build-time sort for
arm64, but maybe something like that is the right thing to do longer term.
> I do wonder how much over head we accumulate, though, by having all
> these relocations, but I suppose that is the situation today in any
> case.
Yeah; I suspect if we want to do something about that we want to do it more
generally, and would probably need to do something like the x86 approach and
rewrite the relocs at build-time to something more compressed. If we applied
the dynamic relocs with the link-time address we'd only need 4 bytes to
identify each pointer to apply an offset to.
I'm not exactly sure how we could do that, nor what the trade-off look like in
practice.
THanks,
Mark.
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Mark Rutland @ 2022-01-27 13:33 UTC (permalink / raw)
To: Sven Schnelle
Cc: keescook, hca, linux-kernel, Steven Rostedt, Sachin Sant,
Yinan Liu, linuxppc-dev, ardb
In-Reply-To: <yt9dsft9gvyo.fsf@linux.ibm.com>
On Thu, Jan 27, 2022 at 02:16:31PM +0100, Sven Schnelle wrote:
> Mark Rutland <mark.rutland@arm.com> writes:
>
> > On Thu, Jan 27, 2022 at 07:46:01AM -0500, Steven Rostedt wrote:
> >> On Thu, 27 Jan 2022 12:27:04 +0000
> >> Mark Rutland <mark.rutland@arm.com> wrote:
> >>
> >> > Ah, so those non-ELF relocations for the mcount_loc table just mean "apply the
> >> > KASLR offset here", which is equivalent for all entries.
> >> >
> >> > That makes sense, thanks!
> >>
> >> And this is why we were having such a hard time understanding each other ;-)
> >
> > ;)
> >
> > With that in mind, I think that we understand that the build-time sort works
> > for:
> >
> > * arch/x86, becuase the non-ELF relocations for mcount_loc happen to be
> > equivalent.
> >
> > * arch/arm, because there's no dynamic relocaiton and the mcount_loc entries
> > have been finalized prior to sorting.
> >
> > ... but doesn't work for anyone else (including arm64) because the ELF
> > relocations are not equivalent, and need special care that is not yet
> > implemented.
>
> For s390 my idea is to just skip the addresses between __start_mcount_loc
> and __stop_mcount_loc, because for these addresses we know that they are
> 64 bits wide, so we just need to add the KASLR offset.
>
> I'm thinking about something like this:
>
> diff --git a/arch/s390/boot/compressed/decompressor.h b/arch/s390/boot/compressed/decompressor.h
> index f75cc31a77dd..015d7e2e94ef 100644
> --- a/arch/s390/boot/compressed/decompressor.h
> +++ b/arch/s390/boot/compressed/decompressor.h
> @@ -25,6 +25,8 @@ struct vmlinux_info {
> unsigned long rela_dyn_start;
> unsigned long rela_dyn_end;
> unsigned long amode31_size;
> + unsigned long start_mcount_loc;
> + unsigned long stop_mcount_loc;
> };
>
> /* Symbols defined by linker scripts */
> diff --git a/arch/s390/boot/startup.c b/arch/s390/boot/startup.c
> index 1aa11a8f57dd..7bb0d88db5c6 100644
> --- a/arch/s390/boot/startup.c
> +++ b/arch/s390/boot/startup.c
> @@ -88,6 +88,11 @@ static void handle_relocs(unsigned long offset)
> dynsym = (Elf64_Sym *) vmlinux.dynsym_start;
> for (rela = rela_start; rela < rela_end; rela++) {
> loc = rela->r_offset + offset;
> + if ((loc >= vmlinux.start_mcount_loc) &&
> + (loc < vmlinux.stop_mcount_loc)) {
> + (*(unsigned long *)loc) += offset;
> + continue;
> + }
> val = rela->r_addend;
> r_sym = ELF64_R_SYM(rela->r_info);
> if (r_sym) {
> @@ -232,6 +237,8 @@ static void offset_vmlinux_info(unsigned long offset)
> vmlinux.rela_dyn_start += offset;
> vmlinux.rela_dyn_end += offset;
> vmlinux.dynsym_start += offset;
> + vmlinux.start_mcount_loc += offset;
> + vmlinux.stop_mcount_loc += offset;
> }
>
> static unsigned long reserve_amode31(unsigned long safe_addr)
> diff --git a/arch/s390/kernel/vmlinux.lds.S b/arch/s390/kernel/vmlinux.lds.S
> index 42c43521878f..51c773405608 100644
> --- a/arch/s390/kernel/vmlinux.lds.S
> +++ b/arch/s390/kernel/vmlinux.lds.S
> @@ -213,6 +213,8 @@ SECTIONS
> QUAD(__rela_dyn_start) /* rela_dyn_start */
> QUAD(__rela_dyn_end) /* rela_dyn_end */
> QUAD(_eamode31 - _samode31) /* amode31_size */
> + QUAD(__start_mcount_loc)
> + QUAD(__stop_mcount_loc)
> } :NONE
>
> /* Debugging sections. */
>
> Not sure whether that would also work on power, and also not sure
> whether i missed something thinking about that. Maybe it doesn't even
> work. ;-)
I don't know enough about s390 or powerpc relocs to say whether that works, but
I can say that approach isn't going to work for arm64 without other signficant
changes.
I want to get the regression fixed ASAP, so can we take a simple patch for -rc2
which disables the build-time sort where it's currently broken (by limiting the
opt-in to arm and x86), then follow-up per-architecture to re-enable it if
desired/safe?
Thanks,
Mark.
^ permalink raw reply
* Re: WARN_ON() is buggy for 32 bit systems
From: Dan Carpenter @ 2022-01-27 13:37 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <87y231l9hz.fsf@mpe.ellerman.id.au>
On Thu, Jan 27, 2022 at 10:10:32PM +1100, Michael Ellerman wrote:
> Dan Carpenter <dan.carpenter@oracle.com> writes:
> > On Wed, Jan 26, 2022 at 12:21:49PM +0000, Christophe Leroy wrote:
> >> The code is enclosed in a #ifdef CONFIG_PPC64, it is not used for PPC32:
> >>
> >> /arch/powerpc/include/asm/bug.h
> >> 99 #ifdef CONFIG_PPC64
> >
> > Ah...
> >
> > You know, life would be a lot easier for me personally if we added an
> > #ifndef __CHECKER__ as well... I can't compile PowerPC code so I can't
> > test a patch like that.
>
> Ubuntu & Fedora both have cross compilers packaged, or there's cross
> compilers on kernel.org. But I assume you mean you'd rather not bother
> compiling for powerpc, which is fair enough.
>
> Do you mean something like below?
Yes, please.
>
> I'm not sure about that, as it would prevent sparse from checking the
> actual BUG_ON code we're using, vs the generic version which we never
> use on 64-bit. Is there a smatch specific macro we could check?
There isn't a Smatch define. This shouldn't affect Sparse at all unless
there was a bug in the WARN_ON() macro.
regards,
dan carpenter
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Steven Rostedt @ 2022-01-27 13:55 UTC (permalink / raw)
To: Mark Rutland
Cc: keescook, hca, linux-kernel, Sven Schnelle, Sachin Sant,
Yinan Liu, linuxppc-dev, ardb
In-Reply-To: <YfKfDnbNGina2lKz@FVFF77S0Q05N>
On Thu, 27 Jan 2022 13:33:02 +0000
Mark Rutland <mark.rutland@arm.com> wrote:
> I want to get the regression fixed ASAP, so can we take a simple patch for -rc2
> which disables the build-time sort where it's currently broken (by limiting the
> opt-in to arm and x86), then follow-up per-architecture to re-enable it if
> desired/safe?
I'm going to retest my patch that makes it an opt in for just x86 and arm
(32bit). I'll be pushing that hopefully later today. I have some other
patches to test as well.
-- Steve
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Ard Biesheuvel @ 2022-01-27 13:59 UTC (permalink / raw)
To: Mark Rutland
Cc: Kees Cook, Linux Kernel Mailing List, Steven Rostedt, Sachin Sant,
Yinan Liu, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <YfKc8MDwenS1iXqQ@FVFF77S0Q05N>
On Thu, 27 Jan 2022 at 14:24, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Thu, Jan 27, 2022 at 02:07:03PM +0100, Ard Biesheuvel wrote:
> > On Thu, 27 Jan 2022 at 13:59, Mark Rutland <mark.rutland@arm.com> wrote:
> > >
> > > On Thu, Jan 27, 2022 at 01:22:17PM +0100, Ard Biesheuvel wrote:
> > > > On Thu, 27 Jan 2022 at 13:20, Mark Rutland <mark.rutland@arm.com> wrote:
> > > > > On Thu, Jan 27, 2022 at 01:03:34PM +0100, Ard Biesheuvel wrote:
> > > > >
> > > > > > These architectures use place-relative extables for the same reason:
> > > > > > place relative references are resolved at build time rather than at
> > > > > > runtime during relocation, making a build time sort feasible.
> > > > > >
> > > > > > arch/alpha/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > > arch/arm64/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > > arch/ia64/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > > arch/parisc/include/asm/uaccess.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > > arch/powerpc/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > > arch/riscv/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > > arch/s390/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > > arch/x86/include/asm/extable.h:#define ARCH_HAS_RELATIVE_EXTABLE
> > > > > >
> > > > > > Note that the swap routine becomes something like the below, given
> > > > > > that the relative references need to be fixed up after the entry
> > > > > > changes place in the sorted list.
> > > > > >
> > > > > > static void swap_ex(void *a, void *b, int size)
> > > > > > {
> > > > > > struct exception_table_entry *x = a, *y = b, tmp;
> > > > > > int delta = b - a;
> > > > > >
> > > > > > tmp = *x;
> > > > > > x->insn = y->insn + delta;
> > > > > > y->insn = tmp.insn - delta;
> > > > > > ...
> > > > > > }
> > > > > >
> > > > > > As a bonus, the resulting footprint of the table in the image is
> > > > > > reduced by 8x, given that every 8 byte pointer has an accompanying 24
> > > > > > byte RELA record, so we go from 32 bytes to 4 bytes for every call to
> > > > > > __gnu_mcount_mc.
> > > > >
> > > > > Absolutely -- it'd be great if we could do that for the callsite locations; the
> > > > > difficulty is that the entries are generated by the compiler itself, so we'd
> > > > > either need some build/link time processing to convert each absolute 64-bit
> > > > > value to a relative 32-bit offset, or new compiler options to generate those as
> > > > > relative offsets from the outset.
> > > >
> > > > Don't we use scripts/recordmcount.pl for that?
> > >
> > > Not quite -- we could adjust it to do so, but today it doesn't consider
> > > existing mcount_loc entries, and just generates new ones where the compiler has
> > > generated calls to mcount, which it finds by scanning the instructions in the
> > > binary. Today it is not used:
> > >
> > > * On arm64 when we default to using `-fpatchable-function-entry=N`. That makes
> > > the compiler insert 2 NOPs in the function prologue, and log the location of
> > > that NOP sled to a section called. `__patchable_function_entries`.
> > >
> > > We need the compiler to do that since we can't reliably identify 2 NOPs in a
> > > function prologue as being intended to be a patch site, as e.g. there could
> > > be notrace functions where the compiler had to insert NOPs for alignment of a
> > > subsequent brnach or similar.
> > >
> > > * On architectures with `-nop-mcount`. On these, it's necessary to use
> > > `-mrecord-mcount` to have the compiler log the patch-site, for the same
> > > reason as with `-fpatchable-function-entry`.
> > >
> > > * On architectures with `-mrecord-mcount` generally, since this avoids the
> > > overhead of scanning each object.
> > >
> > > * On x86 when objtool is used.
> > >
> >
> > Right.
> >
> > I suppose that on arm64, we can work around this by passing
> > --apply-dynamic-relocs to the linker, so that all R_AARCH64_RELATIVE
> > targets are prepopulated with the link time value of the respective
> > addresses. It does cause some bloat, which is why we disable that
> > today, but we could make that dependent on ftrace being enabled.
>
> We'd also need to teach the build-time sort to update the relocations, unless
> you mean to also change the boot-time reloc code to RMW with the offset?
>
Why would that be necessary? Every RELA entry has the same effect on
its target address, as it just adds a fixed offset.
> I think for right now the best thing is to disable the build-time sort for
> arm64, but maybe something like that is the right thing to do longer term.
>
Fair enough.
> > I do wonder how much over head we accumulate, though, by having all
> > these relocations, but I suppose that is the situation today in any
> > case.
>
> Yeah; I suspect if we want to do something about that we want to do it more
> generally, and would probably need to do something like the x86 approach and
> rewrite the relocs at build-time to something more compressed. If we applied
> the dynamic relocs with the link-time address we'd only need 4 bytes to
> identify each pointer to apply an offset to.
>
> I'm not exactly sure how we could do that, nor what the trade-off look like in
> practice.
>
It would make sense for -fpic codegen to use relative offsets in
__mcount_loc, but since we don't actually use -fpic on arm64, that
doesn't really help :-)
^ permalink raw reply
* Re: ppc: hard lockup / hang in v5.17-rc1 under QEMU
From: Miguel Ojeda @ 2022-01-27 14:44 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <871r0tmosw.fsf@mpe.ellerman.id.au>
Hi Michael,
On Thu, Jan 27, 2022 at 11:54 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> It looks like your kernel-ppc64le-release.config does not have the
> hardlockup detector enabled, so I suspect you're hitting the bug
> described in that patch.
On -release it was a hang; but please note that on -debug the hard
lockup was actually being detected (so it was also reproducible with
the detectors enabled).
> That fix will hit linux-next in the next day or so and should be in rc2.
I confirm the fix works for us -- thanks a lot!
Cheers,
Miguel
^ permalink raw reply
* Re: ppc: hard lockup / hang in v5.17-rc1 under QEMU
From: Miguel Ojeda @ 2022-01-27 14:44 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, Cédric Le Goater, linux-kernel
In-Reply-To: <1643269597.wkij0f82dr.astroid@bobo.none>
Hi Nicholas,
On Thu, Jan 27, 2022 at 8:47 AM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> That sounds like my fault actually.
>
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-January/239178.html
Thanks for the reference & fix! I confirm it works in our CI too.
Closing the QEMU issue then.
Cheers,
Miguel
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Mark Rutland @ 2022-01-27 14:54 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Kees Cook, Linux Kernel Mailing List, Steven Rostedt, Sachin Sant,
Yinan Liu, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <CAMj1kXHnQcHmxRrTBQmj0Z2JJ6iWvNCQqSjvPqG_oedWpikfSA@mail.gmail.com>
On Thu, Jan 27, 2022 at 02:59:31PM +0100, Ard Biesheuvel wrote:
> On Thu, 27 Jan 2022 at 14:24, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Thu, Jan 27, 2022 at 02:07:03PM +0100, Ard Biesheuvel wrote:
> > > I suppose that on arm64, we can work around this by passing
> > > --apply-dynamic-relocs to the linker, so that all R_AARCH64_RELATIVE
> > > targets are prepopulated with the link time value of the respective
> > > addresses. It does cause some bloat, which is why we disable that
> > > today, but we could make that dependent on ftrace being enabled.
> >
> > We'd also need to teach the build-time sort to update the relocations, unless
> > you mean to also change the boot-time reloc code to RMW with the offset?
>
> Why would that be necessary? Every RELA entry has the same effect on
> its target address, as it just adds a fixed offset.
Currently in relocate_kernel() we generate the absolute address from the
relocation alone, with the core of the relocation logic being as follows, with
x9 being the pointer to a RELA entry, and x23 being the offset relative to the
default load address:
ldp x12, x13, [x9], #24
ldr x14, [x9, #-8]
add x14, x14, x23 // relocate
str x14, [x12, x23]
... and (as per another reply), a sample RELA entry currently contains:
0xffff8000090b1ab0 // default load VA of pointer to update
0x0000000000000403 // R_AARCH64_RELATIVE
0xffff8000090b6000 // default load VA of addr to write
So either:
* That code stays as-is, and we must update the relocs to correspond to their
new sorted locations, or we'll blat the sorted values with the original
relocs as we do today.
* The code needs to change to RMW: read the existing value, add the offset
(ignoring the content of the RELA entry's addend field), and write it back.
This is what I meant when I said "change the boot-time reloc code to RMW with
the offset".
Does that make sense, or have I misunderstood?
Thanks,
Mark.
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Mark Rutland @ 2022-01-27 14:56 UTC (permalink / raw)
To: Steven Rostedt
Cc: keescook, hca, linux-kernel, Sven Schnelle, Sachin Sant,
Yinan Liu, linuxppc-dev, ardb
In-Reply-To: <20220127085543.200dd38e@gandalf.local.home>
On Thu, Jan 27, 2022 at 08:55:43AM -0500, Steven Rostedt wrote:
> On Thu, 27 Jan 2022 13:33:02 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
>
> > I want to get the regression fixed ASAP, so can we take a simple patch for -rc2
> > which disables the build-time sort where it's currently broken (by limiting the
> > opt-in to arm and x86), then follow-up per-architecture to re-enable it if
> > desired/safe?
>
> I'm going to retest my patch that makes it an opt in for just x86 and arm
> (32bit). I'll be pushing that hopefully later today. I have some other
> patches to test as well.
Great; thanks!
Let me know if you'd like me to give that a spin on arm or arm64.
Thanks,
Mark.
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Ard Biesheuvel @ 2022-01-27 15:01 UTC (permalink / raw)
To: Mark Rutland
Cc: Kees Cook, Linux Kernel Mailing List, Steven Rostedt, Sachin Sant,
Yinan Liu, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <YfKyNwYl/pkmVmDm@FVFF77S0Q05N>
On Thu, 27 Jan 2022 at 15:55, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Thu, Jan 27, 2022 at 02:59:31PM +0100, Ard Biesheuvel wrote:
> > On Thu, 27 Jan 2022 at 14:24, Mark Rutland <mark.rutland@arm.com> wrote:
> > >
> > > On Thu, Jan 27, 2022 at 02:07:03PM +0100, Ard Biesheuvel wrote:
> > > > I suppose that on arm64, we can work around this by passing
> > > > --apply-dynamic-relocs to the linker, so that all R_AARCH64_RELATIVE
> > > > targets are prepopulated with the link time value of the respective
> > > > addresses. It does cause some bloat, which is why we disable that
> > > > today, but we could make that dependent on ftrace being enabled.
> > >
> > > We'd also need to teach the build-time sort to update the relocations, unless
> > > you mean to also change the boot-time reloc code to RMW with the offset?
> >
> > Why would that be necessary? Every RELA entry has the same effect on
> > its target address, as it just adds a fixed offset.
>
> Currently in relocate_kernel() we generate the absolute address from the
> relocation alone, with the core of the relocation logic being as follows, with
> x9 being the pointer to a RELA entry, and x23 being the offset relative to the
> default load address:
>
> ldp x12, x13, [x9], #24
> ldr x14, [x9, #-8]
>
> add x14, x14, x23 // relocate
> str x14, [x12, x23]
>
> ... and (as per another reply), a sample RELA entry currently contains:
>
> 0xffff8000090b1ab0 // default load VA of pointer to update
> 0x0000000000000403 // R_AARCH64_RELATIVE
> 0xffff8000090b6000 // default load VA of addr to write
>
> So either:
>
> * That code stays as-is, and we must update the relocs to correspond to their
> new sorted locations, or we'll blat the sorted values with the original
> relocs as we do today.
>
> * The code needs to change to RMW: read the existing value, add the offset
> (ignoring the content of the RELA entry's addend field), and write it back.
> This is what I meant when I said "change the boot-time reloc code to RMW with
> the offset".
>
> Does that make sense, or have I misunderstood?
>
No you're right. We'd have to use different sequences here depending
on whether the relocation target is populated or not, which currently
we don't care about.
^ permalink raw reply
* Re: [PATCH 6/7] modules: Add CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
From: Miroslav Benes @ 2022-01-27 16:05 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch@vger.kernel.org, Daniel Thompson,
kgdb-bugreport@lists.sourceforge.net, Jason Wessel,
linux-kernel@vger.kernel.org, Douglas Anderson,
linux-mm@kvack.org, Luis Chamberlain, Jessica Yu,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <848d857871f457f4df37e80fad468d615b237c24.1643015752.git.christophe.leroy@csgroup.eu>
> @@ -195,6 +208,9 @@ static void mod_tree_remove(struct module *mod)
> {
> __mod_tree_remove(&mod->core_layout.mtn, &mod_tree);
> mod_tree_remove_init(mod);
> +#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
> + __mod_tree_remove(&mod->core_layout.mtn, &mod_data_tree);
s/core_layout/data_layout/ ?
> +#endif
> }
Miroslav
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Kees Cook @ 2022-01-27 16:41 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, Steven Rostedt, Sachin Sant, Yinan Liu,
linuxppc-dev, ardb
In-Reply-To: <YfKGKWW5UfZ15kCW@FVFF77S0Q05N>
On Thu, Jan 27, 2022 at 11:46:49AM +0000, Mark Rutland wrote:
> I'm not sure how x86 works here; AFAICT the relocations are performed during
> decompression, but it looks like there's some special build-time processing
> associated with that, and the vmlinux doesn't contain standard ELF relocations.
>
> Kees, IIUC you added the x86_64 support there, can you shed any light on if/how
> this works on x86?
I think Sven beat me to it, and this was answered in
https://lore.kernel.org/lkml/yt9dy231gzae.fsf@linux.ibm.com
but let me know if anything needs further info.
An additional note is that x86 is built with "-2G addressing"
(-mcmodel=kernel). There was some work done to make it actually
PIE, which would allow the KASLR base to move further:
https://github.com/KSPP/linux/issues/38
-Kees
--
Kees Cook
^ permalink raw reply
* [PATCH] ftrace: Have architectures opt-in for mcount build time sorting
From: Steven Rostedt @ 2022-01-27 16:42 UTC (permalink / raw)
To: LKML
Cc: Mark Rutland, Kees Cook, Russell King, Ard Biesheuvel,
Sachin Sant, Andrew Morton, Yinan Liu, linuxppc-dev, Ingo Molnar,
linux-arm-kernel
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
First S390 complained that the sorting of the mcount sections at build
time caused the kernel to crash on their architecture. Now PowerPC is
complaining about it too. And also ARM64 appears to be having issues.
It may be necessary to also update the relocation table for the values
in the mcount table. Not only do we have to sort the table, but also
update the relocations that may be applied to the items in the table.
If the system is not relocatable, then it is fine to sort, but if it is,
some architectures may have issues (although x86 does not as it shifts all
addresses the same).
Add a HAVE_BUILDTIME_MCOUNT_SORT that an architecture can set to say it is
safe to do the sorting at build time.
Also update the config to compile in build time sorting in the sorttable
code in scripts/ to depend on CONFIG_BUILDTIME_MCOUNT_SORT.
Link: https://lore.kernel.org/all/944D10DA-8200-4BA9-8D0A-3BED9AA99F82@linux.ibm.com/
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Yinan Liu <yinan@linux.alibaba.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: linuxppc-dev@lists.ozlabs.org
Reported-by: Sachin Sant <sachinp@linux.ibm.com>
Tested-by: Sachin Sant <sachinp@linux.ibm.com>
Fixes: 72b3942a173c ("scripts: ftrace - move the sort-processing in ftrace_init")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
arch/arm/Kconfig | 1 +
arch/x86/Kconfig | 1 +
kernel/trace/Kconfig | 8 +++++++-
scripts/Makefile | 2 +-
4 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index c2724d986fa0..5256ebe57451 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -82,6 +82,7 @@ config ARM
select HAVE_EBPF_JIT if !CPU_ENDIAN_BE32
select HAVE_CONTEXT_TRACKING
select HAVE_C_RECORDMCOUNT
+ select HAVE_BUILDTIME_MCOUNT_SORT
select HAVE_DEBUG_KMEMLEAK if !XIP_KERNEL
select HAVE_DMA_CONTIGUOUS if MMU
select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 7399327d1eff..46080dea5dba 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -186,6 +186,7 @@ config X86
select HAVE_CONTEXT_TRACKING_OFFSTACK if HAVE_CONTEXT_TRACKING
select HAVE_C_RECORDMCOUNT
select HAVE_OBJTOOL_MCOUNT if STACK_VALIDATION
+ select HAVE_BUILDTIME_MCOUNT_SORT
select HAVE_DEBUG_KMEMLEAK
select HAVE_DMA_CONTIGUOUS
select HAVE_DYNAMIC_FTRACE
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 752ed89a293b..7e5b92090faa 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -70,10 +70,16 @@ config HAVE_C_RECORDMCOUNT
help
C version of recordmcount available?
+config HAVE_BUILDTIME_MCOUNT_SORT
+ bool
+ help
+ An architecture selects this if it sorts the mcount_loc section
+ at build time.
+
config BUILDTIME_MCOUNT_SORT
bool
default y
- depends on BUILDTIME_TABLE_SORT && !S390
+ depends on HAVE_BUILDTIME_MCOUNT_SORT
help
Sort the mcount_loc section at build time.
diff --git a/scripts/Makefile b/scripts/Makefile
index b082d2f93357..cedc1f0e21d8 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -32,7 +32,7 @@ HOSTCFLAGS_sorttable.o += -I$(srctree)/tools/arch/x86/include
HOSTCFLAGS_sorttable.o += -DUNWINDER_ORC_ENABLED
endif
-ifdef CONFIG_DYNAMIC_FTRACE
+ifdef CONFIG_BUILDTIME_MCOUNT_SORT
HOSTCFLAGS_sorttable.o += -DMCOUNT_SORT_ENABLED
endif
--
2.33.0
^ permalink raw reply related
* [PATCH v1 0/7] soc: fsl: guts: cleanups and serial_number support
From: Michael Walle @ 2022-01-27 16:41 UTC (permalink / raw)
To: linuxppc-dev, linux-arm-kernel, linux-kernel
Cc: Ulf Hansson, Sudeep Holla, Michael Walle, Arnd Bergmann, Li Yang
This series converts the guts driver from a platform driver to just an
core_initcall. The driver itself cannot (or rather should never) be
unloaded because others depends on detecting the current SoC revision
to apply chip errata. Other SoC drivers do it the same way. Overall I
got rid of all the global static variables.
The last patch finally adds unique id support to the guts driver. But
because the binding for the security fuse processor is still pending,
it is marked as RFC.
Michael Walle (7):
soc: fsl: guts: machine variable might be unset
soc: fsl: guts: remove module_exit() and fsl_guts_remove()
soc: fsl: guts: embed fsl_guts_get_svr() in probe()
soc: fsl: guts: allocate soc_dev_attr on the heap
soc: fsl: guts: use of_root instead of own reference
soc: fsl: guts: drop platform driver
soc: fsl: guts: add serial_number support
drivers/soc/fsl/guts.c | 215 ++++++++++++++++++++++-------------------
1 file changed, 114 insertions(+), 101 deletions(-)
--
2.30.2
^ permalink raw reply
* [PATCH v1 1/7] soc: fsl: guts: machine variable might be unset
From: Michael Walle @ 2022-01-27 16:41 UTC (permalink / raw)
To: linuxppc-dev, linux-arm-kernel, linux-kernel
Cc: Ulf Hansson, Sudeep Holla, Michael Walle, Arnd Bergmann, Li Yang
In-Reply-To: <20220127164125.3651285-1-michael@walle.cc>
If both the model and the compatible properties are missing, then
machine will not be set. Initialize it with NULL.
Fixes: 34c1c21e94ac ("soc: fsl: fix section mismatch build warnings")
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/soc/fsl/guts.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 5ed2fc1c53a0..be18d46c7b0f 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -140,7 +140,7 @@ static int fsl_guts_probe(struct platform_device *pdev)
struct device_node *root, *np = pdev->dev.of_node;
struct device *dev = &pdev->dev;
const struct fsl_soc_die_attr *soc_die;
- const char *machine;
+ const char *machine = NULL;
u32 svr;
/* Initialize guts */
--
2.30.2
^ permalink raw reply related
* [PATCH v1 4/7] soc: fsl: guts: allocate soc_dev_attr on the heap
From: Michael Walle @ 2022-01-27 16:41 UTC (permalink / raw)
To: linuxppc-dev, linux-arm-kernel, linux-kernel
Cc: Ulf Hansson, Sudeep Holla, Michael Walle, Arnd Bergmann, Li Yang
In-Reply-To: <20220127164125.3651285-1-michael@walle.cc>
This is the last global static variable. Drop it and allocate the memory
on the heap instead.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/soc/fsl/guts.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 536377988fb4..383b35a4ed58 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -20,9 +20,6 @@ struct fsl_soc_die_attr {
u32 mask;
};
-static struct soc_device_attribute soc_dev_attr;
-
-
/* SoC die attribute definition for QorIQ platform */
static const struct fsl_soc_die_attr fsl_soc_die[] = {
/*
@@ -116,6 +113,7 @@ static const struct fsl_soc_die_attr *fsl_soc_die_match(
static int fsl_guts_probe(struct platform_device *pdev)
{
struct device_node *root, *np = pdev->dev.of_node;
+ struct soc_device_attribute *soc_dev_attr;
static struct soc_device *soc_dev;
struct device *dev = &pdev->dev;
const struct fsl_soc_die_attr *soc_die;
@@ -124,6 +122,10 @@ static int fsl_guts_probe(struct platform_device *pdev)
bool little_endian;
u32 svr;
+ soc_dev_attr = devm_kzalloc(dev, sizeof(*soc_dev_attr), GFP_KERNEL);
+ if (!soc_dev_attr)
+ return -ENOMEM;
+
little_endian = of_property_read_bool(np, "little-endian");
regs = of_iomap(np, 0);
@@ -141,8 +143,8 @@ static int fsl_guts_probe(struct platform_device *pdev)
if (of_property_read_string(root, "model", &machine))
of_property_read_string_index(root, "compatible", 0, &machine);
if (machine) {
- soc_dev_attr.machine = devm_kstrdup(dev, machine, GFP_KERNEL);
- if (!soc_dev_attr.machine) {
+ soc_dev_attr->machine = devm_kstrdup(dev, machine, GFP_KERNEL);
+ if (!soc_dev_attr->machine) {
of_node_put(root);
return -ENOMEM;
}
@@ -151,30 +153,30 @@ static int fsl_guts_probe(struct platform_device *pdev)
soc_die = fsl_soc_die_match(svr, fsl_soc_die);
if (soc_die) {
- soc_dev_attr.family = devm_kasprintf(dev, GFP_KERNEL,
- "QorIQ %s", soc_die->die);
+ soc_dev_attr->family = devm_kasprintf(dev, GFP_KERNEL,
+ "QorIQ %s", soc_die->die);
} else {
- soc_dev_attr.family = devm_kasprintf(dev, GFP_KERNEL, "QorIQ");
+ soc_dev_attr->family = devm_kasprintf(dev, GFP_KERNEL, "QorIQ");
}
- if (!soc_dev_attr.family)
+ if (!soc_dev_attr->family)
return -ENOMEM;
- soc_dev_attr.soc_id = devm_kasprintf(dev, GFP_KERNEL,
+ soc_dev_attr->soc_id = devm_kasprintf(dev, GFP_KERNEL,
"svr:0x%08x", svr);
- if (!soc_dev_attr.soc_id)
+ if (!soc_dev_attr->soc_id)
return -ENOMEM;
- soc_dev_attr.revision = devm_kasprintf(dev, GFP_KERNEL, "%d.%d",
+ soc_dev_attr->revision = devm_kasprintf(dev, GFP_KERNEL, "%d.%d",
(svr >> 4) & 0xf, svr & 0xf);
- if (!soc_dev_attr.revision)
+ if (!soc_dev_attr->revision)
return -ENOMEM;
- soc_dev = soc_device_register(&soc_dev_attr);
+ soc_dev = soc_device_register(soc_dev_attr);
if (IS_ERR(soc_dev))
return PTR_ERR(soc_dev);
- pr_info("Machine: %s\n", soc_dev_attr.machine);
- pr_info("SoC family: %s\n", soc_dev_attr.family);
+ pr_info("Machine: %s\n", soc_dev_attr->machine);
+ pr_info("SoC family: %s\n", soc_dev_attr->family);
pr_info("SoC ID: %s, Revision: %s\n",
- soc_dev_attr.soc_id, soc_dev_attr.revision);
+ soc_dev_attr->soc_id, soc_dev_attr->revision);
return 0;
}
--
2.30.2
^ permalink raw reply related
* [PATCH v1 3/7] soc: fsl: guts: embed fsl_guts_get_svr() in probe()
From: Michael Walle @ 2022-01-27 16:41 UTC (permalink / raw)
To: linuxppc-dev, linux-arm-kernel, linux-kernel
Cc: Ulf Hansson, Sudeep Holla, Michael Walle, Arnd Bergmann, Li Yang
In-Reply-To: <20220127164125.3651285-1-michael@walle.cc>
Move the reading of the SVR into the probe function as
fsl_guts_get_svr() is the only user of the static guts variable and this
lets us drop that as well as the malloc() for this variable. Also, we
can unmap the memory region after we accessed it, which will simplify
error handling later.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/soc/fsl/guts.c | 41 +++++++++++------------------------------
1 file changed, 11 insertions(+), 30 deletions(-)
diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 0bea43770d51..536377988fb4 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -14,18 +14,12 @@
#include <linux/platform_device.h>
#include <linux/fsl/guts.h>
-struct guts {
- struct ccsr_guts __iomem *regs;
- bool little_endian;
-};
-
struct fsl_soc_die_attr {
char *die;
u32 svr;
u32 mask;
};
-static struct guts *guts;
static struct soc_device_attribute soc_dev_attr;
@@ -119,40 +113,28 @@ static const struct fsl_soc_die_attr *fsl_soc_die_match(
return NULL;
}
-static u32 fsl_guts_get_svr(void)
-{
- u32 svr = 0;
-
- if (!guts || !guts->regs)
- return svr;
-
- if (guts->little_endian)
- svr = ioread32(&guts->regs->svr);
- else
- svr = ioread32be(&guts->regs->svr);
-
- return svr;
-}
-
static int fsl_guts_probe(struct platform_device *pdev)
{
struct device_node *root, *np = pdev->dev.of_node;
static struct soc_device *soc_dev;
struct device *dev = &pdev->dev;
const struct fsl_soc_die_attr *soc_die;
+ struct ccsr_guts __iomem *regs;
const char *machine = NULL;
+ bool little_endian;
u32 svr;
- /* Initialize guts */
- guts = devm_kzalloc(dev, sizeof(*guts), GFP_KERNEL);
- if (!guts)
- return -ENOMEM;
+ little_endian = of_property_read_bool(np, "little-endian");
- guts->little_endian = of_property_read_bool(np, "little-endian");
+ regs = of_iomap(np, 0);
+ if (IS_ERR(regs))
+ return PTR_ERR(regs);
- guts->regs = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(guts->regs))
- return PTR_ERR(guts->regs);
+ if (little_endian)
+ svr = ioread32(®s->svr);
+ else
+ svr = ioread32be(®s->svr);
+ iounmap(regs);
/* Register soc device */
root = of_find_node_by_path("/");
@@ -167,7 +149,6 @@ static int fsl_guts_probe(struct platform_device *pdev)
}
of_node_put(root);
- svr = fsl_guts_get_svr();
soc_die = fsl_soc_die_match(svr, fsl_soc_die);
if (soc_die) {
soc_dev_attr.family = devm_kasprintf(dev, GFP_KERNEL,
--
2.30.2
^ permalink raw reply related
* [RFC PATCH v1 7/7] soc: fsl: guts: add serial_number support
From: Michael Walle @ 2022-01-27 16:41 UTC (permalink / raw)
To: linuxppc-dev, linux-arm-kernel, linux-kernel
Cc: Ulf Hansson, Sudeep Holla, Michael Walle, Arnd Bergmann, Li Yang
In-Reply-To: <20220127164125.3651285-1-michael@walle.cc>
Most layerscapes provide a security fuse processor where the vendor
will store a unique id per part. Unfortunately, we cannot use the
corresponding efuse driver because this driver needs to be ready
early during the boot phase. To get the unique identifier, we just
need to access two registers. Thus we just search the device tree
for the corresponding device, map its memory to read the id and then
unmap it again.
Because it is likely that the offset within the fuses is dependent
on the SoC, we need a per SoC data. Also, the compatible string is
different among the SoCs. For now, this add support for the LS1028A
SoC.
Signed-off-by: Michael Walle <michael@walle.cc>
---
RFC because SFP binding is still pending and needs Rob's ack:
https://lore.kernel.org/linux-devicetree/20220127163728.3650648-2-michael@walle.cc/
drivers/soc/fsl/guts.c | 48 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index be961a9193f4..2d20a344f3c9 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -20,6 +20,11 @@ struct fsl_soc_die_attr {
u32 mask;
};
+struct fsl_soc_data {
+ const char *sfp_compat;
+ u32 uid_offset;
+};
+
/* SoC die attribute definition for QorIQ platform */
static const struct fsl_soc_die_attr fsl_soc_die[] = {
/*
@@ -110,6 +115,33 @@ static const struct fsl_soc_die_attr *fsl_soc_die_match(
return NULL;
}
+static u64 fsl_guts_get_soc_uid(const char *compat, unsigned int offset)
+{
+ struct device_node *np;
+ void __iomem *sfp_base;
+ u64 uid;
+
+ np = of_find_compatible_node(NULL, NULL, compat);
+ if (!np)
+ return 0;
+
+ sfp_base = of_iomap(np, 0);
+
+ uid = ioread32(sfp_base + offset);
+ uid <<= 32;
+ uid |= ioread32(sfp_base + offset + 4);
+
+ iounmap(sfp_base);
+ of_node_put(np);
+
+ return uid;
+}
+
+static const struct fsl_soc_data ls1028a_data = {
+ .sfp_compat = "fsl,ls1028a-sfp",
+ .uid_offset = 0x21c,
+};
+
/*
* Table for matching compatible strings, for device tree
* guts node, for Freescale QorIQ SOCs.
@@ -138,7 +170,7 @@ static const struct of_device_id fsl_guts_of_match[] = {
{ .compatible = "fsl,ls1012a-dcfg", },
{ .compatible = "fsl,ls1046a-dcfg", },
{ .compatible = "fsl,lx2160a-dcfg", },
- { .compatible = "fsl,ls1028a-dcfg", },
+ { .compatible = "fsl,ls1028a-dcfg", .data = &ls1028a_data},
{}
};
@@ -147,16 +179,20 @@ static int __init fsl_guts_init(void)
struct soc_device_attribute *soc_dev_attr;
static struct soc_device *soc_dev;
const struct fsl_soc_die_attr *soc_die;
+ const struct fsl_soc_data *soc_data;
+ const struct of_device_id *match;
struct ccsr_guts __iomem *regs;
const char *machine = NULL;
struct device_node *np;
bool little_endian;
+ u64 soc_uid = 0;
u32 svr;
int ret;
- np = of_find_matching_node_and_match(NULL, fsl_guts_of_match, NULL);
+ np = of_find_matching_node_and_match(NULL, fsl_guts_of_match, &match);
if (!np)
return 0;
+ soc_data = match->data;
soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
if (!soc_dev_attr)
@@ -201,6 +237,13 @@ static int __init fsl_guts_init(void)
if (!soc_dev_attr->revision)
goto err_nomem;
+ if (soc_data)
+ soc_uid = fsl_guts_get_soc_uid(soc_data->sfp_compat,
+ soc_data->uid_offset);
+ if (soc_uid)
+ soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX",
+ soc_uid);
+
soc_dev = soc_device_register(soc_dev_attr);
if (IS_ERR(soc_dev)) {
ret = PTR_ERR(soc_dev);
@@ -221,6 +264,7 @@ static int __init fsl_guts_init(void)
kfree(soc_dev_attr->family);
kfree(soc_dev_attr->soc_id);
kfree(soc_dev_attr->revision);
+ kfree(soc_dev_attr->serial_number);
return ret;
}
--
2.30.2
^ permalink raw reply related
* [PATCH v1 2/7] soc: fsl: guts: remove module_exit() and fsl_guts_remove()
From: Michael Walle @ 2022-01-27 16:41 UTC (permalink / raw)
To: linuxppc-dev, linux-arm-kernel, linux-kernel
Cc: Ulf Hansson, Sudeep Holla, Michael Walle, Arnd Bergmann, Li Yang
In-Reply-To: <20220127164125.3651285-1-michael@walle.cc>
This driver will never be unloaded. Firstly, it is not available as a
module, but more importantly, other drivers will depend on this one to
apply possible chip errata.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/soc/fsl/guts.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index be18d46c7b0f..0bea43770d51 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -27,7 +27,6 @@ struct fsl_soc_die_attr {
static struct guts *guts;
static struct soc_device_attribute soc_dev_attr;
-static struct soc_device *soc_dev;
/* SoC die attribute definition for QorIQ platform */
@@ -138,6 +137,7 @@ static u32 fsl_guts_get_svr(void)
static int fsl_guts_probe(struct platform_device *pdev)
{
struct device_node *root, *np = pdev->dev.of_node;
+ static struct soc_device *soc_dev;
struct device *dev = &pdev->dev;
const struct fsl_soc_die_attr *soc_die;
const char *machine = NULL;
@@ -197,12 +197,6 @@ static int fsl_guts_probe(struct platform_device *pdev)
return 0;
}
-static int fsl_guts_remove(struct platform_device *dev)
-{
- soc_device_unregister(soc_dev);
- return 0;
-}
-
/*
* Table for matching compatible strings, for device tree
* guts node, for Freescale QorIQ SOCs.
@@ -242,7 +236,6 @@ static struct platform_driver fsl_guts_driver = {
.of_match_table = fsl_guts_of_match,
},
.probe = fsl_guts_probe,
- .remove = fsl_guts_remove,
};
static int __init fsl_guts_init(void)
@@ -250,9 +243,3 @@ static int __init fsl_guts_init(void)
return platform_driver_register(&fsl_guts_driver);
}
core_initcall(fsl_guts_init);
-
-static void __exit fsl_guts_exit(void)
-{
- platform_driver_unregister(&fsl_guts_driver);
-}
-module_exit(fsl_guts_exit);
--
2.30.2
^ permalink raw reply related
* [PATCH v1 6/7] soc: fsl: guts: drop platform driver
From: Michael Walle @ 2022-01-27 16:41 UTC (permalink / raw)
To: linuxppc-dev, linux-arm-kernel, linux-kernel
Cc: Ulf Hansson, Sudeep Holla, Michael Walle, Arnd Bergmann, Li Yang
In-Reply-To: <20220127164125.3651285-1-michael@walle.cc>
This driver cannot be unloaded and it will be needed very early in the
boot process because other driver (weakly) depend on it (eg. for chip
errata handling). Drop all the platform driver and devres stuff and
simply make it a core_initcall.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/soc/fsl/guts.c | 127 +++++++++++++++++++++--------------------
1 file changed, 65 insertions(+), 62 deletions(-)
diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 04c3eb6a6e17..be961a9193f4 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -110,19 +110,55 @@ static const struct fsl_soc_die_attr *fsl_soc_die_match(
return NULL;
}
-static int fsl_guts_probe(struct platform_device *pdev)
+/*
+ * Table for matching compatible strings, for device tree
+ * guts node, for Freescale QorIQ SOCs.
+ */
+static const struct of_device_id fsl_guts_of_match[] = {
+ { .compatible = "fsl,qoriq-device-config-1.0", },
+ { .compatible = "fsl,qoriq-device-config-2.0", },
+ { .compatible = "fsl,p1010-guts", },
+ { .compatible = "fsl,p1020-guts", },
+ { .compatible = "fsl,p1021-guts", },
+ { .compatible = "fsl,p1022-guts", },
+ { .compatible = "fsl,p1023-guts", },
+ { .compatible = "fsl,p2020-guts", },
+ { .compatible = "fsl,bsc9131-guts", },
+ { .compatible = "fsl,bsc9132-guts", },
+ { .compatible = "fsl,mpc8536-guts", },
+ { .compatible = "fsl,mpc8544-guts", },
+ { .compatible = "fsl,mpc8548-guts", },
+ { .compatible = "fsl,mpc8568-guts", },
+ { .compatible = "fsl,mpc8569-guts", },
+ { .compatible = "fsl,mpc8572-guts", },
+ { .compatible = "fsl,ls1021a-dcfg", },
+ { .compatible = "fsl,ls1043a-dcfg", },
+ { .compatible = "fsl,ls2080a-dcfg", },
+ { .compatible = "fsl,ls1088a-dcfg", },
+ { .compatible = "fsl,ls1012a-dcfg", },
+ { .compatible = "fsl,ls1046a-dcfg", },
+ { .compatible = "fsl,lx2160a-dcfg", },
+ { .compatible = "fsl,ls1028a-dcfg", },
+ {}
+};
+
+static int __init fsl_guts_init(void)
{
- struct device_node *np = pdev->dev.of_node;
struct soc_device_attribute *soc_dev_attr;
static struct soc_device *soc_dev;
- struct device *dev = &pdev->dev;
const struct fsl_soc_die_attr *soc_die;
struct ccsr_guts __iomem *regs;
const char *machine = NULL;
+ struct device_node *np;
bool little_endian;
u32 svr;
+ int ret;
- soc_dev_attr = devm_kzalloc(dev, sizeof(*soc_dev_attr), GFP_KERNEL);
+ np = of_find_matching_node_and_match(NULL, fsl_guts_of_match, NULL);
+ if (!np)
+ return 0;
+
+ soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
if (!soc_dev_attr)
return -ENOMEM;
@@ -138,87 +174,54 @@ static int fsl_guts_probe(struct platform_device *pdev)
svr = ioread32be(®s->svr);
iounmap(regs);
- /* Register soc device */
if (of_property_read_string(of_root, "model", &machine))
of_property_read_string_index(of_root, "compatible", 0, &machine);
if (machine) {
- soc_dev_attr->machine = devm_kstrdup(dev, machine, GFP_KERNEL);
+ soc_dev_attr->machine = kstrdup(machine, GFP_KERNEL);
if (!soc_dev_attr->machine)
return -ENOMEM;
}
soc_die = fsl_soc_die_match(svr, fsl_soc_die);
if (soc_die) {
- soc_dev_attr->family = devm_kasprintf(dev, GFP_KERNEL,
- "QorIQ %s", soc_die->die);
+ soc_dev_attr->family = kasprintf(GFP_KERNEL, "QorIQ %s",
+ soc_die->die);
} else {
- soc_dev_attr->family = devm_kasprintf(dev, GFP_KERNEL, "QorIQ");
+ soc_dev_attr->family = kasprintf(GFP_KERNEL, "QorIQ");
}
if (!soc_dev_attr->family)
- return -ENOMEM;
- soc_dev_attr->soc_id = devm_kasprintf(dev, GFP_KERNEL,
- "svr:0x%08x", svr);
+ goto err_nomem;
+
+ soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "svr:0x%08x", svr);
if (!soc_dev_attr->soc_id)
- return -ENOMEM;
- soc_dev_attr->revision = devm_kasprintf(dev, GFP_KERNEL, "%d.%d",
- (svr >> 4) & 0xf, svr & 0xf);
+ goto err_nomem;
+
+ soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%d.%d",
+ (svr >> 4) & 0xf, svr & 0xf);
if (!soc_dev_attr->revision)
- return -ENOMEM;
+ goto err_nomem;
soc_dev = soc_device_register(soc_dev_attr);
- if (IS_ERR(soc_dev))
- return PTR_ERR(soc_dev);
+ if (IS_ERR(soc_dev)) {
+ ret = PTR_ERR(soc_dev);
+ goto err;
+ }
pr_info("Machine: %s\n", soc_dev_attr->machine);
pr_info("SoC family: %s\n", soc_dev_attr->family);
pr_info("SoC ID: %s, Revision: %s\n",
soc_dev_attr->soc_id, soc_dev_attr->revision);
- return 0;
-}
-/*
- * Table for matching compatible strings, for device tree
- * guts node, for Freescale QorIQ SOCs.
- */
-static const struct of_device_id fsl_guts_of_match[] = {
- { .compatible = "fsl,qoriq-device-config-1.0", },
- { .compatible = "fsl,qoriq-device-config-2.0", },
- { .compatible = "fsl,p1010-guts", },
- { .compatible = "fsl,p1020-guts", },
- { .compatible = "fsl,p1021-guts", },
- { .compatible = "fsl,p1022-guts", },
- { .compatible = "fsl,p1023-guts", },
- { .compatible = "fsl,p2020-guts", },
- { .compatible = "fsl,bsc9131-guts", },
- { .compatible = "fsl,bsc9132-guts", },
- { .compatible = "fsl,mpc8536-guts", },
- { .compatible = "fsl,mpc8544-guts", },
- { .compatible = "fsl,mpc8548-guts", },
- { .compatible = "fsl,mpc8568-guts", },
- { .compatible = "fsl,mpc8569-guts", },
- { .compatible = "fsl,mpc8572-guts", },
- { .compatible = "fsl,ls1021a-dcfg", },
- { .compatible = "fsl,ls1043a-dcfg", },
- { .compatible = "fsl,ls2080a-dcfg", },
- { .compatible = "fsl,ls1088a-dcfg", },
- { .compatible = "fsl,ls1012a-dcfg", },
- { .compatible = "fsl,ls1046a-dcfg", },
- { .compatible = "fsl,lx2160a-dcfg", },
- { .compatible = "fsl,ls1028a-dcfg", },
- {}
-};
-MODULE_DEVICE_TABLE(of, fsl_guts_of_match);
+ return 0;
-static struct platform_driver fsl_guts_driver = {
- .driver = {
- .name = "fsl-guts",
- .of_match_table = fsl_guts_of_match,
- },
- .probe = fsl_guts_probe,
-};
+err_nomem:
+ ret = -ENOMEM;
+err:
+ kfree(soc_dev_attr->machine);
+ kfree(soc_dev_attr->family);
+ kfree(soc_dev_attr->soc_id);
+ kfree(soc_dev_attr->revision);
-static int __init fsl_guts_init(void)
-{
- return platform_driver_register(&fsl_guts_driver);
+ return ret;
}
core_initcall(fsl_guts_init);
--
2.30.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox