qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] hw/dma: Introduce dma_size_t type definition
@ 2021-12-31 11:48 Philippe Mathieu-Daudé
  2021-12-31 11:48 ` [PATCH 1/8] hw/nvram: Restrict stub to sysemu and tools Philippe Mathieu-Daudé
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-12-31 11:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, David Hildenbrand, Peter Xu, Gerd Hoffmann,
	Paolo Bonzini, Philippe Mathieu-Daudé

Hi,

This series aims to clarify a bit the DMA API, in particular the
'size' argument which is not clear enough (as we use multiple types
for it). The clarification is trivial: add a dma_size_t typedef,
similar to the dma_addr_t one. This helps avoiding build failures
on 32-bit host [*] (and likely overflows calculation too IMO).

Some units using the DMA API are first removed from user-mode
emulation to avoid build failure (they shouldn't be there in
the first place).

Then some variables are renamed for clarity (no functional change).

Finally the dma_size_t typedef is added, and the previous patch
which failed on 32-bit host applied on top (not failing anymore).

Regards,

Phil.

[*] https://www.mail-archive.com/qemu-devel@nongnu.org/msg858825.html

Philippe Mathieu-Daudé (8):
  hw/nvram: Restrict stub to sysemu and tools
  hw/pci: Restrict pci-bus stub to sysemu
  hw/pci: Document pci_dma_map()
  hw/dma: Remove CONFIG_USER_ONLY check
  hw/rdma/rdma_utils: Rename rdma_pci_dma_map 'len' argument
  hw/scsi: Rename SCSIRequest::resid as 'residual'
  hw/dma: Introduce dma_size_t type definition
  hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResult

 hw/rdma/rdma_utils.h      |  4 +-
 include/hw/pci/pci.h      | 22 +++++++---
 include/hw/scsi/scsi.h    |  4 +-
 include/sysemu/dma.h      | 59 ++++++++++++++-------------
 hw/ide/ahci.c             | 10 ++---
 hw/nvme/ctrl.c            |  6 +--
 hw/nvram/fw_cfg.c         |  2 +-
 hw/rdma/rdma_utils.c      | 16 ++++----
 hw/rdma/vmw/pvrdma_main.c |  2 +-
 hw/scsi/lsi53c895a.c      |  4 +-
 hw/scsi/megasas.c         | 85 +++++++++++++++++++++++++++------------
 hw/scsi/scsi-bus.c        | 12 +++---
 hw/scsi/scsi-disk.c       |  4 +-
 hw/usb/libhw.c            |  4 +-
 softmmu/dma-helpers.c     | 38 ++++++++---------
 hw/nvram/meson.build      |  6 ++-
 hw/rdma/trace-events      |  2 +-
 stubs/meson.build         |  4 +-
 18 files changed, 164 insertions(+), 120 deletions(-)

-- 
2.33.1




^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/8] hw/nvram: Restrict stub to sysemu and tools
  2021-12-31 11:48 [PATCH 0/8] hw/dma: Introduce dma_size_t type definition Philippe Mathieu-Daudé
@ 2021-12-31 11:48 ` Philippe Mathieu-Daudé
  2021-12-31 11:48 ` [PATCH 2/8] hw/pci: Restrict pci-bus stub to sysemu Philippe Mathieu-Daudé
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-12-31 11:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, David Hildenbrand, Peter Xu, Gerd Hoffmann,
	Paolo Bonzini, Philippe Mathieu-Daudé

User-mode emulation doesn't need the fw_cfg stub.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/nvram/meson.build | 6 ++++--
 stubs/meson.build    | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/nvram/meson.build b/hw/nvram/meson.build
index 202a5466e63..f5ee9f6b88c 100644
--- a/hw/nvram/meson.build
+++ b/hw/nvram/meson.build
@@ -1,5 +1,7 @@
-# QOM interfaces must be available anytime QOM is used.
-qom_ss.add(files('fw_cfg-interface.c'))
+if have_system or have_tools
+  # QOM interfaces must be available anytime QOM is used.
+  qom_ss.add(files('fw_cfg-interface.c'))
+endif
 
 softmmu_ss.add(files('fw_cfg.c'))
 softmmu_ss.add(when: 'CONFIG_CHRP_NVRAM', if_true: files('chrp_nvram.c'))
diff --git a/stubs/meson.build b/stubs/meson.build
index 71469c1d50a..363f6fa785d 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -11,7 +11,6 @@
 stub_ss.add(files('dump.c'))
 stub_ss.add(files('error-printf.c'))
 stub_ss.add(files('fdset.c'))
-stub_ss.add(files('fw_cfg.c'))
 stub_ss.add(files('gdbstub.c'))
 stub_ss.add(files('get-vm-name.c'))
 if linux_io_uring.found()
@@ -51,6 +50,7 @@
   stub_ss.add(files('replay-tools.c'))
 endif
 if have_system
+  stub_ss.add(files('fw_cfg.c'))
   stub_ss.add(files('semihost.c'))
   stub_ss.add(files('usb-dev-stub.c'))
   stub_ss.add(files('xen-hw-stub.c'))
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/8] hw/pci: Restrict pci-bus stub to sysemu
  2021-12-31 11:48 [PATCH 0/8] hw/dma: Introduce dma_size_t type definition Philippe Mathieu-Daudé
  2021-12-31 11:48 ` [PATCH 1/8] hw/nvram: Restrict stub to sysemu and tools Philippe Mathieu-Daudé
@ 2021-12-31 11:48 ` Philippe Mathieu-Daudé
  2021-12-31 11:48 ` [PATCH 3/8] hw/pci: Document pci_dma_map() Philippe Mathieu-Daudé
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-12-31 11:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, David Hildenbrand, Peter Xu, Gerd Hoffmann,
	Paolo Bonzini, Philippe Mathieu-Daudé

Neither tools nor user-mode emulation require the PCI bus stub.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 stubs/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/stubs/meson.build b/stubs/meson.build
index 363f6fa785d..d359cbe1ad7 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -26,7 +26,6 @@
 stub_ss.add(files('module-opts.c'))
 stub_ss.add(files('monitor.c'))
 stub_ss.add(files('monitor-core.c'))
-stub_ss.add(files('pci-bus.c'))
 stub_ss.add(files('qemu-timer-notify-cb.c'))
 stub_ss.add(files('qmp_memory_device.c'))
 stub_ss.add(files('qmp-command-available.c'))
@@ -51,6 +50,7 @@
 endif
 if have_system
   stub_ss.add(files('fw_cfg.c'))
+  stub_ss.add(files('pci-bus.c'))
   stub_ss.add(files('semihost.c'))
   stub_ss.add(files('usb-dev-stub.c'))
   stub_ss.add(files('xen-hw-stub.c'))
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/8] hw/pci: Document pci_dma_map()
  2021-12-31 11:48 [PATCH 0/8] hw/dma: Introduce dma_size_t type definition Philippe Mathieu-Daudé
  2021-12-31 11:48 ` [PATCH 1/8] hw/nvram: Restrict stub to sysemu and tools Philippe Mathieu-Daudé
  2021-12-31 11:48 ` [PATCH 2/8] hw/pci: Restrict pci-bus stub to sysemu Philippe Mathieu-Daudé
@ 2021-12-31 11:48 ` Philippe Mathieu-Daudé
  2022-01-04  7:25   ` Peter Xu
  2021-12-31 11:48 ` [PATCH 4/8] hw/dma: Remove CONFIG_USER_ONLY check Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-12-31 11:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, David Hildenbrand, Peter Xu, Gerd Hoffmann,
	Paolo Bonzini, Philippe Mathieu-Daudé

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/pci/pci.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 5b36334a28a..f0d707efb8c 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -876,6 +876,18 @@ PCI_DMA_DEFINE_LDST(q_be, q_be, 64);
 
 #undef PCI_DMA_DEFINE_LDST
 
+/**
+ * pci_dma_map: Map a physical memory region into a device PCI address space.
+ *
+ * May map a subset of the requested range, given by and returned in @plen.
+ * May return %NULL and set *@plen to zero(0), if resources needed to perform
+ * the mapping are exhausted.
+ *
+ * @dev: #PCIDevice to be accessed
+ * @addr: address within that device's address space
+ * @plen: pointer to length of buffer; updated on return
+ * @dir: indicates the transfer direction
+ */
 static inline void *pci_dma_map(PCIDevice *dev, dma_addr_t addr,
                                 dma_addr_t *plen, DMADirection dir)
 {
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/8] hw/dma: Remove CONFIG_USER_ONLY check
  2021-12-31 11:48 [PATCH 0/8] hw/dma: Introduce dma_size_t type definition Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2021-12-31 11:48 ` [PATCH 3/8] hw/pci: Document pci_dma_map() Philippe Mathieu-Daudé
@ 2021-12-31 11:48 ` Philippe Mathieu-Daudé
  2021-12-31 11:48 ` [PATCH 5/8] hw/rdma/rdma_utils: Rename rdma_pci_dma_map 'len' argument Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-12-31 11:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, David Hildenbrand, Peter Xu, Gerd Hoffmann,
	Paolo Bonzini, Philippe Mathieu-Daudé

DMA API should not be included in user-mode emulation.
If so, build should fail. Remove the CONFIG_USER_ONLY check.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/sysemu/dma.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
index b3faef41b2f..0db2478a506 100644
--- a/include/sysemu/dma.h
+++ b/include/sysemu/dma.h
@@ -31,8 +31,6 @@ struct QEMUSGList {
     AddressSpace *as;
 };
 
