Re: [PATCH v2 9/9] hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResult

[Philippe Mathieu-Daudé]

On 1/10/22 09:51, David Hildenbrand wrote:
> On 04.01.22 09:54, Philippe Mathieu-Daudé wrote:
>> From: 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.
> 
> I feel like this patch doesn't fit into the context of this series.
> Especially as the cover letter mentiones "Finally we replace misuses
> with dma_addr_t typedef."
> 
> Am I missing something?

OK, I will simply repost it once this series gets merged.

Re: [PATCH v2 9/9] hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResult

[David Hildenbrand]

On 04.01.22 09:54, Philippe Mathieu-Daudé wrote:
> From: 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.

I feel like this patch doesn't fit into the context of this series.
Especially as the cover letter mentiones "Finally we replace misuses
with dma_addr_t typedef."

Am I missing something?

-- 
Thanks,

David / dhildenb

[PATCH v2 9/9] hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResult

[Philippe Mathieu-Daudé]

From: 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 
Signed-off-by: Philippe Mathieu-Daudé 
Signed-off-by: Philippe Mathieu-Daudé 
---
 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_tresidual;
+uint64_t  residual;
 SCSICommand   cmd;
 NotifierList  cancel_notifiers;
 
diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
index 7a8ae4fcd0b..a1ac5bc1b54 100644
--- a/include/sysemu/dma.h
+++ b/include/sysemu/dma.h
@@ -301,8 +301,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_addr_t len, dma_addr_t *residual,
+ QEMUSGList *sg, MemTxAttrs attrs);
+MemTxResult dma_buf_write(void *ptr, dma_addr_t len, dma_addr_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 6c727dd0c08..7ce001cacdb 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 c3c49176110..1f62116af98 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_addr_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 6c1ae6b980f..de613c8b355 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_addr_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_addr_t residual;
 
 memset(&info, 0x0, dcmd_size);
 if (cmd->iov_size < dcmd_siz