linux-kernel - Re: [PATCH] [RFC] EHCI: add to memory barrier to updating hw

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]

Message-ID: <CACVXFVOu-2q74JmHLEo3mCVSAzfc7n8GmqPDMPhEDp0wL37w_w@mail.gmail.com>
Date:	Wed, 17 Jul 2013 16:51:30 +0800
From:	Ming Lei <tom.leiming@...il.com>
To:	김기오 <gioh.kim@....com>
Cc:	Alan Stern <stern@...land.harvard.edu>, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org, Mark Salter <msalter@...hat.com>,
	namhyung.kim@....com, Minchan Kim <minchan.kim@....com>,
	Chanho Min <chanho.min@....com>,
	Jong-Sung Kim <neidhard.kim@....com>,
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] [RFC] EHCI: add to memory barrier to updating hw_next

Cc: ARM list

On Wed, Jul 17, 2013 at 1:03 PM, 김기오 <gioh.kim@....com> wrote:
> Hi,
>
> I have a missing urb completion problem on ARMv7 based platform.
>
> I thought the above problem was caused by coherent memory between the
> EHCI device and CPU so I tryied to allocates device type memory
> for EHCI via dma_declare_coherent_memory at machine initialization step
> so that EHCI always allocates from those device type memory.
> It seems to solve the issue because I didn't see any problem.
>
> But I am not sure it is acceptable solution. So I applied the patch
> https://lkml.org/lkml/2011/8/31/344.
> But it could not solve the problem so that I added another wmb()
> as my patch, and now my platform works fine.

I remember the previous problem reported on Pandaboard firstly was
fixed by Will's commit 11ed0ba1(ARM: 7161/1: errata: no automatic store
buffer drain), so could you try to enable PL310_ERRATA_769419 and
see if your problem can be fixed?

>
> I am not sure what's the exact problem and what wmb I added could solve
> but I just think the problem is related to store buffer flush of hw_next.

Yes, per the above commit, but it might be one hardware problem.

> Anyway, important thing is that it fixed my problem so I expect
> you expert guys could find what I am missing and a right solution.
> IMHO, the patch might miss updating hw_next pointer.
> Am I correct?
>
> I understand the wmb() is just memory barrier, not write-buffer flush.
> But it is true that wmb() can flush write buffer in ARM.
> Anyhow I think that memory type, "normal memory, non-cacheable", may
> have a problem for some devices that needs device type memory.

Currently on ARMv7 DMA coherent buffer has to be bufferable, in theory
we might need one API to flush CPU write buffer, as described in
Documentation/DMA-API-HOWTO.txt:

             Also, on some platforms your driver may need to flush CPU write
             buffers in much the same way as it needs to flush write buffers
             found in PCI bridges (such as by reading a register's value
             after writing it).

But actually on hardware without the problem(such as A15), I don't see
any effect without flushing store buffer explicitly.

>
> I cannot get conclusion from the discussion at
> https://lkml.org/lkml/2011/8/31/344.
> Which can I do for my platform, wmb() or dma_coherent_write_sync()?

If your hardware is covered by PL310_ERRATA_769419, maybe it is
better to enable the errata.

>
> Signed-off-by: Gioh Kim <gioh.kim@....com>
> ---
>  drivers/usb/host/ehci-q.c |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
> index d34b399..779d9e8 100644
> --- a/drivers/usb/host/ehci-q.c
> +++ b/drivers/usb/host/ehci-q.c
> @@ -501,6 +501,7 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
>                         last = list_entry (qtd->qtd_list.prev,
>                                         struct ehci_qtd, qtd_list);
>                         last->hw_next = qtd->hw_next;
> +                       wmb();
>                 }
>
>                 /* remove qtd; it's recycled after possible urb completion */
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Thanks,
-- 
Ming Lei
--
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/