Re: [Qemu-devel] [RFC PATCH V3] Use atomic cmpxchg to atomically check the exclusive value in a STREX

[Mark Burton]

> On 18 Jun 2015, at 21:53, Peter Maydell <address@hidden> wrote:
> 
> On 18 June 2015 at 19:32, Mark Burton <address@hidden> wrote:
>> for the 1<<size thing - I think that code has been used elsewhere, which is 
>> a little worrying - I’ll check.
>> 
>>> On 18 Jun 2015, at 17:56, Peter Maydell <address@hidden> wrote:
>>> 
>>> On 18 June 2015 at 16:44,  <address@hidden> wrote:
>>>> +        uint64_t oldval, *p;
>>>> +        p = address_space_map(cs->as, paddr, &len, true);
>>>> +        if (len == 8 << size) {
>>>> +            oldval = (uint64_t)env->exclusive_val;
>>>> +            result = (atomic_cmpxchg(p, oldval, (uint64_t)newval) == 
>>>> oldval);
>>> 
>>> You can't do an atomic operation on a type that's larger than
>>> the pointer size of the host system. That means that for
>>> code that isn't host specific, like this, in practice you
>>> can't do an atomic operation on a larger size than 4 bytes.
>>> 
>> 
>> I thought they were polymorphic across all types, I didn’t notice
>> the caveat of the size, sorry about that. That makes things more
>> entertaining :-)
> 
> It's polymorphic across most types... It's been suggested
> that the macros should refuse types with size > ptrsize
> on all systems, so you don't have to get bitten by a
> ppc32 compile failure after the fact, but I don't think that
> anybody's written the patch yet.
> 

That would be sensible.

In the meantime, 
It looks to me like (most implementations of) 32 bit x86 support double word 
cmpxchg - but I dont know if the library uses that, and I’d have to find a 
machine to try it on… 
In any case, we could support that relatively easily it seems.
So, we’re talking about 64bit ARM ldrex/strex, being run, in multi-thread mode, 
on a multi-core 32bit probably non x86 host…..
We can add mutex’s around the ld/strex, and effectively implement the mechanism 
as it is upstream today, (fixing where the address/data is stored). That would 
give some protection, but it would not have the advantage of the atomicity that 
gives us somewhat better protection against a non exclusive store breaking the 
lock.

If we were to do that, we would be saying - on multi-host 32 bit non x86 hosts, 
with a 64 bit multi-core, multi thread arm guest, and a guest that uses normal 
stores to break an exclusive lock, we would have a race condition.

I’m minded to say that we should simply not support that case for now. I 
propose that the multi-thread switch will only allow you to switch into 
multi-thread mode if the maximum cmpxchg that can be supported is the same or 
bigger than the guest ptrsize. 

Does that seem reasonable?

Do you have a better idea?

Does anybody know if the current atomic_cmpxchg will support 64 bit on a 
(normal) 32 bit x86, or do we need to special case that with cmpxchg8b ? (I get 
the impression that it will automatically use cmpxchg8b, but not cmpxchg16b - 
but I’m by no means sure).

Cheers

Mark.


> -- PMM


         +44 (0)20 7100 3485 x 210
 +33 (0)5 33 52 01 77x 210

        +33 (0)603762104
        mark.burton