linux-kernel - Re: n900 in next-20170901

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: <20171107053313.GA12447@js1304-P5Q-DELUXE>
Date:   Tue, 7 Nov 2017 14:33:14 +0900
From:   Joonsoo Kim <iamjoonsoo.kim@....com>
To:     Tony Lindgren <tony@...mide.com>
Cc:     Pavel Machek <pavel@....cz>, pali.rohar@...il.com, sre@...nel.org,
        kernel list <linux-kernel@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-omap@...r.kernel.org, khilman@...nel.org,
        aaro.koskinen@....fi, ivo.g.dimitrov.75@...il.com,
        patrikbachan@...il.com, serge@...lyn.com, abcloriens@...il.com,
        "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        Russell King <linux@...linux.org.uk>
Subject: Re: n900 in next-20170901

Hello,

Sorry for dealy. I was on vacation during last week.

On Thu, Oct 26, 2017 at 07:16:27AM -0700, Tony Lindgren wrote:
> * Joonsoo Kim <iamjoonsoo.kim@....com> [171025 21:45]:
> > On Wed, Oct 25, 2017 at 10:31:38AM -0700, Tony Lindgren wrote:
> > > Great, this branch boots on n900! Early parts of the dmesg attached
> > > below.
> > 
> > Sound good. Perhaps, problem is due to the cache management. With my
> > patchset, direct mapping for CMA area is cleared in
> > dma_contiguous_remap() if CONFIG_HIGHMEM. I guess that proper cache
> > operation is required there.
> > 
> > Could you test my newly updated branch again? I re-enable
> > dma_contiguous_remap() to clear direct mapping for CMA area and add
> > proper(?) cache operation although I'm not the expert on that area.
> > 
> > https://github.com/JoonsooKim/linux/tree/cma-debug4-next-20180901
> 
> Yeah that one boots too, dmesg below. I wonder why flushing the range
> with dmac_flush_range() and outer_flush_range() no longer seems enough
> though?

I don't know the reason. AFAIK, it should be enough to flush cpu cache
before clearing page table entry, however, it doesn't work for n900.

flush_cache_all() has not only d-cache flushing but also i-cache
flushing. So, I'd like to test effect of i-cache flushing.

Could you test follwing updated branch?

https://github.com/JoonsooKim/linux/tree/cma-debug4-next-20180901

It has three relevant commits on top and enables CMA memory use.

"arm/dma: remove i-cache flush"
"arm/dma: flush i-cache and d-cache separately"
"arm/dma: call flush_cache_all() and don't do outer cache operation"

I think that flush_cache_all() here looks fine because size of CMA
area is usually large enough to use flush_cache_all() but
understanding root cause would be important so I ask this test. If u
don't mind, please test each commit with reverting one by one and let
me know the result of each test.

> Reverting the arch/arm/mm/dma-mapping.c changes in your patch
> "arm/dma: re-enable to remap the CMA area and flush cache before
> remapping" also boots, but produces the following build warnings:
> 
> arch/arm/mm/dma-mapping.c: In function 'dma_contiguous_remap':
> arch/arm/mm/dma-mapping.c:503:20: warning: passing argument 1 of
> 'cpu_cache.dma_flush_range' makes pointer from integer without a
> cast [-Wint-conversion]
> dmac_flush_range(map.virtual, map.virtual + map.length);
>                     ^~~
> arch/arm/mm/dma-mapping.c:503:20: note: expected 'const void *' but
> argument is of type 'long unsigned int'
> arch/arm/mm/dma-mapping.c:503:33: warning: passing argument 2 of
> 'cpu_cache.dma_flush_range' makes pointer from integer without a
> cast [-Wint-conversion]
> dmac_flush_range(map.virtual, map.virtual + map.length);

Thanks for info.
I think that incorrect type would not be related to this problem.

Thanks.