2.4.9-ac12 ppc ftr_fixup

2.4.9-ac12 ppc ftr_fixup

[Keith Owens]

2.4.9-ac12 has new ppc code for CPU feature fixups.  The ftr_fixup code
only handles entries that are built into the kernel.  timex.h defines
get_cycles() using ftr_fixup and get_cycles() is used all over the
place, including in modules.  AFAICT we need to add modutils support
for ftr_fixup.

Don't write any code yet, Maciej W. Rozycki has some patches for a
similar problem in mips and his fix is nicely extensible.  I just need
confirmation that ftr_fixup needs modutils support.

Re: 2.4.9-ac12 ppc ftr_fixup

[Tom Rini]

On Mon, Aug 27, 2001 at 10:27:22AM +1000, Keith Owens wrote:

> 2.4.9-ac12 has new ppc code for CPU feature fixups.  The ftr_fixup code
> only handles entries that are built into the kernel.  timex.h defines
> get_cycles() using ftr_fixup and get_cycles() is used all over the
> place, including in modules.  AFAICT we need to add modutils support
> for ftr_fixup.

Er, eh?  Excuse me if I'm being obtuse, but where is the problem?  The fixup
stuff is closely tied to bootup and what processor we happen to be on
at the time.  So we won't be trying to fixup any code in a module...

-- 
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

Re: 2.4.9-ac12 ppc ftr_fixup

[Keith Owens]

On Sun, 26 Aug 2001 19:15:36 -0700, 
Tom Rini <trini@kernel.crashing.org> wrote:
>On Mon, Aug 27, 2001 at 10:27:22AM +1000, Keith Owens wrote:
>
>> 2.4.9-ac12 has new ppc code for CPU feature fixups.  The ftr_fixup code
>> only handles entries that are built into the kernel.  timex.h defines
>> get_cycles() using ftr_fixup and get_cycles() is used all over the
>> place, including in modules.  AFAICT we need to add modutils support
>> for ftr_fixup.
>
>Er, eh?  Excuse me if I'm being obtuse, but where is the problem?  The fixup
>stuff is closely tied to bootup and what processor we happen to be on
>at the time.  So we won't be trying to fixup any code in a module...

do_cpu_ftr_fixups() replaces unsupported code with NOP, based on the
table from __start___ftr_fixup to __stop___ftr_fixup which contains all
the data marked as section(__ftr_fixup).  Fine, but it only handles
section __ftr_fixup data in the kernel, it does not write NOP over
__ftr_fixup data in modules.  So any code marked as section __ftr_fixup
in a module executes unchanged.  Unless I am missing something, that is
a problem.

Re: 2.4.9-ac12 ppc ftr_fixup

[Tom Rini]

On Mon, Aug 27, 2001 at 12:47:57PM +1000, Keith Owens wrote:
> 
> On Sun, 26 Aug 2001 19:15:36 -0700,
> Tom Rini <trini@kernel.crashing.org> wrote:
> >On Mon, Aug 27, 2001 at 10:27:22AM +1000, Keith Owens wrote:
> >
> >> 2.4.9-ac12 has new ppc code for CPU feature fixups.  The ftr_fixup code
> >> only handles entries that are built into the kernel.  timex.h defines
> >> get_cycles() using ftr_fixup and get_cycles() is used all over the
> >> place, including in modules.  AFAICT we need to add modutils support
> >> for ftr_fixup.
> >
> >Er, eh?  Excuse me if I'm being obtuse, but where is the problem?  The fixup
> >stuff is closely tied to bootup and what processor we happen to be on
> >at the time.  So we won't be trying to fixup any code in a module...
> 
> do_cpu_ftr_fixups() replaces unsupported code with NOP, based on the
> table from __start___ftr_fixup to __stop___ftr_fixup which contains all
> the data marked as section(__ftr_fixup).  Fine, but it only handles
> section __ftr_fixup data in the kernel, it does not write NOP over
> __ftr_fixup data in modules.  So any code marked as section __ftr_fixup
> in a module executes unchanged.  Unless I am missing something, that is
> a problem.

Hmm..  I'm guessing no one's tried get_cycles from a module on a 601 in
ages...

-- 
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

Re: 2.4.9-ac12 ppc ftr_fixup

[Tom Rini]

