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

[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  0x00007ffff51d69e5 in raise () at /lib64/libc.so.6
>   #1  0x00007ffff51bf895 in abort () at /lib64/libc.so.6
>   #2  0x00007ffff51bf769 in _nl_load_domain.cold () at /lib64/libc.so.6
>   #3  0x00007ffff51cee76 in annobin_assert.c_end () at /lib64/libc.so.6
>   #4  0x0000555557b48a94 in dma_memory_rw (as=0x7fffddd3ca28, addr=4064, 
> buf=0x7fffffff7780, len=16, dir=DMA_DIRECTION_FROM_DEVICE, attrs=...) at 
> /home/phil/source/qemu/include/sysemu/dma.h:111
>   #5  0x0000555557b487e0 in pci_dma_rw (dev=0x7fffddd3c800, addr=4064, 
> buf=0x7fffffff7780, len=16, dir=DMA_DIRECTION_FROM_DEVICE) at 
> /home/phil/source/qemu/include/hw/pci/pci.h:791
>   #6  0x0000555557b47373 in pci_dma_write (dev=0x7fffddd3c800, addr=4064, 
> buf=0x7fffffff7780, len=16) at /home/phil/source/qemu/include/hw/pci/pci.h:804
>   #7  0x0000555557b340b4 in e1000e_write_packet_to_guest 
> (core=0x7fffddd3f4e0, pkt=0x61100006c740, rxr=0x7fffffff7cf0, 
> rss_info=0x7fffffff7d10) at hw/net/e1000e_core.c:1609
>   #8  0x0000555557b30739 in e1000e_receive_iov (core=0x7fffddd3f4e0, 
> iov=0x619000060e80, iovcnt=4) at hw/net/e1000e_core.c:1709
>   #9  0x00005555576e2069 in e1000e_nc_receive_iov (nc=0x61400000a060, 
> iov=0x619000060e80, iovcnt=4) at hw/net/e1000e.c:213
>   #10 0x00005555572a3c34 in net_tx_pkt_sendv (pkt=0x631000028800, 
> nc=0x61400000a060, iov=0x619000060e80, iov_cnt=4) at hw/net/net_tx_pkt.c:556
>   #11 0x00005555572a23e2 in net_tx_pkt_send (pkt=0x631000028800, 
> nc=0x61400000a060) at hw/net/net_tx_pkt.c:633
>   #12 0x00005555572a4c67 in net_tx_pkt_send_loopback (pkt=0x631000028800, 
> nc=0x61400000a060) at hw/net/net_tx_pkt.c:646
>   #13 0x0000555557b70b05 in e1000e_tx_pkt_send (core=0x7fffddd3f4e0, 
> tx=0x7fffddd5f748, queue_index=0) at hw/net/e1000e_core.c:664
>   #14 0x0000555557b6eab8 in e1000e_process_tx_desc (core=0x7fffddd3f4e0, 
> tx=0x7fffddd5f748, dp=0x7fffffff8680, queue_index=0) at 
> hw/net/e1000e_core.c:743
>   #15 0x0000555557b6d65d in e1000e_start_xmit (core=0x7fffddd3f4e0, 
> txr=0x7fffffff88a0) at hw/net/e1000e_core.c:934
>   #16 0x0000555557b5ea38 in e1000e_set_tctl (core=0x7fffddd3f4e0, index=256, 
> val=255) at hw/net/e1000e_core.c:2431
>   #17 0x0000555557b369ef in e1000e_core_write (core=0x7fffddd3f4e0, 
> addr=1027, val=255, size=4) at hw/net/e1000e_core.c:3265
>   #18 0x00005555576de3be in e1000e_mmio_write (opaque=0x7fffddd3c800, 
> addr=1027, val=255, size=4) at hw/net/e1000e.c:109
>   #19 0x0000555558e6b789 in memory_region_write_accessor (mr=0x7fffddd3f110, 
> addr=1027, value=0x7fffffff8eb0, size=4, shift=0, mask=4294967295, attrs=...) 
> at softmmu/memory.c:483
>   #20 0x0000555558e6b05b in access_with_adjusted_size (addr=1027, 
> value=0x7fffffff8eb0, size=1, access_size_min=4, access_size_max=4, 
> access_fn= 0x555558e6b120 <memory_region_write_accessor>, mr=0x7fffddd3f110, 
> attrs=...) at softmmu/memory.c:544
>   #21 0x0000555558e69776 in memory_region_dispatch_write (mr=0x7fffddd3f110, 
> addr=1027, data=255, op=MO_8, attrs=...) at softmmu/memory.c:1465
>   #22 0x0000555558f60462 in flatview_write_continue (fv=0x60600003f9e0, 
> addr=3775005699, attrs=..., ptr=0x6020000e3710, len=1, addr1=1027, l=1, 
> mr=0x7fffddd3f110) at exec.c:3176
>   #23 0x0000555558f4e38b in flatview_write (fv=0x60600003f9e0, 
> addr=3775005699, attrs=..., buf=0x6020000e3710, len=1) at exec.c:3220
>   #24 0x0000555558f4dd4f in address_space_write (as=0x60800000baa0, 
> addr=3775005699, attrs=..., buf=0x6020000e3710, len=1) at exec.c:3315
>   #25 0x000055555916b3e0 in qtest_process_command (chr=0x55555c03f300 
> <qtest_chr>, words=0x604000058150) at softmmu/qtest.c:567
>   #26 0x000055555915f7f2 in qtest_process_inbuf (chr=0x55555c03f300 
> <qtest_chr>, inbuf=0x6190000200e0) at softmmu/qtest.c:710
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  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
>