Re: [Patch 2/6] MIPS: Add HARDWARE_WATCHPOINTS definitions and support code.

[David Daney <ddaney@avtrex.com>]

Geert Uytterhoeven wrote:
> On Wed, 10 Sep 2008, David Daney wrote:
> 
> Given
> 
>> +	case 4:
>> +		write_c0_watchlo3(watches->watchlo[3]);
>> +		/* Write 1 to the I, R, and W bits to clear them, and
>> +		   1 to G so all ASIDs are trapped. */
>> +		write_c0_watchhi3(0x40000007 | watches->watchhi[3]);
>> +	case 3:
>> +		write_c0_watchlo2(watches->watchlo[2]);
>> +		write_c0_watchhi2(0x40000007 | watches->watchhi[2]);
>> +	case 2:
>> +		write_c0_watchlo1(watches->watchlo[1]);
>> +		write_c0_watchhi1(0x40000007 | watches->watchhi[1]);
>> +	case 1:
>> +		write_c0_watchlo0(watches->watchlo[0]);
>> +		write_c0_watchhi0(0x40000007 | watches->watchhi[0]);
> 
> and
> 
>> +	case 4:
>> +		watches->watchhi[3] = (read_c0_watchhi3() & 0x0fff);
>> +	case 3:
>> +		watches->watchhi[2] = (read_c0_watchhi2() & 0x0fff);
>> +	case 2:
>> +		watches->watchhi[1] = (read_c0_watchhi1() & 0x0fff);
>> +	case 1:
>> +		watches->watchhi[0] = (read_c0_watchhi0() & 0x0fff);

[...]

> do the same for each registers, perhaps it makes sense to create
> read_c0_watchhi(), write_c0_watchlo(), and write_c0_watchhi() macros
> that take the watchdog register index as a parameter? Then the above can
> be turned in simple loops.

I thought that too when I first started looking at it, but the
{read,write}_c0_watchhi{0,1,2,3,4,5,6,7} macros expand to a single
machine instruction.  The bit pattern of the instruction is determined
at compile time, so you would need something like the switch statement
somewhere.  Explicitly showing it in the code seemed as good as hiding
the complexity in some macro or access function.

[...]

>> +	c->watch_reg_count = 7;
>> +	t = read_c0_watchhi6();
>> +	if ((t & 0x80000000) == 0)
>> +		return;
>> +
>> +	c->watch_reg_count = 8;
> 
> and here
> 
> BTW, no check for read_c0_watchhi7()?
> 

The current patch uses a maximum of four register sets, since we are
only reporting the number of sets, we don't care about the
characteristics of watchhi[7] and thus don't need to read it.

Thanks,
David Daney