pico-feedback icon indicating copy to clipboard operation
pico-feedback copied to clipboard

crc32 for the DMA sniffer is underspecified

Open slimhazard opened this issue 3 years ago • 11 comments

I started a thread in the Pico/General forum in which this subject is discussed in detail:

https://forums.raspberrypi.com/viewtopic.php?p=2014571

The RP2040 datasheet and SDK doc say that the DMA sniffer computes CRC-32 with the "IEEE802.3 polynomial", but IMO that is not enough to clarify how exactly the CRC is computed. There are variations on the polynomial used by different implementations -- the Wikipedia article lists four of them.

I think it would be much clearer if the docs simply stated the value of the polynomial that the sniffer uses.

As detailed in the thread, I've been trying to automate a data integrity test by sending data via DMA over PIO to a USB host, as well as the sniffer's CRC32 result, to check the CRC32 on the host side. I haven't been able to match it with the values from tools on the host side -- the crc32 and cksum command-line tools, zlib's crc32 function, and my own implementations of crc32 using four different values for the polynomial. That includes cases when I could dump the data to be transferred from a gdb session on the Pico, and found that it matched the data received by the host. So at least in those cases, the problem wasn't data corruption, just the CRC computation.

Probably it's me who's not getting it right, rather than some defect in the sniffer. A clearer statement about what to expect would make it easier to figure out.

Some other questions that have come up while I've been trying to do this:

  • Should dma_hw->data_sniff be initialized before the DMA transfer? I've tried that with 0 and 0xffffffff, but neither choice led to a value that I could match on the host side.
  • Does the transfer width affect the CRC32 computation? For example if it's DMA_SIZE_32, does the CRC algorithm iterate through the 4 bytes in little-endian order?

slimhazard avatar Jun 26 '22 13:06 slimhazard

The RP2040 datasheet and SDK doc say that the DMA sniffer computes CRC-32 with the "IEEE802.3 polynomial", but IMO that is not enough to clarify how exactly the CRC is computed. There are variations on the polynomial used by different implementations -- the Wikipedia article lists four of them.

It uses the Normal polynomial (0x4C11DB7)

As detailed in the thread, I've been trying to automate a data integrity test by sending data via DMA over PIO to a USB host, as well as the sniffer's CRC32 result, to check the CRC32 on the host side. I haven't been able to match it with the values from tools on the host side -- the crc32 and cksum command-line tools, zlib's crc32 function, and my own implementations of crc32 using four different values for the polynomial.

You are likely bumping into the many variations of input reverse, output reverse, initial seed and final XOR.

If you go to this webpage: http://www.sunshine2k.de/coding/javascript/crc/crc_js.html

