All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Sricharan" <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
To: 'Arnd Bergmann' <arnd-r2nGTMty4D4@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org,
	treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	stepanm-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org
Subject: RE: [PATCH V5 6/7] iommu/msm: Use writel_relaxed and add a barrier
Date: Wed, 25 May 2016 22:19:29 +0530	[thread overview]
Message-ID: <000901d1b6a5$6dcd5f90$49681eb0$@codeaurora.org> (raw)
In-Reply-To: <7715835.BXgVNoY5UO@wuerfel>

Hi Arnd,

>> >* On other platforms, polling the register is likely required because
>> >  an MMIO write is "posted", meaning that a sync after writel() will
>> >  only ensure that it has left the CPU write queue, but it may still be
>> >  on the bus fabric and whatever side-effects are triggered by the
>> >  write are normally not guaranteed to be completed even after the
>> >  'sync'. You need to check the datasheet for your IOMMU to find out
>> >  whether the 'dsb' instruction actually has any effect on the IOMMU.
>> >  If not, then neither the barrier that you add here nor the barrier
>> >  in the following writel() is sufficient.
>> >
>>    Thanks for the detailed explanation.
>> i will check this. So with this, i think that if the iommu does not
>>  support polling for its status, then it should listen to 'dsb' otherwise
>>  there will no be no way of make it coherent ?
>
>It's more likely that you have to do a readl() after the writel() to
>any address inside of the device in order to ensure the writel()
>has completed, but that is something that the hardware manual for
>the iommu should describe.
>
 ok, get it. So will do it as required for here.

>> >* The one barrier that is normally required in an IOMMU is between
>> >  updating the in-memory page tables and the following MMIO store
>> >  that triggers the TLB flush for that entry. This barrier is
>> >  implied by writel() but not writel_relaxed(). If you don't have
>> >  a hardware page table walker in your IOMMU, you don't need to worry
>> >  about this.
>> >
>>   To get my understanding correct here, is the barrier required here because
>>    of speculative fetch ?
>
>It doesn't have to be a speculative fetch, it could also be a normal
>fetch triggered by a DMA. The point is that the CPU is free to reorder
>the store to (io page table) memory relative to the store to the
>IOMMU flush register, and when you directly follow up with a store to
>the DMA master that triggers a transfer, this transfer can hit the
>IOMMU and cause the stale page table entry to be read.
>
>I guess in practice the barrier that comes before the MMIO write to the
>DMA master would ensure ordering of the DMA after both the IOPTE
>update and the MMIO flush (unless there was the speculative fetch you
>mentioned), but this is where I'm no longer confident enough in my
>pwn understanding of th ordering rules to rely on that. Having the
>barrier between the IOPTE update and the flush certainly leaves
>no room for ambiguity.
    Ok understand. Also i think speculative access if any, should 
     go away if unmap is done first and then remap by clients.

Regards,
 Sricharan

WARNING: multiple messages have this Message-ID (diff)
From: sricharan@codeaurora.org (Sricharan)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V5 6/7] iommu/msm: Use writel_relaxed and add a barrier
Date: Wed, 25 May 2016 22:19:29 +0530	[thread overview]
Message-ID: <000901d1b6a5$6dcd5f90$49681eb0$@codeaurora.org> (raw)
In-Reply-To: <7715835.BXgVNoY5UO@wuerfel>

Hi Arnd,

>> >* On other platforms, polling the register is likely required because
>> >  an MMIO write is "posted", meaning that a sync after writel() will
>> >  only ensure that it has left the CPU write queue, but it may still be
>> >  on the bus fabric and whatever side-effects are triggered by the
>> >  write are normally not guaranteed to be completed even after the
>> >  'sync'. You need to check the datasheet for your IOMMU to find out
>> >  whether the 'dsb' instruction actually has any effect on the IOMMU.
>> >  If not, then neither the barrier that you add here nor the barrier
>> >  in the following writel() is sufficient.
>> >
>>    Thanks for the detailed explanation.
>> i will check this. So with this, i think that if the iommu does not
>>  support polling for its status, then it should listen to 'dsb' otherwise
>>  there will no be no way of make it coherent ?
>
>It's more likely that you have to do a readl() after the writel() to
>any address inside of the device in order to ensure the writel()
>has completed, but that is something that the hardware manual for
>the iommu should describe.
>
 ok, get it. So will do it as required for here.

