All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will.deacon@arm.com>
To: Vineet Gupta <Vineet.Gupta1@synopsys.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"arc-linux-dev@synopsys.com" <arc-linux-dev@synopsys.com>
Subject: Re: [PATCH 20/28] ARCv2: barriers
Date: Thu, 11 Jun 2015 14:39:53 +0100	[thread overview]
Message-ID: <20150611133952.GA29425@arm.com> (raw)
In-Reply-To: <C2D7FE5348E1B147BCA15975FBA23075665A526F@IN01WEMBXB.internal.synopsys.com>

On Thu, Jun 11, 2015 at 01:13:28PM +0100, Vineet Gupta wrote:
> On Wednesday 10 June 2015 06:31 PM, Will Deacon wrote:
> > On Wed, Jun 10, 2015 at 11:58:40AM +0100, Peter Zijlstra wrote:
> >> On Wed, Jun 10, 2015 at 09:34:18AM +0000, Vineet Gupta wrote:
> >>> On Tuesday 09 June 2015 06:10 PM, Peter Zijlstra wrote:
> >> I think the most interesting part is the device side.
> >>
> >>>>> +/*
> >>>>> + * DSYNC:
> >>>>> + *   - Waits for completion of all outstanding memory operations before any new
> >>>>> + *     operations can begin
> >>>>> + *   - Includes implicit memory operations such as cache/TLB/BPU maintenance ops
> >>>>> + *   - Lighter version of SYNC as it doesn't wait for non-memory operations
> >>>>> + */
> >>>>> +#define mb()		asm volatile("dsync\n" : : : "memory")
> >>>> So mb() is supposed to order against things like DMA memory ops, is DMA
> >>>> part of point 1 or 3, if 3, this is not a suitable instruction.
> >>> Can u please explain the DMA case a bit more ? From what I understood and used in
> >>> say ethernet driver, it is more of a line drawn between say cpu updating a shared
> >>> buffer descriptor and kicking a MMIO register (which in turn could initiate a DMA)
> >>> but I'm not sure how mb() can possibly order with DMA per se (unless there's some
> >>> advanced form of IO-coherency)
> >> I'm afraid I might not be the best of sources here, I tend to stay away
> >> from actual device stuff like that. I've Cc'ed Will Deacon who might be
> >> able to shed a bit more light on this aspect.
> > I'd definitely expect mb() to order arbitrary memory accesses against each
> > other (i.e. regardless of whether or not they're to RAM or MMIO devices).
> > Some drivers use it to "flush the writebuffer" but I don't think that makes
> > a whole lot of sense. Certainly, on ARM, if we want to know that something
> > reached an MMIO endpoint then we'll need a read-back as well as the barrier
> > for the general case.
> >
> > You also need that guarantee in your readl/writel family of macros. It's
> > extremely heavy and rarely needed, which is why I added the _relaxed
> > versions to all architectures.
> 
> Wow - adding that to these accessors will really be heavy - given that a whole
> bunch of drivers still use the stock API (or perhaps don't know / care whether
> they need the readl or the relaxed api. And it is practically impossible to switch
> them over - after if ain't broken how can u fix it. So far we've been testing this
> implementation (readl/writel - w/o any explicit barrier) on slower FPGA builds and
> this includes a whole bunch of designware IP - mmc, eth, gpio.... and don't see
> any ill effects - do you reckon we still need to add it.

Unfortunately, yes, as that's effectively what the kernel requires:

  http://marc.info/?l=linux-kernel&m=121192394430581&w=2
  http://thread.gmane.org/gmane.linux.ide/46414

The conclusion is that x86 *does* provide this ordering in its accessors
and drivers are written to assume that, so either you go round fixing all
the drivers by adding the missing barriers or you implement it in your
accessors (like we have done on ARM). Subtle I/O ordering issues are no
fun to debug.

That's also the reason I added the _relaxed versions, so you can port
drivers one-by-one to the weaker semantics whilst having the potentially
broken drivers continue to work.

> > The "ordering against DMA" is something like reading an MMIO register to
> > determine whether the DMA has completed, then going off to read the contents
> > out of the DMA buffer. The comment you have about DSYNC makes it sound like
> > it's not sufficient for this case.
> 
> IMHO this use case is slightly pedantic - since DMA completion will typically
> follow up with an interrupt (I understand it's still possible to poll a dma status
> reg). at any rate when it comes to dwaring a line between memory accesses -
> regular or mmio, DSYNC is all we got in the ISA so ARCV2 mb() has to use it -
> there's no better option.

Does taking an interrupt ensure visibility of the data on your
architecture? Most non-pci device architectures allow that to race, so
you end up relying on the readX in the irq handler to order the buffer
access.

If you don't have an instruction for this, then I don't understand how
you can perform DMA to/from regions of memory that are mapped as weakly
ordered by the CPU (e.g. how would you write a data buffer then tell the
device to go read from it?).

Will

  reply	other threads:[~2015-06-11 13:40 UTC|newest]

Thread overview: 109+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-09 11:48 [PATCH 00/28] ARCv2 port to Linux - (B) ISA / Core / platform support Vineet Gupta
2015-06-09 11:48 ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 01/28] ARCv2: [intc] HS38 core interrupt controller Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 02/28] ARCv2: Support for ARCv2 ISA and HS38x cores Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 03/28] ARCv2: STAR 9000793984: Handle return from intr to Delay Slot Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 04/28] ARCv2: STAR 9000808988: signals involving " Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 05/28] ARCv2: STAR 9000814690: Really Re-enable interrupts to avoid deadlocks Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 06/28] ARCv2: MMUv4: TLB programming Model changes Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 07/28] ARCv2: MMUv4: cache programming model changes Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 08/28] ARCv2: MMUv4: support aliasing icache config Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 09/28] ARCv2: optimised string/mem lib routines Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 10/28] ARCv2: Adhere to Zero Delay loop restriction Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 11/28] ARCv2: extable: Enable sorting at build time Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-24  5:51   ` Vineet Gupta
2015-06-24  5:51     ` Vineet Gupta
2015-06-29 20:38     ` David Daney
2015-06-30  4:41       ` Vineet Gupta
2015-06-30  4:41         ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 12/28] ARCv2: clocksource: Introduce 64bit local RTC counter Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 13/28] ARC: make plat_smp_ops weak to allow over-rides Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 14/28] ARCv2: SMP: ARConnect debug/robustness Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 15/28] ARCv2: SMP: clocksource: Enable Global Real Time counter Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 16/28] ARCv2: SMP: intc: IDU 2nd level intc for dynamic IRQ distribution Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 17/28] ARC: add compiler barrier to LLSC based cmpxchg Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 12:23   ` Peter Zijlstra
2015-06-09 11:48 ` [PATCH 18/28] ARC: add smp barriers around atomics per memory-barrriers.txt Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 12:30   ` Peter Zijlstra
2015-06-10  9:17     ` Vineet Gupta
2015-06-10 10:53       ` Peter Zijlstra
2015-06-11 13:03         ` Vineet Gupta
2015-06-12 12:15   ` [PATCH v2] ARC: add smp barriers around atomics per Documentation/atomic_ops.txt Vineet Gupta
2015-06-12 12:15     ` Vineet Gupta
2015-06-12 13:04     ` Peter Zijlstra
2015-06-12 13:16       ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 19/28] arch: conditionally define smp_{mb,rmb,wmb} Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 12:32   ` Peter Zijlstra
2015-06-09 11:48 ` [PATCH 20/28] ARCv2: barriers Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 12:40   ` Peter Zijlstra
2015-06-10  9:34     ` Vineet Gupta
2015-06-10 10:58       ` Peter Zijlstra
2015-06-10 13:01         ` Will Deacon
2015-06-11 12:13           ` Vineet Gupta
2015-06-11 13:39             ` Will Deacon [this message]
2015-06-19 13:13               ` Vineet Gupta
2015-06-19 13:13                 ` Vineet Gupta
2015-06-19 13:13                 ` Vineet Gupta
2015-06-22 13:36                 ` Will Deacon
2015-06-22 13:36                   ` Will Deacon
2015-06-22 13:36                   ` Will Deacon
2015-06-23  7:58                   ` [PATCH v2 " Vineet Gupta
2015-06-23  7:58                     ` Vineet Gupta
2015-06-23  8:49                     ` Will Deacon
2015-06-23  9:03                       ` Vineet Gupta
2015-06-23  9:26                         ` Will Deacon
2015-06-23  9:52                           ` [PATCH v3 22/28] " Vineet Gupta
2015-06-23  9:52                             ` Vineet Gupta
2015-06-23 16:28                             ` Will Deacon
2015-06-23  9:25                     ` [PATCH v2 20/28] " Peter Zijlstra
2015-06-23  8:02                   ` [PATCH " Vineet Gupta
2015-06-09 11:48 ` [PATCH 21/28] ARC: Reduce bitops lines of code using macros Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-12 12:20   ` [PATCH v2] " Vineet Gupta
2015-06-12 12:20     ` Vineet Gupta
2015-06-12 13:05     ` Peter Zijlstra
2015-06-09 11:48 ` [PATCH 22/28] ARCv2: STAR 9000837815 workaround hardware exclusive transactions livelock Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 12:35   ` Peter Zijlstra
2015-06-10 10:01     ` Vineet Gupta
2015-06-10 11:02       ` Peter Zijlstra
2015-06-19  9:55         ` [PATCH v2 " Vineet Gupta
2015-06-19  9:55           ` Vineet Gupta
2015-06-19  9:59           ` Will Deacon
2015-06-19 10:09             ` Vineet Gupta
2015-06-23  7:59             ` Vineet Gupta
2015-06-23  7:59               ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 23/28] ARCv2: SLC: Handle explcit flush for DMA ops (w/o IO-coherency) Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 24/28] ARCv2: All bits in place, allow ARCv2 builds Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 25/28] ARCv2: [nsim*hs*] Support simulation platforms for HS38x cores Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 26/28] ARC: [axs101] Prepare for AXS103 Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 27/28] ARCv2: [axs103] Support ARC SDP FPGA platform for HS38x cores Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta
2015-06-09 11:48 ` [PATCH 28/28] ARCv2: [vdk] dts files and defconfig for HS38 VDK Vineet Gupta
2015-06-09 11:48   ` Vineet Gupta

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=20150611133952.GA29425@arm.com \
    --to=will.deacon@arm.com \
    --cc=Vineet.Gupta1@synopsys.com \
    --cc=arc-linux-dev@synopsys.com \
    --cc=arnd@arndb.de \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.