raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.55k stars 881 forks source link

Unexpected values for CRC from DMA sniffer #576

Closed usedbytes closed 2 years ago

usedbytes commented 2 years ago

I'm trying to use the DMA sniffer to calculate the CRC of some data, but it doesn't behave as I'd expect.

In particular, the value it returns for CRC32 of a string of zeroes is zero, which doesn't seem right.

I'm using the following test program:

#include <stdio.h>

#include "hardware/dma.h"
#include "hardware/structs/dma.h"
#include "pico/stdlib.h"

void print_hex(const uint8_t *addr, size_t len)
{
    int row, col, count = 0;

    for (row = 0; count < len; row++) {
        printf("%02x: ", row * 16);
        for (col = 0; (col < 16) && (count < len); col++, count++) {
            printf("%02x ", addr[count]);
        }
        printf("\n");
    }
}

void main(void)
{
    stdio_init_all();

    // Wait for serial open
    sleep_ms(2000);

    struct {
        int len;
        const uint8_t *data;
    } tests[] = {
        {
            .len = 16,
            .data = (uint8_t[]){
                1, 0, 0, 0, 0, 2, 0, 0, 0, 0, 3, 0, 0, 0, 0, 4
            },
        },
        {
            .len = 4,
            .data = (uint8_t[]){
                0, 0, 0, 0,
            },
        },
        {
            .len = 16,
            .data = (uint8_t[]){
                0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
            },
        },
        {
            .len = 8,
            .data = (uint8_t[]){
                1, 1, 1, 1, 1, 1, 1, 1,
            },
        },
    };
    uint32_t dest;

    int test, mode, modes[] = {
        0x0, 0x1, 0x2, 0x3, 0xe, 0xf,
    };

    int channel = dma_claim_unused_channel(true);
    dma_channel_config c = dma_channel_get_default_config(channel);
    channel_config_set_transfer_data_size(&c, DMA_SIZE_32);
    channel_config_set_read_increment(&c, true);
    channel_config_set_write_increment(&c, false);
    channel_config_set_sniff_enable(&c, true);

    for (test = 0; test < sizeof(tests) / sizeof(tests[0]); test++) {
        printf("---\n");
        printf("Data: (%d bytes)\n", tests[test].len);
        print_hex(tests[test].data, tests[test].len);
        printf("DMA_SIZE_32\n");

        for (mode = 0; mode < sizeof(modes) / sizeof(modes[0]); mode++) {
            // Reset sniffer data
            dma_hw->sniff_data = 0;

            dma_sniffer_enable(channel, modes[mode], true);
            dma_channel_configure(channel, &c, &dest, tests[test].data, tests[test].len / 4, true);
            dma_channel_wait_for_finish_blocking(channel);
            dma_sniffer_disable();
            dma_channel_unclaim(channel);

            printf("Mode %x, result: 0x%08x\n", modes[mode], dma_hw->sniff_data);
        }
    }
}

Which gives this output:

---
Data: (16 bytes)
00: 01 00 00 00 00 02 00 00 00 00 03 00 00 00 00 04
DMA_SIZE_32
Mode 0, result: 0xe653cc05
Mode 1, result: 0xe779bfee
Mode 2, result: 0x000086f3
Mode 3, result: 0x0000d6f6
Mode e, result: 0x00000001
Mode f, result: 0x04030201
---
Data: (4 bytes)
00: 00 00 00 00
DMA_SIZE_32
Mode 0, result: 0x00000000
Mode 1, result: 0x00000000
Mode 2, result: 0x00000000
Mode 3, result: 0x00000000
Mode e, result: 0x00000000
Mode f, result: 0x00000000
---
Data: (16 bytes)
00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
DMA_SIZE_32
Mode 0, result: 0x00000000
Mode 1, result: 0x00000000
Mode 2, result: 0x00000000
Mode 3, result: 0x00000000
Mode e, result: 0x00000000
Mode f, result: 0x00000000
---
Data: (8 bytes)
00: 01 01 01 01 01 01 01 01
DMA_SIZE_32
Mode 0, result: 0x4dbe3475
Mode 1, result: 0x5dbfc1d2
Mode 2, result: 0x000072d7
Mode 3, result: 0x0000ccfa
Mode e, result: 0x00000000
Mode f, result: 0x02020202

The "Mode f" values look correct (summation of all the input data, 32-bits at a time), so I'm fairly sure my code is working as expected, but I'm not able to get the CRC values to line up with what I'd expect.

For reference, the same tests in go and their output below. You can run it online here: https://play.golang.org/p/ZZA9B6oNCsw

Go's ChecksumIEEE is using the IEEE802.3 polynomial, the same as is mentioned in the RP2040 datasheet.

package main

import (
    "encoding/hex"
    "fmt"
    "hash/crc32"
)

