Re: [RFC PATCH 12/12] dma: Assert when device writes to indirect memory (such MMIO regions)

2020-09-03 Thread Edgar E. Iglesias
On Thu, Sep 03, 2020 at 01:08:31PM +0200, Philippe Mathieu-Daudé wrote:
> Assert DMA accesses are done on direct memory (in particular
> to catch invalid accesses to MMIO regions).

Hi Philippe,

Is the motivation for this to make it easier to find DMA programming errors?
Shouldn't guest SW use the IOMMU/DMA-APIs to debug those?

There're valid use-cases where DMA devices target non-memory, in particular
in the embedded space but also on PCI systems.

Also, since guest SW programs the DMA registers, guest SW would be able to trig 
this assert at will...

Cheers,
Edgar




> 
> Example with the reproducer from LP#1886362 (see previous commit):
> 
>   qemu-system-i386: include/sysemu/dma.h:111: int dma_memory_rw(AddressSpace 
> *, dma_addr_t, void *, dma_addr_t, DMADirection, MemTxAttrs): Assertion `dir 
> == DMA_DIRECTION_TO_DEVICE || attrs.direct_access' failed.
>   (gdb) bt
>   #0  0x751d69e5 in raise () at /lib64/libc.so.6
>   #1  0x751bf895 in abort () at /lib64/libc.so.6
>   #2  0x751bf769 in _nl_load_domain.cold () at /lib64/libc.so.6
>   #3  0x751cee76 in annobin_assert.c_end () at /lib64/libc.so.6
>   #4  0x57b48a94 in dma_memory_rw (as=0x7fffddd3ca28, addr=4064, 
> buf=0x7fff7780, len=16, dir=DMA_DIRECTION_FROM_DEVICE, attrs=...) at 
> /home/phil/source/qemu/include/sysemu/dma.h:111
>   #5  0x57b487e0 in pci_dma_rw (dev=0x7fffddd3c800, addr=4064, 
> buf=0x7fff7780, len=16, dir=DMA_DIRECTION_FROM_DEVICE) at 
> /home/phil/source/qemu/include/hw/pci/pci.h:791
>   #6  0x57b47373 in pci_dma_write (dev=0x7fffddd3c800, addr=4064, 
> buf=0x7fff7780, len=16) at /home/phil/source/qemu/include/hw/pci/pci.h:804
>   #7  0x57b340b4 in e1000e_write_packet_to_guest 
> (core=0x7fffddd3f4e0, pkt=0x6116c740, rxr=0x7fff7cf0, 
> rss_info=0x7fff7d10) at hw/net/e1000e_core.c:1609
>   #8  0x57b30739 in e1000e_receive_iov (core=0x7fffddd3f4e0, 
> iov=0x61960e80, iovcnt=4) at hw/net/e1000e_core.c:1709
>   #9  0x576e2069 in e1000e_nc_receive_iov (nc=0x6140a060, 
> iov=0x61960e80, iovcnt=4) at hw/net/e1000e.c:213
>   #10 0x572a3c34 in net_tx_pkt_sendv (pkt=0x63128800, 
> nc=0x6140a060, iov=0x61960e80, iov_cnt=4) at hw/net/net_tx_pkt.c:556
>   #11 0x572a23e2 in net_tx_pkt_send (pkt=0x63128800, 
> nc=0x6140a060) at hw/net/net_tx_pkt.c:633
>   #12 0x572a4c67 in net_tx_pkt_send_loopback (pkt=0x63128800, 
> nc=0x6140a060) at hw/net/net_tx_pkt.c:646
>   #13 0x57b70b05 in e1000e_tx_pkt_send (core=0x7fffddd3f4e0, 
> tx=0x7fffddd5f748, queue_index=0) at hw/net/e1000e_core.c:664
>   #14 0x57b6eab8 in e1000e_process_tx_desc (core=0x7fffddd3f4e0, 
> tx=0x7fffddd5f748, dp=0x7fff8680, queue_index=0) at 
> hw/net/e1000e_core.c:743
>   #15 0x57b6d65d in e1000e_start_xmit (core=0x7fffddd3f4e0, 
> txr=0x7fff88a0) at hw/net/e1000e_core.c:934
>   #16 0x57b5ea38 in e1000e_set_tctl (core=0x7fffddd3f4e0, index=256, 
> val=255) at hw/net/e1000e_core.c:2431
>   #17 0x57b369ef in e1000e_core_write (core=0x7fffddd3f4e0, 
> addr=1027, val=255, size=4) at hw/net/e1000e_core.c:3265
>   #18 0x576de3be in e1000e_mmio_write (opaque=0x7fffddd3c800, 
> addr=1027, val=255, size=4) at hw/net/e1000e.c:109
>   #19 0x58e6b789 in memory_region_write_accessor (mr=0x7fffddd3f110, 
> addr=1027, value=0x7fff8eb0, size=4, shift=0, mask=4294967295, attrs=...) 
> at softmmu/memory.c:483
>   #20 0x58e6b05b in access_with_adjusted_size (addr=1027, 
> value=0x7fff8eb0, size=1, access_size_min=4, access_size_max=4, 
> access_fn= 0x58e6b120 , mr=0x7fffddd3f110, 
> attrs=...) at softmmu/memory.c:544
>   #21 0x58e69776 in memory_region_dispatch_write (mr=0x7fffddd3f110, 
> addr=1027, data=255, op=MO_8, attrs=...) at softmmu/memory.c:1465
>   #22 0x58f60462 in flatview_write_continue (fv=0x6063f9e0, 
> addr=3775005699, attrs=..., ptr=0x602e3710, len=1, addr1=1027, l=1, 
> mr=0x7fffddd3f110) at exec.c:3176
>   #23 0x58f4e38b in flatview_write (fv=0x6063f9e0, 
> addr=3775005699, attrs=..., buf=0x602e3710, len=1) at exec.c:3220
>   #24 0x58f4dd4f in address_space_write (as=0x6080baa0, 
> addr=3775005699, attrs=..., buf=0x602e3710, len=1) at exec.c:3315
>   #25 0x5916b3e0 in qtest_process_command (chr=0x5c03f300 
> , words=0x60458150) at softmmu/qtest.c:567
>   #26 0x5915f7f2 in qtest_process_inbuf (chr=0x5c03f300 
> , inbuf=0x619200e0) at softmmu/qtest.c:710
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  include/sysemu/dma.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
> index 8a7dbf0b0f3..a4ba9438a56 100644
> --- a/include/sysemu/dma.h
> +++ b/include/sysemu/dma.h
> @@ -108,6 +108,8 @@ static inline int dma_memory_rw(AddressSpace *as, 
> dma_addr_t addr,
>  

[RFC PATCH 12/12] dma: Assert when device writes to indirect memory (such MMIO regions)

2020-09-03 Thread Philippe Mathieu-Daudé
Assert DMA accesses are done on direct memory (in particular
to catch invalid accesses to MMIO regions).

Example with the reproducer from LP#1886362 (see previous commit):

  qemu-system-i386: include/sysemu/dma.h:111: int dma_memory_rw(AddressSpace *, 
dma_addr_t, void *, dma_addr_t, DMADirection, MemTxAttrs): Assertion `dir == 
DMA_DIRECTION_TO_DEVICE || attrs.direct_access' failed.
  (gdb) bt
  #0  0x751d69e5 in raise () at /lib64/libc.so.6
  #1  0x751bf895 in abort () at /lib64/libc.so.6
  #2  0x751bf769 in _nl_load_domain.cold () at /lib64/libc.so.6
  #3  0x751cee76 in annobin_assert.c_end () at /lib64/libc.so.6
  #4  0x57b48a94 in dma_memory_rw (as=0x7fffddd3ca28, addr=4064, 
