[PATCH 0/3] hw: More dma_memory_read/write() API cleanup

[PATCH 0/3] hw: More dma_memory_read/write() API cleanup

[Philippe Mathieu-Daudé]

Following up "global exec/memory/dma APIs cleanup"
https://www.mail-archive.com/qemu-devel@nongnu.org/msg681475.html

Few more cleanups in PCNET & SCSI ESP devices.

Philippe Mathieu-Daudé (3):
  hw/dma/sparc32_dma: Make espdma_memory_[read/write] static
  hw/scsi/esp: Let ESPDMAMemoryReadWriteFunc take void pointer and
    size_t
  hw/net/pcnet: Let phys_mem_read/write take void pointer and boolean

 hw/net/pcnet.h                 |  4 ++--
 include/hw/scsi/esp.h          |  2 +-
 include/hw/sparc/sparc32_dma.h |  6 ++----
 hw/dma/sparc32_dma.c           |  8 ++++----
 hw/net/pcnet-pci.c             |  5 +++--
 hw/net/pcnet.c                 | 16 ++++++++--------
 hw/scsi/esp-pci.c              |  6 +++---
 7 files changed, 23 insertions(+), 24 deletions(-)

-- 
2.21.1

[PATCH 1/3] hw/dma/sparc32_dma: Make espdma_memory_[read/write] static

[Philippe Mathieu-Daudé]

No code use the espdma_memory_[read/write] functions outside
of hw/dma/sparc32_dma.c, make them static.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/sparc/sparc32_dma.h | 2 --
 hw/dma/sparc32_dma.c           | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
index ab42c5421b..b3811b617d 100644
--- a/include/hw/sparc/sparc32_dma.h
+++ b/include/hw/sparc/sparc32_dma.h
@@ -61,7 +61,5 @@ void ledma_memory_read(void *opaque, hwaddr addr,
                        uint8_t *buf, int len, int do_bswap);
 void ledma_memory_write(void *opaque, hwaddr addr,
                         uint8_t *buf, int len, int do_bswap);
-void espdma_memory_read(void *opaque, uint8_t *buf, int len);
-void espdma_memory_write(void *opaque, uint8_t *buf, int len);
 
 #endif
diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index 3e4da0c47f..c9e313938c 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -142,7 +142,7 @@ static void dma_set_irq(void *opaque, int irq, int level)
     }
 }
 
-void espdma_memory_read(void *opaque, uint8_t *buf, int len)
+static void espdma_memory_read(void *opaque, uint8_t *buf, int len)
 {
     DMADeviceState *s = opaque;
     IOMMUState *is = (IOMMUState *)s->iommu;
@@ -152,7 +152,7 @@ void espdma_memory_read(void *opaque, uint8_t *buf, int len)
     s->dmaregs[1] += len;
 }
 
-void espdma_memory_write(void *opaque, uint8_t *buf, int len)
+static void espdma_memory_write(void *opaque, uint8_t *buf, int len)
 {
     DMADeviceState *s = opaque;
     IOMMUState *is = (IOMMUState *)s->iommu;
-- 
2.21.1

[PATCH 2/3] hw/scsi/esp: Let ESPDMAMemoryReadWriteFunc take void pointer and size_t

[Philippe Mathieu-Daudé]

The ESPDMAMemoryReadWriteFunc handlers end calling
dma_memory_read/write, which both take void pointers (since
introduced in commit d86a77f8). The ESPDMAMemoryReadWriteFunc
handlers can take a void pointer too.

All calls to these handlers in hw/scsi/esp.c use an unsigned
length. Use size_t type.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/scsi/esp.h | 2 +-
 hw/dma/sparc32_dma.c  | 4 ++--
 hw/scsi/esp-pci.c     | 6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 6ba47dac41..fff0ebc4d4 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -6,7 +6,7 @@
 
 /* esp.c */
 #define ESP_MAX_DEVS 7
-typedef void (*ESPDMAMemoryReadWriteFunc)(void *opaque, uint8_t *buf, int len);
+typedef void (*ESPDMAMemoryReadWriteFunc)(void *opaque, void *buf, size_t len);
 
 #define ESP_REGS 16
 #define TI_BUFSZ 16
diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index c9e313938c..fbe58b8fea 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -142,7 +142,7 @@ static void dma_set_irq(void *opaque, int irq, int level)
     }
 }
 
