linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines
@ 2009-02-04  8:02 Renzo Davoli
  2009-03-10 21:44 ` Renzo Davoli
  0 siblings, 1 reply; 14+ messages in thread
From: Renzo Davoli @ 2009-02-04  8:02 UTC (permalink / raw)
  To: LKML, Jeff Dike, Roland McGrath

I have updated the patch already proposed on LKML last summer.

I have split the patch into two parts.

The first one is for all those application where PTRACE_SYSCALL is
managed via tracehook.
Given the wonderful work by Roland McGrath this patch is now
architecture independent and straightforward simple.

The second one is the support of PTRACE_VM for user-mode-linux.
It provides PTRACE_VM for UML processes and uses PTRACE_VM of the hosting 
kernel.

The patches are against 2.6.28.2 but apply to 2.6.28.3 and 2.6.29-pre3 (this
latter with some line offsets).

The description and motivation follows.
-----
Proposal: let us simplify
PTRACE_SYSCALL/PTRACE_SINGLESTEP/PTRACE_SYSEMU/PTRACE_SYSEMU_SINGLESTEP,
and now PTRACE_BLOCKSTEP (which will require soon a PTRACE_SYSEMU_BLOCKSTEP),
my PTRACE_SYSVM...etc. etc.

Summary of the solution:
Use tags in the "addr" parameter of existing
PTRACE_SYSCALL/PTRACE_SINGLESTEP/PTRACE_CONT/PTRACE_BLOCKSTEP calls
to skip the current call (PTRACE_VM_SKIPCALL) or skip the second upcall to 
the VM/debugger after the syscall execution (PTRACE_VM_SKIPEXIT).

Motivation:

The ptrace tag PTRACE_SYSEMU is a feature mainly used for User-Mode Linux,
or at most for other virtual machines aiming to virtualize *all* the syscalls
(total virtual machines).

In fact:
ptrace(PTRACE_SYSEMU, pid, 0, 0)
means that the *next* system call will not be executed.
PTRACE_SYSEMU AFAIK has been implemented only for x86_32.

I already proposed some time ago a different tag: PTRACE_SYSVM 
(and I maintain a patch for it) where:
ptrace(PTRACE_SYSVM, pid, XXX, 0)
1* is the same as PTRACE_SYSCALL when XXX==0,
2* skips the call (and stops before entering the next syscall) when
  PTRACE_VM_SKIPCALL | PTRACE_VM_SKIPEXIT
3* skips the ptrace call after the system call if PTRACE_VM_SKIPEXIT.
  PTRACE_SYSVM has been implemented for x86_32, powerpc_32, um+x86_32.
(x86_64 and ppc64 exist too, but are less tested).

The main difference between SYSEMU and SYSVM is that with SYSVM it is possible
to decide if *this* system call should be executed or not (instead of the next
one).
SYSVM can be used also for partial virtual machines (some syscall gets
virtualized and some others do not), like our umview.

PTRACE_SYSVM above can be used instead of PTRACE_SYSEMU in user-mode linux
and in all the others total virtual machines. In fact, provided user-mode linux
skips *all* the syscalls it does not matter if the upcall happens just after
(SYSEMU) or just before (SYSVM) having skipped the syscall.

Briefly I would like to unify SYSCALL, SYSEMU and SYSVM.
We don't need three different tags (and all their "variations", 
SINGLESTEP->SYSEMU_SINGLESTEP etc).

We could keep PTRACE_SYSCALL, using the addr parameter as in PTRACE_SYSVM.
In this case all the code I have seen (user-mode linux, strace, umview
and googling around) use 0 or 1 for addr (being defined unused).
defining PTRACE_VM_SKIPCALL=4 and PTRACE_VM_SKIPEXIT=2 (i.e. by ignoring
the lsb) everything previously coded using PTRACE_SYSCALL should continue 
to work.
In the same way PTRACE_SINGLESTEP, PTRACE_CONT and PTRACE_BLOCKSTEP can use 
the same tags restarting after a SYSCALL.

This change would eventually simplify both the kernel code
(reducing tags and exceptions) and even user-mode linux and umview.

The skip-exit feature can be implemented in a arch-independent
manner, while for skip_call some simple changes are needed
(the entry assembly code should process the return value of the syscall
tracing function call, like in arch/x86/kernel/Entry_32.S).

Motivation summary:
1) (eventually) Reduce the number of PTRACE tags. The proposed patch
does not add any tag. On the contrary after a period of deprecation
SYSEMU* tags can be eliminated.
2) Backward compatible with existing software (existing UML kernels,
strace already tested). Only software using strange "addr" values
(the addr parameter is currently ignored) could have portability problems.
3) (eventually) simplify kernel code. SYSEMU support is a bit messy and
x86/32 only. These new PTRACE_VM tags for the addr parameter will allow to
get rid of SYSEMU code.
4) It is simple to be ported across the architecture.
It is directly supported by the tracehook mechanism.
5) It is more powerful than PTRACE_SYSEMU. It provides an optimized support for
partial virtualization (some syscalls gets virtualized some other do
not) while keeping support for total virtualization a' la UML.
6) Software currently using PTRACE_SYSEMU can be easily ported to this
new support. The porting for UML (client side) is already in the patch.
All the calls like:
ptrace(PTRACE_SYSEMU, pid, 0, 0)
can be converted into
ptrace(PTRACE_SYSCALL, pid, PTRACE_VM_SKIPCALL, 0)
(but the first PTRACE_SYSCALL, the one which starts up the emulation.
In practice it is possible to set PTRACE_VM_SKIPCALL for the first call,
too. The "addr" tag is ignored being no syscalls pending).

		renzo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines
  2009-02-04  8:02 [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines Renzo Davoli
@ 2009-03-10 21:44 ` Renzo Davoli
  2009-03-13  7:42   ` Américo Wang
  2009-03-16  7:45   ` Américo Wang
  0 siblings, 2 replies; 14+ messages in thread
From: Renzo Davoli @ 2009-03-10 21:44 UTC (permalink / raw)
  To: Américo Wang; +Cc: linux-kernel, Jeff Dike, user-mode-linux-devel

Cong,

I have updated the PTRACE_VM patches.
The patches have been rebased to linux-2.6.29-rc7 but apply to linux-2.6.29-rc7-git3.

The set is composed by two patches.
The first one is for all those architectures where PTRACE_SYSCALL is
managed via tracehook (x86, powerpc etc).
Given the wonderful work by Roland McGrath this patch is now
architecture independent and straightforward simple.

The second one is the support of PTRACE_VM for user-mode-linux.
It provides PTRACE_VM for UML processes and uses PTRACE_VM of the hosting 
kernel.


The description and motivation follows.
-----
Proposal: let us simplify
PTRACE_SYSCALL/PTRACE_SINGLESTEP/PTRACE_SYSEMU/PTRACE_SYSEMU_SINGLESTEP,
and now PTRACE_BLOCKSTEP (which will require soon a PTRACE_SYSEMU_BLOCKSTEP),
my PTRACE_SYSVM...etc. etc.

Summary of the solution:
Use tags in the "addr" parameter of existing
PTRACE_SYSCALL/PTRACE_SINGLESTEP/PTRACE_CONT/PTRACE_BLOCKSTEP calls
to skip the current call (PTRACE_VM_SKIPCALL) or skip the second upcall to 
the VM/debugger after the syscall execution (PTRACE_VM_SKIPEXIT).

Motivation:

The ptrace tag PTRACE_SYSEMU is a feature mainly used for User-Mode Linux,
or at most for other virtual machines aiming to virtualize *all* the syscalls
(total virtual machines).

In fact:
ptrace(PTRACE_SYSEMU, pid, 0, 0)
means that the *next* system call will not be executed.
PTRACE_SYSEMU AFAIK has been implemented only for x86_32.

I already proposed some time ago a different tag: PTRACE_SYSVM 
(and I maintain a patch for it) where:
ptrace(PTRACE_SYSVM, pid, XXX, 0)
1* is the same as PTRACE_SYSCALL when XXX==0,
2* skips the call (and stops before entering the next syscall) when
  PTRACE_VM_SKIPCALL | PTRACE_VM_SKIPEXIT
