Re: [PATCH v2 04/33] accel/tcg: Reorg translator_ld*

2024-05-08 Thread Philippe Mathieu-Daudé

On 7/5/24 18:49, Richard Henderson wrote:

On 5/6/24 15:47, Philippe Mathieu-Daudé wrote:

On 25/4/24 01:31, Richard Henderson wrote:

Reorg translator_access into translator_ld, with a more
memcpy-ish interface.  If both pages are in ram, do not
go through the caller's slow path.

Assert that the access is within the two pages that we are
prepared to protect, per TranslationBlock.  Allow access
prior to pc_first, so long as it is within the first page.

Signed-off-by: Richard Henderson 
---
  accel/tcg/translator.c | 189 ++---
  1 file changed, 101 insertions(+), 88 deletions(-)



  uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, 
vaddr pc)

  {
-    uint64_t ret, plug;
-    void *p = translator_access(env, db, pc, sizeof(ret));
+    uint64_t raw, tgt;
-    if (p) {
-    plugin_insn_append(pc, p, sizeof(ret));
-    return ldq_p(p);
+    if (translator_ld(env, db, , pc, sizeof(raw))) {
+    tgt = tswap64(raw);
+    } else {
+    tgt = cpu_ldl_code(env, pc);


cpu_ldq_code() ?


Oops, yes indeed.  Fixed.


Reviewed-by: Philippe Mathieu-Daudé 





Re: [PATCH v2 04/33] accel/tcg: Reorg translator_ld*

2024-05-07 Thread Richard Henderson

On 5/6/24 15:47, Philippe Mathieu-Daudé wrote:

On 25/4/24 01:31, Richard Henderson wrote:

Reorg translator_access into translator_ld, with a more
memcpy-ish interface.  If both pages are in ram, do not
go through the caller's slow path.

Assert that the access is within the two pages that we are
prepared to protect, per TranslationBlock.  Allow access
prior to pc_first, so long as it is within the first page.

Signed-off-by: Richard Henderson 
---
  accel/tcg/translator.c | 189 ++---
  1 file changed, 101 insertions(+), 88 deletions(-)




  uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
  {
-    uint64_t ret, plug;
-    void *p = translator_access(env, db, pc, sizeof(ret));
+    uint64_t raw, tgt;
-    if (p) {
-    plugin_insn_append(pc, p, sizeof(ret));
-    return ldq_p(p);
+    if (translator_ld(env, db, , pc, sizeof(raw))) {
+    tgt = tswap64(raw);
+    } else {
+    tgt = cpu_ldl_code(env, pc);


cpu_ldq_code() ?


Oops, yes indeed.  Fixed.

r~



Re: [PATCH v2 04/33] accel/tcg: Reorg translator_ld*

2024-05-06 Thread Philippe Mathieu-Daudé

On 25/4/24 01:31, Richard Henderson wrote:

Reorg translator_access into translator_ld, with a more
memcpy-ish interface.  If both pages are in ram, do not
go through the caller's slow path.

Assert that the access is within the two pages that we are
prepared to protect, per TranslationBlock.  Allow access
prior to pc_first, so long as it is within the first page.

Signed-off-by: Richard Henderson 
---
  accel/tcg/translator.c | 189 ++---
  1 file changed, 101 insertions(+), 88 deletions(-)




  uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
  {
-uint64_t ret, plug;
-void *p = translator_access(env, db, pc, sizeof(ret));
+uint64_t raw, tgt;
  
-if (p) {

-plugin_insn_append(pc, p, sizeof(ret));
-return ldq_p(p);
+if (translator_ld(env, db, , pc, sizeof(raw))) {
+tgt = tswap64(raw);
+} else {
+tgt = cpu_ldl_code(env, pc);


cpu_ldq_code() ?


+raw = tswap64(tgt);
  }
-ret = cpu_ldq_code(env, pc);
-plug = tswap64(ret);
-plugin_insn_append(pc, , sizeof(ret));
-return ret;
+plugin_insn_append(pc, , sizeof(raw));
+return tgt;
  }






[PATCH v2 04/33] accel/tcg: Reorg translator_ld*

2024-04-24 Thread Richard Henderson
Reorg translator_access into translator_ld, with a more
memcpy-ish interface.  If both pages are in ram, do not
go through the caller's slow path.

Assert that the access is within the two pages that we are
prepared to protect, per TranslationBlock.  Allow access
prior to pc_first, so long as it is within the first page.

Signed-off-by: Richard Henderson 
---
 accel/tcg/translator.c | 189 ++---
 1 file changed, 101 insertions(+), 88 deletions(-)

diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 92eb77c3a0..dbd54e25a2 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -229,69 +229,88 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, 
int *max_insns,
 }
 }
 
-static void *translator_access(CPUArchState *env, DisasContextBase *db,
-   vaddr pc, size_t len)
+static bool translator_ld(CPUArchState *env, DisasContextBase *db,
+  void *dest, vaddr pc, size_t len)
 {
+TranslationBlock *tb = db->tb;
+vaddr last = pc + len - 1;
 void *host;
-vaddr base, end;
-TranslationBlock *tb;
-
-tb = db->tb;
+vaddr base;
 
 /* Use slow path if first page is MMIO. */
 if (unlikely(tb_page_addr0(tb) == -1)) {
-return NULL;
+return false;
 }
 
-end = pc + len - 1;
-if (likely(is_same_page(db, end))) {
-host = db->host_addr[0];
-base = db->pc_first;
-} else {
+host = db->host_addr[0];
+base = db->pc_first;
+
+if (likely(((base ^ last) & TARGET_PAGE_MASK) == 0)) {
+/* Entire read is from the first page. */
+memcpy(dest, host + (pc - base), len);
+return true;
+}
+
+if (unlikely(((base ^ pc) & TARGET_PAGE_MASK) == 0)) {
+/* Read begins on the first page and extends to the second. */
+size_t len0 = -(pc | TARGET_PAGE_MASK);
+memcpy(dest, host + (pc - base), len0);
+pc += len0;
+dest += len0;
+len -= len0;
+}
+
+/*
+ * The read must conclude on the second page and not extend to a third.
+ *
+ * TODO: We could allow the two pages to be virtually discontiguous,
+ * since we already allow the two pages to be physically discontiguous.
+ * The only reasonable use case would be executing an insn at the end
+ * of the address space wrapping around to the beginning.  For that,
+ * we would need to know the current width of the address space.
+ * In the meantime, assert.
+ */
+base = (base & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+assert(((base ^ pc) & TARGET_PAGE_MASK) == 0);
+assert(((base ^ last) & TARGET_PAGE_MASK) == 0);
+host = db->host_addr[1];
+
+if (host == NULL) {
+tb_page_addr_t page0, old_page1, new_page1;
+
+new_page1 = get_page_addr_code_hostp(env, base, >host_addr[1]);
+
+/*
+ * If the second page is MMIO, treat as if the first page
+ * was MMIO as well, so that we do not cache the TB.
+ */
+if (unlikely(new_page1 == -1)) {
+tb_unlock_pages(tb);
+tb_set_page_addr0(tb, -1);
+return false;
+}
+
+/*
+ * If this is not the first time around, and page1 matches,
+ * then we already have the page locked.  Alternately, we're
+ * not doing anything to prevent the PTE from changing, so
+ * we might wind up with a different page, requiring us to
+ * re-do the locking.
+ */
+old_page1 = tb_page_addr1(tb);
+if (likely(new_page1 != old_page1)) {
+page0 = tb_page_addr0(tb);
+if (unlikely(old_page1 != -1)) {
+tb_unlock_page1(page0, old_page1);
+}
+tb_set_page_addr1(tb, new_page1);
+tb_lock_page1(page0, new_page1);
+}
 host = db->host_addr[1];
-base = TARGET_PAGE_ALIGN(db->pc_first);
-if (host == NULL) {
-tb_page_addr_t page0, old_page1, new_page1;
-
-new_page1 = get_page_addr_code_hostp(env, base, >host_addr[1]);
-
-/*
- * If the second page is MMIO, treat as if the first page
- * was MMIO as well, so that we do not cache the TB.
- */
-if (unlikely(new_page1 == -1)) {
-tb_unlock_pages(tb);
-tb_set_page_addr0(tb, -1);
-return NULL;
-}
-
-/*
- * If this is not the first time around, and page1 matches,
- * then we already have the page locked.  Alternately, we're
- * not doing anything to prevent the PTE from changing, so
- * we might wind up with a different page, requiring us to
- * re-do the locking.
- */
-old_page1 = tb_page_addr1(tb);
-if (likely(new_page1 != old_page1)) {
-page0 = tb_page_addr0(tb);
-if