mbox series

[v3,00/20] Move rom and notdirty handling to cputlb

Message ID 20190922035458.14879-1-richard.henderson@linaro.org
Headers show
Series Move rom and notdirty handling to cputlb | expand

Message

Richard Henderson Sept. 22, 2019, 3:54 a.m. UTC
Ok!  Third time is the charm, because this time it works.

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.

  * 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.

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

Comments

Richard Henderson Sept. 22, 2019, 4:02 a.m. UTC | #1
On 9/21/19 8:54 PM, Richard Henderson wrote:
> 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

I didn't mean to include the TARGET_PAGE_BITS_VARY patches.
Ignore the first 7, or review them at your pleasure.  ;-)


r~
no-reply@patchew.org Sept. 22, 2019, 6:46 a.m. UTC | #2
Patchew URL: https://patchew.org/QEMU/20190922035458.14879-1-richard.henderson@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20190922035458.14879-1-richard.henderson@linaro.org
Subject: [PATCH v3 00/20] Move rom and notdirty handling to cputlb
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20190922035458.14879-1-richard.henderson@linaro.org -> patchew/20190922035458.14879-1-richard.henderson@linaro.org
Switched to a new branch 'test'
34dd130 cputlb: Pass retaddr to tb_check_watchpoint
7d95821 cputlb: Pass retaddr to tb_invalidate_phys_page_fast
0f82f7a cputlb: Remove tb_invalidate_phys_page_range is_cpu_write_access
143b897 cputlb: Remove cpu->mem_io_vaddr
1ac8201 cputlb: Handle TLB_NOTDIRTY in probe_access
d9ff1a3 cputlb: Merge and move memory_notdirty_write_{prepare, complete}
32abbae cputlb: Partially inline memory_region_section_get_iotlb
3976ef9 cputlb: Move NOTDIRTY handling from I/O path to TLB path
4f3e8aa cputlb: Move ROM handling from I/O path to TLB path
3c42e98 exec: Adjust notdirty tracing
e3bb276 cputlb: Introduce TLB_BSWAP
e48326a cputlb: Replace switches in load/store_helper with callback
287b700 cputlb: Disable __always_inline__ without optimization
8c801ae exec: Cache TARGET_PAGE_MASK for TARGET_PAGE_BITS_VARY
2f76655 exec: Tidy TARGET_PAGE_ALIGN
aa7fdf2 exec: Promote TARGET_PAGE_MASK to target_long
20c6e62 exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG
4db4c87 exec: Use const alias for TARGET_PAGE_BITS_VARY
0d3457e exec: Split out variable page size support to exec-vary.c
b4b2c05 exec: Use TARGET_PAGE_BITS_MIN for TLB flags

=== OUTPUT BEGIN ===
1/20 Checking commit b4b2c05eaa12 (exec: Use TARGET_PAGE_BITS_MIN for TLB flags)
2/20 Checking commit 0d3457ee2850 (exec: Split out variable page size support to exec-vary.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#33: 
new file mode 100644

total: 0 errors, 1 warnings, 125 lines checked

Patch 2/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/20 Checking commit 4db4c8782d8c (exec: Use const alias for TARGET_PAGE_BITS_VARY)
ERROR: externs should be avoided in .c files
#58: FILE: exec-vary.c:53:
+extern const TargetPageBits target_page

total: 1 errors, 0 warnings, 82 lines checked

Patch 3/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/20 Checking commit 20c6e620cee9 (exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG)
5/20 Checking commit aa7fdf250f2b (exec: Promote TARGET_PAGE_MASK to target_long)
6/20 Checking commit 2f766556467d (exec: Tidy TARGET_PAGE_ALIGN)
7/20 Checking commit 8c801ae246fe (exec: Cache TARGET_PAGE_MASK for TARGET_PAGE_BITS_VARY)
8/20 Checking commit 287b700d5006 (cputlb: Disable __always_inline__ without optimization)
WARNING: architecture specific defines should be avoided
#50: FILE: include/qemu/compiler.h:178:
+#if defined(__OPTIMIZE__) && __has_attribute(always_inline)

total: 0 errors, 1 warnings, 33 lines checked

Patch 8/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/20 Checking commit e48326ae3127 (cputlb: Replace switches in load/store_helper with callback)
10/20 Checking commit e3bb276cde3b (cputlb: Introduce TLB_BSWAP)
11/20 Checking commit 3c42e98e5a4a (exec: Adjust notdirty tracing)
12/20 Checking commit 4f3e8aa354bc (cputlb: Move ROM handling from I/O path to TLB path)
13/20 Checking commit 3976ef9549a0 (cputlb: Move NOTDIRTY handling from I/O path to TLB path)
14/20 Checking commit 32abbae47942 (cputlb: Partially inline memory_region_section_get_iotlb)
15/20 Checking commit d9ff1a3e3b82 (cputlb: Merge and move memory_notdirty_write_{prepare, complete})
16/20 Checking commit 1ac820151e2f (cputlb: Handle TLB_NOTDIRTY in probe_access)
17/20 Checking commit 143b897be8e4 (cputlb: Remove cpu->mem_io_vaddr)
18/20 Checking commit 0f82f7a7365d (cputlb: Remove tb_invalidate_phys_page_range is_cpu_write_access)
19/20 Checking commit 7d95821f8ce9 (cputlb: Pass retaddr to tb_invalidate_phys_page_fast)
20/20 Checking commit 34dd130e1b50 (cputlb: Pass retaddr to tb_check_watchpoint)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190922035458.14879-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
David Hildenbrand Sept. 23, 2019, 8:23 a.m. UTC | #3
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
>