linux-mips
[Top] [All Lists]

Re: watch exception only for kseg0 addresses..?

To: Daniel Jacobowitz <dan@debian.org>
Subject: Re: watch exception only for kseg0 addresses..?
From: "Maciej W. Rozycki" <macro@ds2.pg.gda.pl>
Date: Wed, 4 Dec 2002 16:45:38 +0100 (MET)
Cc: Ralf Baechle <ralf@linux-mips.org>, linux-mips@linux-mips.org
In-reply-to: <20021204001547.GA8012@nevyn.them.org>
Organization: Technical University of Gdansk
Original-recipient: rfc822;linux-mips@linux-mips.org
Reply-to: "Maciej W. Rozycki" <macro@ds2.pg.gda.pl>
Sender: linux-mips-bounce@linux-mips.org
On Tue, 3 Dec 2002, Daniel Jacobowitz wrote:

> >  As a fallback the approach is just fine, but doesn't is suck
> > performance-wise for watchpoints at the stack?  It certainly sucks for
> > instruction fetches.  While gdb doesn't seem to use hardware breakpoints
> > as they are only really necessary for ROMs, other software may want to
> > (well, gdb too, one day). 
> 
> Page-protection watchpoints on the stack do bite for performance, yes. 
> Most watched variables are not on the stack, though.  People tend to
> watch globals.

 Well, so far I've almost exclusively watched the stack, sometimes
malloc()ed areas, to track down out of bound corruption.  It's really
useful when a program crashes with a SIGSEGV when returning from a
function call or when calling free() with a legal pointer.  Watching
globals has not been really useful for me so far -- they are rarely used
in the first place and you know where they can get modified, so you can
set ordinary breakpoints in contexts of interest. 

> On Mon, Nov 25, 2002 at 04:08:00PM +0100, Ralf Baechle wrote:
> > I assume you got and R4000 manual and the MIPS64 spec.   R4000 implements
> > matching a physical address with a granularity of 8 bytes for load and
> > store operations.
> 
> Not handy.

 Still better than nothing.  Userland doesn't need to care of the
underlying implementation anyway.  You simply have a single watchpoint
available.  The kernel needs to take care when entering and exiting
userland.

> > So how would a prefered ptrace(2) API for hardware watchpoints look like?
> 
> Well, it would be nice to have at least:
>   - query total number
>   - query the granularity, or at least query whether or not the
>     granularity is settable
>   - Set and remove watchpoints.
> 
> Off the top of my head:
> PTRACE_MIPS_WATCHPOINT_INFO
> struct mips_watchpoint_info {
>   u32 num_avail;
>   u32 max_size;
> };

 The information may be provided when reading the registers.

> PTRACE_MIPS_WATCHPOINT_SET
> struct mips_watchpoint_set {
>   u32 index;
>   u32 size;
>   s64 address;
> };

 How about a KISS approach:

typedef struct {
        s64 address;
        u64 mask;
        u64 access;
} mips_watchpoint;

typedef struct {
        s32 api_version;
        s32 nr_watchpoints;
        mips_watchpoint watchpoints[0];
} mips_watchpoint_set;

Then PTRACE_MIPS_WATCHPOINT_GET is used to retrieve current settings,
PTRACE_MIPS_WATCHPOINT_SET is used to alter them.  More details:

PTRACE_MIPS_WATCHPOINT_SET:

Input:

- api_version has to match the version implemented, currently 0,

- nr_watchpoints specifies the number of watchpoint descriptions
  following, >= 0,

- for each watchpoints entry i, (i = 0; i < nr_watchpoints; i++):

  - address specifies the virtual address covered -- properly
    sign-extended for the 32-bit kernel),

  - mask specifies the mask to use against the address -- don't care bits
    set to one,

  - access specifies the access type; currently read, write and exec are
    specified -- we may follow the MIPS32/64 ISA definition.

Output:

- error code: a failure if a protection violation happens when reading
  mips_watchpoint_set, otherwise success.

PTRACE_MIPS_WATCHPOINT_GET:

Input:

- api_version set to the version expected, currently 0, = api_version_i,

- nr_watchpoints specifies the maximum number of watchpoint descriptions
  expected, >= 0, = nr_watchpoints_i

Output:

- error code: a failure if a protection violation happens when writing
  mips_watchpoint_set, otherwise success,

- api_version set to the version supported, currently 0, = api_version_o,

- if (api_version_i == api_version_o):

  - nr_watchpoints set to the number of watchpoints supported, >= 0, =
    nr_watchpoints_o,

  - for each watchpoints entry i, (i = 0; i < min(nr_watchpoints_i,
    nr_watchpoints_o; i++):

    - address set to the value preset for the watchpoint, as obtained
      from hardware,

    - mask set to the value preset for the watchpoint, as obtained from
      hardware,

    - access set to the value preset for the watchpoint, as obtained from
      hardware.

 I think such an interface covers all the functionality we care of now,
including implementation variations (R4000 vs R4650 vs MIPS32/64), and
provides for cheap future expansion.  Additionally thread-global
watchpoints may be handled by adding a bit to the access member if needed. 

 What do you think?

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +


<Prev in Thread] Current Thread [Next in Thread>