Re: [Qemu-devel] [PATCH 08/11] tcg/aarch64: Make direct jump patching thread-safe

[Richard Henderson]

On 04/20/2016 11:22 AM, Alex Bennée wrote:
> Richard Henderson <address@hidden> writes:
>
> > On 04/20/2016 07:01 AM, Alex Bennée wrote:
> > > Sergey Fedorov <address@hidden> writes:
> > >
> > > > From: Sergey Fedorov <address@hidden>
> > > >
> > > > Ensure direct jump patching in AArch64 is atomic by using
> > > > atomic_read()/atomic_set() for code patching.
> > > >
> > > > Signed-off-by: Sergey Fedorov <address@hidden>
> > > > Signed-off-by: Sergey Fedorov <address@hidden>
> > > > ---
> > > >    tcg/aarch64/tcg-target.inc.c | 14 +++++++++++++-
> > > >    1 file changed, 13 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
> > > > index 0ed10a974121..15fdebec921f 100644
> > > > --- a/tcg/aarch64/tcg-target.inc.c
> > > > +++ b/tcg/aarch64/tcg-target.inc.c
> > > > @@ -73,6 +73,18 @@ static inline void reloc_pc26(tcg_insn_unit *code_ptr, 
> > > > tcg_insn_unit *target)
> > > >        *code_ptr = deposit32(*code_ptr, 0, 26, offset);
> > > >    }
> > > >
> > > > +static inline void reloc_pc26_atomic(tcg_insn_unit *code_ptr,
> > > > +                                     tcg_insn_unit *target)
> > > > +{
> > > > +    ptrdiff_t offset = target - code_ptr;
> > > > +    tcg_insn_unit insn;
> > > > +    assert(offset == sextract64(offset, 0, 26));
> > > > +    /* read instruction, mask away previous PC_REL26 parameter contents,
> > > > +       set the proper offset, then write back the instruction. */
> > >
> > > This comment could be moved from here and reloc_pc26 and made common for
> > > the two following functions.
> >
> > There's a significant amount of cleanup that ought to happen here, now that
> > we're not re-translating TBs.  I don't know if Sergey should be gated
> > on that.
>
> Is this stuff already in the works? Otherwise we are trying to get
> pre-cursors to MTTCG into the code (once the tree re-opens) to keep the
> main diff down. This also is beneficial for linux-user stuff.

We're talking past one another.

No, it's not yet in the works.  I'm saying that this patch set should not wait 
for it.  Thus I don't care if what he adds here is a little messy; we'll clean 
it all up at once later.  Thus don't bother refactoring reloc_pc26 and 
reloc_pc26_atomic.


r~