[PATCH] Avoid unpaired stwcx. on some processors

[Becky Bruce]

On Nov 9, 2007, at 5:19 PM, Olof Johansson wrote:

> On Fri, Nov 09, 2007 at 04:08:06PM -0600, Becky Bruce wrote:
>> The context switch code in the kernel issues a dummy stwcx. to  
>> clear the
>> reservation, as recommended by the architecture. However, some  
>> processors
>> can have issues if this stwcx to address A occurs while the  
>> reservation
>> is already held to a different address B.  To avoid this problem,  
>> the dummy
>> stwcx. needs to be paired with a dummy lwarx to the same address.
>>
>> This patch adds the dummy lwarx, and creates a cpu feature bit to  
>> indicate
>> which cpus are affected. Tested on mpc8641_hpcn_defconfig in arch/ 
>> powerpc;
>> build tested in arch/ppc.
>
> You're still exposed even with this patch. The stwcx is there to  
> protect
> from the cases where process 1 does lwarx and get context switched
> out to process 2 that by pure random chance does a stwcx. to the same
> reservation granule and succeeds, in spite of not having done the  
> lwarx
> (on this side of the context switch).

> In exactly that case, process 2 will instead of doing a store to a
> reservation it didn't take on it's own, do a dangling stwcx, which is
> your erratum. Right?

I don't think so. It's not plain old dangling stwcx that's the  
problem.  It's dangling stwcx when the reservation is held to another  
address.  The lwarx that I've added prevents the normally dangling  
stwcx. in the context switch/syscall  path from meeting this  
condition.  Any process that gets swapped in and executes stwcx.  
first thing is fine, because the reservation was previously cleared  
by the stwcx. in the context switch path.

BTW, I think you're missing a key point here, which is this:  
Architecturally, there is a single reservation per core.  On e600 and  
other parts, the stwcx. does *not* take the address into account for  
success.  If you stwcx, and the reservation is held, it succeeds  
regardless of the address.   Fun, no?   That's one of the reasons  
it's so important that the kernel have the dummy stwcx. in place.

Does that make sense?
-B