[cc's trimmed]

On Mon, Aug 27, 2001 at 12:47:57PM +1000, Keith Owens wrote:
> On Sun, 26 Aug 2001 19:15:36 -0700,
> Tom Rini <trini@kernel.crashing.org> wrote:
> >On Mon, Aug 27, 2001 at 10:27:22AM +1000, Keith Owens wrote:
> >
> >> 2.4.9-ac12 has new ppc code for CPU feature fixups.  The ftr_fixup code
> >> only handles entries that are built into the kernel.  timex.h defines
> >> get_cycles() using ftr_fixup and get_cycles() is used all over the
> >> place, including in modules.  AFAICT we need to add modutils support
> >> for ftr_fixup.
> >
> >Er, eh?  Excuse me if I'm being obtuse, but where is the problem?  The fixup
> >stuff is closely tied to bootup and what processor we happen to be on
> >at the time.  So we won't be trying to fixup any code in a module...
>
> do_cpu_ftr_fixups() replaces unsupported code with NOP, based on the
> table from __start___ftr_fixup to __stop___ftr_fixup which contains all
> the data marked as section(__ftr_fixup).  Fine, but it only handles
> section __ftr_fixup data in the kernel, it does not write NOP over
> __ftr_fixup data in modules.  So any code marked as section __ftr_fixup
> in a module executes unchanged.  Unless I am missing something, that is
> a problem.

After a bit more digging, I'm pretty sure it's not much of a problem.
get_cycles seems to be either a) SMP or b) DRM or c) arcnet related.
a and b aren't an issue for 601 (no SMP and possible but unlikely that
DRM will work on a machine w/ a 601).  If there's an arcnet PCI card, I suppose
it could be an issue then...  Anyhow...

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: 2.4.9-ac12 ppc ftr_fixup

[Keith Owens]

On Sun, 26 Aug 2001 19:54:58 -0700, 
Tom Rini <trini@kernel.crashing.org> wrote:
>> >On Mon, Aug 27, 2001 at 10:27:22AM +1000, Keith Owens wrote:
>> >
>> >> 2.4.9-ac12 has new ppc code for CPU feature fixups.  The ftr_fixup code
>> >> only handles entries that are built into the kernel.  timex.h defines
>> >> get_cycles() using ftr_fixup and get_cycles() is used all over the
>> >> place, including in modules.  AFAICT we need to add modutils support
>> >> for ftr_fixup.
>Hmm..  I'm guessing no one's tried get_cycles from a module on a 601 in
>ages...

OK, so insmod needs to pass the ftr_fixup data for modules into the
kernel, I will add the ftp_fixup section as archdata in insmod.  Do not
code any kernel change to use ftr_fixup in modules until Maciej W.
Rozycki's patch for module archdata handling is in.

Re: 2.4.9-ac12 ppc ftr_fixup

[Keith Owens]

On Sun, 26 Aug 2001 20:04:04 -0700,
Tom Rini <trini@kernel.crashing.org> wrote:
>On Mon, Aug 27, 2001 at 12:47:57PM +1000, Keith Owens wrote:
>> do_cpu_ftr_fixups() replaces unsupported code with NOP, based on the
>> table from __start___ftr_fixup to __stop___ftr_fixup which contains all
>> the data marked as section(__ftr_fixup).  Fine, but it only handles
>> section __ftr_fixup data in the kernel, it does not write NOP over
>> __ftr_fixup data in modules.  So any code marked as section __ftr_fixup
>> in a module executes unchanged.  Unless I am missing something, that is
>> a problem.
>
>After a bit more digging, I'm pretty sure it's not much of a problem.
>get_cycles seems to be either a) SMP or b) DRM or c) arcnet related.
>a and b aren't an issue for 601 (no SMP and possible but unlikely that
>DRM will work on a machine w/ a 601).  If there's an arcnet PCI card, I suppose
>it could be an issue then...  Anyhow...

It is more than just get_cycles().  Look at the places that ftr is used,
  fgrep -r BEGIN_FTR_SECTION include/asm-ppc* arch/ppc*
If any of that code ends up in a module then the problem exists.


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: 2.4.9-ac12 ppc ftr_fixup

[Keith Owens]

Please check that this patch against modutils 2.4.7 works.  It puts the
ftr fixup data in the archdata section of PPC modules.  Use
"insmod -nm -o blob module_name" to get a map and check that the
generated blob contains an archdata section containing the ftr fixup
pointers.  Pick any module from 2.4.8-ac12 that contains ftr data,
something using get_cycles will do.

The kernel does not do anything with the ppc archdata from modules yet,
that is a separate patch.


Index: 8.5/obj/obj_ppc.c
--- 8.5/obj/obj_ppc.c Fri, 05 Jan 2001 12:45:19 +1100 kaos (modutils-2.4/c/11_obj_ppc.c 1.1 644)
+++ 8.5(w)/obj/obj_ppc.c Mon, 27 Aug 2001 13:38:47 +1000 kaos (modutils-2.4/c/11_obj_ppc.c 1.1 644)
@@ -249,7 +249,25 @@ arch_finalize_section_address(struct obj
 }

 int
