Re: [PATCH 1/2] dma-mapping: Add dma_addr_is_phys_addr()
On Mon, Oct 14, 2019 at 11:29:24AM +0100, Robin Murphy wrote: >> However, I would like to see the commit message (and maybe the inline >> comments) expanded a bit on what the distinction here is about. Some >> of the text from the next patch would be suitable, about DMA addresses >> usually being in a different address space but not in the case of >> bounce buffering. > > Right, this needs a much tighter definition. "DMA address happens to be a > valid physical address" is true of various IOMMU setups too, but I can't > believe it's meaningful in such cases. > > If what you actually want is "DMA is direct or SWIOTLB" - i.e. "DMA address > is physical address of DMA data (not necessarily the original buffer)" - > wouldn't dma_is_direct() suffice? It would. But drivers have absolutely no business knowing any of this.
RE: [PATCH 1/2] dma-mapping: Add dma_addr_is_phys_addr()
On Mon, Oct 14, 2019 at 11:29:24AM +0100, Robin Murphy wrote: > On 14/10/2019 05:51, David Gibson wrote: > >On Fri, Oct 11, 2019 at 06:25:18PM -0700, Ram Pai wrote: > >>From: Thiago Jung Bauermann > >> > >>In order to safely use the DMA API, virtio needs to know whether DMA > >>addresses are in fact physical addresses and for that purpose, > >>dma_addr_is_phys_addr() is introduced. > >> > >>cc: Benjamin Herrenschmidt > >>cc: David Gibson > >>cc: Michael Ellerman > >>cc: Paul Mackerras > >>cc: Michael Roth > >>cc: Alexey Kardashevskiy > >>cc: Paul Burton > >>cc: Robin Murphy > >>cc: Bartlomiej Zolnierkiewicz > >>cc: Marek Szyprowski > >>cc: Christoph Hellwig > >>Suggested-by: Michael S. Tsirkin > >>Signed-off-by: Ram Pai > >>Signed-off-by: Thiago Jung Bauermann > > > >The change itself looks ok, so > > > >Reviewed-by: David Gibson > > > >However, I would like to see the commit message (and maybe the inline > >comments) expanded a bit on what the distinction here is about. Some > >of the text from the next patch would be suitable, about DMA addresses > >usually being in a different address space but not in the case of > >bounce buffering. > > Right, this needs a much tighter definition. "DMA address happens to > be a valid physical address" is true of various IOMMU setups too, > but I can't believe it's meaningful in such cases. The definition by itself is meaningful AFAICT. At its core its just indicating whether DMA addresses are physical addresses or not. However its up to the caller to use it meaningfully. For non-virtio caller, dma_addr_is_phys_addr() by itself may or may not be meaningful. But for a virtio caller, this information when paired with mem_encrypt_active() is meaningful. For IOMMU setups DMA API will get used regardless of "DMA address happens to be a valid physical address" > > If what you actually want is "DMA is direct or SWIOTLB" - i.e. "DMA > address is physical address of DMA data (not necessarily the > original buffer)" - wouldn't dma_is_direct() suffice? This may also work, I think. MST: thoughts? RP
Re: [PATCH 1/2] dma-mapping: Add dma_addr_is_phys_addr()
On 14/10/2019 05:51, David Gibson wrote:
On Fri, Oct 11, 2019 at 06:25:18PM -0700, Ram Pai wrote:
From: Thiago Jung Bauermann
In order to safely use the DMA API, virtio needs to know whether DMA
addresses are in fact physical addresses and for that purpose,
dma_addr_is_phys_addr() is introduced.
cc: Benjamin Herrenschmidt
cc: David Gibson
cc: Michael Ellerman
cc: Paul Mackerras
cc: Michael Roth
cc: Alexey Kardashevskiy
cc: Paul Burton
cc: Robin Murphy
cc: Bartlomiej Zolnierkiewicz
cc: Marek Szyprowski
cc: Christoph Hellwig
Suggested-by: Michael S. Tsirkin
Signed-off-by: Ram Pai
Signed-off-by: Thiago Jung Bauermann
The change itself looks ok, so
Reviewed-by: David Gibson
However, I would like to see the commit message (and maybe the inline
comments) expanded a bit on what the distinction here is about. Some
of the text from the next patch would be suitable, about DMA addresses
usually being in a different address space but not in the case of
bounce buffering.
Right, this needs a much tighter definition. "DMA address happens to be
a valid physical address" is true of various IOMMU setups too, but I
can't believe it's meaningful in such cases.
If what you actually want is "DMA is direct or SWIOTLB" - i.e. "DMA
address is physical address of DMA data (not necessarily the original
buffer)" - wouldn't dma_is_direct() suffice?
Robin.
---
arch/powerpc/include/asm/dma-mapping.h | 21 +
arch/powerpc/platforms/pseries/Kconfig | 1 +
include/linux/dma-mapping.h| 20
kernel/dma/Kconfig | 3 +++
4 files changed, 45 insertions(+)
diff --git a/arch/powerpc/include/asm/dma-mapping.h
b/arch/powerpc/include/asm/dma-mapping.h
index 565d6f7..f92c0a4b 100644
--- a/arch/powerpc/include/asm/dma-mapping.h
+++ b/arch/powerpc/include/asm/dma-mapping.h
@@ -5,6 +5,8 @@
#ifndef _ASM_DMA_MAPPING_H
#define _ASM_DMA_MAPPING_H
+#include
+
static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
{
/* We don't handle the NULL dev case for ISA for now. We could
@@ -15,4 +17,23 @@ static inline const struct dma_map_ops
*get_arch_dma_ops(struct bus_type *bus)
return NULL;
}
+#ifdef CONFIG_ARCH_HAS_DMA_ADDR_IS_PHYS_ADDR
+/**
+ * dma_addr_is_phys_addr - check whether a device DMA address is a physical
+ * address
+ * @dev: device to check
+ *
+ * Returns %true if any DMA address for this device happens to also be a valid
+ * physical address (not necessarily of the same page).
+ */
+static inline bool dma_addr_is_phys_addr(struct device *dev)
+{
+ /*
+* Secure guests always use the SWIOTLB, therefore DMA addresses are
+* actually the physical address of the bounce buffer.
+*/
+ return is_secure_guest();
+}
+#endif
+
#endif/* _ASM_DMA_MAPPING_H */
diff --git a/arch/powerpc/platforms/pseries/Kconfig
b/arch/powerpc/platforms/pseries/Kconfig
index 9e35cdd..0108150 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -152,6 +152,7 @@ config PPC_SVM
select SWIOTLB
select ARCH_HAS_MEM_ENCRYPT
select ARCH_HAS_FORCE_DMA_UNENCRYPTED
+ select ARCH_HAS_DMA_ADDR_IS_PHYS_ADDR
help
There are certain POWER platforms which support secure guests using
the Protected Execution Facility, with the help of an Ultravisor
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f7d1eea..6df5664 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -693,6 +693,26 @@ static inline bool dma_addressing_limited(struct device
*dev)
dma_get_required_mask(dev);
}
+#ifndef CONFIG_ARCH_HAS_DMA_ADDR_IS_PHYS_ADDR
+/**
+ * dma_addr_is_phys_addr - check whether a device DMA address is a physical
+ * address
+ * @dev: device to check
+ *
+ * Returns %true if any DMA address for this device happens to also be a valid
+ * physical address (not necessarily of the same page).
+ */
+static inline bool dma_addr_is_phys_addr(struct device *dev)
+{
+ /*
+* Except in very specific setups, DMA addresses exist in a different
+* address space from CPU physical addresses and cannot be directly used
+* to reference system memory.
+*/
+ return false;
+}
+#endif
+
#ifdef CONFIG_ARCH_HAS_SETUP_DMA_OPS
void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
const struct iommu_ops *iommu, bool coherent);
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 9decbba..6209b46 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -51,6 +51,9 @@ config ARCH_HAS_DMA_MMAP_PGPROT
config ARCH_HAS_FORCE_DMA_UNENCRYPTED
bool
+config ARCH_HAS_DMA_ADDR_IS_PHYS_ADDR
+ bool
+
config DMA_NONCOHERENT_CACHE_SYNC
bool
Re: [PATCH 1/2] dma-mapping: Add dma_addr_is_phys_addr()
On Fri, Oct 11, 2019 at 06:25:18PM -0700, Ram Pai wrote:
> From: Thiago Jung Bauermann
>
> In order to safely use the DMA API, virtio needs to know whether DMA
> addresses are in fact physical addresses and for that purpose,
> dma_addr_is_phys_addr() is introduced.
>
> cc: Benjamin Herrenschmidt
> cc: David Gibson
> cc: Michael Ellerman
> cc: Paul Mackerras
> cc: Michael Roth
> cc: Alexey Kardashevskiy
> cc: Paul Burton
> cc: Robin Murphy
> cc: Bartlomiej Zolnierkiewicz
> cc: Marek Szyprowski
> cc: Christoph Hellwig
> Suggested-by: Michael S. Tsirkin
> Signed-off-by: Ram Pai
> Signed-off-by: Thiago Jung Bauermann
The change itself looks ok, so
Reviewed-by: David Gibson
However, I would like to see the commit message (and maybe the inline
comments) expanded a bit on what the distinction here is about. Some
of the text from the next patch would be suitable, about DMA addresses
usually being in a different address space but not in the case of
bounce buffering.
> ---
> arch/powerpc/include/asm/dma-mapping.h | 21 +
> arch/powerpc/platforms/pseries/Kconfig | 1 +
> include/linux/dma-mapping.h| 20
> kernel/dma/Kconfig | 3 +++
> 4 files changed, 45 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/dma-mapping.h
> b/arch/powerpc/include/asm/dma-mapping.h
> index 565d6f7..f92c0a4b 100644
> --- a/arch/powerpc/include/asm/dma-mapping.h
> +++ b/arch/powerpc/include/asm/dma-mapping.h
> @@ -5,6 +5,8 @@
> #ifndef _ASM_DMA_MAPPING_H
> #define _ASM_DMA_MAPPING_H
>
> +#include
> +
> static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type
> *bus)
> {
> /* We don't handle the NULL dev case for ISA for now. We could
> @@ -15,4 +17,23 @@ static inline const struct dma_map_ops
> *get_arch_dma_ops(struct bus_type *bus)
> return NULL;
> }
>
> +#ifdef CONFIG_ARCH_HAS_DMA_ADDR_IS_PHYS_ADDR
> +/**
> + * dma_addr_is_phys_addr - check whether a device DMA address is a physical
> + * address
> + * @dev: device to check
> + *
> + * Returns %true if any DMA address for this device happens to also be a
> valid
> + * physical address (not necessarily of the same page).
> + */
> +static inline bool dma_addr_is_phys_addr(struct device *dev)
> +{
> + /*
> + * Secure guests always use the SWIOTLB, therefore DMA addresses are
> + * actually the physical address of the bounce buffer.
> + */
> + return is_secure_guest();
> +}
> +#endif
> +
> #endif /* _ASM_DMA_MAPPING_H */
> diff --git a/arch/powerpc/platforms/pseries/Kconfig
> b/arch/powerpc/platforms/pseries/Kconfig
> index 9e35cdd..0108150 100644
> --- a/arch/powerpc/platforms/pseries/Kconfig
> +++ b/arch/powerpc/platforms/pseries/Kconfig
> @@ -152,6 +152,7 @@ config PPC_SVM
> select SWIOTLB
> select ARCH_HAS_MEM_ENCRYPT
> select ARCH_HAS_FORCE_DMA_UNENCRYPTED
> + select ARCH_HAS_DMA_ADDR_IS_PHYS_ADDR
> help
>There are certain POWER platforms which support secure guests using
>the Protected Execution Facility, with the help of an Ultravisor
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index f7d1eea..6df5664 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -693,6 +693,26 @@ static inline bool dma_addressing_limited(struct device
> *dev)
> dma_get_required_mask(dev);
> }
>
> +#ifndef CONFIG_ARCH_HAS_DMA_ADDR_IS_PHYS_ADDR
> +/**
> + * dma_addr_is_phys_addr - check whether a device DMA address is a physical
> + * address
> + * @dev: device to check
> + *
> + * Returns %true if any DMA address for this device happens to also be a
> valid
> + * physical address (not necessarily of the same page).
> + */
> +static inline bool dma_addr_is_phys_addr(struct device *dev)
> +{
> + /*
> + * Except in very specific setups, DMA addresses exist in a different
> + * address space from CPU physical addresses and cannot be directly used
> + * to reference system memory.
> + */
> + return false;
> +}
> +#endif
> +
> #ifdef CONFIG_ARCH_HAS_SETUP_DMA_OPS
> void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> const struct iommu_ops *iommu, bool coherent);
> diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
> index 9decbba..6209b46 100644
> --- a/kernel/dma/Kconfig
> +++ b/kernel/dma/Kconfig
> @@ -51,6 +51,9 @@ config ARCH_HAS_DMA_MMAP_PGPROT
> config ARCH_HAS_FORCE_DMA_UNENCRYPTED
> bool
>
> +config ARCH_HAS_DMA_ADDR_IS_PHYS_ADDR
> + bool
> +
> config DMA_NONCOHERENT_CACHE_SYNC
> bool
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[PATCH 1/2] dma-mapping: Add dma_addr_is_phys_addr()
From: Thiago Jung Bauermann
In order to safely use the DMA API, virtio needs to know whether DMA
addresses are in fact physical addresses and for that purpose,
dma_addr_is_phys_addr() is introduced.
cc: Benjamin Herrenschmidt
cc: David Gibson
cc: Michael Ellerman
cc: Paul Mackerras
cc: Michael Roth
cc: Alexey Kardashevskiy
cc: Paul Burton
cc: Robin Murphy
cc: Bartlomiej Zolnierkiewicz
cc: Marek Szyprowski
cc: Christoph Hellwig
Suggested-by: Michael S. Tsirkin
Signed-off-by: Ram Pai
Signed-off-by: Thiago Jung Bauermann
---
arch/powerpc/include/asm/dma-mapping.h | 21 +
arch/powerpc/platforms/pseries/Kconfig | 1 +
include/linux/dma-mapping.h| 20
kernel/dma/Kconfig | 3 +++
4 files changed, 45 insertions(+)
diff --git a/arch/powerpc/include/asm/dma-mapping.h
b/arch/powerpc/include/asm/dma-mapping.h
index 565d6f7..f92c0a4b 100644
--- a/arch/powerpc/include/asm/dma-mapping.h
+++ b/arch/powerpc/include/asm/dma-mapping.h
@@ -5,6 +5,8 @@
#ifndef _ASM_DMA_MAPPING_H
#define _ASM_DMA_MAPPING_H
+#include
+
static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
{
/* We don't handle the NULL dev case for ISA for now. We could
@@ -15,4 +17,23 @@ static inline const struct dma_map_ops
*get_arch_dma_ops(struct bus_type *bus)
return NULL;
}
+#ifdef CONFIG_ARCH_HAS_DMA_ADDR_IS_PHYS_ADDR
+/**
+ * dma_addr_is_phys_addr - check whether a device DMA address is a physical
+ * address
+ * @dev: device to check
+ *
+ * Returns %true if any DMA address for this device happens to also be a valid
+ * physical address (not necessarily of the same page).
+ */
+static inline bool dma_addr_is_phys_addr(struct device *dev)
+{
+ /*
+* Secure guests always use the SWIOTLB, therefore DMA addresses are
+* actually the physical address of the bounce buffer.
+*/
+ return is_secure_guest();
+}
+#endif
+
#endif /* _ASM_DMA_MAPPING_H */
diff --git a/arch/powerpc/platforms/pseries/Kconfig
b/arch/powerpc/platforms/pseries/Kconfig
index 9e35cdd..0108150 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -152,6 +152,7 @@ config PPC_SVM
select SWIOTLB
select ARCH_HAS_MEM_ENCRYPT
select ARCH_HAS_FORCE_DMA_UNENCRYPTED
+ select ARCH_HAS_DMA_ADDR_IS_PHYS_ADDR
help
There are certain POWER platforms which support secure guests using
the Protected Execution Facility, with the help of an Ultravisor
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f7d1eea..6df5664 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -693,6 +693,26 @@ static inline bool dma_addressing_limited(struct device
*dev)
dma_get_required_mask(dev);
}
+#ifndef CONFIG_ARCH_HAS_DMA_ADDR_IS_PHYS_ADDR
+/**
+ * dma_addr_is_phys_addr - check whether a device DMA address is a physical
+ * address
+ * @dev: device to check
+ *
+ * Returns %true if any DMA address for this device happens to also be a valid
+ * physical address (not necessarily of the same page).
+ */
+static inline bool dma_addr_is_phys_addr(struct device *dev)
+{
+ /*
+* Except in very specific setups, DMA addresses exist in a different
+* address space from CPU physical addresses and cannot be directly used
+* to reference system memory.
+*/
+ return false;
+}
+#endif
+
#ifdef CONFIG_ARCH_HAS_SETUP_DMA_OPS
void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
const struct iommu_ops *iommu, bool coherent);
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 9decbba..6209b46 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -51,6 +51,9 @@ config ARCH_HAS_DMA_MMAP_PGPROT
config ARCH_HAS_FORCE_DMA_UNENCRYPTED
bool
+config ARCH_HAS_DMA_ADDR_IS_PHYS_ADDR
+ bool
+
config DMA_NONCOHERENT_CACHE_SYNC
bool
--
1.8.3.1
