gh-raspberrypi/pico-sdk

Watch

Fork 0

glitch-free clock not glitch-free #1239

New issue

Closed

opened 2023-02-10 08:09:32 -06:00 by bruelltuete · 15 comments

bruelltuete commented

2023-02-10 08:09:32 -06:00

(Migrated from github.com)

I took inspiration from clocks_init() for switching clocks for entering and leaving power-save sleeping.

clocks_init() first switches clk_ref and clk_sys to a save input, specifically rosc.
But in the order of clk_sys to clk_ref, and afterwards clk_ref to rosc.
It then switches clk_ref to xosc and thus clk_sys indirectly as well.

I have observed that switching clk_ref and indirectly clk_sys in such a way will introduce glitches into clk_sys.
To avoid you'd have to switch clk_sys to anything else first.

(c.f. https://forums.raspberrypi.com/viewtopic.php?t=347154)

I took inspiration from clocks_init() for switching clocks for entering and leaving power-save sleeping. clocks_init() first switches clk_ref and clk_sys to a save input, specifically rosc. But in the order of clk_sys to clk_ref, and afterwards clk_ref to rosc. It then switches clk_ref to xosc and thus clk_sys indirectly as well. I have observed that switching clk_ref and indirectly clk_sys in such a way will introduce glitches into clk_sys. To avoid you'd have to switch clk_sys to anything else first. (c.f. https://forums.raspberrypi.com/viewtopic.php?t=347154)

P33M commented

2023-02-10 08:45:10 -06:00

(Migrated from github.com)

Please provide a minimal C SDK testcase example that demonstrates the lockup.

bruelltuete commented

2023-02-10 09:36:36 -06:00

(Migrated from github.com)

I'll come up with an example...

But just to be clear: I have no way of observing clk_sys directly, I can only see the aftermath of the glitching: xip cache and memory corruption.
Switching clk_sys away from clk_ref first makes all the problems go away.

The pseudo code looks like:

while (true)
{
   do_stuff();

   // switch clk_ref & sys to rosc
   // go to sleep

   // wake up from rtc alarm
   // switch clk_ref & sys to xosc                      <-- after a while bad stuff happens here
   // switch clk_sys to pll
}

I'll come up with an example... But just to be clear: I have no way of observing clk_sys directly, I can only see the aftermath of the glitching: xip cache and memory corruption. Switching clk_sys away from clk_ref first makes all the problems go away. The pseudo code looks like: ``` while (true) { do_stuff(); // switch clk_ref & sys to rosc // go to sleep // wake up from rtc alarm // switch clk_ref & sys to xosc <-- after a while bad stuff happens here // switch clk_sys to pll } ```

bruelltuete commented

2023-02-10 19:57:47 -06:00

(Migrated from github.com)

This crashes after a few seconds:

#include "hardware/clocks.h"
#include "hardware/structs/xip_ctrl.h"

static void __no_inline_not_in_flash_func(xipflush)()
{
    xip_ctrl_hw->flush = XIP_FLUSH_BITS;
    volatile int a = xip_ctrl_hw->flush;
}

int main()
{

    while (true)
    {
        // flush xip cache to make sure that there's something observable relying on clk_sys.
        // if it glitches then fetching instruction bytes from flash will go wrong pretty quickly.
        // if there are no glitches then nothing bad happens.
        xipflush();

        clocks_init();
    }
}

This crashes after a few seconds: ``` #include "hardware/clocks.h" #include "hardware/structs/xip_ctrl.h" static void __no_inline_not_in_flash_func(xipflush)() { xip_ctrl_hw->flush = XIP_FLUSH_BITS; volatile int a = xip_ctrl_hw->flush; } int main() { while (true) { // flush xip cache to make sure that there's something observable relying on clk_sys. // if it glitches then fetching instruction bytes from flash will go wrong pretty quickly. // if there are no glitches then nothing bad happens. xipflush(); clocks_init(); } } ```

bruelltuete commented

2023-02-10 20:00:28 -06:00

(Migrated from github.com)

maybe related: should xosc_init() at https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/hardware_clocks/clocks.c#L138 move below the while loops?

P33M commented

2023-02-11 06:17:59 -06:00

(Migrated from github.com)

xosc_init should be benign if called on an already-enabled xosc.

One thing to note is that if you have clksrc -> clk_ref -> clk_sys, and you switch clk_ref between its two glitchless sources, then there is a period of time where the whole chip is not clocked.

xosc_init should be benign if called on an already-enabled xosc. One thing to note is that if you have clksrc -> clk_ref -> clk_sys, and you switch clk_ref between its two glitchless sources, then there is a period of time where the whole chip is not clocked.

bruelltuete commented

2023-02-11 09:59:16 -06:00

(Migrated from github.com)

One thing to note is that if you have clksrc -> clk_ref -> clk_sys, and you switch clk_ref between its two glitchless sources, then there is a period of time where the whole chip is not clocked.

I think that pause should be ok... as long as there's no pheripheral i/o depending on that (sys-)clock.

edit: and in the case of xip, I was expecting the pause to not matter because there is an explicit clock signal to the flash.

> One thing to note is that if you have clksrc -> clk_ref -> clk_sys, and you switch clk_ref between its two glitchless sources, then there is a period of time where the whole chip is not clocked. I think that pause should be ok... as long as there's no pheripheral i/o depending on that (sys-)clock. edit: and in the case of xip, I was expecting the pause to not matter because there is an explicit clock signal to the flash.

bruelltuete commented

2023-03-06 12:44:38 -06:00

(Migrated from github.com)

any thoughts? was this reproducible on your end?

P33M commented

2023-03-07 08:01:37 -06:00

(Migrated from github.com)

Unfortunately not. Works fine on a Pico (B1 step) that I have here, in either relwithdebuginfo or debug builds.

bruelltuete commented

2023-03-09 08:50:28 -06:00

(Migrated from github.com)

dang...
I only have B2s here.

dang... I only have B2s here.

alastairpatrick commented

2023-03-09 13:59:48 -06:00

(Migrated from github.com)

I can reproduce this on a B1 board every time. Example GDB session, starting with it paused on entry to main():

> disassemble __xipflush_veneer
Dump of assembler code for function __xipflush_veneer:
   0x10003cf8 <+0>:	push	{r0}
   0x10003cfa <+2>:	ldr	r0, [pc, #8]	; (0x10003d04 <__xipflush_veneer+12>)
   0x10003cfc <+4>:	mov	r12, r0
   0x10003cfe <+6>:	pop	{r0}
   0x10003d00 <+8>:	bx	r12
   0x10003d02 <+10>:	nop
   0x10003d04 <+12>:	lsls	r1, r0, #3
   0x10003d06 <+14>:	movs	r0, #0
End of assembler dump.
> continue
Continuing.

Note that the disassembly is of a function in XIP cache and the disassembly is correct. Now wait for it to crash, which takes less than one second:


Thread 
1 stopped.
main () at D:\src\rprand\main.c:18
18	        xipflush();
> disassemble __xipflush_veneer
Dump of assembler code for function __xipflush_veneer:
   0x10003cf8 <+0>:	push	{r0}
   0x10003cfa <+2>:	ldr	r0, [pc, #8]	; (0x10003d04 <__xipflush_veneer+12>)
   0x10003cfc <+4>:	mov	r12, r0
   0x10003cfe <+6>:	pop	{r0}
   0x10003d00 <+8>:	movs	r0, r0
   0x10003d02 <+10>:	movs	r0, r0
   0x10003d04 <+12>:	movs	r0, r0
   0x10003d06 <+14>:	movs	r0, r0
End of assembler dump.

Note that the disassembly of the function has changed and is incorrect, suggesting that XIP read bogus data from flash.

I can reproduce this on a B1 board every time. Example GDB session, starting with it paused on entry to main(): ``` > disassemble __xipflush_veneer Dump of assembler code for function __xipflush_veneer: 0x10003cf8 <+0>: push {r0} 0x10003cfa <+2>: ldr r0, [pc, #8] ; (0x10003d04 <__xipflush_veneer+12>) 0x10003cfc <+4>: mov r12, r0 0x10003cfe <+6>: pop {r0} 0x10003d00 <+8>: bx r12 0x10003d02 <+10>: nop 0x10003d04 <+12>: lsls r1, r0, #3 0x10003d06 <+14>: movs r0, #0 End of assembler dump. > continue Continuing. ``` Note that the disassembly is of a function in XIP cache and the disassembly is correct. Now wait for it to crash, which takes less than one second: ``` Thread 1 stopped. main () at D:\src\rprand\main.c:18 18 xipflush(); > disassemble __xipflush_veneer Dump of assembler code for function __xipflush_veneer: 0x10003cf8 <+0>: push {r0} 0x10003cfa <+2>: ldr r0, [pc, #8] ; (0x10003d04 <__xipflush_veneer+12>) 0x10003cfc <+4>: mov r12, r0 0x10003cfe <+6>: pop {r0} 0x10003d00 <+8>: movs r0, r0 0x10003d02 <+10>: movs r0, r0 0x10003d04 <+12>: movs r0, r0 0x10003d06 <+14>: movs r0, r0 End of assembler dump. ``` Note that the disassembly of the function has changed and is incorrect, suggesting that XIP read bogus data from flash.

P33M commented

2023-03-09 14:01:13 -06:00

(Migrated from github.com)

Please post the full project directory (including cmakelists.txt) as a zip.

alastairpatrick commented

2023-03-09 14:44:48 -06:00

(Migrated from github.com)

Project attached.
repro.zip

Project attached. [repro.zip](https://github.com/raspberrypi/pico-sdk/files/10935888/repro.zip)

bruelltuete commented

2023-04-24 08:48:08 -05:00

(Migrated from github.com)

any luck reproducing this on your end?

P33M commented

2023-04-25 03:53:50 -05:00

(Migrated from github.com)

Negative. Across 4 Picos of random vintage, none of them exhibit the failure mode. Can you upload a .elf with debug symbols that you know will hang on your board? Removes a lot of guesswork.

Edit: In the meantime, is the failure sensitive to core voltage - e.g. hardware/structs/vreg_and_chip_reset.h

Negative. Across 4 Picos of random vintage, none of them exhibit the failure mode. Can you upload a .elf with debug symbols that you know will hang on your board? Removes a lot of guesswork. Edit: In the meantime, is the failure sensitive to core voltage - e.g. [hardware/structs/vreg_and_chip_reset.h](https://www.raspberrypi.com/documentation/pico-sdk/hardware.html#hardware_vreg)

liamfraser commented

2024-08-01 05:06:39 -05:00

(Migrated from github.com)