func main() {
    tests := [][]byte{
        {
            1, 0, 0, 0, 0, 2, 0, 0, 0, 0, 3, 0, 0, 0, 0, 4,
        },
        {
            0, 0, 0, 0,
        },
        {
            0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
        },
        {
            1, 1, 1, 1, 1, 1, 1, 1,
        },
    };

    for _, t := range tests {
        fmt.Println("---")
        fmt.Printf("Data: (%d bytes)\n", len(t))
        fmt.Print(hex.Dump(t))
        fmt.Printf("CRC32: 0x%08x\n", crc32.ChecksumIEEE(t))
    }
}
$ go run gotest.go
---
Data: (16 bytes)
00000000  01 00 00 00 00 02 00 00  00 00 03 00 00 00 00 04  |................|
CRC32: 0x9b46d5b2
---
Data: (4 bytes)
00000000  00 00 00 00                                       |....|
CRC32: 0x2144df1c
---
Data: (16 bytes)
00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
CRC32: 0xecbb4b55
---
Data: (8 bytes)
00000000  01 01 01 01 01 01 01 01                           |........|
CRC32: 0x2ea122d3

As another data point, the crc32 utility I have on my machine ("This utility is supplied with the Archive::Zip module for Perl.", apparently) produces values which agree with go:

$ crc32 <(head -c 4 /dev/zero)
2144df1c
$ crc32 <(head -c 16 /dev/zero)
ecbb4b55

Can you provide more details on how the DMA sniffer CRC32 algorithm works? Am I using it wrong?

lurch commented 2 years ago

From a quick look on Wikipedia, it seems like for CRC32 you need to initialise dma_hw->sniff_data to 0xFFFFFFFF first? (and then XOR it with 0xFFFFFFFF again afterwards)

usedbytes commented 2 years ago

Thank you! That helped a lot.

Using 0xffffffff as a starting value, XOR with 0xffffffff on the result, and using Mode 1, I can get values which are bit-reversed versions of the Go numbers.

Curiously, the OUT_REV control bit doesn't seem to do anything. I think if I could get that to work then Mode 1 + OUT_REV would give me the same values as Go directly.

---
Data: (16 bytes)
00: 01 00 00 00 00 02 00 00 00 00 03 00 00 00 00 04
DMA_SIZE_32
Mode 0, sniff_ctrl: 0x00000001, result: 0x4c811132
Mode 0, sniff_ctrl: 0x00000401, result: 0x4c811132
Mode 1, sniff_ctrl: 0x00000021, result: 0x4dab62d9
Mode 1, sniff_ctrl: 0x00000421, result: 0x4dab62d9
---
Data: (4 bytes)
00: 00 00 00 00
DMA_SIZE_32
Mode 0, sniff_ctrl: 0x00000001, result: 0x38fb2284
Mode 0, sniff_ctrl: 0x00000401, result: 0x38fb2284
Mode 1, sniff_ctrl: 0x00000021, result: 0x38fb2284
Mode 1, sniff_ctrl: 0x00000421, result: 0x38fb2284
---
Data: (16 bytes)
00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
DMA_SIZE_32
Mode 0, sniff_ctrl: 0x00000001, result: 0xaad2dd37
Mode 0, sniff_ctrl: 0x00000401, result: 0xaad2dd37
Mode 1, sniff_ctrl: 0x00000021, result: 0xaad2dd37
Mode 1, sniff_ctrl: 0x00000421, result: 0xaad2dd37
---
Data: (8 bytes)
00: 01 01 01 01 01 01 01 01
DMA_SIZE_32
Mode 0, sniff_ctrl: 0x00000001, result: 0xdb4570d3
Mode 0, sniff_ctrl: 0x00000401, result: 0xdb4570d3
Mode 1, sniff_ctrl: 0x00000021, result: 0xcb448574
Mode 1, sniff_ctrl: 0x00000421, result: 0xcb448574

Go with reverse:

---
Data: (16 bytes)
00000000  01 00 00 00 00 02 00 00  00 00 03 00 00 00 00 04  |................|
CRC32: 0x9b46d5b2, reversed: 0x4dab62d9
---
Data: (4 bytes)
00000000  00 00 00 00                                       |....|
CRC32: 0x2144df1c, reversed: 0x38fb2284
---
Data: (16 bytes)
00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
CRC32: 0xecbb4b55, reversed: 0xaad2dd37
---
Data: (8 bytes)
00000000  01 01 01 01 01 01 01 01                           |........|
CRC32: 0x2ea122d3, reversed: 0xcb448574
kilograham commented 2 years ago

Curiously, the OUT_REV control bit doesn't seem to do anything

That would be disappointing... can you post a code snippet of how you used it

usedbytes commented 2 years ago

In tidying up my snippet to post, I figured it out - I was reading the result after calling dma_sniffer_disable(), which clears OUT_REV.

So, working as intended.

Having a more complete API for the sniffer would be useful so that you don't have to use hardware_structs to seed SNIFF_DATA and configure SNIFF_CTRL.