3* skips the ptrace call after the system call if PTRACE_VM_SKIPEXIT.
  PTRACE_SYSVM has been implemented for x86_32, powerpc_32, um+x86_32.
(x86_64 and ppc64 exist too, but are less tested).

The main difference between SYSEMU and SYSVM is that with SYSVM it is possible
to decide if *this* system call should be executed or not (instead of the next
one).
SYSVM can be used also for partial virtual machines (some syscall gets
virtualized and some others do not), like our umview.

PTRACE_SYSVM above can be used instead of PTRACE_SYSEMU in user-mode linux
and in all the others total virtual machines. In fact, provided user-mode linux
skips *all* the syscalls it does not matter if the upcall happens just after
(SYSEMU) or just before (SYSVM) having skipped the syscall.

Briefly I would like to unify SYSCALL, SYSEMU and SYSVM.
We don't need three different tags (and all their "variations", 
SINGLESTEP->SYSEMU_SINGLESTEP etc).

We could keep PTRACE_SYSCALL, using the addr parameter as in PTRACE_SYSVM.
In this case all the code I have seen (user-mode linux, strace, umview
and googling around) use 0 or 1 for addr (being defined unused).
defining PTRACE_VM_SKIPCALL=4 and PTRACE_VM_SKIPEXIT=2 (i.e. by ignoring
the lsb) everything previously coded using PTRACE_SYSCALL should continue 
to work.
In the same way PTRACE_SINGLESTEP, PTRACE_CONT and PTRACE_BLOCKSTEP can use 
the same tags restarting after a SYSCALL.

This change would eventually simplify both the kernel code
(reducing tags and exceptions) and even user-mode linux and umview.

The skip-exit feature can be implemented in a arch-independent
manner, while for skip_call some simple changes are needed
(the entry assembly code should process the return value of the syscall
tracing function call, like in arch/x86/kernel/Entry_32.S).

Motivation summary:
1) (eventually) Reduce the number of PTRACE tags. The proposed patch
does not add any tag. On the contrary after a period of deprecation
SYSEMU* tags can be eliminated.
2) Backward compatible with existing software (existing UML kernels,
strace already tested). Only software using strange "addr" values
(the addr parameter is currently ignored) could have portability problems.
3) (eventually) simplify kernel code. SYSEMU support is a bit messy and
x86/32 only. These new PTRACE_VM tags for the addr parameter will allow to
get rid of SYSEMU code.
4) It is simple to be ported across the architecture.
It is directly supported by the tracehook mechanism.
5) It is more powerful than PTRACE_SYSEMU. It provides an optimized support for
partial virtualization (some syscalls gets virtualized some other do
not) while keeping support for total virtualization a' la UML.
6) Software currently using PTRACE_SYSEMU can be easily ported to this
new support. The porting for UML (client side) is already in the patch.
All the calls like:
ptrace(PTRACE_SYSEMU, pid, 0, 0)
can be converted into
ptrace(PTRACE_SYSCALL, pid, PTRACE_VM_SKIPCALL, 0)
(but the first PTRACE_SYSCALL, the one which starts up the emulation.
In practice it is possible to set PTRACE_VM_SKIPCALL for the first call,
too. The "addr" tag is ignored being no syscalls pending).

		renzo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines
  2009-03-10 21:44 ` Renzo Davoli
@ 2009-03-13  7:42   ` Américo Wang
  2009-03-16  7:45   ` Américo Wang
  1 sibling, 0 replies; 14+ messages in thread
From: Américo Wang @ 2009-03-13  7:42 UTC (permalink / raw)
  To: Renzo Davoli
  Cc: Américo Wang, linux-kernel, Jeff Dike,
	user-mode-linux-devel, Roland McGrath

On Tue, Mar 10, 2009 at 10:44:36PM +0100, Renzo Davoli wrote:
>Cong,
>
>I have updated the PTRACE_VM patches.
>The patches have been rebased to linux-2.6.29-rc7 but apply to linux-2.6.29-rc7-git3.
>
>The set is composed by two patches.
>The first one is for all those architectures where PTRACE_SYSCALL is
>managed via tracehook (x86, powerpc etc).
>Given the wonderful work by Roland McGrath this patch is now
>architecture independent and straightforward simple.
>
>The second one is the support of PTRACE_VM for user-mode-linux.
>It provides PTRACE_VM for UML processes and uses PTRACE_VM of the hosting 
>kernel.
>

Hello, Renzo!

Thanks very much for your patches!

I am sorting these, give me some more time, since I am not so familiar
with ptrace. :^)

And also let's Cc Roland to see if we can get any feedbacks from him.

Thanks.

-- 
Do what you love, f**k the rest! F**k the regulations!
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines
  2009-03-10 21:44 ` Renzo Davoli
  2009-03-13  7:42   ` Américo Wang
@ 2009-03-16  7:45   ` Américo Wang
  2009-03-24 23:47     ` Renzo Davoli
  1 sibling, 1 reply; 14+ messages in thread
From: Américo Wang @ 2009-03-16  7:45 UTC (permalink / raw)
  To: Renzo Davoli
  Cc: Américo Wang, linux-kernel, Jeff Dike,
	user-mode-linux-devel, mtk.manpages, Roland McGrath

On Tue, Mar 10, 2009 at 10:44:36PM +0100, Renzo Davoli wrote:
>Cong,
>
>I have updated the PTRACE_VM patches.
>The patches have been rebased to linux-2.6.29-rc7 but apply to linux-2.6.29-rc7-git3.
>
>The set is composed by two patches.
>The first one is for all those architectures where PTRACE_SYSCALL is
>managed via tracehook (x86, powerpc etc).
>Given the wonderful work by Roland McGrath this patch is now
>architecture independent and straightforward simple.
>
>The second one is the support of PTRACE_VM for user-mode-linux.
>It provides PTRACE_VM for UML processes and uses PTRACE_VM of the hosting 
>kernel.
>

I just finished reading all of them. Good work! :)

Thanks. Comments below.

>
>The description and motivation follows.
>-----
>Proposal: let us simplify
>PTRACE_SYSCALL/PTRACE_SINGLESTEP/PTRACE_SYSEMU/PTRACE_SYSEMU_SINGLESTEP,
>and now PTRACE_BLOCKSTEP (which will require soon a PTRACE_SYSEMU_BLOCKSTEP),
>my PTRACE_SYSVM...etc. etc.
>
>Summary of the solution:
>Use tags in the "addr" parameter of existing
>PTRACE_SYSCALL/PTRACE_SINGLESTEP/PTRACE_CONT/PTRACE_BLOCKSTEP calls
>to skip the current call (PTRACE_VM_SKIPCALL) or skip the second upcall to 
>the VM/debugger after the syscall execution (PTRACE_VM_SKIPEXIT).

Why not introduce a new request for PTRACE_VM but use *tags* in 'addr'?
We are taking risks of breaking the existing code. :)


>
>Motivation:
>
>The ptrace tag PTRACE_SYSEMU is a feature mainly used for User-Mode Linux,
>or at most for other virtual machines aiming to virtualize *all* the syscalls
>(total virtual machines).
>
>In fact:
>ptrace(PTRACE_SYSEMU, pid, 0, 0)
>means that the *next* system call will not be executed.
>PTRACE_SYSEMU AFAIK has been implemented only for x86_32.

Yes.

>
>I already proposed some time ago a different tag: PTRACE_SYSVM 
>(and I maintain a patch for it) where:
>ptrace(PTRACE_SYSVM, pid, XXX, 0)
>1* is the same as PTRACE_SYSCALL when XXX==0,
>2* skips the call (and stops before entering the next syscall) when
>  PTRACE_VM_SKIPCALL | PTRACE_VM_SKIPEXIT
>3* skips the ptrace call after the system call if PTRACE_VM_SKIPEXIT.
>  PTRACE_SYSVM has been implemented for x86_32, powerpc_32, um+x86_32.
>(x86_64 and ppc64 exist too, but are less tested).


*I think* this approach is better, since it won't break anything.

>
>The main difference between SYSEMU and SYSVM is that with SYSVM it is possible
>to decide if *this* system call should be executed or not (instead of the next
>one).
>SYSVM can be used also for partial virtual machines (some syscall gets
>virtualized and some others do not), like our umview.

Agreed, I like this idea, this one can finally replace SYSEMU.

>
>PTRACE_SYSVM above can be used instead of PTRACE_SYSEMU in user-mode linux
>and in all the others total virtual machines. In fact, provided user-mode linux
>skips *all* the syscalls it does not matter if the upcall happens just after
>(SYSEMU) or just before (SYSVM) having skipped the syscall.

My question is if there are any other usages of SYSEMU beyond UML?

>
>Briefly I would like to unify SYSCALL, SYSEMU and SYSVM.
>We don't need three different tags (and all their "variations", 
>SINGLESTEP->SYSEMU_SINGLESTEP etc).
>
>We could keep PTRACE_SYSCALL, using the addr parameter as in PTRACE_SYSVM.
>In this case all the code I have seen (user-mode linux, strace, umview
>and googling around) use 0 or 1 for addr (being defined unused).
>defining PTRACE_VM_SKIPCALL=4 and PTRACE_VM_SKIPEXIT=2 (i.e. by ignoring
>the lsb) everything previously coded using PTRACE_SYSCALL should continue 
>to work.
>In the same way PTRACE_SINGLESTEP, PTRACE_CONT and PTRACE_BLOCKSTEP can use 
>the same tags restarting after a SYSCALL.

Well, since 'addr' is said to be unused, it can have any value beyond
0 or 1, we are still having the risks of breaking existing code. :(

>
>This change would eventually simplify both the kernel code
>(reducing tags and exceptions) and even user-mode linux and umview.
>
>The skip-exit feature can be implemented in a arch-independent
>manner, while for skip_call some simple changes are needed
>(the entry assembly code should process the return value of the syscall
>tracing function call, like in arch/x86/kernel/Entry_32.S).
>

Anyway, we need to find a balance between unifying old stuffs and
breaking compatibility.

BTW, please always update the corresponding man pages when you change
any syscall interface. So let's Cc Michael Kerrisk.

Thank you!

-- 
Do what you love, f**k the rest! F**k the regulations!
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines
  2009-03-16  7:45   ` Américo Wang
