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

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

[Vincent Chen]

  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.

Vincent Chen (2):
  RISC-V: An infrastructure to add vendor-specific code.
  RISC-V: make dma_map_ops work without cache coherent agent

 arch/riscv/Kconfig                              |   49 +++++
 arch/riscv/Makefile                             |    6 +
 arch/riscv/include/asm/sbi.h                    |    6 +
 arch/riscv/include/asm/vendor-hook.h            |   13 ++
 arch/riscv/kernel/cpufeature.c                  |    5 +
 arch/riscv/kernel/setup.c                       |    6 +-
 arch/riscv/vendor-nds/Kconfig                   |   29 +++
 arch/riscv/vendor-nds/Makefile                  |    1 +
 arch/riscv/vendor-nds/cache.c                   |   83 ++++++++
 arch/riscv/vendor-nds/include/asm/csr.h         |   32 +++
 arch/riscv/vendor-nds/include/asm/dma-mapping.h |   24 +++
 arch/riscv/vendor-nds/include/asm/proc.h        |   17 ++
 arch/riscv/vendor-nds/include/asm/sbi.h         |   17 ++
 arch/riscv/vendor-nds/include/asm/vendor-hook.h |    8 +
 arch/riscv/vendor-nds/noncoherent_dma.c         |  254 +++++++++++++++++++++++
 arch/riscv/vendor-nds/setup.c                   |   16 ++
 16 files changed, 565 insertions(+), 1 deletions(-)
 create mode 100644 arch/riscv/include/asm/vendor-hook.h
 create mode 100644 arch/riscv/vendor-nds/Kconfig
 create mode 100644 arch/riscv/vendor-nds/Makefile
 create mode 100644 arch/riscv/vendor-nds/cache.c
 create mode 100644 arch/riscv/vendor-nds/include/asm/csr.h
 create mode 100644 arch/riscv/vendor-nds/include/asm/dma-mapping.h
 create mode 100644 arch/riscv/vendor-nds/include/asm/proc.h
 create mode 100644 arch/riscv/vendor-nds/include/asm/sbi.h
 create mode 100644 arch/riscv/vendor-nds/include/asm/vendor-hook.h
 create mode 100644 arch/riscv/vendor-nds/noncoherent_dma.c
 create mode 100644 arch/riscv/vendor-nds/setup.c

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

[Vincent Chen]

  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.

Vincent Chen (2):
  RISC-V: An infrastructure to add vendor-specific code.
  RISC-V: make dma_map_ops work without cache coherent agent

 arch/riscv/Kconfig                              |   49 +++++
 arch/riscv/Makefile                             |    6 +
 arch/riscv/include/asm/sbi.h                    |    6 +
 arch/riscv/include/asm/vendor-hook.h            |   13 ++
 arch/riscv/kernel/cpufeature.c                  |    5 +
 arch/riscv/kernel/setup.c                       |    6 +-
 arch/riscv/vendor-nds/Kconfig                   |   29 +++
 arch/riscv/vendor-nds/Makefile                  |    1 +
 arch/riscv/vendor-nds/cache.c                   |   83 ++++++++
 arch/riscv/vendor-nds/include/asm/csr.h         |   32 +++
 arch/riscv/vendor-nds/include/asm/dma-mapping.h |   24 +++
 arch/riscv/vendor-nds/include/asm/proc.h        |   17 ++
 arch/riscv/vendor-nds/include/asm/sbi.h         |   17 ++
 arch/riscv/vendor-nds/include/asm/vendor-hook.h |    8 +
 arch/riscv/vendor-nds/noncoherent_dma.c         |  254 +++++++++++++++++++++++
 arch/riscv/vendor-nds/setup.c                   |   16 ++
 16 files changed, 565 insertions(+), 1 deletions(-)
 create mode 100644 arch/riscv/include/asm/vendor-hook.h
 create mode 100644 arch/riscv/vendor-nds/Kconfig
 create mode 100644 arch/riscv/vendor-nds/Makefile
 create mode 100644 arch/riscv/vendor-nds/cache.c
 create mode 100644 arch/riscv/vendor-nds/include/asm/csr.h
 create mode 100644 arch/riscv/vendor-nds/include/asm/dma-mapping.h
 create mode 100644 arch/riscv/vendor-nds/include/asm/proc.h
 create mode 100644 arch/riscv/vendor-nds/include/asm/sbi.h
 create mode 100644 arch/riscv/vendor-nds/include/asm/vendor-hook.h
 create mode 100644 arch/riscv/vendor-nds/noncoherent_dma.c
 create mode 100644 arch/riscv/vendor-nds/setup.c


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC 1/2] RISC-V: An infrastructure to add vendor-specific code.

[Vincent Chen]

  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
commit, we propose an infrastructure that vendor can easily hook their
specific features into kernel. This infrastructure is developed based on
the following 3 ideas:

1. Keep code readable
     All vendors use the same condition to control the flow of program in
   RISC-V generic port instead of vendor-defined kernel configuration.

2. Vendor can easily add features without considering other vendors.

3. Even if the CPU vendor in kernel configuration does not match the CPU
   vendor on platform, the kernel still can work.

This infrastructure can be divided into 3 parts as below.

A. Folder structure
     We consider each vendor as a sub-architecture of RISC-V. Hence, we
   plane to centralize vendor-dependent code, including source file and
   header file, into respective folder which is located at
   arch/riscv/<vendor>. Like the general architecture, the header files
   are located in arch/riscv/<vendor>/include/asm and
   arch/riscv/<vendor>/include/uapi.

B. Include vendor's file at compile time
   B.1 Vendor's Kconfig
         Vendor can define the needed kernel configuration here. For
       this infrastructure to work properly, user needs to define the
       macro CONFIG_VENDOR_ID and CONFIG_VENDOR_FOLDER_NAME as JEDEC
       manufacturer ID and the name of the vendor folder in this Kconfig.
   B.2 Vendor's source file
         The folder name of the selected vendor is recorded in macro
       CONFIG_VENDOR_FOLDER_NAME. To ensure that compiler only compiles the
       code of the selected vendor, it is only include the Makefile placed
       in CONFIG_VENDOR_FOLDER_NAME.
   B.3 Vendor's header file
         The searching order of vendor's folder is prior to riscv generic
       folder. This design enables vendors to replace the riscv
       generic header file with self-defined header file. Nevertheless,
       vendor can still include the contents of riscv generic header file
       by "include_next".
         The vendor-hook.h is the only exception that cannot be replaced
       because other vendors define their specific function as a dummy
       function here for linkage.

C. Check compatibility in run time
     To avoid kernel panic by incompatible CPU, the compatibility check
   is needed before entering vendor-specific function. We think the
   compatibility check here only needs to ensure that the vendor can safely
   call self-checking mechanism. Hence, the macro
   CHECK_VENDOR_XEXT_ISA_COMPATIBLE which is used to check compatibility
   only compares the vendor's JEDEC manufacturer ID with the content of csr
   $mvendorid.

Signed-off-by: Vincent Chen <vincentc@andestech.com>
---
 arch/riscv/Kconfig                              |   49 +++++++++++++++++++++++
 arch/riscv/Makefile                             |    6 +++
 arch/riscv/include/asm/sbi.h                    |    5 ++
 arch/riscv/include/asm/vendor-hook.h            |   13 ++++++
 arch/riscv/kernel/cpufeature.c                  |    5 ++
 arch/riscv/kernel/setup.c                       |    6 ++-
 arch/riscv/vendor-nds/Kconfig                   |   29 +++++++++++++
 arch/riscv/vendor-nds/Makefile                  |    1 +
 arch/riscv/vendor-nds/include/asm/vendor-hook.h |    8 ++++
 arch/riscv/vendor-nds/setup.c                   |    9 ++++
 10 files changed, 130 insertions(+), 1 deletions(-)
 create mode 100644 arch/riscv/include/asm/vendor-hook.h
 create mode 100644 arch/riscv/vendor-nds/Kconfig
 create mode 100644 arch/riscv/vendor-nds/Makefile
 create mode 100644 arch/riscv/vendor-nds/include/asm/vendor-hook.h
 create mode 100644 arch/riscv/vendor-nds/setup.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index b008b34..a54a115 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -209,6 +209,55 @@ config RISCV_BASE_PMU
 
 endmenu
 
+choice
+	prompt "non-standard extension ISA"
+	default PURE_STANDARD_EXTENSION_ISA
+	help
+	  Select a CPU IP vendor used on the target platform. The features
+          of the extension ISA from selected vendor will be enabled.
+
+	  If you just need RISCV standard ISA, please select 'none'
+
+config RISCV_NDS
+	bool "AndeStar RISC-V ISA support"
+	select SUPPORT_X_EXTENSION_ISA
+	help
+	  Say Y here if you plan to run kernel on the AndeStar RISC-V CPU
+	  and use some of the specific features provided by the AndeStar RISC-V
+	  CPU such as cctl and non-cache coherent agent support.
+
+	  If the CPU used by the target platform is an AndeStar RISC-V CPU but you
+	  don't know what to do here, say Y.
+
+config PURE_STANDARD_EXTENSION_ISA
+	bool "none"
+
+endchoice
+config SUPPORT_X_EXTENSION_ISA
+	bool
+	depends on !PURE_STANDARD_EXTENSION_ISA
+	default n
+	help
+          The statement 'Y' means vendor-provided extension ISA is enabled in
+	  kerenl.
+
+config VENDOR_FOLDER_NAME
+	string
+	depends on SUPPORT_X_EXTENSION_ISA
+        help
+	  The name of the vendor folder in ./arch/riscv. Makefile will use
+          CONFIG_VENDOR_FOLDER_NAME to search the required header file.
+
+config VENDOR_ID
+	hex
+	depends on SUPPORT_X_EXTENSION_ISA
+        help
+	  The JEDEC manufacturer ID of CPU IP vendor. This will be used to
+	  check the compatibility between the kernel and the platform by
+	  comparing $mvendorid and CONFIG_VENDOR_ID.
+
+source "arch/riscv/vendor-nds/Kconfig"
+
 endmenu
 
 menu "Kernel type"
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 61ec424..8616aa9 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -80,6 +80,12 @@ head-y := arch/riscv/kernel/head.o
 
 core-y += arch/riscv/kernel/ arch/riscv/mm/
 
+ifeq ($(CONFIG_SUPPORT_X_EXTENSION_ISA),y)
+riscv-vendor-name := $(CONFIG_VENDOR_FOLDER_NAME:"%"=%)
+LINUXINCLUDE := -I$(srctree)/arch/riscv/$(riscv-vendor-name)/include $(LINUXINCLUDE)
+core-$(CONFIG_RISCV_NDS) += arch/riscv/$(riscv-vendor-name)/
+endif
+
 libs-y += arch/riscv/lib/
 
 all: vmlinux
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index b6bb10b..5e1abf6 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -25,6 +25,7 @@
 #define SBI_REMOTE_SFENCE_VMA 6
 #define SBI_REMOTE_SFENCE_VMA_ASID 7
 #define SBI_SHUTDOWN 8
+#define SBI_GET_MVENDOR_ID 10
 
 #define SBI_CALL(which, arg0, arg1, arg2) ({			\
 	register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0);	\
@@ -97,4 +98,8 @@ static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask,
 	SBI_CALL_1(SBI_REMOTE_SFENCE_VMA_ASID, hart_mask);
 }
 
+static inline unsigned long sbi_get_mvendorid(void)
+{
+	return SBI_CALL_0(SBI_GET_MVENDOR_ID);
+}
 #endif
diff --git a/arch/riscv/include/asm/vendor-hook.h b/arch/riscv/include/asm/vendor-hook.h
new file mode 100644
index 0000000..fbf0f1f
--- /dev/null
+++ b/arch/riscv/include/asm/vendor-hook.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Andes Technology Corporation */
+#include <linux/string.h>
+
+#define CHECK_VENDOR_XEXT_ISA_COMPATIBLE			\
+	((IS_ENABLED(CONFIG_SUPPORT_X_EXTENSION_ISA)) &&	\
+	(mvendorid == CONFIG_VENDOR_ID))
+
+extern unsigned long mvendorid;
+
+#ifndef SETUP_VENDOR_EXTENSION
+#define setup_vendor_extension(x) do {} while (0)
+#endif
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 17011a8..2794cae 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -20,8 +20,10 @@
 #include <linux/of.h>
 #include <asm/processor.h>
 #include <asm/hwcap.h>
+#include <asm/sbi.h>
 
 unsigned long elf_hwcap __read_mostly;
+unsigned long mvendorid;
 
 void riscv_fill_hwcap(void)
 {
@@ -58,4 +60,7 @@ void riscv_fill_hwcap(void)
 		elf_hwcap |= isa2hwcap[(unsigned char)(isa[i])];
 
 	pr_info("elf_hwcap is 0x%lx", elf_hwcap);
+
+	mvendorid = sbi_get_mvendorid();
+
 }
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index b2d26d9..2dcfb21 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -38,6 +38,7 @@
 #include <asm/sbi.h>
 #include <asm/tlbflush.h>
 #include <asm/thread_info.h>
+#include <asm/vendor-hook.h>
 
 #ifdef CONFIG_EARLY_PRINTK
 static void sbi_console_write(struct console *co, const char *buf,
@@ -228,7 +229,6 @@ void __init setup_arch(char **cmdline_p)
 	paging_init();
 	unflatten_device_tree();
 	swiotlb_init(1);
-
 #ifdef CONFIG_SMP
 	setup_smp();
 #endif
@@ -238,5 +238,9 @@ void __init setup_arch(char **cmdline_p)
 #endif
 
 	riscv_fill_hwcap();
+
+	if (CHECK_VENDOR_XEXT_ISA_COMPATIBLE)
+		setup_vendor_extension();
+
 }
 
diff --git a/arch/riscv/vendor-nds/Kconfig b/arch/riscv/vendor-nds/Kconfig
new file mode 100644
index 0000000..19fd485
--- /dev/null
+++ b/arch/riscv/vendor-nds/Kconfig
@@ -0,0 +1,29 @@
+#
+# For a description of the syntax of this configuration file,
+# see Documentation/kbuild/kconfig-language.txt.
+#
+config VENDOR_FOLDER_NAME
+	string
+	depends on RISCV_NDS
+        default 'vendor-nds'
+
+config VENDOR_ID
+	hex
+	depends on RISCV_NDS
+        default 0x31e
+
+config DMA_NONCOHERENT_OPS
+	bool "support DMA consistent memory without cache coherency agent"
+	def_bool y
+	depends on RISCV_NDS
+	select ARCH_HAS_SYNC_DMA_FOR_CPU
+	select ARCH_HAS_SYNC_DMA_FOR_DEVICE
+	help
+	  Say Y here if the cache coherent agent is unsupported.
+	  For some AndeStar RISC-V CPU, the MSB of physical address is used to
+	  disable the cachebility of this address if the cache coherent agent
+	  is unspported. To make this feature work, DMA_NONCOHERENT_OPS shall be
+	  Y to enable needed address translation.
+
+	  Say N here If cache coherent agent is supported.
+
diff --git a/arch/riscv/vendor-nds/Makefile b/arch/riscv/vendor-nds/Makefile
new file mode 100644
index 0000000..392275e
--- /dev/null
+++ b/arch/riscv/vendor-nds/Makefile
@@ -0,0 +1 @@
+obj-y += cache.o noncoherent_dma.o setup.o
diff --git a/arch/riscv/vendor-nds/include/asm/vendor-hook.h b/arch/riscv/vendor-nds/include/asm/vendor-hook.h
new file mode 100644
index 0000000..2876c24
--- /dev/null
+++ b/arch/riscv/vendor-nds/include/asm/vendor-hook.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Andes Technology Corporation */
+#ifdef CONFIG_SUPPORT_X_EXTENSION_ISA
+#define SETUP_VENDOR_EXTENSION
+extern void setup_vendor_extension(void);
+#endif
+
+#include_next <asm/vendor-hook.h>
diff --git a/arch/riscv/vendor-nds/setup.c b/arch/riscv/vendor-nds/setup.c
new file mode 100644
index 0000000..5ceed1b
--- /dev/null
+++ b/arch/riscv/vendor-nds/setup.c
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Andes Technology Corporation
+#include <linux/init.h>
+#include <asm/vendor-hook.h>
+bool riscv_nds_compat_platform;
+void __init setup_vendor_extension(void)
+{
+	riscv_nds_compat_platform = true;
+}
-- 
1.7.1

