[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bi
|
From: |
David Hildenbrand |
|
Subject: |
Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit |
|
Date: |
Thu, 11 Mar 2021 15:03:49 +0100 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 |
On 03.03.21 14:28, Thomas Huth wrote:
From: Richard Henderson <richard.henderson@linaro.org>
If the CCO bit is set, MVPG should not generate an exception but
report page translation faults via a CC code.
Create a new helper, access_prepare_nf, which can use probe_access_flags
in non-faulting mode, and then handle watchpoints.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
[thuth: Added logic to still inject protection exceptions]
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
v4: Add logic to inject protection exceptions if necessary
target/s390x/cpu.h | 3 ++
target/s390x/excp_helper.c | 3 ++
target/s390x/mem_helper.c | 93 ++++++++++++++++++++++++++++----------
3 files changed, 76 insertions(+), 23 deletions(-)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 60d434d5ed..825503c6c0 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -114,6 +114,9 @@ struct CPUS390XState {
uint64_t diag318_info;
+ uint64_t tlb_fill_tec; /* translation exception code during tlb_fill */
+ int tlb_fill_exc; /* exception number seen during tlb_fill */
+
/* Fields up to this point are cleared by a CPU reset */
struct {} end_reset_fields;
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index ce16af394b..c48cd6b46f 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -164,6 +164,9 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
tec = 0; /* unused */
}
+ env->tlb_fill_exc = excp;
+ env->tlb_fill_tec = tec;
+
if (!excp) {
qemu_log_mask(CPU_LOG_MMU,
"%s: set tlb %" PRIx64 " -> %" PRIx64 " (%x)\n",
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 25cfede806..cf741541d3 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -130,28 +130,62 @@ typedef struct S390Access {
int mmu_idx;
} S390Access;
+static bool access_prepare_nf(S390Access *access, CPUS390XState *env,
+ bool nofault, vaddr vaddr1, int size,
+ MMUAccessType access_type,
+ int mmu_idx, uintptr_t ra)
+{
+ void *haddr1, *haddr2 = NULL;
+ int size1, size2;
+ vaddr vaddr2 = 0;
+ int flags;
+
+ assert(size > 0 && size <= 4096);
+
+ size1 = MIN(size, -(vaddr1 | TARGET_PAGE_MASK)),
+ size2 = size - size1;
+
+ flags = probe_access_flags(env, vaddr1, access_type, mmu_idx,
+ nofault, &haddr1, ra);
+ if (unlikely(size2)) {
+ /* The access crosses page boundaries. */
+ vaddr2 = wrap_address(env, vaddr1 + size1);
+ flags |= probe_access_flags(env, vaddr2, access_type, mmu_idx,
+ nofault, &haddr2, ra);
+ }
+
+ if (unlikely(flags & TLB_INVALID_MASK)) {
+ return false;
+ }
+ if (unlikely(flags & TLB_WATCHPOINT)) {
+ /* S390 does not presently use transaction attributes. */
+ cpu_check_watchpoint(env_cpu(env), vaddr1, size,
+ MEMTXATTRS_UNSPECIFIED,
+ (access_type == MMU_DATA_STORE
+ ? BP_MEM_WRITE : BP_MEM_READ), ra);
+ }
+
+ *access = (S390Access) {
+ .vaddr1 = vaddr1,
+ .vaddr2 = vaddr2,
+ .haddr1 = haddr1,
+ .haddr2 = haddr2,
+ .size1 = size1,
+ .size2 = size2,
+ .mmu_idx = mmu_idx
+ };
+ return true;
+}
+
static S390Access access_prepare(CPUS390XState *env, vaddr vaddr, int size,
MMUAccessType access_type, int mmu_idx,
uintptr_t ra)
{
- S390Access access = {
- .vaddr1 = vaddr,
- .size1 = MIN(size, -(vaddr | TARGET_PAGE_MASK)),
- .mmu_idx = mmu_idx,
- };
-
- g_assert(size > 0 && size <= 4096);
- access.haddr1 = probe_access(env, access.vaddr1, access.size1, access_type,
- mmu_idx, ra);
-
- if (unlikely(access.size1 != size)) {
- /* The access crosses page boundaries. */
- access.vaddr2 = wrap_address(env, vaddr + access.size1);
- access.size2 = size - access.size1;
- access.haddr2 = probe_access(env, access.vaddr2, access.size2,
- access_type, mmu_idx, ra);
- }
- return access;
+ S390Access ret;
+ bool ok = access_prepare_nf(&ret, env, false, vaddr, size,
+ access_type, mmu_idx, ra);
+ assert(ok);
+ return ret;
}
/* Helper to handle memset on a single page. */
@@ -845,8 +879,10 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0,
uint64_t r1, uint64_t r2)
const int mmu_idx = cpu_mmu_index(env, false);
const bool f = extract64(r0, 11, 1);
const bool s = extract64(r0, 10, 1);
+ const bool cco = extract64(r0, 8, 1);
uintptr_t ra = GETPC();
S390Access srca, desta;
+ bool ok;
if ((f && s) || extract64(r0, 12, 4)) {
tcg_s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
@@ -858,13 +894,24 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0,
uint64_t r1, uint64_t r2)
/*
* TODO:
* - Access key handling
- * - CC-option with surpression of page-translation exceptions
* - Store r1/r2 register identifiers at real location 162
*/
- srca = access_prepare(env, r2, TARGET_PAGE_SIZE, MMU_DATA_LOAD, mmu_idx,
- ra);
- desta = access_prepare(env, r1, TARGET_PAGE_SIZE, MMU_DATA_STORE, mmu_idx,
- ra);
+ ok = access_prepare_nf(&srca, env, cco, r2, TARGET_PAGE_SIZE,
+ MMU_DATA_LOAD, mmu_idx, ra);
+ if (!ok) {
+ return 2;
+ }
+ ok = access_prepare_nf(&desta, env, cco, r1, TARGET_PAGE_SIZE,
+ MMU_DATA_STORE, mmu_idx, ra);
+ if (!ok) {
+ if (env->tlb_fill_exc == PGM_PROTECTION) {
+ stq_phys(env_cpu(env)->as,
+ env->psa + offsetof(LowCore, trans_exc_code),
+ env->tlb_fill_tec);
+ tcg_s390_program_interrupt(env, PGM_PROTECTION, ra);
+ }
+ return 1;
+ }
access_memmove(env, &desta, &srca, ra);
return 0; /* data moved */
}
As talked with Thomas off-list, there is no trusting on host==NULL
as well (see comment in struct S390Access). host==NULL simply
means we have to do individual ld/st.
The following on top should work. Not perfect, but seems to get
the job done.
From 056b3c9f2ffd43b10d8293e7143cf7af5d1d5022 Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@redhat.com>
Date: Thu, 11 Mar 2021 14:44:45 +0100
Subject: [PATCH] fixup
Signed-off-by: David Hildenbrand <david@redhat.com>
---
target/s390x/mem_helper.c | 48 ++++++++++++++++++++++++---------------
1 file changed, 30 insertions(+), 18 deletions(-)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index cf741541d3..0a9b15ea90 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -130,10 +130,15 @@ typedef struct S390Access {
int mmu_idx;
} S390Access;
-static bool access_prepare_nf(S390Access *access, CPUS390XState *env,
- bool nofault, vaddr vaddr1, int size,
- MMUAccessType access_type,
- int mmu_idx, uintptr_t ra)
+/*
+ * With nofault=1, return the generated PGM_ exception that would have
+ * been injected into the guest (tec stored in env->tlb_fill_tec);
+ * return 0 if no exception was detected.
+ */
+static int access_prepare_nf(S390Access *access, CPUS390XState *env,
+ bool nofault, vaddr vaddr1, int size,
+ MMUAccessType access_type,
+ int mmu_idx, uintptr_t ra)
{
void *haddr1, *haddr2 = NULL;
int size1, size2;
@@ -145,18 +150,24 @@ static bool access_prepare_nf(S390Access *access,
CPUS390XState *env,
size1 = MIN(size, -(vaddr1 | TARGET_PAGE_MASK)),
size2 = size - size1;
+ env->tlb_fill_exc = 0;
flags = probe_access_flags(env, vaddr1, access_type, mmu_idx,
nofault, &haddr1, ra);
+ if (env->tlb_fill_exc) {
+ /* We cannot rely on TLB_INVALID_MASK or haddr being NULL. */
+ return env->tlb_fill_exc;
+ }
if (unlikely(size2)) {
/* The access crosses page boundaries. */
vaddr2 = wrap_address(env, vaddr1 + size1);
flags |= probe_access_flags(env, vaddr2, access_type, mmu_idx,
nofault, &haddr2, ra);
+ if (env->tlb_fill_exc) {
+ /* We cannot rely on TLB_INVALID_MASK or haddr being NULL. */
+ return env->tlb_fill_exc;
+ }
}
- if (unlikely(flags & TLB_INVALID_MASK)) {
- return false;
- }
if (unlikely(flags & TLB_WATCHPOINT)) {
/* S390 does not presently use transaction attributes. */
cpu_check_watchpoint(env_cpu(env), vaddr1, size,
@@ -174,7 +185,7 @@ static bool access_prepare_nf(S390Access *access,
CPUS390XState *env,
.size2 = size2,
.mmu_idx = mmu_idx
};
- return true;
+ return 0;
}
static S390Access access_prepare(CPUS390XState *env, vaddr vaddr, int size,
@@ -182,9 +193,9 @@ static S390Access access_prepare(CPUS390XState *env, vaddr
vaddr, int size,
uintptr_t ra)
{
S390Access ret;
- bool ok = access_prepare_nf(&ret, env, false, vaddr, size,
+ int exc = access_prepare_nf(&ret, env, false, vaddr, size,
access_type, mmu_idx, ra);
- assert(ok);
+ assert(!exc);
return ret;
}
@@ -882,7 +893,7 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t r2)
const bool cco = extract64(r0, 8, 1);
uintptr_t ra = GETPC();
S390Access srca, desta;
- bool ok;
+ int exc;
if ((f && s) || extract64(r0, 12, 4)) {
tcg_s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
@@ -896,15 +907,16 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0,
uint64_t r1, uint64_t r2)
* - Access key handling
* - Store r1/r2 register identifiers at real location 162
*/
- ok = access_prepare_nf(&srca, env, cco, r2, TARGET_PAGE_SIZE,
- MMU_DATA_LOAD, mmu_idx, ra);
- if (!ok) {
+ exc = access_prepare_nf(&srca, env, cco, r2, TARGET_PAGE_SIZE,
+ MMU_DATA_LOAD, mmu_idx, ra);
+ if (exc) {
return 2;
}
- ok = access_prepare_nf(&desta, env, cco, r1, TARGET_PAGE_SIZE,
- MMU_DATA_STORE, mmu_idx, ra);
- if (!ok) {
- if (env->tlb_fill_exc == PGM_PROTECTION) {
+ exc = access_prepare_nf(&desta, env, cco, r1, TARGET_PAGE_SIZE,
+ MMU_DATA_STORE, mmu_idx, ra);
+ if (exc) {
+ fprintf(stderr, "Exception: %d\n", exc);
+ if (exc == PGM_PROTECTION) {
stq_phys(env_cpu(env)->as,
env->psa + offsetof(LowCore, trans_exc_code),
env->tlb_fill_tec);
--
2.29.2
--
Thanks,
David / dhildenb
- Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit, (continued)
- Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit, Richard Henderson, 2021/03/03
- Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit, David Hildenbrand, 2021/03/03
- Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit, Richard Henderson, 2021/03/03
- Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit, David Hildenbrand, 2021/03/03
- Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit, Richard Henderson, 2021/03/03
- Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit, David Hildenbrand, 2021/03/04
- Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit, Cornelia Huck, 2021/03/04
- Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit, Thomas Huth, 2021/03/09
- Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit, David Hildenbrand, 2021/03/10
- Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit, Cornelia Huck, 2021/03/11
Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit,
David Hildenbrand <=