* [Qemu-devel] [PATCH v1 0/2] tcg: Introduce probe_access() and return a host pointer
@ 2019-08-28 11:10 David Hildenbrand
2019-08-28 11:10 ` [Qemu-devel] [PATCH v1 1/2] tcg: Make probe_write() return a pointer to the host page David Hildenbrand
2019-08-28 11:10 ` [Qemu-devel] [PATCH v1 2/2] tcg: Factor out probe_write() logic into probe_access() David Hildenbrand
0 siblings, 2 replies; 7+ messages in thread
From: David Hildenbrand @ 2019-08-28 11:10 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Riku Voipio, David Hildenbrand, Cornelia Huck,
qemu-s390x, Paolo Bonzini, Richard Henderson
Let's return a host pointer from probe_access() if possible and provide
probe_access(). s390x will soon make use of probe_access() and use the
provided host pointer.
Based on tcg-next and "tcg: probe_write() refactorings".
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Riku Voipio <riku.voipio@iki.fi>
David Hildenbrand (2):
tcg: Make probe_write() return a pointer to the host page
tcg: Factor out probe_write() logic into probe_access()
accel/tcg/cputlb.c | 60 +++++++++++++++++++++++++++++++----------
accel/tcg/user-exec.c | 27 +++++++++++++++----
include/exec/exec-all.h | 9 +++++--
3 files changed, 75 insertions(+), 21 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v1 1/2] tcg: Make probe_write() return a pointer to the host page
2019-08-28 11:10 [Qemu-devel] [PATCH v1 0/2] tcg: Introduce probe_access() and return a host pointer David Hildenbrand
@ 2019-08-28 11:10 ` David Hildenbrand
2019-08-28 14:44 ` Richard Henderson
2019-08-28 11:10 ` [Qemu-devel] [PATCH v1 2/2] tcg: Factor out probe_write() logic into probe_access() David Hildenbrand
1 sibling, 1 reply; 7+ messages in thread
From: David Hildenbrand @ 2019-08-28 11:10 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Riku Voipio, David Hildenbrand, Cornelia Huck,
qemu-s390x, Paolo Bonzini, Richard Henderson
... similar to tlb_vaddr_to_host(); however, allow access to the host
page except when TLB_NOTDIRTY or TLB_MMIO is set.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
accel/tcg/cputlb.c | 22 +++++++++++++++++-----
accel/tcg/user-exec.c | 6 ++++--
include/exec/exec-all.h | 4 ++--
3 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 09fe4cdcc4..2077685da0 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1078,11 +1078,12 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
/* Probe for whether the specified guest write access is permitted.
* If it is not permitted then an exception will be taken in the same
* way as if this were a real write access (and we will not return).
- * Otherwise the function will return, and there will be a valid
- * entry in the TLB for this access.
+ * If the access is permitted, returns the host address similar to
+ * tlb_vaddr_to_host(). Returns NULL in case direct access to the host page
+ * is not allowed or if the size is 0.
*/
-void probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
- uintptr_t retaddr)
+void *probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
+ uintptr_t retaddr)
{
uintptr_t index = tlb_index(env, mmu_idx, addr);
CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
@@ -1101,12 +1102,23 @@ void probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
tlb_addr = tlb_addr_write(entry);
}
+ if (!size) {
+ return NULL;
+ }
+
/* Handle watchpoints. */
- if ((tlb_addr & TLB_WATCHPOINT) && size > 0) {
+ if (tlb_addr & TLB_WATCHPOINT) {
cpu_check_watchpoint(env_cpu(env), addr, size,
env_tlb(env)->d[mmu_idx].iotlb[index].attrs,
BP_MEM_WRITE, retaddr);
}
+
+ if (tlb_addr & (TLB_NOTDIRTY | TLB_MMIO)) {
+ /* IO access */
+ return NULL;
+ }
+
+ return (void *)((uintptr_t)addr + entry->addend);
}
void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index b25a342eaa..420184571f 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -188,8 +188,8 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,
g_assert_not_reached();
}
-void probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
- uintptr_t retaddr)
+void *probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
+ uintptr_t retaddr)
{
CPUState *cpu = env_cpu(env);
CPUClass *cc;
@@ -203,6 +203,8 @@ void probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
retaddr);
g_assert_not_reached();
}
+
+ return size ? g2h(addr) : NULL;
}
#if defined(__i386__)
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index cbcc85add3..a7893ed16b 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -310,8 +310,8 @@ static inline void tlb_flush_by_mmuidx_all_cpus_synced(CPUState *cpu,
{
}
#endif
-void probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
- uintptr_t retaddr);
+void *probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
+ uintptr_t retaddr);
#define CODE_GEN_ALIGN 16 /* must be >= of the size of a icache line */
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v1 2/2] tcg: Factor out probe_write() logic into probe_access()
2019-08-28 11:10 [Qemu-devel] [PATCH v1 0/2] tcg: Introduce probe_access() and return a host pointer David Hildenbrand
2019-08-28 11:10 ` [Qemu-devel] [PATCH v1 1/2] tcg: Make probe_write() return a pointer to the host page David Hildenbrand
@ 2019-08-28 11:10 ` David Hildenbrand
2019-08-28 14:52 ` Richard Henderson
1 sibling, 1 reply; 7+ messages in thread
From: David Hildenbrand @ 2019-08-28 11:10 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Riku Voipio, David Hildenbrand, Cornelia Huck,
qemu-s390x, Paolo Bonzini, Richard Henderson
Let's also allow to probe other access types.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
accel/tcg/cputlb.c | 48 +++++++++++++++++++++++++++++------------
accel/tcg/user-exec.c | 25 ++++++++++++++++-----
include/exec/exec-all.h | 9 ++++++--
3 files changed, 61 insertions(+), 21 deletions(-)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 2077685da0..df3df104aa 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1075,31 +1075,51 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
return qemu_ram_addr_from_host_nofail(p);
}
-/* Probe for whether the specified guest write access is permitted.
- * If it is not permitted then an exception will be taken in the same
- * way as if this were a real write access (and we will not return).
- * If the access is permitted, returns the host address similar to
- * tlb_vaddr_to_host(). Returns NULL in case direct access to the host page
- * is not allowed or if the size is 0.
+/*
+ * 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 access is permitted,
+ * returns the host address similar to tlb_vaddr_to_host(). Returns NULL
+ * in case direct access to the host page is not allowed or if the size is 0.
*/
-void *probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
- uintptr_t retaddr)
+void *probe_access(CPUArchState *env, target_ulong addr, int size,
+ MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
{
uintptr_t index = tlb_index(env, mmu_idx, addr);
CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
- target_ulong tlb_addr = tlb_addr_write(entry);
+ 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(addr_write, addr)) {
- tlb_fill(env_cpu(env), addr, size, MMU_DATA_STORE,
- mmu_idx, retaddr);
+ 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_addr_write(entry);
+ tlb_addr = tlb_read_ofs(entry, elt_ofs);
}
if (!size) {
@@ -1110,7 +1130,7 @@ void *probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
if (tlb_addr & TLB_WATCHPOINT) {
cpu_check_watchpoint(env_cpu(env), addr, size,
env_tlb(env)->d[mmu_idx].iotlb[index].attrs,
- BP_MEM_WRITE, retaddr);
+ wp_access, retaddr);
}
if (tlb_addr & (TLB_NOTDIRTY | TLB_MMIO)) {
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 420184571f..c2ee611427 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -188,18 +188,33 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,
g_assert_not_reached();
}
-void *probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
- uintptr_t retaddr)
+void *probe_access(CPUArchState *env, target_ulong addr, int size,
+ MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
{
CPUState *cpu = env_cpu(env);
CPUClass *cc;
+ int flags;
g_assert(-(addr | TARGET_PAGE_MASK) >= size);
+ g_assert(mmu_idx == MMU_USER_IDX);
- if (!guest_addr_valid(addr) ||
- page_check_range(addr, size, PAGE_WRITE) < 0) {
+ switch (access_type) {
+ case MMU_DATA_STORE:
+ flags = PAGE_WRITE;
+ break;
+ case MMU_DATA_LOAD:
+ flags = PAGE_READ;
+ break;
+ case MMU_INST_FETCH:
+ flags = PAGE_EXEC;
+ break;
+ default:
+ g_assert_not_reached();
+ }
+
+ if (!guest_addr_valid(addr) || page_check_range(addr, size, flags) < 0) {
cc = CPU_GET_CLASS(cpu);
- cc->tlb_fill(cpu, addr, size, MMU_DATA_STORE, MMU_USER_IDX, false,
+ cc->tlb_fill(cpu, addr, size, access_type, mmu_idx, false,
retaddr);
g_assert_not_reached();
}
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index a7893ed16b..3eaf39cd0d 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -310,8 +310,13 @@ static inline void tlb_flush_by_mmuidx_all_cpus_synced(CPUState *cpu,
{
}
#endif
-void *probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
- uintptr_t retaddr);
+void *probe_access(CPUArchState *env, target_ulong addr, int size,
+ MMUAccessType access_type, int mmu_idx, uintptr_t retaddr);
+static inline void *probe_write(CPUArchState *env, target_ulong addr, int size,
+ int mmu_idx, uintptr_t retaddr)
+{
+ return probe_access(env, addr, size, MMU_DATA_STORE, mmu_idx, retaddr);
+}
#define CODE_GEN_ALIGN 16 /* must be >= of the size of a icache line */
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/2] tcg: Make probe_write() return a pointer to the host page
2019-08-28 11:10 ` [Qemu-devel] [PATCH v1 1/2] tcg: Make probe_write() return a pointer to the host page David Hildenbrand
@ 2019-08-28 14:44 ` Richard Henderson
2019-08-28 20:34 ` David Hildenbrand
0 siblings, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2019-08-28 14:44 UTC (permalink / raw)
To: David Hildenbrand, qemu-devel
Cc: Thomas Huth, Cornelia Huck, Riku Voipio, qemu-s390x,
Paolo Bonzini, Richard Henderson
On 8/28/19 4:10 AM, David Hildenbrand wrote:
> + * If the access is permitted, returns the host address similar to
> + * tlb_vaddr_to_host(). Returns NULL in case direct access to the host page
> + * is not allowed or if the size is 0.
Maybe we can find some better language -- "not allowed" sounds like a
permissions check, and we longjmp out on permission check failures.
Perhaps "if the page requires i/o access"?
Why are you returning NULL for size 0? Just because the caller hasn't
committed to a size and thus we haven't checked watchpoints?
r~
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v1 2/2] tcg: Factor out probe_write() logic into probe_access()
2019-08-28 11:10 ` [Qemu-devel] [PATCH v1 2/2] tcg: Factor out probe_write() logic into probe_access() David Hildenbrand
@ 2019-08-28 14:52 ` Richard Henderson
2019-08-28 20:30 ` David Hildenbrand
0 siblings, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2019-08-28 14:52 UTC (permalink / raw)
To: David Hildenbrand, qemu-devel
Cc: Thomas Huth, Cornelia Huck, Riku Voipio, qemu-s390x,
Paolo Bonzini, Richard Henderson
On 8/28/19 4:10 AM, David Hildenbrand wrote:
> +void *probe_access(CPUArchState *env, target_ulong addr, int size,
> + MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
> {
> CPUState *cpu = env_cpu(env);
> CPUClass *cc;
> + int flags;
>
> g_assert(-(addr | TARGET_PAGE_MASK) >= size);
> + g_assert(mmu_idx == MMU_USER_IDX);
We ignore mmu_idx pretty much everywhere else.
Why bother now?
r~
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v1 2/2] tcg: Factor out probe_write() logic into probe_access()
2019-08-28 14:52 ` Richard Henderson
@ 2019-08-28 20:30 ` David Hildenbrand
0 siblings, 0 replies; 7+ messages in thread
From: David Hildenbrand @ 2019-08-28 20:30 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: Thomas Huth, Cornelia Huck, Riku Voipio, qemu-s390x,
Paolo Bonzini, Richard Henderson
On 28.08.19 16:52, Richard Henderson wrote:
> On 8/28/19 4:10 AM, David Hildenbrand wrote:
>> +void *probe_access(CPUArchState *env, target_ulong addr, int size,
>> + MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
>> {
>> CPUState *cpu = env_cpu(env);
>> CPUClass *cc;
>> + int flags;
>>
>> g_assert(-(addr | TARGET_PAGE_MASK) >= size);
>> + g_assert(mmu_idx == MMU_USER_IDX);
>
> We ignore mmu_idx pretty much everywhere else.
> Why bother now?
Also true, I can drop this and simply use MMU_USER_IDX below.
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/2] tcg: Make probe_write() return a pointer to the host page
2019-08-28 14:44 ` Richard Henderson
@ 2019-08-28 20:34 ` David Hildenbrand
0 siblings, 0 replies; 7+ messages in thread
From: David Hildenbrand @ 2019-08-28 20:34 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: Thomas Huth, Cornelia Huck, Riku Voipio, qemu-s390x,
Paolo Bonzini, Richard Henderson
On 28.08.19 16:44, Richard Henderson wrote:
> On 8/28/19 4:10 AM, David Hildenbrand wrote:
>> + * If the access is permitted, returns the host address similar to
>> + * tlb_vaddr_to_host(). Returns NULL in case direct access to the host page
>> + * is not allowed or if the size is 0.
>
> Maybe we can find some better language -- "not allowed" sounds like a
> permissions check, and we longjmp out on permission check failures.
>
> Perhaps "if the page requires i/o access"?
Yeah, and NOTDIRTY, emulated via i/o access. I will rephrase.
>
> Why are you returning NULL for size 0? Just because the caller hasn't
> committed to a size and thus we haven't checked watchpoints?
Yes, I think it's the right thing to do. Watchpoints are one part of the
story, the other part is TLB_RECHECK (a.k.a. TLB_INVALID_MASK or
"subpage protection"). Allowing access could lead to hidden BUGs.
I consider somebody wanting to get access to a host page with an access
size of zero an interface violation.
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-08-28 20:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-28 11:10 [Qemu-devel] [PATCH v1 0/2] tcg: Introduce probe_access() and return a host pointer David Hildenbrand
2019-08-28 11:10 ` [Qemu-devel] [PATCH v1 1/2] tcg: Make probe_write() return a pointer to the host page David Hildenbrand
2019-08-28 14:44 ` Richard Henderson
2019-08-28 20:34 ` David Hildenbrand
2019-08-28 11:10 ` [Qemu-devel] [PATCH v1 2/2] tcg: Factor out probe_write() logic into probe_access() David Hildenbrand
2019-08-28 14:52 ` Richard Henderson
2019-08-28 20:30 ` David Hildenbrand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).