[RFC 1/2] RISC-V: An infrastructure to add vendor-specific code.

[Vincent Chen]

  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
commit, we propose an infrastructure that vendor can easily hook their
specific features into kernel. This infrastructure is developed based on
the following 3 ideas:

1. Keep code readable
     All vendors use the same condition to control the flow of program in
   RISC-V generic port instead of vendor-defined kernel configuration.

2. Vendor can easily add features without considering other vendors.

3. Even if the CPU vendor in kernel configuration does not match the CPU
   vendor on platform, the kernel still can work.

This infrastructure can be divided into 3 parts as below.

A. Folder structure
     We consider each vendor as a sub-architecture of RISC-V. Hence, we
   plane to centralize vendor-dependent code, including source file and
   header file, into respective folder which is located at
   arch/riscv/<vendor>. Like the general architecture, the header files
   are located in arch/riscv/<vendor>/include/asm and
   arch/riscv/<vendor>/include/uapi.

B. Include vendor's file at compile time
   B.1 Vendor's Kconfig
         Vendor can define the needed kernel configuration here. For
       this infrastructure to work properly, user needs to define the
       macro CONFIG_VENDOR_ID and CONFIG_VENDOR_FOLDER_NAME as JEDEC
       manufacturer ID and the name of the vendor folder in this Kconfig.
   B.2 Vendor's source file
         The folder name of the selected vendor is recorded in macro
       CONFIG_VENDOR_FOLDER_NAME. To ensure that compiler only compiles the
       code of the selected vendor, it is only include the Makefile placed
       in CONFIG_VENDOR_FOLDER_NAME.
   B.3 Vendor's header file
         The searching order of vendor's folder is prior to riscv generic
       folder. This design enables vendors to replace the riscv
       generic header file with self-defined header file. Nevertheless,
       vendor can still include the contents of riscv generic header file
       by "include_next".
         The vendor-hook.h is the only exception that cannot be replaced
       because other vendors define their specific function as a dummy
       function here for linkage.

C. Check compatibility in run time
     To avoid kernel panic by incompatible CPU, the compatibility check
   is needed before entering vendor-specific function. We think the
   compatibility check here only needs to ensure that the vendor can safely
   call self-checking mechanism. Hence, the macro
   CHECK_VENDOR_XEXT_ISA_COMPATIBLE which is used to check compatibility
   only compares the vendor's JEDEC manufacturer ID with the content of csr
   $mvendorid.

Signed-off-by: Vincent Chen <vincentc@andestech.com>
---
 arch/riscv/Kconfig                              |   49 +++++++++++++++++++++++
 arch/riscv/Makefile                             |    6 +++
 arch/riscv/include/asm/sbi.h                    |    5 ++
 arch/riscv/include/asm/vendor-hook.h            |   13 ++++++
 arch/riscv/kernel/cpufeature.c                  |    5 ++
 arch/riscv/kernel/setup.c                       |    6 ++-
 arch/riscv/vendor-nds/Kconfig                   |   29 +++++++++++++
 arch/riscv/vendor-nds/Makefile                  |    1 +
 arch/riscv/vendor-nds/include/asm/vendor-hook.h |    8 ++++
 arch/riscv/vendor-nds/setup.c                   |    9 ++++
 10 files changed, 130 insertions(+), 1 deletions(-)
 create mode 100644 arch/riscv/include/asm/vendor-hook.h
 create mode 100644 arch/riscv/vendor-nds/Kconfig
 create mode 100644 arch/riscv/vendor-nds/Makefile
 create mode 100644 arch/riscv/vendor-nds/include/asm/vendor-hook.h
 create mode 100644 arch/riscv/vendor-nds/setup.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index b008b34..a54a115 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -209,6 +209,55 @@ config RISCV_BASE_PMU
 
 endmenu
 
+choice
+	prompt "non-standard extension ISA"
+	default PURE_STANDARD_EXTENSION_ISA
+	help
+	  Select a CPU IP vendor used on the target platform. The features
+          of the extension ISA from selected vendor will be enabled.
+
+	  If you just need RISCV standard ISA, please select 'none'
+
+config RISCV_NDS
+	bool "AndeStar RISC-V ISA support"
+	select SUPPORT_X_EXTENSION_ISA
+	help
+	  Say Y here if you plan to run kernel on the AndeStar RISC-V CPU
+	  and use some of the specific features provided by the AndeStar RISC-V
+	  CPU such as cctl and non-cache coherent agent support.
+
+	  If the CPU used by the target platform is an AndeStar RISC-V CPU but you
+	  don't know what to do here, say Y.
+
+config PURE_STANDARD_EXTENSION_ISA
+	bool "none"
+
+endchoice
+config SUPPORT_X_EXTENSION_ISA
+	bool
+	depends on !PURE_STANDARD_EXTENSION_ISA
+	default n
+	help
+          The statement 'Y' means vendor-provided extension ISA is enabled in
+	  kerenl.
+
+config VENDOR_FOLDER_NAME
+	string
+	depends on SUPPORT_X_EXTENSION_ISA
+        help
+	  The name of the vendor folder in ./arch/riscv. Makefile will use
+          CONFIG_VENDOR_FOLDER_NAME to search the required header file.
+
+config VENDOR_ID
+	hex
+	depends on SUPPORT_X_EXTENSION_ISA
+        help
+	  The JEDEC manufacturer ID of CPU IP vendor. This will be used to
+	  check the compatibility between the kernel and the platform by
+	  comparing $mvendorid and CONFIG_VENDOR_ID.
+
+source "arch/riscv/vendor-nds/Kconfig"
+
 endmenu
 
 menu "Kernel type"
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 61ec424..8616aa9 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -80,6 +80,12 @@ head-y := arch/riscv/kernel/head.o
 
 core-y += arch/riscv/kernel/ arch/riscv/mm/
 
+ifeq ($(CONFIG_SUPPORT_X_EXTENSION_ISA),y)
+riscv-vendor-name := $(CONFIG_VENDOR_FOLDER_NAME:"%"=%)
+LINUXINCLUDE := -I$(srctree)/arch/riscv/$(riscv-vendor-name)/include $(LINUXINCLUDE)
+core-$(CONFIG_RISCV_NDS) += arch/riscv/$(riscv-vendor-name)/
+endif
+
 libs-y += arch/riscv/lib/
 
 all: vmlinux
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index b6bb10b..5e1abf6 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -25,6 +25,7 @@
 #define SBI_REMOTE_SFENCE_VMA 6
 #define SBI_REMOTE_SFENCE_VMA_ASID 7
 #define SBI_SHUTDOWN 8
+#define SBI_GET_MVENDOR_ID 10
 
 #define SBI_CALL(which, arg0, arg1, arg2) ({			\
 	register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0);	\
@@ -97,4 +98,8 @@ static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask,
 	SBI_CALL_1(SBI_REMOTE_SFENCE_VMA_ASID, hart_mask);
 }
 
+static inline unsigned long sbi_get_mvendorid(void)
+{
+	return SBI_CALL_0(SBI_GET_MVENDOR_ID);
+}
 #endif
diff --git a/arch/riscv/include/asm/vendor-hook.h b/arch/riscv/include/asm/vendor-hook.h
new file mode 100644
index 0000000..fbf0f1f
--- /dev/null
+++ b/arch/riscv/include/asm/vendor-hook.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Andes Technology Corporation */
+#include <linux/string.h>
+
+#define CHECK_VENDOR_XEXT_ISA_COMPATIBLE			\
+	((IS_ENABLED(CONFIG_SUPPORT_X_EXTENSION_ISA)) &&	\
+	(mvendorid == CONFIG_VENDOR_ID))
+
+extern unsigned long mvendorid;
+
+#ifndef SETUP_VENDOR_EXTENSION
+#define setup_vendor_extension(x) do {} while (0)
+#endif
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 17011a8..2794cae 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -20,8 +20,10 @@
 #include <linux/of.h>
 #include <asm/processor.h>
 #include <asm/hwcap.h>
+#include <asm/sbi.h>
 
 unsigned long elf_hwcap __read_mostly;
+unsigned long mvendorid;
 
 void riscv_fill_hwcap(void)
 {
@@ -58,4 +60,7 @@ void riscv_fill_hwcap(void)
 		elf_hwcap |= isa2hwcap[(unsigned char)(isa[i])];
 
 	pr_info("elf_hwcap is 0x%lx", elf_hwcap);
+
+	mvendorid = sbi_get_mvendorid();
+
 }
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index b2d26d9..2dcfb21 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -38,6 +38,7 @@
 #include <asm/sbi.h>
 #include <asm/tlbflush.h>
 #include <asm/thread_info.h>
+#include <asm/vendor-hook.h>
 
 #ifdef CONFIG_EARLY_PRINTK
 static void sbi_console_write(struct console *co, const char *buf,
@@ -228,7 +229,6 @@ void __init setup_arch(char **cmdline_p)
 	paging_init();
 	unflatten_device_tree();
 	swiotlb_init(1);
-
 #ifdef CONFIG_SMP
 	setup_smp();
 #endif
@@ -238,5 +238,9 @@ void __init setup_arch(char **cmdline_p)
 #endif
 
 	riscv_fill_hwcap();
+
+	if (CHECK_VENDOR_XEXT_ISA_COMPATIBLE)
+		setup_vendor_extension();
+
 }
 
diff --git a/arch/riscv/vendor-nds/Kconfig b/arch/riscv/vendor-nds/Kconfig
new file mode 100644
index 0000000..19fd485
--- /dev/null
+++ b/arch/riscv/vendor-nds/Kconfig
@@ -0,0 +1,29 @@
+#
+# For a description of the syntax of this configuration file,
+# see Documentation/kbuild/kconfig-language.txt.
+#
+config VENDOR_FOLDER_NAME
+	string
+	depends on RISCV_NDS
+        default 'vendor-nds'
+
+config VENDOR_ID
+	hex
+	depends on RISCV_NDS
+        default 0x31e
+
+config DMA_NONCOHERENT_OPS
+	bool "support DMA consistent memory without cache coherency agent"
+	def_bool y
+	depends on RISCV_NDS
+	select ARCH_HAS_SYNC_DMA_FOR_CPU
+	select ARCH_HAS_SYNC_DMA_FOR_DEVICE
+	help
+	  Say Y here if the cache coherent agent is unsupported.
+	  For some AndeStar RISC-V CPU, the MSB of physical address is used to
+	  disable the cachebility of this address if the cache coherent agent
+	  is unspported. To make this feature work, DMA_NONCOHERENT_OPS shall be
+	  Y to enable needed address translation.
+
+	  Say N here If cache coherent agent is supported.
+
diff --git a/arch/riscv/vendor-nds/Makefile b/arch/riscv/vendor-nds/Makefile
new file mode 100644
index 0000000..392275e
--- /dev/null
+++ b/arch/riscv/vendor-nds/Makefile
@@ -0,0 +1 @@
+obj-y += cache.o noncoherent_dma.o setup.o
diff --git a/arch/riscv/vendor-nds/include/asm/vendor-hook.h b/arch/riscv/vendor-nds/include/asm/vendor-hook.h
new file mode 100644
index 0000000..2876c24
--- /dev/null
+++ b/arch/riscv/vendor-nds/include/asm/vendor-hook.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Andes Technology Corporation */
+#ifdef CONFIG_SUPPORT_X_EXTENSION_ISA
+#define SETUP_VENDOR_EXTENSION
+extern void setup_vendor_extension(void);
+#endif
+
+#include_next <asm/vendor-hook.h>
diff --git a/arch/riscv/vendor-nds/setup.c b/arch/riscv/vendor-nds/setup.c
new file mode 100644
index 0000000..5ceed1b
--- /dev/null
+++ b/arch/riscv/vendor-nds/setup.c
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Andes Technology Corporation
+#include <linux/init.h>
+#include <asm/vendor-hook.h>
+bool riscv_nds_compat_platform;
+void __init setup_vendor_extension(void)
+{
+	riscv_nds_compat_platform = true;
+}
-- 
1.7.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC 2/2] RISC-V: make dma_map_ops work without cache coherent agent

[Vincent Chen]

    Currently, DMA operations in RISC-V ports assume the cache coherent
agent is supported. In other words, the functions in struct dma_map_ops
cannot work if the cache coherent agent is unsupported. In RISCV-NDS
extension ISA, the AndeStart RISC-V CPU provides a solution to overcome
this limitation.

  The AndeStart RISC-V CPU provides following two features so that the
functions of struct dma_map_ops can work without cache coherent agent.

  1. Non-cacheable memory
     In standard RISC-V, the memory cacheability is usually
  determined by the platform and there is no a particular scheme to
  adjust it by software at runtime. Hence, platform needs to allocate a
  fixed non-cacheable memory region for OS. This may cause some
  inconvenience and waste to the user. The feature provided by the
  AndeStar RISC-V CPU is that when the cache coherent agent is not
  supported, the user can arbitrarily disable the cacheability of a
  particular memory region at runtime. This feature separates the whole
  PA region is into 2 parts. The PA in higher part is the non-cacheable
  alias to the lower part. Hence, we can think the cacheability of
  targeted PA can be disabled by setting the MSB of physical address to 1.
  Based on this feature, user just requires a general memory and then
  binds the aliasing of the deriving PA in higher part to a new VA by
  io_reamp.

  2. Synchronize specific content between memory and cache
     The RISC-V generic ISA has instructions that allow the user to
  synchronize between cache and memory. However, the synchronized region
  can only be the entire cache. Therefore, the RISC-V generic ISA cannot
  handle the page synchronization required by the struct dma_map_ops.
  The extension ISA provided by the AndeStar RISC-V CPU has the ability
  to synchronize the contents of specific region between memory and
  cache.

  Due to feature requirements, we need to use custom SBI call to
derive the MSB of physical address from bbl. However, there is no
reserved SBI call for vendor in the current specification. Hence we use
a temporary method to send the vendor-defined SBI call in this commit.
We glad to change the implementation when determining the SBI usage for
vendor.

Signed-off-by: Vincent Chen <vincentc@andestech.com>
---
 arch/riscv/include/asm/sbi.h                    |    1 +
 arch/riscv/vendor-nds/cache.c                   |   83 ++++++++
 arch/riscv/vendor-nds/include/asm/csr.h         |   32 +++
 arch/riscv/vendor-nds/include/asm/dma-mapping.h |   24 +++
 arch/riscv/vendor-nds/include/asm/proc.h        |   17 ++
 arch/riscv/vendor-nds/include/asm/sbi.h         |   17 ++
 arch/riscv/vendor-nds/noncoherent_dma.c         |  254 +++++++++++++++++++++++
 arch/riscv/vendor-nds/setup.c                   |    7 +
 8 files changed, 435 insertions(+), 0 deletions(-)
 create mode 100644 arch/riscv/vendor-nds/cache.c
 create mode 100644 arch/riscv/vendor-nds/include/asm/csr.h
 create mode 100644 arch/riscv/vendor-nds/include/asm/dma-mapping.h
 create mode 100644 arch/riscv/vendor-nds/include/asm/proc.h
 create mode 100644 arch/riscv/vendor-nds/include/asm/sbi.h
 create mode 100644 arch/riscv/vendor-nds/noncoherent_dma.c

diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 5e1abf6..731fc38 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -26,6 +26,7 @@
 #define SBI_REMOTE_SFENCE_VMA_ASID 7
 #define SBI_SHUTDOWN 8
 #define SBI_GET_MVENDOR_ID 10
+#define SBI_XEXT_ARCH_RESERVED 0x80000000
 
 #define SBI_CALL(which, arg0, arg1, arg2) ({			\
 	register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0);	\
