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

[Richard Henderson]

On 6/19/23 17:55, Peter Maydell wrote:
> On Mon, 19 Jun 2023 at 16:49, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> > 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.
>
> Is there any particular reason to do so? The default simple
> thing is "if this is state that persists across instructions
> then migrate it"; we usually reserve "do something special in
> post-load" for oddball cases where "just copy the data" doesn't
> work.
>
> target/arm migrates both the exclusive addr and value.

ppc is adding "size", which arm technically should have as well.

> target/mips migrates lladdr but has forgotten llval
> (and perhaps llval_wp and llnewval_wp, depending on what
> those fields do).

So, similarly, would need to handle migration for which all of the required data is not 
present.

The thought is, rather than migrate this new data also, and handle compatibility, simply 
discard all reservations.


r~