[herd] Load-Reserve Store-Conditional may pair on different addresses

[maranget]

The following test may succeed on hardware and is legal.

AArch64 L020
(* Observed on many systems with `-mode presi` or `-s 4` *)
{
 0:X0=x; 0:X2=y;
}
 P0              ;
 LDXR W1,[X0]    ;
 MOV W3,#1       ;
 STXR W4,W3,[X2] ;
exists([y]=1)

Have herd7 to perform store-exclusive even when the reserved address is different. In practive we encode the absence of a reservation with the ResAddr (or RES) register holding zero. Hence the store exclusive may succeeed whenever ResAddr does not hold zero. Further notice that executing a store exclusive instruction voids the reservation, regardless of the write being performed or not.

[relokin]

Overall looks good to me but I have a couple of comments about the tests.

[maranget]

Hi @relokin. What this PR basically does is changing the test that controls performing the store conditional.

First observe that the reservation (a.k.a exclusive monitor) is implemented by using a unique register RES (for AArch64, VMSA and ASL. VMSA+ASL uses an additional flag). Schematic behaviour is a follows

1. Initially this register holds the null pointer (i.e. 0).
2. LDXR sets RES to its effective address.
3. STRX perform a check on RES : (a) old check compare STXR effective address with RES; or (b) new check , checks that the contents of RES is different from null. Perform store when the check passes. Always reset RES to null.

See also the tests L020 (LDXR and STXR to different addresses, store was always failing now it may succeed) and L021 (LDXR followed by two STXR, all to the same address, the second STXR never succeeds).

Finally an new test L106 consists in a simple STXR, without LDXR, the store normally always fails but may succeed when -variant ConstrainedUnredictable is specified.

[maranget]

I have also added L107 (L021 with -variant CU). The current implementation of this PR can perform the second STXR. I am not sure at all that the result is correct, as the description of the local monitor does suggest that this variant have no impact on the local monitor being cleared (here).

AArch64 L107
(* Can the second STXR succeed? *)
Variant=CU
{
int x=1;
0:X0=x;
}
 P0               ;
 MOV W3,#2        ;
 MOV W5,#3        ;
 LDXR W1,[X0]     ;
 STXR W4,W3,[X0]  ;
 STXR W6,W5,[X0]  ;
exists 0:X4=1 /\ 0:X6=0 /\ x=3

[relokin]

Looks good to me. Perhaps one concern I have is that for AArch64 -variant CU is not modelling any architecturally valid behaviours. In other words, it shouldn't be used by most users.

[maranget]

Looks good to me. Perhaps one concern I have is that for AArch64 -variant CU is not modelling any architecturally valid behaviours. In other words, it shouldn't be used by most users.

Thanks @relokin. I'll delay considering merging until PR #1889 is merged.

[maranget]

Hi @relokin and @HadrienRenaud. Rebased on master since prerequisite PR #1889 is merged, all commits squashed into two commits, ready for review.

[HadrienRenaud]

This looks ok to me but I think the handling of reserved addresses in ASL could be cleaner.

[relokin]

Not really the point of this PR so you can go ahead and merge as soon as @HadrienRenaud is happy but I don't understand why we support -variant cu in AArch64. -variant cu models behaviours which are architecturally forbidden. If the point of -variant cu is to enable forbidden behaviours then I think we need to change the name of this variant. CONSTRAINED UNPREDICTABLE has a specific meaning for the AArch64 architecture and the behavious of -variant cu are not CONSTRAINED UNPREDICTABLE. Perhaps these should have been behaviours one would expect from -through all?

[maranget]

I do not understand the intent of -variant CU. I'd gladly keep it out of LR/SC if you think this is the thing to do. As far as I remember, some usage of this variant in mixed-size mode was less problematic.

[maranget]

some usage of this variant in mixed-size mode was less problematic.

My bad, this usage is for generators... Hence we can suppress -variant CU in herd7, having STXR to succeed whatever the context is being the only occurrence of this variant.

[HadrienRenaud]

I believe this is not necessary anymore? I don't think add_reg_if_present is called with setzero?