-static void espdma_memory_read(void *opaque, uint8_t *buf, int len)
+static void espdma_memory_read(void *opaque, void *buf, size_t len)
 {
     DMADeviceState *s = opaque;
     IOMMUState *is = (IOMMUState *)s->iommu;
@@ -152,7 +152,7 @@ static void espdma_memory_read(void *opaque, uint8_t *buf, int len)
     s->dmaregs[1] += len;
 }
 
-static void espdma_memory_write(void *opaque, uint8_t *buf, int len)
+static void espdma_memory_write(void *opaque, void *buf, size_t len)
 {
     DMADeviceState *s = opaque;
     IOMMUState *is = (IOMMUState *)s->iommu;
diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index d5a1f9e017..8f11c4a2d2 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -242,7 +242,7 @@ static uint64_t esp_pci_io_read(void *opaque, hwaddr addr,
     return ret;
 }
 
-static void esp_pci_dma_memory_rw(PCIESPState *pci, uint8_t *buf, int len,
+static void esp_pci_dma_memory_rw(PCIESPState *pci, void *buf, size_t len,
                                   DMADirection dir)
 {
     dma_addr_t addr;
@@ -278,13 +278,13 @@ static void esp_pci_dma_memory_rw(PCIESPState *pci, uint8_t *buf, int len,
     }
 }
 
-static void esp_pci_dma_memory_read(void *opaque, uint8_t *buf, int len)
+static void esp_pci_dma_memory_read(void *opaque, void *buf, size_t len)
 {
     PCIESPState *pci = opaque;
     esp_pci_dma_memory_rw(pci, buf, len, DMA_DIRECTION_TO_DEVICE);
 }
 
-static void esp_pci_dma_memory_write(void *opaque, uint8_t *buf, int len)
+static void esp_pci_dma_memory_write(void *opaque, void *buf, size_t len)
 {
     PCIESPState *pci = opaque;
     esp_pci_dma_memory_rw(pci, buf, len, DMA_DIRECTION_FROM_DEVICE);
-- 
2.21.1

[PATCH 3/3] hw/net/pcnet: Let phys_mem_read/write take void pointer and boolean

[Philippe Mathieu-Daudé]

Both PCNetState::phys_mem_read/write handlers end calling
dma_memory_read/write, which both take void pointers (since
introduced in commit d86a77f8). The PCNetState phys_mem_read/write
handlers can take a void pointer too.
As the length is always a sizeof(), use a size_t type.
The 'do_bswap' is used as a boolean argument. Use an explicit
boolean type.

The changes in hw/net/pcnet.c are produced using the following
Coccinelle script:

    @@
    expression E1, E2, E3, E4;
    @@
    (
    - s->phys_mem_read(E1, E2, (void *)(E3), E4, 0);
    + s->phys_mem_read(E1, E2, E3, E4, false);
    |
    - s->phys_mem_write(E1, E2, (void *)(E3), E4, 0);
    + s->phys_mem_write(E1, E2, E3, E4, false);
    |
    - s->phys_mem_read(E1, E2, (void *)(E3), E4, 1);
    + s->phys_mem_read(E1, E2, E3, E4, true);
    |
    - s->phys_mem_write(E1, E2, (void *)(E3), E4, 1);
    + s->phys_mem_write(E1, E2, E3, E4, true);
    )

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/net/pcnet.h                 |  4 ++--
 include/hw/sparc/sparc32_dma.h |  4 ++--
 hw/dma/sparc32_dma.c           |  4 ++--
 hw/net/pcnet-pci.c             |  5 +++--
 hw/net/pcnet.c                 | 16 ++++++++--------
 5 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/hw/net/pcnet.h b/hw/net/pcnet.h
index f49b213c57..3f15e6f065 100644
--- a/hw/net/pcnet.h
+++ b/hw/net/pcnet.h
@@ -47,9 +47,9 @@ struct PCNetState_st {
     uint8_t buffer[4096];
     qemu_irq irq;
     void (*phys_mem_read)(void *dma_opaque, hwaddr addr,
-                         uint8_t *buf, int len, int do_bswap);
+                          void *buf, size_t len, bool do_bswap);
     void (*phys_mem_write)(void *dma_opaque, hwaddr addr,
-                          uint8_t *buf, int len, int do_bswap);
+                           const void *buf, size_t len, bool do_bswap);
     DeviceState *dma_opaque;
     int tx_busy;
     int looptest;
diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
index b3811b617d..db42f7440d 100644
--- a/include/hw/sparc/sparc32_dma.h
+++ b/include/hw/sparc/sparc32_dma.h
@@ -58,8 +58,8 @@ typedef struct SPARC32DMAState {
 
 /* sparc32_dma.c */
 void ledma_memory_read(void *opaque, hwaddr addr,
-                       uint8_t *buf, int len, int do_bswap);
+                       void *buf, size_t len, bool do_bswap);
 void ledma_memory_write(void *opaque, hwaddr addr,
-                        uint8_t *buf, int len, int do_bswap);
+                        const void *buf, size_t len, bool do_bswap);
 
 #endif
diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index fbe58b8fea..629f9830f7 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -72,7 +72,7 @@ enum {
 
 /* Note: on sparc, the lance 16 bit bus is swapped */
 void ledma_memory_read(void *opaque, hwaddr addr,
-                       uint8_t *buf, int len, int do_bswap)
+                       void *buf, size_t len, bool do_bswap)
 {
     DMADeviceState *s = opaque;
     IOMMUState *is = (IOMMUState *)s->iommu;
@@ -93,7 +93,7 @@ void ledma_memory_read(void *opaque, hwaddr addr,
 }
 
 void ledma_memory_write(void *opaque, hwaddr addr,
-                        uint8_t *buf, int len, int do_bswap)
+                        const void *buf, size_t len, bool do_bswap)
 {
     DMADeviceState *s = opaque;
     IOMMUState *is = (IOMMUState *)s->iommu;
diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
index d1f31e0272..4e4d60b046 100644
--- a/hw/net/pcnet-pci.c
+++ b/hw/net/pcnet-pci.c
@@ -167,13 +167,14 @@ static const MemoryRegionOps pcnet_mmio_ops = {
 };
 
 static void pci_physical_memory_write(void *dma_opaque, hwaddr addr,
-                                      uint8_t *buf, int len, int do_bswap)
+                                      const void *buf, size_t len,
+                                      bool do_bswap)
 {
     pci_dma_write(dma_opaque, addr, buf, len);
 }
 
 static void pci_physical_memory_read(void *dma_opaque, hwaddr addr,
-                                     uint8_t *buf, int len, int do_bswap)
+                                     void *buf, size_t len, bool do_bswap)
 {
     pci_dma_read(dma_opaque, addr, buf, len);
 }
diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index f3f18d8598..d669ca2aa3 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -305,14 +305,14 @@ static inline void pcnet_tmd_load(PCNetState *s, struct pcnet_TMD *tmd,
             int16_t length;
             int16_t status;
         } xda;
-        s->phys_mem_read(s->dma_opaque, addr, (void *)&xda, sizeof(xda), 0);
+        s->phys_mem_read(s->dma_opaque, addr, &xda, sizeof(xda), false);
         tmd->tbadr = le32_to_cpu(xda.tbadr) & 0xffffff;
         tmd->length = le16_to_cpu(xda.length);
         tmd->status = (le32_to_cpu(xda.tbadr) >> 16) & 0xff00;
         tmd->misc = le16_to_cpu(xda.status) << 16;
         tmd->res = 0;
     } else {
-        s->phys_mem_read(s->dma_opaque, addr, (void *)tmd, sizeof(*tmd), 0);
+        s->phys_mem_read(s->dma_opaque, addr, tmd, sizeof(*tmd), false);
         le32_to_cpus(&tmd->tbadr);
         le16_to_cpus((uint16_t *)&tmd->length);
         le16_to_cpus((uint16_t *)&tmd->status);
@@ -339,7 +339,7 @@ static inline void pcnet_tmd_store(PCNetState *s, const struct pcnet_TMD *tmd,
                                 ((tmd->status & 0xff00) << 16));
         xda.length = cpu_to_le16(tmd->length);
         xda.status = cpu_to_le16(tmd->misc >> 16);
-        s->phys_mem_write(s->dma_opaque, addr, (void *)&xda, sizeof(xda), 0);
+        s->phys_mem_write(s->dma_opaque, addr, &xda, sizeof(xda), false);
     } else {
         struct {
             uint32_t tbadr;
@@ -358,7 +358,7 @@ static inline void pcnet_tmd_store(PCNetState *s, const struct pcnet_TMD *tmd,
             xda.tbadr = xda.misc;
             xda.misc = tmp;
         }
-        s->phys_mem_write(s->dma_opaque, addr, (void *)&xda, sizeof(xda), 0);
+        s->phys_mem_write(s->dma_opaque, addr, &xda, sizeof(xda), false);
     }
 }
 
@@ -371,14 +371,14 @@ static inline void pcnet_rmd_load(PCNetState *s, struct pcnet_RMD *rmd,
             int16_t buf_length;
             int16_t msg_length;
 	} rda;
-        s->phys_mem_read(s->dma_opaque, addr, (void *)&rda, sizeof(rda), 0);
+        s->phys_mem_read(s->dma_opaque, addr, &rda, sizeof(rda), false);
         rmd->rbadr = le32_to_cpu(rda.rbadr) & 0xffffff;
         rmd->buf_length = le16_to_cpu(rda.buf_length);
         rmd->status = (le32_to_cpu(rda.rbadr) >> 16) & 0xff00;
         rmd->msg_length = le16_to_cpu(rda.msg_length);
         rmd->res = 0;
     } else {
-        s->phys_mem_read(s->dma_opaque, addr, (void *)rmd, sizeof(*rmd), 0);
+        s->phys_mem_read(s->dma_opaque, addr, rmd, sizeof(*rmd), false);
         le32_to_cpus(&rmd->rbadr);
         le16_to_cpus((uint16_t *)&rmd->buf_length);
         le16_to_cpus((uint16_t *)&rmd->status);
@@ -405,7 +405,7 @@ static inline void pcnet_rmd_store(PCNetState *s, struct pcnet_RMD *rmd,
                                 ((rmd->status & 0xff00) << 16));
         rda.buf_length = cpu_to_le16(rmd->buf_length);
         rda.msg_length = cpu_to_le16(rmd->msg_length);
-        s->phys_mem_write(s->dma_opaque, addr, (void *)&rda, sizeof(rda), 0);
+        s->phys_mem_write(s->dma_opaque, addr, &rda, sizeof(rda), false);
     } else {
         struct {
             uint32_t rbadr;
@@ -424,7 +424,7 @@ static inline void pcnet_rmd_store(PCNetState *s, struct pcnet_RMD *rmd,
             rda.rbadr = rda.msg_length;
             rda.msg_length = tmp;
         }
-        s->phys_mem_write(s->dma_opaque, addr, (void *)&rda, sizeof(rda), 0);
+        s->phys_mem_write(s->dma_opaque, addr, &rda, sizeof(rda), false);
     }
 }
 
-- 
2.21.1

Re: [PATCH 1/3] hw/dma/sparc32_dma: Make espdma_memory_[read/write] static

[Peter Maydell]

On Fri, 21 Feb 2020 at 13:25, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> No code use the espdma_memory_[read/write] functions outside
> of hw/dma/sparc32_dma.c, make them static.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 0/3] hw: More dma_memory_read/write() API cleanup

[Paolo Bonzini]

On 21/02/20 14:25, Philippe Mathieu-Daudé wrote:
> Following up "global exec/memory/dma APIs cleanup"
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg681475.html
> 
> Few more cleanups in PCNET & SCSI ESP devices.

Can you send a pull request for everything?

Paolo

Re: [PATCH 0/3] hw: More dma_memory_read/write() API cleanup

[Mark Cave-Ayland]

On 21/02/2020 13:25, Philippe Mathieu-Daudé wrote:

> Following up "global exec/memory/dma APIs cleanup"
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg681475.html
> 
> Few more cleanups in PCNET & SCSI ESP devices.
> 
> Philippe Mathieu-Daudé (3):
>   hw/dma/sparc32_dma: Make espdma_memory_[read/write] static
>   hw/scsi/esp: Let ESPDMAMemoryReadWriteFunc take void pointer and
>     size_t
>   hw/net/pcnet: Let phys_mem_read/write take void pointer and boolean
> 
>  hw/net/pcnet.h                 |  4 ++--
>  include/hw/scsi/esp.h          |  2 +-
>  include/hw/sparc/sparc32_dma.h |  6 ++----
>  hw/dma/sparc32_dma.c           |  8 ++++----
>  hw/net/pcnet-pci.c             |  5 +++--
>  hw/net/pcnet.c                 | 16 ++++++++--------
>  hw/scsi/esp-pci.c              |  6 +++---
>  7 files changed, 23 insertions(+), 24 deletions(-)

LGTM.

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.