buf=0x7fff7780, len=16, dir=DMA_DIRECTION_FROM_DEVICE, attrs=...) at 
/home/phil/source/qemu/include/sysemu/dma.h:111
  #5  0x57b487e0 in pci_dma_rw (dev=0x7fffddd3c800, addr=4064, 
buf=0x7fff7780, len=16, dir=DMA_DIRECTION_FROM_DEVICE) at 
/home/phil/source/qemu/include/hw/pci/pci.h:791
  #6  0x57b47373 in pci_dma_write (dev=0x7fffddd3c800, addr=4064, 
buf=0x7fff7780, len=16) at /home/phil/source/qemu/include/hw/pci/pci.h:804
  #7  0x57b340b4 in e1000e_write_packet_to_guest (core=0x7fffddd3f4e0, 
pkt=0x6116c740, rxr=0x7fff7cf0, rss_info=0x7fff7d10) at 
hw/net/e1000e_core.c:1609
  #8  0x57b30739 in e1000e_receive_iov (core=0x7fffddd3f4e0, 
iov=0x61960e80, iovcnt=4) at hw/net/e1000e_core.c:1709
  #9  0x576e2069 in e1000e_nc_receive_iov (nc=0x6140a060, 
iov=0x61960e80, iovcnt=4) at hw/net/e1000e.c:213
  #10 0x572a3c34 in net_tx_pkt_sendv (pkt=0x63128800, 
