[PATCH v1 0/2] dma/xlnx-zdma: Fix descriptor loading wrt host endianness

[PATCH v1 0/2] dma/xlnx-zdma: Fix descriptor loading wrt host endianness

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Hi,

This fixes the endinannes related bugs with descriptor loading
that Peter pointed out.

Cheers,
Edgar

Edgar E. Iglesias (2):
  dma/xlnx-zdma: Fix descriptor loading (MEM) wrt endianness
  dma/xlnx-zdma: Fix descriptor loading (REG) wrt endianness

 hw/dma/xlnx-zdma.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

-- 
2.20.1

[PATCH v1 1/2] dma/xlnx-zdma: Fix descriptor loading (MEM) wrt endianness

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Fix descriptor loading from memory wrt host endianness.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/dma/xlnx-zdma.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c
index 1c45367f3c..5f4775f663 100644
--- a/hw/dma/xlnx-zdma.c
+++ b/hw/dma/xlnx-zdma.c
@@ -299,19 +299,22 @@ static void zdma_put_regaddr64(XlnxZDMA *s, unsigned int basereg, uint64_t addr)
     s->regs[basereg + 1] = addr >> 32;
 }
 
-static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf)
+static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr,
+                                 XlnxZDMADescr *descr)
 {
     /* ZDMA descriptors must be aligned to their own size.  */
     if (addr % sizeof(XlnxZDMADescr)) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "zdma: unaligned descriptor at %" PRIx64,
                       addr);
-        memset(buf, 0x0, sizeof(XlnxZDMADescr));
+        memset(descr, 0x0, sizeof(XlnxZDMADescr));
         s->error = true;
         return false;
     }
 
-    address_space_read(s->dma_as, addr, s->attr, buf, sizeof(XlnxZDMADescr));
+    descr->addr = address_space_ldq_le(s->dma_as, addr, s->attr, NULL);
+    descr->size = address_space_ldl_le(s->dma_as, addr + 8, s->attr, NULL);
+    descr->attr = address_space_ldl_le(s->dma_as, addr + 12, s->attr, NULL);
     return true;
 }
 
@@ -344,7 +347,7 @@ static void zdma_update_descr_addr(XlnxZDMA *s, bool type,
     } else {
         addr = zdma_get_regaddr64(s, basereg);
         addr += sizeof(s->dsc_dst);
-        address_space_read(s->dma_as, addr, s->attr, (void *) &next, 8);
+        next = address_space_ldq_le(s->dma_as, addr, s->attr, NULL);
     }
 
     zdma_put_regaddr64(s, basereg, next);
-- 
2.20.1

[PATCH v1 2/2] dma/xlnx-zdma: Fix descriptor loading (REG) wrt endianness

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Fix descriptor loading from registers wrt host endianness.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/dma/xlnx-zdma.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c
index 5f4775f663..4121a1b489 100644
--- a/hw/dma/xlnx-zdma.c
+++ b/hw/dma/xlnx-zdma.c
@@ -299,6 +299,14 @@ static void zdma_put_regaddr64(XlnxZDMA *s, unsigned int basereg, uint64_t addr)
     s->regs[basereg + 1] = addr >> 32;
 }
 
