* [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions
@ 2020-09-03 11:08 Philippe Mathieu-Daudé
2020-09-03 11:08 ` [PATCH 01/12] pci: pass along the return value of dma_memory_rw Philippe Mathieu-Daudé
` (14 more replies)
0 siblings, 15 replies; 29+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-03 11:08 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland,
Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost,
Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota,
Peter Chubb, Joel Stanley, Richard Henderson, Laszlo Ersek,
Robert Foley, Alistair Francis, Richard Henderson,
Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka,
Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson,
Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery,
Philippe Mathieu-Daudé,
Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé,
Andrew Baumann, qemu-ppc, Klaus Jensen, Paolo Bonzini
Hi,
I'm not suppose to work on this but I couldn't sleep so kept
wondering about this problem the whole night and eventually
woke up to write this quickly, so comments are scarce, sorry.
The first part is obvious anyway, simply pass MemTxAttrs argument.
The main patch is:
"exec/memattrs: Introduce MemTxAttrs::direct_access field".
This way we can restrict accesses to ROM/RAM by setting the
'direct_access' field. Illegal accesses return MEMTX_BUS_ERROR.
Next patch restrict PCI DMA accesses by setting the direct_access
field.
Finally we add an assertion for any DMA write access to indirect
memory to kill a class of bug recently found by Alexander while
fuzzing.
Regards,
Phil.
Klaus Jensen (1):
pci: pass along the return value of dma_memory_rw
Philippe Mathieu-Daudé (11):
dma: Let dma_memory_valid() take MemTxAttrs argument
dma: Let dma_memory_set() take MemTxAttrs argument
dma: Let dma_memory_rw_relaxed() take MemTxAttrs argument
dma: Let dma_memory_rw() take MemTxAttrs argument
dma: Let dma_memory_read/write() take MemTxAttrs argument
dma: Let dma_memory_map() take MemTxAttrs argument
docs/devel/loads-stores: Add regexp for DMA functions
dma: Let load/store DMA functions take MemTxAttrs argument
exec/memattrs: Introduce MemTxAttrs::direct_access field
hw/pci: Only allow PCI slave devices to write to direct memory
dma: Assert when device writes to indirect memory (such MMIO regions)
docs/devel/loads-stores.rst | 2 ++
include/exec/memattrs.h | 3 ++
include/hw/pci/pci.h | 21 ++++++++++---
include/hw/ppc/spapr_vio.h | 26 +++++++++------
include/sysemu/dma.h | 59 +++++++++++++++++++++--------------
dma-helpers.c | 12 ++++---
exec.c | 8 +++++
hw/arm/musicpal.c | 13 ++++----
hw/arm/smmu-common.c | 3 +-
hw/arm/smmuv3.c | 14 ++++++---
hw/core/generic-loader.c | 3 +-
hw/display/virtio-gpu.c | 8 +++--
hw/dma/pl330.c | 12 ++++---
hw/dma/sparc32_dma.c | 16 ++++++----
hw/dma/xlnx-zynq-devcfg.c | 6 ++--
hw/dma/xlnx_dpdma.c | 10 +++---
hw/hyperv/vmbus.c | 8 +++--
hw/i386/amd_iommu.c | 16 +++++-----
hw/i386/intel_iommu.c | 28 ++++++++++-------
hw/ide/ahci.c | 9 ++++--
hw/ide/macio.c | 2 +-
hw/intc/pnv_xive.c | 7 +++--
hw/intc/spapr_xive.c | 3 +-
hw/intc/xive.c | 7 +++--
hw/misc/bcm2835_property.c | 3 +-
hw/misc/macio/mac_dbdma.c | 10 +++---
hw/net/allwinner-sun8i-emac.c | 21 ++++++++-----
hw/net/ftgmac100.c | 25 +++++++++------
hw/net/imx_fec.c | 32 ++++++++++++-------
hw/nvram/fw_cfg.c | 16 ++++++----
hw/pci-host/pnv_phb3.c | 5 +--
hw/pci-host/pnv_phb3_msi.c | 9 ++++--
hw/pci-host/pnv_phb4.c | 7 +++--
hw/sd/allwinner-sdhost.c | 14 +++++----
hw/sd/sdhci.c | 35 +++++++++++++--------
hw/usb/hcd-dwc2.c | 8 ++---
hw/usb/hcd-ehci.c | 6 ++--
hw/usb/hcd-ohci.c | 28 ++++++++++-------
hw/usb/libhw.c | 3 +-
hw/virtio/virtio.c | 6 ++--
trace-events | 1 +
41 files changed, 334 insertions(+), 191 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 01/12] pci: pass along the return value of dma_memory_rw 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé @ 2020-09-03 11:08 ` Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 02/12] dma: Let dma_memory_valid() take MemTxAttrs argument Philippe Mathieu-Daudé ` (13 subsequent siblings) 14 siblings, 0 replies; 29+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-03 11:08 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, Keith Busch, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Philippe Mathieu-Daudé, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Paolo Bonzini From: Klaus Jensen <k.jensen@samsung.com> Some might actually care about the return value of dma_memory_rw. So let us pass it along instead of ignoring it. There are no existing users of the return value, so this patch should be safe. Signed-off-by: Klaus Jensen <k.jensen@samsung.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Acked-by: Keith Busch <kbusch@kernel.org> Message-Id: <20191011070141.188713-2-its@irrelevant.dk> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/hw/pci/pci.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 4ca7258b5b7..896cef9ad47 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -788,8 +788,7 @@ static inline AddressSpace *pci_get_address_space(PCIDevice *dev) static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr, void *buf, dma_addr_t len, DMADirection dir) { - dma_memory_rw(pci_get_address_space(dev), addr, buf, len, dir); - return 0; + return dma_memory_rw(pci_get_address_space(dev), addr, buf, len, dir); } static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr, -- 2.26.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 02/12] dma: Let dma_memory_valid() take MemTxAttrs argument 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 01/12] pci: pass along the return value of dma_memory_rw Philippe Mathieu-Daudé @ 2020-09-03 11:08 ` Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 03/12] dma: Let dma_memory_set() " Philippe Mathieu-Daudé ` (12 subsequent siblings) 14 siblings, 0 replies; 29+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-03 11:08 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Philippe Mathieu-Daudé, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Paolo Bonzini Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/hw/ppc/spapr_vio.h | 2 +- include/sysemu/dma.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h index bed7df60e35..f134f6cf574 100644 --- a/include/hw/ppc/spapr_vio.h +++ b/include/hw/ppc/spapr_vio.h @@ -96,7 +96,7 @@ static inline void spapr_vio_irq_pulse(SpaprVioDevice *dev) static inline bool spapr_vio_dma_valid(SpaprVioDevice *dev, uint64_t taddr, uint32_t size, DMADirection dir) { - return dma_memory_valid(&dev->as, taddr, size, dir); + return dma_memory_valid(&dev->as, taddr, size, dir, MEMTXATTRS_UNSPECIFIED); } static inline int spapr_vio_dma_read(SpaprVioDevice *dev, uint64_t taddr, diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h index 80c5bc3e02d..fe3999dba59 100644 --- a/include/sysemu/dma.h +++ b/include/sysemu/dma.h @@ -73,11 +73,11 @@ static inline void dma_barrier(AddressSpace *as, DMADirection dir) * dma_memory_{read,write}() and check for errors */ static inline bool dma_memory_valid(AddressSpace *as, dma_addr_t addr, dma_addr_t len, - DMADirection dir) + DMADirection dir, MemTxAttrs attrs) { return address_space_access_valid(as, addr, len, dir == DMA_DIRECTION_FROM_DEVICE, - MEMTXATTRS_UNSPECIFIED); + attrs); } static inline int dma_memory_rw_relaxed(AddressSpace *as, dma_addr_t addr, -- 2.26.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 03/12] dma: Let dma_memory_set() take MemTxAttrs argument 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 01/12] pci: pass along the return value of dma_memory_rw Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 02/12] dma: Let dma_memory_valid() take MemTxAttrs argument Philippe Mathieu-Daudé @ 2020-09-03 11:08 ` Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 04/12] dma: Let dma_memory_rw_relaxed() " Philippe Mathieu-Daudé ` (11 subsequent siblings) 14 siblings, 0 replies; 29+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-03 11:08 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Philippe Mathieu-Daudé, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Paolo Bonzini Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/hw/ppc/spapr_vio.h | 3 ++- include/sysemu/dma.h | 3 ++- dma-helpers.c | 6 +++--- hw/nvram/fw_cfg.c | 3 ++- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h index f134f6cf574..6e5c0840248 100644 --- a/include/hw/ppc/spapr_vio.h +++ b/include/hw/ppc/spapr_vio.h @@ -116,7 +116,8 @@ static inline int spapr_vio_dma_write(SpaprVioDevice *dev, uint64_t taddr, static inline int spapr_vio_dma_set(SpaprVioDevice *dev, uint64_t taddr, uint8_t c, uint32_t size) { - return (dma_memory_set(&dev->as, taddr, c, size) != 0) ? + return (dma_memory_set(&dev->as, taddr, + c, size, MEMTXATTRS_UNSPECIFIED) != 0) ? H_DEST_PARM : H_SUCCESS; } diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h index fe3999dba59..34f957cc278 100644 --- a/include/sysemu/dma.h +++ b/include/sysemu/dma.h @@ -123,7 +123,8 @@ static inline int dma_memory_write(AddressSpace *as, dma_addr_t addr, DMA_DIRECTION_FROM_DEVICE); } -int dma_memory_set(AddressSpace *as, dma_addr_t addr, uint8_t c, dma_addr_t len); +int dma_memory_set(AddressSpace *as, dma_addr_t addr, + uint8_t c, dma_addr_t len, MemTxAttrs attrs); static inline void *dma_memory_map(AddressSpace *as, dma_addr_t addr, dma_addr_t *len, diff --git a/dma-helpers.c b/dma-helpers.c index 41ef24a63b6..49d66716469 100644 --- a/dma-helpers.c +++ b/dma-helpers.c @@ -18,7 +18,8 @@ /* #define DEBUG_IOMMU */ -int dma_memory_set(AddressSpace *as, dma_addr_t addr, uint8_t c, dma_addr_t len) +int dma_memory_set(AddressSpace *as, dma_addr_t addr, + uint8_t c, dma_addr_t len, MemTxAttrs attrs) { dma_barrier(as, DMA_DIRECTION_FROM_DEVICE); @@ -30,8 +31,7 @@ int dma_memory_set(AddressSpace *as, dma_addr_t addr, uint8_t c, dma_addr_t len) memset(fillbuf, c, FILLBUF_SIZE); while (len > 0) { l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE; - error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED, - fillbuf, l); + error |= address_space_write(as, addr, attrs, fillbuf, l); len -= l; addr += l; } diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index f3a4728288e..a15de06a10c 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -397,7 +397,8 @@ static void fw_cfg_dma_transfer(FWCfgState *s) * tested before. */ if (read) { - if (dma_memory_set(s->dma_as, dma.address, 0, len)) { + if (dma_memory_set(s->dma_as, dma.address, 0, len, + MEMTXATTRS_UNSPECIFIED)) { dma.control |= FW_CFG_DMA_CTL_ERROR; } } -- 2.26.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 04/12] dma: Let dma_memory_rw_relaxed() take MemTxAttrs argument 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé ` (2 preceding siblings ...) 2020-09-03 11:08 ` [PATCH 03/12] dma: Let dma_memory_set() " Philippe Mathieu-Daudé @ 2020-09-03 11:08 ` Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 05/12] dma: Let dma_memory_rw() " Philippe Mathieu-Daudé ` (10 subsequent siblings) 14 siblings, 0 replies; 29+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-03 11:08 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Philippe Mathieu-Daudé, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Paolo Bonzini Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/sysemu/dma.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h index 34f957cc278..6068323e48f 100644 --- a/include/sysemu/dma.h +++ b/include/sysemu/dma.h @@ -82,23 +82,26 @@ static inline bool dma_memory_valid(AddressSpace *as, static inline int dma_memory_rw_relaxed(AddressSpace *as, dma_addr_t addr, void *buf, dma_addr_t len, - DMADirection dir) + DMADirection dir, MemTxAttrs attrs) { - return (bool)address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED, + return (bool)address_space_rw(as, addr, attrs, buf, len, dir == DMA_DIRECTION_FROM_DEVICE); } static inline int dma_memory_read_relaxed(AddressSpace *as, dma_addr_t addr, void *buf, dma_addr_t len) { - return dma_memory_rw_relaxed(as, addr, buf, len, DMA_DIRECTION_TO_DEVICE); + return dma_memory_rw_relaxed(as, addr, buf, len, + DMA_DIRECTION_TO_DEVICE, + MEMTXATTRS_UNSPECIFIED); } static inline int dma_memory_write_relaxed(AddressSpace *as, dma_addr_t addr, const void *buf, dma_addr_t len) { return dma_memory_rw_relaxed(as, addr, (void *)buf, len, - DMA_DIRECTION_FROM_DEVICE); + DMA_DIRECTION_FROM_DEVICE, + MEMTXATTRS_UNSPECIFIED); } static inline int dma_memory_rw(AddressSpace *as, dma_addr_t addr, @@ -107,7 +110,8 @@ static inline int dma_memory_rw(AddressSpace *as, dma_addr_t addr, { dma_barrier(as, dir); - return dma_memory_rw_relaxed(as, addr, buf, len, dir); + return dma_memory_rw_relaxed(as, addr, buf, len, dir, + MEMTXATTRS_UNSPECIFIED); } static inline int dma_memory_read(AddressSpace *as, dma_addr_t addr, -- 2.26.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 05/12] dma: Let dma_memory_rw() take MemTxAttrs argument 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé ` (3 preceding siblings ...) 2020-09-03 11:08 ` [PATCH 04/12] dma: Let dma_memory_rw_relaxed() " Philippe Mathieu-Daudé @ 2020-09-03 11:08 ` Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 06/12] dma: Let dma_memory_read/write() " Philippe Mathieu-Daudé ` (9 subsequent siblings) 14 siblings, 0 replies; 29+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-03 11:08 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Philippe Mathieu-Daudé, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Paolo Bonzini Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/hw/pci/pci.h | 3 ++- include/sysemu/dma.h | 10 +++++----- dma-helpers.c | 3 ++- hw/intc/spapr_xive.c | 3 ++- hw/usb/hcd-ohci.c | 10 ++++++---- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 896cef9ad47..0c3217e019c 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -788,7 +788,8 @@ static inline AddressSpace *pci_get_address_space(PCIDevice *dev) static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr, void *buf, dma_addr_t len, DMADirection dir) { - return dma_memory_rw(pci_get_address_space(dev), addr, buf, len, dir); + return dma_memory_rw(pci_get_address_space(dev), addr, buf, len, + dir, MEMTXATTRS_UNSPECIFIED); } static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr, diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h index 6068323e48f..f03edeab173 100644 --- a/include/sysemu/dma.h +++ b/include/sysemu/dma.h @@ -106,25 +106,25 @@ static inline int dma_memory_write_relaxed(AddressSpace *as, dma_addr_t addr, static inline int dma_memory_rw(AddressSpace *as, dma_addr_t addr, void *buf, dma_addr_t len, - DMADirection dir) + DMADirection dir, MemTxAttrs attrs) { dma_barrier(as, dir); - return dma_memory_rw_relaxed(as, addr, buf, len, dir, - MEMTXATTRS_UNSPECIFIED); + return dma_memory_rw_relaxed(as, addr, buf, len, dir, attrs); } static inline int dma_memory_read(AddressSpace *as, dma_addr_t addr, void *buf, dma_addr_t len) { - return dma_memory_rw(as, addr, buf, len, DMA_DIRECTION_TO_DEVICE); + return dma_memory_rw(as, addr, buf, len, + DMA_DIRECTION_TO_DEVICE, MEMTXATTRS_UNSPECIFIED); } static inline int dma_memory_write(AddressSpace *as, dma_addr_t addr, const void *buf, dma_addr_t len) { return dma_memory_rw(as, addr, (void *)buf, len, - DMA_DIRECTION_FROM_DEVICE); + DMA_DIRECTION_FROM_DEVICE, MEMTXATTRS_UNSPECIFIED); } int dma_memory_set(AddressSpace *as, dma_addr_t addr, diff --git a/dma-helpers.c b/dma-helpers.c index 49d66716469..50473bb1996 100644 --- a/dma-helpers.c +++ b/dma-helpers.c @@ -305,7 +305,8 @@ static uint64_t dma_buf_rw(uint8_t *ptr, int32_t len, QEMUSGList *sg, while (len > 0) { ScatterGatherEntry entry = sg->sg[sg_cur_index++]; int32_t xfer = MIN(len, entry.len); - dma_memory_rw(sg->as, entry.base, ptr, xfer, dir); + dma_memory_rw(sg->as, entry.base, ptr, xfer, dir, + MEMTXATTRS_UNSPECIFIED); ptr += xfer; len -= xfer; resid -= xfer; diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index 4bd0d606ba1..dbf73a8bf47 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -1666,7 +1666,8 @@ static target_ulong h_int_esb(PowerPCCPU *cpu, mmio_addr = xive->vc_base + xive_source_esb_mgmt(xsrc, lisn) + offset; if (dma_memory_rw(&address_space_memory, mmio_addr, &data, 8, - (flags & SPAPR_XIVE_ESB_STORE))) { + (flags & SPAPR_XIVE_ESB_STORE), + MEMTXATTRS_UNSPECIFIED)) { qemu_log_mask(LOG_GUEST_ERROR, "XIVE: failed to access ESB @0x%" HWADDR_PRIx "\n", mmio_addr); return H_HARDWARE; diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index 1e6e85e86a8..bac1adf439c 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -586,7 +586,8 @@ static int ohci_copy_td(OHCIState *ohci, struct ohci_td *td, if (n > len) n = len; - if (dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf, n, dir)) { + if (dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf, + n, dir, MEMTXATTRS_UNSPECIFIED)) { return -1; } if (n == len) { @@ -595,7 +596,7 @@ static int ohci_copy_td(OHCIState *ohci, struct ohci_td *td, ptr = td->be & ~0xfffu; buf += n; if (dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf, - len - n, dir)) { + len - n, dir, MEMTXATTRS_UNSPECIFIED)) { return -1; } return 0; @@ -613,7 +614,8 @@ static int ohci_copy_iso_td(OHCIState *ohci, if (n > len) n = len; - if (dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf, n, dir)) { + if (dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf, + n, dir, MEMTXATTRS_UNSPECIFIED)) { return -1; } if (n == len) { @@ -622,7 +624,7 @@ static int ohci_copy_iso_td(OHCIState *ohci, ptr = end_addr & ~0xfffu; buf += n; if (dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf, - len - n, dir)) { + len - n, dir, MEMTXATTRS_UNSPECIFIED)) { return -1; } return 0; -- 2.26.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 06/12] dma: Let dma_memory_read/write() take MemTxAttrs argument 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé ` (4 preceding siblings ...) 2020-09-03 11:08 ` [PATCH 05/12] dma: Let dma_memory_rw() " Philippe Mathieu-Daudé @ 2020-09-03 11:08 ` Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 07/12] dma: Let dma_memory_map() " Philippe Mathieu-Daudé ` (8 subsequent siblings) 14 siblings, 0 replies; 29+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-03 11:08 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Philippe Mathieu-Daudé, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Paolo Bonzini Patch created mechanically using spatch with this script: @@ expression E1, E2, E3, E4; @@ ( - dma_memory_read(E1, E2, E3, E4) + dma_memory_read(E1, E2, E3, E4, MEMTXATTRS_UNSPECIFIED) | - dma_memory_write(E1, E2, E3, E4) + dma_memory_write(E1, E2, E3, E4, MEMTXATTRS_UNSPECIFIED) ) Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/hw/ppc/spapr_vio.h | 6 ++++-- include/sysemu/dma.h | 18 ++++++++++-------- hw/arm/musicpal.c | 13 +++++++------ hw/arm/smmu-common.c | 3 ++- hw/arm/smmuv3.c | 14 +++++++++----- hw/core/generic-loader.c | 3 ++- hw/dma/pl330.c | 12 ++++++++---- hw/dma/sparc32_dma.c | 16 ++++++++++------ hw/dma/xlnx-zynq-devcfg.c | 6 ++++-- hw/dma/xlnx_dpdma.c | 10 ++++++---- hw/i386/amd_iommu.c | 16 +++++++++------- hw/i386/intel_iommu.c | 28 +++++++++++++++++----------- hw/ide/macio.c | 2 +- hw/intc/xive.c | 7 ++++--- hw/misc/bcm2835_property.c | 3 ++- hw/misc/macio/mac_dbdma.c | 10 ++++++---- hw/net/allwinner-sun8i-emac.c | 21 ++++++++++++++------- hw/net/ftgmac100.c | 25 ++++++++++++++++--------- hw/net/imx_fec.c | 32 ++++++++++++++++++++------------ hw/nvram/fw_cfg.c | 9 ++++++--- hw/pci-host/pnv_phb3.c | 5 +++-- hw/pci-host/pnv_phb3_msi.c | 9 ++++++--- hw/pci-host/pnv_phb4.c | 7 ++++--- hw/sd/allwinner-sdhost.c | 14 ++++++++------ hw/sd/sdhci.c | 35 ++++++++++++++++++++++------------- hw/usb/hcd-dwc2.c | 8 ++++---- hw/usb/hcd-ehci.c | 6 ++++-- hw/usb/hcd-ohci.c | 18 +++++++++++------- 28 files changed, 219 insertions(+), 137 deletions(-) diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h index 6e5c0840248..8168f4fc5a5 100644 --- a/include/hw/ppc/spapr_vio.h +++ b/include/hw/ppc/spapr_vio.h @@ -102,14 +102,16 @@ static inline bool spapr_vio_dma_valid(SpaprVioDevice *dev, uint64_t taddr, static inline int spapr_vio_dma_read(SpaprVioDevice *dev, uint64_t taddr, void *buf, uint32_t size) { - return (dma_memory_read(&dev->as, taddr, buf, size) != 0) ? + return (dma_memory_read(&dev->as, taddr, + buf, size, MEMTXATTRS_UNSPECIFIED) != 0) ? H_DEST_PARM : H_SUCCESS; } static inline int spapr_vio_dma_write(SpaprVioDevice *dev, uint64_t taddr, const void *buf, uint32_t size) { - return (dma_memory_write(&dev->as, taddr, buf, size) != 0) ? + return (dma_memory_write(&dev->as, taddr, + buf, size, MEMTXATTRS_UNSPECIFIED) != 0) ? H_DEST_PARM : H_SUCCESS; } diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h index f03edeab173..f0880b79e74 100644 --- a/include/sysemu/dma.h +++ b/include/sysemu/dma.h @@ -114,17 +114,19 @@ static inline int dma_memory_rw(AddressSpace *as, dma_addr_t addr, } static inline int dma_memory_read(AddressSpace *as, dma_addr_t addr, - void *buf, dma_addr_t len) + void *buf, dma_addr_t len, + MemTxAttrs attrs) { return dma_memory_rw(as, addr, buf, len, - DMA_DIRECTION_TO_DEVICE, MEMTXATTRS_UNSPECIFIED); + DMA_DIRECTION_TO_DEVICE, attrs); } static inline int dma_memory_write(AddressSpace *as, dma_addr_t addr, - const void *buf, dma_addr_t len) + const void *buf, dma_addr_t len, + MemTxAttrs attrs) { return dma_memory_rw(as, addr, (void *)buf, len, - DMA_DIRECTION_FROM_DEVICE, MEMTXATTRS_UNSPECIFIED); + DMA_DIRECTION_FROM_DEVICE, attrs); } int dma_memory_set(AddressSpace *as, dma_addr_t addr, @@ -156,7 +158,7 @@ static inline void dma_memory_unmap(AddressSpace *as, dma_addr_t addr) \ { \ uint##_bits##_t val; \ - dma_memory_read(as, addr, &val, (_bits) / 8); \ + dma_memory_read(as, addr, &val, (_bits) / 8, MEMTXATTRS_UNSPECIFIED); \ return _end##_bits##_to_cpu(val); \ } \ static inline void st##_sname##_##_end##_dma(AddressSpace *as, \ @@ -164,20 +166,20 @@ static inline void dma_memory_unmap(AddressSpace *as, uint##_bits##_t val) \ { \ val = cpu_to_##_end##_bits(val); \ - dma_memory_write(as, addr, &val, (_bits) / 8); \ + dma_memory_write(as, addr, &val, (_bits) / 8, MEMTXATTRS_UNSPECIFIED); \ } static inline uint8_t ldub_dma(AddressSpace *as, dma_addr_t addr) { uint8_t val; - dma_memory_read(as, addr, &val, 1); + dma_memory_read(as, addr, &val, 1, MEMTXATTRS_UNSPECIFIED); return val; } static inline void stb_dma(AddressSpace *as, dma_addr_t addr, uint8_t val) { - dma_memory_write(as, addr, &val, 1); + dma_memory_write(as, addr, &val, 1, MEMTXATTRS_UNSPECIFIED); } DEFINE_LDST_DMA(uw, w, 16, le); diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c index f2f4fc02649..5d9959e8ddd 100644 --- a/hw/arm/musicpal.c +++ b/hw/arm/musicpal.c @@ -187,13 +187,13 @@ static void eth_rx_desc_put(AddressSpace *dma_as, uint32_t addr, cpu_to_le16s(&desc->buffer_size); cpu_to_le32s(&desc->buffer); cpu_to_le32s(&desc->next); - dma_memory_write(dma_as, addr, desc, sizeof(*desc)); + dma_memory_write(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED); } static void eth_rx_desc_get(AddressSpace *dma_as, uint32_t addr, mv88w8618_rx_desc *desc) { - dma_memory_read(dma_as, addr, desc, sizeof(*desc)); + dma_memory_read(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED); le32_to_cpus(&desc->cmdstat); le16_to_cpus(&desc->bytes); le16_to_cpus(&desc->buffer_size); @@ -217,7 +217,7 @@ static ssize_t eth_receive(NetClientState *nc, const uint8_t *buf, size_t size) eth_rx_desc_get(&s->dma_as, desc_addr, &desc); if ((desc.cmdstat & MP_ETH_RX_OWN) && desc.buffer_size >= size) { dma_memory_write(&s->dma_as, desc.buffer + s->vlan_header, - buf, size); + buf, size, MEMTXATTRS_UNSPECIFIED); desc.bytes = size + s->vlan_header; desc.cmdstat &= ~MP_ETH_RX_OWN; s->cur_rx[i] = desc.next; @@ -243,13 +243,13 @@ static void eth_tx_desc_put(AddressSpace *dma_as, uint32_t addr, cpu_to_le16s(&desc->bytes); cpu_to_le32s(&desc->buffer); cpu_to_le32s(&desc->next); - dma_memory_write(dma_as, addr, desc, sizeof(*desc)); + dma_memory_write(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED); } static void eth_tx_desc_get(AddressSpace *dma_as, uint32_t addr, mv88w8618_tx_desc *desc) { - dma_memory_read(dma_as, addr, desc, sizeof(*desc)); + dma_memory_read(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED); le32_to_cpus(&desc->cmdstat); le16_to_cpus(&desc->res); le16_to_cpus(&desc->bytes); @@ -271,7 +271,8 @@ static void eth_send(mv88w8618_eth_state *s, int queue_index) if (desc.cmdstat & MP_ETH_TX_OWN) { len = desc.bytes; if (len < 2048) { - dma_memory_read(&s->dma_as, desc.buffer, buf, len); + dma_memory_read(&s->dma_as, desc.buffer, buf, len, + MEMTXATTRS_UNSPECIFIED); qemu_send_packet(qemu_get_queue(s->nic), buf, len); } desc.cmdstat &= ~MP_ETH_TX_OWN; diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c index 3838db13952..f42ee237204 100644 --- a/hw/arm/smmu-common.c +++ b/hw/arm/smmu-common.c @@ -188,7 +188,8 @@ static int get_pte(dma_addr_t baseaddr, uint32_t index, uint64_t *pte, dma_addr_t addr = baseaddr + index * sizeof(*pte); /* TODO: guarantee 64-bit single-copy atomicity */ - ret = dma_memory_read(&address_space_memory, addr, pte, sizeof(*pte)); + ret = dma_memory_read(&address_space_memory, addr, pte, sizeof(*pte), + MEMTXATTRS_UNSPECIFIED); if (ret != MEMTX_OK) { info->type = SMMU_PTW_ERR_WALK_EABT; diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index 0122700e725..71d0eea2af1 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -101,7 +101,8 @@ static inline MemTxResult queue_read(SMMUQueue *q, void *data) { dma_addr_t addr = Q_CONS_ENTRY(q); - return dma_memory_read(&address_space_memory, addr, data, q->entry_size); + return dma_memory_read(&address_space_memory, addr, data, q->entry_size, + MEMTXATTRS_UNSPECIFIED); } static MemTxResult queue_write(SMMUQueue *q, void *data) @@ -109,7 +110,8 @@ static MemTxResult queue_write(SMMUQueue *q, void *data) dma_addr_t addr = Q_PROD_ENTRY(q); MemTxResult ret; - ret = dma_memory_write(&address_space_memory, addr, data, q->entry_size); + ret = dma_memory_write(&address_space_memory, addr, data, q->entry_size, + MEMTXATTRS_UNSPECIFIED); if (ret != MEMTX_OK) { return ret; } @@ -283,7 +285,8 @@ static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr, STE *buf, trace_smmuv3_get_ste(addr); /* TODO: guarantee 64-bit single-copy atomicity */ - ret = dma_memory_read(&address_space_memory, addr, buf, sizeof(*buf)); + ret = dma_memory_read(&address_space_memory, addr, buf, sizeof(*buf), + MEMTXATTRS_UNSPECIFIED); if (ret != MEMTX_OK) { qemu_log_mask(LOG_GUEST_ERROR, "Cannot fetch pte at address=0x%"PRIx64"\n", addr); @@ -304,7 +307,8 @@ static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid, trace_smmuv3_get_cd(addr); /* TODO: guarantee 64-bit single-copy atomicity */ - ret = dma_memory_read(&address_space_memory, addr, buf, sizeof(*buf)); + ret = dma_memory_read(&address_space_memory, addr, buf, sizeof(*buf), + MEMTXATTRS_UNSPECIFIED); if (ret != MEMTX_OK) { qemu_log_mask(LOG_GUEST_ERROR, "Cannot fetch pte at address=0x%"PRIx64"\n", addr); @@ -409,7 +413,7 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, l1ptr = (dma_addr_t)(strtab_base + l1_ste_offset * sizeof(l1std)); /* TODO: guarantee 64-bit single-copy atomicity */ ret = dma_memory_read(&address_space_memory, l1ptr, &l1std, - sizeof(l1std)); + sizeof(l1std), MEMTXATTRS_UNSPECIFIED); if (ret != MEMTX_OK) { qemu_log_mask(LOG_GUEST_ERROR, "Could not read L1PTR at 0X%"PRIx64"\n", l1ptr); diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c index a242c076f69..ba48ebae993 100644 --- a/hw/core/generic-loader.c +++ b/hw/core/generic-loader.c @@ -57,7 +57,8 @@ static void generic_loader_reset(void *opaque) if (s->data_len) { assert(s->data_len < sizeof(s->data)); - dma_memory_write(s->cpu->as, s->addr, &s->data, s->data_len); + dma_memory_write(s->cpu->as, s->addr, &s->data, s->data_len, + MEMTXATTRS_UNSPECIFIED); } } diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c index 0bd63a43f50..0361524c067 100644 --- a/hw/dma/pl330.c +++ b/hw/dma/pl330.c @@ -1107,7 +1107,8 @@ static inline const PL330InsnDesc *pl330_fetch_insn(PL330Chan *ch) uint8_t opcode; int i; - dma_memory_read(&address_space_memory, ch->pc, &opcode, 1); + dma_memory_read(&address_space_memory, ch->pc, &opcode, 1, + MEMTXATTRS_UNSPECIFIED); for (i = 0; insn_desc[i].size; i++) { if ((opcode & insn_desc[i].opmask) == insn_desc[i].opcode) { return &insn_desc[i]; @@ -1121,7 +1122,8 @@ static inline void pl330_exec_insn(PL330Chan *ch, const PL330InsnDesc *insn) uint8_t buf[PL330_INSN_MAXSIZE]; assert(insn->size <= PL330_INSN_MAXSIZE); - dma_memory_read(&address_space_memory, ch->pc, buf, insn->size); + dma_memory_read(&address_space_memory, ch->pc, buf, insn->size, + MEMTXATTRS_UNSPECIFIED); insn->exec(ch, buf[0], &buf[1], insn->size - 1); } @@ -1185,7 +1187,8 @@ static int pl330_exec_cycle(PL330Chan *channel) if (q != NULL && q->len <= pl330_fifo_num_free(&s->fifo)) { int len = q->len - (q->addr & (q->len - 1)); - dma_memory_read(&address_space_memory, q->addr, buf, len); + dma_memory_read(&address_space_memory, q->addr, buf, len, + MEMTXATTRS_UNSPECIFIED); trace_pl330_exec_cycle(q->addr, len); if (trace_event_get_state_backends(TRACE_PL330_HEXDUMP)) { pl330_hexdump(buf, len); @@ -1216,7 +1219,8 @@ static int pl330_exec_cycle(PL330Chan *channel) fifo_res = pl330_fifo_get(&s->fifo, buf, len, q->tag); } if (fifo_res == PL330_FIFO_OK || q->z) { - dma_memory_write(&address_space_memory, q->addr, buf, len); + dma_memory_write(&address_space_memory, q->addr, buf, len, + MEMTXATTRS_UNSPECIFIED); trace_pl330_exec_cycle(q->addr, len); if (trace_event_get_state_backends(TRACE_PL330_HEXDUMP)) { pl330_hexdump(buf, len); diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c index bcd1626fbd5..00c22232ceb 100644 --- a/hw/dma/sparc32_dma.c +++ b/hw/dma/sparc32_dma.c @@ -81,11 +81,11 @@ void ledma_memory_read(void *opaque, hwaddr addr, addr |= s->dmaregs[3]; trace_ledma_memory_read(addr, len); if (do_bswap) { - dma_memory_read(&is->iommu_as, addr, buf, len); + dma_memory_read(&is->iommu_as, addr, buf, len, MEMTXATTRS_UNSPECIFIED); } else { addr &= ~1; len &= ~1; - dma_memory_read(&is->iommu_as, addr, buf, len); + dma_memory_read(&is->iommu_as, addr, buf, len, MEMTXATTRS_UNSPECIFIED); for(i = 0; i < len; i += 2) { bswap16s((uint16_t *)(buf + i)); } @@ -103,7 +103,8 @@ void ledma_memory_write(void *opaque, hwaddr addr, addr |= s->dmaregs[3]; trace_ledma_memory_write(addr, len); if (do_bswap) { - dma_memory_write(&is->iommu_as, addr, buf, len); + dma_memory_write(&is->iommu_as, addr, buf, len, + MEMTXATTRS_UNSPECIFIED); } else { addr &= ~1; len &= ~1; @@ -114,7 +115,8 @@ void ledma_memory_write(void *opaque, hwaddr addr, for(i = 0; i < l; i += 2) { tmp_buf[i >> 1] = bswap16(*(uint16_t *)(buf + i)); } - dma_memory_write(&is->iommu_as, addr, tmp_buf, l); + dma_memory_write(&is->iommu_as, addr, tmp_buf, l, + MEMTXATTRS_UNSPECIFIED); len -= l; buf += l; addr += l; @@ -148,7 +150,8 @@ void espdma_memory_read(void *opaque, uint8_t *buf, int len) IOMMUState *is = (IOMMUState *)s->iommu; trace_espdma_memory_read(s->dmaregs[1], len); - dma_memory_read(&is->iommu_as, s->dmaregs[1], buf, len); + dma_memory_read(&is->iommu_as, s->dmaregs[1], buf, len, + MEMTXATTRS_UNSPECIFIED); s->dmaregs[1] += len; } @@ -158,7 +161,8 @@ void espdma_memory_write(void *opaque, uint8_t *buf, int len) IOMMUState *is = (IOMMUState *)s->iommu; trace_espdma_memory_write(s->dmaregs[1], len); - dma_memory_write(&is->iommu_as, s->dmaregs[1], buf, len); + dma_memory_write(&is->iommu_as, s->dmaregs[1], buf, len, + MEMTXATTRS_UNSPECIFIED); s->dmaregs[1] += len; } diff --git a/hw/dma/xlnx-zynq-devcfg.c b/hw/dma/xlnx-zynq-devcfg.c index e33112b6f0e..f5ad1a0d22c 100644 --- a/hw/dma/xlnx-zynq-devcfg.c +++ b/hw/dma/xlnx-zynq-devcfg.c @@ -161,12 +161,14 @@ static void xlnx_zynq_devcfg_dma_go(XlnxZynqDevcfg *s) btt = MIN(btt, dmah->dest_len); } DB_PRINT("reading %x bytes from %x\n", btt, dmah->src_addr); - dma_memory_read(&address_space_memory, dmah->src_addr, buf, btt); + dma_memory_read(&address_space_memory, dmah->src_addr, buf, btt, + MEMTXATTRS_UNSPECIFIED); dmah->src_len -= btt; dmah->src_addr += btt; if (loopback && (dmah->src_len || dmah->dest_len)) { DB_PRINT("writing %x bytes from %x\n", btt, dmah->dest_addr); - dma_memory_write(&address_space_memory, dmah->dest_addr, buf, btt); + dma_memory_write(&address_space_memory, dmah->dest_addr, buf, btt, + MEMTXATTRS_UNSPECIFIED); dmah->dest_len -= btt; dmah->dest_addr += btt; } diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c index b40c897de2c..321f30a6535 100644 --- a/hw/dma/xlnx_dpdma.c +++ b/hw/dma/xlnx_dpdma.c @@ -652,7 +652,7 @@ size_t xlnx_dpdma_start_operation(XlnxDPDMAState *s, uint8_t channel, } if (dma_memory_read(&address_space_memory, desc_addr, &desc, - sizeof(DPDMADescriptor))) { + sizeof(DPDMADescriptor), MEMTXATTRS_UNSPECIFIED)) { s->registers[DPDMA_EISR] |= ((1 << 1) << channel); xlnx_dpdma_update_irq(s); s->operation_finished[channel] = true; @@ -708,7 +708,8 @@ size_t xlnx_dpdma_start_operation(XlnxDPDMAState *s, uint8_t channel, if (dma_memory_read(&address_space_memory, source_addr[0], &s->data[channel][ptr], - line_size)) { + line_size, + MEMTXATTRS_UNSPECIFIED)) { s->registers[DPDMA_ISR] |= ((1 << 12) << channel); xlnx_dpdma_update_irq(s); DPRINTF("Can't get data.\n"); @@ -736,7 +737,8 @@ size_t xlnx_dpdma_start_operation(XlnxDPDMAState *s, uint8_t channel, if (dma_memory_read(&address_space_memory, source_addr[frag], &(s->data[channel][ptr]), - fragment_len)) { + fragment_len, + MEMTXATTRS_UNSPECIFIED)) { s->registers[DPDMA_ISR] |= ((1 << 12) << channel); xlnx_dpdma_update_irq(s); DPRINTF("Can't get data.\n"); @@ -754,7 +756,7 @@ size_t xlnx_dpdma_start_operation(XlnxDPDMAState *s, uint8_t channel, DPRINTF("update the descriptor with the done flag set.\n"); xlnx_dpdma_desc_set_done(&desc); dma_memory_write(&address_space_memory, desc_addr, &desc, - sizeof(DPDMADescriptor)); + sizeof(DPDMADescriptor), MEMTXATTRS_UNSPECIFIED); } if (xlnx_dpdma_desc_completion_interrupt(&desc)) { diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 74a93a5d93f..3c2a1a61db5 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -181,7 +181,7 @@ static void amdvi_log_event(AMDVIState *s, uint64_t *evt) } if (dma_memory_write(&address_space_memory, s->evtlog + s->evtlog_tail, - evt, AMDVI_EVENT_LEN)) { + evt, AMDVI_EVENT_LEN, MEMTXATTRS_UNSPECIFIED)) { trace_amdvi_evntlog_fail(s->evtlog, s->evtlog_tail); } @@ -376,7 +376,8 @@ static void amdvi_completion_wait(AMDVIState *s, uint64_t *cmd) } if (extract64(cmd[0], 0, 1)) { if (dma_memory_write(&address_space_memory, addr, &data, - AMDVI_COMPLETION_DATA_SIZE)) { + AMDVI_COMPLETION_DATA_SIZE, + MEMTXATTRS_UNSPECIFIED)) { trace_amdvi_completion_wait_fail(addr); } } @@ -502,7 +503,7 @@ static void amdvi_cmdbuf_exec(AMDVIState *s) uint64_t cmd[2]; if (dma_memory_read(&address_space_memory, s->cmdbuf + s->cmdbuf_head, - cmd, AMDVI_COMMAND_SIZE)) { + cmd, AMDVI_COMMAND_SIZE, MEMTXATTRS_UNSPECIFIED)) { trace_amdvi_command_read_fail(s->cmdbuf, s->cmdbuf_head); amdvi_log_command_error(s, s->cmdbuf + s->cmdbuf_head); return; @@ -836,7 +837,7 @@ static bool amdvi_get_dte(AMDVIState *s, int devid, uint64_t *entry) uint32_t offset = devid * AMDVI_DEVTAB_ENTRY_SIZE; if (dma_memory_read(&address_space_memory, s->devtab + offset, entry, - AMDVI_DEVTAB_ENTRY_SIZE)) { + AMDVI_DEVTAB_ENTRY_SIZE, MEMTXATTRS_UNSPECIFIED)) { trace_amdvi_dte_get_fail(s->devtab, offset); /* log error accessing dte */ amdvi_log_devtab_error(s, devid, s->devtab + offset, 0); @@ -881,7 +882,8 @@ static inline uint64_t amdvi_get_pte_entry(AMDVIState *s, uint64_t pte_addr, { uint64_t pte; - if (dma_memory_read(&address_space_memory, pte_addr, &pte, sizeof(pte))) { + if (dma_memory_read(&address_space_memory, pte_addr, + &pte, sizeof(pte), MEMTXATTRS_UNSPECIFIED)) { trace_amdvi_get_pte_hwerror(pte_addr); amdvi_log_pagetab_error(s, devid, pte_addr, 0); pte = 0; @@ -1048,7 +1050,7 @@ static int amdvi_get_irte(AMDVIState *s, MSIMessage *origin, uint64_t *dte, trace_amdvi_ir_irte(irte_root, offset); if (dma_memory_read(&address_space_memory, irte_root + offset, - irte, sizeof(*irte))) { + irte, sizeof(*irte), MEMTXATTRS_UNSPECIFIED)) { trace_amdvi_ir_err("failed to get irte"); return -AMDVI_IR_GET_IRTE; } @@ -1108,7 +1110,7 @@ static int amdvi_get_irte_ga(AMDVIState *s, MSIMessage *origin, uint64_t *dte, trace_amdvi_ir_irte(irte_root, offset); if (dma_memory_read(&address_space_memory, irte_root + offset, - irte, sizeof(*irte))) { + irte, sizeof(*irte), MEMTXATTRS_UNSPECIFIED)) { trace_amdvi_ir_err("failed to get irte_ga"); return -AMDVI_IR_GET_IRTE; } diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 749eb6ad632..8de57dd995e 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -570,7 +570,8 @@ static int vtd_get_root_entry(IntelIOMMUState *s, uint8_t index, dma_addr_t addr; addr = s->root + index * sizeof(*re); - if (dma_memory_read(&address_space_memory, addr, re, sizeof(*re))) { + if (dma_memory_read(&address_space_memory, addr, + re, sizeof(*re), MEMTXATTRS_UNSPECIFIED)) { re->lo = 0; return -VTD_FR_ROOT_TABLE_INV; } @@ -603,7 +604,8 @@ static int vtd_get_context_entry_from_root(IntelIOMMUState *s, } addr = addr + index * ce_size; - if (dma_memory_read(&address_space_memory, addr, ce, ce_size)) { + if (dma_memory_read(&address_space_memory, addr, + ce, ce_size, MEMTXATTRS_UNSPECIFIED)) { return -VTD_FR_CONTEXT_TABLE_INV; } @@ -640,8 +642,8 @@ static uint64_t vtd_get_slpte(dma_addr_t base_addr, uint32_t index) assert(index < VTD_SL_PT_ENTRY_NR); if (dma_memory_read(&address_space_memory, - base_addr + index * sizeof(slpte), &slpte, - sizeof(slpte))) { + base_addr + index * sizeof(slpte), + &slpte, sizeof(slpte), MEMTXATTRS_UNSPECIFIED)) { slpte = (uint64_t)-1; return slpte; } @@ -705,7 +707,8 @@ static int vtd_get_pdire_from_pdir_table(dma_addr_t pasid_dir_base, index = VTD_PASID_DIR_INDEX(pasid); entry_size = VTD_PASID_DIR_ENTRY_SIZE; addr = pasid_dir_base + index * entry_size; - if (dma_memory_read(&address_space_memory, addr, pdire, entry_size)) { + if (dma_memory_read(&address_space_memory, addr, + pdire, entry_size, MEMTXATTRS_UNSPECIFIED)) { return -VTD_FR_PASID_TABLE_INV; } @@ -729,7 +732,8 @@ static int vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState *s, index = VTD_PASID_TABLE_INDEX(pasid); entry_size = VTD_PASID_ENTRY_SIZE; addr = addr + index * entry_size; - if (dma_memory_read(&address_space_memory, addr, pe, entry_size)) { + if (dma_memory_read(&address_space_memory, addr, + pe, entry_size, MEMTXATTRS_UNSPECIFIED)) { return -VTD_FR_PASID_TABLE_INV; } @@ -2262,7 +2266,8 @@ static bool vtd_get_inv_desc(IntelIOMMUState *s, uint32_t dw = s->iq_dw ? 32 : 16; dma_addr_t addr = base_addr + offset * dw; - if (dma_memory_read(&address_space_memory, addr, inv_desc, dw)) { + if (dma_memory_read(&address_space_memory, addr, + inv_desc, dw, MEMTXATTRS_UNSPECIFIED)) { error_report_once("Read INV DESC failed."); return false; } @@ -2295,8 +2300,9 @@ static bool vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc) dma_addr_t status_addr = inv_desc->hi; trace_vtd_inv_desc_wait_sw(status_addr, status_data); status_data = cpu_to_le32(status_data); - if (dma_memory_write(&address_space_memory, status_addr, &status_data, - sizeof(status_data))) { + if (dma_memory_write(&address_space_memory, status_addr, + &status_data, sizeof(status_data), + MEMTXATTRS_UNSPECIFIED)) { trace_vtd_inv_desc_wait_write_fail(inv_desc->hi, inv_desc->lo); return false; } @@ -3106,8 +3112,8 @@ static int vtd_irte_get(IntelIOMMUState *iommu, uint16_t index, } addr = iommu->intr_root + index * sizeof(*entry); - if (dma_memory_read(&address_space_memory, addr, entry, - sizeof(*entry))) { + if (dma_memory_read(&address_space_memory, addr, + entry, sizeof(*entry), MEMTXATTRS_UNSPECIFIED)) { error_report_once("%s: read failed: ind=0x%x addr=0x%" PRIx64, __func__, index, addr); return -VTD_FR_IR_ROOT_INVAL; diff --git a/hw/ide/macio.c b/hw/ide/macio.c index 62a599a0751..a93a2b75a85 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -97,7 +97,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) /* Non-block ATAPI transfer - just copy to RAM */ s->io_buffer_size = MIN(s->io_buffer_size, io->len); dma_memory_write(&address_space_memory, io->addr, s->io_buffer, - s->io_buffer_size); + s->io_buffer_size, MEMTXATTRS_UNSPECIFIED); io->len = 0; ide_atapi_cmd_ok(s); m->dma_active = false; diff --git a/hw/intc/xive.c b/hw/intc/xive.c index 489e6256ef7..8e8618c1ab4 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -1236,8 +1236,8 @@ void xive_end_queue_pic_print_info(XiveEND *end, uint32_t width, Monitor *mon) uint64_t qaddr = qaddr_base + (qindex << 2); uint32_t qdata = -1; - if (dma_memory_read(&address_space_memory, qaddr, &qdata, - sizeof(qdata))) { + if (dma_memory_read(&address_space_memory, qaddr, + &qdata, sizeof(qdata), MEMTXATTRS_UNSPECIFIED)) { qemu_log_mask(LOG_GUEST_ERROR, "XIVE: failed to read EQ @0x%" HWADDR_PRIx "\n", qaddr); return; @@ -1300,7 +1300,8 @@ static void xive_end_enqueue(XiveEND *end, uint32_t data) uint32_t qdata = cpu_to_be32((qgen << 31) | (data & 0x7fffffff)); uint32_t qentries = 1 << (qsize + 10); - if (dma_memory_write(&address_space_memory, qaddr, &qdata, sizeof(qdata))) { + if (dma_memory_write(&address_space_memory, qaddr, + &qdata, sizeof(qdata), MEMTXATTRS_UNSPECIFIED)) { qemu_log_mask(LOG_GUEST_ERROR, "XIVE: failed to write END data @0x%" HWADDR_PRIx "\n", qaddr); return; diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c index 73941bdae97..76ea511d53d 100644 --- a/hw/misc/bcm2835_property.c +++ b/hw/misc/bcm2835_property.c @@ -69,7 +69,8 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value) break; case 0x00010003: /* Get board MAC address */ resplen = sizeof(s->macaddr.a); - dma_memory_write(&s->dma_as, value + 12, s->macaddr.a, resplen); + dma_memory_write(&s->dma_as, value + 12, s->macaddr.a, resplen, + MEMTXATTRS_UNSPECIFIED); break; case 0x00010004: /* Get board serial */ qemu_log_mask(LOG_UNIMP, diff --git a/hw/misc/macio/mac_dbdma.c b/hw/misc/macio/mac_dbdma.c index e220f1a9277..efcc02609fd 100644 --- a/hw/misc/macio/mac_dbdma.c +++ b/hw/misc/macio/mac_dbdma.c @@ -94,7 +94,7 @@ static void dbdma_cmdptr_load(DBDMA_channel *ch) DBDMA_DPRINTFCH(ch, "dbdma_cmdptr_load 0x%08x\n", ch->regs[DBDMA_CMDPTR_LO]); dma_memory_read(&address_space_memory, ch->regs[DBDMA_CMDPTR_LO], - &ch->current, sizeof(dbdma_cmd)); + &ch->current, sizeof(dbdma_cmd), MEMTXATTRS_UNSPECIFIED); } static void dbdma_cmdptr_save(DBDMA_channel *ch) @@ -104,7 +104,7 @@ static void dbdma_cmdptr_save(DBDMA_channel *ch) le16_to_cpu(ch->current.xfer_status), le16_to_cpu(ch->current.res_count)); dma_memory_write(&address_space_memory, ch->regs[DBDMA_CMDPTR_LO], - &ch->current, sizeof(dbdma_cmd)); + &ch->current, sizeof(dbdma_cmd), MEMTXATTRS_UNSPECIFIED); } static void kill_channel(DBDMA_channel *ch) @@ -371,7 +371,8 @@ static void load_word(DBDMA_channel *ch, int key, uint32_t addr, return; } - dma_memory_read(&address_space_memory, addr, ¤t->cmd_dep, len); + dma_memory_read(&address_space_memory, addr, ¤t->cmd_dep, len, + MEMTXATTRS_UNSPECIFIED); if (conditional_wait(ch)) goto wait; @@ -403,7 +404,8 @@ static void store_word(DBDMA_channel *ch, int key, uint32_t addr, return; } - dma_memory_write(&address_space_memory, addr, ¤t->cmd_dep, len); + dma_memory_write(&address_space_memory, addr, ¤t->cmd_dep, len, + MEMTXATTRS_UNSPECIFIED); if (conditional_wait(ch)) goto wait; diff --git a/hw/net/allwinner-sun8i-emac.c b/hw/net/allwinner-sun8i-emac.c index 38d328587e3..5189ad77527 100644 --- a/hw/net/allwinner-sun8i-emac.c +++ b/hw/net/allwinner-sun8i-emac.c @@ -345,7 +345,8 @@ static uint32_t allwinner_sun8i_emac_next_desc(AwSun8iEmacState *s, { uint32_t paddr = desc->next; - dma_memory_read(&s->dma_as, paddr, desc, sizeof(*desc)); + dma_memory_read(&s->dma_as, paddr, desc, sizeof(*desc), + MEMTXATTRS_UNSPECIFIED); if ((desc->status & DESC_STATUS_CTL) && (desc->status2 & DESC_STATUS2_BUF_SIZE_MASK) >= min_size) { @@ -364,7 +365,8 @@ static uint32_t allwinner_sun8i_emac_get_desc(AwSun8iEmacState *s, /* Note that the list is a cycle. Last entry points back to the head. */ while (desc_addr != 0) { - dma_memory_read(&s->dma_as, desc_addr, desc, sizeof(*desc)); + dma_memory_read(&s->dma_as, desc_addr, desc, sizeof(*desc), + MEMTXATTRS_UNSPECIFIED); if ((desc->status & DESC_STATUS_CTL) && (desc->status2 & DESC_STATUS2_BUF_SIZE_MASK) >= min_size) { @@ -397,7 +399,8 @@ static void allwinner_sun8i_emac_flush_desc(AwSun8iEmacState *s, FrameDescriptor *desc, uint32_t phys_addr) { - dma_memory_write(&s->dma_as, phys_addr, desc, sizeof(*desc)); + dma_memory_write(&s->dma_as, phys_addr, desc, sizeof(*desc), + MEMTXATTRS_UNSPECIFIED); } static bool allwinner_sun8i_emac_can_receive(NetClientState *nc) @@ -455,7 +458,8 @@ static ssize_t allwinner_sun8i_emac_receive(NetClientState *nc, << RX_DESC_STATUS_FRM_LEN_SHIFT; } - dma_memory_write(&s->dma_as, desc.addr, buf, desc_bytes); + dma_memory_write(&s->dma_as, desc.addr, buf, desc_bytes, + MEMTXATTRS_UNSPECIFIED); allwinner_sun8i_emac_flush_desc(s, &desc, s->rx_desc_curr); trace_allwinner_sun8i_emac_receive(s->rx_desc_curr, desc.addr, desc_bytes); @@ -506,7 +510,8 @@ static void allwinner_sun8i_emac_transmit(AwSun8iEmacState *s) desc.status |= TX_DESC_STATUS_LENGTH_ERR; break; } - dma_memory_read(&s->dma_as, desc.addr, packet_buf + packet_bytes, bytes); + dma_memory_read(&s->dma_as, desc.addr, packet_buf + packet_bytes, + bytes, MEMTXATTRS_UNSPECIFIED); packet_bytes += bytes; desc.status &= ~DESC_STATUS_CTL; allwinner_sun8i_emac_flush_desc(s, &desc, s->tx_desc_curr); @@ -628,7 +633,8 @@ static uint64_t allwinner_sun8i_emac_read(void *opaque, hwaddr offset, break; case REG_TX_CUR_BUF: /* Transmit Current Buffer */ if (s->tx_desc_curr != 0) { - dma_memory_read(&s->dma_as, s->tx_desc_curr, &desc, sizeof(desc)); + dma_memory_read(&s->dma_as, s->tx_desc_curr, &desc, sizeof(desc), + MEMTXATTRS_UNSPECIFIED); value = desc.addr; } else { value = 0; @@ -641,7 +647,8 @@ static uint64_t allwinner_sun8i_emac_read(void *opaque, hwaddr offset, break; case REG_RX_CUR_BUF: /* Receive Current Buffer */ if (s->rx_desc_curr != 0) { - dma_memory_read(&s->dma_as, s->rx_desc_curr, &desc, sizeof(desc)); + dma_memory_read(&s->dma_as, s->rx_desc_curr, &desc, sizeof(desc), + MEMTXATTRS_UNSPECIFIED); value = desc.addr; } else { value = 0; diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c index 5f4b26fc5f3..725aeb65f85 100644 --- a/hw/net/ftgmac100.c +++ b/hw/net/ftgmac100.c @@ -453,7 +453,8 @@ static void do_phy_ctl(FTGMAC100State *s) static int ftgmac100_read_bd(FTGMAC100Desc *bd, dma_addr_t addr) { - if (dma_memory_read(&address_space_memory, addr, bd, sizeof(*bd))) { + if (dma_memory_read(&address_space_memory, addr, + bd, sizeof(*bd), MEMTXATTRS_UNSPECIFIED)) { qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to read descriptor @ 0x%" HWADDR_PRIx "\n", __func__, addr); return -1; @@ -473,7 +474,8 @@ static int ftgmac100_write_bd(FTGMAC100Desc *bd, dma_addr_t addr) lebd.des1 = cpu_to_le32(bd->des1); lebd.des2 = cpu_to_le32(bd->des2); lebd.des3 = cpu_to_le32(bd->des3); - if (dma_memory_write(&address_space_memory, addr, &lebd, sizeof(lebd))) { + if (dma_memory_write(&address_space_memory, addr, + &lebd, sizeof(lebd), MEMTXATTRS_UNSPECIFIED)) { qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to write descriptor @ 0x%" HWADDR_PRIx "\n", __func__, addr); return -1; @@ -514,7 +516,8 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring, len = sizeof(s->frame) - frame_size; } - if (dma_memory_read(&address_space_memory, bd.des3, ptr, len)) { + if (dma_memory_read(&address_space_memory, bd.des3, + ptr, len, MEMTXATTRS_UNSPECIFIED)) { qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to read packet @ 0x%x\n", __func__, bd.des3); s->isr |= FTGMAC100_INT_NO_NPTXBUF; @@ -976,20 +979,24 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const uint8_t *buf, bd.des1 = lduw_be_p(buf + 14) | FTGMAC100_RXDES1_VLANTAG_AVAIL; if (s->maccr & FTGMAC100_MACCR_RM_VLAN) { - dma_memory_write(&address_space_memory, buf_addr, buf, 12); - dma_memory_write(&address_space_memory, buf_addr + 12, buf + 16, - buf_len - 16); + dma_memory_write(&address_space_memory, buf_addr, buf, 12, + MEMTXATTRS_UNSPECIFIED); + dma_memory_write(&address_space_memory, buf_addr + 12, + buf + 16, buf_len - 16, + MEMTXATTRS_UNSPECIFIED); } else { - dma_memory_write(&address_space_memory, buf_addr, buf, buf_len); + dma_memory_write(&address_space_memory, buf_addr, buf, + buf_len, MEMTXATTRS_UNSPECIFIED); } } else { bd.des1 = 0; - dma_memory_write(&address_space_memory, buf_addr, buf, buf_len); + dma_memory_write(&address_space_memory, buf_addr, buf, buf_len, + MEMTXATTRS_UNSPECIFIED); } buf += buf_len; if (size < 4) { dma_memory_write(&address_space_memory, buf_addr + buf_len, - crc_ptr, 4 - size); + crc_ptr, 4 - size, MEMTXATTRS_UNSPECIFIED); crc_ptr += 4 - size; } diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c index 2c148040414..ff4c345f09f 100644 --- a/hw/net/imx_fec.c +++ b/hw/net/imx_fec.c @@ -389,19 +389,22 @@ static void imx_phy_write(IMXFECState *s, int reg, uint32_t val) static void imx_fec_read_bd(IMXFECBufDesc *bd, dma_addr_t addr) { - dma_memory_read(&address_space_memory, addr, bd, sizeof(*bd)); + dma_memory_read(&address_space_memory, addr, bd, sizeof(*bd), + MEMTXATTRS_UNSPECIFIED); trace_imx_fec_read_bd(addr, bd->flags, bd->length, bd->data); } static void imx_fec_write_bd(IMXFECBufDesc *bd, dma_addr_t addr) { - dma_memory_write(&address_space_memory, addr, bd, sizeof(*bd)); + dma_memory_write(&address_space_memory, addr, bd, sizeof(*bd), + MEMTXATTRS_UNSPECIFIED); } static void imx_enet_read_bd(IMXENETBufDesc *bd, dma_addr_t addr) { - dma_memory_read(&address_space_memory, addr, bd, sizeof(*bd)); + dma_memory_read(&address_space_memory, addr, bd, sizeof(*bd), + MEMTXATTRS_UNSPECIFIED); trace_imx_enet_read_bd(addr, bd->flags, bd->length, bd->data, bd->option, bd->status); @@ -409,7 +412,8 @@ static void imx_enet_read_bd(IMXENETBufDesc *bd, dma_addr_t addr) static void imx_enet_write_bd(IMXENETBufDesc *bd, dma_addr_t addr) { - dma_memory_write(&address_space_memory, addr, bd, sizeof(*bd)); + dma_memory_write(&address_space_memory, addr, bd, sizeof(*bd), + MEMTXATTRS_UNSPECIFIED); } static void imx_eth_update(IMXFECState *s) @@ -476,7 +480,8 @@ static void imx_fec_do_tx(IMXFECState *s) len = ENET_MAX_FRAME_SIZE - frame_size; s->regs[ENET_EIR] |= ENET_INT_BABT; } - dma_memory_read(&address_space_memory, bd.data, ptr, len); + dma_memory_read(&address_space_memory, bd.data, ptr, len, + MEMTXATTRS_UNSPECIFIED); ptr += len; frame_size += len; if (bd.flags & ENET_BD_L) { @@ -557,7 +562,8 @@ static void imx_enet_do_tx(IMXFECState *s, uint32_t index) len = ENET_MAX_FRAME_SIZE - frame_size; s->regs[ENET_EIR] |= ENET_INT_BABT; } - dma_memory_read(&address_space_memory, bd.data, ptr, len); + dma_memory_read(&address_space_memory, bd.data, ptr, len, + MEMTXATTRS_UNSPECIFIED); ptr += len; frame_size += len; if (bd.flags & ENET_BD_L) { @@ -1109,11 +1115,12 @@ static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf, buf_len += size - 4; } buf_addr = bd.data; - dma_memory_write(&address_space_memory, buf_addr, buf, buf_len); + dma_memory_write(&address_space_memory, buf_addr, buf, buf_len, + MEMTXATTRS_UNSPECIFIED); buf += buf_len; if (size < 4) { dma_memory_write(&address_space_memory, buf_addr + buf_len, - crc_ptr, 4 - size); + crc_ptr, 4 - size, MEMTXATTRS_UNSPECIFIED); crc_ptr += 4 - size; } bd.flags &= ~ENET_BD_E; @@ -1216,8 +1223,8 @@ static ssize_t imx_enet_receive(NetClientState *nc, const uint8_t *buf, */ const uint8_t zeros[2] = { 0 }; - dma_memory_write(&address_space_memory, buf_addr, - zeros, sizeof(zeros)); + dma_memory_write(&address_space_memory, buf_addr, zeros, + sizeof(zeros), MEMTXATTRS_UNSPECIFIED); buf_addr += sizeof(zeros); buf_len -= sizeof(zeros); @@ -1226,11 +1233,12 @@ static ssize_t imx_enet_receive(NetClientState *nc, const uint8_t *buf, shift16 = false; } - dma_memory_write(&address_space_memory, buf_addr, buf, buf_len); + dma_memory_write(&address_space_memory, buf_addr, buf, buf_len, + MEMTXATTRS_UNSPECIFIED); buf += buf_len; if (size < 4) { dma_memory_write(&address_space_memory, buf_addr + buf_len, - crc_ptr, 4 - size); + crc_ptr, 4 - size, MEMTXATTRS_UNSPECIFIED); crc_ptr += 4 - size; } bd.flags &= ~ENET_BD_E; diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index a15de06a10c..877df8752a0 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -355,7 +355,8 @@ static void fw_cfg_dma_transfer(FWCfgState *s) dma_addr = s->dma_addr; s->dma_addr = 0; - if (dma_memory_read(s->dma_as, dma_addr, &dma, sizeof(dma))) { + if (dma_memory_read(s->dma_as, dma_addr, + &dma, sizeof(dma), MEMTXATTRS_UNSPECIFIED)) { stl_be_dma(s->dma_as, dma_addr + offsetof(FWCfgDmaAccess, control), FW_CFG_DMA_CTL_ERROR); return; @@ -417,7 +418,8 @@ static void fw_cfg_dma_transfer(FWCfgState *s) */ if (read) { if (dma_memory_write(s->dma_as, dma.address, - &e->data[s->cur_offset], len)) { + &e->data[s->cur_offset], len, + MEMTXATTRS_UNSPECIFIED)) { dma.control |= FW_CFG_DMA_CTL_ERROR; } } @@ -425,7 +427,8 @@ static void fw_cfg_dma_transfer(FWCfgState *s) if (!e->allow_write || len != dma.length || dma_memory_read(s->dma_as, dma.address, - &e->data[s->cur_offset], len)) { + &e->data[s->cur_offset], len, + MEMTXATTRS_UNSPECIFIED)) { dma.control |= FW_CFG_DMA_CTL_ERROR; } else if (e->write_cb) { e->write_cb(e->callback_opaque, s->cur_offset, len); diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c index 82132c12ca9..3e0938e5f4e 100644 --- a/hw/pci-host/pnv_phb3.c +++ b/hw/pci-host/pnv_phb3.c @@ -714,7 +714,8 @@ static bool pnv_phb3_resolve_pe(PnvPhb3DMASpace *ds) bus_num = pci_bus_num(ds->bus); addr = rtt & PHB_RTT_BASE_ADDRESS_MASK; addr += 2 * ((bus_num << 8) | ds->devfn); - if (dma_memory_read(&address_space_memory, addr, &rte, sizeof(rte))) { + if (dma_memory_read(&address_space_memory, addr, &rte, + sizeof(rte), MEMTXATTRS_UNSPECIFIED)) { phb3_error(ds->phb, "Failed to read RTT entry at 0x%"PRIx64, addr); /* Set error bits ? fence ? ... */ return false; @@ -793,7 +794,7 @@ static void pnv_phb3_translate_tve(PnvPhb3DMASpace *ds, hwaddr addr, /* Grab the TCE address */ taddr = base | (((addr >> sh) & ((1ul << tbl_shift) - 1)) << 3); if (dma_memory_read(&address_space_memory, taddr, &tce, - sizeof(tce))) { + sizeof(tce), MEMTXATTRS_UNSPECIFIED)) { phb3_error(phb, "Failed to read TCE at 0x%"PRIx64, taddr); return; } diff --git a/hw/pci-host/pnv_phb3_msi.c b/hw/pci-host/pnv_phb3_msi.c index 099d2092a2c..8bcbc2cc4f3 100644 --- a/hw/pci-host/pnv_phb3_msi.c +++ b/hw/pci-host/pnv_phb3_msi.c @@ -53,7 +53,8 @@ static bool phb3_msi_read_ive(PnvPHB3 *phb, int srcno, uint64_t *out_ive) return false; } - if (dma_memory_read(&address_space_memory, ive_addr, &ive, sizeof(ive))) { + if (dma_memory_read(&address_space_memory, ive_addr, + &ive, sizeof(ive), MEMTXATTRS_UNSPECIFIED)) { qemu_log_mask(LOG_GUEST_ERROR, "Failed to read IVE at 0x%" PRIx64, ive_addr); return false; @@ -73,7 +74,8 @@ static void phb3_msi_set_p(Phb3MsiState *msi, int srcno, uint8_t gen) return; } - if (dma_memory_write(&address_space_memory, ive_addr + 4, &p, 1)) { + if (dma_memory_write(&address_space_memory, ive_addr + 4, + &p, 1, MEMTXATTRS_UNSPECIFIED)) { qemu_log_mask(LOG_GUEST_ERROR, "Failed to write IVE (set P) at 0x%" PRIx64, ive_addr); } @@ -89,7 +91,8 @@ static void phb3_msi_set_q(Phb3MsiState *msi, int srcno) return; } - if (dma_memory_write(&address_space_memory, ive_addr + 5, &q, 1)) { + if (dma_memory_write(&address_space_memory, ive_addr + 5, + &q, 1, MEMTXATTRS_UNSPECIFIED)) { qemu_log_mask(LOG_GUEST_ERROR, "Failed to write IVE (set Q) at 0x%" PRIx64, ive_addr); } diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c index 75ad766fe0b..4369a97b9d2 100644 --- a/hw/pci-host/pnv_phb4.c +++ b/hw/pci-host/pnv_phb4.c @@ -889,7 +889,8 @@ static bool pnv_phb4_resolve_pe(PnvPhb4DMASpace *ds) bus_num = pci_bus_num(ds->bus); addr = rtt & PHB_RTT_BASE_ADDRESS_MASK; addr += 2 * ((bus_num << 8) | ds->devfn); - if (dma_memory_read(&address_space_memory, addr, &rte, sizeof(rte))) { + if (dma_memory_read(&address_space_memory, addr, + &rte, sizeof(rte), MEMTXATTRS_UNSPECIFIED)) { phb_error(ds->phb, "Failed to read RTT entry at 0x%"PRIx64, addr); /* Set error bits ? fence ? ... */ return false; @@ -958,8 +959,8 @@ static void pnv_phb4_translate_tve(PnvPhb4DMASpace *ds, hwaddr addr, while ((lev--) >= 0) { /* Grab the TCE address */ taddr = base | (((addr >> sh) & ((1ul << tbl_shift) - 1)) << 3); - if (dma_memory_read(&address_space_memory, taddr, &tce, - sizeof(tce))) { + if (dma_memory_read(&address_space_memory, taddr, + &tce, sizeof(tce), MEMTXATTRS_UNSPECIFIED)) { phb_error(ds->phb, "Failed to read TCE at 0x%"PRIx64, taddr); return; } diff --git a/hw/sd/allwinner-sdhost.c b/hw/sd/allwinner-sdhost.c index e82afb75eb6..e98956ebb76 100644 --- a/hw/sd/allwinner-sdhost.c +++ b/hw/sd/allwinner-sdhost.c @@ -309,7 +309,8 @@ static uint32_t allwinner_sdhost_process_desc(AwSdHostState *s, uint8_t buf[1024]; /* Read descriptor */ - dma_memory_read(&s->dma_as, desc_addr, desc, sizeof(*desc)); + dma_memory_read(&s->dma_as, desc_addr, desc, sizeof(*desc), + MEMTXATTRS_UNSPECIFIED); if (desc->size == 0) { desc->size = klass->max_desc_size; } else if (desc->size > klass->max_desc_size) { @@ -335,23 +336,24 @@ static uint32_t allwinner_sdhost_process_desc(AwSdHostState *s, /* Write to SD bus */ if (is_write) { dma_memory_read(&s->dma_as, - (desc->addr & DESC_SIZE_MASK) + num_done, - buf, buf_bytes); + (desc->addr & DESC_SIZE_MASK) + num_done, buf, + buf_bytes, MEMTXATTRS_UNSPECIFIED); sdbus_write_data(&s->sdbus, buf, buf_bytes); /* Read from SD bus */ } else { sdbus_read_data(&s->sdbus, buf, buf_bytes); dma_memory_write(&s->dma_as, - (desc->addr & DESC_SIZE_MASK) + num_done, - buf, buf_bytes); + (desc->addr & DESC_SIZE_MASK) + num_done, buf, + buf_bytes, MEMTXATTRS_UNSPECIFIED); } num_done += buf_bytes; } /* Clear hold flag and flush descriptor */ desc->status &= ~DESC_STATUS_HOLD; - dma_memory_write(&s->dma_as, desc_addr, desc, sizeof(*desc)); + dma_memory_write(&s->dma_as, desc_addr, desc, sizeof(*desc), + MEMTXATTRS_UNSPECIFIED); return num_done; } diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 1785d7e1f79..987d76e72ae 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -604,8 +604,8 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s) s->blkcnt--; } } - dma_memory_write(s->dma_as, s->sdmasysad, - &s->fifo_buffer[begin], s->data_count - begin); + dma_memory_write(s->dma_as, s->sdmasysad, &s->fifo_buffer[begin], + s->data_count - begin, MEMTXATTRS_UNSPECIFIED); s->sdmasysad += s->data_count - begin; if (s->data_count == block_size) { s->data_count = 0; @@ -626,8 +626,8 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s) s->data_count = block_size; boundary_count -= block_size - begin; } - dma_memory_read(s->dma_as, s->sdmasysad, - &s->fifo_buffer[begin], s->data_count - begin); + dma_memory_read(s->dma_as, s->sdmasysad, &s->fifo_buffer[begin], + s->data_count - begin, MEMTXATTRS_UNSPECIFIED); s->sdmasysad += s->data_count - begin; if (s->data_count == block_size) { sdbus_write_data(&s->sdbus, s->fifo_buffer, block_size); @@ -659,9 +659,11 @@ static void sdhci_sdma_transfer_single_block(SDHCIState *s) if (s->trnmod & SDHC_TRNS_READ) { sdbus_read_data(&s->sdbus, s->fifo_buffer, datacnt); - dma_memory_write(s->dma_as, s->sdmasysad, s->fifo_buffer, datacnt); + dma_memory_write(s->dma_as, s->sdmasysad, s->fifo_buffer, datacnt, + MEMTXATTRS_UNSPECIFIED); } else { - dma_memory_read(s->dma_as, s->sdmasysad, s->fifo_buffer, datacnt); + dma_memory_read(s->dma_as, s->sdmasysad, s->fifo_buffer, datacnt, + MEMTXATTRS_UNSPECIFIED); sdbus_write_data(&s->sdbus, s->fifo_buffer, datacnt); } s->blkcnt--; @@ -683,7 +685,8 @@ static void get_adma_description(SDHCIState *s, ADMADescr *dscr) hwaddr entry_addr = (hwaddr)s->admasysaddr; switch (SDHC_DMA_TYPE(s->hostctl1)) { case SDHC_CTRL_ADMA2_32: - dma_memory_read(s->dma_as, entry_addr, &adma2, sizeof(adma2)); + dma_memory_read(s->dma_as, entry_addr, &adma2, sizeof(adma2), + MEMTXATTRS_UNSPECIFIED); adma2 = le64_to_cpu(adma2); /* The spec does not specify endianness of descriptor table. * We currently assume that it is LE. @@ -694,7 +697,8 @@ static void get_adma_description(SDHCIState *s, ADMADescr *dscr) dscr->incr = 8; break; case SDHC_CTRL_ADMA1_32: - dma_memory_read(s->dma_as, entry_addr, &adma1, sizeof(adma1)); + dma_memory_read(s->dma_as, entry_addr, &adma1, sizeof(adma1), + MEMTXATTRS_UNSPECIFIED); adma1 = le32_to_cpu(adma1); dscr->addr = (hwaddr)(adma1 & 0xFFFFF000); dscr->attr = (uint8_t)extract32(adma1, 0, 7); @@ -706,10 +710,13 @@ static void get_adma_description(SDHCIState *s, ADMADescr *dscr) } break; case SDHC_CTRL_ADMA2_64: - dma_memory_read(s->dma_as, entry_addr, &dscr->attr, 1); - dma_memory_read(s->dma_as, entry_addr + 2, &dscr->length, 2); + dma_memory_read(s->dma_as, entry_addr, &dscr->attr, 1, + MEMTXATTRS_UNSPECIFIED); + dma_memory_read(s->dma_as, entry_addr + 2, &dscr->length, 2, + MEMTXATTRS_UNSPECIFIED); dscr->length = le16_to_cpu(dscr->length); - dma_memory_read(s->dma_as, entry_addr + 4, &dscr->addr, 8); + dma_memory_read(s->dma_as, entry_addr + 4, &dscr->addr, 8, + MEMTXATTRS_UNSPECIFIED); dscr->addr = le64_to_cpu(dscr->addr); dscr->attr &= (uint8_t) ~0xC0; dscr->incr = 12; @@ -767,7 +774,8 @@ static void sdhci_do_adma(SDHCIState *s) } dma_memory_write(s->dma_as, dscr.addr, &s->fifo_buffer[begin], - s->data_count - begin); + s->data_count - begin, + MEMTXATTRS_UNSPECIFIED); dscr.addr += s->data_count - begin; if (s->data_count == block_size) { s->data_count = 0; @@ -791,7 +799,8 @@ static void sdhci_do_adma(SDHCIState *s) } dma_memory_read(s->dma_as, dscr.addr, &s->fifo_buffer[begin], - s->data_count - begin); + s->data_count - begin, + MEMTXATTRS_UNSPECIFIED); dscr.addr += s->data_count - begin; if (s->data_count == block_size) { sdbus_write_data(&s->sdbus, s->fifo_buffer, block_size); diff --git a/hw/usb/hcd-dwc2.c b/hw/usb/hcd-dwc2.c index 97688d21bf0..fe684f92b7a 100644 --- a/hw/usb/hcd-dwc2.c +++ b/hw/usb/hcd-dwc2.c @@ -261,8 +261,8 @@ static void dwc2_handle_packet(DWC2State *s, uint32_t devadr, USBDevice *dev, if (pid != USB_TOKEN_IN) { trace_usb_dwc2_memory_read(hcdma, tlen); - if (dma_memory_read(&s->dma_as, hcdma, - s->usb_buf[chan], tlen) != MEMTX_OK) { + if (dma_memory_read(&s->dma_as, hcdma, s->usb_buf[chan], tlen, + MEMTXATTRS_UNSPECIFIED) != MEMTX_OK) { qemu_log_mask(LOG_GUEST_ERROR, "%s: dma_memory_read failed\n", __func__); } @@ -317,8 +317,8 @@ babble: if (pid == USB_TOKEN_IN) { trace_usb_dwc2_memory_write(hcdma, actual); - if (dma_memory_write(&s->dma_as, hcdma, s->usb_buf[chan], - actual) != MEMTX_OK) { + if (dma_memory_write(&s->dma_as, hcdma, s->usb_buf[chan], actual, + MEMTXATTRS_UNSPECIFIED) != MEMTX_OK) { qemu_log_mask(LOG_GUEST_ERROR, "%s: dma_memory_write failed\n", __func__); } diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 2b995443fbf..03ea5279f9b 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -383,7 +383,8 @@ static inline int get_dwords(EHCIState *ehci, uint32_t addr, } for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) { - dma_memory_read(ehci->as, addr, buf, sizeof(*buf)); + dma_memory_read(ehci->as, addr, buf, sizeof(*buf), + MEMTXATTRS_UNSPECIFIED); *buf = le32_to_cpu(*buf); } @@ -405,7 +406,8 @@ static inline int put_dwords(EHCIState *ehci, uint32_t addr, for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) { uint32_t tmp = cpu_to_le32(*buf); - dma_memory_write(ehci->as, addr, &tmp, sizeof(tmp)); + dma_memory_write(ehci->as, addr, &tmp, sizeof(tmp), + MEMTXATTRS_UNSPECIFIED); } return num; diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index bac1adf439c..3df6a141b5f 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -452,7 +452,8 @@ static inline int get_dwords(OHCIState *ohci, addr += ohci->localmem_base; for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) { - if (dma_memory_read(ohci->as, addr, buf, sizeof(*buf))) { + if (dma_memory_read(ohci->as, addr, + buf, sizeof(*buf), MEMTXATTRS_UNSPECIFIED)) { return -1; } *buf = le32_to_cpu(*buf); @@ -471,7 +472,8 @@ static inline int put_dwords(OHCIState *ohci, for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) { uint32_t tmp = cpu_to_le32(*buf); - if (dma_memory_write(ohci->as, addr, &tmp, sizeof(tmp))) { + if (dma_memory_write(ohci->as, addr, + &tmp, sizeof(tmp), MEMTXATTRS_UNSPECIFIED)) { return -1; } } @@ -488,7 +490,8 @@ static inline int get_words(OHCIState *ohci, addr += ohci->localmem_base; for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) { - if (dma_memory_read(ohci->as, addr, buf, sizeof(*buf))) { + if (dma_memory_read(ohci->as, addr, + buf, sizeof(*buf), MEMTXATTRS_UNSPECIFIED)) { return -1; } *buf = le16_to_cpu(*buf); @@ -507,7 +510,8 @@ static inline int put_words(OHCIState *ohci, for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) { uint16_t tmp = cpu_to_le16(*buf); - if (dma_memory_write(ohci->as, addr, &tmp, sizeof(tmp))) { + if (dma_memory_write(ohci->as, addr, + &tmp, sizeof(tmp), MEMTXATTRS_UNSPECIFIED)) { return -1; } } @@ -537,8 +541,8 @@ static inline int ohci_read_iso_td(OHCIState *ohci, static inline int ohci_read_hcca(OHCIState *ohci, dma_addr_t addr, struct ohci_hcca *hcca) { - return dma_memory_read(ohci->as, addr + ohci->localmem_base, - hcca, sizeof(*hcca)); + return dma_memory_read(ohci->as, addr + ohci->localmem_base, hcca, + sizeof(*hcca), MEMTXATTRS_UNSPECIFIED); } static inline int ohci_put_ed(OHCIState *ohci, @@ -572,7 +576,7 @@ static inline int ohci_put_hcca(OHCIState *ohci, return dma_memory_write(ohci->as, addr + ohci->localmem_base + HCCA_WRITEBACK_OFFSET, (char *)hcca + HCCA_WRITEBACK_OFFSET, - HCCA_WRITEBACK_SIZE); + HCCA_WRITEBACK_SIZE, MEMTXATTRS_UNSPECIFIED); } /* Read/Write the contents of a TD from/to main memory. */ -- 2.26.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 07/12] dma: Let dma_memory_map() take MemTxAttrs argument 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé ` (5 preceding siblings ...) 2020-09-03 11:08 ` [PATCH 06/12] dma: Let dma_memory_read/write() " Philippe Mathieu-Daudé @ 2020-09-03 11:08 ` Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 08/12] docs/devel/loads-stores: Add regexp for DMA functions Philippe Mathieu-Daudé ` (7 subsequent siblings) 14 siblings, 0 replies; 29+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-03 11:08 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Philippe Mathieu-Daudé, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Paolo Bonzini Patch created mechanically using spatch with this script: @@ expression E1, E2, E3, E4; @@ - dma_memory_map(E1, E2, E3, E4) + dma_memory_map(E1, E2, E3, E4, MEMTXATTRS_UNSPECIFIED) Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/hw/pci/pci.h | 3 ++- include/sysemu/dma.h | 4 ++-- dma-helpers.c | 3 ++- hw/display/virtio-gpu.c | 8 ++++++-- hw/hyperv/vmbus.c | 8 +++++--- hw/ide/ahci.c | 9 ++++++--- hw/usb/libhw.c | 3 ++- hw/virtio/virtio.c | 6 ++++-- 8 files changed, 29 insertions(+), 15 deletions(-) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 0c3217e019c..a221dfb3b08 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -831,7 +831,8 @@ static inline void *pci_dma_map(PCIDevice *dev, dma_addr_t addr, { void *buf; - buf = dma_memory_map(pci_get_address_space(dev), addr, plen, dir); + buf = dma_memory_map(pci_get_address_space(dev), addr, plen, dir, + MEMTXATTRS_UNSPECIFIED); return buf; } diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h index f0880b79e74..9d0e145d76f 100644 --- a/include/sysemu/dma.h +++ b/include/sysemu/dma.h @@ -134,13 +134,13 @@ int dma_memory_set(AddressSpace *as, dma_addr_t addr, static inline void *dma_memory_map(AddressSpace *as, dma_addr_t addr, dma_addr_t *len, - DMADirection dir) + DMADirection dir, MemTxAttrs attrs) { hwaddr xlen = *len; void *p; p = address_space_map(as, addr, &xlen, dir == DMA_DIRECTION_FROM_DEVICE, - MEMTXATTRS_UNSPECIFIED); + attrs); *len = xlen; return p; } diff --git a/dma-helpers.c b/dma-helpers.c index 50473bb1996..dfac123680b 100644 --- a/dma-helpers.c +++ b/dma-helpers.c @@ -143,7 +143,8 @@ static void dma_blk_cb(void *opaque, int ret) while (dbs->sg_cur_index < dbs->sg->nsg) { cur_addr = dbs->sg->sg[dbs->sg_cur_index].base + dbs->sg_cur_byte; cur_len = dbs->sg->sg[dbs->sg_cur_index].len - dbs->sg_cur_byte; - mem = dma_memory_map(dbs->sg->as, cur_addr, &cur_len, dbs->dir); + mem = dma_memory_map(dbs->sg->as, cur_addr, &cur_len, dbs->dir, + MEMTXATTRS_UNSPECIFIED); /* * Make reads deterministic in icount mode. Windows sometimes issues * disk read requests with overlapping SGs. It leads diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 5f0dd7c1500..be7f5cdee46 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -648,7 +648,9 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g, hwaddr len = l; (*iov)[i].iov_len = l; (*iov)[i].iov_base = dma_memory_map(VIRTIO_DEVICE(g)->dma_as, - a, &len, DMA_DIRECTION_TO_DEVICE); + a, &len, + DMA_DIRECTION_TO_DEVICE, + MEMTXATTRS_UNSPECIFIED); if (addr) { (*addr)[i] = a; } @@ -1049,7 +1051,9 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size, hwaddr len = res->iov[i].iov_len; res->iov[i].iov_base = dma_memory_map(VIRTIO_DEVICE(g)->dma_as, - res->addrs[i], &len, DMA_DIRECTION_TO_DEVICE); + res->addrs[i], &len, + DMA_DIRECTION_TO_DEVICE, + MEMTXATTRS_UNSPECIFIED); if (!res->iov[i].iov_base || len != res->iov[i].iov_len) { /* Clean up the half-a-mapping we just created... */ diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c index 75af6b83dde..56621d72e5b 100644 --- a/hw/hyperv/vmbus.c +++ b/hw/hyperv/vmbus.c @@ -372,7 +372,8 @@ static ssize_t gpadl_iter_io(GpadlIter *iter, void *buf, uint32_t len) maddr = (iter->gpadl->gfns[idx] << TARGET_PAGE_BITS) | off_in_page; - iter->map = dma_memory_map(iter->as, maddr, &mlen, iter->dir); + iter->map = dma_memory_map(iter->as, maddr, &mlen, iter->dir, + MEMTXATTRS_UNSPECIFIED); if (mlen != pgleft) { dma_memory_unmap(iter->as, iter->map, mlen, iter->dir, 0); iter->map = NULL; @@ -488,7 +489,8 @@ int vmbus_map_sgl(VMBusChanReq *req, DMADirection dir, struct iovec *iov, goto err; } - iov[ret_cnt].iov_base = dma_memory_map(sgl->as, a, &l, dir); + iov[ret_cnt].iov_base = dma_memory_map(sgl->as, a, &l, dir, + MEMTXATTRS_UNSPECIFIED); if (!l) { ret = -EFAULT; goto err; @@ -564,7 +566,7 @@ static vmbus_ring_buffer *ringbuf_map_hdr(VMBusRingBufCommon *ringbuf) dma_addr_t mlen = sizeof(*rb); rb = dma_memory_map(ringbuf->as, ringbuf->rb_addr, &mlen, - DMA_DIRECTION_FROM_DEVICE); + DMA_DIRECTION_FROM_DEVICE, MEMTXATTRS_UNSPECIFIED); if (mlen != sizeof(*rb)) { dma_memory_unmap(ringbuf->as, rb, mlen, DMA_DIRECTION_FROM_DEVICE, 0); diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index b696c6291a3..fe1a4e2b1a2 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -249,7 +249,8 @@ static void map_page(AddressSpace *as, uint8_t **ptr, uint64_t addr, dma_memory_unmap(as, *ptr, len, DMA_DIRECTION_FROM_DEVICE, len); } - *ptr = dma_memory_map(as, addr, &len, DMA_DIRECTION_FROM_DEVICE); + *ptr = dma_memory_map(as, addr, &len, DMA_DIRECTION_FROM_DEVICE, + MEMTXATTRS_UNSPECIFIED); if (len < wanted) { dma_memory_unmap(as, *ptr, len, DMA_DIRECTION_FROM_DEVICE, len); *ptr = NULL; @@ -938,7 +939,8 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, /* map PRDT */ if (!(prdt = dma_memory_map(ad->hba->as, prdt_addr, &prdt_len, - DMA_DIRECTION_TO_DEVICE))){ + DMA_DIRECTION_TO_DEVICE, + MEMTXATTRS_UNSPECIFIED))) { trace_ahci_populate_sglist_no_map(ad->hba, ad->port_no); return -1; } @@ -1299,7 +1301,8 @@ static int handle_cmd(AHCIState *s, int port, uint8_t slot) tbl_addr = le64_to_cpu(cmd->tbl_addr); cmd_len = 0x80; cmd_fis = dma_memory_map(s->as, tbl_addr, &cmd_len, - DMA_DIRECTION_FROM_DEVICE); + DMA_DIRECTION_FROM_DEVICE, + MEMTXATTRS_UNSPECIFIED); if (!cmd_fis) { trace_handle_cmd_badfis(s, port); return -1; diff --git a/hw/usb/libhw.c b/hw/usb/libhw.c index 9c33a1640f7..f350eae443d 100644 --- a/hw/usb/libhw.c +++ b/hw/usb/libhw.c @@ -36,7 +36,8 @@ int usb_packet_map(USBPacket *p, QEMUSGList *sgl) while (len) { dma_addr_t xlen = len; - mem = dma_memory_map(sgl->as, base, &xlen, dir); + mem = dma_memory_map(sgl->as, base, &xlen, dir, + MEMTXATTRS_UNSPECIFIED); if (!mem) { goto err; } diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index e9830252176..c951131ba39 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -1320,7 +1320,8 @@ static bool virtqueue_map_desc(VirtIODevice *vdev, unsigned int *p_num_sg, iov[num_sg].iov_base = dma_memory_map(vdev->dma_as, pa, &len, is_write ? DMA_DIRECTION_FROM_DEVICE : - DMA_DIRECTION_TO_DEVICE); + DMA_DIRECTION_TO_DEVICE, + MEMTXATTRS_UNSPECIFIED); if (!iov[num_sg].iov_base) { virtio_error(vdev, "virtio: bogus descriptor or out of resources"); goto out; @@ -1369,7 +1370,8 @@ static void virtqueue_map_iovec(VirtIODevice *vdev, struct iovec *sg, sg[i].iov_base = dma_memory_map(vdev->dma_as, addr[i], &len, is_write ? DMA_DIRECTION_FROM_DEVICE : - DMA_DIRECTION_TO_DEVICE); + DMA_DIRECTION_TO_DEVICE, + MEMTXATTRS_UNSPECIFIED); if (!sg[i].iov_base) { error_report("virtio: error trying to map MMIO memory"); exit(1); -- 2.26.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 08/12] docs/devel/loads-stores: Add regexp for DMA functions 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé ` (6 preceding siblings ...) 2020-09-03 11:08 ` [PATCH 07/12] dma: Let dma_memory_map() " Philippe Mathieu-Daudé @ 2020-09-03 11:08 ` Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 09/12] dma: Let load/store DMA functions take MemTxAttrs argument Philippe Mathieu-Daudé ` (6 subsequent siblings) 14 siblings, 0 replies; 29+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-03 11:08 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Philippe Mathieu-Daudé, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Paolo Bonzini Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- docs/devel/loads-stores.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/devel/loads-stores.rst b/docs/devel/loads-stores.rst index 9a944ef1af6..5b20f907e4d 100644 --- a/docs/devel/loads-stores.rst +++ b/docs/devel/loads-stores.rst @@ -477,6 +477,8 @@ make sure our existing code is doing things correctly. Regexes for git grep - ``\<dma_memory_\(read\|write\|rw\)\>`` + - ``\<ldu\?[bwlq]\(_[bl]e\)\?_dma\>`` + - ``\<st[bwlq]\(_[bl]e\)\?_dma\>`` ``pci_dma_*`` and ``{ld,st}*_pci_dma`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -- 2.26.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 09/12] dma: Let load/store DMA functions take MemTxAttrs argument 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé ` (7 preceding siblings ...) 2020-09-03 11:08 ` [PATCH 08/12] docs/devel/loads-stores: Add regexp for DMA functions Philippe Mathieu-Daudé @ 2020-09-03 11:08 ` Philippe Mathieu-Daudé 2020-09-03 11:08 ` [RFC PATCH 10/12] exec/memattrs: Introduce MemTxAttrs::direct_access field Philippe Mathieu-Daudé ` (5 subsequent siblings) 14 siblings, 0 replies; 29+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-03 11:08 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Philippe Mathieu-Daudé, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Paolo Bonzini Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/hw/pci/pci.h | 6 ++++-- include/hw/ppc/spapr_vio.h | 15 ++++++++++----- include/sysemu/dma.h | 20 ++++++++++++-------- hw/intc/pnv_xive.c | 7 ++++--- hw/nvram/fw_cfg.c | 4 ++-- 5 files changed, 32 insertions(+), 20 deletions(-) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index a221dfb3b08..8f901e6c289 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -808,12 +808,14 @@ static inline int pci_dma_write(PCIDevice *dev, dma_addr_t addr, static inline uint##_bits##_t ld##_l##_pci_dma(PCIDevice *dev, \ dma_addr_t addr) \ { \ - return ld##_l##_dma(pci_get_address_space(dev), addr); \ + return ld##_l##_dma(pci_get_address_space(dev), addr, \ + MEMTXATTRS_UNSPECIFIED); \ } \ static inline void st##_s##_pci_dma(PCIDevice *dev, \ dma_addr_t addr, uint##_bits##_t val) \ { \ - st##_s##_dma(pci_get_address_space(dev), addr, val); \ + st##_s##_dma(pci_get_address_space(dev), addr, val, \ + MEMTXATTRS_UNSPECIFIED); \ } PCI_DMA_DEFINE_LDST(ub, b, 8); diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h index 8168f4fc5a5..e4db1aa7662 100644 --- a/include/hw/ppc/spapr_vio.h +++ b/include/hw/ppc/spapr_vio.h @@ -123,11 +123,16 @@ static inline int spapr_vio_dma_set(SpaprVioDevice *dev, uint64_t taddr, H_DEST_PARM : H_SUCCESS; } -#define vio_stb(_dev, _addr, _val) (stb_dma(&(_dev)->as, (_addr), (_val))) -#define vio_sth(_dev, _addr, _val) (stw_be_dma(&(_dev)->as, (_addr), (_val))) -#define vio_stl(_dev, _addr, _val) (stl_be_dma(&(_dev)->as, (_addr), (_val))) -#define vio_stq(_dev, _addr, _val) (stq_be_dma(&(_dev)->as, (_addr), (_val))) -#define vio_ldq(_dev, _addr) (ldq_be_dma(&(_dev)->as, (_addr))) +#define vio_stb(_dev, _addr, _val) (stb_dma(&(_dev)->as, (_addr), (_val), \ + MEMTXATTRS_UNSPECIFIED)) +#define vio_sth(_dev, _addr, _val) (stw_be_dma(&(_dev)->as, (_addr), (_val), \ + MEMTXATTRS_UNSPECIFIED)) +#define vio_stl(_dev, _addr, _val) (stl_be_dma(&(_dev)->as, (_addr), (_val), \ + MEMTXATTRS_UNSPECIFIED)) +#define vio_stq(_dev, _addr, _val) (stq_be_dma(&(_dev)->as, (_addr), (_val), \ + MEMTXATTRS_UNSPECIFIED)) +#define vio_ldq(_dev, _addr) (ldq_be_dma(&(_dev)->as, (_addr), \ + MEMTXATTRS_UNSPECIFIED)) int spapr_vio_send_crq(SpaprVioDevice *dev, uint8_t *crq); diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h index 9d0e145d76f..8a7dbf0b0f3 100644 --- a/include/sysemu/dma.h +++ b/include/sysemu/dma.h @@ -155,31 +155,35 @@ static inline void dma_memory_unmap(AddressSpace *as, #define DEFINE_LDST_DMA(_lname, _sname, _bits, _end) \ static inline uint##_bits##_t ld##_lname##_##_end##_dma(AddressSpace *as, \ - dma_addr_t addr) \ + dma_addr_t addr, \ + MemTxAttrs attrs) \ { \ uint##_bits##_t val; \ - dma_memory_read(as, addr, &val, (_bits) / 8, MEMTXATTRS_UNSPECIFIED); \ + dma_memory_read(as, addr, &val, (_bits) / 8, attrs); \ return _end##_bits##_to_cpu(val); \ } \ static inline void st##_sname##_##_end##_dma(AddressSpace *as, \ dma_addr_t addr, \ - uint##_bits##_t val) \ + uint##_bits##_t val, \ + MemTxAttrs attrs) \ { \ val = cpu_to_##_end##_bits(val); \ - dma_memory_write(as, addr, &val, (_bits) / 8, MEMTXATTRS_UNSPECIFIED); \ + dma_memory_write(as, addr, &val, (_bits) / 8, attrs); \ } -static inline uint8_t ldub_dma(AddressSpace *as, dma_addr_t addr) +static inline uint8_t ldub_dma(AddressSpace *as, dma_addr_t addr, + MemTxAttrs attrs) { uint8_t val; - dma_memory_read(as, addr, &val, 1, MEMTXATTRS_UNSPECIFIED); + dma_memory_read(as, addr, &val, 1, attrs); return val; } -static inline void stb_dma(AddressSpace *as, dma_addr_t addr, uint8_t val) +static inline void stb_dma(AddressSpace *as, dma_addr_t addr, uint8_t val, + MemTxAttrs attrs) { - dma_memory_write(as, addr, &val, 1, MEMTXATTRS_UNSPECIFIED); + dma_memory_write(as, addr, &val, 1, attrs); } DEFINE_LDST_DMA(uw, w, 16, le); diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c index 5f69626b3a8..3d7d6e0ca4a 100644 --- a/hw/intc/pnv_xive.c +++ b/hw/intc/pnv_xive.c @@ -171,7 +171,7 @@ static uint64_t pnv_xive_vst_addr_indirect(PnvXive *xive, uint32_t type, /* Get the page size of the indirect table. */ vsd_addr = vsd & VSD_ADDRESS_MASK; - vsd = ldq_be_dma(&address_space_memory, vsd_addr); + vsd = ldq_be_dma(&address_space_memory, vsd_addr, MEMTXATTRS_UNSPECIFIED); if (!(vsd & VSD_ADDRESS_MASK)) { #ifdef XIVE_DEBUG @@ -194,7 +194,8 @@ static uint64_t pnv_xive_vst_addr_indirect(PnvXive *xive, uint32_t type, /* Load the VSD we are looking for, if not already done */ if (vsd_idx) { vsd_addr = vsd_addr + vsd_idx * XIVE_VSD_SIZE; - vsd = ldq_be_dma(&address_space_memory, vsd_addr); + vsd = ldq_be_dma(&address_space_memory, vsd_addr, + MEMTXATTRS_UNSPECIFIED); if (!(vsd & VSD_ADDRESS_MASK)) { #ifdef XIVE_DEBUG @@ -541,7 +542,7 @@ static uint64_t pnv_xive_vst_per_subpage(PnvXive *xive, uint32_t type) /* Get the page size of the indirect table. */ vsd_addr = vsd & VSD_ADDRESS_MASK; - vsd = ldq_be_dma(&address_space_memory, vsd_addr); + vsd = ldq_be_dma(&address_space_memory, vsd_addr, MEMTXATTRS_UNSPECIFIED); if (!(vsd & VSD_ADDRESS_MASK)) { #ifdef XIVE_DEBUG diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 877df8752a0..b8a936bd04a 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -358,7 +358,7 @@ static void fw_cfg_dma_transfer(FWCfgState *s) if (dma_memory_read(s->dma_as, dma_addr, &dma, sizeof(dma), MEMTXATTRS_UNSPECIFIED)) { stl_be_dma(s->dma_as, dma_addr + offsetof(FWCfgDmaAccess, control), - FW_CFG_DMA_CTL_ERROR); + FW_CFG_DMA_CTL_ERROR, MEMTXATTRS_UNSPECIFIED); return; } @@ -444,7 +444,7 @@ static void fw_cfg_dma_transfer(FWCfgState *s) } stl_be_dma(s->dma_as, dma_addr + offsetof(FWCfgDmaAccess, control), - dma.control); + dma.control, MEMTXATTRS_UNSPECIFIED); trace_fw_cfg_read(s, 0); } -- 2.26.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [RFC PATCH 10/12] exec/memattrs: Introduce MemTxAttrs::direct_access field 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé ` (8 preceding siblings ...) 2020-09-03 11:08 ` [PATCH 09/12] dma: Let load/store DMA functions take MemTxAttrs argument Philippe Mathieu-Daudé @ 2020-09-03 11:08 ` Philippe Mathieu-Daudé 2020-09-03 11:08 ` [RFC PATCH 11/12] hw/pci: Only allow PCI slave devices to write to direct memory Philippe Mathieu-Daudé ` (4 subsequent siblings) 14 siblings, 0 replies; 29+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-03 11:08 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Philippe Mathieu-Daudé, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Paolo Bonzini Add the 'direct_access' bit to the memory attributes to restrict bus master access to ROM/RAM. Have read/write accessors return MEMTX_BUS_ERROR if an access is restricted and the region is not ROM/RAM ('direct'). Add corresponding trace events. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/exec/memattrs.h | 3 +++ exec.c | 8 ++++++++ trace-events | 1 + 3 files changed, 12 insertions(+) diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h index 95f2d20d55b..c27cf639a96 100644 --- a/include/exec/memattrs.h +++ b/include/exec/memattrs.h @@ -35,6 +35,8 @@ typedef struct MemTxAttrs { unsigned int secure:1; /* Memory access is usermode (unprivileged) */ unsigned int user:1; + /* Bus master restricted to ROM/RAM slaves */ + unsigned int direct_access:1; /* Requester ID (for MSI for example) */ unsigned int requester_id:16; /* Invert endianness for this page */ @@ -66,6 +68,7 @@ typedef struct MemTxAttrs { #define MEMTX_OK 0 #define MEMTX_ERROR (1U << 0) /* device returned an error */ #define MEMTX_DECODE_ERROR (1U << 1) /* nothing at that address */ +#define MEMTX_BUS_ERROR (1U << 2) /* bus returned an error */ typedef uint32_t MemTxResult; #endif diff --git a/exec.c b/exec.c index 7683afb6a8e..e6185eb04de 100644 --- a/exec.c +++ b/exec.c @@ -3213,6 +3213,10 @@ static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs, l = len; mr = flatview_translate(fv, addr, &addr1, &l, true, attrs); + if (attrs.direct_access && !memory_access_is_direct(mr, true)) { + trace_memory_access_illegal(true, addr, len, mr->name); + return MEMTX_BUS_ERROR; + } result = flatview_write_continue(fv, addr, attrs, buf, len, addr1, l, mr); @@ -3275,6 +3279,10 @@ static MemTxResult flatview_read(FlatView *fv, hwaddr addr, l = len; mr = flatview_translate(fv, addr, &addr1, &l, false, attrs); + if (attrs.direct_access && !memory_access_is_direct(mr, false)) { + trace_memory_access_illegal(false, addr, len, mr->name); + return MEMTX_BUS_ERROR; + } return flatview_read_continue(fv, addr, attrs, buf, len, addr1, l, mr); } diff --git a/trace-events b/trace-events index 42107ebc697..931896735b1 100644 --- a/trace-events +++ b/trace-events @@ -54,6 +54,7 @@ find_ram_offset_loop(uint64_t size, uint64_t candidate, uint64_t offset, uint64_ ram_block_discard_range(const char *rbname, void *hva, size_t length, bool need_madvise, bool need_fallocate, int ret) "%s@%p + 0x%zx: madvise: %d fallocate: %d ret: %d" memory_notdirty_write_access(uint64_t vaddr, uint64_t ram_addr, unsigned size) "0x%" PRIx64 " ram_addr 0x%" PRIx64 " size %u" memory_notdirty_set_dirty(uint64_t vaddr) "0x%" PRIx64 +memory_access_illegal(bool is_write, uint64_t addr, uint64_t len, const char *mr_name) "is_write:%u addr:0x%08" PRIx64 " size:0x%04" PRIx64 " region:'%s'" # memory.c memory_region_ops_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u" -- 2.26.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [RFC PATCH 11/12] hw/pci: Only allow PCI slave devices to write to direct memory 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé ` (9 preceding siblings ...) 2020-09-03 11:08 ` [RFC PATCH 10/12] exec/memattrs: Introduce MemTxAttrs::direct_access field Philippe Mathieu-Daudé @ 2020-09-03 11:08 ` Philippe Mathieu-Daudé 2020-09-03 12:26 ` Paolo Bonzini 2020-09-03 11:08 ` [RFC PATCH 12/12] dma: Assert when device writes to indirect memory (such MMIO regions) Philippe Mathieu-Daudé ` (3 subsequent siblings) 14 siblings, 1 reply; 29+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-03 11:08 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Philippe Mathieu-Daudé, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Paolo Bonzini Do not allow PCI slaves to write to indirect memory regions such MMIO. This fixes LP#1886362 and LP#1888606. Example with the former reproducer: $ cat << EOF | \ qemu-system-i386 -M q35,accel=qtest \ -qtest stdio \ -trace memory_access\* \ outl 0xcf8 0x80001010 outl 0xcfc 0xe1020000 outl 0xcf8 0x80001014 outl 0xcf8 0x80001004 outw 0xcfc 0x7 outl 0xcf8 0x800010a2 write 0xe102003b 0x1 0xff write 0xe1020103 0x1e 0xffffff055c5e5c30be4511d084ffffffffffffffffffffffffffffffffff write 0xe1020420 0x4 0xffffffff write 0xe1020424 0x4 0xffffffff write 0xe102042b 0x1 0xff write 0xe1020430 0x4 0x055c5e5c write 0x5c041 0x1 0x04 write 0x5c042 0x1 0x02 write 0x5c043 0x1 0xe1 write 0x5c048 0x1 0x8a write 0x5c04a 0x1 0x31 write 0x5c04b 0x1 0xff write 0xe1020403 0x1 0xff EOF 562564:memory_access_illegal is_write:1 addr:0xe1020400 size:0x000e region:'e1000e-mmio' 562592:memory_access_illegal is_write:1 addr:0xe102040e size:0x007c region:'e1000e-mmio' 562601:memory_access_illegal is_write:1 addr:0xe102048a size:0x0004 region:'e1000e-mmio' Reported-by: Alexander Bulekov <alxndr@bu.edu> Buglink: https://bugs.launchpad.net/qemu/+bug/1886362 Buglink: https://bugs.launchpad.net/qemu/+bug/1888606 Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/hw/pci/pci.h | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 8f901e6c289..cd97268b3a8 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -788,8 +788,12 @@ static inline AddressSpace *pci_get_address_space(PCIDevice *dev) static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr, void *buf, dma_addr_t len, DMADirection dir) { + MemTxAttrs attrs = { + .direct_access = (dir == DMA_DIRECTION_FROM_DEVICE), + .requester_id = pci_requester_id(dev), + }; return dma_memory_rw(pci_get_address_space(dev), addr, buf, len, - dir, MEMTXATTRS_UNSPECIFIED); + dir, attrs); } static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr, @@ -808,14 +812,18 @@ static inline int pci_dma_write(PCIDevice *dev, dma_addr_t addr, static inline uint##_bits##_t ld##_l##_pci_dma(PCIDevice *dev, \ dma_addr_t addr) \ { \ - return ld##_l##_dma(pci_get_address_space(dev), addr, \ - MEMTXATTRS_UNSPECIFIED); \ + MemTxAttrs attrs = { \ + .requester_id = pci_requester_id(dev), \ + }; \ + return ld##_l##_dma(pci_get_address_space(dev), addr, attrs); \ } \ static inline void st##_s##_pci_dma(PCIDevice *dev, \ dma_addr_t addr, uint##_bits##_t val) \ { \ - st##_s##_dma(pci_get_address_space(dev), addr, val, \ - MEMTXATTRS_UNSPECIFIED); \ + MemTxAttrs attrs = { \ + .requester_id = pci_requester_id(dev), \ + }; \ + st##_s##_dma(pci_get_address_space(dev), addr, val, attrs); \ } PCI_DMA_DEFINE_LDST(ub, b, 8); -- 2.26.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 11/12] hw/pci: Only allow PCI slave devices to write to direct memory 2020-09-03 11:08 ` [RFC PATCH 11/12] hw/pci: Only allow PCI slave devices to write to direct memory Philippe Mathieu-Daudé @ 2020-09-03 12:26 ` Paolo Bonzini 2020-09-03 13:18 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 29+ messages in thread From: Paolo Bonzini @ 2020-09-03 12:26 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Klaus Jensen, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Peter Chubb On 03/09/20 13:08, Philippe Mathieu-Daudé wrote: > Do not allow PCI slaves to write to indirect memory > regions such MMIO. > > This fixes LP#1886362 and LP#1888606. What is a "PCI slave"? Which devices would still be allowed to write? I'm worried that there are cases of MMIO reads that would be broken. They are certainly niche these days, but they should still work; the most "famous" one is perhaps the old BASIC DEF SEG=&HB800 BLOAD "picture.pic", 0 Paolo ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 11/12] hw/pci: Only allow PCI slave devices to write to direct memory 2020-09-03 12:26 ` Paolo Bonzini @ 2020-09-03 13:18 ` Philippe Mathieu-Daudé 2020-09-03 21:43 ` Paolo Bonzini 0 siblings, 1 reply; 29+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-03 13:18 UTC (permalink / raw) To: Paolo Bonzini, qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Klaus Jensen, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Peter Chubb On 9/3/20 2:26 PM, Paolo Bonzini wrote: > On 03/09/20 13:08, Philippe Mathieu-Daudé wrote: >> Do not allow PCI slaves to write to indirect memory >> regions such MMIO. >> >> This fixes LP#1886362 and LP#1888606. > > What is a "PCI slave"? TBH I at a quick look at the PCIe SPEC, but not PCI. PCIe might be able to check the requester_id to check if the access is valid, but I'm not sure where to add this check. > Which devices would still be allowed to write? As of this patch, all the non-PCI, but I plan to add a similar check for USB on top of this series. > I'm worried that there are cases of MMIO reads that would be broken. > They are certainly niche these days, but they should still work; the > most "famous" one is perhaps the old BASIC > > DEF SEG=&HB800 > BLOAD "picture.pic", 0 This looks like ISA stuff. I don't think ISA does such checks (and didn't plan to add them there) but I'd need to verify. Do you have an acceptance test? > > Paolo > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 11/12] hw/pci: Only allow PCI slave devices to write to direct memory 2020-09-03 13:18 ` Philippe Mathieu-Daudé @ 2020-09-03 21:43 ` Paolo Bonzini 0 siblings, 0 replies; 29+ messages in thread From: Paolo Bonzini @ 2020-09-03 21:43 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Klaus Jensen, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Peter Chubb On 03/09/20 15:18, Philippe Mathieu-Daudé wrote: > As of this patch, all the non-PCI, but I plan to add a similar > check for USB on top of this series. Do you mean for memory-mapped USB host controllers? >> I'm worried that there are cases of MMIO reads that would be broken. >> They are certainly niche these days, but they should still work; the >> most "famous" one is perhaps the old BASIC >> >> DEF SEG=&HB800 >> BLOAD "picture.pic", 0 > > This looks like ISA stuff. I don't think ISA does such checks > (and didn't plan to add them there) but I'd need to verify. It works on bare metal even with a modern video card. > Do you have an acceptance test? Nope. :( Paolo ^ permalink raw reply [flat|nested] 29+ messages in thread
* [RFC PATCH 12/12] dma: Assert when device writes to indirect memory (such MMIO regions) 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé ` (10 preceding siblings ...) 2020-09-03 11:08 ` [RFC PATCH 11/12] hw/pci: Only allow PCI slave devices to write to direct memory Philippe Mathieu-Daudé @ 2020-09-03 11:08 ` Philippe Mathieu-Daudé 2020-09-03 13:51 ` Edgar E. Iglesias 2020-09-03 13:37 ` [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Laszlo Ersek ` (2 subsequent siblings) 14 siblings, 1 reply; 29+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-03 11:08 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Philippe Mathieu-Daudé, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Paolo Bonzini 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 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 12/12] dma: Assert when device writes to indirect memory (such MMIO regions) 2020-09-03 11:08 ` [RFC PATCH 12/12] dma: Assert when device writes to indirect memory (such MMIO regions) Philippe Mathieu-Daudé @ 2020-09-03 13:51 ` Edgar E. Iglesias 0 siblings, 0 replies; 29+ messages in thread From: Edgar E. Iglesias @ 2020-09-03 13:51 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, qemu-devel, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Klaus Jensen, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Paolo Bonzini 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 > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé ` (11 preceding siblings ...) 2020-09-03 11:08 ` [RFC PATCH 12/12] dma: Assert when device writes to indirect memory (such MMIO regions) Philippe Mathieu-Daudé @ 2020-09-03 13:37 ` Laszlo Ersek 2020-09-03 13:58 ` Peter Maydell 2020-09-05 2:27 ` Li Qiang 2020-09-08 14:37 ` Stefan Hajnoczi 14 siblings, 1 reply; 29+ messages in thread From: Laszlo Ersek @ 2020-09-03 13:37 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Klaus Jensen, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Paolo Bonzini Hi Phil, On 09/03/20 13:08, Philippe Mathieu-Daudé wrote: > Hi, > > I'm not suppose to work on this but I couldn't sleep so kept > wondering about this problem the whole night and eventually > woke up to write this quickly, so comments are scarce, sorry. > > The first part is obvious anyway, simply pass MemTxAttrs argument. > > The main patch is: > "exec/memattrs: Introduce MemTxAttrs::direct_access field". > This way we can restrict accesses to ROM/RAM by setting the > 'direct_access' field. Illegal accesses return MEMTX_BUS_ERROR. > > Next patch restrict PCI DMA accesses by setting the direct_access > field. > > Finally we add an assertion for any DMA write access to indirect > memory to kill a class of bug recently found by Alexander while > fuzzing. I've briefly checked LP#1886362 and LP#1888606, and as much as I understand them, they seem tricky. It's not clear how we can recognize long chains of DMA-to-MMIO transfers, and interrupt them. Peter mentions an approach at the end of <https://bugs.launchpad.net/qemu/+bug/1886362/comments/5> that I believe to understand, but -- according to him -- it seems too much work. And, I'm not too familiar with the qemu memory model, so I don't have comments on your solution. Maybe we can have some kind of "depth counter" for such recursive DMA-to-MMIO calls (even across multiple device models), and put an artificial limit on them, such as 5 or 10. This could be controlled on the QEMU command line perhaps? I don't think such chains work unto arbitrary depths on physical hardware either. Sorry that I don't have any sensible comments here. I hope I didn't misunderstand the problem at least. Laszlo ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions 2020-09-03 13:37 ` [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Laszlo Ersek @ 2020-09-03 13:58 ` Peter Maydell 2020-09-03 14:24 ` Edgar E. Iglesias 0 siblings, 1 reply; 29+ messages in thread From: Peter Maydell @ 2020-09-03 13:58 UTC (permalink / raw) To: Laszlo Ersek Cc: Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, QEMU Developers, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, Qemu-block, Li Qiang, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Philippe Mathieu-Daudé, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Klaus Jensen, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Paolo Bonzini On Thu, 3 Sep 2020 at 14:37, Laszlo Ersek <lersek@redhat.com> wrote: > Peter mentions an approach at the end of > <https://bugs.launchpad.net/qemu/+bug/1886362/comments/5> that I believe > to understand, but -- according to him -- it seems too much work. It also would only be effective for MMIO, not for qemu_irq lines... > I don't think such chains work unto arbitrary depths on physical > hardware either. Real hardware by and large doesn't get designed with this kind of DMA-to-self as a consideration either, but unfortunately it's not really very useful as a model to base QEMU's behaviour on: (1) real hardware is usually massively parallel, so the logic that handles incoming MMIO is decoupled anyway from logic that does outgoing DMA. (Arguably the "do all DMA in a bottom-half" idea is kind of following the hardware design.) Similarly simple "raise this outbound signal" logic just works as an instantaneous action that causes the device on the other end to change its state/do something parallel, whereas for QEMU we need to actually call some code in the device on the other end and so we serialize this stuff, sandwiching a bit of "device B code" in the middle of a run of "device A code". So a lot more of this stuff "just happens to work" on h/w than we get with QEMU. (2) if software running on real h/w does do something silly with programming a device to DMA to itself then the worst case is generally that they manage to wedge that device (or the whole machine, if you're really unlucky), in which case the response is "don't do that then". There isn't the same "guest code can escape the VM" security boundary that QEMU needs to guard against [*]. [*] I do wonder about hardware-device-passthrough setups; I don't think I would care to pass through an arbitrary device to an untrusted guest... thanks -- PMM ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions 2020-09-03 13:58 ` Peter Maydell @ 2020-09-03 14:24 ` Edgar E. Iglesias 2020-09-03 15:46 ` Paolo Bonzini 0 siblings, 1 reply; 29+ messages in thread From: Edgar E. Iglesias @ 2020-09-03 14:24 UTC (permalink / raw) To: Peter Maydell Cc: Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, QEMU Developers, Peter Xu, Gerd Hoffmann, Robert Foley, Edgar E . Iglesias, Qemu-block, Li Qiang, Laszlo Ersek, Emilio G . Cota, Peter Chubb, Joel Stanley, Richard Henderson, Philippe Mathieu-Daudé, Eduardo Habkost, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Klaus Jensen, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Paolo Bonzini On Thu, Sep 03, 2020 at 02:58:19PM +0100, Peter Maydell wrote: > On Thu, 3 Sep 2020 at 14:37, Laszlo Ersek <lersek@redhat.com> wrote: > > Peter mentions an approach at the end of > > <https://bugs.launchpad.net/qemu/+bug/1886362/comments/5> that I believe > > to understand, but -- according to him -- it seems too much work. > > It also would only be effective for MMIO, not for qemu_irq lines... > > > I don't think such chains work unto arbitrary depths on physical > > hardware either. > > Real hardware by and large doesn't get designed with this kind > of DMA-to-self as a consideration either, but unfortunately it's > not really very useful as a model to base QEMU's behaviour on: > > (1) real hardware is usually massively parallel, so the logic > that handles incoming MMIO is decoupled anyway from logic > that does outgoing DMA. (Arguably the "do all DMA in a > bottom-half" idea is kind of following the hardware design.) > Similarly simple "raise this outbound signal" logic just > works as an instantaneous action that causes the device on > the other end to change its state/do something parallel, > whereas for QEMU we need to actually call some code in the > device on the other end and so we serialize this stuff, > sandwiching a bit of "device B code" in the middle of a > run of "device A code". So a lot more of this stuff "just > happens to work" on h/w than we get with QEMU. > (2) if software running on real h/w does do something silly with > programming a device to DMA to itself then the worst case is > generally that they manage to wedge that device (or the whole > machine, if you're really unlucky), in which case the response > is "don't do that then". There isn't the same "guest code > can escape the VM" security boundary that QEMU needs to guard > against [*]. > > [*] I do wonder about hardware-device-passthrough setups; I > don't think I would care to pass through an arbitrary device > to an untrusted guest... Hmm, I guess it would make sense to have a configurable option in KVM to isolate passthrough devices so they only can DMA to guest RAM... Cheers, Edgar ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions 2020-09-03 14:24 ` Edgar E. Iglesias @ 2020-09-03 15:46 ` Paolo Bonzini 2020-09-03 15:50 ` Edgar E. Iglesias 0 siblings, 1 reply; 29+ messages in thread From: Paolo Bonzini @ 2020-09-03 15:46 UTC (permalink / raw) To: Edgar E. Iglesias, Peter Maydell Cc: Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, QEMU Developers, Peter Xu, Gerd Hoffmann, Robert Foley, Edgar E . Iglesias, Qemu-block, Li Qiang, Emilio G . Cota, Joel Stanley, Richard Henderson, Philippe Mathieu-Daudé, Eduardo Habkost, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Laszlo Ersek, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Peter Chubb On 03/09/20 16:24, Edgar E. Iglesias wrote: >> [*] I do wonder about hardware-device-passthrough setups; I >> don't think I would care to pass through an arbitrary device >> to an untrusted guest... > Hmm, I guess it would make sense to have a configurable option in KVM > to isolate passthrough devices so they only can DMA to guest RAM... Passthrough devices are always protected by the IOMMU, anything else would be obviously insane^H^H^Hecure. :) Paolo ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions 2020-09-03 15:46 ` Paolo Bonzini @ 2020-09-03 15:50 ` Edgar E. Iglesias 2020-09-03 17:53 ` Paolo Bonzini 0 siblings, 1 reply; 29+ messages in thread From: Edgar E. Iglesias @ 2020-09-03 15:50 UTC (permalink / raw) To: Paolo Bonzini Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, QEMU Developers, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Qemu-block, Li Qiang, Emilio G . Cota, Joel Stanley, Richard Henderson, Philippe Mathieu-Daudé, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Laszlo Ersek, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Peter Chubb On Thu, Sep 03, 2020 at 05:46:39PM +0200, Paolo Bonzini wrote: > On 03/09/20 16:24, Edgar E. Iglesias wrote: > >> [*] I do wonder about hardware-device-passthrough setups; I > >> don't think I would care to pass through an arbitrary device > >> to an untrusted guest... > > Hmm, I guess it would make sense to have a configurable option in KVM > > to isolate passthrough devices so they only can DMA to guest RAM... > > Passthrough devices are always protected by the IOMMU, anything else > would be obviously insane^H^H^Hecure. :) Really? To always do that blindly seems wrong. I'm refering to the passthrough device not being able to reach registers of other passthrough devices within the same guest. Obviously the IOMMU should be setup so that passthrough devices don't reach\ other guests or the host. Cheers, Edgar ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions 2020-09-03 15:50 ` Edgar E. Iglesias @ 2020-09-03 17:53 ` Paolo Bonzini 2020-09-03 19:46 ` Edgar E. Iglesias 0 siblings, 1 reply; 29+ messages in thread From: Paolo Bonzini @ 2020-09-03 17:53 UTC (permalink / raw) To: Edgar E. Iglesias Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, QEMU Developers, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Qemu-block, Li Qiang, Emilio G . Cota, Joel Stanley, Richard Henderson, Philippe Mathieu-Daudé, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Laszlo Ersek, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Peter Chubb On 03/09/20 17:50, Edgar E. Iglesias wrote: >>> Hmm, I guess it would make sense to have a configurable option in KVM >>> to isolate passthrough devices so they only can DMA to guest RAM... >> >> Passthrough devices are always protected by the IOMMU, anything else >> would be obviously insane^H^H^Hecure. :) > > Really? To always do that blindly seems wrong. > > I'm refering to the passthrough device not being able to reach registers > of other passthrough devices within the same guest. Ah okay; sorry, I misunderstood. That makes more sense now! Multiple devices are put in the same IOMMU "container" (page table basically), and that takes care of reaching registers of other passthrough devices. Paolo > Obviously the IOMMU should be setup so that passthrough devices don't reach\ > other guests or the host. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions 2020-09-03 17:53 ` Paolo Bonzini @ 2020-09-03 19:46 ` Edgar E. Iglesias 2020-09-04 2:50 ` Jason Wang 0 siblings, 1 reply; 29+ messages in thread From: Edgar E. Iglesias @ 2020-09-03 19:46 UTC (permalink / raw) To: Paolo Bonzini Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, QEMU Developers, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Qemu-block, Li Qiang, Emilio G . Cota, Joel Stanley, Richard Henderson, Philippe Mathieu-Daudé, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, David Gibson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Laszlo Ersek, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Peter Chubb On Thu, Sep 03, 2020 at 07:53:33PM +0200, Paolo Bonzini wrote: > On 03/09/20 17:50, Edgar E. Iglesias wrote: > >>> Hmm, I guess it would make sense to have a configurable option in KVM > >>> to isolate passthrough devices so they only can DMA to guest RAM... > >> > >> Passthrough devices are always protected by the IOMMU, anything else > >> would be obviously insane^H^H^Hecure. :) > > > > Really? To always do that blindly seems wrong. > > > > I'm refering to the passthrough device not being able to reach registers > > of other passthrough devices within the same guest. > > Ah okay; sorry, I misunderstood. That makes more sense now! > > Multiple devices are put in the same IOMMU "container" (page table > basically), and that takes care of reaching registers of other > passthrough devices. Thanks, yes, that's a sane default. What I was trying to say before is that it may make sense to allow the user to "harden" the setup by selectivly putting certain passthrough devs on a separate group that can *only* DMA access guest RAM (not other device regs). Some devs need access to other device's regs but many passthrough devs don't need DMA access to anything else but RAM (e.g an Ethernet MAC). That could mitigate the damage caused by wild DMA pointers... Cheers, Edgar ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions 2020-09-03 19:46 ` Edgar E. Iglesias @ 2020-09-04 2:50 ` Jason Wang 0 siblings, 0 replies; 29+ messages in thread From: Jason Wang @ 2020-09-04 2:50 UTC (permalink / raw) To: Edgar E. Iglesias, Paolo Bonzini Cc: Peter Maydell, Michael S. Tsirkin, Mark Cave-Ayland, QEMU Developers, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Qemu-block, Li Qiang, Emilio G . Cota, Joel Stanley, David Gibson, Philippe Mathieu-Daudé, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Jan Kiszka, Cédric Le Goater, Stefan Hajnoczi, John Snow, Richard Henderson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Laszlo Ersek, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Peter Chubb On 2020/9/4 上午3:46, Edgar E. Iglesias wrote: > On Thu, Sep 03, 2020 at 07:53:33PM +0200, Paolo Bonzini wrote: >> On 03/09/20 17:50, Edgar E. Iglesias wrote: >>>>> Hmm, I guess it would make sense to have a configurable option in KVM >>>>> to isolate passthrough devices so they only can DMA to guest RAM... >>>> Passthrough devices are always protected by the IOMMU, anything else >>>> would be obviously insane^H^H^Hecure. :) >>> Really? To always do that blindly seems wrong. >>> >>> I'm refering to the passthrough device not being able to reach registers >>> of other passthrough devices within the same guest. >> Ah okay; sorry, I misunderstood. That makes more sense now! >> >> Multiple devices are put in the same IOMMU "container" (page table >> basically), and that takes care of reaching registers of other >> passthrough devices. > Thanks, yes, that's a sane default. What I was trying to say before is that > it may make sense to allow the user to "harden" the setup by selectivly > putting certain passthrough devs on a separate group that can *only* > DMA access guest RAM (not other device regs). This makes sens but it requires the knowledge from the management layer of whether P2P is needed which is probably not easy. Thanks > > Some devs need access to other device's regs but many passthrough devs don't > need DMA access to anything else but RAM (e.g an Ethernet MAC). > > That could mitigate the damage caused by wild DMA pointers... > > Cheers, > Edgar > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé ` (12 preceding siblings ...) 2020-09-03 13:37 ` [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Laszlo Ersek @ 2020-09-05 2:27 ` Li Qiang 2020-09-08 14:37 ` Stefan Hajnoczi 14 siblings, 0 replies; 29+ messages in thread From: Li Qiang @ 2020-09-05 2:27 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, Qemu Developers, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Joel Stanley, David Gibson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Peter Chubb, Cédric Le Goater, Stefan Hajnoczi, Paolo Bonzini, John Snow, Richard Henderson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Klaus Jensen, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Jan Kiszka Philippe Mathieu-Daudé <philmd@redhat.com> 于2020年9月3日周四 下午7:09写道： > > Hi, > > I'm not suppose to work on this but I couldn't sleep so kept > wondering about this problem the whole night and eventually > woke up to write this quickly, so comments are scarce, sorry. > > The first part is obvious anyway, simply pass MemTxAttrs argument. > > The main patch is: > "exec/memattrs: Introduce MemTxAttrs::direct_access field". > This way we can restrict accesses to ROM/RAM by setting the > 'direct_access' field. Illegal accesses return MEMTX_BUS_ERROR. > > Next patch restrict PCI DMA accesses by setting the direct_access > field. > > Finally we add an assertion for any DMA write access to indirect > memory to kill a class of bug recently found by Alexander while > fuzzing. > Hi Philippe, I have reviewed your patches. Your patch just deny the DMA write to MMIO for PCI device. 1. The DMA write to MMIO is allowed for P2P. Unconditionally deny is not right I think. Maybe we can add some flag for the device as property so the device can indicate whether it supports DMA to MMIO. But this method needs define we should apply the restrict to DMA to MMIO initiator or target. If the target, we need to find the target PCI device. 2. I think the MMIO read maybe also suffers the reentrant issue If the MMIO read handler does complicated work. 3. As your patch just consider the PCI case. This reentrant is quite complicated if we consider the no-PCI the qemu_irq cases. I agree to address the PCI cases first. Thanks, Li Qiang > Regards, > > Phil. > > Klaus Jensen (1): > pci: pass along the return value of dma_memory_rw > > Philippe Mathieu-Daudé (11): > dma: Let dma_memory_valid() take MemTxAttrs argument > dma: Let dma_memory_set() take MemTxAttrs argument > dma: Let dma_memory_rw_relaxed() take MemTxAttrs argument > dma: Let dma_memory_rw() take MemTxAttrs argument > dma: Let dma_memory_read/write() take MemTxAttrs argument > dma: Let dma_memory_map() take MemTxAttrs argument > docs/devel/loads-stores: Add regexp for DMA functions > dma: Let load/store DMA functions take MemTxAttrs argument > exec/memattrs: Introduce MemTxAttrs::direct_access field > hw/pci: Only allow PCI slave devices to write to direct memory > dma: Assert when device writes to indirect memory (such MMIO regions) > > docs/devel/loads-stores.rst | 2 ++ > include/exec/memattrs.h | 3 ++ > include/hw/pci/pci.h | 21 ++++++++++--- > include/hw/ppc/spapr_vio.h | 26 +++++++++------ > include/sysemu/dma.h | 59 +++++++++++++++++++++-------------- > dma-helpers.c | 12 ++++--- > exec.c | 8 +++++ > hw/arm/musicpal.c | 13 ++++---- > hw/arm/smmu-common.c | 3 +- > hw/arm/smmuv3.c | 14 ++++++--- > hw/core/generic-loader.c | 3 +- > hw/display/virtio-gpu.c | 8 +++-- > hw/dma/pl330.c | 12 ++++--- > hw/dma/sparc32_dma.c | 16 ++++++---- > hw/dma/xlnx-zynq-devcfg.c | 6 ++-- > hw/dma/xlnx_dpdma.c | 10 +++--- > hw/hyperv/vmbus.c | 8 +++-- > hw/i386/amd_iommu.c | 16 +++++----- > hw/i386/intel_iommu.c | 28 ++++++++++------- > hw/ide/ahci.c | 9 ++++-- > hw/ide/macio.c | 2 +- > hw/intc/pnv_xive.c | 7 +++-- > hw/intc/spapr_xive.c | 3 +- > hw/intc/xive.c | 7 +++-- > hw/misc/bcm2835_property.c | 3 +- > hw/misc/macio/mac_dbdma.c | 10 +++--- > hw/net/allwinner-sun8i-emac.c | 21 ++++++++----- > hw/net/ftgmac100.c | 25 +++++++++------ > hw/net/imx_fec.c | 32 ++++++++++++------- > hw/nvram/fw_cfg.c | 16 ++++++---- > hw/pci-host/pnv_phb3.c | 5 +-- > hw/pci-host/pnv_phb3_msi.c | 9 ++++-- > hw/pci-host/pnv_phb4.c | 7 +++-- > hw/sd/allwinner-sdhost.c | 14 +++++---- > hw/sd/sdhci.c | 35 +++++++++++++-------- > hw/usb/hcd-dwc2.c | 8 ++--- > hw/usb/hcd-ehci.c | 6 ++-- > hw/usb/hcd-ohci.c | 28 ++++++++++------- > hw/usb/libhw.c | 3 +- > hw/virtio/virtio.c | 6 ++-- > trace-events | 1 + > 41 files changed, 334 insertions(+), 191 deletions(-) > > -- > 2.26.2 > > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé ` (13 preceding siblings ...) 2020-09-05 2:27 ` Li Qiang @ 2020-09-08 14:37 ` Stefan Hajnoczi 2020-09-09 13:23 ` Peter Maydell 14 siblings, 1 reply; 29+ messages in thread From: Stefan Hajnoczi @ 2020-09-08 14:37 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Peter Maydell, Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, qemu-devel, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, qemu-block, Li Qiang, Emilio G . Cota, Joel Stanley, David Gibson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Peter Chubb, Cédric Le Goater, Stefan Hajnoczi, Paolo Bonzini, Richard Henderson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Klaus Jensen, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Jan Kiszka [-- Attachment #1: Type: text/plain, Size: 395 bytes --] On Thu, Sep 03, 2020 at 01:08:19PM +0200, Philippe Mathieu-Daudé wrote: > The main patch is: > "exec/memattrs: Introduce MemTxAttrs::direct_access field". > This way we can restrict accesses to ROM/RAM by setting the > 'direct_access' field. Illegal accesses return MEMTX_BUS_ERROR. QEMU needs to simulate the behavior of real hardware. What is the behavior of real hardware? Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions 2020-09-08 14:37 ` Stefan Hajnoczi @ 2020-09-09 13:23 ` Peter Maydell 2020-09-09 13:41 ` Stefan Hajnoczi 0 siblings, 1 reply; 29+ messages in thread From: Peter Maydell @ 2020-09-09 13:23 UTC (permalink / raw) To: Stefan Hajnoczi Cc: Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, QEMU Developers, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, Qemu-block, Li Qiang, Emilio G . Cota, Joel Stanley, David Gibson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Peter Chubb, Cédric Le Goater, Stefan Hajnoczi, Paolo Bonzini, Richard Henderson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Philippe Mathieu-Daudé, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Jan Kiszka On Wed, 9 Sep 2020 at 10:12, Stefan Hajnoczi <stefanha@gmail.com> wrote: > > On Thu, Sep 03, 2020 at 01:08:19PM +0200, Philippe Mathieu-Daudé wrote: > > The main patch is: > > "exec/memattrs: Introduce MemTxAttrs::direct_access field". > > This way we can restrict accesses to ROM/RAM by setting the > > 'direct_access' field. Illegal accesses return MEMTX_BUS_ERROR. > > QEMU needs to simulate the behavior of real hardware. What is the > behavior of real hardware? It varies, depending on the hardware. The most common thing is probably "happens to work by luck", which is OK for hardware but doesn't help us much since our software implementation is naturally more serialized than hardware is and since we don't want to allow guests to make QEMU crash or misbehave. thanks -- PMM ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions 2020-09-09 13:23 ` Peter Maydell @ 2020-09-09 13:41 ` Stefan Hajnoczi 0 siblings, 0 replies; 29+ messages in thread From: Stefan Hajnoczi @ 2020-09-09 13:41 UTC (permalink / raw) To: Peter Maydell Cc: Michael S. Tsirkin, Jason Wang, Mark Cave-Ayland, QEMU Developers, Peter Xu, Gerd Hoffmann, Edgar E. Iglesias, Eduardo Habkost, Edgar E . Iglesias, Qemu-block, Li Qiang, Emilio G . Cota, Joel Stanley, David Gibson, Laszlo Ersek, Robert Foley, Alistair Francis, Richard Henderson, Beniamino Galvani, Eric Auger, qemu-arm, Peter Chubb, Cédric Le Goater, Stefan Hajnoczi, Paolo Bonzini, Richard Henderson, Tony Nguyen, Prasad J Pandit, Alexander Bulekov, Andrew Jeffery, Philippe Mathieu-Daudé, Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé, Andrew Baumann, qemu-ppc, Klaus Jensen, Jan Kiszka On Wed, Sep 9, 2020 at 2:23 PM Peter Maydell <peter.maydell@linaro.org> wrote: > On Wed, 9 Sep 2020 at 10:12, Stefan Hajnoczi <stefanha@gmail.com> wrote: > > On Thu, Sep 03, 2020 at 01:08:19PM +0200, Philippe Mathieu-Daudé wrote: > > > The main patch is: > > > "exec/memattrs: Introduce MemTxAttrs::direct_access field". > > > This way we can restrict accesses to ROM/RAM by setting the > > > 'direct_access' field. Illegal accesses return MEMTX_BUS_ERROR. > > > > QEMU needs to simulate the behavior of real hardware. What is the > > behavior of real hardware? > > It varies, depending on the hardware. The most common thing > is probably "happens to work by luck", which is OK for hardware > but doesn't help us much since our software implementation is > naturally more serialized than hardware is and since we don't > want to allow guests to make QEMU crash or misbehave. The memory API bounce buffer mechanism is evidence that some board(s) need or needed it. At a minimum we need to find out the reason for the bounce buffer mechanism to avoid breaking guests. Stefan ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2020-09-09 13:42 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-03 11:08 [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 01/12] pci: pass along the return value of dma_memory_rw Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 02/12] dma: Let dma_memory_valid() take MemTxAttrs argument Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 03/12] dma: Let dma_memory_set() " Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 04/12] dma: Let dma_memory_rw_relaxed() " Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 05/12] dma: Let dma_memory_rw() " Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 06/12] dma: Let dma_memory_read/write() " Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 07/12] dma: Let dma_memory_map() " Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 08/12] docs/devel/loads-stores: Add regexp for DMA functions Philippe Mathieu-Daudé 2020-09-03 11:08 ` [PATCH 09/12] dma: Let load/store DMA functions take MemTxAttrs argument Philippe Mathieu-Daudé 2020-09-03 11:08 ` [RFC PATCH 10/12] exec/memattrs: Introduce MemTxAttrs::direct_access field Philippe Mathieu-Daudé 2020-09-03 11:08 ` [RFC PATCH 11/12] hw/pci: Only allow PCI slave devices to write to direct memory Philippe Mathieu-Daudé 2020-09-03 12:26 ` Paolo Bonzini 2020-09-03 13:18 ` Philippe Mathieu-Daudé 2020-09-03 21:43 ` Paolo Bonzini 2020-09-03 11:08 ` [RFC PATCH 12/12] dma: Assert when device writes to indirect memory (such MMIO regions) Philippe Mathieu-Daudé 2020-09-03 13:51 ` Edgar E. Iglesias 2020-09-03 13:37 ` [RFC PATCH 00/12] hw: Forbid DMA write accesses to MMIO regions Laszlo Ersek 2020-09-03 13:58 ` Peter Maydell 2020-09-03 14:24 ` Edgar E. Iglesias 2020-09-03 15:46 ` Paolo Bonzini 2020-09-03 15:50 ` Edgar E. Iglesias 2020-09-03 17:53 ` Paolo Bonzini 2020-09-03 19:46 ` Edgar E. Iglesias 2020-09-04 2:50 ` Jason Wang 2020-09-05 2:27 ` Li Qiang 2020-09-08 14:37 ` Stefan Hajnoczi 2020-09-09 13:23 ` Peter Maydell 2020-09-09 13:41 ` Stefan Hajnoczi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.