-#ifndef CONFIG_USER_ONLY
-
 /*
  * When an IOMMU is present, bus addresses become distinct from
  * CPU/memory physical addresses and may be a different size.  Because
@@ -288,7 +286,6 @@ void qemu_sglist_init(QEMUSGList *qsg, DeviceState *dev, int alloc_hint,
                       AddressSpace *as);
 void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, dma_addr_t len);
 void qemu_sglist_destroy(QEMUSGList *qsg);
-#endif
 
 typedef BlockAIOCB *DMAIOFunc(int64_t offset, QEMUIOVector *iov,
                               BlockCompletionFunc *cb, void *cb_opaque,
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/8] hw/rdma/rdma_utils: Rename rdma_pci_dma_map 'len' argument
  2021-12-31 11:48 [PATCH 0/8] hw/dma: Introduce dma_size_t type definition Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2021-12-31 11:48 ` [PATCH 4/8] hw/dma: Remove CONFIG_USER_ONLY check Philippe Mathieu-Daudé
@ 2021-12-31 11:48 ` Philippe Mathieu-Daudé
  2022-01-18 10:13   ` Yuval Shaia
  2021-12-31 11:48 ` [PATCH 6/8] hw/scsi: Rename SCSIRequest::resid as 'residual' Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-12-31 11:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, David Hildenbrand, Peter Xu, Gerd Hoffmann,
	Paolo Bonzini, Philippe Mathieu-Daudé

Various APIs use 'pval' naming for 'pointer to val'.
rdma_pci_dma_map() uses 'plen' for 'PCI length', but since
'PCI' is already explicit in the function name, simplify
and rename the argument 'len'. No logical change.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/rdma/rdma_utils.h |  2 +-
 hw/rdma/rdma_utils.c | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/rdma/rdma_utils.h b/hw/rdma/rdma_utils.h
index 9fd0efd940b..0c6414e7e0a 100644
--- a/hw/rdma/rdma_utils.h
+++ b/hw/rdma/rdma_utils.h
@@ -38,7 +38,7 @@ typedef struct RdmaProtectedGSList {
     GSList *list;
 } RdmaProtectedGSList;
 
-void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t plen);
+void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t len);
 void rdma_pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len);
 void rdma_protected_gqueue_init(RdmaProtectedGQueue *list);
 void rdma_protected_gqueue_destroy(RdmaProtectedGQueue *list);
diff --git a/hw/rdma/rdma_utils.c b/hw/rdma/rdma_utils.c
index 98df58f6897..61cb8ede0fd 100644
--- a/hw/rdma/rdma_utils.c
+++ b/hw/rdma/rdma_utils.c
@@ -17,29 +17,29 @@
 #include "trace.h"
 #include "rdma_utils.h"
 
-void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t plen)
+void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t len)
 {
     void *p;
-    hwaddr len = plen;
+    hwaddr pci_len = len;
 
     if (!addr) {
         rdma_error_report("addr is NULL");
         return NULL;
     }
 
-    p = pci_dma_map(dev, addr, &len, DMA_DIRECTION_TO_DEVICE);
+    p = pci_dma_map(dev, addr, &pci_len, DMA_DIRECTION_TO_DEVICE);
     if (!p) {
         rdma_error_report("pci_dma_map fail, addr=0x%"PRIx64", len=%"PRId64,
-                          addr, len);
+                          addr, pci_len);
         return NULL;
     }
 
-    if (len != plen) {
-        rdma_pci_dma_unmap(dev, p, len);
+    if (pci_len != len) {
+        rdma_pci_dma_unmap(dev, p, pci_len);
         return NULL;
     }
 
-    trace_rdma_pci_dma_map(addr, p, len);
+    trace_rdma_pci_dma_map(addr, p, pci_len);
 
     return p;
 }
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/8] hw/scsi: Rename SCSIRequest::resid as 'residual'
  2021-12-31 11:48 [PATCH 0/8] hw/dma: Introduce dma_size_t type definition Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2021-12-31 11:48 ` [PATCH 5/8] hw/rdma/rdma_utils: Rename rdma_pci_dma_map 'len' argument Philippe Mathieu-Daudé
@ 2021-12-31 11:48 ` Philippe Mathieu-Daudé
  2021-12-31 11:49 ` [PATCH 7/8] hw/dma: Introduce dma_size_t type definition Philippe Mathieu-Daudé
  2021-12-31 11:49 ` [PATCH 8/8] hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResult Philippe Mathieu-Daudé
  7 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-12-31 11:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, David Hildenbrand, Peter Xu, Gerd Hoffmann,
	Paolo Bonzini, Philippe Mathieu-Daudé

The 'resid' field is slightly confusing and could be
interpreted as some ID. Rename it as 'residual' which
is clearer to review. No logical change.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/scsi/scsi.h |  4 ++--
 hw/scsi/megasas.c      | 42 +++++++++++++++++++++++++-----------------
 hw/scsi/scsi-bus.c     | 10 +++++-----
 hw/scsi/scsi-disk.c    |  4 ++--
 softmmu/dma-helpers.c  | 26 +++++++++++++-------------
 5 files changed, 47 insertions(+), 39 deletions(-)

diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 2ef80af6dca..b27d133b113 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -30,7 +30,7 @@ struct SCSIRequest {
     int16_t           status;
     int16_t           host_status;
     void              *hba_private;
-    size_t            resid;
+    size_t            residual;
     SCSICommand       cmd;
     NotifierList      cancel_notifiers;
 
@@ -125,7 +125,7 @@ struct SCSIBusInfo {
                      void *hba_private);
     void (*transfer_data)(SCSIRequest *req, uint32_t arg);
     void (*fail)(SCSIRequest *req);
-    void (*complete)(SCSIRequest *req, size_t resid);
+    void (*complete)(SCSIRequest *req, size_t residual);
     void (*cancel)(SCSIRequest *req);
     void (*change)(SCSIBus *bus, SCSIDevice *dev, SCSISense sense);
     QEMUSGList *(*get_sg_list)(SCSIRequest *req);
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index dc9bbdb740e..cb019549371 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -1045,7 +1045,8 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
     uint64_t pd_size;
     uint16_t pd_id = ((sdev->id & 0xFF) << 8) | (lun & 0xFF);
     uint8_t cmdbuf[6];
-    size_t len, resid;
+    size_t len;
+    size_t residual;
 
     if (!cmd->iov_buf) {
         cmd->iov_buf = g_malloc0(dcmd_size);
@@ -1112,9 +1113,10 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
     info->connected_port_bitmap = 0x1;
     info->device_speed = 1;
     info->link_speed = 1;
-    resid = dma_buf_read(cmd->iov_buf, dcmd_size, &cmd->qsg, MEMTXATTRS_UNSPECIFIED);
+    residual = dma_buf_read(cmd->iov_buf, dcmd_size, &cmd->qsg,
+                            MEMTXATTRS_UNSPECIFIED);
     g_free(cmd->iov_buf);
-    cmd->iov_size = dcmd_size - resid;
+    cmd->iov_size = dcmd_size - residual;
     cmd->iov_buf = NULL;
     return MFI_STAT_OK;
 }
@@ -1149,7 +1151,8 @@ static int megasas_dcmd_pd_get_info(MegasasState *s, MegasasCmd *cmd)
 static int megasas_dcmd_ld_get_list(MegasasState *s, MegasasCmd *cmd)
 {
     struct mfi_ld_list info;
-    size_t dcmd_size = sizeof(info), resid;
+    size_t dcmd_size = sizeof(info);
+    size_t residual;
     uint32_t num_ld_disks = 0, max_ld_disks;
     uint64_t ld_size;
     BusChild *kid;
@@ -1184,8 +1187,9 @@ static int megasas_dcmd_ld_get_list(MegasasState *s, MegasasCmd *cmd)
     info.ld_count = cpu_to_le32(num_ld_disks);
     trace_megasas_dcmd_ld_get_list(cmd->index, num_ld_disks, max_ld_disks);
 
-    resid = dma_buf_read(&info, dcmd_size, &cmd->qsg, MEMTXATTRS_UNSPECIFIED);
-    cmd->iov_size = dcmd_size - resid;
+    residual = dma_buf_read(&info, dcmd_size, &cmd->qsg,
+                            MEMTXATTRS_UNSPECIFIED);
+    cmd->iov_size = dcmd_size - residual;
     return MFI_STAT_OK;
 }
 
@@ -1193,7 +1197,8 @@ static int megasas_dcmd_ld_list_query(MegasasState *s, MegasasCmd *cmd)
 {
     uint16_t flags;
     struct mfi_ld_targetid_list info;
-    size_t dcmd_size = sizeof(info), resid;
+    size_t dcmd_size = sizeof(info);
+    size_t residual;
     uint32_t num_ld_disks = 0, max_ld_disks = s->fw_luns;
     BusChild *kid;
 
@@ -1233,8 +1238,9 @@ static int megasas_dcmd_ld_list_query(MegasasState *s, MegasasCmd *cmd)
     info.size = dcmd_size;
     trace_megasas_dcmd_ld_get_list(cmd->index, num_ld_disks, max_ld_disks);
 
-    resid = dma_buf_read(&info, dcmd_size, &cmd->qsg, MEMTXATTRS_UNSPECIFIED);
-    cmd->iov_size = dcmd_size - resid;
+    residual = dma_buf_read(&info, dcmd_size, &cmd->qsg,
+                            MEMTXATTRS_UNSPECIFIED);
+    cmd->iov_size = dcmd_size - residual;
     return MFI_STAT_OK;
 }
 
@@ -1244,7 +1250,8 @@ static int megasas_ld_get_info_submit(SCSIDevice *sdev, int lun,
     struct mfi_ld_info *info = cmd->iov_buf;
     size_t dcmd_size = sizeof(struct mfi_ld_info);
     uint8_t cdb[6];
-    ssize_t len, resid;
+    ssize_t len;
+    size_t residual;
     uint16_t sdev_id = ((sdev->id & 0xFF) << 8) | (lun & 0xFF);
     uint64_t ld_size;
 
@@ -1283,9 +1290,10 @@ static int megasas_ld_get_info_submit(SCSIDevice *sdev, int lun,
     info->ld_config.span[0].num_blocks = info->size;
     info->ld_config.span[0].array_ref = cpu_to_le16(sdev_id);
 
-    resid = dma_buf_read(cmd->iov_buf, dcmd_size, &cmd->qsg, MEMTXATTRS_UNSPECIFIED);
+    residual = dma_buf_read(cmd->iov_buf, dcmd_size, &cmd->qsg,
+                            MEMTXATTRS_UNSPECIFIED);
     g_free(cmd->iov_buf);
-    cmd->iov_size = dcmd_size - resid;
+    cmd->iov_size = dcmd_size - residual;
     cmd->iov_buf = NULL;
     return MFI_STAT_OK;
 }
@@ -1617,13 +1625,13 @@ static int megasas_handle_dcmd(MegasasState *s, MegasasCmd *cmd)
 }
 
 static int megasas_finish_internal_dcmd(MegasasCmd *cmd,
-                                        SCSIRequest *req, size_t resid)
+                                        SCSIRequest *req, size_t residual)
 {
     int retval = MFI_STAT_OK;
     int lun = req->lun;
 
     trace_megasas_dcmd_internal_finish(cmd->index, cmd->dcmd_opcode, lun);
-    cmd->iov_size -= resid;
+    cmd->iov_size -= residual;
     switch (cmd->dcmd_opcode) {
     case MFI_DCMD_PD_GET_INFO:
         retval = megasas_pd_get_info_submit(req->dev, lun, cmd);
@@ -1865,12 +1873,12 @@ static void megasas_xfer_complete(SCSIRequest *req, uint32_t len)
     }
 }
 
-static void megasas_command_complete(SCSIRequest *req, size_t resid)
+static void megasas_command_complete(SCSIRequest *req, size_t residual)
 {
     MegasasCmd *cmd = req->hba_private;
     uint8_t cmd_status = MFI_STAT_OK;
 
-    trace_megasas_command_complete(cmd->index, req->status, resid);
+    trace_megasas_command_complete(cmd->index, req->status, residual);
 
     if (req->io_canceled) {
         return;
@@ -1880,7 +1888,7 @@ static void megasas_command_complete(SCSIRequest *req, size_t resid)
         /*
          * Internal command complete
          */