+static void zdma_load_descriptor_reg(XlnxZDMA *s, unsigned int reg,
+                                     XlnxZDMADescr *descr)
+{
+    descr->addr = zdma_get_regaddr64(s, reg);
+    descr->size = s->regs[reg + 2];
+    descr->attr = s->regs[reg + 3];
+}
+
 static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr,
                                  XlnxZDMADescr *descr)
 {
@@ -324,8 +332,7 @@ static void zdma_load_src_descriptor(XlnxZDMA *s)
     unsigned int ptype = ARRAY_FIELD_EX32(s->regs, ZDMA_CH_CTRL0, POINT_TYPE);
 
     if (ptype == PT_REG) {
-        memcpy(&s->dsc_src, &s->regs[R_ZDMA_CH_SRC_DSCR_WORD0],
-               sizeof(s->dsc_src));
+        zdma_load_descriptor_reg(s, R_ZDMA_CH_SRC_DSCR_WORD0, &s->dsc_src);
         return;
     }
 
@@ -360,8 +367,7 @@ static void zdma_load_dst_descriptor(XlnxZDMA *s)
     bool dst_type;
 
     if (ptype == PT_REG) {
-        memcpy(&s->dsc_dst, &s->regs[R_ZDMA_CH_DST_DSCR_WORD0],
-               sizeof(s->dsc_dst));
+        zdma_load_descriptor_reg(s, R_ZDMA_CH_DST_DSCR_WORD0, &s->dsc_dst);
         return;
     }
 
-- 
2.20.1

Re: [PATCH v1 2/2] dma/xlnx-zdma: Fix descriptor loading (REG) wrt endianness

[Francisco Iglesias]

On [2020 Apr 04] Sat 14:27:18, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Fix descriptor loading from registers wrt host endianness.
> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
>  hw/dma/xlnx-zdma.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c
> index 5f4775f663..4121a1b489 100644
> --- a/hw/dma/xlnx-zdma.c
> +++ b/hw/dma/xlnx-zdma.c
> @@ -299,6 +299,14 @@ static void zdma_put_regaddr64(XlnxZDMA *s, unsigned int basereg, uint64_t addr)
>      s->regs[basereg + 1] = addr >> 32;
>  }
>  
> +static void zdma_load_descriptor_reg(XlnxZDMA *s, unsigned int reg,
> +                                     XlnxZDMADescr *descr)
> +{
> +    descr->addr = zdma_get_regaddr64(s, reg);
> +    descr->size = s->regs[reg + 2];
> +    descr->attr = s->regs[reg + 3];
> +}
> +
>  static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr,
>                                   XlnxZDMADescr *descr)
>  {
> @@ -324,8 +332,7 @@ static void zdma_load_src_descriptor(XlnxZDMA *s)
>      unsigned int ptype = ARRAY_FIELD_EX32(s->regs, ZDMA_CH_CTRL0, POINT_TYPE);
>  
>      if (ptype == PT_REG) {
> -        memcpy(&s->dsc_src, &s->regs[R_ZDMA_CH_SRC_DSCR_WORD0],
> -               sizeof(s->dsc_src));
> +        zdma_load_descriptor_reg(s, R_ZDMA_CH_SRC_DSCR_WORD0, &s->dsc_src);
>          return;
>      }
>  
> @@ -360,8 +367,7 @@ static void zdma_load_dst_descriptor(XlnxZDMA *s)
>      bool dst_type;
>  
>      if (ptype == PT_REG) {
> -        memcpy(&s->dsc_dst, &s->regs[R_ZDMA_CH_DST_DSCR_WORD0],
> -               sizeof(s->dsc_dst));
> +        zdma_load_descriptor_reg(s, R_ZDMA_CH_DST_DSCR_WORD0, &s->dsc_dst);
>          return;
>      }
>  
> -- 
> 2.20.1
> 

Re: [PATCH v1 1/2] dma/xlnx-zdma: Fix descriptor loading (MEM) wrt endianness

[Francisco Iglesias]

On [2020 Apr 04] Sat 14:27:17, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Fix descriptor loading from memory wrt host endianness.
> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
>  hw/dma/xlnx-zdma.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c
> index 1c45367f3c..5f4775f663 100644
> --- a/hw/dma/xlnx-zdma.c
> +++ b/hw/dma/xlnx-zdma.c
> @@ -299,19 +299,22 @@ static void zdma_put_regaddr64(XlnxZDMA *s, unsigned int basereg, uint64_t addr)
>      s->regs[basereg + 1] = addr >> 32;
>  }
>  
> -static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf)
> +static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr,
> +                                 XlnxZDMADescr *descr)
>  {
>      /* ZDMA descriptors must be aligned to their own size.  */
>      if (addr % sizeof(XlnxZDMADescr)) {
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "zdma: unaligned descriptor at %" PRIx64,
>                        addr);
> -        memset(buf, 0x0, sizeof(XlnxZDMADescr));
> +        memset(descr, 0x0, sizeof(XlnxZDMADescr));
>          s->error = true;
>          return false;
>      }
>  
> -    address_space_read(s->dma_as, addr, s->attr, buf, sizeof(XlnxZDMADescr));
> +    descr->addr = address_space_ldq_le(s->dma_as, addr, s->attr, NULL);
> +    descr->size = address_space_ldl_le(s->dma_as, addr + 8, s->attr, NULL);
> +    descr->attr = address_space_ldl_le(s->dma_as, addr + 12, s->attr, NULL);
>      return true;
>  }
>  
> @@ -344,7 +347,7 @@ static void zdma_update_descr_addr(XlnxZDMA *s, bool type,
>      } else {
>          addr = zdma_get_regaddr64(s, basereg);
>          addr += sizeof(s->dsc_dst);
> -        address_space_read(s->dma_as, addr, s->attr, (void *) &next, 8);
> +        next = address_space_ldq_le(s->dma_as, addr, s->attr, NULL);
>      }
>  
>      zdma_put_regaddr64(s, basereg, next);
> -- 
> 2.20.1
> 

