Re: [Qemu-devel] [PATCH] translate-all: honour CF_NOCACHE in tb_gen_code

[Emilio G. Cota]

On Mon, Jul 09, 2018 at 17:59:02 +0100, Peter Maydell wrote:
> Hi -- I've been having a look at getting QEMU to support execution
> from MMIO regions by doing a "generate a single-insn CF_NOCACHE TB".
> As part of that, rth and I ran into a question about this change:
> why does tb_link_page() still add a CF_NOCACHE TB to the page lists?
> That is, why does this patch guard the "add to the hash table" part
> of the function, rather than just saying "return doing nothing
> if CF_NOCACHE"?

I think either way would work, although I have not tried the
alternative you suggest.

I'd say from a correctness viewpoint it seems "more correct" to
add the TB to the TB list, if only for the serialization imposed
by the page locks in softmmu mode.

Think for instance of MTTCG, where you might have a NOCACHE
generation concurrent with an invalidation of the same page where
guest code will be read from. Having to go through the page
locks at least enforces some ordering, which seems reasonable.

That said, currently CF_NOCACHE is only used in icount (i.e. !MTTCG),
so the above is not a practical concern. But for your MMIO
use case, which AFAIK will be abled for all TCG modes, I'd rather
keep the serialization in.

> I'm pretty clueless about this bit of the code, so I'm quite
> probably missing something.

The original design was changed to support parallel code generation.
docs/devel/multi-thread-tcg.txt documents those changes, although
it assumes that the original design is known by the reader.

That design can be sketched as follows. TBs are tracked by:

1. pc_ptr: host address of the translated code, which
   is managed with tcg_tb_lookup/remove/insert, with lookups
   usually being called from cpu_restore_state. These lookups
   are rare; insertions are a lot more common (one per
   generated TB).

2. phys_pc, pc, flags, etc. (all from the guest's viewpoint).
   These guest TB lookups are very common, so we have private
   per-vCPU caches plus a global hash table.

3. PageDesc: each guest page is tracked in a radix tree, whose
   root is l1_map in translate-all.c. PageDesc's also keep
   a list of the associated guest TBs, so that we can selectively
   invalidate TBs at a page granularity (eventually calling
   tb_phys_invalidate for each TB).

Most of tb_gen_code is spent in code generation, which in
softmmu works exclusively on thread-local data since commit
3468b59 ("tcg: enable multiple TCG contexts in softmmu", 2017-10-24).
(in user-mode we have a single context protected by mmap_lock.)

Once the code is generated, tb_add_page does 2 and 3 above,
and then tb_gen_code does 1 at the very end.

Hope that helps a little bit. Thanks,

                Emilio