diff mbox

[V2,2/5] arm64 : Introduce support for ACPI _CCA object

Message ID 1430838729-21572-3-git-send-email-Suravee.Suthikulpanit@amd.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Suravee Suthikulpanit May 5, 2015, 3:12 p.m. UTC
From http://www.uefi.org/sites/default/files/resources/ACPI_6.0.pdf,
section 6.2.17 _CCA states that ARM platforms require ACPI _CCA
object to be specified for DMA-cabpable devices. This patch introduces
ACPI_MUST_HAVE_CCA in arm64 Kconfig to specify such requirement.

In this case of missing _CCA, arm64 would assign dummy_dma_ops
to disable DMA capability of the device.

Signed-off-by: Mark Salter <msalter@redhat.com>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 arch/arm64/Kconfig                   |   2 +
 arch/arm64/include/asm/dma-mapping.h |  18 +++++-
 arch/arm64/mm/dma-mapping.c          | 104 +++++++++++++++++++++++++++++++++++
 3 files changed, 122 insertions(+), 2 deletions(-)

Comments

Arnd Bergmann May 5, 2015, 3:44 p.m. UTC | #1
On Tuesday 05 May 2015 10:12:06 Suravee Suthikulpanit wrote:
> +struct dma_map_ops dummy_dma_ops = {
> +       .alloc                  = __dummy_alloc,
> +       .free                   = __dummy_free,
> +       .mmap                   = __dummy_mmap,
> +       .map_page               = __dummy_map_page,
> +       .unmap_page             = __dummy_unmap_page,
> +       .map_sg                 = __dummy_map_sg,
> +       .unmap_sg               = __dummy_unmap_sg,
> +       .sync_single_for_cpu    = __dummy_sync_single_for_cpu,
> +       .sync_single_for_device = __dummy_sync_single_for_device,
> +       .sync_sg_for_cpu        = __dummy_sync_sg_for_cpu,
> +       .sync_sg_for_device     = __dummy_sync_sg_for_device,
> +       .mapping_error          = __dummy_mapping_error,
> +       .dma_supported          = __dummy_dma_supported,
> +};
> +EXPORT_SYMBOL(dummy_dma_ops);
> +
> 

This will clearly work, but I think it's easier to just leave
the dma_mask pointer as NULL when creating the platform device,
which should let the normal dma ops fail all the callbacks.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Suravee Suthikulpanit May 5, 2015, 4:09 p.m. UTC | #2
On 5/5/2015 10:44 AM, Arnd Bergmann wrote:
> On Tuesday 05 May 2015 10:12:06 Suravee Suthikulpanit wrote:
>> +struct dma_map_ops dummy_dma_ops = {
>> +       .alloc                  = __dummy_alloc,
>> +       .free                   = __dummy_free,
>> +       .mmap                   = __dummy_mmap,
>> +       .map_page               = __dummy_map_page,
>> +       .unmap_page             = __dummy_unmap_page,
>> +       .map_sg                 = __dummy_map_sg,
>> +       .unmap_sg               = __dummy_unmap_sg,
>> +       .sync_single_for_cpu    = __dummy_sync_single_for_cpu,
>> +       .sync_single_for_device = __dummy_sync_single_for_device,
>> +       .sync_sg_for_cpu        = __dummy_sync_sg_for_cpu,
>> +       .sync_sg_for_device     = __dummy_sync_sg_for_device,
>> +       .mapping_error          = __dummy_mapping_error,
>> +       .dma_supported          = __dummy_dma_supported,
>> +};
>> +EXPORT_SYMBOL(dummy_dma_ops);
>> +
>>
>
> This will clearly work, but I think it's easier to just leave
> the dma_mask pointer as NULL when creating the platform device,
> which should let the normal dma ops fail all the callbacks.
>
> 	Arnd
>

However, codes in several places are making use of dma_map_ops without 
checking if the ops are NULL (i.e. 
include/asm-generic/dma-mapping-common.h and in arch-specific 
implementation). If setting it to NULL is what we are planning to 
support, we would need to scrub the current code to put NULL check. 
Also, would you consider if that is safe to do going forward?

Thanks,
Suravee

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Robin Murphy May 6, 2015, 10:08 a.m. UTC | #3
Hi Suravee,