-        cmd_status = megasas_finish_internal_dcmd(cmd, req, resid);
+        cmd_status = megasas_finish_internal_dcmd(cmd, req, residual);
         if (cmd_status == MFI_STAT_INVALID_STATUS) {
             return;
         }
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 2b5e9dca311..3466e680ac7 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -760,7 +760,7 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
     }
 
     req->cmd = cmd;
-    req->resid = req->cmd.xfer;
+    req->residual = req->cmd.xfer;
 
     switch (buf[0]) {
     case INQUIRY:
@@ -1408,7 +1408,7 @@ void scsi_req_data(SCSIRequest *req, int len)
     trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
     assert(req->cmd.mode != SCSI_XFER_NONE);
     if (!req->sg) {
-        req->resid -= len;
+        req->residual -= len;
         req->bus->info->transfer_data(req, len);
         return;
     }
@@ -1421,9 +1421,9 @@ void scsi_req_data(SCSIRequest *req, int len)
 
     buf = scsi_req_get_buf(req);
     if (req->cmd.mode == SCSI_XFER_FROM_DEV) {
-        req->resid = dma_buf_read(buf, len, req->sg, MEMTXATTRS_UNSPECIFIED);
+        req->residual = dma_buf_read(buf, len, req->sg, MEMTXATTRS_UNSPECIFIED);
     } else {
-        req->resid = dma_buf_write(buf, len, req->sg, MEMTXATTRS_UNSPECIFIED);
+        req->residual = dma_buf_write(buf, len, req->sg, MEMTXATTRS_UNSPECIFIED);
     }
     scsi_req_continue(req);
 }
@@ -1512,7 +1512,7 @@ void scsi_req_complete(SCSIRequest *req, int status)
 
     scsi_req_ref(req);
     scsi_req_dequeue(req);
