Re: [PATCH 2/4] dma: Let ld*_dma() take MemTxAttrs argument

[Cédric Le Goater]

On 12/18/21 15:51, Philippe Mathieu-Daudé wrote:
> Let devices specify transaction attributes when calling ld*_dma().
>
> Keep the default MEMTXATTRS_UNSPECIFIED in the few callers.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   include/hw/pci/pci.h       |  3 ++-
>   include/hw/ppc/spapr_vio.h |  3 ++-
>   include/sysemu/dma.h       | 11 ++++++-----
>   hw/intc/pnv_xive.c         |  7 ++++---
>   hw/usb/hcd-xhci.c          |  6 +++---
>   5 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index d07e9707b48..0613308b1b6 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -854,7 +854,8 @@ static inline MemTxResult 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) 
> \
> diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h
> index e87f8e6f596..d2ec9b0637f 100644
> --- a/include/hw/ppc/spapr_vio.h
> +++ b/include/hw/ppc/spapr_vio.h
> @@ -126,7 +126,8 @@ static inline int spapr_vio_dma_set(SpaprVioDevice *dev, 
> uint64_t taddr,
>           (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)))
> +#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 2a60d2c5d61..b711d390a4f 100644
> --- a/include/sysemu/dma.h
> +++ b/include/sysemu/dma.h
> @@ -242,10 +242,11 @@ 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,      \
> @@ -254,14 +255,14 @@ static inline void dma_memory_unmap(AddressSpace *as,
>                                                    MemTxAttrs attrs)      \
>       {                                                                   \
>           val = cpu_to_##_end##_bits(val);                                \
> -        dma_memory_write(as, addr, &val, (_bits) / 8, attrs); \
> +        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;
>   }
>   
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index ad43483612e..d9249bbc0c1 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -172,7 +172,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
> @@ -195,7 +195,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
> @@ -542,7 +543,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/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index ed2b9ea456e..d960b814587 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -2062,7 +2062,7 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, 
> unsigned int slotid,
>       assert(slotid >= 1 && slotid <= xhci->numslots);
>   
>       dcbaap = xhci_addr64(xhci->dcbaap_low, xhci->dcbaap_high);
> -    poctx = ldq_le_dma(xhci->as, dcbaap + 8 * slotid);
> +    poctx = ldq_le_dma(xhci->as, dcbaap + 8 * slotid, MEMTXATTRS_UNSPECIFIED);
>       ictx = xhci_mask64(pictx);
>       octx = xhci_mask64(poctx);
>   
> @@ -3437,8 +3437,8 @@ static int usb_xhci_post_load(void *opaque, int version_id)
>           if (!slot->addressed) {
>               continue;
>           }
> -        slot->ctx =
> -            xhci_mask64(ldq_le_dma(xhci->as, dcbaap + 8 * slotid));
> +        slot->ctx = xhci_mask64(ldq_le_dma(xhci->as, dcbaap + 8 * slotid,
> +                                           MEMTXATTRS_UNSPECIFIED));
>           xhci_dma_read_u32s(xhci, slot->ctx, slot_ctx, sizeof(slot_ctx));
>           slot->uport = xhci_lookup_uport(xhci, slot_ctx);
>           if (!slot->uport) {