Re: [RFC 0/2] RISC-V: A proposal to add vendor-specific code

[Karsten Merker]

On Wed, Oct 31, 2018 at 10:27:05AM -0700, Palmer Dabbelt wrote:
> On Wed, 31 Oct 2018 04:16:10 PDT (-0700), anup@xxxxxxxxxxxxxx wrote:
> > On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@xxxxxxxxxxxxx> wrote:
> > > 
> > >   RISC-V permits each vendor to develop respective extension ISA based
> > > on RISC-V standard ISA. This means that these vendor-specific features
> > > may be compatible to their compiler and CPU. Therefore, each vendor may
> > > be considered a sub-architecture of RISC-V. Currently, vendors do not
> > > have the appropriate examples to add these specific features to the
> > > kernel. In this RFC set, we propose an infrastructure that vendor can
> > > easily hook their specific features into kernel. The first commit is
> > > the main body of this infrastructure. In the second commit, we provide
> > > a solution that allows dma_map_ops() to work without cache coherent
> > > agent support. Cache coherent agent is unsupported for low-end CPUs in
> > > the AndeStar RISC-V series. In order for Linux to run on these CPUs, we
> > > need this solution to overcome the limitation of cache coherent agent
> > > support. Hence, it also can be used as an example for the first commit.
> > > 
> > >   I am glad to discuss any ideas, so if you have any idea, please give
> > > me some feedback.
> > > 
> > I agree that we need a place for vendor-specific ISA extensions and
> > having vendor-specific directories is also good.
> > 
> > What I don't support is the approach of having compile time selection
> > of vendor-specific ISA extension.
> > 
> > We should have runtime probing for compatible vendor-specific ISA
> > extension. Also, it should be possible to link multiple vendor-specific
> > SA extensions to same kernel image. This way we can have a single
> > kernel image (along with various vendor-specific ISA extensions) which
> > works on variety of targets/hosts.
> > 
> > As an example or runtime probing you can look at how IRQCHIP or
> > CLOCKSOURCE drivers are probed. The vendor-specific ISA extension
> > hooks should called in similar fashion.
> 
> Yes, I agree.  My biggest concern here is that we ensure that
> one kernel can boot on implementations from all vendors.  I
> haven't had a chance to look at the patches yet, but it should
> be possible to:
> 
> * Build a kernel that has vendor-specific code from multiple vendors.
> * Detect the implementation an run time and select the correct extra
>   code.

>From a distro point of view we definitely want to have one kernel
image that is bootable everywhere.  Debian won't support any
platform that requires a per-platform or per-vendor kernel, and I
assume that the same will be true for Fedora and Suse.

One thing that I have stumbled upon while looking at the patches
is that they seem to assume that X-type ISA extensions are
strictly per vendor.  Although that is probably true in the
majority of cases, it doesn't necessarily have to be - I could
e.g. imagine that the DSP extensions from the PULP cores might
be used by multiple vendors.  If such an extension would have
state that needs to be saved on context switch, it would need
corresponding kernel support.  Using "PULP" (or any other
open-source project) as the vendor in such a case leads to
another potential issue: the patches base everything on a JEDEC
vendor ID that is compared to the contents of the mvendorid CSR,
but such a JEDEC vendor ID usually doesn't exist for open-source
implementations; the majority of those have mvendorid set to
zero.

Regards,
Karsten
-- 
Gem. Par. 28 Abs. 4 Bundesdatenschutzgesetz widerspreche ich der Nutzung
sowie der Weitergabe meiner personenbezogenen Daten fÃr Zwecke der
Werbung sowie der Markt- oder Meinungsforschung.