Re: [PATCH RFC 0/3] riscv: Add DMA_COHERENT support

From: Guo Ren <guoren@kernel.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "Palmer Dabbelt" <palmer@dabbelt.com>,
	"Anup Patel" <Anup.Patel@wdc.com>,
	"Anup Patel" <anup@brainfault.org>,
	"Drew Fustini" <drew@beagleboard.org>,
	"Christoph Hellwig" <hch@lst.de>,
	wefu@redhat.com, "Wei Wu (吴伟)" <lazyparser@gmail.com>,
	linux-riscv <linux-riscv@lists.infradead.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	linux-sunxi@lists.linux.dev, "Guo Ren" <guoren@linux.alibaba.com>,
	"Paul Walmsley" <paul.walmsley@sifive.com>
Subject: Re: [PATCH RFC 0/3] riscv: Add DMA_COHERENT support
Date: Fri, 4 Jun 2021 22:47:22 +0800	[thread overview]
Message-ID: <CAJF2gTTpurWpPUcA2JkF0rOFztKQgFBhOF9zQyuyi_-sxszhRQ@mail.gmail.com> (raw)
In-Reply-To: <CAK8P3a03sxxnzpZPxNnXLtCFOFBZ6espEj4V5y=K+59dOLJc6A@mail.gmail.com>

Hi Arnd & Palmer,

Sorry for the delayed reply, I'm working on the next version of the patch.

On Fri, Jun 4, 2021 at 5:56 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Jun 3, 2021 at 5:39 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > On Wed, 02 Jun 2021 23:00:29 PDT (-0700), Anup Patel wrote:
> > >> This implementation, which adds some Kconfig entries that control page table
> > >> bits, definately isn't suitable for upstream.  Allowing users to set arbitrary
> > >> page table bits will eventually conflict with the standard, and is just going to
> > >> be a mess.  It'll also lead to kernels that are only compatible with specific
> > >> designs, which we're trying very hard to avoid.  At a bare minimum we'll need
> > >> some way to detect systems with these page table bits before setting them,
> > >> and some description of what the bits actually do so we can reason about
> > >> them.
> > >
> > > Yes, vendor specific Kconfig options are strict NO NO. We can't give-up the
> > > goal of unified kernel image for all platforms.
Okay,  Agree. Please help review the next version of the patch.

> >
> > I think this is just a phrasing issue, but just to be sure:
> >
> > IMO it's not that they're vendor-specific Kconfig options, it's that
> > turning them on will conflict with standard systems (and other vendors).
> > We've already got the ability to select sets of Kconfig settings that
> > will only boot on one vendor's system, which is fine, as long as there
> > remains a set of Kconfig settings that will boot on all systems.
> >
> > An example here would be the errata: every system has errata of some
> > sort, so if we start flipping off various vendor's errata Kconfigs
> > you'll end up with kernels that only function properly on some systems.
> > That's fine with me, as long as it's possible to turn on all vendor's
> > errata Kconfigs at the same time and the resulting kernel functions
> > correctly on all systems.
>
> Yes, this is generally the goal, and it would be great to have that
> working in a way where a 'defconfig' build just turns on all the options
> that are needed to use any SoC specific features and drivers while
> still working on all hardware. There are however limits you may run
> into at some point, and other architectures usually only manage to span
> some 10 to 15 years of hardware implementations with a single
> kernel before it get really hard.
I could follow the goal in the next version of the patchset. Please
help review, thx.

>
> To give some common examples that make it break down:
>
> - 32-bit vs 64-bit already violates that rule on risc-v (as it does on
>   most other architectures)
>
> - architectures that support both big-endian and little-endian kernels
>   tend to have platforms that require one or the other (e.g. mips,
>   though not arm). Not an issue for you.
>
> - page table formats are the main cause of incompatibility: arm32
>   and x86-32 require three-level tables for certain features, but those
>   are incompatible with older cores, arm64 supports three different
>   page sizes, but none of them works on all cores (4KB almost works
>   everywhere).
>
> - SMP-enabled ARMv7 kernels can be configured to run on either
>   ARMv6 or ARMv8, but not both, in this case because of incompatible
>   barrier instructions.
>
> - 32-bit Arm has a couple more remaining features that require building
>   a machine specific kernel if enabled because they hardcode physical
>   addresses: early printk (debug_ll, not the normal earlycon), NOMMU,
>   and XIP.
>
>        Arnd

-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/