Re: [PATCH 2/4] target/ppc: Ensure stcx size matches larx

[Richard Henderson <richard.henderson@linaro.org>]

On 6/5/23 08:27, Nicholas Piggin wrote:
> On Sun Jun 4, 2023 at 8:28 PM AEST, Nicholas Piggin wrote:
>> Differently-sized larx/stcx. pairs can succeed if the starting address
>> matches. Add a size check to require stcx. exactly match the larx that
>> established the reservation.
> 
> Hmm, question: reserve_addr is a VMSTATE field, but reserve_val is not
> (nor reserve_size after this patch).
> 
> Blue Swirl added that with commit a456d59c20f ("VM load/save support for
> PPC CPU"), and when reserve_val was added in commit 18b21a2f83a
> ("target-ppc: retain l{w,d}arx loaded value") it did not get migrated.
> 
> Could we end up with reserve_addr != -1, but with a bogus reserve_val,
> which could then permit a stcx. incorrectly? Not entirely outlandish if
> reserve_val starts out initialised to zero.
> 
> Could we just clear the reserve in cpu_post_load? It is permitted to be
> lost for an implementation-specific reason. Doesn't seem necessary to
> try keep it alive over a migration.

It's not a bad idea to flush the reservation over migrate.
You can do

-       VMSTATE_UINTTL(env.reserve_addr, PowerPCCPU),
+       VMSTATE_UNUSED(sizeof(target_long))

when making that change.

Peter, any thoughts on this?  If we're going to do one guest, we might ought to do the 
same for all load-lock/store-conditional guests.


r~