@ 2009-03-24 23:47     ` Renzo Davoli
  2009-03-29 16:32       ` Américo Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Renzo Davoli @ 2009-03-24 23:47 UTC (permalink / raw)
  To: Am??rico Wang
  Cc: linux-kernel, Jeff Dike, user-mode-linux-devel, mtk.manpages,
	Roland McGrath

> I just finished reading all of them. Good work! :)
> Thanks. Comments below.
I am thanking you, not viceversa!
> 
> Why not introduce a new request for PTRACE_VM but use *tags* in 'addr'?
> We are taking risks of breaking the existing code. :)

Yes, there is a minimal risk to break some code. This is a con.
On the other side there are two main pros for this proposal:
1- the code is now extremely simple 
2- if we define a different tag for syscall (e.g. PTRACE_VM), we need also
different tags for PTRACE_VM_SINGLESTEP, PTRACE_VM_SINGLEBLOCK and maybe
others in the future.
Using the addr field we don't need this multiplication of tags
(and we could soon delete PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP).

> My question is if there are any other usages of SYSEMU beyond UML?
At this time I have not find any other usage.
> 
> Well, since 'addr' is said to be unused, it can have any value beyond
> 0 or 1, we are still having the risks of breaking existing code. :(
True. 
> 
> >This change would eventually simplify both the kernel code
> >(reducing tags and exceptions) and even user-mode linux and umview.
> >
I forgot to change this sentence
> >The skip-exit feature can be implemented in a arch-independent
> >manner, while for skip_call some simple changes are needed
> >(the entry assembly code should process the return value of the syscall
> >tracing function call, like in arch/x86/kernel/Entry_32.S).
Both features are now arch independent after McGrath work on tracing hooks.
> 
> BTW, please always update the corresponding man pages when you change
> any syscall interface. So let's Cc Michael Kerrisk.

You are right, I'll patch the man page as soon as possible.

	renzo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines
  2009-03-24 23:47     ` Renzo Davoli
@ 2009-03-29 16:32       ` Américo Wang
  2009-04-04 10:17         ` Renzo Davoli
  0 siblings, 1 reply; 14+ messages in thread
From: Américo Wang @ 2009-03-29 16:32 UTC (permalink / raw)
  To: Renzo Davoli
  Cc: Am??rico Wang, linux-kernel, Jeff Dike, user-mode-linux-devel,
	mtk.manpages, Roland McGrath

On Wed, Mar 25, 2009 at 12:47:53AM +0100, Renzo Davoli wrote:
>> Why not introduce a new request for PTRACE_VM but use *tags* in 'addr'?
>> We are taking risks of breaking the existing code. :)
>
>Yes, there is a minimal risk to break some code. This is a con.
>On the other side there are two main pros for this proposal:
>1- the code is now extremely simple 

Why adding a new request for ptrace is harder? I don't think so. :)


>2- if we define a different tag for syscall (e.g. PTRACE_VM), we need also
>different tags for PTRACE_VM_SINGLESTEP, PTRACE_VM_SINGLEBLOCK and maybe
>others in the future.
>Using the addr field we don't need this multiplication of tags
>(and we could soon delete PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP).
>

Yes? We could also remove PTRACE_SYSEMU* if we had PTRACE_VM to replace
it. I would like to hear more from you on this point.

Thanks.

-- 
Do what you love, f**k the rest! F**k the regulations!
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines
  2009-03-29 16:32       ` Américo Wang
@ 2009-04-04 10:17         ` Renzo Davoli
  2009-04-07 17:36           ` Américo Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Renzo Davoli @ 2009-04-04 10:17 UTC (permalink / raw)
  To: Américo Wang
  Cc: linux-kernel, Jeff Dike, user-mode-linux-devel, mtk.manpages,
	Roland McGrath

Dear Cong,

On Mon, Mar 30, 2009 at 12:32:28AM +0800, Américo Wang wrote:
> On Wed, Mar 25, 2009 at 12:47:53AM +0100, Renzo Davoli wrote:
> >1- the code is now extremely simple 
> Why adding a new request for ptrace is harder? I don't think so. :)
> >2- if we define a different tag for syscall (e.g. PTRACE_VM), we need also
> >different tags for PTRACE_VM_SINGLESTEP, PTRACE_VM_SINGLEBLOCK and maybe
> >others in the future.
> >Using the addr field we don't need this multiplication of tags
> >(and we could soon delete PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP).
> Yes? We could also remove PTRACE_SYSEMU* if we had PTRACE_VM to replace
> it. I would like to hear more from you on this point.

I am sorry for the delay of this reply.
I am having a quite busy time and I like to analyze problems before
giving answers.

Let me point out that the primary issue is the following one:
PTRACE_SYSEMU is a limited feature. It is twofold limited:
- it is supported only for x86
- it provides support for "total" virtual machines like User-Mode Linux
and it is useless for "partial" virtual machine like fakeroot-ng, umview
or others.
I have published a proposal for a new support that is able to overpass
these limits. PTRACE_SYSEMU/SYSEMU_SINGLESTEP could be deprecated.
There will be some cleaning up of the kernel code when the deprecated
tags are eliminated.

Whether it is nicer to use the existing tags or defining new tags is a
secondary issue. I support the hypothesis of reusing the existing tags and use
values in the addr field but if the community says that it is nicer/better to
have separate tags it is quite easy to update my patches (and umview).

Let us discuss this latter point.

PTRACE has a number of "resume" tags:
PTRACE_SYSCALL, PTRACE_SINGLESTEP, PTRACE_SINGLEBLOCK and currently
PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP.
all these call are managed in the code by the ptrace_resume function.

My patch #1 (kernel/ptrace.c function ptrace_request) forwards the addr 
parameter to ptrace_resume which saves the VM bits in some bits inside 
task_struct's ptrace field.

If we want to use different tags like:
PTRACE_VM PTRACE_VM_SINGLESTEP PTRACE_VM_SINGLEBLOCK:
the better implementation I can envision, adds another group of switch cases
as follows (kernel/ptrace.c function ptrace_request):
 ....
 #ifdef PTRACE_SINGLESTEP
   case PTRACE_SINGLESTEP:
 #endif
 #ifdef PTRACE_SINGLEBLOCK
   case PTRACE_SINGLEBLOCK:
 #endif
 #ifdef PTRACE_SYSEMU
   case PTRACE_SYSEMU:
   case PTRACE_SYSEMU_SINGLESTEP:
 #endif
   case PTRACE_SYSCALL:
   case PTRACE_CONT:
     return ptrace_resume(child, request, 0, data);
+/* statements added for PTRACE_VM management */
+#ifdef PTRACE_VM
+  case PTRACE_VM:
+#ifdef PTRACE_VM_SINGLESTEP
+  case PTRACE_VM_SINGLESTEP:
+#endif
+#ifdef PTRACE_VM_SINGLEBLOCK
+  case PTRACE_VM_SINGLEBLOCK:
+#endif
+    return ptrace_resume(child, PTRACE_VM_TAGS_MAPPING(request), addr, data);
+#endif
....
  
where PTRACE_VM_TAGS_MAPPING is a macro that maps each VM tag to the
corresponding one (PTRACE_VM->PTRACE_SYSCALL, 
PTRACE_VM_SINGLESTEP->PTRACE_SINGLESTEP, and so on).

Other implementations inside ptrace_resume would end up in a nightmare of
definitions like is_vm, is_vm_singlestep ... similar to is_singlestep,
is_sysemu_singlestep and a lot of cases and subcases in ptrace_resume: more
lines of code and more cases during the execution.

This is my position:
There are three points in favour of reusing the existing tags and 
one against.

In favour #1: Do we really want to add some tags to the jungle of PTRACE tags?
at the end of this message I have attached a list of the tags defined in
2.6.29.  I know that the set of integers although limited is quite large, but
the idea to have a slalom between already defined constant is not nice.  (Note
that some globally defined tags like PTRACE_ATTACH have been redefined in some
architectures. PTRACE_SYSEMU value itself has been redefined in frv, blackfin,
sh). Of course, each architecture uses its own constants but this is not an 
example of good design.

We could use a high order bit to map VM calls to make things simpler.
e.g.
#define PTRACE_VM_BIT 0x80000000
#define PTRACE_VM (PTRACE_VM_BIT | PTRACE_SYSCALL)
#define PTRACE_VM_SINGLESTEP (PTRACE_VM_BIT | PTRACE_SINGLESTEP)
...
in this way:
#define PTRACE_VM_TAGS_MAPPING(REQ) ((REQ) & ~PTRACE_VM_BIT)

In favour#2: Using specific PTRACE_VM* tags the definitions above should be
added in the specific ptrace.h files for each architecture supporting PTRACE_VM
(those supporting tracehook), skipping the others, thus adding a number of
lines.  When a new "resume" tag is added, all these ptrace.h files will need
to be patched.  
If we use the addr field, it is simply ignored for architectures where
the new feature is not supported, there is no need of architecure dependent
definitions and new tags are managed automagically.
Summing up "in favour" #1 and #2 the saving in terms of lines of code is
higher with the current patch than by using specific tags.

In favour#3: The more cases in a switch the more code is generated.
Using specific PTRACE_VM* tags there are some more instruction to be executed
twice for each system call.  The loss of performance is very low, but not null.
When dealing with virtual machines speed is a must, and when possible, the
single useless machine instruction should be avoided.

Against #1: Using the patch I proposed which encodes PTRACE_VM tags in the
addr field, there is a remote possibility to break some existing code.
gdb, fakeroot_ng, UML uses addr = 0. strace for some unknown reasons
uses addr = (char *) 1. 
The addr tags in my patches do not use the less significant bit.
The tools I tested on a kernel with my patch installed worked like a charm.

I'd summarize this discussion as follows. I have a strong position about
the need of a more powerful/portable support for virtual machines based
in system call emulation (and partial emulation).
I have a weak preference about the interface, it is my opinion that the one 
I proposed is better for some reasons:
- less lines of code/more lines of code will be saved when SYSEMU is eliminated
- a bit faster
- no need for arch dependent definitions.

I have in my todo file to update the ptrace(2) man page for this new feature.
I'll do it as soon as we decide for the best interface.

Thank you to have read my posting up to here ;-)

	renzo

