Re: 2.4.9-ac12 ppc ftr_fixup

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 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/