diff --git a/arch/riscv/vendor-nds/cache.c b/arch/riscv/vendor-nds/cache.c
new file mode 100644
index 0000000..d9754ac
--- /dev/null
+++ b/arch/riscv/vendor-nds/cache.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Andes Technology Corporation
+#include <linux/irqflags.h>
+#include <linux/module.h>
+#include <linux/cpu.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/cacheinfo.h>
+#include <linux/sizes.h>
+#include <asm/csr.h>
+#include <asm/proc.h>
+
+
+DEFINE_PER_CPU(struct riscv_nds_cache_info, cpu_cache_info) = {
+	.init_done = 0,
+	.dcache_line_size = SZ_32
+};
+static void fill_cpu_cache_info(struct riscv_nds_cache_info *cpu_ci)
+{
+	struct cpu_cacheinfo *this_cpu_ci =
+			get_cpu_cacheinfo(smp_processor_id());
+	struct cacheinfo *this_leaf = this_cpu_ci->info_list;
+	unsigned int i = 0;
+
+	for (; i < this_cpu_ci->num_leaves ; i++, this_leaf++) {
+		if (this_leaf->type == CACHE_TYPE_DATA)
+			cpu_ci->dcache_line_size = this_leaf->coherency_line_size;
+	}
+	cpu_ci->init_done = true;
+}
+
+
+inline int get_cache_line_size(void)
+{
+	struct riscv_nds_cache_info *cpu_ci =
+		&per_cpu(cpu_cache_info, smp_processor_id());
+
+	if (unlikely(cpu_ci->init_done == false))
+		fill_cpu_cache_info(cpu_ci);
+	return cpu_ci->dcache_line_size;
+}
+void cpu_dcache_wb_range(unsigned long start, unsigned long end)
+{
+	int line_size = get_cache_line_size();
+
+	while (end > start) {
+		csr_write(ucctlbeginaddr, start);
+		csr_write(ucctlcommand, CCTL_L1D_VA_WB);
+		start += line_size;
+	}
+}
+
+void cpu_dcache_inval_range(unsigned long start, unsigned long end)
+{
+	int line_size = get_cache_line_size();
+
+	while (end > start) {
+		csr_write(ucctlbeginaddr, start);
+		csr_write(ucctlcommand, CCTL_L1D_VA_INVAL);
+		start += line_size;
+	}
+}
+void cpu_dma_inval_range(unsigned long start, unsigned long end)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	cpu_dcache_inval_range(start, end);
+	local_irq_restore(flags);
+
+}
+EXPORT_SYMBOL(cpu_dma_inval_range);
+
+void cpu_dma_wb_range(unsigned long start, unsigned long end)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	cpu_dcache_wb_range(start, end);
+	local_irq_restore(flags);
+}
+EXPORT_SYMBOL(cpu_dma_wb_range);
+
diff --git a/arch/riscv/vendor-nds/include/asm/csr.h b/arch/riscv/vendor-nds/include/asm/csr.h
new file mode 100644
index 0000000..6027e6d
--- /dev/null
+++ b/arch/riscv/vendor-nds/include/asm/csr.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Andes Technology Corporation */
+#include_next <asm/csr.h>
+
+/* mdcm_cfg: Data Cache/Memory Configuration Register */
+#define MDCM_CFG_DEST_OFFSET		0
+#define MDCM_CFG_DWAY_OFFSET		3
+#define MDCM_CFG_DSZ_OFFSET		6
+#define MDCM_CFG_DLCK_OFFSET		9
+#define MDCM_CFG_DC_ECC_OFFSET		10
+#define MDCM_CFG_DLMB_OFFSET		12
+#define MDCM_CFG_DLMSZ_OFFSET		15
+#define MDCM_CFG_ULM_2BANK_OFFSET	20
+#define MDCM_CFG_DLM_ECC_OFFSET		21
+
+
+#define MDCM_CFG_DEST_MASK	(0x7  << MDCM_CFG_DEST_OFFSET)
+#define MDCM_CFG_DWAY_MASK	(0x7  << MDCM_CFG_DWAY_OFFSET)
+#define MDCM_CFG_DSZ_MASK	(0x7  << MDCM_CFG_DSZ_OFFSET)
+#define MDCM_CFG_DLCK_MASK	(0x1  << MDCM_CFG_DLCK_OFFSET)
+#define MDCM_CFG_DC_ECC_MASK	(0x3  << MDCM_CFG_DC_ECC_OFFSET)
+#define MDCM_CFG_DLMB_MASK	(0x7  << MDCM_CFG_DLMB_OFFSET)
+#define MDCM_CFG_DLMSZ_MASK	(0x1f << MDCM_CFG_DLMSZ_OFFSET)
+#define MDCM_CFG_ULM_2BANK_MASK	(0x1  << MDCM_CFG_ULM_2BANK_OFFSET)
+#define MDCM_CFG_DLM_ECC_MASK	(0x3  << MDCM_CFG_DLM_ECC_OFFSET)
+
+
+/* ucctlcommand */
+/* D-cache operation */
+#define CCTL_L1D_VA_INVAL	0
+#define CCTL_L1D_VA_WB		1
+#define CCTL_L1D_VA_WBINVAL	2
diff --git a/arch/riscv/vendor-nds/include/asm/dma-mapping.h b/arch/riscv/vendor-nds/include/asm/dma-mapping.h
new file mode 100644
index 0000000..30b4183
--- /dev/null
+++ b/arch/riscv/vendor-nds/include/asm/dma-mapping.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Andes Technology Corporation */
+#ifndef _RISCV_ASM_DMA_MAPPING_H
+#define _RISCV_ASM_DMA_MAPPING_H 1
+
+
+#ifdef CONFIG_SWIOTLB
+#include <linux/swiotlb.h>
+static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
+{
+#ifdef CONFIG_DMA_NONCOHERENT_OPS
+	extern const struct dma_map_ops swiotlb_noncoh_dma_ops;
+	extern bool riscv_nds_compat_platform;
+
+	if (riscv_nds_compat_platform)
+		return &swiotlb_noncoh_dma_ops;
+#endif
+	return &swiotlb_dma_ops;
+}
+#else
+#include <asm-generic/dma-mapping.h>
+#endif /* CONFIG_SWIOTLB */
+
+#endif /* _RISCV_ASM_DMA_MAPPING_H */
diff --git a/arch/riscv/vendor-nds/include/asm/proc.h b/arch/riscv/vendor-nds/include/asm/proc.h
new file mode 100644
index 0000000..a2684f4
--- /dev/null
+++ b/arch/riscv/vendor-nds/include/asm/proc.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Andes Technology Corporation */
+#include <asm/io.h>
+#include <asm/page.h>
+
+void cpu_dma_inval_range(unsigned long start, unsigned long end);
+
+void cpu_dma_wb_range(unsigned long start, unsigned long end);
+
+/*
+ * When the content of riscv_nds_cache_info has been initilized by function
+ * fill_cpu_cache_info(), member init_done is set to true
+ */
+struct riscv_nds_cache_info {
+	bool init_done;
+	int dcache_line_size;
+};
diff --git a/arch/riscv/vendor-nds/include/asm/sbi.h b/arch/riscv/vendor-nds/include/asm/sbi.h
new file mode 100644
index 0000000..7a002a4
--- /dev/null
+++ b/arch/riscv/vendor-nds/include/asm/sbi.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Andes Technology Corporation */
+#ifndef _AMS_RISCV_NDS_SBI_H
+#define _AMS_RISCV_NDS_SBI_H
+
+#include_next <asm/sbi.h>
+#define SBI_RISCV_NDS_GET_MAX_PA (0x0)
+
+#define SBI_RISCV_NDS_CALL_0(which) \
+	SBI_CALL(SBI_XEXT_ARCH_RESERVED|which, 0, 0, 0)
+
+
+static inline phys_addr_t sbi_call_riscv_nds_get_maxpa(void)
+{
+	return SBI_RISCV_NDS_CALL_0(SBI_RISCV_NDS_GET_MAX_PA);
+}
+#endif
diff --git a/arch/riscv/vendor-nds/noncoherent_dma.c b/arch/riscv/vendor-nds/noncoherent_dma.c
new file mode 100644
index 0000000..470dc90
--- /dev/null
+++ b/arch/riscv/vendor-nds/noncoherent_dma.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Andes Technology Corporation
+#include <linux/gfp.h>
+#include <linux/mm.h>
+#include <linux/swiotlb.h>
+#include <linux/dma-mapping.h>
+#include <linux/dma-direct.h>
+#include <linux/scatterlist.h>
+#include <asm/proc.h>
+
+phys_addr_t pa_msb;
+#define dma_remap(pa, size) ioremap((pa|(pa_msb << PAGE_SHIFT)), size)
+#define dma_unmap(vaddr) iounmap((void __force __iomem *)vaddr)
+
+static void dma_flush_page(struct page *page, size_t size)
+{
+	unsigned long k_d_vaddr;
+	/*
+	 * Invalidate any data that might be lurking in the
+	 * kernel direct-mapped region for device DMA.
+	 */
+	k_d_vaddr = (unsigned long)page_address(page);
+	memset((void *)k_d_vaddr, 0, size);
+	cpu_dma_wb_range(k_d_vaddr, k_d_vaddr + size);
+	cpu_dma_inval_range(k_d_vaddr, k_d_vaddr + size);
+
+}
+
+static inline void cache_op(phys_addr_t paddr, size_t size,
+			    void (*fn)(unsigned long start, unsigned long end))
+{
+	unsigned long start;
+
+	start = (unsigned long)phys_to_virt(paddr);
+	fn(start, start + size);
+}
+
+void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
+			      size_t size, enum dma_data_direction dir)
+{
+	switch (dir) {
+	case DMA_FROM_DEVICE:
+		break;
+	case DMA_TO_DEVICE:
+	case DMA_BIDIRECTIONAL:
+		cache_op(paddr, size, cpu_dma_wb_range);
+		break;
+	default:
+		BUG();
+	}
+}
+
+void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr,
+			   size_t size, enum dma_data_direction dir)
+{
+	switch (dir) {
+	case DMA_TO_DEVICE:
+		break;
+	case DMA_FROM_DEVICE:
+	case DMA_BIDIRECTIONAL:
+		cache_op(paddr, size, cpu_dma_inval_range);
+		break;
+	default:
+		BUG();
+	}
+}
+
+#ifdef CONFIG_32BIT
+void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
+		     gfp_t gfp, unsigned long attrs)
+{
+	void *kvaddr, *coherent_kvaddr;
+
+	size = PAGE_ALIGN(size);
+	kvaddr = dma_direct_alloc(dev, size, handle, gfp, attrs);
+	if (!kvaddr)
+		goto no_mem;
+	coherent_kvaddr = dma_remap(dma_to_phys(dev, *handle), size);
+	if (!coherent_kvaddr)
+		goto no_map;
+
+	dma_flush_page(virt_to_page(kvaddr), size);
+	return coherent_kvaddr;
+no_map:
+	dma_direct_free(dev, size, kvaddr, *handle, attrs);
+no_mem:
+	return NULL;
+}
+
+void arch_dma_free(struct device *dev, size_t size, void *vaddr,
+		   dma_addr_t handle, unsigned long attrs)
+{
+	void *kvaddr = phys_to_virt(dma_to_phys(dev, handle));
+
+	size = PAGE_ALIGN(size);
+	dma_unmap(vaddr);
+	dma_direct_free(dev, size, kvaddr, handle, attrs);
+}
+#else
+void *arch_dma_alloc(struct device *dev, size_t size,
+		     dma_addr_t *handle, gfp_t gfp, unsigned long attrs)
+{
+	void *kvaddr, *coherent_kvaddr;
+
+	size = PAGE_ALIGN(size);
+	kvaddr = swiotlb_alloc(dev, size, handle, gfp, attrs);
+	if (!kvaddr)
+		goto no_mem;
+	coherent_kvaddr = dma_remap(dma_to_phys(dev, *handle), size);
+	if (!coherent_kvaddr)
+		goto no_map;
+
+	dma_flush_page(virt_to_page(kvaddr), size);
+	return coherent_kvaddr;
+no_map:
+	swiotlb_free(dev, size, kvaddr, *handle, attrs);
+no_mem:
+	return NULL;
+}
+
+void arch_dma_free(struct device *dev, size_t size, void *vaddr,
+		   dma_addr_t handle, unsigned long attrs)
+{
+	void *swiotlb_addr = phys_to_virt(dma_to_phys(dev, handle));
+
+	size = PAGE_ALIGN(size);
+	dma_unmap(vaddr);
+	swiotlb_free(dev, size, swiotlb_addr, handle, attrs);
+}
+
+static dma_addr_t dma_riscv_swiotlb_map_page(struct device *dev,
+					     struct page *page,
+					     unsigned long offset, size_t size,
+					     enum dma_data_direction dir,
+					     unsigned long attrs)
+{
+	dma_addr_t dev_addr;
+
+	dev_addr = swiotlb_map_page(dev, page, offset, size, dir, attrs);
+	if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+		arch_sync_dma_for_device(dev, dma_to_phys(dev, dev_addr),
+					 size, dir);
+
+	return dev_addr;
+}
+
+static int dma_riscv_swiotlb_map_sg(struct device *dev,
+				    struct scatterlist *sgl,
+				    int nelems, enum dma_data_direction dir,
+				    unsigned long attrs)
+{
+	struct scatterlist *sg;
+	int i, ret;
+
+	ret = swiotlb_map_sg_attrs(dev, sgl, nelems, dir, attrs);
+	if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+		for_each_sg(sgl, sg, ret, i)
+			arch_sync_dma_for_device(dev,
+					     dma_to_phys(dev, sg->dma_address),
+					     sg->length, dir);
+
+	return ret;
+}
+
+static void dma_riscv_swiotlb_unmap_page(struct device *dev,
+					 dma_addr_t dev_addr, size_t size,
+					 enum dma_data_direction dir,
+					 unsigned long attrs)
+{
+	if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+		arch_sync_dma_for_cpu(dev, dma_to_phys(dev, dev_addr),
+				      size, dir);
+	swiotlb_unmap_page(dev, dev_addr, size, dir, attrs);
+}
+
+static void dma_riscv_swiotlb_unmap_sg(struct device *dev,
+				       struct scatterlist *sgl, int nelems,
+				       enum dma_data_direction dir,
+				       unsigned long attrs)
+{
+	struct scatterlist *sg;
+	int i;
+
+	if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+		for_each_sg(sgl, sg, nelems, i)
+			arch_sync_dma_for_cpu(dev,
+					  dma_to_phys(dev, sg->dma_address),
+					  sg->length, dir);
+	swiotlb_unmap_sg_attrs(dev, sgl, nelems, dir, attrs);
+}
+
+static void dma_riscv_swiotlb_sync_single_for_cpu(struct device *dev,
+						  dma_addr_t dev_addr,
+						  size_t size,
+						  enum dma_data_direction dir)
+{
+	arch_sync_dma_for_cpu(dev, dma_to_phys(dev, dev_addr), size, dir);
+	swiotlb_sync_single_for_cpu(dev, dev_addr, size, dir);
+}
+
+static void dma_riscv_swiotlb_sync_single_for_device(struct device *dev,
+						     dma_addr_t dev_addr,
+						     size_t size,
+						     enum dma_data_direction
+						     dir)
+{
+	swiotlb_sync_single_for_device(dev, dev_addr, size, dir);
+	arch_sync_dma_for_device(dev, dma_to_phys(dev, dev_addr), size, dir);
+}
+
+static void dma_riscv_swiotlb_sync_sg_for_cpu(struct device *dev,
+					      struct scatterlist *sgl,
+					      int nelems,
+					      enum dma_data_direction dir)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(sgl, sg, nelems, i)
+		arch_sync_dma_for_cpu(dev, dma_to_phys(dev, sg->dma_address),
+				  sg->length, dir);
+	swiotlb_sync_sg_for_cpu(dev, sgl, nelems, dir);
+}
+
+static void dma_riscv_swiotlb_sync_sg_for_device(struct device *dev,
+						 struct scatterlist *sgl,
+						 int nelems,
+						 enum dma_data_direction dir)
+{
+	struct scatterlist *sg;
+	int i;
+
+	swiotlb_sync_sg_for_device(dev, sgl, nelems, dir);
+	for_each_sg(sgl, sg, nelems, i)
+		arch_sync_dma_for_device(dev,
+				     dma_to_phys(dev, sg->dma_address),
+				     sg->length, dir);
+}
+
+const struct dma_map_ops swiotlb_noncoh_dma_ops = {
+	.alloc			= arch_dma_alloc,
+	.free			= arch_dma_free,
+	.dma_supported		= swiotlb_dma_supported,
+	.map_page		= dma_riscv_swiotlb_map_page,
+	.map_sg			= dma_riscv_swiotlb_map_sg,
+	.unmap_page		= dma_riscv_swiotlb_unmap_page,
+	.unmap_sg		= dma_riscv_swiotlb_unmap_sg,
+	.sync_single_for_cpu	= dma_riscv_swiotlb_sync_single_for_cpu,
+	.sync_single_for_device	= dma_riscv_swiotlb_sync_single_for_device,
+	.sync_sg_for_cpu	= dma_riscv_swiotlb_sync_sg_for_cpu,
+	.sync_sg_for_device	= dma_riscv_swiotlb_sync_sg_for_device,
+};
+EXPORT_SYMBOL(swiotlb_noncoh_dma_ops);
+#endif
diff --git a/arch/riscv/vendor-nds/setup.c b/arch/riscv/vendor-nds/setup.c
index 5ceed1b..4b494eb 100644
--- a/arch/riscv/vendor-nds/setup.c
+++ b/arch/riscv/vendor-nds/setup.c
@@ -1,9 +1,16 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (C) 2018 Andes Technology Corporation
 #include <linux/init.h>
