#2369 closed defect (fixed)
[PowerPC Book E] Invalid mftb instruction in _CPU_Counter_read()
Reported by: | Nick Withers | Owned by: | Nick Withers <nick.withers@…> |
---|---|---|---|
Priority: | normal | Milestone: | 4.11.1 |
Component: | unspecified | Version: | 4.11 |
Severity: | normal | Keywords: | |
Cc: | Blocked By: | ||
Blocking: |
Description
_CPU_Counter_read(), called e.g., when RTEMS profiling is enabled, attempts to use the mftb instruction to access the time base. This instruction does not exist on Book E processors (such as the e500 used in the MVME3100) and causes an exception on those architectures.
At least RTEMS profiling therefore does not work at least with the mvme3100 BSP.
This...:
diff --git a/cpukit/score/cpu/powerpc/rtems/score/cpu.h b/cpukit/score/cpu/powerpc/rtems/score/cpu.h index 06cab2c..45298a4 100644 --- a/cpukit/score/cpu/powerpc/rtems/score/cpu.h +++ b/cpukit/score/cpu/powerpc/rtems/score/cpu.h @@ -842,7 +842,7 @@ static inline CPU_Counter_ticks _CPU_Counter_read( void ) /* Use Alternate Time Base */ __asm__ volatile( "mfspr %0, 526" : "=r" (value) ); #else - __asm__ volatile( "mftb %0" : "=r" (value) ); + __asm__ volatile( "mfspr %0, 268" : "=r" (value) ); #endif return value;
...sorts it out on the mvme3100 and I don't *think* will break anything for other BSPs (I believe SPR 268 is always valid).
I wonder if we wouldn't be better off using PPC_Get_timebase_register(), though, which also checks the upper 32-bits of the timebase? Maybe that doesn't matter for the cases where _CPU_Counter_read() 's called?
Attachments (1)
Change History (21)
comment:1 follow-up: 2 Changed on 07/09/15 at 05:51:57 by Sebastian Huber
comment:2 Changed on 07/09/15 at 06:14:47 by Nick Withers
Replying to sebastian.huber:
The ppc8540 is the multilib intended for the e500 processors, why is this not used?
No idea - where's that picked up from? I've been wondering for a quite a while why I get "-mcpu=powerpc" when building RTEMS and not "-mcpu=8540"... is that intentional / anything to do with it?
I'm thoroughly confused by PowerPC versioning in general... According to Figure 4 from http://www.freescale.com/files/32bit/doc/white_paper/POWRPCARCPRMRM.pdf there's no alternate time base on Book E, so the mfspr %0, 526 doesn't seem correct here... But then going off http://cache.freescale.com/files/32bit/doc/ref_manual/E500CORERM.pdf Table 3.44, there are a bunch of instructions that are valid for Book E but invalid for the e500, so presumably the e500 doesn't actually conform to Book E... or something?!
The marketing guff for the MVME3100 board says "System-on-chip Freescale MPC8540 with PowerPC® e500 processor core" - does that imply that not all MPC8540s use e500 cores?
comment:3 follow-up: 4 Changed on 07/09/15 at 06:22:58 by Sebastian Huber
A BSP for the e500 should use the "-mcpu=8540 -meabi -msdata=sysv -fno-common" options.
I am not sure if the SPR 268 exists really on all PowerPCs supported by RTEMS:
static inline uint64_t PPC_Get_timebase_register( void )
{
uint32_t tbr_low;
uint32_t tbr_high;
uint32_t tbr_high_old;
uint64_t tbr;
do {
#if defined(mpx8xx)
/* See comment above (CPU_Get_timebase_low) */
asm volatile( "mftbu %0" : "=r" (tbr_high_old));
asm volatile( "mftb %0" : "=r" (tbr_low));
asm volatile( "mftbu %0" : "=r" (tbr_high));
#else
asm volatile( "mfspr %0, 269" : "=r" (tbr_high_old));
asm volatile( "mfspr %0, 268" : "=r" (tbr_low));
asm volatile( "mfspr %0, 269" : "=r" (tbr_high));
#endif
} while ( tbr_high_old != tbr_high );
tbr = tbr_high;
tbr <<= 32;
tbr |= tbr_low;
return tbr;
}
However these exceptions look more like museum hardware.
comment:4 Changed on 07/09/15 at 07:48:30 by Nick Withers
Replying to sebastian.huber:
A BSP for the e500 should use the "-mcpu=8540 -meabi -msdata=sysv -fno-common" options.
Cheers - I tried the following:
diff --git a/c/src/lib/libbsp/powerpc/mvme3100/make/custom/mvme3100.cfg b/c/src/lib/libbsp/powerpc/mvme3100/make/custom/mvme3100.cfg index ddd6d23..3872f18 100644 --- a/c/src/lib/libbsp/powerpc/mvme3100/make/custom/mvme3100.cfg +++ b/c/src/lib/libbsp/powerpc/mvme3100/make/custom/mvme3100.cfg @@ -10,10 +10,10 @@ RTEMS_CPU_MODEL=e500 # This contains the compiler options necessary to select the CPU model # and (hopefully) optimize for it. -CPU_CFLAGS = -mcpu=powerpc -msoft-float -D__ppc_generic +CPU_CFLAGS = -mcpu=8540 -meabi -msdata=sysv -fno-common -D__ppc_generic
...and wasn't very successful [1]:
powerpc-rtems4.11-gcc -B../../../../../mvme3100/lib/ -specs bsp_specs -qrtems -mcpu=8540 -meabi -msdata=sysv -fno-common -D__ppc_generic -O2 -g -Wall -Wmissing-prototypes -Wimplicit-function-declaration -Wstrict-prototypes -Wnested-externs -mcpu=8540 -meabi -msdata=sysv -fno-common -D__ppc_generic -o loopback.exe init.o /usr/home/nick/rtems/rtems-4.11/bin/../lib/gcc/powerpc-rtems4.11/4.9.3/../../../../powerpc-rtems4.11/bin/ld: ../../../../../mvme3100/lib/librtemsbsp.a(bspgetworkarea.o): the target (__rtems_end) of a R_PPC_SDAREL16 relocation is in the wrong output section (.bss) powerpc-rtems4.11-nm -g -n loopback.exe > loopback.num powerpc-rtems4.11-size loopback.exe text data bss dec hex filename 390421 7124 19317 416862 65c5e loopback.exe powerpc-rtems4.11-objcopy -O binary loopback.exe loopback.ralf gmake[6]: Leaving directory '/usr/home/nick/rtems/git/build/rtems/powerpc-rtems4.11/c/mvme3100/testsuites/samples/loopback' Making all in pppd gmake[6]: Entering directory '/usr/home/nick/rtems/git/build/rtems/powerpc-rtems4.11/c/mvme3100/testsuites/samples/pppd' powerpc-rtems4.11-gcc -B../../../../../mvme3100/lib/ -specs bsp_specs -qrtems -DHAVE_CONFIG_H -I. -I../../../../../../../../rtems/c/src/../../testsuites/samples/pppd -I.. -mcpu=8540 -meabi -msdata=sysv -fno-common -D__ppc_generic -O2 -g -Wall -Wmissing-prototypes -Wimplicit-function-declaration -Wstrict-prototypes -Wnested-externs -MT init.o -MD -MP -MF .deps/init.Tpo -c -o init.o ../../../../../../../../rtems/c/src/../../testsuites/samples/pppd/init.c mv -f .deps/init.Tpo .deps/init.Po powerpc-rtems4.11-gcc -B../../../../../mvme3100/lib/ -specs bsp_specs -qrtems -DHAVE_CONFIG_H -I. -I../../../../../../../../rtems/c/src/../../testsuites/samples/pppd -I.. -mcpu=8540 -meabi -msdata=sysv -fno-common -D__ppc_generic -O2 -g -Wall -Wmissing-prototypes -Wimplicit-function-declaration -Wstrict-prototypes -Wnested-externs -MT pppdapp.o -MD -MP -MF .deps/pppdapp.Tpo -c -o pppdapp.o ../../../../../../../../rtems/c/src/../../testsuites/samples/pppd/pppdapp.c mv -f .deps/pppdapp.Tpo .deps/pppdapp.Po powerpc-rtems4.11-gcc -B../../../../../mvme3100/lib/ -specs bsp_specs -qrtems -mcpu=8540 -meabi -msdata=sysv -fno-common -D__ppc_generic -O2 -g -Wall -Wmissing-prototypes -Wimplicit-function-declaration -Wstrict-prototypes -Wnested-externs -mcpu=8540 -meabi -msdata=sysv -fno-common -D__ppc_generic -o pppd.exe init.o pppdapp.o -lpppd /usr/home/nick/rtems/rtems-4.11/bin/../lib/gcc/powerpc-rtems4.11/4.9.3/../../../../powerpc-rtems4.11/bin/ld: ../../../../../mvme3100/lib/librtemsbsp.a(bspgetworkarea.o): the target (__rtems_end) of a R_PPC_SDAREL16 relocation is in the wrong output section (.bss) ../../../../../mvme3100/lib/librtemsbsp.a(bspgetworkarea.o): In function `bsp_work_area_initialize': /home/nick/rtems/git/build/rtems/powerpc-rtems4.11/c/mvme3100/lib/libbsp/powerpc/mvme3100/../../../../../../../../../rtems/c/src/lib/libbsp/powerpc/mvme3100/../../powerpc/shared/startup/bspgetworkarea.c:17:(.text+0x6): relocation truncated to fit: R_PPC_SDAREL16 against symbol `__rtems_end' defined in .bss section in pppd.exe collect2: error: ld returned 1 exit status Makefile:639: recipe for target 'pppd.exe' failed gmake[6]: *** [pppd.exe] Error 1 gmake[6]: Leaving directory '/usr/home/nick/rtems/git/build/rtems/powerpc-rtems4.11/c/mvme3100/testsuites/samples/pppd' Makefile:718: recipe for target 'all-local' failed gmake[5]: *** [all-local] Error 1 gmake[5]: Leaving directory '/usr/home/nick/rtems/git/build/rtems/powerpc-rtems4.11/c/mvme3100/testsuites/samples' Makefile:319: recipe for target 'all' failed gmake[4]: *** [all] Error 2 gmake[4]: Leaving directory '/usr/home/nick/rtems/git/build/rtems/powerpc-rtems4.11/c/mvme3100/testsuites/samples' Makefile:385: recipe for target 'all-recursive' failed gmake[3]: *** [all-recursive] Error 1 gmake[3]: Leaving directory '/usr/home/nick/rtems/git/build/rtems/powerpc-rtems4.11/c/mvme3100/testsuites' Makefile:496: recipe for target 'all-recursive' failed gmake[2]: *** [all-recursive] Error 1 gmake[2]: Leaving directory '/usr/home/nick/rtems/git/build/rtems/powerpc-rtems4.11/c/mvme3100' Makefile:359: recipe for target 'all-recursive' failed gmake[1]: *** [all-recursive] Error 1 gmake[1]: Leaving directory '/usr/home/nick/rtems/git/build/rtems/powerpc-rtems4.11/c' Makefile:482: recipe for target 'all-recursive' failed gmake: *** [all-recursive] Error 1
I am not sure if the SPR 268 exists really on all PowerPCs supported by RTEMS:
static inline uint64_t PPC_Get_timebase_register( void )
{
uint32_t tbr_low;
uint32_t tbr_high;
uint32_t tbr_high_old;
uint64_t tbr;
do {
#if defined(mpx8xx)
defined(mpc860) defined(mpc821)
/* See comment above (CPU_Get_timebase_low) */
asm volatile( "mftbu %0" : "=r" (tbr_high_old));
asm volatile( "mftb %0" : "=r" (tbr_low));
asm volatile( "mftbu %0" : "=r" (tbr_high));
#else
asm volatile( "mfspr %0, 269" : "=r" (tbr_high_old));
asm volatile( "mfspr %0, 268" : "=r" (tbr_low));
asm volatile( "mfspr %0, 269" : "=r" (tbr_high));
#endif
} while ( tbr_high_old != tbr_high );
tbr = tbr_high;
tbr <<= 32;
tbr |= tbr_low;
return tbr;
}
However these exceptions look more like museum hardware.
Seems like they exist on the MPC860 at least (though they can't be used in place of mttb as they're user-level / read-only).
[1] -D__ppc_generic was left in to shut powerpc.h up, but I'll investigate whether that's appropriate and/or explicitly whack an mpc8540 check in there
comment:5 Changed on 07/09/15 at 08:00:47 by Sebastian Huber <sebastian.huber@…>
comment:6 follow-up: 7 Changed on 07/14/15 at 01:45:36 by Nick Withers
Sorry Sebastian, I'm still trying to work through this...
I seem to have problems with my toolset and can't even get current-source-builder-built GDB to talk to my BDI-3000.
It looks like SPE might be getting enabled (e.g., *trying* to boot gives "Welcome to rtems-4.10.99.0(PowerPC/Generic (E500/float-gprs/SPE)/mvme3100)"), so I'm trying to figure out whether the previous GCC 4.9.2 patches definitely made it in to 4.9.3 (would that explain GDB not being able to talk to the BDI though?!). Dunno what I'm on about here, still digging :-P
comment:7 Changed on 07/14/15 at 02:00:56 by Nick Withers
Replying to nick.withers:
I seem to have problems with my toolset and can't even get current-source-builder-built GDB to talk to my BDI-3000.
It looks like SPE might be getting enabled (e.g., *trying* to boot gives "Welcome to rtems-4.10.99.0(PowerPC/Generic (E500/float-gprs/SPE)/mvme3100)"), so I'm trying to figure out whether the previous GCC 4.9.2 patches definitely made it in to 4.9.3 (would that explain GDB not being able to talk to the BDI though?!).
The patches did indeed make it in.
Latest GDB actually does work fine talking to my BDI - but only if it's not given the (presumably incorrectly-SPE-enabled) binary.
Any ideas?
comment:8 Changed on 07/14/15 at 04:50:23 by Nick Withers
If I build with "CPU_CFLAGS = -mcpu=8540 -meabi -msdata=sysv -msoft-float -mno-spe -fno-common" all seems kosher (or at least as kosher as it did with -mcpu=powerpc before changing the CFLAGS from what's in Git HEAD, though I haven't got back to the profiling bit yet and am expecting the Alternate Time Base access in _CPU_Counter_read() to fail).
GDB reports arch of "powerpc:common", which is what I was seeing with -mcpu=powerpc.
Without -msoft-float -mno-spe it reports "powerpc:e500" and is an unhappy camper, e.g.:
(gdb) target remote bdi:2159 Remote debugging using bdi:2159 Remote 'g' packet reply is too long: deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e4008a0ac40066d2e01020298deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefIf I build with -msoft-float but not mno-spe it seems to try to link against hard-float multilibs:
/usr/home/nick/rtems/rtems-4.11/bin/../lib/gcc/powerpc-rtems4.11/4.9.3/../../../../powerpc-rtems4.11/bin/ld: Warning: /usr/home/nick/rtems/rtems-4.11/bin/../lib/gcc/powerpc-rtems4.11/4.9.3/../../../../powerpc-rtems4.11/lib/libc.a(lib_a-printf.o) uses hard float, hello.exe uses soft floatGoing off this in score/powerpc.h:
#if defined(_SOFT_FLOAT) \ || defined(__NO_FPRS__) /* e500 has unified integer/FP registers */ \ || defined(__PPC_CPU_E6500__) #define PPC_HAS_FPU 0 #else #define PPC_HAS_FPU 1 #endif...the intention's that we should be soft-float on the e500?
comment:9 Changed on 07/14/15 at 06:28:03 by Sebastian Huber
We can still use your original patch and use the SPR 268. I don't care that much about the legacy PowerPCs that lack this register. Some of the newer cores for automotive chips don't have a time base at all, but this is a different issue.
For e500 you have the following options
- soft-float (-msoft-float -mno-spe),
- SPE with 32-bit float (-mspe -mabi=spe -mfloat-gprs=single_, and
- SPE with 64-bit float (e500v2, -mspe -mabi=spe -mfloat-gprs=double).
comment:10 Changed on 07/15/15 at 05:54:57 by Nick Withers
https://sourceware.org/ml/libc-alpha/2008-04/msg00036.html and https://llvm.org/bugs/show_bug.cgi?id=23680 have some interesting discussion about
mftb
et al., including a quote from the PPC ISA 2.07 doco which says mfspr 268/269 should be preferred and that "It is believed that the mfspr instruction can be used to read the Time Base on most processors that comply with versions of the architecture that precede Version 2.01. Processors for which mfspr cannot be used to read the Time Base include the following.
- 601
- POWER3".
This and e.g., the MPC860 definitely having 'em would seem to contradict part of the comment against
CPU_Get_timebase_low
. Also, I don't understand the "we run in supervisory mode so that should work on all CPUs" bit... From my reading it seems clear SPR 268 / 269 should be readable in user mode too.
Also, psim (GDB 7.9) now seems to handle
mftb
andmfspr
fine (GDB disass showsmftb
in the disassembly for both, but they're the requested instructions - opcodes {31, 371} and {31, 339} - underneath), so the "PSIM currently lacks support for reading SPRs 268/269" comment seems past its use-by date. [1]
Regarding the "Alternate Time Base" currently used if
defined(ppc8540) || defined(__PPC_CPU_E6500__)
, http://www.freescale.com/files/32bit/doc/white_paper/POWRPCARCPRMRM.pdf says: "The EIS defines an alternate time base APU, which is implemented on the e500v2. It is intended for measuring time in implementation-defined intervals. It differs from the PowerPC-defined time base in that it is not writable, it counts at a different, typically higher, frequency". I'd like to unify_CPU_Counter_read()
,CPU_Get_timebase_low
andPPC_Get_timebase_register()
so we're consistent with how we grab the time base... Should I get rid of alternate time base accesses in_CPU_Counter_read()
? I believe that's the only place they're used (through grepping for "526").
On to the CFLAGS...
1 Seems to work. GDB no longer reports it as an "powerpc:e500" but "powerpc:common", but I guess the former just means that the binary's SPE-with-shared-registers-enabled or something and the latter doesn't indicate the compilation wasn't correctly targeted.
ppc8540
*is* defined, where it wasn't before.
I tried 2 with
CPU_CFLAGS = -mcpu=8540 -meabi -msdata=sysv -mspe -mabi=spe -mfloat-gprs=single -fno-common
(that right?) but the board resets at some point soon after bsp_start(). GDB won't talk to it (as per above). I also tried twiddlingPPC_HAS_FPU
on andPPC_HAS_DOUBLE
off without success.
I suppose 2 would be significantly better for users with single-precision floating point applications?
[1] The sim/2376 ticket mentioned is now https://sourceware.org/bugzilla/show_bug.cgi?id=9481 . I'll shoot 'em off an email momentarily to let 'em know it's no longer an issue
Changed on 07/15/15 at 06:24:08 by Nick Withers
Attachment: ppc-mfspr.patch added Proposed patch to use mfspr from the main time base universally; does not address MVME3100 CFLAGS issues
comment:11 Changed on 07/15/15 at 07:03:19 by Nick Withers <nick.withers@…>
Owner: set to Nick Withers <nick.withers@…> Resolution: → fixed Status: new → closed comment:12 follow-up: 14 Changed on 07/15/15 at 07:05:15 by Sebastian Huber
Sorry, I didn't noticed your patch. the Alternate Time Base may run with a much higher frequency, so I would like to keep it.
comment:13 Changed on 07/15/15 at 07:20:13 by Sebastian Huber
I would leave the c/src/lib/libcpu/powerpc/shared/include/powerpc-utility.h as is.
comment:14 follow-up: 15 Changed on 07/15/15 at 08:20:12 by Nick Withers
Replying to sebastian.huber:
Sorry, I didn't noticed your patch. the Alternate Time Base may run with a much higher frequency, so I would like to keep it.
rtems_bsp_delay_in_bus_cycles()
at least is gonna be wrong (or misleadingly named, perhaps) for platforms using the ATB where it happens not to be operating at bus frequency, isn't it?
Next thing though is that the ATB isn't supported for e500v1s ( http://cache.freescale.com/files/32bit/doc/ref_manual/E500CORERM.pdf p 1-19 )... Is
_CPU_Counter_read()
's
#if defined(ppc8540) || defined(__PPC_CPU_E6500__)meant to be
#if defined(ppc8548) || defined(__PPC_CPU_E6500__)...or similar?
It'd be quite nice if we (GCC?) had individual feature flags for some of this stuff, given how all-over-the-place the various PPC cores are, wouldn't it? I'll have a crack if you think so.
Also, would you be alright with removing the "OTOH, PSIM currently lacks support for reading SPRs 268/269. You need GDB patch sim/2376 to avoid a crash..." comment in powerpc-utility.h? https://sourceware.org/bugzilla/show_bug.cgi?id=9481 is now closed ('twas Joel fixed the issue, back in 2010).
I've cunningly avoided thinking properly about whether ignoring the upper portion of the time base is a good idea... But, for example, isn't
rtems_bsp_delay()
going to stuff up if the lower 32 bits wrap?
comment:15 follow-up: 16 Changed on 07/15/15 at 08:26:30 by Sebastian Huber
Replying to nick.withers:
Replying to sebastian.huber:
Sorry, I didn't noticed your patch. the Alternate Time Base may run with a much higher frequency, so I would like to keep it.
rtems_bsp_delay_in_bus_cycles()
at least is gonna be wrong (or misleadingly named, perhaps) for platforms using the ATB where it happens not to be operating at bus frequency, isn't it?
Next thing though is that the ATB isn't supported for e500v1s ( http://cache.freescale.com/files/32bit/doc/ref_manual/E500CORERM.pdf p 1-19 )... Is
_CPU_Counter_read()
's
#if defined(ppc8540) || defined(__PPC_CPU_E6500__)meant to be
#if defined(ppc8548) || defined(__PPC_CPU_E6500__)...or similar?
This is bad, since we don't have a ppc8548 multilib. In this case we have to drop the defined(ppc8540).
It'd be quite nice if we (GCC?) had individual feature flags for some of this stuff, given how all-over-the-place the various PPC cores are, wouldn't it? I'll have a crack if you think so.
Also, would you be alright with removing the "OTOH, PSIM currently lacks support for reading SPRs 268/269. You need GDB patch sim/2376 to avoid a crash..." comment in powerpc-utility.h? https://sourceware.org/bugzilla/show_bug.cgi?id=9481 is now closed ('twas Joel fixed the issue, back in 2010).
Ok, can you please send a separate patch to the mailing list.
I've cunningly avoided thinking properly about whether ignoring the upper portion of the time base is a good idea... But, for example, isn't
rtems_bsp_delay()
going to stuff up if the lower 32 bits wrap?
The use case for the _CPU_Counter_read() is to measure short time intervals for the SMP lock profiling and short delay loops for device drivers. Do you really want to busy wait for 123 seconds?
The rtems_bsp_*() are legacy functions. I would not use them.
comment:16 follow-ups: 17 18 Changed on 07/15/15 at 08:33:06 by Nick Withers
Replying to sebastian.huber:
Replying to nick.withers:
Replying to sebastian.huber:
Sorry, I didn't noticed your patch. the Alternate Time Base may run with a much higher frequency, so I would like to keep it.
rtems_bsp_delay_in_bus_cycles()
at least is gonna be wrong (or misleadingly named, perhaps) for platforms using the ATB where it happens not to be operating at bus frequency, isn't it?
Next thing though is that the ATB isn't supported for e500v1s ( http://cache.freescale.com/files/32bit/doc/ref_manual/E500CORERM.pdf p 1-19 )... Is
_CPU_Counter_read()
's
#if defined(ppc8540) || defined(__PPC_CPU_E6500__)meant to be
#if defined(ppc8548) || defined(__PPC_CPU_E6500__)...or similar?
This is bad, since we don't have a ppc8548 multilib. In this case we have to drop the defined(ppc8540).
It'd be quite nice if we (GCC?) had individual feature flags for some of this stuff, given how all-over-the-place the various PPC cores are, wouldn't it? I'll have a crack if you think so.
Also, would you be alright with removing the "OTOH, PSIM currently lacks support for reading SPRs 268/269. You need GDB patch sim/2376 to avoid a crash..." comment in powerpc-utility.h? https://sourceware.org/bugzilla/show_bug.cgi?id=9481 is now closed ('twas Joel fixed the issue, back in 2010).
Ok, can you please send a separate patch to the mailing list.
Shall do.
I've cunningly avoided thinking properly about whether ignoring the upper portion of the time base is a good idea... But, for example, isn't
rtems_bsp_delay()
going to stuff up if the lower 32 bits wrap?
The use case for the _CPU_Counter_read() is to measure short time intervals for the SMP lock profiling and short delay loops for device drivers. Do you really want to busy wait for 123 seconds?
Do you really want to busy wait 123 seconds instead of the intended short time interval because you lost the race and the lower 32 bits of the counter wrapped?
comment:17 Changed on 07/15/15 at 08:39:13 by Sebastian Huber
Replying to nick.withers:
Replying to sebastian.huber:
Replying to nick.withers:
Replying to sebastian.huber:
[...]
The use case for the _CPU_Counter_read() is to measure short time intervals for the SMP lock profiling and short delay loops for device drivers. Do you really want to busy wait for 123 seconds?
Do you really want to busy wait 123 seconds instead of the intended short time interval because you lost the race and the lower 32 bits of the counter wrapped?
The rtems_counter_delay_ticks() doesn't have this problem. I just may not account for the interrupted time if this time is in the range of one time base period.
comment:18 follow-up: 19 Changed on 07/15/15 at 08:41:13 by Nick Withers
Replying to nick.withers:
Replying to sebastian.huber:
Replying to nick.withers:
I've cunningly avoided thinking properly about whether ignoring the upper portion of the time base is a good idea... But, for example, isn't
rtems_bsp_delay()
going to stuff up if the lower 32 bits wrap?
The use case for the _CPU_Counter_read() is to measure short time intervals for the SMP lock profiling and short delay loops for device drivers. Do you really want to busy wait for 123 seconds?
Do you really want to busy wait 123 seconds instead of the intended short time interval because you lost the race and the lower 32 bits of the counter wrapped?
Actually, I'm full of it there I think, sorry. The 32 bit unsigned multiplication's gonna wrap too in that case, right?
comment:19 Changed on 07/15/15 at 08:45:37 by Sebastian Huber
Replying to nick.withers:
Replying to nick.withers:
Replying to sebastian.huber:
Replying to nick.withers:
I've cunningly avoided thinking properly about whether ignoring the upper portion of the time base is a good idea... But, for example, isn't
rtems_bsp_delay()
going to stuff up if the lower 32 bits wrap?
The use case for the _CPU_Counter_read() is to measure short time intervals for the SMP lock profiling and short delay loops for device drivers. Do you really want to busy wait for 123 seconds?
Do you really want to busy wait 123 seconds instead of the intended short time interval because you lost the race and the lower 32 bits of the counter wrapped?
Actually, I'm full of it there I think, sorry. The 32 bit unsigned multiplication's gonna wrap too in that case, right?
Yes.
comment:20 Changed on 07/15/15 at 08:53:25 by Sebastian Huber <sebastian.huber@…>
Note: See TracTickets for help on using tickets.Download in other formats:
We have:
The ppc8540 is the multilib intended for the e500 processors, why is this not used?