Re: [Qemu-devel] [PATCH v2 06/27] target/sh4: Handle user-space atomics

[Richard Henderson]

On 07/15/2017 12:14 PM, Aurelien Jarno wrote:
> On 2017-07-06 16:20, Richard Henderson wrote:
> > For uniprocessors, SH4 uses optimistic restartable atomic sequences.
> > Upon an interrupt, a real kernel would simply notice magic values in
> > the registers and reset the PC to the start of the sequence.
> >
> > For QEMU, we cannot do this in quite the same way.  Instead, we notice
> > the normal start of such a sequence (mov #-x,r15), and start a new TB
> > that can be executed under cpu_exec_step_atomic.
>
> Do you have actually have a good documentation about gUSA? I have found
> a few documents (some of them in Japanese), the most complete one being
> the LinuxTag paper. The ABI is also described in the kernel and the
> glibc. That said I am missing the following informations:

Kernel sources and glibc are good.  The description in GCC is pretty good as 
well:

https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/config/sh/sync.md?revision=243994&view=markup#l53

> - What kind of instructions are allowed in the atomic sequence? Your
>    patch takes into account branches, but are there allowed? used in
>    practice? What about FP instructions?

Any sequence that can be restarted.

> - Does the atomic sequence is actually allowed to cross pages?

I don't see why not.  It would be restarted on a page fault, but assuming that 
a process can have 2 pages in ram doesn't seem onerous.

> - Is there any alignement required? The paper mention adding a nop to
>    gUSA_exchange_and_add to align the end point to 4 bytes.

That's about the mov.l instruction that computes the address.


> This looks fine. I guess this can be improved in a another patch by
> changing the caller to pass a single target address and if the condition
> is true or false. This would avoid having to detect that here, and would
> not make the code below more complicated.

Sure.

> > +    if (ctx->tbflags & GUSA_EXCLUSIVE) {
> > +        /* Regardless of single-stepping or the end of the page,
> > +           we must complete execution of the gUSA region while
> > +           holding the exclusive lock.  */
> > +        *pmax_insns = max_insns;
> > +        return 0;
> >       }
>
> What are the consequence of not stopping the translation when crossing
> the page in user mode? If it doesn't have any, we should probably change
> the code to never stop when crossing pages.

The only consequence is recognizing a segv "too early".  It's a quirk that, 
frankly, I'm willing to accept.

> > +    if (ctx.tbflags & GUSA_EXCLUSIVE) {
> > +        /* Ending the region of exclusivity.  Clear the bits.  */
> > +        ctx.envflags &= ~GUSA_MASK;
> > +    }
> > +
>
> IIUC this assumes the number of instructions in the sequence is always
> executed. I guess this is not correct if the TCG op buffer is full. Some
> non-privileged instructions might also stop the translation, but they
> are all FPU instructions, so I guess the section is simply not valid in
> that case.

The TCG op buffer is always empty when we begin the sequence.  Since we're 
limited to 128 bytes, or 64 instructions, I'm not concerned about running out. 
I attempt to make sure that all of the paths out -- like exceptions and 
branches -- that don't do what we want have GUSA state zapped.

At which point ya gets what ya gets.  Not atomic, but not really wrong either.


r~