+#include <asm/sbi.h>
 #include <asm/vendor-hook.h>
+extern phys_addr_t pa_msb;
 bool riscv_nds_compat_platform;
+static void __init setup_maxpa(void)
+{
+	pa_msb = sbi_call_riscv_nds_get_maxpa();
+}
 void __init setup_vendor_extension(void)
 {
 	riscv_nds_compat_platform = true;
+	setup_maxpa();
 }
-- 
1.7.1

[RFC 2/2] RISC-V: make dma_map_ops work without cache coherent agent

[Vincent Chen]

    Currently, DMA operations in RISC-V ports assume the cache coherent
agent is supported. In other words, the functions in struct dma_map_ops
cannot work if the cache coherent agent is unsupported. In RISCV-NDS
extension ISA, the AndeStart RISC-V CPU provides a solution to overcome
this limitation.

  The AndeStart RISC-V CPU provides following two features so that the
functions of struct dma_map_ops can work without cache coherent agent.

  1. Non-cacheable memory
     In standard RISC-V, the memory cacheability is usually
  determined by the platform and there is no a particular scheme to
  adjust it by software at runtime. Hence, platform needs to allocate a
  fixed non-cacheable memory region for OS. This may cause some
  inconvenience and waste to the user. The feature provided by the
  AndeStar RISC-V CPU is that when the cache coherent agent is not
  supported, the user can arbitrarily disable the cacheability of a
  particular memory region at runtime. This feature separates the whole
  PA region is into 2 parts. The PA in higher part is the non-cacheable
  alias to the lower part. Hence, we can think the cacheability of
  targeted PA can be disabled by setting the MSB of physical address to 1.
  Based on this feature, user just requires a general memory and then
  binds the aliasing of the deriving PA in higher part to a new VA by
  io_reamp.

  2. Synchronize specific content between memory and cache
     The RISC-V generic ISA has instructions that allow the user to
  synchronize between cache and memory. However, the synchronized region
  can only be the entire cache. Therefore, the RISC-V generic ISA cannot
  handle the page synchronization required by the struct dma_map_ops.
  The extension ISA provided by the AndeStar RISC-V CPU has the ability
  to synchronize the contents of specific region between memory and
  cache.

  Due to feature requirements, we need to use custom SBI call to
derive the MSB of physical address from bbl. However, there is no
reserved SBI call for vendor in the current specification. Hence we use
a temporary method to send the vendor-defined SBI call in this commit.
We glad to change the implementation when determining the SBI usage for
vendor.

Signed-off-by: Vincent Chen <vincentc@andestech.com>
---
 arch/riscv/include/asm/sbi.h                    |    1 +
 arch/riscv/vendor-nds/cache.c                   |   83 ++++++++
 arch/riscv/vendor-nds/include/asm/csr.h         |   32 +++
 arch/riscv/vendor-nds/include/asm/dma-mapping.h |   24 +++
 arch/riscv/vendor-nds/include/asm/proc.h        |   17 ++
 arch/riscv/vendor-nds/include/asm/sbi.h         |   17 ++
 arch/riscv/vendor-nds/noncoherent_dma.c         |  254 +++++++++++++++++++++++
 arch/riscv/vendor-nds/setup.c                   |    7 +
 8 files changed, 435 insertions(+), 0 deletions(-)
 create mode 100644 arch/riscv/vendor-nds/cache.c
 create mode 100644 arch/riscv/vendor-nds/include/asm/csr.h
 create mode 100644 arch/riscv/vendor-nds/include/asm/dma-mapping.h
 create mode 100644 arch/riscv/vendor-nds/include/asm/proc.h
 create mode 100644 arch/riscv/vendor-nds/include/asm/sbi.h
 create mode 100644 arch/riscv/vendor-nds/noncoherent_dma.c

diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 5e1abf6..731fc38 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -26,6 +26,7 @@
 #define SBI_REMOTE_SFENCE_VMA_ASID 7
 #define SBI_SHUTDOWN 8
 #define SBI_GET_MVENDOR_ID 10
+#define SBI_XEXT_ARCH_RESERVED 0x80000000
 
 #define SBI_CALL(which, arg0, arg1, arg2) ({			\
 	register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0);	\
diff --git a/arch/riscv/vendor-nds/cache.c b/arch/riscv/vendor-nds/cache.c
new file mode 100644
index 0000000..d9754ac
--- /dev/null
+++ b/arch/riscv/vendor-nds/cache.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Andes Technology Corporation
+#include <linux/irqflags.h>
+#include <linux/module.h>
+#include <linux/cpu.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/cacheinfo.h>
+#include <linux/sizes.h>
+#include <asm/csr.h>
+#include <asm/proc.h>
+
+
+DEFINE_PER_CPU(struct riscv_nds_cache_info, cpu_cache_info) = {
+	.init_done = 0,
+	.dcache_line_size = SZ_32
+};
+static void fill_cpu_cache_info(struct riscv_nds_cache_info *cpu_ci)
+{
+	struct cpu_cacheinfo *this_cpu_ci =
+			get_cpu_cacheinfo(smp_processor_id());
+	struct cacheinfo *this_leaf = this_cpu_ci->info_list;
+	unsigned int i = 0;
+
+	for (; i < this_cpu_ci->num_leaves ; i++, this_leaf++) {
+		if (this_leaf->type == CACHE_TYPE_DATA)
+			cpu_ci->dcache_line_size = this_leaf->coherency_line_size;
+	}
+	cpu_ci->init_done = true;
+}
+
+
+inline int get_cache_line_size(void)
+{
+	struct riscv_nds_cache_info *cpu_ci =
+		&per_cpu(cpu_cache_info, smp_processor_id());
+
+	if (unlikely(cpu_ci->init_done == false))
+		fill_cpu_cache_info(cpu_ci);
+	return cpu_ci->dcache_line_size;
+}
+void cpu_dcache_wb_range(unsigned long start, unsigned long end)
+{
+	int line_size = get_cache_line_size();
+
+	while (end > start) {
+		csr_write(ucctlbeginaddr, start);
+		csr_write(ucctlcommand, CCTL_L1D_VA_WB);
+		start += line_size;
+	}
+}
+
+void cpu_dcache_inval_range(unsigned long start, unsigned long end)
+{
+	int line_size = get_cache_line_size();
+
+	while (end > start) {
+		csr_write(ucctlbeginaddr, start);
+		csr_write(ucctlcommand, CCTL_L1D_VA_INVAL);
+		start += line_size;
+	}
+}
+void cpu_dma_inval_range(unsigned long start, unsigned long end)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	cpu_dcache_inval_range(start, end);
+	local_irq_restore(flags);
+
+}
+EXPORT_SYMBOL(cpu_dma_inval_range);
+
+void cpu_dma_wb_range(unsigned long start, unsigned long end)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	cpu_dcache_wb_range(start, end);
+	local_irq_restore(flags);
+}
+EXPORT_SYMBOL(cpu_dma_wb_range);
+
diff --git a/arch/riscv/vendor-nds/include/asm/csr.h b/arch/riscv/vendor-nds/include/asm/csr.h
new file mode 100644
index 0000000..6027e6d
--- /dev/null
+++ b/arch/riscv/vendor-nds/include/asm/csr.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Andes Technology Corporation */
+#include_next <asm/csr.h>
+
+/* mdcm_cfg: Data Cache/Memory Configuration Register */
+#define MDCM_CFG_DEST_OFFSET		0
+#define MDCM_CFG_DWAY_OFFSET		3
+#define MDCM_CFG_DSZ_OFFSET		6
+#define MDCM_CFG_DLCK_OFFSET		9
+#define MDCM_CFG_DC_ECC_OFFSET		10
+#define MDCM_CFG_DLMB_OFFSET		12
+#define MDCM_CFG_DLMSZ_OFFSET		15
+#define MDCM_CFG_ULM_2BANK_OFFSET	20
+#define MDCM_CFG_DLM_ECC_OFFSET		21
+
+
+#define MDCM_CFG_DEST_MASK	(0x7  << MDCM_CFG_DEST_OFFSET)
+#define MDCM_CFG_DWAY_MASK	(0x7  << MDCM_CFG_DWAY_OFFSET)
+#define MDCM_CFG_DSZ_MASK	(0x7  << MDCM_CFG_DSZ_OFFSET)
+#define MDCM_CFG_DLCK_MASK	(0x1  << MDCM_CFG_DLCK_OFFSET)
+#define MDCM_CFG_DC_ECC_MASK	(0x3  << MDCM_CFG_DC_ECC_OFFSET)
+#define MDCM_CFG_DLMB_MASK	(0x7  << MDCM_CFG_DLMB_OFFSET)
+#define MDCM_CFG_DLMSZ_MASK	(0x1f << MDCM_CFG_DLMSZ_OFFSET)
+#define MDCM_CFG_ULM_2BANK_MASK	(0x1  << MDCM_CFG_ULM_2BANK_OFFSET)
+#define MDCM_CFG_DLM_ECC_MASK	(0x3  << MDCM_CFG_DLM_ECC_OFFSET)
+
+
+/* ucctlcommand */
+/* D-cache operation */
+#define CCTL_L1D_VA_INVAL	0
+#define CCTL_L1D_VA_WB		1
+#define CCTL_L1D_VA_WBINVAL	2
diff --git a/arch/riscv/vendor-nds/include/asm/dma-mapping.h b/arch/riscv/vendor-nds/include/asm/dma-mapping.h
new file mode 100644
index 0000000..30b4183
--- /dev/null
+++ b/arch/riscv/vendor-nds/include/asm/dma-mapping.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Andes Technology Corporation */
+#ifndef _RISCV_ASM_DMA_MAPPING_H
+#define _RISCV_ASM_DMA_MAPPING_H 1
+
+
+#ifdef CONFIG_SWIOTLB
+#include <linux/swiotlb.h>
+static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
+{
+#ifdef CONFIG_DMA_NONCOHERENT_OPS
+	extern const struct dma_map_ops swiotlb_noncoh_dma_ops;
+	extern bool riscv_nds_compat_platform;
+
+	if (riscv_nds_compat_platform)
+		return &swiotlb_noncoh_dma_ops;
+#endif
+	return &swiotlb_dma_ops;
+}
+#else
+#include <asm-generic/dma-mapping.h>
+#endif /* CONFIG_SWIOTLB */
+
+#endif /* _RISCV_ASM_DMA_MAPPING_H */
diff --git a/arch/riscv/vendor-nds/include/asm/proc.h b/arch/riscv/vendor-nds/include/asm/proc.h
new file mode 100644
index 0000000..a2684f4
--- /dev/null
+++ b/arch/riscv/vendor-nds/include/asm/proc.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Andes Technology Corporation */
+#include <asm/io.h>
+#include <asm/page.h>
+
+void cpu_dma_inval_range(unsigned long start, unsigned long end);
+
+void cpu_dma_wb_range(unsigned long start, unsigned long end);
+
+/*
+ * When the content of riscv_nds_cache_info has been initilized by function
+ * fill_cpu_cache_info(), member init_done is set to true
+ */
+struct riscv_nds_cache_info {
+	bool init_done;
+	int dcache_line_size;
+};
diff --git a/arch/riscv/vendor-nds/include/asm/sbi.h b/arch/riscv/vendor-nds/include/asm/sbi.h
new file mode 100644
index 0000000..7a002a4
--- /dev/null
+++ b/arch/riscv/vendor-nds/include/asm/sbi.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Andes Technology Corporation */
+#ifndef _AMS_RISCV_NDS_SBI_H
+#define _AMS_RISCV_NDS_SBI_H
+
+#include_next <asm/sbi.h>
+#define SBI_RISCV_NDS_GET_MAX_PA (0x0)
+
+#define SBI_RISCV_NDS_CALL_0(which) \
+	SBI_CALL(SBI_XEXT_ARCH_RESERVED|which, 0, 0, 0)
+
+
+static inline phys_addr_t sbi_call_riscv_nds_get_maxpa(void)
+{
+	return SBI_RISCV_NDS_CALL_0(SBI_RISCV_NDS_GET_MAX_PA);
+}
+#endif
diff --git a/arch/riscv/vendor-nds/noncoherent_dma.c b/arch/riscv/vendor-nds/noncoherent_dma.c
new file mode 100644
index 0000000..470dc90
--- /dev/null
+++ b/arch/riscv/vendor-nds/noncoherent_dma.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Andes Technology Corporation
+#include <linux/gfp.h>
+#include <linux/mm.h>
+#include <linux/swiotlb.h>
+#include <linux/dma-mapping.h>
+#include <linux/dma-direct.h>
+#include <linux/scatterlist.h>
+#include <asm/proc.h>
+
+phys_addr_t pa_msb;
+#define dma_remap(pa, size) ioremap((pa|(pa_msb << PAGE_SHIFT)), size)
+#define dma_unmap(vaddr) iounmap((void __force __iomem *)vaddr)
+
+static void dma_flush_page(struct page *page, size_t size)
+{
+	unsigned long k_d_vaddr;
+	/*
+	 * Invalidate any data that might be lurking in the
+	 * kernel direct-mapped region for device DMA.
+	 */
+	k_d_vaddr = (unsigned long)page_address(page);
+	memset((void *)k_d_vaddr, 0, size);
+	cpu_dma_wb_range(k_d_vaddr, k_d_vaddr + size);
+	cpu_dma_inval_range(k_d_vaddr, k_d_vaddr + size);
+
+}
+
+static inline void cache_op(phys_addr_t paddr, size_t size,
+			    void (*fn)(unsigned long start, unsigned long end))
+{
+	unsigned long start;
+
+	start = (unsigned long)phys_to_virt(paddr);
+	fn(start, start + size);
+}
+
+void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
+			      size_t size, enum dma_data_direction dir)
+{
+	switch (dir) {
+	case DMA_FROM_DEVICE:
+		break;
+	case DMA_TO_DEVICE:
+	case DMA_BIDIRECTIONAL:
+		cache_op(paddr, size, cpu_dma_wb_range);
+		break;
+	default:
+		BUG();
+	}
+}
+
+void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr,
+			   size_t size, enum dma_data_direction dir)
+{
+	switch (dir) {
+	case DMA_TO_DEVICE:
+		break;
+	case DMA_FROM_DEVICE:
+	case DMA_BIDIRECTIONAL:
+		cache_op(paddr, size, cpu_dma_inval_range);
+		break;
+	default:
+		BUG();
+	}
+}
+
+#ifdef CONFIG_32BIT
+void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
+		     gfp_t gfp, unsigned long attrs)
+{
+	void *kvaddr, *coherent_kvaddr;
+
+	size = PAGE_ALIGN(size);
+	kvaddr = dma_direct_alloc(dev, size, handle, gfp, attrs);
+	if (!kvaddr)
+		goto no_mem;
+	coherent_kvaddr = dma_remap(dma_to_phys(dev, *handle), size);
+	if (!coherent_kvaddr)
+		goto no_map;
+
+	dma_flush_page(virt_to_page(kvaddr), size);
+	return coherent_kvaddr;
+no_map:
+	dma_direct_free(dev, size, kvaddr, *handle, attrs);
+no_mem:
+	return NULL;
+}
+
+void arch_dma_free(struct device *dev, size_t size, void *vaddr,
+		   dma_addr_t handle, unsigned long attrs)
+{
+	void *kvaddr = phys_to_virt(dma_to_phys(dev, handle));
+
+	size = PAGE_ALIGN(size);
+	dma_unmap(vaddr);
+	dma_direct_free(dev, size, kvaddr, handle, attrs);
+}
+#else
+void *arch_dma_alloc(struct device *dev, size_t size,
+		     dma_addr_t *handle, gfp_t gfp, unsigned long attrs)
+{
+	void *kvaddr, *coherent_kvaddr;
+
+	size = PAGE_ALIGN(size);
+	kvaddr = swiotlb_alloc(dev, size, handle, gfp, attrs);
+	if (!kvaddr)
+		goto no_mem;
+	coherent_kvaddr = dma_remap(dma_to_phys(dev, *handle), size);
+	if (!coherent_kvaddr)
+		goto no_map;
+
+	dma_flush_page(virt_to_page(kvaddr), size);
+	return coherent_kvaddr;
+no_map:
+	swiotlb_free(dev, size, kvaddr, *handle, attrs);
+no_mem:
+	return NULL;
+}
+
+void arch_dma_free(struct device *dev, size_t size, void *vaddr,
+		   dma_addr_t handle, unsigned long attrs)
+{
+	void *swiotlb_addr = phys_to_virt(dma_to_phys(dev, handle));
+
+	size = PAGE_ALIGN(size);
+	dma_unmap(vaddr);
+	swiotlb_free(dev, size, swiotlb_addr, handle, attrs);
+}
+
+static dma_addr_t dma_riscv_swiotlb_map_page(struct device *dev,
+					     struct page *page,
+					     unsigned long offset, size_t size,
+					     enum dma_data_direction dir,
+					     unsigned long attrs)
+{
+	dma_addr_t dev_addr;
+
+	dev_addr = swiotlb_map_page(dev, page, offset, size, dir, attrs);
+	if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+		arch_sync_dma_for_device(dev, dma_to_phys(dev, dev_addr),
+					 size, dir);
+
+	return dev_addr;
+}
+
+static int dma_riscv_swiotlb_map_sg(struct device *dev,
+				    struct scatterlist *sgl,
+				    int nelems, enum dma_data_direction dir,
+				    unsigned long attrs)
+{
+	struct scatterlist *sg;
+	int i, ret;
+
+	ret = swiotlb_map_sg_attrs(dev, sgl, nelems, dir, attrs);
+	if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+		for_each_sg(sgl, sg, ret, i)
+			arch_sync_dma_for_device(dev,
+					     dma_to_phys(dev, sg->dma_address),
+					     sg->length, dir);
+
+	return ret;
+}
+
+static void dma_riscv_swiotlb_unmap_page(struct device *dev,
+					 dma_addr_t dev_addr, size_t size,
+					 enum dma_data_direction dir,
+					 unsigned long attrs)
+{
+	if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+		arch_sync_dma_for_cpu(dev, dma_to_phys(dev, dev_addr),
+				      size, dir);
+	swiotlb_unmap_page(dev, dev_addr, size, dir, attrs);
+}
+
+static void dma_riscv_swiotlb_unmap_sg(struct device *dev,
+				       struct scatterlist *sgl, int nelems,
+				       enum dma_data_direction dir,
+				       unsigned long attrs)
+{
+	struct scatterlist *sg;
+	int i;
+
+	if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+		for_each_sg(sgl, sg, nelems, i)
+			arch_sync_dma_for_cpu(dev,
+					  dma_to_phys(dev, sg->dma_address),
+					  sg->length, dir);
+	swiotlb_unmap_sg_attrs(dev, sgl, nelems, dir, attrs);
+}
+
+static void dma_riscv_swiotlb_sync_single_for_cpu(struct device *dev,
+						  dma_addr_t dev_addr,
+						  size_t size,
+						  enum dma_data_direction dir)
+{
+	arch_sync_dma_for_cpu(dev, dma_to_phys(dev, dev_addr), size, dir);
+	swiotlb_sync_single_for_cpu(dev, dev_addr, size, dir);
+}
+
+static void dma_riscv_swiotlb_sync_single_for_device(struct device *dev,
+						     dma_addr_t dev_addr,
+						     size_t size,
+						     enum dma_data_direction
+						     dir)
+{
+	swiotlb_sync_single_for_device(dev, dev_addr, size, dir);
+	arch_sync_dma_for_device(dev, dma_to_phys(dev, dev_addr), size, dir);
+}
+
+static void dma_riscv_swiotlb_sync_sg_for_cpu(struct device *dev,
+					      struct scatterlist *sgl,
+					      int nelems,
+					      enum dma_data_direction dir)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(sgl, sg, nelems, i)
+		arch_sync_dma_for_cpu(dev, dma_to_phys(dev, sg->dma_address),
+				  sg->length, dir);
+	swiotlb_sync_sg_for_cpu(dev, sgl, nelems, dir);
+}
+
+static void dma_riscv_swiotlb_sync_sg_for_device(struct device *dev,
+						 struct scatterlist *sgl,
+						 int nelems,
+						 enum dma_data_direction dir)
+{
+	struct scatterlist *sg;
+	int i;
+
+	swiotlb_sync_sg_for_device(dev, sgl, nelems, dir);
+	for_each_sg(sgl, sg, nelems, i)
+		arch_sync_dma_for_device(dev,
+				     dma_to_phys(dev, sg->dma_address),
+				     sg->length, dir);
+}
+
+const struct dma_map_ops swiotlb_noncoh_dma_ops = {
+	.alloc			= arch_dma_alloc,
+	.free			= arch_dma_free,
+	.dma_supported		= swiotlb_dma_supported,
+	.map_page		= dma_riscv_swiotlb_map_page,
+	.map_sg			= dma_riscv_swiotlb_map_sg,
+	.unmap_page		= dma_riscv_swiotlb_unmap_page,
+	.unmap_sg		= dma_riscv_swiotlb_unmap_sg,
+	.sync_single_for_cpu	= dma_riscv_swiotlb_sync_single_for_cpu,
+	.sync_single_for_device	= dma_riscv_swiotlb_sync_single_for_device,
+	.sync_sg_for_cpu	= dma_riscv_swiotlb_sync_sg_for_cpu,
+	.sync_sg_for_device	= dma_riscv_swiotlb_sync_sg_for_device,
+};
+EXPORT_SYMBOL(swiotlb_noncoh_dma_ops);
+#endif
diff --git a/arch/riscv/vendor-nds/setup.c b/arch/riscv/vendor-nds/setup.c
index 5ceed1b..4b494eb 100644
--- a/arch/riscv/vendor-nds/setup.c
+++ b/arch/riscv/vendor-nds/setup.c
@@ -1,9 +1,16 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (C) 2018 Andes Technology Corporation
 #include <linux/init.h>
