linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: catalin.marinas@arm.com (Catalin Marinas)
To: linux-arm-kernel@lists.infradead.org
Subject: [Linaro-acpi] [PATCH 2/2] ACPI / scan: Parse _CCA and setup device coherency
Date: Fri, 1 May 2015 12:06:44 +0100	[thread overview]
Message-ID: <20150501110644.GF27755@e104818-lin.cambridge.arm.com> (raw)
In-Reply-To: <2817500.sgztgK2NzB@wuerfel>

On Wed, Apr 29, 2015 at 05:54:02PM +0200, Arnd Bergmann wrote:
> On Wednesday 29 April 2015 14:57:10 Suthikulpanit, Suravee wrote:
> > Otherwise, it would seem inconsistent with what states in the ACPI spec:
> >  
> >   CCA objects are only relevant for devices that can access
> >   CPU-visible memory, such as devices that are DMA capable. On ARM
> >   based systems, the _CCA object must be supplied all such devices.
> >   On Intel platforms, if the _CCA object is not supplied, the OSPM
> >   will assume the devices are hardware cache coherent.
> > 
> > From the statement above, I interpreted as if it is not present, it would
> > be non-coherent.
> 
> My guess is that this section was included for Windows Phone, which runs
> on embedded SoCs that usually have noncoherent DMA in a particular way.
> 
> Linux however only uses ACPI for servers, so that case does not happen.
> 
> I guess it would be reasonable to add a run-time warning here if you
> try to do DMA on a device that does not have CCA set, and you should
> probably set the DMA mask to 0 in that case as well.

I agree, if _CCA isn't present, we should not allow DMA. With DT, the
default dma_ops point to non-coherent but with ACPI, we could change
the default to a dummy set of dma_ops which don't do anything (or just
return NULL). Something like below, untested:


diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
index 9437e3dc5833..3fd6ef019c8f 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -31,10 +31,14 @@ extern struct dma_map_ops *dma_ops;
 
 static inline struct dma_map_ops *__generic_dma_ops(struct device *dev)
 {
-	if (unlikely(!dev) || !dev->archdata.dma_ops)
+	if (!dev)
 		return dma_ops;
-	else
+	else if (dev->archdata.dma_ops)
 		return dev->archdata.dma_ops;
+	else if (!acpi_disabled)
+		return dummy_dma_ops;
+	else
+		return dma_ops;
 }
 
 static inline struct dma_map_ops *get_dma_ops(struct device *dev)
@@ -48,6 +52,8 @@ 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


The core code should not call arch_setup_dma_ops() if no _CCA option is
found.

> Note that there are lots of ways in which you could have noncoherent DMA:
> the default on ARM32 is that it requires uncached access or explicit
> cache flushes, but it's also possible to have an SMP system where a device
> is only coherent with some of the CPUs and requires explicit synchronization
> (not flushes) otherwise. In a multi-level cache hierarchy, there could be
> all sorts of combinations of flushes and syncs you would need to do.
> 
> With DT, we handle this using SoC-specific overrides for platforms that
> are noncoherent in funny ways, see
> http://lxr.free-electrons.com/source/arch/arm/mach-mvebu/coherency.c?v=3.18#L263
> for instance.

It looks like mach-mvebu no longer needs this, according to commit
1bd4d8a6de5c (ARM: mvebu: use arm_coherent_dma_ops and re-enable hardware
I/O coherency).

Even if some hardware needs this, it's usually because it has some
broken assumptions about barriers which most likely are architecture
non-compliant. We can work around it on a case by case basis (SoC
quirks). One option would be to disable coherency altogether for that
device, even if the performance is affected (e.g. no partial coherency).
Another possibility may be to add a bus driver for that broken
interconnect which installs its own dma ops for each device attached.

> If we just disallow DMA to devices that are marked with _CCA=0
> in ACPI, we can avoid this case, or discuss it by the time someone has hardware
> that wants it, and then make a more informed decision about it.

I don't think we should disallow DMA to devices with _CCA == 0 (only to
those that don't have a _CCA property at all) as long as _CCA == 0 has
clear semantics like only architected cache maintenance required (and
that's what the ARMv8 ARM requires from compliant system caches).

-- 
Catalin

  reply	other threads:[~2015-05-01 11:06 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-29 13:44 [PATCH 0/2] ACPI : Introduce support for _CCA object Suravee Suthikulpanit
2015-04-29 13:44 ` [PATCH 1/2] arm/arm64: ACPI: Introduce CONFIG_ACPI_MUST_HAVE_CCA Suravee Suthikulpanit
2015-04-29 14:04   ` Catalin Marinas
2015-04-29 14:31     ` Suravee Suthikulpanit
2015-04-29 14:42       ` Catalin Marinas
2015-04-29 14:44         ` Suravee Suthikulpanit
2015-04-30 13:47         ` Hanjun Guo
2015-04-30 13:50           ` Will Deacon
2015-04-30 14:14             ` Hanjun Guo
2015-04-30 15:01             ` Lorenzo Pieralisi
2015-04-29 13:44 ` [PATCH 2/2] ACPI / scan: Parse _CCA and setup device coherency Suravee Suthikulpanit
2015-04-29 14:03   ` Arnd Bergmann
2015-04-29 14:45     ` Suravee Suthikulpanit
2015-04-29 14:47       ` [Linaro-acpi] " Arnd Bergmann
2015-04-29 14:57         ` Suthikulpanit, Suravee
2015-04-29 15:39           ` Al Stone
2015-04-29 16:15             ` Arnd Bergmann
2015-04-29 15:54           ` Arnd Bergmann
2015-05-01 11:06             ` Catalin Marinas [this message]
2015-05-08 14:08               ` Arnd Bergmann
2015-05-11 17:10                 ` Catalin Marinas
2015-05-11 17:24                   ` Robin Murphy
2015-04-29 16:25   ` Arnd Bergmann
2015-04-29 21:53     ` Suravee Suthikulpanit
2015-04-30  8:23       ` [Linaro-acpi] " Arnd Bergmann
2015-04-30 10:41         ` Will Deacon
2015-04-30 10:47           ` Arnd Bergmann
2015-04-30 11:07             ` Will Deacon
2015-04-30 11:24               ` Arnd Bergmann
2015-04-30 11:46                 ` Will Deacon
2015-04-30 13:03                   ` Arnd Bergmann
2015-04-30 13:13                     ` Will Deacon
2015-04-30 13:52                       ` Arnd Bergmann
2015-04-30 15:55                         ` Catalin Marinas
2015-05-08 14:01                           ` Arnd Bergmann
2015-04-30 23:39         ` Suravee Suthikulanit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150501110644.GF27755@e104818-lin.cambridge.arm.com \
    --to=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).