You can set this up for the reset configuration of the sniffer CRC32 as follows:

  • Polynomial 0x4d11db7 (CRC32)
  • No input or output reflection
  • Initial value 0xffffffff (assuming you write this to dma_hw->data_sniff beforehand
  • Final XOR 0x00000000 (unless you do an XOR yourself)

More commonly, input reflection is used, which you can enable by setting DMA_SNIFF_CTRL_CALC to 0x1 -> Calculate a CRC-32 (IEEE802.3 polynomial) with bit reversed data. You can also enable inversion of the final result on read, by setting SNIFF_CTRL_OUT_INV, and bit-reverse the final result, by setting SNIFF_CTRL_OUT_REV. If you set all of these, you can match the default CRC32 options on that web page:

  • Polynomial 0x4d11db7
  • Input and output reflection
  • Initial value 0xffffffff
  • Final XOR value 0xffffffff

Should dma_hw->data_sniff be initialized before the DMA transfer?

Yes, this is really just an accumulator that the sniffer updates after each transfer, so you need to initialise it with the digest seed value

Does the transfer width affect the CRC32 computation?

For 8-bit transfers, the sniffer digests 8 bits at a time. For 16-bit transfers, 16 bits, so on. Digesting the same array twice with the different transfer sizes should give the same result.

By default the sniffer processes the individual bytes in little-endian order, with the bit order in those bytes set according to the CTRL_CALC value. You can reverse the byte order by setting CTRL_BSWAP.

Wren6991 avatar Jun 27 '22 09:06 Wren6991

Do you think the documentation needs to be clarified @Wren6991 ?

aallan avatar Jun 27 '22 09:06 aallan

Yes, once I'm sure we have enough information I'll do a docs update

Wren6991 avatar Jun 27 '22 09:06 Wren6991

Thanks Luke!

aallan avatar Jun 27 '22 09:06 aallan

@Wren6991 that's excellent information, thank you. +1 for addition to the docs.

slimhazard avatar Jun 27 '22 11:06 slimhazard

I had a tickly feeling that something similar came up before, and managed to track down https://github.com/raspberrypi/pico-sdk/issues/576

lurch avatar Jun 27 '22 13:06 lurch

I have this working now so that the CRC32 computed by the sniffer matches the value returned by zlib's crc32 function (which is the same value in the output of the crc32 command-line utility). Except for an issue with endianness on the host side, which is easily addressed, more about that in a bit.

@Wren6991 your hints about OUT_INV and OUT_REV in SNIFF_CTRL turned out to be critical. I believe that there is no API for those bits in the SDK, you can only set them in the dma_hw struct. At any rate I overlooked them, and both need to be set to get the result.

It should be mentioned that I have turned on byte swap in the DMA channel. The data are generated as uint32_t on the Pico, then pushed most-significant-byte-first over PIO. In order for the data to be read as uint32_t on the little-endian host, exactly as originally generated on the Pico, the byte swap is necessary.

I have this configuration for DMA:

	cfg = dma_channel_get_default_config(chan);
	channel_config_set_read_increment(&cfg, true);
	channel_config_set_write_increment(&cfg, false);
	channel_config_set_transfer_data_size(&cfg, DMA_SIZE_32);
	channel_config_set_dreq(&cfg, pio_get_dreq(pio, sm, true));
	channel_config_set_bswap(&cfg, true);
	dma_channel_configure(chan, &cfg, &pio->txf[sm], NULL, REC_WORDS,
			      false);
	dma_sniffer_enable(chan, 1, true);
	dma_sniffer_set_byte_swap_enabled(true);
	hw_set_bits(&dma_hw->sniff_ctrl,
		    (DMA_SNIFF_CTRL_OUT_INV_BITS | DMA_SNIFF_CTRL_OUT_REV_BITS));

So the sniffer sets bit-reversed 802.3 crc32, byte swap enabled, and the OUT_INV and OUT_REV bits of SNIFF_CTRL are set.

Then in the main loop, on each iteration:

		dma_hw->sniff_data = 0xffffffff;
		dma_channel_set_read_addr(chan, addr, true);
		dma_channel_wait_for_finish_blocking(chan);
		pio_sm_put(pio, sm, dma_hw->sniff_data);

The result is that the CRC received by the host after the end of the transferred data matches zlib's crc32, except that they have opposite byte order. On the host side I do this:

		// crc is the value sent from the Pico.
		crc = __builtin_bswap32(crc);
		crc_zlib = crc32(0L, Z_NULL, 0);
		crc_zlib = crc32(crc_zlib, (uint8_t *)buf, BUFSZ);

Then the two CRCs match.

It makes me wonder why the byte orders don't match, but verifying the CRC is easy enough.

Thanks again to @Wren6991. And I have a new feedback suggestion: add access to OUT_INV and OUT_REV to SDK API. Is it enough to say so here, or should I add a new issue to pico-feedback or pico-sdk?

slimhazard avatar Jun 29 '22 19:06 slimhazard

And I have a new feedback suggestion: add access to OUT_INV and OUT_REV to SDK API. Is it enough to say so here, or should I add a new issue to pico-feedback or pico-sdk?

Probably worth creating an issue on pico-sdk, given that this issue here was originally about the documentation.

lurch avatar Jun 29 '22 19:06 lurch

It makes me wonder why the byte orders don't match, but verifying the CRC is easy enough.

I didn't fully follow who is in which endianness, but the sniffer is downstream of the byte swap performed by the read master when channel BSWAP is enabled. The sniffer also has a byte swap option, which can be set to cancel this out.

This is an important detail so I'll make sure to note this too when I add the previous info to the docs.

Wren6991 avatar Jun 30 '22 09:06 Wren6991

The byte swap settings for DMA channels and the sniffer are already documented, in both the RP2040 datasheet and the SDK docs. As well as the sniffer being downstream of the DMA transfer, and the canceling effect when both byte swap configs are turned on.

I could go into more explanation of what I observed, but I think it's specific to my use case, probably not something general that the docs need to get into. Let my try a tl;dr version of what I think happened with my app: the sniffer value is sent over the same PIO used to send the data, and the state machine pushes out 32-bit values with the most significant byte first (8 bits at a time, shifting left). The result, as I now suspect, is that the sniffer's crc32 value is read in byte-reversed order by the host.

tl;dr of the tl;dr: in my opinion nothing new on the subject of byte swap needs to be added to the docs.

slimhazard avatar Jun 30 '22 10:06 slimhazard

Can we tag this as CRC? (Would be nice if all issues were tagged to the respective modules.)

daveythacher avatar Aug 08 '23 19:08 daveythacher