qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] hw/dma: sifive_pdma: support high 32-bit access of 64-bi


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH 1/2] hw/dma: sifive_pdma: support high 32-bit access of 64-bit register
Date: Tue, 28 Dec 2021 11:33:59 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0

Hi Jim and Frank,

On 12/28/21 01:52, Jim Shu wrote:
> Real PDMA support high 32-bit read/write memory access of 64-bit
> register.
> 
> The following result is PDMA tested in U-Boot on Unmatched board:
> 
> 1. Real PDMA is allowed high 32-bit read/write to 64-bit register.
> => mw.l 0x3000000 0x0                      <= Disclaim channel 0
> => mw.l 0x3000000 0x1                      <= Claim channel 0
> => mw.l 0x3000010 0x80000000               <= Write low 32-bit NextDest 
> (NextDest = 0x280000000)
> => mw.l 0x3000014 0x2                      <= Write high 32-bit NextDest
> => md.l 0x3000010 1                        <= Dump low 32-bit NextDest
> 03000010: 80000000
> => md.l 0x3000014 1                        <= Dump high 32-bit NextDest
> 03000014: 00000002
> => mw.l 0x3000018 0x80001000               <= Write low 32-bit NextSrc 
> (NextSrc = 0x280001000)
> => mw.l 0x300001c 0x2                      <= Write high 32-bit NextSrc
> => md.l 0x3000018 1                        <= Dump low 32-bit NextSrc
> 03000010: 80001000
> => md.l 0x300001c 1                        <= Dump high 32-bit NextSrc
> 03000014: 00000002
> 
> 2. PDMA transfer from 0x280001000 to 0x280000000 is OK.
> => mw.q 0x3000008 0x4                      <= NextBytes = 4
> => mw.l 0x3000004 0x22000000               <= wsize = rsize = 2 (2^2 = 4 
> bytes)
> => mw.l 0x280000000 0x87654321             <= Fill test data to dst
> => mw.l 0x280001000 0x12345678             <= Fill test data to src
> => md.l 0x280000000 1; md.l 0x280001000 1  <= Dump src/dst memory contents
> 280000000: 87654321                              !Ce.
> 280001000: 12345678                              xV4.
> => md.l 0x3000000 8                        <= Dump PDMA status
> 03000000: 00000001 22000000 00000004 00000000    ......."........
> 03000010: 80000000 00000002 80001000 00000002    ................
> => mw.l 0x3000000 0x3                      <= Set channel 0 run and claim bits
> => md.l 0x3000000 8                        <= Dump PDMA status
> 03000000: 40000001 22000000 00000004 00000000    ...@..."........
> 03000010: 80000000 00000002 80001000 00000002    ................
> => md.l 0x280000000 1; md.l 0x280001000 1  <= Dump src/dst memory contents
> 280000000: 12345678                               xV4.
> 280001000: 12345678                               xV4.
> 
> Signed-off-by: Jim Shu <jim.shu@sifive.com>
> Reviewed-by: Frank Chang <frank.chang@sifive.com>
> ---
>  hw/dma/sifive_pdma.c | 174 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 152 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> index 85fe34f5f3..b8b198ab4e 100644
> --- a/hw/dma/sifive_pdma.c
> +++ b/hw/dma/sifive_pdma.c
> @@ -177,18 +177,44 @@ static inline void 
> sifive_pdma_update_irq(SiFivePDMAState *s, int ch)
>      s->chan[ch].state = DMA_CHAN_STATE_IDLE;
>  }
>  
> -static uint64_t sifive_pdma_read(void *opaque, hwaddr offset, unsigned size)
> +static uint64_t sifive_pdma_readq(SiFivePDMAState *s, int ch, hwaddr offset)
>  {
> -    SiFivePDMAState *s = opaque;
> -    int ch = SIFIVE_PDMA_CHAN_NO(offset);
>      uint64_t val = 0;
>  
> -    if (ch >= SIFIVE_PDMA_CHANS) {
> -        qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid channel no %d\n",
> -                      __func__, ch);
> -        return 0;
> +    offset &= 0xfff;
> +    switch (offset) {
> +    case DMA_NEXT_BYTES:
> +        val = s->chan[ch].next_bytes;
> +        break;
> +    case DMA_NEXT_DST:
> +        val = s->chan[ch].next_dst;
> +        break;
> +    case DMA_NEXT_SRC:
> +        val = s->chan[ch].next_src;
> +        break;
> +    case DMA_EXEC_BYTES:
> +        val = s->chan[ch].exec_bytes;
> +        break;
> +    case DMA_EXEC_DST:
> +        val = s->chan[ch].exec_dst;
> +        break;
> +    case DMA_EXEC_SRC:
> +        val = s->chan[ch].exec_src;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Unexpected 64-bit access to 0x%" HWADDR_PRIX "\n",
> +                      __func__, offset);
> +        break;
>      }
>  
> +    return val;
> +}
> +
> +static uint32_t sifive_pdma_readl(SiFivePDMAState *s, int ch, hwaddr offset)
> +{
> +    uint32_t val = 0;
> +
>      offset &= 0xfff;
>      switch (offset) {
>      case DMA_CONTROL:
> @@ -198,28 +224,47 @@ static uint64_t sifive_pdma_read(void *opaque, hwaddr 
> offset, unsigned size)
>          val = s->chan[ch].next_config;
>          break;
>      case DMA_NEXT_BYTES:
> -        val = s->chan[ch].next_bytes;
> +        val = extract64(s->chan[ch].next_bytes, 0, 32);
> +        break;
> +    case DMA_NEXT_BYTES + 4:
> +        val = extract64(s->chan[ch].next_bytes, 32, 32);
>          break;
>      case DMA_NEXT_DST:
> -        val = s->chan[ch].next_dst;
> +        val = extract64(s->chan[ch].next_dst, 0, 32);
> +        break;
> +    case DMA_NEXT_DST + 4:
> +        val = extract64(s->chan[ch].next_dst, 32, 32);
>          break;
>      case DMA_NEXT_SRC:
> -        val = s->chan[ch].next_src;
> +        val = extract64(s->chan[ch].next_src, 0, 32);
> +        break;
> +    case DMA_NEXT_SRC + 4:
> +        val = extract64(s->chan[ch].next_src, 32, 32);
>          break;
>      case DMA_EXEC_CONFIG:
>          val = s->chan[ch].exec_config;
>          break;
>      case DMA_EXEC_BYTES:
> -        val = s->chan[ch].exec_bytes;
> +        val = extract64(s->chan[ch].exec_bytes, 0, 32);
> +        break;
> +    case DMA_EXEC_BYTES + 4:
> +        val = extract64(s->chan[ch].exec_bytes, 32, 32);
>          break;
>      case DMA_EXEC_DST:
> -        val = s->chan[ch].exec_dst;
> +        val = extract64(s->chan[ch].exec_dst, 0, 32);
> +        break;
> +    case DMA_EXEC_DST + 4:
> +        val = extract64(s->chan[ch].exec_dst, 32, 32);
>          break;
>      case DMA_EXEC_SRC:
> -        val = s->chan[ch].exec_src;
> +        val = extract64(s->chan[ch].exec_src, 0, 32);
> +        break;
> +    case DMA_EXEC_SRC + 4:
> +        val = extract64(s->chan[ch].exec_src, 32, 32);
>          break;
>      default:
> -        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Unexpected 32-bit access to 0x%" HWADDR_PRIX "\n",
>                        __func__, offset);
>          break;
>      }
> @@ -227,19 +272,66 @@ static uint64_t sifive_pdma_read(void *opaque, hwaddr 
> offset, unsigned size)
>      return val;
>  }
>  
> -static void sifive_pdma_write(void *opaque, hwaddr offset,
> -                              uint64_t value, unsigned size)
> +static uint64_t sifive_pdma_read(void *opaque, hwaddr offset, unsigned size)
>  {
>      SiFivePDMAState *s = opaque;
>      int ch = SIFIVE_PDMA_CHAN_NO(offset);
> -    bool claimed, run;
> +    uint64_t val = 0;
>  
>      if (ch >= SIFIVE_PDMA_CHANS) {
>          qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid channel no %d\n",
>                        __func__, ch);
> -        return;
> +        return 0;
> +    }
> +
> +    switch (size) {
> +    case 8:
> +        val = sifive_pdma_readq(s, ch, offset);
> +        break;
> +    case 4:
> +        val = sifive_pdma_readl(s, ch, offset);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid read size %u to PDMA\n",
> +                      __func__, size);
> +        return 0;
>      }
>  
> +    return val;
> +}
> +
> +static void sifive_pdma_writeq(SiFivePDMAState *s, int ch,
> +                               hwaddr offset, uint64_t value)
> +{
> +    offset &= 0xfff;
> +    switch (offset) {
> +    case DMA_NEXT_BYTES:
> +        s->chan[ch].next_bytes = value;
> +        break;
> +    case DMA_NEXT_DST:
> +        s->chan[ch].next_dst = value;
> +        break;
> +    case DMA_NEXT_SRC:
> +        s->chan[ch].next_src = value;
> +        break;
> +    case DMA_EXEC_BYTES:
> +    case DMA_EXEC_DST:
> +    case DMA_EXEC_SRC:
> +        /* these are read-only registers */
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Unexpected 64-bit access to 0x%" HWADDR_PRIX "\n",
> +                      __func__, offset);
> +        break;
> +    }
> +}
> +
> +static void sifive_pdma_writel(SiFivePDMAState *s, int ch,
> +                               hwaddr offset, uint32_t value)
> +{
> +    bool claimed, run;
> +
>      offset &= 0xfff;
>      switch (offset) {
>      case DMA_CONTROL:
> @@ -282,13 +374,24 @@ static void sifive_pdma_write(void *opaque, hwaddr 
> offset,
>          s->chan[ch].next_config = value;
>          break;
>      case DMA_NEXT_BYTES:
> -        s->chan[ch].next_bytes = value;
> +        s->chan[ch].next_bytes =
> +            deposit64(s->chan[ch].next_bytes, 0, 32, value);
> +        break;
> +    case DMA_NEXT_BYTES + 4:
> +        s->chan[ch].next_bytes =
> +            deposit64(s->chan[ch].next_bytes, 32, 32, value);
>          break;
>      case DMA_NEXT_DST:
> -        s->chan[ch].next_dst = value;
> +        s->chan[ch].next_dst = deposit64(s->chan[ch].next_dst, 0, 32, value);
> +        break;
> +    case DMA_NEXT_DST + 4:
> +        s->chan[ch].next_dst = deposit64(s->chan[ch].next_dst, 32, 32, 
> value);
>          break;
>      case DMA_NEXT_SRC:
> -        s->chan[ch].next_src = value;
> +        s->chan[ch].next_src = deposit64(s->chan[ch].next_src, 0, 32, value);
> +        break;
> +    case DMA_NEXT_SRC + 4:
> +        s->chan[ch].next_src = deposit64(s->chan[ch].next_src, 32, 32, 
> value);
>          break;
>      case DMA_EXEC_CONFIG:
>      case DMA_EXEC_BYTES:
> @@ -297,12 +400,39 @@ static void sifive_pdma_write(void *opaque, hwaddr 
> offset,
>          /* these are read-only registers */
>          break;
>      default:
> -        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Unexpected 32-bit access to 0x%" HWADDR_PRIX "\n",
>                        __func__, offset);
>          break;
>      }
>  }
>  
> +static void sifive_pdma_write(void *opaque, hwaddr offset,
> +                              uint64_t value, unsigned size)
> +{
> +    SiFivePDMAState *s = opaque;
> +    int ch = SIFIVE_PDMA_CHAN_NO(offset);
> +
> +    if (ch >= SIFIVE_PDMA_CHANS) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid channel no %d\n",
> +                      __func__, ch);
> +        return;
> +    }
> +
> +    switch (size) {
> +    case 8:
> +        sifive_pdma_writeq(s, ch, offset, value);
> +        break;
> +    case 4:
> +        sifive_pdma_writel(s, ch, offset, (uint32_t) value);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid write size %u to PDMA\n",
> +                      __func__, size);
> +        break;
> +    }
> +}
> +
>  static const MemoryRegionOps sifive_pdma_ops = {
>      .read = sifive_pdma_read,
>      .write = sifive_pdma_write,

Would this work instead?

-- >8 --
diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
index 85fe34f5f31..8f879de6f60 100644
--- a/hw/dma/sifive_pdma.c
+++ b/hw/dma/sifive_pdma.c
@@ -308,10 +308,14 @@ static const MemoryRegionOps sifive_pdma_ops = {
     .write = sifive_pdma_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
     /* there are 32-bit and 64-bit wide registers */
-    .impl = {
+    .valid = {
         .min_access_size = 4,
         .max_access_size = 8,
     }
+    .impl = {
+        .min_access_size = 8,
+        .max_access_size = 8,
+    }
 };

---




reply via email to

[Prev in Thread] Current Thread [Next in Thread]