Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs

From: Olof Johansson
Date: Mon May 22 2017 - 21:27:27 EST


Hi,


On Mon, May 22, 2017 at 5:41 PM, Palmer Dabbelt <palmer@xxxxxxxxxxx> wrote:
> ---
> arch/riscv/.gitignore | 35 ++++
> arch/riscv/Kconfig | 300 +++++++++++++++++++++++++++++++++++
> arch/riscv/Makefile | 64 ++++++++
> arch/riscv/configs/riscv32_spike | 47 ++++++
> arch/riscv/configs/riscv64_freedom-u | 52 ++++++
> arch/riscv/configs/riscv64_qemu | 64 ++++++++
> arch/riscv/configs/riscv64_spike | 45 ++++++
> 7 files changed, 607 insertions(+)
> create mode 100644 arch/riscv/.gitignore
> create mode 100644 arch/riscv/Kconfig
> create mode 100644 arch/riscv/Makefile
> create mode 100644 arch/riscv/configs/riscv32_spike
> create mode 100644 arch/riscv/configs/riscv64_freedom-u
> create mode 100644 arch/riscv/configs/riscv64_qemu
> create mode 100644 arch/riscv/configs/riscv64_spike

Nearly all other platforms have _defconfig in the config names. It
might get a bit excessive to prepend riscv{32,64} to all of them
though. Most other platforms have shortened it to, for example,
spike_defconfig, spike64_defconfig, qemu_defconfig,
freedom-u_defconfig.

Not going to argue too much about the color of the shed here, but
using the _defconfig naming is recommended.

>
> diff --git a/arch/riscv/.gitignore b/arch/riscv/.gitignore
> new file mode 100644
> index 000000000000..376d06eb5d52
> --- /dev/null
> +++ b/arch/riscv/.gitignore
> @@ -0,0 +1,35 @@
> +# Now un-ignore all files.
> +!*
> +
> +# But then re-ignore the files listed in the Linux .gitignore
> +# Normal rules
> +#
> +.*
> +*.o
> +*.o.*
> +*.a
> +*.s
> +*.ko
> +*.so
> +*.so.dbg
> +*.mod.c
> +*.i
> +*.lst
> +*.symtypes
> +*.order
> +modules.builtin
> +*.elf
> +*.bin
> +*.gz
> +*.bz2
> +*.lzma
> +*.xz
> +*.lzo
> +*.patch
> +*.gcno

I don't think you need to do any of this, just inherit the global one
(by not having one here)?

> +
> +include/generated

This is already covered by the global .gitignore.

> +kernel/vmlinux.lds

And if needed this should be added to the arch/riscv/kernel/.gitignore
file instead.


> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> new file mode 100644
> index 000000000000..510ead1d3343
> --- /dev/null
> +++ b/arch/riscv/Kconfig
> @@ -0,0 +1,300 @@
> +#
> +# For a description of the syntax of this configuration file,
> +# see Documentation/kbuild/kconfig-language.txt.
> +#
> +
> +config RISCV
> + def_bool y
> + select OF
> + select OF_EARLY_FLATTREE
> + select OF_IRQ
> + select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
> + select ARCH_WANT_FRAME_POINTERS
> + select CLONE_BACKWARDS
> + select COMMON_CLK
> + select GENERIC_CLOCKEVENTS
> + select GENERIC_CPU_DEVICES
> + select GENERIC_IRQ_SHOW
> + select GENERIC_PCI_IOMAP
> + select GENERIC_STRNCPY_FROM_USER
> + select GENERIC_STRNLEN_USER
> + select GENERIC_SMP_IDLE_THREAD
> + select GENERIC_ATOMIC64 if !64BIT || !RV_ATOMIC
> + select ARCH_WANT_OPTIONAL_GPIOLIB
> + select HAVE_MEMBLOCK
> + select HAVE_DMA_API_DEBUG
> + select HAVE_DMA_CONTIGUOUS
> + select HAVE_GENERIC_DMA_COHERENT
> + select IRQ_DOMAIN
> + select NO_BOOTMEM
> + select RV_ATOMIC if SMP
> + select RV_SYSRISCV_ATOMIC if !RV_ATOMIC
> + select SPARSE_IRQ
> + select SYSCTL_EXCEPTION_TRACE
> + select HAVE_ARCH_TRACEHOOK
> + select MODULES_USE_ELF_RELA if MODULES
> +
> +config MMU
> + def_bool y
> +
> +# even on 32-bit, physical (and DMA) addresses are > 32-bits
> +config ARCH_PHYS_ADDR_T_64BIT
> + def_bool y
> +
> +config ARCH_DMA_ADDR_T_64BIT
> + def_bool y
> +
> +config STACKTRACE_SUPPORT
> + def_bool y
> +
> +config RWSEM_GENERIC_SPINLOCK
> + def_bool y
> +
> +config GENERIC_BUG
> + def_bool y
> + depends on BUG
> + select GENERIC_BUG_RELATIVE_POINTERS if 64BIT
> +
> +config GENERIC_BUG_RELATIVE_POINTERS
> + bool
> +
> +config GENERIC_CALIBRATE_DELAY
> + def_bool y
> +
> +config GENERIC_CSUM
> + def_bool y
> +
> +config GENERIC_HWEIGHT
> + def_bool y
> +
> +config PGTABLE_LEVELS
> + int
> + default 3 if 64BIT
> + default 2
> +
> +config HAVE_KPROBES
> + def_bool n
> +
> +config DMA_NOOP_OPS
> + def_bool y
> +
> +menu "Platform type"
> +
> +config SMP
> + bool "Symmetric Multi-Processing"
> + help
> + This enables support for systems with more than one CPU. If
> + you say N here, the kernel will run on single and
> + multiprocessor machines, but will use only one CPU of a
> + multiprocessor machine. If you say Y here, the kernel will run
> + on many, but not all, single processor machines. On a single
> + processor machine, the kernel will run faster if you say N
> + here.
> +
> + If you don't know what to do here, say N.
> +
> +config NR_CPUS
> + int "Maximum number of CPUs (2-32)"
> + range 2 32
> + depends on SMP
> + default "8"
> +
> +choice
> + prompt "CPU selection"
> + default CPU_RV_GENERIC
> +
> +config CPU_RV_GENERIC
> + bool "Generic RISC-V"
> + select CPU_SUPPORTS_32BIT_KERNEL
> + select CPU_SUPPORTS_64BIT_KERNEL