-    req->bus->info->complete(req, req->resid);
+    req->bus->info->complete(req, req->residual);
 
     /* Cancelled requests might end up being completed instead of cancelled */
     notifier_list_notify(&req->cancel_notifiers, req);
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index d4914178ea0..9c0dc7b9468 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -420,7 +420,7 @@ static void scsi_do_read(SCSIDiskReq *r, int ret)
 
     if (r->req.sg) {
         dma_acct_start(s->qdev.conf.blk, &r->acct, r->req.sg, BLOCK_ACCT_READ);
-        r->req.resid -= r->req.sg->size;
+        r->req.residual -= r->req.sg->size;
         r->req.aiocb = dma_blk_io(blk_get_aio_context(s->qdev.conf.blk),
                                   r->req.sg, r->sector << BDRV_SECTOR_BITS,
                                   BDRV_SECTOR_SIZE,
@@ -580,7 +580,7 @@ static void scsi_write_data(SCSIRequest *req)
 
     if (r->req.sg) {
         dma_acct_start(s->qdev.conf.blk, &r->acct, r->req.sg, BLOCK_ACCT_WRITE);
-        r->req.resid -= r->req.sg->size;
+        r->req.residual -= r->req.sg->size;
         r->req.aiocb = dma_blk_io(blk_get_aio_context(s->qdev.conf.blk),
                                   r->req.sg, r->sector << BDRV_SECTOR_BITS,
                                   BDRV_SECTOR_SIZE,
diff --git a/softmmu/dma-helpers.c b/softmmu/dma-helpers.c
index b0be1564797..4563a775aa7 100644
--- a/softmmu/dma-helpers.c
+++ b/softmmu/dma-helpers.c
@@ -294,49 +294,49 @@ BlockAIOCB *dma_blk_write(BlockBackend *blk,
 }
 
 
-static MemTxResult dma_buf_rw(void *buf, int32_t len, uint64_t *residp,
+static MemTxResult dma_buf_rw(void *buf, int32_t len, uint64_t *residual,
                               QEMUSGList *sg, DMADirection dir,
                               MemTxAttrs attrs)
 {
     uint8_t *ptr = buf;
-    uint64_t resid;
+    uint64_t xresidual;
     int sg_cur_index;
     MemTxResult res = MEMTX_OK;
 
-    resid = sg->size;
+    xresidual = sg->size;
     sg_cur_index = 0;
-    len = MIN(len, resid);
+    len = MIN(len, xresidual);
     while (len > 0) {
         ScatterGatherEntry entry = sg->sg[sg_cur_index++];
         int32_t xfer = MIN(len, entry.len);
         res |= dma_memory_rw(sg->as, entry.base, ptr, xfer, dir, attrs);
         ptr += xfer;
         len -= xfer;
-        resid -= xfer;
+        xresidual -= xfer;
     }
 
-    if (residp) {
-        *residp = resid;
+    if (residual) {
+        *residual = xresidual;
     }
     return res;
 }
 
 uint64_t dma_buf_read(void *ptr, int32_t len, QEMUSGList *sg, MemTxAttrs attrs)
 {
-    uint64_t resid;
+    uint64_t residual;
 
-    dma_buf_rw(ptr, len, &resid, sg, DMA_DIRECTION_FROM_DEVICE, attrs);
+    dma_buf_rw(ptr, len, &residual, sg, DMA_DIRECTION_FROM_DEVICE, attrs);
 
-    return resid;
+    return residual;
 }
 
 uint64_t dma_buf_write(void *ptr, int32_t len, QEMUSGList *sg, MemTxAttrs attrs)
 {
-    uint64_t resid;
+    uint64_t residual;
 
-    dma_buf_rw(ptr, len, &resid, sg, DMA_DIRECTION_TO_DEVICE, attrs);
+    dma_buf_rw(ptr, len, &residual, sg, DMA_DIRECTION_TO_DEVICE, attrs);
 
-    return resid;
+    return residual;
 }
 
 void dma_acct_start(BlockBackend *blk, BlockAcctCookie *cookie,
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/8] hw/dma: Introduce dma_size_t type definition
  2021-12-31 11:48 [PATCH 0/8] hw/dma: Introduce dma_size_t type definition Philippe Mathieu-Daudé
                   ` (5 preceding siblings ...)
  2021-12-31 11:48 ` [PATCH 6/8] hw/scsi: Rename SCSIRequest::resid as 'residual' Philippe Mathieu-Daudé
@ 2021-12-31 11:49 ` Philippe Mathieu-Daudé
  2022-01-03 10:20   ` David Hildenbrand
  2021-12-31 11:49 ` [PATCH 8/8] hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResult Philippe Mathieu-Daudé
  7 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-12-31 11:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, David Hildenbrand, Peter Xu, Gerd Hoffmann,
	Paolo Bonzini, Philippe Mathieu-Daudé

The DMA API defines the dma_addr_t type for DMA addresses.
Some sizes related to the API also use this type, which is
a bit confusing when reviewing.

Add a similar 'dma_size_t' definition (and its format string).

Update the obvious places where dma_size_t should be used,
and adapt a pair of format strings.

Move QEMUSGList declaration after dma_size_t definition
so this structure can use the new type.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/rdma/rdma_utils.h      |  4 ++--
 include/hw/pci/pci.h      | 10 ++++----
 include/sysemu/dma.h      | 50 ++++++++++++++++++++-------------------
 hw/ide/ahci.c             |  2 +-
 hw/nvme/ctrl.c            |  2 +-
 hw/nvram/fw_cfg.c         |  2 +-
 hw/rdma/rdma_utils.c      |  6 ++---
 hw/rdma/vmw/pvrdma_main.c |  2 +-
 hw/scsi/lsi53c895a.c      |  4 ++--
 hw/scsi/megasas.c         | 10 ++++----
 hw/usb/libhw.c            |  4 ++--
 softmmu/dma-helpers.c     | 10 ++++----
 hw/rdma/trace-events      |  2 +-
 13 files changed, 55 insertions(+), 53 deletions(-)

diff --git a/hw/rdma/rdma_utils.h b/hw/rdma/rdma_utils.h
index 0c6414e7e0a..5446e4e2500 100644
--- a/hw/rdma/rdma_utils.h
+++ b/hw/rdma/rdma_utils.h
@@ -38,8 +38,8 @@ typedef struct RdmaProtectedGSList {
     GSList *list;
 } RdmaProtectedGSList;
 
-void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t len);
-void rdma_pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len);
+void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_size_t len);
+void rdma_pci_dma_unmap(PCIDevice *dev, void *buffer, dma_size_t len);
 void rdma_protected_gqueue_init(RdmaProtectedGQueue *list);
 void rdma_protected_gqueue_destroy(RdmaProtectedGQueue *list);
 void rdma_protected_gqueue_append_int64(RdmaProtectedGQueue *list,
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index f0d707efb8c..f2257b2cd8c 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -805,7 +805,7 @@ static inline AddressSpace *pci_get_address_space(PCIDevice *dev)
  * @dir: indicates the transfer direction
  */
 static inline MemTxResult pci_dma_rw(PCIDevice *dev, dma_addr_t addr,
-                                     void *buf, dma_addr_t len,
+                                     void *buf, dma_size_t len,
                                      DMADirection dir, MemTxAttrs attrs)
 {
     return dma_memory_rw(pci_get_address_space(dev), addr, buf, len,
@@ -825,7 +825,7 @@ static inline MemTxResult pci_dma_rw(PCIDevice *dev, dma_addr_t addr,
  * @len: length of the data transferred
  */
 static inline MemTxResult pci_dma_read(PCIDevice *dev, dma_addr_t addr,
-                                       void *buf, dma_addr_t len)
+                                       void *buf, dma_size_t len)
 {
     return pci_dma_rw(dev, addr, buf, len,
                       DMA_DIRECTION_TO_DEVICE, MEMTXATTRS_UNSPECIFIED);
@@ -844,7 +844,7 @@ static inline MemTxResult pci_dma_read(PCIDevice *dev, dma_addr_t addr,
  * @len: the number of bytes to write
  */
 static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
-                                        const void *buf, dma_addr_t len)
+                                        const void *buf, dma_size_t len)
 {
     return pci_dma_rw(dev, addr, (void *) buf, len,
                       DMA_DIRECTION_FROM_DEVICE, MEMTXATTRS_UNSPECIFIED);
@@ -889,7 +889,7 @@ PCI_DMA_DEFINE_LDST(q_be, q_be, 64);
  * @dir: indicates the transfer direction
  */
 static inline void *pci_dma_map(PCIDevice *dev, dma_addr_t addr,
-                                dma_addr_t *plen, DMADirection dir)
+                                dma_size_t *plen, DMADirection dir)
 {
     void *buf;
 
@@ -898,7 +898,7 @@ static inline void *pci_dma_map(PCIDevice *dev, dma_addr_t addr,
     return buf;
 }
 
-static inline void pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len,
+static inline void pci_dma_unmap(PCIDevice *dev, void *buffer, dma_size_t len,
                                  DMADirection dir, dma_addr_t access_len)
 {
     dma_memory_unmap(pci_get_address_space(dev), buffer, len, dir, access_len);
diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
index 0db2478a506..45a2567848c 100644
--- a/include/sysemu/dma.h
+++ b/include/sysemu/dma.h
@@ -15,22 +15,11 @@
 #include "block/block.h"
 #include "block/accounting.h"
 
-typedef struct ScatterGatherEntry ScatterGatherEntry;
-
 typedef enum {
     DMA_DIRECTION_TO_DEVICE = 0,
     DMA_DIRECTION_FROM_DEVICE = 1,
 } DMADirection;
 
-struct QEMUSGList {
-    ScatterGatherEntry *sg;
-    int nsg;
-    int nalloc;
-    size_t size;
-    DeviceState *dev;
-    AddressSpace *as;
-};
-
 /*
  * When an IOMMU is present, bus addresses become distinct from
  * CPU/memory physical addresses and may be a different size.  Because
@@ -39,9 +28,22 @@ struct QEMUSGList {
  * least most) cases.
  */
 typedef uint64_t dma_addr_t;
+typedef uint64_t dma_size_t;
 
 #define DMA_ADDR_BITS 64
 #define DMA_ADDR_FMT "%" PRIx64
+#define DMA_SIZE_FMT "%" PRIx64
+
+typedef struct ScatterGatherEntry ScatterGatherEntry;
+
+struct QEMUSGList {
+    ScatterGatherEntry *sg;
+    int nsg;
+    int nalloc;
+    dma_size_t size;
+    DeviceState *dev;
+    AddressSpace *as;
+};
 
 static inline void dma_barrier(AddressSpace *as, DMADirection dir)
 {
@@ -70,7 +72,7 @@ static inline void dma_barrier(AddressSpace *as, DMADirection dir)
  * useful for certain cases, but usually you should just use
  * dma_memory_{read,write}() and check for errors */
 static inline bool dma_memory_valid(AddressSpace *as,
-                                    dma_addr_t addr, dma_addr_t len,
+                                    dma_addr_t addr, dma_size_t len,
                                     DMADirection dir, MemTxAttrs attrs)
 {
     return address_space_access_valid(as, addr, len,
@@ -80,7 +82,7 @@ static inline bool dma_memory_valid(AddressSpace *as,
 
 static inline MemTxResult dma_memory_rw_relaxed(AddressSpace *as,
                                                 dma_addr_t addr,
-                                                void *buf, dma_addr_t len,
+                                                void *buf, dma_size_t len,
                                                 DMADirection dir,
                                                 MemTxAttrs attrs)
 {
@@ -90,7 +92,7 @@ static inline MemTxResult dma_memory_rw_relaxed(AddressSpace *as,
 
 static inline MemTxResult dma_memory_read_relaxed(AddressSpace *as,
                                                   dma_addr_t addr,
-                                                  void *buf, dma_addr_t len)
+                                                  void *buf, dma_size_t len)
 {
     return dma_memory_rw_relaxed(as, addr, buf, len,
                                  DMA_DIRECTION_TO_DEVICE,
@@ -100,7 +102,7 @@ static inline MemTxResult dma_memory_read_relaxed(AddressSpace *as,
 static inline MemTxResult dma_memory_write_relaxed(AddressSpace *as,
                                                    dma_addr_t addr,
                                                    const void *buf,
-                                                   dma_addr_t len)
+                                                   dma_size_t len)
 {
     return dma_memory_rw_relaxed(as, addr, (void *)buf, len,
                                  DMA_DIRECTION_FROM_DEVICE,
@@ -122,7 +124,7 @@ static inline MemTxResult dma_memory_write_relaxed(AddressSpace *as,
  * @attrs: memory transaction attributes
  */
 static inline MemTxResult dma_memory_rw(AddressSpace *as, dma_addr_t addr,
-                                        void *buf, dma_addr_t len,
+                                        void *buf, dma_size_t len,
                                         DMADirection dir, MemTxAttrs attrs)
 {
     dma_barrier(as, dir);
@@ -144,7 +146,7 @@ static inline MemTxResult dma_memory_rw(AddressSpace *as, dma_addr_t addr,
  * @attrs: memory transaction attributes
  */
 static inline MemTxResult dma_memory_read(AddressSpace *as, dma_addr_t addr,
-                                          void *buf, dma_addr_t len,
+                                          void *buf, dma_size_t len,
                                           MemTxAttrs attrs)
 {
     return dma_memory_rw(as, addr, buf, len,
@@ -165,7 +167,7 @@ static inline MemTxResult dma_memory_read(AddressSpace *as, dma_addr_t addr,
  * @attrs: memory transaction attributes
  */
 static inline MemTxResult dma_memory_write(AddressSpace *as, dma_addr_t addr,
-                                           const void *buf, dma_addr_t len,
+                                           const void *buf, dma_size_t len,
                                            MemTxAttrs attrs)
 {
     return dma_memory_rw(as, addr, (void *)buf, len,
@@ -186,7 +188,7 @@ static inline MemTxResult dma_memory_write(AddressSpace *as, dma_addr_t addr,
  * @attrs: memory transaction attributes
  */
 MemTxResult dma_memory_set(AddressSpace *as, dma_addr_t addr,
-                           uint8_t c, dma_addr_t len, MemTxAttrs attrs);
+                           uint8_t c, dma_size_t len, MemTxAttrs attrs);
 
 /**
  * address_space_map: Map a physical memory region into a host virtual address.
@@ -203,7 +205,7 @@ MemTxResult dma_memory_set(AddressSpace *as, dma_addr_t addr,
  * @attrs: memory attributes
  */
 static inline void *dma_memory_map(AddressSpace *as,
-                                   dma_addr_t addr, dma_addr_t *len,
+                                   dma_addr_t addr, dma_size_t *len,
                                    DMADirection dir, MemTxAttrs attrs)
 {
     hwaddr xlen = *len;
@@ -230,8 +232,8 @@ static inline void *dma_memory_map(AddressSpace *as,
  * @access_len: amount of data actually transferred
  */
 static inline void dma_memory_unmap(AddressSpace *as,
-                                    void *buffer, dma_addr_t len,
-                                    DMADirection dir, dma_addr_t access_len)
+                                    void *buffer, dma_size_t len,
+                                    DMADirection dir, dma_size_t access_len)
 {
     address_space_unmap(as, buffer, (hwaddr)len,
                         dir == DMA_DIRECTION_FROM_DEVICE, access_len);
@@ -279,12 +281,12 @@ DEFINE_LDST_DMA(q, q, 64, be);
 
 struct ScatterGatherEntry {
     dma_addr_t base;
-    dma_addr_t len;
+    dma_size_t len;
 };
 
 void qemu_sglist_init(QEMUSGList *qsg, DeviceState *dev, int alloc_hint,
                       AddressSpace *as);
-void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, dma_addr_t len);
+void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, dma_size_t len);
 void qemu_sglist_destroy(QEMUSGList *qsg);
 
 typedef BlockAIOCB *DMAIOFunc(int64_t offset, QEMUIOVector *iov,
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 205dfdc6622..256bbc377ce 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1159,7 +1159,7 @@ static void process_ncq_command(AHCIState *s, int port, const uint8_t *cmd_fis,
     ahci_populate_sglist(ad, &ncq_tfs->sglist, ncq_tfs->cmdh, size, 0);
 
     if (ncq_tfs->sglist.size < size) {
-        error_report("ahci: PRDT length for NCQ command (0x%zx) "
+        error_report("ahci: PRDT length for NCQ command (0x" DMA_SIZE_FMT ") "
                      "is smaller than the requested size (0x%zx)",
                      ncq_tfs->sglist.size, size);
         ncq_err(ncq_tfs);
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 462f79a1f60..4620e662fc9 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -1147,7 +1147,7 @@ static uint16_t nvme_tx(NvmeCtrl *n, NvmeSg *sg, uint8_t *ptr, uint32_t len,
 
     if (sg->flags & NVME_SG_DMA) {
         const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
-        uint64_t residual;
+        dma_size_t residual;
 
         if (dir == NVME_TX_DIRECTION_TO_DEVICE) {
             residual = dma_buf_write(ptr, len, &sg->qsg, attrs);
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index e5f3c981841..a30bff8a14a 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -346,12 +346,12 @@ static void fw_cfg_data_mem_write(void *opaque, hwaddr addr,
 
 static void fw_cfg_dma_transfer(FWCfgState *s)
 {
-    dma_addr_t len;
     FWCfgDmaAccess dma;
     int arch;
     FWCfgEntry *e;
     int read = 0, write = 0;
     dma_addr_t dma_addr;
+    dma_size_t len;
 
     /* Reset the address before the next access */
     dma_addr = s->dma_addr;
diff --git a/hw/rdma/rdma_utils.c b/hw/rdma/rdma_utils.c
index 61cb8ede0fd..677d7ffba53 100644
--- a/hw/rdma/rdma_utils.c
+++ b/hw/rdma/rdma_utils.c
@@ -17,10 +17,10 @@
 #include "trace.h"
 #include "rdma_utils.h"
 
-void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t len)
+void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_size_t len)
 {
     void *p;
-    hwaddr pci_len = len;
+    dma_size_t pci_len = len;
 
     if (!addr) {
         rdma_error_report("addr is NULL");
@@ -44,7 +44,7 @@ void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t len)
     return p;
 }
 
-void rdma_pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len)
+void rdma_pci_dma_unmap(PCIDevice *dev, void *buffer, dma_size_t len)
 {
     trace_rdma_pci_dma_unmap(buffer);
     if (buffer) {
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 91206dbb8eb..bf3d542302e 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -124,7 +124,7 @@ static int init_dev_ring(PvrdmaRing *ring, PvrdmaRingState **ring_state,
                           (num_pages - 1) * TARGET_PAGE_SIZE /
                           sizeof(struct pvrdma_cqne),
                           sizeof(struct pvrdma_cqne),
-                          (dma_addr_t *)&tbl[1], (dma_addr_t)num_pages - 1);
+                          (dma_addr_t *)&tbl[1], (dma_size_t)num_pages - 1);
     if (rc) {
         rc = -ENOMEM;
         goto out_free_ring_state;
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 4c431adb774..6d63bffc804 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -444,7 +444,7 @@ static void lsi_execute_script(LSIState *s);
 static void lsi_reselect(LSIState *s, lsi_request *p);
 
 static inline void lsi_mem_read(LSIState *s, dma_addr_t addr,
-                               void *buf, dma_addr_t len)
+                                void *buf, dma_size_t len)
 {
     if (s->dmode & LSI_DMODE_SIOM) {
         address_space_read(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
@@ -455,7 +455,7 @@ static inline void lsi_mem_read(LSIState *s, dma_addr_t addr,
 }
 
 static inline void lsi_mem_write(LSIState *s, dma_addr_t addr,
-                                const void *buf, dma_addr_t len)
+                                 const void *buf, dma_size_t len)
 {
     if (s->dmode & LSI_DMODE_DIOM) {
         address_space_write(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index cb019549371..7456a25970a 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -1046,7 +1046,7 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
     uint16_t pd_id = ((sdev->id & 0xFF) << 8) | (lun & 0xFF);
     uint8_t cmdbuf[6];
     size_t len;
-    size_t residual;
+    dma_size_t residual;
 
     if (!cmd->iov_buf) {
         cmd->iov_buf = g_malloc0(dcmd_size);
@@ -1152,7 +1152,7 @@ static int megasas_dcmd_ld_get_list(MegasasState *s, MegasasCmd *cmd)
 {
     struct mfi_ld_list info;
     size_t dcmd_size = sizeof(info);
-    size_t residual;
+    dma_size_t residual;
     uint32_t num_ld_disks = 0, max_ld_disks;
     uint64_t ld_size;
     BusChild *kid;
@@ -1198,7 +1198,7 @@ static int megasas_dcmd_ld_list_query(MegasasState *s, MegasasCmd *cmd)
     uint16_t flags;
     struct mfi_ld_targetid_list info;
     size_t dcmd_size = sizeof(info);
-    size_t residual;
+    dma_size_t residual;
     uint32_t num_ld_disks = 0, max_ld_disks = s->fw_luns;
     BusChild *kid;
 
@@ -1251,7 +1251,7 @@ static int megasas_ld_get_info_submit(SCSIDevice *sdev, int lun,
     size_t dcmd_size = sizeof(struct mfi_ld_info);
     uint8_t cdb[6];
     ssize_t len;
-    size_t residual;
+    dma_size_t residual;
     uint16_t sdev_id = ((sdev->id & 0xFF) << 8) | (lun & 0xFF);
     uint64_t ld_size;
 
@@ -1625,7 +1625,7 @@ static int megasas_handle_dcmd(MegasasState *s, MegasasCmd *cmd)
 }
 
 static int megasas_finish_internal_dcmd(MegasasCmd *cmd,
-                                        SCSIRequest *req, size_t residual)
+                                        SCSIRequest *req, dma_size_t residual)
 {
     int retval = MFI_STAT_OK;
     int lun = req->lun;
diff --git a/hw/usb/libhw.c b/hw/usb/libhw.c
index f350eae443d..f632258459a 100644
--- a/hw/usb/libhw.c
+++ b/hw/usb/libhw.c
@@ -32,10 +32,10 @@ int usb_packet_map(USBPacket *p, QEMUSGList *sgl)
 
     for (i = 0; i < sgl->nsg; i++) {
         dma_addr_t base = sgl->sg[i].base;
-        dma_addr_t len = sgl->sg[i].len;
+        dma_size_t len = sgl->sg[i].len;
 
         while (len) {
-            dma_addr_t xlen = len;
+            dma_size_t xlen = len;
             mem = dma_memory_map(sgl->as, base, &xlen, dir,
                                  MEMTXATTRS_UNSPECIFIED);
             if (!mem) {
diff --git a/softmmu/dma-helpers.c b/softmmu/dma-helpers.c
index 4563a775aa7..66ba0b2c361 100644
--- a/softmmu/dma-helpers.c
+++ b/softmmu/dma-helpers.c
@@ -19,7 +19,7 @@
 /* #define DEBUG_IOMMU */
 
 MemTxResult dma_memory_set(AddressSpace *as, dma_addr_t addr,
-                           uint8_t c, dma_addr_t len, MemTxAttrs attrs)
+                           uint8_t c, dma_size_t len, MemTxAttrs attrs)
 {
     dma_barrier(as, DMA_DIRECTION_FROM_DEVICE);
 
@@ -51,7 +51,7 @@ void qemu_sglist_init(QEMUSGList *qsg, DeviceState *dev, int alloc_hint,
     object_ref(OBJECT(dev));
 }
 
-void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, dma_addr_t len)
+void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, dma_size_t len)
 {
     if (qsg->nsg == qsg->nalloc) {
         qsg->nalloc = 2 * qsg->nalloc + 1;
@@ -294,12 +294,12 @@ BlockAIOCB *dma_blk_write(BlockBackend *blk,
 }
 
 
-static MemTxResult dma_buf_rw(void *buf, int32_t len, uint64_t *residual,
+static MemTxResult dma_buf_rw(void *buf, dma_size_t len, dma_size_t *residual,
                               QEMUSGList *sg, DMADirection dir,
                               MemTxAttrs attrs)
 {
     uint8_t *ptr = buf;
-    uint64_t xresidual;
+    dma_size_t xresidual;
     int sg_cur_index;
     MemTxResult res = MEMTX_OK;
 
@@ -308,7 +308,7 @@ static MemTxResult dma_buf_rw(void *buf, int32_t len, uint64_t *residual,
     len = MIN(len, xresidual);
     while (len > 0) {
         ScatterGatherEntry entry = sg->sg[sg_cur_index++];
-        int32_t xfer = MIN(len, entry.len);
+        dma_size_t xfer = MIN(len, entry.len);
         res |= dma_memory_rw(sg->as, entry.base, ptr, xfer, dir, attrs);
         ptr += xfer;
         len -= xfer;
diff --git a/hw/rdma/trace-events b/hw/rdma/trace-events
index 9accb149734..c23175120e1 100644
--- a/hw/rdma/trace-events
+++ b/hw/rdma/trace-events
@@ -27,5 +27,5 @@ rdma_rm_alloc_qp(uint32_t rm_qpn, uint32_t backend_qpn, uint8_t qp_type) "rm_qpn
 rdma_rm_modify_qp(uint32_t qpn, uint32_t attr_mask, int qp_state, uint8_t sgid_idx) "qpn=0x%x, attr_mask=0x%x, qp_state=%d, sgid_idx=%d"
 
 # rdma_utils.c
-rdma_pci_dma_map(uint64_t addr, void *vaddr, uint64_t len) "0x%"PRIx64" -> %p (len=%" PRId64")"
+rdma_pci_dma_map(uint64_t addr, void *vaddr, uint64_t len) "0x%"PRIx64" -> %p (len=%" PRIu64")"
 rdma_pci_dma_unmap(void *vaddr) "%p"
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 8/8] hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResult
  2021-12-31 11:48 [PATCH 0/8] hw/dma: Introduce dma_size_t type definition Philippe Mathieu-Daudé
                   ` (6 preceding siblings ...)
  2021-12-31 11:49 ` [PATCH 7/8] hw/dma: Introduce dma_size_t type definition Philippe Mathieu-Daudé
@ 2021-12-31 11:49 ` Philippe Mathieu-Daudé
  2021-12-31 11:51   ` Philippe Mathieu-Daudé
  7 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-12-31 11:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Klaus Jensen, David Hildenbrand, Peter Xu,
	Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé

Since the previous commit, dma_buf_rw() returns a MemTxResult
type. Do not discard it, return it to the caller.

Since both dma_buf_read/dma_buf_write functions were previously
returning the QEMUSGList size not consumed, add an extra argument
where the unconsummed size can be stored.

Update the few callers.

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/scsi/scsi.h |  2 +-
 include/sysemu/dma.h   |  6 +++--
 hw/ide/ahci.c          |  8 +++---
 hw/nvme/ctrl.c         |  4 +--
 hw/scsi/megasas.c      | 59 ++++++++++++++++++++++++++++++------------
 hw/scsi/scsi-bus.c     |  6 +++--
 softmmu/dma-helpers.c  | 18 +++++--------
 7 files changed, 63 insertions(+), 40 deletions(-)

diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index b27d133b113..1ffb367f94f 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -30,7 +30,7 @@ struct SCSIRequest {
     int16_t           status;
     int16_t           host_status;
     void              *hba_private;
-    size_t            residual;
+    uint64_t          residual;
     SCSICommand       cmd;
     NotifierList      cancel_notifiers;
 
diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
index 45a2567848c..77a346d5ed1 100644
--- a/include/sysemu/dma.h
+++ b/include/sysemu/dma.h
@@ -303,8 +303,10 @@ BlockAIOCB *dma_blk_read(BlockBackend *blk,
 BlockAIOCB *dma_blk_write(BlockBackend *blk,
                           QEMUSGList *sg, uint64_t offset, uint32_t align,
                           BlockCompletionFunc *cb, void *opaque);
-uint64_t dma_buf_read(void *ptr, int32_t len, QEMUSGList *sg, MemTxAttrs attrs);
-uint64_t dma_buf_write(void *ptr, int32_t len, QEMUSGList *sg, MemTxAttrs attrs);
+MemTxResult dma_buf_read(void *ptr, dma_size_t len, dma_size_t *residual,
+                         QEMUSGList *sg, MemTxAttrs attrs);
+MemTxResult dma_buf_write(void *ptr, dma_size_t len, dma_size_t *residual,
+                          QEMUSGList *sg, MemTxAttrs attrs);
 
 void dma_acct_start(BlockBackend *blk, BlockAcctCookie *cookie,
                     QEMUSGList *sg, enum BlockAcctType type);
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 256bbc377ce..564293fd492 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1384,9 +1384,9 @@ static void ahci_pio_transfer(const IDEDMA *dma)
         const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
 
         if (is_write) {
-            dma_buf_write(s->data_ptr, size, &s->sg, attrs);
+            dma_buf_write(s->data_ptr, size, NULL, &s->sg, attrs);
         } else {
-            dma_buf_read(s->data_ptr, size, &s->sg, attrs);
+            dma_buf_read(s->data_ptr, size, NULL, &s->sg, attrs);
         }
     }
 
@@ -1479,9 +1479,9 @@ static int ahci_dma_rw_buf(const IDEDMA *dma, bool is_write)
     }
 
     if (is_write) {
-        dma_buf_read(p, l, &s->sg, MEMTXATTRS_UNSPECIFIED);
+        dma_buf_read(p, l, NULL, &s->sg, MEMTXATTRS_UNSPECIFIED);
     } else {
-        dma_buf_write(p, l, &s->sg, MEMTXATTRS_UNSPECIFIED);
+        dma_buf_write(p, l, NULL, &s->sg, MEMTXATTRS_UNSPECIFIED);
     }
 
     /* free sglist, update byte count */
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 4620e662fc9..72faa94a4a3 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -1150,9 +1150,9 @@ static uint16_t nvme_tx(NvmeCtrl *n, NvmeSg *sg, uint8_t *ptr, uint32_t len,
         dma_size_t residual;
 
         if (dir == NVME_TX_DIRECTION_TO_DEVICE) {
-            residual = dma_buf_write(ptr, len, &sg->qsg, attrs);
+            dma_buf_write(ptr, len, &residual, &sg->qsg, attrs);
         } else {
-            residual = dma_buf_read(ptr, len, &sg->qsg, attrs);
+            dma_buf_read(ptr, len, &residual, &sg->qsg, attrs);
         }
 
         if (unlikely(residual)) {
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 7456a25970a..f2daeeb7005 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -750,6 +750,7 @@ static int megasas_ctrl_get_info(MegasasState *s, MegasasCmd *cmd)
     size_t dcmd_size = sizeof(info);
     BusChild *kid;
     int num_pd_disks = 0;
+    dma_size_t residual;
 
     memset(&info, 0x0, dcmd_size);
     if (cmd->iov_size < dcmd_size) {
@@ -860,7 +861,9 @@ static int megasas_ctrl_get_info(MegasasState *s, MegasasCmd *cmd)
                                        MFI_INFO_PDMIX_SATA |
                                        MFI_INFO_PDMIX_LD);
 
-    cmd->iov_size -= dma_buf_read(&info, dcmd_size, &cmd->qsg, MEMTXATTRS_UNSPECIFIED);
+    dma_buf_read(&info, dcmd_size, &residual, &cmd->qsg,
+                 MEMTXATTRS_UNSPECIFIED);
+    cmd->iov_size -= residual;
     return MFI_STAT_OK;
 }
 
@@ -868,6 +871,7 @@ static int megasas_mfc_get_defaults(MegasasState *s, MegasasCmd *cmd)
 {
     struct mfi_defaults info;
     size_t dcmd_size = sizeof(struct mfi_defaults);
+    dma_size_t residual;
 
     memset(&info, 0x0, dcmd_size);
     if (cmd->iov_size < dcmd_size) {
@@ -890,7 +894,9 @@ static int megasas_mfc_get_defaults(MegasasState *s, MegasasCmd *cmd)
     info.disable_preboot_cli = 1;
     info.cluster_disable = 1;
 
-    cmd->iov_size -= dma_buf_read(&info, dcmd_size, &cmd->qsg, MEMTXATTRS_UNSPECIFIED);
+    dma_buf_read(&info, dcmd_size, &residual, &cmd->qsg,
+                 MEMTXATTRS_UNSPECIFIED);
+    cmd->iov_size -= residual;
     return MFI_STAT_OK;
 }
 
@@ -898,6 +904,7 @@ static int megasas_dcmd_get_bios_info(MegasasState *s, MegasasCmd *cmd)
 {
     struct mfi_bios_data info;
     size_t dcmd_size = sizeof(info);
+    dma_size_t residual;
 
     memset(&info, 0x0, dcmd_size);
     if (cmd->iov_size < dcmd_size) {
@@ -911,7 +918,9 @@ static int megasas_dcmd_get_bios_info(MegasasState *s, MegasasCmd *cmd)
         info.expose_all_drives = 1;
     }
 
-    cmd->iov_size -= dma_buf_read(&info, dcmd_size, &cmd->qsg, MEMTXATTRS_UNSPECIFIED);
+    dma_buf_read(&info, dcmd_size, &residual, &cmd->qsg,
+                 MEMTXATTRS_UNSPECIFIED);
+    cmd->iov_size -= residual;
     return MFI_STAT_OK;
 }
 
@@ -919,10 +928,13 @@ static int megasas_dcmd_get_fw_time(MegasasState *s, MegasasCmd *cmd)
 {
     uint64_t fw_time;
     size_t dcmd_size = sizeof(fw_time);
+    dma_size_t residual;
 
     fw_time = cpu_to_le64(megasas_fw_time());
 
-    cmd->iov_size -= dma_buf_read(&fw_time, dcmd_size, &cmd->qsg, MEMTXATTRS_UNSPECIFIED);
+    dma_buf_read(&fw_time, dcmd_size, &residual, &cmd->qsg,
+                 MEMTXATTRS_UNSPECIFIED);
+    cmd->iov_size -= residual;
     return MFI_STAT_OK;
 }
 
@@ -942,6 +954,7 @@ static int megasas_event_info(MegasasState *s, MegasasCmd *cmd)
 {
     struct mfi_evt_log_state info;
     size_t dcmd_size = sizeof(info);
+    dma_size_t residual;
 
     memset(&info, 0, dcmd_size);
 
@@ -949,7 +962,9 @@ static int megasas_event_info(MegasasState *s, MegasasCmd *cmd)
     info.shutdown_seq_num = cpu_to_le32(s->shutdown_event);
     info.boot_seq_num = cpu_to_le32(s->boot_event);
 
-    cmd->iov_size -= dma_buf_read(&info, dcmd_size, &cmd->qsg, MEMTXATTRS_UNSPECIFIED);
+    dma_buf_read(&info, dcmd_size, &residual, &cmd->qsg,
+                 MEMTXATTRS_UNSPECIFIED);
+    cmd->iov_size -= residual;
     return MFI_STAT_OK;
 }
 
@@ -979,6 +994,7 @@ static int megasas_dcmd_pd_get_list(MegasasState *s, MegasasCmd *cmd)
     size_t dcmd_size = sizeof(info);
     BusChild *kid;
     uint32_t offset, dcmd_limit, num_pd_disks = 0, max_pd_disks;
+    dma_size_t residual;
 
     memset(&info, 0, dcmd_size);
     offset = 8;
@@ -1018,7 +1034,9 @@ static int megasas_dcmd_pd_get_list(MegasasState *s, MegasasCmd *cmd)
     info.size = cpu_to_le32(offset);
     info.count = cpu_to_le32(num_pd_disks);
 
-    cmd->iov_size -= dma_buf_read(&info, offset, &cmd->qsg, MEMTXATTRS_UNSPECIFIED);
+    dma_buf_read(&info, offset, &residual, &cmd->qsg,
+                 MEMTXATTRS_UNSPECIFIED);
+    cmd->iov_size -= residual;
     return MFI_STAT_OK;
 }
 
@@ -1113,8 +1131,9 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
     info->connected_port_bitmap = 0x1;
     info->device_speed = 1;
     info->link_speed = 1;
-    residual = dma_buf_read(cmd->iov_buf, dcmd_size, &cmd->qsg,
-                            MEMTXATTRS_UNSPECIFIED);
+    dma_buf_read(cmd->iov_buf, dcmd_size, &residual, &cmd->qsg,
+                 MEMTXATTRS_UNSPECIFIED);
+    cmd->iov_size -= residual;
     g_free(cmd->iov_buf);
     cmd->iov_size = dcmd_size - residual;
     cmd->iov_buf = NULL;
@@ -1187,8 +1206,8 @@ static int megasas_dcmd_ld_get_list(MegasasState *s, MegasasCmd *cmd)
     info.ld_count = cpu_to_le32(num_ld_disks);
     trace_megasas_dcmd_ld_get_list(cmd->index, num_ld_disks, max_ld_disks);
 
-    residual = dma_buf_read(&info, dcmd_size, &cmd->qsg,
-                            MEMTXATTRS_UNSPECIFIED);
+    dma_buf_read(&info, dcmd_size, &residual, &cmd->qsg,
+                 MEMTXATTRS_UNSPECIFIED);
     cmd->iov_size = dcmd_size - residual;
     return MFI_STAT_OK;
 }
@@ -1238,8 +1257,8 @@ static int megasas_dcmd_ld_list_query(MegasasState *s, MegasasCmd *cmd)
     info.size = dcmd_size;
     trace_megasas_dcmd_ld_get_list(cmd->index, num_ld_disks, max_ld_disks);
 
-    residual = dma_buf_read(&info, dcmd_size, &cmd->qsg,
-                            MEMTXATTRS_UNSPECIFIED);
+    dma_buf_read(&info, dcmd_size, &residual, &cmd->qsg,
+                 MEMTXATTRS_UNSPECIFIED);
     cmd->iov_size = dcmd_size - residual;
     return MFI_STAT_OK;
 }
@@ -1290,8 +1309,8 @@ static int megasas_ld_get_info_submit(SCSIDevice *sdev, int lun,
     info->ld_config.span[0].num_blocks = info->size;
     info->ld_config.span[0].array_ref = cpu_to_le16(sdev_id);
 
-    residual = dma_buf_read(cmd->iov_buf, dcmd_size, &cmd->qsg,
-                            MEMTXATTRS_UNSPECIFIED);
+    dma_buf_read(cmd->iov_buf, dcmd_size, &residual, &cmd->qsg,
+                 MEMTXATTRS_UNSPECIFIED);
     g_free(cmd->iov_buf);
     cmd->iov_size = dcmd_size - residual;
     cmd->iov_buf = NULL;
@@ -1336,6 +1355,7 @@ static int megasas_dcmd_cfg_read(MegasasState *s, MegasasCmd *cmd)
     struct mfi_config_data *info;
     int num_pd_disks = 0, array_offset, ld_offset;
     BusChild *kid;
+    dma_size_t residual;
 
     if (cmd->iov_size > 4096) {
         return MFI_STAT_INVALID_PARAMETER;
@@ -1410,7 +1430,9 @@ static int megasas_dcmd_cfg_read(MegasasState *s, MegasasCmd *cmd)
         ld_offset += sizeof(struct mfi_ld_config);
     }
 
-    cmd->iov_size -= dma_buf_read(data, info->size, &cmd->qsg, MEMTXATTRS_UNSPECIFIED);
+    dma_buf_read(data, info->size, &residual, &cmd->qsg,
+                 MEMTXATTRS_UNSPECIFIED);
+    cmd->iov_size -= residual;
     return MFI_STAT_OK;
 }
 
@@ -1418,6 +1440,7 @@ static int megasas_dcmd_get_properties(MegasasState *s, MegasasCmd *cmd)
 {
     struct mfi_ctrl_props info;
     size_t dcmd_size = sizeof(info);
+    dma_size_t residual;
 
     memset(&info, 0x0, dcmd_size);
     if (cmd->iov_size < dcmd_size) {
@@ -1440,7 +1463,9 @@ static int megasas_dcmd_get_properties(MegasasState *s, MegasasCmd *cmd)
     info.ecc_bucket_leak_rate = cpu_to_le16(1440);
     info.expose_encl_devices = 1;
 
-    cmd->iov_size -= dma_buf_read(&info, dcmd_size, &cmd->qsg, MEMTXATTRS_UNSPECIFIED);
+    dma_buf_read(&info, dcmd_size, &residual, &cmd->qsg,
+                 MEMTXATTRS_UNSPECIFIED);
+    cmd->iov_size -= residual;
     return MFI_STAT_OK;
 }
 
@@ -1485,7 +1510,7 @@ static int megasas_dcmd_set_properties(MegasasState *s, MegasasCmd *cmd)
                                             dcmd_size);
         return MFI_STAT_INVALID_PARAMETER;
     }
-    dma_buf_write(&info, dcmd_size, &cmd->qsg, MEMTXATTRS_UNSPECIFIED);
+    dma_buf_write(&info, dcmd_size, NULL, &cmd->qsg, MEMTXATTRS_UNSPECIFIED);
     trace_megasas_dcmd_unsupported(cmd->index, cmd->iov_size);
     return MFI_STAT_OK;
 }
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 3466e680ac7..4057e04ce89 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1421,9 +1421,11 @@ void scsi_req_data(SCSIRequest *req, int len)
 
     buf = scsi_req_get_buf(req);
     if (req->cmd.mode == SCSI_XFER_FROM_DEV) {
-        req->residual = dma_buf_read(buf, len, req->sg, MEMTXATTRS_UNSPECIFIED);
+        dma_buf_read(buf, len, &req->residual, req->sg,
+                     MEMTXATTRS_UNSPECIFIED);
     } else {
-        req->residual = dma_buf_write(buf, len, req->sg, MEMTXATTRS_UNSPECIFIED);
+        dma_buf_write(buf, len, &req->residual, req->sg,
+                      MEMTXATTRS_UNSPECIFIED);
     }
     scsi_req_continue(req);
 }
diff --git a/softmmu/dma-helpers.c b/softmmu/dma-helpers.c
index 66ba0b2c361..25120ea2792 100644
--- a/softmmu/dma-helpers.c
+++ b/softmmu/dma-helpers.c
@@ -321,22 +321,16 @@ static MemTxResult dma_buf_rw(void *buf, dma_size_t len, dma_size_t *residual,
     return res;
 }
 
-uint64_t dma_buf_read(void *ptr, int32_t len, QEMUSGList *sg, MemTxAttrs attrs)
+MemTxResult dma_buf_read(void *ptr, dma_size_t len, dma_size_t *residual,
+                         QEMUSGList *sg, MemTxAttrs attrs)
 {
-    uint64_t residual;
-
-    dma_buf_rw(ptr, len, &residual, sg, DMA_DIRECTION_FROM_DEVICE, attrs);
-
-    return residual;
+    return dma_buf_rw(ptr, len, residual, sg, DMA_DIRECTION_FROM_DEVICE, attrs);
 }
 
-uint64_t dma_buf_write(void *ptr, int32_t len, QEMUSGList *sg, MemTxAttrs attrs)
+MemTxResult dma_buf_write(void *ptr, dma_size_t len, dma_size_t *residual,
+                          QEMUSGList *sg, MemTxAttrs attrs)
 {
-    uint64_t residual;
-
-    dma_buf_rw(ptr, len, &residual, sg, DMA_DIRECTION_TO_DEVICE, attrs);
-
-    return residual;
+    return dma_buf_rw(ptr, len, residual, sg, DMA_DIRECTION_TO_DEVICE, attrs);
 }
 
 void dma_acct_start(BlockBackend *blk, BlockAcctCookie *cookie,
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 8/8] hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResult
  2021-12-31 11:49 ` [PATCH 8/8] hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResult Philippe Mathieu-Daudé
@ 2021-12-31 11:51   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-12-31 11:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Klaus Jensen, David Hildenbrand, Peter Xu,
	Gerd Hoffmann, Paolo Bonzini

On 12/31/21 12:49, Philippe Mathieu-Daudé wrote:
> Since the previous commit, dma_buf_rw() returns a MemTxResult
> type. Do not discard it, return it to the caller.
> 
> Since both dma_buf_read/dma_buf_write functions were previously
> returning the QEMUSGList size not consumed, add an extra argument
> where the unconsummed size can be stored.
> 
> Update the few callers.
> 
> Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/hw/scsi/scsi.h |  2 +-
>  include/sysemu/dma.h   |  6 +++--
>  hw/ide/ahci.c          |  8 +++---
>  hw/nvme/ctrl.c         |  4 +--
>  hw/scsi/megasas.c      | 59 ++++++++++++++++++++++++++++++------------
>  hw/scsi/scsi-bus.c     |  6 +++--
>  softmmu/dma-helpers.c  | 18 +++++--------
>  7 files changed, 63 insertions(+), 40 deletions(-)
> 
> diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
> index b27d133b113..1ffb367f94f 100644
> --- a/include/hw/scsi/scsi.h
> +++ b/include/hw/scsi/scsi.h
> @@ -30,7 +30,7 @@ struct SCSIRequest {
>      int16_t           status;
>      int16_t           host_status;
>      void              *hba_private;
> -    size_t            residual;
> +    uint64_t          residual;

Oops I forgot to mention this change since the previous version.

>      SCSICommand       cmd;
>      NotifierList      cancel_notifiers;
>  
> diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
> index 45a2567848c..77a346d5ed1 100644
> --- a/include/sysemu/dma.h
> +++ b/include/sysemu/dma.h
> @@ -303,8 +303,10 @@ BlockAIOCB *dma_blk_read(BlockBackend *blk,
>  BlockAIOCB *dma_blk_write(BlockBackend *blk,
>                            QEMUSGList *sg, uint64_t offset, uint32_t align,
>                            BlockCompletionFunc *cb, void *opaque);
> -uint64_t dma_buf_read(void *ptr, int32_t len, QEMUSGList *sg, MemTxAttrs attrs);
> -uint64_t dma_buf_write(void *ptr, int32_t len, QEMUSGList *sg, MemTxAttrs attrs);
> +MemTxResult dma_buf_read(void *ptr, dma_size_t len, dma_size_t *residual,
> +                         QEMUSGList *sg, MemTxAttrs attrs);
> +MemTxResult dma_buf_write(void *ptr, dma_size_t len, dma_size_t *residual,
> +                          QEMUSGList *sg, MemTxAttrs attrs);
>  



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 7/8] hw/dma: Introduce dma_size_t type definition
  2021-12-31 11:49 ` [PATCH 7/8] hw/dma: Introduce dma_size_t type definition Philippe Mathieu-Daudé
@ 2022-01-03 10:20   ` David Hildenbrand
  2022-01-04  8:50     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 15+ messages in thread
From: David Hildenbrand @ 2022-01-03 10:20 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, Peter Xu, Gerd Hoffmann, Michael S. Tsirkin


>  /*
>   * When an IOMMU is present, bus addresses become distinct from
>   * CPU/memory physical addresses and may be a different size.  Because
> @@ -39,9 +28,22 @@ struct QEMUSGList {
>   * least most) cases.
>   */
>  typedef uint64_t dma_addr_t;
> +typedef uint64_t dma_size_t;

This is a bit inconsistent with other address types (hwaddr,
ram_addr_t), no?

-> git grep "_size_t"

What sticks out are "external" mach_vm_size_t and png_size_t.

To me, it logically makes sense that both types are equal, because we're
operating on the same address space (with the same size).

So at least I don't see the benefit here, but I'd love to be enlightened  :)

-- 
Thanks,

David / dhildenb



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/8] hw/pci: Document pci_dma_map()
  2021-12-31 11:48 ` [PATCH 3/8] hw/pci: Document pci_dma_map() Philippe Mathieu-Daudé
@ 2022-01-04  7:25   ` Peter Xu
  2022-01-04  8:36     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Xu @ 2022-01-04  7:25 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: David Hildenbrand, Michael S. Tsirkin, qemu-devel, Gerd Hoffmann,
	Paolo Bonzini

On Fri, Dec 31, 2021 at 12:48:56PM +0100, Philippe Mathieu-Daudé wrote:
> +/**
> + * pci_dma_map: Map a physical memory region into a device PCI address space.

Shouldn't this be: "Map device PCI address space range into host virtual
address"?

-- 
Peter Xu



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/8] hw/pci: Document pci_dma_map()
  2022-01-04  7:25   ` Peter Xu
@ 2022-01-04  8:36     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-01-04  8:36 UTC (permalink / raw)
  To: Peter Xu
  Cc: Paolo Bonzini, Michael S. Tsirkin, Gerd Hoffmann,
	qemu-devel@nongnu.org Developers, David Hildenbrand

On Tue, Jan 4, 2022 at 8:27 AM Peter Xu <peterx@redhat.com> wrote:
>
> On Fri, Dec 31, 2021 at 12:48:56PM +0100, Philippe Mathieu-Daudé wrote:
> > +/**
> > + * pci_dma_map: Map a physical memory region into a device PCI address space.
>
> Shouldn't this be: "Map device PCI address space range into host virtual
> address"?

Yes, certainly... Thank you!


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 7/8] hw/dma: Introduce dma_size_t type definition
  2022-01-03 10:20   ` David Hildenbrand
@ 2022-01-04  8:50     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-01-04  8:50 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Paolo Bonzini, Michael S. Tsirkin,
	qemu-devel@nongnu.org Developers, Peter Xu, Gerd Hoffmann

On Mon, Jan 3, 2022 at 11:22 AM David Hildenbrand <david@redhat.com> wrote:
> >  /*
> >   * When an IOMMU is present, bus addresses become distinct from
> >   * CPU/memory physical addresses and may be a different size.  Because
> > @@ -39,9 +28,22 @@ struct QEMUSGList {
> >   * least most) cases.
> >   */
> >  typedef uint64_t dma_addr_t;
> > +typedef uint64_t dma_size_t;
>
> This is a bit inconsistent with other address types (hwaddr,
> ram_addr_t), no?

Indeed.

> -> git grep "_size_t"
>
> What sticks out are "external" mach_vm_size_t and png_size_t.
>
> To me, it logically makes sense that both types are equal, because we're
> operating on the same address space (with the same size).
>
> So at least I don't see the benefit here, but I'd love to be enlightened  :)

No, you are right, I was not convinced either. I'll stick to dma_addr_t.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/8] hw/rdma/rdma_utils: Rename rdma_pci_dma_map 'len' argument
  2021-12-31 11:48 ` [PATCH 5/8] hw/rdma/rdma_utils: Rename rdma_pci_dma_map 'len' argument Philippe Mathieu-Daudé
@ 2022-01-18 10:13   ` Yuval Shaia
  0 siblings, 0 replies; 15+ messages in thread
From: Yuval Shaia @ 2022-01-18 10:13 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: David Hildenbrand, Michael S. Tsirkin, QEMU Developers, Peter Xu,
	Gerd Hoffmann, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 2550 bytes --]

On Fri, 31 Dec 2021 at 13:54, Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> Various APIs use 'pval' naming for 'pointer to val'.
> rdma_pci_dma_map() uses 'plen' for 'PCI length', but since
> 'PCI' is already explicit in the function name, simplify
> and rename the argument 'len'. No logical change.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/rdma/rdma_utils.h |  2 +-
>  hw/rdma/rdma_utils.c | 14 +++++++-------
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/hw/rdma/rdma_utils.h b/hw/rdma/rdma_utils.h
> index 9fd0efd940b..0c6414e7e0a 100644
> --- a/hw/rdma/rdma_utils.h
> +++ b/hw/rdma/rdma_utils.h
> @@ -38,7 +38,7 @@ typedef struct RdmaProtectedGSList {
>      GSList *list;
>  } RdmaProtectedGSList;
>
> -void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t plen);
> +void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t len);
>  void rdma_pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len);
>  void rdma_protected_gqueue_init(RdmaProtectedGQueue *list);
>  void rdma_protected_gqueue_destroy(RdmaProtectedGQueue *list);
> diff --git a/hw/rdma/rdma_utils.c b/hw/rdma/rdma_utils.c
> index 98df58f6897..61cb8ede0fd 100644
> --- a/hw/rdma/rdma_utils.c
> +++ b/hw/rdma/rdma_utils.c
> @@ -17,29 +17,29 @@
>  #include "trace.h"
>  #include "rdma_utils.h"
>
> -void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t plen)
> +void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t len)
>  {
>      void *p;
> -    hwaddr len = plen;
> +    hwaddr pci_len = len;
>
>      if (!addr) {
>          rdma_error_report("addr is NULL");
>          return NULL;
>      }
>
> -    p = pci_dma_map(dev, addr, &len, DMA_DIRECTION_TO_DEVICE);
> +    p = pci_dma_map(dev, addr, &pci_len, DMA_DIRECTION_TO_DEVICE);
>      if (!p) {
>          rdma_error_report("pci_dma_map fail, addr=0x%"PRIx64",
> len=%"PRId64,
> -                          addr, len);
> +                          addr, pci_len);
>          return NULL;
>      }
>
> -    if (len != plen) {
> -        rdma_pci_dma_unmap(dev, p, len);
> +    if (pci_len != len) {
> +        rdma_pci_dma_unmap(dev, p, pci_len);
>          return NULL;
>      }
>
> -    trace_rdma_pci_dma_map(addr, p, len);
> +    trace_rdma_pci_dma_map(addr, p, pci_len);
>
>      return p;
>  }
>

