All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH-for-8.0 0/5] accel/tcg: Restrict page_collection structure to system TB maintainance
@ 2022-12-09  9:36 Philippe Mathieu-Daudé
  2022-12-09  9:36 ` [PATCH-for-8.0 1/5] accel/tcg: Restrict cpu_io_recompile() to system emulation Philippe Mathieu-Daudé
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-09  9:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Alex Bennée, Paolo Bonzini,
	Stefan Hajnoczi, Philippe Mathieu-Daudé

Few cleanups noticed while reviewing Richard's "Rewrite user-only
vma tracking" v3 ([*], based on top of it).

- Move accel/tcg/ trace events out of trace-root.h
- Refactor tb_invalidate_phys_range_fast() to restrict page_collection
  to sysemu.

[*] https://lore.kernel.org/qemu-devel/20221209051914.398215-1-richard.henderson@linaro.org/
Based-on: <20221209051914.398215-1-richard.henderson@linaro.org>

Philippe Mathieu-Daudé (5):
  accel/tcg: Restrict cpu_io_recompile() to system emulation
  accel/tcg: Remove trace events from trace-root.h
  accel/tcg: Rename tb_invalidate_phys_page_[locked_]fast()
  accel/tcg: Factor tb_invalidate_phys_range_fast() out
  accel/tcg: Restrict page_collection structure to system TB
    maintainance

 accel/tcg/cputlb.c     |  7 ++-----
 accel/tcg/internal.h   | 12 ++++--------
 accel/tcg/tb-maint.c   | 33 +++++++++++++++++++++++----------
 accel/tcg/trace-events |  4 ++++
 trace-events           |  4 ----
 5 files changed, 33 insertions(+), 27 deletions(-)

-- 
2.38.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH-for-8.0 1/5] accel/tcg: Restrict cpu_io_recompile() to system emulation
  2022-12-09  9:36 [PATCH-for-8.0 0/5] accel/tcg: Restrict page_collection structure to system TB maintainance Philippe Mathieu-Daudé
@ 2022-12-09  9:36 ` Philippe Mathieu-Daudé
  2022-12-16 12:07   ` Alex Bennée
  2022-12-09  9:36 ` [PATCH-for-8.0 2/5] accel/tcg: Remove trace events from trace-root.h Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-09  9:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Alex Bennée, Paolo Bonzini,
	Stefan Hajnoczi, Philippe Mathieu-Daudé

Missed in commit 6526919224 ("accel/tcg: Restrict cpu_io_recompile()
from other accelerators").

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h
index e1429a53ac..35419f3fe1 100644
--- a/accel/tcg/internal.h
+++ b/accel/tcg/internal.h
@@ -43,12 +43,12 @@ void tb_invalidate_phys_page_fast(struct page_collection *pages,
 struct page_collection *page_collection_lock(tb_page_addr_t start,
                                              tb_page_addr_t end);
 void page_collection_unlock(struct page_collection *set);
+G_NORETURN void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
 #endif /* CONFIG_SOFTMMU */
 
 TranslationBlock *tb_gen_code(CPUState *cpu, target_ulong pc,
                               target_ulong cs_base, uint32_t flags,
                               int cflags);
-G_NORETURN void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
 void page_init(void);
 void tb_htable_init(void);
 void tb_reset_jump(TranslationBlock *tb, int n);
-- 
2.38.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH-for-8.0 2/5] accel/tcg: Remove trace events from trace-root.h
  2022-12-09  9:36 [PATCH-for-8.0 0/5] accel/tcg: Restrict page_collection structure to system TB maintainance Philippe Mathieu-Daudé
  2022-12-09  9:36 ` [PATCH-for-8.0 1/5] accel/tcg: Restrict cpu_io_recompile() to system emulation Philippe Mathieu-Daudé
@ 2022-12-09  9:36 ` Philippe Mathieu-Daudé
  2022-12-16 12:09   ` Alex Bennée
  2022-12-09  9:36 ` [PATCH-for-8.0 3/5] accel/tcg: Rename tb_invalidate_phys_page_[locked_]fast() Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-09  9:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Alex Bennée, Paolo Bonzini,
	Stefan Hajnoczi, Philippe Mathieu-Daudé

Commit d9bb58e510 ("tcg: move tcg related files into accel/tcg/
subdirectory") introduced accel/tcg/trace-events, so we don't
need to use the root trace-events anymore.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/cputlb.c     | 2 +-
 accel/tcg/trace-events | 4 ++++
 trace-events           | 4 ----
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 6f1c00682b..ac459478f4 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -33,7 +33,7 @@
 #include "qemu/atomic.h"
 #include "qemu/atomic128.h"
 #include "exec/translate-all.h"
-#include "trace/trace-root.h"
+#include "trace.h"
 #include "tb-hash.h"
 #include "internal.h"
 #ifdef CONFIG_PLUGIN
diff --git a/accel/tcg/trace-events b/accel/tcg/trace-events
index 59eab96f26..4e9b450520 100644
--- a/accel/tcg/trace-events
+++ b/accel/tcg/trace-events
@@ -6,5 +6,9 @@ exec_tb(void *tb, uintptr_t pc) "tb:%p pc=0x%"PRIxPTR
 exec_tb_nocache(void *tb, uintptr_t pc) "tb:%p pc=0x%"PRIxPTR
 exec_tb_exit(void *last_tb, unsigned int flags) "tb:%p flags=0x%x"
 
+# cputlb.c
+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
+
 # translate-all.c
 translate_block(void *tb, uintptr_t pc, const void *tb_code) "tb:%p, pc:0x%"PRIxPTR", tb_code:%p"
diff --git a/trace-events b/trace-events
index 035f3d570d..b6b84b175e 100644
--- a/trace-events
+++ b/trace-events
@@ -42,10 +42,6 @@ find_ram_offset(uint64_t size, uint64_t offset) "size: 0x%" PRIx64 " @ 0x%" PRIx
 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"
 
-# accel/tcg/cputlb.c
-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
-
 # job.c
 job_state_transition(void *job,  int ret, const char *legal, const char *s0, const char *s1) "job %p (ret: %d) attempting %s transition (%s-->%s)"
 job_apply_verb(void *job, const char *state, const char *verb, const char *legal) "job %p in state %s; applying verb %s (%s)"
-- 
2.38.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH-for-8.0 3/5] accel/tcg: Rename tb_invalidate_phys_page_[locked_]fast()
  2022-12-09  9:36 [PATCH-for-8.0 0/5] accel/tcg: Restrict page_collection structure to system TB maintainance Philippe Mathieu-Daudé
  2022-12-09  9:36 ` [PATCH-for-8.0 1/5] accel/tcg: Restrict cpu_io_recompile() to system emulation Philippe Mathieu-Daudé
  2022-12-09  9:36 ` [PATCH-for-8.0 2/5] accel/tcg: Remove trace events from trace-root.h Philippe Mathieu-Daudé
@ 2022-12-09  9:36 ` Philippe Mathieu-Daudé
  2022-12-16 12:11   ` Alex Bennée
  2022-12-16 17:31   ` Richard Henderson
  2022-12-09  9:36 ` [PATCH-for-8.0 4/5] accel/tcg: Factor tb_invalidate_phys_range_fast() out Philippe Mathieu-Daudé
  2022-12-09  9:36 ` [PATCH-for-8.0 5/5] accel/tcg: Restrict page_collection structure to system TB maintainance Philippe Mathieu-Daudé
  4 siblings, 2 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-09  9:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Alex Bennée, Paolo Bonzini,
	Stefan Hajnoczi, Philippe Mathieu-Daudé

Emphasize this function is called with pages locked.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/cputlb.c   | 2 +-
 accel/tcg/internal.h | 6 +++---
 accel/tcg/tb-maint.c | 6 +++---
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index ac459478f4..6402fe11c1 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1510,7 +1510,7 @@ static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size,
     if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {
         struct page_collection *pages
             = page_collection_lock(ram_addr, ram_addr + size);
-        tb_invalidate_phys_page_fast(pages, ram_addr, size, retaddr);
+        tb_invalidate_phys_page_locked_fast(pages, ram_addr, size, retaddr);
         page_collection_unlock(pages);
     }
 
diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h
index 35419f3fe1..d8b58c1e70 100644
--- a/accel/tcg/internal.h
+++ b/accel/tcg/internal.h
@@ -37,9 +37,9 @@ void page_table_config_init(void);
 
 #ifdef CONFIG_SOFTMMU
 struct page_collection;
-void tb_invalidate_phys_page_fast(struct page_collection *pages,
-                                  tb_page_addr_t start, int len,
-                                  uintptr_t retaddr);
+void tb_invalidate_phys_page_locked_fast(struct page_collection *pages,
+                                         tb_page_addr_t start, int len,
+                                         uintptr_t retaddr);
 struct page_collection *page_collection_lock(tb_page_addr_t start,
                                              tb_page_addr_t end);
 void page_collection_unlock(struct page_collection *set);
diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index 0c56e81d8c..bf84728910 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -1200,9 +1200,9 @@ void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end)
  *
  * Call with all @pages in the range [@start, @start + len[ locked.
  */
-void tb_invalidate_phys_page_fast(struct page_collection *pages,
-                                  tb_page_addr_t start, int len,
-                                  uintptr_t retaddr)
+void tb_invalidate_phys_page_locked_fast(struct page_collection *pages,
+                                         tb_page_addr_t start, int len,
+                                         uintptr_t retaddr)
 {
     PageDesc *p;
 
-- 
2.38.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH-for-8.0 4/5] accel/tcg: Factor tb_invalidate_phys_range_fast() out
  2022-12-09  9:36 [PATCH-for-8.0 0/5] accel/tcg: Restrict page_collection structure to system TB maintainance Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2022-12-09  9:36 ` [PATCH-for-8.0 3/5] accel/tcg: Rename tb_invalidate_phys_page_[locked_]fast() Philippe Mathieu-Daudé
@ 2022-12-09  9:36 ` Philippe Mathieu-Daudé
  2022-12-16 12:17   ` Alex Bennée
  2022-12-09  9:36 ` [PATCH-for-8.0 5/5] accel/tcg: Restrict page_collection structure to system TB maintainance Philippe Mathieu-Daudé
  4 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-09  9:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Alex Bennée, Paolo Bonzini,
	Stefan Hajnoczi, Philippe Mathieu-Daudé

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/cputlb.c   |  5 +----
 accel/tcg/internal.h |  3 +++
 accel/tcg/tb-maint.c | 21 +++++++++++++++++----
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 6402fe11c1..03674d598f 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1508,10 +1508,7 @@ static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size,
     trace_memory_notdirty_write_access(mem_vaddr, ram_addr, size);
 
     if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {
-        struct page_collection *pages
-            = page_collection_lock(ram_addr, ram_addr + size);
-        tb_invalidate_phys_page_locked_fast(pages, ram_addr, size, retaddr);
-        page_collection_unlock(pages);
+        tb_invalidate_phys_range_fast(ram_addr, size, retaddr);
     }
 
     /*
diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h
index d8b58c1e70..db078390b1 100644
--- a/accel/tcg/internal.h
+++ b/accel/tcg/internal.h
@@ -42,6 +42,9 @@ void tb_invalidate_phys_page_locked_fast(struct page_collection *pages,
                                          uintptr_t retaddr);
 struct page_collection *page_collection_lock(tb_page_addr_t start,
                                              tb_page_addr_t end);
+void tb_invalidate_phys_range_fast(ram_addr_t ram_addr,
+                                   unsigned size,
+                                   uintptr_t retaddr);
 void page_collection_unlock(struct page_collection *set);
 G_NORETURN void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
 #endif /* CONFIG_SOFTMMU */
diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index bf84728910..4dc2fa1060 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -1194,10 +1194,6 @@ void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end)
 }
 
 /*
- * len must be <= 8 and start must be a multiple of len.
- * Called via softmmu_template.h when code areas are written to with
- * iothread mutex not held.
- *
  * Call with all @pages in the range [@start, @start + len[ locked.
  */
 void tb_invalidate_phys_page_locked_fast(struct page_collection *pages,
@@ -1215,4 +1211,21 @@ void tb_invalidate_phys_page_locked_fast(struct page_collection *pages,
     tb_invalidate_phys_page_range__locked(pages, p, start, start + len,
                                           retaddr);
 }
+
+/*
+ * len must be <= 8 and start must be a multiple of len.
+ * Called via softmmu_template.h when code areas are written to with
+ * iothread mutex not held.
+ */
+void tb_invalidate_phys_range_fast(ram_addr_t ram_addr,
+                                   unsigned size,
+                                   uintptr_t retaddr)
+{
+    struct page_collection *pages;
+
+    pages = page_collection_lock(ram_addr, ram_addr + size);
+    tb_invalidate_phys_page_locked_fast(pages, ram_addr, size, retaddr);
+    page_collection_unlock(pages);
+}
+
 #endif /* CONFIG_USER_ONLY */
-- 
2.38.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH-for-8.0 5/5] accel/tcg: Restrict page_collection structure to system TB maintainance
  2022-12-09  9:36 [PATCH-for-8.0 0/5] accel/tcg: Restrict page_collection structure to system TB maintainance Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2022-12-09  9:36 ` [PATCH-for-8.0 4/5] accel/tcg: Factor tb_invalidate_phys_range_fast() out Philippe Mathieu-Daudé
@ 2022-12-09  9:36 ` Philippe Mathieu-Daudé
  2022-12-16 12:22   ` Alex Bennée
  4 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-09  9:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Alex Bennée, Paolo Bonzini,
	Stefan Hajnoczi, Philippe Mathieu-Daudé

Only the system emulation part of TB maintainance uses the
page_collection structure. Restrict its declaration (and the
functions requiring it) to tb-maint.c.

Convert the 'len' argument of tb_invalidate_phys_page_locked_fast()
from signed to unsigned.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/internal.h |  7 -------
 accel/tcg/tb-maint.c | 12 ++++++------
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h
index db078390b1..6edff16fb0 100644
--- a/accel/tcg/internal.h
+++ b/accel/tcg/internal.h
@@ -36,16 +36,9 @@ void page_table_config_init(void);
 #endif
 
 #ifdef CONFIG_SOFTMMU
-struct page_collection;
-void tb_invalidate_phys_page_locked_fast(struct page_collection *pages,
-                                         tb_page_addr_t start, int len,
-                                         uintptr_t retaddr);
-struct page_collection *page_collection_lock(tb_page_addr_t start,
-                                             tb_page_addr_t end);
 void tb_invalidate_phys_range_fast(ram_addr_t ram_addr,
                                    unsigned size,
                                    uintptr_t retaddr);
-void page_collection_unlock(struct page_collection *set);
 G_NORETURN void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
 #endif /* CONFIG_SOFTMMU */
 
diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index 4dc2fa1060..10d7e4b7a8 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -523,8 +523,8 @@ static gint tb_page_addr_cmp(gconstpointer ap, gconstpointer bp, gpointer udata)
  * intersecting TBs.
  * Locking order: acquire locks in ascending order of page index.
  */
-struct page_collection *
-page_collection_lock(tb_page_addr_t start, tb_page_addr_t end)
+static struct page_collection *page_collection_lock(tb_page_addr_t start,
+                                                    tb_page_addr_t end)
 {
     struct page_collection *set = g_malloc(sizeof(*set));
     tb_page_addr_t index;
@@ -568,7 +568,7 @@ page_collection_lock(tb_page_addr_t start, tb_page_addr_t end)
     return set;
 }
 
-void page_collection_unlock(struct page_collection *set)
+static void page_collection_unlock(struct page_collection *set)
 {
     /* entries are unlocked and freed via page_entry_destroy */
     g_tree_destroy(set->tree);
@@ -1196,9 +1196,9 @@ void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end)
 /*
  * Call with all @pages in the range [@start, @start + len[ locked.
  */
-void tb_invalidate_phys_page_locked_fast(struct page_collection *pages,
-                                         tb_page_addr_t start, int len,
-                                         uintptr_t retaddr)
+static void tb_invalidate_phys_page_locked_fast(struct page_collection *pages,
+                                                tb_page_addr_t start,
+                                                unsigned len, uintptr_t retaddr)
 {
     PageDesc *p;
 
-- 
2.38.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH-for-8.0 1/5] accel/tcg: Restrict cpu_io_recompile() to system emulation
  2022-12-09  9:36 ` [PATCH-for-8.0 1/5] accel/tcg: Restrict cpu_io_recompile() to system emulation Philippe Mathieu-Daudé
@ 2022-12-16 12:07   ` Alex Bennée
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Bennée @ 2022-12-16 12:07 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Richard Henderson, Paolo Bonzini, Stefan Hajnoczi


Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Missed in commit 6526919224 ("accel/tcg: Restrict cpu_io_recompile()
> from other accelerators").
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH-for-8.0 2/5] accel/tcg: Remove trace events from trace-root.h
  2022-12-09  9:36 ` [PATCH-for-8.0 2/5] accel/tcg: Remove trace events from trace-root.h Philippe Mathieu-Daudé
@ 2022-12-16 12:09   ` Alex Bennée
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Bennée @ 2022-12-16 12:09 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Richard Henderson, Paolo Bonzini, Stefan Hajnoczi


Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Commit d9bb58e510 ("tcg: move tcg related files into accel/tcg/
> subdirectory") introduced accel/tcg/trace-events, so we don't
> need to use the root trace-events anymore.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

BTW I wonder how much of a hot-path cputlb really is to jump the hoops
of DEBUG_TLB and friends. They could certainly be rationalised.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH-for-8.0 3/5] accel/tcg: Rename tb_invalidate_phys_page_[locked_]fast()
  2022-12-09  9:36 ` [PATCH-for-8.0 3/5] accel/tcg: Rename tb_invalidate_phys_page_[locked_]fast() Philippe Mathieu-Daudé
@ 2022-12-16 12:11   ` Alex Bennée
  2022-12-16 17:31   ` Richard Henderson
  1 sibling, 0 replies; 12+ messages in thread
From: Alex Bennée @ 2022-12-16 12:11 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Richard Henderson, Paolo Bonzini, Stefan Hajnoczi


Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Emphasize this function is called with pages locked.

The _locked should be a suffix rather than in the function and possibly
a __locked for continuity with the other __locked functions in the file.

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  accel/tcg/cputlb.c   | 2 +-
>  accel/tcg/internal.h | 6 +++---
>  accel/tcg/tb-maint.c | 6 +++---
>  3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index ac459478f4..6402fe11c1 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1510,7 +1510,7 @@ static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size,
>      if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {
>          struct page_collection *pages
>              = page_collection_lock(ram_addr, ram_addr + size);
> -        tb_invalidate_phys_page_fast(pages, ram_addr, size, retaddr);
> +        tb_invalidate_phys_page_locked_fast(pages, ram_addr, size, retaddr);
>          page_collection_unlock(pages);
>      }
>  
> diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h
> index 35419f3fe1..d8b58c1e70 100644
> --- a/accel/tcg/internal.h
> +++ b/accel/tcg/internal.h
> @@ -37,9 +37,9 @@ void page_table_config_init(void);
>  
>  #ifdef CONFIG_SOFTMMU
>  struct page_collection;
> -void tb_invalidate_phys_page_fast(struct page_collection *pages,
> -                                  tb_page_addr_t start, int len,
> -                                  uintptr_t retaddr);
> +void tb_invalidate_phys_page_locked_fast(struct page_collection *pages,
> +                                         tb_page_addr_t start, int len,
> +                                         uintptr_t retaddr);
>  struct page_collection *page_collection_lock(tb_page_addr_t start,
>                                               tb_page_addr_t end);
>  void page_collection_unlock(struct page_collection *set);
> diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
> index 0c56e81d8c..bf84728910 100644
> --- a/accel/tcg/tb-maint.c
> +++ b/accel/tcg/tb-maint.c
> @@ -1200,9 +1200,9 @@ void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end)
>   *
>   * Call with all @pages in the range [@start, @start + len[ locked.
>   */
> -void tb_invalidate_phys_page_fast(struct page_collection *pages,
> -                                  tb_page_addr_t start, int len,
> -                                  uintptr_t retaddr)
> +void tb_invalidate_phys_page_locked_fast(struct page_collection *pages,
> +                                         tb_page_addr_t start, int len,
> +                                         uintptr_t retaddr)
>  {
>      PageDesc *p;


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH-for-8.0 4/5] accel/tcg: Factor tb_invalidate_phys_range_fast() out
  2022-12-09  9:36 ` [PATCH-for-8.0 4/5] accel/tcg: Factor tb_invalidate_phys_range_fast() out Philippe Mathieu-Daudé
@ 2022-12-16 12:17   ` Alex Bennée
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Bennée @ 2022-12-16 12:17 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Richard Henderson, Paolo Bonzini, Stefan Hajnoczi


Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH-for-8.0 5/5] accel/tcg: Restrict page_collection structure to system TB maintainance
  2022-12-09  9:36 ` [PATCH-for-8.0 5/5] accel/tcg: Restrict page_collection structure to system TB maintainance Philippe Mathieu-Daudé
@ 2022-12-16 12:22   ` Alex Bennée
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Bennée @ 2022-12-16 12:22 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Richard Henderson, Paolo Bonzini, Stefan Hajnoczi


Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Only the system emulation part of TB maintainance uses the
> page_collection structure. Restrict its declaration (and the
> functions requiring it) to tb-maint.c.
>
> Convert the 'len' argument of tb_invalidate_phys_page_locked_fast()
> from signed to unsigned.

You could push that cleanup higher because I think we only ever have
DATA_SIZE which is always in a fixed range.

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  accel/tcg/internal.h |  7 -------
>  accel/tcg/tb-maint.c | 12 ++++++------
>  2 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h
> index db078390b1..6edff16fb0 100644
> --- a/accel/tcg/internal.h
> +++ b/accel/tcg/internal.h
> @@ -36,16 +36,9 @@ void page_table_config_init(void);
>  #endif
>  
>  #ifdef CONFIG_SOFTMMU
> -struct page_collection;
> -void tb_invalidate_phys_page_locked_fast(struct page_collection *pages,
> -                                         tb_page_addr_t start, int len,
> -                                         uintptr_t retaddr);
> -struct page_collection *page_collection_lock(tb_page_addr_t start,
> -                                             tb_page_addr_t end);
>  void tb_invalidate_phys_range_fast(ram_addr_t ram_addr,
>                                     unsigned size,
>                                     uintptr_t retaddr);
> -void page_collection_unlock(struct page_collection *set);
>  G_NORETURN void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
>  #endif /* CONFIG_SOFTMMU */
>  
> diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
> index 4dc2fa1060..10d7e4b7a8 100644
> --- a/accel/tcg/tb-maint.c
> +++ b/accel/tcg/tb-maint.c
> @@ -523,8 +523,8 @@ static gint tb_page_addr_cmp(gconstpointer ap, gconstpointer bp, gpointer udata)
>   * intersecting TBs.
>   * Locking order: acquire locks in ascending order of page index.
>   */
> -struct page_collection *
> -page_collection_lock(tb_page_addr_t start, tb_page_addr_t end)
> +static struct page_collection *page_collection_lock(tb_page_addr_t start,
> +                                                    tb_page_addr_t end)
>  {
>      struct page_collection *set = g_malloc(sizeof(*set));
>      tb_page_addr_t index;
> @@ -568,7 +568,7 @@ page_collection_lock(tb_page_addr_t start, tb_page_addr_t end)
>      return set;
>  }
>  
> -void page_collection_unlock(struct page_collection *set)
> +static void page_collection_unlock(struct page_collection *set)
>  {
>      /* entries are unlocked and freed via page_entry_destroy */
>      g_tree_destroy(set->tree);
> @@ -1196,9 +1196,9 @@ void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end)
>  /*
>   * Call with all @pages in the range [@start, @start + len[ locked.
>   */
> -void tb_invalidate_phys_page_locked_fast(struct page_collection *pages,
> -                                         tb_page_addr_t start, int len,
> -                                         uintptr_t retaddr)
> +static void tb_invalidate_phys_page_locked_fast(struct page_collection *pages,
> +                                                tb_page_addr_t start,
> +                                                unsigned len, uintptr_t retaddr)
>  {
>      PageDesc *p;


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH-for-8.0 3/5] accel/tcg: Rename tb_invalidate_phys_page_[locked_]fast()
  2022-12-09  9:36 ` [PATCH-for-8.0 3/5] accel/tcg: Rename tb_invalidate_phys_page_[locked_]fast() Philippe Mathieu-Daudé
  2022-12-16 12:11   ` Alex Bennée
@ 2022-12-16 17:31   ` Richard Henderson
  1 sibling, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2022-12-16 17:31 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Alex Bennée, Paolo Bonzini, Stefan Hajnoczi

On 12/9/22 01:36, Philippe Mathieu-Daudé wrote:
> Emphasize this function is called with pages locked.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   accel/tcg/cputlb.c   | 2 +-
>   accel/tcg/internal.h | 6 +++---
>   accel/tcg/tb-maint.c | 6 +++---
>   3 files changed, 7 insertions(+), 7 deletions(-)

Our other subroutines of the same form use the suffix "__locked".
As this is my only quibble with the series, I'll fix it up when applying.


r~


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-12-16 17:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-09  9:36 [PATCH-for-8.0 0/5] accel/tcg: Restrict page_collection structure to system TB maintainance Philippe Mathieu-Daudé
2022-12-09  9:36 ` [PATCH-for-8.0 1/5] accel/tcg: Restrict cpu_io_recompile() to system emulation Philippe Mathieu-Daudé
2022-12-16 12:07   ` Alex Bennée
2022-12-09  9:36 ` [PATCH-for-8.0 2/5] accel/tcg: Remove trace events from trace-root.h Philippe Mathieu-Daudé
2022-12-16 12:09   ` Alex Bennée
2022-12-09  9:36 ` [PATCH-for-8.0 3/5] accel/tcg: Rename tb_invalidate_phys_page_[locked_]fast() Philippe Mathieu-Daudé
2022-12-16 12:11   ` Alex Bennée
2022-12-16 17:31   ` Richard Henderson
2022-12-09  9:36 ` [PATCH-for-8.0 4/5] accel/tcg: Factor tb_invalidate_phys_range_fast() out Philippe Mathieu-Daudé
2022-12-16 12:17   ` Alex Bennée
2022-12-09  9:36 ` [PATCH-for-8.0 5/5] accel/tcg: Restrict page_collection structure to system TB maintainance Philippe Mathieu-Daudé
2022-12-16 12:22   ` Alex Bennée

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.