Re: [RFC] Unified watchpoints for x86 platforms

[Eli Zaretskii <eliz@delorie.com>]

> From: Mark Kettenis <kettenis@wins.uva.nl>
> Date: 15 Feb 2001 17:17:17 +0100
> 
> In general, your proposal looks
> fine, but I think it is best to export functions similar to GDB's
> target_* functions:
> 
> int i386_remove_watchpoint (CORE_ADDR addr, int len,
> 			    enum target_hw_bp_type type);
> int i386_insert_watchpoint (CORE_ADDR addr, int len,
>                             enum target_hw_bp_type type);
> 
> int i386_insert_hw_breakpoint (CORE_ADDR addr, void *shadow);
> int i386_remove_hw_breakpoint (CORE_ADDR addr, void *shadow);

I didn't want to use those names because they are taken by
i386v-nat.c.  If we use these names, we will have to modify
i386v-nat.c, which is used by many x86 platforms.  I didn't want such
a profound effect; some platform maintainers might not want the new
interface, and will be less than happy to rewrite their code for no
good reason.

> > >   int i386_hwbp_insert (int pid, CORE_ADDR addr, int len, int kind);
>
> Is there any particular reason why you need the PID argument?

Just the existing interfaces.  DJGPP obviously doesn't use that
argument.

> > >     HW_READ	     break if the region is accessed for reading[1]
> > >     HW_WRITE	     break if the region is accessed for writing
> > >     HW_ACCESS	     break if the region is accessed for either
> > > 		     reading or writing
> > >     HW_IO_ACCESS     same as HW_ACCESS type, but for I/O read/write
> > > 		     access[2]
> > >     HW_EXECUTE       instruction execution breakpoint
> 
> Please consider using an enum instead of an int.  It looks as if GDB's
> convention is to use lower-case names for enum values.

I intended to use an enum.  I believe the upper-case names were just a
coincidence.

> > >   void HWBP_SET_ADDR (int pid, int dr_num, CORE_ADDR addr);
> > > 
> > >   This macro sets the debug register DR_NUM in the inferior to watch
> > >   the address ADDR.  DR_NUM can be in the range [0..3].
> > > 
> > >   void HWBP_SET_CONTROL (int pid, unsigned int val);
> > > 
> > >   This macro sets the DR7 debug control register in the inferior to
> > >   the value VAL.
> > > 
> > >   unsigned int HWBP_GET_STATUS (int pid);
> > > 
> > >   This macro returns the value of the DR6 debug status register from
> > >   the inferior.
> 
> Why not have simply long I386_GET_DR(int regnum) and I386_SET_DR(int
> regnum, long value) and let the system-dependent decide how to map the
> debug register number ([0..7], as used in the Intel documentation)
> into whatever is needed?

Why bother the target end to do that?  They will all do the same, and
the code I'll write knows exactly when it needs what register.

In addition, I386_GET_DR might mislead someone into thinking that it
actually gets the value of any DRi from the inferior.  That was not my
intention: I don't need to fetch any debug register except DR6.

Thanks for the feedback.

I think I will start coding this weekend, so if someone has more
input, please hurry ;-)