On 05/05/15 16:12, Suravee Suthikulpanit wrote:
>  From http://www.uefi.org/sites/default/files/resources/ACPI_6.0.pdf,
> section 6.2.17 _CCA states that ARM platforms require ACPI _CCA
> object to be specified for DMA-cabpable devices. This patch introduces
> ACPI_MUST_HAVE_CCA in arm64 Kconfig to specify such requirement.
>
> In this case of missing _CCA, arm64 would assign dummy_dma_ops
> to disable DMA capability of the device.
>
> Signed-off-by: Mark Salter <msalter@redhat.com>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> ---

[...]
> +static void __dummy_sync_single_for_cpu(struct device *dev,
> +					dma_addr_t dev_addr, size_t size,
> +					enum dma_data_direction dir)
> +{
> +}
> +
> +static void __dummy_sync_single_for_device(struct device *dev,
> +					   dma_addr_t dev_addr, size_t size,
> +					   enum dma_data_direction dir)
> +{
> +}

Minor point, but I don't see the need to have multiple dummy functions 
with identical signatures - just have a generic dummy_sync_single and 
assign it to both ops.

> +static void __dummy_sync_sg_for_cpu(struct device *dev,
> +				    struct scatterlist *sgl, int nelems,
> +				    enum dma_data_direction dir)
> +{
> +}
> +
> +static void __dummy_sync_sg_for_device(struct device *dev,
> +				       struct scatterlist *sgl, int nelems,
> +				       enum dma_data_direction dir)
> +{
> +}

Ditto here with dummy_sync_sg.

I wonder if there's any argument for putting the dummy DMA ops somewhere 
common, like drivers/base/dma-mapping.c?

Robin.

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Suravee Suthikulpanit May 6, 2015, 2:34 p.m. UTC | #4
On 5/6/2015 5:08 AM, Robin Murphy wrote:
> [...]
>> +static void __dummy_sync_single_for_cpu(struct device *dev,
>> +                    dma_addr_t dev_addr, size_t size,
>> +                    enum dma_data_direction dir)
>> +{
>> +}
>> +
>> +static void __dummy_sync_single_for_device(struct device *dev,
>> +                       dma_addr_t dev_addr, size_t size,
>> +                       enum dma_data_direction dir)
>> +{
>> +}
>
> Minor point, but I don't see the need to have multiple dummy functions
> with identical signatures - just have a generic dummy_sync_single and
> assign it to both ops.
>
>> +static void __dummy_sync_sg_for_cpu(struct device *dev,
>> +                    struct scatterlist *sgl, int nelems,
>> +                    enum dma_data_direction dir)
>> +{
>> +}
>> +
>> +static void __dummy_sync_sg_for_device(struct device *dev,
>> +                       struct scatterlist *sgl, int nelems,
>> +                       enum dma_data_direction dir)
>> +{
>> +}
>
> Ditto here with dummy_sync_sg.

Hi Robin,

Good point. I'll take care of that in V3.

>
> I wonder if there's any argument for putting the dummy DMA ops somewhere
> common, like drivers/base/dma-mapping.c?
>
> Robin.

Hm.. If this approach will be adopted by other architectures, then it 
would make sense. Currently, this is only used by arm64. So, I think it 
is okay to leave this here for now.

Thanks,
Suravee

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 4269dba..4f4bbaaf 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1,7 +1,9 @@ 
 config ARM64
 	def_bool y
 	select ACPI_GENERIC_GSI if ACPI
+	select ACPI_MUST_HAVE_CCA if ACPI
 	select ACPI_REDUCED_HARDWARE_ONLY if ACPI
+	select ACPI_SUPPORT_CCA_ZERO if ACPI
 	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_GCOV_PROFILE_ALL
diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
index 9437e3d..f0d6d0b 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -18,6 +18,7 @@ 
 
 #ifdef __KERNEL__
 
+#include <linux/acpi.h>
 #include <linux/types.h>
 #include <linux/vmalloc.h>
 
@@ -28,13 +29,23 @@ 
 
 #define DMA_ERROR_CODE	(~(dma_addr_t)0)
 extern struct dma_map_ops *dma_ops;
+extern struct dma_map_ops dummy_dma_ops;
 
 static inline struct dma_map_ops *__generic_dma_ops(struct device *dev)
 {
-	if (unlikely(!dev) || !dev->archdata.dma_ops)
+	if (unlikely(!dev))
 		return dma_ops;
-	else
+	else if (dev->archdata.dma_ops)
 		return dev->archdata.dma_ops;
+	else if (acpi_disabled)
+		return dma_ops;
+
+	/*
+	 * When ACPI is enabled, if arch_set_dma_ops is not called,
+	 * we will disable device DMA capability by setting it
+	 * to dummy_dma_ops.
+	 */
+	return &dummy_dma_ops;
 }
 
 static inline struct dma_map_ops *get_dma_ops(struct device *dev)
