linux-kernel - Re: [PATCH 1/7] mips: dmi: Fix early remap on MIPS32

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: <c73d9dbf-b637-47ff-ae2d-6f8987345410@app.fastmail.com>
Date:   Mon, 27 Nov 2023 21:08:11 +0000
From:   "Jiaxun Yang" <jiaxun.yang@...goat.com>
To:     "Serge Semin" <fancer.lancer@...il.com>
Cc:     "Thomas Bogendoerfer" <tsbogend@...ha.franken.de>,
        "Arnd Bergmann" <arnd@...db.de>,
        "Andrew Morton" <akpm@...ux-foundation.org>,
        "Mike Rapoport" <rppt@...nel.org>,
        "Matthew Wilcox" <willy@...radead.org>,
        "Tiezhu Yang" <yangtiezhu@...ngson.cn>,
        "Huacai Chen" <chenhuacai@...nel.org>,
        "Yinglu Yang" <yangyinglu@...ngson.cn>,
        "Alexey Malahov" <Alexey.Malahov@...kalelectronics.ru>,
        "Aleksandar Rikalo" <aleksandar.rikalo@...mia.com>,
        "Aleksandar Rikalo" <arikalo@...il.com>,
        "Dragan Mladjenovic" <dragan.mladjenovic@...mia.com>,
        "Chao-ying Fu" <cfu@...ecomp.com>, "Marc Zyngier" <maz@...nel.org>,
        "linux-mips@...r.kernel.org" <linux-mips@...r.kernel.org>,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/7] mips: dmi: Fix early remap on MIPS32



在2023年11月27日十一月 下午4:23，Serge Semin写道：
[...]
>> I just made a quick grep in tree, and it seems like we don't have much
>> user of ioremap_cache (as well as ioremap_uc/wc) here so I think it is
>> a safe assumption.
>
> I wouldn't say there aren't much users. ioremap_wc() and it's
> devm-version is widely utilized in the GPU and network and some other
> subsystems. ioremap_cache() isn't widespread indeed. In anyway even a
> single user must be supported in safely calling the method if it's
> provided by the arch-code, otherwise the method could be considered as
> just a bogus stub to have the kernel successfully built. I bet you'll
> agree with that. But that's not the point in this case,
>
> A bit later you also noted:
>
> On Fri, Nov 24, 2023 at 10:34:49PM +0000, Jiaxun Yang wrote:
>> A nip, _page_cachable_default is set in cpu_cache_init() as well. We'd
>> better move it to cpu-probe.c, or give it a reasonable default value.
>
> Right. Thanks. To be honest I haven't noticed that before your
> message. _page_cachable_default is indeed initialized in the
> cpu_cache_init() method, several steps after it would be used in the
> framework of dmi_remap_early(). On the other hand ioremap_cache() is
> defined as ioremap_prot() with the _page_cachable_default variable
> passed. So my code will still correctly work unless
> _page_cachable_default is pre-initialized with something other than
> zero. On the other hand we can't easily change its default value
> because it will affect and likely break the r3k (CPU_R3000) and Octeon
> based platforms, because it's utilized to initialize the
> protection-map table. Of course we can fix the r3k_cache_init() and
> octeon_cache_init() methods too so they would get the
> _page_cachable_default variable back to zero, but it will also make
> things around it more complicated.
>
> Also note, moving the _page_cachable_default initialization to the
> earlier stages like cpu_probe() won't work better because the field
> value may get change for instance in the framework of the smp_setup()
> function (see cps_smp_setup()).
>
> So after all the considerations above this solution now looks even
> clumsier than before.( Any idea how to make it better?

I think the best solution maybe just use CKSEG0 to setup map here.

Btw I was thinking about 64 bit here, I thought for 64bit we would
just embedded prot into XKPHYS, however I quickly figure out
ioremap_cache was never implemented properly on 64-bit system,
so does ioremap_wc.

> u64 base = (flags == _CACHE_UNCACHED ? IO_BASE : UNCAC_BASE);

Which is always uncached mapping.

>> 
[...]
>
> Note the memory might be clobbered even before dmi_setup() for
> instance by means of the early_memtest() method. In anyway it would be
> better if the system booloader would have already reserved the DMI
> memory (in DTB) or it would have been done by the platform-specific
> plat_mem_setup() method.

Unfortunately, too many machines are shipped with those badly designed
firmware. We rely on dmi_setup code to scan and preserve dmi table from
random location in memory.

>
>> The second is we may have some early quirks depends on DMI
>> information.
>
> Which quirks do you mean to be dependent in between the current
> dmi_setup() call place and the cpu_cache_init() method invocation?

I think we don't have any for now.

-- 
- Jiaxun