On Thu, 2 Apr 2020 at 14:46, Edgar E. Iglesias <[email protected]> wrote:
>
> From: "Edgar E. Iglesias" <[email protected]>
>
> Reorganize the descriptor handling so that CUR_DSCR always
> points to the next descriptor to be processed.
>
> Signed-off-by: Edgar E. Iglesias <[email protected]>
> ---
This is just moved-code in this patch so I think it's
a separate issue, but it looks like you have an endianness
bug here:
> +static void zdma_update_descr_addr(XlnxZDMA *s, bool type,
> + unsigned int basereg)
> +{
> + uint64_t addr, next;
> +
> + if (type == DTYPE_LINEAR) {
> + addr = zdma_get_regaddr64(s, basereg);
> + next = addr + sizeof(s->dsc_dst);
> + } else {
> + addr = zdma_get_regaddr64(s, basereg);
> + addr += sizeof(s->dsc_dst);
> + address_space_read(s->dma_as, addr, s->attr, (void *) &next, 8);
This reads 8 bytes into the uint64_t 'next', which means
that the value from C's point of view will be different
for big-endian and little-endian hosts. You probably wanted
address_space_ldq_le(), assuming the h/w always does
little-endian reads and that these get_regaddr64 and
put_regaddr64 functions work with host-endian integers.
There's a similar problem elsewhere in the device where
it does this:
address_space_read(s->dma_as, addr, s->attr, buf, sizeof(XlnxZDMADescr));
and assumes the guest structure is the same layout and
endianness as the host struct XlnxDMADescr.
I'm not a huge fan of defining host C structs to match
guest data structures, but if you want to go that way
you ought to (a) be byteswapping the contents appropriately
and (b) have a compile-time assert that the size of the
struct is what you think it is and the compiler hasn't
inserted any helpful extra padding.
thanks
-- PMM