@@ -48,6 +59,9 @@  static inline struct dma_map_ops *get_dma_ops(struct device *dev)
 static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
 				      struct iommu_ops *iommu, bool coherent)
 {
+	if (!acpi_disabled && !dev->archdata.dma_ops)
+		dev->archdata.dma_ops = dma_ops;
+
 	dev->archdata.dma_coherent = coherent;
 }
 #define arch_setup_dma_ops	arch_setup_dma_ops
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index ef7d112..31d2b52 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -415,6 +415,110 @@  out:
 	return -ENOMEM;
 }
 
+/********************************************
+ * The following APIs are for dummy DMA ops *
+ ********************************************/
+
+static void *__dummy_alloc(struct device *dev, size_t size,
+			   dma_addr_t *dma_handle, gfp_t flags,
+			   struct dma_attrs *attrs)
+{
+	return NULL;
+}
+
+static void __dummy_free(struct device *dev, size_t size,
+			 void *vaddr, dma_addr_t dma_handle,
+			 struct dma_attrs *attrs)
+{
+}
+
+static int __dummy_mmap(struct device *dev,
+			struct vm_area_struct *vma,
+			void *cpu_addr, dma_addr_t dma_addr, size_t size,
+			struct dma_attrs *attrs)
+{
+	return -ENXIO;
+}
+
+static dma_addr_t __dummy_map_page(struct device *dev, struct page *page,
+				   unsigned long offset, size_t size,
+				   enum dma_data_direction dir,
+				   struct dma_attrs *attrs)
+{
+	return DMA_ERROR_CODE;
+}
+
+static void __dummy_unmap_page(struct device *dev, dma_addr_t dev_addr,
+			       size_t size, enum dma_data_direction dir,
+			       struct dma_attrs *attrs)
+{
+}
+
+static int __dummy_map_sg(struct device *dev, struct scatterlist *sgl,
+			  int nelems, enum dma_data_direction dir,
+			  struct dma_attrs *attrs)
+{
+	return 0;
+}
+
+static void __dummy_unmap_sg(struct device *dev,
+			     struct scatterlist *sgl, int nelems,
+			     enum dma_data_direction dir,
+			     struct dma_attrs *attrs)
+{
+}
+
+static void __dummy_sync_single_for_cpu(struct device *dev,
+					dma_addr_t dev_addr, size_t size,
+					enum dma_data_direction dir)
+{
+}
+
+static void __dummy_sync_single_for_device(struct device *dev,
+					   dma_addr_t dev_addr, size_t size,
+					   enum dma_data_direction dir)
+{
+}
+
+static void __dummy_sync_sg_for_cpu(struct device *dev,
+				    struct scatterlist *sgl, int nelems,
+				    enum dma_data_direction dir)
+{
+}
+
+static void __dummy_sync_sg_for_device(struct device *dev,
+				       struct scatterlist *sgl, int nelems,
+				       enum dma_data_direction dir)
+{
+}
+
+static int __dummy_mapping_error(struct device *hwdev, dma_addr_t dma_addr)
+{
+	return 1;
+}
+
+static int __dummy_dma_supported(struct device *hwdev, u64 mask)
+{
+	return 0;
+}
+
+struct dma_map_ops dummy_dma_ops = {
+	.alloc                  = __dummy_alloc,
+	.free                   = __dummy_free,
+	.mmap                   = __dummy_mmap,
+	.map_page               = __dummy_map_page,
+	.unmap_page             = __dummy_unmap_page,
+	.map_sg                 = __dummy_map_sg,
+	.unmap_sg               = __dummy_unmap_sg,
+	.sync_single_for_cpu    = __dummy_sync_single_for_cpu,
+	.sync_single_for_device = __dummy_sync_single_for_device,
+	.sync_sg_for_cpu        = __dummy_sync_sg_for_cpu,
+	.sync_sg_for_device     = __dummy_sync_sg_for_device,
+	.mapping_error          = __dummy_mapping_error,
+	.dma_supported          = __dummy_dma_supported,
+};
+EXPORT_SYMBOL(dummy_dma_ops);
+
 static int __init arm64_dma_init(void)
 {
 	int ret;