On Wed, 24 Apr 2024 at 13:53, Alexandra Diupina <adiup...@astralinux.ru> wrote:
>
> Add a type cast and use extract64() instead of extract32()
> to avoid integer overflow on addition. Fix bit fields
> extraction according to documentation.
> Also fix host-endianness bug by swapping desc fields from guest
> memory order to host memory order after dma_memory_read().
Thanks for this patch. Could you split it into two, please?
The error in handling of the descriptor extension fields is a
separate problem from the endianness bug, so they should be
fixed in separate patches.
> @@ -660,6 +660,24 @@ size_t xlnx_dpdma_start_operation(XlnxDPDMAState *s,
> uint8_t channel,
> break;
> }
>
> + /* Convert from LE into host endianness. */
> + desc.control = le32_to_cpu(desc.control);
> + desc.descriptor_id = le32_to_cpu(desc.descriptor_id);
> + desc.xfer_size = le32_to_cpu(desc.xfer_size);
> + desc.line_size_stride = le32_to_cpu(desc.line_size_stride);
> + desc.timestamp_lsb = le32_to_cpu(desc.timestamp_lsb);
> + desc.timestamp_msb = le32_to_cpu(desc.timestamp_msb);
> + desc.address_extension = le32_to_cpu(desc.address_extension);
> + desc.next_descriptor = le32_to_cpu(desc.next_descriptor);
> + desc.source_address = le32_to_cpu(desc.source_address);
> + desc.address_extension_23 = le32_to_cpu(desc.address_extension_23);
> + desc.address_extension_45 = le32_to_cpu(desc.address_extension_45);
> + desc.source_address2 = le32_to_cpu(desc.source_address2);
> + desc.source_address3 = le32_to_cpu(desc.source_address3);
> + desc.source_address4 = le32_to_cpu(desc.source_address4);
> + desc.source_address5 = le32_to_cpu(desc.source_address5);
> + desc.crc = le32_to_cpu(desc.crc);
> +
> xlnx_dpdma_update_desc_info(s, channel, &desc);
>
> #ifdef DEBUG_DPDMA
I would suggest factoring out a function like
static MemTxResult xlnx_dpdma_read_descriptor(XlnxDPDMAState *s,
uint64_t desc_addr,
DPDMADescriptor *desc)
which wraps up "read the descriptor from desc_addr and fill in desc"
as a single operation (by calling dma_memory_read() and then
doing the byteswap).
thanks
-- PMM