+#include <asm/sbi.h>
 #include <asm/vendor-hook.h>
+extern phys_addr_t pa_msb;
 bool riscv_nds_compat_platform;
+static void __init setup_maxpa(void)
+{
+	pa_msb = sbi_call_riscv_nds_get_maxpa();
+}
 void __init setup_vendor_extension(void)
 {
 	riscv_nds_compat_platform = true;
+	setup_maxpa();
 }
-- 
1.7.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Anup Patel]

On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com> 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.

Regards,
Anup

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

[Anup Patel]

On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com> 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.

Regards,
Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Arnd Bergmann]

On 10/31/18, Anup Patel <anup@brainfault.org> wrote:
> On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com>
> 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.

Agreed, we did this on arm32 in the past, and it took us a long time
to change all the modern platforms (ARMv6/7/8) to be usable in a
shared kernel. It's better to avoid that and keep everything together
like we did on arm64 from the start.

One thing we do on arm32 is to support combinations of different
instruction set variants in a combined kernel through callback pointers
that turn into direct function calls when the kernel is configured for
only a single CPU type. This might be something to add later on
riscv, but I probably wouldn't do it right away.

      Arnd

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

[Arnd Bergmann]

On 10/31/18, Anup Patel <anup@brainfault.org> wrote:
> On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com>
> 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.

Agreed, we did this on arm32 in the past, and it took us a long time
to change all the modern platforms (ARMv6/7/8) to be usable in a
shared kernel. It's better to avoid that and keep everything together
like we did on arm64 from the start.

One thing we do on arm32 is to support combinations of different
instruction set variants in a combined kernel through callback pointers
that turn into direct function calls when the kernel is configured for
only a single CPU type. This might be something to add later on
riscv, but I probably wouldn't do it right away.

      Arnd

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Christoph Hellwig]

On Wed, Oct 31, 2018 at 04:46:10PM +0530, Anup Patel wrote:
> I agree that we need a place for vendor-specific ISA extensions and
> having vendor-specific directories is also good.

The only sensible answer is that we should not allow vendor specific
extensions in the kernel at all.  We need to standardize cache flushing
and we need to do it soon and not introduce horrible bandaids because
the foundation did not do its work.

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

[Christoph Hellwig]

On Wed, Oct 31, 2018 at 04:46:10PM +0530, Anup Patel wrote:
> I agree that we need a place for vendor-specific ISA extensions and
> having vendor-specific directories is also good.

The only sensible answer is that we should not allow vendor specific
extensions in the kernel at all.  We need to standardize cache flushing
and we need to do it soon and not introduce horrible bandaids because
the foundation did not do its work.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Palmer Dabbelt]

On Wed, 31 Oct 2018 04:16:10 PDT (-0700), anup at brainfault.org wrote:
> On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com> 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.

This is essentially the same as my feedback for the performance counter stuff, 
which IIRC is what prompted adding a vendor-specific extensions.

If I was going to do this, I'd split it up such that the vendor-specific 
additions are per-subsystem.  That way we can focus on building a decent 
interface for each subsystem that needs vendor-specific support rather than 
just bundling everything together where the vendor-specific stuff will get all 
tangled together.

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

[Palmer Dabbelt]

On Wed, 31 Oct 2018 04:16:10 PDT (-0700), anup@brainfault.org wrote:
> On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com> 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.

This is essentially the same as my feedback for the performance counter stuff, 
which IIRC is what prompted adding a vendor-specific extensions.

If I was going to do this, I'd split it up such that the vendor-specific 
additions are per-subsystem.  That way we can focus on building a decent 
interface for each subsystem that needs vendor-specific support rather than 
just bundling everything together where the vendor-specific stuff will get all 
tangled together.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Olof Johansson]

On Wed, Oct 31, 2018 at 10:27 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Wed, 31 Oct 2018 04:16:10 PDT (-0700), anup at brainfault.org wrote:
> > On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com> 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.
>
> This is essentially the same as my feedback for the performance counter stuff,
> which IIRC is what prompted adding a vendor-specific extensions.
>
> If I was going to do this, I'd split it up such that the vendor-specific
> additions are per-subsystem.  That way we can focus on building a decent
> interface for each subsystem that needs vendor-specific support rather than
> just bundling everything together where the vendor-specific stuff will get all
> tangled together.

For short-term, powerpc's model of a machine descriptor with function
pointers that's either filled in at runtime, or set to the right
pre-defined structure per platform, should cover most of this I think?

Even if you have cases where an indirect branch isn't feasible,
performance-wise, _getting_ the function address from the table and
doing runtime alternatives-style patching still gives one place to
keep it.

I'm not sure if we need a table/struct per subsystem, or if one larger
shared one is sufficient to start. Since this is all in-kernel code
without external interface, I'd suggest starting simple and refactor
later if needed.


-Olof

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

[Olof Johansson]

On Wed, Oct 31, 2018 at 10:27 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Wed, 31 Oct 2018 04:16:10 PDT (-0700), anup@brainfault.org wrote:
> > On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com> 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.
>
> This is essentially the same as my feedback for the performance counter stuff,
> which IIRC is what prompted adding a vendor-specific extensions.
>
> If I was going to do this, I'd split it up such that the vendor-specific
> additions are per-subsystem.  That way we can focus on building a decent
> interface for each subsystem that needs vendor-specific support rather than
> just bundling everything together where the vendor-specific stuff will get all
> tangled together.

For short-term, powerpc's model of a machine descriptor with function
pointers that's either filled in at runtime, or set to the right
pre-defined structure per platform, should cover most of this I think?

Even if you have cases where an indirect branch isn't feasible,
performance-wise, _getting_ the function address from the table and
doing runtime alternatives-style patching still gives one place to
keep it.

I'm not sure if we need a table/struct per subsystem, or if one larger
shared one is sufficient to start. Since this is all in-kernel code
without external interface, I'd suggest starting simple and refactor
later if needed.


-Olof

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Alan Kao]

On Wed, Oct 31, 2018 at 07:17:45AM -0700, Christoph Hellwig wrote:
> On Wed, Oct 31, 2018 at 04:46:10PM +0530, Anup Patel wrote:
> > I agree that we need a place for vendor-specific ISA extensions and
> > having vendor-specific directories is also good.
> 
> The only sensible answer is that we should not allow vendor specific
> extensions in the kernel at all.  ...

How can this even be possible if a extension includes an extra register
set as some domain-specific context?  In such a case, kernel should
at least process the context during any context switch, just like how it
deals with the FP context.

> ... We need to standardize cache flushing 
> and we need to do it soon and not introduce horrible bandaids because
> the foundation did not do its work.
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Alan Kao]

On Wed, Oct 31, 2018 at 07:17:45AM -0700, Christoph Hellwig wrote:
> On Wed, Oct 31, 2018 at 04:46:10PM +0530, Anup Patel wrote:
> > I agree that we need a place for vendor-specific ISA extensions and
> > having vendor-specific directories is also good.
> 
> The only sensible answer is that we should not allow vendor specific
> extensions in the kernel at all.  ...

How can this even be possible if a extension includes an extra register
set as some domain-specific context?  In such a case, kernel should
at least process the context during any context switch, just like how it
deals with the FP context.

> ... We need to standardize cache flushing 
> and we need to do it soon and not introduce horrible bandaids because
> the foundation did not do its work.
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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@brainfault.org wrote:
> > On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com> 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.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Palmer Dabbelt]

On Wed, 31 Oct 2018 17:55:42 PDT (-0700), alankao at andestech.com wrote:
> On Wed, Oct 31, 2018 at 07:17:45AM -0700, Christoph Hellwig wrote:
>> On Wed, Oct 31, 2018 at 04:46:10PM +0530, Anup Patel wrote:
>> > I agree that we need a place for vendor-specific ISA extensions and
>> > having vendor-specific directories is also good.
>>
>> The only sensible answer is that we should not allow vendor specific
>> extensions in the kernel at all.  ...
>
> How can this even be possible if a extension includes an extra register
> set as some domain-specific context?  In such a case, kernel should
> at least process the context during any context switch, just like how it
> deals with the FP context.

Ya, I think there are cases where vendor-specific extensions are going to be 
necessary to handle within the kernel.  Right now the only one I can think of 
is the performance counter stuff, where we explicitly allow vendor-specific 
counters as part of the ISA spec.

For stateful extensions, we currently have a standard mechanism where the XS 
bits get set in sstatus and the actual save/restore code is hidden behind an 
SBI call.  That call doesn't currently exist, but if we just go ahead and add 
one it should be easy to support this from within Linux.  We'll need to figure 
out how to enable these custom extensions from userspace, but that seems 
tractable as well.  We'll probably also want some fast-path for the V extension 
(and any other stateful standard extensions), but I think as long as the V 
extension adds a quick check for dirtiness then it's not a big deal.

Do you guys have stateful extensions?  We're trying really hard to avoid them 
at SiFive because they're a huge headache, so unless there's a compelling base 
of software using one I don't want to go add support if we can avoid it.

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

[Palmer Dabbelt]