-----
Summary of ptrace tags. 
Entries without leading path are globally defined in include/linux/ptrace.h.

#define PTRACE_TRACEME		   0
#define PTRACE_PEEKTEXT		   1
#define PTRACE_PEEKDATA		   2
#define PTRACE_PEEKUSR		   3
#define PTRACE_POKETEXT		   4
#define PTRACE_POKEDATA		   5
#define PTRACE_POKEUSR		   6
#define PTRACE_CONT		   7
#define PTRACE_KILL		   8
#define PTRACE_SINGLESTEP	   9
arch/sparc/include/asm/ptrace.h:#define PTRACE_SPARC_DETACH       11
include/asm-frv/ptrace.h:#define PTRACE_GETREGS		12
include/asm-m32r/ptrace.h:#define PTRACE_GETREGS		12
include/asm-mn10300/ptrace.h:#define PTRACE_GETREGS            12
arch/arm/include/asm/ptrace.h:#define PTRACE_GETREGS		12
arch/avr32/include/asm/ptrace.h:#define PTRACE_GETREGS		12
arch/blackfin/include/asm/ptrace.h:#define PTRACE_GETREGS            12
arch/cris/include/asm/ptrace.h:#define PTRACE_GETREGS            12
arch/h8300/include/asm/ptrace.h:#define PTRACE_GETREGS            12
arch/ia64/include/asm/ptrace.h:#define PTRACE_SINGLEBLOCK	12	/* resume execution until next branch */
arch/m68k/include/asm/ptrace.h:#define PTRACE_GETREGS            12
arch/mips/include/asm/ptrace.h:#define PTRACE_GETREGS		12
arch/parisc/include/asm/ptrace.h:#define PTRACE_SINGLEBLOCK	12	/* resume execution until next branch */
arch/powerpc/include/asm/ptrace.h:#define PTRACE_GETREGS            12
arch/sparc/include/asm/ptrace.h:#define PTRACE_GETREGS            12
arch/sh/include/asm/ptrace.h:#define PTRACE_GETREGS		12	/* General registers */
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_GETREGS            12
arch/xtensa/include/asm/ptrace.h:#define PTRACE_GETREGS		12
include/asm-frv/ptrace.h:#define PTRACE_SETREGS		13
include/asm-m32r/ptrace.h:#define PTRACE_SETREGS		13
include/asm-mn10300/ptrace.h:#define PTRACE_SETREGS            13
arch/arm/include/asm/ptrace.h:#define PTRACE_SETREGS		13
arch/avr32/include/asm/ptrace.h:#define PTRACE_SETREGS		13
arch/blackfin/include/asm/ptrace.h:#define PTRACE_SETREGS            13	/* ptrace signal  */
arch/cris/include/asm/ptrace.h:#define PTRACE_SETREGS            13
arch/h8300/include/asm/ptrace.h:#define PTRACE_SETREGS            13
arch/ia64/include/asm/ptrace.h:#define PTRACE_OLD_GETSIGINFO	13	/* (replaced by PTRACE_GETSIGINFO in <linux/ptrace.h>)  */
arch/powerpc/include/asm/ptrace.h:#define PTRACE_SETREGS            13
arch/m68k/include/asm/ptrace.h:#define PTRACE_SETREGS            13
arch/mips/include/asm/ptrace.h:#define PTRACE_SETREGS		13
arch/sparc/include/asm/ptrace.h:#define PTRACE_SETREGS            13
arch/sh/include/asm/ptrace.h:#define PTRACE_SETREGS		13
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_SETREGS            13
arch/xtensa/include/asm/ptrace.h:#define PTRACE_SETREGS		13
include/asm-frv/ptrace.h:#define PTRACE_GETFPREGS	14
include/asm-mn10300/ptrace.h:#define PTRACE_GETFPREGS          14
arch/arm/include/asm/ptrace.h:#define PTRACE_GETFPREGS	14
arch/ia64/include/asm/ptrace.h:#define PTRACE_OLD_SETSIGINFO	14	/* (replaced by PTRACE_SETSIGINFO in <linux/ptrace.h>)  */
arch/m68k/include/asm/ptrace.h:#define PTRACE_GETFPREGS          14
arch/mips/include/asm/ptrace.h:#define PTRACE_GETFPREGS		14
arch/powerpc/include/asm/ptrace.h:#define PTRACE_GETFPREGS          14
arch/sparc/include/asm/ptrace.h:#define PTRACE_GETFPREGS          14
arch/sh/include/asm/ptrace.h:#define PTRACE_GETFPREGS	14	/* FPU registers */
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_GETFPREGS          14
include/asm-frv/ptrace.h:#define PTRACE_SETFPREGS	15
include/asm-mn10300/ptrace.h:#define PTRACE_SETFPREGS          15
arch/arm/include/asm/ptrace.h:#define PTRACE_SETFPREGS	15
arch/m68k/include/asm/ptrace.h:#define PTRACE_SETFPREGS          15
arch/mips/include/asm/ptrace.h:#define PTRACE_SETFPREGS		15
arch/powerpc/include/asm/ptrace.h:#define PTRACE_SETFPREGS          15
arch/sparc/include/asm/ptrace.h:#define PTRACE_SETFPREGS          15
arch/sh/include/asm/ptrace.h:#define PTRACE_SETFPREGS	15
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_SETFPREGS          15
#define PTRACE_ATTACH		  16
arch/sparc/include/asm/ptrace.h:#define PTRACE_READDATA           16
#define PTRACE_DETACH		  17
arch/sparc/include/asm/ptrace.h:#define PTRACE_WRITEDATA          17
arch/arm/include/asm/ptrace.h:#define PTRACE_GETWMMXREGS	18
arch/ia64/include/asm/ptrace.h:#define PTRACE_GETREGS		18	/* get all registers (pt_all_user_regs) in one shot */
arch/mips/include/asm/ptrace.h:/* #define PTRACE_GETFPXREGS		18 */
arch/powerpc/include/asm/ptrace.h:#define PTRACE_GETVRREGS	18
arch/sparc/include/asm/ptrace.h:#define PTRACE_READTEXT           18
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_GETFPXREGS         18
arch/xtensa/include/asm/ptrace.h:#define PTRACE_GETXTREGS	18
arch/arm/include/asm/ptrace.h:#define PTRACE_SETWMMXREGS	19
arch/ia64/include/asm/ptrace.h:#define PTRACE_SETREGS		19	/* set all registers (pt_all_user_regs) in one shot */
arch/mips/include/asm/ptrace.h:/* #define PTRACE_SETFPXREGS		19 */
arch/powerpc/include/asm/ptrace.h:#define PTRACE_SETVRREGS	19
arch/sparc/include/asm/ptrace.h:#define PTRACE_WRITETEXT          19
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_SETFPXREGS         19
arch/xtensa/include/asm/ptrace.h:#define PTRACE_SETXTREGS	19
arch/powerpc/include/asm/ptrace.h:#define PTRACE_GETEVRREGS	20
arch/sparc/include/asm/ptrace.h:#define PTRACE_GETFPAREGS         20
include/asm-m32r/ptrace.h:#define PTRACE_OLDSETOPTIONS	21
arch/arm/include/asm/ptrace.h:#define PTRACE_OLDSETOPTIONS	21
arch/ia64/include/asm/ptrace.h:#define PTRACE_OLDSETOPTIONS	21
arch/mips/include/asm/ptrace.h:#define PTRACE_OLDSETOPTIONS	21
arch/powerpc/include/asm/ptrace.h:#define PTRACE_SETEVRREGS	21
arch/s390/include/asm/ptrace.h:#define PTRACE_OLDSETOPTIONS         21
arch/s390/include/asm/ptrace.h:#define PTRACE_PROT                       21
arch/sparc/include/asm/ptrace.h:#define PTRACE_SETFPAREGS         21
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_OLDSETOPTIONS      21
arch/arm/include/asm/ptrace.h:#define PTRACE_GET_THREAD_AREA	22
arch/powerpc/include/asm/ptrace.h:#define PTRACE_GETREGS64	  22
arch/sparc/include/asm/ptrace.h:#define PTRACE_GETREGS64	  22
arch/arm/include/asm/ptrace.h:#define PTRACE_SET_SYSCALL	23
arch/powerpc/include/asm/ptrace.h:#define PTRACE_SETREGS64	  23
arch/sparc/include/asm/ptrace.h:#define PTRACE_SETREGS64	  23
#define PTRACE_SYSCALL		  24
arch/arm/include/asm/ptrace.h:#define PTRACE_GETCRUNCHREGS	25
arch/mips/include/asm/ptrace.h:#define PTRACE_GET_THREAD_AREA	25
arch/powerpc/include/asm/ptrace.h:#define PTRACE_GET_DEBUGREG	25
arch/sparc/include/asm/ptrace.h:#define PTRACE_GETFPREGS64	  25
arch/arm/include/asm/ptrace.h:#define PTRACE_SETCRUNCHREGS	26
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_GET_THREAD_AREA    25
arch/mips/include/asm/ptrace.h:#define PTRACE_SET_THREAD_AREA	26
arch/powerpc/include/asm/ptrace.h:#define PTRACE_SET_DEBUGREG	26
arch/sparc/include/asm/ptrace.h:#define PTRACE_SETFPREGS64	  26
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_SET_THREAD_AREA    26
arch/powerpc/include/asm/ptrace.h:#define PTRACE_GETVSRREGS	27
arch/powerpc/include/asm/ptrace.h:#define PTRACE_SETVSRREGS	28
arch/x86/include/asm/ptrace-abi.h:# define PTRACE_ARCH_PRCTL	  30
arch/um/include/shared/ptrace_user.h:#define PTRACE_SYSEMU 31
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_SYSEMU		  31
include/asm-frv/ptrace.h:#define PTRACE_GETFDPIC		31	/* get the ELF fdpic loadmap address */
arch/blackfin/include/asm/ptrace.h:#define PTRACE_GETFDPIC           31
arch/sh/include/asm/ptrace.h:#define PTRACE_GETFDPIC		31	/* get the ELF fdpic loadmap address */
arch/um/include/shared/ptrace_user.h:#define PTRACE_SYSEMU_SINGLESTEP 32
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_SYSEMU_SINGLESTEP  32
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_SINGLEBLOCK	33	/* resume execution until next branch */
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_BTS_CONFIG	40
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_BTS_STATUS	41
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_BTS_SIZE		42
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_BTS_GET		43
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_BTS_CLEAR	44
arch/x86/include/asm/ptrace-abi.h:#define PTRACE_BTS_DRAIN	45
arch/sh/include/asm/ptrace.h:#define	PTRACE_GETDSPREGS	55	/* DSP registers */
arch/sh/include/asm/ptrace.h:#define	PTRACE_SETDSPREGS	56
arch/mips/include/asm/ptrace.h:#define PTRACE_PEEKTEXT_3264	0xc0
arch/mips/include/asm/ptrace.h:#define PTRACE_PEEKDATA_3264	0xc1
arch/mips/include/asm/ptrace.h:#define PTRACE_POKETEXT_3264	0xc2
arch/mips/include/asm/ptrace.h:#define PTRACE_POKEDATA_3264	0xc3
arch/mips/include/asm/ptrace.h:#define PTRACE_GET_THREAD_AREA_3264	0xc4
arch/mips/include/asm/ptrace.h:#define PTRACE_GET_WATCH_REGS	0xd0
arch/mips/include/asm/ptrace.h:#define PTRACE_SET_WATCH_REGS	0xd1
arch/powerpc/include/asm/ptrace.h:#define PPC_PTRACE_POKEUSR_3264  0x90
arch/powerpc/include/asm/ptrace.h:#define PPC_PTRACE_PEEKUSR_3264  0x91
arch/powerpc/include/asm/ptrace.h:#define PPC_PTRACE_POKEDATA_3264 0x92
arch/powerpc/include/asm/ptrace.h:#define PPC_PTRACE_POKETEXT_3264 0x93
arch/powerpc/include/asm/ptrace.h:#define PPC_PTRACE_PEEKDATA_3264 0x94
arch/powerpc/include/asm/ptrace.h:#define PPC_PTRACE_PEEKTEXT_3264 0x95
arch/powerpc/include/asm/ptrace.h:#define PPC_PTRACE_SETFPREGS	0x96	/* Set FPRs 0 - 31 */
arch/powerpc/include/asm/ptrace.h:#define PPC_PTRACE_GETFPREGS	0x97	/* Get FPRs 0 - 31 */
arch/powerpc/include/asm/ptrace.h:#define PPC_PTRACE_SETREGS	0x98	/* Set GPRs 0 - 31 */
arch/powerpc/include/asm/ptrace.h:#define PPC_PTRACE_GETREGS	0x99	/* Get GPRs 0 - 31 */
#define PTRACE_SETOPTIONS	0x4200
#define PTRACE_GETEVENTMSG	0x4201
#define PTRACE_GETSIGINFO	0x4202
#define PTRACE_SETSIGINFO	0x4203
arch/s390/include/asm/ptrace.h:#define PTRACE_PEEKUSR_AREA           0x5000
arch/s390/include/asm/ptrace.h:#define PTRACE_POKEUSR_AREA           0x5001
arch/s390/include/asm/ptrace.h:#define PTRACE_PEEKTEXT_AREA	      0x5002
arch/s390/include/asm/ptrace.h:#define PTRACE_PEEKDATA_AREA	      0x5003
arch/s390/include/asm/ptrace.h:#define PTRACE_POKETEXT_AREA	      0x5004
arch/s390/include/asm/ptrace.h:#define PTRACE_POKEDATA_AREA 	      0x5005

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines
  2009-04-04 10:17         ` Renzo Davoli
@ 2009-04-07 17:36           ` Américo Wang
  2009-04-08 12:18             ` Renzo Davoli
  0 siblings, 1 reply; 14+ messages in thread
From: Américo Wang @ 2009-04-07 17:36 UTC (permalink / raw)
  To: Renzo Davoli
  Cc: Américo Wang, linux-kernel, Jeff Dike,
	user-mode-linux-devel, mtk.manpages, Roland McGrath

On Sat, Apr 04, 2009 at 12:17:09PM +0200, Renzo Davoli wrote:
>Dear Cong,
>
>On Mon, Mar 30, 2009 at 12:32:28AM +0800, Américo Wang wrote:
>> On Wed, Mar 25, 2009 at 12:47:53AM +0100, Renzo Davoli wrote:
>> >1- the code is now extremely simple 
>> Why adding a new request for ptrace is harder? I don't think so. :)
>> >2- if we define a different tag for syscall (e.g. PTRACE_VM), we need also
>> >different tags for PTRACE_VM_SINGLESTEP, PTRACE_VM_SINGLEBLOCK and maybe
>> >others in the future.
>> >Using the addr field we don't need this multiplication of tags
>> >(and we could soon delete PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP).
>> Yes? We could also remove PTRACE_SYSEMU* if we had PTRACE_VM to replace
>> it. I would like to hear more from you on this point.
>
>I am sorry for the delay of this reply.
>I am having a quite busy time and I like to analyze problems before
>giving answers.

No problem. :)

>
>Let me point out that the primary issue is the following one:
>PTRACE_SYSEMU is a limited feature. It is twofold limited:
>- it is supported only for x86
>- it provides support for "total" virtual machines like User-Mode Linux
>and it is useless for "partial" virtual machine like fakeroot-ng, umview
>or others.
>I have published a proposal for a new support that is able to overpass
>these limits. PTRACE_SYSEMU/SYSEMU_SINGLESTEP could be deprecated.
>There will be some cleaning up of the kernel code when the deprecated
>tags are eliminated.
>
>Whether it is nicer to use the existing tags or defining new tags is a
>secondary issue. I support the hypothesis of reusing the existing tags and use
>values in the addr field but if the community says that it is nicer/better to
>have separate tags it is quite easy to update my patches (and umview).
>
>Let us discuss this latter point.
>
>PTRACE has a number of "resume" tags:
>PTRACE_SYSCALL, PTRACE_SINGLESTEP, PTRACE_SINGLEBLOCK and currently
>PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP.
>all these call are managed in the code by the ptrace_resume function.
>
>My patch #1 (kernel/ptrace.c function ptrace_request) forwards the addr 
>parameter to ptrace_resume which saves the VM bits in some bits inside 
>task_struct's ptrace field.
>
>If we want to use different tags like:
>PTRACE_VM PTRACE_VM_SINGLESTEP PTRACE_VM_SINGLEBLOCK:
>the better implementation I can envision, adds another group of switch cases
>as follows (kernel/ptrace.c function ptrace_request):
> ....
> #ifdef PTRACE_SINGLESTEP
>   case PTRACE_SINGLESTEP:
> #endif
> #ifdef PTRACE_SINGLEBLOCK
>   case PTRACE_SINGLEBLOCK:
> #endif
> #ifdef PTRACE_SYSEMU
>   case PTRACE_SYSEMU:
>   case PTRACE_SYSEMU_SINGLESTEP:
> #endif
>   case PTRACE_SYSCALL:
>   case PTRACE_CONT:
>     return ptrace_resume(child, request, 0, data);
>+/* statements added for PTRACE_VM management */
>+#ifdef PTRACE_VM
>+  case PTRACE_VM:
>+#ifdef PTRACE_VM_SINGLESTEP
>+  case PTRACE_VM_SINGLESTEP:
>+#endif
>+#ifdef PTRACE_VM_SINGLEBLOCK
>+  case PTRACE_VM_SINGLEBLOCK:
>+#endif
>+    return ptrace_resume(child, PTRACE_VM_TAGS_MAPPING(request), addr, data);
>+#endif
>....
>  

Hmmm, I see your points. Thanks for your analysis.

I didn't mean to introduce three new requests for ptrace().
My point is, actually, the same with your first proposal in this thread,
i.e. introducing a new request PTRACE_SYSVM, and two tags in 'addr' for it,
i.e. PTRACE_VM_SKIPCALL, PTRACE_VM_SKIPEXIT.

This will not break any code, and is also easy to implement as you
stated above. Isn't this what you want? Why do you drop this idea now?

Thanks.


-- 
Do what you love, f**k the rest! F**k the regulations!
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines
  2009-04-07 17:36           ` Américo Wang
@ 2009-04-08 12:18             ` Renzo Davoli
  2009-04-13 16:36               ` Américo Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Renzo Davoli @ 2009-04-08 12:18 UTC (permalink / raw)
  To: Américo Wang
  Cc: linux-kernel, Jeff Dike, user-mode-linux-devel, mtk.manpages,
	Roland McGrath

> > #endif
> > #ifdef PTRACE_SINGLEBLOCK
> >   case PTRACE_SINGLEBLOCK:
> > #endif
> > #ifdef PTRACE_SYSEMU
> >   case PTRACE_SYSEMU:
> >   case PTRACE_SYSEMU_SINGLESTEP:
> > #endif
> >   case PTRACE_SYSCALL:
> >   case PTRACE_CONT:
> >     return ptrace_resume(child, request, 0, data);
> >+/* statements added for PTRACE_VM management */
> >+#ifdef PTRACE_VM
> >+  case PTRACE_VM:
> >+#ifdef PTRACE_VM_SINGLESTEP
> >+  case PTRACE_VM_SINGLESTEP:
> >+#endif
> >+#ifdef PTRACE_VM_SINGLEBLOCK
> >+  case PTRACE_VM_SINGLEBLOCK:
> >+#endif
> >+    return ptrace_resume(child, PTRACE_VM_TAGS_MAPPING(request), addr, data);
> >+#endif
> >....
> >  
> 
> Hmmm, I see your points. Thanks for your analysis.
> 
The "resume tags" SYSCALL, SINGLESTEP/SINGLEBLOCK, CONT give to ptrace
the command to resume and indicate when ptrace must stop next time.
The VM_SKIPCALL, VM_SKIPEXIT tags refer to the current system call.
The two sets are independent, orthogonal.

