* [PATCH 01/16] accel/tcg: Add block comment for probe_access
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 11:50 ` Peter Maydell
2020-03-11 6:44 ` [PATCH 02/16] accel/tcg: Add probe_access_flags Richard Henderson
` (16 subsequent siblings)
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/exec-all.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 350c4b451b..d656a1f05c 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -330,6 +330,23 @@ static inline void tlb_flush_by_mmuidx_all_cpus_synced(CPUState *cpu,
{
}
#endif
+/**
+ * probe_access:
+ * @env: CPUArchState
+ * @addr: guest virtual address to look up
+ * @size: size of the access
+ * @access_type: read, write or execute permission
+ * @mmu_idx: MMU index to use for lookup
+ * @retaddr: return address for unwinding
+ *
+ * Look up the guest virtual address @addr. Raise an exception if the
+ * page does not satisfy @access_type. Raise an exception if the
+ * access (@addr, @size) hits a watchpoint. For writes, mark a clean
+ * page as dirty.
+ *
+ * Finally, return the host address for a page that is backed by RAM,
+ * or NULL if the page requires I/O.
+ */
void *probe_access(CPUArchState *env, target_ulong addr, int size,
MMUAccessType access_type, int mmu_idx, uintptr_t retaddr);
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH 02/16] accel/tcg: Add probe_access_flags
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
2020-03-11 6:44 ` [PATCH 01/16] accel/tcg: Add block comment for probe_access Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-03-12 4:33 ` [PATCH v2 " Richard Henderson
2020-03-12 4:38 ` [PATCH " Richard Henderson
2020-03-11 6:44 ` [PATCH 03/16] exec: Add cpu_probe_watchpoint Richard Henderson
` (15 subsequent siblings)
17 siblings, 2 replies; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
This new interface will allow targets to probe for a page
and then handle watchpoints themselves. This will be most
useful for vector predicated memory operations, where one
page lookup can be used for many operations, and one test
can avoid many watchpoint checks.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/cpu-all.h | 13 ++-
include/exec/exec-all.h | 22 +++++
accel/tcg/cputlb.c | 178 ++++++++++++++++++++--------------------
accel/tcg/user-exec.c | 36 +++++---
4 files changed, 150 insertions(+), 99 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 49384bb66a..43ddcf024c 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -328,7 +328,18 @@ CPUArchState *cpu_copy(CPUArchState *env);
| CPU_INTERRUPT_TGT_EXT_3 \
| CPU_INTERRUPT_TGT_EXT_4)
-#if !defined(CONFIG_USER_ONLY)
+#ifdef CONFIG_USER_ONLY
+
+/*
+ * Allow some level of source compatibility with softmmu. We do not
+ * support any of the more exotic features, so only invalid pages may
+ * be signaled by probe_access_flags().
+ */
+#define TLB_INVALID_MASK (1 << (TARGET_PAGE_BITS_MIN - 1))
+#define TLB_MMIO 0
+#define TLB_WATCHPOINT 0
+
+#else
/*
* Flags stored in the low bits of the TLB virtual address.
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index d656a1f05c..8792bea07a 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -362,6 +362,28 @@ static inline void *probe_read(CPUArchState *env, target_ulong addr, int size,
return probe_access(env, addr, size, MMU_DATA_LOAD, mmu_idx, retaddr);
}
+/**
+ * probe_access_flags:
+ * @env: CPUArchState
+ * @addr: guest virtual address to look up
+ * @access_type: read, write or execute permission
+ * @mmu_idx: MMU index to use for lookup
+ * @nonfault: suppress the fault
+ * @phost: return value for host address
+ * @retaddr: return address for unwinding
+ *
+ * Similar to probe_access, loosely returning the TLB_FLAGS_MASK for
+ * the page, and storing the host address for RAM in @phost.
+ *
+ * If @nonfault is set, do not raise an exception but return TLB_INVALID_MASK.
+ * Do not handle watchpoints, but include TLB_WATCHPOINT in the returned flags.
+ * Do handle clean pages, so exclude TLB_NOTDIRY from the returned flags.
+ * For simplicity, all "mmio-like" flags are folded to TLB_MMIO.
+ */
+int probe_access_flags(CPUArchState *env, target_ulong addr,
+ MMUAccessType access_type, int mmu_idx,
+ bool nonfault, void **phost, uintptr_t retaddr);
+
#define CODE_GEN_ALIGN 16 /* must be >= of the size of a icache line */
/* Estimated block size for TB allocation. */
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index e3b5750c3b..687474ccea 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1231,86 +1231,16 @@ static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size,
}
}
-/*
- * Probe for whether the specified guest access is permitted. If it is not
- * permitted then an exception will be taken in the same way as if this
- * were a real access (and we will not return).
- * If the size is 0 or the page requires I/O access, returns NULL; otherwise,
- * returns the address of the host page similar to tlb_vaddr_to_host().
- */
-void *probe_access(CPUArchState *env, target_ulong addr, int size,
- MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+static int probe_access_internal(CPUArchState *env, target_ulong addr,
+ int fault_size, MMUAccessType access_type,
+ int mmu_idx, bool nonfault,
+ void **phost, uintptr_t retaddr)
{
uintptr_t index = tlb_index(env, mmu_idx, addr);
CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
- target_ulong tlb_addr;
- size_t elt_ofs;
- int wp_access;
-
- g_assert(-(addr | TARGET_PAGE_MASK) >= size);
-
- switch (access_type) {
- case MMU_DATA_LOAD:
- elt_ofs = offsetof(CPUTLBEntry, addr_read);
- wp_access = BP_MEM_READ;
- break;
- case MMU_DATA_STORE:
- elt_ofs = offsetof(CPUTLBEntry, addr_write);
- wp_access = BP_MEM_WRITE;
- break;
- case MMU_INST_FETCH:
- elt_ofs = offsetof(CPUTLBEntry, addr_code);
- wp_access = BP_MEM_READ;
- break;
- default:
- g_assert_not_reached();
- }
- tlb_addr = tlb_read_ofs(entry, elt_ofs);
-
- if (unlikely(!tlb_hit(tlb_addr, addr))) {
- if (!victim_tlb_hit(env, mmu_idx, index, elt_ofs,
- addr & TARGET_PAGE_MASK)) {
- tlb_fill(env_cpu(env), addr, size, access_type, mmu_idx, retaddr);
- /* TLB resize via tlb_fill may have moved the entry. */
- index = tlb_index(env, mmu_idx, addr);
- entry = tlb_entry(env, mmu_idx, addr);
- }
- tlb_addr = tlb_read_ofs(entry, elt_ofs);
- }
-
- if (!size) {
- return NULL;
- }
-
- if (unlikely(tlb_addr & TLB_FLAGS_MASK)) {
- CPUIOTLBEntry *iotlbentry = &env_tlb(env)->d[mmu_idx].iotlb[index];
-
- /* Reject I/O access, or other required slow-path. */
- if (tlb_addr & (TLB_MMIO | TLB_BSWAP | TLB_DISCARD_WRITE)) {
- return NULL;
- }
-
- /* Handle watchpoints. */
- if (tlb_addr & TLB_WATCHPOINT) {
- cpu_check_watchpoint(env_cpu(env), addr, size,
- iotlbentry->attrs, wp_access, retaddr);
- }
-
- /* Handle clean RAM pages. */
- if (tlb_addr & TLB_NOTDIRTY) {
- notdirty_write(env_cpu(env), addr, size, iotlbentry, retaddr);
- }
- }
-
- return (void *)((uintptr_t)addr + entry->addend);
-}
-
-void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
- MMUAccessType access_type, int mmu_idx)
-{
- CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
- target_ulong tlb_addr, page;
+ target_ulong tlb_addr, page_addr;
size_t elt_ofs;
+ int flags;
switch (access_type) {
case MMU_DATA_LOAD:
@@ -1325,20 +1255,19 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
default:
g_assert_not_reached();
}
-
- page = addr & TARGET_PAGE_MASK;
tlb_addr = tlb_read_ofs(entry, elt_ofs);
- if (!tlb_hit_page(tlb_addr, page)) {
- uintptr_t index = tlb_index(env, mmu_idx, addr);
-
- if (!victim_tlb_hit(env, mmu_idx, index, elt_ofs, page)) {
+ page_addr = addr & TARGET_PAGE_MASK;
+ if (!tlb_hit_page(tlb_addr, page_addr)) {
+ if (!victim_tlb_hit(env, mmu_idx, index, elt_ofs, page_addr)) {
CPUState *cs = env_cpu(env);
CPUClass *cc = CPU_GET_CLASS(cs);
- if (!cc->tlb_fill(cs, addr, 0, access_type, mmu_idx, true, 0)) {
+ if (!cc->tlb_fill(cs, addr, fault_size, access_type,
+ mmu_idx, nonfault, retaddr)) {
/* Non-faulting page table read failed. */
- return NULL;
+ *phost = NULL;
+ return TLB_INVALID_MASK;
}
/* TLB resize via tlb_fill may have moved the entry. */
@@ -1346,15 +1275,90 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
}
tlb_addr = tlb_read_ofs(entry, elt_ofs);
}
+ flags = tlb_addr & TLB_FLAGS_MASK;
- if (tlb_addr & ~TARGET_PAGE_MASK) {
- /* IO access */
+ /* Fold all "mmio-like" bits into TLB_MMIO. This is not RAM. */
+ if (unlikely(flags & ~(TLB_WATCHPOINT | TLB_NOTDIRTY))) {
+ *phost = NULL;
+ return TLB_MMIO;
+ }
+
+ /* Everything else is RAM. */
+ *phost = (void *)((uintptr_t)addr + entry->addend);
+ return flags;
+}
+
+int probe_access_flags(CPUArchState *env, target_ulong addr,
+ MMUAccessType access_type, int mmu_idx,
+ bool nonfault, void **phost, uintptr_t retaddr)
+{
+ void *host;
+ int flags;
+
+ flags = probe_access_internal(env, addr, 0, access_type, mmu_idx,
+ nonfault, &host, retaddr);
+
+ /* Handle clean RAM pages. */
+ if (unlikely(flags & TLB_NOTDIRTY)) {
+ uintptr_t index = tlb_index(env, mmu_idx, addr);
+ CPUIOTLBEntry *iotlbentry = &env_tlb(env)->d[mmu_idx].iotlb[index];
+
+ notdirty_write(env_cpu(env), addr, 1, iotlbentry, retaddr);
+ flags &= ~TLB_NOTDIRTY;
+ }
+
+ return flags;
+}
+
+void *probe_access(CPUArchState *env, target_ulong addr, int size,
+ MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+{
+ void *host;
+ int flags;
+
+ g_assert(-(addr | TARGET_PAGE_MASK) >= size);
+
+ flags = probe_access_internal(env, addr, size, access_type, mmu_idx,
+ false, &host, retaddr);
+
+ /* Per the interface, size == 0 merely faults the access. */
+ if (size == 0) {
return NULL;
}
- return (void *)((uintptr_t)addr + entry->addend);
+ if (unlikely(flags & (TLB_NOTDIRTY | TLB_WATCHPOINT))) {
+ uintptr_t index = tlb_index(env, mmu_idx, addr);
+ CPUIOTLBEntry *iotlbentry = &env_tlb(env)->d[mmu_idx].iotlb[index];
+
+ /* Handle clean RAM pages. */
+ if (flags & TLB_NOTDIRTY) {
+ notdirty_write(env_cpu(env), addr, 1, iotlbentry, retaddr);
+ }
+
+ /* Handle watchpoints. */
+ if (flags & TLB_WATCHPOINT) {
+ int wp_access = (access_type == MMU_DATA_STORE
+ ? BP_MEM_WRITE : BP_MEM_READ);
+ cpu_check_watchpoint(env_cpu(env), addr, size,
+ iotlbentry->attrs, wp_access, retaddr);
+ }
+ }
+
+ return host;
}
+void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
+ MMUAccessType access_type, int mmu_idx)
+{
+ void *host;
+ int flags;
+
+ flags = probe_access_internal(env, addr, 0, access_type,
+ mmu_idx, true, &host, 0);
+
+ /* No combination of flags are expected by the caller. */
+ return flags ? NULL : host;
+}
#ifdef CONFIG_PLUGIN
/*
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 4be78eb9b3..c52dd8a95a 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -190,13 +190,12 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,
g_assert_not_reached();
}
-void *probe_access(CPUArchState *env, target_ulong addr, int size,
- MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+int probe_access_flags(CPUArchState *env, target_ulong addr,
+ MMUAccessType access_type, int mmu_idx,
+ bool nonfault, void **phost, uintptr_t retaddr)
{
int flags;
- g_assert(-(addr | TARGET_PAGE_MASK) >= size);
-
switch (access_type) {
case MMU_DATA_STORE:
flags = PAGE_WRITE;
@@ -211,15 +210,30 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size,
g_assert_not_reached();
}
- if (!guest_addr_valid(addr) || page_check_range(addr, size, flags) < 0) {
- CPUState *cpu = env_cpu(env);
- CPUClass *cc = CPU_GET_CLASS(cpu);
- cc->tlb_fill(cpu, addr, size, access_type, MMU_USER_IDX, false,
- retaddr);
- g_assert_not_reached();
+ if (!guest_addr_valid(addr) || page_check_range(addr, 1, flags) < 0) {
+ if (nonfault) {
+ *phost = NULL;
+ return TLB_INVALID_MASK;
+ } else {
+ CPUState *cpu = env_cpu(env);
+ CPUClass *cc = CPU_GET_CLASS(cpu);
+ cc->tlb_fill(cpu, addr, 0, access_type, MMU_USER_IDX, false, retaddr);
+ g_assert_not_reached();
+ }
}
- return size ? g2h(addr) : NULL;
+ *phost = g2h(addr);
+ return 0;
+}
+
+void *probe_access(CPUArchState *env, target_ulong addr, int size,
+ MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+{
+ void *host;
+
+ g_assert(-(addr | TARGET_PAGE_MASK) >= size);
+ probe_access_flags(env, addr, access_type, mmu_idx, false, &host, retaddr);
+ return host;
}
#if defined(__i386__)
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH v2 02/16] accel/tcg: Add probe_access_flags
2020-03-11 6:44 ` [PATCH 02/16] accel/tcg: Add probe_access_flags Richard Henderson
@ 2020-03-12 4:33 ` Richard Henderson
2020-04-16 11:54 ` Peter Maydell
2020-03-12 4:38 ` [PATCH " Richard Henderson
1 sibling, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-12 4:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemuarm
This new interface will allow targets to probe for a page
and then handle watchpoints themselves. This will be most
useful for vector predicated memory operations, where one
page lookup can be used for many operations, and one test
can avoid many watchpoint checks.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v2: Fix return of host pointer in softmmu probe_access_flags.
---
include/exec/cpu-all.h | 13 ++-
include/exec/exec-all.h | 22 +++++
accel/tcg/cputlb.c | 177 ++++++++++++++++++++--------------------
accel/tcg/user-exec.c | 36 +++++---
4 files changed, 149 insertions(+), 99 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 49384bb66a..43ddcf024c 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -328,7 +328,18 @@ CPUArchState *cpu_copy(CPUArchState *env);
| CPU_INTERRUPT_TGT_EXT_3 \
| CPU_INTERRUPT_TGT_EXT_4)
-#if !defined(CONFIG_USER_ONLY)
+#ifdef CONFIG_USER_ONLY
+
+/*
+ * Allow some level of source compatibility with softmmu. We do not
+ * support any of the more exotic features, so only invalid pages may
+ * be signaled by probe_access_flags().
+ */
+#define TLB_INVALID_MASK (1 << (TARGET_PAGE_BITS_MIN - 1))
+#define TLB_MMIO 0
+#define TLB_WATCHPOINT 0
+
+#else
/*
* Flags stored in the low bits of the TLB virtual address.
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index d656a1f05c..8792bea07a 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -362,6 +362,28 @@ static inline void *probe_read(CPUArchState *env, target_ulong addr, int size,
return probe_access(env, addr, size, MMU_DATA_LOAD, mmu_idx, retaddr);
}
+/**
+ * probe_access_flags:
+ * @env: CPUArchState
+ * @addr: guest virtual address to look up
+ * @access_type: read, write or execute permission
+ * @mmu_idx: MMU index to use for lookup
+ * @nonfault: suppress the fault
+ * @phost: return value for host address
+ * @retaddr: return address for unwinding
+ *
+ * Similar to probe_access, loosely returning the TLB_FLAGS_MASK for
+ * the page, and storing the host address for RAM in @phost.
+ *
+ * If @nonfault is set, do not raise an exception but return TLB_INVALID_MASK.
+ * Do not handle watchpoints, but include TLB_WATCHPOINT in the returned flags.
+ * Do handle clean pages, so exclude TLB_NOTDIRY from the returned flags.
+ * For simplicity, all "mmio-like" flags are folded to TLB_MMIO.
+ */
+int probe_access_flags(CPUArchState *env, target_ulong addr,
+ MMUAccessType access_type, int mmu_idx,
+ bool nonfault, void **phost, uintptr_t retaddr);
+
#define CODE_GEN_ALIGN 16 /* must be >= of the size of a icache line */
/* Estimated block size for TB allocation. */
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index e3b5750c3b..bbe265ce28 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1231,86 +1231,16 @@ static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size,
}
}
-/*
- * Probe for whether the specified guest access is permitted. If it is not
- * permitted then an exception will be taken in the same way as if this
- * were a real access (and we will not return).
- * If the size is 0 or the page requires I/O access, returns NULL; otherwise,
- * returns the address of the host page similar to tlb_vaddr_to_host().
- */
-void *probe_access(CPUArchState *env, target_ulong addr, int size,
- MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+static int probe_access_internal(CPUArchState *env, target_ulong addr,
+ int fault_size, MMUAccessType access_type,
+ int mmu_idx, bool nonfault,
+ void **phost, uintptr_t retaddr)
{
uintptr_t index = tlb_index(env, mmu_idx, addr);
CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
- target_ulong tlb_addr;
- size_t elt_ofs;
- int wp_access;
-
- g_assert(-(addr | TARGET_PAGE_MASK) >= size);
-
- switch (access_type) {
- case MMU_DATA_LOAD:
- elt_ofs = offsetof(CPUTLBEntry, addr_read);
- wp_access = BP_MEM_READ;
- break;
- case MMU_DATA_STORE:
- elt_ofs = offsetof(CPUTLBEntry, addr_write);
- wp_access = BP_MEM_WRITE;
- break;
- case MMU_INST_FETCH:
- elt_ofs = offsetof(CPUTLBEntry, addr_code);
- wp_access = BP_MEM_READ;
- break;
- default:
- g_assert_not_reached();
- }
- tlb_addr = tlb_read_ofs(entry, elt_ofs);
-
- if (unlikely(!tlb_hit(tlb_addr, addr))) {
- if (!victim_tlb_hit(env, mmu_idx, index, elt_ofs,
- addr & TARGET_PAGE_MASK)) {
- tlb_fill(env_cpu(env), addr, size, access_type, mmu_idx, retaddr);
- /* TLB resize via tlb_fill may have moved the entry. */
- index = tlb_index(env, mmu_idx, addr);
- entry = tlb_entry(env, mmu_idx, addr);
- }
- tlb_addr = tlb_read_ofs(entry, elt_ofs);
- }
-
- if (!size) {
- return NULL;
- }
-
- if (unlikely(tlb_addr & TLB_FLAGS_MASK)) {
- CPUIOTLBEntry *iotlbentry = &env_tlb(env)->d[mmu_idx].iotlb[index];
-
- /* Reject I/O access, or other required slow-path. */
- if (tlb_addr & (TLB_MMIO | TLB_BSWAP | TLB_DISCARD_WRITE)) {
- return NULL;
- }
-
- /* Handle watchpoints. */
- if (tlb_addr & TLB_WATCHPOINT) {
- cpu_check_watchpoint(env_cpu(env), addr, size,
- iotlbentry->attrs, wp_access, retaddr);
- }
-
- /* Handle clean RAM pages. */
- if (tlb_addr & TLB_NOTDIRTY) {
- notdirty_write(env_cpu(env), addr, size, iotlbentry, retaddr);
- }
- }
-
- return (void *)((uintptr_t)addr + entry->addend);
-}
-
-void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
- MMUAccessType access_type, int mmu_idx)
-{
- CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
- target_ulong tlb_addr, page;
+ target_ulong tlb_addr, page_addr;
size_t elt_ofs;
+ int flags;
switch (access_type) {
case MMU_DATA_LOAD:
@@ -1325,20 +1255,19 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
default:
g_assert_not_reached();
}
-
- page = addr & TARGET_PAGE_MASK;
tlb_addr = tlb_read_ofs(entry, elt_ofs);
- if (!tlb_hit_page(tlb_addr, page)) {
- uintptr_t index = tlb_index(env, mmu_idx, addr);
-
- if (!victim_tlb_hit(env, mmu_idx, index, elt_ofs, page)) {
+ page_addr = addr & TARGET_PAGE_MASK;
+ if (!tlb_hit_page(tlb_addr, page_addr)) {
+ if (!victim_tlb_hit(env, mmu_idx, index, elt_ofs, page_addr)) {
CPUState *cs = env_cpu(env);
CPUClass *cc = CPU_GET_CLASS(cs);
- if (!cc->tlb_fill(cs, addr, 0, access_type, mmu_idx, true, 0)) {
+ if (!cc->tlb_fill(cs, addr, fault_size, access_type,
+ mmu_idx, nonfault, retaddr)) {
/* Non-faulting page table read failed. */
- return NULL;
+ *phost = NULL;
+ return TLB_INVALID_MASK;
}
/* TLB resize via tlb_fill may have moved the entry. */
@@ -1346,15 +1275,89 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
}
tlb_addr = tlb_read_ofs(entry, elt_ofs);
}
+ flags = tlb_addr & TLB_FLAGS_MASK;
- if (tlb_addr & ~TARGET_PAGE_MASK) {
- /* IO access */
+ /* Fold all "mmio-like" bits into TLB_MMIO. This is not RAM. */
+ if (unlikely(flags & ~(TLB_WATCHPOINT | TLB_NOTDIRTY))) {
+ *phost = NULL;
+ return TLB_MMIO;
+ }
+
+ /* Everything else is RAM. */
+ *phost = (void *)((uintptr_t)addr + entry->addend);
+ return flags;
+}
+
+int probe_access_flags(CPUArchState *env, target_ulong addr,
+ MMUAccessType access_type, int mmu_idx,
+ bool nonfault, void **phost, uintptr_t retaddr)
+{
+ int flags;
+
+ flags = probe_access_internal(env, addr, 0, access_type, mmu_idx,
+ nonfault, phost, retaddr);
+
+ /* Handle clean RAM pages. */
+ if (unlikely(flags & TLB_NOTDIRTY)) {
+ uintptr_t index = tlb_index(env, mmu_idx, addr);
+ CPUIOTLBEntry *iotlbentry = &env_tlb(env)->d[mmu_idx].iotlb[index];
+
+ notdirty_write(env_cpu(env), addr, 1, iotlbentry, retaddr);
+ flags &= ~TLB_NOTDIRTY;
+ }
+
+ return flags;
+}
+
+void *probe_access(CPUArchState *env, target_ulong addr, int size,
+ MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+{
+ void *host;
+ int flags;
+
+ g_assert(-(addr | TARGET_PAGE_MASK) >= size);
+
+ flags = probe_access_internal(env, addr, size, access_type, mmu_idx,
+ false, &host, retaddr);
+
+ /* Per the interface, size == 0 merely faults the access. */
+ if (size == 0) {
return NULL;
}
- return (void *)((uintptr_t)addr + entry->addend);
+ if (unlikely(flags & (TLB_NOTDIRTY | TLB_WATCHPOINT))) {
+ uintptr_t index = tlb_index(env, mmu_idx, addr);
+ CPUIOTLBEntry *iotlbentry = &env_tlb(env)->d[mmu_idx].iotlb[index];
+
+ /* Handle clean RAM pages. */
+ if (flags & TLB_NOTDIRTY) {
+ notdirty_write(env_cpu(env), addr, 1, iotlbentry, retaddr);
+ }
+
+ /* Handle watchpoints. */
+ if (flags & TLB_WATCHPOINT) {
+ int wp_access = (access_type == MMU_DATA_STORE
+ ? BP_MEM_WRITE : BP_MEM_READ);
+ cpu_check_watchpoint(env_cpu(env), addr, size,
+ iotlbentry->attrs, wp_access, retaddr);
+ }
+ }
+
+ return host;
}
+void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
+ MMUAccessType access_type, int mmu_idx)
+{
+ void *host;
+ int flags;
+
+ flags = probe_access_internal(env, addr, 0, access_type,
+ mmu_idx, true, &host, 0);
+
+ /* No combination of flags are expected by the caller. */
+ return flags ? NULL : host;
+}
#ifdef CONFIG_PLUGIN
/*
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 4be78eb9b3..c52dd8a95a 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -190,13 +190,12 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,
g_assert_not_reached();
}
-void *probe_access(CPUArchState *env, target_ulong addr, int size,
- MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+int probe_access_flags(CPUArchState *env, target_ulong addr,
+ MMUAccessType access_type, int mmu_idx,
+ bool nonfault, void **phost, uintptr_t retaddr)
{
int flags;
- g_assert(-(addr | TARGET_PAGE_MASK) >= size);
-
switch (access_type) {
case MMU_DATA_STORE:
flags = PAGE_WRITE;
@@ -211,15 +210,30 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size,
g_assert_not_reached();
}
- if (!guest_addr_valid(addr) || page_check_range(addr, size, flags) < 0) {
- CPUState *cpu = env_cpu(env);
- CPUClass *cc = CPU_GET_CLASS(cpu);
- cc->tlb_fill(cpu, addr, size, access_type, MMU_USER_IDX, false,
- retaddr);
- g_assert_not_reached();
+ if (!guest_addr_valid(addr) || page_check_range(addr, 1, flags) < 0) {
+ if (nonfault) {
+ *phost = NULL;
+ return TLB_INVALID_MASK;
+ } else {
+ CPUState *cpu = env_cpu(env);
+ CPUClass *cc = CPU_GET_CLASS(cpu);
+ cc->tlb_fill(cpu, addr, 0, access_type, MMU_USER_IDX, false, retaddr);
+ g_assert_not_reached();
+ }
}
- return size ? g2h(addr) : NULL;
+ *phost = g2h(addr);
+ return 0;
+}
+
+void *probe_access(CPUArchState *env, target_ulong addr, int size,
+ MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+{
+ void *host;
+
+ g_assert(-(addr | TARGET_PAGE_MASK) >= size);
+ probe_access_flags(env, addr, access_type, mmu_idx, false, &host, retaddr);
+ return host;
}
#if defined(__i386__)
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v2 02/16] accel/tcg: Add probe_access_flags
2020-03-12 4:33 ` [PATCH v2 " Richard Henderson
@ 2020-04-16 11:54 ` Peter Maydell
2020-04-18 0:05 ` Richard Henderson
0 siblings, 1 reply; 44+ messages in thread
From: Peter Maydell @ 2020-04-16 11:54 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemuarm, QEMU Developers
On Thu, 12 Mar 2020 at 04:34, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This new interface will allow targets to probe for a page
> and then handle watchpoints themselves. This will be most
> useful for vector predicated memory operations, where one
> page lookup can be used for many operations, and one test
> can avoid many watchpoint checks.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v2: Fix return of host pointer in softmmu probe_access_flags.
> ---
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index e3b5750c3b..bbe265ce28 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1231,86 +1231,16 @@ static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size,
> }
> }
>
> -/*
> - * Probe for whether the specified guest access is permitted. If it is not
> - * permitted then an exception will be taken in the same way as if this
> - * were a real access (and we will not return).
> - * If the size is 0 or the page requires I/O access, returns NULL; otherwise,
> - * returns the address of the host page similar to tlb_vaddr_to_host().
> - */
> -void *probe_access(CPUArchState *env, target_ulong addr, int size,
> - MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
> +static int probe_access_internal(CPUArchState *env, target_ulong addr,
> + int fault_size, MMUAccessType access_type,
> + int mmu_idx, bool nonfault,
> + void **phost, uintptr_t retaddr)
> {
> uintptr_t index = tlb_index(env, mmu_idx, addr);
> CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
> - target_ulong tlb_addr;
> - size_t elt_ofs;
> - int wp_access;
> -
> - g_assert(-(addr | TARGET_PAGE_MASK) >= size);
> -
> - switch (access_type) {
> - case MMU_DATA_LOAD:
> - elt_ofs = offsetof(CPUTLBEntry, addr_read);
> - wp_access = BP_MEM_READ;
> - break;
> - case MMU_DATA_STORE:
> - elt_ofs = offsetof(CPUTLBEntry, addr_write);
> - wp_access = BP_MEM_WRITE;
> - break;
> - case MMU_INST_FETCH:
> - elt_ofs = offsetof(CPUTLBEntry, addr_code);
> - wp_access = BP_MEM_READ;
> - break;
> - default:
> - g_assert_not_reached();
> - }
> - tlb_addr = tlb_read_ofs(entry, elt_ofs);
> -
> - if (unlikely(!tlb_hit(tlb_addr, addr))) {
> - if (!victim_tlb_hit(env, mmu_idx, index, elt_ofs,
> - addr & TARGET_PAGE_MASK)) {
> - tlb_fill(env_cpu(env), addr, size, access_type, mmu_idx, retaddr);
> - /* TLB resize via tlb_fill may have moved the entry. */
> - index = tlb_index(env, mmu_idx, addr);
> - entry = tlb_entry(env, mmu_idx, addr);
> - }
> - tlb_addr = tlb_read_ofs(entry, elt_ofs);
> - }
All of the code above seems to have disappeared in this
refactoring -- it's not in probe_access_internal()
but it hasn't moved to the new probe_access().
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index 4be78eb9b3..c52dd8a95a 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -190,13 +190,12 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,
> g_assert_not_reached();
> }
>
> -void *probe_access(CPUArchState *env, target_ulong addr, int size,
> - MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
> +int probe_access_flags(CPUArchState *env, target_ulong addr,
> + MMUAccessType access_type, int mmu_idx,
> + bool nonfault, void **phost, uintptr_t retaddr)
> {
> int flags;
>
> - g_assert(-(addr | TARGET_PAGE_MASK) >= size);
> -
> switch (access_type) {
> case MMU_DATA_STORE:
> flags = PAGE_WRITE;
> @@ -211,15 +210,30 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size,
> g_assert_not_reached();
> }
>
> - if (!guest_addr_valid(addr) || page_check_range(addr, size, flags) < 0) {
> - CPUState *cpu = env_cpu(env);
> - CPUClass *cc = CPU_GET_CLASS(cpu);
> - cc->tlb_fill(cpu, addr, size, access_type, MMU_USER_IDX, false,
> - retaddr);
> - g_assert_not_reached();
> + if (!guest_addr_valid(addr) || page_check_range(addr, 1, flags) < 0) {
> + if (nonfault) {
> + *phost = NULL;
> + return TLB_INVALID_MASK;
> + } else {
> + CPUState *cpu = env_cpu(env);
> + CPUClass *cc = CPU_GET_CLASS(cpu);
> + cc->tlb_fill(cpu, addr, 0, access_type, MMU_USER_IDX, false, retaddr);
> + g_assert_not_reached();
> + }
> }
>
> - return size ? g2h(addr) : NULL;
> + *phost = g2h(addr);
> + return 0;
> +}
> +
> +void *probe_access(CPUArchState *env, target_ulong addr, int size,
> + MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
> +{
> + void *host;
> +
> + g_assert(-(addr | TARGET_PAGE_MASK) >= size);
> + probe_access_flags(env, addr, access_type, mmu_idx, false, &host, retaddr);
> + return host;
> }
probe_access() used to pass the 'size' argument through to
page_check_range() and cc->tlb_fill(); after this refactoring
it no longer does that.
thanks
-- PMM
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v2 02/16] accel/tcg: Add probe_access_flags
2020-04-16 11:54 ` Peter Maydell
@ 2020-04-18 0:05 ` Richard Henderson
0 siblings, 0 replies; 44+ messages in thread
From: Richard Henderson @ 2020-04-18 0:05 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemuarm, QEMU Developers
On 4/16/20 4:54 AM, Peter Maydell wrote:
>> - target_ulong tlb_addr;
>> - size_t elt_ofs;
>> - int wp_access;
>> -
>> - g_assert(-(addr | TARGET_PAGE_MASK) >= size);
>> -
>> - switch (access_type) {
>> - case MMU_DATA_LOAD:
>> - elt_ofs = offsetof(CPUTLBEntry, addr_read);
>> - wp_access = BP_MEM_READ;
>> - break;
>> - case MMU_DATA_STORE:
>> - elt_ofs = offsetof(CPUTLBEntry, addr_write);
>> - wp_access = BP_MEM_WRITE;
>> - break;
>> - case MMU_INST_FETCH:
>> - elt_ofs = offsetof(CPUTLBEntry, addr_code);
>> - wp_access = BP_MEM_READ;
>> - break;
>> - default:
>> - g_assert_not_reached();
>> - }
>> - tlb_addr = tlb_read_ofs(entry, elt_ofs);
>> -
>> - if (unlikely(!tlb_hit(tlb_addr, addr))) {
>> - if (!victim_tlb_hit(env, mmu_idx, index, elt_ofs,
>> - addr & TARGET_PAGE_MASK)) {
>> - tlb_fill(env_cpu(env), addr, size, access_type, mmu_idx, retaddr);
>> - /* TLB resize via tlb_fill may have moved the entry. */
>> - index = tlb_index(env, mmu_idx, addr);
>> - entry = tlb_entry(env, mmu_idx, addr);
>> - }
>> - tlb_addr = tlb_read_ofs(entry, elt_ofs);
>> - }
>
> All of the code above seems to have disappeared in this
> refactoring -- it's not in probe_access_internal()
> but it hasn't moved to the new probe_access().
It's certainly still there. I think this is diff being a bit daft. There were
2 copies of this code before -- in probe_access and tlb_vaddr_to_host.
>> +void *probe_access(CPUArchState *env, target_ulong addr, int size,
>> + MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
>> +{
>> + void *host;
>> +
>> + g_assert(-(addr | TARGET_PAGE_MASK) >= size);
>> + probe_access_flags(env, addr, access_type, mmu_idx, false, &host, retaddr);
>> + return host;
>> }
>
> probe_access() used to pass the 'size' argument through to
> page_check_range() and cc->tlb_fill(); after this refactoring
> it no longer does that.
Seems I've fixed this in my tree since posting v2 -- probe_access now uses
probe_access_internal directly. Anyway, definitely fixed for the next round.
r~
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 02/16] accel/tcg: Add probe_access_flags
2020-03-11 6:44 ` [PATCH 02/16] accel/tcg: Add probe_access_flags Richard Henderson
2020-03-12 4:33 ` [PATCH v2 " Richard Henderson
@ 2020-03-12 4:38 ` Richard Henderson
1 sibling, 0 replies; 44+ messages in thread
From: Richard Henderson @ 2020-03-12 4:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
On 3/10/20 11:44 PM, Richard Henderson wrote:
> +int probe_access_flags(CPUArchState *env, target_ulong addr,
> + MMUAccessType access_type, int mmu_idx,
> + bool nonfault, void **phost, uintptr_t retaddr)
> +{
> + void *host;
> + int flags;
> +
> + flags = probe_access_internal(env, addr, 0, access_type, mmu_idx,
> + nonfault, &host, retaddr);
Bug here. Should have passed along phost to probe_access_internal instead of a
new local host variable.
I've sent a v2 for this patch, in-reply-to. I'll not repost the whole patch
set until I've also addressed any review.
r~
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 03/16] exec: Add cpu_probe_watchpoint
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
2020-03-11 6:44 ` [PATCH 01/16] accel/tcg: Add block comment for probe_access Richard Henderson
2020-03-11 6:44 ` [PATCH 02/16] accel/tcg: Add probe_access_flags Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 12:08 ` Peter Maydell
2020-03-11 6:44 ` [PATCH 04/16] target/arm: Use cpu_*_data_ra for sve_ldst_tlb_fn Richard Henderson
` (14 subsequent siblings)
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
Allow probing of a watchpoint *without* raising an exception.
This is of most use for no-fault loads, which should indicate
via some architectural means that the load did not occur.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/hw/core/cpu.h | 7 +++++++
exec.c | 19 +++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 73e9a869a4..8ec44267a4 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1090,6 +1090,12 @@ static inline void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
{
}
+static inline bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
+ int flags)
+{
+ return false;
+}
+
static inline int cpu_watchpoint_address_matches(CPUState *cpu,
vaddr addr, vaddr len)
{
@@ -1104,6 +1110,7 @@ void cpu_watchpoint_remove_by_ref(CPUState *cpu, CPUWatchpoint *watchpoint);
void cpu_watchpoint_remove_all(CPUState *cpu, int mask);
void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
MemTxAttrs attrs, int flags, uintptr_t ra);
+bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len, int flags);
int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len);
#endif
diff --git a/exec.c b/exec.c
index 0cc500d53a..2b8f601b9e 100644
--- a/exec.c
+++ b/exec.c
@@ -2735,6 +2735,25 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
}
}
+bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len, int flags)
+{
+ CPUClass *cc = CPU_GET_CLASS(cpu);
+ CPUWatchpoint *wp;
+
+ assert(tcg_enabled());
+
+ addr = cc->adjust_watchpoint_address(cpu, addr, len);
+ QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
+ if (watchpoint_address_matches(wp, addr, len) &&
+ (wp->flags & flags) &&
+ (!(wp->flags & BP_CPU) ||
+ !cc->debug_check_watchpoint(cpu, wp))) {
+ return true;
+ }
+ }
+ return false;
+}
+
static MemTxResult flatview_read(FlatView *fv, hwaddr addr,
MemTxAttrs attrs, void *buf, hwaddr len);
static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH 03/16] exec: Add cpu_probe_watchpoint
2020-03-11 6:44 ` [PATCH 03/16] exec: Add cpu_probe_watchpoint Richard Henderson
@ 2020-04-16 12:08 ` Peter Maydell
2020-04-18 2:14 ` Richard Henderson
0 siblings, 1 reply; 44+ messages in thread
From: Peter Maydell @ 2020-04-16 12:08 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, QEMU Developers
On Wed, 11 Mar 2020 at 06:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Allow probing of a watchpoint *without* raising an exception.
> This is of most use for no-fault loads, which should indicate
> via some architectural means that the load did not occur.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/hw/core/cpu.h | 7 +++++++
> exec.c | 19 +++++++++++++++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 73e9a869a4..8ec44267a4 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -1090,6 +1090,12 @@ static inline void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
> {
> }
>
> +static inline bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
> + int flags)
> +{
> + return false;
> +}
> +
> static inline int cpu_watchpoint_address_matches(CPUState *cpu,
> vaddr addr, vaddr len)
> {
> @@ -1104,6 +1110,7 @@ void cpu_watchpoint_remove_by_ref(CPUState *cpu, CPUWatchpoint *watchpoint);
> void cpu_watchpoint_remove_all(CPUState *cpu, int mask);
> void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
> MemTxAttrs attrs, int flags, uintptr_t ra);
> +bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len, int flags);
Could we have a doc comment for the new function?
> int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len);
> #endif
>
> diff --git a/exec.c b/exec.c
> index 0cc500d53a..2b8f601b9e 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2735,6 +2735,25 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
> }
> }
>
> +bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len, int flags)
> +{
> + CPUClass *cc = CPU_GET_CLASS(cpu);
> + CPUWatchpoint *wp;
> +
> + assert(tcg_enabled());
> +
> + addr = cc->adjust_watchpoint_address(cpu, addr, len);
> + QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
> + if (watchpoint_address_matches(wp, addr, len) &&
> + (wp->flags & flags) &&
> + (!(wp->flags & BP_CPU) ||
> + !cc->debug_check_watchpoint(cpu, wp))) {
> + return true;
> + }
> + }
> + return false;
> +}
Clearly the insn emulation needs to do the right thing for
guest architectural watchpoints, but should a gdb watchpoint
also affect no-fault-load behaviour? I suppose making them
both behave the same way is probably the least-surprising choice.
thanks
-- PMM
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 03/16] exec: Add cpu_probe_watchpoint
2020-04-16 12:08 ` Peter Maydell
@ 2020-04-18 2:14 ` Richard Henderson
0 siblings, 0 replies; 44+ messages in thread
From: Richard Henderson @ 2020-04-18 2:14 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers
On 4/16/20 5:08 AM, Peter Maydell wrote:
>> void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
>> MemTxAttrs attrs, int flags, uintptr_t ra);
>> +bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len, int flags);
>
> Could we have a doc comment for the new function?
>
>> int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len);
Hah. In the process of doing that, I notice that
cpu_watchpoint_address_matches actually does what I want.
I have added documentation for cpu_check_watchpoint and
cpu_watchpoint_address_matches and have dropped this new function.
> Clearly the insn emulation needs to do the right thing for
> guest architectural watchpoints, but should a gdb watchpoint
> also affect no-fault-load behaviour? I suppose making them
> both behave the same way is probably the least-surprising choice.
In both cases we need to interrupt the execution in order to actually honor the
watchpoint. So yes, treating them the same seems the only reasonable way.
r~
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 04/16] target/arm: Use cpu_*_data_ra for sve_ldst_tlb_fn
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (2 preceding siblings ...)
2020-03-11 6:44 ` [PATCH 03/16] exec: Add cpu_probe_watchpoint Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 12:19 ` Peter Maydell
2020-03-11 6:44 ` [PATCH 05/16] target/arm: Drop manual handling of set/clear_helper_retaddr Richard Henderson
` (13 subsequent siblings)
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
Use the "normal" memory access functions, rather than the
softmmu internal helper functions directly.
Since fb901c905dc3, cpu_mem_index is now a simple extract
from env->hflags and not a large computation. Which means
that it's now more work to pass around this value than it
is to recompute it.
This only adjusts the primitives, and does not clean up
all of the uses within sve_helper.c.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/sve_helper.c | 241 ++++++++++++++++++----------------------
1 file changed, 107 insertions(+), 134 deletions(-)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index fdfa652094..f8b310a16b 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -3991,12 +3991,15 @@ typedef intptr_t sve_ld1_host_fn(void *vd, void *vg, void *host,
* Load one element into @vd + @reg_off from (@env, @vaddr, @ra).
* The controlling predicate is known to be true.
*/
-typedef void sve_ld1_tlb_fn(CPUARMState *env, void *vd, intptr_t reg_off,
- target_ulong vaddr, TCGMemOpIdx oi, uintptr_t ra);
-typedef sve_ld1_tlb_fn sve_st1_tlb_fn;
+typedef void sve_ldst1_tlb_fn(CPUARMState *env, void *vd, intptr_t reg_off,
+ target_ulong vaddr, uintptr_t retaddr);
/*
* Generate the above primitives.
+ *
+ * For *_tlb, this uses the cpu_*_data_ra helpers. There are not
+ * endian-specific versions of these, so we must handle endianness
+ * locally.
*/
#define DO_LD_HOST(NAME, H, TYPEE, TYPEM, HOST) \
@@ -4016,27 +4019,25 @@ static intptr_t sve_##NAME##_host(void *vd, void *vg, void *host, \
return mem_off; \
}
-#ifdef CONFIG_SOFTMMU
-#define DO_LD_TLB(NAME, H, TYPEE, TYPEM, HOST, MOEND, TLB) \
+#define DO_LD_TLB(NAME, H, TYPEE, TYPEM, BSWAP, TLB) \
static void sve_##NAME##_tlb(CPUARMState *env, void *vd, intptr_t reg_off, \
- target_ulong addr, TCGMemOpIdx oi, uintptr_t ra) \
+ target_ulong addr, uintptr_t ra) \
{ \
- TYPEM val = TLB(env, addr, oi, ra); \
+ TYPEM val = BSWAP(TLB(env, addr, ra)); \
*(TYPEE *)(vd + H(reg_off)) = val; \
}
-#else
-#define DO_LD_TLB(NAME, H, TYPEE, TYPEM, HOST, MOEND, TLB) \
+
+#define DO_ST_TLB(NAME, H, TYPEE, TYPEM, BSWAP, TLB) \
static void sve_##NAME##_tlb(CPUARMState *env, void *vd, intptr_t reg_off, \
- target_ulong addr, TCGMemOpIdx oi, uintptr_t ra) \
+ target_ulong addr, uintptr_t ra) \
{ \
- TYPEM val = HOST(g2h(addr)); \
- *(TYPEE *)(vd + H(reg_off)) = val; \
+ TYPEM val = *(TYPEE *)(vd + H(reg_off)); \
+ TLB(env, addr, BSWAP(val), ra); \
}
-#endif
#define DO_LD_PRIM_1(NAME, H, TE, TM) \
DO_LD_HOST(NAME, H, TE, TM, ldub_p) \
- DO_LD_TLB(NAME, H, TE, TM, ldub_p, 0, helper_ret_ldub_mmu)
+ DO_LD_TLB(NAME, H, TE, TM, , cpu_ldub_data_ra)
DO_LD_PRIM_1(ld1bb, H1, uint8_t, uint8_t)
DO_LD_PRIM_1(ld1bhu, H1_2, uint16_t, uint8_t)
@@ -4046,39 +4047,67 @@ DO_LD_PRIM_1(ld1bss, H1_4, uint32_t, int8_t)
DO_LD_PRIM_1(ld1bdu, , uint64_t, uint8_t)
DO_LD_PRIM_1(ld1bds, , uint64_t, int8_t)
-#define DO_LD_PRIM_2(NAME, end, MOEND, H, TE, TM, PH, PT) \
- DO_LD_HOST(NAME##_##end, H, TE, TM, PH##_##end##_p) \
- DO_LD_TLB(NAME##_##end, H, TE, TM, PH##_##end##_p, \
- MOEND, helper_##end##_##PT##_mmu)
+#define DO_ST_PRIM_1(NAME, H, TE, TM) \
+ DO_ST_TLB(st1##NAME, H, TE, TM, , cpu_stb_data_ra)
-DO_LD_PRIM_2(ld1hh, le, MO_LE, H1_2, uint16_t, uint16_t, lduw, lduw)
-DO_LD_PRIM_2(ld1hsu, le, MO_LE, H1_4, uint32_t, uint16_t, lduw, lduw)
-DO_LD_PRIM_2(ld1hss, le, MO_LE, H1_4, uint32_t, int16_t, lduw, lduw)
-DO_LD_PRIM_2(ld1hdu, le, MO_LE, , uint64_t, uint16_t, lduw, lduw)
-DO_LD_PRIM_2(ld1hds, le, MO_LE, , uint64_t, int16_t, lduw, lduw)
+DO_ST_PRIM_1(bb, H1, uint8_t, uint8_t)
+DO_ST_PRIM_1(bh, H1_2, uint16_t, uint8_t)
+DO_ST_PRIM_1(bs, H1_4, uint32_t, uint8_t)
+DO_ST_PRIM_1(bd, , uint64_t, uint8_t)
-DO_LD_PRIM_2(ld1ss, le, MO_LE, H1_4, uint32_t, uint32_t, ldl, ldul)
-DO_LD_PRIM_2(ld1sdu, le, MO_LE, , uint64_t, uint32_t, ldl, ldul)
-DO_LD_PRIM_2(ld1sds, le, MO_LE, , uint64_t, int32_t, ldl, ldul)
+#define DO_LD_PRIM_2(NAME, H, TE, TM, BSWAP, LD) \
+ DO_LD_HOST(ld1##NAME##_be, H, TE, TM, LD##_be_p) \
+ DO_LD_HOST(ld1##NAME##_le, H, TE, TM, LD##_le_p) \
+ DO_LD_TLB(ld1##NAME##_be, H, TE, TM, be_##BSWAP, cpu_##LD##_data_ra) \
+ DO_LD_TLB(ld1##NAME##_le, H, TE, TM, le_##BSWAP, cpu_##LD##_data_ra)
-DO_LD_PRIM_2(ld1dd, le, MO_LE, , uint64_t, uint64_t, ldq, ldq)
+#define DO_ST_PRIM_2(NAME, H, TE, TM, BSWAP, ST) \
+ DO_ST_TLB(st1##NAME##_be, H, TE, TM, be_##BSWAP, cpu_##ST##_data_ra) \
+ DO_ST_TLB(st1##NAME##_le, H, TE, TM, le_##BSWAP, cpu_##ST##_data_ra)
-DO_LD_PRIM_2(ld1hh, be, MO_BE, H1_2, uint16_t, uint16_t, lduw, lduw)
-DO_LD_PRIM_2(ld1hsu, be, MO_BE, H1_4, uint32_t, uint16_t, lduw, lduw)
-DO_LD_PRIM_2(ld1hss, be, MO_BE, H1_4, uint32_t, int16_t, lduw, lduw)
-DO_LD_PRIM_2(ld1hdu, be, MO_BE, , uint64_t, uint16_t, lduw, lduw)
-DO_LD_PRIM_2(ld1hds, be, MO_BE, , uint64_t, int16_t, lduw, lduw)
+#ifdef TARGET_WORDS_BIGENDIAN
+# define be_bswap16(x) ((uint16_t)(x))
+# define be_bswap32(x) ((uint32_t)(x))
+# define be_bswap64(x) ((uint64_t)(x))
+# define le_bswap16(x) bswap16(x)
+# define le_bswap32(x) bswap32(x)
+# define le_bswap64(x) bswap64(x)
+#else
+# define be_bswap16(x) bswap16(x)
+# define be_bswap32(x) bswap32(x)
+# define be_bswap64(x) bswap64(x)
+# define le_bswap16(x) ((uint16_t)(x))
+# define le_bswap32(x) ((uint32_t)(x))
+# define le_bswap64(x) ((uint64_t)(x))
+#endif
-DO_LD_PRIM_2(ld1ss, be, MO_BE, H1_4, uint32_t, uint32_t, ldl, ldul)
-DO_LD_PRIM_2(ld1sdu, be, MO_BE, , uint64_t, uint32_t, ldl, ldul)
-DO_LD_PRIM_2(ld1sds, be, MO_BE, , uint64_t, int32_t, ldl, ldul)
+DO_LD_PRIM_2(hh, H1_2, uint16_t, uint16_t, bswap16, lduw)
+DO_LD_PRIM_2(hsu, H1_4, uint32_t, uint16_t, bswap16, lduw)
+DO_LD_PRIM_2(hss, H1_4, uint32_t, int16_t, bswap16, lduw)
+DO_LD_PRIM_2(hdu, , uint64_t, uint16_t, bswap16, lduw)
+DO_LD_PRIM_2(hds, , uint64_t, int16_t, bswap16, lduw)
-DO_LD_PRIM_2(ld1dd, be, MO_BE, , uint64_t, uint64_t, ldq, ldq)
+DO_ST_PRIM_2(hh, H1_2, uint16_t, uint16_t, bswap16, stw)
+DO_ST_PRIM_2(hs, H1_4, uint32_t, uint16_t, bswap16, stw)
+DO_ST_PRIM_2(hd, , uint64_t, uint16_t, bswap16, stw)
+
+DO_LD_PRIM_2(ss, H1_4, uint32_t, uint32_t, bswap32, ldl)
+DO_LD_PRIM_2(sdu, , uint64_t, uint32_t, bswap32, ldl)
+DO_LD_PRIM_2(sds, , uint64_t, int32_t, bswap32, ldl)
+
+DO_ST_PRIM_2(ss, H1_4, uint32_t, uint32_t, bswap32, stl)
+DO_ST_PRIM_2(sd, , uint64_t, uint32_t, bswap32, stl)
+
+DO_LD_PRIM_2(dd, , uint64_t, uint64_t, bswap64, ldq)
+DO_ST_PRIM_2(dd, , uint64_t, uint64_t, bswap64, stq)
#undef DO_LD_TLB
+#undef DO_ST_TLB
#undef DO_LD_HOST
#undef DO_LD_PRIM_1
+#undef DO_ST_PRIM_1
#undef DO_LD_PRIM_2
+#undef DO_ST_PRIM_2
/*
* Skip through a sequence of inactive elements in the guarding predicate @vg,
@@ -4152,7 +4181,7 @@ static void sve_ld1_r(CPUARMState *env, void *vg, const target_ulong addr,
uint32_t desc, const uintptr_t retaddr,
const int esz, const int msz,
sve_ld1_host_fn *host_fn,
- sve_ld1_tlb_fn *tlb_fn)
+ sve_ldst1_tlb_fn *tlb_fn)
{
const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
const int mmu_idx = get_mmuidx(oi);
@@ -4234,7 +4263,7 @@ static void sve_ld1_r(CPUARMState *env, void *vg, const target_ulong addr,
* on I/O memory, it may succeed but not bring in the TLB entry.
* But even then we have still made forward progress.
*/
- tlb_fn(env, &scratch, reg_off, addr + mem_off, oi, retaddr);
+ tlb_fn(env, &scratch, reg_off, addr + mem_off, retaddr);
reg_off += 1 << esz;
}
#endif
@@ -4293,9 +4322,8 @@ DO_LD1_2(ld1dd, 3, 3)
*/
static void sve_ld2_r(CPUARMState *env, void *vg, target_ulong addr,
uint32_t desc, int size, uintptr_t ra,
- sve_ld1_tlb_fn *tlb_fn)
+ sve_ldst1_tlb_fn *tlb_fn)
{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
intptr_t i, oprsz = simd_oprsz(desc);
ARMVectorReg scratch[2] = { };
@@ -4305,8 +4333,8 @@ static void sve_ld2_r(CPUARMState *env, void *vg, target_ulong addr,
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
if (pg & 1) {
- tlb_fn(env, &scratch[0], i, addr, oi, ra);
- tlb_fn(env, &scratch[1], i, addr + size, oi, ra);
+ tlb_fn(env, &scratch[0], i, addr, ra);
+ tlb_fn(env, &scratch[1], i, addr + size, ra);
}
i += size, pg >>= size;
addr += 2 * size;
@@ -4321,9 +4349,8 @@ static void sve_ld2_r(CPUARMState *env, void *vg, target_ulong addr,
static void sve_ld3_r(CPUARMState *env, void *vg, target_ulong addr,
uint32_t desc, int size, uintptr_t ra,
- sve_ld1_tlb_fn *tlb_fn)
+ sve_ldst1_tlb_fn *tlb_fn)
{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
intptr_t i, oprsz = simd_oprsz(desc);
ARMVectorReg scratch[3] = { };
@@ -4333,9 +4360,9 @@ static void sve_ld3_r(CPUARMState *env, void *vg, target_ulong addr,
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
if (pg & 1) {
- tlb_fn(env, &scratch[0], i, addr, oi, ra);
- tlb_fn(env, &scratch[1], i, addr + size, oi, ra);
- tlb_fn(env, &scratch[2], i, addr + 2 * size, oi, ra);
+ tlb_fn(env, &scratch[0], i, addr, ra);
+ tlb_fn(env, &scratch[1], i, addr + size, ra);
+ tlb_fn(env, &scratch[2], i, addr + 2 * size, ra);
}
i += size, pg >>= size;
addr += 3 * size;
@@ -4351,9 +4378,8 @@ static void sve_ld3_r(CPUARMState *env, void *vg, target_ulong addr,
static void sve_ld4_r(CPUARMState *env, void *vg, target_ulong addr,
uint32_t desc, int size, uintptr_t ra,
- sve_ld1_tlb_fn *tlb_fn)
+ sve_ldst1_tlb_fn *tlb_fn)
{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
intptr_t i, oprsz = simd_oprsz(desc);
ARMVectorReg scratch[4] = { };
@@ -4363,10 +4389,10 @@ static void sve_ld4_r(CPUARMState *env, void *vg, target_ulong addr,
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
if (pg & 1) {
- tlb_fn(env, &scratch[0], i, addr, oi, ra);
- tlb_fn(env, &scratch[1], i, addr + size, oi, ra);
- tlb_fn(env, &scratch[2], i, addr + 2 * size, oi, ra);
- tlb_fn(env, &scratch[3], i, addr + 3 * size, oi, ra);
+ tlb_fn(env, &scratch[0], i, addr, ra);
+ tlb_fn(env, &scratch[1], i, addr + size, ra);
+ tlb_fn(env, &scratch[2], i, addr + 2 * size, ra);
+ tlb_fn(env, &scratch[3], i, addr + 3 * size, ra);
}
i += size, pg >>= size;
addr += 4 * size;
@@ -4459,7 +4485,7 @@ static void sve_ldff1_r(CPUARMState *env, void *vg, const target_ulong addr,
uint32_t desc, const uintptr_t retaddr,
const int esz, const int msz,
sve_ld1_host_fn *host_fn,
- sve_ld1_tlb_fn *tlb_fn)
+ sve_ldst1_tlb_fn *tlb_fn)
{
const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
const int mmu_idx = get_mmuidx(oi);
@@ -4519,7 +4545,7 @@ static void sve_ldff1_r(CPUARMState *env, void *vg, const target_ulong addr,
* Perform one normal read, which will fault or not.
* But it is likely to bring the page into the tlb.
*/
- tlb_fn(env, vd, reg_off, addr + mem_off, oi, retaddr);
+ tlb_fn(env, vd, reg_off, addr + mem_off, retaddr);
/* After any fault, zero any leading predicated false elts. */
swap_memzero(vd, reg_off);
@@ -4671,60 +4697,14 @@ DO_LDFF1_LDNF1_2(dd, 3, 3)
#undef DO_LDFF1_LDNF1_1
#undef DO_LDFF1_LDNF1_2
-/*
- * Store contiguous data, protected by a governing predicate.
- */
-
-#ifdef CONFIG_SOFTMMU
-#define DO_ST_TLB(NAME, H, TYPEM, HOST, MOEND, TLB) \
-static void sve_##NAME##_tlb(CPUARMState *env, void *vd, intptr_t reg_off, \
- target_ulong addr, TCGMemOpIdx oi, uintptr_t ra) \
-{ \
- TLB(env, addr, *(TYPEM *)(vd + H(reg_off)), oi, ra); \
-}
-#else
-#define DO_ST_TLB(NAME, H, TYPEM, HOST, MOEND, TLB) \
-static void sve_##NAME##_tlb(CPUARMState *env, void *vd, intptr_t reg_off, \
- target_ulong addr, TCGMemOpIdx oi, uintptr_t ra) \
-{ \
- HOST(g2h(addr), *(TYPEM *)(vd + H(reg_off))); \
-}
-#endif
-
-DO_ST_TLB(st1bb, H1, uint8_t, stb_p, 0, helper_ret_stb_mmu)
-DO_ST_TLB(st1bh, H1_2, uint16_t, stb_p, 0, helper_ret_stb_mmu)
-DO_ST_TLB(st1bs, H1_4, uint32_t, stb_p, 0, helper_ret_stb_mmu)
-DO_ST_TLB(st1bd, , uint64_t, stb_p, 0, helper_ret_stb_mmu)
-
-DO_ST_TLB(st1hh_le, H1_2, uint16_t, stw_le_p, MO_LE, helper_le_stw_mmu)
-DO_ST_TLB(st1hs_le, H1_4, uint32_t, stw_le_p, MO_LE, helper_le_stw_mmu)
-DO_ST_TLB(st1hd_le, , uint64_t, stw_le_p, MO_LE, helper_le_stw_mmu)
-
-DO_ST_TLB(st1ss_le, H1_4, uint32_t, stl_le_p, MO_LE, helper_le_stl_mmu)
-DO_ST_TLB(st1sd_le, , uint64_t, stl_le_p, MO_LE, helper_le_stl_mmu)
-
-DO_ST_TLB(st1dd_le, , uint64_t, stq_le_p, MO_LE, helper_le_stq_mmu)
-
-DO_ST_TLB(st1hh_be, H1_2, uint16_t, stw_be_p, MO_BE, helper_be_stw_mmu)
-DO_ST_TLB(st1hs_be, H1_4, uint32_t, stw_be_p, MO_BE, helper_be_stw_mmu)
-DO_ST_TLB(st1hd_be, , uint64_t, stw_be_p, MO_BE, helper_be_stw_mmu)
-
-DO_ST_TLB(st1ss_be, H1_4, uint32_t, stl_be_p, MO_BE, helper_be_stl_mmu)
-DO_ST_TLB(st1sd_be, , uint64_t, stl_be_p, MO_BE, helper_be_stl_mmu)
-
-DO_ST_TLB(st1dd_be, , uint64_t, stq_be_p, MO_BE, helper_be_stq_mmu)
-
-#undef DO_ST_TLB
-
/*
* Common helpers for all contiguous 1,2,3,4-register predicated stores.
*/
static void sve_st1_r(CPUARMState *env, void *vg, target_ulong addr,
uint32_t desc, const uintptr_t ra,
const int esize, const int msize,
- sve_st1_tlb_fn *tlb_fn)
+ sve_ldst1_tlb_fn *tlb_fn)
{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
intptr_t i, oprsz = simd_oprsz(desc);
void *vd = &env->vfp.zregs[rd];
@@ -4734,7 +4714,7 @@ static void sve_st1_r(CPUARMState *env, void *vg, target_ulong addr,
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
if (pg & 1) {
- tlb_fn(env, vd, i, addr, oi, ra);
+ tlb_fn(env, vd, i, addr, ra);
}
i += esize, pg >>= esize;
addr += msize;
@@ -4746,9 +4726,8 @@ static void sve_st1_r(CPUARMState *env, void *vg, target_ulong addr,
static void sve_st2_r(CPUARMState *env, void *vg, target_ulong addr,
uint32_t desc, const uintptr_t ra,
const int esize, const int msize,
- sve_st1_tlb_fn *tlb_fn)
+ sve_ldst1_tlb_fn *tlb_fn)
{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
intptr_t i, oprsz = simd_oprsz(desc);
void *d1 = &env->vfp.zregs[rd];
@@ -4759,8 +4738,8 @@ static void sve_st2_r(CPUARMState *env, void *vg, target_ulong addr,
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
if (pg & 1) {
- tlb_fn(env, d1, i, addr, oi, ra);
- tlb_fn(env, d2, i, addr + msize, oi, ra);
+ tlb_fn(env, d1, i, addr, ra);
+ tlb_fn(env, d2, i, addr + msize, ra);
}
i += esize, pg >>= esize;
addr += 2 * msize;
@@ -4772,9 +4751,8 @@ static void sve_st2_r(CPUARMState *env, void *vg, target_ulong addr,
static void sve_st3_r(CPUARMState *env, void *vg, target_ulong addr,
uint32_t desc, const uintptr_t ra,
const int esize, const int msize,
- sve_st1_tlb_fn *tlb_fn)
+ sve_ldst1_tlb_fn *tlb_fn)
{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
intptr_t i, oprsz = simd_oprsz(desc);
void *d1 = &env->vfp.zregs[rd];
@@ -4786,9 +4764,9 @@ static void sve_st3_r(CPUARMState *env, void *vg, target_ulong addr,
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
if (pg & 1) {
- tlb_fn(env, d1, i, addr, oi, ra);
- tlb_fn(env, d2, i, addr + msize, oi, ra);
- tlb_fn(env, d3, i, addr + 2 * msize, oi, ra);
+ tlb_fn(env, d1, i, addr, ra);
+ tlb_fn(env, d2, i, addr + msize, ra);
+ tlb_fn(env, d3, i, addr + 2 * msize, ra);
}
i += esize, pg >>= esize;
addr += 3 * msize;
@@ -4800,9 +4778,8 @@ static void sve_st3_r(CPUARMState *env, void *vg, target_ulong addr,
static void sve_st4_r(CPUARMState *env, void *vg, target_ulong addr,
uint32_t desc, const uintptr_t ra,
const int esize, const int msize,
- sve_st1_tlb_fn *tlb_fn)
+ sve_ldst1_tlb_fn *tlb_fn)
{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
intptr_t i, oprsz = simd_oprsz(desc);
void *d1 = &env->vfp.zregs[rd];
@@ -4815,10 +4792,10 @@ static void sve_st4_r(CPUARMState *env, void *vg, target_ulong addr,
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
if (pg & 1) {
- tlb_fn(env, d1, i, addr, oi, ra);
- tlb_fn(env, d2, i, addr + msize, oi, ra);
- tlb_fn(env, d3, i, addr + 2 * msize, oi, ra);
- tlb_fn(env, d4, i, addr + 3 * msize, oi, ra);
+ tlb_fn(env, d1, i, addr, ra);
+ tlb_fn(env, d2, i, addr + msize, ra);
+ tlb_fn(env, d3, i, addr + 2 * msize, ra);
+ tlb_fn(env, d4, i, addr + 3 * msize, ra);
}
i += esize, pg >>= esize;
addr += 4 * msize;
@@ -4914,9 +4891,8 @@ static target_ulong off_zd_d(void *reg, intptr_t reg_ofs)
static void sve_ld1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
target_ulong base, uint32_t desc, uintptr_t ra,
- zreg_off_fn *off_fn, sve_ld1_tlb_fn *tlb_fn)
+ zreg_off_fn *off_fn, sve_ldst1_tlb_fn *tlb_fn)
{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
intptr_t i, oprsz = simd_oprsz(desc);
ARMVectorReg scratch = { };
@@ -4927,7 +4903,7 @@ static void sve_ld1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
do {
if (likely(pg & 1)) {
target_ulong off = off_fn(vm, i);
- tlb_fn(env, &scratch, i, base + (off << scale), oi, ra);
+ tlb_fn(env, &scratch, i, base + (off << scale), ra);
}
i += 4, pg >>= 4;
} while (i & 15);
@@ -4940,9 +4916,8 @@ static void sve_ld1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
static void sve_ld1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
target_ulong base, uint32_t desc, uintptr_t ra,
- zreg_off_fn *off_fn, sve_ld1_tlb_fn *tlb_fn)
+ zreg_off_fn *off_fn, sve_ldst1_tlb_fn *tlb_fn)
{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
intptr_t i, oprsz = simd_oprsz(desc) / 8;
ARMVectorReg scratch = { };
@@ -4952,7 +4927,7 @@ static void sve_ld1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
uint8_t pg = *(uint8_t *)(vg + H1(i));
if (likely(pg & 1)) {
target_ulong off = off_fn(vm, i * 8);
- tlb_fn(env, &scratch, i * 8, base + (off << scale), oi, ra);
+ tlb_fn(env, &scratch, i * 8, base + (off << scale), ra);
}
}
clear_helper_retaddr();
@@ -5114,7 +5089,7 @@ DO_LD_NF(dd_be, , uint64_t, uint64_t, ldq_be_p)
*/
static inline void sve_ldff1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
target_ulong base, uint32_t desc, uintptr_t ra,
- zreg_off_fn *off_fn, sve_ld1_tlb_fn *tlb_fn,
+ zreg_off_fn *off_fn, sve_ldst1_tlb_fn *tlb_fn,
sve_ld1_nf_fn *nonfault_fn)
{
const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
@@ -5130,7 +5105,7 @@ static inline void sve_ldff1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
set_helper_retaddr(ra);
addr = off_fn(vm, reg_off);
addr = base + (addr << scale);
- tlb_fn(env, vd, reg_off, addr, oi, ra);
+ tlb_fn(env, vd, reg_off, addr, ra);
/* The rest of the reads will be non-faulting. */
clear_helper_retaddr();
@@ -5156,7 +5131,7 @@ static inline void sve_ldff1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
static inline void sve_ldff1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
target_ulong base, uint32_t desc, uintptr_t ra,
- zreg_off_fn *off_fn, sve_ld1_tlb_fn *tlb_fn,
+ zreg_off_fn *off_fn, sve_ldst1_tlb_fn *tlb_fn,
sve_ld1_nf_fn *nonfault_fn)
{
const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
@@ -5172,7 +5147,7 @@ static inline void sve_ldff1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
set_helper_retaddr(ra);
addr = off_fn(vm, reg_off);
addr = base + (addr << scale);
- tlb_fn(env, vd, reg_off, addr, oi, ra);
+ tlb_fn(env, vd, reg_off, addr, ra);
/* The rest of the reads will be non-faulting. */
clear_helper_retaddr();
@@ -5282,9 +5257,8 @@ DO_LDFF1_ZPZ_D(dd_be, zd)
static void sve_st1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
target_ulong base, uint32_t desc, uintptr_t ra,
- zreg_off_fn *off_fn, sve_ld1_tlb_fn *tlb_fn)
+ zreg_off_fn *off_fn, sve_ldst1_tlb_fn *tlb_fn)
{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
intptr_t i, oprsz = simd_oprsz(desc);
@@ -5294,7 +5268,7 @@ static void sve_st1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
do {
if (likely(pg & 1)) {
target_ulong off = off_fn(vm, i);
- tlb_fn(env, vd, i, base + (off << scale), oi, ra);
+ tlb_fn(env, vd, i, base + (off << scale), ra);
}
i += 4, pg >>= 4;
} while (i & 15);
@@ -5304,9 +5278,8 @@ static void sve_st1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
static void sve_st1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
target_ulong base, uint32_t desc, uintptr_t ra,
- zreg_off_fn *off_fn, sve_ld1_tlb_fn *tlb_fn)
+ zreg_off_fn *off_fn, sve_ldst1_tlb_fn *tlb_fn)
{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
intptr_t i, oprsz = simd_oprsz(desc) / 8;
@@ -5315,7 +5288,7 @@ static void sve_st1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
uint8_t pg = *(uint8_t *)(vg + H1(i));
if (likely(pg & 1)) {
target_ulong off = off_fn(vm, i * 8);
- tlb_fn(env, vd, i * 8, base + (off << scale), oi, ra);
+ tlb_fn(env, vd, i * 8, base + (off << scale), ra);
}
}
clear_helper_retaddr();
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH 04/16] target/arm: Use cpu_*_data_ra for sve_ldst_tlb_fn
2020-03-11 6:44 ` [PATCH 04/16] target/arm: Use cpu_*_data_ra for sve_ldst_tlb_fn Richard Henderson
@ 2020-04-16 12:19 ` Peter Maydell
2020-04-18 2:16 ` Richard Henderson
0 siblings, 1 reply; 44+ messages in thread
From: Peter Maydell @ 2020-04-16 12:19 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, QEMU Developers
On Wed, 11 Mar 2020 at 06:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Use the "normal" memory access functions, rather than the
> softmmu internal helper functions directly.
>
> Since fb901c905dc3, cpu_mem_index is now a simple extract
> from env->hflags and not a large computation. Which means
> that it's now more work to pass around this value than it
> is to recompute it.
>
> This only adjusts the primitives, and does not clean up
> all of the uses within sve_helper.c.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/sve_helper.c | 241 ++++++++++++++++++----------------------
> 1 file changed, 107 insertions(+), 134 deletions(-)
> +#ifdef TARGET_WORDS_BIGENDIAN
> +# define be_bswap16(x) ((uint16_t)(x))
> +# define be_bswap32(x) ((uint32_t)(x))
> +# define be_bswap64(x) ((uint64_t)(x))
> +# define le_bswap16(x) bswap16(x)
> +# define le_bswap32(x) bswap32(x)
> +# define le_bswap64(x) bswap64(x)
> +#else
> +# define be_bswap16(x) bswap16(x)
> +# define be_bswap32(x) bswap32(x)
> +# define be_bswap64(x) bswap64(x)
> +# define le_bswap16(x) ((uint16_t)(x))
> +# define le_bswap32(x) ((uint32_t)(x))
> +# define le_bswap64(x) ((uint64_t)(x))
> +#endif
Am I confused, or are these just reimplementing
cpu_to_be16()/cpu_to_le16()/le16_to_cpu()/be16_to_cpu() etc from bswap.h ?
(It seems a pity to have to lose the memory subsystem handling
endianness for us.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 04/16] target/arm: Use cpu_*_data_ra for sve_ldst_tlb_fn
2020-04-16 12:19 ` Peter Maydell
@ 2020-04-18 2:16 ` Richard Henderson
0 siblings, 0 replies; 44+ messages in thread
From: Richard Henderson @ 2020-04-18 2:16 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers
On 4/16/20 5:19 AM, Peter Maydell wrote:
> On Wed, 11 Mar 2020 at 06:44, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Use the "normal" memory access functions, rather than the
>> softmmu internal helper functions directly.
>>
>> Since fb901c905dc3, cpu_mem_index is now a simple extract
>> from env->hflags and not a large computation. Which means
>> that it's now more work to pass around this value than it
>> is to recompute it.
>>
>> This only adjusts the primitives, and does not clean up
>> all of the uses within sve_helper.c.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> target/arm/sve_helper.c | 241 ++++++++++++++++++----------------------
>> 1 file changed, 107 insertions(+), 134 deletions(-)
>
>> +#ifdef TARGET_WORDS_BIGENDIAN
>> +# define be_bswap16(x) ((uint16_t)(x))
>> +# define be_bswap32(x) ((uint32_t)(x))
>> +# define be_bswap64(x) ((uint64_t)(x))
>> +# define le_bswap16(x) bswap16(x)
>> +# define le_bswap32(x) bswap32(x)
>> +# define le_bswap64(x) bswap64(x)
>> +#else
>> +# define be_bswap16(x) bswap16(x)
>> +# define be_bswap32(x) bswap32(x)
>> +# define be_bswap64(x) bswap64(x)
>> +# define le_bswap16(x) ((uint16_t)(x))
>> +# define le_bswap32(x) ((uint32_t)(x))
>> +# define le_bswap64(x) ((uint64_t)(x))
>> +#endif
>
> Am I confused, or are these just reimplementing
> cpu_to_be16()/cpu_to_le16()/le16_to_cpu()/be16_to_cpu() etc from bswap.h ?
Those depend on HOST_WORDS_BIGENDIAN, this is TARGET_WORDS_BIGENDIAN.
> (It seems a pity to have to lose the memory subsystem handling
> endianness for us.)
It is. I have added a patch to improve the memory subsystem.
r~
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 05/16] target/arm: Drop manual handling of set/clear_helper_retaddr
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (3 preceding siblings ...)
2020-03-11 6:44 ` [PATCH 04/16] target/arm: Use cpu_*_data_ra for sve_ldst_tlb_fn Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 12:23 ` Peter Maydell
2020-03-11 6:44 ` [PATCH 06/16] target/arm: Add sve infrastructure for page lookup Richard Henderson
` (12 subsequent siblings)
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
Since we converted back to cpu_*_data_ra, we do not need to
do this ourselves.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/sve_helper.c | 38 --------------------------------------
1 file changed, 38 deletions(-)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index f8b310a16b..8b470991db 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4155,12 +4155,6 @@ static intptr_t max_for_page(target_ulong base, intptr_t mem_off,
return MIN(split, mem_max - mem_off) + mem_off;
}
-#ifndef CONFIG_USER_ONLY
-/* These are normally defined only for CONFIG_USER_ONLY in <exec/cpu_ldst.h> */
-static inline void set_helper_retaddr(uintptr_t ra) { }
-static inline void clear_helper_retaddr(void) { }
-#endif
-
/*
* The result of tlb_vaddr_to_host for user-only is just g2h(x),
* which is always non-null. Elide the useless test.
@@ -4202,7 +4196,6 @@ static void sve_ld1_r(CPUARMState *env, void *vg, const target_ulong addr,
return;
}
mem_off = reg_off >> diffsz;
- set_helper_retaddr(retaddr);
/*
* If the (remaining) load is entirely within a single page, then:
@@ -4217,7 +4210,6 @@ static void sve_ld1_r(CPUARMState *env, void *vg, const target_ulong addr,
if (test_host_page(host)) {
mem_off = host_fn(vd, vg, host - mem_off, mem_off, mem_max);
tcg_debug_assert(mem_off == mem_max);
- clear_helper_retaddr();
/* After having taken any fault, zero leading inactive elements. */
swap_memzero(vd, reg_off);
return;
@@ -4268,7 +4260,6 @@ static void sve_ld1_r(CPUARMState *env, void *vg, const target_ulong addr,
}
#endif
- clear_helper_retaddr();
memcpy(vd, &scratch, reg_max);
}
@@ -4328,7 +4319,6 @@ static void sve_ld2_r(CPUARMState *env, void *vg, target_ulong addr,
intptr_t i, oprsz = simd_oprsz(desc);
ARMVectorReg scratch[2] = { };
- set_helper_retaddr(ra);
for (i = 0; i < oprsz; ) {
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
@@ -4340,7 +4330,6 @@ static void sve_ld2_r(CPUARMState *env, void *vg, target_ulong addr,
addr += 2 * size;
} while (i & 15);
}
- clear_helper_retaddr();
/* Wait until all exceptions have been raised to write back. */
memcpy(&env->vfp.zregs[rd], &scratch[0], oprsz);
@@ -4355,7 +4344,6 @@ static void sve_ld3_r(CPUARMState *env, void *vg, target_ulong addr,
intptr_t i, oprsz = simd_oprsz(desc);
ARMVectorReg scratch[3] = { };
- set_helper_retaddr(ra);
for (i = 0; i < oprsz; ) {
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
@@ -4368,7 +4356,6 @@ static void sve_ld3_r(CPUARMState *env, void *vg, target_ulong addr,
addr += 3 * size;
} while (i & 15);
}
- clear_helper_retaddr();
/* Wait until all exceptions have been raised to write back. */
memcpy(&env->vfp.zregs[rd], &scratch[0], oprsz);
@@ -4384,7 +4371,6 @@ static void sve_ld4_r(CPUARMState *env, void *vg, target_ulong addr,
intptr_t i, oprsz = simd_oprsz(desc);
ARMVectorReg scratch[4] = { };
- set_helper_retaddr(ra);
for (i = 0; i < oprsz; ) {
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
@@ -4398,7 +4384,6 @@ static void sve_ld4_r(CPUARMState *env, void *vg, target_ulong addr,
addr += 4 * size;
} while (i & 15);
}
- clear_helper_retaddr();
/* Wait until all exceptions have been raised to write back. */
memcpy(&env->vfp.zregs[rd], &scratch[0], oprsz);
@@ -4505,7 +4490,6 @@ static void sve_ldff1_r(CPUARMState *env, void *vg, const target_ulong addr,
return;
}
mem_off = reg_off >> diffsz;
- set_helper_retaddr(retaddr);
/*
* If the (remaining) load is entirely within a single page, then:
@@ -4520,7 +4504,6 @@ static void sve_ldff1_r(CPUARMState *env, void *vg, const target_ulong addr,
if (test_host_page(host)) {
mem_off = host_fn(vd, vg, host - mem_off, mem_off, mem_max);
tcg_debug_assert(mem_off == mem_max);
- clear_helper_retaddr();
/* After any fault, zero any leading inactive elements. */
swap_memzero(vd, reg_off);
return;
@@ -4563,7 +4546,6 @@ static void sve_ldff1_r(CPUARMState *env, void *vg, const target_ulong addr,
}
#endif
- clear_helper_retaddr();
record_fault(env, reg_off, reg_max);
}
@@ -4709,7 +4691,6 @@ static void sve_st1_r(CPUARMState *env, void *vg, target_ulong addr,
intptr_t i, oprsz = simd_oprsz(desc);
void *vd = &env->vfp.zregs[rd];
- set_helper_retaddr(ra);
for (i = 0; i < oprsz; ) {
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
@@ -4720,7 +4701,6 @@ static void sve_st1_r(CPUARMState *env, void *vg, target_ulong addr,
addr += msize;
} while (i & 15);
}
- clear_helper_retaddr();
}
static void sve_st2_r(CPUARMState *env, void *vg, target_ulong addr,
@@ -4733,7 +4713,6 @@ static void sve_st2_r(CPUARMState *env, void *vg, target_ulong addr,
void *d1 = &env->vfp.zregs[rd];
void *d2 = &env->vfp.zregs[(rd + 1) & 31];
- set_helper_retaddr(ra);
for (i = 0; i < oprsz; ) {
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
@@ -4745,7 +4724,6 @@ static void sve_st2_r(CPUARMState *env, void *vg, target_ulong addr,
addr += 2 * msize;
} while (i & 15);
}
- clear_helper_retaddr();
}
static void sve_st3_r(CPUARMState *env, void *vg, target_ulong addr,
@@ -4759,7 +4737,6 @@ static void sve_st3_r(CPUARMState *env, void *vg, target_ulong addr,
void *d2 = &env->vfp.zregs[(rd + 1) & 31];
void *d3 = &env->vfp.zregs[(rd + 2) & 31];
- set_helper_retaddr(ra);
for (i = 0; i < oprsz; ) {
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
@@ -4772,7 +4749,6 @@ static void sve_st3_r(CPUARMState *env, void *vg, target_ulong addr,
addr += 3 * msize;
} while (i & 15);
}
- clear_helper_retaddr();
}
static void sve_st4_r(CPUARMState *env, void *vg, target_ulong addr,
@@ -4787,7 +4763,6 @@ static void sve_st4_r(CPUARMState *env, void *vg, target_ulong addr,
void *d3 = &env->vfp.zregs[(rd + 2) & 31];
void *d4 = &env->vfp.zregs[(rd + 3) & 31];
- set_helper_retaddr(ra);
for (i = 0; i < oprsz; ) {
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
@@ -4801,7 +4776,6 @@ static void sve_st4_r(CPUARMState *env, void *vg, target_ulong addr,
addr += 4 * msize;
} while (i & 15);
}
- clear_helper_retaddr();
}
#define DO_STN_1(N, NAME, ESIZE) \
@@ -4897,7 +4871,6 @@ static void sve_ld1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
intptr_t i, oprsz = simd_oprsz(desc);
ARMVectorReg scratch = { };
- set_helper_retaddr(ra);
for (i = 0; i < oprsz; ) {
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
@@ -4908,7 +4881,6 @@ static void sve_ld1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
i += 4, pg >>= 4;
} while (i & 15);
}
- clear_helper_retaddr();
/* Wait until all exceptions have been raised to write back. */
memcpy(vd, &scratch, oprsz);
@@ -4922,7 +4894,6 @@ static void sve_ld1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
intptr_t i, oprsz = simd_oprsz(desc) / 8;
ARMVectorReg scratch = { };
- set_helper_retaddr(ra);
for (i = 0; i < oprsz; i++) {
uint8_t pg = *(uint8_t *)(vg + H1(i));
if (likely(pg & 1)) {
@@ -4930,7 +4901,6 @@ static void sve_ld1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
tlb_fn(env, &scratch, i * 8, base + (off << scale), ra);
}
}
- clear_helper_retaddr();
/* Wait until all exceptions have been raised to write back. */
memcpy(vd, &scratch, oprsz * 8);
@@ -5102,13 +5072,11 @@ static inline void sve_ldff1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
reg_off = find_next_active(vg, 0, reg_max, MO_32);
if (likely(reg_off < reg_max)) {
/* Perform one normal read, which will fault or not. */
- set_helper_retaddr(ra);
addr = off_fn(vm, reg_off);
addr = base + (addr << scale);
tlb_fn(env, vd, reg_off, addr, ra);
/* The rest of the reads will be non-faulting. */
- clear_helper_retaddr();
}
/* After any fault, zero the leading predicated false elements. */
@@ -5144,13 +5112,11 @@ static inline void sve_ldff1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
reg_off = find_next_active(vg, 0, reg_max, MO_64);
if (likely(reg_off < reg_max)) {
/* Perform one normal read, which will fault or not. */
- set_helper_retaddr(ra);
addr = off_fn(vm, reg_off);
addr = base + (addr << scale);
tlb_fn(env, vd, reg_off, addr, ra);
/* The rest of the reads will be non-faulting. */
- clear_helper_retaddr();
}
/* After any fault, zero the leading predicated false elements. */
@@ -5262,7 +5228,6 @@ static void sve_st1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
intptr_t i, oprsz = simd_oprsz(desc);
- set_helper_retaddr(ra);
for (i = 0; i < oprsz; ) {
uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
do {
@@ -5273,7 +5238,6 @@ static void sve_st1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
i += 4, pg >>= 4;
} while (i & 15);
}
- clear_helper_retaddr();
}
static void sve_st1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
@@ -5283,7 +5247,6 @@ static void sve_st1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
intptr_t i, oprsz = simd_oprsz(desc) / 8;
- set_helper_retaddr(ra);
for (i = 0; i < oprsz; i++) {
uint8_t pg = *(uint8_t *)(vg + H1(i));
if (likely(pg & 1)) {
@@ -5291,7 +5254,6 @@ static void sve_st1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
tlb_fn(env, vd, i * 8, base + (off << scale), ra);
}
}
- clear_helper_retaddr();
}
#define DO_ST1_ZPZ_S(MEM, OFS) \
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH 06/16] target/arm: Add sve infrastructure for page lookup
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (4 preceding siblings ...)
2020-03-11 6:44 ` [PATCH 05/16] target/arm: Drop manual handling of set/clear_helper_retaddr Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 13:03 ` Peter Maydell
2020-03-11 6:44 ` [PATCH 07/16] target/arm: Adjust interface of sve_ld1_host_fn Richard Henderson
` (11 subsequent siblings)
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
For contiguous predicated memory operations, we want to
minimize the number of tlb lookups performed. We have
open-coded this for sve_ld1_r, but for correctness with
MTE we will need this for all of the memory operations.
Create a structure that holds the bounds of active elements,
and metadata for two pages. Add routines to find those
active elements, lookup the pages, and run watchpoints
for those pages.
Temporarily mark the functions unused to avoid Werror.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/sve_helper.c | 240 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 240 insertions(+)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 8b470991db..3f653e46a0 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4155,6 +4155,246 @@ static intptr_t max_for_page(target_ulong base, intptr_t mem_off,
return MIN(split, mem_max - mem_off) + mem_off;
}
+/*
+ * Resolve the guest virtual address to info->host and info->flags.
+ * If @nofault, return false if the page is invalid, otherwise
+ * exit via page fault exception.
+ */
+
+typedef struct {
+ void *host;
+ int flags;
+ MemTxAttrs attrs;
+} SVEHostPage;
+
+static bool sve_probe_page(SVEHostPage *info, bool nofault,
+ CPUARMState *env, target_ulong addr,
+ int mem_off, MMUAccessType access_type,
+ int mmu_idx, uintptr_t retaddr)
+{
+ int flags;
+
+ addr += mem_off;
+ flags = probe_access_flags(env, addr, access_type, mmu_idx, nofault,
+ &info->host, retaddr);
+ info->flags = flags;
+
+ if (flags & TLB_INVALID_MASK) {
+ g_assert(nofault);
+ return false;
+ }
+
+ /* Ensure that info->host[] is relative to addr, not addr + mem_off. */
+ info->host -= mem_off;
+
+#ifdef CONFIG_USER_ONLY
+ memset(&info->attrs, 0, sizeof(info->attrs));
+#else
+ /*
+ * Find the iotlbentry for addr and return the transaction attributes.
+ * This *must* be present in the TLB because we just found the mapping.
+ */
+ {
+ uintptr_t index = tlb_index(env, mmu_idx, addr);
+
+# ifdef CONFIG_DEBUG_TCG
+ CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
+ target_ulong comparator = (access_type == MMU_DATA_LOAD
+ ? entry->addr_read
+ : tlb_addr_write(entry));
+ g_assert(tlb_hit(comparator, addr));
+# endif
+
+ CPUIOTLBEntry *iotlbentry = &env_tlb(env)->d[mmu_idx].iotlb[index];
+ info->attrs = iotlbentry->attrs;
+ }
+#endif
+
+ return true;
+}
+
+
+/*
+ * Analyse contiguous data, protected by a governing predicate.
+ */
+
+typedef enum {
+ FAULT_NO,
+ FAULT_FIRST,
+ FAULT_ALL,
+} SVEContFault;
+
+typedef struct {
+ /* First and last element wholy contained within the two pages. */
+ int16_t mem_off_first[2];
+ int16_t reg_off_first[2];
+ int16_t reg_off_last[2];
+
+ /* One element that is misaligned and spans both pages. */
+ int16_t mem_off_split;
+ int16_t reg_off_split;
+ int16_t page_split;
+
+ /* TLB data for the two pages. */
+ SVEHostPage page[2];
+} SVEContLdSt;
+
+/*
+ * Find first active element on each page, and a loose bound for the
+ * final element on each page. Identify any single element that spans
+ * the page boundary. Return true if there are any active elements.
+ */
+static bool __attribute__((unused))
+sve_cont_ldst_elements(SVEContLdSt *info, target_ulong addr, uint64_t *vg,
+ intptr_t reg_max, int esz, int msize)
+{
+ const int esize = 1 << esz;
+ const uint64_t pg_mask = pred_esz_masks[esz];
+ intptr_t reg_off_first = -1, reg_off_last = -1, reg_off_split;
+ intptr_t mem_off_last, mem_off_split;
+ intptr_t page_split, elt_split;
+ intptr_t i;
+
+ /* Set all of the element indicies to -1, and the TLB data to 0. */
+ memset(info, -1, offsetof(SVEContLdSt, page));
+ memset(info->page, 0, sizeof(info->page));
+
+ /* Gross scan over the entire predicate to find bounds. */
+ i = 0;
+ do {
+ uint64_t pg = vg[i] & pg_mask;
+ if (pg) {
+ reg_off_last = i * 64 + 63 - clz64(pg);
+ if (reg_off_first < 0) {
+ reg_off_first = i * 64 + ctz64(pg);
+ }
+ }
+ } while (++i * 64 < reg_max);
+
+ if (unlikely(reg_off_first < 0)) {
+ /* No active elements, no pages touched. */
+ return false;
+ }
+ tcg_debug_assert(reg_off_last >= 0 && reg_off_last < reg_max);
+
+ info->reg_off_first[0] = reg_off_first;
+ info->mem_off_first[0] = (reg_off_first >> esz) * msize;
+ mem_off_last = (reg_off_last >> esz) * msize;
+
+ page_split = -(addr | TARGET_PAGE_MASK);
+ if (likely(mem_off_last + msize <= page_split)) {
+ /* The entire operation fits within a single page. */
+ info->reg_off_last[0] = reg_off_last;
+ return true;
+ }
+
+ info->page_split = page_split;
+ elt_split = page_split / msize;
+ reg_off_split = elt_split << esz;
+ mem_off_split = elt_split * msize;
+
+ /*
+ * This is the last full element on the first page, but it is not
+ * necessarily active. If there is no full element, i.e. the first
+ * active element is the one that's split, this value remains -1.
+ * It is useful as iteration bounds.
+ */
+ if (elt_split != 0) {
+ info->reg_off_last[0] = reg_off_split - esize;
+ }
+
+ /* Determine if an unaligned element spans the pages. */
+ if (page_split % msize != 0) {
+ /* It is helpful to know if the split element is active. */
+ if ((vg[reg_off_split >> 6] >> (reg_off_split & 63)) & 1) {
+ info->reg_off_split = reg_off_split;
+ info->mem_off_split = mem_off_split;
+
+ if (reg_off_split == reg_off_last) {
+ /* The page crossing element is last. */
+ return true;
+ }
+ }
+ reg_off_split += esize;
+ mem_off_split += msize;
+ }
+
+ /*
+ * We do want the first active element on the second page, because
+ * this may affect the address reported in an exception.
+ */
+ reg_off_split = find_next_active(vg, reg_off_split, reg_max, esz);
+ tcg_debug_assert(reg_off_split <= reg_off_last);
+ info->reg_off_first[1] = reg_off_split;
+ info->mem_off_first[1] = (reg_off_split >> esz) * msize;
+ info->reg_off_last[1] = reg_off_last;
+ return true;
+}
+
+/*
+ * Resolve the guest virtual addresses to info->page[].
+ * Control the generation of page faults with @fault. Return false if
+ * there is no work to do, which can only happen with @fault == FAULT_NO.
+ */
+static bool __attribute__((unused))
+sve_cont_ldst_pages(SVEContLdSt *info, SVEContFault fault, CPUARMState *env,
+ target_ulong addr, MMUAccessType access_type,
+ uintptr_t retaddr)
+{
+ int mmu_idx = cpu_mmu_index(env, false);
+ int mem_off = info->mem_off_first[0];
+ bool nofault = fault == FAULT_NO;
+ bool have_work = true;
+
+ if (!sve_probe_page(&info->page[0], nofault, env, addr, mem_off,
+ access_type, mmu_idx, retaddr)) {
+ /* No work to be done. */
+ return false;
+ }
+
+ if (likely(info->page_split < 0)) {
+ /* The entire operation was on the one page. */
+ return true;
+ }
+
+ /*
+ * If the second page is invalid, then we want the fault address to be
+ * the first byte on that page which is accessed.
+ */
+ if (info->mem_off_split >= 0) {
+ /*
+ * There is an element split across the pages. The fault address
+ * should be the first byte of the second page.
+ */
+ mem_off = info->page_split;
+ /*
+ * If the split element is also the first active element
+ * of the vector, then: For first-fault we should continue
+ * to generate faults for the second page. For no-fault,
+ * we have work only if the second page is valid.
+ */
+ if (info->mem_off_first[0] < info->mem_off_split) {
+ nofault = FAULT_FIRST;
+ have_work = false;
+ }
+ } else {
+ /*
+ * There is no element split across the pages. The fault address
+ * should be the first active element on the second page.
+ */
+ mem_off = info->mem_off_first[1];
+ /*
+ * There must have been one active element on the first page,
+ * so we're out of first-fault territory.
+ */
+ nofault = fault != FAULT_ALL;
+ }
+
+ have_work |= sve_probe_page(&info->page[1], nofault, env, addr, mem_off,
+ access_type, mmu_idx, retaddr);
+ return have_work;
+}
+
/*
* The result of tlb_vaddr_to_host for user-only is just g2h(x),
* which is always non-null. Elide the useless test.
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH 06/16] target/arm: Add sve infrastructure for page lookup
2020-03-11 6:44 ` [PATCH 06/16] target/arm: Add sve infrastructure for page lookup Richard Henderson
@ 2020-04-16 13:03 ` Peter Maydell
2020-04-18 3:11 ` Richard Henderson
0 siblings, 1 reply; 44+ messages in thread
From: Peter Maydell @ 2020-04-16 13:03 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, QEMU Developers
On Wed, 11 Mar 2020 at 06:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> For contiguous predicated memory operations, we want to
> minimize the number of tlb lookups performed. We have
> open-coded this for sve_ld1_r, but for correctness with
> MTE we will need this for all of the memory operations.
>
> Create a structure that holds the bounds of active elements,
> and metadata for two pages. Add routines to find those
> active elements, lookup the pages, and run watchpoints
> for those pages.
>
> Temporarily mark the functions unused to avoid Werror.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/sve_helper.c | 240 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 240 insertions(+)
>
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 8b470991db..3f653e46a0 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -4155,6 +4155,246 @@ static intptr_t max_for_page(target_ulong base, intptr_t mem_off,
> return MIN(split, mem_max - mem_off) + mem_off;
> }
>
> +/*
> + * Resolve the guest virtual address to info->host and info->flags.
> + * If @nofault, return false if the page is invalid, otherwise
> + * exit via page fault exception.
> + */
> +
> +typedef struct {
> + void *host;
> + int flags;
> + MemTxAttrs attrs;
> +} SVEHostPage;
> +
> +static bool sve_probe_page(SVEHostPage *info, bool nofault,
> + CPUARMState *env, target_ulong addr,
> + int mem_off, MMUAccessType access_type,
> + int mmu_idx, uintptr_t retaddr)
> +{
> + int flags;
> +
> + addr += mem_off;
> + flags = probe_access_flags(env, addr, access_type, mmu_idx, nofault,
> + &info->host, retaddr);
> + info->flags = flags;
> +
> + if (flags & TLB_INVALID_MASK) {
> + g_assert(nofault);
> + return false;
> + }
> +
> + /* Ensure that info->host[] is relative to addr, not addr + mem_off. */
> + info->host -= mem_off;
> +
> +#ifdef CONFIG_USER_ONLY
> + memset(&info->attrs, 0, sizeof(info->attrs));
Could just write "info->attrs = {};" ?
> +#else
> + /*
> + * Find the iotlbentry for addr and return the transaction attributes.
> + * This *must* be present in the TLB because we just found the mapping.
> + */
> + {
> + uintptr_t index = tlb_index(env, mmu_idx, addr);
> +
> +# ifdef CONFIG_DEBUG_TCG
> + CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
> + target_ulong comparator = (access_type == MMU_DATA_LOAD
> + ? entry->addr_read
> + : tlb_addr_write(entry));
> + g_assert(tlb_hit(comparator, addr));
> +# endif
> +
> + CPUIOTLBEntry *iotlbentry = &env_tlb(env)->d[mmu_idx].iotlb[index];
> + info->attrs = iotlbentry->attrs;
> + }
> +#endif
> +
> + return true;
> +}
> +
> +
> +/*
> + * Analyse contiguous data, protected by a governing predicate.
> + */
> +
> +typedef enum {
> + FAULT_NO,
> + FAULT_FIRST,
> + FAULT_ALL,
> +} SVEContFault;
> +
> +typedef struct {
> + /* First and last element wholy contained within the two pages. */
"wholly"
> + int16_t mem_off_first[2];
> + int16_t reg_off_first[2];
> + int16_t reg_off_last[2];
It would be helpful to document what these actually are,
and in particular what the behaviour is if the whole thing
fits in a single page. (Judging by the code, the elements
at index 1 for the 2nd page are set to -1 ?)
> +
> + /* One element that is misaligned and spans both pages. */
> + int16_t mem_off_split;
> + int16_t reg_off_split;
> + int16_t page_split;
> +
> + /* TLB data for the two pages. */
> + SVEHostPage page[2];
> +} SVEContLdSt;
> +
> +/*
> + * Find first active element on each page, and a loose bound for the
> + * final element on each page. Identify any single element that spans
> + * the page boundary. Return true if there are any active elements.
> + */
> +static bool __attribute__((unused))
> +sve_cont_ldst_elements(SVEContLdSt *info, target_ulong addr, uint64_t *vg,
> + intptr_t reg_max, int esz, int msize)
> +{
> + const int esize = 1 << esz;
> + const uint64_t pg_mask = pred_esz_masks[esz];
> + intptr_t reg_off_first = -1, reg_off_last = -1, reg_off_split;
> + intptr_t mem_off_last, mem_off_split;
> + intptr_t page_split, elt_split;
> + intptr_t i;
intptr_t seems like a funny type to be using here, since these
aren't actually related to pointers as far as I can tell.
It's also odd that the type is not the same one used in the SVEContLdSt
struct for the corresponding fields.
> +
> + /* Set all of the element indicies to -1, and the TLB data to 0. */
"indices"
> + memset(info, -1, offsetof(SVEContLdSt, page));
I guess this isn't conceptually much different from zeroing
out integer struct fields, but it feels a bit less safe somehow.
> + memset(info->page, 0, sizeof(info->page));
> +
> + /* Gross scan over the entire predicate to find bounds. */
> + i = 0;
> + do {
> + uint64_t pg = vg[i] & pg_mask;
> + if (pg) {
> + reg_off_last = i * 64 + 63 - clz64(pg);
> + if (reg_off_first < 0) {
> + reg_off_first = i * 64 + ctz64(pg);
> + }
> + }
> + } while (++i * 64 < reg_max);
> +
> + if (unlikely(reg_off_first < 0)) {
> + /* No active elements, no pages touched. */
> + return false;
> + }
> + tcg_debug_assert(reg_off_last >= 0 && reg_off_last < reg_max);
> +
> + info->reg_off_first[0] = reg_off_first;
> + info->mem_off_first[0] = (reg_off_first >> esz) * msize;
> + mem_off_last = (reg_off_last >> esz) * msize;
> +
> + page_split = -(addr | TARGET_PAGE_MASK);
What is the negation for ?
> + if (likely(mem_off_last + msize <= page_split)) {
> + /* The entire operation fits within a single page. */
> + info->reg_off_last[0] = reg_off_last;
> + return true;
> + }
> +
> + info->page_split = page_split;
> + elt_split = page_split / msize;
> + reg_off_split = elt_split << esz;
> + mem_off_split = elt_split * msize;
> +
> + /*
> + * This is the last full element on the first page, but it is not
> + * necessarily active. If there is no full element, i.e. the first
> + * active element is the one that's split, this value remains -1.
> + * It is useful as iteration bounds.
> + */
> + if (elt_split != 0) {
> + info->reg_off_last[0] = reg_off_split - esize;
> + }
> +
> + /* Determine if an unaligned element spans the pages. */
> + if (page_split % msize != 0) {
> + /* It is helpful to know if the split element is active. */
> + if ((vg[reg_off_split >> 6] >> (reg_off_split & 63)) & 1) {
> + info->reg_off_split = reg_off_split;
> + info->mem_off_split = mem_off_split;
> +
> + if (reg_off_split == reg_off_last) {
> + /* The page crossing element is last. */
> + return true;
> + }
> + }
> + reg_off_split += esize;
> + mem_off_split += msize;
> + }
> +
> + /*
> + * We do want the first active element on the second page, because
> + * this may affect the address reported in an exception.
> + */
> + reg_off_split = find_next_active(vg, reg_off_split, reg_max, esz);
> + tcg_debug_assert(reg_off_split <= reg_off_last);
> + info->reg_off_first[1] = reg_off_split;
> + info->mem_off_first[1] = (reg_off_split >> esz) * msize;
> + info->reg_off_last[1] = reg_off_last;
> + return true;
> +}
thanks
-- PMM
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 06/16] target/arm: Add sve infrastructure for page lookup
2020-04-16 13:03 ` Peter Maydell
@ 2020-04-18 3:11 ` Richard Henderson
0 siblings, 0 replies; 44+ messages in thread
From: Richard Henderson @ 2020-04-18 3:11 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers
On 4/16/20 6:03 AM, Peter Maydell wrote:
>> +#ifdef CONFIG_USER_ONLY
>> + memset(&info->attrs, 0, sizeof(info->attrs));
>
> Could just write "info->attrs = {};" ?
Not quite. Correct syntax would be attrs = (MemTxAttrs){ }.
I don't see that as an improvement over memset though.
>> + int16_t mem_off_first[2];
>> + int16_t reg_off_first[2];
>> + int16_t reg_off_last[2];
>
> It would be helpful to document what these actually are,
> and in particular what the behaviour is if the whole thing
> fits in a single page. (Judging by the code, the elements
> at index 1 for the 2nd page are set to -1 ?)
Yes, that's right. I've added some more detail here.
>> + intptr_t reg_off_first = -1, reg_off_last = -1, reg_off_split;
>> + intptr_t mem_off_last, mem_off_split;
>> + intptr_t page_split, elt_split;
>> + intptr_t i;
>
> intptr_t seems like a funny type to be using here, since these
> aren't actually related to pointers as far as I can tell.
They're used as array indexes. If we use "int", the compiler keeps
re-extending the value.
>> + memset(info, -1, offsetof(SVEContLdSt, page));
>
> I guess this isn't conceptually much different from zeroing
> out integer struct fields, but it feels a bit less safe somehow.
It seems easier than setting 9 fields separately...
>> + page_split = -(addr | TARGET_PAGE_MASK);
>
> What is the negation for ?
Computation of remaining bytes in the page:
-(x | TARGET_PAGE_MASK)
= -(x | -TARGET_PAGE_SIZE)
= TARGET_PAGE_SIZE - (x & -TARGET_PAGE_SIZE)
We use this all over qemu.
r~
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 07/16] target/arm: Adjust interface of sve_ld1_host_fn
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (5 preceding siblings ...)
2020-03-11 6:44 ` [PATCH 06/16] target/arm: Add sve infrastructure for page lookup Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 13:16 ` Peter Maydell
2020-03-11 6:44 ` [PATCH 08/16] target/arm: Use SVEContLdSt in sve_ld1_r Richard Henderson
` (10 subsequent siblings)
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
The current interface includes a loop; change it to load a
single element. We will then be able to use the function
for ld{2,3,4} where individual vector elements are not adjacent.
Replace each call with the simplest possible loop over active
elements.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/sve_helper.c | 127 +++++++++++++++++++++-------------------
1 file changed, 66 insertions(+), 61 deletions(-)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 3f653e46a0..1397c2b634 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -3972,20 +3972,10 @@ void HELPER(sve_fcmla_zpzzz_d)(CPUARMState *env, void *vg, uint32_t desc)
*/
/*
- * Load elements into @vd, controlled by @vg, from @host + @mem_ofs.
- * Memory is valid through @host + @mem_max. The register element
- * indicies are inferred from @mem_ofs, as modified by the types for
- * which the helper is built. Return the @mem_ofs of the first element
- * not loaded (which is @mem_max if they are all loaded).
- *
- * For softmmu, we have fully validated the guest page. For user-only,
- * we cannot fully validate without taking the mmap lock, but since we
- * know the access is within one host page, if any access is valid they
- * all must be valid. However, when @vg is all false, it may be that
- * no access is valid.
+ * Load elements into @vd + @reg_off, from @host,
+ * or the reverse for stores.
*/
-typedef intptr_t sve_ld1_host_fn(void *vd, void *vg, void *host,
- intptr_t mem_ofs, intptr_t mem_max);
+typedef void sve_ldst1_host_fn(void *vd, intptr_t reg_off, void *host);
/*
* Load one element into @vd + @reg_off from (@env, @vaddr, @ra).
@@ -4000,23 +3990,16 @@ typedef void sve_ldst1_tlb_fn(CPUARMState *env, void *vd, intptr_t reg_off,
* For *_tlb, this uses the cpu_*_data_ra helpers. There are not
* endian-specific versions of these, so we must handle endianness
* locally.
+ *
+ * For *_host, this is a trivial application of the <qemu/bswap.h>
+ * endian-specific access followed by a store into the vector register.
*/
#define DO_LD_HOST(NAME, H, TYPEE, TYPEM, HOST) \
-static intptr_t sve_##NAME##_host(void *vd, void *vg, void *host, \
- intptr_t mem_off, const intptr_t mem_max) \
-{ \
- intptr_t reg_off = mem_off * (sizeof(TYPEE) / sizeof(TYPEM)); \
- uint64_t *pg = vg; \
- while (mem_off + sizeof(TYPEM) <= mem_max) { \
- TYPEM val = 0; \
- if (likely((pg[reg_off >> 6] >> (reg_off & 63)) & 1)) { \
- val = HOST(host + mem_off); \
- } \
- *(TYPEE *)(vd + H(reg_off)) = val; \
- mem_off += sizeof(TYPEM), reg_off += sizeof(TYPEE); \
- } \
- return mem_off; \
+static void sve_##NAME##_host(void *vd, intptr_t reg_off, void *host) \
+{ \
+ TYPEM val = HOST(host); \
+ *(TYPEE *)(vd + H(reg_off)) = val; \
}
#define DO_LD_TLB(NAME, H, TYPEE, TYPEM, BSWAP, TLB) \
@@ -4414,7 +4397,7 @@ static inline bool test_host_page(void *host)
static void sve_ld1_r(CPUARMState *env, void *vg, const target_ulong addr,
uint32_t desc, const uintptr_t retaddr,
const int esz, const int msz,
- sve_ld1_host_fn *host_fn,
+ sve_ldst1_host_fn *host_fn,
sve_ldst1_tlb_fn *tlb_fn)
{
const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
@@ -4448,8 +4431,12 @@ static void sve_ld1_r(CPUARMState *env, void *vg, const target_ulong addr,
if (likely(split == mem_max)) {
host = tlb_vaddr_to_host(env, addr + mem_off, MMU_DATA_LOAD, mmu_idx);
if (test_host_page(host)) {
- mem_off = host_fn(vd, vg, host - mem_off, mem_off, mem_max);
- tcg_debug_assert(mem_off == mem_max);
+ intptr_t i = reg_off;
+ host -= mem_off;
+ do {
+ host_fn(vd, i, host + (i >> diffsz));
+ i = find_next_active(vg, i + (1 << esz), reg_max, esz);
+ } while (i < reg_max);
/* After having taken any fault, zero leading inactive elements. */
swap_memzero(vd, reg_off);
return;
@@ -4462,7 +4449,12 @@ static void sve_ld1_r(CPUARMState *env, void *vg, const target_ulong addr,
*/
#ifdef CONFIG_USER_ONLY
swap_memzero(&scratch, reg_off);
- host_fn(&scratch, vg, g2h(addr), mem_off, mem_max);
+ host = g2h(addr);
+ do {
+ host_fn(&scratch, reg_off, host + (reg_off >> diffsz));
+ reg_off += 1 << esz;
+ reg_off = find_next_active(vg, reg_off, reg_max, esz);
+ } while (reg_off < reg_max);
#else
memset(&scratch, 0, reg_max);
goto start;
@@ -4480,9 +4472,13 @@ static void sve_ld1_r(CPUARMState *env, void *vg, const target_ulong addr,
host = tlb_vaddr_to_host(env, addr + mem_off,
MMU_DATA_LOAD, mmu_idx);
if (host) {
- mem_off = host_fn(&scratch, vg, host - mem_off,
- mem_off, split);
- reg_off = mem_off << diffsz;
+ host -= mem_off;
+ do {
+ host_fn(&scratch, reg_off, host + mem_off);
+ reg_off += 1 << esz;
+ reg_off = find_next_active(vg, reg_off, reg_max, esz);
+ mem_off = reg_off >> diffsz;
+ } while (split - mem_off >= (1 << msz));
continue;
}
}
@@ -4709,7 +4705,7 @@ static void record_fault(CPUARMState *env, uintptr_t i, uintptr_t oprsz)
static void sve_ldff1_r(CPUARMState *env, void *vg, const target_ulong addr,
uint32_t desc, const uintptr_t retaddr,
const int esz, const int msz,
- sve_ld1_host_fn *host_fn,
+ sve_ldst1_host_fn *host_fn,
sve_ldst1_tlb_fn *tlb_fn)
{
const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
@@ -4719,7 +4715,7 @@ static void sve_ldff1_r(CPUARMState *env, void *vg, const target_ulong addr,
const int diffsz = esz - msz;
const intptr_t reg_max = simd_oprsz(desc);
const intptr_t mem_max = reg_max >> diffsz;
- intptr_t split, reg_off, mem_off;
+ intptr_t split, reg_off, mem_off, i;
void *host;
/* Skip to the first active element. */
@@ -4742,28 +4738,18 @@ static void sve_ldff1_r(CPUARMState *env, void *vg, const target_ulong addr,
if (likely(split == mem_max)) {
host = tlb_vaddr_to_host(env, addr + mem_off, MMU_DATA_LOAD, mmu_idx);
if (test_host_page(host)) {
- mem_off = host_fn(vd, vg, host - mem_off, mem_off, mem_max);
- tcg_debug_assert(mem_off == mem_max);
+ i = reg_off;
+ host -= mem_off;
+ do {
+ host_fn(vd, i, host + (i >> diffsz));
+ i = find_next_active(vg, i + (1 << esz), reg_max, esz);
+ } while (i < reg_max);
/* After any fault, zero any leading inactive elements. */
swap_memzero(vd, reg_off);
return;
}
}
-#ifdef CONFIG_USER_ONLY
- /*
- * The page(s) containing this first element at ADDR+MEM_OFF must
- * be valid. Considering that this first element may be misaligned
- * and cross a page boundary itself, take the rest of the page from
- * the last byte of the element.
- */
- split = max_for_page(addr, mem_off + (1 << msz) - 1, mem_max);
- mem_off = host_fn(vd, vg, g2h(addr), mem_off, split);
-
- /* After any fault, zero any leading inactive elements. */
- swap_memzero(vd, reg_off);
- reg_off = mem_off << diffsz;
-#else
/*
* Perform one normal read, which will fault or not.
* But it is likely to bring the page into the tlb.
@@ -4780,11 +4766,15 @@ static void sve_ldff1_r(CPUARMState *env, void *vg, const target_ulong addr,
if (split >= (1 << msz)) {
host = tlb_vaddr_to_host(env, addr + mem_off, MMU_DATA_LOAD, mmu_idx);
if (host) {
- mem_off = host_fn(vd, vg, host - mem_off, mem_off, split);
- reg_off = mem_off << diffsz;
+ host -= mem_off;
+ do {
+ host_fn(vd, reg_off, host + mem_off);
+ reg_off += 1 << esz;
+ reg_off = find_next_active(vg, reg_off, reg_max, esz);
+ mem_off = reg_off >> diffsz;
+ } while (split - mem_off >= (1 << msz));
}
}
-#endif
record_fault(env, reg_off, reg_max);
}
@@ -4794,7 +4784,7 @@ static void sve_ldff1_r(CPUARMState *env, void *vg, const target_ulong addr,
*/
static void sve_ldnf1_r(CPUARMState *env, void *vg, const target_ulong addr,
uint32_t desc, const int esz, const int msz,
- sve_ld1_host_fn *host_fn)
+ sve_ldst1_host_fn *host_fn)
{
const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
void *vd = &env->vfp.zregs[rd];
@@ -4809,7 +4799,13 @@ static void sve_ldnf1_r(CPUARMState *env, void *vg, const target_ulong addr,
host = tlb_vaddr_to_host(env, addr, MMU_DATA_LOAD, mmu_idx);
if (likely(page_check_range(addr, mem_max, PAGE_READ) == 0)) {
/* The entire operation is valid and will not fault. */
- host_fn(vd, vg, host, 0, mem_max);
+ reg_off = 0;
+ do {
+ mem_off = reg_off >> diffsz;
+ host_fn(vd, reg_off, host + mem_off);
+ reg_off += 1 << esz;
+ reg_off = find_next_active(vg, reg_off, reg_max, esz);
+ } while (reg_off < reg_max);
return;
}
#endif
@@ -4829,8 +4825,12 @@ static void sve_ldnf1_r(CPUARMState *env, void *vg, const target_ulong addr,
if (page_check_range(addr + mem_off, 1 << msz, PAGE_READ) == 0) {
/* At least one load is valid; take the rest of the page. */
split = max_for_page(addr, mem_off + (1 << msz) - 1, mem_max);
- mem_off = host_fn(vd, vg, host, mem_off, split);
- reg_off = mem_off << diffsz;
+ do {
+ host_fn(vd, reg_off, host + mem_off);
+ reg_off += 1 << esz;
+ reg_off = find_next_active(vg, reg_off, reg_max, esz);
+ mem_off = reg_off >> diffsz;
+ } while (split - mem_off >= (1 << msz));
}
#else
/*
@@ -4851,8 +4851,13 @@ static void sve_ldnf1_r(CPUARMState *env, void *vg, const target_ulong addr,
host = tlb_vaddr_to_host(env, addr + mem_off, MMU_DATA_LOAD, mmu_idx);
split = max_for_page(addr, mem_off, mem_max);
if (host && split >= (1 << msz)) {
- mem_off = host_fn(vd, vg, host - mem_off, mem_off, split);
- reg_off = mem_off << diffsz;
+ host -= mem_off;
+ do {
+ host_fn(vd, reg_off, host + mem_off);
+ reg_off += 1 << esz;
+ reg_off = find_next_active(vg, reg_off, reg_max, esz);
+ mem_off = reg_off >> diffsz;
+ } while (split - mem_off >= (1 << msz));
}
#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH 07/16] target/arm: Adjust interface of sve_ld1_host_fn
2020-03-11 6:44 ` [PATCH 07/16] target/arm: Adjust interface of sve_ld1_host_fn Richard Henderson
@ 2020-04-16 13:16 ` Peter Maydell
0 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-04-16 13:16 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, QEMU Developers
On Wed, 11 Mar 2020 at 06:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The current interface includes a loop; change it to load a
> single element. We will then be able to use the function
> for ld{2,3,4} where individual vector elements are not adjacent.
>
> Replace each call with the simplest possible loop over active
> elements.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
(I haven't attempted to verify the details of all the
arithmetic, but I assume you've tested it all...)
thanks
-- PMM
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 08/16] target/arm: Use SVEContLdSt in sve_ld1_r
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (6 preceding siblings ...)
2020-03-11 6:44 ` [PATCH 07/16] target/arm: Adjust interface of sve_ld1_host_fn Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 13:26 ` Peter Maydell
2020-03-11 6:44 ` [PATCH 09/16] target/arm: Handle watchpoints " Richard Henderson
` (9 subsequent siblings)
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
First use of the new helper functions, so we can remove the
unused markup. No longer need a scratch for user-only, as
we completely probe the page set before reading; system mode
still requires a scratch for MMIO.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/sve_helper.c | 188 +++++++++++++++++++++-------------------
1 file changed, 97 insertions(+), 91 deletions(-)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 1397c2b634..b827900a4e 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4227,9 +4227,9 @@ typedef struct {
* final element on each page. Identify any single element that spans
* the page boundary. Return true if there are any active elements.
*/
-static bool __attribute__((unused))
-sve_cont_ldst_elements(SVEContLdSt *info, target_ulong addr, uint64_t *vg,
- intptr_t reg_max, int esz, int msize)
+static bool sve_cont_ldst_elements(SVEContLdSt *info, target_ulong addr,
+ uint64_t *vg, intptr_t reg_max,
+ int esz, int msize)
{
const int esize = 1 << esz;
const uint64_t pg_mask = pred_esz_masks[esz];
@@ -4319,10 +4319,9 @@ sve_cont_ldst_elements(SVEContLdSt *info, target_ulong addr, uint64_t *vg,
* Control the generation of page faults with @fault. Return false if
* there is no work to do, which can only happen with @fault == FAULT_NO.
*/
-static bool __attribute__((unused))
-sve_cont_ldst_pages(SVEContLdSt *info, SVEContFault fault, CPUARMState *env,
- target_ulong addr, MMUAccessType access_type,
- uintptr_t retaddr)
+static bool sve_cont_ldst_pages(SVEContLdSt *info, SVEContFault fault,
+ CPUARMState *env, target_ulong addr,
+ MMUAccessType access_type, uintptr_t retaddr)
{
int mmu_idx = cpu_mmu_index(env, false);
int mem_off = info->mem_off_first[0];
@@ -4394,109 +4393,116 @@ static inline bool test_host_page(void *host)
/*
* Common helper for all contiguous one-register predicated loads.
*/
-static void sve_ld1_r(CPUARMState *env, void *vg, const target_ulong addr,
- uint32_t desc, const uintptr_t retaddr,
- const int esz, const int msz,
- sve_ldst1_host_fn *host_fn,
- sve_ldst1_tlb_fn *tlb_fn)
+static inline QEMU_ALWAYS_INLINE
+void sve_ld1_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
+ uint32_t desc, const uintptr_t retaddr,
+ const int esz, const int msz,
+ sve_ldst1_host_fn *host_fn,
+ sve_ldst1_tlb_fn *tlb_fn)
{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
- const int mmu_idx = get_mmuidx(oi);
const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
void *vd = &env->vfp.zregs[rd];
- const int diffsz = esz - msz;
const intptr_t reg_max = simd_oprsz(desc);
- const intptr_t mem_max = reg_max >> diffsz;
- ARMVectorReg scratch;
+ intptr_t reg_off, reg_last, mem_off;
+ SVEContLdSt info;
void *host;
- intptr_t split, reg_off, mem_off;
+ int flags;
- /* Find the first active element. */
- reg_off = find_next_active(vg, 0, reg_max, esz);
- if (unlikely(reg_off == reg_max)) {
+ /* Find the active elements. */
+ if (!sve_cont_ldst_elements(&info, addr, vg, reg_max, esz, 1 << msz)) {
/* The entire predicate was false; no load occurs. */
memset(vd, 0, reg_max);
return;
}
- mem_off = reg_off >> diffsz;
- /*
- * If the (remaining) load is entirely within a single page, then:
- * For softmmu, and the tlb hits, then no faults will occur;
- * For user-only, either the first load will fault or none will.
- * We can thus perform the load directly to the destination and
- * Vd will be unmodified on any exception path.
- */
- split = max_for_page(addr, mem_off, mem_max);
- if (likely(split == mem_max)) {
- host = tlb_vaddr_to_host(env, addr + mem_off, MMU_DATA_LOAD, mmu_idx);
- if (test_host_page(host)) {
- intptr_t i = reg_off;
- host -= mem_off;
- do {
- host_fn(vd, i, host + (i >> diffsz));
- i = find_next_active(vg, i + (1 << esz), reg_max, esz);
- } while (i < reg_max);
- /* After having taken any fault, zero leading inactive elements. */
- swap_memzero(vd, reg_off);
- return;
- }
- }
+ /* Probe the page(s). Exit with exception for any invalid page. */
+ sve_cont_ldst_pages(&info, FAULT_ALL, env, addr, MMU_DATA_LOAD, retaddr);
- /*
- * Perform the predicated read into a temporary, thus ensuring
- * if the load of the last element faults, Vd is not modified.
- */
+ flags = info.page[0].flags | info.page[1].flags;
+ if (unlikely(flags != 0)) {
#ifdef CONFIG_USER_ONLY
- swap_memzero(&scratch, reg_off);
- host = g2h(addr);
- do {
- host_fn(&scratch, reg_off, host + (reg_off >> diffsz));
- reg_off += 1 << esz;
- reg_off = find_next_active(vg, reg_off, reg_max, esz);
- } while (reg_off < reg_max);
+ g_assert_not_reached();
#else
- memset(&scratch, 0, reg_max);
- goto start;
- while (1) {
- reg_off = find_next_active(vg, reg_off, reg_max, esz);
- if (reg_off >= reg_max) {
- break;
- }
- mem_off = reg_off >> diffsz;
- split = max_for_page(addr, mem_off, mem_max);
+ /*
+ * At least one page includes MMIO (or watchpoints).
+ * Any bus operation can fail with cpu_transaction_failed,
+ * which for ARM will raise SyncExternal. Perform the load
+ * into scratch memory to preserve register state until the end.
+ */
+ ARMVectorReg scratch;
- start:
- if (split - mem_off >= (1 << msz)) {
- /* At least one whole element on this page. */
- host = tlb_vaddr_to_host(env, addr + mem_off,
- MMU_DATA_LOAD, mmu_idx);
- if (host) {
- host -= mem_off;
- do {
- host_fn(&scratch, reg_off, host + mem_off);
- reg_off += 1 << esz;
- reg_off = find_next_active(vg, reg_off, reg_max, esz);
- mem_off = reg_off >> diffsz;
- } while (split - mem_off >= (1 << msz));
- continue;
+ memset(&scratch, 0, reg_max);
+ mem_off = info.mem_off_first[0];
+ reg_off = info.reg_off_first[0];
+ reg_last = info.reg_off_last[1];
+ if (reg_last < 0) {
+ reg_last = info.reg_off_split;
+ if (reg_last < 0) {
+ reg_last = info.reg_off_last[0];
}
}
- /*
- * Perform one normal read. This may fault, longjmping out to the
- * main loop in order to raise an exception. It may succeed, and
- * as a side-effect load the TLB entry for the next round. Finally,
- * in the extremely unlikely case we're performing this operation
- * on I/O memory, it may succeed but not bring in the TLB entry.
- * But even then we have still made forward progress.
- */
- tlb_fn(env, &scratch, reg_off, addr + mem_off, retaddr);
- reg_off += 1 << esz;
- }
-#endif
+ do {
+ uint64_t pg = vg[reg_off >> 6];
+ do {
+ if ((pg >> (reg_off & 63)) & 1) {
+ tlb_fn(env, &scratch, reg_off, addr + mem_off, retaddr);
+ }
+ reg_off += 1 << esz;
+ mem_off += 1 << msz;
+ } while (reg_off & 63);
+ } while (reg_off <= reg_last);
- memcpy(vd, &scratch, reg_max);
+ memcpy(vd, &scratch, reg_max);
+ return;
+#endif
+ }
+
+ /* The entire operation is in RAM, on valid pages. */
+
+ memset(vd, 0, reg_max);
+ mem_off = info.mem_off_first[0];
+ reg_off = info.reg_off_first[0];
+ reg_last = info.reg_off_last[0];
+ host = info.page[0].host;
+
+ while (reg_off <= reg_last) {
+ uint64_t pg = vg[reg_off >> 6];
+ do {
+ if ((pg >> (reg_off & 63)) & 1) {
+ host_fn(vd, reg_off, host + mem_off);
+ }
+ reg_off += 1 << esz;
+ mem_off += 1 << msz;
+ } while (reg_off <= reg_last && (reg_off & 63));
+ }
+
+ /*
+ * Use the slow path to manage the cross-page misalignment.
+ * But we know this is RAM and cannot trap.
+ */
+ mem_off = info.mem_off_split;
+ if (unlikely(mem_off >= 0)) {
+ tlb_fn(env, vd, info.reg_off_split, addr + mem_off, retaddr);
+ }
+
+ mem_off = info.mem_off_first[1];
+ if (unlikely(mem_off >= 0)) {
+ reg_off = info.reg_off_first[1];
+ reg_last = info.reg_off_last[1];
+ host = info.page[1].host;
+
+ do {
+ uint64_t pg = vg[reg_off >> 6];
+ do {
+ if ((pg >> (reg_off & 63)) & 1) {
+ host_fn(vd, reg_off, host + mem_off);
+ }
+ reg_off += 1 << esz;
+ mem_off += 1 << msz;
+ } while (reg_off & 63);
+ } while (reg_off <= reg_last);
+ }
}
#define DO_LD1_1(NAME, ESZ) \
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH 08/16] target/arm: Use SVEContLdSt in sve_ld1_r
2020-03-11 6:44 ` [PATCH 08/16] target/arm: Use SVEContLdSt in sve_ld1_r Richard Henderson
@ 2020-04-16 13:26 ` Peter Maydell
2020-04-18 3:41 ` Richard Henderson
0 siblings, 1 reply; 44+ messages in thread
From: Peter Maydell @ 2020-04-16 13:26 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, QEMU Developers
On Wed, 11 Mar 2020 at 06:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> First use of the new helper functions, so we can remove the
> unused markup. No longer need a scratch for user-only, as
> we completely probe the page set before reading; system mode
> still requires a scratch for MMIO.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> + /* The entire operation is in RAM, on valid pages. */
> +
> + memset(vd, 0, reg_max);
> + mem_off = info.mem_off_first[0];
> + reg_off = info.reg_off_first[0];
> + reg_last = info.reg_off_last[0];
> + host = info.page[0].host;
> +
> + while (reg_off <= reg_last) {
> + uint64_t pg = vg[reg_off >> 6];
> + do {
> + if ((pg >> (reg_off & 63)) & 1) {
> + host_fn(vd, reg_off, host + mem_off);
> + }
> + reg_off += 1 << esz;
> + mem_off += 1 << msz;
> + } while (reg_off <= reg_last && (reg_off & 63));
> + }
> +
> + /*
> + * Use the slow path to manage the cross-page misalignment.
> + * But we know this is RAM and cannot trap.
> + */
> + mem_off = info.mem_off_split;
> + if (unlikely(mem_off >= 0)) {
> + tlb_fn(env, vd, info.reg_off_split, addr + mem_off, retaddr);
> + }
> +
> + mem_off = info.mem_off_first[1];
> + if (unlikely(mem_off >= 0)) {
> + reg_off = info.reg_off_first[1];
> + reg_last = info.reg_off_last[1];
> + host = info.page[1].host;
> +
> + do {
> + uint64_t pg = vg[reg_off >> 6];
> + do {
> + if ((pg >> (reg_off & 63)) & 1) {
> + host_fn(vd, reg_off, host + mem_off);
> + }
> + reg_off += 1 << esz;
> + mem_off += 1 << msz;
> + } while (reg_off & 63);
> + } while (reg_off <= reg_last);
Does this loop for the second page need to be phrased
differently than the loop for the first page was? I was
expecting the two chunks of code to be identical, and they
almost are, but not quite...
Either way
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 08/16] target/arm: Use SVEContLdSt in sve_ld1_r
2020-04-16 13:26 ` Peter Maydell
@ 2020-04-18 3:41 ` Richard Henderson
0 siblings, 0 replies; 44+ messages in thread
From: Richard Henderson @ 2020-04-18 3:41 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers
On 4/16/20 6:26 AM, Peter Maydell wrote:
> On Wed, 11 Mar 2020 at 06:44, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> First use of the new helper functions, so we can remove the
>> unused markup. No longer need a scratch for user-only, as
>> we completely probe the page set before reading; system mode
>> still requires a scratch for MMIO.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>
>> + /* The entire operation is in RAM, on valid pages. */
>> +
>> + memset(vd, 0, reg_max);
>> + mem_off = info.mem_off_first[0];
>> + reg_off = info.reg_off_first[0];
>> + reg_last = info.reg_off_last[0];
>> + host = info.page[0].host;
>> +
>> + while (reg_off <= reg_last) {
>> + uint64_t pg = vg[reg_off >> 6];
>> + do {
>> + if ((pg >> (reg_off & 63)) & 1) {
>> + host_fn(vd, reg_off, host + mem_off);
>> + }
>> + reg_off += 1 << esz;
>> + mem_off += 1 << msz;
>> + } while (reg_off <= reg_last && (reg_off & 63));
>> + }
>> +
>> + /*
>> + * Use the slow path to manage the cross-page misalignment.
>> + * But we know this is RAM and cannot trap.
>> + */
>> + mem_off = info.mem_off_split;
>> + if (unlikely(mem_off >= 0)) {
>> + tlb_fn(env, vd, info.reg_off_split, addr + mem_off, retaddr);
>> + }
>> +
>> + mem_off = info.mem_off_first[1];
>> + if (unlikely(mem_off >= 0)) {
>> + reg_off = info.reg_off_first[1];
>> + reg_last = info.reg_off_last[1];
>> + host = info.page[1].host;
>> +
>> + do {
>> + uint64_t pg = vg[reg_off >> 6];
>> + do {
>> + if ((pg >> (reg_off & 63)) & 1) {
>> + host_fn(vd, reg_off, host + mem_off);
>> + }
>> + reg_off += 1 << esz;
>> + mem_off += 1 << msz;
>> + } while (reg_off & 63);
>> + } while (reg_off <= reg_last);
>
> Does this loop for the second page need to be phrased
> differently than the loop for the first page was? I was
> expecting the two chunks of code to be identical, and they
> almost are, but not quite...
Yes, they do need to be different. In particular, the first page may only have
the one element that crosses the page boundary, so we may have reg_first >
reg_last. The second page is never has that problem.
r~
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 09/16] target/arm: Handle watchpoints in sve_ld1_r
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (7 preceding siblings ...)
2020-03-11 6:44 ` [PATCH 08/16] target/arm: Use SVEContLdSt in sve_ld1_r Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 13:29 ` Peter Maydell
2020-03-11 6:44 ` [PATCH 10/16] target/arm: Use SVEContLdSt for multi-register contiguous loads Richard Henderson
` (8 subsequent siblings)
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
Handle all of the watchpoints for active elements all at once,
before we've modified the vector register. This removes the
TLB_WATCHPOINT bit from page[].flags, which means that we can
use the normal fast path via RAM.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/sve_helper.c | 70 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 69 insertions(+), 1 deletion(-)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index b827900a4e..2c0e838ed3 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4377,6 +4377,68 @@ static bool sve_cont_ldst_pages(SVEContLdSt *info, SVEContFault fault,
return have_work;
}
+static void sve_cont_ldst_watchpoints(SVEContLdSt *info, CPUARMState *env,
+ uint64_t *vg, target_ulong addr,
+ int esize, int msize, int wp_access,
+ uintptr_t retaddr)
+{
+#ifndef CONFIG_USER_ONLY
+ intptr_t mem_off, reg_off, reg_last;
+ int flags0 = info->page[0].flags;
+ int flags1 = info->page[1].flags;
+
+ if (likely(!((flags0 | flags1) & TLB_WATCHPOINT))) {
+ return;
+ }
+
+ /* Indicate that watchpoints are handled. */
+ info->page[0].flags = flags0 & ~TLB_WATCHPOINT;
+ info->page[1].flags = flags1 & ~TLB_WATCHPOINT;
+
+ if (flags0 & TLB_WATCHPOINT) {
+ mem_off = info->mem_off_first[0];
+ reg_off = info->reg_off_first[0];
+ reg_last = info->reg_off_last[0];
+
+ while (reg_off <= reg_last) {
+ uint64_t pg = vg[reg_off >> 6];
+ do {
+ if ((pg >> (reg_off & 63)) & 1) {
+ cpu_check_watchpoint(env_cpu(env), addr + mem_off, msize,
+ info->page[0].attrs, wp_access, retaddr);
+ }
+ reg_off += esize;
+ mem_off += msize;
+ } while (reg_off <= reg_last && (reg_off & 63));
+ }
+ }
+
+ mem_off = info->mem_off_split;
+ if (mem_off >= 0) {
+ cpu_check_watchpoint(env_cpu(env), addr + mem_off, msize,
+ info->page[0].attrs, wp_access, retaddr);
+ }
+
+ mem_off = info->mem_off_first[1];
+ if ((flags1 & TLB_WATCHPOINT) && mem_off >= 0) {
+ reg_off = info->reg_off_first[1];
+ reg_last = info->reg_off_last[1];
+
+ do {
+ uint64_t pg = vg[reg_off >> 6];
+ do {
+ if ((pg >> (reg_off & 63)) & 1) {
+ cpu_check_watchpoint(env_cpu(env), addr + mem_off, msize,
+ info->page[1].attrs, wp_access, retaddr);
+ }
+ reg_off += esize;
+ mem_off += msize;
+ } while (reg_off & 63);
+ } while (reg_off <= reg_last);
+ }
+#endif
+}
+
/*
* The result of tlb_vaddr_to_host for user-only is just g2h(x),
* which is always non-null. Elide the useless test.
@@ -4418,13 +4480,19 @@ void sve_ld1_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
/* Probe the page(s). Exit with exception for any invalid page. */
sve_cont_ldst_pages(&info, FAULT_ALL, env, addr, MMU_DATA_LOAD, retaddr);
+ /* Handle watchpoints for all active elements. */
+ sve_cont_ldst_watchpoints(&info, env, vg, addr, 1 << esz, 1 << msz,
+ BP_MEM_READ, retaddr);
+
+ /* TODO: MTE check. */
+
flags = info.page[0].flags | info.page[1].flags;
if (unlikely(flags != 0)) {
#ifdef CONFIG_USER_ONLY
g_assert_not_reached();
#else
/*
- * At least one page includes MMIO (or watchpoints).
+ * At least one page includes MMIO.
* Any bus operation can fail with cpu_transaction_failed,
* which for ARM will raise SyncExternal. Perform the load
* into scratch memory to preserve register state until the end.
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH 09/16] target/arm: Handle watchpoints in sve_ld1_r
2020-03-11 6:44 ` [PATCH 09/16] target/arm: Handle watchpoints " Richard Henderson
@ 2020-04-16 13:29 ` Peter Maydell
0 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-04-16 13:29 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, QEMU Developers
On Wed, 11 Mar 2020 at 06:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Handle all of the watchpoints for active elements all at once,
> before we've modified the vector register. This removes the
> TLB_WATCHPOINT bit from page[].flags, which means that we can
> use the normal fast path via RAM.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> + if (flags0 & TLB_WATCHPOINT) {
> + mem_off = info->mem_off_first[0];
> + reg_off = info->reg_off_first[0];
> + reg_last = info->reg_off_last[0];
> +
> + while (reg_off <= reg_last) {
> + uint64_t pg = vg[reg_off >> 6];
> + do {
> + if ((pg >> (reg_off & 63)) & 1) {
> + cpu_check_watchpoint(env_cpu(env), addr + mem_off, msize,
> + info->page[0].attrs, wp_access, retaddr);
> + }
> + reg_off += esize;
> + mem_off += msize;
> + } while (reg_off <= reg_last && (reg_off & 63));
> + }
> + }
> +
> + mem_off = info->mem_off_split;
> + if (mem_off >= 0) {
> + cpu_check_watchpoint(env_cpu(env), addr + mem_off, msize,
> + info->page[0].attrs, wp_access, retaddr);
> + }
> +
> + mem_off = info->mem_off_first[1];
> + if ((flags1 & TLB_WATCHPOINT) && mem_off >= 0) {
> + reg_off = info->reg_off_first[1];
> + reg_last = info->reg_off_last[1];
> +
> + do {
> + uint64_t pg = vg[reg_off >> 6];
> + do {
> + if ((pg >> (reg_off & 63)) & 1) {
> + cpu_check_watchpoint(env_cpu(env), addr + mem_off, msize,
> + info->page[1].attrs, wp_access, retaddr);
> + }
> + reg_off += esize;
> + mem_off += msize;
> + } while (reg_off & 63);
> + } while (reg_off <= reg_last);
> + }
Another pair of almost-but-not-quite-the-same loops.
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 10/16] target/arm: Use SVEContLdSt for multi-register contiguous loads
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (8 preceding siblings ...)
2020-03-11 6:44 ` [PATCH 09/16] target/arm: Handle watchpoints " Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 13:35 ` Peter Maydell
2020-03-11 6:44 ` [PATCH 11/16] target/arm: Update contiguous first-fault and no-fault loads Richard Henderson
` (7 subsequent siblings)
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/sve_helper.c | 223 ++++++++++++++--------------------------
1 file changed, 79 insertions(+), 144 deletions(-)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 2c0e838ed3..56407446eb 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4453,27 +4453,28 @@ static inline bool test_host_page(void *host)
}
/*
- * Common helper for all contiguous one-register predicated loads.
+ * Common helper for all contiguous 1,2,3,4-register predicated stores.
*/
static inline QEMU_ALWAYS_INLINE
-void sve_ld1_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
+void sve_ldN_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
uint32_t desc, const uintptr_t retaddr,
- const int esz, const int msz,
+ const int esz, const int msz, const int N,
sve_ldst1_host_fn *host_fn,
sve_ldst1_tlb_fn *tlb_fn)
{
const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
- void *vd = &env->vfp.zregs[rd];
const intptr_t reg_max = simd_oprsz(desc);
intptr_t reg_off, reg_last, mem_off;
SVEContLdSt info;
void *host;
- int flags;
+ int flags, i;
/* Find the active elements. */
- if (!sve_cont_ldst_elements(&info, addr, vg, reg_max, esz, 1 << msz)) {
+ if (!sve_cont_ldst_elements(&info, addr, vg, reg_max, esz, N << msz)) {
/* The entire predicate was false; no load occurs. */
- memset(vd, 0, reg_max);
+ for (i = 0; i < N; ++i) {
+ memset(&env->vfp.zregs[(rd + i) & 31], 0, reg_max);
+ }
return;
}
@@ -4481,7 +4482,7 @@ void sve_ld1_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
sve_cont_ldst_pages(&info, FAULT_ALL, env, addr, MMU_DATA_LOAD, retaddr);
/* Handle watchpoints for all active elements. */
- sve_cont_ldst_watchpoints(&info, env, vg, addr, 1 << esz, 1 << msz,
+ sve_cont_ldst_watchpoints(&info, env, vg, addr, 1 << esz, N << msz,
BP_MEM_READ, retaddr);
/* TODO: MTE check. */
@@ -4497,9 +4498,8 @@ void sve_ld1_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
* which for ARM will raise SyncExternal. Perform the load
* into scratch memory to preserve register state until the end.
*/
- ARMVectorReg scratch;
+ ARMVectorReg scratch[4] = { };
- memset(&scratch, 0, reg_max);
mem_off = info.mem_off_first[0];
reg_off = info.reg_off_first[0];
reg_last = info.reg_off_last[1];
@@ -4514,21 +4514,29 @@ void sve_ld1_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
uint64_t pg = vg[reg_off >> 6];
do {
if ((pg >> (reg_off & 63)) & 1) {
- tlb_fn(env, &scratch, reg_off, addr + mem_off, retaddr);
+ for (i = 0; i < N; ++i) {
+ tlb_fn(env, &scratch[i], reg_off,
+ addr + mem_off + (i << msz), retaddr);
+ }
}
reg_off += 1 << esz;
- mem_off += 1 << msz;
+ mem_off += N << msz;
} while (reg_off & 63);
} while (reg_off <= reg_last);
- memcpy(vd, &scratch, reg_max);
+ for (i = 0; i < N; ++i) {
+ memcpy(&env->vfp.zregs[(rd + i) & 31], &scratch[i], reg_max);
+ }
return;
#endif
}
/* The entire operation is in RAM, on valid pages. */
- memset(vd, 0, reg_max);
+ for (i = 0; i < N; ++i) {
+ memset(&env->vfp.zregs[(rd + i) & 31], 0, reg_max);
+ }
+
mem_off = info.mem_off_first[0];
reg_off = info.reg_off_first[0];
reg_last = info.reg_off_last[0];
@@ -4538,10 +4546,13 @@ void sve_ld1_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
uint64_t pg = vg[reg_off >> 6];
do {
if ((pg >> (reg_off & 63)) & 1) {
- host_fn(vd, reg_off, host + mem_off);
+ for (i = 0; i < N; ++i) {
+ host_fn(&env->vfp.zregs[(rd + i) & 31], reg_off,
+ host + mem_off + (i << msz));
+ }
}
reg_off += 1 << esz;
- mem_off += 1 << msz;
+ mem_off += N << msz;
} while (reg_off <= reg_last && (reg_off & 63));
}
@@ -4551,7 +4562,11 @@ void sve_ld1_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
*/
mem_off = info.mem_off_split;
if (unlikely(mem_off >= 0)) {
- tlb_fn(env, vd, info.reg_off_split, addr + mem_off, retaddr);
+ reg_off = info.reg_off_split;
+ for (i = 0; i < N; ++i) {
+ tlb_fn(env, &env->vfp.zregs[(rd + i) & 31], reg_off,
+ addr + mem_off + (i << msz), retaddr);
+ }
}
mem_off = info.mem_off_first[1];
@@ -4564,10 +4579,13 @@ void sve_ld1_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
uint64_t pg = vg[reg_off >> 6];
do {
if ((pg >> (reg_off & 63)) & 1) {
- host_fn(vd, reg_off, host + mem_off);
+ for (i = 0; i < N; ++i) {
+ host_fn(&env->vfp.zregs[(rd + i) & 31], reg_off,
+ host + mem_off + (i << msz));
+ }
}
reg_off += 1 << esz;
- mem_off += 1 << msz;
+ mem_off += N << msz;
} while (reg_off & 63);
} while (reg_off <= reg_last);
}
@@ -4577,7 +4595,7 @@ void sve_ld1_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
void HELPER(sve_##NAME##_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
- sve_ld1_r(env, vg, addr, desc, GETPC(), ESZ, 0, \
+ sve_ldN_r(env, vg, addr, desc, GETPC(), ESZ, MO_8, 1, \
sve_##NAME##_host, sve_##NAME##_tlb); \
}
@@ -4585,159 +4603,76 @@ void HELPER(sve_##NAME##_r)(CPUARMState *env, void *vg, \
void HELPER(sve_##NAME##_le_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
- sve_ld1_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, \
+ sve_ldN_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, 1, \
sve_##NAME##_le_host, sve_##NAME##_le_tlb); \
} \
void HELPER(sve_##NAME##_be_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
- sve_ld1_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, \
+ sve_ldN_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, 1, \
sve_##NAME##_be_host, sve_##NAME##_be_tlb); \
}
-DO_LD1_1(ld1bb, 0)
-DO_LD1_1(ld1bhu, 1)
-DO_LD1_1(ld1bhs, 1)
-DO_LD1_1(ld1bsu, 2)
-DO_LD1_1(ld1bss, 2)
-DO_LD1_1(ld1bdu, 3)
-DO_LD1_1(ld1bds, 3)
+DO_LD1_1(ld1bb, MO_8)
+DO_LD1_1(ld1bhu, MO_16)
+DO_LD1_1(ld1bhs, MO_16)
+DO_LD1_1(ld1bsu, MO_32)
+DO_LD1_1(ld1bss, MO_32)
+DO_LD1_1(ld1bdu, MO_64)
+DO_LD1_1(ld1bds, MO_64)
-DO_LD1_2(ld1hh, 1, 1)
-DO_LD1_2(ld1hsu, 2, 1)
-DO_LD1_2(ld1hss, 2, 1)
-DO_LD1_2(ld1hdu, 3, 1)
-DO_LD1_2(ld1hds, 3, 1)
+DO_LD1_2(ld1hh, MO_16, MO_16)
+DO_LD1_2(ld1hsu, MO_32, MO_16)
+DO_LD1_2(ld1hss, MO_32, MO_16)
+DO_LD1_2(ld1hdu, MO_64, MO_16)
+DO_LD1_2(ld1hds, MO_64, MO_16)
-DO_LD1_2(ld1ss, 2, 2)
-DO_LD1_2(ld1sdu, 3, 2)
-DO_LD1_2(ld1sds, 3, 2)
+DO_LD1_2(ld1ss, MO_32, MO_32)
+DO_LD1_2(ld1sdu, MO_64, MO_32)
+DO_LD1_2(ld1sds, MO_64, MO_32)
-DO_LD1_2(ld1dd, 3, 3)
+DO_LD1_2(ld1dd, MO_64, MO_64)
#undef DO_LD1_1
#undef DO_LD1_2
-/*
- * Common helpers for all contiguous 2,3,4-register predicated loads.
- */
-static void sve_ld2_r(CPUARMState *env, void *vg, target_ulong addr,
- uint32_t desc, int size, uintptr_t ra,
- sve_ldst1_tlb_fn *tlb_fn)
-{
- const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
- intptr_t i, oprsz = simd_oprsz(desc);
- ARMVectorReg scratch[2] = { };
-
- for (i = 0; i < oprsz; ) {
- uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
- do {
- if (pg & 1) {
- tlb_fn(env, &scratch[0], i, addr, ra);
- tlb_fn(env, &scratch[1], i, addr + size, ra);
- }
- i += size, pg >>= size;
- addr += 2 * size;
- } while (i & 15);
- }
-
- /* Wait until all exceptions have been raised to write back. */
- memcpy(&env->vfp.zregs[rd], &scratch[0], oprsz);
- memcpy(&env->vfp.zregs[(rd + 1) & 31], &scratch[1], oprsz);
-}
-
-static void sve_ld3_r(CPUARMState *env, void *vg, target_ulong addr,
- uint32_t desc, int size, uintptr_t ra,
- sve_ldst1_tlb_fn *tlb_fn)
-{
- const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
- intptr_t i, oprsz = simd_oprsz(desc);
- ARMVectorReg scratch[3] = { };
-
- for (i = 0; i < oprsz; ) {
- uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
- do {
- if (pg & 1) {
- tlb_fn(env, &scratch[0], i, addr, ra);
- tlb_fn(env, &scratch[1], i, addr + size, ra);
- tlb_fn(env, &scratch[2], i, addr + 2 * size, ra);
- }
- i += size, pg >>= size;
- addr += 3 * size;
- } while (i & 15);
- }
-
- /* Wait until all exceptions have been raised to write back. */
- memcpy(&env->vfp.zregs[rd], &scratch[0], oprsz);
- memcpy(&env->vfp.zregs[(rd + 1) & 31], &scratch[1], oprsz);
- memcpy(&env->vfp.zregs[(rd + 2) & 31], &scratch[2], oprsz);
-}
-
-static void sve_ld4_r(CPUARMState *env, void *vg, target_ulong addr,
- uint32_t desc, int size, uintptr_t ra,
- sve_ldst1_tlb_fn *tlb_fn)
-{
- const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
- intptr_t i, oprsz = simd_oprsz(desc);
- ARMVectorReg scratch[4] = { };
-
- for (i = 0; i < oprsz; ) {
- uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
- do {
- if (pg & 1) {
- tlb_fn(env, &scratch[0], i, addr, ra);
- tlb_fn(env, &scratch[1], i, addr + size, ra);
- tlb_fn(env, &scratch[2], i, addr + 2 * size, ra);
- tlb_fn(env, &scratch[3], i, addr + 3 * size, ra);
- }
- i += size, pg >>= size;
- addr += 4 * size;
- } while (i & 15);
- }
-
- /* Wait until all exceptions have been raised to write back. */
- memcpy(&env->vfp.zregs[rd], &scratch[0], oprsz);
- memcpy(&env->vfp.zregs[(rd + 1) & 31], &scratch[1], oprsz);
- memcpy(&env->vfp.zregs[(rd + 2) & 31], &scratch[2], oprsz);
- memcpy(&env->vfp.zregs[(rd + 3) & 31], &scratch[3], oprsz);
-}
-
#define DO_LDN_1(N) \
-void QEMU_FLATTEN HELPER(sve_ld##N##bb_r) \
- (CPUARMState *env, void *vg, target_ulong addr, uint32_t desc) \
-{ \
- sve_ld##N##_r(env, vg, addr, desc, 1, GETPC(), sve_ld1bb_tlb); \
+void HELPER(sve_ld##N##bb_r)(CPUARMState *env, void *vg, \
+ target_ulong addr, uint32_t desc) \
+{ \
+ sve_ldN_r(env, vg, addr, desc, GETPC(), MO_8, MO_8, N, \
+ sve_ld1bb_host, sve_ld1bb_tlb); \
}
-#define DO_LDN_2(N, SUFF, SIZE) \
-void QEMU_FLATTEN HELPER(sve_ld##N##SUFF##_le_r) \
- (CPUARMState *env, void *vg, target_ulong addr, uint32_t desc) \
+#define DO_LDN_2(N, SUFF, ESZ) \
+void HELPER(sve_ld##N##SUFF##_le_r)(CPUARMState *env, void *vg, \
+ target_ulong addr, uint32_t desc) \
{ \
- sve_ld##N##_r(env, vg, addr, desc, SIZE, GETPC(), \
- sve_ld1##SUFF##_le_tlb); \
+ sve_ldN_r(env, vg, addr, desc, GETPC(), ESZ, ESZ, N, \
+ sve_ld1##SUFF##_le_host, sve_ld1##SUFF##_le_tlb); \
} \
-void QEMU_FLATTEN HELPER(sve_ld##N##SUFF##_be_r) \
- (CPUARMState *env, void *vg, target_ulong addr, uint32_t desc) \
+void HELPER(sve_ld##N##SUFF##_be_r)(CPUARMState *env, void *vg, \
+ target_ulong addr, uint32_t desc) \
{ \
- sve_ld##N##_r(env, vg, addr, desc, SIZE, GETPC(), \
- sve_ld1##SUFF##_be_tlb); \
+ sve_ldN_r(env, vg, addr, desc, GETPC(), ESZ, ESZ, N, \
+ sve_ld1##SUFF##_be_host, sve_ld1##SUFF##_be_tlb); \
}
DO_LDN_1(2)
DO_LDN_1(3)
DO_LDN_1(4)
-DO_LDN_2(2, hh, 2)
-DO_LDN_2(3, hh, 2)
-DO_LDN_2(4, hh, 2)
+DO_LDN_2(2, hh, MO_16)
+DO_LDN_2(3, hh, MO_16)
+DO_LDN_2(4, hh, MO_16)
-DO_LDN_2(2, ss, 4)
-DO_LDN_2(3, ss, 4)
-DO_LDN_2(4, ss, 4)
+DO_LDN_2(2, ss, MO_32)
+DO_LDN_2(3, ss, MO_32)
+DO_LDN_2(4, ss, MO_32)
-DO_LDN_2(2, dd, 8)
-DO_LDN_2(3, dd, 8)
-DO_LDN_2(4, dd, 8)
+DO_LDN_2(2, dd, MO_64)
+DO_LDN_2(3, dd, MO_64)
+DO_LDN_2(4, dd, MO_64)
#undef DO_LDN_1
#undef DO_LDN_2
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH 10/16] target/arm: Use SVEContLdSt for multi-register contiguous loads
2020-03-11 6:44 ` [PATCH 10/16] target/arm: Use SVEContLdSt for multi-register contiguous loads Richard Henderson
@ 2020-04-16 13:35 ` Peter Maydell
2020-04-18 3:26 ` Richard Henderson
0 siblings, 1 reply; 44+ messages in thread
From: Peter Maydell @ 2020-04-16 13:35 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, QEMU Developers
On Wed, 11 Mar 2020 at 06:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/sve_helper.c | 223 ++++++++++++++--------------------------
> 1 file changed, 79 insertions(+), 144 deletions(-)
> #define DO_LDN_1(N) \
> -void QEMU_FLATTEN HELPER(sve_ld##N##bb_r) \
> - (CPUARMState *env, void *vg, target_ulong addr, uint32_t desc) \
> -{ \
> - sve_ld##N##_r(env, vg, addr, desc, 1, GETPC(), sve_ld1bb_tlb); \
> +void HELPER(sve_ld##N##bb_r)(CPUARMState *env, void *vg, \
> + target_ulong addr, uint32_t desc) \
> +{ \
> + sve_ldN_r(env, vg, addr, desc, GETPC(), MO_8, MO_8, N, \
> + sve_ld1bb_host, sve_ld1bb_tlb); \
> }
Deliberately losing the QEMU_FLATTEN here?
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 10/16] target/arm: Use SVEContLdSt for multi-register contiguous loads
2020-04-16 13:35 ` Peter Maydell
@ 2020-04-18 3:26 ` Richard Henderson
0 siblings, 0 replies; 44+ messages in thread
From: Richard Henderson @ 2020-04-18 3:26 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers
On 4/16/20 6:35 AM, Peter Maydell wrote:
> On Wed, 11 Mar 2020 at 06:44, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> target/arm/sve_helper.c | 223 ++++++++++++++--------------------------
>> 1 file changed, 79 insertions(+), 144 deletions(-)
>
>
>> #define DO_LDN_1(N) \
>> -void QEMU_FLATTEN HELPER(sve_ld##N##bb_r) \
>> - (CPUARMState *env, void *vg, target_ulong addr, uint32_t desc) \
>> -{ \
>> - sve_ld##N##_r(env, vg, addr, desc, 1, GETPC(), sve_ld1bb_tlb); \
>> +void HELPER(sve_ld##N##bb_r)(CPUARMState *env, void *vg, \
>> + target_ulong addr, uint32_t desc) \
>> +{ \
>> + sve_ldN_r(env, vg, addr, desc, GETPC(), MO_8, MO_8, N, \
>> + sve_ld1bb_host, sve_ld1bb_tlb); \
>> }
>
> Deliberately losing the QEMU_FLATTEN here?
Yes, the main helper is now marked QEMU_ALWAYS_INLINE, and there are further
subroutines -- like sve_check_watchpoints -- that we do not necessarily want to
inline.
r~
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 11/16] target/arm: Update contiguous first-fault and no-fault loads
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (9 preceding siblings ...)
2020-03-11 6:44 ` [PATCH 10/16] target/arm: Use SVEContLdSt for multi-register contiguous loads Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 14:15 ` Peter Maydell
2020-03-11 6:44 ` [PATCH 12/16] target/arm: Use SVEContLdSt for contiguous stores Richard Henderson
` (6 subsequent siblings)
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
With sve_cont_ldst_pages, the differences between first-fault and no-fault
are minimal, so unify the routines. With cpu_probe_watchpoint, we are able
to make progress through pages with TLB_WATCHPOINT set when the watchpoint
does not actually fire.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/sve_helper.c | 340 ++++++++++++++++++----------------------
1 file changed, 156 insertions(+), 184 deletions(-)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 56407446eb..9198e1612a 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4126,18 +4126,6 @@ static intptr_t find_next_active(uint64_t *vg, intptr_t reg_off,
return reg_off;
}
-/*
- * Return the maximum offset <= @mem_max which is still within the page
- * referenced by @base + @mem_off.
- */
-static intptr_t max_for_page(target_ulong base, intptr_t mem_off,
- intptr_t mem_max)
-{
- target_ulong addr = base + mem_off;
- intptr_t split = -(intptr_t)(addr | TARGET_PAGE_MASK);
- return MIN(split, mem_max - mem_off) + mem_off;
-}
-
/*
* Resolve the guest virtual address to info->host and info->flags.
* If @nofault, return false if the page is invalid, otherwise
@@ -4439,19 +4427,6 @@ static void sve_cont_ldst_watchpoints(SVEContLdSt *info, CPUARMState *env,
#endif
}
-/*
- * The result of tlb_vaddr_to_host for user-only is just g2h(x),
- * which is always non-null. Elide the useless test.
- */
-static inline bool test_host_page(void *host)
-{
-#ifdef CONFIG_USER_ONLY
- return true;
-#else
- return likely(host != NULL);
-#endif
-}
-
/*
* Common helper for all contiguous 1,2,3,4-register predicated stores.
*/
@@ -4709,167 +4684,161 @@ static void record_fault(CPUARMState *env, uintptr_t i, uintptr_t oprsz)
}
/*
- * Common helper for all contiguous first-fault loads.
+ * Common helper for all contiguous no-fault and first-fault loads.
*/
-static void sve_ldff1_r(CPUARMState *env, void *vg, const target_ulong addr,
- uint32_t desc, const uintptr_t retaddr,
- const int esz, const int msz,
- sve_ldst1_host_fn *host_fn,
- sve_ldst1_tlb_fn *tlb_fn)
+static inline QEMU_ALWAYS_INLINE
+void sve_ldnfff1_r(CPUARMState *env, void *vg, const target_ulong addr,
+ uint32_t desc, const uintptr_t retaddr,
+ const int esz, const int msz, const SVEContFault fault,
+ sve_ldst1_host_fn *host_fn,
+ sve_ldst1_tlb_fn *tlb_fn)
{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
- const int mmu_idx = get_mmuidx(oi);
const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
void *vd = &env->vfp.zregs[rd];
- const int diffsz = esz - msz;
const intptr_t reg_max = simd_oprsz(desc);
- const intptr_t mem_max = reg_max >> diffsz;
- intptr_t split, reg_off, mem_off, i;
+ intptr_t reg_off, mem_off, reg_last;
+ SVEContLdSt info;
+ int flags;
void *host;
- /* Skip to the first active element. */
- reg_off = find_next_active(vg, 0, reg_max, esz);
- if (unlikely(reg_off == reg_max)) {
+ /* Find the active elements. */
+ if (!sve_cont_ldst_elements(&info, addr, vg, reg_max, esz, 1 << msz)) {
/* The entire predicate was false; no load occurs. */
memset(vd, 0, reg_max);
return;
}
- mem_off = reg_off >> diffsz;
+ reg_off = info.reg_off_first[0];
- /*
- * If the (remaining) load is entirely within a single page, then:
- * For softmmu, and the tlb hits, then no faults will occur;
- * For user-only, either the first load will fault or none will.
- * We can thus perform the load directly to the destination and
- * Vd will be unmodified on any exception path.
- */
- split = max_for_page(addr, mem_off, mem_max);
- if (likely(split == mem_max)) {
- host = tlb_vaddr_to_host(env, addr + mem_off, MMU_DATA_LOAD, mmu_idx);
- if (test_host_page(host)) {
- i = reg_off;
- host -= mem_off;
- do {
- host_fn(vd, i, host + (i >> diffsz));
- i = find_next_active(vg, i + (1 << esz), reg_max, esz);
- } while (i < reg_max);
- /* After any fault, zero any leading inactive elements. */
+ /* Probe the page(s). */
+ if (!sve_cont_ldst_pages(&info, fault, env, addr, MMU_DATA_LOAD, retaddr)) {
+ /* Fault on first element. */
+ tcg_debug_assert(fault == FAULT_NO);
+ memset(vd, 0, reg_max);
+ goto do_fault;
+ }
+
+ mem_off = info.mem_off_first[0];
+ flags = info.page[0].flags;
+
+ if (fault == FAULT_FIRST) {
+ /*
+ * Special handling of the first active element,
+ * if it crosses a page boundary or is MMIO.
+ */
+ bool is_split = mem_off == info.mem_off_split;
+ /* TODO: MTE check. */
+ if (unlikely(flags != 0) || unlikely(is_split)) {
+ /*
+ * Use the slow path for cross-page handling.
+ * Might trap for MMIO or watchpoints.
+ */
+ tlb_fn(env, vd, reg_off, addr + mem_off, retaddr);
+
+ /* After any fault, zero the other elements. */
swap_memzero(vd, reg_off);
- return;
+ reg_off += 1 << esz;
+ mem_off += 1 << msz;
+ swap_memzero(vd + reg_off, reg_max - reg_off);
+
+ if (is_split) {
+ goto second_page;
+ }
+ } else {
+ memset(vd, 0, reg_max);
+ }
+ } else {
+ memset(vd, 0, reg_max);
+ if (unlikely(mem_off == info.mem_off_split)) {
+ /* The first active element crosses a page boundary. */
+ flags |= info.page[1].flags;
+ if (unlikely(flags & TLB_MMIO)) {
+ /* Some page is MMIO, see below. */
+ goto do_fault;
+ }
+ if (unlikely(flags & TLB_WATCHPOINT) &&
+ cpu_probe_watchpoint(env_cpu(env), addr + mem_off,
+ 1 << msz, BP_MEM_READ)) {
+ /* Watchpoint hit, see below. */
+ goto do_fault;
+ }
+ /* TODO: MTE check. */
+ /*
+ * Use the slow path for cross-page handling.
+ * This is RAM, without a watchpoint, and will not trap.
+ */
+ tlb_fn(env, vd, reg_off, addr + mem_off, retaddr);
+ goto second_page;
}
}
/*
- * Perform one normal read, which will fault or not.
- * But it is likely to bring the page into the tlb.
+ * From this point on, all memory operations are MemSingleNF.
+ *
+ * Per the MemSingleNF pseudocode, a no-fault load from Device memory
+ * must not actually hit the bus -- it returns (UNKNOWN, FAULT) instead.
+ * If you map non-RAM with Normal memory attributes and do a NF
+ * load then it should access the bus -- but doing so is illegal.
+ *
+ * While we do not have access to the memory attributes from the PTE
+ * to tell Device memory from Normal memory, we can validly assume that
+ * non-RAM has been mapped as Device memory. Thus we indicate fault
+ * on all MMIO.
+ *
+ * Similarly, CPU_BP breakpoints would raise exceptions, and so
+ * return (UNKNOWN, FAULT). For simplicity, we consider gdb and
+ * architectural breakpoints the same.
*/
- tlb_fn(env, vd, reg_off, addr + mem_off, retaddr);
+ if (unlikely(flags & TLB_MMIO)) {
+ goto do_fault;
+ }
- /* After any fault, zero any leading predicated false elts. */
- swap_memzero(vd, reg_off);
- mem_off += 1 << msz;
- reg_off += 1 << esz;
+ reg_last = info.reg_off_last[0];
+ host = info.page[0].host;
- /* Try again to read the balance of the page. */
- split = max_for_page(addr, mem_off - 1, mem_max);
- if (split >= (1 << msz)) {
- host = tlb_vaddr_to_host(env, addr + mem_off, MMU_DATA_LOAD, mmu_idx);
- if (host) {
- host -= mem_off;
- do {
+ do {
+ uint64_t pg = *(uint64_t *)(vg + (reg_off >> 3));
+ do {
+ if ((pg >> (reg_off & 63)) & 1) {
+ if (unlikely(flags & TLB_WATCHPOINT) &&
+ cpu_probe_watchpoint(env_cpu(env), addr + mem_off,
+ 1 << msz, BP_MEM_READ)) {
+ goto do_fault;
+ }
+ /* TODO: MTE check. */
host_fn(vd, reg_off, host + mem_off);
- reg_off += 1 << esz;
- reg_off = find_next_active(vg, reg_off, reg_max, esz);
- mem_off = reg_off >> diffsz;
- } while (split - mem_off >= (1 << msz));
- }
- }
-
- record_fault(env, reg_off, reg_max);
-}
-
-/*
- * Common helper for all contiguous no-fault loads.
- */
-static void sve_ldnf1_r(CPUARMState *env, void *vg, const target_ulong addr,
- uint32_t desc, const int esz, const int msz,
- sve_ldst1_host_fn *host_fn)
-{
- const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
- void *vd = &env->vfp.zregs[rd];
- const int diffsz = esz - msz;
- const intptr_t reg_max = simd_oprsz(desc);
- const intptr_t mem_max = reg_max >> diffsz;
- const int mmu_idx = cpu_mmu_index(env, false);
- intptr_t split, reg_off, mem_off;
- void *host;
-
-#ifdef CONFIG_USER_ONLY
- host = tlb_vaddr_to_host(env, addr, MMU_DATA_LOAD, mmu_idx);
- if (likely(page_check_range(addr, mem_max, PAGE_READ) == 0)) {
- /* The entire operation is valid and will not fault. */
- reg_off = 0;
- do {
- mem_off = reg_off >> diffsz;
- host_fn(vd, reg_off, host + mem_off);
+ }
reg_off += 1 << esz;
- reg_off = find_next_active(vg, reg_off, reg_max, esz);
- } while (reg_off < reg_max);
- return;
- }
-#endif
+ mem_off += 1 << msz;
+ } while (reg_off <= reg_last && (reg_off & 63));
+ } while (reg_off <= reg_last);
- /* There will be no fault, so we may modify in advance. */
- memset(vd, 0, reg_max);
-
- /* Skip to the first active element. */
- reg_off = find_next_active(vg, 0, reg_max, esz);
- if (unlikely(reg_off == reg_max)) {
- /* The entire predicate was false; no load occurs. */
- return;
- }
- mem_off = reg_off >> diffsz;
-
-#ifdef CONFIG_USER_ONLY
- if (page_check_range(addr + mem_off, 1 << msz, PAGE_READ) == 0) {
- /* At least one load is valid; take the rest of the page. */
- split = max_for_page(addr, mem_off + (1 << msz) - 1, mem_max);
- do {
- host_fn(vd, reg_off, host + mem_off);
- reg_off += 1 << esz;
- reg_off = find_next_active(vg, reg_off, reg_max, esz);
- mem_off = reg_off >> diffsz;
- } while (split - mem_off >= (1 << msz));
- }
-#else
/*
- * If the address is not in the TLB, we have no way to bring the
- * entry into the TLB without also risking a fault. Note that
- * the corollary is that we never load from an address not in RAM.
- *
- * This last is out of spec, in a weird corner case.
- * Per the MemNF/MemSingleNF pseudocode, a NF load from Device memory
- * must not actually hit the bus -- it returns UNKNOWN data instead.
- * But if you map non-RAM with Normal memory attributes and do a NF
- * load then it should access the bus. (Nobody ought actually do this
- * in the real world, obviously.)
- *
- * Then there are the annoying special cases with watchpoints...
- * TODO: Add a form of non-faulting loads using cc->tlb_fill(probe=true).
+ * MemSingleNF is allowed to fail for any reason. We have special
+ * code above to handle the first element crossing a page boundary.
+ * As an implementation choice, decline to handle a cross-page element
+ * in any other position.
*/
- host = tlb_vaddr_to_host(env, addr + mem_off, MMU_DATA_LOAD, mmu_idx);
- split = max_for_page(addr, mem_off, mem_max);
- if (host && split >= (1 << msz)) {
- host -= mem_off;
- do {
- host_fn(vd, reg_off, host + mem_off);
- reg_off += 1 << esz;
- reg_off = find_next_active(vg, reg_off, reg_max, esz);
- mem_off = reg_off >> diffsz;
- } while (split - mem_off >= (1 << msz));
+ reg_off = info.reg_off_split;
+ if (reg_off >= 0) {
+ goto do_fault;
}
-#endif
+ second_page:
+ reg_off = info.reg_off_first[1];
+ if (likely(reg_off < 0)) {
+ /* No active elements on the second page. All done. */
+ return;
+ }
+
+ /*
+ * MemSingleNF is allowed to fail for any reason. As an implementation
+ * choice, decline to handle elements on the second page. This should
+ * be low frequency as the guest walks through memory -- the next
+ * iteration of the guest's loop should be aligned on the page boundary,
+ * and then all following iterations will stay aligned.
+ */
+
+ do_fault:
record_fault(env, reg_off, reg_max);
}
@@ -4877,58 +4846,61 @@ static void sve_ldnf1_r(CPUARMState *env, void *vg, const target_ulong addr,
void HELPER(sve_ldff1##PART##_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
- sve_ldff1_r(env, vg, addr, desc, GETPC(), ESZ, 0, \
- sve_ld1##PART##_host, sve_ld1##PART##_tlb); \
+ sve_ldnfff1_r(env, vg, addr, desc, GETPC(), ESZ, MO_8, FAULT_FIRST, \
+ sve_ld1##PART##_host, sve_ld1##PART##_tlb); \
} \
void HELPER(sve_ldnf1##PART##_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
- sve_ldnf1_r(env, vg, addr, desc, ESZ, 0, sve_ld1##PART##_host); \
+ sve_ldnfff1_r(env, vg, addr, desc, GETPC(), ESZ, MO_8, FAULT_NO, \
+ sve_ld1##PART##_host, sve_ld1##PART##_tlb); \
}
#define DO_LDFF1_LDNF1_2(PART, ESZ, MSZ) \
void HELPER(sve_ldff1##PART##_le_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
- sve_ldff1_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, \
- sve_ld1##PART##_le_host, sve_ld1##PART##_le_tlb); \
+ sve_ldnfff1_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, FAULT_FIRST, \
+ sve_ld1##PART##_le_host, sve_ld1##PART##_le_tlb); \
} \
void HELPER(sve_ldnf1##PART##_le_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
- sve_ldnf1_r(env, vg, addr, desc, ESZ, MSZ, sve_ld1##PART##_le_host); \
+ sve_ldnfff1_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, FAULT_NO, \
+ sve_ld1##PART##_le_host, sve_ld1##PART##_le_tlb); \
} \
void HELPER(sve_ldff1##PART##_be_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
- sve_ldff1_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, \
- sve_ld1##PART##_be_host, sve_ld1##PART##_be_tlb); \
+ sve_ldnfff1_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, FAULT_FIRST, \
+ sve_ld1##PART##_be_host, sve_ld1##PART##_be_tlb); \
} \
void HELPER(sve_ldnf1##PART##_be_r)(CPUARMState *env, void *vg, \
target_ulong addr, uint32_t desc) \
{ \
- sve_ldnf1_r(env, vg, addr, desc, ESZ, MSZ, sve_ld1##PART##_be_host); \
+ sve_ldnfff1_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, FAULT_NO, \
+ sve_ld1##PART##_be_host, sve_ld1##PART##_be_tlb); \
}
-DO_LDFF1_LDNF1_1(bb, 0)
-DO_LDFF1_LDNF1_1(bhu, 1)
-DO_LDFF1_LDNF1_1(bhs, 1)
-DO_LDFF1_LDNF1_1(bsu, 2)
-DO_LDFF1_LDNF1_1(bss, 2)
-DO_LDFF1_LDNF1_1(bdu, 3)
-DO_LDFF1_LDNF1_1(bds, 3)
+DO_LDFF1_LDNF1_1(bb, MO_8)
+DO_LDFF1_LDNF1_1(bhu, MO_16)
+DO_LDFF1_LDNF1_1(bhs, MO_16)
+DO_LDFF1_LDNF1_1(bsu, MO_32)
+DO_LDFF1_LDNF1_1(bss, MO_32)
+DO_LDFF1_LDNF1_1(bdu, MO_64)
+DO_LDFF1_LDNF1_1(bds, MO_64)
-DO_LDFF1_LDNF1_2(hh, 1, 1)
-DO_LDFF1_LDNF1_2(hsu, 2, 1)
-DO_LDFF1_LDNF1_2(hss, 2, 1)
-DO_LDFF1_LDNF1_2(hdu, 3, 1)
-DO_LDFF1_LDNF1_2(hds, 3, 1)
+DO_LDFF1_LDNF1_2(hh, MO_16, MO_16)
+DO_LDFF1_LDNF1_2(hsu, MO_32, MO_16)
+DO_LDFF1_LDNF1_2(hss, MO_32, MO_16)
+DO_LDFF1_LDNF1_2(hdu, MO_64, MO_16)
+DO_LDFF1_LDNF1_2(hds, MO_64, MO_16)
-DO_LDFF1_LDNF1_2(ss, 2, 2)
-DO_LDFF1_LDNF1_2(sdu, 3, 2)
-DO_LDFF1_LDNF1_2(sds, 3, 2)
+DO_LDFF1_LDNF1_2(ss, MO_32, MO_32)
+DO_LDFF1_LDNF1_2(sdu, MO_64, MO_32)
+DO_LDFF1_LDNF1_2(sds, MO_64, MO_32)
-DO_LDFF1_LDNF1_2(dd, 3, 3)
+DO_LDFF1_LDNF1_2(dd, MO_64, MO_64)
#undef DO_LDFF1_LDNF1_1
#undef DO_LDFF1_LDNF1_2
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH 11/16] target/arm: Update contiguous first-fault and no-fault loads
2020-03-11 6:44 ` [PATCH 11/16] target/arm: Update contiguous first-fault and no-fault loads Richard Henderson
@ 2020-04-16 14:15 ` Peter Maydell
2020-04-18 3:36 ` Richard Henderson
0 siblings, 1 reply; 44+ messages in thread
From: Peter Maydell @ 2020-04-16 14:15 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, QEMU Developers
On Wed, 11 Mar 2020 at 06:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> With sve_cont_ldst_pages, the differences between first-fault and no-fault
> are minimal, so unify the routines. With cpu_probe_watchpoint, we are able
> to make progress through pages with TLB_WATCHPOINT set when the watchpoint
> does not actually fire.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> /*
> - * Perform one normal read, which will fault or not.
> - * But it is likely to bring the page into the tlb.
> + * From this point on, all memory operations are MemSingleNF.
> + *
> + * Per the MemSingleNF pseudocode, a no-fault load from Device memory
> + * must not actually hit the bus -- it returns (UNKNOWN, FAULT) instead.
> + * If you map non-RAM with Normal memory attributes and do a NF
> + * load then it should access the bus -- but doing so is illegal.
> + *
> + * While we do not have access to the memory attributes from the PTE
> + * to tell Device memory from Normal memory, we can validly assume that
> + * non-RAM has been mapped as Device memory. Thus we indicate fault
> + * on all MMIO.
I don't think you can assume this; for instance a QEMU 'romd'
device might reasonably be mapped as Normal memory but currently
be in "send all accesses to my read/write functions" mode.
> + *
> + * Similarly, CPU_BP breakpoints would raise exceptions, and so
> + * return (UNKNOWN, FAULT). For simplicity, we consider gdb and
> + * architectural breakpoints the same.
> */
thanks
-- PMM
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 11/16] target/arm: Update contiguous first-fault and no-fault loads
2020-04-16 14:15 ` Peter Maydell
@ 2020-04-18 3:36 ` Richard Henderson
0 siblings, 0 replies; 44+ messages in thread
From: Richard Henderson @ 2020-04-18 3:36 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers
On 4/16/20 7:15 AM, Peter Maydell wrote:
>> + * While we do not have access to the memory attributes from the PTE
>> + * to tell Device memory from Normal memory, we can validly assume that
>> + * non-RAM has been mapped as Device memory. Thus we indicate fault
>> + * on all MMIO.
>
> I don't think you can assume this; for instance a QEMU 'romd'
> device might reasonably be mapped as Normal memory but currently
> be in "send all accesses to my read/write functions" mode.
If that is so, then TLB_MMIO will be set in the tlb_entry comparator. See
accel/tcg/cputlb.c:882.
r~
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 12/16] target/arm: Use SVEContLdSt for contiguous stores
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (10 preceding siblings ...)
2020-03-11 6:44 ` [PATCH 11/16] target/arm: Update contiguous first-fault and no-fault loads Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 14:18 ` Peter Maydell
2020-03-11 6:44 ` [PATCH 13/16] target/arm: Reuse sve_probe_page for gather first-fault loads Richard Henderson
` (5 subsequent siblings)
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
Follow the model set up for contiguous loads. This handles
watchpoints correctly for contiguous stores, recognizing the
exception before any changes to memory.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/sve_helper.c | 288 ++++++++++++++++++++++------------------
1 file changed, 162 insertions(+), 126 deletions(-)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 9198e1612a..c6f22928d5 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4002,6 +4002,13 @@ static void sve_##NAME##_host(void *vd, intptr_t reg_off, void *host) \
*(TYPEE *)(vd + H(reg_off)) = val; \
}
+#define DO_ST_HOST(NAME, H, TYPEE, TYPEM, HOST) \
+static void sve_##NAME##_host(void *vd, intptr_t reg_off, void *host) \
+{ \
+ TYPEM val = *(TYPEE *)(vd + H(reg_off)); \
+ HOST(host, val); \
+}
+
#define DO_LD_TLB(NAME, H, TYPEE, TYPEM, BSWAP, TLB) \
static void sve_##NAME##_tlb(CPUARMState *env, void *vd, intptr_t reg_off, \
target_ulong addr, uintptr_t ra) \
@@ -4031,6 +4038,7 @@ DO_LD_PRIM_1(ld1bdu, , uint64_t, uint8_t)
DO_LD_PRIM_1(ld1bds, , uint64_t, int8_t)
#define DO_ST_PRIM_1(NAME, H, TE, TM) \
+ DO_ST_HOST(st1##NAME, H, TE, TM, stb_p) \
DO_ST_TLB(st1##NAME, H, TE, TM, , cpu_stb_data_ra)
DO_ST_PRIM_1(bb, H1, uint8_t, uint8_t)
@@ -4045,6 +4053,8 @@ DO_ST_PRIM_1(bd, , uint64_t, uint8_t)
DO_LD_TLB(ld1##NAME##_le, H, TE, TM, le_##BSWAP, cpu_##LD##_data_ra)
#define DO_ST_PRIM_2(NAME, H, TE, TM, BSWAP, ST) \
+ DO_ST_HOST(st1##NAME##_be, H, TE, TM, ST##_be_p) \
+ DO_ST_HOST(st1##NAME##_le, H, TE, TM, ST##_le_p) \
DO_ST_TLB(st1##NAME##_be, H, TE, TM, be_##BSWAP, cpu_##ST##_data_ra) \
DO_ST_TLB(st1##NAME##_le, H, TE, TM, le_##BSWAP, cpu_##ST##_data_ra)
@@ -4906,151 +4916,177 @@ DO_LDFF1_LDNF1_2(dd, MO_64, MO_64)
#undef DO_LDFF1_LDNF1_2
/*
- * Common helpers for all contiguous 1,2,3,4-register predicated stores.
+ * Common helper for all contiguous 1,2,3,4-register predicated stores.
*/
-static void sve_st1_r(CPUARMState *env, void *vg, target_ulong addr,
- uint32_t desc, const uintptr_t ra,
- const int esize, const int msize,
- sve_ldst1_tlb_fn *tlb_fn)
+
+static inline QEMU_ALWAYS_INLINE
+void sve_stN_r(CPUARMState *env, uint64_t *vg, target_ulong addr, uint32_t desc,
+ const uintptr_t retaddr, const int esz,
+ const int msz, const int N,
+ sve_ldst1_host_fn *host_fn,
+ sve_ldst1_tlb_fn *tlb_fn)
{
const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
- intptr_t i, oprsz = simd_oprsz(desc);
- void *vd = &env->vfp.zregs[rd];
+ const intptr_t reg_max = simd_oprsz(desc);
+ intptr_t reg_off, reg_last, mem_off;
+ SVEContLdSt info;
+ void *host;
+ int i, flags;
- for (i = 0; i < oprsz; ) {
- uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
- do {
- if (pg & 1) {
- tlb_fn(env, vd, i, addr, ra);
+ /* Find the active elements. */
+ if (!sve_cont_ldst_elements(&info, addr, vg, reg_max, esz, N << msz)) {
+ /* The entire predicate was false; no store occurs. */
+ return;
+ }
+
+ /* Probe the page(s). Exit with exception for any invalid page. */
+ sve_cont_ldst_pages(&info, FAULT_ALL, env, addr, MMU_DATA_STORE, retaddr);
+
+ /* Handle watchpoints for all active elements. */
+ sve_cont_ldst_watchpoints(&info, env, vg, addr, 1 << esz, N << msz,
+ BP_MEM_WRITE, retaddr);
+
+ /* TODO: MTE check. */
+
+ flags = info.page[0].flags | info.page[1].flags;
+ if (unlikely(flags != 0)) {
+#ifdef CONFIG_USER_ONLY
+ g_assert_not_reached();
+#else
+ /*
+ * At least one page includes MMIO.
+ * Any bus operation can fail with cpu_transaction_failed,
+ * which for ARM will raise SyncExternal. We cannot avoid
+ * this fault and will leave with the store incomplete.
+ */
+ mem_off = info.mem_off_first[0];
+ reg_off = info.reg_off_first[0];
+ reg_last = info.reg_off_last[1];
+ if (reg_last < 0) {
+ reg_last = info.reg_off_split;
+ if (reg_last < 0) {
+ reg_last = info.reg_off_last[0];
}
- i += esize, pg >>= esize;
- addr += msize;
- } while (i & 15);
+ }
+
+ do {
+ uint64_t pg = vg[reg_off >> 6];
+ do {
+ if ((pg >> (reg_off & 63)) & 1) {
+ for (i = 0; i < N; ++i) {
+ tlb_fn(env, &env->vfp.zregs[(rd + i) & 31], reg_off,
+ addr + mem_off + (i << msz), retaddr);
+ }
+ }
+ reg_off += 1 << esz;
+ mem_off += N << msz;
+ } while (reg_off & 63);
+ } while (reg_off <= reg_last);
+ return;
+#endif
+ }
+
+ mem_off = info.mem_off_first[0];
+ reg_off = info.reg_off_first[0];
+ reg_last = info.reg_off_last[0];
+ host = info.page[0].host;
+
+ while (reg_off <= reg_last) {
+ uint64_t pg = vg[reg_off >> 6];
+ do {
+ if ((pg >> (reg_off & 63)) & 1) {
+ for (i = 0; i < N; ++i) {
+ host_fn(&env->vfp.zregs[(rd + i) & 31], reg_off,
+ host + mem_off + (i << msz));
+ }
+ }
+ reg_off += 1 << esz;
+ mem_off += N << msz;
+ } while (reg_off <= reg_last && (reg_off & 63));
+ }
+
+ /*
+ * Use the slow path to manage the cross-page misalignment.
+ * But we know this is RAM and cannot trap.
+ */
+ mem_off = info.mem_off_split;
+ if (unlikely(mem_off >= 0)) {
+ reg_off = info.reg_off_split;
+ for (i = 0; i < N; ++i) {
+ tlb_fn(env, &env->vfp.zregs[(rd + i) & 31], reg_off,
+ addr + mem_off + (i << msz), retaddr);
+ }
+ }
+
+ mem_off = info.mem_off_first[1];
+ if (unlikely(mem_off >= 0)) {
+ reg_off = info.reg_off_first[1];
+ reg_last = info.reg_off_last[1];
+ host = info.page[1].host;
+
+ do {
+ uint64_t pg = vg[reg_off >> 6];
+ do {
+ if ((pg >> (reg_off & 63)) & 1) {
+ for (i = 0; i < N; ++i) {
+ host_fn(&env->vfp.zregs[(rd + i) & 31], reg_off,
+ host + mem_off + (i << msz));
+ }
+ }
+ reg_off += 1 << esz;
+ mem_off += N << msz;
+ } while (reg_off & 63);
+ } while (reg_off <= reg_last);
}
}
-static void sve_st2_r(CPUARMState *env, void *vg, target_ulong addr,
- uint32_t desc, const uintptr_t ra,
- const int esize, const int msize,
- sve_ldst1_tlb_fn *tlb_fn)
-{
- const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
- intptr_t i, oprsz = simd_oprsz(desc);
- void *d1 = &env->vfp.zregs[rd];
- void *d2 = &env->vfp.zregs[(rd + 1) & 31];
-
- for (i = 0; i < oprsz; ) {
- uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
- do {
- if (pg & 1) {
- tlb_fn(env, d1, i, addr, ra);
- tlb_fn(env, d2, i, addr + msize, ra);
- }
- i += esize, pg >>= esize;
- addr += 2 * msize;
- } while (i & 15);
- }
-}
-
-static void sve_st3_r(CPUARMState *env, void *vg, target_ulong addr,
- uint32_t desc, const uintptr_t ra,
- const int esize, const int msize,
- sve_ldst1_tlb_fn *tlb_fn)
-{
- const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
- intptr_t i, oprsz = simd_oprsz(desc);
- void *d1 = &env->vfp.zregs[rd];
- void *d2 = &env->vfp.zregs[(rd + 1) & 31];
- void *d3 = &env->vfp.zregs[(rd + 2) & 31];
-
- for (i = 0; i < oprsz; ) {
- uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
- do {
- if (pg & 1) {
- tlb_fn(env, d1, i, addr, ra);
- tlb_fn(env, d2, i, addr + msize, ra);
- tlb_fn(env, d3, i, addr + 2 * msize, ra);
- }
- i += esize, pg >>= esize;
- addr += 3 * msize;
- } while (i & 15);
- }
-}
-
-static void sve_st4_r(CPUARMState *env, void *vg, target_ulong addr,
- uint32_t desc, const uintptr_t ra,
- const int esize, const int msize,
- sve_ldst1_tlb_fn *tlb_fn)
-{
- const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
- intptr_t i, oprsz = simd_oprsz(desc);
- void *d1 = &env->vfp.zregs[rd];
- void *d2 = &env->vfp.zregs[(rd + 1) & 31];
- void *d3 = &env->vfp.zregs[(rd + 2) & 31];
- void *d4 = &env->vfp.zregs[(rd + 3) & 31];
-
- for (i = 0; i < oprsz; ) {
- uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
- do {
- if (pg & 1) {
- tlb_fn(env, d1, i, addr, ra);
- tlb_fn(env, d2, i, addr + msize, ra);
- tlb_fn(env, d3, i, addr + 2 * msize, ra);
- tlb_fn(env, d4, i, addr + 3 * msize, ra);
- }
- i += esize, pg >>= esize;
- addr += 4 * msize;
- } while (i & 15);
- }
-}
-
-#define DO_STN_1(N, NAME, ESIZE) \
-void QEMU_FLATTEN HELPER(sve_st##N##NAME##_r) \
- (CPUARMState *env, void *vg, target_ulong addr, uint32_t desc) \
+#define DO_STN_1(N, NAME, ESZ) \
+void HELPER(sve_st##N##NAME##_r)(CPUARMState *env, void *vg, \
+ target_ulong addr, uint32_t desc) \
{ \
- sve_st##N##_r(env, vg, addr, desc, GETPC(), ESIZE, 1, \
- sve_st1##NAME##_tlb); \
+ sve_stN_r(env, vg, addr, desc, GETPC(), ESZ, MO_8, N, \
+ sve_st1##NAME##_host, sve_st1##NAME##_tlb); \
}
-#define DO_STN_2(N, NAME, ESIZE, MSIZE) \
-void QEMU_FLATTEN HELPER(sve_st##N##NAME##_le_r) \
- (CPUARMState *env, void *vg, target_ulong addr, uint32_t desc) \
+#define DO_STN_2(N, NAME, ESZ, MSZ) \
+void HELPER(sve_st##N##NAME##_le_r)(CPUARMState *env, void *vg, \
+ target_ulong addr, uint32_t desc) \
{ \
- sve_st##N##_r(env, vg, addr, desc, GETPC(), ESIZE, MSIZE, \
- sve_st1##NAME##_le_tlb); \
+ sve_stN_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, N, \
+ sve_st1##NAME##_le_host, sve_st1##NAME##_le_tlb); \
} \
-void QEMU_FLATTEN HELPER(sve_st##N##NAME##_be_r) \
- (CPUARMState *env, void *vg, target_ulong addr, uint32_t desc) \
+void HELPER(sve_st##N##NAME##_be_r)(CPUARMState *env, void *vg, \
+ target_ulong addr, uint32_t desc) \
{ \
- sve_st##N##_r(env, vg, addr, desc, GETPC(), ESIZE, MSIZE, \
- sve_st1##NAME##_be_tlb); \
+ sve_stN_r(env, vg, addr, desc, GETPC(), ESZ, MSZ, N, \
+ sve_st1##NAME##_be_host, sve_st1##NAME##_be_tlb); \
}
-DO_STN_1(1, bb, 1)
-DO_STN_1(1, bh, 2)
-DO_STN_1(1, bs, 4)
-DO_STN_1(1, bd, 8)
-DO_STN_1(2, bb, 1)
-DO_STN_1(3, bb, 1)
-DO_STN_1(4, bb, 1)
+DO_STN_1(1, bb, MO_8)
+DO_STN_1(1, bh, MO_16)
+DO_STN_1(1, bs, MO_32)
+DO_STN_1(1, bd, MO_64)
+DO_STN_1(2, bb, MO_8)
+DO_STN_1(3, bb, MO_8)
+DO_STN_1(4, bb, MO_8)
-DO_STN_2(1, hh, 2, 2)
-DO_STN_2(1, hs, 4, 2)
-DO_STN_2(1, hd, 8, 2)
-DO_STN_2(2, hh, 2, 2)
-DO_STN_2(3, hh, 2, 2)
-DO_STN_2(4, hh, 2, 2)
+DO_STN_2(1, hh, MO_16, MO_16)
+DO_STN_2(1, hs, MO_32, MO_16)
+DO_STN_2(1, hd, MO_64, MO_16)
+DO_STN_2(2, hh, MO_16, MO_16)
+DO_STN_2(3, hh, MO_16, MO_16)
+DO_STN_2(4, hh, MO_16, MO_16)
-DO_STN_2(1, ss, 4, 4)
-DO_STN_2(1, sd, 8, 4)
-DO_STN_2(2, ss, 4, 4)
-DO_STN_2(3, ss, 4, 4)
-DO_STN_2(4, ss, 4, 4)
+DO_STN_2(1, ss, MO_32, MO_32)
+DO_STN_2(1, sd, MO_64, MO_32)
+DO_STN_2(2, ss, MO_32, MO_32)
+DO_STN_2(3, ss, MO_32, MO_32)
+DO_STN_2(4, ss, MO_32, MO_32)
-DO_STN_2(1, dd, 8, 8)
-DO_STN_2(2, dd, 8, 8)
-DO_STN_2(3, dd, 8, 8)
-DO_STN_2(4, dd, 8, 8)
+DO_STN_2(1, dd, MO_64, MO_64)
+DO_STN_2(2, dd, MO_64, MO_64)
+DO_STN_2(3, dd, MO_64, MO_64)
+DO_STN_2(4, dd, MO_64, MO_64)
#undef DO_STN_1
#undef DO_STN_2
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH 13/16] target/arm: Reuse sve_probe_page for gather first-fault loads
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (11 preceding siblings ...)
2020-03-11 6:44 ` [PATCH 12/16] target/arm: Use SVEContLdSt for contiguous stores Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 14:23 ` Peter Maydell
2020-03-11 6:44 ` [PATCH 14/16] target/arm: Reuse sve_probe_page for scatter stores Richard Henderson
` (4 subsequent siblings)
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
This avoids the need for a separate set of helpers to implement
no-fault semantics, and will enable MTE in the future.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/sve_helper.c | 323 ++++++++++++++++------------------------
1 file changed, 127 insertions(+), 196 deletions(-)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index c6f22928d5..a33699adf8 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -5255,231 +5255,162 @@ DO_LD1_ZPZ_D(dd_be, zd)
/* First fault loads with a vector index. */
-/* Load one element into VD+REG_OFF from (ENV,VADDR) without faulting.
- * The controlling predicate is known to be true. Return true if the
- * load was successful.
- */
-typedef bool sve_ld1_nf_fn(CPUARMState *env, void *vd, intptr_t reg_off,
- target_ulong vaddr, int mmu_idx);
-
-#ifdef CONFIG_SOFTMMU
-#define DO_LD_NF(NAME, H, TYPEE, TYPEM, HOST) \
-static bool sve_ld##NAME##_nf(CPUARMState *env, void *vd, intptr_t reg_off, \
- target_ulong addr, int mmu_idx) \
-{ \
- target_ulong next_page = -(addr | TARGET_PAGE_MASK); \
- if (likely(next_page - addr >= sizeof(TYPEM))) { \
- void *host = tlb_vaddr_to_host(env, addr, MMU_DATA_LOAD, mmu_idx); \
- if (likely(host)) { \
- TYPEM val = HOST(host); \
- *(TYPEE *)(vd + H(reg_off)) = val; \
- return true; \
- } \
- } \
- return false; \
-}
-#else
-#define DO_LD_NF(NAME, H, TYPEE, TYPEM, HOST) \
-static bool sve_ld##NAME##_nf(CPUARMState *env, void *vd, intptr_t reg_off, \
- target_ulong addr, int mmu_idx) \
-{ \
- if (likely(page_check_range(addr, sizeof(TYPEM), PAGE_READ))) { \
- TYPEM val = HOST(g2h(addr)); \
- *(TYPEE *)(vd + H(reg_off)) = val; \
- return true; \
- } \
- return false; \
-}
-#endif
-
-DO_LD_NF(bsu, H1_4, uint32_t, uint8_t, ldub_p)
-DO_LD_NF(bss, H1_4, uint32_t, int8_t, ldsb_p)
-DO_LD_NF(bdu, , uint64_t, uint8_t, ldub_p)
-DO_LD_NF(bds, , uint64_t, int8_t, ldsb_p)
-
-DO_LD_NF(hsu_le, H1_4, uint32_t, uint16_t, lduw_le_p)
-DO_LD_NF(hss_le, H1_4, uint32_t, int16_t, ldsw_le_p)
-DO_LD_NF(hsu_be, H1_4, uint32_t, uint16_t, lduw_be_p)
-DO_LD_NF(hss_be, H1_4, uint32_t, int16_t, ldsw_be_p)
-DO_LD_NF(hdu_le, , uint64_t, uint16_t, lduw_le_p)
-DO_LD_NF(hds_le, , uint64_t, int16_t, ldsw_le_p)
-DO_LD_NF(hdu_be, , uint64_t, uint16_t, lduw_be_p)
-DO_LD_NF(hds_be, , uint64_t, int16_t, ldsw_be_p)
-
-DO_LD_NF(ss_le, H1_4, uint32_t, uint32_t, ldl_le_p)
-DO_LD_NF(ss_be, H1_4, uint32_t, uint32_t, ldl_be_p)
-DO_LD_NF(sdu_le, , uint64_t, uint32_t, ldl_le_p)
-DO_LD_NF(sds_le, , uint64_t, int32_t, ldl_le_p)
-DO_LD_NF(sdu_be, , uint64_t, uint32_t, ldl_be_p)
-DO_LD_NF(sds_be, , uint64_t, int32_t, ldl_be_p)
-
-DO_LD_NF(dd_le, , uint64_t, uint64_t, ldq_le_p)
-DO_LD_NF(dd_be, , uint64_t, uint64_t, ldq_be_p)
-
/*
- * Common helper for all gather first-faulting loads.
+ * Common helpers for all gather first-faulting loads.
*/
-static inline void sve_ldff1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
- target_ulong base, uint32_t desc, uintptr_t ra,
- zreg_off_fn *off_fn, sve_ldst1_tlb_fn *tlb_fn,
- sve_ld1_nf_fn *nonfault_fn)
+
+static inline QEMU_ALWAYS_INLINE
+void sve_ldff1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
+ target_ulong base, uint32_t desc, uintptr_t retaddr,
+ const int esz, const int msz, zreg_off_fn *off_fn,
+ sve_ldst1_host_fn *host_fn,
+ sve_ldst1_tlb_fn *tlb_fn)
{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
- const int mmu_idx = get_mmuidx(oi);
+ const int mmu_idx = cpu_mmu_index(env, false);
const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
- intptr_t reg_off, reg_max = simd_oprsz(desc);
- target_ulong addr;
+ const int esize = 1 << esz;
+ const int msize = 1 << msz;
+ const intptr_t reg_max = simd_oprsz(desc);
+ intptr_t reg_off;
+ SVEHostPage info;
+ target_ulong addr, in_page;
/* Skip to the first true predicate. */
- reg_off = find_next_active(vg, 0, reg_max, MO_32);
- if (likely(reg_off < reg_max)) {
- /* Perform one normal read, which will fault or not. */
- addr = off_fn(vm, reg_off);
- addr = base + (addr << scale);
- tlb_fn(env, vd, reg_off, addr, ra);
-
- /* The rest of the reads will be non-faulting. */
+ reg_off = find_next_active(vg, 0, reg_max, esz);
+ if (unlikely(reg_off >= reg_max)) {
+ /* The entire predicate was false; no load occurs. */
+ memset(vd, 0, reg_max);
+ return;
}
- /* After any fault, zero the leading predicated false elements. */
+ /*
+ * Probe the first element, allowing faults.
+ */
+ addr = base + (off_fn(vm, reg_off) << scale);
+ tlb_fn(env, vd, reg_off, addr, retaddr);
+
+ /* After any fault, zero the other elements. */
swap_memzero(vd, reg_off);
+ reg_off += esize;
+ swap_memzero(vd + reg_off, reg_max - reg_off);
- while (likely((reg_off += 4) < reg_max)) {
- uint64_t pg = *(uint64_t *)(vg + (reg_off >> 6) * 8);
- if (likely((pg >> (reg_off & 63)) & 1)) {
- addr = off_fn(vm, reg_off);
- addr = base + (addr << scale);
- if (!nonfault_fn(env, vd, reg_off, addr, mmu_idx)) {
- record_fault(env, reg_off, reg_max);
- break;
+ /*
+ * Probe the remaining elements, not allowing faults.
+ */
+ while (reg_off < reg_max) {
+ uint64_t pg = vg[reg_off >> 6];
+ do {
+ if (likely((pg >> (reg_off & 63)) & 1)) {
+ addr = base + (off_fn(vm, reg_off) << scale);
+ in_page = -(addr | TARGET_PAGE_MASK);
+
+ if (unlikely(in_page < msize)) {
+ /* Stop if the element crosses a page boundary. */
+ goto fault;
+ }
+
+ sve_probe_page(&info, true, env, addr, 0, MMU_DATA_LOAD,
+ mmu_idx, retaddr);
+ if (unlikely(info.flags & (TLB_INVALID_MASK | TLB_MMIO))) {
+ goto fault;
+ }
+ if (unlikely(info.flags & TLB_WATCHPOINT) &&
+ cpu_probe_watchpoint(env_cpu(env), addr, msize,
+ BP_MEM_READ)) {
+ goto fault;
+ }
+ /* TODO: MTE check. */
+
+ host_fn(vd, reg_off, info.host);
}
- } else {
- *(uint32_t *)(vd + H1_4(reg_off)) = 0;
- }
+ reg_off += esize;
+ } while (reg_off & 63);
}
+ return;
+
+ fault:
+ record_fault(env, reg_off, reg_max);
}
-static inline void sve_ldff1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
- target_ulong base, uint32_t desc, uintptr_t ra,
- zreg_off_fn *off_fn, sve_ldst1_tlb_fn *tlb_fn,
- sve_ld1_nf_fn *nonfault_fn)
-{
- const TCGMemOpIdx oi = extract32(desc, SIMD_DATA_SHIFT, MEMOPIDX_SHIFT);
- const int mmu_idx = get_mmuidx(oi);
- const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
- intptr_t reg_off, reg_max = simd_oprsz(desc);
- target_ulong addr;
-
- /* Skip to the first true predicate. */
- reg_off = find_next_active(vg, 0, reg_max, MO_64);
- if (likely(reg_off < reg_max)) {
- /* Perform one normal read, which will fault or not. */
- addr = off_fn(vm, reg_off);
- addr = base + (addr << scale);
- tlb_fn(env, vd, reg_off, addr, ra);
-
- /* The rest of the reads will be non-faulting. */
- }
-
- /* After any fault, zero the leading predicated false elements. */
- swap_memzero(vd, reg_off);
-
- while (likely((reg_off += 8) < reg_max)) {
- uint8_t pg = *(uint8_t *)(vg + H1(reg_off >> 3));
- if (likely(pg & 1)) {
- addr = off_fn(vm, reg_off);
- addr = base + (addr << scale);
- if (!nonfault_fn(env, vd, reg_off, addr, mmu_idx)) {
- record_fault(env, reg_off, reg_max);
- break;
- }
- } else {
- *(uint64_t *)(vd + reg_off) = 0;
- }
- }
+#define DO_LDFF1_ZPZ_S(MEM, OFS, MSZ) \
+void HELPER(sve_ldff##MEM##_##OFS)(CPUARMState *env, void *vd, void *vg, \
+ void *vm, target_ulong base, uint32_t desc) \
+{ \
+ sve_ldff1_z(env, vd, vg, vm, base, desc, GETPC(), MO_32, MSZ, \
+ off_##OFS##_s, sve_ld1##MEM##_host, sve_ld1##MEM##_tlb); \
}
-#define DO_LDFF1_ZPZ_S(MEM, OFS) \
-void HELPER(sve_ldff##MEM##_##OFS) \
- (CPUARMState *env, void *vd, void *vg, void *vm, \
- target_ulong base, uint32_t desc) \
-{ \
- sve_ldff1_zs(env, vd, vg, vm, base, desc, GETPC(), \
- off_##OFS##_s, sve_ld1##MEM##_tlb, sve_ld##MEM##_nf); \
+#define DO_LDFF1_ZPZ_D(MEM, OFS, MSZ) \
+void HELPER(sve_ldff##MEM##_##OFS)(CPUARMState *env, void *vd, void *vg, \
+ void *vm, target_ulong base, uint32_t desc) \
+{ \
+ sve_ldff1_z(env, vd, vg, vm, base, desc, GETPC(), MO_64, MSZ, \
+ off_##OFS##_d, sve_ld1##MEM##_host, sve_ld1##MEM##_tlb); \
}
-#define DO_LDFF1_ZPZ_D(MEM, OFS) \
-void HELPER(sve_ldff##MEM##_##OFS) \
- (CPUARMState *env, void *vd, void *vg, void *vm, \
- target_ulong base, uint32_t desc) \
-{ \
- sve_ldff1_zd(env, vd, vg, vm, base, desc, GETPC(), \
- off_##OFS##_d, sve_ld1##MEM##_tlb, sve_ld##MEM##_nf); \
-}
+DO_LDFF1_ZPZ_S(bsu, zsu, MO_8)
+DO_LDFF1_ZPZ_S(bsu, zss, MO_8)
+DO_LDFF1_ZPZ_D(bdu, zsu, MO_8)
+DO_LDFF1_ZPZ_D(bdu, zss, MO_8)
+DO_LDFF1_ZPZ_D(bdu, zd, MO_8)
-DO_LDFF1_ZPZ_S(bsu, zsu)
-DO_LDFF1_ZPZ_S(bsu, zss)
-DO_LDFF1_ZPZ_D(bdu, zsu)
-DO_LDFF1_ZPZ_D(bdu, zss)
-DO_LDFF1_ZPZ_D(bdu, zd)
+DO_LDFF1_ZPZ_S(bss, zsu, MO_8)
+DO_LDFF1_ZPZ_S(bss, zss, MO_8)
+DO_LDFF1_ZPZ_D(bds, zsu, MO_8)
+DO_LDFF1_ZPZ_D(bds, zss, MO_8)
+DO_LDFF1_ZPZ_D(bds, zd, MO_8)
-DO_LDFF1_ZPZ_S(bss, zsu)
-DO_LDFF1_ZPZ_S(bss, zss)
-DO_LDFF1_ZPZ_D(bds, zsu)
-DO_LDFF1_ZPZ_D(bds, zss)
-DO_LDFF1_ZPZ_D(bds, zd)
+DO_LDFF1_ZPZ_S(hsu_le, zsu, MO_16)
+DO_LDFF1_ZPZ_S(hsu_le, zss, MO_16)
+DO_LDFF1_ZPZ_D(hdu_le, zsu, MO_16)
+DO_LDFF1_ZPZ_D(hdu_le, zss, MO_16)
+DO_LDFF1_ZPZ_D(hdu_le, zd, MO_16)
-DO_LDFF1_ZPZ_S(hsu_le, zsu)
-DO_LDFF1_ZPZ_S(hsu_le, zss)
-DO_LDFF1_ZPZ_D(hdu_le, zsu)
-DO_LDFF1_ZPZ_D(hdu_le, zss)
-DO_LDFF1_ZPZ_D(hdu_le, zd)
+DO_LDFF1_ZPZ_S(hsu_be, zsu, MO_16)
+DO_LDFF1_ZPZ_S(hsu_be, zss, MO_16)
+DO_LDFF1_ZPZ_D(hdu_be, zsu, MO_16)
+DO_LDFF1_ZPZ_D(hdu_be, zss, MO_16)
+DO_LDFF1_ZPZ_D(hdu_be, zd, MO_16)
-DO_LDFF1_ZPZ_S(hsu_be, zsu)
-DO_LDFF1_ZPZ_S(hsu_be, zss)
-DO_LDFF1_ZPZ_D(hdu_be, zsu)
-DO_LDFF1_ZPZ_D(hdu_be, zss)
-DO_LDFF1_ZPZ_D(hdu_be, zd)
+DO_LDFF1_ZPZ_S(hss_le, zsu, MO_16)
+DO_LDFF1_ZPZ_S(hss_le, zss, MO_16)
+DO_LDFF1_ZPZ_D(hds_le, zsu, MO_16)
+DO_LDFF1_ZPZ_D(hds_le, zss, MO_16)
+DO_LDFF1_ZPZ_D(hds_le, zd, MO_16)
-DO_LDFF1_ZPZ_S(hss_le, zsu)
-DO_LDFF1_ZPZ_S(hss_le, zss)
-DO_LDFF1_ZPZ_D(hds_le, zsu)
-DO_LDFF1_ZPZ_D(hds_le, zss)
-DO_LDFF1_ZPZ_D(hds_le, zd)
+DO_LDFF1_ZPZ_S(hss_be, zsu, MO_16)
+DO_LDFF1_ZPZ_S(hss_be, zss, MO_16)
+DO_LDFF1_ZPZ_D(hds_be, zsu, MO_16)
+DO_LDFF1_ZPZ_D(hds_be, zss, MO_16)
+DO_LDFF1_ZPZ_D(hds_be, zd, MO_16)
-DO_LDFF1_ZPZ_S(hss_be, zsu)
-DO_LDFF1_ZPZ_S(hss_be, zss)
-DO_LDFF1_ZPZ_D(hds_be, zsu)
-DO_LDFF1_ZPZ_D(hds_be, zss)
-DO_LDFF1_ZPZ_D(hds_be, zd)
+DO_LDFF1_ZPZ_S(ss_le, zsu, MO_32)
+DO_LDFF1_ZPZ_S(ss_le, zss, MO_32)
+DO_LDFF1_ZPZ_D(sdu_le, zsu, MO_32)
+DO_LDFF1_ZPZ_D(sdu_le, zss, MO_32)
+DO_LDFF1_ZPZ_D(sdu_le, zd, MO_32)
-DO_LDFF1_ZPZ_S(ss_le, zsu)
-DO_LDFF1_ZPZ_S(ss_le, zss)
-DO_LDFF1_ZPZ_D(sdu_le, zsu)
-DO_LDFF1_ZPZ_D(sdu_le, zss)
-DO_LDFF1_ZPZ_D(sdu_le, zd)
+DO_LDFF1_ZPZ_S(ss_be, zsu, MO_32)
+DO_LDFF1_ZPZ_S(ss_be, zss, MO_32)
+DO_LDFF1_ZPZ_D(sdu_be, zsu, MO_32)
+DO_LDFF1_ZPZ_D(sdu_be, zss, MO_32)
+DO_LDFF1_ZPZ_D(sdu_be, zd, MO_32)
-DO_LDFF1_ZPZ_S(ss_be, zsu)
-DO_LDFF1_ZPZ_S(ss_be, zss)
-DO_LDFF1_ZPZ_D(sdu_be, zsu)
-DO_LDFF1_ZPZ_D(sdu_be, zss)
-DO_LDFF1_ZPZ_D(sdu_be, zd)
+DO_LDFF1_ZPZ_D(sds_le, zsu, MO_32)
+DO_LDFF1_ZPZ_D(sds_le, zss, MO_32)
+DO_LDFF1_ZPZ_D(sds_le, zd, MO_32)
-DO_LDFF1_ZPZ_D(sds_le, zsu)
-DO_LDFF1_ZPZ_D(sds_le, zss)
-DO_LDFF1_ZPZ_D(sds_le, zd)
+DO_LDFF1_ZPZ_D(sds_be, zsu, MO_32)
+DO_LDFF1_ZPZ_D(sds_be, zss, MO_32)
+DO_LDFF1_ZPZ_D(sds_be, zd, MO_32)
-DO_LDFF1_ZPZ_D(sds_be, zsu)
-DO_LDFF1_ZPZ_D(sds_be, zss)
-DO_LDFF1_ZPZ_D(sds_be, zd)
+DO_LDFF1_ZPZ_D(dd_le, zsu, MO_64)
+DO_LDFF1_ZPZ_D(dd_le, zss, MO_64)
+DO_LDFF1_ZPZ_D(dd_le, zd, MO_64)
-DO_LDFF1_ZPZ_D(dd_le, zsu)
-DO_LDFF1_ZPZ_D(dd_le, zss)
-DO_LDFF1_ZPZ_D(dd_le, zd)
-
-DO_LDFF1_ZPZ_D(dd_be, zsu)
-DO_LDFF1_ZPZ_D(dd_be, zss)
-DO_LDFF1_ZPZ_D(dd_be, zd)
+DO_LDFF1_ZPZ_D(dd_be, zsu, MO_64)
+DO_LDFF1_ZPZ_D(dd_be, zss, MO_64)
+DO_LDFF1_ZPZ_D(dd_be, zd, MO_64)
/* Stores with a vector index. */
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH 14/16] target/arm: Reuse sve_probe_page for scatter stores
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (12 preceding siblings ...)
2020-03-11 6:44 ` [PATCH 13/16] target/arm: Reuse sve_probe_page for gather first-fault loads Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 14:25 ` Peter Maydell
2020-03-11 6:44 ` [PATCH 15/16] target/arm: Reuse sve_probe_page for gather loads Richard Henderson
` (3 subsequent siblings)
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/sve_helper.c | 182 ++++++++++++++++++++++++----------------
1 file changed, 111 insertions(+), 71 deletions(-)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index a33699adf8..e8b8f2560d 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -5414,94 +5414,134 @@ DO_LDFF1_ZPZ_D(dd_be, zd, MO_64)
/* Stores with a vector index. */
-static void sve_st1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
- target_ulong base, uint32_t desc, uintptr_t ra,
- zreg_off_fn *off_fn, sve_ldst1_tlb_fn *tlb_fn)
+static inline QEMU_ALWAYS_INLINE
+void sve_st1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
+ target_ulong base, uint32_t desc, uintptr_t retaddr,
+ int esize, int msize, zreg_off_fn *off_fn,
+ sve_ldst1_host_fn *host_fn,
+ sve_ldst1_tlb_fn *tlb_fn)
{
const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
- intptr_t i, oprsz = simd_oprsz(desc);
+ const int mmu_idx = cpu_mmu_index(env, false);
+ const intptr_t reg_max = simd_oprsz(desc);
+ void *host[ARM_MAX_VQ * 4];
+ intptr_t reg_off, i;
+ SVEHostPage info, info2;
- for (i = 0; i < oprsz; ) {
- uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
+ /*
+ * Probe all of the elements for host addresses and flags.
+ */
+ i = reg_off = 0;
+ do {
+ uint64_t pg = vg[reg_off >> 6];
do {
- if (likely(pg & 1)) {
- target_ulong off = off_fn(vm, i);
- tlb_fn(env, vd, i, base + (off << scale), ra);
+ target_ulong addr = base + (off_fn(vm, reg_off) << scale);
+ target_ulong in_page = -(addr | TARGET_PAGE_MASK);
+
+ host[i] = NULL;
+ if (likely((pg >> (reg_off & 63)) & 1)) {
+ if (likely(in_page >= msize)) {
+ sve_probe_page(&info, false, env, addr, 0, MMU_DATA_STORE,
+ mmu_idx, retaddr);
+ host[i] = info.host;
+ } else {
+ /*
+ * Element crosses the page boundary.
+ * Probe both pages, but do not record the host address,
+ * so that we use the slow path.
+ */
+ sve_probe_page(&info, false, env, addr, 0,
+ MMU_DATA_STORE, mmu_idx, retaddr);
+ sve_probe_page(&info2, false, env, addr + in_page, 0,
+ MMU_DATA_STORE, mmu_idx, retaddr);
+ info.flags |= info2.flags;
+ }
+
+ if (unlikely(info.flags & TLB_WATCHPOINT)) {
+ cpu_check_watchpoint(env_cpu(env), addr, msize,
+ info.attrs, BP_MEM_WRITE, retaddr);
+ }
+ /* TODO: MTE check. */
}
- i += 4, pg >>= 4;
- } while (i & 15);
- }
-}
+ i += 1;
+ reg_off += esize;
+ } while (reg_off & 63);
+ } while (reg_off < reg_max);
-static void sve_st1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
- target_ulong base, uint32_t desc, uintptr_t ra,
- zreg_off_fn *off_fn, sve_ldst1_tlb_fn *tlb_fn)
-{
- const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
- intptr_t i, oprsz = simd_oprsz(desc) / 8;
-
- for (i = 0; i < oprsz; i++) {
- uint8_t pg = *(uint8_t *)(vg + H1(i));
- if (likely(pg & 1)) {
- target_ulong off = off_fn(vm, i * 8);
- tlb_fn(env, vd, i * 8, base + (off << scale), ra);
+ /*
+ * Now that we have recognized all exceptions except SyncExternal
+ * (from TLB_MMIO), which we cannot avoid, perform all of the stores.
+ *
+ * Note for the common case of an element in RAM, not crossing a page
+ * boundary, we have stored the host address in host[]. This doubles
+ * as a first-level check against the predicate, since only enabled
+ * elements have non-null host addresses.
+ */
+ i = reg_off = 0;
+ do {
+ void *h = host[i];
+ if (likely(h != NULL)) {
+ host_fn(vd, reg_off, h);
+ } else if ((vg[reg_off >> 6] >> (reg_off & 63)) & 1) {
+ target_ulong addr = base + (off_fn(vm, reg_off) << scale);
+ tlb_fn(env, vd, reg_off, addr, retaddr);
}
- }
+ i += 1;
+ reg_off += esize;
+ } while (reg_off < reg_max);
}
-#define DO_ST1_ZPZ_S(MEM, OFS) \
-void QEMU_FLATTEN HELPER(sve_st##MEM##_##OFS) \
- (CPUARMState *env, void *vd, void *vg, void *vm, \
- target_ulong base, uint32_t desc) \
-{ \
- sve_st1_zs(env, vd, vg, vm, base, desc, GETPC(), \
- off_##OFS##_s, sve_st1##MEM##_tlb); \
+#define DO_ST1_ZPZ_S(MEM, OFS, MSZ) \
+void HELPER(sve_st##MEM##_##OFS)(CPUARMState *env, void *vd, void *vg, \
+ void *vm, target_ulong base, uint32_t desc) \
+{ \
+ sve_st1_z(env, vd, vg, vm, base, desc, GETPC(), 4, 1 << MSZ, \
+ off_##OFS##_s, sve_st1##MEM##_host, sve_st1##MEM##_tlb); \
}
-#define DO_ST1_ZPZ_D(MEM, OFS) \
-void QEMU_FLATTEN HELPER(sve_st##MEM##_##OFS) \
- (CPUARMState *env, void *vd, void *vg, void *vm, \
- target_ulong base, uint32_t desc) \
-{ \
- sve_st1_zd(env, vd, vg, vm, base, desc, GETPC(), \
- off_##OFS##_d, sve_st1##MEM##_tlb); \
+#define DO_ST1_ZPZ_D(MEM, OFS, MSZ) \
+void HELPER(sve_st##MEM##_##OFS)(CPUARMState *env, void *vd, void *vg, \
+ void *vm, target_ulong base, uint32_t desc) \
+{ \
+ sve_st1_z(env, vd, vg, vm, base, desc, GETPC(), 8, 1 << MSZ, \
+ off_##OFS##_d, sve_st1##MEM##_host, sve_st1##MEM##_tlb); \
}
-DO_ST1_ZPZ_S(bs, zsu)
-DO_ST1_ZPZ_S(hs_le, zsu)
-DO_ST1_ZPZ_S(hs_be, zsu)
-DO_ST1_ZPZ_S(ss_le, zsu)
-DO_ST1_ZPZ_S(ss_be, zsu)
+DO_ST1_ZPZ_S(bs, zsu, MO_8)
+DO_ST1_ZPZ_S(hs_le, zsu, MO_16)
+DO_ST1_ZPZ_S(hs_be, zsu, MO_16)
+DO_ST1_ZPZ_S(ss_le, zsu, MO_32)
+DO_ST1_ZPZ_S(ss_be, zsu, MO_32)
-DO_ST1_ZPZ_S(bs, zss)
-DO_ST1_ZPZ_S(hs_le, zss)
-DO_ST1_ZPZ_S(hs_be, zss)
-DO_ST1_ZPZ_S(ss_le, zss)
-DO_ST1_ZPZ_S(ss_be, zss)
+DO_ST1_ZPZ_S(bs, zss, MO_8)
+DO_ST1_ZPZ_S(hs_le, zss, MO_16)
+DO_ST1_ZPZ_S(hs_be, zss, MO_16)
+DO_ST1_ZPZ_S(ss_le, zss, MO_32)
+DO_ST1_ZPZ_S(ss_be, zss, MO_32)
-DO_ST1_ZPZ_D(bd, zsu)
-DO_ST1_ZPZ_D(hd_le, zsu)
-DO_ST1_ZPZ_D(hd_be, zsu)
-DO_ST1_ZPZ_D(sd_le, zsu)
-DO_ST1_ZPZ_D(sd_be, zsu)
-DO_ST1_ZPZ_D(dd_le, zsu)
-DO_ST1_ZPZ_D(dd_be, zsu)
+DO_ST1_ZPZ_D(bd, zsu, MO_8)
+DO_ST1_ZPZ_D(hd_le, zsu, MO_16)
+DO_ST1_ZPZ_D(hd_be, zsu, MO_16)
+DO_ST1_ZPZ_D(sd_le, zsu, MO_32)
+DO_ST1_ZPZ_D(sd_be, zsu, MO_32)
+DO_ST1_ZPZ_D(dd_le, zsu, MO_64)
+DO_ST1_ZPZ_D(dd_be, zsu, MO_64)
-DO_ST1_ZPZ_D(bd, zss)
-DO_ST1_ZPZ_D(hd_le, zss)
-DO_ST1_ZPZ_D(hd_be, zss)
-DO_ST1_ZPZ_D(sd_le, zss)
-DO_ST1_ZPZ_D(sd_be, zss)
-DO_ST1_ZPZ_D(dd_le, zss)
-DO_ST1_ZPZ_D(dd_be, zss)
+DO_ST1_ZPZ_D(bd, zss, MO_8)
+DO_ST1_ZPZ_D(hd_le, zss, MO_16)
+DO_ST1_ZPZ_D(hd_be, zss, MO_16)
+DO_ST1_ZPZ_D(sd_le, zss, MO_32)
+DO_ST1_ZPZ_D(sd_be, zss, MO_32)
+DO_ST1_ZPZ_D(dd_le, zss, MO_64)
+DO_ST1_ZPZ_D(dd_be, zss, MO_64)
-DO_ST1_ZPZ_D(bd, zd)
-DO_ST1_ZPZ_D(hd_le, zd)
-DO_ST1_ZPZ_D(hd_be, zd)
-DO_ST1_ZPZ_D(sd_le, zd)
-DO_ST1_ZPZ_D(sd_be, zd)
-DO_ST1_ZPZ_D(dd_le, zd)
-DO_ST1_ZPZ_D(dd_be, zd)
+DO_ST1_ZPZ_D(bd, zd, MO_8)
+DO_ST1_ZPZ_D(hd_le, zd, MO_16)
+DO_ST1_ZPZ_D(hd_be, zd, MO_16)
+DO_ST1_ZPZ_D(sd_le, zd, MO_32)
+DO_ST1_ZPZ_D(sd_be, zd, MO_32)
+DO_ST1_ZPZ_D(dd_le, zd, MO_64)
+DO_ST1_ZPZ_D(dd_be, zd, MO_64)
#undef DO_ST1_ZPZ_S
#undef DO_ST1_ZPZ_D
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH 15/16] target/arm: Reuse sve_probe_page for gather loads
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (13 preceding siblings ...)
2020-03-11 6:44 ` [PATCH 14/16] target/arm: Reuse sve_probe_page for scatter stores Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 14:21 ` Peter Maydell
2020-03-11 6:44 ` [PATCH 16/16] target/arm: Remove sve_memopidx Richard Henderson
` (2 subsequent siblings)
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/sve_helper.c | 208 +++++++++++++++++++++-------------------
1 file changed, 109 insertions(+), 99 deletions(-)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index e8b8f2560d..4ad870f1ee 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -5125,130 +5125,140 @@ static target_ulong off_zd_d(void *reg, intptr_t reg_ofs)
return *(uint64_t *)(reg + reg_ofs);
}
-static void sve_ld1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
- target_ulong base, uint32_t desc, uintptr_t ra,
- zreg_off_fn *off_fn, sve_ldst1_tlb_fn *tlb_fn)
+static inline QEMU_ALWAYS_INLINE
+void sve_ld1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
+ target_ulong base, uint32_t desc, uintptr_t retaddr,
+ int esize, int msize, zreg_off_fn *off_fn,
+ sve_ldst1_host_fn *host_fn,
+ sve_ldst1_tlb_fn *tlb_fn)
{
const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
- intptr_t i, oprsz = simd_oprsz(desc);
- ARMVectorReg scratch = { };
+ const int mmu_idx = cpu_mmu_index(env, false);
+ const intptr_t reg_max = simd_oprsz(desc);
+ ARMVectorReg scratch;
+ intptr_t reg_off;
+ SVEHostPage info, info2;
- for (i = 0; i < oprsz; ) {
- uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
+ memset(&scratch, 0, reg_max);
+ reg_off = 0;
+ do {
+ uint64_t pg = vg[reg_off >> 6];
do {
if (likely(pg & 1)) {
- target_ulong off = off_fn(vm, i);
- tlb_fn(env, &scratch, i, base + (off << scale), ra);
+ target_ulong addr = base + (off_fn(vm, reg_off) << scale);
+ target_ulong in_page = -(addr | TARGET_PAGE_MASK);
+
+ sve_probe_page(&info, false, env, addr, 0, MMU_DATA_LOAD,
+ mmu_idx, retaddr);
+
+ if (likely(in_page >= msize)) {
+ if (unlikely(info.flags & TLB_WATCHPOINT)) {
+ cpu_check_watchpoint(env_cpu(env), addr, msize,
+ info.attrs, BP_MEM_READ, retaddr);
+ }
+ /* TODO: MTE check */
+ host_fn(&scratch, reg_off, info.host);
+ } else {
+ /* Element crosses the page boundary. */
+ sve_probe_page(&info2, false, env, addr + in_page, 0,
+ MMU_DATA_LOAD, mmu_idx, retaddr);
+ if (unlikely((info.flags | info2.flags) & TLB_WATCHPOINT)) {
+ cpu_check_watchpoint(env_cpu(env), addr,
+ msize, info.attrs,
+ BP_MEM_READ, retaddr);
+ }
+ /* TODO: MTE check */
+ tlb_fn(env, &scratch, reg_off, addr, retaddr);
+ }
}
- i += 4, pg >>= 4;
- } while (i & 15);
- }
+ reg_off += esize;
+ pg >>= esize;
+ } while (reg_off & 63);
+ } while (reg_off < reg_max);
/* Wait until all exceptions have been raised to write back. */
- memcpy(vd, &scratch, oprsz);
+ memcpy(vd, &scratch, reg_max);
}
-static void sve_ld1_zd(CPUARMState *env, void *vd, void *vg, void *vm,
- target_ulong base, uint32_t desc, uintptr_t ra,
- zreg_off_fn *off_fn, sve_ldst1_tlb_fn *tlb_fn)
-{
- const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
- intptr_t i, oprsz = simd_oprsz(desc) / 8;
- ARMVectorReg scratch = { };
-
- for (i = 0; i < oprsz; i++) {
- uint8_t pg = *(uint8_t *)(vg + H1(i));
- if (likely(pg & 1)) {
- target_ulong off = off_fn(vm, i * 8);
- tlb_fn(env, &scratch, i * 8, base + (off << scale), ra);
- }
- }
-
- /* Wait until all exceptions have been raised to write back. */
- memcpy(vd, &scratch, oprsz * 8);
+#define DO_LD1_ZPZ_S(MEM, OFS, MSZ) \
+void HELPER(sve_ld##MEM##_##OFS)(CPUARMState *env, void *vd, void *vg, \
+ void *vm, target_ulong base, uint32_t desc) \
+{ \
+ sve_ld1_z(env, vd, vg, vm, base, desc, GETPC(), 4, 1 << MSZ, \
+ off_##OFS##_s, sve_ld1##MEM##_host, sve_ld1##MEM##_tlb); \
}
-#define DO_LD1_ZPZ_S(MEM, OFS) \
-void QEMU_FLATTEN HELPER(sve_ld##MEM##_##OFS) \
- (CPUARMState *env, void *vd, void *vg, void *vm, \
- target_ulong base, uint32_t desc) \
-{ \
- sve_ld1_zs(env, vd, vg, vm, base, desc, GETPC(), \
- off_##OFS##_s, sve_ld1##MEM##_tlb); \
+#define DO_LD1_ZPZ_D(MEM, OFS, MSZ) \
+void HELPER(sve_ld##MEM##_##OFS)(CPUARMState *env, void *vd, void *vg, \
+ void *vm, target_ulong base, uint32_t desc) \
+{ \
+ sve_ld1_z(env, vd, vg, vm, base, desc, GETPC(), 8, 1 << MSZ, \
+ off_##OFS##_d, sve_ld1##MEM##_host, sve_ld1##MEM##_tlb); \
}
-#define DO_LD1_ZPZ_D(MEM, OFS) \
-void QEMU_FLATTEN HELPER(sve_ld##MEM##_##OFS) \
- (CPUARMState *env, void *vd, void *vg, void *vm, \
- target_ulong base, uint32_t desc) \
-{ \
- sve_ld1_zd(env, vd, vg, vm, base, desc, GETPC(), \
- off_##OFS##_d, sve_ld1##MEM##_tlb); \
-}
+DO_LD1_ZPZ_S(bsu, zsu, MO_8)
+DO_LD1_ZPZ_S(bsu, zss, MO_8)
+DO_LD1_ZPZ_D(bdu, zsu, MO_8)
+DO_LD1_ZPZ_D(bdu, zss, MO_8)
+DO_LD1_ZPZ_D(bdu, zd, MO_8)
-DO_LD1_ZPZ_S(bsu, zsu)
-DO_LD1_ZPZ_S(bsu, zss)
-DO_LD1_ZPZ_D(bdu, zsu)
-DO_LD1_ZPZ_D(bdu, zss)
-DO_LD1_ZPZ_D(bdu, zd)
+DO_LD1_ZPZ_S(bss, zsu, MO_8)
+DO_LD1_ZPZ_S(bss, zss, MO_8)
+DO_LD1_ZPZ_D(bds, zsu, MO_8)
+DO_LD1_ZPZ_D(bds, zss, MO_8)
+DO_LD1_ZPZ_D(bds, zd, MO_8)
-DO_LD1_ZPZ_S(bss, zsu)
-DO_LD1_ZPZ_S(bss, zss)
-DO_LD1_ZPZ_D(bds, zsu)
-DO_LD1_ZPZ_D(bds, zss)
-DO_LD1_ZPZ_D(bds, zd)
+DO_LD1_ZPZ_S(hsu_le, zsu, MO_16)
+DO_LD1_ZPZ_S(hsu_le, zss, MO_16)
+DO_LD1_ZPZ_D(hdu_le, zsu, MO_16)
+DO_LD1_ZPZ_D(hdu_le, zss, MO_16)
+DO_LD1_ZPZ_D(hdu_le, zd, MO_16)
-DO_LD1_ZPZ_S(hsu_le, zsu)
-DO_LD1_ZPZ_S(hsu_le, zss)
-DO_LD1_ZPZ_D(hdu_le, zsu)
-DO_LD1_ZPZ_D(hdu_le, zss)
-DO_LD1_ZPZ_D(hdu_le, zd)
+DO_LD1_ZPZ_S(hsu_be, zsu, MO_16)
+DO_LD1_ZPZ_S(hsu_be, zss, MO_16)
+DO_LD1_ZPZ_D(hdu_be, zsu, MO_16)
+DO_LD1_ZPZ_D(hdu_be, zss, MO_16)
+DO_LD1_ZPZ_D(hdu_be, zd, MO_16)
-DO_LD1_ZPZ_S(hsu_be, zsu)
-DO_LD1_ZPZ_S(hsu_be, zss)
-DO_LD1_ZPZ_D(hdu_be, zsu)
-DO_LD1_ZPZ_D(hdu_be, zss)
-DO_LD1_ZPZ_D(hdu_be, zd)
+DO_LD1_ZPZ_S(hss_le, zsu, MO_16)
+DO_LD1_ZPZ_S(hss_le, zss, MO_16)
+DO_LD1_ZPZ_D(hds_le, zsu, MO_16)
+DO_LD1_ZPZ_D(hds_le, zss, MO_16)
+DO_LD1_ZPZ_D(hds_le, zd, MO_16)
-DO_LD1_ZPZ_S(hss_le, zsu)
-DO_LD1_ZPZ_S(hss_le, zss)
-DO_LD1_ZPZ_D(hds_le, zsu)
-DO_LD1_ZPZ_D(hds_le, zss)
-DO_LD1_ZPZ_D(hds_le, zd)
+DO_LD1_ZPZ_S(hss_be, zsu, MO_16)
+DO_LD1_ZPZ_S(hss_be, zss, MO_16)
+DO_LD1_ZPZ_D(hds_be, zsu, MO_16)
+DO_LD1_ZPZ_D(hds_be, zss, MO_16)
+DO_LD1_ZPZ_D(hds_be, zd, MO_16)
-DO_LD1_ZPZ_S(hss_be, zsu)
-DO_LD1_ZPZ_S(hss_be, zss)
-DO_LD1_ZPZ_D(hds_be, zsu)
-DO_LD1_ZPZ_D(hds_be, zss)
-DO_LD1_ZPZ_D(hds_be, zd)
+DO_LD1_ZPZ_S(ss_le, zsu, MO_32)
+DO_LD1_ZPZ_S(ss_le, zss, MO_32)
+DO_LD1_ZPZ_D(sdu_le, zsu, MO_32)
+DO_LD1_ZPZ_D(sdu_le, zss, MO_32)
+DO_LD1_ZPZ_D(sdu_le, zd, MO_32)
-DO_LD1_ZPZ_S(ss_le, zsu)
-DO_LD1_ZPZ_S(ss_le, zss)
-DO_LD1_ZPZ_D(sdu_le, zsu)
-DO_LD1_ZPZ_D(sdu_le, zss)
-DO_LD1_ZPZ_D(sdu_le, zd)
+DO_LD1_ZPZ_S(ss_be, zsu, MO_32)
+DO_LD1_ZPZ_S(ss_be, zss, MO_32)
+DO_LD1_ZPZ_D(sdu_be, zsu, MO_32)
+DO_LD1_ZPZ_D(sdu_be, zss, MO_32)
+DO_LD1_ZPZ_D(sdu_be, zd, MO_32)
-DO_LD1_ZPZ_S(ss_be, zsu)
-DO_LD1_ZPZ_S(ss_be, zss)
-DO_LD1_ZPZ_D(sdu_be, zsu)
-DO_LD1_ZPZ_D(sdu_be, zss)
-DO_LD1_ZPZ_D(sdu_be, zd)
+DO_LD1_ZPZ_D(sds_le, zsu, MO_32)
+DO_LD1_ZPZ_D(sds_le, zss, MO_32)
+DO_LD1_ZPZ_D(sds_le, zd, MO_32)
-DO_LD1_ZPZ_D(sds_le, zsu)
-DO_LD1_ZPZ_D(sds_le, zss)
-DO_LD1_ZPZ_D(sds_le, zd)
+DO_LD1_ZPZ_D(sds_be, zsu, MO_32)
+DO_LD1_ZPZ_D(sds_be, zss, MO_32)
+DO_LD1_ZPZ_D(sds_be, zd, MO_32)
-DO_LD1_ZPZ_D(sds_be, zsu)
-DO_LD1_ZPZ_D(sds_be, zss)
-DO_LD1_ZPZ_D(sds_be, zd)
+DO_LD1_ZPZ_D(dd_le, zsu, MO_64)
+DO_LD1_ZPZ_D(dd_le, zss, MO_64)
+DO_LD1_ZPZ_D(dd_le, zd, MO_64)
-DO_LD1_ZPZ_D(dd_le, zsu)
-DO_LD1_ZPZ_D(dd_le, zss)
-DO_LD1_ZPZ_D(dd_le, zd)
-
-DO_LD1_ZPZ_D(dd_be, zsu)
-DO_LD1_ZPZ_D(dd_be, zss)
-DO_LD1_ZPZ_D(dd_be, zd)
+DO_LD1_ZPZ_D(dd_be, zsu, MO_64)
+DO_LD1_ZPZ_D(dd_be, zss, MO_64)
+DO_LD1_ZPZ_D(dd_be, zd, MO_64)
#undef DO_LD1_ZPZ_S
#undef DO_LD1_ZPZ_D
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH 16/16] target/arm: Remove sve_memopidx
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (14 preceding siblings ...)
2020-03-11 6:44 ` [PATCH 15/16] target/arm: Reuse sve_probe_page for gather loads Richard Henderson
@ 2020-03-11 6:44 ` Richard Henderson
2020-04-16 14:27 ` Peter Maydell
2020-03-11 7:10 ` [PATCH 00/16] target/arm: sve load/store improvements no-reply
2020-04-16 14:28 ` Peter Maydell
17 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2020-03-11 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
None of the sve helpers use TCGMemOpIdx any longer, so we can
stop passing it.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/internals.h | 5 -----
target/arm/sve_helper.c | 14 +++++++-------
target/arm/translate-sve.c | 17 +++--------------
3 files changed, 10 insertions(+), 26 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index e633aff36e..a833e3941d 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -979,11 +979,6 @@ static inline int arm_num_ctx_cmps(ARMCPU *cpu)
}
}
-/* Note make_memop_idx reserves 4 bits for mmu_idx, and MO_BSWAP is bit 3.
- * Thus a TCGMemOpIdx, without any MO_ALIGN bits, fits in 8 bits.
- */
-#define MEMOPIDX_SHIFT 8
-
/**
* v7m_using_psp: Return true if using process stack pointer
* Return true if the CPU is currently using the process stack
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 4ad870f1ee..80453953ad 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4447,7 +4447,7 @@ void sve_ldN_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
sve_ldst1_host_fn *host_fn,
sve_ldst1_tlb_fn *tlb_fn)
{
- const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
+ const unsigned rd = simd_data(desc);
const intptr_t reg_max = simd_oprsz(desc);
intptr_t reg_off, reg_last, mem_off;
SVEContLdSt info;
@@ -4703,7 +4703,7 @@ void sve_ldnfff1_r(CPUARMState *env, void *vg, const target_ulong addr,
sve_ldst1_host_fn *host_fn,
sve_ldst1_tlb_fn *tlb_fn)
{
- const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
+ const unsigned rd = simd_data(desc);
void *vd = &env->vfp.zregs[rd];
const intptr_t reg_max = simd_oprsz(desc);
intptr_t reg_off, mem_off, reg_last;
@@ -4926,7 +4926,7 @@ void sve_stN_r(CPUARMState *env, uint64_t *vg, target_ulong addr, uint32_t desc,
sve_ldst1_host_fn *host_fn,
sve_ldst1_tlb_fn *tlb_fn)
{
- const unsigned rd = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 5);
+ const unsigned rd = simd_data(desc);
const intptr_t reg_max = simd_oprsz(desc);
intptr_t reg_off, reg_last, mem_off;
SVEContLdSt info;
@@ -5132,9 +5132,9 @@ void sve_ld1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
sve_ldst1_host_fn *host_fn,
sve_ldst1_tlb_fn *tlb_fn)
{
- const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
const int mmu_idx = cpu_mmu_index(env, false);
const intptr_t reg_max = simd_oprsz(desc);
+ const int scale = simd_data(desc);
ARMVectorReg scratch;
intptr_t reg_off;
SVEHostPage info, info2;
@@ -5277,10 +5277,10 @@ void sve_ldff1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
sve_ldst1_tlb_fn *tlb_fn)
{
const int mmu_idx = cpu_mmu_index(env, false);
- const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
+ const intptr_t reg_max = simd_oprsz(desc);
+ const int scale = simd_data(desc);
const int esize = 1 << esz;
const int msize = 1 << msz;
- const intptr_t reg_max = simd_oprsz(desc);
intptr_t reg_off;
SVEHostPage info;
target_ulong addr, in_page;
@@ -5431,9 +5431,9 @@ void sve_st1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
sve_ldst1_host_fn *host_fn,
sve_ldst1_tlb_fn *tlb_fn)
{
- const int scale = extract32(desc, SIMD_DATA_SHIFT + MEMOPIDX_SHIFT, 2);
const int mmu_idx = cpu_mmu_index(env, false);
const intptr_t reg_max = simd_oprsz(desc);
+ const int scale = simd_data(desc);
void *host[ARM_MAX_VQ * 4];
intptr_t reg_off, i;
SVEHostPage info, info2;
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index b35bad245e..7bd7de80e6 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -4584,11 +4584,6 @@ static const uint8_t dtype_esz[16] = {
3, 2, 1, 3
};
-static TCGMemOpIdx sve_memopidx(DisasContext *s, int dtype)
-{
- return make_memop_idx(s->be_data | dtype_mop[dtype], get_mem_index(s));
-}
-
static void do_mem_zpa(DisasContext *s, int zt, int pg, TCGv_i64 addr,
int dtype, gen_helper_gvec_mem *fn)
{
@@ -4601,9 +4596,7 @@ static void do_mem_zpa(DisasContext *s, int zt, int pg, TCGv_i64 addr,
* registers as pointers, so encode the regno into the data field.
* For consistency, do this even for LD1.
*/
- desc = sve_memopidx(s, dtype);
- desc |= zt << MEMOPIDX_SHIFT;
- desc = simd_desc(vsz, vsz, desc);
+ desc = simd_desc(vsz, vsz, zt);
t_desc = tcg_const_i32(desc);
t_pg = tcg_temp_new_ptr();
@@ -4835,9 +4828,7 @@ static void do_ldrq(DisasContext *s, int zt, int pg, TCGv_i64 addr, int msz)
int desc, poff;
/* Load the first quadword using the normal predicated load helpers. */
- desc = sve_memopidx(s, msz_dtype(s, msz));
- desc |= zt << MEMOPIDX_SHIFT;
- desc = simd_desc(16, 16, desc);
+ desc = simd_desc(16, 16, zt);
t_desc = tcg_const_i32(desc);
poff = pred_full_reg_offset(s, pg);
@@ -5066,9 +5057,7 @@ static void do_mem_zpz(DisasContext *s, int zt, int pg, int zm,
TCGv_i32 t_desc;
int desc;
- desc = sve_memopidx(s, msz_dtype(s, msz));
- desc |= scale << MEMOPIDX_SHIFT;
- desc = simd_desc(vsz, vsz, desc);
+ desc = simd_desc(vsz, vsz, scale);
t_desc = tcg_const_i32(desc);
tcg_gen_addi_ptr(t_pg, cpu_env, pred_full_reg_offset(s, pg));
--
2.20.1
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH 00/16] target/arm: sve load/store improvements
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (15 preceding siblings ...)
2020-03-11 6:44 ` [PATCH 16/16] target/arm: Remove sve_memopidx Richard Henderson
@ 2020-03-11 7:10 ` no-reply
2020-04-16 14:28 ` Peter Maydell
17 siblings, 0 replies; 44+ messages in thread
From: no-reply @ 2020-03-11 7:10 UTC (permalink / raw)
To: richard.henderson; +Cc: peter.maydell, qemu-arm, qemu-devel
Patchew URL: https://patchew.org/QEMU/20200311064420.30606-1-richard.henderson@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH 00/16] target/arm: sve load/store improvements
Message-id: 20200311064420.30606-1-richard.henderson@linaro.org
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 ===
Switched to a new branch 'test'
4cb4cfb target/arm: Remove sve_memopidx
9b43c4c target/arm: Reuse sve_probe_page for gather loads
3032a49 target/arm: Reuse sve_probe_page for scatter stores
abf11d7 target/arm: Reuse sve_probe_page for gather first-fault loads
020f874 target/arm: Use SVEContLdSt for contiguous stores
de31b30 target/arm: Update contiguous first-fault and no-fault loads
627528c target/arm: Use SVEContLdSt for multi-register contiguous loads
678a2d7 target/arm: Handle watchpoints in sve_ld1_r
b6827bc target/arm: Use SVEContLdSt in sve_ld1_r
c6fa5f6 target/arm: Adjust interface of sve_ld1_host_fn
ba3d3ce target/arm: Add sve infrastructure for page lookup
0cf3b33 target/arm: Drop manual handling of set/clear_helper_retaddr
7faf8fb target/arm: Use cpu_*_data_ra for sve_ldst_tlb_fn
d8ed5c8 exec: Add cpu_probe_watchpoint
4ff4098 accel/tcg: Add probe_access_flags
4b6c3d0 accel/tcg: Add block comment for probe_access
=== OUTPUT BEGIN ===
1/16 Checking commit 4b6c3d0e10c9 (accel/tcg: Add block comment for probe_access)
2/16 Checking commit 4ff409819099 (accel/tcg: Add probe_access_flags)
WARNING: line over 80 characters
#273: FILE: accel/tcg/user-exec.c:220:
+ cc->tlb_fill(cpu, addr, 0, access_type, MMU_USER_IDX, false, retaddr);
total: 0 errors, 1 warnings, 312 lines checked
Patch 2/16 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/16 Checking commit d8ed5c85deea (exec: Add cpu_probe_watchpoint)
ERROR: trailing whitespace
#31: FILE: exec.c:2747:
+ if (watchpoint_address_matches(wp, addr, len) && $
total: 1 errors, 0 warnings, 44 lines checked
Patch 3/16 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/16 Checking commit 7faf8fbb5a67 (target/arm: Use cpu_*_data_ra for sve_ldst_tlb_fn)
ERROR: spaces required around that '*' (ctx:WxV)
#176: FILE: target/arm/sve_helper.c:4184:
+ sve_ldst1_tlb_fn *tlb_fn)
^
total: 1 errors, 0 warnings, 479 lines checked
Patch 4/16 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/16 Checking commit 0cf3b3397565 (target/arm: Drop manual handling of set/clear_helper_retaddr)
6/16 Checking commit ba3d3cec8772 (target/arm: Add sve infrastructure for page lookup)
7/16 Checking commit c6fa5f6f82a3 (target/arm: Adjust interface of sve_ld1_host_fn)
8/16 Checking commit b6827bcde171 (target/arm: Use SVEContLdSt in sve_ld1_r)
9/16 Checking commit 678a2d7de202 (target/arm: Handle watchpoints in sve_ld1_r)
WARNING: line over 80 characters
#51: FILE: target/arm/sve_helper.c:4408:
+ info->page[0].attrs, wp_access, retaddr);
WARNING: line over 80 characters
#75: FILE: target/arm/sve_helper.c:4432:
+ info->page[1].attrs, wp_access, retaddr);
total: 0 errors, 2 warnings, 88 lines checked
Patch 9/16 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/16 Checking commit 627528c11533 (target/arm: Use SVEContLdSt for multi-register contiguous loads)
11/16 Checking commit de31b300b881 (target/arm: Update contiguous first-fault and no-fault loads)
12/16 Checking commit 020f874a48bf (target/arm: Use SVEContLdSt for contiguous stores)
13/16 Checking commit abf11d771910 (target/arm: Reuse sve_probe_page for gather first-fault loads)
14/16 Checking commit 3032a491bbe5 (target/arm: Reuse sve_probe_page for scatter stores)
15/16 Checking commit 9b43c4ccb322 (target/arm: Reuse sve_probe_page for gather loads)
16/16 Checking commit 4cb4cfb45691 (target/arm: Remove sve_memopidx)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20200311064420.30606-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
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 00/16] target/arm: sve load/store improvements
2020-03-11 6:44 [PATCH 00/16] target/arm: sve load/store improvements Richard Henderson
` (16 preceding siblings ...)
2020-03-11 7:10 ` [PATCH 00/16] target/arm: sve load/store improvements no-reply
@ 2020-04-16 14:28 ` Peter Maydell
17 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-04-16 14:28 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, QEMU Developers
On Wed, 11 Mar 2020 at 06:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The goal here is to support MTE, but there's some cleanup to do.
>
> Technically, we have sufficient interfaces in cputlb.c now, but it
> requires multiple tlb lookups on different interfaces to do so.
>
> Adding probe_access_flags() allows probing the tlb and getting out
> some of the flags buried in the tlb comparator, such as TLB_MMIO
> and TLB_WATCHPOINT. In addition, we get no-fault semantics,
> which we don't have via probe_acccess().
>
> Adding cpu_probe_watchpoint() allows to *not* stop a first-fault
> or no-fault load when the page contains a watchpoint, but the actual
> access does not hit.
>
> Having these available means that we can handle all of the watchpoints
> for a given set of loads/stores all at once, before we begin doing any
> actual memory operations. Further, the actual memory operation on a
> page of ram that has a watchpoint can still use the fast path.
>
> Looking forward to MTE, we can examine the Tagged bit on a per-page
> basis and avoid dozens of mte_check calls that must be Unchecked.
> That comes later, in a new version of the MTE patch set, but I do
> add comments for where the checks should be added.
Series reviewed; I didn't bother to flag up the checkpatch
complaints, but I think all but one of them are legit, so
please fix those too.
thanks
-- PMM
^ permalink raw reply [flat|nested] 44+ messages in thread