On Wed, 31 Oct 2018 17:55:42 PDT (-0700), alankao@andestech.com wrote:
> On Wed, Oct 31, 2018 at 07:17:45AM -0700, Christoph Hellwig wrote:
>> On Wed, Oct 31, 2018 at 04:46:10PM +0530, Anup Patel wrote:
>> > I agree that we need a place for vendor-specific ISA extensions and
>> > having vendor-specific directories is also good.
>>
>> The only sensible answer is that we should not allow vendor specific
>> extensions in the kernel at all.  ...
>
> How can this even be possible if a extension includes an extra register
> set as some domain-specific context?  In such a case, kernel should
> at least process the context during any context switch, just like how it
> deals with the FP context.

Ya, I think there are cases where vendor-specific extensions are going to be 
necessary to handle within the kernel.  Right now the only one I can think of 
is the performance counter stuff, where we explicitly allow vendor-specific 
counters as part of the ISA spec.

For stateful extensions, we currently have a standard mechanism where the XS 
bits get set in sstatus and the actual save/restore code is hidden behind an 
SBI call.  That call doesn't currently exist, but if we just go ahead and add 
one it should be easy to support this from within Linux.  We'll need to figure 
out how to enable these custom extensions from userspace, but that seems 
tractable as well.  We'll probably also want some fast-path for the V extension 
(and any other stateful standard extensions), but I think as long as the V 
extension adds a quick check for dirtiness then it's not a big deal.

Do you guys have stateful extensions?  We're trying really hard to avoid them 
at SiFive because they're a huge headache, so unless there's a compelling base 
of software using one I don't want to go add support if we can avoid it.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Alan Kao]

On Thu, Nov 01, 2018 at 10:50:04AM -0700, Palmer Dabbelt wrote:
> On Wed, 31 Oct 2018 17:55:42 PDT (-0700), alankao at andestech.com wrote:
> >On Wed, Oct 31, 2018 at 07:17:45AM -0700, Christoph Hellwig wrote:
> >>On Wed, Oct 31, 2018 at 04:46:10PM +0530, Anup Patel wrote:
> >>> I agree that we need a place for vendor-specific ISA extensions and
> >>> having vendor-specific directories is also good.
> >>
> >>The only sensible answer is that we should not allow vendor specific
> >>extensions in the kernel at all.  ...
> >
> >How can this even be possible if a extension includes an extra register
> >set as some domain-specific context?  In such a case, kernel should
> >at least process the context during any context switch, just like how it
> >deals with the FP context.
> 
> Ya, I think there are cases where vendor-specific extensions are going to be
> necessary to handle within the kernel.  Right now the only one I can think
> of is the performance counter stuff, where we explicitly allow
> vendor-specific counters as part of the ISA spec.
> 
> For stateful extensions, we currently have a standard mechanism where the XS
> bits get set in sstatus and the actual save/restore code is hidden behind an
> SBI call.  That call doesn't currently exist, but if we just go ahead and
> add one it should be easy to support this from within Linux.  We'll need to
> figure out how to enable these custom extensions from userspace, but that
> seems tractable as well.  We'll probably also want some fast-path for the V
> extension (and any other stateful standard extensions), but I think as long
> as the V extension adds a quick check for dirtiness then it's not a big
> deal.
> 
> Do you guys have stateful extensions?  We're trying really hard to avoid
> them at SiFive because they're a huge headache, so unless there's a
> compelling base of software using one I don't want to go add support if we
> can avoid it.

Currently no, but the future is hard to see.  As long as the extensible freedom
claimed by the RISC-V foundation remains true, such extensions may have their
role to play.  Don't worry now, I was just to give a example that in some 
possible vendor-specific cases the kernel cannot keep itself from involving.

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

[Alan Kao]

On Thu, Nov 01, 2018 at 10:50:04AM -0700, Palmer Dabbelt wrote:
> On Wed, 31 Oct 2018 17:55:42 PDT (-0700), alankao@andestech.com wrote:
> >On Wed, Oct 31, 2018 at 07:17:45AM -0700, Christoph Hellwig wrote:
> >>On Wed, Oct 31, 2018 at 04:46:10PM +0530, Anup Patel wrote:
> >>> I agree that we need a place for vendor-specific ISA extensions and
> >>> having vendor-specific directories is also good.
> >>
> >>The only sensible answer is that we should not allow vendor specific
> >>extensions in the kernel at all.  ...
> >
> >How can this even be possible if a extension includes an extra register
> >set as some domain-specific context?  In such a case, kernel should
> >at least process the context during any context switch, just like how it
> >deals with the FP context.
> 
> Ya, I think there are cases where vendor-specific extensions are going to be
> necessary to handle within the kernel.  Right now the only one I can think
> of is the performance counter stuff, where we explicitly allow
> vendor-specific counters as part of the ISA spec.
> 
> For stateful extensions, we currently have a standard mechanism where the XS
> bits get set in sstatus and the actual save/restore code is hidden behind an
> SBI call.  That call doesn't currently exist, but if we just go ahead and
> add one it should be easy to support this from within Linux.  We'll need to
> figure out how to enable these custom extensions from userspace, but that
> seems tractable as well.  We'll probably also want some fast-path for the V
> extension (and any other stateful standard extensions), but I think as long
> as the V extension adds a quick check for dirtiness then it's not a big
> deal.
> 
> Do you guys have stateful extensions?  We're trying really hard to avoid
> them at SiFive because they're a huge headache, so unless there's a
> compelling base of software using one I don't want to go add support if we
> can avoid it.

Currently no, but the future is hard to see.  As long as the extensible freedom
claimed by the RISC-V foundation remains true, such extensions may have their
role to play.  Don't worry now, I was just to give a example that in some 
possible vendor-specific cases the kernel cannot keep itself from involving.


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Vincent Chen]

On Fri, Nov 02, 2018 at 01:48:57AM +0800, Karsten Merker wrote:
> On Wed, Oct 31, 2018 at 10:27:05AM -0700, Palmer Dabbelt wrote:
> > On Wed, 31 Oct 2018 04:16:10 PDT (-0700), anup at brainfault.org wrote:
> > > On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com> 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.
>
Many thanks for kinds of comments. I quickly synthesize the comments and
list them as below.
1. The kernel image shall include all vendor-specific code.
2. This kernel image can only enable particular vendor-specific features
   based on the CPU vendor in the running platform.
   - The runtime probing mechanism can refer to arm32/arm64, powerpc,
     IRQCHIP driver or CLOCKSOURCE driver
   - For some cases, such as open-source projects, using CSR $mvendorid
     to identify the compatibility is not appropriate.
I think the above requirements are reasonable, but I have questions about
the first requirement in practice. As far as I know, vendors are allowed
to add specific instructions and CSRs which are incompatible with other
vendors to their own ISA extensions. If I understand the first requirement
correctly, it implies that we need a "super" RISC-V toolchain. This "super"
RISC-V toolchain should recognize all kinds of vendor-specific instructions
and CSRs, so that it can compile vendor sources into objects successfully;
then it should recognize all kinds of vendor-specific relocations, so that
it can link the objects successfully. Each of them is not trivial at the
time of this proposal and in foreseeable future.

If it will take a long time to complete this "super" toolchain, I suppose
the infrastructure in this RFC might be a temporary solution before it is
ready. This scheme does not suffer the compilation problems caused by the
lack of the super toolchain because the selection of vendor-specific ISA
extension is determined at compile time. In addition, the mechanism for
checking compatibility at runtime ensures that the kernel with
vendor-specific feature runs on CPUs of other vendors just like pure
RISC-V kernel. In other words, this scheme, to some extent, satisfies the
requirements that one kernel image is bootable everywhere.

Regards,
Vincent

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

[Vincent Chen]

On Fri, Nov 02, 2018 at 01:48:57AM +0800, Karsten Merker wrote:
> On Wed, Oct 31, 2018 at 10:27:05AM -0700, Palmer Dabbelt wrote:
> > On Wed, 31 Oct 2018 04:16:10 PDT (-0700), anup@brainfault.org wrote:
> > > On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com> 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.
>
Many thanks for kinds of comments. I quickly synthesize the comments and
list them as below.
1. The kernel image shall include all vendor-specific code.
2. This kernel image can only enable particular vendor-specific features
   based on the CPU vendor in the running platform.
   - The runtime probing mechanism can refer to arm32/arm64, powerpc,
     IRQCHIP driver or CLOCKSOURCE driver
   - For some cases, such as open-source projects, using CSR $mvendorid
     to identify the compatibility is not appropriate.
I think the above requirements are reasonable, but I have questions about
the first requirement in practice. As far as I know, vendors are allowed
to add specific instructions and CSRs which are incompatible with other
vendors to their own ISA extensions. If I understand the first requirement
correctly, it implies that we need a "super" RISC-V toolchain. This "super"
RISC-V toolchain should recognize all kinds of vendor-specific instructions
and CSRs, so that it can compile vendor sources into objects successfully;
then it should recognize all kinds of vendor-specific relocations, so that
it can link the objects successfully. Each of them is not trivial at the
time of this proposal and in foreseeable future.

If it will take a long time to complete this "super" toolchain, I suppose
the infrastructure in this RFC might be a temporary solution before it is
ready. This scheme does not suffer the compilation problems caused by the
lack of the super toolchain because the selection of vendor-specific ISA
extension is determined at compile time. In addition, the mechanism for
checking compatibility at runtime ensures that the kernel with
vendor-specific feature runs on CPUs of other vendors just like pure
RISC-V kernel. In other words, this scheme, to some extent, satisfies the
requirements that one kernel image is bootable everywhere.

Regards,
Vincent

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Christoph Hellwig]

On Mon, Nov 05, 2018 at 02:58:07PM +0800, Vincent Chen wrote:
> Many thanks for kinds of comments. I quickly synthesize the comments and
> list them as below.
> 1. The kernel image shall include all vendor-specific code.

I fundamentally disagree with this? and think it should be the contrary.

1. The kernel shall support no vendor specific instructions whatsoever,
period.

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

[Christoph Hellwig]

On Mon, Nov 05, 2018 at 02:58:07PM +0800, Vincent Chen wrote:
> Many thanks for kinds of comments. I quickly synthesize the comments and
> list them as below.
> 1. The kernel image shall include all vendor-specific code.

I fundamentally disagree with this… and think it should be the contrary.

1. The kernel shall support no vendor specific instructions whatsoever,
period.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Arnd Bergmann]

On 11/5/18, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Nov 05, 2018 at 02:58:07PM +0800, Vincent Chen wrote:
>> Many thanks for kinds of comments. I quickly synthesize the comments and
>> list them as below.
>> 1. The kernel image shall include all vendor-specific code.
>
> I fundamentally disagree with this? and think it should be the contrary.
>
> 1. The kernel shall support no vendor specific instructions whatsoever,
> period.

I think what was meant above is

1. If a vendor extension requires kernel support, that support
must be able to be built into a kernel image without breaking support
for CPUs that do not have that extension, to allow building a single
kernel image that works on all CPUs.

       Arnd

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

[Arnd Bergmann]

On 11/5/18, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Nov 05, 2018 at 02:58:07PM +0800, Vincent Chen wrote:
>> Many thanks for kinds of comments. I quickly synthesize the comments and
>> list them as below.
>> 1. The kernel image shall include all vendor-specific code.
>
> I fundamentally disagree with this… and think it should be the contrary.
>
> 1. The kernel shall support no vendor specific instructions whatsoever,
> period.

I think what was meant above is

1. If a vendor extension requires kernel support, that support
must be able to be built into a kernel image without breaking support
for CPUs that do not have that extension, to allow building a single
kernel image that works on all CPUs.

       Arnd

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Christoph Hellwig]

On Mon, Nov 05, 2018 at 09:52:52AM +0100, Arnd Bergmann wrote:
> > I fundamentally disagree with this? and think it should be the contrary.
> >
> > 1. The kernel shall support no vendor specific instructions whatsoever,
> > period.
> 
> I think what was meant above is
> 
> 1. If a vendor extension requires kernel support, that support
> must be able to be built into a kernel image without breaking support
> for CPUs that do not have that extension, to allow building a single
> kernel image that works on all CPUs.

No.  This literally means no vendor extensions involving instructions
or CSRs in the kernel.  They are fine for userspace, or for the M-mode
code including impementation of the SBI, but not for the kernel.

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

[Christoph Hellwig]

On Mon, Nov 05, 2018 at 09:52:52AM +0100, Arnd Bergmann wrote:
> > I fundamentally disagree with this… and think it should be the contrary.
> >
> > 1. The kernel shall support no vendor specific instructions whatsoever,
> > period.
> 
> I think what was meant above is
> 
> 1. If a vendor extension requires kernel support, that support
> must be able to be built into a kernel image without breaking support
> for CPUs that do not have that extension, to allow building a single
> kernel image that works on all CPUs.

No.  This literally means no vendor extensions involving instructions
or CSRs in the kernel.  They are fine for userspace, or for the M-mode
code including impementation of the SBI, but not for the kernel.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Arnd Bergmann]

On 11/5/18, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Nov 05, 2018 at 09:52:52AM +0100, Arnd Bergmann wrote:
>> > I fundamentally disagree with this? and think it should be the
>> > contrary.
>> >
>> > 1. The kernel shall support no vendor specific instructions whatsoever,
>> > period.
>>
>> I think what was meant above is
>>
>> 1. If a vendor extension requires kernel support, that support
>> must be able to be built into a kernel image without breaking support
>> for CPUs that do not have that extension, to allow building a single
>> kernel image that works on all CPUs.
>
> No.  This literally means no vendor extensions involving instructions
> or CSRs in the kernel.  They are fine for userspace, or for the M-mode
> code including impementation of the SBI, but not for the kernel.

I was trying to interpret what Vincent wrote, not what you wrote,
you were pretty clear already ;-)

With the stricter policy you suggest, we'd loose the ability to support
some extensions that might be common:

- an extension for user space that adds new registers that must be
  saved and restored on a task switch, e.g. FPU, DSP or NPU
  instructions. ARM supports several incompatible extensions like
  that in one kernel, and this is really ugly, but I suspect RISC-V
  will already need the same thing to support all combinations of
  standard extensions, so from a practical perspective it's not
  much different for custom extension, aside from the question
  how far you want to go to discourage custom extensions by
  requiring users to patch their kernels.

- A crypto instruction for a cipher that is used in the kernel
  for speeding up network or block data encryption.
  This would typically be a standalone loadable module, so
  the impact of allowing custom extensions in addition to
  standard ones is minimal.

       Arnd

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

[Arnd Bergmann]

On 11/5/18, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Nov 05, 2018 at 09:52:52AM +0100, Arnd Bergmann wrote:
>> > I fundamentally disagree with this… and think it should be the
>> > contrary.
>> >
>> > 1. The kernel shall support no vendor specific instructions whatsoever,
>> > period.
>>
>> I think what was meant above is
>>
>> 1. If a vendor extension requires kernel support, that support
>> must be able to be built into a kernel image without breaking support
>> for CPUs that do not have that extension, to allow building a single
>> kernel image that works on all CPUs.
>
> No.  This literally means no vendor extensions involving instructions
> or CSRs in the kernel.  They are fine for userspace, or for the M-mode
> code including impementation of the SBI, but not for the kernel.

I was trying to interpret what Vincent wrote, not what you wrote,
you were pretty clear already ;-)

With the stricter policy you suggest, we'd loose the ability to support
some extensions that might be common:

- an extension for user space that adds new registers that must be
  saved and restored on a task switch, e.g. FPU, DSP or NPU
  instructions. ARM supports several incompatible extensions like
  that in one kernel, and this is really ugly, but I suspect RISC-V
  will already need the same thing to support all combinations of
  standard extensions, so from a practical perspective it's not
  much different for custom extension, aside from the question
  how far you want to go to discourage custom extensions by
  requiring users to patch their kernels.

- A crypto instruction for a cipher that is used in the kernel
  for speeding up network or block data encryption.
  This would typically be a standalone loadable module, so
  the impact of allowing custom extensions in addition to
  standard ones is minimal.

       Arnd

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Nick Kossifidis]

Hello Vincent,

???? 2018-10-31 12:35, Vincent Chen ??????:
> 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.

So if I got this right, in your case you've added some CSRs 
(ucctlbeginaddr
and ucctlcommand) for marking regions as non-cacheable, and you provide 
your
own get_arch_dma_ops by overriding the default header files with your
vendor-specific ones.

Some things that are IMHO wrong with the proposed approach:

a) By directly modifying your custom CSRs, it means that we will need
compiler support in order to compile a kernel with your code in it. This
will break CI systems and will introduce various issues on testing and
reviewing your code. In general if we need custom toolchains to compile
the kernel, that may be unavailable (vendors will not always open source
their compiler support), we won't be able to maintain a decent level of
code quality in the tree. How can the maintainer push your code on the
repository if he/she can't even perform a basic compilation test ?

In contrast with ARM and others that have a standard set of possible
extensions (e.g. NEON, crc32 etc), and provide compiler support for 
those,
a similar approach is not valid for RISC-V. We could demand that vendors
add compiler support e.g. on gcc before submitting a patch with custom
assembly but I don't think this approach is feasible (one vendor's CSRs
or custom instructions may overlap with another's). I believe we should
just use SBI calls instead and let the firmware (and/or custom userspace
libraries) handle the custom CSRs/assembly instructions.

This is a concern also for lib/ and crypto/ where vendors might want to 
use
their own extensions for providing optimized assembly for their cores. 
It's
not a big deal to use SBI and handle vendor-specific stuff on firmware 
and/or
userspace, it's actually more flexible for the vendors since they can 
have
their own process for maintaining their firmware and releasing it in 
their
own terms/license etc. If we see that the SBI has too much overhead or 
is not
enough we can design it in a better way or extend it. It will still be
standard and easier to maintain than a fragmented ecosystem of mostly
unusable code, inside the mainline kernel.

In case we need to save/store registers related to a custom extension, I
guess we can also have an SBI call for saving/restoring custom registers
to/from S-managed memory and we should be ok. It should also be possible
to do any extra state handling in firmware if needed. I believe we can 
and
should avoid custom assembly on the kernel at all costs !


b) By using CONFIG_VENDOR_FOLDER_NAME you add a compile time dependency 
that
allows this kernel image to be built for a specific vendor. This is 
problematic
in different ways. At first it's not possible to have a kernel image 
that's
generic and can be used for all RISC-V systems. That's what distros want
and that's how they 've been working so far.

Also in case a vendor has many different boards with different 
implementation
details how will this approach help ? It would make more sense to have 
something
like arch/riscv/<platform>. Also have in mind that some extensions might 
be
available as IP to vendors so multiple vendors might use the same 
extension
made/licensed from another vendor. I think arch/riscv/<extension> is 
more
appropriate. Since we can use mvendorid/marchid/mimpid to identify that
at runtime maybe we can have some wrapper code that initializes a struct 
of
function pointers early on setup_arch(), allowing vendors to populate it
according to the available extensions in their hw, based on the above 
ids.

We can also just use device-tree for that and mark the used extensions 
there.
We can even pass extension-specific parameters this way (e.g. to save 
CSRs)
in case of extensions that can be parametrized on hw design phase (I'm
thinking of IPs sold from companies with licenses for unlocking the X 
feature
or for supporting up to Y channels/slots/instances/whatever).


In any case it's an interesting subject that we definitely need to think 
about,
thanks for bringing this up !

Regards,
Nick

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

[Nick Kossifidis]

Hello Vincent,

Στις 2018-10-31 12:35, Vincent Chen έγραψε:
> 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.

So if I got this right, in your case you've added some CSRs 
(ucctlbeginaddr
and ucctlcommand) for marking regions as non-cacheable, and you provide 
your
own get_arch_dma_ops by overriding the default header files with your
vendor-specific ones.

Some things that are IMHO wrong with the proposed approach:

a) By directly modifying your custom CSRs, it means that we will need
compiler support in order to compile a kernel with your code in it. This
will break CI systems and will introduce various issues on testing and
reviewing your code. In general if we need custom toolchains to compile
the kernel, that may be unavailable (vendors will not always open source
their compiler support), we won't be able to maintain a decent level of
code quality in the tree. How can the maintainer push your code on the
repository if he/she can't even perform a basic compilation test ?

In contrast with ARM and others that have a standard set of possible
extensions (e.g. NEON, crc32 etc), and provide compiler support for 
those,
a similar approach is not valid for RISC-V. We could demand that vendors
add compiler support e.g. on gcc before submitting a patch with custom
assembly but I don't think this approach is feasible (one vendor's CSRs
or custom instructions may overlap with another's). I believe we should
just use SBI calls instead and let the firmware (and/or custom userspace
libraries) handle the custom CSRs/assembly instructions.

This is a concern also for lib/ and crypto/ where vendors might want to 
use
their own extensions for providing optimized assembly for their cores. 
It's
not a big deal to use SBI and handle vendor-specific stuff on firmware 
and/or
userspace, it's actually more flexible for the vendors since they can 
have
their own process for maintaining their firmware and releasing it in 
their
own terms/license etc. If we see that the SBI has too much overhead or 
is not
enough we can design it in a better way or extend it. It will still be
standard and easier to maintain than a fragmented ecosystem of mostly
unusable code, inside the mainline kernel.

In case we need to save/store registers related to a custom extension, I
guess we can also have an SBI call for saving/restoring custom registers
to/from S-managed memory and we should be ok. It should also be possible
to do any extra state handling in firmware if needed. I believe we can 
and
should avoid custom assembly on the kernel at all costs !


b) By using CONFIG_VENDOR_FOLDER_NAME you add a compile time dependency 
that
allows this kernel image to be built for a specific vendor. This is 
problematic
in different ways. At first it's not possible to have a kernel image 
that's
generic and can be used for all RISC-V systems. That's what distros want
and that's how they 've been working so far.

Also in case a vendor has many different boards with different 
implementation
details how will this approach help ? It would make more sense to have 
something
like arch/riscv/<platform>. Also have in mind that some extensions might 
be
available as IP to vendors so multiple vendors might use the same 
extension
made/licensed from another vendor. I think arch/riscv/<extension> is 
more
appropriate. Since we can use mvendorid/marchid/mimpid to identify that
at runtime maybe we can have some wrapper code that initializes a struct 
of
function pointers early on setup_arch(), allowing vendors to populate it
according to the available extensions in their hw, based on the above 
ids.

We can also just use device-tree for that and mark the used extensions 
there.
We can even pass extension-specific parameters this way (e.g. to save 
CSRs)
in case of extensions that can be parametrized on hw design phase (I'm
thinking of IPs sold from companies with licenses for unlocking the X 
feature
or for supporting up to Y channels/slots/instances/whatever).


In any case it's an interesting subject that we definitely need to think 
about,
thanks for bringing this up !

Regards,
Nick

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Christoph Hellwig]

On Mon, Nov 05, 2018 at 09:39:29PM +0200, Nick Kossifidis wrote:
> a) By directly modifying your custom CSRs, it means that we will need
> compiler support in order to compile a kernel with your code in it. This
> will break CI systems and will introduce various issues on testing and
> reviewing your code. In general if we need custom toolchains to compile
> the kernel, that may be unavailable (vendors will not always open source
> their compiler support), we won't be able to maintain a decent level of
> code quality in the tree. How can the maintainer push your code on the
> repository if he/she can't even perform a basic compilation test ?

And that (besides avoiding the wild growth of extensions) is the major
reason why accepting vendor specific CSRs or instructions is a no-go.

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

[Christoph Hellwig]

On Mon, Nov 05, 2018 at 09:39:29PM +0200, Nick Kossifidis wrote:
> a) By directly modifying your custom CSRs, it means that we will need
> compiler support in order to compile a kernel with your code in it. This
> will break CI systems and will introduce various issues on testing and
> reviewing your code. In general if we need custom toolchains to compile
> the kernel, that may be unavailable (vendors will not always open source
> their compiler support), we won't be able to maintain a decent level of
> code quality in the tree. How can the maintainer push your code on the
> repository if he/she can't even perform a basic compilation test ?

And that (besides avoiding the wild growth of extensions) is the major
reason why accepting vendor specific CSRs or instructions is a no-go.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Christoph Hellwig]

On Mon, Nov 05, 2018 at 02:51:33PM +0100, Arnd Bergmann wrote:
> With the stricter policy you suggest, we'd loose the ability to support
> some extensions that might be common:
> 
> - an extension for user space that adds new registers that must be
>   saved and restored on a task switch, e.g. FPU, DSP or NPU
>   instructions. ARM supports several incompatible extensions like
>   that in one kernel, and this is really ugly, but I suspect RISC-V
>   will already need the same thing to support all combinations of
>   standard extensions, so from a practical perspective it's not
>   much different for custom extension, aside from the question
>   how far you want to go to discourage custom extensions by
>   requiring users to patch their kernels.

Palmer already explain that this is supposed to be handled by the
XS bit + SBI calls.  I'm personally not totally sold on the SBI call
and standard ways to save the state in the instruction set, similar
to modern x86 might be a better option, but that is something the
privileged spec working group will have to decide.

> - A crypto instruction for a cipher that is used in the kernel
>   for speeding up network or block data encryption.
>   This would typically be a standalone loadable module, so
>   the impact of allowing custom extensions in addition to
>   standard ones is minimal.

And that is a prime example for something that should never be vendor
specific.  If an instruction set extension is useful for something
entirely generic it should be standardized in a working group as an
extension.

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

[Christoph Hellwig]

On Mon, Nov 05, 2018 at 02:51:33PM +0100, Arnd Bergmann wrote:
> With the stricter policy you suggest, we'd loose the ability to support
> some extensions that might be common:
> 
> - an extension for user space that adds new registers that must be
>   saved and restored on a task switch, e.g. FPU, DSP or NPU
>   instructions. ARM supports several incompatible extensions like
>   that in one kernel, and this is really ugly, but I suspect RISC-V
>   will already need the same thing to support all combinations of
>   standard extensions, so from a practical perspective it's not
>   much different for custom extension, aside from the question
>   how far you want to go to discourage custom extensions by
>   requiring users to patch their kernels.

Palmer already explain that this is supposed to be handled by the
XS bit + SBI calls.  I'm personally not totally sold on the SBI call
and standard ways to save the state in the instruction set, similar
to modern x86 might be a better option, but that is something the
privileged spec working group will have to decide.

> - A crypto instruction for a cipher that is used in the kernel
>   for speeding up network or block data encryption.
>   This would typically be a standalone loadable module, so
>   the impact of allowing custom extensions in addition to
>   standard ones is minimal.

And that is a prime example for something that should never be vendor
specific.  If an instruction set extension is useful for something
entirely generic it should be standardized in a working group as an
extension.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Palmer Dabbelt]

On Sun, 04 Nov 2018 22:58:07 PST (-0800), vincentc at andestech.com wrote:
> On Fri, Nov 02, 2018 at 01:48:57AM +0800, Karsten Merker wrote:
>> On Wed, Oct 31, 2018 at 10:27:05AM -0700, Palmer Dabbelt wrote:
>> > On Wed, 31 Oct 2018 04:16:10 PDT (-0700), anup at brainfault.org wrote:
>> > > On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com> 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.
>>
> Many thanks for kinds of comments. I quickly synthesize the comments and
> list them as below.
> 1. The kernel image shall include all vendor-specific code.
> 2. This kernel image can only enable particular vendor-specific features
>    based on the CPU vendor in the running platform.
>    - The runtime probing mechanism can refer to arm32/arm64, powerpc,
>      IRQCHIP driver or CLOCKSOURCE driver
>    - For some cases, such as open-source projects, using CSR $mvendorid
>      to identify the compatibility is not appropriate.
> I think the above requirements are reasonable, but I have questions about
> the first requirement in practice. As far as I know, vendors are allowed
> to add specific instructions and CSRs which are incompatible with other
> vendors to their own ISA extensions. If I understand the first requirement
> correctly, it implies that we need a "super" RISC-V toolchain. This "super"
> RISC-V toolchain should recognize all kinds of vendor-specific instructions
> and CSRs, so that it can compile vendor sources into objects successfully;
> then it should recognize all kinds of vendor-specific relocations, so that
> it can link the objects successfully. Each of them is not trivial at the
> time of this proposal and in foreseeable future.
>
> If it will take a long time to complete this "super" toolchain, I suppose
> the infrastructure in this RFC might be a temporary solution before it is
> ready. This scheme does not suffer the compilation problems caused by the
> lack of the super toolchain because the selection of vendor-specific ISA
> extension is determined at compile time. In addition, the mechanism for
> checking compatibility at runtime ensures that the kernel with
> vendor-specific feature runs on CPUs of other vendors just like pure
> RISC-V kernel. In other words, this scheme, to some extent, satisfies the
> requirements that one kernel image is bootable everywhere.

I don't want anything in the kernel that can't be compiled by upstream GCC, as 
that will be a huge mess.  As far as I'm concerned, the best way to move 
forward is to figure out how each style of extension can be integrated.  Right 
now, what I see is:

* Performance counters.  Here we should be safe, as there's an ISA-mandated 
  space in which to put non-standard performance counters.  The support here is 
  just figuring out how to interpret the bits.  This naturally fits into our 
  current device-tree based mechanisms for probing hardware, and will be easy 
  to maintain in the kernel.
* Cache management.  It appears these are currently being described as 
  instructions, which means they won't compile by default.  Here I think the 
  best bet is to rely on the SBI, and if that's too slow to build a "SBI VDSO" 
  mechanism to accelerate the relevant bits.  It is a bit of a headache, but 
  we're not going to have anything standardized here quickly.

If those are the only two concerns then I think we're OK.  Are there any other 
extension you're worried about?

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

[Palmer Dabbelt]

On Sun, 04 Nov 2018 22:58:07 PST (-0800), vincentc@andestech.com wrote:
> On Fri, Nov 02, 2018 at 01:48:57AM +0800, Karsten Merker wrote:
>> On Wed, Oct 31, 2018 at 10:27:05AM -0700, Palmer Dabbelt wrote:
>> > On Wed, 31 Oct 2018 04:16:10 PDT (-0700), anup@brainfault.org wrote:
>> > > On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com> 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.
>>
> Many thanks for kinds of comments. I quickly synthesize the comments and
> list them as below.
> 1. The kernel image shall include all vendor-specific code.
> 2. This kernel image can only enable particular vendor-specific features
>    based on the CPU vendor in the running platform.
>    - The runtime probing mechanism can refer to arm32/arm64, powerpc,
>      IRQCHIP driver or CLOCKSOURCE driver
>    - For some cases, such as open-source projects, using CSR $mvendorid
>      to identify the compatibility is not appropriate.
> I think the above requirements are reasonable, but I have questions about
> the first requirement in practice. As far as I know, vendors are allowed
> to add specific instructions and CSRs which are incompatible with other
> vendors to their own ISA extensions. If I understand the first requirement
> correctly, it implies that we need a "super" RISC-V toolchain. This "super"
> RISC-V toolchain should recognize all kinds of vendor-specific instructions
> and CSRs, so that it can compile vendor sources into objects successfully;
> then it should recognize all kinds of vendor-specific relocations, so that
> it can link the objects successfully. Each of them is not trivial at the
> time of this proposal and in foreseeable future.
>
> If it will take a long time to complete this "super" toolchain, I suppose
> the infrastructure in this RFC might be a temporary solution before it is
> ready. This scheme does not suffer the compilation problems caused by the
> lack of the super toolchain because the selection of vendor-specific ISA
> extension is determined at compile time. In addition, the mechanism for
> checking compatibility at runtime ensures that the kernel with
> vendor-specific feature runs on CPUs of other vendors just like pure
> RISC-V kernel. In other words, this scheme, to some extent, satisfies the
> requirements that one kernel image is bootable everywhere.

I don't want anything in the kernel that can't be compiled by upstream GCC, as 
that will be a huge mess.  As far as I'm concerned, the best way to move 
forward is to figure out how each style of extension can be integrated.  Right 
now, what I see is:

* Performance counters.  Here we should be safe, as there's an ISA-mandated 
  space in which to put non-standard performance counters.  The support here is 
  just figuring out how to interpret the bits.  This naturally fits into our 
  current device-tree based mechanisms for probing hardware, and will be easy 
  to maintain in the kernel.
* Cache management.  It appears these are currently being described as 
  instructions, which means they won't compile by default.  Here I think the 
  best bet is to rely on the SBI, and if that's too slow to build a "SBI VDSO" 
  mechanism to accelerate the relevant bits.  It is a bit of a headache, but 
  we're not going to have anything standardized here quickly.

