DMA Descriptors Lists are reused with dma_descriptors_impl!

[i404788]

Bug description

It seems like dma_descriptors_impl! re-uses the same DESCRIPTORS reference (

esp-hal/esp-hal/src/dma/mod.rs

Line 703 in 2105ba0

static mut DESCRIPTORS: [$crate::dma::DmaDescriptor; COUNT] =

). Disallowing multiple descriptor chains.

It could be that this is intended but I don't see an obvious way to get to multiple descriptor lists.

To Reproduce

Initialize an array of descriptors like so

  let mut buffers = {
    // Create an array of uninitialized values.
    let mut array: [Option<DmaTxBuf>; DMA_SUPERCHUNK_COUNT] =
      [const { None }; DMA_SUPERCHUNK_COUNT];

    for element in array.iter_mut() {
      let (_, tx_desc) =
        esp_hal::dma_descriptors_chunk_size!(0, DMA_SUPERCHUNK_SIZE, DMA_CHUNK_SIZE);
      defmt::warn!("Descriptor head: {}", tx_desc.as_mut_ptr());
      let tx_buffer = dma_alloc_buffer!(DMA_SUPERCHUNK_SIZE, DMA_ALIGNMENT as usize);
      let display_buf = DmaTxBuf::new_with_config(tx_desc, tx_buffer, DMA_ALIGNMENT).unwrap();
      element.replace(display_buf);
    }
    array
  };

And observe that the log shows the same pointer for each descriptor:

WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??
WARN Descriptor head: 0x3fc8c6c4
0x3fc8c6c4 - main::display::display::DmaFrameBuffer::new::DESCRIPTORS
    at ??:??

Expected behavior

Each call to dma_descriptors_chunk_size makes a new descriptor list.

Environment

• Target device: ESP32S3
• Crate name and version: git main

[bugadani]

I don't know if we should count this as a bug in the code, but maybe in the documentation. You are not really supposed to call that macro in a loop - it's a convenience solution for less special use cases. You should probably create one big list of descriptors and manually sort them out into chunks with the size you need, in your loop.

[i404788]

Fair enough, I don't think there is any documentation/examples on using DMA buffer/descriptors without hitting this macro; so multiple calls to dma_buffers! or even a combination of different dma_*! macros may fail silently like above. All the macros say create in the docs so you expect separate instances.

For now I'll try creating them manually to resolve my current roadblock.

[bugadani]

You should be able to use the macro multiple times, but the way statics work in rust mean that you get one static variables per macro use. I.e. if you have dma_descriptors! written once in your source code (it doesn't matter how many times you actually end up calling it), you'll get a single set of DMA descriptors. But the following works, too:

let (rx_descriptors, tx_descriptors) = esp_hal::dma_descriptors!(RX_SIZE, TX_SIZE);
let (other_rx_desc, other_tx_desc) = esp_hal::dma_descriptors!(RX_SIZE, TX_SIZE);

// These are not the same set of descriptors
core::assert_ne!(other_rx_desc.as_ptr(), rx_descriptors.as_ptr());
core::assert_ne!(other_tx_desc.as_ptr(), tx_descriptors.as_ptr());

[Dominaezzz]

As the macro is doing unsafe {} for the user, I think this is a (soundness) bug in the code/macro.
One that is annoying to fix as I don't want to throw a critical section at this sort of thing.
Perhaps a solution with generics might be better, then users can handle the static part themselves

[i404788]

Ah I see, yeah I was confused with how the macros resolve, that does make sense then. It would be nice if it's more natural to use.

const-generics + 'mkstatic' do seem like a potential solution, haven't gotten to fixing my example so maybe I'll do a proposal to add to examples.