Re: [PATCH v1 1/2] dma/xlnx-zdma: Fix descriptor loading (MEM) wrt endianness

[Philippe Mathieu-Daudé]

On 4/4/20 2:27 PM, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Fix descriptor loading from memory wrt host endianness.
> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>   hw/dma/xlnx-zdma.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c
> index 1c45367f3c..5f4775f663 100644
> --- a/hw/dma/xlnx-zdma.c
> +++ b/hw/dma/xlnx-zdma.c
> @@ -299,19 +299,22 @@ static void zdma_put_regaddr64(XlnxZDMA *s, unsigned int basereg, uint64_t addr)
>       s->regs[basereg + 1] = addr >> 32;
>   }
>   
> -static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf)
> +static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr,
> +                                 XlnxZDMADescr *descr)
>   {
>       /* ZDMA descriptors must be aligned to their own size.  */
>       if (addr % sizeof(XlnxZDMADescr)) {
>           qemu_log_mask(LOG_GUEST_ERROR,
>                         "zdma: unaligned descriptor at %" PRIx64,
>                         addr);
> -        memset(buf, 0x0, sizeof(XlnxZDMADescr));
> +        memset(descr, 0x0, sizeof(XlnxZDMADescr));
>           s->error = true;
>           return false;
>       }
>   
> -    address_space_read(s->dma_as, addr, s->attr, buf, sizeof(XlnxZDMADescr));
> +    descr->addr = address_space_ldq_le(s->dma_as, addr, s->attr, NULL);
> +    descr->size = address_space_ldl_le(s->dma_as, addr + 8, s->attr, NULL);
> +    descr->attr = address_space_ldl_le(s->dma_as, addr + 12, s->attr, NULL);

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>       return true;
>   }
>   
> @@ -344,7 +347,7 @@ static void zdma_update_descr_addr(XlnxZDMA *s, bool type,
>       } else {
>           addr = zdma_get_regaddr64(s, basereg);
>           addr += sizeof(s->dsc_dst);
> -        address_space_read(s->dma_as, addr, s->attr, (void *) &next, 8);
> +        next = address_space_ldq_le(s->dma_as, addr, s->attr, NULL);
>       }
>   
>       zdma_put_regaddr64(s, basereg, next);
> 

Re: [PATCH v1 0/2] dma/xlnx-zdma: Fix descriptor loading wrt host endianness

[Peter Maydell]

On Sat, 4 Apr 2020 at 13:26, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
>
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Hi,
>
> This fixes the endinannes related bugs with descriptor loading
> that Peter pointed out.
>
> Cheers,
> Edgar
>
> Edgar E. Iglesias (2):
>   dma/xlnx-zdma: Fix descriptor loading (MEM) wrt endianness
>   dma/xlnx-zdma: Fix descriptor loading (REG) wrt endianness



Applied to target-arm.next for 5.1, thanks.

-- PMM