Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1647525 > unrolled thread
| Started by | Palmer Dabbelt <palmer@dabbelt.com> |
|---|---|
| First post | 2017-05-23 02:50 +0200 |
| Last post | 2017-06-07 15:20 +0200 |
| Articles | 20 on this page of 108 — 14 participants |
Back to article view | Back to linux.kernel
RISC-V Linux Port v1 Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-23 02:50 +0200
[PATCH 5/7] RISC-V: arch/riscv/lib Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-23 02:50 +0200
Re: [PATCH 5/7] RISC-V: arch/riscv/lib Geert Uytterhoeven <geert@linux-m68k.org> - 2017-05-23 12:50 +0200
Re: [PATCH 5/7] RISC-V: arch/riscv/lib Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-24 00:10 +0200
Re: [PATCH 5/7] RISC-V: arch/riscv/lib Arnd Bergmann <arnd@arndb.de> - 2017-05-23 13:30 +0200
Re: [PATCH 5/7] RISC-V: arch/riscv/lib Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-25 04:00 +0200
Re: [PATCH 5/7] RISC-V: arch/riscv/lib Arnd Bergmann <arnd@arndb.de> - 2017-05-26 11:20 +0200
Re: [PATCH 5/7] RISC-V: arch/riscv/lib Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-06 07:00 +0200
Re: [PATCH 5/7] RISC-V: arch/riscv/lib Arnd Bergmann <arnd@arndb.de> - 2017-06-06 11:40 +0200
Re: [PATCH 5/7] RISC-V: arch/riscv/lib Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-06 23:00 +0200
Re: [PATCH 5/7] RISC-V: arch/riscv/lib Arnd Bergmann <arnd@arndb.de> - 2017-06-07 09:40 +0200
[PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-23 02:50 +0200
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Olof Johansson <olof@lixom.net> - 2017-05-23 03:30 +0200
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Randy Dunlap <rdunlap@infradead.org> - 2017-05-23 03:40 +0200
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-23 06:50 +0200
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-23 06:50 +0200
Re: [patches] Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Olof Johansson <olof@lixom.net> - 2017-05-23 07:20 +0200
Re: [patches] Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-05-23 23:10 +0200
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Olof Johansson <olof@lixom.net> - 2017-05-23 07:30 +0200
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-23 17:30 +0200
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Geert Uytterhoeven <geert@linux-m68k.org> - 2017-05-23 13:00 +0200
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-25 04:00 +0200
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Arnd Bergmann <arnd@arndb.de> - 2017-05-23 13:50 +0200
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-27 03:30 +0200
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Arnd Bergmann <arnd@arndb.de> - 2017-05-29 13:20 +0200
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-06 07:00 +0200
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Arnd Bergmann <arnd@arndb.de> - 2017-06-06 11:30 +0200
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-06 22:40 +0200
[PATCH 3/7] RISC-V: Device Tree Documentation Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-23 02:50 +0200
Re: [PATCH 3/7] RISC-V: Device Tree Documentation Arnd Bergmann <arnd@arndb.de> - 2017-05-23 14:10 +0200
Re: [PATCH 3/7] RISC-V: Device Tree Documentation Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-27 03:30 +0200
Re: RISC-V Linux Port v1 Olof Johansson <olof@lixom.net> - 2017-05-23 03:20 +0200
Re: RISC-V Linux Port v1 Randy Dunlap <rdunlap@infradead.org> - 2017-05-23 03:30 +0200
Re: RISC-V Linux Port v1 Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-23 05:40 +0200
Re: RISC-V Linux Port v1 Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-23 05:40 +0200
Re: RISC-V Linux Port v1 Tobias Klauser <tklauser@distanz.ch> - 2017-05-23 08:50 +0200
Re: RISC-V Linux Port v1 Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-23 17:50 +0200
Re: RISC-V Linux Port v1 Randy Dunlap <rdunlap@infradead.org> - 2017-05-23 04:20 +0200
Re: RISC-V Linux Port v1 Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-23 06:50 +0200
Re: [PATCH 6/7] RISC-V: arch/riscv/kernel Olof Johansson <olof@lixom.net> - 2017-05-23 04:20 +0200
Re: [PATCH 6/7] RISC-V: arch/riscv/kernel Palmer Dabbelt <palmer@dabbelt.com> - 2017-05-25 04:00 +0200
Re: [PATCH 6/7] RISC-V: arch/riscv/kernel Arnd Bergmann <arnd@arndb.de> - 2017-05-25 22:00 +0200
Re: [PATCH 6/7] RISC-V: arch/riscv/kernel Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-06 07:00 +0200
Re: [PATCH 6/7] RISC-V: arch/riscv/kernel Arnd Bergmann <arnd@arndb.de> - 2017-06-06 11:10 +0200
Re: [PATCH 6/7] RISC-V: arch/riscv/kernel Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-06 22:40 +0200
Re: [PATCH 4/7] RISC-V: arch/riscv/include Arnd Bergmann <arnd@arndb.de> - 2017-05-23 15:00 +0200
Re: [PATCH 4/7] RISC-V: arch/riscv/include Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-05-23 23:30 +0200
Re: [PATCH 4/7] RISC-V: arch/riscv/include Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-03 04:10 +0200
Re: [PATCH 4/7] RISC-V: arch/riscv/include Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-01 03:00 +0200
Re: [PATCH 4/7] RISC-V: arch/riscv/include Arnd Bergmann <arnd@arndb.de> - 2017-06-01 11:10 +0200
Re: [PATCH 4/7] RISC-V: arch/riscv/include Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-06 07:00 +0200
Re: [PATCH 4/7] RISC-V: arch/riscv/include Arnd Bergmann <arnd@arndb.de> - 2017-06-06 11:00 +0200
Re: [PATCH 4/7] RISC-V: arch/riscv/include Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-06 21:10 +0200
Re: [PATCH 6/7] RISC-V: arch/riscv/kernel Arnd Bergmann <arnd@arndb.de> - 2017-05-23 15:40 +0200
Re: [PATCH 6/7] RISC-V: arch/riscv/kernel Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-03 02:00 +0200
Re: [PATCH 6/7] RISC-V: arch/riscv/kernel Arnd Bergmann <arnd@arndb.de> - 2017-06-06 11:10 +0200
Re: [PATCH 6/7] RISC-V: arch/riscv/kernel Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-06 22:40 +0200
Re: [PATCH 6/7] RISC-V: arch/riscv/kernel Pavel Machek <pavel@ucw.cz> - 2017-05-25 19:10 +0200
Re: [PATCH 6/7] RISC-V: arch/riscv/kernel Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-03 05:40 +0200
[PATCH 17/17] RISC-V: Makefile and Kconfig Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
[PATCH 03/17] base: fix order of OF initialization Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
Re: [PATCH 03/17] base: fix order of OF initialization Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:10 +0200
Re: [PATCH 03/17] base: fix order of OF initialization Mark Rutland <mark.rutland@arm.com> - 2017-06-07 11:40 +0200
[PATCH 10/17] irqchip: New RISC-V PLIC Driver Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
Re: [PATCH 10/17] irqchip: New RISC-V PLIC Driver Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:20 +0200
Re: [PATCH 10/17] irqchip: New RISC-V PLIC Driver Arnd Bergmann <arnd@arndb.de> - 2017-06-07 10:00 +0200
Re: [PATCH 10/17] irqchip: New RISC-V PLIC Driver Marc Zyngier <marc.zyngier@arm.com> - 2017-06-07 13:00 +0200
[PATCH 12/17] tty: New RISC-V SBI Console Driver Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
Re: [PATCH 12/17] tty: New RISC-V SBI Console Driver Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:20 +0200
Re: [PATCH 12/17] tty: New RISC-V SBI Console Driver Arnd Bergmann <arnd@arndb.de> - 2017-06-07 10:00 +0200
[PATCH 07/17] lib: Add shared copies of some GCC library routines Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
[PATCH 02/17] pcie-xilinx: add missing 5th legacy interrupt Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
Re: [PATCH 02/17] pcie-xilinx: add missing 5th legacy interrupt Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:20 +0200
Re: [PATCH 02/17] pcie-xilinx: add missing 5th legacy interrupt Marc Zyngier <marc.zyngier@arm.com> - 2017-06-07 11:30 +0200
[PATCH 04/17] Documentation: atomic_ops.txt is core-api/atomic_ops.rst Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
Re: [PATCH 04/17] Documentation: atomic_ops.txt is core-api/atomic_ops.rst Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:20 +0200
Re: [PATCH 04/17] Documentation: atomic_ops.txt is core-api/atomic_ops.rst Will Deacon <will.deacon@arm.com> - 2017-06-07 11:30 +0200
[PATCH 05/17] MAINTAINERS: Add RISC-V Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
[PATCH 14/17] RISC-V: lib files Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
RISC-V Linux Port v2 Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
[PATCH 06/17] pci: Add generic pcibios_{fixup_bus,align_resource} Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
Re: [PATCH 06/17] pci: Add generic pcibios_{fixup_bus,align_resource} Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:30 +0200
Re: [PATCH 06/17] pci: Add generic pcibios_{fixup_bus,align_resource} Arnd Bergmann <arnd@arndb.de> - 2017-06-07 10:10 +0200
[PATCH 08/17] dts: include documentation for the RISC-V interrupt controllers Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
Re: [PATCH 08/17] dts: include documentation for the RISC-V interrupt controllers Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:20 +0200
Re: [PATCH 08/17] dts: include documentation for the RISC-V interrupt controllers Mark Rutland <mark.rutland@arm.com> - 2017-06-07 12:20 +0200
[PATCH 11/17] irqchip: RISC-V Local Interrupt Controller Driver Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
Re: [PATCH 11/17] irqchip: RISC-V Local Interrupt Controller Driver Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:20 +0200
[PATCH 01/17] drivers: support PCIe in RISCV Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
Re: [PATCH 01/17] drivers: support PCIe in RISCV Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:20 +0200
Re: [PATCH 01/17] drivers: support PCIe in RISCV Christoph Hellwig <hch@infradead.org> - 2017-06-07 16:30 +0200
[PATCH 15/17] RISC-V: Add mm subdirectory Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
[PATCH 09/17] clocksource/timer-riscv: New RISC-V Clocksource Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
Re: [PATCH 09/17] clocksource/timer-riscv: New RISC-V Clocksource Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:20 +0200
Re: [PATCH 09/17] clocksource/timer-riscv: New RISC-V Clocksource Arnd Bergmann <arnd@arndb.de> - 2017-06-07 09:30 +0200
Re: [PATCH 09/17] clocksource/timer-riscv: New RISC-V Clocksource Marc Zyngier <marc.zyngier@arm.com> - 2017-06-07 11:50 +0200
Re: RISC-V Linux Port v2 David Howells <dhowells@redhat.com> - 2017-06-07 09:30 +0200
Re: [PATCH 13/17] RISC-V: Add include subdirectory Arnd Bergmann <arnd@arndb.de> - 2017-06-07 10:20 +0200
Re: RISC-V Linux Port v2 Will Deacon <will.deacon@arm.com> - 2017-06-07 11:30 +0200
Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 14:00 +0200
Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 14:30 +0200
Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 14:10 +0200
Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 14:30 +0200
Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 14:40 +0200
Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 15:00 +0200
Re: [PATCH 13/17] RISC-V: Add include subdirectory Will Deacon <will.deacon@arm.com> - 2017-06-07 15:20 +0200
Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 14:50 +0200
Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 15:20 +0200
Page 3 of 6 — ← Prev page 1 2 [3] 4 5 6 Next page →
| From | Palmer Dabbelt <palmer@dabbelt.com> |
|---|---|
| Date | 2017-05-25 04:00 +0200 |
| Subject | Re: [PATCH 6/7] RISC-V: arch/riscv/kernel |
| Message-ID | <tKQed-1E7-5@gated-at.bofh.it> |
| In reply to | #1647581 |
On Mon, 22 May 2017 19:11:35 PDT (-0700), olof@lixom.net wrote:
> On Mon, May 22, 2017 at 5:41 PM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> What's missing from this patchset (ideally) is a good writeup under
> DOcumentation/ on expectations of system state (and/or configuration)
> upon entry of the kernel. For comparison, see the arm64 documentation
> where they were quite specific in this.
We don't have a spec written for this yet, but one is being written. Is it OK
if I wait until there's a spec?
> This patch is also pushing size limits, and is getting unwieldy to
> comment on. I'll point out a few things below with plenty of snipped
> out lines.
I'm also going to start dropping diffs, as this is very big.
>> +++ b/arch/riscv/kernel/asm-offsets.c
>> @@ -0,0 +1,113 @@
>> +/*
>> + * Copyright (C) 2012 Regents of the University of California
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation, version 2.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
>> + * NON INFRINGEMENT. See the GNU General Public License for
>> + * more details.
>
> Hmm, I haven't seen these terms used often, but they seem to exist
> around the tree in a few places. arch/tile is littered with them.
>
> I am not a lawyer, but I can't seem any reference to "good title" in
> the GPLv2 text.
>
> Rather than having to go through the process of figuring out if this
> license header is acceptable or not, you might find it easier to just
> go with something more established.
This almost certainly came from Tilera: I stole our ptrace from there becuase
that was the ISA I understood best, and that header must have proliferated
everywhere else.
I've changed it to a header I copied from ARM
https://github.com/riscv/riscv-linux/commit/ccc4f51b40b28adf01b14ed6578bf26dc02f1425
>> +static int __populate_cache_leaves(unsigned int cpu)
>> +{
>> + struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
>> + struct cacheinfo *this_leaf = this_cpu_ci->info_list;
>> + struct device_node *np = of_cpu_device_node_get(cpu);
>> + int levels = 1, level = 1;
>> +
>> + if (of_property_read_bool(np, "cache-size")) ci_leaf_init(this_leaf++, np, CACHE_TYPE_UNIFIED, level);
>> + if (of_property_read_bool(np, "i-cache-size")) ci_leaf_init(this_leaf++, np, CACHE_TYPE_INST, level);
>> + if (of_property_read_bool(np, "d-cache-size")) ci_leaf_init(this_leaf++, np, CACHE_TYPE_DATA, level);
>
> Please run checkpatch, kernel coding style doesn't use one-line ifs
> (here nor elsewhere).
I went through and fixed many of the checkpatch messages. The ones that are
left fall into the following categories:
* Lots of uses of BUG/BUG_ON instead of WARN_ON. Lots of these are in boot
code, but some of them can probably be fixed.
* Parens around single-statement __asm__ macros. For these I also get a
message when they're wrapped in "do {} while (0)", so I'm not sure what else
to do.
* Parens around macros like "#define RISCV_PTR .dword". These can't have
parens because they go directly to the assembler, so I'm considering this a
false-positive.
* Warnings about volatile in function declarations in bitops.h. These are
copied from other architectures. There were a handful of other volatiles
that I fixed,, but I think these should stay.
* Definitions like ARCH_HAS_SETUP_ADDITIONAL_PAGES, these are also present in
other architectures.
* We added new typedefs, I can remove these if that's a problem. They're
there to match our other code (bootloader and simulator).
* A handful of lines over 80 characters that I think are onerous to break any
more.
* Some warnings about printk() not having a KERN_ prefix. I fixed a handful
of these, but the remaining ones I don't know how to fix (in show_regs, for
example, where arm64 also has them).
* Extern declarations in C files, all of which link to symbols in assembly or
linker scripts. These were copied from other architectures.
There's also a bunch of false positives:
* The spelling of SEPC, which is correct (Supervisor Exception Program
Counter).
* Fall-through warnings, probably getting confused by the break looking like
"break; \" (they're in macros).
I'll make another pass on these before a v2 patch set.
>> +/* Return -1 if not a valid hart */
>> +int riscv_of_processor_hart(struct device_node *node)
>> +{
>> + const char *isa, *status;
>> + u32 hart;
>> +
>> + if (!of_device_is_compatible(node, "riscv")) return -1;
>> + if (of_property_read_u32(node, "reg", &hart) || hart >= NR_CPUS) return -1;
>> + if (of_property_read_string(node, "status", &status) || strcmp(status, "okay")) return -1;
>> + if (of_property_read_string(node, "riscv,isa", &isa) || isa[0] != 'r' || isa[1] != 'v') return -1;
>> +
>> + return hart;
>> +}
>
> We usually prefer to see real -E<foo> returns instead of -1 in the kernel.
Makes sense. https://github.com/riscv/riscv-linux/commit/10ef72b2aa16b2b69f9f349cffc06d12e183a56e
>> +asmlinkage void __irq_entry do_IRQ(unsigned int cause, struct pt_regs *regs)
>> +{
>> + struct pt_regs *old_regs = set_irq_regs(regs);
>> + irq_enter();
>> +
>> + /* There are three classes of interrupt: timer, software, and
>> + external devices. We dispatch between them here. External
>> + device interrupts use the generic IRQ mechanisms. */
>> + switch (cause) {
>> + case INTERRUPT_CAUSE_TIMER:
>> + riscv_timer_interrupt();
>> + break;
>> + case INTERRUPT_CAUSE_SOFTWARE:
>> + riscv_software_interrupt();
>> + break;
>> + default: {
>> + struct irq_domain *domain = per_cpu(riscv_irq_data, smp_processor_id()).domain;
>
> Move this up to top of function and remove the { } wrap, please.
https://github.com/riscv/riscv-linux/commit/17879136caa05ed9f686736b3343f4b2063920ab
>> +static void riscv_irq_enable(struct irq_data *d)
>> +{
>> + struct riscv_irq_data *data = irq_data_get_irq_chip_data(d);
>> + atomic_long_or((1 << (long)d->hwirq), &per_cpu(riscv_early_sie, data->hart));
>
> This is a bit dense to get into without a few words of how it's
> expected to work.
OK, how does this look? https://github.com/riscv/riscv-linux/commit/112fd2d882c2363508a660061da558d772a4ff0b
>> +static int riscv_intc_init(struct device_node *node, struct device_node *parent)
>> +{
>> + int hart;
>> +
>> + if (parent) return 0; // should have no interrupt parent
>> +
>> + if ((hart = riscv_of_processor_hart(node->parent)) >= 0) {
>
> Common pattern in kernel is to detect error instead:
> hart = riscv_of_processor_hart(node->parent);
> if (hart < 0) {
> <from your else side here>
> return 0;
> }
> <body if your if statement here>
>
> return 0;
OK. I've fixed this one here
https://github.com/riscv/riscv-linux/commit/5f48d9ba0d3cd19dc0bf95f66370de4cfcd84cca
I'll try to remember to fix any others that I come across.
>> diff --git a/arch/riscv/kernel/pci.c b/arch/riscv/kernel/pci.c
>> new file mode 100644
>> index 000000000000..4191a5ffdd67
>> --- /dev/null
>> +++ b/arch/riscv/kernel/pci.c
>> @@ -0,0 +1,36 @@
>> +/*
>> + * Code borrowed from arch/arm64/kernel/pci.c
>
> So, you should add recursive reference from there (i.e. powerpc).
>
> But in the end, there's essentially no code in this file. :)
Well, now there's more copyright notices than lines of code... :)
https://github.com/riscv/riscv-linux/commit/5ad312f755935319fdbb6739377b400ea81cd2ec
>> +#define MAX_DEVICES 1024 // 0 is reserved
>
> Seems like an odd comment to have here (and should probably not go at
> the end of the line)
Device 0 in the PLIC is reserved to mean "no device", which means "MAX_DEVICES"
is a bit of an odd name (there can only be MAX_DEVICES-1 devices). I've added
a larger comment to describe this better.
https://github.com/riscv/riscv-linux/commit/9d16413051dd86db0fb8a792f3d5f05ce788d145
>> +#define PLIC_HART_CONTEXT(data, i) (struct plic_hart_context *)((char*)data->reg + HART_BASE + HART_SIZE*i)
>> +#define PLIC_ENABLE_CONTEXT(data, i) (struct plic_enable_context *)((char*)data->reg + ENABLE_BASE + ENABLE_SIZE*i)
>> +#define PLIC_PRIORITY(data) (struct plic_priority *)((char *)data->reg + PRIORITY_BASE)
>
> Since you have typecasting and stuff here, small static inlines with
> appropriate return types seems slightly tidier.
OK. https://github.com/riscv/riscv-linux/commit/c416649b203276d944be595d87caf042c998727f
>> +static void plic_chained_handle_irq(struct irq_desc *desc)
>> +{
>> + struct plic_handler *handler = irq_desc_get_handler_data(desc);
>> + struct irq_chip *chip = irq_desc_get_chip(desc);
>
> Whitespace.
These were fixed along with the other checkpatch messages.
> [wrapping up review of this patch at this point to keep size down]
>
>
> -Olof
Thanks for the comments! I'll batch these up into a v2 when I'm done with
everyone's comments from this round.
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-05-25 22:00 +0200 |
| Subject | Re: [PATCH 6/7] RISC-V: arch/riscv/kernel |
| Message-ID | <tL75n-40J-1@gated-at.bofh.it> |
| In reply to | #1650112 |
On Thu, May 25, 2017 at 3:59 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> On Mon, 22 May 2017 19:11:35 PDT (-0700), olof@lixom.net wrote:
> * Parens around single-statement __asm__ macros. For these I also get a
> message when they're wrapped in "do {} while (0)", so I'm not sure what else
> to do.
I would generally recommend using inline functions for those, and only do
macros when you need them.
> * Parens around macros like "#define RISCV_PTR .dword". These can't have
> parens because they go directly to the assembler, so I'm considering this a
> false-positive.
agreed
> * Warnings about volatile in function declarations in bitops.h. These are
> copied from other architectures. There were a handful of other volatiles
> that I fixed,, but I think these should stay.
Agreed, bitops.h is one of the few headers that should use 'volatile'.
> * Definitions like ARCH_HAS_SETUP_ADDITIONAL_PAGES, these are also present in
> other architectures.
What is the warning here? I would assume that you should leave this
unchanged as well.
> * We added new typedefs, I can remove these if that's a problem. They're
> there to match our other code (bootloader and simulator).
It depends. What typedefs are those? Removing the typedefs in both
the kernel and the other code that uses the same types is likely the
best option here.
> * A handful of lines over 80 characters that I think are onerous to break any
> more.
Right, don't worry about it too much, and use common sense for this
warning.
> * Some warnings about printk() not having a KERN_ prefix. I fixed a handful
> of these, but the remaining ones I don't know how to fix (in show_regs, for
> example, where arm64 also has them).
KERN_CONT
> * Extern declarations in C files, all of which link to symbols in assembly or
> linker scripts. These were copied from other architectures.
I would try to fix those by using a header even if there is only one user.
I'd actually like to get a compile-time warning for those in the long run,
maybe with 'make W=1', so better don't introduce new ones.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Palmer Dabbelt <palmer@dabbelt.com> |
|---|---|
| Date | 2017-06-06 07:00 +0200 |
| Subject | Re: [PATCH 6/7] RISC-V: arch/riscv/kernel |
| Message-ID | <tPeKZ-460-15@gated-at.bofh.it> |
| In reply to | #1650780 |
On Thu, 25 May 2017 12:51:54 PDT (-0700), Arnd Bergmann wrote:
> On Thu, May 25, 2017 at 3:59 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> On Mon, 22 May 2017 19:11:35 PDT (-0700), olof@lixom.net wrote:
>
>> * Parens around single-statement __asm__ macros. For these I also get a
>> message when they're wrapped in "do {} while (0)", so I'm not sure what else
>> to do.
>
> I would generally recommend using inline functions for those, and only do
> macros when you need them.
We've tried to avoid new macros, so these are mostly from places where other
architectures use macros (like mb) or where we need to use CPP token pasting
(like __op_bit). Should I change things like mb?
>> * Parens around macros like "#define RISCV_PTR .dword". These can't have
>> parens because they go directly to the assembler, so I'm considering this a
>> false-positive.
>
> agreed
>
>> * Warnings about volatile in function declarations in bitops.h. These are
>> copied from other architectures. There were a handful of other volatiles
>> that I fixed,, but I think these should stay.
>
> Agreed, bitops.h is one of the few headers that should use 'volatile'.
>
>> * Definitions like ARCH_HAS_SETUP_ADDITIONAL_PAGES, these are also present in
>> other architectures.
>
> What is the warning here? I would assume that you should leave this
> unchanged as well.
ERROR: #define of 'ARCH_HAS_SETUP_ADDITIONAL_PAGES' is wrong - use Kconfig variables or standard guards instead
#2533: FILE: arch/riscv/include/asm/elf.h:79:
+#define ARCH_HAS_SETUP_ADDITIONAL_PAGES
>> * We added new typedefs, I can remove these if that's a problem. They're
>> there to match our other code (bootloader and simulator).
>
> It depends. What typedefs are those? Removing the typedefs in both
> the kernel and the other code that uses the same types is likely the
> best option here.
OK, I'll add it to my TODO list.
>> * A handful of lines over 80 characters that I think are onerous to break any
>> more.
>
> Right, don't worry about it too much, and use common sense for this
> warning.
>
>> * Some warnings about printk() not having a KERN_ prefix. I fixed a handful
>> of these, but the remaining ones I don't know how to fix (in show_regs, for
>> example, where arm64 also has them).
>
> KERN_CONT
https://github.com/riscv/riscv-linux/commit/98e8fe9cb19d495180a9be03a0aa48c0183dd5be
>> * Extern declarations in C files, all of which link to symbols in assembly or
>> linker scripts. These were copied from other architectures.
>
> I would try to fix those by using a header even if there is only one user.
> I'd actually like to get a compile-time warning for those in the long run,
> maybe with 'make W=1', so better don't introduce new ones.
OK, I'll fix them.
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-06-06 11:10 +0200 |
| Subject | Re: [PATCH 6/7] RISC-V: arch/riscv/kernel |
| Message-ID | <tPiEV-6IR-9@gated-at.bofh.it> |
| In reply to | #1658350 |
On Tue, Jun 6, 2017 at 6:56 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> On Thu, 25 May 2017 12:51:54 PDT (-0700), Arnd Bergmann wrote:
>> On Thu, May 25, 2017 at 3:59 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>> On Mon, 22 May 2017 19:11:35 PDT (-0700), olof@lixom.net wrote:
>>
>>> * Definitions like ARCH_HAS_SETUP_ADDITIONAL_PAGES, these are also present in
>>> other architectures.
>>
>> What is the warning here? I would assume that you should leave this
>> unchanged as well.
>
> ERROR: #define of 'ARCH_HAS_SETUP_ADDITIONAL_PAGES' is wrong - use Kconfig variables or standard guards instead
> #2533: FILE: arch/riscv/include/asm/elf.h:79:
> +#define ARCH_HAS_SETUP_ADDITIONAL_PAGES
Ok, you can definitely ignore this one. The warning is meant to prevent adding
new macros like that, but the macro already exists in the other architectures,
and I see no point in converting them all into a CONFIG_ symbol.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Palmer Dabbelt <palmer@dabbelt.com> |
|---|---|
| Date | 2017-06-06 22:40 +0200 |
| Subject | Re: [PATCH 6/7] RISC-V: arch/riscv/kernel |
| Message-ID | <tPtqF-5fp-15@gated-at.bofh.it> |
| In reply to | #1658500 |
On Tue, 06 Jun 2017 02:03:51 PDT (-0700), Arnd Bergmann wrote: > On Tue, Jun 6, 2017 at 6:56 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote: >> On Thu, 25 May 2017 12:51:54 PDT (-0700), Arnd Bergmann wrote: >>> On Thu, May 25, 2017 at 3:59 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote: >>>> On Mon, 22 May 2017 19:11:35 PDT (-0700), olof@lixom.net wrote: >>> >>>> * Definitions like ARCH_HAS_SETUP_ADDITIONAL_PAGES, these are also present in >>>> other architectures. >>> >>> What is the warning here? I would assume that you should leave this >>> unchanged as well. >> >> ERROR: #define of 'ARCH_HAS_SETUP_ADDITIONAL_PAGES' is wrong - use Kconfig variables or standard guards instead >> #2533: FILE: arch/riscv/include/asm/elf.h:79: >> +#define ARCH_HAS_SETUP_ADDITIONAL_PAGES > > Ok, you can definitely ignore this one. The warning is meant to prevent adding > new macros like that, but the macro already exists in the other architectures, > and I see no point in converting them all into a CONFIG_ symbol. Sounds good.
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-05-23 15:00 +0200 |
| Subject | Re: [PATCH 4/7] RISC-V: arch/riscv/include |
| Message-ID | <tKhzP-3jV-9@gated-at.bofh.it> |
| In reply to | #1647525 |
On Tue, May 23, 2017 at 2:41 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> +/**
> + * atomic_read - read atomic variable
> + * @v: pointer of type atomic_t
> + *
> + * Atomically reads the value of @v.
> + */
> +static inline int atomic_read(const atomic_t *v)
> +{
> + return *((volatile int *)(&(v->counter)));
> +}
> +/**
> + * atomic_set - set atomic variable
> + * @v: pointer of type atomic_t
> + * @i: required value
> + *
> + * Atomically sets the value of @v to @i.
> + */
> +static inline void atomic_set(atomic_t *v, int i)
> +{
> + v->counter = i;
> +}
These commonly use READ_ONCE() and WRITE_ONCE,
I'd recommend doing the same here to be on the safe side.
> +/**
> + * atomic64_read - read atomic64 variable
> + * @v: pointer of type atomic64_t
> + *
> + * Atomically reads the value of @v.
> + */
> +static inline s64 atomic64_read(const atomic64_t *v)
> +{
> + return *((volatile long *)(&(v->counter)));
> +}
> +
> +/**
> + * atomic64_set - set atomic64 variable
> + * @v: pointer to type atomic64_t
> + * @i: required value
> + *
> + * Atomically sets the value of @v to @i.
> + */
> +static inline void atomic64_set(atomic64_t *v, s64 i)
> +{
> + v->counter = i;
> +}
same here
> diff --git a/arch/riscv/include/asm/bug.h b/arch/riscv/include/asm/bug.h
> new file mode 100644
> index 000000000000..10d894ac3137
> --- /dev/null
> +++ b/arch/riscv/include/asm/bug.h
> @@ -0,0 +1,81 @@
> +/*
>
> +#ifndef _ASM_RISCV_BUG_H
> +#define _ASM_RISCV_BUG_H
> +#ifdef CONFIG_GENERIC_BUG
> +#define __BUG_INSN _AC(0x00100073, UL) /* sbreak */
Please have a look at the modifications I did for !CONFIG_BUG
on x86, arm and arm64. It's generally better to define BUG to a
trap even when CONFIG_BUG is disabled, otherwise you run
into undefined behavior in some code, and gcc will print annoying
warnings about that.
> +#ifndef _ASM_RISCV_CACHE_H
> +#define _ASM_RISCV_CACHE_H
> +
> +#define L1_CACHE_SHIFT 6
> +
> +#define L1_CACHE_BYTES (1 << L1_CACHE_SHIFT)
Is this the only valid cache line size on riscv, or just the largest
one that is allowed?
> +
> +static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
> +{
> + return (dma_addr_t)paddr;
> +}
> +
> +static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dev_addr)
> +{
> + return (phys_addr_t)dev_addr;
> +}
What do you need these for? If possible, try to remove them.
> +static inline void dma_cache_sync(struct device *dev, void *vaddr, size_t size, enum dma_data_direction dir)
> +{
> + /*
> + * RISC-V is cache-coherent, so this is mostly a no-op.
> + * However, we do need to ensure that dma_cache_sync()
> + * enforces order, hence the mb().
> + */
> + mb();
> +}
Do you even support any drivers that use
dma_alloc_noncoherent()/dma_cache_sync()?
I would guess you can just leave this out.
> diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
> new file mode 100644
> index 000000000000..d942555a7a08
> --- /dev/null
> +++ b/arch/riscv/include/asm/io.h
> @@ -0,0 +1,36 @@
> +#ifndef _ASM_RISCV_IO_H
> +#define _ASM_RISCV_IO_H
> +
> +#include <asm-generic/io.h>
I would recommend providing your own {read,write}{b,w,l,q}{,_relaxed}
helpers using inline assembly, to prevent the compiler for breaking
up accesses into byte accesses.
Also, most architectures require to some synchronization after a
non-relaxed readl() to prevent prefetching of DMA buffers, and
before a writel() to flush write buffers when a DMA gets triggered.
> +#ifdef __KERNEL__
> +
> +#ifdef CONFIG_MMU
> +
> +extern void __iomem *ioremap(phys_addr_t offset, unsigned long size);
> +
> +#define ioremap_nocache(addr, size) ioremap((addr), (size))
> +#define ioremap_wc(addr, size) ioremap((addr), (size))
> +#define ioremap_wt(addr, size) ioremap((addr), (size))
Is this a hard architecture limitation? Normally you really want
write-combined access on frame buffer memory and a few other
cases for performance reasons, and ioremap_wc() gets used
for by memremap() for addressing RAM in some cases, and you
normally don't want to have PTEs for the same memory using
cached and uncached page flags
> diff --git a/arch/riscv/include/asm/serial.h b/arch/riscv/include/asm/serial.h
> new file mode 100644
> index 000000000000..d783dbe80a4b
> --- /dev/null
> +++ b/arch/riscv/include/asm/serial.h
> @@ -0,0 +1,43 @@
> +/*
> + * Copyright (C) 2014 Regents of the University of California
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation, version 2.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT. See the GNU General Public License for
> + * more details.
> + */
> +
> +#ifndef _ASM_RISCV_SERIAL_H
> +#define _ASM_RISCV_SERIAL_H
> +
> +/*
> + * FIXME: interim serial support for riscv-qemu
> + *
> + * Currently requires that the emulator itself create a hole at addresses
> + * 0x3f8 - 0x3ff without looking through page tables.
This sounds like something we want to fix in qemu and not have in the
mainline kernel. In particular, something seems really wrong if your
inb()/outb() get remapped to physical CPU address 0+offset.
> diff --git a/arch/riscv/include/asm/setup.h b/arch/riscv/include/asm/setup.h
> new file mode 100644
> index 000000000000..e457854e9988
> --- /dev/null
> +++ b/arch/riscv/include/asm/setup.h
> @@ -0,0 +1,20 @@
> +/*
> + * Copyright (C) 2012 Regents of the University of California
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation, version 2.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT. See the GNU General Public License for
> + * more details.
> + */
> +
> +#ifndef _ASM_RISCV_SETUP_H
> +#define _ASM_RISCV_SETUP_H
> +
> +#include <asm-generic/setup.h>
> +
> +#endif /* _ASM_RISCV_SETUP_H */
Can you remove this file and add it to asm/Kbuild as generic-y instead?
> +/*
> + * low level task data that entry.S needs immediate access to
> + * - this struct should fit entirely inside of one cache line
> + * - this struct resides at the bottom of the supervisor stack
> + * - if the members of this struct changes, the assembly constants
> + * in asm-offsets.c must be updated accordingly
> + */
> +struct thread_info {
> + struct task_struct *task; /* main task structure */
> + unsigned long flags; /* low level flags */
> + __u32 cpu; /* current CPU */
> + int preempt_count; /* 0 => preemptable, <0 => BUG */
> + mm_segment_t addr_limit;
> +};
Please see 15f4eae70d36 ("x86: Move thread_info into task_struct")
and try to do the same.
> +#else /* !CONFIG_MMU */
> +
> +static inline void flush_tlb_all(void)
> +{
> + BUG();
> +}
> +
> +static inline void flush_tlb_mm(struct mm_struct *mm)
> +{
> + BUG();
> +}
The NOMMU support is rather incomplete and CONFIG_MMU is
hard-enabled, so I'd just drop any !CONFIG_MMU #ifdefs.
> diff --git a/arch/riscv/include/uapi/asm/Kbuild b/arch/riscv/include/uapi/asm/Kbuild
> new file mode 100644
> index 000000000000..276b6dae745c
> --- /dev/null
> +++ b/arch/riscv/include/uapi/asm/Kbuild
> @@ -0,0 +1,10 @@
> +# UAPI Header export list
> +include include/uapi/asm-generic/Kbuild.asm
> +
> +header-y += auxvec.h
> +header-y += bitsperlong.h
> +header-y += byteorder.h
> +header-y += ptrace.h
> +header-y += sigcontext.h
> +header-y += siginfo.h
> +header-y += unistd.h
Please see
fcc8487d477a ("uapi: export all headers under uapi directories")
and adapt the file accordingly
> +#include <asm-generic/unistd.h>
> +
> +#define __NR_sysriscv __NR_arch_specific_syscall
> +#ifndef __riscv_atomic
> +__SYSCALL(__NR_sysriscv, sys_sysriscv)
> +#endif
Please make this a straight cmpxchg syscall and remove the multiplexer.
Why does the definition depend on __riscv_atomic rather than the
Kconfig symbol?
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Benjamin Herrenschmidt <benh@kernel.crashing.org> |
|---|---|
| Date | 2017-05-23 23:30 +0200 |
| Subject | Re: [PATCH 4/7] RISC-V: arch/riscv/include |
| Message-ID | <tKpxo-AI-21@gated-at.bofh.it> |
| In reply to | #1647997 |
On Tue, 2017-05-23 at 14:55 +0200, Arnd Bergmann wrote:
> > +
> > +#include <asm-generic/io.h>
>
> I would recommend providing your own {read,write}{b,w,l,q}{,_relaxed}
> helpers using inline assembly, to prevent the compiler for breaking
> up accesses into byte accesses.
>
> Also, most architectures require to some synchronization after a
> non-relaxed readl() to prevent prefetching of DMA buffers, and
> before a writel() to flush write buffers when a DMA gets triggered.
Right, I was about to comment on that one.
The question Palmer is about the ordering semantics of non-cached
storage.
What kind of ordering is provided architecturally ? Especially
between cachable and non-cachable loads and stores ?
Also, you have PCIe right ? What is the behaviour of MSIs ?
Does your HW provide a guarantee that in the case of a series of DMA
writes to memory by a device followed by an MSI, the CPU getting the
MSI will only get it after all the previous DMA writes have reached
coherency ? (Unlike LSIs where the driver is required to do an MMIO
read from the device, MSIs are expected to be ordered with data).
Another things with the read*() accessors. It's not uncommon for
a driver to do:
writel(1, reset_reg);
readl(reset_reg); /* flush posted writes */
udelay(10);
writel(0, reset_reg);
Now, in the above case, what can typically happen if you aren't careful
is that the readl which is intended to "push" the previous writel, will
not actually do its job because the return value hasn't been "consumed"
by the processor. Thus, the CPU will stick that on some kind of load
queue and won't actually wait for the return value before hitting the
delay loop.
Thus you might end up in a situation where the writel of 1 to the
device is itself reaching the device way after you started the delay
loop, and thus end up violating the delay requirement of the HW.
On powerpc we solve that by using a special instruction construct
inside the read* accessors that prevents the CPU from executing
subsequent instructions until the read value has been returned.
You may want to consider something similar.
Cheers,
Ben.
[toc] | [prev] | [next] | [standalone]
| From | Palmer Dabbelt <palmer@dabbelt.com> |
|---|---|
| Date | 2017-06-03 04:10 +0200 |
| Subject | Re: [PATCH 4/7] RISC-V: arch/riscv/include |
| Message-ID | <tO6FQ-QT-9@gated-at.bofh.it> |
| In reply to | #1648763 |
On Tue, 23 May 2017 14:23:50 PDT (-0700), benh@kernel.crashing.org wrote:
> On Tue, 2017-05-23 at 14:55 +0200, Arnd Bergmann wrote:
>> > +
>> > +#include <asm-generic/io.h>
>>
>> I would recommend providing your own {read,write}{b,w,l,q}{,_relaxed}
>> helpers using inline assembly, to prevent the compiler for breaking
>> up accesses into byte accesses.
>>
>> Also, most architectures require to some synchronization after a
>> non-relaxed readl() to prevent prefetching of DMA buffers, and
>> before a writel() to flush write buffers when a DMA gets triggered.
>
> Right, I was about to comment on that one.
Well, you're both correct: what was there just isn't correct. Our
implementations were safe because they don't have aggressive MMIO systems, but
that won't remain true for long.
I've gone ahead and added a proper IO implementation patterned on arm64. It'll
be part of the v2 patch set. Here's the bulk of the patch, if you're curious
https://github.com/riscv/riscv-linux/commit/e200fa29a69451ef4d575076e4d2af6b7877b1fa
> The question Palmer is about the ordering semantics of non-cached
> storage.
>
> What kind of ordering is provided architecturally ? Especially
> between cachable and non-cachable loads and stores ?
The memory model on RISC-V is pretty weak. Without fences there are no
ordering constraints. We provide 2 "ordering spaces" (an odd name I just made
up, there's one address space): the IO space and the regular memory space. The
base RISC-V ordering primitive is a fence, which takes a predecessor set and
successor set. Fences can look like
fence IORW,IORW
where the left side is the predecessor set and the right side is the successor
set. The fence enforces ordering between any operations in the two sets: all
operations in the predecessor set must be globally visible before any operation
in the successor set becomes visible anywhere.
For example, if you're emitting a DMA transaction you'd have to do something
like
build_message_in_memory()
fence w,o
set_control_register()
with the fence ensuring all the memory writes are visible before the control
register write.
More information can be found in the ISA manuals
https://github.com/riscv/riscv-isa-manual/releases/download/riscv-user-2.2/riscv-spec-v2.2.pdf
https://github.com/riscv/riscv-isa-manual/releases/download/riscv-priv-1.10/riscv-privileged-v1.10.pdf
> Also, you have PCIe right ? What is the behaviour of MSIs ?
>
> Does your HW provide a guarantee that in the case of a series of DMA
> writes to memory by a device followed by an MSI, the CPU getting the
> MSI will only get it after all the previous DMA writes have reached
> coherency ? (Unlike LSIs where the driver is required to do an MMIO
> read from the device, MSIs are expected to be ordered with data).
We do have PCIe, but I'm not particularly familiar with it as I haven't spent
any time hacking on our PCIe hardware or driver. My understanding here is that
PCIe defines that MSIs must not be reordered before the DMA writes, so the
implementation is required to enforce this ordering. Thus it's a problem for
the PCIe controller implementation (which isn't covered by RISC-V) and therefor
doesn't need any ordering enforced by the driver.
If I'm correct in the assumption that the hardware is required to enforce these
ordering constraints then I think this isn't a RISC-V issue. I'll go bug our
PCIe guys to make sure everything is kosher in that case, but it's an ordering
constraint that is possible to enforce in our coherence protocol so it's not a
fundamental problem (and just an implementation one at that).
If the hardware isn't required to enforce the ordering, then we'll need a fence
before handling the data.
> Another things with the read*() accessors. It's not uncommon for
> a driver to do:
>
> writel(1, reset_reg);
> readl(reset_reg); /* flush posted writes */
> udelay(10);
> writel(0, reset_reg);
>
> Now, in the above case, what can typically happen if you aren't careful
> is that the readl which is intended to "push" the previous writel, will
> not actually do its job because the return value hasn't been "consumed"
> by the processor. Thus, the CPU will stick that on some kind of load
> queue and won't actually wait for the return value before hitting the
> delay loop.
>
> Thus you might end up in a situation where the writel of 1 to the
> device is itself reaching the device way after you started the delay
> loop, and thus end up violating the delay requirement of the HW.
>
> On powerpc we solve that by using a special instruction construct
> inside the read* accessors that prevents the CPU from executing
> subsequent instructions until the read value has been returned.
>
> You may want to consider something similar.
Ooh, that's a fun one :). I bugged Andrew (the ISA wizard), and this might
require some clarification in the ISA manual. The code we emit will look
something like
fence io,o
st 1, RESET_REG
ld RESET_REG
fence o,io
loop:
rdtime
blt loop
fence io,o
st 0, RESET_REG
Since the fences just enforce ordering between the loads and stores, there's
nothing that prevents the processor from releasing the store before the delay
loop completes. I think we might be safe here because you're not allowed to
make speculative writes visible, but arguably that's not a speculative write
because you can predict the timer will keep increasing. The distinction is
somewhat academic, though: I'm not sure what purpose that very specific sort of
predictor would have aside from breaking existing code.
This issue of "how is time ordered" has come up a handful of times, most of
which don't have the loop. The idea of adding the result rdtime instruction to
the IO space. I've opened a spec bug
https://github.com/riscv/riscv-isa-manual/issues/78
Thanks!
[toc] | [prev] | [next] | [standalone]
| From | Palmer Dabbelt <palmer@dabbelt.com> |
|---|---|
| Date | 2017-06-01 03:00 +0200 |
| Subject | Re: [PATCH 4/7] RISC-V: arch/riscv/include |
| Message-ID | <tNmD0-3T7-1@gated-at.bofh.it> |
| In reply to | #1647997 |
On Tue, 23 May 2017 05:55:15 PDT (-0700), Arnd Bergmann wrote:
> On Tue, May 23, 2017 at 2:41 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> +/**
>> + * atomic_read - read atomic variable
>> + * @v: pointer of type atomic_t
>> + *
>> + * Atomically reads the value of @v.
>> + */
>> +static inline int atomic_read(const atomic_t *v)
>> +{
>> + return *((volatile int *)(&(v->counter)));
>> +}
>> +/**
>> + * atomic_set - set atomic variable
>> + * @v: pointer of type atomic_t
>> + * @i: required value
>> + *
>> + * Atomically sets the value of @v to @i.
>> + */
>> +static inline void atomic_set(atomic_t *v, int i)
>> +{
>> + v->counter = i;
>> +}
>
> These commonly use READ_ONCE() and WRITE_ONCE,
> I'd recommend doing the same here to be on the safe side.
Makes sense. https://github.com/riscv/riscv-linux/commit/77647f9e4dccab68c69a212a63c9efe1db2b7b1c
>> diff --git a/arch/riscv/include/asm/bug.h b/arch/riscv/include/asm/bug.h
>> new file mode 100644
>> index 000000000000..10d894ac3137
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/bug.h
>> @@ -0,0 +1,81 @@
>> +/*
>>
>> +#ifndef _ASM_RISCV_BUG_H
>> +#define _ASM_RISCV_BUG_H
>
>> +#ifdef CONFIG_GENERIC_BUG
>> +#define __BUG_INSN _AC(0x00100073, UL) /* sbreak */
>
> Please have a look at the modifications I did for !CONFIG_BUG
> on x86, arm and arm64. It's generally better to define BUG to a
> trap even when CONFIG_BUG is disabled, otherwise you run
> into undefined behavior in some code, and gcc will print annoying
> warnings about that.
OK, seems like a good thing. https://github.com/riscv/riscv-linux/commit/67db001653614c6555424b3812d7edfba12a6d4c
>> +#ifndef _ASM_RISCV_CACHE_H
>> +#define _ASM_RISCV_CACHE_H
>> +
>> +#define L1_CACHE_SHIFT 6
>> +
>> +#define L1_CACHE_BYTES (1 << L1_CACHE_SHIFT)
>
> Is this the only valid cache line size on riscv, or just the largest
> one that is allowed?
The RISC-V ISA manual doesn't actually mention caches anywhere, so there's no
restriction on L1 cache line size (we tried to keep microarchitecture out of
the ISA specification). We provide the actual cache parameters as part of the
device tree, but it looks like this needs to be known staticly in some places
so we can't use that everywhere.
We could always make this a Kconfig parameter.
>> +
>> +static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
>> +{
>> + return (dma_addr_t)paddr;
>> +}
>> +
>> +static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dev_addr)
>> +{
>> + return (phys_addr_t)dev_addr;
>> +}
>
> What do you need these for? If possible, try to remove them.
>
>> +static inline void dma_cache_sync(struct device *dev, void *vaddr, size_t size, enum dma_data_direction dir)
>> +{
>> + /*
>> + * RISC-V is cache-coherent, so this is mostly a no-op.
>> + * However, we do need to ensure that dma_cache_sync()
>> + * enforces order, hence the mb().
>> + */
>> + mb();
>> +}
>
> Do you even support any drivers that use
> dma_alloc_noncoherent()/dma_cache_sync()?
>
> I would guess you can just leave this out.
These must have been vestigial code, they appear safe to remove.
https://github.com/riscv/riscv-linux/commit/d1c88783d5ff66464a25173f7a4af139f0ebf5e2
>> diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
>> new file mode 100644
>> index 000000000000..d942555a7a08
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/io.h
>> @@ -0,0 +1,36 @@
>
>> +#ifndef _ASM_RISCV_IO_H
>> +#define _ASM_RISCV_IO_H
>> +
>> +#include <asm-generic/io.h>
>
> I would recommend providing your own {read,write}{b,w,l,q}{,_relaxed}
> helpers using inline assembly, to prevent the compiler for breaking
> up accesses into byte accesses.
>
> Also, most architectures require to some synchronization after a
> non-relaxed readl() to prevent prefetching of DMA buffers, and
> before a writel() to flush write buffers when a DMA gets triggered.
Makes sense. These were all OK on existing implementations (as there's no
writable PMAs, so all MMIO regions are strictly ordered), but that's not
actually what the RISC-V ISA says. I patterned this on arm64
https://github.com/riscv/riscv-linux/commit/e200fa29a69451ef4d575076e4d2af6b7877b1fa
where I think the only odd thing is our definition of mmiowb
+/* IO barriers. These only fence on the IO bits because they're only required
+ * to order device access. We're defining mmiowb because our AMO instructions
+ * (which are used to implement locks) don't specify ordering. From Chapter 7
+ * of v2.2 of the user ISA:
+ * "The bits order accesses to one of the two address domains, memory or I/O,
+ * depending on which address domain the atomic instruction is accessing. No
+ * ordering constraint is implied to accesses to the other domain, and a FENCE
+ * instruction should be used to order across both domains."
+ */
+
+#define __iormb() __asm__ __volatile__ ("fence i,io" : : : "memory");
+#define __iowmb() __asm__ __volatile__ ("fence io,o" : : : "memory");
+
+#define mmiowb() __asm__ __volatile__ ("fence io,io" : : : "memory");
which I think is correct.
>> +#ifdef __KERNEL__
>> +
>> +#ifdef CONFIG_MMU
>> +
>> +extern void __iomem *ioremap(phys_addr_t offset, unsigned long size);
>> +
>> +#define ioremap_nocache(addr, size) ioremap((addr), (size))
>> +#define ioremap_wc(addr, size) ioremap((addr), (size))
>> +#define ioremap_wt(addr, size) ioremap((addr), (size))
>
> Is this a hard architecture limitation? Normally you really want
> write-combined access on frame buffer memory and a few other
> cases for performance reasons, and ioremap_wc() gets used
> for by memremap() for addressing RAM in some cases, and you
> normally don't want to have PTEs for the same memory using
> cached and uncached page flags
This is currently an architecture limitation. In RISC-V these properties are
known as PMAs (Physical Memory Attributes). While the supervisor spec mentions
PMAs, it doesn't provide a mechanism to read or write them so they are
essentially unspecified. PMAs will be properly defined as part of the platform
specification, which isn't written yet.
>> diff --git a/arch/riscv/include/asm/serial.h b/arch/riscv/include/asm/serial.h
>> new file mode 100644
>> index 000000000000..d783dbe80a4b
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/serial.h
>> @@ -0,0 +1,43 @@
>> +/*
>> + * Copyright (C) 2014 Regents of the University of California
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation, version 2.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
>> + * NON INFRINGEMENT. See the GNU General Public License for
>> + * more details.
>> + */
>> +
>> +#ifndef _ASM_RISCV_SERIAL_H
>> +#define _ASM_RISCV_SERIAL_H
>> +
>> +/*
>> + * FIXME: interim serial support for riscv-qemu
>> + *
>> + * Currently requires that the emulator itself create a hole at addresses
>> + * 0x3f8 - 0x3ff without looking through page tables.
>
> This sounds like something we want to fix in qemu and not have in the
> mainline kernel. In particular, something seems really wrong if your
> inb()/outb() get remapped to physical CPU address 0+offset.
Sorry, we had some hacks floating around for QEMU from before we actually had
any devices interfaces working (ie, before device tree and proper MMIO
support). I'll go through and drop these before v2.
>> diff --git a/arch/riscv/include/asm/setup.h b/arch/riscv/include/asm/setup.h
>> new file mode 100644
>> index 000000000000..e457854e9988
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/setup.h
>> @@ -0,0 +1,20 @@
>> +/*
>> + * Copyright (C) 2012 Regents of the University of California
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation, version 2.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
>> + * NON INFRINGEMENT. See the GNU General Public License for
>> + * more details.
>> + */
>> +
>> +#ifndef _ASM_RISCV_SETUP_H
>> +#define _ASM_RISCV_SETUP_H
>> +
>> +#include <asm-generic/setup.h>
>> +
>> +#endif /* _ASM_RISCV_SETUP_H */
>
> Can you remove this file and add it to asm/Kbuild as generic-y instead?
Yes. https://github.com/riscv/riscv-linux/commit/8f35ce93bd1230ac0cb4aa92e5673c61e50dd862
>> +/*
>> + * low level task data that entry.S needs immediate access to
>> + * - this struct should fit entirely inside of one cache line
>> + * - this struct resides at the bottom of the supervisor stack
>> + * - if the members of this struct changes, the assembly constants
>> + * in asm-offsets.c must be updated accordingly
>> + */
>> +struct thread_info {
>> + struct task_struct *task; /* main task structure */
>> + unsigned long flags; /* low level flags */
>> + __u32 cpu; /* current CPU */
>> + int preempt_count; /* 0 => preemptable, <0 => BUG */
>> + mm_segment_t addr_limit;
>> +};
>
> Please see 15f4eae70d36 ("x86: Move thread_info into task_struct")
> and try to do the same.
OK, here's my attempt
https://github.com/riscv/riscv-linux/commit/c618553e7aa65c85564a5d0a868ec7e6cf634afd
Since there's some actual meat, I left a commit message (these are more just
notes for me for my v2, I'll be squashing everything)
"
This is patterned more off the arm64 move than the x86 one, since we
still need to have at least addr_limit to emulate FS.
The patch itself changes sscratch from holding SP to holding TP, which
contains a pointer to task_struct. thread_info must be at a 0 offset
from task_struct, but it looks like that's already enforced with a big
comment. We now store both the user and kernel SP in task_struct, but
those are really acting more as extra scratch space than pemanent
storage.
"
>> +#else /* !CONFIG_MMU */
>> +
>> +static inline void flush_tlb_all(void)
>> +{
>> + BUG();
>> +}
>> +
>> +static inline void flush_tlb_mm(struct mm_struct *mm)
>> +{
>> + BUG();
>> +}
>
> The NOMMU support is rather incomplete and CONFIG_MMU is
> hard-enabled, so I'd just drop any !CONFIG_MMU #ifdefs.
OK. I've left in the "#ifdef CONFIG_MMU" blocks as the #ifdef/#endif doesn't
really add any code, but I can go ahead and drop the #ifdef if you think that's
better.
https://github.com/riscv/riscv-linux/commit/e98ca23adfb9422bebc87cbfb58f70d4a63cf067
>> diff --git a/arch/riscv/include/uapi/asm/Kbuild b/arch/riscv/include/uapi/asm/Kbuild
>> new file mode 100644
>> index 000000000000..276b6dae745c
>> --- /dev/null
>> +++ b/arch/riscv/include/uapi/asm/Kbuild
>> @@ -0,0 +1,10 @@
>> +# UAPI Header export list
>> +include include/uapi/asm-generic/Kbuild.asm
>> +
>> +header-y += auxvec.h
>> +header-y += bitsperlong.h
>> +header-y += byteorder.h
>> +header-y += ptrace.h
>> +header-y += sigcontext.h
>> +header-y += siginfo.h
>> +header-y += unistd.h
>
> Please see
> fcc8487d477a ("uapi: export all headers under uapi directories")
>
> and adapt the file accordingly
https://github.com/riscv/riscv-linux/commit/52c5e300b498742390434891db34f9dbacd082e9
>> +#include <asm-generic/unistd.h>
>> +
>> +#define __NR_sysriscv __NR_arch_specific_syscall
>> +#ifndef __riscv_atomic
>> +__SYSCALL(__NR_sysriscv, sys_sysriscv)
>> +#endif
>
> Please make this a straight cmpxchg syscall and remove the multiplexer.
> Why does the definition depend on __riscv_atomic rather than the
> Kconfig symbol?
I think that was just an oversight: that's not the right switch. Either you or
someone else pointed out some problems with this. There's going to be an
interposer in the VDSO, and then we'll always enable the system call.
I can change this to two system calls: sysriscv_cmpxchg32 and
sysriscv_cmpxchg64.
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-06-01 11:10 +0200 |
| Subject | Re: [PATCH 4/7] RISC-V: arch/riscv/include |
| Message-ID | <tNuhb-Bs-15@gated-at.bofh.it> |
| In reply to | #1654738 |
On Thu, Jun 1, 2017 at 2:56 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> On Tue, 23 May 2017 05:55:15 PDT (-0700), Arnd Bergmann wrote:
>> On Tue, May 23, 2017 at 2:41 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>> +#ifndef _ASM_RISCV_CACHE_H
>>> +#define _ASM_RISCV_CACHE_H
>>> +
>>> +#define L1_CACHE_SHIFT 6
>>> +
>>> +#define L1_CACHE_BYTES (1 << L1_CACHE_SHIFT)
>>
>> Is this the only valid cache line size on riscv, or just the largest
>> one that is allowed?
>
> The RISC-V ISA manual doesn't actually mention caches anywhere, so there's no
> restriction on L1 cache line size (we tried to keep microarchitecture out of
> the ISA specification). We provide the actual cache parameters as part of the
> device tree, but it looks like this needs to be known staticly in some places
> so we can't use that everywhere.
>
> We could always make this a Kconfig parameter.
The cache line size is used in a couple of places, let's go through the most
common ones to see where that abstraction might be leaky and you actually
get an architectural effect:
- On SMP machines, ____cacheline_aligned_in_smp is used to annotate
data structures used in lockless algorithms, typically with one CPU writing
to some members of a structure, and another CPU reading from it but
not writing the same members. Depending on the architecture, having a
larger actual alignment than L1_CACHE_BYTES will either lead to
bad performance from cache line ping pong, or actual data corruption.
- On systems with DMA masters that are not fully coherent,
____cacheline_aligned is used to annotate data structures used
for DMA buffers, to make sure that the cache maintenance operations
in dma_sync_*_for_*() helpers don't corrup data outside of the
DMA buffer. You don't seem to support noncoherent DMA masters
or the cache maintenance operations required to use those, so this
might not be a problem until someone adds an extension for those.
- Depending on the bus interconnect, a coherent DMA master might
not be able to update partial cache lines, so you need the same
annotation.
- The kmalloc() family of memory allocators aligns data to the cache
line size, for both DMA and SMP synchronization above.
- Many architectures have cache line prefetch, flush, zero or copy
instructions that are used for important performance optimizations
but that are typically defined on a cacheline granularity. I don't
think you currently have any of them, but it seems likely that there
will be demand for them later.
Having a larger than necessary alignment can waste substantial amounts
of memory for arrays of cache line aligned structures (typically
per-cpu arrays), but otherwise should not cause harm.
>>> diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
>>> new file mode 100644
>>> index 000000000000..d942555a7a08
>>> --- /dev/null
>>> +++ b/arch/riscv/include/asm/io.h
>>> @@ -0,0 +1,36 @@
>>
>>> +#ifndef _ASM_RISCV_IO_H
>>> +#define _ASM_RISCV_IO_H
>>> +
>>> +#include <asm-generic/io.h>
>>
>> I would recommend providing your own {read,write}{b,w,l,q}{,_relaxed}
>> helpers using inline assembly, to prevent the compiler for breaking
>> up accesses into byte accesses.
>>
>> Also, most architectures require to some synchronization after a
>> non-relaxed readl() to prevent prefetching of DMA buffers, and
>> before a writel() to flush write buffers when a DMA gets triggered.
>
> Makes sense. These were all OK on existing implementations (as there's no
> writable PMAs, so all MMIO regions are strictly ordered), but that's not
> actually what the RISC-V ISA says. I patterned this on arm64
>
> https://github.com/riscv/riscv-linux/commit/e200fa29a69451ef4d575076e4d2af6b7877b1fa
>
> where I think the only odd thing is our definition of mmiowb
>
> +/* IO barriers. These only fence on the IO bits because they're only required
> + * to order device access. We're defining mmiowb because our AMO instructions
> + * (which are used to implement locks) don't specify ordering. From Chapter 7
> + * of v2.2 of the user ISA:
> + * "The bits order accesses to one of the two address domains, memory or I/O,
> + * depending on which address domain the atomic instruction is accessing. No
> + * ordering constraint is implied to accesses to the other domain, and a FENCE
> + * instruction should be used to order across both domains."
> + */
> +
> +#define __iormb() __asm__ __volatile__ ("fence i,io" : : : "memory");
> +#define __iowmb() __asm__ __volatile__ ("fence io,o" : : : "memory");
Looks ok, yes.
> +#define mmiowb() __asm__ __volatile__ ("fence io,io" : : : "memory");
>
> which I think is correct.
I can never remember what exactly this one does.
>>> +#ifdef __KERNEL__
>>> +
>>> +#ifdef CONFIG_MMU
>>> +
>>> +extern void __iomem *ioremap(phys_addr_t offset, unsigned long size);
>>> +
>>> +#define ioremap_nocache(addr, size) ioremap((addr), (size))
>>> +#define ioremap_wc(addr, size) ioremap((addr), (size))
>>> +#define ioremap_wt(addr, size) ioremap((addr), (size))
>>
>> Is this a hard architecture limitation? Normally you really want
>> write-combined access on frame buffer memory and a few other
>> cases for performance reasons, and ioremap_wc() gets used
>> for by memremap() for addressing RAM in some cases, and you
>> normally don't want to have PTEs for the same memory using
>> cached and uncached page flags
>
> This is currently an architecture limitation. In RISC-V these properties are
> known as PMAs (Physical Memory Attributes). While the supervisor spec mentions
> PMAs, it doesn't provide a mechanism to read or write them so they are
> essentially unspecified. PMAs will be properly defined as part of the platform
> specification, which isn't written yet.
Ok. Maybe add that as a comment above these definitions then.
>>> +/*
>>> + * low level task data that entry.S needs immediate access to
>>> + * - this struct should fit entirely inside of one cache line
>>> + * - this struct resides at the bottom of the supervisor stack
>>> + * - if the members of this struct changes, the assembly constants
>>> + * in asm-offsets.c must be updated accordingly
>>> + */
>>> +struct thread_info {
>>> + struct task_struct *task; /* main task structure */
>>> + unsigned long flags; /* low level flags */
>>> + __u32 cpu; /* current CPU */
>>> + int preempt_count; /* 0 => preemptable, <0 => BUG */
>>> + mm_segment_t addr_limit;
>>> +};
>>
>> Please see 15f4eae70d36 ("x86: Move thread_info into task_struct")
>> and try to do the same.
>
> OK, here's my attempt
>
> https://github.com/riscv/riscv-linux/commit/c618553e7aa65c85564a5d0a868ec7e6cf634afd
>
> Since there's some actual meat, I left a commit message (these are more just
> notes for me for my v2, I'll be squashing everything)
>
> "
> This is patterned more off the arm64 move than the x86 one, since we
> still need to have at least addr_limit to emulate FS.
>
> The patch itself changes sscratch from holding SP to holding TP, which
> contains a pointer to task_struct. thread_info must be at a 0 offset
> from task_struct, but it looks like that's already enforced with a big
> comment. We now store both the user and kernel SP in task_struct, but
> those are really acting more as extra scratch space than pemanent
> storage.
> "
I haven't looked at all the details of the x86 patch, but it seems they
decided to put the arch specific members into 'struct thread_struct'
rather than 'struct thread_info', so I'd suggest you do the same here for
consistency, unless there is a strong reason against doing it.
>>> +#else /* !CONFIG_MMU */
>>> +
>>> +static inline void flush_tlb_all(void)
>>> +{
>>> + BUG();
>>> +}
>>> +
>>> +static inline void flush_tlb_mm(struct mm_struct *mm)
>>> +{
>>> + BUG();
>>> +}
>>
>> The NOMMU support is rather incomplete and CONFIG_MMU is
>> hard-enabled, so I'd just drop any !CONFIG_MMU #ifdefs.
>
> OK. I've left in the "#ifdef CONFIG_MMU" blocks as the #ifdef/#endif doesn't
> really add any code, but I can go ahead and drop the #ifdef if you think that's
> better.
>
> https://github.com/riscv/riscv-linux/commit/e98ca23adfb9422bebc87cbfb58f70d4a63cf067
Ok.
>>> +#include <asm-generic/unistd.h>
>>> +
>>> +#define __NR_sysriscv __NR_arch_specific_syscall
>>> +#ifndef __riscv_atomic
>>> +__SYSCALL(__NR_sysriscv, sys_sysriscv)
>>> +#endif
>>
>> Please make this a straight cmpxchg syscall and remove the multiplexer.
>> Why does the definition depend on __riscv_atomic rather than the
>> Kconfig symbol?
>
> I think that was just an oversight: that's not the right switch. Either you or
> someone else pointed out some problems with this. There's going to be an
> interposer in the VDSO, and then we'll always enable the system call.
>
> I can change this to two system calls: sysriscv_cmpxchg32 and
> sysriscv_cmpxchg64.
Sounds good.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Palmer Dabbelt <palmer@dabbelt.com> |
|---|---|
| Date | 2017-06-06 07:00 +0200 |
| Subject | Re: [PATCH 4/7] RISC-V: arch/riscv/include |
| Message-ID | <tPeL0-460-19@gated-at.bofh.it> |
| In reply to | #1654947 |
On Thu, 01 Jun 2017 02:00:22 PDT (-0700), Arnd Bergmann wrote:
> On Thu, Jun 1, 2017 at 2:56 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> On Tue, 23 May 2017 05:55:15 PDT (-0700), Arnd Bergmann wrote:
>>> On Tue, May 23, 2017 at 2:41 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
>
>>>> +#ifndef _ASM_RISCV_CACHE_H
>>>> +#define _ASM_RISCV_CACHE_H
>>>> +
>>>> +#define L1_CACHE_SHIFT 6
>>>> +
>>>> +#define L1_CACHE_BYTES (1 << L1_CACHE_SHIFT)
>>>
>>> Is this the only valid cache line size on riscv, or just the largest
>>> one that is allowed?
>>
>> The RISC-V ISA manual doesn't actually mention caches anywhere, so there's no
>> restriction on L1 cache line size (we tried to keep microarchitecture out of
>> the ISA specification). We provide the actual cache parameters as part of the
>> device tree, but it looks like this needs to be known staticly in some places
>> so we can't use that everywhere.
>>
>> We could always make this a Kconfig parameter.
>
> The cache line size is used in a couple of places, let's go through the most
> common ones to see where that abstraction might be leaky and you actually
> get an architectural effect:
>
> - On SMP machines, ____cacheline_aligned_in_smp is used to annotate
> data structures used in lockless algorithms, typically with one CPU writing
> to some members of a structure, and another CPU reading from it but
> not writing the same members. Depending on the architecture, having a
> larger actual alignment than L1_CACHE_BYTES will either lead to
> bad performance from cache line ping pong, or actual data corruption.
On RISC-V it's just a performance problem, so at least it's not catastrophic.
> - On systems with DMA masters that are not fully coherent,
> ____cacheline_aligned is used to annotate data structures used
> for DMA buffers, to make sure that the cache maintenance operations
> in dma_sync_*_for_*() helpers don't corrup data outside of the
> DMA buffer. You don't seem to support noncoherent DMA masters
> or the cache maintenance operations required to use those, so this
> might not be a problem until someone adds an extension for those.
>
> - Depending on the bus interconnect, a coherent DMA master might
> not be able to update partial cache lines, so you need the same
> annotation.
Well, our (SiFive's) bus is easy to master so hopefully we won't end up doing
that. There is, of course, the rest of the world -- but that's just a bridge
we'll have to cross later (if such an implementation arises).
> - The kmalloc() family of memory allocators aligns data to the cache
> line size, for both DMA and SMP synchronization above.
Ya, but luckily just a performance problem on RISC-V.
> - Many architectures have cache line prefetch, flush, zero or copy
> instructions that are used for important performance optimizations
> but that are typically defined on a cacheline granularity. I don't
> think you currently have any of them, but it seems likely that there
> will be demand for them later.
We actually have an implicit prefetch (loads to x0, the zero register), but
it still has all the load side-effects so nothing uses it.
> Having a larger than necessary alignment can waste substantial amounts
> of memory for arrays of cache line aligned structures (typically
> per-cpu arrays), but otherwise should not cause harm.
I bugged our L1 guy and he says 64-byte lines are a bit of a magic number
because of how they line up with DIMMs. Since there's no spec to define this,
there's no correct answer. I'd be amenable to making this a Kconfig option,
but I think we'll leave it alone for now. It does match the extant
implementations.
>>>> diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
>>>> new file mode 100644
>>>> index 000000000000..d942555a7a08
>>>> --- /dev/null
>>>> +++ b/arch/riscv/include/asm/io.h
>>>> @@ -0,0 +1,36 @@
>>>
>>>> +#ifndef _ASM_RISCV_IO_H
>>>> +#define _ASM_RISCV_IO_H
>>>> +
>>>> +#include <asm-generic/io.h>
>>>
>>> I would recommend providing your own {read,write}{b,w,l,q}{,_relaxed}
>>> helpers using inline assembly, to prevent the compiler for breaking
>>> up accesses into byte accesses.
>>>
>>> Also, most architectures require to some synchronization after a
>>> non-relaxed readl() to prevent prefetching of DMA buffers, and
>>> before a writel() to flush write buffers when a DMA gets triggered.
>>
>> Makes sense. These were all OK on existing implementations (as there's no
>> writable PMAs, so all MMIO regions are strictly ordered), but that's not
>> actually what the RISC-V ISA says. I patterned this on arm64
>>
>> https://github.com/riscv/riscv-linux/commit/e200fa29a69451ef4d575076e4d2af6b7877b1fa
>>
>> where I think the only odd thing is our definition of mmiowb
>>
>> +/* IO barriers. These only fence on the IO bits because they're only required
>> + * to order device access. We're defining mmiowb because our AMO instructions
>> + * (which are used to implement locks) don't specify ordering. From Chapter 7
>> + * of v2.2 of the user ISA:
>> + * "The bits order accesses to one of the two address domains, memory or I/O,
>> + * depending on which address domain the atomic instruction is accessing. No
>> + * ordering constraint is implied to accesses to the other domain, and a FENCE
>> + * instruction should be used to order across both domains."
>> + */
>> +
>> +#define __iormb() __asm__ __volatile__ ("fence i,io" : : : "memory");
>> +#define __iowmb() __asm__ __volatile__ ("fence io,o" : : : "memory");
>
> Looks ok, yes.
>
>> +#define mmiowb() __asm__ __volatile__ ("fence io,io" : : : "memory");
>>
>> which I think is correct.
>
> I can never remember what exactly this one does.
I can't find the reference again, but what I found said that if your atomics
(or whatever's used for locking) don't stay ordered with your MMIO accesses,
then you should define mmiowb to ensure ordering. I managed to screw this up,
as there's no "w" in the successor set (to actually enforce the AMO ordering).
This is somewhat confirmed by
https://lkml.org/lkml/2006/8/31/174
Subject: Re: When to use mmiowb()?
AFAICT, they're both right. Generally, mmiowb() should be used prior to
unlock in a critical section whose last PIO operation is a writeX.
Thus, I think the actual fence should be at least
fence o,w
Documentation/memory-barries.txt says
"
The Linux kernel also has a special barrier for use with memory-mapped I/O
writes:
mmiowb();
This is a variation on the mandatory write barrier that causes writes to weakly
ordered I/O regions to be partially ordered. Its effects may go beyond the
CPU->Hardware interface and actually affect the hardware at some level.
See the subsection "Acquires vs I/O accesses" for more information.
"
"
ACQUIRES VS I/O ACCESSES
------------------------
Under certain circumstances (especially involving NUMA), I/O accesses within
two spinlocked sections on two different CPUs may be seen as interleaved by the
PCI bridge, because the PCI bridge does not necessarily participate in the
cache-coherence protocol, and is therefore incapable of issuing the required
read memory barriers.
For example:
CPU 1 CPU 2
=============================== ===============================
spin_lock(Q)
writel(0, ADDR)
writel(1, DATA);
spin_unlock(Q);
spin_lock(Q);
writel(4, ADDR);
writel(5, DATA);
spin_unlock(Q);
may be seen by the PCI bridge as follows:
STORE *ADDR = 0, STORE *ADDR = 4, STORE *DATA = 1, STORE *DATA = 5
which would probably cause the hardware to malfunction.
What is necessary here is to intervene with an mmiowb() before dropping the
spinlock, for example:
CPU 1 CPU 2
=============================== ===============================
spin_lock(Q)
writel(0, ADDR)
writel(1, DATA);
mmiowb();
spin_unlock(Q);
spin_lock(Q);
writel(4, ADDR);
writel(5, DATA);
mmiowb();
spin_unlock(Q);
this will ensure that the two stores issued on CPU 1 appear at the PCI bridge
before either of the stores issued on CPU 2.
Furthermore, following a store by a load from the same device obviates the need
for the mmiowb(), because the load forces the store to complete before the load
is performed:
CPU 1 CPU 2
=============================== ===============================
spin_lock(Q)
writel(0, ADDR)
a = readl(DATA);
spin_unlock(Q);
spin_lock(Q);
writel(4, ADDR);
b = readl(DATA);
spin_unlock(Q);
See Documentation/driver-api/device-io.rst for more information.
"
which matches what's above. I think "fence o,w" is sufficient for a mmiowb on
RISC-V. I'll make the change.
>>>> +#ifdef __KERNEL__
>>>> +
>>>> +#ifdef CONFIG_MMU
>>>> +
>>>> +extern void __iomem *ioremap(phys_addr_t offset, unsigned long size);
>>>> +
>>>> +#define ioremap_nocache(addr, size) ioremap((addr), (size))
>>>> +#define ioremap_wc(addr, size) ioremap((addr), (size))
>>>> +#define ioremap_wt(addr, size) ioremap((addr), (size))
>>>
>>> Is this a hard architecture limitation? Normally you really want
>>> write-combined access on frame buffer memory and a few other
>>> cases for performance reasons, and ioremap_wc() gets used
>>> for by memremap() for addressing RAM in some cases, and you
>>> normally don't want to have PTEs for the same memory using
>>> cached and uncached page flags
>>
>> This is currently an architecture limitation. In RISC-V these properties are
>> known as PMAs (Physical Memory Attributes). While the supervisor spec mentions
>> PMAs, it doesn't provide a mechanism to read or write them so they are
>> essentially unspecified. PMAs will be properly defined as part of the platform
>> specification, which isn't written yet.
>
> Ok. Maybe add that as a comment above these definitions then.
OK.
>>>> +/*
>>>> + * low level task data that entry.S needs immediate access to
>>>> + * - this struct should fit entirely inside of one cache line
>>>> + * - this struct resides at the bottom of the supervisor stack
>>>> + * - if the members of this struct changes, the assembly constants
>>>> + * in asm-offsets.c must be updated accordingly
>>>> + */
>>>> +struct thread_info {
>>>> + struct task_struct *task; /* main task structure */
>>>> + unsigned long flags; /* low level flags */
>>>> + __u32 cpu; /* current CPU */
>>>> + int preempt_count; /* 0 => preemptable, <0 => BUG */
>>>> + mm_segment_t addr_limit;
>>>> +};
>>>
>>> Please see 15f4eae70d36 ("x86: Move thread_info into task_struct")
>>> and try to do the same.
>>
>> OK, here's my attempt
>>
>> https://github.com/riscv/riscv-linux/commit/c618553e7aa65c85564a5d0a868ec7e6cf634afd
>>
>> Since there's some actual meat, I left a commit message (these are more just
>> notes for me for my v2, I'll be squashing everything)
>>
>> "
>> This is patterned more off the arm64 move than the x86 one, since we
>> still need to have at least addr_limit to emulate FS.
>>
>> The patch itself changes sscratch from holding SP to holding TP, which
>> contains a pointer to task_struct. thread_info must be at a 0 offset
>> from task_struct, but it looks like that's already enforced with a big
>> comment. We now store both the user and kernel SP in task_struct, but
>> those are really acting more as extra scratch space than pemanent
>> storage.
>> "
>
> I haven't looked at all the details of the x86 patch, but it seems they
> decided to put the arch specific members into 'struct thread_struct'
> rather than 'struct thread_info', so I'd suggest you do the same here for
> consistency, unless there is a strong reason against doing it.
We actually can't put them in "struct thread_struct" in a sane manner. On
context switches on RISC-V there are no register saved, instead we use the
instructions
csrrw REG, sscratch, REG
which swaps some register (used to be sp, now tp) with the "sscratch" CSR (a
register only visible to the supervisor). At this point we only have one
register to work with in order to save the user state. Since "struct
thread_info" is 0-offset from "struct task_struct", we're guaranteed to be able
to access it using our one addressing mode (a 12-bit signed constant offset
from a register).
Unfortunately, "struct thread_struct" is at a potentially large offset from the
saved TP. We could do something silly like
addi tp, tp, OFFSET_1
addi tp, tp, OFFSET_2
addi tp, tp, OFFSET_3
but for an "allyesconfig" we end up with offsets of about 9KiB, which would
require 4 additional instructions to find "struct thread_struct". While this
is possible, I'd prefer to avoid the extra cycles. We usually handle long
immediate with a two-instruction sequence
li REG, IMM31-12 (loads the high bits of REG, zeroing the low bits)
addi REG, REG, IMM11-0 (loads the low bits of REG, leaving the high bits alone)
but there's no scratch register here so we can't use that. We don't have a
long-immediate-add instruction.
The arguments in x86 land for moving everything to "struct thread_struct" were
that they always know it's in "struct task_struct", but since that's all we're
supporting on RISC-V I don't think it counts. There were also discussions of
eliminating "struct thread_info", but unless there's something at the start of
"struct task_struct" then RISC-V will have to pay a bunch of cycles on a
context switch.
>>>> +#else /* !CONFIG_MMU */
>>>> +
>>>> +static inline void flush_tlb_all(void)
>>>> +{
>>>> + BUG();
>>>> +}
>>>> +
>>>> +static inline void flush_tlb_mm(struct mm_struct *mm)
>>>> +{
>>>> + BUG();
>>>> +}
>>>
>>> The NOMMU support is rather incomplete and CONFIG_MMU is
>>> hard-enabled, so I'd just drop any !CONFIG_MMU #ifdefs.
>>
>> OK. I've left in the "#ifdef CONFIG_MMU" blocks as the #ifdef/#endif doesn't
>> really add any code, but I can go ahead and drop the #ifdef if you think that's
>> better.
>>
>> https://github.com/riscv/riscv-linux/commit/e98ca23adfb9422bebc87cbfb58f70d4a63cf067
>
> Ok.
>
>>>> +#include <asm-generic/unistd.h>
>>>> +
>>>> +#define __NR_sysriscv __NR_arch_specific_syscall
>>>> +#ifndef __riscv_atomic
>>>> +__SYSCALL(__NR_sysriscv, sys_sysriscv)
>>>> +#endif
>>>
>>> Please make this a straight cmpxchg syscall and remove the multiplexer.
>>> Why does the definition depend on __riscv_atomic rather than the
>>> Kconfig symbol?
>>
>> I think that was just an oversight: that's not the right switch. Either you or
>> someone else pointed out some problems with this. There's going to be an
>> interposer in the VDSO, and then we'll always enable the system call.
>>
>> I can change this to two system calls: sysriscv_cmpxchg32 and
>> sysriscv_cmpxchg64.
>
> Sounds good.
>
> Arnd
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-06-06 11:00 +0200 |
| Subject | Re: [PATCH 4/7] RISC-V: arch/riscv/include |
| Message-ID | <tPivf-6qB-1@gated-at.bofh.it> |
| In reply to | #1658351 |
On Tue, Jun 6, 2017 at 6:56 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> On Thu, 01 Jun 2017 02:00:22 PDT (-0700), Arnd Bergmann wrote:
>> On Thu, Jun 1, 2017 at 2:56 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>> On Tue, 23 May 2017 05:55:15 PDT (-0700), Arnd Bergmann wrote:
>>>> On Tue, May 23, 2017 at 2:41 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> - Many architectures have cache line prefetch, flush, zero or copy
>> instructions that are used for important performance optimizations
>> but that are typically defined on a cacheline granularity. I don't
>> think you currently have any of them, but it seems likely that there
>> will be demand for them later.
>
> We actually have an implicit prefetch (loads to x0, the zero register), but
> it still has all the load side-effects so nothing uses it.
>
>> Having a larger than necessary alignment can waste substantial amounts
>> of memory for arrays of cache line aligned structures (typically
>> per-cpu arrays), but otherwise should not cause harm.
>
> I bugged our L1 guy and he says 64-byte lines are a bit of a magic number
> because of how they line up with DIMMs. Since there's no spec to define this,
> there's no correct answer. I'd be amenable to making this a Kconfig option,
> but I think we'll leave it alone for now. It does match the extant
> implementations.
Hmm, this sounds like a hole in the architecture definition: if you have an
instruction that performs a prefetch (even one that is not easily usable),
I would argue that the cache line size has become a feature of the
architecture and is no longer strictly an implementation detail of the
microarchitecture.
Regarding the memory interface, a lot of systems use two DIMMs on
each memory channel for 128-bit parallel buses, and with LP-DDRx
controllers, you might have a width as small as 16 bits.
>>>>> diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
>>>>> new file mode 100644
>>>>> index 000000000000..d942555a7a08
>>>>> --- /dev/null
>>>>> +++ b/arch/riscv/include/asm/io.h
>>>>> @@ -0,0 +1,36 @@
>>>>
>>>>> +#ifndef _ASM_RISCV_IO_H
>>>>> +#define _ASM_RISCV_IO_H
>>>>> +
>>>>> +#include <asm-generic/io.h>
>>>>
>>>> I would recommend providing your own {read,write}{b,w,l,q}{,_relaxed}
>>>> helpers using inline assembly, to prevent the compiler for breaking
>>>> up accesses into byte accesses.
>>>>
>>>> Also, most architectures require to some synchronization after a
>>>> non-relaxed readl() to prevent prefetching of DMA buffers, and
>>>> before a writel() to flush write buffers when a DMA gets triggered.
>>>
>>> Makes sense. These were all OK on existing implementations (as there's no
>>> writable PMAs, so all MMIO regions are strictly ordered), but that's not
>>> actually what the RISC-V ISA says. I patterned this on arm64
>>>
>>> https://github.com/riscv/riscv-linux/commit/e200fa29a69451ef4d575076e4d2af6b7877b1fa
>>>
>>> where I think the only odd thing is our definition of mmiowb
>>>
>>> +/* IO barriers. These only fence on the IO bits because they're only required
>>> + * to order device access. We're defining mmiowb because our AMO instructions
>>> + * (which are used to implement locks) don't specify ordering. From Chapter 7
>>> + * of v2.2 of the user ISA:
>>> + * "The bits order accesses to one of the two address domains, memory or I/O,
>>> + * depending on which address domain the atomic instruction is accessing. No
>>> + * ordering constraint is implied to accesses to the other domain, and a FENCE
>>> + * instruction should be used to order across both domains."
>>> + */
>>> +
>>> +#define __iormb() __asm__ __volatile__ ("fence i,io" : : : "memory");
>>> +#define __iowmb() __asm__ __volatile__ ("fence io,o" : : : "memory");
>>
>> Looks ok, yes.
>>
>>> +#define mmiowb() __asm__ __volatile__ ("fence io,io" : : : "memory");
>>>
>>> which I think is correct.
>>
>> I can never remember what exactly this one does.
>
> I can't find the reference again, but what I found said that if your atomics
> (or whatever's used for locking) don't stay ordered with your MMIO accesses,
> then you should define mmiowb to ensure ordering. I managed to screw this up,
> as there's no "w" in the successor set (to actually enforce the AMO ordering).
> This is somewhat confirmed by
>
> https://lkml.org/lkml/2006/8/31/174
> Subject: Re: When to use mmiowb()?
> AFAICT, they're both right. Generally, mmiowb() should be used prior to
> unlock in a critical section whose last PIO operation is a writeX.
>
> Thus, I think the actual fence should be at least
>
> fence o,w
...
> which matches what's above. I think "fence o,w" is sufficient for a mmiowb on
> RISC-V. I'll make the change.
This sounds reasonable according to the documentation, but with your
longer explanation of the barriers, I think the __iormb/__iowmb definitions
above are wrong. What you actually need I think is
void writel(u32 v, volatile void __iomem *addr)
{
asm volatile("fence w,o" : : : "memory");
writel_relaxed(v, addr);
}
u32 readl(volatile void __iomem *addr)
{
u32 ret = readl_relaxed(addr);
asm volatile("fence i,r" : : : "memory");
return ret;
}
to synchronize between DMA and I/O. The barriers you listed above
in contrast appear to be directed at synchronizing I/O with other I/O.
We normally assume that this is not required when you have
subsequent MMIO accesses on the same device (on PCI) or the
same address region (per ARM architecture and others). If you do
need to enforce ordering between MMIO, you might even need to
add those barriers in the relaxed version to be portable with drivers
written for ARM SoCs:
void writel_relaxed(u32 v, volatile void __iomem *addr)
{
__raw_writel((__force u32)cpu_to_le32(v, addr)
asm volatile("fence o,io" : : : "memory");
}
u32 readl_relaxed(volatile void __iomem *addr)
{
asm volatile("fence i,io" : : : "memory");
return le32_to_cpu((__force __le32)__raw_readl(addr));
}
You then end up with a barrier before and after each regular
readl/writel in order to synchronize both with DMA and MMIO
instrictructions, and you still need the extre mmiowb() to
synchronize against the spinlock.
My memory on mmiowb is still a bit cloudy, but I think we don't
need that on ARM, and while PowerPC originally needed it, it is
now implied by the spin_unlock(). If this is actually right, you might
want to do the same here. Very few drivers actually use mmiowb(),
but there might be more drivers that would need it if your
spin_unlock() doesn't synchronize against writel() or writel_relaxed().
Maybe it the mmiowb() should really be implied by writel() but not
writel_relaxed()? That might be sensible, but only if we do it
the same way on powerpc, which currently doesn't have
writel_relaxed() any more relaxed than writel()
> The arguments in x86 land for moving everything to "struct thread_struct" were
> that they always know it's in "struct task_struct", but since that's all we're
> supporting on RISC-V I don't think it counts. There were also discussions of
> eliminating "struct thread_info", but unless there's something at the start of
> "struct task_struct" then RISC-V will have to pay a bunch of cycles on a
> context switch.
Ok. Since you have THREAD_INFO_IN_TASK, it probably doesn't matter
too much then, it's just a bit inconsistent with the other architectures, but
the effect is the same, so just do it the way that is more efficient for you.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Palmer Dabbelt <palmer@dabbelt.com> |
|---|---|
| Date | 2017-06-06 21:10 +0200 |
| Subject | Re: [PATCH 4/7] RISC-V: arch/riscv/include |
| Message-ID | <tPs1z-4ry-9@gated-at.bofh.it> |
| In reply to | #1658481 |
On Tue, 06 Jun 2017 01:54:23 PDT (-0700), Arnd Bergmann wrote:
> On Tue, Jun 6, 2017 at 6:56 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> On Thu, 01 Jun 2017 02:00:22 PDT (-0700), Arnd Bergmann wrote:
>>> On Thu, Jun 1, 2017 at 2:56 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>>> On Tue, 23 May 2017 05:55:15 PDT (-0700), Arnd Bergmann wrote:
>>>>> On Tue, May 23, 2017 at 2:41 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>>>>> diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
>>>>>> new file mode 100644
>>>>>> index 000000000000..d942555a7a08
>>>>>> --- /dev/null
>>>>>> +++ b/arch/riscv/include/asm/io.h
>>>>>> @@ -0,0 +1,36 @@
>>>>>
>>>>>> +#ifndef _ASM_RISCV_IO_H
>>>>>> +#define _ASM_RISCV_IO_H
>>>>>> +
>>>>>> +#include <asm-generic/io.h>
>>>>>
>>>>> I would recommend providing your own {read,write}{b,w,l,q}{,_relaxed}
>>>>> helpers using inline assembly, to prevent the compiler for breaking
>>>>> up accesses into byte accesses.
>>>>>
>>>>> Also, most architectures require to some synchronization after a
>>>>> non-relaxed readl() to prevent prefetching of DMA buffers, and
>>>>> before a writel() to flush write buffers when a DMA gets triggered.
>>>>
>>>> Makes sense. These were all OK on existing implementations (as there's no
>>>> writable PMAs, so all MMIO regions are strictly ordered), but that's not
>>>> actually what the RISC-V ISA says. I patterned this on arm64
>>>>
>>>> https://github.com/riscv/riscv-linux/commit/e200fa29a69451ef4d575076e4d2af6b7877b1fa
>>>>
>>>> where I think the only odd thing is our definition of mmiowb
>>>>
>>>> +/* IO barriers. These only fence on the IO bits because they're only required
>>>> + * to order device access. We're defining mmiowb because our AMO instructions
>>>> + * (which are used to implement locks) don't specify ordering. From Chapter 7
>>>> + * of v2.2 of the user ISA:
>>>> + * "The bits order accesses to one of the two address domains, memory or I/O,
>>>> + * depending on which address domain the atomic instruction is accessing. No
>>>> + * ordering constraint is implied to accesses to the other domain, and a FENCE
>>>> + * instruction should be used to order across both domains."
>>>> + */
>>>> +
>>>> +#define __iormb() __asm__ __volatile__ ("fence i,io" : : : "memory");
>>>> +#define __iowmb() __asm__ __volatile__ ("fence io,o" : : : "memory");
>>>
>>> Looks ok, yes.
>>>
>>>> +#define mmiowb() __asm__ __volatile__ ("fence io,io" : : : "memory");
>>>>
>>>> which I think is correct.
>>>
>>> I can never remember what exactly this one does.
>>
>> I can't find the reference again, but what I found said that if your atomics
>> (or whatever's used for locking) don't stay ordered with your MMIO accesses,
>> then you should define mmiowb to ensure ordering. I managed to screw this up,
>> as there's no "w" in the successor set (to actually enforce the AMO ordering).
>> This is somewhat confirmed by
>>
>> https://lkml.org/lkml/2006/8/31/174
>> Subject: Re: When to use mmiowb()?
>> AFAICT, they're both right. Generally, mmiowb() should be used prior to
>> unlock in a critical section whose last PIO operation is a writeX.
>>
>> Thus, I think the actual fence should be at least
>>
>> fence o,w
> ...
>> which matches what's above. I think "fence o,w" is sufficient for a mmiowb on
>> RISC-V. I'll make the change.
>
> This sounds reasonable according to the documentation, but with your
> longer explanation of the barriers, I think the __iormb/__iowmb definitions
> above are wrong. What you actually need I think is
>
> void writel(u32 v, volatile void __iomem *addr)
> {
> asm volatile("fence w,o" : : : "memory");
> writel_relaxed(v, addr);
> }
>
> u32 readl(volatile void __iomem *addr)
> {
> u32 ret = readl_relaxed(addr);
> asm volatile("fence i,r" : : : "memory");
> return ret;
> }
>
> to synchronize between DMA and I/O. The barriers you listed above
> in contrast appear to be directed at synchronizing I/O with other I/O.
> We normally assume that this is not required when you have
> subsequent MMIO accesses on the same device (on PCI) or the
> same address region (per ARM architecture and others). If you do
> need to enforce ordering between MMIO, you might even need to
> add those barriers in the relaxed version to be portable with drivers
> written for ARM SoCs:
>
> void writel_relaxed(u32 v, volatile void __iomem *addr)
> {
> __raw_writel((__force u32)cpu_to_le32(v, addr)
> asm volatile("fence o,io" : : : "memory");
> }
>
> u32 readl_relaxed(volatile void __iomem *addr)
> {
> asm volatile("fence i,io" : : : "memory");
> return le32_to_cpu((__force __le32)__raw_readl(addr));
> }
>
> You then end up with a barrier before and after each regular
> readl/writel in order to synchronize both with DMA and MMIO
> instrictructions, and you still need the extre mmiowb() to
> synchronize against the spinlock.
Ah, thanks. I guess I just had those wrong. I'll fix the non-relaxed versions
and add relaxed versions that have the fence.
> My memory on mmiowb is still a bit cloudy, but I think we don't
> need that on ARM, and while PowerPC originally needed it, it is
> now implied by the spin_unlock(). If this is actually right, you might
> want to do the same here. Very few drivers actually use mmiowb(),
> but there might be more drivers that would need it if your
> spin_unlock() doesn't synchronize against writel() or writel_relaxed().
> Maybe it the mmiowb() should really be implied by writel() but not
> writel_relaxed()? That might be sensible, but only if we do it
> the same way on powerpc, which currently doesn't have
> writel_relaxed() any more relaxed than writel()
I like the idea of putting this in spin_unlock() better, particularly if
PowerPC does it that way. I was worried that with mmiowb being such an
esoteric thing that it would be wrong all over the place, this feels much
better.
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-05-23 15:40 +0200 |
| Subject | Re: [PATCH 6/7] RISC-V: arch/riscv/kernel |
| Message-ID | <tKicy-3OV-19@gated-at.bofh.it> |
| In reply to | #1647525 |
> +IRQCHIP_DECLARE(riscv, "riscv,cpu-intc", riscv_intc_init);
Please move the majority of this file into drivers/irqchip as a
standalone driver.
> diff --git a/arch/riscv/kernel/pci.c b/arch/riscv/kernel/pci.c
> new file mode 100644
> index 000000000000..4191a5ffdd67
> --- /dev/null
> +++ b/arch/riscv/kernel/pci.c
> @@ -0,0 +1,36 @@
> +/*
> + * Code borrowed from arch/arm64/kernel/pci.c
> + *
> + * Copyright (C) 2003 Anton Blanchard <anton@au.ibm.com>, IBM
> + * Copyright (C) 2014 ARM Ltd.
> + * Copyright (C) 2017 SiFive
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +#include <linux/slab.h>
> +#include <linux/pci.h>
> +
> +/*
> + * Called after each bus is probed, but before its children are examined
> + */
> +void pcibios_fixup_bus(struct pci_bus *bus)
> +{
> + /* nothing to do, expected to be removed in the future */
> +}
> +/*
> + * We don't have to worry about legacy ISA devices, so nothing to do here
> + */
> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> + resource_size_t size, resource_size_t align)
> +{
> + return res->start;
> +}
Can you add a patch to remove the need for this, and send that to the
PCI maintainers?
In the long run, I think we want both of these to be pci host bridge
driver specific callbacks rather than per-architecture definitions, but
for the moment, moving the empty version as a __weak copy
into drivers/pci/ should be sufficient.
[note: don't ever use __weak elsewhere, the use in PCI is
only done for historic reasons and we want to get rid of that
too, but for now it's more important to avoid adding yet another
pointless copy]
If you don't care about LPC/ISA devices, then your PCI_MIN_IO
should also be zero instead of 0x1000
> diff --git a/arch/riscv/kernel/plic.c b/arch/riscv/kernel/plic.c
> new file mode 100644
> index 000000000000..5b3d4241f4e2
> --- /dev/null
> +++ b/arch/riscv/kernel/plic.c
drivers/irqchip/riscv-plic.c
The file needs some work for following coding style, once that
is done, please submit to the irqchip maintainers.
> +#define PLIC_HART_CONTEXT(data, i) (struct plic_hart_context *)((char*)data->reg + HART_BASE + HART_SIZE*i)
> +#define PLIC_ENABLE_CONTEXT(data, i) (struct plic_enable_context *)((char*)data->reg + ENABLE_BASE + ENABLE_SIZE*i)
> +#define PLIC_PRIORITY(data) (struct plic_priority *)((char *)data->reg + PRIORITY_BASE)
> +
> +struct plic_hart_context {
> + volatile u32 threshold;
> + volatile u32 claim;
> +};
> +
> +struct plic_enable_context {
> + atomic_t mask[32]; // 32-bit * 32-entry
> +};
> +
> +struct plic_priority {
> + volatile u32 prio[MAX_DEVICES];
> +};
The 'volatile' seems misplaced here. What is it for?
> +// TODO: add a /sys interface to set priority + per-hart enables for steering
No driver-private sysfs interfaces please for irqchips please.
See http://elixir.free-electrons.com/linux/latest/source/Documentation/IRQ-affinity.txt
for setting the affinity.
> diff --git a/arch/riscv/kernel/reset.c b/arch/riscv/kernel/reset.c
> new file mode 100644
> index 000000000000..58bad9598e21
> --- /dev/null
> +++ b/arch/riscv/kernel/reset.c
> @@ -0,0 +1,33 @@
> +/*
> + * Copyright (C) 2012 Regents of the University of California
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation, version 2.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT. See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/reboot.h>
> +#include <linux/export.h>
> +#include <asm/sbi.h>
> +
> +void (*pm_power_off)(void) = machine_power_off;
> +EXPORT_SYMBOL(pm_power_off);
> +
> +void machine_restart(char *cmd)
> +{
> +}
Call do_kernel_restart(cmd) here.
> +void machine_halt(void)
> +{
> +}
This should not return. Either make it call sbi_shutdown as well,
or use the ARM implementation:
void machine_halt(void)
{
local_irq_disable();
smp_send_stop();
while (1);
}
> diff --git a/arch/riscv/kernel/sbi-con.c b/arch/riscv/kernel/sbi-con.c
> new file mode 100644
> index 000000000000..86baeb5ef0cd
> --- /dev/null
> +++ b/arch/riscv/kernel/sbi-con.c
As Olof said, move it to drivers/tty/hvc/ and use those helpers.
> diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
> new file mode 100644
> index 000000000000..3e07308e24f5
> --- /dev/null
> +++ b/arch/riscv/kernel/sys_riscv.c
> @@ -0,0 +1,85 @@
> +/*
> + * Copyright (C) 2012 Regents of the University of California
> + * Copyright (C) 2014 Darius Rad <darius@bluespec.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation, version 2.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT. See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/syscalls.h>
> +#include <asm/unistd.h>
> +
> +SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
> + unsigned long, prot, unsigned long, flags,
> + unsigned long, fd, off_t, offset)
> +{
> + if (unlikely(offset & (~PAGE_MASK)))
> + return -EINVAL;
> + return sys_mmap_pgoff(addr, len, prot, flags, fd, offset >> PAGE_SHIFT);
> +}
> +
> +#ifndef CONFIG_64BIT
> +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len,
> + unsigned long, prot, unsigned long, flags,
> + unsigned long, fd, off_t, offset)
> +{
> + /* Note that the shift for mmap2 is constant (12),
> + regardless of PAGE_SIZE */
> + if (unlikely(offset & (~PAGE_MASK >> 12)))
> + return -EINVAL;
> + return sys_mmap_pgoff(addr, len, prot, flags, fd,
> + offset >> (PAGE_SHIFT - 12));
> +}
> +#endif /* !CONFIG_64BIT */
The first one should be CONFIG_64BIT only.
> +#ifdef CONFIG_RV_SYSRISCV_ATOMIC
> +SYSCALL_DEFINE4(sysriscv, unsigned long, cmd, unsigned long, arg1,
> + unsigned long, arg2, unsigned long, arg3)
> +{
> + unsigned long flags;
> + unsigned long prev;
> + unsigned int *ptr;
> + unsigned int err;
> +
> + switch (cmd) {
> + case RISCV_ATOMIC_CMPXCHG:
> + ptr = (unsigned int *)arg1;
> + if (!access_ok(VERIFY_WRITE, ptr, sizeof(unsigned int)))
> + return -EFAULT;
> +
> + preempt_disable();
> + raw_local_irq_save(flags);
> + err = __get_user(prev, ptr);
> + if (likely(!err && prev == arg2))
> + err = __put_user(arg3, ptr);
> + raw_local_irq_restore(flags);
> + preempt_enable();
> +
> + return unlikely(err) ? err : prev;
> +
> + case RISCV_ATOMIC_CMPXCHG64:
Make these two separate syscalls and get rid of the wrapper
(I already mentioned it in the header file comments, but it
fits better here).
It may be good to have an optimized version in the vdso
that does an atomic operation directly if the CPU supports
it.
> diff --git a/arch/riscv/kernel/time.c b/arch/riscv/kernel/time.c
> new file mode 100644
> index 000000000000..ce8c459fadaa
> --- /dev/null
> +++ b/arch/riscv/kernel/time.c
drivers/clocksource/riscv-timer.c, and submit it to the
respective maintainers.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Palmer Dabbelt <palmer@dabbelt.com> |
|---|---|
| Date | 2017-06-03 02:00 +0200 |
| Subject | Re: [PATCH 6/7] RISC-V: arch/riscv/kernel |
| Message-ID | <tO4E2-7Qc-11@gated-at.bofh.it> |
| In reply to | #1648050 |
On Tue, 23 May 2017 06:35:23 PDT (-0700), Arnd Bergmann wrote:
>> +IRQCHIP_DECLARE(riscv, "riscv,cpu-intc", riscv_intc_init);
>
> Please move the majority of this file into drivers/irqchip as a
> standalone driver.
OK.
https://github.com/riscv/riscv-linux/commit/549c7f5ef63d7be04c9cac7e332ef81ec6ffe103
>> diff --git a/arch/riscv/kernel/pci.c b/arch/riscv/kernel/pci.c
>> new file mode 100644
>> index 000000000000..4191a5ffdd67
>> --- /dev/null
>> +++ b/arch/riscv/kernel/pci.c
>> @@ -0,0 +1,36 @@
>> +/*
>> + * Code borrowed from arch/arm64/kernel/pci.c
>> + *
>> + * Copyright (C) 2003 Anton Blanchard <anton@au.ibm.com>, IBM
>> + * Copyright (C) 2014 ARM Ltd.
>> + * Copyright (C) 2017 SiFive
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * version 2 as published by the Free Software Foundation.
>> + *
>> + */
>> +
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mm.h>
>> +#include <linux/slab.h>
>> +#include <linux/pci.h>
>> +
>> +/*
>> + * Called after each bus is probed, but before its children are examined
>> + */
>> +void pcibios_fixup_bus(struct pci_bus *bus)
>> +{
>> + /* nothing to do, expected to be removed in the future */
>> +}
>> +/*
>> + * We don't have to worry about legacy ISA devices, so nothing to do here
>> + */
>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>> + resource_size_t size, resource_size_t align)
>> +{
>> + return res->start;
>> +}
>
> Can you add a patch to remove the need for this, and send that to the
> PCI maintainers?
>
> In the long run, I think we want both of these to be pci host bridge
> driver specific callbacks rather than per-architecture definitions, but
> for the moment, moving the empty version as a __weak copy
> into drivers/pci/ should be sufficient.
>
> [note: don't ever use __weak elsewhere, the use in PCI is
> only done for historic reasons and we want to get rid of that
> too, but for now it's more important to avoid adding yet another
> pointless copy]
Sounds good
https://github.com/riscv/riscv-linux/commit/4aa540bf849b2a190e288e7d25d262dee21306b3
https://github.com/riscv/riscv-linux/commit/bb3b4c6ca4841538d101f2b9c437f5dccda0b3a7
> If you don't care about LPC/ISA devices, then your PCI_MIN_IO
> should also be zero instead of 0x1000
Sorry, but the only Google results for PCI_MIN_IO is this email. There don't
appear to be any relevant references to PCI_MIN in the kernel
$ git grep PCI_MIN_
arch/mips/include/asm/mach-loongson64/cs5536/cs5536_pci.h: ((PCI_MAX_LATENCY << 24) | (PCI_MIN_GRANT << 16) | \
arch/mips/include/asm/mach-loongson64/cs5536/cs5536_pci.h:#define PCI_MIN_GRANT 0x00
drivers/ata/pata_hpt366.c: pci_write_config_byte(dev, PCI_MIN_GNT, 0x08);
drivers/ata/pata_hpt37x.c: pci_write_config_byte(dev, PCI_MIN_GNT, 0x08);
drivers/ata/pata_hpt3x2n.c: pci_write_config_byte(dev, PCI_MIN_GNT, 0x08);
drivers/ide/hpt366.c: pci_write_config_byte(dev, PCI_MIN_GNT, 0x08);
drivers/net/fddi/skfp/h/skfbi.h:#define PCI_MIN_GNT 0x3e /* 8 bit Min_Gnt */
drivers/net/fddi/skfp/h/skfbi.h:/* PCI_MIN_GNT 8 bit Min_Gnt */
include/uapi/linux/pci_regs.h:#define PCI_MIN_GNT 0x3e /* 8 bits */
I'm afraid that I'm not sure what to do here.
>> diff --git a/arch/riscv/kernel/plic.c b/arch/riscv/kernel/plic.c
>> new file mode 100644
>> index 000000000000..5b3d4241f4e2
>> --- /dev/null
>> +++ b/arch/riscv/kernel/plic.c
>
> drivers/irqchip/riscv-plic.c
>
> The file needs some work for following coding style, once that
> is done, please submit to the irqchip maintainers.
I've addressed most of this thanks to some other code reviews, so I'm going to
drop the comments that are about things I've already fixed.
>> +// TODO: add a /sys interface to set priority + per-hart enables for steering
>
> No driver-private sysfs interfaces please for irqchips please.
> See http://elixir.free-electrons.com/linux/latest/source/Documentation/IRQ-affinity.txt
> for setting the affinity.
We'll do it the right way when we add IRQ affinity control. Thanks for the
heads up!
>> diff --git a/arch/riscv/kernel/reset.c b/arch/riscv/kernel/reset.c
>> new file mode 100644
>> index 000000000000..58bad9598e21
>> --- /dev/null
>> +++ b/arch/riscv/kernel/reset.c
>> @@ -0,0 +1,33 @@
>> +/*
>> + * Copyright (C) 2012 Regents of the University of California
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation, version 2.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
>> + * NON INFRINGEMENT. See the GNU General Public License for
>> + * more details.
>> + */
>> +
>> +#include <linux/reboot.h>
>> +#include <linux/export.h>
>> +#include <asm/sbi.h>
>> +
>> +void (*pm_power_off)(void) = machine_power_off;
>> +EXPORT_SYMBOL(pm_power_off);
>> +
>> +void machine_restart(char *cmd)
>> +{
>> +}
>
> Call do_kernel_restart(cmd) here.
>
>> +void machine_halt(void)
>> +{
>> +}
>
> This should not return. Either make it call sbi_shutdown as well,
> or use the ARM implementation:
>
> void machine_halt(void)
> {
> local_irq_disable();
> smp_send_stop();
> while (1);
> }
OK.
https://github.com/riscv/riscv-linux/commit/5f486cb73e3a0a5a218d26781e9eae651e59203f
>> diff --git a/arch/riscv/kernel/sbi-con.c b/arch/riscv/kernel/sbi-con.c
>> new file mode 100644
>> index 000000000000..86baeb5ef0cd
>> --- /dev/null
>> +++ b/arch/riscv/kernel/sbi-con.c
>
> As Olof said, move it to drivers/tty/hvc/ and use those helpers.
Ah, that's great: now there's almost no code left :)
https://github.com/riscv/riscv-linux/commit/8adad12c5525a70b8837196b8f2d4ac003a7647c
>> diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
>> new file mode 100644
>> index 000000000000..3e07308e24f5
>> --- /dev/null
>> +++ b/arch/riscv/kernel/sys_riscv.c
>> @@ -0,0 +1,85 @@
>> +/*
>> + * Copyright (C) 2012 Regents of the University of California
>> + * Copyright (C) 2014 Darius Rad <darius@bluespec.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation, version 2.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
>> + * NON INFRINGEMENT. See the GNU General Public License for
>> + * more details.
>> + */
>> +
>> +#include <linux/syscalls.h>
>> +#include <asm/unistd.h>
>> +
>> +SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
>> + unsigned long, prot, unsigned long, flags,
>> + unsigned long, fd, off_t, offset)
>> +{
>> + if (unlikely(offset & (~PAGE_MASK)))
>> + return -EINVAL;
>> + return sys_mmap_pgoff(addr, len, prot, flags, fd, offset >> PAGE_SHIFT);
>> +}
>> +
>> +#ifndef CONFIG_64BIT
>> +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len,
>> + unsigned long, prot, unsigned long, flags,
>> + unsigned long, fd, off_t, offset)
>> +{
>> + /* Note that the shift for mmap2 is constant (12),
>> + regardless of PAGE_SIZE */
>> + if (unlikely(offset & (~PAGE_MASK >> 12)))
>> + return -EINVAL;
>> + return sys_mmap_pgoff(addr, len, prot, flags, fd,
>> + offset >> (PAGE_SHIFT - 12));
>> +}
>> +#endif /* !CONFIG_64BIT */
>
> The first one should be CONFIG_64BIT only.
OK. https://github.com/riscv/riscv-linux/commit/ed7545c6765ba7d705e1bc6ce7b67bb7e8cb0926
>> +#ifdef CONFIG_RV_SYSRISCV_ATOMIC
>> +SYSCALL_DEFINE4(sysriscv, unsigned long, cmd, unsigned long, arg1,
>> + unsigned long, arg2, unsigned long, arg3)
>> +{
>> + unsigned long flags;
>> + unsigned long prev;
>> + unsigned int *ptr;
>> + unsigned int err;
>> +
>> + switch (cmd) {
>> + case RISCV_ATOMIC_CMPXCHG:
>> + ptr = (unsigned int *)arg1;
>> + if (!access_ok(VERIFY_WRITE, ptr, sizeof(unsigned int)))
>> + return -EFAULT;
>> +
>> + preempt_disable();
>> + raw_local_irq_save(flags);
>> + err = __get_user(prev, ptr);
>> + if (likely(!err && prev == arg2))
>> + err = __put_user(arg3, ptr);
>> + raw_local_irq_restore(flags);
>> + preempt_enable();
>> +
>> + return unlikely(err) ? err : prev;
>> +
>> + case RISCV_ATOMIC_CMPXCHG64:
>
> Make these two separate syscalls and get rid of the wrapper
> (I already mentioned it in the header file comments, but it
> fits better here).
>
> It may be good to have an optimized version in the vdso
> that does an atomic operation directly if the CPU supports
> it.
Yep. It's on the list.
>> diff --git a/arch/riscv/kernel/time.c b/arch/riscv/kernel/time.c
>> new file mode 100644
>> index 000000000000..ce8c459fadaa
>> --- /dev/null
>> +++ b/arch/riscv/kernel/time.c
>
> drivers/clocksource/riscv-timer.c, and submit it to the
> respective maintainers.
Sounds good.
https://github.com/riscv/riscv-linux/commit/d9fcab4603c158755e19663dec0040b28ea1aad1
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-06-06 11:10 +0200 |
| Subject | Re: [PATCH 6/7] RISC-V: arch/riscv/kernel |
| Message-ID | <tPiEW-6IR-25@gated-at.bofh.it> |
| In reply to | #1656635 |
On Sat, Jun 3, 2017 at 1:56 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> On Tue, 23 May 2017 06:35:23 PDT (-0700), Arnd Bergmann wrote:
>>> +IRQCHIP_DECLARE(riscv, "riscv,cpu-intc", riscv_intc_init);
>> If you don't care about LPC/ISA devices, then your PCI_MIN_IO
>> should also be zero instead of 0x1000
>
> Sorry, but the only Google results for PCI_MIN_IO is this email. There don't
> appear to be any relevant references to PCI_MIN in the kernel
>
> $ git grep PCI_MIN_
> arch/mips/include/asm/mach-loongson64/cs5536/cs5536_pci.h: ((PCI_MAX_LATENCY << 24) | (PCI_MIN_GRANT << 16) | \
> arch/mips/include/asm/mach-loongson64/cs5536/cs5536_pci.h:#define PCI_MIN_GRANT 0x00
> drivers/ata/pata_hpt366.c: pci_write_config_byte(dev, PCI_MIN_GNT, 0x08);
> drivers/ata/pata_hpt37x.c: pci_write_config_byte(dev, PCI_MIN_GNT, 0x08);
> drivers/ata/pata_hpt3x2n.c: pci_write_config_byte(dev, PCI_MIN_GNT, 0x08);
> drivers/ide/hpt366.c: pci_write_config_byte(dev, PCI_MIN_GNT, 0x08);
> drivers/net/fddi/skfp/h/skfbi.h:#define PCI_MIN_GNT 0x3e /* 8 bit Min_Gnt */
> drivers/net/fddi/skfp/h/skfbi.h:/* PCI_MIN_GNT 8 bit Min_Gnt */
> include/uapi/linux/pci_regs.h:#define PCI_MIN_GNT 0x3e /* 8 bits */
>
> I'm afraid that I'm not sure what to do here.
Sorry, I meant PCIBIOS_MIN_IO
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Palmer Dabbelt <palmer@dabbelt.com> |
|---|---|
| Date | 2017-06-06 22:40 +0200 |
| Subject | Re: [PATCH 6/7] RISC-V: arch/riscv/kernel |
| Message-ID | <tPtqF-5fp-9@gated-at.bofh.it> |
| In reply to | #1658508 |
On Tue, 06 Jun 2017 02:01:23 PDT (-0700), Arnd Bergmann wrote: > On Sat, Jun 3, 2017 at 1:56 AM, Palmer Dabbelt <palmer@dabbelt.com> wrote: >> On Tue, 23 May 2017 06:35:23 PDT (-0700), Arnd Bergmann wrote: >>>> +IRQCHIP_DECLARE(riscv, "riscv,cpu-intc", riscv_intc_init); >>> If you don't care about LPC/ISA devices, then your PCI_MIN_IO >>> should also be zero instead of 0x1000 >> >> Sorry, but the only Google results for PCI_MIN_IO is this email. There don't >> appear to be any relevant references to PCI_MIN in the kernel >> >> $ git grep PCI_MIN_ >> arch/mips/include/asm/mach-loongson64/cs5536/cs5536_pci.h: ((PCI_MAX_LATENCY << 24) | (PCI_MIN_GRANT << 16) | \ >> arch/mips/include/asm/mach-loongson64/cs5536/cs5536_pci.h:#define PCI_MIN_GRANT 0x00 >> drivers/ata/pata_hpt366.c: pci_write_config_byte(dev, PCI_MIN_GNT, 0x08); >> drivers/ata/pata_hpt37x.c: pci_write_config_byte(dev, PCI_MIN_GNT, 0x08); >> drivers/ata/pata_hpt3x2n.c: pci_write_config_byte(dev, PCI_MIN_GNT, 0x08); >> drivers/ide/hpt366.c: pci_write_config_byte(dev, PCI_MIN_GNT, 0x08); >> drivers/net/fddi/skfp/h/skfbi.h:#define PCI_MIN_GNT 0x3e /* 8 bit Min_Gnt */ >> drivers/net/fddi/skfp/h/skfbi.h:/* PCI_MIN_GNT 8 bit Min_Gnt */ >> include/uapi/linux/pci_regs.h:#define PCI_MIN_GNT 0x3e /* 8 bits */ >> >> I'm afraid that I'm not sure what to do here. > > Sorry, I meant PCIBIOS_MIN_IO OK, fixed.
[toc] | [prev] | [next] | [standalone]
| From | Pavel Machek <pavel@ucw.cz> |
|---|---|
| Date | 2017-05-25 19:10 +0200 |
| Subject | Re: [PATCH 6/7] RISC-V: arch/riscv/kernel |
| Message-ID | <tL4qR-2vP-5@gated-at.bofh.it> |
| In reply to | #1647525 |
[Multipart message — attachments visible in raw view] — view raw
Hi!
> +static void ci_leaf_init(struct cacheinfo *this_leaf,
> + struct device_node *node,
> + enum cache_type type, unsigned int level)
> +{
> + this_leaf->of_node = node;
> + this_leaf->level = level;
> + this_leaf->type = type;
> + this_leaf->physical_line_partition = 1; // not a sector cache
> + this_leaf->attributes = CACHE_WRITE_BACK | CACHE_READ_ALLOCATE | CACHE_WRITE_ALLOCATE; // TODO: add to DTS
> +}
You may want to run the patches through checkpatch. (Comment style,
long lines).
> +static int __populate_cache_leaves(unsigned int cpu)
> +{
> + struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
> + struct cacheinfo *this_leaf = this_cpu_ci->info_list;
> + struct device_node *np = of_cpu_device_node_get(cpu);
> + int levels = 1, level = 1;
> +
> + if (of_property_read_bool(np, "cache-size")) ci_leaf_init(this_leaf++, np, CACHE_TYPE_UNIFIED, level);
> + if (of_property_read_bool(np, "i-cache-size")) ci_leaf_init(this_leaf++, np, CACHE_TYPE_INST, level);
> + if (of_property_read_bool(np, "d-cache-size")) ci_leaf_init(this_leaf++, np, CACHE_TYPE_DATA, level);
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[toc] | [prev] | [next] | [standalone]
| From | Palmer Dabbelt <palmer@dabbelt.com> |
|---|---|
| Date | 2017-06-03 05:40 +0200 |
| Subject | Re: [PATCH 6/7] RISC-V: arch/riscv/kernel |
| Message-ID | <tO84W-1J3-3@gated-at.bofh.it> |
| In reply to | #1650660 |
On Thu, 25 May 2017 10:05:05 PDT (-0700), pavel@ucw.cz wrote:
>> +static void ci_leaf_init(struct cacheinfo *this_leaf,
>> + struct device_node *node,
>> + enum cache_type type, unsigned int level)
>> +{
>> + this_leaf->of_node = node;
>> + this_leaf->level = level;
>> + this_leaf->type = type;
>> + this_leaf->physical_line_partition = 1; // not a sector cache
>> + this_leaf->attributes = CACHE_WRITE_BACK | CACHE_READ_ALLOCATE | CACHE_WRITE_ALLOCATE; // TODO: add to DTS
>> +}
>
> You may want to run the patches through checkpatch. (Comment style,
> long lines).
Thanks. Someone else suggested this and I've fixed most of the errors. I'll
submit a v2 with everyone's feedback once I get through my mail.
[toc] | [prev] | [next] | [standalone]
| From | Palmer Dabbelt <palmer@dabbelt.com> |
|---|---|
| Date | 2017-06-07 01:10 +0200 |
| Subject | [PATCH 17/17] RISC-V: Makefile and Kconfig |
| Message-ID | <tPvLP-6UA-1@gated-at.bofh.it> |
| In reply to | #1647525 |
This patch adds RISC-V support to the build infastructure. Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com> --- Makefile | 3 +- arch/riscv/Kconfig | 318 ++++++++++++++++++++++++++++++ arch/riscv/Makefile | 64 ++++++ arch/riscv/configs/freedom-u500_defconfig | 53 +++++ arch/riscv/configs/spike32_defconfig | 50 +++++ arch/riscv/configs/spike64_defconfig | 46 +++++ 6 files changed, 533 insertions(+), 1 deletion(-) create mode 100644 arch/riscv/Kconfig create mode 100644 arch/riscv/Makefile create mode 100644 arch/riscv/configs/freedom-u500_defconfig create mode 100644 arch/riscv/configs/spike32_defconfig create mode 100644 arch/riscv/configs/spike64_defconfig diff --git a/Makefile b/Makefile index 853ae9179af9..88711cbcc3ca 100644 --- a/Makefile +++ b/Makefile @@ -232,7 +232,8 @@ SUBARCH := $(shell uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ \ -e s/arm.*/arm/ -e s/sa110/arm/ \ -e s/s390x/s390/ -e s/parisc64/parisc/ \ -e s/ppc.*/powerpc/ -e s/mips.*/mips/ \ - -e s/sh[234].*/sh/ -e s/aarch64.*/arm64/ ) + -e s/sh[234].*/sh/ -e s/aarch64.*/arm64/ \ + -e s/riscv.*/riscv/) # Cross compiling and selecting different set of gcc/bin-utils # --------------------------------------------------------------------------- diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig new file mode 100644 index 000000000000..64fcb17886c6 --- /dev/null +++ b/arch/riscv/Kconfig @@ -0,0 +1,318 @@ +# +# 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 || !ISA_A + 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 ISA_A if SMP + select SYSRISCV_ATOMIC if !ISA_A + select SPARSE_IRQ + select SYSCTL_EXCEPTION_TRACE + select HAVE_ARCH_TRACEHOOK + select MODULES_USE_ELF_RELA if MODULES + select THREAD_INFO_IN_TASK + select RISCV_IRQ_INTC + +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" + +config CPU_SUPPORTS_32BIT_KERNEL + bool +config CPU_SUPPORTS_64BIT_KERNEL + bool + +choice + prompt "Base ISA" + default ARCH_RV64I + +config ARCH_RV32I + bool "RV32I" + select CPU_SUPPORTS_32BIT_KERNEL + select 32BIT + select GENERIC_ASHLDI3 + select GENERIC_ASHRDI3 + select GENERIC_LSHRDI3 + +config ARCH_RV64I + bool "RV64I" + select CPU_SUPPORTS_64BIT_KERNEL + select 64BIT + +endchoice + +choice + prompt "CPU Tuning" + default TUNE_GENERIC + +config TUNE_GENERIC + bool "generic" + +endchoice + +config ISA_C + bool "Emit compressed instructions when building Linux" + default n + help + Adds "C" to the ISA subsets that the toolchain is allowed to emit + when building Linux, which results in compressed instructions in the + Linux binary. + + If you don't know what to do here, say Y. + +config ISA_A + bool "Emit atomic instructions when building Linux" + default y + help + Adds "A" to the ISA subsets that the toolchain is allowed to emit + when building Linux, which results in atomic instructions in the + Linux binary. + + If you don't know what to do here, say Y. + +config SYSRISCV_ATOMIC + bool "Include support for atomic operation syscalls" + default !ISA_A + help + If atomic memory instructions are present, i.e., + CONFIG_ISA_A, 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_ISA_A is unset, this option is mandatory. + + If you don't know what to do here, say N. + +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 + +source "mm/Kconfig" + +source "kernel/Kconfig.preempt" + +source "kernel/Kconfig.hz" + +endmenu + +menu "Bus support" + +config PCI + bool "PCI support" + select PCI_MSI + help + This feature enables support for PCI bus system. If you say Y + here, the kernel will include drivers and infrastructure code + to support PCI bus devices. + + If you don't know what to do here, say Y. + +config PCI_DOMAINS + def_bool PCI + +config PCI_DOMAINS_GENERIC + def_bool PCI + +source "drivers/pci/Kconfig" + +endmenu + +source "init/Kconfig" + +source "kernel/Kconfig.freezer" + +menu "Executable file formats" + +source "fs/Kconfig.binfmt" + +endmenu + +menu "Power management options" + +source kernel/power/Kconfig + +endmenu + +source "net/Kconfig" + +source "drivers/Kconfig" + +source "fs/Kconfig" + +menu "Kernel hacking" + +config CMDLINE_BOOL + bool "Built-in kernel command line" + default n + help + For most platforms, it is firmware or second stage bootloader + that by default specifies the kernel command line options. + However, it might be necessary or advantageous to either override + the default kernel command line or add a few extra options to it. + For such cases, this option allows hardcoding command line options + directly into the kernel. + + For that, choose 'Y' here and fill in the extra boot parameters + in CONFIG_CMDLINE. + + The built-in options will be concatenated to the default command + line if CMDLINE_OVERRIDE is set to 'N'. Otherwise, the default + command line will be ignored and replaced by the built-in string. + +config CMDLINE + string "Built-in kernel command string" + depends on CMDLINE_BOOL + default "" + help + Supply command-line options at build time by entering them here. + +config CMDLINE_OVERRIDE + bool "Built-in command line overrides bootloader arguments" + default n + depends on CMDLINE_BOOL + help + Set this option to 'Y' to have the kernel ignore the bootloader + or firmware command line. Instead, the built-in command line + will be used exclusively. + + If you don't know what to do here, say N. + +config EARLY_PRINTK + bool "Early printk" + default n + help + This option enables special console drivers which allow the kernel + to print messages very early in the bootup process. + + This is useful for kernel debugging when your machine crashes very + early before the console code is initialized. For normal operation + it is not recommended because it looks ugly and doesn't cooperate + with klogd/syslogd or the X server. You should normally N here, + unless you want to debug such a crash. + + +source "lib/Kconfig.debug" + +config CMDLINE_BOOL + bool +endmenu + +source "security/Kconfig" + +source "crypto/Kconfig" + +source "lib/Kconfig" + diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile new file mode 100644 index 000000000000..66c4a5e383f9 --- /dev/null +++ b/arch/riscv/Makefile @@ -0,0 +1,64 @@ +# This file is included by the global makefile so that you can add your own +# architecture-specific flags and dependencies. Remember to do have actions +# for "archclean" and "archdep" for cleaning up and making dependencies for +# this architecture +# +# This file is subject to the terms and conditions of the GNU General Public +# License. See the file "COPYING" in the main directory of this archive +# for more details. +# + +LDFLAGS := +OBJCOPYFLAGS := -O binary +LDFLAGS_vmlinux := +KBUILD_AFLAGS_MODULE += -fPIC +KBUILD_CFLAGS_MODULE += -fPIC + +KBUILD_DEFCONFIG = spike64_defconfig + +export BITS +ifeq ($(CONFIG_ARCH_RV64I),y) + BITS := 64 + UTS_MACHINE := riscv64 + + KBUILD_CFLAGS += -mabi=lp64 + KBUILD_AFLAGS += -mabi=lp64 + KBUILD_MARCH = rv64im + LDFLAGS += -melf64lriscv +else + BITS := 32 + UTS_MACHINE := riscv32 + + KBUILD_CFLAGS += -mabi=ilp32 + KBUILD_AFLAGS += -mabi=ilp32 + KBUILD_MARCH = rv32im + LDFLAGS += -melf32lriscv +endif + +KBUILD_CFLAGS += -Wall + +ifeq ($(CONFIG_ISA_A),y) + KBUILD_ARCH_A = a +endif +ifeq ($(CONFIG_ISA_C),y) + KBUILD_ARCH_C = c +endif + +KBUILD_AFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)fd$(KBUILD_ARCH_C) + +KBUILD_CFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)$(KBUILD_ARCH_C) +KBUILD_CFLAGS += -mno-save-restore + +# GCC versions that support the "-mstrict-align" option default to allowing +# unaligned accesses. While unaligned accesses are explicitly allowed in the +# RISC-V ISA, they're emulated by machine mode traps on all extant +# architectures. It's faster to have GCC emit only aligned accesses. +KBUILD_CFLAGS += $(call cc-option,-mstrict-align) + +head-y := arch/riscv/kernel/head.o + +core-y += arch/riscv/kernel/ arch/riscv/mm/ + +libs-y += arch/riscv/lib/ + +all: vmlinux diff --git a/arch/riscv/configs/freedom-u500_defconfig b/arch/riscv/configs/freedom-u500_defconfig new file mode 100644 index 000000000000..b37908d45067 --- /dev/null +++ b/arch/riscv/configs/freedom-u500_defconfig @@ -0,0 +1,53 @@ +CONFIG_CROSS_COMPILE="riscv64-unknown-linux-gnu-" +CONFIG_DEFAULT_HOSTNAME="ucbvax" +# CONFIG_CROSS_MEMORY_ATTACH is not set +# CONFIG_FHANDLE is not set +CONFIG_NAMESPACES=y +# CONFIG_SGETMASK_SYSCALL is not set +CONFIG_EMBEDDED=y +# CONFIG_BLK_DEV_BSG is not set +CONFIG_PARTITION_ADVANCED=y +# CONFIG_EFI_PARTITION is not set +# CONFIG_IOSCHED_DEADLINE is not set +# CONFIG_COMPACTION is not set +CONFIG_HZ_100=y +CONFIG_PCI_MSI=y +CONFIG_NET=y +CONFIG_UNIX=y +CONFIG_INET=y +# CONFIG_INET_XFRM_MODE_TRANSPORT is not set +# CONFIG_INET_XFRM_MODE_TUNNEL is not set +# CONFIG_INET_XFRM_MODE_BEET is not set +# CONFIG_INET_DIAG is not set +# CONFIG_IPV6 is not set +# CONFIG_WIRELESS is not set +CONFIG_DEVTMPFS=y +CONFIG_DEVTMPFS_MOUNT=y +# CONFIG_FIRMWARE_IN_KERNEL is not set +CONFIG_OF=y +# CONFIG_BLK_DEV is not set +# CONFIG_INPUT_KEYBOARD is not set +# CONFIG_INPUT_MOUSE is not set +# CONFIG_VT is not set +CONFIG_DEVKMEM=y +# CONFIG_HW_RANDOM is not set +# CONFIG_HWMON is not set +CONFIG_FB=y +# CONFIG_USB_SUPPORT is not set +# CONFIG_IOMMU_SUPPORT is not set +CONFIG_EXT2_FS=y +# CONFIG_FILE_LOCKING is not set +# CONFIG_DNOTIFY is not set +# CONFIG_INOTIFY_USER is not set +# CONFIG_PROC_PAGE_MONITOR is not set +# CONFIG_SYSFS is not set +CONFIG_TMPFS=y +# CONFIG_MISC_FILESYSTEMS is not set +# CONFIG_NETWORK_FILESYSTEMS is not set +CONFIG_PRINTK_TIME=y +# CONFIG_UNUSED_SYMBOLS is not set +CONFIG_DEBUG_SECTION_MISMATCH=y +# CONFIG_FRAME_POINTER is not set +# CONFIG_EARLY_PRINTK is not set +# CONFIG_CRYPTO_HW is not set +CONFIG_TTY=y diff --git a/arch/riscv/configs/spike32_defconfig b/arch/riscv/configs/spike32_defconfig new file mode 100644 index 000000000000..cf3431e94311 --- /dev/null +++ b/arch/riscv/configs/spike32_defconfig @@ -0,0 +1,50 @@ +CONFIG_64BIT=n +CONFIG_32BIT=y +CONFIG_ARCH_RV64I=n +CONFIG_ARCH_RV32I=y +CONFIG_PCI=y +CONFIG_DEFAULT_HOSTNAME="ucbvax" +# CONFIG_CROSS_MEMORY_ATTACH is not set +# CONFIG_FHANDLE is not set +CONFIG_NAMESPACES=y +CONFIG_EMBEDDED=y +# CONFIG_BLK_DEV_BSG is not set +CONFIG_PARTITION_ADVANCED=y +# CONFIG_EFI_PARTITION is not set +# CONFIG_IOSCHED_DEADLINE is not set +CONFIG_NET=y +CONFIG_UNIX=y +CONFIG_INET=y +# CONFIG_INET_XFRM_MODE_TRANSPORT is not set +# CONFIG_INET_XFRM_MODE_TUNNEL is not set +# CONFIG_INET_XFRM_MODE_BEET is not set +# CONFIG_INET_DIAG is not set +# CONFIG_IPV6 is not set +# CONFIG_WIRELESS is not set +CONFIG_DEVTMPFS=y +CONFIG_DEVTMPFS_MOUNT=y +# CONFIG_FIRMWARE_IN_KERNEL is not set +# CONFIG_BLK_DEV is not set +# CONFIG_INPUT_KEYBOARD is not set +# CONFIG_INPUT_MOUSE is not set +# CONFIG_VT is not set +CONFIG_DEVKMEM=y +# CONFIG_HW_RANDOM is not set +# CONFIG_HWMON is not set +CONFIG_FB=y +# CONFIG_USB_SUPPORT is not set +# CONFIG_IOMMU_SUPPORT is not set +CONFIG_EXT2_FS=y +# CONFIG_FILE_LOCKING is not set +# CONFIG_DNOTIFY is not set +# CONFIG_INOTIFY_USER is not set +# CONFIG_PROC_PAGE_MONITOR is not set +# CONFIG_SYSFS is not set +CONFIG_TMPFS=y +# CONFIG_MISC_FILESYSTEMS is not set +# CONFIG_NETWORK_FILESYSTEMS is not set +CONFIG_PRINTK_TIME=y +CONFIG_DEBUG_SECTION_MISMATCH=y +# CONFIG_FRAME_POINTER is not set +# CONFIG_CRYPTO_HW is not set +CONFIG_TTY=y diff --git a/arch/riscv/configs/spike64_defconfig b/arch/riscv/configs/spike64_defconfig new file mode 100644 index 000000000000..5ad6644df541 --- /dev/null +++ b/arch/riscv/configs/spike64_defconfig @@ -0,0 +1,46 @@ +CONFIG_PCI=y +CONFIG_DEFAULT_HOSTNAME="ucbvax" +# CONFIG_CROSS_MEMORY_ATTACH is not set +# CONFIG_FHANDLE is not set +CONFIG_NAMESPACES=y +CONFIG_EMBEDDED=y +# CONFIG_BLK_DEV_BSG is not set +CONFIG_PARTITION_ADVANCED=y +# CONFIG_EFI_PARTITION is not set +# CONFIG_IOSCHED_DEADLINE is not set +CONFIG_NET=y +CONFIG_UNIX=y +CONFIG_INET=y +# CONFIG_INET_XFRM_MODE_TRANSPORT is not set +# CONFIG_INET_XFRM_MODE_TUNNEL is not set +# CONFIG_INET_XFRM_MODE_BEET is not set +# CONFIG_INET_DIAG is not set +# CONFIG_IPV6 is not set +# CONFIG_WIRELESS is not set +CONFIG_DEVTMPFS=y +CONFIG_DEVTMPFS_MOUNT=y +# CONFIG_FIRMWARE_IN_KERNEL is not set +# CONFIG_BLK_DEV is not set +# CONFIG_INPUT_KEYBOARD is not set +# CONFIG_INPUT_MOUSE is not set +# CONFIG_VT is not set +CONFIG_DEVKMEM=y +# CONFIG_HW_RANDOM is not set +# CONFIG_HWMON is not set +CONFIG_FB=y +# CONFIG_USB_SUPPORT is not set +# CONFIG_IOMMU_SUPPORT is not set +CONFIG_EXT2_FS=y +# CONFIG_FILE_LOCKING is not set +# CONFIG_DNOTIFY is not set +# CONFIG_INOTIFY_USER is not set +# CONFIG_PROC_PAGE_MONITOR is not set +# CONFIG_SYSFS is not set +CONFIG_TMPFS=y +# CONFIG_MISC_FILESYSTEMS is not set +# CONFIG_NETWORK_FILESYSTEMS is not set +CONFIG_PRINTK_TIME=y +CONFIG_DEBUG_SECTION_MISMATCH=y +# CONFIG_FRAME_POINTER is not set +# CONFIG_CRYPTO_HW is not set +CONFIG_TTY=y -- 2.13.0
[toc] | [prev] | [next] | [standalone]
Page 3 of 6 — ← Prev page 1 2 [3] 4 5 6 Next page →
Back to top | Article view | linux.kernel
csiph-web