Re: [RFC] Unified watchpoints for x86 platforms

[Mark Kettenis <kettenis@wins.uva.nl>]

   Date: Thu, 15 Feb 2001 12:32:03 -0500 (EST)
   From: 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.

Ah, but that isn't an issue here.  Targets using the old code from
i386v-nat.c won't use the generic code and vice versa.  Moreover, I
think I'll re-implement the stuff in i386v-nat.c with the help of the
generic code.  And as a last resort we can always rename the i386_*
functions in i386v-nat.c into i386v_*.  And really the only platforms
that actually use that code Linux and SCO Open Server 5.

So please use the names I suggest above.  I think they're much clearer.

   > > >   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.

Linux and SCO Open Server don't need it either.  So please drop it.

   > > >     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.

See Andrews reply on that.

   > > >   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.

It would make the implementation on Linux easier :-).  But the
difference isn't really that big.

   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.

Good point.

Mark