nc=0x6140a060, iov=0x61960e80, iov_cnt=4) at hw/net/net_tx_pkt.c:556
  #11 0x572a23e2 in net_tx_pkt_send (pkt=0x63128800, 
nc=0x6140a060) at hw/net/net_tx_pkt.c:633
  #12 0x572a4c67 in net_tx_pkt_send_loopback (pkt=0x63128800, 
nc=0x6140a060) at hw/net/net_tx_pkt.c:646
  #13 0x57b70b05 in e1000e_tx_pkt_send (core=0x7fffddd3f4e0, 
tx=0x7fffddd5f748, queue_index=0) at hw/net/e1000e_core.c:664
  #14 0x57b6eab8 in e1000e_process_tx_desc (core=0x7fffddd3f4e0, 
tx=0x7fffddd5f748, dp=0x7fff8680, queue_index=0) at hw/net/e1000e_core.c:743
  #15 0x57b6d65d in e1000e_start_xmit (core=0x7fffddd3f4e0, 
txr=0x7fff88a0) at hw/net/e1000e_core.c:934
  #16 0x57b5ea38 in e1000e_set_tctl (core=0x7fffddd3f4e0, index=256, 
val=255) at hw/net/e1000e_core.c:2431
  #17 0x57b369ef in e1000e_core_write (core=0x7fffddd3f4e0, addr=1027, 
val=255, size=4) at hw/net/e1000e_core.c:3265
  #18 0x576de3be in e1000e_mmio_write (opaque=0x7fffddd3c800, 
addr=1027, val=255, size=4) at hw/net/e1000e.c:109
  #19 0x58e6b789 in memory_region_write_accessor (mr=0x7fffddd3f110, 
addr=1027, value=0x7fff8eb0, size=4, shift=0, mask=4294967295, attrs=...) 
at softmmu/memory.c:483
  #20 0x58e6b05b in access_with_adjusted_size (addr=1027, 
value=0x7fff8eb0, size=1, access_size_min=4, access_size_max=4, access_fn= 
0x58e6b120 , mr=0x7fffddd3f110, attrs=...) at 
softmmu/memory.c:544
  #21 0x58e69776 in memory_region_dispatch_write (mr=0x7fffddd3f110, 
addr=1027, data=255, op=MO_8, attrs=...) at softmmu/memory.c:1465
  #22 0x58f60462 in flatview_write_continue (fv=0x6063f9e0, 
addr=3775005699, attrs=..., ptr=0x602e3710, len=1, addr1=1027, l=1, 
mr=0x7fffddd3f110) at exec.c:3176
  #23 0x58f4e38b in flatview_write (fv=0x6063f9e0, addr=3775005699, 
attrs=..., buf=0x602e3710, len=1) at exec.c:3220
  #24 0x58f4dd4f in address_space_write (as=0x6080baa0, 
addr=3775005699, attrs=..., buf=0x602e3710, len=1) at exec.c:3315
  #25 0x5916b3e0 in qtest_process_command (chr=0x5c03f300 
, words=0x60458150) at softmmu/qtest.c:567
  #26 0x5915f7f2 in qtest_process_inbuf (chr=0x5c03f300 
, inbuf=0x619200e0) at softmmu/qtest.c:710

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/sysemu/dma.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
index 8a7dbf0b0f3..a4ba9438a56 100644
--- a/include/sysemu/dma.h
+++ b/include/sysemu/dma.h
@@ -108,6 +108,8 @@ static inline int dma_memory_rw(AddressSpace *as, 
dma_addr_t addr,
 void *buf, dma_addr_t len,
 DMADirection dir, MemTxAttrs attrs)
 {
+assert(dir == DMA_DIRECTION_TO_DEVICE || attrs.direct_access);
+
 dma_barrier(as, dir);
 
 return dma_memory_rw_relaxed(as, addr, buf, len, dir, attrs);
-- 
2.26.2