If those are the only two concerns then I think we're OK.  Are there any other 
extension you're worried about?

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Palmer Dabbelt]

On Mon, 05 Nov 2018 00:52:52 PST (-0800), Arnd Bergmann wrote:
> On 11/5/18, Christoph Hellwig <hch@infradead.org> wrote:
>> On Mon, Nov 05, 2018 at 02:58:07PM +0800, Vincent Chen wrote:
>>> Many thanks for kinds of comments. I quickly synthesize the comments and
>>> list them as below.
>>> 1. The kernel image shall include all vendor-specific code.
>>
>> I fundamentally disagree with this? and think it should be the contrary.
>>
>> 1. The kernel shall support no vendor specific instructions whatsoever,
>> period.
>
> I think what was meant above is
>
> 1. If a vendor extension requires kernel support, that support
> must be able to be built into a kernel image without breaking support
> for CPUs that do not have that extension, to allow building a single
> kernel image that works on all CPUs.

Yes.  I don't want anything that won't compile with upstream GCC, but I also 
don't want to have a Kconfig that says "make the kernel only work on $VENDOR's 
implementation".  I think this can be achieved, at least for the cases I've 
seen so far.

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

[Palmer Dabbelt]

On Mon, 05 Nov 2018 00:52:52 PST (-0800), Arnd Bergmann wrote:
> On 11/5/18, Christoph Hellwig <hch@infradead.org> wrote:
>> On Mon, Nov 05, 2018 at 02:58:07PM +0800, Vincent Chen wrote:
>>> Many thanks for kinds of comments. I quickly synthesize the comments and
>>> list them as below.
>>> 1. The kernel image shall include all vendor-specific code.
>>
>> I fundamentally disagree with this… and think it should be the contrary.
>>
>> 1. The kernel shall support no vendor specific instructions whatsoever,
>> period.
>
> I think what was meant above is
>
> 1. If a vendor extension requires kernel support, that support
> must be able to be built into a kernel image without breaking support
> for CPUs that do not have that extension, to allow building a single
> kernel image that works on all CPUs.

Yes.  I don't want anything that won't compile with upstream GCC, but I also 
don't want to have a Kconfig that says "make the kernel only work on $VENDOR's 
implementation".  I think this can be achieved, at least for the cases I've 
seen so far.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Arnd Bergmann]

On 11/7/18, Palmer Dabbelt <palmer@sifive.com> wrote:
> On Mon, 05 Nov 2018 00:52:52 PST (-0800), Arnd Bergmann wrote:
>> On 11/5/18, Christoph Hellwig <hch@infradead.org> wrote:
>>> On Mon, Nov 05, 2018 at 02:58:07PM +0800, Vincent Chen wrote:
>>>> Many thanks for kinds of comments. I quickly synthesize the comments
>>>> and
>>>> list them as below.
>>>> 1. The kernel image shall include all vendor-specific code.
>>>
>>> I fundamentally disagree with this? and think it should be the contrary.
>>>
>>> 1. The kernel shall support no vendor specific instructions whatsoever,
>>> period.
>>
>> I think what was meant above is
>>
>> 1. If a vendor extension requires kernel support, that support
>> must be able to be built into a kernel image without breaking support
>> for CPUs that do not have that extension, to allow building a single
>> kernel image that works on all CPUs.
>
> Yes.  I don't want anything that won't compile with upstream GCC, but I also
>
> don't want to have a Kconfig that says "make the kernel only work on
> $VENDOR's  implementation".  I think this can be achieved, at least for the
> cases I've seen so far.

I think over time, the implementations will diverge. Ignoring the question
of vendor extensions for the moment, you will definitely have to deal with
combinations of (future) standard extensions. I can see two ways of
doing that: Either each extension is a separate Kconfig option and you
have to know which one to enable or disable for a particular target,
or you list each platform separately with one Kconfig option, and
have Kconfig/Kbuild work out which features to enable or disable
based on that to get the fastest and most featureful kernel that still
works on all enabled platforms.

       Arnd

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

[Arnd Bergmann]

On 11/7/18, Palmer Dabbelt <palmer@sifive.com> wrote:
> On Mon, 05 Nov 2018 00:52:52 PST (-0800), Arnd Bergmann wrote:
>> On 11/5/18, Christoph Hellwig <hch@infradead.org> wrote:
>>> On Mon, Nov 05, 2018 at 02:58:07PM +0800, Vincent Chen wrote:
>>>> Many thanks for kinds of comments. I quickly synthesize the comments
>>>> and
>>>> list them as below.
>>>> 1. The kernel image shall include all vendor-specific code.
>>>
>>> I fundamentally disagree with this… and think it should be the contrary.
>>>
>>> 1. The kernel shall support no vendor specific instructions whatsoever,
>>> period.
>>
>> I think what was meant above is
>>
>> 1. If a vendor extension requires kernel support, that support
>> must be able to be built into a kernel image without breaking support
>> for CPUs that do not have that extension, to allow building a single
>> kernel image that works on all CPUs.
>
> Yes.  I don't want anything that won't compile with upstream GCC, but I also
>
> don't want to have a Kconfig that says "make the kernel only work on
> $VENDOR's  implementation".  I think this can be achieved, at least for the
> cases I've seen so far.

I think over time, the implementations will diverge. Ignoring the question
of vendor extensions for the moment, you will definitely have to deal with
combinations of (future) standard extensions. I can see two ways of
doing that: Either each extension is a separate Kconfig option and you
have to know which one to enable or disable for a particular target,
or you list each platform separately with one Kconfig option, and
have Kconfig/Kbuild work out which features to enable or disable
based on that to get the fastest and most featureful kernel that still
works on all enabled platforms.

       Arnd

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Vincent Chen]

On Wed, Nov 07, 2018 at 07:45:52AM +0800, Palmer Dabbelt wrote:
> On Sun, 04 Nov 2018 22:58:07 PST (-0800), vincentc at andestech.com wrote:
> > On Fri, Nov 02, 2018 at 01:48:57AM +0800, Karsten Merker wrote:
> >> On Wed, Oct 31, 2018 at 10:27:05AM -0700, Palmer Dabbelt wrote:
> >> > On Wed, 31 Oct 2018 04:16:10 PDT (-0700), anup at brainfault.org wrote:
> >> > > On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com> 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.
> >>
> > Many thanks for kinds of comments. I quickly synthesize the comments and
> > list them as below.
> > 1. The kernel image shall include all vendor-specific code.
> > 2. This kernel image can only enable particular vendor-specific features
> >    based on the CPU vendor in the running platform.
> >    - The runtime probing mechanism can refer to arm32/arm64, powerpc,
> >      IRQCHIP driver or CLOCKSOURCE driver
> >    - For some cases, such as open-source projects, using CSR $mvendorid
> >      to identify the compatibility is not appropriate.
> > I think the above requirements are reasonable, but I have questions about
> > the first requirement in practice. As far as I know, vendors are allowed
> > to add specific instructions and CSRs which are incompatible with other
> > vendors to their own ISA extensions. If I understand the first requirement
> > correctly, it implies that we need a "super" RISC-V toolchain. This "super"
> > RISC-V toolchain should recognize all kinds of vendor-specific instructions
> > and CSRs, so that it can compile vendor sources into objects successfully;
> > then it should recognize all kinds of vendor-specific relocations, so that
> > it can link the objects successfully. Each of them is not trivial at the
> > time of this proposal and in foreseeable future.
> >
> > If it will take a long time to complete this "super" toolchain, I suppose
> > the infrastructure in this RFC might be a temporary solution before it is
> > ready. This scheme does not suffer the compilation problems caused by the
> > lack of the super toolchain because the selection of vendor-specific ISA
> > extension is determined at compile time. In addition, the mechanism for
> > checking compatibility at runtime ensures that the kernel with
> > vendor-specific feature runs on CPUs of other vendors just like pure
> > RISC-V kernel. In other words, this scheme, to some extent, satisfies the
> > requirements that one kernel image is bootable everywhere.
> 
> I don't want anything in the kernel that can't be compiled by upstream GCC, as 
> that will be a huge mess.  As far as I'm concerned, the best way to move 
> forward is to figure out how each style of extension can be integrated.  Right 
> now, what I see is:
> 
> * Performance counters.  Here we should be safe, as there's an ISA-mandated 
>   space in which to put non-standard performance counters.  The support here is 
>   just figuring out how to interpret the bits.  This naturally fits into our 
>   current device-tree based mechanisms for probing hardware, and will be easy 
>   to maintain in the kernel.

Sure it is the case for the baseline PMU.  But the full function set of
perf is somehow limited in RISC-V as Alan mentioned in
Documentations/riscv/pmu.txt in his preliminary port.  He is planning to
share this topic and will provide some suggestions to enhance the
privileged spec at LPC next week.  I won't go into details to not go out
of focus, but I believe the vendor-specifics-go-SBI principle applies here.


> * Cache management.  It appears these are currently being described as 
>   instructions, which means they won't compile by default.  Here I think the 
>   best bet is to rely on the SBI, and if that's too slow to build a "SBI VDSO" 
>   mechanism to accelerate the relevant bits.  It is a bit of a headache, but 
>   we're not going to have anything standardized here quickly.
> 
> If those are the only two concerns then I think we're OK.  Are there any other 
> extension you're worried about?
No, currently only these two extensions.



Thanks everyone for all the comments. I quickly summarize these comments in
this round.
1. Kernel can support vendor specific features if the sources of these
   specific features can be compiled using upstream GCC.
   - The specific instructions and CSRs which cannot be compiled by
     upstream GCC shall rely on the SBI.
   - Using "SBI VDSO" mechanism to accelerate the SBI procedure 

2. arch/riscv/<extension> is more appropriate than arch/riscv/<vendor>
   - Some extensions might be available as IP to vendors, so multiple
     vendors might use the same extension made/licensed from another vendor

Basically, we glad to accept the above resolutions or comments except for
a tiny misunderstanding. The cache management in our extension is achieved
actually by CSR operation. Hence, we still hope the specific CSRs can be
supported from kernel now because the standard "SBI VDSO" scheme takes time
to develop. we propose a scheme that kernel can support the specific CSRs
and meet the criterion of compilation simultaneously. The upstream GCC
cannot recognize the specific CSRs because the CSRs access is made by the
CSRs name. If we change the access method from the CSR name to the CSR
number, the compilation problem of upstream GCC will not exist. This
scheme may suffer another problem, CSR number conflict. We think this
problem may be solved by runtime probing mechanism and vendor-defined
callbacks. The CSR number conflicts may occur in the following two
conditions. 
1. the specific CSR number conflicts with the vendors which do not support
   this extension. The runtime probing mechanism can avoid this case.
2. the specific CSR number conflicts with the vendors which support this
   extension. In this case, each vendor which supports this extension needs
   to define self-callbacks to access these specific CSRs through the
   vendor-defined CSR number.

If most people can accept the this scheme, we will include it in our next
version patch.


Regards
Vincent

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

[Vincent Chen]

On Wed, Nov 07, 2018 at 07:45:52AM +0800, Palmer Dabbelt wrote:
> On Sun, 04 Nov 2018 22:58:07 PST (-0800), vincentc@andestech.com wrote:
> > On Fri, Nov 02, 2018 at 01:48:57AM +0800, Karsten Merker wrote:
> >> On Wed, Oct 31, 2018 at 10:27:05AM -0700, Palmer Dabbelt wrote:
> >> > On Wed, 31 Oct 2018 04:16:10 PDT (-0700), anup@brainfault.org wrote:
> >> > > On Wed, Oct 31, 2018 at 4:06 PM Vincent Chen <vincentc@andestech.com> 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.
> >>
> > Many thanks for kinds of comments. I quickly synthesize the comments and
> > list them as below.
> > 1. The kernel image shall include all vendor-specific code.
> > 2. This kernel image can only enable particular vendor-specific features
> >    based on the CPU vendor in the running platform.
> >    - The runtime probing mechanism can refer to arm32/arm64, powerpc,
> >      IRQCHIP driver or CLOCKSOURCE driver
> >    - For some cases, such as open-source projects, using CSR $mvendorid
> >      to identify the compatibility is not appropriate.
> > I think the above requirements are reasonable, but I have questions about
> > the first requirement in practice. As far as I know, vendors are allowed
> > to add specific instructions and CSRs which are incompatible with other
> > vendors to their own ISA extensions. If I understand the first requirement
> > correctly, it implies that we need a "super" RISC-V toolchain. This "super"
> > RISC-V toolchain should recognize all kinds of vendor-specific instructions
> > and CSRs, so that it can compile vendor sources into objects successfully;
> > then it should recognize all kinds of vendor-specific relocations, so that
> > it can link the objects successfully. Each of them is not trivial at the
> > time of this proposal and in foreseeable future.
> >
> > If it will take a long time to complete this "super" toolchain, I suppose
> > the infrastructure in this RFC might be a temporary solution before it is
> > ready. This scheme does not suffer the compilation problems caused by the
> > lack of the super toolchain because the selection of vendor-specific ISA
> > extension is determined at compile time. In addition, the mechanism for
> > checking compatibility at runtime ensures that the kernel with
> > vendor-specific feature runs on CPUs of other vendors just like pure
> > RISC-V kernel. In other words, this scheme, to some extent, satisfies the
> > requirements that one kernel image is bootable everywhere.
> 
> I don't want anything in the kernel that can't be compiled by upstream GCC, as 
> that will be a huge mess.  As far as I'm concerned, the best way to move 
> forward is to figure out how each style of extension can be integrated.  Right 
> now, what I see is:
> 
> * Performance counters.  Here we should be safe, as there's an ISA-mandated 
>   space in which to put non-standard performance counters.  The support here is 
>   just figuring out how to interpret the bits.  This naturally fits into our 
>   current device-tree based mechanisms for probing hardware, and will be easy 
>   to maintain in the kernel.

Sure it is the case for the baseline PMU.  But the full function set of
perf is somehow limited in RISC-V as Alan mentioned in
Documentations/riscv/pmu.txt in his preliminary port.  He is planning to
share this topic and will provide some suggestions to enhance the
privileged spec at LPC next week.  I won't go into details to not go out
of focus, but I believe the vendor-specifics-go-SBI principle applies here.


> * Cache management.  It appears these are currently being described as 
>   instructions, which means they won't compile by default.  Here I think the 
>   best bet is to rely on the SBI, and if that's too slow to build a "SBI VDSO" 
>   mechanism to accelerate the relevant bits.  It is a bit of a headache, but 
>   we're not going to have anything standardized here quickly.
> 
> If those are the only two concerns then I think we're OK.  Are there any other 
> extension you're worried about?
No, currently only these two extensions.



Thanks everyone for all the comments. I quickly summarize these comments in
this round.
1. Kernel can support vendor specific features if the sources of these
   specific features can be compiled using upstream GCC.
   - The specific instructions and CSRs which cannot be compiled by
     upstream GCC shall rely on the SBI.
   - Using "SBI VDSO" mechanism to accelerate the SBI procedure 

2. arch/riscv/<extension> is more appropriate than arch/riscv/<vendor>
   - Some extensions might be available as IP to vendors, so multiple
     vendors might use the same extension made/licensed from another vendor

Basically, we glad to accept the above resolutions or comments except for
a tiny misunderstanding. The cache management in our extension is achieved
actually by CSR operation. Hence, we still hope the specific CSRs can be
supported from kernel now because the standard "SBI VDSO" scheme takes time
to develop. we propose a scheme that kernel can support the specific CSRs
and meet the criterion of compilation simultaneously. The upstream GCC
cannot recognize the specific CSRs because the CSRs access is made by the
CSRs name. If we change the access method from the CSR name to the CSR
number, the compilation problem of upstream GCC will not exist. This
scheme may suffer another problem, CSR number conflict. We think this
problem may be solved by runtime probing mechanism and vendor-defined
callbacks. The CSR number conflicts may occur in the following two
conditions. 
1. the specific CSR number conflicts with the vendors which do not support
   this extension. The runtime probing mechanism can avoid this case.
2. the specific CSR number conflicts with the vendors which support this
   extension. In this case, each vendor which supports this extension needs
   to define self-callbacks to access these specific CSRs through the
   vendor-defined CSR number.

If most people can accept the this scheme, we will include it in our next
version patch.


Regards
Vincent

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv