Re: [Qemu-devel] [PATCH 6/9] pl080: use specific endian ld/st_phys

[Peter Maydell]

On 5 July 2011 17:28, Alexander Graf <address@hidden> wrote:
> --- a/hw/pl080.c
> +++ b/hw/pl080.c
> @@ -199,10 +199,10 @@ again:
>             if (size == 0) {
>                 /* Transfer complete.  */
>                 if (ch->lli) {
> -                    ch->src = ldl_phys(ch->lli);
> -                    ch->dest = ldl_phys(ch->lli + 4);
> -                    ch->ctrl = ldl_phys(ch->lli + 12);
> -                    ch->lli = ldl_phys(ch->lli + 8);
> +                    ch->src = ldl_le_phys(ch->lli);
> +                    ch->dest = ldl_le_phys(ch->lli + 4);
> +                    ch->ctrl = ldl_le_phys(ch->lli + 12);
> +                    ch->lli = ldl_le_phys(ch->lli + 8);
>                 } else {
>                     ch->conf &= ~PL080_CCONF_E;
>                 }

(Mostly this email is to get this info into the archive before I
forget; we talked about it on IRC. It's not intended to be a nak,
which is just as well given the patch is already committed :-))

This is no worse than the existing code, but it's not actually
the right behaviour. Bit 0 of ch->lli indicates which AHB master
we make the request on; you use that to identify which of bits
1 and 2 in the DMACConfiguration Register to test to determine
whether to be little endian or big endian for each master:

  if (ch->conf & (PL080_CONF_M1 << (ch->lli & 1))) {
     bigendian;
  } else {
     littleendian;
  }

However since that function pl080_run() has this early on:
 hw_error("DMA active\n");
we'll never reach this code, so it's all a bit moot :-)

(There are no doubt other bits of that code which would need
fixing to properly support bigendian DMA too, which is a bit
much effort for functionality that isn't used.)

-- PMM