Re: [PATCH 03/20] asm-generic: Drop getrlimit and setrlimit syscalls from default list
On Tue, 20 Jun 2017 08:27:36 PDT (-0700), Arnd Bergmann wrote: > On Tue, Jun 20, 2017 at 4:54 PM, Yury Norov wrote: >> On Tue, Jun 20, 2017 at 04:20:43PM +0200, Arnd Bergmann wrote: >>> On Tue, Jun 20, 2017 at 3:37 PM, Yury Norov >>> wrote: >>> > On Mon, Jun 19, 2017 at 11:10:23PM +0100, James Hogan wrote: >>> >> On Mon, Jun 19, 2017 at 11:58:41PM +0200, Arnd Bergmann wrote: > >>> > I would also notice riscv people and welcome to the discussion. >>> > >>> > As there is more than 1 arch that goes to be added to linux soon, >>> > maybe it's better to upstream my ans James' patches separately >>> > from other ilp32 patches? Arnd? >>> >>> Do you mean upstream those two patches slightly later? That's >>> fine with me, I don't care much whether the old new stat is part >>> of the syscall table for arm64-ilp32 or not, I'd leave that up to >>> you, depending on whether you want to do the rework or not. >> >> I mean that if we want to deprecate rlimit and stat syscalls for >> architectures that are under development now, it's better to upstream >> patches that actually deprecate it as early as possible. > > Makes sense. > >>> I suppose the arm64-ilp32 could benefit from not having to support >>> the old arm32 stat structure, but doing the new syscalls based on >>> statx could delay the glibc port some more, as there are some open >>> questions about how that would best be integrated. >> >> OK. Let's leave things as is. But then I don't see any reason to >> add unxstat patch to ilp32 series if ilp32 will not disable it. > > Right, that's what I meant: let's leave the rlimit patch in your series > as it matches the work you have already done, and is the right > thing to do, and let's do the unxstat patch separately so it doesn't > cause you extra work. Thanks for the heads up. We're in the process of submitting glibc now with the goal of getting into 2.26, so I think that means we'll be stuck with stat. I'm perfectly happy to deprecate whatever is feasible, though.
Re: [PATCH 1/7] RISC-V: Top-Level Makefile for riscv{32,64}
On Tue, 23 May 2017 04:30:50 PDT (-0700), Arnd Bergmann wrote: > On Tue, May 23, 2017 at 2:41 AM, Palmer Dabbelt wrote: >> RISC-V has both 32-bit and 64-bit base ISAs, but they are very similar. >> Like some other platforms, we'd like to share one arch directory between >> the two of them. > > I think we mainly do the others for backwards-compatibility with ancient > build scripts, and we don't need that here. Instead, you could add one more > line to the 'SUBARCH:=' statement that interprets the uname output. I don't think that does the same thing. The desired effect of this diff is: * "uname -m" when running on a RISC-V machine returns either riscv32 or riscv64, as that's what tools like autoconf expect when trying to find tuples. * I can cross compile for riscv32 and riscv64. That's currently controlled by a Kconfig setting, but ARCH=riscv32 vs ARCH=riscv64 controlls what defconfig sets. * I can natively compile for riscv32 and riscv64. That uses the same Kconfig setting, and the same ARCH=riscv32 vs ARCH=riscv64 switch for defconfig. Neither of the two Kconfig issues is a big deal, but we de need "uname -m" to return "riscv64" or "riscv32" not "riscv". I think the only way to do that is to set SRCARCH, but I'd be happy to change it if there's a better way. I think if I just do this diff --git a/Makefile b/Makefile index 0606f28..4adc609 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 # --- @@ -269,14 +270,6 @@ ifeq ($(ARCH),x86_64) SRCARCH := x86 endif -# Additional ARCH settings for RISC-V -ifeq ($(ARCH),riscv32) - SRCARCH := riscv -endif -ifeq ($(ARCH),riscv64) - SRCARCH := riscv -endif - # Additional ARCH settings for sparc ifeq ($(ARCH),sparc32) SRCARCH := sparc then I'll end up with "uname -m" as "riscv" -- I haven't tried it, but that's why we ended up with this diff in the first place.
Re: [PATCH 2/7] RISC-V: arch/riscv Makefile and Kconfigs
On Tue, 23 May 2017 04:46:22 PDT (-0700), Arnd Bergmann wrote: > On Tue, May 23, 2017 at 2:41 AM, Palmer Dabbelt wrote: >> --- >> arch/riscv/.gitignore| 35 >> arch/riscv/Kconfig | 300 >> +++ >> arch/riscv/Makefile | 64 >> arch/riscv/configs/riscv32_spike | 47 ++ >> arch/riscv/configs/riscv64_freedom-u | 52 ++ >> arch/riscv/configs/riscv64_qemu | 64 >> arch/riscv/configs/riscv64_spike | 45 ++ >> 7 files changed, 607 insertions(+) >> create mode 100644 arch/riscv/.gitignore >> create mode 100644 arch/riscv/Kconfig >> create mode 100644 arch/riscv/Makefile >> create mode 100644 arch/riscv/configs/riscv32_spike >> create mode 100644 arch/riscv/configs/riscv64_freedom-u >> create mode 100644 arch/riscv/configs/riscv64_qemu >> create mode 100644 arch/riscv/configs/riscv64_spike >> >> diff --git a/arch/riscv/.gitignore b/arch/riscv/.gitignore >> new file mode 100644 >> index ..376d06eb5d52 >> --- /dev/null >> +++ b/arch/riscv/.gitignore >> @@ -0,0 +1,35 @@ >> +# Now un-ignore all files. >> +!* >> + >> +# But then re-ignore the files listed in the Linux .gitignore >> +# Normal rules >> +# >> +.* >> +*.o >> +*.o.* >> +*.a > > This doesn't seem to belong here: There is no reason for riscv > to be different from all other architectures. Is something wrong > with the top-level .gitignore? If so, we should just fix it there. Sorry, this snuck in from how we used to track the kernel. It's been fixed. >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >> new file mode 100644 >> index ..510ead1d3343 >> --- /dev/null >> +++ b/arch/riscv/Kconfig >> @@ -0,0 +1,300 @@ >> +# >> +# For a description of the syntax of this configuration file, >> +# see Documentation/kbuild/kconfig-language.txt. >> +# >> + >> +config RISCV >> + def_bool y >> + select OF >> + select OF_EARLY_FLATTREE >> + select OF_IRQ >> + select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE >> + select ARCH_WANT_FRAME_POINTERS >> + select CLONE_BACKWARDS >> + select COMMON_CLK >> + select GENERIC_CLOCKEVENTS >> + select GENERIC_CPU_DEVICES >> + select GENERIC_IRQ_SHOW >> + select GENERIC_PCI_IOMAP > > You normally don't want GENERIC_PCI_IOMAP, unless your > inb()/outb() uses other instructions than your readl()/writel() We don't have any special instructions for inb/outb, but there is a special fence to ensure ordering. It looks like most of my candidates for patterning things after use GENERIC_PCI_IOMAP, but I ended up setting GENERIC_IOMAP and things still at least build and boot without any PCI https://github.com/riscv/riscv-linux/commit/c43c599b0dd9d15886c03a9dd179f8936b0cbb2e I'll be sure to check if I broke PCI, as I don't actually know what's going on here. >> +config MMU >> + def_bool y > > Just a general question: has there been any interest in a no-MMU > version? One of the fun things about RISC-V is that there's at least some interest in _everything_... :). There hasn't been any serious interest, and I don't know of anyone building systems where no-MMU Linux would run (our DDR phy is a lot bigger than our MMU), but I wouldn't rule it out. >> +# even on 32-bit, physical (and DMA) addresses are > 32-bits >> +config ARCH_PHYS_ADDR_T_64BIT >> + def_bool y >> + >> +config ARCH_DMA_ADDR_T_64BIT >> + def_bool y > > Are you required to use 64-bit addressing for RAM on 32-bit > architectures though? Using 32-bit dma_addr_t and phys_addr_t > when possible makes some code noticeably more efficient. > >> +config PGTABLE_LEVELS >> + int >> + default 3 if 64BIT >> + default 2 > > With 2-level page tables, you usually can't address much more > than 32-bit physical memory anyway, so I'd guess that most > 32-bit chips would actually put their RAM under the 4GB boundary. We can address 34 bits of physical address space on Sv32 (the 32-bit virtual addressing scheme in RV32). If this is a meaningful performance constraint then we could always make this configurable. >> +config RV_ATOMIC >> + bool "Use atomic memory instructions (RV32A or RV64A)" >> + default y >> + >> +config RV_SYSRISCV_ATOMIC >> + bool "Include support for atomic operation syscalls" >> +
Re: [PATCH 3/7] RISC-V: Device Tree Documentation
On Tue, 23 May 2017 05:03:17 PDT (-0700), Arnd Bergmann wrote: > On Tue, May 23, 2017 at 2:41 AM, Palmer Dabbelt wrote: >> --- >> .../interrupt-controller/riscv,cpu-intc.txt| 46 >> ++ >> .../bindings/interrupt-controller/riscv,plic0.txt | 44 >> + > > The patch needs a description, and should be sent to the irqchip maintainers > and the devicetree maintainers for review, along for the respective > drivers/irqchip/ > patch. OK, I'll include that as part of my v2.
Pull Request for linux-next
I'd like to request that you add the following tree to linux-next git://git.kernel.org/pub/scm/linux/kernel/git/palmer/linux.git for-linux-next It contains the RISC-V port, which has been through 9 rounds of review on various public Linux mailing lists. While the port isn't perfect, I've split out the commits for which we have general consensus and tagged them with "for-linux-next" (as opposed to our "riscv-next" tree). I intend to continue updating the "for-linux-next" tag as we get more patches in good shape, with the idea being that the RISC-V arch port patches go in through here. I know it's a bit late in the cycle, but we've been waiting for you to come back from vacation before submitting the request so it didn't get lost somewhere. Thanks!
[GIT PULL] RISC-V Port for Linux 4.15 v9
up patches have been dropped, we'll do them as a separate patch set later. * We've the Kconfig entries from CONFIG_ISA_* to CONFIG_RISCV_ISA_*, to make grep easier. * There have been a handful of memory model related tweaks in I/O land, particularly relating the PCI and the upcoming platform specification. There are significant comments in the relevant files. This is still a WIP, but I think we're close to getting as good as we're going to get until we end up with some more specifications. (v6) As it's been only a day since the v5 patch set, the changes are pretty minimal: * The patch set is now based on linux-next/master, which I believe is a better base now that we're getting closer to upstream. * EARLY_PRINTK is no longer an option. Since the SBI console is reasonable, there's no penalty to enabling it (and thus no benefit to disabling it). * The mmap syscalls were refactored a bit. (v5) Things have really started to calm down, so this is fairly similar to the v4 patch set. The most interesting changes include: * We've moved back to a single patch set. * SMP support has been fixed, I was accidentally running on a non-SMP configuration. There were various mistakes all over the tree as a result of this. * The cmpxchg syscalls have been removed, as they were deemed a bad idea. As a result, RISC-V Linux systems mandate the A extension. The corresponding Kconfig entry to enable builds on non-A systems has been removed. * A few more atomic fixes: mostly fence changes, but those resulted in a handful of additional macros that were no longer necessary. * riscv_early_sie has been removed. (v4) There have only been a few changes since the v3 patch set: * The cmpxchg64 syscall is no longer enabled on 32-bit systems. It's not possible to provide this on SMP systems, and it's not necessary as glibc knows not to call it. * We provide a ELF_HWCAP so users can determine the ISA of the machine the kernel is running on. * The multi-line comments are in a better form. * There were a handful of headers that could be replaced with the asm-generic versions, and a few unnecessary definitions. * We no longer use printk, but instead use pr_*. * A few Kconfig and defconfig entries have been cleaned up. (v3) A highlight of the changes since the v2 patch set includes: * We've split out all our drivers into separate patch sets, which I've already sent out to the relevant maintainers. I haven't included those patches in this patch set, but some of them are necessary to build our port. A git tree that contains all our patch sets merged together lives at <https://github.com/riscv/riscv-linux/tree/riscv-for-submission-v3>. * The patch set is now split up differently: rather than being split per directory it is split per topic. Hopefully this will make it easier to review the port on the mailing list. The split is a bit rough, so you probably still want to look at the patch set as a whole. * atomic.h has been completely rewritten and is hopefully now correct. I've attempted to sanitize the various other memory model related code as well, and I think it should all be sane now aside from a handful of FIXMEs commented in the code. * We've changed the cmpexchg syscall to always exist and to not be multiplexed. There is also a VDSO entry for compare and exchange, which allows kernels with the A extension to execute user code without the A extension reasonably fast. * Our user-visible register state now contains enough space for the Q extension for 128-bit floating point, as well as a few words to allow extensibility to future ISA extensions like the eventual V extension for vectors. * A handful of driver cleanups, but these have been split into separate patch sets now so I won't duplicate them here. (v2) A highlight of the changes since the v1 patch set includes: * We've split out our drivers into the right places, which means now there's a lot more patches. I'll be submitting these patches to various subsystem maintainers and including them in any future RISC-V patch sets until they've been merged. * The SBI console driver has been completely rewritten to use the HVC helpers and is now significantly smaller. * We've begun to use weaker barriers as opposed to just the big "fence". There's still some work to do here, specifically: - We need fences in the relaxed MMIO functions. - The non-relaxed MMIO functions are missing R/W bits on their fences. - Many AMOs need the aq and rl bits set. * We now have thread_info in task_struct. As a result, sscratch now contains TP instead of SP. This was necessary because thread_info is no longer on the stack. * A few shared routines have been added that we use instead of crea
Re: [GIT PULL] RISC-V Port for Linux 4.15 v9
On Tue, 14 Nov 2017 01:04:42 PST (-0800), Arnd Bergmann wrote: > On Mon, Nov 13, 2017 at 10:56 PM, Palmer Dabbelt wrote: >> The following changes since commit bebc6082da0a9f5d47a1ea2edc099bf671058bd4: >> >> Linux 4.14 (2017-11-12 10:46:13 -0800) >> >> are available in the git repository at: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/palmer/linux.git >> tags/riscv-for-linus-4.15-arch-v9 >> >> for you to fetch changes up to 512d88db5ef3de56f392f761657c2ab2cadc0498: >> >> Merge tag 'v4.14' into for-linus (2017-11-13 13:17:51 -0800) >> >> >> RISC-V Port for Linux 4.15 v9 >> >> This tag contains the core RISC-V Linux port, which has been through >> nine rounds of review on various mailing lists. The port is not >> complete: there's some cleanup patches moving through the review >> process, a whole bunch of drivers that need some work, and a lot of >> feature additions that will be needed. >> >> The patches contained in this tag have been through nine rounds of >> review on the various mailing lists. I have some outstanding cleanup >> patches, but since there's been so much review on these patches I >> thought it would be best to submit them as-is and then submit explicit >> cleanup patches so everyone can review them. This first patch set is >> big enough that it's a bit of a pain to constantly rewrite, and it's >> caused a few headaches with various contributors. >> >> The port is definately a work in progress. While what's there builds >> and boots with 4.14, it's a bit hard to actually see anything happen >> because there are no device drivers yet. I maintain a staging branch >> that contains all the device drivers and cleanup that actually works, >> but those patches won't all be ready for a while. I'd like to get what >> we currently have into your tree so everyone can start working from a >> single base -- of particular importance is allowing the glibc >> upstreaming process to proceed so we can sort out any possibly lingering >> user-visible ABI problems we might have. > > For the contents: > > Reviewed-by: Arnd Bergmann Thanks! > > As you say, there are a few FIXME's left, but nothing that can't > just be handled after the merge. > > Two small things I noticed about the pull request: > > - I see you pulled in the v4.14 tag, presumably to have a cleaner > merge base. It's better not to do that kind of "back-merge", it > will mess up the git history and it doesn't really help with anything > important like bisection through the series. Oh, sorry. I wasn't sure what to do. I've gone ahead and tagged riscv-for-linus-4.15-arch-v9-premerge with my pre-merge commit, in case that's better to merge in. > - The last commit before the backmerge is from Sep 26. I would > assume that you have done other work since then (I saw at > least one patch). Shouldn't they be included here? I guess my thought was to include just the parts that had been fully through the on-list review process, and then to submit another cleanup pull request later with all our fixes. The big chunk of those are going to come from talking with the glibc people to finalize our ABI, which is easier to do once we've been merged in. It's a bit of a chicken and egg problem, so I thought I'd just break the cycle by getting _something_ in :).
Re: [GIT PULL] RISC-V Port for Linux 4.15 v9
On Tue, 14 Nov 2017 11:23:08 PST (-0800), will.dea...@arm.com wrote: Hi Palmer, On Mon, Nov 13, 2017 at 01:56:22PM -0800, Palmer Dabbelt wrote: The following changes since commit bebc6082da0a9f5d47a1ea2edc099bf671058bd4: Linux 4.14 (2017-11-12 10:46:13 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/palmer/linux.git tags/riscv-for-linus-4.15-arch-v9 for you to fetch changes up to 512d88db5ef3de56f392f761657c2ab2cadc0498: Merge tag 'v4.14' into for-linus (2017-11-13 13:17:51 -0800) [...] RISC-V: Atomic and Locking Code I had some open comments on this patch: http://lkml.kernel.org/r/20171024141032.gd13...@arm.com Amongst other things, you're adding a spin_unlock_wait and I think your test bitops are missing barriers. I'm not suggesting this holds up the merge, but a reply would've been nice :/ Sorry about that, it must have gotten buried in my inbox somewhere. We have a patch already to remove spin_unlock_wait, it's just not in this because it didn't get any comments. I'll look through the rest of your comments. Will
Re: [PATCH v9 05/12] RISC-V: Atomic and Locking Code
On Tue, 24 Oct 2017 07:10:33 PDT (-0700), will.dea...@arm.com wrote: Hi Palmer, Some late comments on this which you might want to address as you get the chance. Sorry, this disappeared into my inbox. I've replied in-line to all your comments, but in the interest of making sure I didn't lose the message again the code contained here has been minimally tested. All the commits live on a branch now, so hopefully I won't lose it this time :). On Tue, Sep 26, 2017 at 06:56:31PM -0700, Palmer Dabbelt wrote: This contains all the code that directly interfaces with the RISC-V memory model. While this code corforms to the current RISC-V ISA specifications (user 2.2 and priv 1.10), the memory model is somewhat underspecified in those documents. There is a working group that hopes to produce a formal memory model by the end of the year, but my understanding is that the basic definitions we're relying on here won't change significantly. Reviewed-by: Arnd Bergmann Signed-off-by: Palmer Dabbelt --- arch/riscv/include/asm/atomic.h | 375 arch/riscv/include/asm/barrier.h| 68 ++ arch/riscv/include/asm/bitops.h | 218 +++ arch/riscv/include/asm/cacheflush.h | 39 arch/riscv/include/asm/cmpxchg.h| 134 arch/riscv/include/asm/io.h | 303 ++ arch/riscv/include/asm/spinlock.h | 165 ++ arch/riscv/include/asm/spinlock_types.h | 33 +++ arch/riscv/include/asm/tlb.h| 24 ++ arch/riscv/include/asm/tlbflush.h | 64 ++ 10 files changed, 1423 insertions(+) create mode 100644 arch/riscv/include/asm/atomic.h create mode 100644 arch/riscv/include/asm/barrier.h create mode 100644 arch/riscv/include/asm/bitops.h create mode 100644 arch/riscv/include/asm/cacheflush.h create mode 100644 arch/riscv/include/asm/cmpxchg.h create mode 100644 arch/riscv/include/asm/io.h create mode 100644 arch/riscv/include/asm/spinlock.h create mode 100644 arch/riscv/include/asm/spinlock_types.h create mode 100644 arch/riscv/include/asm/tlb.h create mode 100644 arch/riscv/include/asm/tlbflush.h I'm snipping out some parts, because it's long. I've also dropped some CCs, as it's old and I think I had a few too many people at the time (the patches were all joined together). +#define ATOMIC_OP(op, asm_op, c_op, I, asm_type, c_type, prefix) \ +static __always_inline void atomic##prefix##_##op(c_type i, atomic##prefix##_t *v) \ +{ \ + __asm__ __volatile__ ( \ + "amo" #asm_op "." #asm_type " zero, %1, %0" \ + : "+A" (v->counter) \ + : "r" (I) \ + : "memory"); \ +} + +#ifdef CONFIG_GENERIC_ATOMIC64 +#define ATOMIC_OPS(op, asm_op, c_op, I)\ +ATOMIC_OP (op, asm_op, c_op, I, w, int, ) +#else +#define ATOMIC_OPS(op, asm_op, c_op, I)\ +ATOMIC_OP (op, asm_op, c_op, I, w, int, ) \ +ATOMIC_OP (op, asm_op, c_op, I, d, long, 64) +#endif + +ATOMIC_OPS(add, add, +, i) +ATOMIC_OPS(sub, add, +, -i) +ATOMIC_OPS(and, and, &, i) +ATOMIC_OPS( or, or, |, i) +ATOMIC_OPS(xor, xor, ^, i) What is the point in the c_op parameter to these things? I guess there isn't one, it just lingered from a handful of refactorings. It's used in some of the other functions if we need to do a C operation after the atomic. How does this look? commit 5db229491a205ad0e1aa18287e3b342176c62d30 (HEAD -> staging-mm) Author: Palmer Dabbelt Date: Tue Nov 14 11:35:37 2017 -0800 RISC-V: Remove c_op from ATOMIC_OP This was an unused macro parameter. Signed-off-by: Palmer Dabbelt diff --git a/arch/riscv/include/asm/atomic.h b/arch/riscv/include/asm/atomic.h index e2e37c57cbeb..a76a094c18f9 100644 --- a/arch/riscv/include/asm/atomic.h +++ b/arch/riscv/include/asm/atomic.h @@ -50,30 +50,30 @@ static __always_inline void atomic64_set(atomic64_t *v, long i) * have the AQ or RL bits set. These don't return anything, so there's only * one version to worry about. */ -#define ATOMIC_OP(op, asm_op, c_op, I, asm_type, c_type, prefix) \ -static __always_inline void atomic##prefix##_##op(c_type i, atomic##prefix##_t *v) \ -{
Re: [PATCH] Remove #ifdef CONFIG_64BIT from all asm-generic/fcntl.h
On Mon, 07 Sep 2015 06:16:37 PDT (-0700), a...@arndb.de wrote: > On Tuesday 01 September 2015 17:10:10 Palmer Dabbelt wrote: >> From: Palmer Dabbelt >> >> When working on the RISC-V port I noticed that F_SETLK64 was being >> defined on our 64-bit platform, despite our port being so new that >> we've only ever had the 64-bit file ops. Since there's not compat >> layer for these, this causes fcntl to bail out. >> >> It turns out that one of the ways in with F_SETLK64 was being defined >> (there's some more in glibc, but that's a whole different story... :)) >> is the result of CONFIG_64BIT showing up in this user-visible header. >> confirms this isn't sane, so I replaced it >> with a __BITS_PER_LONG check. >> >> I went ahead and grep'd for any more of these (with >> headers_install_all), and this was the only one I found. >> >> Signed-off-by: Palmer Dabbelt >> Reviewed-by: Andrew Waterman >> Reviewed-by: Albert Ou > > Looks good to me. Are you planning to submit the RISC-V port upstream > any time soon? If so, just keep the patch in your tree and add my > > Acked-by: Arnd Bergmann The RISC-V stuff is still a few months off, that's why I submitted this upstream stand-alone. The supervisor specification isn't 100% set in stone yet, and we're waiting on that before upstreaming anything significant. > However, I did see a lot of similar bugs now that you point me to it: > > $ grep -r \\\ obj-tmp/usr/include/asm-generic/fcntl.h:#ifndef CONFIG_64BIT > obj-tmp/usr/include/asm-generic/mman-common.h:#ifdef > CONFIG_MMAP_ALLOW_UNINITIALIZED > obj-tmp/usr/include/asm-generic/unistd.h:#ifdef CONFIG_MMU > obj-tmp/usr/include/asm-generic/unistd.h:#endif /* CONFIG_MMU */ > obj-tmp/usr/include/linux/atmdev.h:#ifdef CONFIG_COMPAT > obj-tmp/usr/include/linux/elfcore.h:#ifdef CONFIG_BINFMT_ELF_FDPIC > obj-tmp/usr/include/linux/eventpoll.h:#ifdef CONFIG_PM_SLEEP > obj-tmp/usr/include/linux/fb.h:#ifdef CONFIG_FB_BACKLIGHT > obj-tmp/usr/include/linux/flat.h:#ifdef CONFIG_BINFMT_SHARED_FLAT > obj-tmp/usr/include/linux/hw_breakpoint.h:#ifdef > CONFIG_HAVE_MIXED_BREAKPOINTS_REGS > obj-tmp/usr/include/linux/pktcdvd.h:#if defined(CONFIG_CDROM_PKTCDVD_WCACHE) > obj-tmp/usr/include/linux/raw.h:#define MAX_RAW_MINORS CONFIG_MAX_RAW_DEVS > obj-tmp/usr/include/asm/ptrace.h:#ifdef CONFIG_CPU_ENDIAN_BE8 > > These all have the same problem, and we should fix them, as well as > (probably) adding an automated check to scripts/headers_install.sh. Well, I was going to go fix them all and ran a very similar grep, but I think I got a lot of false-positives. If I understand correctly, it's allowed to have CONFIG_* when guarded by __KERNEL__ in user-visible headers? Now that I've written that, I realize it'd be pretty easy to just use cpp to drop everything inside __KERNEL__ and then look for CONFIG_*. If you want, I can try to do that, fix what triggers the check, and re-submit everything together? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 01/13] Remove #ifdef CONFIG_64BIT from all asm-generic/fcntl.h
When working on the RISC-V port I noticed that F_SETLK64 was being defined on our 64-bit platform, despite our port being so new that we've only ever had the 64-bit file ops. Since there's not compat layer for these, this causes fcntl to bail out. It turns out that one of the ways in with F_SETLK64 was being defined (there's some more in glibc, but that's a whole different story... :)) is the result of CONFIG_64BIT showing up in this user-visible header. confirms this isn't sane, so I replaced it with a __BITS_PER_LONG check. Signed-off-by: Palmer Dabbelt Reviewed-by: Andrew Waterman Reviewed-by: Albert Ou --- include/uapi/asm-generic/fcntl.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h index e063effe0cc1..14a5c8237d84 100644 --- a/include/uapi/asm-generic/fcntl.h +++ b/include/uapi/asm-generic/fcntl.h @@ -1,6 +1,7 @@ #ifndef _ASM_GENERIC_FCNTL_H #define _ASM_GENERIC_FCNTL_H +#include #include /* @@ -115,7 +116,7 @@ #define F_GETSIG 11 /* for sockets. */ #endif -#ifndef CONFIG_64BIT +#if (__BITS_PER_LONG == 32) #ifndef F_GETLK64 #define F_GETLK64 12 /* using 'struct flock64' */ #define F_SETLK64 13 -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 07/13] Make FB_BACKLIGHT_{LEVELS,MAX} always visible
Nothing else in the kernel defines this, and this header is visible to userspace. Rather than hiding it in an #ifdef, I think it's sane to just make this visible to userspace. Signed-off-by: Palmer Dabbelt Reviewed-by: Andrew Waterman Reviewed-by: Albert Ou --- include/uapi/linux/fb.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h index fb795c3b3c17..8926f13bc19f 100644 --- a/include/uapi/linux/fb.h +++ b/include/uapi/linux/fb.h @@ -392,11 +392,8 @@ struct fb_cursor { struct fb_image image; /* Cursor image */ }; -#ifdef CONFIG_FB_BACKLIGHT /* Settings for the generic backlight code */ #define FB_BACKLIGHT_LEVELS128 #define FB_BACKLIGHT_MAX 0xFF -#endif - #endif /* _UAPI_LINUX_FB_H */ -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove #ifdef CONFIG_64BIT from all asm-generic/fcntl.h
I cut the RISC-V stuff, but I intend to reply to it later. As you said, it's just a different topic. >>> However, I did see a lot of similar bugs now that you point me to it: >>> >>> $ grep -r \\\>> obj-tmp/usr/include/asm-generic/fcntl.h:#ifndef CONFIG_64BIT >>> obj-tmp/usr/include/asm-generic/mman-common.h:#ifdef >>> CONFIG_MMAP_ALLOW_UNINITIALIZED >>> obj-tmp/usr/include/asm-generic/unistd.h:#ifdef CONFIG_MMU >>> obj-tmp/usr/include/asm-generic/unistd.h:#endif /* CONFIG_MMU */ >>> obj-tmp/usr/include/linux/atmdev.h:#ifdef CONFIG_COMPAT >>> obj-tmp/usr/include/linux/elfcore.h:#ifdef CONFIG_BINFMT_ELF_FDPIC >>> obj-tmp/usr/include/linux/eventpoll.h:#ifdef CONFIG_PM_SLEEP >>> obj-tmp/usr/include/linux/fb.h:#ifdef CONFIG_FB_BACKLIGHT >>> obj-tmp/usr/include/linux/flat.h:#ifdef CONFIG_BINFMT_SHARED_FLAT >>> obj-tmp/usr/include/linux/hw_breakpoint.h:#ifdef >>> CONFIG_HAVE_MIXED_BREAKPOINTS_REGS >>> obj-tmp/usr/include/linux/pktcdvd.h:#if defined(CONFIG_CDROM_PKTCDVD_WCACHE) >>> obj-tmp/usr/include/linux/raw.h:#define MAX_RAW_MINORS CONFIG_MAX_RAW_DEVS >>> obj-tmp/usr/include/asm/ptrace.h:#ifdef CONFIG_CPU_ENDIAN_BE8 >>> >>> These all have the same problem, and we should fix them, as well as >>> (probably) adding an automated check to scripts/headers_install.sh. >> >> Well, I was going to go fix them all and ran a very similar grep, but >> I think I got a lot of false-positives. If I understand correctly, >> it's allowed to have CONFIG_* when guarded by __KERNEL__ in >> user-visible headers? > > That is right. It turns out there was actually a header checking script (scripts/headers_check.pl), and it already had a check for this. The check was just disabled because there was "too much noise". Rather than putting it in headers_install I've just fixed that script. I'm definately lacking in perl powers, so I have no idea if what I've done is sane. Specifically: there's a global variable and a line over 80 characters, but since there's a bunch of other violations I figure it's fine. >> Now that I've written that, I realize it'd be pretty easy to just use >> cpp to drop everything inside __KERNEL__ and then look for CONFIG_*. > > The lines quoted above are from the output of 'make headers_install', > which already drops everything inside of __KERNEL__. A lot of them > probably just need to add that #ifdef, or move the portion of the > header file to the normal (non-uabi) file. > >> If you want, I can try to do that, fix what triggers the check, and >> re-submit everything together? > > That would be great, yes. OK. I think this has turned into more of a RFC than a PATCH, though... I've just #ifdef'd things for now to reduce the diff size, though I think it might be cleaner to move some of them to the non-user headers (ep_take_care_of_epollwakeup(), USE_WCACHING, MAX_RAW_MINORS). I'm pretty far out of my depth here, so these should all be carefully considered, but there's a few that scare me more ("struct elf_prstatus", "enum by_type_idx", AT_VECTOR_SIZE_ARCH). I think there's only one actual bug here (MAP_UNINITIALIZED), the rest just quiet the checking script. Each patch has my rationale for what I did. Since this touches a whole lot of stuff, I've added a whole bunch of CCs. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 09/13] Hide bp_type_idx behind #ifdef __KERNEL__
I'm actually not sure what to do here: if this enum is meant to be used by userspace, then it has to be the same regardless of kernel configuration. One option would be to have the kernel expose all the values to userspace and then map them internally if CONFIG_HAVE_MIXED_BREAKPOINT_REGS isn't set, but that feels like it'd be more invasive. Here I took the simple and fail-fast route to hide all the definitions. Signed-off-by: Palmer Dabbelt Reviewed-by: Andrew Waterman Reviewed-by: Albert Ou --- include/uapi/linux/hw_breakpoint.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/uapi/linux/hw_breakpoint.h b/include/uapi/linux/hw_breakpoint.h index b04000a2296a..2498bfbf56c4 100644 --- a/include/uapi/linux/hw_breakpoint.h +++ b/include/uapi/linux/hw_breakpoint.h @@ -17,14 +17,16 @@ enum { HW_BREAKPOINT_INVALID = HW_BREAKPOINT_RW | HW_BREAKPOINT_X, }; +#ifdef __KERNEL__ enum bp_type_idx { TYPE_INST = 0, -#ifdef CONFIG_HAVE_MIXED_BREAKPOINTS_REGS +#if defined(CONFIG_HAVE_MIXED_BREAKPOINTS_REGS) TYPE_DATA = 0, #else TYPE_DATA = 1, #endif TYPE_MAX }; +#endif /* __KERNEL__ */ #endif /* _UAPI_LINUX_HW_BREAKPOINT_H */ -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 13/13] Re-enable and clean up "check_config()" in headers_check.pl
I recently got bit by a CONFIG_ in userspace bug. This has apparently happened before, but the check got disabled for triggering too much. In order to reduce false positives, I added some hueristics to avoid detecting comments. Since these tests all pass, I've now re-enabled them. Signed-off-by: Palmer Dabbelt Reviewed-by: Andrew Waterman Reviewed-by: Albert Ou --- scripts/headers_check.pl | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/scripts/headers_check.pl b/scripts/headers_check.pl index 62320f93e903..dd413ad9c850 100755 --- a/scripts/headers_check.pl +++ b/scripts/headers_check.pl @@ -40,7 +40,7 @@ foreach my $file (@files) { &check_asm_types(); &check_sizetypes(); &check_declarations(); - # Dropped for now. Too much noise &check_config(); + &check_config(); } close $fh; } @@ -76,9 +76,24 @@ sub check_declarations } } +my $check_config_in_multiline_comment = 0; sub check_config { - if ($line =~ m/[^a-zA-Z0-9_]+CONFIG_([a-zA-Z0-9_]+)[^a-zA-Z0-9_]/) { + my $nocomments = $line; + $nocomments =~ s/\/\*.*\*\///; # Remove ANSI-style comments (/* to */) + $nocomments =~ s/\/\/.*//; # Remove C99-style comments (// to EOL) + + # Check to see if we're within a multiline comment, and if so + # just remove the whole line. I tried matching on '^ * ', but + # there's one header that doesn't prepend multi-line comments + # with * so that won't work. + if ($nocomments =~ m/\/\*/) { $check_config_in_multiline_comment = 1; } + if ($nocomments =~ m/\*\//) { $check_config_in_multiline_comment = 0; } + if ($check_config_in_multiline_comment == 1) { $nocomments = "" } + + # Check to see if there is something that looks like CONFIG_ + # inside a userspace-accessible header file and if so, print that out. + if ($nocomments =~ m/[^a-zA-Z0-9_]+CONFIG_([a-zA-Z0-9_]+)[^a-zA-Z0-9_]/) { printf STDERR "$filename:$lineno: leaks CONFIG_$1 to userspace where it is not valid\n"; } } -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 08/13] Hide MAX_SHARED_LIBS behind #ifdef __KERNEL__
I'm not sure what this is, but it doesn't feel like something that should be exposed to userspace here. I'm assuming this file was exposed for the structure in it, which doesn't depend on MAX_SHARED_LIBS. Signed-off-by: Palmer Dabbelt Reviewed-by: Andrew Waterman Reviewed-by: Albert Ou --- include/uapi/linux/flat.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/uapi/linux/flat.h b/include/uapi/linux/flat.h index 88cd6baba8f3..4ed679f3591e 100644 --- a/include/uapi/linux/flat.h +++ b/include/uapi/linux/flat.h @@ -13,11 +13,13 @@ #defineFLAT_VERSION0x0004L +#ifdef __KERNEL__ #ifdef CONFIG_BINFMT_SHARED_FLAT #defineMAX_SHARED_LIBS (4) #else #defineMAX_SHARED_LIBS (1) #endif +#endif /*__KERNEL__*/ /* * To make everything easier to port and manage cross platform -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 10/13] Hide USE_WCACHING behind #ifdef __KERNEL__
I don't think this was ever intended to be exposed to userspace. Signed-off-by: Palmer Dabbelt Reviewed-by: Andrew Waterman Reviewed-by: Albert Ou --- include/uapi/linux/pktcdvd.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/uapi/linux/pktcdvd.h b/include/uapi/linux/pktcdvd.h index 2640b9d4e243..e2a49bfee74e 100644 --- a/include/uapi/linux/pktcdvd.h +++ b/include/uapi/linux/pktcdvd.h @@ -33,11 +33,13 @@ * able to successfully recover with this option (drive will return good * status as soon as the cdb is validated). */ +#ifdef __KERNEL__ #if defined(CONFIG_CDROM_PKTCDVD_WCACHE) #define USE_WCACHING 1 #else #define USE_WCACHING 0 #endif +#endif /* __KERNEL__ */ /* * No user-servicable parts beyond this point -> -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 12/13] Hide AT_VECTOR_SIZE_ARCH behind #ifdef __KERNEL__
I think this actually isn't a good idea, but I can't find anything outside of the kernel that's using this so I'm going to hide it. Signed-off-by: Palmer Dabbelt Reviewed-by: Andrew Waterman Reviewed-by: Albert Ou --- arch/x86/include/uapi/asm/auxvec.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/include/uapi/asm/auxvec.h b/arch/x86/include/uapi/asm/auxvec.h index 77203ac352de..88e5fa6d97e7 100644 --- a/arch/x86/include/uapi/asm/auxvec.h +++ b/arch/x86/include/uapi/asm/auxvec.h @@ -9,11 +9,13 @@ #endif #define AT_SYSINFO_EHDR33 +#ifdef __KERNEL__ /* entries in ARCH_DLINFO: */ #if defined(CONFIG_IA32_EMULATION) || !defined(CONFIG_X86_64) # define AT_VECTOR_SIZE_ARCH 2 #else /* else it's non-compat x86-64 */ # define AT_VECTOR_SIZE_ARCH 1 #endif +#endif /* __KERNEL__ */ #endif /* _ASM_X86_AUXVEC_H */ -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 11/13] Hide MAX_RAW_MINORS behind #ifdef __KERNEL__
I don't think this was ever meant to be exposed to userspace. Signed-off-by: Palmer Dabbelt Reviewed-by: Andrew Waterman Reviewed-by: Albert Ou --- include/uapi/linux/raw.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/uapi/linux/raw.h b/include/uapi/linux/raw.h index 62d543e70603..6d8d87524ff2 100644 --- a/include/uapi/linux/raw.h +++ b/include/uapi/linux/raw.h @@ -13,6 +13,8 @@ struct raw_config_request __u64 block_minor; }; +#ifdef __KERNEL__ #define MAX_RAW_MINORS CONFIG_MAX_RAW_DEVS +#endif #endif /* __LINUX_RAW_H */ -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 06/13] Hide ep_take_care_of_epollwakeup() behind #ifdef __KERNEL__
This doesn't make any sense to expose to userspace. Signed-off-by: Palmer Dabbelt Reviewed-by: Andrew Waterman Reviewed-by: Albert Ou --- include/uapi/linux/eventpoll.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/uapi/linux/eventpoll.h b/include/uapi/linux/eventpoll.h index bc81fb2e1f0e..290426bfb0aa 100644 --- a/include/uapi/linux/eventpoll.h +++ b/include/uapi/linux/eventpoll.h @@ -61,6 +61,7 @@ struct epoll_event { __u64 data; } EPOLL_PACKED; +#ifdef __KERNEL__ #ifdef CONFIG_PM_SLEEP static inline void ep_take_care_of_epollwakeup(struct epoll_event *epev) { @@ -73,4 +74,6 @@ static inline void ep_take_care_of_epollwakeup(struct epoll_event *epev) epev->events &= ~EPOLLWAKEUP; } #endif +#endif /*__KERNEL__*/ + #endif /* _UAPI_LINUX_EVENTPOLL_H */ -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 04/13] Always expose MAP_UNINITIALIZED to userspace
This used to be hidden behind CONFIG_MMAP_ALLOW_UNINITIALIZED, so userspace wouldn't actually ever see it. While I could have kept hiding it, the man pages seem to indicate that MAP_UNINITIALIZED should be visible: mmap(2) MAP_UNINITIALIZED (since Linux 2.6.33) Don't clear anonymous pages. This flag is intended to improve performance on embedded devices. This flag is honored only if the kernel was configured with the CONFIG_MMAP_ALLOW_UNINITIALIZED option. Because of the security implications, that option is normally enabled only on embedded devices (i.e., devices where one has complete control of the contents of user memory). and since the only time it shows up in my /usr/include is in this header I believe this should have been visible to userspace (as non-zero, which wouldn't do anything when or'd into the flags) all along. Signed-off-by: Palmer Dabbelt Reviewed-by: Andrew Waterman Reviewed-by: Albert Ou --- include/uapi/asm-generic/mman-common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h index ddc3b36f1046..e58d1911ecc6 100644 --- a/include/uapi/asm-generic/mman-common.h +++ b/include/uapi/asm-generic/mman-common.h @@ -19,7 +19,7 @@ #define MAP_TYPE 0x0f/* Mask for type of mapping */ #define MAP_FIXED 0x10/* Interpret addr exactly */ #define MAP_ANONYMOUS 0x20/* don't use a file */ -#ifdef CONFIG_MMAP_ALLOW_UNINITIALIZED +#if !defined(__KERNEL__) || defined(CONFIG_MMAP_ALLOW_UNINITIALIZED) # define MAP_UNINITIALIZED 0x400 /* For anonymous mmap, memory could be uninitialized */ #else # define MAP_UNINITIALIZED 0x0 /* Don't support this flag */ -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 05/13] Hide some of "struct elf_prstatus" behind #ifdef __KERNEL__
This one scares me: while I can't find any system calls that directly take this as an argument, a comment in " Generic ptrace interface that exports the architecture specific regsets using the corresponding NT_* types (which are also used in the core dump). Please note that the NT_PRSTATUS note type in a core dump contains a full 'struct elf_prstatus'. But the user_regset for NT_PRSTATUS contains just the elf_gregset_t that is the pr_reg field of 'struct elf_prstatus'. For all the other user_regset flavors, the user_regset layout and the ELF core dump note payload are exactly the same layout. " seems to indicate that it's possible to see this sometimes. Since this would only be visible to userspace in a somewhat convoluted manner, I'm going to try and keep it as it was. Signed-off-by: Palmer Dabbelt Reviewed-by: Andrew Waterman Reviewed-by: Albert Ou --- include/uapi/linux/elfcore.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/uapi/linux/elfcore.h b/include/uapi/linux/elfcore.h index 569737cfb557..f9320b588937 100644 --- a/include/uapi/linux/elfcore.h +++ b/include/uapi/linux/elfcore.h @@ -60,7 +60,7 @@ struct elf_prstatus longpr_instr; /* Current instruction */ #endif elf_gregset_t pr_reg; /* GP registers */ -#ifdef CONFIG_BINFMT_ELF_FDPIC +#if defined(__KERNEL__) && defined(CONFIG_BINFMT_ELF_FDPIC) /* When using FDPIC, the loadmap addresses need to be communicated * to GDB in order for GDB to do the necessary relocations. The * fields (below) used to communicate this information are placed -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 03/13] Hide COMPAT_ATM_ADDPARTY behind #ifdef __KERNEL__
This used to just be behind an #ifdef COMPAT_COMPAT, so most of userspace wouldn't have seen the definition before. This change just makes the __KERNEL__ part explicit to quiet the header checker. Signed-off-by: Palmer Dabbelt Reviewed-by: Andrew Waterman Reviewed-by: Albert Ou --- include/uapi/linux/atmdev.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/uapi/linux/atmdev.h b/include/uapi/linux/atmdev.h index 93e0ec008ca8..f8b6223165da 100644 --- a/include/uapi/linux/atmdev.h +++ b/include/uapi/linux/atmdev.h @@ -100,7 +100,7 @@ struct atm_dev_stats { /* use backend to make new if */ #define ATM_ADDPARTY _IOW('a', ATMIOC_SPECIAL+4,struct atm_iobuf) /* add party to p2mp call */ -#ifdef CONFIG_COMPAT +#if defined(__KERNEL__) && defined(CONFIG_COMPAT) /* It actually takes struct sockaddr_atmsvc, not struct atm_iobuf */ #define COMPAT_ATM_ADDPARTY_IOW('a', ATMIOC_SPECIAL+4,struct compat_atm_iobuf) #endif -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 02/13] Always expose __SYSCALL(... fork ...)
I think this change actually doesn't do anything: __NR_fork was still being defined either way, and on my machine fork() in comes from libc. That said, I don't think there's any way to determine this automatically, so this at least quiets the checker. Signed-off-by: Palmer Dabbelt Reviewed-by: Andrew Waterman Reviewed-by: Albert Ou --- include/uapi/asm-generic/unistd.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h index e016bd9b1a04..e027ef7aa01f 100644 --- a/include/uapi/asm-generic/unistd.h +++ b/include/uapi/asm-generic/unistd.h @@ -865,11 +865,11 @@ __SYSCALL(__NR_uselib, sys_uselib) __SYSCALL(__NR__sysctl, sys_sysctl) #define __NR_fork 1079 -#ifdef CONFIG_MMU +#if !defined(__KERNEL__) || defined(CONFIG_MMU) __SYSCALL(__NR_fork, sys_fork) #else __SYSCALL(__NR_fork, sys_ni_syscall) -#endif /* CONFIG_MMU */ +#endif /* !__KERNEL__ || CONFIG_MMU */ #undef __NR_syscalls #define __NR_syscalls (__NR_fork+1) -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Remove #ifdef CONFIG_64BIT from all asm-generic/fcntl.h
From: Palmer Dabbelt When working on the RISC-V port I noticed that F_SETLK64 was being defined on our 64-bit platform, despite our port being so new that we've only ever had the 64-bit file ops. Since there's not compat layer for these, this causes fcntl to bail out. It turns out that one of the ways in with F_SETLK64 was being defined (there's some more in glibc, but that's a whole different story... :)) is the result of CONFIG_64BIT showing up in this user-visible header. confirms this isn't sane, so I replaced it with a __BITS_PER_LONG check. I went ahead and grep'd for any more of these (with headers_install_all), and this was the only one I found. Signed-off-by: Palmer Dabbelt Reviewed-by: Andrew Waterman Reviewed-by: Albert Ou --- include/uapi/asm-generic/fcntl.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h index e063effe0cc1..14a5c8237d84 100644 --- a/include/uapi/asm-generic/fcntl.h +++ b/include/uapi/asm-generic/fcntl.h @@ -1,6 +1,7 @@ #ifndef _ASM_GENERIC_FCNTL_H #define _ASM_GENERIC_FCNTL_H +#include #include /* @@ -115,7 +116,7 @@ #define F_GETSIG 11 /* for sockets. */ #endif -#ifndef CONFIG_64BIT +#if (__BITS_PER_LONG == 32) #ifndef F_GETLK64 #define F_GETLK64 12 /* using 'struct flock64' */ #define F_SETLK64 13 -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: simplified RISC-V interrupt and clocksource handling v2
On Thu, 02 Aug 2018 04:49:57 PDT (-0700), Christoph Hellwig wrote: This series tries adds support for interrupt handling and timers for the RISC-V architecture. The basic per-hart interrupt handling implemented by the scause and sie CSRs is extremely simple and implemented directly in arch/riscv/kernel/irq.c. In addition there is a irqchip driver for the PLIC external interrupt controller, which is called through the set_handle_irq API, and a clocksource driver that gets its timer interrupt directly from the low-level interrupt handling. Compared to previous iterations this version does not try to use an irqchip driver for the low-level interrupt handling. This saves a couple indirect calls and an additional read of the scause CSR in the hot path, makes the code much simpler and last but not least avoid the dependency on a device tree for a mandatory architectural feature. A git tree is available here (contains a few more patches before the ones in this series) git://git.infradead.org/users/hch/riscv.git riscv-irq-simple.2 Gitweb: http://git.infradead.org/users/hch/riscv.git/shortlog/refs/heads/riscv-irq-simple.2 Changes since v1: - rename the plic driver to irq-sifive-plic - switch to a default compatible of sifive,plic0 (still supporting the riscv,plic0 name for compatibility) - add a reference for the SiFive PLIC register layout - fix plic_toggle addressing for large numbers of hwirqs - remove the call to ack_bad_irq - use a raw spinlock for plic_toggle_lock - use the irq_desc cpumask in the plic enable/disable methods - add back OF contexid parsing in the plic driver - don't allow COMPILE_TEST builds of the clocksource driver, as it depends on - default the clocksource driver to y - clean up naming in the clocksource driver - remove the MINDELTA and MAXDELTA #defines - various DT binding fixes Ah, thank you so much. This is great! With this patch set applied on top of rc7 I can boot QEMU master and get to the Fedora root file system. I'll review the patch set properly, but at least for now I think a Tested-by: Palmer Dabbelt is warranted. What's the best way to go about merging this? There's quite a bit of arch/riscv diff here so I don't mind taking it through the RISC-V tree, but there's also some irqchip and clocksource stuff as well so I'm not sure if that's OK to do.
Re: [PATCH 3/3] irqchip: RISC-V Local Interrupt Controller Driver
On Fri, 22 Jun 2018 17:08:58 PDT (-0700), rdun...@infradead.org wrote: On 06/22/2018 04:20 PM, Palmer Dabbelt wrote: From: Palmer Dabbelt This patch adds a driver that manages the local interrupts on each RISC-V hart, as specifiec by the RISC-V supervisor level ISA manual. The local interrupt controller manages software interrupts, timer interrupts, and hardware interrupts (which are routed via the platform level interrupt controller). Per-hart local interrupt controllers are found on all RISC-V systems. CC: Michael Clark Signed-off-by: Palmer Dabbelt --- drivers/irqchip/Kconfig | 13 +++ drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-riscv-intc.c | 215 +++ 3 files changed, 229 insertions(+) create mode 100644 drivers/irqchip/irq-riscv-intc.c diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index e9233db16e03..bf7fc86673b1 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -372,3 +372,16 @@ config QCOM_PDC IRQs for Qualcomm Technologies Inc (QTI) mobile chips. endmenu + +config RISCV_INTC + #bool "RISC-V Interrupt Controller" Hi, What does the leading '#' do? It's just a comment. I'd seen this idiom used before to say "here's what this option would say if it was optional, which it may be at some point in the future, but for now it's just always enabled." + depends on RISCV + default y + help + This enables support for the local interrupt controller found in + standard RISC-V systems. The local interrupt controller handles + timer interrupts, software interrupts, and hardware interrupts. + Without a local interrupt controller the system will be unable to + handle any interrupts, including those passed via the PLIC. + + If you don't know what to do here, say Y. diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index 15f268f646bf..74e333cc274c 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -87,3 +87,4 @@ obj-$(CONFIG_MESON_IRQ_GPIO) += irq-meson-gpio.o obj-$(CONFIG_GOLDFISH_PIC) += irq-goldfish-pic.o obj-$(CONFIG_NDS32)+= irq-ativic32.o obj-$(CONFIG_QCOM_PDC) += qcom-pdc.o +obj-$(CONFIG_RISCV_INTC) += irq-riscv-intc.o
Re: Finish the GENERIC_IRQ_MULTI_HANDLER conversion
On Sun, 24 Jun 2018 06:15:25 PDT (-0700), t...@linutronix.de wrote: On Thu, 21 Jun 2018, Palmer Dabbelt wrote: A while ago I sent a patch set that adds a GENERIC_IRQ_MULTI_HANDLER, which is an exact copy of the existing IRQ_MULTI_HANDLER support in the arm port, which is being used unconditionally by arm64 and openrisc. GENERIC_IRQ_MULTI_HANDLER is currently being used by the RISC-V port. I managed to make a few mistakes in my original patch set and as a result my conversion of the other architectures of GENERIC_IRQ_MULTI_HANDLER was dropped. This patch set finishes up my original patch set by converting arm, arm64, and openrisc over to the new GENERIC_IRQ_MULTI_HANDLER support and then removing MULTI_IRQ_HANDLER as it's obselete. At the time I wrote this I gave it fairly extensive build testing, but went I recently rebased it I just tested the full patch set on arm, arm64, and openrisc defconfigs. Various flavors of this patch set have bounced around a few times before, but I'm calling this a whole new patch set as it builds on top of what was merged. I'll take the whole pile through tip irq/core which probably makes the most sense unless there are any objections from architecture maintainers. Thanks! Unless I've managed to screw something up I don't see these in your tree, but I also don't see any feedback from anyone else.
Re: [PATCH 2/3] dt-bindings: interrupt-controller: RISC-V local interrupt controller docs
On Mon, 25 Jun 2018 13:04:48 PDT (-0700), christ...@boehmwalder.at wrote: On Fri, Jun 22, 2018 at 04:20:05PM -0700, Palmer Dabbelt wrote: From: Palmer Dabbelt This patch adds documentation on the RISC-V local interrupt controller, which is a per-hart interrupt controller that manages all interrupts entering a RISC-V hart. This interrupt controller is present on all RISC-V systems. Signed-off-by: Palmer Dabbelt --- .../interrupt-controller/riscv,cpu-intc.txt| 41 ++ 1 file changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt new file mode 100644 index ..61900e2e3868 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt @@ -0,0 +1,41 @@ +RISC-V Hart-Level Interrupt Controller (HLIC) +- + +RISC-V cores include Control Status Registers (CSRs) which are local to each +hart and can be read or written by software. Some of these CSRs are used to +control local interrupts connected to the core. Every interrupt is ultimately +routed through a hart's HLIC before it interrupts that hart. + +The RISC-V supervisor ISA manual specifies three interrupt sources that are +attached to every HLIC: software interrupts, the timer interrupt, and external +interrupts. Software interrupts are used to send IPIs between cores. The +timer interrupt comes from an architecturally mandated real-time timer that is +controller via SBI calls and CSR reads. External interrupts connect all other +device interrupts to the HLIC, which are routed via the platform-level +interrupt controller (PLIC). + +All RISC-V systems that conform to the supervisor ISA specification are +required to have a HLIC with these three interrupt sources present. Since the +interrupt map is defined by the ISA it's not listed in the HLIC's device tree +entry, though external interrupt controllers (like the PLIC, for example) will +need to define how their interrupts map to the relevant HLICs. + +Required properties: +- compatible : "riscv,cpu-intc" +- #interrupt-cells : should be <1> +- interrupt-controller : Identifies the node as an interrupt controller + +Furthermore, this interrupt-controller MUST be embedded inside the cpu +definition of the hart whose CSRs control these local interrupts. + +An example device tree entry for a HLIC is show below. Spotted a typo here, "show" -> "shown". Thanks. It looks like we're actually dropping this binding and integrating this first-level interrupt controller into the core RISC-V arch code as keeping it split out results in too many inefficiencies. + + cpu1: cpu@1 { + compatible = "riscv"; + ... + cpu1-intc: interrupt-controller { + #interrupt-cells = <1>; + compatible = "riscv,cpu-intc"; + interrupt-controller; + }; + }; -- 2.16.4 Also, I've noticed that double spaces after punctuation are used pretty inconsistently throughout the document. Is that intended? No. I noticed this in the PLIC document as well, I'll fix that one up. Thanks!
Re: [PATCH] riscv: remove unnecessary of_platform_populate call
On Mon, 09 Jul 2018 08:50:05 PDT (-0700), r...@kernel.org wrote: On Tue, Jun 19, 2018 at 3:41 PM Rob Herring wrote: The DT core will call of_platform_default_populate, so it is not necessary for arch specific code to call it unless there are custom match entries, auxdata or parent device. Neither of those apply here, so remove the call. Cc: Palmer Dabbelt Cc: Albert Ou Cc: linux-ri...@lists.infradead.org Signed-off-by: Rob Herring --- arch/riscv/kernel/setup.c | 5 - 1 file changed, 5 deletions(-) Ping? Sorry for not saying anything, I don't have a good scheme for automatically tracking patches as they flow through my tree and my inbox is a bit backed up. This landed as part of b19b9282093588e73401f9d4981310a8de975f7d. Thanks for the fix! diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index ee44a48faf79..f0d2070866d4 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -220,8 +220,3 @@ void __init setup_arch(char **cmdline_p) riscv_fill_hwcap(); } -static int __init riscv_device_init(void) -{ - return of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); -} -subsys_initcall_sync(riscv_device_init);
[PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
This file is expected to be included multiple times in the same file in order to allow the __SYSCALL macro to generate system call tables. With a global include guard we end up missing __NR_riscv_flush_icache in the syscall table, which results in icache flushes that escape the vDSO call to not actually do anything. The fix is to move to per-#define include guards, which allows the system call tables to actually be populated. Thanks to Macrus Comstedt for finding and fixing the bug! I also went ahead and fixed the SPDX header to use a //-style comment, which I've been told is the canonical way to do it. Cc: Marcus Comstedt Signed-off-by: Palmer Dabbelt --- arch/riscv/include/uapi/asm/syscalls.h | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/arch/riscv/include/uapi/asm/syscalls.h b/arch/riscv/include/uapi/asm/syscalls.h index 818655b0d535..882a6aa09a33 100644 --- a/arch/riscv/include/uapi/asm/syscalls.h +++ b/arch/riscv/include/uapi/asm/syscalls.h @@ -1,10 +1,11 @@ -/* SPDX-License-Identifier: GPL-2.0 */ +// SPDX-License-Identifier: GPL-2.0 /* - * Copyright (C) 2017 SiFive + * Copyright (C) 2017-2018 SiFive */ -#ifndef _ASM__UAPI__SYSCALLS_H -#define _ASM__UAPI__SYSCALLS_H +/* There is explicitly no include guard here because this file is expected to + * be included multiple times in order to define the syscall macros via + * __SYSCALL. */ /* * Allows the instruction cache to be flushed from userspace. Despite RISC-V @@ -20,7 +21,7 @@ * caller. We don't currently do anything with the address range, that's just * in there for forwards compatibility. */ +#ifndef __NR_riscv_flush_icache #define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15) -__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) - #endif +__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) -- 2.16.4
Re: [PATCH 6/6] dt-bindings: interrupt-controller: RISC-V PLIC documentation
On Wed, 01 Aug 2018 11:26:31 PDT (-0700), r...@kernel.org wrote: On Wed, Aug 1, 2018 at 1:12 AM Christoph Hellwig wrote: On Tue, Jul 31, 2018 at 04:46:30PM -0600, Rob Herring wrote: > Perhaps this should be 'sifive,plic0' Excepet for the fact this the old name has already been in shipping hardware and release of qemu and other emulators it should. Not really my problem that they didn't follow the process and upstream their binding first. But this alone is just a string identifier, so I don't really care that much. If things are really a mess, then the next implementations will have to have better compatible strings. More likely, I'll just see folks trying to add various properties to deal with all the differences. You could always define a better compatible and leave 'riscv,plic0' as a fallback to avoid breaking things. Ya, sorry about that. FWIW, we don't consider any of the bindings stable until they're actually accepted upstream, so it's on us to fix our bootloaders to match what actually lands upstream. Luckily there's not that much hardware out there and none of it is in production, so I'm OK forcing people to upgrade bootloaders to make this all work. I think it's probably best to leave the extra compat string out of the kernel proper, as then it'll never be enshrined as a RISC-V standard. > Normally this would have an SoC specific compatible too. Sometimes we > can get away without, but it doesn't seem like the PLIC is very tightly > specified nor has common implementations. It is a giant f***cking mess to be honest. Adding a highlevel spec to the ISA but not a register layout is completely idotic, but if you look at the current riscv-sw list this decision is still defended by SiFive / the RISC-V foundation. The whole stale of the RISC-V platform Ecosystem is rather pathetic unfortunately, and people don't seem to be willing to learn from past good practice nor mistakes in ARM land. Interrupt controllers are where the differentiation is. ;) Again, sorry about that :). The RISC-V platform specification really should have started a year ago, but I'm afraid there's just a bit too much going on on my end. If it helps any, we just submitted a plumbers dev room with one topic being the RISC-V platform spec, so I guess I'm in official trouble now it there isn't at least something to talk about by November...
[PATCH] spi-nor: add support for is25wp256d
From: "Wesley W. Terpstra" This is used of the HiFive Unleashed development board. Signed-off-by: Wesley W. Terpstra Signed-off-by: Palmer Dabbelt --- drivers/mtd/spi-nor/spi-nor.c | 47 ++- include/linux/mtd/spi-nor.h | 2 ++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index d9c368c44194..e9a3557a3c23 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -1072,6 +1072,9 @@ static const struct flash_info spi_nor_ids[] = { SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, { "is25wp128", INFO(0x9d7018, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, + { "is25wp256d", INFO(0x9d7019, 0, 32 * 1024, 1024, + SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) + }, /* Macronix */ { "mx25l512e", INFO(0xc22010, 0, 64 * 1024, 1, SECT_4K) }, @@ -1515,6 +1518,45 @@ static int macronix_quad_enable(struct spi_nor *nor) return 0; } +/** + * issi_unlock() - clear BP[0123] write-protection. + * @nor: pointer to a 'struct spi_nor' + * + * Bits [2345] of the Status Register are BP[0123]. + * ISSI chips use a different block protection scheme than other chips. + * Just disable the write-protect unilaterally. + * + * Return: 0 on success, -errno otherwise. + */ +static int issi_unlock(struct spi_nor *nor) +{ + int ret, val; + u8 mask = SR_BP0 | SR_BP1 | SR_BP2 | SR_BP3; + + val = read_sr(nor); + if (val < 0) + return val; + if (!(val & mask)) + return 0; + + write_enable(nor); + + write_sr(nor, val & ~mask); + + ret = spi_nor_wait_till_ready(nor); + if (ret) + return ret; + + ret = read_sr(nor); + if (ret > 0 && !(ret & mask)) { + dev_info(nor->dev, "ISSI Block Protection Bits cleared\n"); + return 0; + } else { + dev_err(nor->dev, "ISSI Block Protection Bits not cleared\n"); + return -EINVAL; + } +} + /* * Write status Register and configuration register with 2 bytes * The first byte will be written to the status register, while the @@ -2747,6 +2789,9 @@ static int spi_nor_init(struct spi_nor *nor) spi_nor_wait_till_ready(nor); } + if (JEDEC_MFR(nor->info) == SNOR_MFR_ISSI) + issi_unlock(nor); + if (nor->quad_enable) { err = nor->quad_enable(nor); if (err) { @@ -2926,7 +2971,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, if (ret) return ret; - if (nor->addr_width) { + if (nor->addr_width && JEDEC_MFR(info) != SNOR_MFR_ISSI) { /* already configured from SFDP */ } else if (info->addr_width) { nor->addr_width = info->addr_width; diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index e60da0d34cc1..da422a37d383 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -23,6 +23,7 @@ #define SNOR_MFR_ATMEL CFI_MFR_ATMEL #define SNOR_MFR_GIGADEVICE0xc8 #define SNOR_MFR_INTEL CFI_MFR_INTEL +#define SNOR_MFR_ISSI 0x9d #define SNOR_MFR_MICRONCFI_MFR_ST /* ST Micro <--> Micron */ #define SNOR_MFR_MACRONIX CFI_MFR_MACRONIX #define SNOR_MFR_SPANSION CFI_MFR_AMD @@ -121,6 +122,7 @@ #define SR_BP0 BIT(2) /* Block protect 0 */ #define SR_BP1 BIT(3) /* Block protect 1 */ #define SR_BP2 BIT(4) /* Block protect 2 */ +#define SR_BP3 BIT(5) /* Block protect 3 (on ISSI chips) */ #define SR_TB BIT(5) /* Top/Bottom protect */ #define SR_SRWDBIT(7) /* SR write protect */ /* Spansion/Cypress specific status bits */ -- 2.16.4
Re: [PATCH 3/6] irqchip: RISC-V Local Interrupt Controller Driver
On Wed, 01 Aug 2018 11:55:06 PDT (-0700), t...@linutronix.de wrote: On Wed, 25 Jul 2018, Christoph Hellwig wrote: On Wed, Jul 25, 2018 at 12:18:39PM +0100, Marc Zyngier wrote: > This feels odd. It means that you cannot have the following sequence: > >local_irq_disable(); >enable_irq(x); // where x is owned by a remote hart > > as smp_call_function_single() requires interrupts to be enabled. > > More fundamentally, why are you trying to make these interrupts look > global while they aren't? arm/arm64 have similar restrictions with GICv2 > and earlier, and treats these interrupts as per-cpu. > > Given that the drivers that deal with drivers connected to the per-hart > irqchip are themselves likely to be aware of the per-cpu aspect, it > would make sense to align things (we've been through that same > discussion about the clocksource driver a few weeks back). Right now the only direct consumers are said clocksource, the PLIC driver later in this series and the RISC-V arch IPI code. None of them is going to do a manual enable_irq, so I guess the remote case of the code is simply dead code. I'll take a look at converting them to per-cpu. I guess the GICv2 driver is the best template? Confused. The timer and the IPI are separate causes and have nothing to do with the per cpu irq domain. That's what the low level interrupt handling code tells me. If I understand correctly then the per cpu irq domain is for device interrupts, right? If so, then this simply cannot work and there is no way to make it work with per cpu interrupts either. Is there some high level documentation about the design (or the lack of) or can someone give a concise explanation how this stuff is supposed to work? The ISA spec defines this, and I'm not sure I can be a whole lot more concise than what's there but I'll try: This driver is for the ISA-mandated interrupts present on all RISC-V systems. These interrupts are tracked on a per-hart (hart is a RISC-V term that means hardware thread, it a hyperthread in Intel land) basis. On RISC-V systems there is a set of CSRs (control and status registers) local to each hart. These registers can be accessed by special instructions, and are used to perform hart-local actions that are of a medium speed: things like accessing the floating point exception state or changing the page table base pointer. When a hart is in supervisor mode, it has access to a handful of CSRs that are related to interrupts: * stvec: The trap vector, which is the entry point for both interrupts and exceptions. * sie: Supervisor Interrupts Enabled. This is a bit mask of enabled interrupts. * sip: Supervisor Interrupts Pending. This is a bit mask of pending interrupts, which can be polled when interrupts are disabled (or I guess when they're enabled too, but that's not particularly useful). * scause: Supervisor Trap Cause. This allows code to determine what sort of trap was taken. * sepc: Supervisor Exception PC (actually the PC for all traps). This allows code to determine how to get back to where it needs to continue after handling a trap. * stval: Supervisor Trap VALue. This provides additional trap-specific information: for example, if a trap says "you attempted to write to an address that isn't mapped or is read only" then stval contains the address that the write was directed at. As such, the ISA itself doesn't really differentiate between exceptions and interrupts: they both cause a privilege transition to happen, redirect execution to stvec, and write a handful of CSRs as side effects. It's up to software to determine the difference. In order to make it fast for software to determine if a scause value is an exception or an interrupt that is encoded in the high bit. There are 11 defined exceptions, which include system calls, breakpoints, page faults, and access faults -- that's all handled in core RISC-V arch code. Additionally, there are 6 defined interrupt types: * User software interrupt * User timer interrupt * User external interrupt * Supervisor software interrupt * Supervisor timer interrupt * Supervisor external interrupt The user interrupts are unsupported by Linux and thus never happen -- they're really designed for real-time embedded code, in POSIX land we use the standard signal delivery mechanisms as it'd be insane to put that in an ISA. Thus, functionally there are three interrupt types: * Supervisor software interrupt: These are never triggered by a hardware device, and are therefor left for software use. On Linux we use these to indicate that a message may have arrived in the per-hart message queue, which is used to implement IPIs. * Supervisor timer interrupt: When the RISC-V ISA was originally defined we thought that it was important to mandate that accurate real-time facilities exist, so there's a handful of time-related bits encoded into the ISA. In retrospect it seems like this didn't end up being so cl
Re: [PATCH v2] riscv: Add support to no-FPU systems
On Wed, 01 Aug 2018 11:23:43 PDT (-0700), Andrew Waterman wrote: On Wed, Aug 1, 2018 at 10:55 AM, Palmer Dabbelt wrote: On Tue, 26 Jun 2018 21:22:26 PDT (-0700), alan...@andestech.com wrote: This patch adds an option, CONFIG_FPU, to enable/disable floating procedures. Also, some style issues are fixed. Signed-off-by: Alan Kao Cc: Greentime Hu Cc: Zong Li --- arch/riscv/Kconfig | 9 arch/riscv/Makefile| 19 +++ arch/riscv/include/asm/switch_to.h | 6 +++ arch/riscv/kernel/entry.S | 3 +- arch/riscv/kernel/process.c| 7 ++- arch/riscv/kernel/signal.c | 82 +- 6 files changed, 90 insertions(+), 36 deletions(-) diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 6debcc4afc72..6069597ba73f 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -232,6 +232,15 @@ config RISCV_BASE_PMU endmenu +config FPU + bool "FPU support" + default y + help + Say N here if you want to disable all floating-point related procedure + in the kernel. + + If you don't know what to do here, say Y. + endmenu Sorry for letting this slide for a bit. While I'm not opposed to a solution that requires a FPU Kconfig option, it'd be a bit better if we could detect this at boot time. I think this should be possible because at one point this actually worked and we could boot the same kernel on FPU and no-FPU systems. I believe it would suffice to have start_thread set sstatus.FS to OFF for no-FPU systems (vs. INITIAL for systems with FPU). The ISA string in the devicetree should indicate whether F/D extensions are present. That said, it makes sense to me to additionally provide the Kconfig option. This would elide the sstatus.SD check for no-FPU systems, shaving a couple instructions off the context-switch path. It would also enable mimicking the behavior of a no-FPU system even when the FPU is present. That sounds like a good argument. Do you mind submitting a two-part patch set, to: * Allow FPU kernels to detect and run correctly on non-FPU systems. You should be able to detect these very early by writing to sstatus, or later by looking at the device tree. * Add a Kconfig option to disable the FPU entirely (which is pretty much this patch). Thanks! If that's not possible then we'll have to take something like this. There were some comments on this v2 but I don't see a v3, did I miss one?
Re: [PATCH] RISC-V: Add the directive for alignment of stvec's value
On Wed, 01 Aug 2018 18:26:48 PDT (-0700), zong...@gmail.com wrote: Palmer Dabbelt 於 2018年8月2日 週四 上午8:38寫道: On Wed, 20 Jun 2018 18:40:07 PDT (-0700), z...@andestech.com wrote: > The stvec's value must be 4 byte alignment by specification definition. > This directive avoids to stvec be set the non-alignment value by the > following code in head.S > > /* Point stvec to virtual address of intruction after satp write */ > la a0, 1f > add a0, a0, a1 > csrw stvec, a0 > > Signed-off-by: Zong Li > --- > arch/riscv/kernel/head.S | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S > index 396ec7b349ce..ae7b204f531c 100644 > --- a/arch/riscv/kernel/head.S > +++ b/arch/riscv/kernel/head.S > @@ -94,6 +94,7 @@ relocate: > or a0, a0, a1 > sfence.vma > csrw sptbr, a0 > +.align 2 > 1: > /* Set trap vector to spin forever to help debug */ > la a0, .Lsecondary_park I don't think this is actually correct: you shouldn't be aligning the address at which stvec is set, but the address that stvec is set to. If this is fixing anything then it's probably just by coincidence as it's causing .Lsecondary_park to be aligned. I think this patch is the correct fix diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S index 6e07ed37bbff..d1beecf1d060 100644 --- a/arch/riscv/kernel/head.S +++ b/arch/riscv/kernel/head.S @@ -143,6 +143,7 @@ relocate: tail smp_callin #endif +.align 2 .Lsecondary_park: /* We lack SMP support or have too many harts, so park this hart */ wfi Thanks for pointing this out! The position which I set in this patch is also be set to stvec for jumping to correct VA at satp enabling. The label .Lsecondary_park is also be set to stvec, I don't notice it because as you said, this label address is correct just by coincidence. I think there are two places need to be aligned. The first setting as following: relocate: ... /* Point stvec to virtual address of intruction after satp write */ la a0, 1f add a0, a0, a1 csrw stvec, a0<--- first set ... sfence.vma csrw sptbr, a0 1: /* Set trap vector to spin forever to help debug */ la a0, .Lsecondary_park csrw stvec, a0 I agree. I think this should do it: commit a8c5f596bcd528c5468d621bc84cd901632a912a gpg: Signature made Fri 03 Aug 2018 09:13:56 PM PDT gpg:using RSA key 00CE76D1834960DFCE886DF8EF4CA1502CCBAB41 gpg:issuer "pal...@dabbelt.com" gpg: Good signature from "Palmer Dabbelt " [ultimate] gpg: aka "Palmer Dabbelt " [ultimate] Author: Palmer Dabbelt Date: Wed Aug 1 17:38:30 2018 -0700 RISC-V: Align early stvec targets The ISA mandantes that stvec is 4-byte aligned, but we weren't actually respecting in the early boot code. These trap vector are never expected to actually be used unless there's a bug in early in the boot process of the kernel, which explains why the bug wasn't noticed until recently. Thanks to Zong for finding the bug! CC: z...@andestech.com Signed-off-by: Palmer Dabbelt diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S index 6e07ed37bbff..c4d2c63f9a29 100644 --- a/arch/riscv/kernel/head.S +++ b/arch/riscv/kernel/head.S @@ -94,6 +94,7 @@ relocate: or a0, a0, a1 sfence.vma csrw sptbr, a0 +.align 2 1: /* Set trap vector to spin forever to help debug */ la a0, .Lsecondary_park @@ -143,6 +144,7 @@ relocate: tail smp_callin #endif +.align 2 .Lsecondary_park: /* We lack SMP support or have too many harts, so park this hart */ wfi
Re: [PATCH 09/11] RISC-V: Support per-hart timebase-frequency
On Fri, 03 Aug 2018 05:33:57 PDT (-0700), Christoph Hellwig wrote: On Thu, Aug 02, 2018 at 03:19:49PM -0700, Atish Patra wrote: On 8/2/18 4:50 AM, Christoph Hellwig wrote: From: Palmer Dabbelt Follow the updated DT specs and read the timebase-frequency from the CPU 0 node. However, the DT in the HighFive Unleashed has the entry at the wrong place. Even the example in github also at wrong place. https://github.com/riscv/riscv-device-tree-doc/pull/8/commits/2461d481329c55005fcbe684f0d6bdb3b7f0a432 DT should be consistent between Documentation and the one in the hardware. I can fix them in bbl & submit a bbl patch. But I am not sure if that's an acceptable way to do it. I'll need to have comments from Palmer and/or someone else at SiFive here. Personally I really don't care where we document the timebase, as this patch supports both locations anywhere. For now I'll just update the commit log to state that more explicitly. You're welcome to submit a BBL patch to make this all match, but from my understanding of the device tree spec putting timebase-frequency in either place should be legal so it's not a critical fix. That said, it's better to have them match than not match.
Re: [PATCH] spi-nor: add support for is25wp256d
On Sat, 04 Aug 2018 02:27:54 PDT (-0700), marek.va...@gmail.com wrote: On 08/04/2018 03:49 AM, Palmer Dabbelt wrote: From: "Wesley W. Terpstra" This is used of the HiFive Unleashed development board. Signed-off-by: Wesley W. Terpstra Signed-off-by: Palmer Dabbelt --- drivers/mtd/spi-nor/spi-nor.c | 47 ++- include/linux/mtd/spi-nor.h | 2 ++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index d9c368c44194..e9a3557a3c23 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -1072,6 +1072,9 @@ static const struct flash_info spi_nor_ids[] = { SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, { "is25wp128", INFO(0x9d7018, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, + { "is25wp256d", INFO(0x9d7019, 0, 32 * 1024, 1024, Is there a reason for the trailing 'd' in is25wp256d ? I'd drop it. I'm honestly not sure. There are data sheets for both of them, but I don't see much of a difference http://www.issi.com/WW/pdf/IS25LP(WP)256D.pdf http://www.issi.com/WW/pdf/25LP-WP256.pdf Following the pattern, I'd expect to see { "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, versus { "is25wp256d", INFO(0x9d7019, 0, 32 * 1024, 1024, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) }, So in other words: the d less sections that are larger, and also has the 4B opcodes flag set. From the documentation in looks like the non-d version supports 3 and 4 byte opcodes, so I guess it's just a different physical layout? In the data sheet for both I see "Pages can be erased in groups of 4Kbyte sectors, 32Kbyte blocks, 64Kbyte blocks, and/or the entire chip" which indicates to me that maybe we've just selected the larger section size? If so then I'll change it to the first one in the new patch. + SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) + }, /* Macronix */ { "mx25l512e", INFO(0xc22010, 0, 64 * 1024, 1, SECT_4K) }, @@ -1515,6 +1518,45 @@ static int macronix_quad_enable(struct spi_nor *nor) return 0; } +/** + * issi_unlock() - clear BP[0123] write-protection. + * @nor: pointer to a 'struct spi_nor' + * + * Bits [2345] of the Status Register are BP[0123]. + * ISSI chips use a different block protection scheme than other chips. + * Just disable the write-protect unilaterally. + * + * Return: 0 on success, -errno otherwise. + */ +static int issi_unlock(struct spi_nor *nor) +{ + int ret, val; + u8 mask = SR_BP0 | SR_BP1 | SR_BP2 | SR_BP3; + + val = read_sr(nor); + if (val < 0) + return val; + if (!(val & mask)) + return 0; + + write_enable(nor); + + write_sr(nor, val & ~mask); + + ret = spi_nor_wait_till_ready(nor); + if (ret) + return ret; + + ret = read_sr(nor); + if (ret > 0 && !(ret & mask)) { + dev_info(nor->dev, "ISSI Block Protection Bits cleared\n"); + return 0; Is the dev_info() really needed ? Nope. I'll spin a v2 pending the above discussion. + } else { + dev_err(nor->dev, "ISSI Block Protection Bits not cleared\n"); + return -EINVAL; + } +} [...] Thanks!
Re: RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
On Sat, 04 Aug 2018 01:54:38 PDT (-0700), Christoph Hellwig wrote: >> index 818655b0d535..882a6aa09a33 100644 >> --- a/arch/riscv/include/uapi/asm/syscalls.h >> +++ b/arch/riscv/include/uapi/asm/syscalls.h >> @@ -1,10 +1,11 @@ >> -/* SPDX-License-Identifier: GPL-2.0 */ >> +// SPDX-License-Identifier: GPL-2.0 > > /* */ is the required style for headers, // is only for other files. > >> +/* There is explicitly no include guard here because this file is expected >> to >> + * be included multiple times in order to define the syscall macros via >> + * __SYSCALL. */ > > Normal Linux comment style would be: > > /* > * There is explicitly no include guard here because this file is expected to > * be included multiple times in order to define the syscall macros via > * __SYSCALL. > */ > > Also syscalls.h isn't included directly anywhere, but through > , so we'll probably need a similar comment there as well. I've attached a v2 with both of these fixed. Thanks!
[PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
This file is expected to be included multiple times in the same file in order to allow the __SYSCALL macro to generate system call tables. With a global include guard we end up missing __NR_riscv_flush_icache in the syscall table, which results in icache flushes that escape the vDSO call to not actually do anything. The fix is to move to per-#define include guards, which allows the system call tables to actually be populated. Thanks to Macrus Comstedt for finding and fixing the bug! I also went ahead and fixed the SPDX header to use a //-style comment, which I've been told is the canonical way to do it. Cc: Marcus Comstedt Signed-off-by: Palmer Dabbelt --- arch/riscv/include/asm/unistd.h| 5 + arch/riscv/include/uapi/asm/syscalls.h | 15 +-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h index 080fb28061de..508be1780323 100644 --- a/arch/riscv/include/asm/unistd.h +++ b/arch/riscv/include/asm/unistd.h @@ -11,6 +11,11 @@ * GNU General Public License for more details. */ +/* + * There is explicitly no include guard here because this file is expected to + * included multiple times. See uapi/asm/syscalls.h for more info. + */ + #define __ARCH_WANT_SYS_CLONE #include #include diff --git a/arch/riscv/include/uapi/asm/syscalls.h b/arch/riscv/include/uapi/asm/syscalls.h index 818655b0d535..690beb002d1d 100644 --- a/arch/riscv/include/uapi/asm/syscalls.h +++ b/arch/riscv/include/uapi/asm/syscalls.h @@ -1,10 +1,13 @@ -/* SPDX-License-Identifier: GPL-2.0 */ +// SPDX-License-Identifier: GPL-2.0 /* - * Copyright (C) 2017 SiFive + * Copyright (C) 2017-2018 SiFive */ -#ifndef _ASM__UAPI__SYSCALLS_H -#define _ASM__UAPI__SYSCALLS_H +/* + * There is explicitly no include guard here because this file is expected to + * be included multiple times in order to define the syscall macros via + * __SYSCALL. + */ /* * Allows the instruction cache to be flushed from userspace. Despite RISC-V @@ -20,7 +23,7 @@ * caller. We don't currently do anything with the address range, that's just * in there for forwards compatibility. */ +#ifndef __NR_riscv_flush_icache #define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15) -__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) - #endif +__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) -- 2.16.4
Re: linux-next: Signed-off-by missing for commit in the risc-v tree
On Sun, 05 Aug 2018 14:37:05 PDT (-0700), Stephen Rothwell wrote: Hi Palmer, Commit bce17edfe6af ("fixup: ". " in PLIC docs") is missing a Signed-off-by from its author and committer. Oh, sorry about that. It should be gone, it was just meant as a review comment.
Re: [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
On Mon, 06 Aug 2018 14:00:53 PDT (-0700), rdun...@infradead.org wrote: On 08/06/2018 01:42 PM, Palmer Dabbelt wrote: This file is expected to be included multiple times in the same file in order to allow the __SYSCALL macro to generate system call tables. With a global include guard we end up missing __NR_riscv_flush_icache in the syscall table, which results in icache flushes that escape the vDSO call to not actually do anything. The fix is to move to per-#define include guards, which allows the system call tables to actually be populated. Thanks to Macrus Comstedt for finding and fixing the bug! I also went ahead and fixed the SPDX header to use a //-style comment, which I've been told is the canonical way to do it. Cc: Marcus Comstedt Signed-off-by: Palmer Dabbelt --- arch/riscv/include/asm/unistd.h| 5 + arch/riscv/include/uapi/asm/syscalls.h | 15 +-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h index 080fb28061de..508be1780323 100644 --- a/arch/riscv/include/asm/unistd.h +++ b/arch/riscv/include/asm/unistd.h @@ -11,6 +11,11 @@ * GNU General Public License for more details. */ +/* + * There is explicitly no include guard here because this file is expected to to .. be .. included Thanks. I'm not going to spin a v3 unless there's more feedback, but I'll include it in the PR. + * included multiple times. See uapi/asm/syscalls.h for more info. + */ + #define __ARCH_WANT_SYS_CLONE #include #include diff --git a/arch/riscv/include/uapi/asm/syscalls.h b/arch/riscv/include/uapi/asm/syscalls.h index 818655b0d535..690beb002d1d 100644 --- a/arch/riscv/include/uapi/asm/syscalls.h +++ b/arch/riscv/include/uapi/asm/syscalls.h @@ -1,10 +1,13 @@ -/* SPDX-License-Identifier: GPL-2.0 */ +// SPDX-License-Identifier: GPL-2.0 /* - * Copyright (C) 2017 SiFive + * Copyright (C) 2017-2018 SiFive */ -#ifndef _ASM__UAPI__SYSCALLS_H -#define _ASM__UAPI__SYSCALLS_H +/* + * There is explicitly no include guard here because this file is expected to + * be included multiple times in order to define the syscall macros via like that one :) + * __SYSCALL. + */ /* * Allows the instruction cache to be flushed from userspace. Despite RISC-V
Re: [PATCH] spi-nor: add support for is25wp256d
On Mon, 06 Aug 2018 14:05:11 PDT (-0700), marek.va...@gmail.com wrote: On 08/06/2018 10:58 PM, Palmer Dabbelt wrote: On Sat, 04 Aug 2018 02:27:54 PDT (-0700), marek.va...@gmail.com wrote: On 08/04/2018 03:49 AM, Palmer Dabbelt wrote: From: "Wesley W. Terpstra" This is used of the HiFive Unleashed development board. Signed-off-by: Wesley W. Terpstra Signed-off-by: Palmer Dabbelt --- drivers/mtd/spi-nor/spi-nor.c | 47 ++- include/linux/mtd/spi-nor.h | 2 ++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index d9c368c44194..e9a3557a3c23 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -1072,6 +1072,9 @@ static const struct flash_info spi_nor_ids[] = { SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, { "is25wp128", INFO(0x9d7018, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, + { "is25wp256d", INFO(0x9d7019, 0, 32 * 1024, 1024, Is there a reason for the trailing 'd' in is25wp256d ? I'd drop it. I'm honestly not sure. There are data sheets for both of them, but I don't see much of a difference http://www.issi.com/WW/pdf/IS25LP(WP)256D.pdf http://www.issi.com/WW/pdf/25LP-WP256.pdf Following the pattern, I'd expect to see { "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, versus { "is25wp256d", INFO(0x9d7019, 0, 32 * 1024, 1024, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) }, They have the same ID ? Do we support the variant without the d already? Sorry for being a bit vague there. There is no is25wp256 in the list already, but if I follow the pattern from the other similar chips I get a different value for is25wp256 than what was proposed in the patch for an is25wp256d. From my understanding the different values are just because we picked a different block size, which seems possible because the original version of this patch was written before the other is25wp devices were added to the list. What I'm proposing is adding an is25wp256 with the same block size as the other similar entries in the list. Looking at the data sheets they appear to have the same values for the "Read Product Identification" instruction. The data sheets are shared with the is25lp256, which there is an entry for and matches my expected ID and block sizes. So in other words: the d less sections that are larger, and also has the 4B opcodes flag set. From the documentation in looks like the non-d version supports 3 and 4 byte opcodes, so I guess it's just a different physical layout? In the data sheet for both I see "Pages can be erased in groups of 4Kbyte sectors, 32Kbyte blocks, 64Kbyte blocks, and/or the entire chip" which indicates to me that maybe we've just selected the larger section size? If so then I'll change it to the first one in the new patch. + SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) + }, /* Macronix */ { "mx25l512e", INFO(0xc22010, 0, 64 * 1024, 1, SECT_4K) }, @@ -1515,6 +1518,45 @@ static int macronix_quad_enable(struct spi_nor *nor) return 0; } +/** + * issi_unlock() - clear BP[0123] write-protection. + * @nor: pointer to a 'struct spi_nor' + * + * Bits [2345] of the Status Register are BP[0123]. + * ISSI chips use a different block protection scheme than other chips. + * Just disable the write-protect unilaterally. + * + * Return: 0 on success, -errno otherwise. + */ +static int issi_unlock(struct spi_nor *nor) +{ + int ret, val; + u8 mask = SR_BP0 | SR_BP1 | SR_BP2 | SR_BP3; + + val = read_sr(nor); + if (val < 0) + return val; + if (!(val & mask)) + return 0; + + write_enable(nor); + + write_sr(nor, val & ~mask); + + ret = spi_nor_wait_till_ready(nor); + if (ret) + return ret; + + ret = read_sr(nor); + if (ret > 0 && !(ret & mask)) { + dev_info(nor->dev, "ISSI Block Protection Bits cleared\n"); + return 0; Is the dev_info() really needed ? Nope. I'll spin a v2 pending the above discussion. + } else { + dev_err(nor->dev, "ISSI Block Protection Bits not cleared\n"); + return -EINVAL; + } +} [...] Thanks!
[PATCH v2 0/2] spi-nor: add support for is25wp256
This adds support for the is25wp256 flash chip, which is on our HiFive Unleashed board. Additionally it adds support for ISSI's special unlocking scheme, which we need to unlock block protection on the whole chip. Changes since v1 [<20180804014947.24601-1-pal...@sifive.com>]: * There are now two patches, as it's logically two separate changes. * The chip is called is25wp256 instead of is25wp256d, with the corresponding commit text also renamed. * The block size has been changed to match the other similar chips that are already supported. * SPI_NOR_4B_OPCODES is no longer set, to match the other similar chips that are are already supported. [PATCH v2 1/2] spi-nor: add support for ISSI's block unlocking scheme [PATCH v2 2/2] spi-nor: add support for is25wp256
[PATCH v2 2/2] spi-nor: add support for is25wp256
From: "Wesley W. Terpstra" This is used of the HiFive Unleashed development board, and follows the pattern of similar ISSI devices already listed. Signed-off-by: Wesley W. Terpstra Signed-off-by: Palmer Dabbelt --- drivers/mtd/spi-nor/spi-nor.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index aab93463a5e7..f10017b4543d 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -1072,6 +1072,8 @@ static const struct flash_info spi_nor_ids[] = { SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, { "is25wp128", INFO(0x9d7018, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, + { "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512, + SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, /* Macronix */ { "mx25l512e", INFO(0xc22010, 0, 64 * 1024, 1, SECT_4K) }, -- 2.16.4
[PATCH v2 1/2] spi-nor: add support for ISSI's block unlocking scheme
From: "Wesley W. Terpstra" ISSI uses a non-standard scheme to control block protection, with bit 5 of the status registerr controlling an additional block protection bit. This patch disables all the block protection bits whenever an ISSI chip is seen. We might also want to trigger an error when writing SR_TB to these chips, as it aliases with this extra protection bit in the status register. It looks like that's always conditional on SNOR_F_HAS_SR_TB, so at least what's there is safe. Signed-off-by: Wesley W. Terpstra Signed-off-by: Palmer Dabbelt --- drivers/mtd/spi-nor/spi-nor.c | 43 ++- include/linux/mtd/spi-nor.h | 2 ++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index d9c368c44194..aab93463a5e7 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -1515,6 +1515,44 @@ static int macronix_quad_enable(struct spi_nor *nor) return 0; } +/** + * issi_unlock() - clear BP[0123] write-protection. + * @nor: pointer to a 'struct spi_nor' + * + * Bits [2345] of the Status Register are BP[0123]. + * ISSI chips use a different block protection scheme than other chips. + * Just disable the write-protect unilaterally. + * + * Return: 0 on success, -errno otherwise. + */ +static int issi_unlock(struct spi_nor *nor) +{ + int ret, val; + u8 mask = SR_BP0 | SR_BP1 | SR_BP2 | SR_BP3; + + val = read_sr(nor); + if (val < 0) + return val; + if (!(val & mask)) + return 0; + + write_enable(nor); + + write_sr(nor, val & ~mask); + + ret = spi_nor_wait_till_ready(nor); + if (ret) + return ret; + + ret = read_sr(nor); + if (ret > 0 && !(ret & mask)) { + return 0; + } else { + dev_err(nor->dev, "ISSI Block Protection Bits not cleared\n"); + return -EINVAL; + } +} + /* * Write status Register and configuration register with 2 bytes * The first byte will be written to the status register, while the @@ -2747,6 +2785,9 @@ static int spi_nor_init(struct spi_nor *nor) spi_nor_wait_till_ready(nor); } + if (JEDEC_MFR(nor->info) == SNOR_MFR_ISSI) + issi_unlock(nor); + if (nor->quad_enable) { err = nor->quad_enable(nor); if (err) { @@ -2926,7 +2967,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, if (ret) return ret; - if (nor->addr_width) { + if (nor->addr_width && JEDEC_MFR(info) != SNOR_MFR_ISSI) { /* already configured from SFDP */ } else if (info->addr_width) { nor->addr_width = info->addr_width; diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index e60da0d34cc1..da422a37d383 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -23,6 +23,7 @@ #define SNOR_MFR_ATMEL CFI_MFR_ATMEL #define SNOR_MFR_GIGADEVICE0xc8 #define SNOR_MFR_INTEL CFI_MFR_INTEL +#define SNOR_MFR_ISSI 0x9d #define SNOR_MFR_MICRONCFI_MFR_ST /* ST Micro <--> Micron */ #define SNOR_MFR_MACRONIX CFI_MFR_MACRONIX #define SNOR_MFR_SPANSION CFI_MFR_AMD @@ -121,6 +122,7 @@ #define SR_BP0 BIT(2) /* Block protect 0 */ #define SR_BP1 BIT(3) /* Block protect 1 */ #define SR_BP2 BIT(4) /* Block protect 2 */ +#define SR_BP3 BIT(5) /* Block protect 3 (on ISSI chips) */ #define SR_TB BIT(5) /* Top/Bottom protect */ #define SR_SRWDBIT(7) /* SR write protect */ /* Spansion/Cypress specific status bits */ -- 2.16.4
Re: [PATCH 03/11] dt-bindings: interrupt-controller: RISC-V PLIC documentation
On Mon, 06 Aug 2018 13:59:48 PDT (-0700), robh...@kernel.org wrote: On Thu, Aug 2, 2018 at 4:08 PM Atish Patra wrote: On 8/2/18 4:50 AM, Christoph Hellwig wrote: > From: Palmer Dabbelt > > This patch adds documentation for the platform-level interrupt > controller (PLIC) found in all RISC-V systems. This interrupt > controller routes interrupts from all the devices in the system to each > hart-local interrupt controller. > > Note: the DTS bindings for the PLIC aren't set in stone yet, as we might > want to change how we're specifying holes in the hart list. > > Signed-off-by: Palmer Dabbelt > [hch: various fixes and updates] > Signed-off-by: Christoph Hellwig > --- > .../interrupt-controller/sifive,plic0.txt | 57 +++ > 1 file changed, 57 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt > new file mode 100644 > index ..c756cd208a93 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt > @@ -0,0 +1,57 @@ > +SiFive Platform-Level Interrupt Controller (PLIC) > +- > + > +SiFive SOCs include an implementation of the Platform-Level Interrupt Controller > +(PLIC) high-level specification in the RISC-V Privileged Architecture > +specification. The PLIC connects all external interrupts in the system to all > +hart contexts in the system, via the external interrupt source in each hart. > + > +A hart context is a privilege mode in a hardware execution thread. For example, > +in an 4 core system with 2-way SMT, you have 8 harts and probably at least two > +privilege modes per hart; machine mode and supervisor mode. > + > +Each interrupt can be enabled on per-context basis. Any context can claim > +a pending enabled interrupt and then release it once it has been handled. > + > +Each interrupt has a configurable priority. Higher priority interrupts are > +serviced first. Each context can specify a priority threshold. Interrupts > +with priority below this threshold will not cause the PLIC to raise its > +interrupt line leading to the context. > + > +While the PLIC supports both edge-triggered and level-triggered interrupts, > +interrupt handlers are oblivious to this distinction and therefore it is not > +specified in the PLIC device-tree binding. > + > +While the RISC-V ISA doesn't specify a memory layout for the PLIC, the > +"sifive,plic0" device is a concrete implementation of the PLIC that contains a > +specific memory layout, which is documented in chapter 8 of the SiFive U5 > +Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>. > + > +Required properties: > +- compatible : "sifive,plic0" I think there was a thread bouncing around somewhere where decided to pick the official name of the compatible string to be "sifive,plic-1.0". The idea here is that the PLIC is compatible across all of SiFive's current implementations, but there's some limitations in the memory map that will probably cause us to spin a version 2 at some point so we want major version number. The minor number is just in case we find some errata in the PLIC. > +- #address-cells : should be <0> > +- #interrupt-cells : should be <1> > +- interrupt-controller : Identifies the node as an interrupt controller > +- reg : Should contain 1 register range (address and length) The one in the real device tree has two entries. reg = <0x 0x0c00 0x 0x0400>; Is it intentional or just incorrect entry left over from earlier days? > + reg = <0xc00 0x400>; Looks to me like one has #size-cells and #address-cells set to 2 and the example is using 1. Yes. For some background on how this works: we have a hardware generator that has a tree-of-busses abstraction, and each device is attached to some point on that tree. When a device gets attached to the bus, we also generate a device tree entry. For whatever system I generated the example PLIC device tree entry from, it must have been attached to a bus with addresses of 32 bits or less, which resulted in #address-cells and #size-cells being 1. Christoph has a HiFive Unleashed, which has a fu540-c000 on it, which is probably not what I generated as an example -- the fu540-c000 is a complicated configuration, when I mess around with the hardware I tend to use simple ones as I'm not really a hardware guy. I suppose the bus that the PLIC is hanging off on the fu540-c000 has addresses wider than 32 bi
Re: simplified RISC-V interrupt and clocksource handling v3
On Sat, 04 Aug 2018 01:23:11 PDT (-0700), Christoph Hellwig wrote: This series tries adds support for interrupt handling and timers for the RISC-V architecture. The basic per-hart interrupt handling implemented by the scause and sie CSRs is extremely simple and implemented directly in arch/riscv/kernel/irq.c. In addition there is a irqchip driver for the PLIC external interrupt controller, which is called through the set_handle_irq API, and a clocksource driver that gets its timer interrupt directly from the low-level interrupt handling. Compared to previous iterations this version does not try to use an irqchip driver for the low-level interrupt handling. This saves a couple indirect calls and an additional read of the scause CSR in the hot path, makes the code much simpler and last but not least avoid the dependency on a device tree for a mandatory architectural feature. A git tree is available here (contains a few more patches before the ones in this series) git://git.infradead.org/users/hch/riscv.git riscv-irq-simple.3 Gitweb: http://git.infradead.org/users/hch/riscv.git/shortlog/refs/heads/riscv-irq-simple.3 Changes since v2: - actually use SEIE instead of STIE in the plic driver - rename the default compat string for the plic to sifive,u5-plic - various spelling fixes - drop a superflous derefence in the plic driver that is taken care of by the following loop - drop the patch to document the enable method - not relevant for the rest of the series - drop the patches for the per-hart timebase frequency - not relevant for the rest of the series. - use riscv_of_processor_hart in the timer driver Changes since v1: - rename the plic driver to irq-sifive-plic - switch to a default compatible of sifive,plic0 (still supporting the riscv,plic0 name for compatibility) - add a reference for the SiFive PLIC register layout - fix plic_toggle addressing for large numbers of hwirqs - remove the call to ack_bad_irq - use a raw spinlock for plic_toggle_lock - use the irq_desc cpumask in the plic enable/disable methods - add back OF contexid parsing in the plic driver - don't allow COMPILE_TEST builds of the clocksource driver, as it depends on - default the clocksource driver to y - clean up naming in the clocksource driver - remove the MINDELTA and MAXDELTA #defines - various DT binding fixes Thanks! Modulo the one device tree issue I replied to in patch 3 this looks great! We've already gotten the ACKs to take this through the RISC-V tree, so I'm going to put this along with the queued RISC-V patches on our for-next branch, including my proposed change for "sifive,plic-1.0" but leaving the device tree bindings with #{address,size}-cells=1. We can always change this, but I'd like to get this out so people can start playing with it earlier rather than later. Thanks to everyone for all the help!
Finish the GENERIC_IRQ_MULTI_HANDLER conversion
A while ago I sent a patch set that adds a GENERIC_IRQ_MULTI_HANDLER, which is an exact copy of the existing IRQ_MULTI_HANDLER support in the arm port, which is being used unconditionally by arm64 and openrisc. GENERIC_IRQ_MULTI_HANDLER is currently being used by the RISC-V port. I managed to make a few mistakes in my original patch set and as a result my conversion of the other architectures of GENERIC_IRQ_MULTI_HANDLER was dropped. This patch set finishes up my original patch set by converting arm, arm64, and openrisc over to the new GENERIC_IRQ_MULTI_HANDLER support and then removing MULTI_IRQ_HANDLER as it's obselete. At the time I wrote this I gave it fairly extensive build testing, but went I recently rebased it I just tested the full patch set on arm, arm64, and openrisc defconfigs. Various flavors of this patch set have bounced around a few times before, but I'm calling this a whole new patch set as it builds on top of what was merged.
[PATCH 3/5] arm64: Use the new GENERIC_IRQ_MULTI_HANDLER
It appears arm64 copied arm's GENERIC_IRQ_MULTI_HANDLER code, but made it unconditional. I wanted to make this generic so it could be used by the RISC-V port. This patch converts the arm64 code to use the new generic code, which simply consists of deleting the arm64 code and setting MULTI_IRQ_HANDLER instead. Reviewed-by: Christoph Hellwig Signed-off-by: Palmer Dabbelt --- arch/arm64/Kconfig | 4 +--- arch/arm64/include/asm/irq.h | 2 -- arch/arm64/kernel/irq.c | 10 -- 3 files changed, 1 insertion(+), 15 deletions(-) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 42c090cf0292..3d1011957823 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -74,6 +74,7 @@ config ARM64 select GENERIC_CPU_AUTOPROBE select GENERIC_EARLY_IOREMAP select GENERIC_IDLE_POLL_SETUP + select GENERIC_IRQ_MULTI_HANDLER select GENERIC_IRQ_PROBE select GENERIC_IRQ_SHOW select GENERIC_IRQ_SHOW_LEVEL @@ -264,9 +265,6 @@ config ARCH_SUPPORTS_UPROBES config ARCH_PROC_KCORE_TEXT def_bool y -config MULTI_IRQ_HANDLER - def_bool y - source "init/Kconfig" source "kernel/Kconfig.freezer" diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h index a0fee6985e6a..b2b0c6405eb0 100644 --- a/arch/arm64/include/asm/irq.h +++ b/arch/arm64/include/asm/irq.h @@ -8,8 +8,6 @@ struct pt_regs; -extern void set_handle_irq(void (*handle_irq)(struct pt_regs *)); - static inline int nr_legacy_irqs(void) { return 0; diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c index 60e5fc661f74..780a12f59a8f 100644 --- a/arch/arm64/kernel/irq.c +++ b/arch/arm64/kernel/irq.c @@ -42,16 +42,6 @@ int arch_show_interrupts(struct seq_file *p, int prec) return 0; } -void (*handle_arch_irq)(struct pt_regs *) = NULL; - -void __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) -{ - if (handle_arch_irq) - return; - - handle_arch_irq = handle_irq; -} - #ifdef CONFIG_VMAP_STACK static void init_irq_stacks(void) { -- 2.16.4
Re: Finish the GENERIC_IRQ_MULTI_HANDLER conversion
On Thu, Jun 21, 2018 at 11:17 AM, Palmer Dabbelt wrote: > A while ago I sent a patch set that adds a GENERIC_IRQ_MULTI_HANDLER, > which is an exact copy of the existing IRQ_MULTI_HANDLER support in the > arm port, which is being used unconditionally by arm64 and openrisc. > GENERIC_IRQ_MULTI_HANDLER is currently being used by the RISC-V port. I > managed to make a few mistakes in my original patch set and as a result > my conversion of the other architectures of GENERIC_IRQ_MULTI_HANDLER > was dropped. > > This patch set finishes up my original patch set by converting arm, > arm64, and openrisc over to the new GENERIC_IRQ_MULTI_HANDLER support > and then removing MULTI_IRQ_HANDLER as it's obselete. > > At the time I wrote this I gave it fairly extensive build testing, but > went I recently rebased it I just tested the full patch set on arm, > arm64, and openrisc defconfigs. > > Various flavors of this patch set have bounced around a few times > before, but I'm calling this a whole new patch set as it builds on top > of what was merged. Looks like I managed to lose the patches. They should be threaded under this message...
[PATCH 5/5] irq: Remove MULTI_IRQ_HANDLER as it's now obselete
Now that every user of MULTI_IRQ_HANDLER has been convereted over to use GENERIC_IRQ_MULTI_HANDLER we can remove the references to MULTI_IRQ_HANDLER. Signed-off-by: Palmer Dabbelt --- drivers/irqchip/Kconfig | 24 kernel/irq/Kconfig | 1 - 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 7527f6a9adae..d564d21245c5 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -8,8 +8,7 @@ config ARM_GIC bool select IRQ_DOMAIN select IRQ_DOMAIN_HIERARCHY - select MULTI_IRQ_HANDLER - select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER select GENERIC_IRQ_EFFECTIVE_AFF_MASK config ARM_GIC_PM @@ -35,8 +34,7 @@ config GIC_NON_BANKED config ARM_GIC_V3 bool select IRQ_DOMAIN - select MULTI_IRQ_HANDLER - select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER select IRQ_DOMAIN_HIERARCHY select PARTITION_PERCPU select GENERIC_IRQ_EFFECTIVE_AFF_MASK @@ -68,8 +66,7 @@ config ARM_NVIC config ARM_VIC bool select IRQ_DOMAIN - select MULTI_IRQ_HANDLER - select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER config ARM_VIC_NR int @@ -96,16 +93,14 @@ config ATMEL_AIC_IRQ bool select GENERIC_IRQ_CHIP select IRQ_DOMAIN - select MULTI_IRQ_HANDLER - select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER select SPARSE_IRQ config ATMEL_AIC5_IRQ bool select GENERIC_IRQ_CHIP select IRQ_DOMAIN - select MULTI_IRQ_HANDLER - select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER select SPARSE_IRQ config I8259 @@ -142,8 +137,7 @@ config DW_APB_ICTL config FARADAY_FTINTC010 bool select IRQ_DOMAIN - select MULTI_IRQ_HANDLER - select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER select SPARSE_IRQ config HISILICON_IRQ_MBIGEN @@ -168,8 +162,7 @@ config CLPS711X_IRQCHIP bool depends on ARCH_CLPS711X select IRQ_DOMAIN - select MULTI_IRQ_HANDLER - select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER select SPARSE_IRQ default y @@ -188,8 +181,7 @@ config OMAP_IRQCHIP config ORION_IRQCHIP bool select IRQ_DOMAIN - select MULTI_IRQ_HANDLER - select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER config PIC32_EVIC bool diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig index c6766f326072..5f3e2baefca9 100644 --- a/kernel/irq/Kconfig +++ b/kernel/irq/Kconfig @@ -134,7 +134,6 @@ config GENERIC_IRQ_DEBUGFS endmenu config GENERIC_IRQ_MULTI_HANDLER - depends on !MULTI_IRQ_HANDLER bool help Allow to specify the low level IRQ handler at run time. -- 2.16.4
[PATCH 4/5] openrisc: Use the new GENERIC_IRQ_MULTI_HANDLER
It appears that openrisc copied arm64's GENERIC_IRQ_MULTI_HANDLER code (which came from arm). I wanted to make this generic so I could use it in the RISC-V port. This patch converts the openrisc code to use the generic version. Acked-by: Stafford Horne Signed-off-by: Palmer Dabbelt --- arch/openrisc/Kconfig | 5 + arch/openrisc/include/asm/irq.h | 2 -- arch/openrisc/kernel/irq.c | 7 --- 3 files changed, 1 insertion(+), 13 deletions(-) diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig index 9ecad05bfc73..dfb6a79ba7ff 100644 --- a/arch/openrisc/Kconfig +++ b/arch/openrisc/Kconfig @@ -27,7 +27,6 @@ config OPENRISC select GENERIC_STRNLEN_USER select GENERIC_SMP_IDLE_THREAD select MODULES_USE_ELF_RELA - select MULTI_IRQ_HANDLER select HAVE_DEBUG_STACKOVERFLOW select OR1K_PIC select CPU_NO_EFFICIENT_FFS if !OPENRISC_HAVE_INST_FF1 @@ -36,6 +35,7 @@ config OPENRISC select ARCH_USE_QUEUED_RWLOCKS select OMPIC if SMP select ARCH_WANT_FRAME_POINTERS + select GENERIC_IRQ_MULTI_HANDLER config CPU_BIG_ENDIAN def_bool y @@ -69,9 +69,6 @@ config STACKTRACE_SUPPORT config LOCKDEP_SUPPORT def_bool y -config MULTI_IRQ_HANDLER - def_bool y - source "init/Kconfig" source "kernel/Kconfig.freezer" diff --git a/arch/openrisc/include/asm/irq.h b/arch/openrisc/include/asm/irq.h index d9eee0a2b7b4..eb612b1865d2 100644 --- a/arch/openrisc/include/asm/irq.h +++ b/arch/openrisc/include/asm/irq.h @@ -24,6 +24,4 @@ #define NO_IRQ (-1) -extern void set_handle_irq(void (*handle_irq)(struct pt_regs *)); - #endif /* __ASM_OPENRISC_IRQ_H__ */ diff --git a/arch/openrisc/kernel/irq.c b/arch/openrisc/kernel/irq.c index 35e478a93116..5f9445effaf8 100644 --- a/arch/openrisc/kernel/irq.c +++ b/arch/openrisc/kernel/irq.c @@ -41,13 +41,6 @@ void __init init_IRQ(void) irqchip_init(); } -static void (*handle_arch_irq)(struct pt_regs *); - -void __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) -{ - handle_arch_irq = handle_irq; -} - void __irq_entry do_IRQ(struct pt_regs *regs) { handle_arch_irq(regs); -- 2.16.4
[PATCH 2/5] arm: Convert to GENERIC_IRQ_MULTI_HANDLER
This converts the ARM port to use the recently added GENERIC_IRQ_MULTI_HANDLER, which is essentially just a copy of ARM's existhing MULTI_IRQ_HANDLER. The only changes are: * handle_arch_irq is now defined in a generic C file instead of an arm-specific assembly file. * handle_arch_irq is now marked as __ro_after_init. Signed-off-by: Palmer Dabbelt --- arch/arm/Kconfig | 19 +++ arch/arm/include/asm/irq.h | 5 - arch/arm/include/asm/mach/arch.h | 2 +- arch/arm/kernel/entry-armv.S | 10 ++ arch/arm/kernel/irq.c| 10 -- arch/arm/kernel/setup.c | 2 +- 6 files changed, 11 insertions(+), 37 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 54eeb8d00bc6..b6be2b1be75d 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -337,8 +337,8 @@ config ARCH_MULTIPLATFORM select TIMER_OF select COMMON_CLK select GENERIC_CLOCKEVENTS + select GENERIC_IRQ_MULTI_HANDLER select MIGHT_HAVE_PCI - select MULTI_IRQ_HANDLER select PCI_DOMAINS if PCI select SPARSE_IRQ select USE_OF @@ -465,9 +465,9 @@ config ARCH_DOVE bool "Marvell Dove" select CPU_PJ4 select GENERIC_CLOCKEVENTS + select GENERIC_IRQ_MULTI_HANDLER select GPIOLIB select MIGHT_HAVE_PCI - select MULTI_IRQ_HANDLER select MVEBU_MBUS select PINCTRL select PINCTRL_DOVE @@ -512,8 +512,8 @@ config ARCH_LPC32XX select COMMON_CLK select CPU_ARM926T select GENERIC_CLOCKEVENTS + select GENERIC_IRQ_MULTI_HANDLER select GPIOLIB - select MULTI_IRQ_HANDLER select SPARSE_IRQ select USE_OF help @@ -532,11 +532,11 @@ config ARCH_PXA select TIMER_OF select CPU_XSCALE if !CPU_XSC3 select GENERIC_CLOCKEVENTS + select GENERIC_IRQ_MULTI_HANDLER select GPIO_PXA select GPIOLIB select HAVE_IDE select IRQ_DOMAIN - select MULTI_IRQ_HANDLER select PLAT_PXA select SPARSE_IRQ help @@ -572,11 +572,11 @@ config ARCH_SA1100 select CPU_FREQ select CPU_SA1100 select GENERIC_CLOCKEVENTS + select GENERIC_IRQ_MULTI_HANDLER select GPIOLIB select HAVE_IDE select IRQ_DOMAIN select ISA - select MULTI_IRQ_HANDLER select NEED_MACH_MEMORY_H select SPARSE_IRQ help @@ -590,10 +590,10 @@ config ARCH_S3C24XX select GENERIC_CLOCKEVENTS select GPIO_SAMSUNG select GPIOLIB + select GENERIC_IRQ_MULTI_HANDLER select HAVE_S3C2410_I2C if I2C select HAVE_S3C2410_WATCHDOG if WATCHDOG select HAVE_S3C_RTC if RTC_CLASS - select MULTI_IRQ_HANDLER select NEED_MACH_IO_H select SAMSUNG_ATAGS select USE_OF @@ -627,10 +627,10 @@ config ARCH_OMAP1 select CLKSRC_MMIO select GENERIC_CLOCKEVENTS select GENERIC_IRQ_CHIP + select GENERIC_IRQ_MULTI_HANDLER select GPIOLIB select HAVE_IDE select IRQ_DOMAIN - select MULTI_IRQ_HANDLER select NEED_MACH_IO_H if PCCARD select NEED_MACH_MEMORY_H select SPARSE_IRQ @@ -921,11 +921,6 @@ config IWMMXT Enable support for iWMMXt context switching at run time if running on a CPU that supports it. -config MULTI_IRQ_HANDLER - bool - help - Allow each machine to specify it's own IRQ handler at run time. - if !MMU source "arch/arm/Kconfig-nommu" endif diff --git a/arch/arm/include/asm/irq.h b/arch/arm/include/asm/irq.h index b6f319606e30..c883fcbe93b6 100644 --- a/arch/arm/include/asm/irq.h +++ b/arch/arm/include/asm/irq.h @@ -31,11 +31,6 @@ extern void asm_do_IRQ(unsigned int, struct pt_regs *); void handle_IRQ(unsigned int, struct pt_regs *); void init_IRQ(void); -#ifdef CONFIG_MULTI_IRQ_HANDLER -extern void (*handle_arch_irq)(struct pt_regs *); -extern void set_handle_irq(void (*handle_irq)(struct pt_regs *)); -#endif - #ifdef CONFIG_SMP extern void arch_trigger_cpumask_backtrace(const cpumask_t *mask, bool exclude_self); diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h index 5c1ad11aa392..bb8851208e17 100644 --- a/arch/arm/include/asm/mach/arch.h +++ b/arch/arm/include/asm/mach/arch.h @@ -59,7 +59,7 @@ struct machine_desc { void(*init_time)(void); void(*init_machine)(void); void(*init_late)(void); -#ifdef CONFIG_MULTI_IRQ_HANDLER +#ifdef CONFIG_GENERIC_IRQ_MULTI_HANDLER void(*handle_irq)(struct pt_regs *); #endif void(*restart)(enum reboot_mode, const char *); diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S index 179a9f6bd
[PATCH 1/5] irq: Port the ARM IRQ drivers to GENERIC_IRQ_MULTI_HANDLER
GENERIC_IRQ_MULTI_HANDLER is incompatible with MULTI_IRQ_HANDLER because they define the same symbols. Multiple generic irqchip drivers select MULTI_IRQ_HANDLER, which is now defined on all architectures that provide set_handle_irq(). This patch selects GENERIC_IRQ_MULTI_HANDLER for all drivers that used to select MULTI_IRQ_HANDLER, but only when MULTI_IRQ_HANDLER doesn't exist. I'll then convert every architecture over from MULTI_IRQ_HANDLER to GENERIC_IRQ_MULTI_HANDLER before removing the extra MULTI_IRQ_HANDLER scaffolding. CC: Shea Levy CC: Arnd Bergmann Signed-off-by: Palmer Dabbelt --- drivers/irqchip/Kconfig | 8 1 file changed, 8 insertions(+) diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index e9233db16e03..7527f6a9adae 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -9,6 +9,7 @@ config ARM_GIC select IRQ_DOMAIN select IRQ_DOMAIN_HIERARCHY select MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER select GENERIC_IRQ_EFFECTIVE_AFF_MASK config ARM_GIC_PM @@ -35,6 +36,7 @@ config ARM_GIC_V3 bool select IRQ_DOMAIN select MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER select IRQ_DOMAIN_HIERARCHY select PARTITION_PERCPU select GENERIC_IRQ_EFFECTIVE_AFF_MASK @@ -67,6 +69,7 @@ config ARM_VIC bool select IRQ_DOMAIN select MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER config ARM_VIC_NR int @@ -94,6 +97,7 @@ config ATMEL_AIC_IRQ select GENERIC_IRQ_CHIP select IRQ_DOMAIN select MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER select SPARSE_IRQ config ATMEL_AIC5_IRQ @@ -101,6 +105,7 @@ config ATMEL_AIC5_IRQ select GENERIC_IRQ_CHIP select IRQ_DOMAIN select MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER select SPARSE_IRQ config I8259 @@ -138,6 +143,7 @@ config FARADAY_FTINTC010 bool select IRQ_DOMAIN select MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER select SPARSE_IRQ config HISILICON_IRQ_MBIGEN @@ -163,6 +169,7 @@ config CLPS711X_IRQCHIP depends on ARCH_CLPS711X select IRQ_DOMAIN select MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER select SPARSE_IRQ default y @@ -182,6 +189,7 @@ config ORION_IRQCHIP bool select IRQ_DOMAIN select MULTI_IRQ_HANDLER + select GENERIC_IRQ_MULTI_HANDLER if !MULTI_IRQ_HANDLER config PIC32_EVIC bool -- 2.16.4
[PATCH] MAINTAINERS: Add Daniel Lustig as a LKMM reviewer
Dan runs the RISC-V memory model working group. I've been forwarding him LKMM emails that end up in my inbox, but I'm far from an expert in this stuff. He requested to be added as a reviewer, which seem sane to me as it'll take a human out of the loop. CC: Daniel Lustig Signed-off-by: Palmer Dabbelt --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9d5eeff51b5f..b980635220e5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8316,6 +8316,7 @@ M:Jade Alglave M: Luc Maranget M: "Paul E. McKenney" R: Akira Yokosawa +R:`Daniel Lustig L: linux-kernel@vger.kernel.org S: Supported T: git git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git -- 2.16.4
[PATCH] RISC-V: Add early printk support via the SBI console
This code lives entirely within the RISC-V arch code. I've left it within an "#ifdef CONFIG_EARLY_PRINTK" despite always having EARLY_PRINTK support on RISC-V just in case someone wants to remove it. Signed-off-by: Palmer Dabbelt --- arch/riscv/kernel/setup.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index ee44a48faf79..494c15ba4610 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -39,6 +39,27 @@ #include #include +#ifdef CONFIG_EARLY_PRINTK +static void sbi_console_write(struct console *co, const char *buf, + unsigned int n) +{ + int i; + + for (i = 0; i < n; ++i) { + if (buf[i] == '\n') + sbi_console_putchar('\r'); + sbi_console_putchar(buf[i]); + } +} + +struct console riscv_sbi_early_console_dev __initdata = { + .name = "early", + .write = sbi_console_write, + .flags = CON_PRINTBUFFER | CON_BOOT | CON_ANYTIME, + .index = -1 +}; +#endif + #ifdef CONFIG_DUMMY_CONSOLE struct screen_info screen_info = { .orig_video_lines = 30, @@ -195,6 +216,12 @@ static void __init setup_bootmem(void) void __init setup_arch(char **cmdline_p) { +#if defined(CONFIG_EARLY_PRINTK) + if (likely(early_console == NULL)) { + early_console = &riscv_sbi_early_console_dev; + register_console(early_console); + } +#endif *cmdline_p = boot_command_line; parse_early_param(); -- 2.16.4
Re: [PATCH] MAINTAINERS: Add Daniel Lustig as a LKMM reviewer
On Fri, 22 Jun 2018 14:48:56 PDT (-0700), Daniel Lustig wrote: On 6/22/2018 2:17 PM, Palmer Dabbelt wrote: Dan runs the RISC-V memory model working group. I've been forwarding him LKMM emails that end up in my inbox, but I'm far from an expert in this stuff. He requested to be added as a reviewer, which seem sane to me as it'll take a human out of the loop. CC: Daniel Lustig Signed-off-by: Palmer Dabbelt Looks like there's an accidental backquote before my name? Once that gets fixed: Acked-by: Daniel Lustig Thanks. Unless anyone has any opposition I'll submit the fixed patch as part of my next pull request. commit 9d01337e4724be4d34bfe848a7c64d14bfdb89ea gpg: Signature made Fri 22 Jun 2018 03:35:24 PM PDT gpg:using RSA key 00CE76D1834960DFCE886DF8EF4CA1502CCBAB41 gpg:issuer "pal...@dabbelt.com" gpg: Good signature from "Palmer Dabbelt " [ultimate] gpg: aka "Palmer Dabbelt " [ultimate] Author: Palmer Dabbelt Date: Fri Jun 22 14:04:42 2018 -0700 MAINTAINERS: Add Daniel Lustig as a LKMM reviewer Dan runs the RISC-V memory model working group. I've been forwarding him LKMM emails that end up in my inbox, but I'm far from an expert in this stuff. He requested to be added as a reviewer, which seem sane to me as it'll take a human out of the loop. CC: Daniel Lustig Acked-by: Daniel Lustig Signed-off-by: Palmer Dabbelt diff --git a/MAINTAINERS b/MAINTAINERS index 9d5eeff51b5f..ac8ed55dbe9b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8316,6 +8316,7 @@ M:Jade Alglave M: Luc Maranget M: "Paul E. McKenney" R: Akira Yokosawa +R: Daniel Lustig L: linux-kernel@vger.kernel.org S: Supported T: git git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git
[PATCH 3/3] irqchip: RISC-V Local Interrupt Controller Driver
From: Palmer Dabbelt This patch adds a driver that manages the local interrupts on each RISC-V hart, as specifiec by the RISC-V supervisor level ISA manual. The local interrupt controller manages software interrupts, timer interrupts, and hardware interrupts (which are routed via the platform level interrupt controller). Per-hart local interrupt controllers are found on all RISC-V systems. CC: Michael Clark Signed-off-by: Palmer Dabbelt --- drivers/irqchip/Kconfig | 13 +++ drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-riscv-intc.c | 215 +++ 3 files changed, 229 insertions(+) create mode 100644 drivers/irqchip/irq-riscv-intc.c diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index e9233db16e03..bf7fc86673b1 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -372,3 +372,16 @@ config QCOM_PDC IRQs for Qualcomm Technologies Inc (QTI) mobile chips. endmenu + +config RISCV_INTC + #bool "RISC-V Interrupt Controller" + depends on RISCV + default y + help + This enables support for the local interrupt controller found in + standard RISC-V systems. The local interrupt controller handles + timer interrupts, software interrupts, and hardware interrupts. + Without a local interrupt controller the system will be unable to + handle any interrupts, including those passed via the PLIC. + + If you don't know what to do here, say Y. diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index 15f268f646bf..74e333cc274c 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -87,3 +87,4 @@ obj-$(CONFIG_MESON_IRQ_GPIO) += irq-meson-gpio.o obj-$(CONFIG_GOLDFISH_PIC) += irq-goldfish-pic.o obj-$(CONFIG_NDS32)+= irq-ativic32.o obj-$(CONFIG_QCOM_PDC) += qcom-pdc.o +obj-$(CONFIG_RISCV_INTC) += irq-riscv-intc.o diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c new file mode 100644 index ..7ca3060e76b4 --- /dev/null +++ b/drivers/irqchip/irq-riscv-intc.c @@ -0,0 +1,215 @@ +/* + * Copyright (C) 2012 Regents of the University of California + * Copyright (C) 2017-2018 SiFive + */ +/* SPDX-License-Identifier: GPL-2.0 */ + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +#define PTR_BITS (8 * sizeof(uintptr_t)) + +struct riscv_irq_data { + struct irq_chip chip; + struct irq_domain *domain; + int hart; + charname[20]; +}; +DEFINE_PER_CPU(struct riscv_irq_data, riscv_irq_data); + +static void riscv_software_interrupt(void) +{ +#ifdef CONFIG_SMP + irqreturn_t ret; + + ret = handle_ipi(); + + WARN_ON(ret == IRQ_NONE); +#else + /* +* We currently only use software interrupts to pass inter-processor +* interrupts, so if a non-SMP system gets a software interrupt then we +* don't know what to do. +*/ + pr_warning("Software Interrupt without CONFIG_SMP\n"); +#endif +} + +static void riscv_intc_irq(struct pt_regs *regs) +{ + struct pt_regs *old_regs = set_irq_regs(regs); + struct irq_domain *domain; + unsigned long cause = csr_read(scause); + + /* +* The high order bit of the trap cause register is always set for +* interrupts, which allows us to differentiate them from exceptions +* quickly. The INTERRUPT_CAUSE_* macros don't contain that bit, so we +* need to mask it off here. +*/ + WARN_ON((cause & (1UL << (PTR_BITS - 1))) == 0); + cause = cause & ~(1UL << (PTR_BITS - 1)); + + 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: + domain = per_cpu(riscv_irq_data, smp_processor_id()).domain; + generic_handle_irq(irq_find_mapping(domain, cause)); + break; + } + + irq_exit(); + set_irq_regs(old_regs); +} + +static int riscv_irqdomain_map(struct irq_domain *d, unsigned int irq, + irq_hw_number_t hwirq) +{ + struct riscv_irq_data *data = d->host_data; + + irq_set_chip_and_handler(irq, &data->chip, handle_simple_irq); + irq_set_chip_data(irq, data); + irq_set_noprobe(irq); + irq_set_affinity(irq, cpumask_of(data->
[PATCH 2/3] dt-bindings: interrupt-controller: RISC-V local interrupt controller docs
From: Palmer Dabbelt This patch adds documentation on the RISC-V local interrupt controller, which is a per-hart interrupt controller that manages all interrupts entering a RISC-V hart. This interrupt controller is present on all RISC-V systems. Signed-off-by: Palmer Dabbelt --- .../interrupt-controller/riscv,cpu-intc.txt| 41 ++ 1 file changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt new file mode 100644 index ..61900e2e3868 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt @@ -0,0 +1,41 @@ +RISC-V Hart-Level Interrupt Controller (HLIC) +- + +RISC-V cores include Control Status Registers (CSRs) which are local to each +hart and can be read or written by software. Some of these CSRs are used to +control local interrupts connected to the core. Every interrupt is ultimately +routed through a hart's HLIC before it interrupts that hart. + +The RISC-V supervisor ISA manual specifies three interrupt sources that are +attached to every HLIC: software interrupts, the timer interrupt, and external +interrupts. Software interrupts are used to send IPIs between cores. The +timer interrupt comes from an architecturally mandated real-time timer that is +controller via SBI calls and CSR reads. External interrupts connect all other +device interrupts to the HLIC, which are routed via the platform-level +interrupt controller (PLIC). + +All RISC-V systems that conform to the supervisor ISA specification are +required to have a HLIC with these three interrupt sources present. Since the +interrupt map is defined by the ISA it's not listed in the HLIC's device tree +entry, though external interrupt controllers (like the PLIC, for example) will +need to define how their interrupts map to the relevant HLICs. + +Required properties: +- compatible : "riscv,cpu-intc" +- #interrupt-cells : should be <1> +- interrupt-controller : Identifies the node as an interrupt controller + +Furthermore, this interrupt-controller MUST be embedded inside the cpu +definition of the hart whose CSRs control these local interrupts. + +An example device tree entry for a HLIC is show below. + + cpu1: cpu@1 { + compatible = "riscv"; + ... + cpu1-intc: interrupt-controller { + #interrupt-cells = <1>; + compatible = "riscv,cpu-intc"; + interrupt-controller; + }; + }; -- 2.16.4
[PATCH 1/3] RISC-V: Don't include irq-riscv-intc.h
This file has never existed in the upstream kernel, but it's guarded by an #ifdef that's also never existed in the upstream kernel. As a part of our interrupt controller refactoring this header is no longer necessary, but this reference managed to sneak in anyway. Signed-off-by: Palmer Dabbelt --- arch/riscv/kernel/irq.c | 4 1 file changed, 4 deletions(-) diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c index b74cbfbce2d0..7bcdaed15703 100644 --- a/arch/riscv/kernel/irq.c +++ b/arch/riscv/kernel/irq.c @@ -16,10 +16,6 @@ #include #include -#ifdef CONFIG_RISCV_INTC -#include -#endif - void __init init_IRQ(void) { irqchip_init(); -- 2.16.4
Driver for the RISC-V Interrupt Controller
The RISC-V ISA mandantes the presence of a simple, per-hart (hardware thread) interrupt controller availiable to supervisor mode. This patch set adds a driver for this interrupt controller. The patch set itself has been around in various flavors for a while, but as far as I remember it's never been properly cleaned up and submitted before. As it currently stands it's essentially three seperate patches, but as they're all intertwined I'm keeping them together. Sorry if that's the wrong thing to do. The patches are: * A cleanup to arch/riscv to remove a bit of the old initialization mechanism that snuck in to our arch port. This patch is trivial, so unless there's any feedback on it specificly I'll include it in my next pull request. * The addition of device tree bindings to describe "riscv,cpu-intc". * The actual irqchip driver. If this gets merged before the arch/riscv patch then it'll cause our build to fail, but I'm assuming this will be targeted at the next merge window so we should be safe.
Re: [PATCH] MAINTAINERS: Add Daniel Lustig as a LKMM reviewer
On Sat, 23 Jun 2018 09:46:45 PDT (-0700), paul...@linux.vnet.ibm.com wrote: On Sat, Jun 23, 2018 at 03:10:17AM +0200, Andrea Parri wrote: > > Thanks. Unless anyone has any opposition I'll submit the fixed > > patch as part of my next pull request. > > Works for me, especially if this means that Daniel is RISC-V's official > representative. ;-) I'd rather the "fixed patch" go through the LKMM's tree. If not for other, we tend to use get_maintainer.pl on your (TBD ;/) development branch... True, but that is only an issue for a few weeks until it goes upstream. I am OK either way. I'm happy to have the patch go through the LKMM tree. The fixed patch is here https://git.kernel.org/pub/scm/linux/kernel/git/palmer/linux.git/patch/?id=778c83c0bf1e1dcc6054c40945ed05b42b71c97c Thanx, Paul > Acked-by: Paul E. McKenney > > > commit 9d01337e4724be4d34bfe848a7c64d14bfdb89ea > > gpg: Signature made Fri 22 Jun 2018 03:35:24 PM PDT > > gpg:using RSA key 00CE76D1834960DFCE886DF8EF4CA1502CCBAB41 > > gpg: issuer "pal...@dabbelt.com" > > gpg: Good signature from "Palmer Dabbelt " [ultimate] > > gpg: aka "Palmer Dabbelt " [ultimate] > > Author: Palmer Dabbelt > > Date: Fri Jun 22 14:04:42 2018 -0700 > > > >MAINTAINERS: Add Daniel Lustig as a LKMM reviewer Nit: an LKMM > > > >Dan runs the RISC-V memory model working group. I've been forwarding > >him LKMM emails that end up in my inbox, but I'm far from an expert in > >this stuff. He requested to be added as a reviewer, which seem sane to Nit: which seems > >me as it'll take a human out of the loop. > > > >CC: Daniel Lustig > >Acked-by: Daniel Lustig > >Signed-off-by: Palmer Dabbelt Glad to read this! Please feel free to add: Acked-by: Andrea Parri Andrea > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 9d5eeff51b5f..ac8ed55dbe9b 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -8316,6 +8316,7 @@ M: Jade Alglave > > M:Luc Maranget > > M:"Paul E. McKenney" > > R:Akira Yokosawa > > +R: Daniel Lustig > > L:linux-kernel@vger.kernel.org > > S:Supported > > T:git git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git
[PATCH 0/8] RISC-V: Assorted Cleanups
I finally got around to answering a very old email in my inbox that was a response to our original patch set. These are meant to cause little to no functional changes to the port, but I've given them very little testing so I wouldn't be surprised if I've managed to screw something up here. I'll be sure to give these some testing before putting them on for-next, but I figured it'd be better to send them out now rather than later.
[PATCH 1/8] RISC-V: Provide a cleaner raw_smp_processor_id()
I'm not sure how I managed to miss this the first time, but this is much better. Signed-off-by: Palmer Dabbelt --- arch/riscv/include/asm/smp.h | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h index 36016845461d..27fd085188df 100644 --- a/arch/riscv/include/asm/smp.h +++ b/arch/riscv/include/asm/smp.h @@ -14,11 +14,6 @@ #ifndef _ASM_RISCV_SMP_H #define _ASM_RISCV_SMP_H -/* This both needs asm-offsets.h and is used when generating it. */ -#ifndef GENERATING_ASM_OFFSETS -#include -#endif - #include #include @@ -33,13 +28,12 @@ void arch_send_call_function_ipi_mask(struct cpumask *mask); /* Hook for the generic smp_call_function_single() routine. */ void arch_send_call_function_single_ipi(int cpu); -/* - * This is particularly ugly: it appears we can't actually get the definition - * of task_struct here, but we need access to the CPU this task is running on. - * Instead of using C we're using asm-offsets.h to get the current processor - * ID. - */ -#define raw_smp_processor_id() (*((int*)((char*)get_current() + TASK_TI_CPU))) +/* Obtains the hart ID of the currently executing task. This relies on + * THREAD_INFO_IN_TASK, but we define that unconditionally. */ +#ifdef CONFIG_THREAD_INFO_IN_TASK +#define get_ti()((struct thread_info *)get_current()) +#define raw_smp_processor_id() (get_ti()->cpu) +#endif #endif /* CONFIG_SMP */ -- 2.16.4
[PATCH 8/8] RISC-V: Disable preemption before enabling interrupts when booting secondary harts
I'm not sure, but I think this was a bug: if the scheduler fired right here then I believe it would blow up. Signed-off-by: Palmer Dabbelt --- arch/riscv/kernel/smpboot.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c index 953bc540207d..45515cc70181 100644 --- a/arch/riscv/kernel/smpboot.c +++ b/arch/riscv/kernel/smpboot.c @@ -110,7 +110,9 @@ asmlinkage void __init smp_callin(void) /* Remote TLB flushes are ignored while the CPU is offline, so emit a local * TLB flush right now just in case. */ local_flush_tlb_all(); - local_irq_enable(); + /* Disable preemption before enabling interrupts, so we don't try to +* schedule a CPU that hasn't actually started yet. */ preempt_disable(); + local_irq_enable(); cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); } -- 2.16.4
[PATCH 3/8] RISC-V: Rename riscv_of_processor_hart to riscv_of_processor_hartid
It's a bit confusing exactly what this function does: it actually returns the hartid of an OF processor node, failing with -1 on invalid nodes. I've changed the name to _hartid() in order to make that a bit more clear, as well as adding a comment. Signed-off-by: Palmer Dabbelt --- arch/riscv/include/asm/processor.h | 4 +++- arch/riscv/kernel/cpu.c| 2 +- arch/riscv/kernel/smpboot.c| 2 +- drivers/clocksource/riscv_timer.c | 2 +- drivers/irqchip/irq-sifive-plic.c | 2 +- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h index 3fe4af8147d2..9d32670d84a4 100644 --- a/arch/riscv/include/asm/processor.h +++ b/arch/riscv/include/asm/processor.h @@ -88,7 +88,9 @@ static inline void wait_for_interrupt(void) } struct device_node; -extern int riscv_of_processor_hart(struct device_node *node); +/* Returns the hart ID of the given device tree node, or -1 if the device tree + * node isn't a RISC-V hart. */ +extern int riscv_of_processor_hartid(struct device_node *node); extern void riscv_fill_hwcap(void); diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index ca6c81e54e37..19e98c1710dd 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -16,7 +16,7 @@ #include /* Return -1 if not a valid hart */ -int riscv_of_processor_hart(struct device_node *node) +int riscv_of_processor_hartid(struct device_node *node) { const char *isa, *status; u32 hart; diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c index 56abab6a9812..5f29f8562cf6 100644 --- a/arch/riscv/kernel/smpboot.c +++ b/arch/riscv/kernel/smpboot.c @@ -53,7 +53,7 @@ void __init setup_smp(void) int hart, im_okay_therefore_i_am = 0; while ((dn = of_find_node_by_type(dn, "cpu"))) { - hart = riscv_of_processor_hart(dn); + hart = riscv_of_processor_hartid(dn); if (hart >= 0) { set_cpu_possible(hart, true); set_cpu_present(hart, true); diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c index 4e8b347e43e2..ad7453fc3129 100644 --- a/drivers/clocksource/riscv_timer.c +++ b/drivers/clocksource/riscv_timer.c @@ -84,7 +84,7 @@ void riscv_timer_interrupt(void) static int __init riscv_timer_init_dt(struct device_node *n) { - int cpu_id = riscv_of_processor_hart(n), error; + int cpu_id = riscv_of_processor_hartid(n), error; struct clocksource *cs; if (cpu_id != smp_processor_id()) diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c index 532e9d68c704..c55eaa31cde2 100644 --- a/drivers/irqchip/irq-sifive-plic.c +++ b/drivers/irqchip/irq-sifive-plic.c @@ -176,7 +176,7 @@ static int plic_find_hart_id(struct device_node *node) { for (; node; node = node->parent) { if (of_device_is_compatible(node, "riscv")) - return riscv_of_processor_hart(node); + return riscv_of_processor_hartid(node); } return -1; -- 2.16.4
[PATCH 4/8] RISC-V: Filter ISA and MMU values in cpuinfo
We shouldn't be directly passing device tree values to userspace, both because there could be mistakes in device trees and because the kernel doesn't support arbitrary ISAs. Signed-off-by: Palmer Dabbelt --- arch/riscv/kernel/cpu.c | 62 +++-- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index 19e98c1710dd..a18b4e3962a1 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -58,6 +58,57 @@ int riscv_of_processor_hartid(struct device_node *node) #ifdef CONFIG_PROC_FS +static void print_isa(struct seq_file *f, const char *orig_isa) +{ +static const char *ext = "mafdc"; +const char *isa = orig_isa; +const char *e; + +/* Linux doesn't support rv32e or rv128i, and we only support booting + * kernels on harts with the same ISA that the kernel is compiled for. */ +#if defined(CONFIG_32BIT) +if (strncmp(isa, "rv32i", 5) != 0) +return; +#elif defined(CONFIG_64BIT) +if (strncmp(isa, "rv64i", 5) != 0) +return; +#endif + +/* Print the base ISA, as we already know it's legal. */ +seq_printf(f, "isa\t: "); +seq_write(f, isa, 5); +isa += 5; + +/* Check the rest of the ISA string for valid extensions, printing those we + * find. RISC-V ISA strings define an order, so we only print the + * extension bits when they're in order. */ +for (e = ext; *e != '\0'; ++e) { +if (isa[0] == e[0]) { +seq_write(f, isa, 1); +isa++; +} +} + +/* If we were given an unsupported ISA in the device tree then print a bit + * of info describing what went wrong. */ +if (isa[0] != '\0') +pr_info("unsupported ISA \"%s\" in device tree", orig_isa); +} + +static void print_mmu(struct seq_file *f, const char *mmu_type) +{ +#if defined(CONFIG_32BIT) +if (strcmp(mmu_type, "riscv,sv32") != 0) +return; +#elif defined(CONFIG_64BIT) +if ((strcmp(mmu_type, "riscv,sv39") != 0) +&& (strcmp(mmu_type, "riscv,sv48") != 0)) +return; +#endif + +seq_printf(f, "mmu\t: %s\n", mmu_type+6); +} + static void *c_start(struct seq_file *m, loff_t *pos) { *pos = cpumask_next(*pos - 1, cpu_online_mask); @@ -83,13 +134,10 @@ static int c_show(struct seq_file *m, void *v) const char *compat, *isa, *mmu; seq_printf(m, "hart\t: %lu\n", hart_id); - if (!of_property_read_string(node, "riscv,isa", &isa) - && isa[0] == 'r' - && isa[1] == 'v') - seq_printf(m, "isa\t: %s\n", isa); - if (!of_property_read_string(node, "mmu-type", &mmu) - && !strncmp(mmu, "riscv,", 6)) - seq_printf(m, "mmu\t: %s\n", mmu+6); + if (!of_property_read_string(node, "riscv,isa", &isa)) +print_isa(m, isa); + if (!of_property_read_string(node, "mmu-type", &mmu)) +print_mmu(m, mmu); if (!of_property_read_string(node, "compatible", &compat) && strcmp(compat, "riscv")) seq_printf(m, "uarch\t: %s\n", compat); -- 2.16.4
[PATCH 6/8] RISC-V: Use mmgrab()
f1f1007644ff ("mm: add new mmgrab() helper") added a helper that we missed out on. Signed-off-by: Palmer Dabbelt --- arch/riscv/kernel/smpboot.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c index e1f6a5ad0416..5437a04babcd 100644 --- a/arch/riscv/kernel/smpboot.c +++ b/arch/riscv/kernel/smpboot.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -79,8 +80,8 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle) * the spinning harts that they can continue the boot process. */ smp_mb(); - __cpu_up_stack_pointer[cpu] = task_stack_page(tidle) + THREAD_SIZE; - __cpu_up_task_pointer[cpu] = tidle; + WRITE_ONCE(__cpu_up_stack_pointer[cpu], task_stack_page(tidle) + THREAD_SIZE); + WRITE_ONCE(__cpu_up_task_pointer[cpu], tidle); while (!cpu_online(cpu)) cpu_relax(); @@ -100,7 +101,7 @@ asmlinkage void __init smp_callin(void) struct mm_struct *mm = &init_mm; /* All kernel threads share the same mm context. */ - atomic_inc(&mm->mm_count); + mmgrab(mm); current->active_mm = mm; trap_init(); -- 2.16.4
[PATCH 7/8] RISC-V: Comment on the TLB flush in smp_callin()
This isn't readily apparent from reading the code. Signed-off-by: Palmer Dabbelt --- arch/riscv/kernel/smpboot.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c index 5437a04babcd..953bc540207d 100644 --- a/arch/riscv/kernel/smpboot.c +++ b/arch/riscv/kernel/smpboot.c @@ -107,6 +107,8 @@ asmlinkage void __init smp_callin(void) trap_init(); notify_cpu_starting(smp_processor_id()); set_cpu_online(smp_processor_id(), 1); + /* Remote TLB flushes are ignored while the CPU is offline, so emit a local +* TLB flush right now just in case. */ local_flush_tlb_all(); local_irq_enable(); preempt_disable(); -- 2.16.4
[PATCH 5/8] RISC-V: Rename im_okay_therefore_i_am to found_boot_cpu
The old name was a bit odd. Signed-off-by: Palmer Dabbelt --- arch/riscv/kernel/smpboot.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c index 5f29f8562cf6..e1f6a5ad0416 100644 --- a/arch/riscv/kernel/smpboot.c +++ b/arch/riscv/kernel/smpboot.c @@ -50,7 +50,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus) void __init setup_smp(void) { struct device_node *dn = NULL; - int hart, im_okay_therefore_i_am = 0; + int hart, found_boot_cpu = 0; while ((dn = of_find_node_by_type(dn, "cpu"))) { hart = riscv_of_processor_hartid(dn); @@ -58,13 +58,13 @@ void __init setup_smp(void) set_cpu_possible(hart, true); set_cpu_present(hart, true); if (hart == smp_processor_id()) { - BUG_ON(im_okay_therefore_i_am); - im_okay_therefore_i_am = 1; + BUG_ON(found_boot_cpu); + found_boot_cpu = 1; } } } - BUG_ON(!im_okay_therefore_i_am); + BUG_ON(!found_boot_cpu); } int __cpu_up(unsigned int cpu, struct task_struct *tidle) -- 2.16.4
[PATCH 2/8] RISC-V: Don't set cacheinfo.{physical_line_partition,attributes}
These are just hard coded in the RISC-V port, which doesn't make any sense. We should probably be setting these from device tree entries when they exist, but for now I think it's saner to just leave them all as their default values. Signed-off-by: Palmer Dabbelt --- arch/riscv/kernel/cacheinfo.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c index 0bc86e5f8f3f..cb35ffd8ec6b 100644 --- a/arch/riscv/kernel/cacheinfo.c +++ b/arch/riscv/kernel/cacheinfo.c @@ -22,13 +22,6 @@ static void ci_leaf_init(struct cacheinfo *this_leaf, { this_leaf->level = level; this_leaf->type = type; - /* not a sector cache */ - this_leaf->physical_line_partition = 1; - /* TODO: Add to DTS */ - this_leaf->attributes = - CACHE_WRITE_BACK - | CACHE_READ_ALLOCATE - | CACHE_WRITE_ALLOCATE; } static int __init_cache_level(unsigned int cpu) -- 2.16.4
Re: [PATCH] riscv: tlb: Provide definition of tlb_flush() before including tlb.h
On Fri, 24 Aug 2018 11:22:55 PDT (-0700), li...@roeck-us.net wrote: From: Will Deacon As of commit fd1102f0aade ("mm: mmu_notifier fix for tlb_end_vma"), asm-generic/tlb.h now calls tlb_flush() from a static inline function, so we need to make sure that it's declared before #including the asm-generic header in the arch header. Reported-by: Guenter Roeck Fixes: fd1102f0aade ("mm: mmu_notifier fix for tlb_end_vma") Signed-off-by: Will Deacon [groeck: Use forward declaration instead of moving inline function] Signed-off-by: Guenter Roeck --- arch/riscv/include/asm/tlb.h | 4 1 file changed, 4 insertions(+) diff --git a/arch/riscv/include/asm/tlb.h b/arch/riscv/include/asm/tlb.h index c229509288ea..439dc7072e05 100644 --- a/arch/riscv/include/asm/tlb.h +++ b/arch/riscv/include/asm/tlb.h @@ -14,6 +14,10 @@ #ifndef _ASM_RISCV_TLB_H #define _ASM_RISCV_TLB_H +struct mmu_gather; + +static void tlb_flush(struct mmu_gather *tlb); + #include static inline void tlb_flush(struct mmu_gather *tlb) Thanks! I'm going to take this into my for-linus branch which I'm still hoping to tag today, pending my email queue.
Re: [PATCH v2 01/17] asm: simd context helper API
On Fri, 24 Aug 2018 14:38:33 PDT (-0700), ja...@zx2c4.com wrote: Sometimes it's useful to amortize calls to XSAVE/XRSTOR and the related FPU/SIMD functions over a number of calls, because FPU restoration is quite expensive. This adds a simple header for carrying out this pattern: simd_context_t simd_context = simd_get(); while ((item = get_item_from_queue()) != NULL) { encrypt_item(item, simd_context); simd_context = simd_relax(simd_context); } simd_put(simd_context); The relaxation step ensures that we don't trample over preemption, and the get/put API should be a familiar paradigm in the kernel. Signed-off-by: Jason A. Donenfeld Cc: Andy Lutomirski Cc: Greg KH Cc: Samuel Neves Cc: linux-a...@vger.kernel.org --- arch/alpha/include/asm/Kbuild | 5 ++-- arch/arc/include/asm/Kbuild| 1 + arch/arm/include/asm/simd.h| 42 ++ arch/arm64/include/asm/simd.h | 37 +- arch/c6x/include/asm/Kbuild| 3 ++- arch/h8300/include/asm/Kbuild | 3 ++- arch/hexagon/include/asm/Kbuild| 1 + arch/ia64/include/asm/Kbuild | 1 + arch/m68k/include/asm/Kbuild | 1 + arch/microblaze/include/asm/Kbuild | 1 + arch/mips/include/asm/Kbuild | 1 + arch/nds32/include/asm/Kbuild | 7 ++--- arch/nios2/include/asm/Kbuild | 1 + arch/openrisc/include/asm/Kbuild | 7 ++--- arch/parisc/include/asm/Kbuild | 1 + arch/powerpc/include/asm/Kbuild| 3 ++- arch/riscv/include/asm/Kbuild | 3 ++- arch/s390/include/asm/Kbuild | 3 ++- arch/sh/include/asm/Kbuild | 1 + arch/sparc/include/asm/Kbuild | 1 + arch/um/include/asm/Kbuild | 3 ++- arch/unicore32/include/asm/Kbuild | 1 + arch/x86/include/asm/simd.h| 30 - arch/xtensa/include/asm/Kbuild | 1 + include/asm-generic/simd.h | 15 +++ include/linux/simd.h | 28 26 files changed, 180 insertions(+), 21 deletions(-) create mode 100644 arch/arm/include/asm/simd.h create mode 100644 include/linux/simd.h ... diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild index 576ffdca06ba..8d3e7aef3234 100644 --- a/arch/riscv/include/asm/Kbuild +++ b/arch/riscv/include/asm/Kbuild @@ -4,9 +4,9 @@ generic-y += checksum.h generic-y += cputime.h generic-y += device.h generic-y += div64.h -generic-y += dma.h generic-y += dma-contiguous.h generic-y += dma-mapping.h +generic-y += dma.h generic-y += emergency-restart.h generic-y += errno.h generic-y += exec.h If this is the canonical ordering and doing so makes your life easier then I'm OK taking this as a separate patch into the RISC-V tree, but if not then feel free to roll something like this up into your next patch set. @@ -45,6 +45,7 @@ generic-y += setup.h generic-y += shmbuf.h generic-y += shmparam.h generic-y += signal.h +generic-y += simd.h generic-y += socket.h generic-y += sockios.h generic-y += stat.h Either way, this looks fine for as far as the RISC-V stuff goes as it's pretty much a NOP. As long as it stays a NOP then feel free to add a Reviewed-by: Palmer Dabbelt as far as the RISC-V parts are conceded. It looks like there's a lot of other issues, though, so it's not much of a review :)
Re: Linux 4.19-rc1
On Mon, 27 Aug 2018 06:44:59 PDT (-0700), li...@roeck-us.net wrote: On Sun, Aug 26, 2018 at 02:49:14PM -0700, Linus Torvalds wrote: So two weeks have passed, and the merge window for 4.19 is over. [ ... ] Anyway, go forth and test, Build results: total: 132 pass: 129 fail: 3 Failed builds: riscv:defconfig riscv:allnoconfig sparc32:allmodconfig Qemu test results: total: 299 pass: 297 fail: 2 Failed tests: riscv:virt:defconfig:initrd riscv:virt:defconfig:virtio-blk:rootfs --- riscv: In file included from arch/riscv/include/asm/tlb.h:17:0, from arch/riscv/include/asm/pgalloc.h:19, from arch/riscv/mm/fault.c:30: include/asm-generic/tlb.h: In function 'tlb_flush_mmu_tlbonly': include/asm-generic/tlb.h:147:2: error: implicit declaration of function 'tlb_flush' Known problem, patch submitted. Thanks for the patch (as well as running the build farm)! It should be in the PR I submit today, assuming I can get through my email... --- sparc32:allmodconfig: arch/sparc/kernel/head_32.o: In function `current_pc': arch/sparc/kernel/.tmp_head_32.o:(.head.text+0x5040): relocation truncated to fit: R_SPARC_WDISP22 against `.init.text' arch/sparc/kernel/head_32.o: In function `halt_notsup': arch/sparc/kernel/.tmp_head_32.o:(.head.text+0x5100): relocation truncated to fit: R_SPARC_WDISP22 against `.init.text' arch/sparc/kernel/head_32.o: In function `leon_init': arch/sparc/kernel/.tmp_head_32.o:(.init.text+0xa4): relocation truncated to fit: R_SPARC_WDISP22 against symbol `leon_smp_cpu_startup' defined in .text section in arch/sparc/kernel/trampoline_32.o arch/sparc/kernel/process_32.o:(.fixup+0x4): relocation truncated to fit: R_SPARC_WDISP22 against `.text' arch/sparc/kernel/process_32.o:(.fixup+0xc): relocation truncated to fit: R_SPARC_WDISP22 against `.text' arch/sparc/kernel/signal_32.o:(.fixup+0x0): relocation truncated to fit: R_SPARC_WDISP22 against `.text' arch/sparc/kernel/signal_32.o:(.fixup+0x8): relocation truncated to fit: R_SPARC_WDISP22 against `.text' arch/sparc/kernel/signal_32.o:(.fixup+0x10): relocation truncated to fit: R_SPARC_WDISP22 against `.text' arch/sparc/kernel/signal_32.o:(.fixup+0x18): relocation truncated to fit: R_SPARC_WDISP22 against `.text' arch/sparc/kernel/signal_32.o:(.fixup+0x20): relocation truncated to fit: R_SPARC_WDISP22 against `.text' arch/sparc/kernel/signal_32.o:(.fixup+0x28): additional relocation overflows omitted from the output For the most part this is due to calls / short jumps between .head.text, .text, and .init.text. Probably old, now seen because the image is now too large. --- On top of that, there are various runtime warnings. sh: WARNING: CPU: 0 PID: 932 at mm/slab.c:2666 cache_alloc_refill+0x8a/0x594 Known problem. Fix was under discussion. I don't know if it was accepted. https://marc.info/?t=15330142692&r=1&w=2 https://www.spinics.net/lists/linux-sh/msg53298.html --- sparc: WARNING: CPU: 0 PID: 1 at ./include/linux/dma-mapping.h:516 esp_sbus_probe+0x408/0x6e8 WARNING: CPU: 0 PID: 1 at ./include/linux/dma-mapping.h:516 sparc_lance_probe_one+0x428/0x4f Missing initialization of coherent_dma_mask in the respective drivers. --- Each platform driver instantiated through a devicetree node now generates the following warning: esp ffd38e00: DMA mask not set It isn't a traceback so it may fly under the radar. There is nothing the drivers can do about it; the message is generated by the core before the driver probe function is called. No idea what a correct fix might be. Guenter
[PATCH] RISC-V: Mask out the F extension on systems without D
The RISC-V Linux port doesn't support systems that have the F extension but don't have the D extension -- we actually don't support systems without D either, but Alan's patch set is rectifying that soon. For now I think we can leave this in a semi-broken state and just wait for Alan's patch set to get merged for proper non-FPU support -- the patch set is starting to look good, so doing something in-between doesn't seem like it's worth the work. I don't think it's worth fretting about support for systems with F but not D for now: our glibc ABIs are IMAC and IMAFDC so they probably won't end up being popular. We can always extend this in the future. CC: Alan Kao Signed-off-by: Palmer Dabbelt --- arch/riscv/kernel/cpufeature.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 17011a870044..652d102ffa06 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -57,5 +57,12 @@ void riscv_fill_hwcap(void) for (i = 0; i < strlen(isa); ++i) elf_hwcap |= isa2hwcap[(unsigned char)(isa[i])]; + /* We don't support systems with F but without D, so mask those out +* here. */ + if ((elf_hwcap & COMPAT_HWCAP_ISA_F) && !(elf_hwcap & COMPAT_HWCAP_ISA_D)) { + pr_info("This kernel does not support systems with F but not D"); + elf_hwcap &= ~COMPAT_HWCAP_ISA_F; + } + pr_info("elf_hwcap is 0x%lx", elf_hwcap); } -- 2.16.4
Re: [PATCH] RISC-V: Mask out the F extension on systems without D
On Tue, 28 Aug 2018 00:10:32 PDT (-0700), alan...@andestech.com wrote: Hi Palmer, On Mon, Aug 27, 2018 at 03:03:52PM -0700, Palmer Dabbelt wrote: The RISC-V Linux port doesn't support systems that have the F extension but don't have the D extension -- we actually don't support systems without D either, but Alan's patch set is rectifying that soon. For now I think we can leave this in a semi-broken state and just wait for Alan's patch set to get merged for proper non-FPU support -- the patch set is starting to look good, so doing something in-between doesn't seem like it's worth the work. I don't think it's worth fretting about support for systems with F but not D for now: our glibc ABIs are IMAC and IMAFDC so they probably won't end up being popular. We can always extend this in the future. CC: Alan Kao Signed-off-by: Palmer Dabbelt --- arch/riscv/kernel/cpufeature.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 17011a870044..652d102ffa06 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -57,5 +57,12 @@ void riscv_fill_hwcap(void) for (i = 0; i < strlen(isa); ++i) elf_hwcap |= isa2hwcap[(unsigned char)(isa[i])]; + /* We don't support systems with F but without D, so mask those out +* here. */ + if ((elf_hwcap & COMPAT_HWCAP_ISA_F) && !(elf_hwcap & COMPAT_HWCAP_ISA_D)) { + pr_info("This kernel does not support systems with F but not D"); + elf_hwcap &= ~COMPAT_HWCAP_ISA_F; + } + The commit message does address the problem and this patch does provide checks and helpful information to users, but I wonder if we really need this patch, for two reasons: * Just as you mentioned, current glibc ABI does not support such a thing as IMAFC, so probably no one has had trouble with this. To be honest, I suppose that anybody (RISC-V enthusiasts or vendors) who really need F-only support in kernel should get themself involved in the development by sending patches to improve. * There are corner cases to let a F-only machine to pass the check in this patch. For instance, a vendor decides to name her extension ISA as doom, and supports single-precision FP only, so her ISA string would be IMAFCXdoom. The variable elf_hwcap is calculated at the loop in line 57,58, the 'd' from Xdoom would bypass the check, while the underlying machine does not support double-precision FP. Ah, yes, that makes sense. I'd go the other way here and just be strict about parsing the ISA string: it's defined to be listed in a particular order, so we should really only be accepting legal ISA strings. I'll submit a second patch to fix this behavior. pr_info("elf_hwcap is 0x%lx", elf_hwcap); } -- 2.16.4 I don't know if the reasons make sense to you, but anyway that's all I would like to say about this patch. Alan
Re: [PATCH] riscv: Drop setup_initrd
On Thu, 09 Aug 2018 21:11:40 PDT (-0700), li...@roeck-us.net wrote: setup_initrd() does not appear to serve a practical purpose other than preventing qemu boots with "-initrd" parameter, so let's drop it. Signed-off-by: Guenter Roeck --- arch/riscv/kernel/setup.c | 39 --- 1 file changed, 39 deletions(-) diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index 2e56af3281f8..579f58a42974 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -82,41 +82,6 @@ EXPORT_SYMBOL(empty_zero_page); /* The lucky hart to first increment this variable will boot the other cores */ atomic_t hart_lottery; -#ifdef CONFIG_BLK_DEV_INITRD -static void __init setup_initrd(void) -{ - extern char __initramfs_start[]; - extern unsigned long __initramfs_size; - unsigned long size; - - if (__initramfs_size > 0) { - initrd_start = (unsigned long)(&__initramfs_start); - initrd_end = initrd_start + __initramfs_size; - } - - if (initrd_start >= initrd_end) { - printk(KERN_INFO "initrd not found or empty"); - goto disable; - } - if (__pa(initrd_end) > PFN_PHYS(max_low_pfn)) { - printk(KERN_ERR "initrd extends beyond end of memory"); - goto disable; - } - - size = initrd_end - initrd_start; - memblock_reserve(__pa(initrd_start), size); - initrd_below_start_ok = 1; - - printk(KERN_INFO "Initial ramdisk at: 0x%p (%lu bytes)\n", - (void *)(initrd_start), size); - return; -disable: - pr_cont(" - disabling initrd\n"); - initrd_start = 0; - initrd_end = 0; -} -#endif /* CONFIG_BLK_DEV_INITRD */ - pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss; pgd_t trampoline_pg_dir[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE); @@ -195,10 +160,6 @@ static void __init setup_bootmem(void) set_max_mapnr(PFN_DOWN(mem_size)); max_low_pfn = pfn_base + PFN_DOWN(mem_size); -#ifdef CONFIG_BLK_DEV_INITRD - setup_initrd(); -#endif /* CONFIG_BLK_DEV_INITRD */ - early_init_fdt_reserve_self(); early_init_fdt_scan_reserved_mem(); memblock_allow_resize(); With this patch I can't boot in QEMU. It might be something with my setup, though. I'm testing with a unified BBL+vmlinux, and using QEMU 3.0.0-rc3 (I should probably update that one, but I don't remember anything going by). Do you have time to take a look? Here's my QEMU commandline ./riscv64-softmmu/qemu-system-riscv64-nographic-machine virt-smp 4-m 2G -kernel /home/palmer/work/riscv/riscv-pk/build/bbl -object rng-random,filename=/dev/urandom,id=rng0-device virtio-rng-device,rng=rng0-append "console=ttyS0 ro root=/dev/vda"-device virtio-blk-device,drive=hd0 -drive file=stage4-disk.img,format=raw,id=hd0-device virtio-net-device,netdev=usernet -netdev user,id=usernet,hostfwd=tcp::1-:22 I just tagged the PR I plan to submit for RC2 tomorrow, so at least it should be easy for everyone to get on the same page.
Re: [PATCH] riscv: Drop setup_initrd
On Tue, 28 Aug 2018 14:59:59 PDT (-0700), li...@roeck-us.net wrote: On Tue, Aug 28, 2018 at 11:46:09PM +0200, Andreas Schwab wrote: On Aug 28 2018, Guenter Roeck wrote: > On Tue, Aug 28, 2018 at 01:10:20PM -0700, Palmer Dabbelt wrote: >> On Thu, 09 Aug 2018 21:11:40 PDT (-0700), li...@roeck-us.net wrote: >> >setup_initrd() does not appear to serve a practical purpose other than >> >preventing qemu boots with "-initrd" parameter, so let's drop it. >> > >> >Signed-off-by: Guenter Roeck >> >--- >> > arch/riscv/kernel/setup.c | 39 --- >> > 1 file changed, 39 deletions(-) >> > >> >diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c >> >index 2e56af3281f8..579f58a42974 100644 >> >--- a/arch/riscv/kernel/setup.c >> >+++ b/arch/riscv/kernel/setup.c >> >@@ -82,41 +82,6 @@ EXPORT_SYMBOL(empty_zero_page); >> > /* The lucky hart to first increment this variable will boot the other cores */ >> > atomic_t hart_lottery; >> > >> >-#ifdef CONFIG_BLK_DEV_INITRD >> >-static void __init setup_initrd(void) >> >-{ >> >- extern char __initramfs_start[]; >> >- extern unsigned long __initramfs_size; >> >- unsigned long size; >> >- >> >- if (__initramfs_size > 0) { >> >- initrd_start = (unsigned long)(&__initramfs_start); >> >- initrd_end = initrd_start + __initramfs_size; >> >- } > > The underlying problem is probably that __initramfs_size == 512 even > if there is no embedded initrd. Result is that initrd_start and initrd_end > are always overwritten, even if provided and even if there is no embedded > initrd. Result is that initrd_start and initrd_end are always overwritten, > and -initrd from the qemu command line is always ignored. > > A less invasive fix than mine would be > > - if (__initramfs_size > 0) { > + if (__initramfs_size > 0 && !initrd_start) { > > Any chance you can test that with your setup ? You should just delete the last four lines above. They serve no purpose. You mean the entire if() statement plus the variable declarations ? That works for me, for both embedded initrd and initrd specified with -initrd option, but we still need someone to test if it works for Palmer's use case, ie with vmlinux (and possibly initrd) embedded in bbl. This still boots my Fedora images diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index db20dc630e7e..aee603123030 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -85,15 +85,8 @@ atomic_t hart_lottery; #ifdef CONFIG_BLK_DEV_INITRD static void __init setup_initrd(void) { -extern char __initramfs_start[]; -extern unsigned long __initramfs_size; unsigned long size; - if (__initramfs_size > 0) { -initrd_start = (unsigned long)(&__initramfs_start); -initrd_end = initrd_start + __initramfs_size; -} - if (initrd_start >= initrd_end) { printk(KERN_INFO "initrd not found or empty"); goto disable; but I have not tried an integrated initramfs.
Re: [PATCH] riscv: Drop setup_initrd
On Tue, 28 Aug 2018 15:12:38 PDT (-0700), li...@roeck-us.net wrote: On Tue, Aug 28, 2018 at 03:03:00PM -0700, Palmer Dabbelt wrote: On Tue, 28 Aug 2018 14:59:59 PDT (-0700), li...@roeck-us.net wrote: >On Tue, Aug 28, 2018 at 11:46:09PM +0200, Andreas Schwab wrote: >>On Aug 28 2018, Guenter Roeck wrote: >> >>> On Tue, Aug 28, 2018 at 01:10:20PM -0700, Palmer Dabbelt wrote: >>>> On Thu, 09 Aug 2018 21:11:40 PDT (-0700), li...@roeck-us.net wrote: >>>> >setup_initrd() does not appear to serve a practical purpose other than >>>> >preventing qemu boots with "-initrd" parameter, so let's drop it. >>>> > >>>> >Signed-off-by: Guenter Roeck >>>> >--- >>>> > arch/riscv/kernel/setup.c | 39 --- >>>> > 1 file changed, 39 deletions(-) >>>> > >>>> >diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c >>>> >index 2e56af3281f8..579f58a42974 100644 >>>> >--- a/arch/riscv/kernel/setup.c >>>> >+++ b/arch/riscv/kernel/setup.c >>>> >@@ -82,41 +82,6 @@ EXPORT_SYMBOL(empty_zero_page); >>>> > /* The lucky hart to first increment this variable will boot the other cores */ >>>> > atomic_t hart_lottery; >>>> > >>>> >-#ifdef CONFIG_BLK_DEV_INITRD >>>> >-static void __init setup_initrd(void) >>>> >-{ >>>> >- extern char __initramfs_start[]; >>>> >- extern unsigned long __initramfs_size; >>>> >- unsigned long size; >>>> >- >>>> >- if (__initramfs_size > 0) { >>>> >- initrd_start = (unsigned long)(&__initramfs_start); >>>> >- initrd_end = initrd_start + __initramfs_size; >>>> >- } >>> >>> The underlying problem is probably that __initramfs_size == 512 even >>> if there is no embedded initrd. Result is that initrd_start and initrd_end >>> are always overwritten, even if provided and even if there is no embedded >>> initrd. Result is that initrd_start and initrd_end are always overwritten, >>> and -initrd from the qemu command line is always ignored. >>> >>> A less invasive fix than mine would be >>> >>> - if (__initramfs_size > 0) { >>> + if (__initramfs_size > 0 && !initrd_start) { >>> >>> Any chance you can test that with your setup ? >> >>You should just delete the last four lines above. They serve no purpose. >> > >You mean the entire if() statement plus the variable declarations ? > >That works for me, for both embedded initrd and initrd specified with >-initrd option, but we still need someone to test if it works for >Palmer's use case, ie with vmlinux (and possibly initrd) embedded in >bbl. This still boots my Fedora images diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index db20dc630e7e..aee603123030 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -85,15 +85,8 @@ atomic_t hart_lottery; #ifdef CONFIG_BLK_DEV_INITRD static void __init setup_initrd(void) { -extern char __initramfs_start[]; -extern unsigned long __initramfs_size; unsigned long size; -if (__initramfs_size > 0) { -initrd_start = (unsigned long)(&__initramfs_start); -initrd_end = initrd_start + __initramfs_size; -} - if (initrd_start >= initrd_end) { printk(KERN_INFO "initrd not found or empty"); goto disable; but I have not tried an integrated initramfs. I tested the above both with external initrd and with integrated initramfs; both work for me. Should I resend my patch, using the above, or do you want to create a patch yourself ? You should send one, then it'll go through my regular pre-PR testing flow to make sure it works on my end. I just never trust these inline patches to be exact, even if it's unlikely there's any confusion on a patch this simple (at least, mechanically simple -- I'm afraid I still don't understand why the logic is incorrect).
Re: [PATCH] riscv: Drop setup_initrd
On Tue, 28 Aug 2018 17:36:28 PDT (-0700), li...@roeck-us.net wrote: On 08/28/2018 05:09 PM, Palmer Dabbelt wrote: On Tue, 28 Aug 2018 15:12:38 PDT (-0700), li...@roeck-us.net wrote: On Tue, Aug 28, 2018 at 03:03:00PM -0700, Palmer Dabbelt wrote: On Tue, 28 Aug 2018 14:59:59 PDT (-0700), li...@roeck-us.net wrote: >On Tue, Aug 28, 2018 at 11:46:09PM +0200, Andreas Schwab wrote: >>On Aug 28 2018, Guenter Roeck wrote: >> >>> On Tue, Aug 28, 2018 at 01:10:20PM -0700, Palmer Dabbelt wrote: >>>> On Thu, 09 Aug 2018 21:11:40 PDT (-0700), li...@roeck-us.net wrote: >>>> >setup_initrd() does not appear to serve a practical purpose other than >>>> >preventing qemu boots with "-initrd" parameter, so let's drop it. >>>> > >>>> >Signed-off-by: Guenter Roeck >>>> >--- >>>> > arch/riscv/kernel/setup.c | 39 --- >>>> > 1 file changed, 39 deletions(-) >>>> > >>>> >diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c >>>> >index 2e56af3281f8..579f58a42974 100644 >>>> >--- a/arch/riscv/kernel/setup.c >>>> >+++ b/arch/riscv/kernel/setup.c >>>> >@@ -82,41 +82,6 @@ EXPORT_SYMBOL(empty_zero_page); >>>> > /* The lucky hart to first increment this variable will boot the other cores */ >>>> > atomic_t hart_lottery; >>>> > >>>> >-#ifdef CONFIG_BLK_DEV_INITRD >>>> >-static void __init setup_initrd(void) >>>> >-{ >>>> >- extern char __initramfs_start[]; >>>> >- extern unsigned long __initramfs_size; >>>> >- unsigned long size; >>>> >- >>>> >- if (__initramfs_size > 0) { >>>> >- initrd_start = (unsigned long)(&__initramfs_start); >>>> >- initrd_end = initrd_start + __initramfs_size; >>>> >- } >>> >>> The underlying problem is probably that __initramfs_size == 512 even >>> if there is no embedded initrd. Result is that initrd_start and initrd_end >>> are always overwritten, even if provided and even if there is no embedded >>> initrd. Result is that initrd_start and initrd_end are always overwritten, >>> and -initrd from the qemu command line is always ignored. >>> >>> A less invasive fix than mine would be >>> >>> - if (__initramfs_size > 0) { >>> + if (__initramfs_size > 0 && !initrd_start) { >>> >>> Any chance you can test that with your setup ? >> >>You should just delete the last four lines above. They serve no purpose. >> > >You mean the entire if() statement plus the variable declarations ? > >That works for me, for both embedded initrd and initrd specified with >-initrd option, but we still need someone to test if it works for >Palmer's use case, ie with vmlinux (and possibly initrd) embedded in >bbl. This still boots my Fedora images diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index db20dc630e7e..aee603123030 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -85,15 +85,8 @@ atomic_t hart_lottery; #ifdef CONFIG_BLK_DEV_INITRD static void __init setup_initrd(void) { - extern char __initramfs_start[]; - extern unsigned long __initramfs_size; unsigned long size; - if (__initramfs_size > 0) { - initrd_start = (unsigned long)(&__initramfs_start); - initrd_end = initrd_start + __initramfs_size; - } - if (initrd_start >= initrd_end) { printk(KERN_INFO "initrd not found or empty"); goto disable; but I have not tried an integrated initramfs. I tested the above both with external initrd and with integrated initramfs; both work for me. Should I resend my patch, using the above, or do you want to create a patch yourself ? You should send one, then it'll go through my regular pre-PR testing flow to make sure it works on my end. I just never trust these inline patches to be exact, even if it's unlikely there's any confusion on a patch this simple (at least, mechanically simple -- I'm afraid I still don't understand why the logic is incorrect). Done. There is no need to override initrd_start and initrd_end; populate_rootfs() uses __initramfs_start and __initramfs_size directly when loading a built-in initramfs. initrd_start and initrd_end, on the other side, are for an external initrd, loaded separately (and initialized in __early_init_dt_declare_initrd()). OK, that makes sense. I dropped your patch onto for-next, assuming nothing's gone wrong it should go in next week. It boots a Fedora rootfs for me. Thanks!
[GIT PULL] RISC-V Fixes and Cleanups for 4.19-rc2
The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3: Linux 4.19-rc1 (2018-08-26 14:11:59 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/palmer/riscv-linux.git tags/riscv-for-linus-4.19-rc2 for you to fetch changes up to 47d80a68f10d3290204a12f7836a9a8190dfc327: RISC-V: Use a less ugly workaround for unused variable warnings (2018-08-28 12:58:36 -0700) RISC-V Fixes and Cleanups for 4.19-rc2 This tag contains a handful of patches that filtered their way in during the merge window but just didn't make the deadline. It includes: * Additional documentation in the riscv,cpu-intc device tree binding that resulted from some feedback I missed in the original patch set. * A build fix that provides the definition of tlb_flush() before including tlb.h, which fixes a RISC-V build regression introduced during this merge window. * A cosmetic cleanup to sys_riscv_flush_icache(). Palmer Dabbelt (2): dt-bindings: riscv,cpu-intc: Cleanups from a missed review RISC-V: Use a less ugly workaround for unused variable warnings Will Deacon (1): riscv: tlb: Provide definition of tlb_flush() before including tlb.h .../bindings/interrupt-controller/riscv,cpu-intc.txt | 14 +++--- arch/riscv/include/asm/tlb.h | 4 arch/riscv/kernel/sys_riscv.c | 15 +-- 3 files changed, 16 insertions(+), 17 deletions(-)
Re: [PATCH 03/11] dt-bindings: interrupt-controller: RISC-V PLIC documentation
On Wed, 08 Aug 2018 07:16:14 PDT (-0700), robh...@kernel.org wrote: On Tue, Aug 7, 2018 at 8:17 PM Palmer Dabbelt wrote: On Mon, 06 Aug 2018 13:59:48 PDT (-0700), robh...@kernel.org wrote: > On Thu, Aug 2, 2018 at 4:08 PM Atish Patra wrote: >> >> On 8/2/18 4:50 AM, Christoph Hellwig wrote: >> > From: Palmer Dabbelt >> > >> > This patch adds documentation for the platform-level interrupt >> > controller (PLIC) found in all RISC-V systems. This interrupt >> > controller routes interrupts from all the devices in the system to each >> > hart-local interrupt controller. >> > >> > Note: the DTS bindings for the PLIC aren't set in stone yet, as we might >> > want to change how we're specifying holes in the hart list. >> > >> > Signed-off-by: Palmer Dabbelt >> > [hch: various fixes and updates] >> > Signed-off-by: Christoph Hellwig >> > --- >> > .../interrupt-controller/sifive,plic0.txt | 57 +++ >> > 1 file changed, 57 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt >> > >> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt >> > new file mode 100644 >> > index ..c756cd208a93 >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt >> > @@ -0,0 +1,57 @@ >> > +SiFive Platform-Level Interrupt Controller (PLIC) >> > +- >> > + >> > +SiFive SOCs include an implementation of the Platform-Level Interrupt Controller >> > +(PLIC) high-level specification in the RISC-V Privileged Architecture >> > +specification. The PLIC connects all external interrupts in the system to all >> > +hart contexts in the system, via the external interrupt source in each hart. >> > + >> > +A hart context is a privilege mode in a hardware execution thread. For example, >> > +in an 4 core system with 2-way SMT, you have 8 harts and probably at least two >> > +privilege modes per hart; machine mode and supervisor mode. >> > + >> > +Each interrupt can be enabled on per-context basis. Any context can claim >> > +a pending enabled interrupt and then release it once it has been handled. >> > + >> > +Each interrupt has a configurable priority. Higher priority interrupts are >> > +serviced first. Each context can specify a priority threshold. Interrupts >> > +with priority below this threshold will not cause the PLIC to raise its >> > +interrupt line leading to the context. >> > + >> > +While the PLIC supports both edge-triggered and level-triggered interrupts, >> > +interrupt handlers are oblivious to this distinction and therefore it is not >> > +specified in the PLIC device-tree binding. >> > + >> > +While the RISC-V ISA doesn't specify a memory layout for the PLIC, the >> > +"sifive,plic0" device is a concrete implementation of the PLIC that contains a >> > +specific memory layout, which is documented in chapter 8 of the SiFive U5 >> > +Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>. >> > + >> > +Required properties: >> > +- compatible : "sifive,plic0" I think there was a thread bouncing around somewhere where decided to pick the official name of the compatible string to be "sifive,plic-1.0". The idea here is that the PLIC is compatible across all of SiFive's current implementations, but there's some limitations in the memory map that will probably cause us to spin a version 2 at some point so we want major version number. The minor number is just in case we find some errata in the PLIC. Is 1.0 an actual version number corresponding to an exact, revision controlled version of the IP or just something you made up? Looks like the latter to me and I'm not a fan of s/w folks making up version numbers for h/w. Standard naming convention is ,- unless you have good reason to deviate (IP for FPGAs where version numbers are exposed to customers is one example). The hardware versioning scheme calls it "riscv,plic0", which is what we were originally using. This PLIC isn't actually defined as a RISC-V spec, so we wanted to change it to a "sifive,*" name instead. Since we were going to change the compat string anyway, I thought we'd just introduce a minor number to be safe. Since "0.0" is an aw
Re: [PATCH 6/8] dt-bindings: interrupt-controller: RISC-V PLIC documentation
On Wed, 08 Aug 2018 09:15:58 PDT (-0700), robh...@kernel.org wrote: On Wed, Aug 8, 2018 at 8:59 AM Christoph Hellwig wrote: On Wed, Aug 08, 2018 at 08:29:50AM -0600, Rob Herring wrote: > Version numbers on the individual patches would be nice... We've never done these in the subsystems I'm involved with. Too much clutter in the subject lines for information that is easily deductable. Unfortunately not in Gmail which doesn't thread properly. But patchwork also provides the version tag which I use to sort my reviews. > > +Example: > > + > > + plic: interrupt-controller@c00 { > > + #address-cells = <0>; > > + #interrupt-cells = <1>; > > + compatible = "riscv,plic0"; > > + interrupt-controller; > > + interrupts-extended = < > > + &cpu0-intc 11 > > + &cpu1-intc 11 &cpu1-intc 9 > > + &cpu2-intc 11 &cpu2-intc 9 > > + &cpu3-intc 11 &cpu3-intc 9 > > + &cpu4-intc 11 &cpu4-intc 9>; > > I'm confused why this is still here if you are dropping the cpu intc binding? We need some parent that identifies the core (hart in RISC-V terminology). The way the code now works is that it just walks up the parent chain until it finds a CPU node, so it either accepts the legacy intc node inbetween, or it accepts the cpu node directly as the intc node is pointless. I guess for the documentation we should instead just point to the "riscv" cpu nodes instead? That's not valid and dtc will tell you that. 'interrupts' (via interrupt-parent) or 'interrupts-extended' has to point to an 'interrupt-controller' node. I guess you could make the cpu nodes interrupt-controllers. That's a bit strange, but I can't think of a reason why that wouldn't work. Just because the cpu-intc is not made to be an irqchip in the kernel doesn't mean it can't still be represented as an interrupt-controller in DT. It shouldn't be designed just around how some OS happens to implement things. FWIW, I like this approach. There is an interrupt widget in the hardware, so having the device tree represent it seems like a good idea. > I also noticed the cpu binding refers to "riscv,cpu-intc" as well. > That needs to be fixed too if there's a change. Only in the examples. I'd be fine with dropping them, but let's keep that separate from the interrupt support. You need to sort out how this is all tied together and works because right now you are supporting 2 ways and one is undocumented and the other is invalid. Changing things later is only going to be more painful. Rob
Re: [PATCH 03/11] dt-bindings: interrupt-controller: RISC-V PLIC documentation
On Wed, 08 Aug 2018 16:32:07 PDT (-0700), robh...@kernel.org wrote: On Wed, Aug 8, 2018 at 1:38 PM Palmer Dabbelt wrote: On Wed, 08 Aug 2018 07:16:14 PDT (-0700), robh...@kernel.org wrote: > On Tue, Aug 7, 2018 at 8:17 PM Palmer Dabbelt wrote: >> >> On Mon, 06 Aug 2018 13:59:48 PDT (-0700), robh...@kernel.org wrote: >> > On Thu, Aug 2, 2018 at 4:08 PM Atish Patra wrote: >> >> >> >> On 8/2/18 4:50 AM, Christoph Hellwig wrote: >> >> > From: Palmer Dabbelt >> >> > >> >> > This patch adds documentation for the platform-level interrupt >> >> > controller (PLIC) found in all RISC-V systems. This interrupt >> >> > controller routes interrupts from all the devices in the system to each >> >> > hart-local interrupt controller. >> >> > >> >> > Note: the DTS bindings for the PLIC aren't set in stone yet, as we might >> >> > want to change how we're specifying holes in the hart list. >> >> > >> >> > Signed-off-by: Palmer Dabbelt >> >> > [hch: various fixes and updates] >> >> > Signed-off-by: Christoph Hellwig >> >> > --- >> >> > .../interrupt-controller/sifive,plic0.txt | 57 +++ >> >> > 1 file changed, 57 insertions(+) >> >> > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt >> >> > >> >> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt >> >> > new file mode 100644 >> >> > index ..c756cd208a93 >> >> > --- /dev/null >> >> > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt >> >> > @@ -0,0 +1,57 @@ >> >> > +SiFive Platform-Level Interrupt Controller (PLIC) >> >> > +- >> >> > + >> >> > +SiFive SOCs include an implementation of the Platform-Level Interrupt Controller >> >> > +(PLIC) high-level specification in the RISC-V Privileged Architecture >> >> > +specification. The PLIC connects all external interrupts in the system to all >> >> > +hart contexts in the system, via the external interrupt source in each hart. >> >> > + >> >> > +A hart context is a privilege mode in a hardware execution thread. For example, >> >> > +in an 4 core system with 2-way SMT, you have 8 harts and probably at least two >> >> > +privilege modes per hart; machine mode and supervisor mode. >> >> > + >> >> > +Each interrupt can be enabled on per-context basis. Any context can claim >> >> > +a pending enabled interrupt and then release it once it has been handled. >> >> > + >> >> > +Each interrupt has a configurable priority. Higher priority interrupts are >> >> > +serviced first. Each context can specify a priority threshold. Interrupts >> >> > +with priority below this threshold will not cause the PLIC to raise its >> >> > +interrupt line leading to the context. >> >> > + >> >> > +While the PLIC supports both edge-triggered and level-triggered interrupts, >> >> > +interrupt handlers are oblivious to this distinction and therefore it is not >> >> > +specified in the PLIC device-tree binding. >> >> > + >> >> > +While the RISC-V ISA doesn't specify a memory layout for the PLIC, the >> >> > +"sifive,plic0" device is a concrete implementation of the PLIC that contains a >> >> > +specific memory layout, which is documented in chapter 8 of the SiFive U5 >> >> > +Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>. >> >> > + >> >> > +Required properties: >> >> > +- compatible : "sifive,plic0" >> >> I think there was a thread bouncing around somewhere where decided to pick the >> official name of the compatible string to be "sifive,plic-1.0". The idea here >> is that the PLIC is compatible across all of SiFive's current implementations, >> but there's some limitations in the memory map that will probably cause us to >> spin a version 2 at some point so we want major version number. The minor >> number is just in case we find some errata in the PLIC. > > Is 1.0 an actua
[PATCH v2 2/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
This file is expected to be included multiple times in the same file in order to allow the __SYSCALL macro to generate system call tables. With a global include guard we end up missing __NR_riscv_flush_icache in the syscall table, which results in icache flushes that escape the vDSO call to not actually do anything. The fix is to move to per-#define include guards, which allows the system call tables to actually be populated. Thanks to Macrus Comstedt for finding and fixing the bug! I also went ahead and fixed the SPDX header to use a //-style comment, which I've been told is the canonical way to do it. Cc: Marcus Comstedt Signed-off-by: Palmer Dabbelt --- arch/riscv/include/asm/unistd.h| 5 + arch/riscv/include/uapi/asm/syscalls.h | 15 +-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h index 080fb28061de..0caea01d5cca 100644 --- a/arch/riscv/include/asm/unistd.h +++ b/arch/riscv/include/asm/unistd.h @@ -11,6 +11,11 @@ * GNU General Public License for more details. */ +/* + * There is explicitly no include guard here because this file is expected to + * be included multiple times. See uapi/asm/syscalls.h for more info. + */ + #define __ARCH_WANT_SYS_CLONE #include #include diff --git a/arch/riscv/include/uapi/asm/syscalls.h b/arch/riscv/include/uapi/asm/syscalls.h index 818655b0d535..690beb002d1d 100644 --- a/arch/riscv/include/uapi/asm/syscalls.h +++ b/arch/riscv/include/uapi/asm/syscalls.h @@ -1,10 +1,13 @@ -/* SPDX-License-Identifier: GPL-2.0 */ +// SPDX-License-Identifier: GPL-2.0 /* - * Copyright (C) 2017 SiFive + * Copyright (C) 2017-2018 SiFive */ -#ifndef _ASM__UAPI__SYSCALLS_H -#define _ASM__UAPI__SYSCALLS_H +/* + * There is explicitly no include guard here because this file is expected to + * be included multiple times in order to define the syscall macros via + * __SYSCALL. + */ /* * Allows the instruction cache to be flushed from userspace. Despite RISC-V @@ -20,7 +23,7 @@ * caller. We don't currently do anything with the address range, that's just * in there for forwards compatibility. */ +#ifndef __NR_riscv_flush_icache #define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15) -__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) - #endif +__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) -- 2.16.4
[PATCH v2 0/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.
It turns out that we weren't actually hooking sys_riscv_flush_icache into the syscall table, which results in any flush_icache() call that escapes the vDSO to silently do nothing. Changes since v1: * sys_riscv_flush_icache is now defined even when SMP=n, which allows this patch set to build against SMP=n and SMP=y.
[PATCH v2 1/2] RISC-V: Define sys_riscv_flush_icache when SMP=n
This would be necessary to make non-SMP builds work, but there is another error in the implementation of our syscall linkage that actually just causes sys_riscv_flush_icache to never build. I've build tested this on allnoconfig and allnoconfig+SMP=y, as well as defconfig like normal. CC: Christoph Hellwig CC: Guenter Roeck In-Reply-To: <20180809055830.ga17...@infradead.org> In-Reply-To: <20180809132612.ga31...@roeck-us.net> Signed-off-by: Palmer Dabbelt --- arch/riscv/include/asm/vdso.h | 2 -- arch/riscv/kernel/sys_riscv.c | 10 -- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/arch/riscv/include/asm/vdso.h b/arch/riscv/include/asm/vdso.h index 541544d64c33..ec6180a4b55d 100644 --- a/arch/riscv/include/asm/vdso.h +++ b/arch/riscv/include/asm/vdso.h @@ -38,8 +38,6 @@ struct vdso_data { (void __user *)((unsigned long)(base) + __vdso_##name); \ }) -#ifdef CONFIG_SMP asmlinkage long sys_riscv_flush_icache(uintptr_t, uintptr_t, uintptr_t); -#endif #endif /* _ASM_RISCV_VDSO_H */ diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c index f7181ed8aafc..180da8d4e14a 100644 --- a/arch/riscv/kernel/sys_riscv.c +++ b/arch/riscv/kernel/sys_riscv.c @@ -48,7 +48,6 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, } #endif /* !CONFIG_64BIT */ -#ifdef CONFIG_SMP /* * Allows the instruction cache to be flushed from userspace. Despite RISC-V * having a direct 'fence.i' instruction available to userspace (which we @@ -66,15 +65,22 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, SYSCALL_DEFINE3(riscv_flush_icache, uintptr_t, start, uintptr_t, end, uintptr_t, flags) { +#ifdef CONFIG_SMP struct mm_struct *mm = current->mm; bool local = (flags & SYS_RISCV_FLUSH_ICACHE_LOCAL) != 0; +#endif /* Check the reserved flags. */ if (unlikely(flags & ~SYS_RISCV_FLUSH_ICACHE_ALL)) return -EINVAL; + /* +* Without CONFIG_SMP flush_icache_mm is a NOP, which generates unused +* variable warnings all over this function. +*/ +#ifdef CONFIG_SMP flush_icache_mm(mm, local); +#endif return 0; } -#endif -- 2.16.4
Re: [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
On Thu, 09 Aug 2018 06:26:12 PDT (-0700), li...@roeck-us.net wrote: On Fri, Aug 03, 2018 at 12:53:44PM -0700, Palmer Dabbelt wrote: This file is expected to be included multiple times in the same file in order to allow the __SYSCALL macro to generate system call tables. With a global include guard we end up missing __NR_riscv_flush_icache in the syscall table, which results in icache flushes that escape the vDSO call to not actually do anything. The fix is to move to per-#define include guards, which allows the system call tables to actually be populated. Thanks to Macrus Comstedt for finding and fixing the bug! I also went ahead and fixed the SPDX header to use a //-style comment, which I've been told is the canonical way to do it. Cc: Marcus Comstedt Signed-off-by: Palmer Dabbelt Fails to build riscv:allnoconfig. CC arch/riscv/kernel/syscall_table.o ./arch/riscv/include/uapi/asm/syscalls.h:29:36: error: ‘sys_riscv_flush_icache’ undeclared here (not in a function); did you mean ‘__NR_riscv_flush_icache’? Thanks. I added you to another patch set.
Re: [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
On Wed, 08 Aug 2018 22:58:30 PDT (-0700), Christoph Hellwig wrote: This actually seems to break the compilation for me in for-next: hch@carbon:~/work/linux$ make ARCH=riscv CALLscripts/checksyscalls.sh :1335:2: warning: #warning syscall rseq not implemented [-Wcpp] CHK include/generated/compile.h CC arch/riscv/kernel/syscall_table.o ./arch/riscv/include/uapi/asm/syscalls.h:29:36: error: 'sys_riscv_flush_icache' undeclared here (not in a function); did you mean '__NR_riscv_flush_icache'? __SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) ^ arch/riscv/kernel/syscall_table.c:21:37: note: in definition of macro '__SYSCALL' #define __SYSCALL(nr, call) [nr] = (call), ^~~~ make[1]: *** [scripts/Makefile.build:318: arch/riscv/kernel/syscall_table.o] Error 1 make: *** [Makefile:1029: arch/riscv/kernel] Error 2 Reverting it for now, but I guess this might hit others too.. Thanks. I dropped this from for-next and just sent out a replacement.
RISC-V: Don't use a global include guard for uapi/asm/syscalls.
It turns out that we weren't actually hooking sys_riscv_flush_icache into the syscall table, which results in any flush_icache() call that escapes the vDSO to silently do nothing. Changes since v2: * sys_riscv_flush_icache actually flushes the icache when SMP=n. Thanks to Andrew for pointing out the I screwed it up! Changes since v1: * sys_riscv_flush_icache is now defined even when SMP=n, which allows this patch set to build against SMP=n and SMP=y.
[PATCH v3 2/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
This file is expected to be included multiple times in the same file in order to allow the __SYSCALL macro to generate system call tables. With a global include guard we end up missing __NR_riscv_flush_icache in the syscall table, which results in icache flushes that escape the vDSO call to not actually do anything. The fix is to move to per-#define include guards, which allows the system call tables to actually be populated. Thanks to Macrus Comstedt for finding and fixing the bug! I also went ahead and fixed the SPDX header to use a //-style comment, which I've been told is the canonical way to do it. Cc: Marcus Comstedt Signed-off-by: Palmer Dabbelt --- arch/riscv/include/asm/unistd.h| 5 + arch/riscv/include/uapi/asm/syscalls.h | 15 +-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h index 080fb28061de..0caea01d5cca 100644 --- a/arch/riscv/include/asm/unistd.h +++ b/arch/riscv/include/asm/unistd.h @@ -11,6 +11,11 @@ * GNU General Public License for more details. */ +/* + * There is explicitly no include guard here because this file is expected to + * be included multiple times. See uapi/asm/syscalls.h for more info. + */ + #define __ARCH_WANT_SYS_CLONE #include #include diff --git a/arch/riscv/include/uapi/asm/syscalls.h b/arch/riscv/include/uapi/asm/syscalls.h index 818655b0d535..690beb002d1d 100644 --- a/arch/riscv/include/uapi/asm/syscalls.h +++ b/arch/riscv/include/uapi/asm/syscalls.h @@ -1,10 +1,13 @@ -/* SPDX-License-Identifier: GPL-2.0 */ +// SPDX-License-Identifier: GPL-2.0 /* - * Copyright (C) 2017 SiFive + * Copyright (C) 2017-2018 SiFive */ -#ifndef _ASM__UAPI__SYSCALLS_H -#define _ASM__UAPI__SYSCALLS_H +/* + * There is explicitly no include guard here because this file is expected to + * be included multiple times in order to define the syscall macros via + * __SYSCALL. + */ /* * Allows the instruction cache to be flushed from userspace. Despite RISC-V @@ -20,7 +23,7 @@ * caller. We don't currently do anything with the address range, that's just * in there for forwards compatibility. */ +#ifndef __NR_riscv_flush_icache #define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15) -__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) - #endif +__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) -- 2.16.4
[PATCH v3 1/2] RISC-V: Define sys_riscv_flush_icache when SMP=n
This would be necessary to make non-SMP builds work, but there is another error in the implementation of our syscall linkage that actually just causes sys_riscv_flush_icache to never build. I've build tested this on allnoconfig and allnoconfig+SMP=y, as well as defconfig like normal. CC: Christoph Hellwig CC: Guenter Roeck In-Reply-To: <20180809055830.ga17...@infradead.org> In-Reply-To: <20180809132612.ga31...@roeck-us.net> Signed-off-by: Palmer Dabbelt --- arch/riscv/include/asm/vdso.h | 2 -- arch/riscv/kernel/sys_riscv.c | 12 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/arch/riscv/include/asm/vdso.h b/arch/riscv/include/asm/vdso.h index 541544d64c33..ec6180a4b55d 100644 --- a/arch/riscv/include/asm/vdso.h +++ b/arch/riscv/include/asm/vdso.h @@ -38,8 +38,6 @@ struct vdso_data { (void __user *)((unsigned long)(base) + __vdso_##name); \ }) -#ifdef CONFIG_SMP asmlinkage long sys_riscv_flush_icache(uintptr_t, uintptr_t, uintptr_t); -#endif #endif /* _ASM_RISCV_VDSO_H */ diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c index f7181ed8aafc..568026ccf6e8 100644 --- a/arch/riscv/kernel/sys_riscv.c +++ b/arch/riscv/kernel/sys_riscv.c @@ -48,7 +48,6 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, } #endif /* !CONFIG_64BIT */ -#ifdef CONFIG_SMP /* * Allows the instruction cache to be flushed from userspace. Despite RISC-V * having a direct 'fence.i' instruction available to userspace (which we @@ -66,15 +65,24 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, SYSCALL_DEFINE3(riscv_flush_icache, uintptr_t, start, uintptr_t, end, uintptr_t, flags) { +#ifdef CONFIG_SMP struct mm_struct *mm = current->mm; bool local = (flags & SYS_RISCV_FLUSH_ICACHE_LOCAL) != 0; +#endif /* Check the reserved flags. */ if (unlikely(flags & ~SYS_RISCV_FLUSH_ICACHE_ALL)) return -EINVAL; + /* +* Without CONFIG_SMP flush_icache_mm is a just a flush_icache_all(), +* which generates unused variable warnings all over this function. +*/ +#ifdef CONFIG_SMP flush_icache_mm(mm, local); +#else + flush_icache_all(); +#endif return 0; } -#endif -- 2.16.4
Re: [PATCH v2 2/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
On Thu, 09 Aug 2018 14:24:22 PDT (-0700), li...@roeck-us.net wrote: On Thu, Aug 09, 2018 at 01:25:24PM -0700, Palmer Dabbelt wrote: This file is expected to be included multiple times in the same file in order to allow the __SYSCALL macro to generate system call tables. With a global include guard we end up missing __NR_riscv_flush_icache in the syscall table, which results in icache flushes that escape the vDSO call to not actually do anything. The fix is to move to per-#define include guards, which allows the system call tables to actually be populated. Thanks to Macrus Comstedt for finding and fixing the bug! I also went ahead and fixed the SPDX header to use a //-style comment, which I've been told is the canonical way to do it. Cc: Marcus Comstedt Signed-off-by: Palmer Dabbelt [Compile-]Tested-by: Guenter Roeck on top of linux-next after reverting the version of the patch there. I also tried to run the resulting image (defconfig) with qemu (built from https://github.com/riscv/riscv-qemu.git), but that still doesn't work. I assume there are still some patches missing ? Do you have the PLIC patches? They'll be necessary to make this all work, and there's a v4 out now that when combined with for-next should get you to userspace. https://lore.kernel.org/lkml/20180809075602.989-1-...@lst.de/T/#u Also, what is your methodology? I follow https://wiki.qemu.org/Documentation/Platforms/RISCV and could could natively compile and run hello world with an earlier version of Christoph's patch set, which is really only cosmetically different than the v4. I use qemu's master branch as well, which when I tried was exactly 3.0.0-rc3.
Re: FW: [PATCH v2] RISC-V: Add the directive for alignment of stvec's value
On Thu, 09 Aug 2018 17:37:39 PDT (-0700), zong...@gmail.com wrote: Subject: [PATCH v2] RISC-V: Add the directive for alignment of stvec's value The stvec's value must be 4 byte alignment by specification definition. These directives avoid to stvec be set the non-alignment value. Signed-off-by: Zong Li --- arch/riscv/kernel/head.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S index 3b6293f..11066d5 100644 --- a/arch/riscv/kernel/head.S +++ b/arch/riscv/kernel/head.S @@ -94,6 +94,7 @@ relocate: or a0, a0, a1 sfence.vma csrw sptbr, a0 +.align 2 1: /* Set trap vector to spin forever to help debug */ la a0, .Lsecondary_park @@ -143,6 +144,7 @@ relocate: tail smp_callin #endif +.align 2 .Lsecondary_park: /* We lack SMP support or have too many harts, so park this hart */ wfi Thanks, this got lost in the shuffle somewhere. It's in for-next now.
Re: [PATCH v2 2/2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
On Thu, 09 Aug 2018 19:40:55 PDT (-0700), li...@roeck-us.net wrote: On 08/09/2018 06:03 PM, Palmer Dabbelt wrote: On Thu, 09 Aug 2018 14:24:22 PDT (-0700), li...@roeck-us.net wrote: On Thu, Aug 09, 2018 at 01:25:24PM -0700, Palmer Dabbelt wrote: This file is expected to be included multiple times in the same file in order to allow the __SYSCALL macro to generate system call tables. With a global include guard we end up missing __NR_riscv_flush_icache in the syscall table, which results in icache flushes that escape the vDSO call to not actually do anything. The fix is to move to per-#define include guards, which allows the system call tables to actually be populated. Thanks to Macrus Comstedt for finding and fixing the bug! I also went ahead and fixed the SPDX header to use a //-style comment, which I've been told is the canonical way to do it. Cc: Marcus Comstedt Signed-off-by: Palmer Dabbelt [Compile-]Tested-by: Guenter Roeck on top of linux-next after reverting the version of the patch there. I also tried to run the resulting image (defconfig) with qemu (built from https://github.com/riscv/riscv-qemu.git), but that still doesn't work. I assume there are still some patches missing ? Do you have the PLIC patches? They'll be necessary to make this all work, and there's a v4 out now that when combined with for-next should get you to userspace. https://lore.kernel.org/lkml/20180809075602.989-1-...@lst.de/T/#u Yes, after merging that branch on top of linux-next I can boot into Linux. If I add my "riscv: Drop setup_initrd" patch as well, I can boot using initrd, otherwise I have to use virtio-blk-device. Awesome! If you patch isn't on for-next then I must have missed it, do you mind sending me a pointer? I can't find any references in my email. Also, what is your methodology? I follow https://wiki.qemu.org/Documentation/Platforms/RISCV and could could natively compile and run hello world with an earlier version of Christoph's patch set, which is really only cosmetically different than the v4. I use qemu's master branch as well, which when I tried was exactly 3.0.0-rc3. That doesn't work for me, possibly because I don't specify a bbl image with -kernel but vmlinux (using -bios for the bbl image). I use branch qemu-for-upstream of https://github.com/riscv/riscv-qemu.git. Guenter
Re: [PATCH v3 1/2] RISC-V: Define sys_riscv_flush_icache when SMP=n
On Fri, 10 Aug 2018 01:38:04 PDT (-0700), Christoph Hellwig wrote: On Thu, Aug 09, 2018 at 03:19:51PM -0700, Palmer Dabbelt wrote: This would be necessary to make non-SMP builds work, but there is another error in the implementation of our syscall linkage that actually just causes sys_riscv_flush_icache to never build. I've build tested this on allnoconfig and allnoconfig+SMP=y, as well as defconfig like normal. Would't it make sense to use COND_SYSCALL to stub out the syscall for !SMP builds? I'm not sure. We can implement the syscall fine in !SMP, it's just that the vDSO is expected to always eat these calls because in non-SMP mode you can do a global fence.i by just doing a local fence.i (there's only one hart). The original rationale behind not having the syscall in non-SMP mode was to limit the user ABI, but on looking again that seems like it's just a bit of extra complexity that doesn't help anything. It's already been demonstrated that nothing is checking the error because it's been silently slipping past userspace for six months, so the extra complexity seems like it'll just cause someone else to have to chase the bug in the future. But I'm really OK either way. Is there a precedent for what to do here?
Re: [PATCH 03/11] dt-bindings: interrupt-controller: RISC-V PLIC documentation
On Fri, 10 Aug 2018 09:57:03 PDT (-0700), robh...@kernel.org wrote: On Thu, Aug 9, 2018 at 12:29 AM Palmer Dabbelt wrote: On Wed, 08 Aug 2018 16:32:07 PDT (-0700), robh...@kernel.org wrote: > On Wed, Aug 8, 2018 at 1:38 PM Palmer Dabbelt wrote: >> >> On Wed, 08 Aug 2018 07:16:14 PDT (-0700), robh...@kernel.org wrote: >> > On Tue, Aug 7, 2018 at 8:17 PM Palmer Dabbelt wrote: >> >> >> >> On Mon, 06 Aug 2018 13:59:48 PDT (-0700), robh...@kernel.org wrote: >> >> > On Thu, Aug 2, 2018 at 4:08 PM Atish Patra wrote: >> >> >> >> >> >> On 8/2/18 4:50 AM, Christoph Hellwig wrote: >> >> >> > From: Palmer Dabbelt >> >> >> > >> >> >> > This patch adds documentation for the platform-level interrupt >> >> >> > controller (PLIC) found in all RISC-V systems. This interrupt >> >> >> > controller routes interrupts from all the devices in the system to each >> >> >> > hart-local interrupt controller. >> >> >> > >> >> >> > Note: the DTS bindings for the PLIC aren't set in stone yet, as we might >> >> >> > want to change how we're specifying holes in the hart list. >> >> >> > >> >> >> > Signed-off-by: Palmer Dabbelt >> >> >> > [hch: various fixes and updates] >> >> >> > Signed-off-by: Christoph Hellwig >> >> >> > --- >> >> >> > .../interrupt-controller/sifive,plic0.txt | 57 +++ >> >> >> > 1 file changed, 57 insertions(+) >> >> >> > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt >> >> >> > >> >> >> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt >> >> >> > new file mode 100644 >> >> >> > index ..c756cd208a93 >> >> >> > --- /dev/null >> >> >> > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt >> >> >> > @@ -0,0 +1,57 @@ >> >> >> > +SiFive Platform-Level Interrupt Controller (PLIC) >> >> >> > +- >> >> >> > + >> >> >> > +SiFive SOCs include an implementation of the Platform-Level Interrupt Controller >> >> >> > +(PLIC) high-level specification in the RISC-V Privileged Architecture >> >> >> > +specification. The PLIC connects all external interrupts in the system to all >> >> >> > +hart contexts in the system, via the external interrupt source in each hart. >> >> >> > + >> >> >> > +A hart context is a privilege mode in a hardware execution thread. For example, >> >> >> > +in an 4 core system with 2-way SMT, you have 8 harts and probably at least two >> >> >> > +privilege modes per hart; machine mode and supervisor mode. >> >> >> > + >> >> >> > +Each interrupt can be enabled on per-context basis. Any context can claim >> >> >> > +a pending enabled interrupt and then release it once it has been handled. >> >> >> > + >> >> >> > +Each interrupt has a configurable priority. Higher priority interrupts are >> >> >> > +serviced first. Each context can specify a priority threshold. Interrupts >> >> >> > +with priority below this threshold will not cause the PLIC to raise its >> >> >> > +interrupt line leading to the context. >> >> >> > + >> >> >> > +While the PLIC supports both edge-triggered and level-triggered interrupts, >> >> >> > +interrupt handlers are oblivious to this distinction and therefore it is not >> >> >> > +specified in the PLIC device-tree binding. >> >> >> > + >> >> >> > +While the RISC-V ISA doesn't specify a memory layout for the PLIC, the >> >> >> > +"sifive,plic0" device is a concrete implementation of the PLIC that contains a >> >> >> > +specific memory layout, which is documented in chapter 8 of the SiFive U5 >> >> >> > +Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>. >> >>
Re: [RFC] RISC-V: Fix !CONFIG_SMP compilation error
On Fri, 10 Aug 2018 11:31:08 PDT (-0700), atish.pa...@wdc.com wrote: On 8/6/18 4:17 PM, Atish Patra wrote: Enabling both CONFIG_PERF_EVENTS without !CONFIG_SMP generates following compilation error. arch/riscv/include/asm/perf_event.h:80:2: error: expected specifier-qualifier-list before 'irqreturn_t' irqreturn_t (*handle_irq)(int irq_num, void *dev); ^~~ Include interrupt.h in proper place to avoid compilation error. Signed-off-by: Atish Patra --- arch/riscv/include/asm/perf_event.h | 1 + arch/riscv/kernel/perf_event.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/riscv/include/asm/perf_event.h b/arch/riscv/include/asm/perf_event.h index 0e638a0c..aefbfaa6 100644 --- a/arch/riscv/include/asm/perf_event.h +++ b/arch/riscv/include/asm/perf_event.h @@ -10,6 +10,7 @@ #include #include +#include #define RISCV_BASE_COUNTERS 2 diff --git a/arch/riscv/kernel/perf_event.c b/arch/riscv/kernel/perf_event.c index b0e10c4e..a243fae1 100644 --- a/arch/riscv/kernel/perf_event.c +++ b/arch/riscv/kernel/perf_event.c @@ -27,7 +27,6 @@ #include #include #include -#include #include #include #include ping ? Sorry about this, it got dropped. This looks fine and fixes the bug, so I've gone and put it on for-next. Thanks!
Re: [PATCH v3 1/2] RISC-V: Define sys_riscv_flush_icache when SMP=n
On Fri, 10 Aug 2018 11:47:15 PDT (-0700), li...@roeck-us.net wrote: On Fri, Aug 10, 2018 at 11:27:37AM -0700, Palmer Dabbelt wrote: On Fri, 10 Aug 2018 01:38:04 PDT (-0700), Christoph Hellwig wrote: >On Thu, Aug 09, 2018 at 03:19:51PM -0700, Palmer Dabbelt wrote: >>This would be necessary to make non-SMP builds work, but there is >>another error in the implementation of our syscall linkage that actually >>just causes sys_riscv_flush_icache to never build. I've build tested >>this on allnoconfig and allnoconfig+SMP=y, as well as defconfig like >>normal. > >Would't it make sense to use COND_SYSCALL to stub out the syscall >for !SMP builds? I'm not sure. We can implement the syscall fine in !SMP, it's just that the vDSO is expected to always eat these calls because in non-SMP mode you can do a global fence.i by just doing a local fence.i (there's only one hart). The original rationale behind not having the syscall in non-SMP mode was to limit the user ABI, but on looking again that seems like it's just a bit of extra complexity that doesn't help anything. It's already been demonstrated Doesn't this mean that some userspace code will only run if the kernel was compiled for SMP ? I always thought that was unacceptable. Well, the officially sanctioned way to obtain this functionality is via a vDSO call. On non-SMP systems it will never make the system call. As a result we thought we'd keep it out of the ABI, but after looking again it seems yucky to do so. Here's the vDSO entry, for reference: ENTRY(__vdso_flush_icache) .cfi_startproc #ifdef CONFIG_SMP li a7, __NR_riscv_flush_icache ecall #else fence.i li a0, 0 #endif ret .cfi_endproc ENDPROC(__vdso_flush_icache) Note that glibc has a fallback to make the system call if it can't find the vDSO entry, but then doesn't have a secondary fallback to emit a local fence.i if the system call doesn't exist. It seems easier to fix the kernel to always provide the syscall and just call it a bug.