-arch_archdata (struct obj_file *fin, struct obj_section *sec)
+arch_archdata (struct obj_file *f, struct obj_section *archdata_sec)
 {
+  struct archdata {
+    unsigned tgt_long __start___ftr_fixup;
+    unsigned tgt_long __stop___ftr_fixup;
+  } *ad;
+  struct obj_section *sec;
+
+  if (archdata_sec->contents)
+    free(archdata_sec->contents);
+  archdata_sec->header.sh_size = 0;
+  sec = obj_find_section(f, "__ftr_fixup");
+  if (sec) {
+    ad = (struct archdata *) (archdata_sec->contents) = xmalloc(sizeof(*ad));
+    memset(ad, 0, sizeof(*ad));
+    archdata_sec->header.sh_size = sizeof(*ad);
+    ad->__start___ftr_fixup = sec->header.sh_addr;
+    ad->__stop___ftr_fixup = sec->header.sh_addr + sec->header.sh_size;
+  }
+
   return 0;
 }


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: 2.4.9-ac12 ppc ftr_fixup

[Tom Rini]

On Mon, Aug 27, 2001 at 01:45:51PM +1000, Keith Owens wrote:

> Please check that this patch against modutils 2.4.7 works.  It puts the
> ftr fixup data in the archdata section of PPC modules.  Use
> "insmod -nm -o blob module_name" to get a map and check that the
> generated blob contains an archdata section containing the ftr fixup
> pointers.  Pick any module from 2.4.8-ac12 that contains ftr data,
> something using get_cycles will do.

Well, I attempted to anyhow.  The get_cycles calls are protected w/
a define that's usually off (and some quick trying couldn't find the
right combo of defines to turn on and have it compile), so I hacked
drivers/char/drm/drm_context.h (this is based on Linus' tree, which is all
I've got right now, sorry) to blindly call get_cycles.  Assuming that
r128_context_switch is the context_switch in drivers/char/drm/drm_context.h,
I had an __archdata section, but it was empty.

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: 2.4.9-ac12 ppc ftr_fixup

[Keith Owens]

On Sun, 26 Aug 2001 21:24:15 -0700,
Tom Rini <trini@kernel.crashing.org> wrote:
>right combo of defines to turn on and have it compile), so I hacked
>drivers/char/drm/drm_context.h (this is based on Linus' tree, which is all
>I've got right now, sorry) to blindly call get_cycles.  Assuming that
>r128_context_switch is the context_switch in drivers/char/drm/drm_context.h,
>I had an __archdata section, but it was empty.

Did the .o file have section __ftr_fixup?  Use objdump -h.  And did
insmod pick up that newly compile module or did it pick up the
installed version?


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: 2.4.9-ac12 ppc ftr_fixup

[Tom Rini]

On Mon, Aug 27, 2001 at 02:31:20PM +1000, Keith Owens wrote:
> On Sun, 26 Aug 2001 21:24:15 -0700,
> Tom Rini <trini@kernel.crashing.org> wrote:
> >right combo of defines to turn on and have it compile), so I hacked
> >drivers/char/drm/drm_context.h (this is based on Linus' tree, which is all
> >I've got right now, sorry) to blindly call get_cycles.  Assuming that
> >r128_context_switch is the context_switch in drivers/char/drm/drm_context.h,
> >I had an __archdata section, but it was empty.
>
> Did the .o file have section __ftr_fixup?  Use objdump -h.  And did
> insmod pick up that newly compile module or did it pick up the
> installed version?

I guess I should go get some sleep now... __archdata is 8 not 0, but the
last section I see (insmod -nm ./r128.o > r128.out) is the .bss...

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: 2.4.9-ac12 ppc ftr_fixup

[Keith Owens]

On Sun, 26 Aug 2001 21:54:36 -0700,
Tom Rini <trini@kernel.crashing.org> wrote:
>I guess I should go get some sleep now... __archdata is 8 not 0, but the
>last section I see (insmod -nm ./r128.o > r128.out) is the .bss...

There is no __insmod_modname_S symbol for __archdata.  Use
  insmod -nm -O blob ./r128.o
so insmod writes the binary object to blob.  Because it also uses -n,
the address starts at an arbitrary 0x12340000.  Get the address of
__archdata from the start of the map, and
  objdump -b binary -s --adjust-vma=0x12340000 --start-addr=0x1234xxxx blob
0x1234xxxx is the start address of __archdata.  Alas if __archdata is
all zeroes, objdump dumps nothing, you might have to use a lower start
address to see an all zero section.  However __archdata should not be
zeroes, if it exists.


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: 2.4.9-ac12 ppc ftr_fixup

[Benjamin Herrenschmidt]

>Don't write any code yet, Maciej W. Rozycki has some patches for a
>similar problem in mips and his fix is nicely extensible.  I just need
>confirmation that ftr_fixup needs modutils support.

Right. The feature fixup was originally intended for altivec-specific
cruft in arch/ppc/kernel/*.S, and was only later extended to a few
more things like get_cycles(). It doesn't handle modules, and that
can ideed be an issue. 

I'm looking forward to your modutils. The kernel's do_cpu_ftr_fixups
function has to be modified to take the pointer & size of the fixup
section so that it can be called for modules as well.

Ben.