I may want to skip this system call and stop either at the next system call
or at the next block, instruction or never.
As usual, everything is possible with or without some tags, the difference
in in this case in terms of clearness of the interface.

If we'd provide only a PTRACE_VM call (namely a PTRACE_VM_SYSCALL)
to resume up to the next syscall it was not possible to use it 
to implement a virtualized "ptrace".
The virtual ptrace call may need to stop the process after an instruction
or a block as it was requested to do so.
In this case the VM monitor should use PTRACE_SINGLE* without the
VM_SKIP* optimization (maybe faking the execution of a getpid
to skip a system call, like in the old times of User mode Linux).
For a similar reason PTRACE_SYSEMY_SINGLESTEP was added in the kernel.

WHy we should make life harder to VM monitor designer?

We could also have a unique PTRACE_VM tag and encode both
SYSCALL/SINGLESTEP/SINGLEBLOCK/BLOCK
and
SKIPCALL/SKIPEXIT
in different bits inside the addr field.

Again, this is a trick to use just one tag.
It is a matter of values.
Efficiency is the meaning of this patch, so it is a conditio
sine qua non.
Apart from efficiency, what do we want most?
Clearness of interface design?
Back compatibility for very improbable cases?

I bet on the former, usually it is an insurance for the future.

	renzo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines
  2009-04-08 12:18             ` Renzo Davoli
@ 2009-04-13 16:36               ` Américo Wang
  2009-04-17  8:18                 ` Américo Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Américo Wang @ 2009-04-13 16:36 UTC (permalink / raw)
  To: Renzo Davoli
  Cc: Américo Wang, linux-kernel, Jeff Dike,
	user-mode-linux-devel, mtk.manpages, Roland McGrath

On Wed, Apr 08, 2009 at 02:18:51PM +0200, Renzo Davoli wrote:
>> > #endif
>> > #ifdef PTRACE_SINGLEBLOCK
>> >   case PTRACE_SINGLEBLOCK:
>> > #endif
>> > #ifdef PTRACE_SYSEMU
>> >   case PTRACE_SYSEMU:
>> >   case PTRACE_SYSEMU_SINGLESTEP:
>> > #endif
>> >   case PTRACE_SYSCALL:
>> >   case PTRACE_CONT:
>> >     return ptrace_resume(child, request, 0, data);
>> >+/* statements added for PTRACE_VM management */
>> >+#ifdef PTRACE_VM
>> >+  case PTRACE_VM:
>> >+#ifdef PTRACE_VM_SINGLESTEP
>> >+  case PTRACE_VM_SINGLESTEP:
>> >+#endif
>> >+#ifdef PTRACE_VM_SINGLEBLOCK
>> >+  case PTRACE_VM_SINGLEBLOCK:
>> >+#endif
>> >+    return ptrace_resume(child, PTRACE_VM_TAGS_MAPPING(request), addr, data);
>> >+#endif
>> >....
>> >  
>> 
>> Hmmm, I see your points. Thanks for your analysis.
>> 
>The "resume tags" SYSCALL, SINGLESTEP/SINGLEBLOCK, CONT give to ptrace
>the command to resume and indicate when ptrace must stop next time.
>The VM_SKIPCALL, VM_SKIPEXIT tags refer to the current system call.
>The two sets are independent, orthogonal.

I see.

>
>I may want to skip this system call and stop either at the next system call
>or at the next block, instruction or never.
>As usual, everything is possible with or without some tags, the difference
>in in this case in terms of clearness of the interface.

Yes, sure.

>
>If we'd provide only a PTRACE_VM call (namely a PTRACE_VM_SYSCALL)
>to resume up to the next syscall it was not possible to use it 
>to implement a virtualized "ptrace".
>The virtual ptrace call may need to stop the process after an instruction
>or a block as it was requested to do so.
>In this case the VM monitor should use PTRACE_SINGLE* without the
>VM_SKIP* optimization (maybe faking the execution of a getpid
>to skip a system call, like in the old times of User mode Linux).
>For a similar reason PTRACE_SYSEMY_SINGLESTEP was added in the kernel.
>
>WHy we should make life harder to VM monitor designer?
>
>We could also have a unique PTRACE_VM tag and encode both
>SYSCALL/SINGLESTEP/SINGLEBLOCK/BLOCK
>and
>SKIPCALL/SKIPEXIT
>in different bits inside the addr field.
>
>Again, this is a trick to use just one tag.
>It is a matter of values.
>Efficiency is the meaning of this patch, so it is a conditio
>sine qua non.
>Apart from efficiency, what do we want most?
>Clearness of interface design?
>Back compatibility for very improbable cases?
>
>I bet on the former, usually it is an insurance for the future.

This is almost exactly what I said in my first mail, I
have no objection to your patch, I like it, I just wanted to try
to find a balance.

Anyway, I will test your patch tomorrow, and will send you more
feedbacks soon.

Thanks.

-- 
Live like a child, think like the god.
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines
  2009-04-13 16:36               ` Américo Wang
