Re: [Qemu-arm] [PATCH v9 24/25] target-arm: ensure all cross vCPUs TLB flushes complete

[Peter Maydell]

On 1 February 2017 at 15:05, Alex Bennée <address@hidden> wrote:
> Previously flushes on other vCPUs would only get serviced when they
> exited their TranslationBlocks. While this isn't overly problematic it
> violates the semantics of TLB flush from the point of view of source
> vCPU.
>
> To solve this we call the cputlb *_all_cpus_synced() functions to do
> the flushes and ask it to ensure all flushes are completed before we
> start the next instruction. As this involves exiting the cpu_loop we
> need to ensure the PC is saved before the tlb helper functions are
> called.

So, to continue the discussion from my comments on the previous round
of this patch: I definitely think that having the helper function
longjump out of the main loop is awkward and is going to cause
problems (for any target doing that, not just ARM). At the moment
we use the longjump-out approach primarily for "this instruction
didn't actually execute". That works fine because "don't actually
do anything else we generated code for" is what we want there.
Trying to longjump-out for "this instruction *did* execute but
we want to get back to the main loop" is more problematic, because
now we really do want to run the generated code that follows,
and so you end up having to duplicate the equivalent operations,
which is error-prone. ("yield" also falls into this bucket, but
we got away with that because it's practically a no-op. Really
we should fix it too, I suspect we get single-step of it wrong.)

So instead we should avoid the longjumping and have a mechanism
for making sure the code we generate after this helper is called
ends the TB in a way that takes us back out to the top level
in the right way.

The "do a TLB sync and longjump out" functions are an invitation
to target CPU implementors to write buggy code, so we should
I think not have them at all.

thanks
-- PMM