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

[Palmer Dabbelt]

On Tue, 23 May 2017 04:46:22 PDT (-0700), Arnd Bergmann wrote:
> 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.

Sorry, this snuck in from how we used to track the kernel.  It's been fixed.

>> 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()

We don't have any special instructions for inb/outb, but there is a special
fence to ensure ordering.

It looks like most of my candidates for patterning things after use
GENERIC_PCI_IOMAP, but I ended up setting GENERIC_IOMAP and things still at
least build and boot without any PCI

  https://github.com/riscv/riscv-linux/commit/c43c599b0dd9d15886c03a9dd179f8936b0cbb2e

I'll be sure to check if I broke PCI, as I don't actually know what's going on
here.

>> +config MMU
>> +       def_bool y
>
> Just a general question: has there been any interest in a no-MMU
> version?

One of the fun things about RISC-V is that there's at least some interest in
_everything_... :).  There hasn't been any serious interest, and I don't know
of anyone building systems where no-MMU Linux would run (our DDR phy is a lot
bigger than our MMU), but I wouldn't rule it out.

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

We can address 34 bits of physical address space on Sv32 (the 32-bit virtual
addressing scheme in RV32).  If this is a meaningful performance constraint
then we could always make this configurable.

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

Done

  https://github.com/riscv/riscv-linux/commit/921703e0a6639d10826e5bbad73f4998977dc244

> 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.

I think the theory is that most Linux-capable systems are going to have the A
extension, and that this is really there for educational/hobbyist use.

The actual implementation isn't that expensive: it's no-SMP-only, so it's just
a disable/enable interrupts that wraps a compare+exchange.  I think putting it
in the VDSO is actually the right thing to do: that way non-A user code will
still have reasonable performance.

I'll add it to my TODO list.

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

OK.  I'm a bit worried we need this to run DOOM, but I'll give everything a
shot without it and see what happens

  https://github.com/riscv/riscv-linux/commit/710b5bb81ab99cf1d49a74bb706f6aae179fc111