[PATCH 12/13] hw/pci/pci_device: Introduce ld/st_endian_pci_dma() API

Philippe Mathieu-Daudé posted 13 patches 1 month, 3 weeks ago
[PATCH 12/13] hw/pci/pci_device: Introduce ld/st_endian_pci_dma() API
Posted by Philippe Mathieu-Daudé 1 month, 3 weeks ago
Introduce the ld/st_endian_pci_dma() API, which takes an extra
boolean argument to dispatch to ld/st_{be,le}_pci_dma() methods.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
TODO: Update docstring regexp
---
 include/hw/pci/pci_device.h | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index ff619241a4..dc9b17dded 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -300,7 +300,29 @@ static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
 
 #define PCI_DMA_DEFINE_LDST_END(_l, _s, _bits) \
     PCI_DMA_DEFINE_LDST(_l##_le, _s##_le, _bits) \
-    PCI_DMA_DEFINE_LDST(_l##_be, _s##_be, _bits)
+    PCI_DMA_DEFINE_LDST(_l##_be, _s##_be, _bits) \
+    static inline MemTxResult ld##_l##_endian_pci_dma(bool is_big_endian, \
+                                                      PCIDevice *dev, \
+                                                      dma_addr_t addr, \
+                                                      uint##_bits##_t *val, \
+                                                      MemTxAttrs attrs) \
+    { \
+        AddressSpace *pci_as = pci_get_address_space(dev); \
+        return is_big_endian \
+               ? ld##_l##_be_dma(pci_as, addr, val, attrs) \
+               : ld##_l##_le_dma(pci_as, addr, val, attrs); \
+    } \
+    static inline MemTxResult st##_s##_endian_pci_dma(bool is_big_endian, \
+                                                      PCIDevice *dev, \
+                                                      dma_addr_t addr, \
+                                                      uint##_bits##_t val, \
+                                                      MemTxAttrs attrs) \
+    { \
+        AddressSpace *pci_as = pci_get_address_space(dev); \
+        return is_big_endian \
+               ? st##_s##_be_dma(pci_as, addr, val, attrs) \
+               : st##_s##_le_dma(pci_as, addr, val, attrs); \
+    }
 
 PCI_DMA_DEFINE_LDST(ub, b, 8);
 PCI_DMA_DEFINE_LDST_END(uw, w, 16)
-- 
2.45.2


Re: [PATCH 12/13] hw/pci/pci_device: Introduce ld/st_endian_pci_dma() API
Posted by Richard Henderson 1 month, 3 weeks ago
On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Introduce the ld/st_endian_pci_dma() API, which takes an extra
> boolean argument to dispatch to ld/st_{be,le}_pci_dma() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update docstring regexp
> ---
>   include/hw/pci/pci_device.h | 24 +++++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> index ff619241a4..dc9b17dded 100644
> --- a/include/hw/pci/pci_device.h
> +++ b/include/hw/pci/pci_device.h
> @@ -300,7 +300,29 @@ static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
>   
>   #define PCI_DMA_DEFINE_LDST_END(_l, _s, _bits) \
>       PCI_DMA_DEFINE_LDST(_l##_le, _s##_le, _bits) \
> -    PCI_DMA_DEFINE_LDST(_l##_be, _s##_be, _bits)
> +    PCI_DMA_DEFINE_LDST(_l##_be, _s##_be, _bits) \
> +    static inline MemTxResult ld##_l##_endian_pci_dma(bool is_big_endian, \
> +                                                      PCIDevice *dev, \
> +                                                      dma_addr_t addr, \
> +                                                      uint##_bits##_t *val, \
> +                                                      MemTxAttrs attrs) \
> +    { \
> +        AddressSpace *pci_as = pci_get_address_space(dev); \
> +        return is_big_endian \
> +               ? ld##_l##_be_dma(pci_as, addr, val, attrs) \
> +               : ld##_l##_le_dma(pci_as, addr, val, attrs); \
> +    } \

Like the address_space_* functions, I think the endianness is being handled at the wrong 
level here.  Improve sysemu/dma.h first.


r~

Re: [PATCH 12/13] hw/pci/pci_device: Introduce ld/st_endian_pci_dma() API
Posted by Pierrick Bouvier 1 month, 3 weeks ago
On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Introduce the ld/st_endian_pci_dma() API, which takes an extra
> boolean argument to dispatch to ld/st_{be,le}_pci_dma() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update docstring regexp
> ---
>   include/hw/pci/pci_device.h | 24 +++++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> index ff619241a4..dc9b17dded 100644
> --- a/include/hw/pci/pci_device.h
> +++ b/include/hw/pci/pci_device.h
> @@ -300,7 +300,29 @@ static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
>   
>   #define PCI_DMA_DEFINE_LDST_END(_l, _s, _bits) \
>       PCI_DMA_DEFINE_LDST(_l##_le, _s##_le, _bits) \
> -    PCI_DMA_DEFINE_LDST(_l##_be, _s##_be, _bits)
> +    PCI_DMA_DEFINE_LDST(_l##_be, _s##_be, _bits) \
> +    static inline MemTxResult ld##_l##_endian_pci_dma(bool is_big_endian, \
> +                                                      PCIDevice *dev, \
> +                                                      dma_addr_t addr, \
> +                                                      uint##_bits##_t *val, \
> +                                                      MemTxAttrs attrs) \
> +    { \
> +        AddressSpace *pci_as = pci_get_address_space(dev); \
> +        return is_big_endian \
> +               ? ld##_l##_be_dma(pci_as, addr, val, attrs) \
> +               : ld##_l##_le_dma(pci_as, addr, val, attrs); \
> +    } \
> +    static inline MemTxResult st##_s##_endian_pci_dma(bool is_big_endian, \
> +                                                      PCIDevice *dev, \
> +                                                      dma_addr_t addr, \
> +                                                      uint##_bits##_t val, \
> +                                                      MemTxAttrs attrs) \
> +    { \
> +        AddressSpace *pci_as = pci_get_address_space(dev); \
> +        return is_big_endian \
> +               ? st##_s##_be_dma(pci_as, addr, val, attrs) \
> +               : st##_s##_le_dma(pci_as, addr, val, attrs); \
> +    }
>   
>   PCI_DMA_DEFINE_LDST(ub, b, 8);
>   PCI_DMA_DEFINE_LDST_END(uw, w, 16)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>