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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <48C933C2.3070906@avtrex.com>
Date:	Thu, 11 Sep 2008 08:05:38 -0700
From:	David Daney <ddaney@...rex.com>
To:	Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:	linux-mips@...ux-mips.org, linux-kernel@...r.kernel.org
Subject: Re: [Patch 2/6] MIPS: Add HARDWARE_WATCHPOINTS definitions and support
 code.

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/