linux-kernel - Re: [PATCH v1 14/15] scsi: ufs: commit descriptors before setting the doorbell

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 27 Aug 2015 12:02:46 -0500
From:	Rob Herring <robherring2@...il.com>
To:	Yaniv Gardi <ygardi@...eaurora.org>
Cc:	linux-arm-msm <linux-arm-msm@...r.kernel.org>,
	linux-scsi@...r.kernel.org, Dolev Raviv <draviv@...eaurora.org>,
	Gilad Broner <gbroner@...eaurora.org>,
	linux-scsi-owner@...r.kernel.org,
	Jej B <james.bottomley@...senpartnership.com>,
	Santosh Y <santoshsy@...il.com>,
	Subhash Jadavani <subhashj@...eaurora.org>, hch@...radead.org,
	Paul Bolle <pebolle@...cali.nl>,
	"James E.J. Bottomley" <jbottomley@...n.com>,
	Vinayak Holikatti <vinholikatti@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 14/15] scsi: ufs: commit descriptors before setting the doorbell

On Thu, Aug 27, 2015 at 7:28 AM,  <ygardi@...eaurora.org> wrote:
>>> On Aug 21, 2015 3:10 PM, "Yaniv Gardi" <ygardi@...eaurora.org> wrote:
>>>>
>>>> Add a write memory barrier to make sure descriptors prepared are
>>>> actually
>>>> written to memory before ringing the doorbell. We have also added the
>>>> write memory barrier after ringing the doorbell register so that
>>>> controller sees the new request immediately.

[...]

>>>> +       /* Make sure descriptors are ready before ringing the doorbell
>>>> */
>>>> +       wmb();
>>>
>>> The writel for the doorbell will do a barrier first. (I didn't check
>>> what exactly ufshcd_writel does, but that is why I don't like these
>>> private access wrappers.)
>
> the barrier here is important and a must.
> before it we prepare descriptors.
> after it we write to DOOR-BELL.
> (and after the DOOR-BELL we have another one.)
> if we remove it, we might get the DOOR-BELL written, before the
> descriptors written.

If you dig into what writel does, you will see that what the code here
ends up being is:

descriptor setup
wmb()
__iomwb() -> wmb on arm64
doorbell register write

So explain why you need 2 barriers.

Barriers in a driver are a red flag. Usually they are not needed, but
sometimes they are. You have to be able to explain why if they are.
Pretty much every descriptor based DMA device works as you describe
and has the ordering problem. Because of that, the core code takes
care of this.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/