@ 2009-04-17  8:18                 ` Américo Wang
  2009-04-25  9:19                   ` Renzo Davoli
  0 siblings, 1 reply; 14+ messages in thread
From: Américo Wang @ 2009-04-17  8:18 UTC (permalink / raw)
  To: Américo Wang
  Cc: Renzo Davoli, linux-kernel, Jeff Dike, user-mode-linux-devel,
	mtk.manpages, Roland McGrath

On Tue, Apr 14, 2009 at 12:36:03AM +0800, Américo Wang wrote:
>Anyway, I will test your patch tomorrow, and will send you more
>feedbacks soon.
>

Sorry for the delay.

I applied your 2 patches to linus-tree, compile and run, when I run
the same UML binary inside UML, I got:

Locating the bottom of the address space ... do_syscall_stub : ret =
-13, offset = 1052680, data = 0a98a008
do_syscall_stub: syscall 192 failed, return value = 0xfffffff3,
expected return value = 0x0
 syscall parameters: 0x0 0x1000 0x7 0x11 0x3 0x19c0
Failed to flush page for address 0x0
Killed

I don't have enough time to dig this, would like to have a look at it?

Thanks.

-- 
Live like a child, think like the god.
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines
  2009-04-17  8:18                 ` Américo Wang
@ 2009-04-25  9:19                   ` Renzo Davoli
  2009-04-30  8:27                     ` Américo Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Renzo Davoli @ 2009-04-25  9:19 UTC (permalink / raw)
  To: Am??rico Wang
  Cc: linux-kernel, Jeff Dike, user-mode-linux-devel, mtk.manpages,
	Roland McGrath

On Fri, Apr 17, 2009 at 04:18:23PM +0800, Am??rico Wang wrote:
> On Tue, Apr 14, 2009 at 12:36:03AM +0800, Américo Wang wrote:
> >Anyway, I will test your patch tomorrow, and will send you more
> >feedbacks soon.
> 
> Sorry for the delay.
Me, too!

UML on UML stopped working long time ago, I wrote a note on it
on March 8. See:
http://article.gmane.org/gmane.linux.uml.devel/12085
Here I am having the same behavior with and without my patches, thus
(in my opinion) the bug is not related with the new tags.

It seems that there are some problems on the virtual memory
when UML runs UML.
With older kernels (<2.6.28.2) the error is:
Eeek! page_mapcount(page) went negative! (-1)
the error is still there now but the message I get is different:

BUG: Bad page map in process linux.umlv2  pte:00164045 pmd:07b5e1e1
page:08905c80 flags:00000400 count:-2 mapcount:-4 mapping:(null) index:0
addr:00100000 vm_flags:00060055 anon_vma:(null) mapping:(null) index:100
vma->vm_ops->fault: special_mapping_fault+0x0/0x47
0fc01d64:  [<081a9c5a>] dump_stack+0x1c/0x20
0fc01d7c:  [<080a233f>] print_bad_pte+0x16d/0x17d
0fc01dac:  [<080a2c20>] unmap_vmas+0x273/0x46f
0fc01e04:  [<080a5c69>] unmap_region+0x82/0xfe
0fc01e3c:  [<080a68b1>] do_munmap+0x1f6/0x245
0fc01e6c:  [<080a7118>] mmap_region+0xc4/0x3fd
0fc01eb8:  [<080a765b>] do_mmap_pgoff+0x20a/0x253
0fc01ef0:  [<08059b2f>] sys_mmap2+0x60/0x8e
0fc01f28:  [<0805b0df>] handle_syscall+0x7f/0x9c
0fc01f78:  [<0806a27a>] userspace+0x2d0/0x377
0fc01fe0:  [<08058e38>] fork_handler+0x53/0x5b
0fc01ffc:  [<00000000>] 0x0
(and then many others)
It seems that the page table of the UML process inside UML gets garbled:
I think that neither "count" should never reach -2 nor "mapcount" -4. 

The numbers of count and mapcount decrease at each faulty UML execution
(the following are outputs of tests without my patch)
$ ./linux ....
0> page:08905d00 flags:00000400 count:1 mapcount:-1 mapping:(null) index:0
$ ./linux ....
1> page:08905d00 flags:00000400 count:0 mapcount:-2 mapping:(null) index:0
2> page:08905d00 flags:00000400 count:-1 mapcount:-3 mapping:(null) index:0
3> page:08905d00 flags:00000400 count:-2 mapcount:-4 mapping:(null) index:0

It seems that a page gets released twice.

I have successfully tested my paches in various configurations:
- patched-kernel running patched-UML
- patched-kernel running unpatched-UML
- patched-kernel running umview
- patched-UML running umview
- UML on UML fails in the same way with and without the patch.
I am currently running the patched kernel on my laptop.

In my spare time (does it exist? ;-) I'll try to track the UML bug
(hoping that Jeff can do it before me, Jeff are you online? ;-)

renzo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines
  2009-04-25  9:19                   ` Renzo Davoli
