Re: [Qemu-devel] [PATCH v3 23/40] target/mips: Implement emulation of nanoMIPS LLWP/SCWP pair

[Aleksandar Markovic]

> From: Richard Henderson <address@hidden>
> Sent: Saturday, July 21, 2018 8:15 PM
> On 07/19/2018 05:54 AM, Stefan Markovic wrote:
> > From: Yongbok Kim <address@hidden>
> >
> > Implement nanoMIPS LLWP and SCWP instruction pair.
> >
> > Signed-off-by: Yongbok Kim <address@hidden>
> > Signed-off-by: Aleksandar Markovic <address@hidden>
> > Signed-off-by: Stefan Markovic <address@hidden>
> > ---
> >  linux-user/mips/cpu_loop.c |  25 ++++++++---
> >  target/mips/cpu.h          |   2 +
> >  target/mips/helper.h       |   2 +
> >  target/mips/op_helper.c    |  35 +++++++++++++++
> >  target/mips/translate.c    | 107 
> > +++++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 166 insertions(+), 5 deletions(-)
>
> Hmm.  Well, it's ok as far as it goes, but I'd really really like to see
> target/mips to be updated to use actual atomic operations.  Otherwise
> mips*-linux-user will never be reliable and mips*-softmmu cannot run SMP in
> multi-threaded mode.
>
> While converting the rest of target/mips to atomic operations is perhaps out 
> of
> scope for this patch set, there's really no reason not to do these two
> instructions correctly from the start.  It'll save the trouble of rewriting
> them from scratch later.
>
> Please see target/arm/translate.c, gen_load_exclusive and gen_store_exclusive,
> for the size == 3 case.  That is arm32 doing a 64-bit "paired" atomic
> operation, just like you are attempting here.
>
> Note that single-copy atomic semantics apply in both cases, so LLWP must
> perform one 64-bit load, not two 32-bit loads.  The store in SCWP must happen
> with a 64-bit atomic cmpxchg operation.
>
>
> r~

Hi, Richard.

The improved version of this patch, that addresses the concerns you mentioned, 
may be included in the next version of this series, which is scheduled to be 
sent in next few days.

The reason we are a little sluggish with response to your reviews is that we 
are still completing functionality (mostly linux-user-related). However, we'll 
focus on the interaction with reviewers as soon as we are out of that phase.

Regards,
Aleksandar