>> >* The one barrier that is normally required in an IOMMU is between
>> >  updating the in-memory page tables and the following MMIO store
>> >  that triggers the TLB flush for that entry. This barrier is
>> >  implied by writel() but not writel_relaxed(). If you don't have
>> >  a hardware page table walker in your IOMMU, you don't need to worry
>> >  about this.
>> >
>>   To get my understanding correct here, is the barrier required here because
>>    of speculative fetch ?
>
>It doesn't have to be a speculative fetch, it could also be a normal
>fetch triggered by a DMA. The point is that the CPU is free to reorder
>the store to (io page table) memory relative to the store to the
>IOMMU flush register, and when you directly follow up with a store to
>the DMA master that triggers a transfer, this transfer can hit the
>IOMMU and cause the stale page table entry to be read.
>
>I guess in practice the barrier that comes before the MMIO write to the
>DMA master would ensure ordering of the DMA after both the IOPTE
>update and the MMIO flush (unless there was the speculative fetch you
>mentioned), but this is where I'm no longer confident enough in my
>pwn understanding of th ordering rules to rely on that. Having the
>barrier between the IOPTE update and the flush certainly leaves
>no room for ambiguity.
    Ok understand. Also i think speculative access if any, should 
     go away if unmap is done first and then remap by clients.

Regards,
 Sricharan

  reply	other threads:[~2016-05-25 16:49 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-20 10:54 [PATCH V5 0/7] iommu/msm: Add DT adaptation and generic bindings support Sricharan R
2016-05-20 10:54 ` Sricharan R
2016-05-20 10:54 ` [PATCH V5 1/7] iommu/msm: Add DT adaptation Sricharan R
2016-05-20 10:54   ` Sricharan R
2016-05-20 10:54 ` [PATCH V5 2/7] documentation: iommu: Add bindings for msm,iommu-v0 ip Sricharan R
2016-05-20 10:54   ` [PATCH V5 2/7] documentation: iommu: Add bindings for msm, iommu-v0 ip Sricharan R
     [not found]   ` <1463741694-1735-3-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2016-05-23 21:23     ` [PATCH V5 2/7] documentation: iommu: Add bindings for msm,iommu-v0 ip Rob Herring
2016-05-23 21:23       ` Rob Herring
2016-05-20 10:54 ` [PATCH V5 4/7] iommu/msm: Add support for generic master bindings Sricharan R
2016-05-20 10:54   ` Sricharan R
2016-05-20 10:54 ` [PATCH V5 5/7] iommu/msm: use generic ARMV7S short descriptor pagetable ops Sricharan R
2016-05-20 10:54   ` Sricharan R
2016-05-20 10:54 ` [PATCH V5 6/7] iommu/msm: Use writel_relaxed and add a barrier Sricharan R
2016-05-20 10:54   ` Sricharan R
     [not found]   ` <1463741694-1735-7-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2016-05-20 11:44     ` Arnd Bergmann
2016-05-20 11:44       ` Arnd Bergmann
2016-05-20 12:20       ` Arnd Bergmann
2016-05-20 12:20         ` Arnd Bergmann
2016-05-23  6:05       ` Sricharan
2016-05-23  6:05         ` Sricharan
2016-05-24 14:00         ` Arnd Bergmann
2016-05-24 14:00           ` Arnd Bergmann
2016-05-25 10:45           ` Sricharan
2016-05-25 10:45             ` Sricharan
2016-05-25 12:18             ` Arnd Bergmann
2016-05-25 12:18               ` Arnd Bergmann
2016-05-25 13:19               ` Sricharan
2016-05-25 13:19                 ` Sricharan
2016-05-25 14:15                 ` Arnd Bergmann
2016-05-25 14:15                   ` Arnd Bergmann
2016-05-25 16:49                   ` Sricharan [this message]
2016-05-25 16:49                     ` Sricharan
2016-05-20 10:54 ` [PATCH V5 7/7] iommu/msm: Remove driver BROKEN Sricharan R
2016-05-20 10:54   ` Sricharan R
2016-05-23  2:53 ` [PATCH V5 0/7] iommu/msm: Add DT adaptation and generic bindings support Archit Taneja
2016-05-23  2:53   ` Archit Taneja
     [not found] ` <1463741694-1735-1-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2016-05-20 10:54   ` [PATCH V5 3/7] iommu/msm: Move the contents from msm_iommu_dev.c to msm_iommu.c Sricharan R
2016-05-20 10:54     ` Sricharan R
2016-05-23  8:10   ` [PATCH V5 0/7] iommu/msm: Add DT adaptation and generic bindings support Srinivas Kandagatla
2016-05-23  8:10     ` Srinivas Kandagatla

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='000901d1b6a5$6dcd5f90$49681eb0$@codeaurora.org' \
    --to=sricharan-sgv2jx0feol9jmxxk+q4oq@public.gmane.org \
    --cc=architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=arnd-r2nGTMty4D4@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=stepanm-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.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.