@ 2009-04-30  8:27                     ` Américo Wang
  2009-05-18  9:45                       ` Amerigo Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Américo Wang @ 2009-04-30  8:27 UTC (permalink / raw)
  To: Renzo Davoli
  Cc: Am??rico Wang, linux-kernel, Jeff Dike, user-mode-linux-devel,
	mtk.manpages, Roland McGrath

On Sat, Apr 25, 2009 at 11:19:28AM +0200, Renzo Davoli wrote:
>
>UML on UML stopped working long time ago, I wrote a note on it
>on March 8. See:
>http://article.gmane.org/gmane.linux.uml.devel/12085
>Here I am having the same behavior with and without my patches, thus
>(in my opinion) the bug is not related with the new tags.

Hmm, I thought these two patches of you are aimed to solve this,
but they are not, right?

>
>It seems that there are some problems on the virtual memory
>when UML runs UML.
>With older kernels (<2.6.28.2) the error is:
>Eeek! page_mapcount(page) went negative! (-1)
>the error is still there now but the message I get is different:
>
>BUG: Bad page map in process linux.umlv2  pte:00164045 pmd:07b5e1e1
>page:08905c80 flags:00000400 count:-2 mapcount:-4 mapping:(null) index:0
>addr:00100000 vm_flags:00060055 anon_vma:(null) mapping:(null) index:100
>vma->vm_ops->fault: special_mapping_fault+0x0/0x47
>0fc01d64:  [<081a9c5a>] dump_stack+0x1c/0x20
>0fc01d7c:  [<080a233f>] print_bad_pte+0x16d/0x17d
>0fc01dac:  [<080a2c20>] unmap_vmas+0x273/0x46f
>0fc01e04:  [<080a5c69>] unmap_region+0x82/0xfe
>0fc01e3c:  [<080a68b1>] do_munmap+0x1f6/0x245
>0fc01e6c:  [<080a7118>] mmap_region+0xc4/0x3fd
>0fc01eb8:  [<080a765b>] do_mmap_pgoff+0x20a/0x253
>0fc01ef0:  [<08059b2f>] sys_mmap2+0x60/0x8e
>0fc01f28:  [<0805b0df>] handle_syscall+0x7f/0x9c
>0fc01f78:  [<0806a27a>] userspace+0x2d0/0x377
>0fc01fe0:  [<08058e38>] fork_handler+0x53/0x5b
>0fc01ffc:  [<00000000>] 0x0
>(and then many others)
>It seems that the page table of the UML process inside UML gets garbled:
>I think that neither "count" should never reach -2 nor "mapcount" -4. 
>
>The numbers of count and mapcount decrease at each faulty UML execution
>(the following are outputs of tests without my patch)
>$ ./linux ....
>0> page:08905d00 flags:00000400 count:1 mapcount:-1 mapping:(null) index:0
>$ ./linux ....
>1> page:08905d00 flags:00000400 count:0 mapcount:-2 mapping:(null) index:0
>2> page:08905d00 flags:00000400 count:-1 mapcount:-3 mapping:(null) index:0
>3> page:08905d00 flags:00000400 count:-2 mapcount:-4 mapping:(null) index:0
>
>It seems that a page gets released twice.

I will take a look at this...

>
>I have successfully tested my paches in various configurations:
>- patched-kernel running patched-UML
>- patched-kernel running unpatched-UML
>- patched-kernel running umview
>- patched-UML running umview
>- UML on UML fails in the same way with and without the patch.
>I am currently running the patched kernel on my laptop.

Thanks, I will give them another testing soon...

-- 
Live like a child, think like the god.
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines
  2009-04-30  8:27                     ` Américo Wang
@ 2009-05-18  9:45                       ` Amerigo Wang
  0 siblings, 0 replies; 14+ messages in thread
From: Amerigo Wang @ 2009-05-18  9:45 UTC (permalink / raw)
  To: Américo Wang
  Cc: Renzo Davoli, linux-kernel, Jeff Dike, user-mode-linux-devel,
	mtk.manpages, Roland McGrath, akpm

On Thu, Apr 30, 2009 at 04:27:11PM +0800, Américo Wang wrote:
>On Sat, Apr 25, 2009 at 11:19:28AM +0200, Renzo Davoli wrote:
>
>>
>>I have successfully tested my paches in various configurations:
>>- patched-kernel running patched-UML
>>- patched-kernel running unpatched-UML
>>- patched-kernel running umview
>>- patched-UML running umview
>>- UML on UML fails in the same way with and without the patch.
>>I am currently running the patched kernel on my laptop.
>
>Thanks, I will give them another testing soon...

Sorry for the long delay...

I reviewed the whole patch set again, and tested it on both i386
and x86_64, it looks very fine!! :-)

So I would like to see these patches to be merged.

Andrew, could you please take these two patches for us??

Thank you!


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2009-05-18  9:50 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-04  8:02 [PATCH 0/2] ptrace_vm: ptrace for syscall emulation virtual machines Renzo Davoli
2009-03-10 21:44 ` Renzo Davoli
2009-03-13  7:42   ` Américo Wang
2009-03-16  7:45   ` Américo Wang
2009-03-24 23:47     ` Renzo Davoli
2009-03-29 16:32       ` Américo Wang
2009-04-04 10:17         ` Renzo Davoli
2009-04-07 17:36           ` Américo Wang
2009-04-08 12:18             ` Renzo Davoli
2009-04-13 16:36               ` Américo Wang
2009-04-17  8:18                 ` Américo Wang
2009-04-25  9:19                   ` Renzo Davoli
2009-04-30  8:27                     ` Américo Wang
2009-05-18  9:45                       ` Amerigo Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).