diff mbox series

[v3,11/20] exec: Adjust notdirty tracing

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

Commit Message

Richard Henderson Sept. 22, 2019, 3:54 a.m. UTC 
The memory_region_tb_read tracepoint is unreachable, since notdirty
is supposed to apply only to writes.  The memory_region_tb_write
tracepoint is mis-named, because notdirty is not only used for TB
invalidation.  It is also used for e.g. VGA RAM updates and migration.

Replace memory_region_tb_write with memory_notdirty_write_access,
and place it in memory_notdirty_write_prepare where it can catch
all of the instances.  Add memory_notdirty_set_dirty to log when
we no longer intercept writes to a page.

Reviewed-by: David Hildenbrand <david@redhat.com>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 exec.c       | 3 +++
 memory.c     | 4 ----
 trace-events | 4 ++--
 3 files changed, 5 insertions(+), 6 deletions(-)

-- 
2.17.1

Comments

Philippe Mathieu-Daudé Sept. 23, 2019, 9:17 a.m. UTC | #1 
On 9/22/19 5:54 AM, Richard Henderson wrote:
> The memory_region_tb_read tracepoint is unreachable, since notdirty

> is supposed to apply only to writes.  The memory_region_tb_write

> tracepoint is mis-named, because notdirty is not only used for TB

> invalidation.  It is also used for e.g. VGA RAM updates and migration.

> 

> Replace memory_region_tb_write with memory_notdirty_write_access,

> and place it in memory_notdirty_write_prepare where it can catch

> all of the instances.  Add memory_notdirty_set_dirty to log when

> we no longer intercept writes to a page.

> 

> Reviewed-by: David Hildenbrand <david@redhat.com>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  exec.c       | 3 +++

>  memory.c     | 4 ----

>  trace-events | 4 ++--

>  3 files changed, 5 insertions(+), 6 deletions(-)

> 

> diff --git a/exec.c b/exec.c

> index 33bd0e36c1..7ce0515635 100644

> --- a/exec.c

> +++ b/exec.c

> @@ -2721,6 +2721,8 @@ void memory_notdirty_write_prepare(NotDirtyInfo *ndi,

>      ndi->size = size;

>      ndi->pages = NULL;

>  

> +    trace_memory_notdirty_write_access(mem_vaddr, ram_addr, size);

> +

>      assert(tcg_enabled());

>      if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {

>          ndi->pages = page_collection_lock(ram_addr, ram_addr + size);

> @@ -2745,6 +2747,7 @@ void memory_notdirty_write_complete(NotDirtyInfo *ndi)

>      /* we remove the notdirty callback only if the code has been

>         flushed */

>      if (!cpu_physical_memory_is_clean(ndi->ram_addr)) {

> +        trace_memory_notdirty_set_dirty(ndi->mem_vaddr);

>          tlb_set_dirty(ndi->cpu, ndi->mem_vaddr);

>      }

>  }

> diff --git a/memory.c b/memory.c

> index b9dd6b94ca..57c44c97db 100644

> --- a/memory.c

> +++ b/memory.c

> @@ -438,7 +438,6 @@ static MemTxResult  memory_region_read_accessor(MemoryRegion *mr,

>          /* Accesses to code which has previously been translated into a TB show

>           * up in the MMIO path, as accesses to the io_mem_notdirty

>           * MemoryRegion. */

> -        trace_memory_region_tb_read(get_cpu_index(), addr, tmp, size);

>      } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {

>          hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);

>          trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);

> @@ -465,7 +464,6 @@ static MemTxResult memory_region_read_with_attrs_accessor(MemoryRegion *mr,

>          /* Accesses to code which has previously been translated into a TB show

>           * up in the MMIO path, as accesses to the io_mem_notdirty

>           * MemoryRegion. */

> -        trace_memory_region_tb_read(get_cpu_index(), addr, tmp, size);

>      } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {

>          hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);

>          trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);

> @@ -490,7 +488,6 @@ static MemTxResult memory_region_write_accessor(MemoryRegion *mr,

>          /* Accesses to code which has previously been translated into a TB show

>           * up in the MMIO path, as accesses to the io_mem_notdirty

>           * MemoryRegion. */

> -        trace_memory_region_tb_write(get_cpu_index(), addr, tmp, size);

>      } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {

>          hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);

>          trace_memory_region_ops_write(get_cpu_index(), mr, abs_addr, tmp, size);

> @@ -515,7 +512,6 @@ static MemTxResult memory_region_write_with_attrs_accessor(MemoryRegion *mr,

>          /* Accesses to code which has previously been translated into a TB show

>           * up in the MMIO path, as accesses to the io_mem_notdirty

>           * MemoryRegion. */

> -        trace_memory_region_tb_write(get_cpu_index(), addr, tmp, size);

>      } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {

>          hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);

>          trace_memory_region_ops_write(get_cpu_index(), mr, abs_addr, tmp, size);

> diff --git a/trace-events b/trace-events

> index 823a4ae64e..20821ba545 100644

> --- a/trace-events

> +++ b/trace-events

> @@ -52,14 +52,14 @@ dma_map_wait(void *dbs) "dbs=%p"

>  find_ram_offset(uint64_t size, uint64_t offset) "size: 0x%" PRIx64 " @ 0x%" PRIx64

>  find_ram_offset_loop(uint64_t size, uint64_t candidate, uint64_t offset, uint64_t next, uint64_t mingap) "trying size: 0x%" PRIx64 " @ 0x%" PRIx64 ", offset: 0x%" PRIx64" next: 0x%" PRIx64 " mingap: 0x%" PRIx64

>  ram_block_discard_range(const char *rbname, void *hva, size_t length, bool need_madvise, bool need_fallocate, int ret) "%s@%p + 0x%zx: madvise: %d fallocate: %d ret: %d"

> +memory_notdirty_write_access(uint64_t vaddr, uint64_t ram_addr, unsigned size) "0x%" PRIx64 " ram_addr 0x%" PRIx64 " size %u"

> +memory_notdirty_set_dirty(uint64_t vaddr) "0x%" PRIx64

>  

>  # memory.c

>  memory_region_ops_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"

>  memory_region_ops_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"

>  memory_region_subpage_read(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u"

>  memory_region_subpage_write(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u"

> -memory_region_tb_read(int cpu_index, uint64_t addr, uint64_t value, unsigned size) "cpu %d addr 0x%"PRIx64" value 0x%"PRIx64" size %u"

> -memory_region_tb_write(int cpu_index, uint64_t addr, uint64_t value, unsigned size) "cpu %d addr 0x%"PRIx64" value 0x%"PRIx64" size %u"

>  memory_region_ram_device_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"

>  memory_region_ram_device_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"

>  flatview_new(void *view, void *root) "%p (root %p)"

> 


Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
diff mbox series

Patch

diff --git a/exec.c b/exec.c
index 33bd0e36c1..7ce0515635 100644
--- a/exec.c
+++ b/exec.c
@@ -2721,6 +2721,8 @@  void memory_notdirty_write_prepare(NotDirtyInfo *ndi,
     ndi->size = size;
     ndi->pages = NULL;
 
+    trace_memory_notdirty_write_access(mem_vaddr, ram_addr, size);
+
     assert(tcg_enabled());
     if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {
         ndi->pages = page_collection_lock(ram_addr, ram_addr + size);
@@ -2745,6 +2747,7 @@  void memory_notdirty_write_complete(NotDirtyInfo *ndi)
     /* we remove the notdirty callback only if the code has been
        flushed */
     if (!cpu_physical_memory_is_clean(ndi->ram_addr)) {
+        trace_memory_notdirty_set_dirty(ndi->mem_vaddr);
         tlb_set_dirty(ndi->cpu, ndi->mem_vaddr);
     }
 }
diff --git a/memory.c b/memory.c
index b9dd6b94ca..57c44c97db 100644
--- a/memory.c
+++ b/memory.c
@@ -438,7 +438,6 @@  static MemTxResult  memory_region_read_accessor(MemoryRegion *mr,
         /* Accesses to code which has previously been translated into a TB show
          * up in the MMIO path, as accesses to the io_mem_notdirty
          * MemoryRegion. */
-        trace_memory_region_tb_read(get_cpu_index(), addr, tmp, size);
     } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);
@@ -465,7 +464,6 @@  static MemTxResult memory_region_read_with_attrs_accessor(MemoryRegion *mr,
         /* Accesses to code which has previously been translated into a TB show
          * up in the MMIO path, as accesses to the io_mem_notdirty
          * MemoryRegion. */
-        trace_memory_region_tb_read(get_cpu_index(), addr, tmp, size);
     } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);
@@ -490,7 +488,6 @@  static MemTxResult memory_region_write_accessor(MemoryRegion *mr,
         /* Accesses to code which has previously been translated into a TB show
          * up in the MMIO path, as accesses to the io_mem_notdirty
          * MemoryRegion. */
-        trace_memory_region_tb_write(get_cpu_index(), addr, tmp, size);
     } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_write(get_cpu_index(), mr, abs_addr, tmp, size);
@@ -515,7 +512,6 @@  static MemTxResult memory_region_write_with_attrs_accessor(MemoryRegion *mr,
         /* Accesses to code which has previously been translated into a TB show
          * up in the MMIO path, as accesses to the io_mem_notdirty
          * MemoryRegion. */
-        trace_memory_region_tb_write(get_cpu_index(), addr, tmp, size);
     } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_write(get_cpu_index(), mr, abs_addr, tmp, size);
diff --git a/trace-events b/trace-events
index 823a4ae64e..20821ba545 100644
--- a/trace-events
+++ b/trace-events
@@ -52,14 +52,14 @@  dma_map_wait(void *dbs) "dbs=%p"
 find_ram_offset(uint64_t size, uint64_t offset) "size: 0x%" PRIx64 " @ 0x%" PRIx64
 find_ram_offset_loop(uint64_t size, uint64_t candidate, uint64_t offset, uint64_t next, uint64_t mingap) "trying size: 0x%" PRIx64 " @ 0x%" PRIx64 ", offset: 0x%" PRIx64" next: 0x%" PRIx64 " mingap: 0x%" PRIx64
 ram_block_discard_range(const char *rbname, void *hva, size_t length, bool need_madvise, bool need_fallocate, int ret) "%s@%p + 0x%zx: madvise: %d fallocate: %d ret: %d"
+memory_notdirty_write_access(uint64_t vaddr, uint64_t ram_addr, unsigned size) "0x%" PRIx64 " ram_addr 0x%" PRIx64 " size %u"
+memory_notdirty_set_dirty(uint64_t vaddr) "0x%" PRIx64
 
 # memory.c
 memory_region_ops_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_ops_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_subpage_read(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_subpage_write(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u"
-memory_region_tb_read(int cpu_index, uint64_t addr, uint64_t value, unsigned size) "cpu %d addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
-memory_region_tb_write(int cpu_index, uint64_t addr, uint64_t value, unsigned size) "cpu %d addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_ram_device_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
 memory_region_ram_device_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
 flatview_new(void *view, void *root) "%p (root %p)"