Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1677060 > unrolled thread

RISC-V Linux Port v3

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RISC-V Linux Port v3 Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-28 21:00 +0200
    [PATCH 6/9] RISC-V: Device, timer, IRQs, and the SBI Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-28 21:00 +0200
      Re: [PATCH 6/9] RISC-V: Device, timer, IRQs, and the SBI Tobias Klauser <tklauser@distanz.ch> - 2017-06-29 10:40 +0200
        Re: [PATCH 6/9] RISC-V: Device, timer, IRQs, and the SBI Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-30 01:00 +0200
          Re: [PATCH 6/9] RISC-V: Device, timer, IRQs, and the SBI Tobias Klauser <tklauser@distanz.ch> - 2017-06-30 10:00 +0200
    [PATCH 4/9] RISC-V: ELF and module implementation Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-28 21:00 +0200
    [PATCH 7/9] RISC-V: Paging and MMU Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-28 21:10 +0200
      Re: [PATCH 7/9] RISC-V: Paging and MMU James Hogan <james.hogan@imgtec.com> - 2017-06-29 01:20 +0200
        Re: [PATCH 7/9] RISC-V: Paging and MMU Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-30 00:20 +0200
    [PATCH 9/9] RISC-V: Build Infastructure Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-28 21:10 +0200
      Re: [PATCH 9/9] RISC-V: Build Infastructure Karsten Merker <merker@debian.org> - 2017-06-28 23:10 +0200
        Re: [PATCH 9/9] RISC-V: Build Infastructure Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-28 23:20 +0200
      Re: [PATCH 9/9] RISC-V: Build Infastructure James Hogan <james.hogan@imgtec.com> - 2017-06-28 23:30 +0200
        Re: [PATCH 9/9] RISC-V: Build Infastructure Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-29 18:40 +0200
      Re: [PATCH 9/9] RISC-V: Build Infastructure James Hogan <james.hogan@imgtec.com> - 2017-06-29 01:00 +0200
        Re: [PATCH 9/9] RISC-V: Build Infastructure Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-30 00:20 +0200
    Re: [PATCH 8/9] RISC-V: User-facing API Thomas Gleixner <tglx@linutronix.de> - 2017-06-29 00:00 +0200
    Re: [PATCH 8/9] RISC-V: User-facing API Thomas Gleixner <tglx@linutronix.de> - 2017-06-29 00:00 +0200
      Re: [PATCH 8/9] RISC-V: User-facing API Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-29 19:30 +0200
    Re: [PATCH 8/9] RISC-V: User-facing API James Hogan <james.hogan@imgtec.com> - 2017-06-29 00:50 +0200
      Re: [PATCH 8/9] RISC-V: User-facing API Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-29 23:50 +0200
    Re: [PATCH 5/9] RISC-V: Task implementation James Hogan <james.hogan@imgtec.com> - 2017-06-29 01:40 +0200
      Re: [PATCH 5/9] RISC-V: Task implementation Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-30 01:00 +0200
    Re: [PATCH 5/9] RISC-V: Task implementation Tobias Klauser <tklauser@distanz.ch> - 2017-06-29 10:30 +0200
      Re: [PATCH 5/9] RISC-V: Task implementation Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-30 01:00 +0200
    Re: [PATCH 1/9] RISC-V: Init and Halt Code Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-29 11:50 +0200
      Re: [PATCH 1/9] RISC-V: Init and Halt Code Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-30 01:00 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1678221 — Re: [PATCH 8/9] RISC-V: User-facing API

On Wed, 28 Jun 2017 15:42:37 PDT (-0700), james.hogan@imgtec.com wrote:
> Hi Palmer,
>
> On Wed, Jun 28, 2017 at 11:55:37AM -0700, Palmer Dabbelt wrote:
>> diff --git a/arch/riscv/include/asm/syscalls.h b/arch/riscv/include/asm/syscalls.h
>> new file mode 100644
>> index 000000000000..d85267c4f7ea
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/syscalls.h
>> @@ -0,0 +1,25 @@
> ...
>> +/* kernel/sys_riscv.c */
>> +asmlinkage long sys_sysriscv(unsigned long, unsigned long,
>> +	unsigned long, unsigned long);
>
> You suggested in the cover letter this wasn't muxed any longer, maybe
> you should have a prototype for each of the cmpxchg syscalls instead?

Sorry, I just missed that.  I'll fix it for the v4

  diff --git a/arch/riscv/include/asm/syscalls.h b/arch/riscv/include/asm/syscalls.h
  index d85267c4f7ea..6490274fbb76 100644
  --- a/arch/riscv/include/asm/syscalls.h
  +++ b/arch/riscv/include/asm/syscalls.h
  @@ -19,7 +19,7 @@
   #include <asm-generic/syscalls.h>

   /* kernel/sys_riscv.c */
  -asmlinkage long sys_sysriscv(unsigned long, unsigned long,
  -       unsigned long, unsigned long);
  +asmlinkage long sys_sysriscv_cmpxchg32(u32 __user * ptr, u32 new, u32 old);
  +asmlinkage long sys_sysriscv_cmpxchg64(u64 __user * ptr, u64 new, u64 old);

   #endif /* _ASM_RISCV_SYSCALLS_H */

>> diff --git a/arch/riscv/include/uapi/asm/ptrace.h b/arch/riscv/include/uapi/asm/ptrace.h
>> new file mode 100644
>> index 000000000000..01aee1654eae
>> --- /dev/null
>> +++ b/arch/riscv/include/uapi/asm/ptrace.h
> ...
>> +struct __riscv_f_ext_state {
>> +	__u32 f[32];
>> +	__u32 fcsr;
>> +};
>> +
>> +struct __riscv_d_ext_state {
>> +	__u64 f[32];
>> +	__u32 fcsr;
>> +};
>> +
>> +struct __riscv_q_ext_state {
>> +	__u64 f[64] __attribute__((aligned(16)));
>> +	__u32 fcsr;
>> +	/* Reserved for expansion of sigcontext structure.  Currently zeroed
>> +	 * upon signal, and must be zero upon sigreturn.  */
>> +	__u32 reserved[3];
>> +};
>> +
>> +union __riscv_fp_state {
>> +	struct __riscv_f_ext_state f;
>> +	struct __riscv_d_ext_state d;
>> +	struct __riscv_q_ext_state q;
>> +};
>
> Out of interest, how does one tell which fp format is in use?

We might need another tag here -- I'll talk to Andrew (who did the glibc side
of this) and make sure we can handle something like running F user code on a D
kernel.

>> diff --git a/arch/riscv/include/uapi/asm/ucontext.h b/arch/riscv/include/uapi/asm/ucontext.h
>> new file mode 100644
>> index 000000000000..52eff9febcfd
>> --- /dev/null
>> +++ b/arch/riscv/include/uapi/asm/ucontext.h
> ...
>> +struct ucontext {
>> +	unsigned long	  uc_flags;
>> +	struct ucontext	 *uc_link;
>> +	stack_t		  uc_stack;
>> +	sigset_t	  uc_sigmask;
>> +	/* glibc uses a 1024-bit sigset_t */
>> +	__u8		  __unused[1024 / 8 - sizeof(sigset_t)];
>> +	/* last for future expansion */
>> +	struct sigcontext uc_mcontext;
>> +};
>
> Any particular reason not to use the asm-generic ucontext?

In the generic ucontext, 'uc_sigmask' is at the end of the structure so it can
be expanded.  Since we want our mcontext to be expandable as well, we
pre-allocate some expandable space for sigmask and then put mcontext at the
end.

We stole this idea from arm64.

>> diff --git a/arch/riscv/include/uapi/asm/unistd.h b/arch/riscv/include/uapi/asm/unistd.h
>> new file mode 100644
>> index 000000000000..7e3909ac3c18
>> --- /dev/null
>> +++ b/arch/riscv/include/uapi/asm/unistd.h
> ...
>> +/* FIXME: This exists for now in order to maintain compatibility with our
>> + * pre-upstream glibc, and will be removed for our real Linux submission.
>> + */
>> +#define __ARCH_WANT_RENAMEAT
>> +
>
> Don't forget ;-)
>
> Have you seen the patches floating around for dropping
> getrlimit/setrlimit (in favour of prlimit64) and fstatat64/fstat64 (in
> favour of statx)? I guess its no big deal.

Yes, but we're trying to make this glibc release so we decided to hold off on
them.  If we can't make it then we might reconsider, but they seem like fairly
small issues.

>> +#include <asm-generic/unistd.h>
>> +
>> +/*
>> + * These system calls add support for AMOs on RISC-V systems without support
>> + * for the A extension.
>> + */
>> +#define __NR_sysriscv_cmpxchg32		(__NR_arch_specific_syscall + 0)
>> +#define __NR_sysriscv_cmpxchg64		(__NR_arch_specific_syscall + 1)
>
> I think you need the magic __SYSCALL invocations here like in
> include/uapi/asm/unistd.h, otherwise they won't get included in your
> syscall table.

OK, I've added those.

  diff --git a/arch/riscv/include/uapi/asm/unistd.h b/arch/riscv/include/uapi/asm/unistd.h
  index 7e3909ac3c18..3cdb32912ac7 100644
  --- a/arch/riscv/include/uapi/asm/unistd.h
  +++ b/arch/riscv/include/uapi/asm/unistd.h
  @@ -23,4 +23,6 @@
    * for the A extension.
    */
   #define __NR_sysriscv_cmpxchg32                (__NR_arch_specific_syscall + 0)
  +__SYSCALL(__NR_sysriscv_cmpxchg32, sys_sysriscv_cmpxchg32)
   #define __NR_sysriscv_cmpxchg64                (__NR_arch_specific_syscall + 1)
  +__SYSCALL(__NR_sysriscv_cmpxchg64, sys_sysriscv_cmpxchg64)

>> diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
>> new file mode 100644
>> index 000000000000..69b3b2d10664
>> --- /dev/null
>> +++ b/arch/riscv/kernel/ptrace.c
> ...
>> +enum riscv_regset {
>> +	REGSET_X,
>> +};
>> +
>> +/*
>> + * Get registers from task and ready the result for userspace.
>> + */
>> +static char *getregs(struct task_struct *child, struct pt_regs *uregs)
>> +{
>> +	*uregs = *task_pt_regs(child);
>> +	return (char *)uregs;
>> +}
>> +
>> +/* Put registers back to task. */
>> +static void putregs(struct task_struct *child, struct pt_regs *uregs)
>> +{
>> +	struct pt_regs *regs = task_pt_regs(child);
>> +	*regs = *uregs;
>> +}
>> +
>> +static int riscv_gpr_get(struct task_struct *target,
>> +			 const struct user_regset *regset,
>> +			 unsigned int pos, unsigned int count,
>> +			 void *kbuf, void __user *ubuf)
>> +{
>> +	struct pt_regs regs;
>> +
>> +	getregs(target, &regs);
>> +
>> +	return user_regset_copyout(&pos, &count, &kbuf, &ubuf, &regs, 0,
>> +				   sizeof(regs));
>
> Shouldn't this be limited to sizeof(struct user_regs_struct)?
>
> Why not copy straight out of task_pt_regs(target) instead of bouncing
> via the stack?

IIRC this code used to be more complicated as it supported the two different
ptrace register APIs.  There's no reason to have this function now, so I've
just pulled into the only caller.

>> +}
>> +
>> +static int riscv_gpr_set(struct task_struct *target,
>> +			 const struct user_regset *regset,
>> +			 unsigned int pos, unsigned int count,
>> +			 const void *kbuf, const void __user *ubuf)
>> +{
>> +	int ret;
>> +	struct pt_regs regs;
>> +
>> +	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &regs, 0,
>> +				 sizeof(regs));
>
> likewise.
>
> In fact if userland supplies insufficient data then this looks
> vulnerable to a kernel stack data leak, since regs will remain partially
> uninitialised and then get written to the target regs where it can be
> read back again.
>
> If you're going to bounce via the stack I think you need to fully
> initialise before using user_regset_copyin, or you could just copy
> directly into task_pt_regs(target) for now since, at least for the
> current internal struct pt_regs, the begining of pt_regs appears to
> match user_regs_struct.
>
>> +	if (ret)
>> +		return ret;
>> +
>> +	putregs(target, &regs);
>
> Similarly this needs to be careful not to overwrite the supervisor
> registers with whatever was on kernel stack (assuming only partially
> copied as suggested above)?
>
>> +
>> +	return 0;
>> +}
>> +
>> +
>> +static const struct user_regset riscv_user_regset[] = {
>> +	[REGSET_X] = {
>> +		.core_note_type = NT_PRSTATUS,
>> +		.n = ELF_NGREG,
>> +		.size = sizeof(elf_greg_t),
>> +		.align = sizeof(elf_greg_t),
>> +		.get = &riscv_gpr_get,
>> +		.set = &riscv_gpr_set,
>> +	},
>
> Will the FP registers get exposed at some point as well?
>
>> diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
>> new file mode 100644
>> index 000000000000..ab699efe636e
>> --- /dev/null
>> +++ b/arch/riscv/kernel/sys_riscv.c
> ...
>> +SYSCALL_DEFINE3(sysriscv_cmpxchg32, unsigned long, arg1, unsigned long, arg2,
>> +		unsigned long, arg3)
>> +{
>> +	unsigned long flags;
>> +	unsigned long prev;
>
> should that be unsigned int? Else on 64-bit half of it could be left
> uninitialised.
>
>> +	unsigned int *ptr;
>
> should that be tagged with __user?
>
>> +	unsigned int err;
>> +
>> +	ptr = (unsigned int *)arg1;
>
> I presume you'll need to cast to __user __force to keep sparse happy
> here.

This should be fixed, I was just lazy when converting from the multiplexed
syscall version.

>
>> +	if (!access_ok(VERIFY_WRITE, ptr, sizeof(unsigned int)))
>> +		return -EFAULT;
>> +
>> +	preempt_disable();
>> +	raw_local_irq_save(flags);
>> +	err = __get_user(prev, ptr);
>> +	if (likely(!err && prev == arg2))
>> +		err = __put_user(arg3, ptr);
>> +	raw_local_irq_restore(flags);
>> +	preempt_enable();
>
> Are user accesses safe from atomic context? What if it needs paging in?
>
> You could disable page faults but then I think you'd have to handle the
> EFAULT again outside of atomic context to try getting it paged in, and
> then retry in atomic context. Or perhaps there's a cleaner way that
> doesn't come to mind late at night.
>
> I'm not sure OTOH whether copy on write (i.e. affecting the __put_user()
> but not the __get_user() would be problematic. I suppose as long as it
> can safely allocate a page it should be fine... Should be possible to
> test using madvise(MADV_DONTNEED) (which I think makes pages use the
> zero page with copy-on-write).
>
> Also if this is going to be included on SMP kernels (where I gather
> proper atomics are available), does it need an SMP safe version too
> which uses proper atomics?

On 64-bit machines with the A extension (which is required for SMP) then that's
the right thing to do -- we're actually doing it in the VDSO right now, but
there's no reason not to do it in the syscall as well.

On 32-bit machines, I think it's still not safe as we don't have a 64-bit CAS
even with the A extension.  I think the best thing to do is actually to
disallow the 64-bit CAS on 32-bit machines -- we could disallow this on just
SMP machines, but I think it's saner do disallow it everywhere so we don't end
up with binaries that won't run on SMP kernels.

I'll try to figure out if userspace can work without it, but I think it should
be OK as we don't have double-word CAS on 64-bit.

>> +
>> +	return unlikely(err) ? err : prev;
>> +}
>> +
>> +SYSCALL_DEFINE3(sysriscv_cmpxchg64, unsigned long, arg1, unsigned long, arg2,
>> +		unsigned long, arg3)
>> +{
>> +	unsigned long flags;
>> +	unsigned long prev;
>> +	unsigned int *ptr;
>
> should that be unsigned long __user *?
>
>> +	unsigned int err;
>> +
>> +	ptr = (unsigned int *)arg1;
>> +	if (!access_ok(VERIFY_WRITE, ptr, sizeof(unsigned long)))
>> +		return -EFAULT;
>> +
>> +	preempt_disable();
>> +	raw_local_irq_save(flags);
>> +	err = __get_user(prev, ptr);
>> +	if (likely(!err && prev == arg2))
>> +		err = __put_user(arg3, ptr);
>> +	raw_local_irq_restore(flags);
>> +	preempt_enable();
>
> Likewise to other comments above.
>
> This doesn't look much different to sysriscv_cmpxchg32 on 32-bit. Is it
> meant to be excluded from 32-bit kernels? If so definition of the __NR_
> constant and the __SYSCALL magic in uapi/asm/unistd.h should I presume
> be conditional on the ABI.

Sorry, that was just a copy-and-paste error.  This is intended to actually be a
64-bit CAS on 32-bit machines -- though maybe that was a bad idea.

[toc] | [prev] | [next] | [standalone]

#1677277 — Re: [PATCH 5/9] RISC-V: Task implementation

[Multipart message — attachments visible in raw view] — view raw

On Wed, Jun 28, 2017 at 11:55:34AM -0700, Palmer Dabbelt wrote:
> diff --git a/arch/riscv/include/asm/kprobes.h b/arch/riscv/include/asm/kprobes.h
> new file mode 100644
> index 000000000000..1190de7a0f74
> --- /dev/null
> +++ b/arch/riscv/include/asm/kprobes.h
> @@ -0,0 +1,22 @@
...
> +#ifdef CONFIG_KPROBES
> +#error "RISC-V doesn't skpport CONFIG_KPROBES"
> +#endif

I'm wondering where your fallback definition of e.g. NOKPROBE_SYMBOL
comes from then.

Could you just use the asm-generic one?

> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
> new file mode 100644
> index 000000000000..b13d3ea3bf79
> --- /dev/null
> +++ b/arch/riscv/kernel/process.c
> @@ -0,0 +1,131 @@
...
> +void show_regs(struct pt_regs *regs)
> +{
> +	show_regs_print_info(KERN_DEFAULT);
> +
> +	printk(KERN_CONT "sepc: " REG_FMT " ra : " REG_FMT " sp : " REG_FMT "\n",
> +		regs->sepc, regs->ra, regs->sp);

I've noticed inconsistent use of pr_* and printk(KERN_* in this
patchset. Maybe now would be the best time to switch everything to pr_*.

> +	/* Reset FPU context
> +	 *	frm: round to nearest, ties to even (IEEE default)
> +	 *	fflags: accrued exceptions cleared
> +	 */

Similarly lots of multiline comments which don't follow the standard
style in Documentation/process/coding-style.rst. Maybe now is the best
time to convert if you're going to.

Cheers
James

[toc] | [prev] | [next] | [standalone]

#1678265 — Re: [PATCH 5/9] RISC-V: Task implementation

On Wed, 28 Jun 2017 16:32:55 PDT (-0700), james.hogan@imgtec.com wrote:
> On Wed, Jun 28, 2017 at 11:55:34AM -0700, Palmer Dabbelt wrote:
>> diff --git a/arch/riscv/include/asm/kprobes.h b/arch/riscv/include/asm/kprobes.h
>> new file mode 100644
>> index 000000000000..1190de7a0f74
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/kprobes.h
>> @@ -0,0 +1,22 @@
> ...
>> +#ifdef CONFIG_KPROBES
>> +#error "RISC-V doesn't skpport CONFIG_KPROBES"
>> +#endif
>
> I'm wondering where your fallback definition of e.g. NOKPROBE_SYMBOL
> comes from then.
>
> Could you just use the asm-generic one?

I believe so.

>> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
>> new file mode 100644
>> index 000000000000..b13d3ea3bf79
>> --- /dev/null

> ...
>> +void show_regs(struct pt_regs *regs)
>> +{
>> +	show_regs_print_info(KERN_DEFAULT);
>> +
>> +	printk(KERN_CONT "sepc: " REG_FMT " ra : " REG_FMT " sp : " REG_FMT "\n",
>> +		regs->sepc, regs->ra, regs->sp);
>
> I've noticed inconsistent use of pr_* and printk(KERN_* in this
> patchset. Maybe now would be the best time to switch everything to pr_*.

I went through and fixed them all.

>> +	/* Reset FPU context
>> +	 *	frm: round to nearest, ties to even (IEEE default)
>> +	 *	fflags: accrued exceptions cleared
>> +	 */
>
> Similarly lots of multiline comments which don't follow the standard
> style in Documentation/process/coding-style.rst. Maybe now is the best
> time to convert if you're going to.

Someone else found one and suggested this, it's on my TODO list.

Thanks!

[toc] | [prev] | [next] | [standalone]

#1677519 — Re: [PATCH 5/9] RISC-V: Task implementation

On 2017-06-28 at 20:55:34 +0200, Palmer Dabbelt <palmer@dabbelt.com> wrote:
[...]
> diff --git a/arch/riscv/include/asm/kprobes.h b/arch/riscv/include/asm/kprobes.h
> new file mode 100644
> index 000000000000..1190de7a0f74
> --- /dev/null
> +++ b/arch/riscv/include/asm/kprobes.h
> @@ -0,0 +1,22 @@
> +/*
> + * Copyright (C) 2017 SiFive
> + *
> + *   This program is free software; you can redistribute it and/or
> + *   modify it under the terms of the GNU General Public License
> + *   as published by the Free Software Foundation, version 2.
> + *
> + *   This program is distributed in the hope that it will be useful,
> + *   but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *   GNU General Public License for more details.
> + */
> +
> +
> +#ifndef ASM_RISCV_KPROBES_H
> +#define ASM_RISCV_KPROBES_H
> +
> +#ifdef CONFIG_KPROBES
> +#error "RISC-V doesn't skpport CONFIG_KPROBES"

Typo: s/skpport/support/

> +#endif
> +
> +#endif
> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> new file mode 100644
> index 000000000000..65aa014db9b4
> --- /dev/null
> +++ b/arch/riscv/include/asm/processor.h
> @@ -0,0 +1,102 @@
> +/*
> + * Copyright (C) 2012 Regents of the University of California
> + *
> + *   This program is free software; you can redistribute it and/or
> + *   modify it under the terms of the GNU General Public License
> + *   as published by the Free Software Foundation, version 2.
> + *
> + *   This program is distributed in the hope that it will be useful,
> + *   but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *   GNU General Public License for more details.
> + */
> +
> +#ifndef _ASM_RISCV_PROCESSOR_H
> +#define _ASM_RISCV_PROCESSOR_H
> +
> +#include <linux/const.h>
> +
> +#include <asm/ptrace.h>
> +
> +/*
> + * This decides where the kernel will search for a free chunk of vm
> + * space during mmap's.
> + */
> +#define TASK_UNMAPPED_BASE	PAGE_ALIGN(TASK_SIZE >> 1)
> +
> +#ifdef __KERNEL__
> +#define STACK_TOP		TASK_SIZE
> +#define STACK_TOP_MAX		STACK_TOP
> +#define STACK_ALIGN		16
> +#endif /* __KERNEL__ */
> +
> +#ifndef __ASSEMBLY__
> +
> +struct task_struct;
> +struct pt_regs;
> +
> +/*
> + * Default implementation of macro that returns current
> + * instruction pointer ("program counter").
> + */
> +#define current_text_addr()	({ __label__ _l; _l: &&_l; })
> +
> +/* CPU-specific state of a task */
> +struct thread_struct {
> +	/* Callee-saved registers */
> +	unsigned long ra;
> +	unsigned long sp;	/* Kernel mode stack */
> +	unsigned long s[12];	/* s[0]: frame pointer */
> +	struct __riscv_d_ext_state fstate;
> +};
> +
> +#define INIT_THREAD {					\
> +	.sp = sizeof(init_stack) + (long)&init_stack,	\
> +}
> +
> +/* Return saved (kernel) PC of a blocked thread. */
> +#define thread_saved_pc(t)	((t)->thread.ra)
> +#define thread_saved_sp(t)	((t)->thread.sp)
> +#define thread_saved_fp(t)	((t)->thread.s[0])

These aren't needed outside of arch-specific code (anymore) and the
riscv port doesn't seem to be using them, so they can be omitted.

[toc] | [prev] | [next] | [standalone]

#1678266 — Re: [PATCH 5/9] RISC-V: Task implementation

On Thu, 29 Jun 2017 01:22:23 PDT (-0700), tklauser@distanz.ch wrote:
> On 2017-06-28 at 20:55:34 +0200, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> [...]
>> diff --git a/arch/riscv/include/asm/kprobes.h b/arch/riscv/include/asm/kprobes.h
>> new file mode 100644
>> index 000000000000..1190de7a0f74
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/kprobes.h
>> @@ -0,0 +1,22 @@
>> +/*
>> + * Copyright (C) 2017 SiFive
>> + *
>> + *   This program is free software; you can redistribute it and/or
>> + *   modify it under the terms of the GNU General Public License
>> + *   as published by the Free Software Foundation, version 2.
>> + *
>> + *   This program is distributed in the hope that it will be useful,
>> + *   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *   GNU General Public License for more details.
>> + */
>> +
>> +
>> +#ifndef ASM_RISCV_KPROBES_H
>> +#define ASM_RISCV_KPROBES_H
>> +
>> +#ifdef CONFIG_KPROBES
>> +#error "RISC-V doesn't skpport CONFIG_KPROBES"
>
> Typo: s/skpport/support/

Thanks.

>> +#endif
>> +
>> +#endif
>> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
>> new file mode 100644
>> index 000000000000..65aa014db9b4
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/processor.h
>> @@ -0,0 +1,102 @@
>> +/*
>> + * Copyright (C) 2012 Regents of the University of California
>> + *
>> + *   This program is free software; you can redistribute it and/or
>> + *   modify it under the terms of the GNU General Public License
>> + *   as published by the Free Software Foundation, version 2.
>> + *
>> + *   This program is distributed in the hope that it will be useful,
>> + *   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *   GNU General Public License for more details.
>> + */
>> +
>> +#ifndef _ASM_RISCV_PROCESSOR_H
>> +#define _ASM_RISCV_PROCESSOR_H
>> +
>> +#include <linux/const.h>
>> +
>> +#include <asm/ptrace.h>
>> +
>> +/*
>> + * This decides where the kernel will search for a free chunk of vm
>> + * space during mmap's.
>> + */
>> +#define TASK_UNMAPPED_BASE	PAGE_ALIGN(TASK_SIZE >> 1)
>> +
>> +#ifdef __KERNEL__
>> +#define STACK_TOP		TASK_SIZE
>> +#define STACK_TOP_MAX		STACK_TOP
>> +#define STACK_ALIGN		16
>> +#endif /* __KERNEL__ */
>> +
>> +#ifndef __ASSEMBLY__
>> +
>> +struct task_struct;
>> +struct pt_regs;
>> +
>> +/*
>> + * Default implementation of macro that returns current
>> + * instruction pointer ("program counter").
>> + */
>> +#define current_text_addr()	({ __label__ _l; _l: &&_l; })
>> +
>> +/* CPU-specific state of a task */
>> +struct thread_struct {
>> +	/* Callee-saved registers */
>> +	unsigned long ra;
>> +	unsigned long sp;	/* Kernel mode stack */
>> +	unsigned long s[12];	/* s[0]: frame pointer */
>> +	struct __riscv_d_ext_state fstate;
>> +};
>> +
>> +#define INIT_THREAD {					\
>> +	.sp = sizeof(init_stack) + (long)&init_stack,	\
>> +}
>> +
>> +/* Return saved (kernel) PC of a blocked thread. */
>> +#define thread_saved_pc(t)	((t)->thread.ra)
>> +#define thread_saved_sp(t)	((t)->thread.sp)
>> +#define thread_saved_fp(t)	((t)->thread.s[0])
>
> These aren't needed outside of arch-specific code (anymore) and the
> riscv port doesn't seem to be using them, so they can be omitted.

Great.  I've removed them, I'll include that in the v4

  diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
  index 53768048b9de..b62873633bde 100644
  --- a/arch/riscv/include/asm/processor.h
  +++ b/arch/riscv/include/asm/processor.h
  @@ -52,11 +52,6 @@ struct thread_struct {
          .sp = sizeof(init_stack) + (long)&init_stack,   \
   }

  -/* Return saved (kernel) PC of a blocked thread. */
  -#define thread_saved_pc(t)     ((t)->thread.ra)
  -#define thread_saved_sp(t)     ((t)->thread.sp)
  -#define thread_saved_fp(t)     ((t)->thread.s[0])
  -
   #define task_pt_regs(tsk)                                              \
          ((struct pt_regs *)(task_stack_page(tsk) + THREAD_SIZE          \
                              - ALIGN(sizeof(struct pt_regs), STACK_ALIGN)))

Thanks!

[toc] | [prev] | [next] | [standalone]

#1677570 — Re: [PATCH 1/9] RISC-V: Init and Halt Code

Hi Palmer,

On Wed, Jun 28, 2017 at 8:55 PM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> This contains the various __init C functions, the initial assembly
> kernel entry point, and the code to reset the system.  When a file was
> init-related, it contains

...?

> Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1678263 — Re: [PATCH 1/9] RISC-V: Init and Halt Code

On Thu, 29 Jun 2017 02:44:32 PDT (-0700), geert@linux-m68k.org wrote:
> Hi Palmer,
>
> On Wed, Jun 28, 2017 at 8:55 PM, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> This contains the various __init C functions, the initial assembly
>> kernel entry point, and the code to reset the system.  When a file was
>> init-related, it contains
>
> ...?

Whoops.  I'm not even sure how to make a sentence out of that -- I guess I've
been mangling too many patch sets...

I'll try and be coherent next time :).

>
>> Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
>
> Gr{oetje,eeting}s,
>
>                         Geert

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web