Is this even needed at this point? If the only CPU you can pick
supports this, you might as well not make it an option (CPU_RV_GENERIC
that is), and just make CPU_SUPPORTS_{32,64}BIT_KERNEL 'def_bool y'
for now.

> +
> +endchoice
> +
> +config PLIC
> + bool "Platform-Level Interrupt Controller"
> + default y
> + help
> + This enables support for the PLIC chip found in standard RISC-V
> + systems. The PLIC is the top-most interrupt controller found in
> + the system, connected directly to the core complex. All other
> + interrupt sources (MSI, GPIO, etc) are subordinate to the PLIC.
> +
> + If you don't know what to do here, say Y.
> +
> +config CPU_SUPPORTS_32BIT_KERNEL
> + bool
> +config CPU_SUPPORTS_64BIT_KERNEL
> + bool
> +
> +config SBI_CONSOLE
> + tristate "SBI console support"
> + select TTY
> + default y

Usually you end up having a DRIVER_FOO and DRIVER_FOO_CONSOLE option
to enable registering it as console.

Also, unless there's strong reason to keep it under arch/, it should
probably go under drivers/tty/.

> +config RVC
> + bool "Use compressed instructions (RV32C or RV64C)"
> + default n

What does "use" here mean? Use during build, or allow userspace to use them?

> +
> +config RV_ATOMIC
> + bool "Use atomic memory instructions (RV32A or RV64A)"
> + default y

Same for this.

> +config RV_SYSRISCV_ATOMIC
> + bool "Include support for atomic operation syscalls"
> + default n
> + help
> + If atomic memory instructions are present, i.e.,
> + CONFIG_RV_ATOMIC, this includes support for the syscall that
> + provides atomic accesses. This is only useful to run
> + binaries that require atomic access but were compiled with
> + -mno-atomic.
> +
> + If CONFIG_RV_ATOMIC is unset, this option is mandatory.

If it's mandatory then Kconfig language should make it so.

> +config RV_PUM
> + def_bool y
> + prompt "Protect User Memory" if EXPERT
> + ---help---
> + Protect User Memory (PUM) prevents the kernel from inadvertently
> + accessing user-space memory. There is a small performance cost
> + and kernel size increase if this is enabled.
> +
> + If unsure, say Y.
> +
> +endmenu
> +
> +menu "Kernel type"
> +
> +choice
> + prompt "Kernel code model"
> + default 64BIT
> +
> +config 32BIT
> + bool "32-bit kernel"
> + depends on CPU_SUPPORTS_32BIT_KERNEL
> + help
> + Select this option to build a 32-bit kernel.
> +
> +config 64BIT
> + bool "64-bit kernel"
> + depends on CPU_SUPPORTS_64BIT_KERNEL
> + help
> + Select this option to build a 64-bit kernel.
> +
> +endchoice
> +
[...]


-Olof