mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Matt Helsley <matthltc@us.ibm.com>
To: Matt Helsley <matthltc@us.ibm.com>
Cc: Christoffer Dall <christofferdall@christofferdall.dk>,
	Oren Laadan <orenl@cs.columbia.edu>,
	Russell King - ARM Linux <linux@arm.linux.org.uk>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	containers <containers@lists.linux-foundation.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Roland McGrath <roland@redhat.com>
Subject: Re: [C/R ARM][PATCH 1/3] ARM: Rudimentary syscall interfaces
Date: Wed, 24 Mar 2010 18:17:53 -0700	[thread overview]
Message-ID: <20100325011753.GF5704@count0.beaverton.ibm.com> (raw)
In-Reply-To: <20100325011132.GE5704@count0.beaverton.ibm.com>

On Wed, Mar 24, 2010 at 06:11:32PM -0700, Matt Helsley wrote:
> On Wed, Mar 24, 2010 at 08:36:39PM +0100, Christoffer Dall wrote:
<snip>
> > Re-using the assembly code or factoring it out so that it can be used
> > from multiple places doesn't seem very pleasing to me, as the assembly
> > code is in the critical path and written specifically for the context
> > of a process entering the kernel. Please correct me if I'm wrong.
> > 
> > I imagine simply a function in C, more or less re-implementing the
> > logic that's already in entry-common.S, might do the trick. I wouldn't
> > worry much about the performance in this case as it will not be used
> > often. The following _untested_ snippet illustrates my idea:
> > 
> > ---
> >  arch/arm/include/asm/syscall.h |   93 +++++++++++++++++++++++++++++++++++++++-
> >  1 files changed, 92 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/syscall.h b/arch/arm/include/asm/syscall.h
> > index 3b3248f..a7f2615 100644
> > --- a/arch/arm/include/asm/syscall.h
> > +++ b/arch/arm/include/asm/syscall.h
> > @@ -10,10 +10,101 @@
> >  #ifndef _ASM_ARM_SYSCALLS_H
> >  #define _ASM_ARM_SYSCALLS_H
> > 
> > +static inline int get_swi_instruction(struct task_struct *task,
> > +				      struct pt_regs *regs,
> > +				      unsigned long *instr)
> > +{
> > +	struct page *page = NULL;
> > +	unsigned long instr_addr;
> > +	unsigned long *ptr;
> > +	int ret;
> > +
> > +	instr_addr = regs->ARM_pc - 4;
> > +
> > +	down_read(&task->mm->mmap_sem);
> > +	ret = get_user_pages(task, task->mm, instr_addr,
> > +			     1, 0, 0, &page, NULL);
> > +	up_read(&task->mm->mmap_sem);
> > +
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ptr = (unsigned long *)kmap_atomic(page, KM_USER1);
> > +	memcpy(instr,
> > +	       ptr + (instr_addr >> PAGE_SHIFT),
> 			^shouldn't this be:
> 		      instr_addr & PAGE_MASK

Oops, made my own mistake. I think the address of the kmap'd instruction
would be:

	ptr + (instr_addr & ~PAGE_MASK)

Cheers,
	-Matt Helsley

  reply	other threads:[~2010-03-25  1:17 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-22  1:06 [C/R ARM][PATCH 0/3] Linux Checkpoint-Restart - ARM port Christoffer Dall
2010-03-22  1:06 ` [C/R ARM][PATCH 1/3] ARM: Rudimentary syscall interfaces Christoffer Dall
2010-03-23 20:53   ` Russell King - ARM Linux
2010-03-24  2:03     ` Matt Helsley
2010-03-24  4:57       ` Oren Laadan
2010-03-24 14:02         ` Matt Helsley
2010-03-24 15:53           ` Oren Laadan
2010-03-24 19:36             ` Christoffer Dall
2010-03-25  1:11               ` Matt Helsley
2010-03-25  1:17                 ` Matt Helsley [this message]
2010-03-25 10:29                   ` Christoffer Dall
2010-03-25  1:35                 ` Oren Laadan
2010-03-25 10:34                   ` Christoffer Dall
2010-03-22  1:06 ` [C/R ARM][PATCH 2/3] ARM: Add the eclone system call Christoffer Dall
2010-03-23 21:06   ` Russell King - ARM Linux
2010-03-24 18:19     ` Sukadev Bhattiprolu
2010-03-24 19:42     ` Christoffer Dall
2010-03-22  1:06 ` [C/R ARM][PATCH 3/3] c/r: ARM implementation of checkpoint/restart Christoffer Dall
2010-03-23 16:09   ` Serge E. Hallyn
2010-03-24 19:46     ` Christoffer Dall
2010-03-23 21:18   ` Russell King - ARM Linux
2010-03-24  1:53     ` Matt Helsley
2010-03-24 20:48     ` Christoffer Dall
2010-03-26  2:47       ` Jamie Lokier
2010-03-26  3:02         ` Paul Mundt
2010-03-26  3:55           ` Jamie Lokier
2010-03-28 22:55           ` Christoffer Dall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100325011753.GF5704@count0.beaverton.ibm.com \
    --to=matthltc@us.ibm.com \
    --cc=christofferdall@christofferdall.dk \
    --cc=containers@lists.linux-foundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=orenl@cs.columbia.edu \
    --cc=roland@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome