From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6A8C3C4167B for ; Mon, 27 Nov 2023 16:23:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 106656B02F3; Mon, 27 Nov 2023 11:23:36 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0B6766B02F4; Mon, 27 Nov 2023 11:23:36 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EC1266B02F5; Mon, 27 Nov 2023 11:23:35 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id DD8876B02F3 for ; Mon, 27 Nov 2023 11:23:35 -0500 (EST) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 9800E14041E for ; Mon, 27 Nov 2023 16:23:35 +0000 (UTC) X-FDA: 81504254790.29.A10175B Received: from mail-lf1-f54.google.com (mail-lf1-f54.google.com [209.85.167.54]) by imf17.hostedemail.com (Postfix) with ESMTP id 940D94002D for ; Mon, 27 Nov 2023 16:23:33 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=I39ixxwJ; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf17.hostedemail.com: domain of fancer.lancer@gmail.com designates 209.85.167.54 as permitted sender) smtp.mailfrom=fancer.lancer@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1701102213; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=U2WmaNst220bl2JYvYWQafBEL6DaauacmewQ2Ui+7EQ=; b=xJ2ng+gfQiBsTPnVdjIb5I/lt8GNburLAbjMXaGXMDAqaRFXh+ndMDOU7IcN03TdJ0osjo AArdPE2UXgXwoNGElrjtrGWJF3be1es6lbEcMYmDXGivIQtNNPbaXkU7SRrQH+ifchmDAH u3st+KeD78dFm6wKb2GTDe19DhKXYnE= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=I39ixxwJ; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf17.hostedemail.com: domain of fancer.lancer@gmail.com designates 209.85.167.54 as permitted sender) smtp.mailfrom=fancer.lancer@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1701102213; a=rsa-sha256; cv=none; b=iPNiy4i4Tmb51QE5tTX2yj/QXTAZhQhD2XRi89pIBMwzLq6/4TfLND53vnYt64UsnWa4V8 k6MSjx42TzmLzYVK9thZK4KVRED5r6y0M+PJNoVqmfdrBRxiVTEEUWAlhrNzj9JnF3NSOq 4rqKqz3wA2hTC2dI1YcVu5CH4G5n4bE= Received: by mail-lf1-f54.google.com with SMTP id 2adb3069b0e04-507be298d2aso6017102e87.1 for ; Mon, 27 Nov 2023 08:23:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701102212; x=1701707012; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=U2WmaNst220bl2JYvYWQafBEL6DaauacmewQ2Ui+7EQ=; b=I39ixxwJ0NLm5BZV5nGqXQQTBCG69EVWy/Fyx9oxcyDAsKfb5yuNNIbJgygWJr7yCk yirrG8qoswnigtyuinv1sNIF8mmZ84ARJ5MiPixucLorGynp60gRJsVHG+LGjLFAc3PZ qQaPW3Gd3VjbCmWIEJaO1pXu41jfPwJk0xAYMRglubtAw4xvlFeu2HMQ1/AuHNRGT2NY 6XNk51P/U0GjdFxaN88hAqeLW0gASXAoUf2+CdilShPDvkrzhS/aS9D3+WYmmgYdH626 JlSawZeRtTRFs9Wy2kqhyGxhecrZI21HL8lJzDA3LyyGfrP3tOad4ykiZUXFDEacdbP0 Y+zg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701102212; x=1701707012; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=U2WmaNst220bl2JYvYWQafBEL6DaauacmewQ2Ui+7EQ=; b=X7xZzO9DbJjtgpPjLCsqPjIWTdqjKiO4KTbquYs8Il9D4yHKEq31TLfqw/I8Egj180 GEZpF6WEn79AE77FbzICziEKoBw7HbUePB/T9ewRTCU7RwObYm2J7X9W/vr+9GzdFEJ9 iTkCrfaheZrwtKY/h+mCk6EKZJfX2+JIf8qFt9sanEGl9gbonqKjC+cG/Xf9s8wEA9Uh RjZ8DQWF7f4khDdj4yN4ifDq8c/v4DHbF9+iVm+cj+PbV3LmFR/RictXWuSRPz16Q7Le mjbdJoFleaeG1tJpdnArKK5PZ1YLTaukGefpAH9y567I/yiXGNQLw+FUumz2tXtBYe5L vgaA== X-Gm-Message-State: AOJu0YwebyrVceUli4GcHU2hy8FUvAjMrHr1A7BXGFsVardQxcJEYK8Y mr6mqmO6lSxeZDdobJQWPos= X-Google-Smtp-Source: AGHT+IG7xu7bSIz5oLxdKNj7/aZosFlhEMUeLxrFPbwCQdns90kewsQKelW62cF7HIq6A7e/TTD+rw== X-Received: by 2002:a05:6512:3c86:b0:507:a40e:d8bf with SMTP id h6-20020a0565123c8600b00507a40ed8bfmr10570132lfv.7.1701102211320; Mon, 27 Nov 2023 08:23:31 -0800 (PST) Received: from mobilestation ([178.176.56.174]) by smtp.gmail.com with ESMTPSA id d36-20020a0565123d2400b0050aa8d38c40sm1533916lfv.168.2023.11.27.08.23.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Nov 2023 08:23:30 -0800 (PST) Date: Mon, 27 Nov 2023 19:23:27 +0300 From: Serge Semin To: Jiaxun Yang Cc: Thomas Bogendoerfer , Arnd Bergmann , Andrew Morton , Mike Rapoport , Matthew Wilcox , Tiezhu Yang , Huacai Chen , Yinglu Yang , Alexey Malahov , Aleksandar Rikalo , Aleksandar Rikalo , Dragan Mladjenovic , Chao-ying Fu , Marc Zyngier , "linux-mips@vger.kernel.org" , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/7] mips: dmi: Fix early remap on MIPS32 Message-ID: References: <20231122182419.30633-2-fancer.lancer@gmail.com> <8ca730b9-fa8c-46ea-bdc5-158da0f29c3a@app.fastmail.com> <245d3985-9085-4be0-8c74-d95d06334584@app.fastmail.com> <3iksuovvsln3cw3xpmjd7f7xixfvwaneu4ok56fnookvyolpco@wrxxew3thgnq> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 940D94002D X-Stat-Signature: nudsa9fy8n145p3a68ttrpzzb93n5ik1 X-HE-Tag: 1701102213-992437 X-HE-Meta: U2FsdGVkX18tbzup4l3hKYVnVnb17QVq+hxOhOSUKv24ZyyQBNxakeHspcsAdmNxFthKNBvV/Xd2dNaUBEZfb/7/Isly8l0oELOErJ89yo2qfaUDTecKKqbZ91121OL0qS07XP8QNmx8MR5N2C87CfgBrh0r3ZdJHeT9u/38c7mEfGWEqq0eMKs+yo2sJl8u5VqxLkvFkoa9bj0PAaVuMxbS6eGjYzp9QAX83NxPRPEhMS4lnZFVcuvOqrQluoMsD3bFSDz33y8KxiESahRMergzfejJjZ61OW6OQJ8NcsgL9opgHGdgt3MU2PcR7rVaVvaU9fTJIGrtGApgzwYXFXKV1UakrAztDFLiSW9JV7n0Eoa9/1fBA/ZPivQNGxXVq1rh5uuzrcaDowQxrQFCgMSwsF2ZbLn+xaIRUl/M93JHv4Ww3AAX2qYdS1yXEr+JK3XFMM0i+a+8QN5T5Ee7XMS6DRWExtif2CJLTflBJ3KmkOmoP7oFzxQUAAtM2GQGwzjdTsgj9PN36GCpDFEnCAXEoWsGKUiWNYFAXRV881hufzdOG9q+/ikMxVEHO4kMZM+xhCALuzmD/LU4qdYVf//fytGi+WjDzm1068zaVmQU9wt6XMpxIM1gTmJwsiSH+VMLljo3wcK0DRApnwtfq6nJ4jy+RHVXz32RgcV+HuBO9hvRq35AtE6RpdokEQ3VkIsn0aHT1P48R1c1kS7IJLMs2w7W1giYGEq44MZbyCGuaC+HUksR7rzFp3rc+SUJcxgGTVbLde4O4MKihClJa3OdG1MV4FIu7HJGjjW0nz6PTi43ctJbnF3yJaTvhUeafIdn6P7J+CkXvSHttPQKJOJ+BIdtW4Eod6EDIhkrcCCil41YcpO/pJ6GaUsQqgpU0S6hvN+rsVgXU9zvDgJg/iCvpFayRVbid/cBdQmFRCUXy8WHL1YLPBLPtItXtsHOVdGDaLCoeUNF+Aiji1Z e3XeXVJO cXm/8Xqb7+cVx+hJbx+0GNTrXii8phhQLTs0YvJDovY54T7WL+sSbEIS9Wst2j3FDhsqLtLTafOqtUGUfHeyhHcjo71LUwHNFDI5vRod2ojPhdQZxFVfy1DuGuLdbYpHQdPj/hM3dEYZkw8z5G4RV239ru+VglMxgWr5UQM+PbGrsLt1LvMowZaGE9AZU4LH+Mmv6v05YNMnJZR8Xjj/y3SnT6ySzHfLoHGw4iJa5Cd92hHHraWCGrifyo7S9elXEAepdZr0bDqP8sTxtNmd1YxDjsc32g4cvh2DptofWNdAzE+zU38PgggytZmsT6Zks1CpkqCZvj0Als2i3WjjlyW2XUNNI2ftHPAc0TRZBuN30EBIMPfl0Uf+yiqek2f0QaTref3RqXxUuu3ppHxfHwLSEPhQIY1HhRuiqy2AAfdfhWgsgG8uR5owRz0T2qTi/UGdJias9sdCAVjJ34ZanV2V02WuFrcJRWKNYcA+UUs1bEYZIMbZmk/M841zwA8KVLVGr5k5n7LkhX7oo4+n1Yc/WLz+kT5fzCCgqsuOro5hr9fHzlQhOZgsgUw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Hi Jiaxun On Fri, Nov 24, 2023 at 10:03:49PM +0000, Jiaxun Yang wrote: > 在2023年11月24日十一月 下午6:52，Serge Semin写道： > > On Thu, Nov 23, 2023 at 05:33:31PM +0000, Jiaxun Yang wrote: > >> > [...] > >> Actually dmi_setup() is called before cpu_cache_init(). > > > > To preliminary sum the discussion, indeed there can be issues on the > > platforms which have DMI initialized on the cached region. Here are > > several solutions and additional difficulties I think may be caused by > > implementing them: > > Thanks for such detailed conclusion! > I'd prefer go solution 1, with comments below. > > > > 1. Use unmapped cached region utilization in the MIPS32 ioremap_prot() > > method. > > This solution a bit clumsy than it looks on the first glance. > > ioremap_prot() can be used for various types of the cachability > > mapping. Currently it's a default-cacheable CA preserved in the > > _page_cachable_default variable and Write-combined CA saved in > > boot_cpu_data.writecombine. Based on that we would have needed to use > > the unmapped cached region utilized for the IO-remaps called with the > > "_page_cachable_default" mapping flags passed only. The rest of the IO > > range mappings, including the write-combined ones, would have been > > handled by VM means. This would have made the ioremap_prot() a bit > > less maintainable, but still won't be that hard to implement (unless I > > miss something): > > --- a/arch/mips/mm/ioremap.c > > +++ b/arch/mips/mm/ioremap.c > > /* > > - * Map uncached objects in the low 512mb of address space using KSEG1, > > - * otherwise map using page tables. > > + * Map uncached/default-cached objects in the low 512mb of address > > + * space using KSEG1/KSEG0, otherwise map using page tables. > > */ > > - if (IS_LOW512(phys_addr) && IS_LOW512(last_addr) && > > - flags == _CACHE_UNCACHED) > > - return (void __iomem *) CKSEG1ADDR(phys_addr); > > + if (IS_LOW512(phys_addr) && IS_LOW512(last_addr)) { > > + if (flags == _CACHE_UNCACHED) > > + return (void __iomem *) CKSEG1ADDR(phys_addr); > > + else if (flags == _page_cachable_default) > > + return (void __iomem *) CKSEG0ADDR(phys_addr); > > + } > > > > Currently I can't figure out what obvious problems it may cause. But > > It seems suspicious that the cacheable IO-mapping hasn't been > > implemented by the unmapped cacheable region in the first place. In > > anyway this solution looks more dangerous than solution 2. because it > > affects all the MIPS32 platforms at once. > > 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? > > > > > 2. Convert dmi_remap_early() to ioremap_uc() (actually just ioremap() > > as noted by Arnd). > > As Jiaxun correctly noted this may cause problems on the platforms > > which don't flush caches before jumping out to the kernel. Thomas said > > that kernel flushes the caches early on boot, but Jiaxun noted that > > it's still done after early DMI setup. So the issue with solution 2 is > > that the setup_arch() method calls dmi_setup() before it flushes the > > caches by means of the cpu_cache_init() method. I guess it can be > > fixed just by moving the dmi_setup() method invocation to be after the > > cpu_cache_init() is called. This solution looks much less invasive > > than solution 1. > > I recall Tiezhu made dmi_setup() here for reasons. The first reason is that > DMI is placed at memory space that is not reserved, so it may get clobbered > after mm is up. 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. > 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? -Serge(y) > > Thanks. > > > > So what do you think? What solution do you prefer? Perhaps > > alternative? > > > > -Serge(y) > > > >> > >> Thanks > >> > > >> > Thomas. > >> > > >> > -- > >> > Crap can work. Given enough thrust pigs will fly, but it's not necessarily a > >> > good idea. [ RFC1925, 2.3 ] > >> > >> -- > >> - Jiaxun > > -- > - Jiaxun