Re: [PATCH v3 00/20] Move rom and notdirty handling to cputlb

[David Hildenbrand <david@redhat.com>]

On 22.09.19 05:54, Richard Henderson wrote:
> Ok!  Third time is the charm, because this time it works.

Yeay :) I don't wanna know how hard it was to debug that...

> 
> New to v3:
> 
>   * Covert io_mem_rom with a new TLB_ROM bit.
> 
>   * This in turn means that there are no longer any special RAM
>     case along along the MMIO path -- they all have devices on
>     the other end.

That sounds like a really nice cleanup.

> 
>   * This in turn means that we can fold the bulk of 
>     memory_region_section_get_iotlb into tlb_set_page_with_attrs,
>     a couple of redundant tests vs the MemoryRegion.
>     The result in patch 14 is, IMO, much more understandable.
> 
>   * Fold away uses of cpu->mem_io_pc in tb_invalidate_phys_page__locked,
>     the cause of the problems for my previous two patch sets.
> 
>     BTW, I was correct with my guess in the v2 cover letter that the use
>     of memory_notdirty_write_{prepare,complete} within atomic_mmu_lookup
>     must have been broken, for not setting mem_io_pc.  :-P
> 
>   * Fix a missed use of cpu->mem_io_pc in tb_check_watchpoint,
>     which meant that the previous TLB_WATCHPOINT cleanup was a
>     titch broken.

So there was a PC already getting stored.

> 
> The remaining two users of cpu->mem_io_pc are hw/misc/mips_itu.c and
> target/i386/helper.c.  I haven't looked, but I assume that these are
> legitimately on the MMIO path, and there probably isn't a decent way
> to remove the uses.
> 
> 
> r~
> 
> 
> Richard Henderson (20):
>   exec: Use TARGET_PAGE_BITS_MIN for TLB flags
>   exec: Split out variable page size support to exec-vary.c
>   exec: Use const alias for TARGET_PAGE_BITS_VARY
>   exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG
>   exec: Promote TARGET_PAGE_MASK to target_long
>   exec: Tidy TARGET_PAGE_ALIGN
>   exec: Cache TARGET_PAGE_MASK for TARGET_PAGE_BITS_VARY
>   cputlb: Disable __always_inline__ without optimization
>   cputlb: Replace switches in load/store_helper with callback
>   cputlb: Introduce TLB_BSWAP
>   exec: Adjust notdirty tracing
>   cputlb: Move ROM handling from I/O path to TLB path
>   cputlb: Move NOTDIRTY handling from I/O path to TLB path
>   cputlb: Partially inline memory_region_section_get_iotlb
>   cputlb: Merge and move memory_notdirty_write_{prepare,complete}
>   cputlb: Handle TLB_NOTDIRTY in probe_access
>   cputlb: Remove cpu->mem_io_vaddr
>   cputlb: Remove tb_invalidate_phys_page_range is_cpu_write_access
>   cputlb: Pass retaddr to tb_invalidate_phys_page_fast
>   cputlb: Pass retaddr to tb_check_watchpoint
> 
>  Makefile.target                |   2 +-
>  accel/tcg/translate-all.h      |   8 +-
>  include/exec/cpu-all.h         |  48 ++--
>  include/exec/cpu-common.h      |   3 -
>  include/exec/exec-all.h        |   6 +-
>  include/exec/memory-internal.h |  65 ------
>  include/hw/core/cpu.h          |   2 -
>  include/qemu-common.h          |   6 +
>  include/qemu/compiler.h        |  11 +
>  accel/tcg/cputlb.c             | 388 +++++++++++++++++++--------------
>  accel/tcg/translate-all.c      |  51 ++---
>  exec-vary.c                    |  88 ++++++++
>  exec.c                         | 192 +---------------
>  hw/core/cpu.c                  |   1 -
>  memory.c                       |  20 --
>  trace-events                   |   4 +-
>  16 files changed, 403 insertions(+), 492 deletions(-)
>  create mode 100644 exec-vary.c
> 


-- 

Thanks,

David / dhildenb