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

From: Arnd Bergmann
Date: Tue May 23 2017 - 07:46:29 EST


On Tue, May 23, 2017 at 2:41 AM, 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
>
> 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

This doesn't seem to belong here: There is no reason for riscv
to be different from all other architectures. Is something wrong
with the top-level .gitignore? If so, we should just fix it there.

> 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

You normally don't want GENERIC_PCI_IOMAP, unless your
inb()/outb() uses other instructions than your readl()/writel()

> +config MMU
> + def_bool y

Just a general question: has there been any interest in a no-MMU
version?

> +# 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

Are you required to use 64-bit addressing for RAM on 32-bit
architectures though? Using 32-bit dma_addr_t and phys_addr_t
when possible makes some code noticeably more efficient.

> +config PGTABLE_LEVELS
> + int
> + default 3 if 64BIT
> + default 2

With 2-level page tables, you usually can't address much more
than 32-bit physical memory anyway, so I'd guess that most
32-bit chips would actually put their RAM under the 4GB boundary.

> +config RV_ATOMIC
> + bool "Use atomic memory instructions (RV32A or RV64A)"
> + default y
> +
> +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.

Just express this in Kconfig terms to prevent misconfiguration:

config RV_SYSRISCV_ATOMIC
bool "Include support for atomic operation syscalls" if RV_ATOMIC
default !RV_ATOMIC

I wonder what the cost would be of always providing the syscalls
for compatibility. This is also something worth putting into a VDSO
instead of exposing the syscall:

That way, user space that is built with -mno-atomic can call into
the vdso, which depending on the hardware support will perform
the atomic operation directly or enter the syscall.

> +config PCI_DOMAINS
> + def_bool PCI
> +
> +config PCI_DOMAINS_GENERIC
> + def_bool PCI
> +
> +config PCI_SYSCALL
> + def_bool PCI

I don't think you want PCI_SYSCALL

Arnd