Reviewed-by: Yuval Shaia <yuval.shaia.ml@gmail.com>
Tested-by: Yuval Shaia <yuval.shaia.ml@gmail.com>


> --
> 2.33.1
>
>
>

[-- Attachment #2: Type: text/html, Size: 3578 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-01-18 10:18 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-31 11:48 [PATCH 0/8] hw/dma: Introduce dma_size_t type definition Philippe Mathieu-Daudé
2021-12-31 11:48 ` [PATCH 1/8] hw/nvram: Restrict stub to sysemu and tools Philippe Mathieu-Daudé
2021-12-31 11:48 ` [PATCH 2/8] hw/pci: Restrict pci-bus stub to sysemu Philippe Mathieu-Daudé
2021-12-31 11:48 ` [PATCH 3/8] hw/pci: Document pci_dma_map() Philippe Mathieu-Daudé
2022-01-04  7:25   ` Peter Xu
2022-01-04  8:36     ` Philippe Mathieu-Daudé
2021-12-31 11:48 ` [PATCH 4/8] hw/dma: Remove CONFIG_USER_ONLY check Philippe Mathieu-Daudé
2021-12-31 11:48 ` [PATCH 5/8] hw/rdma/rdma_utils: Rename rdma_pci_dma_map 'len' argument Philippe Mathieu-Daudé
2022-01-18 10:13   ` Yuval Shaia
2021-12-31 11:48 ` [PATCH 6/8] hw/scsi: Rename SCSIRequest::resid as 'residual' Philippe Mathieu-Daudé
2021-12-31 11:49 ` [PATCH 7/8] hw/dma: Introduce dma_size_t type definition Philippe Mathieu-Daudé
2022-01-03 10:20   ` David Hildenbrand
2022-01-04  8:50     ` Philippe Mathieu-Daudé
2021-12-31 11:49 ` [PATCH 8/8] hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResult Philippe Mathieu-Daudé
2021-12-31 11:51   ` Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).