* [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
@ 2021-03-03 13:28 Thomas Huth
2021-03-03 18:22 ` Richard Henderson
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Thomas Huth @ 2021-03-03 13:28 UTC (permalink / raw)
To: qemu-devel, David Hildenbrand
Cc: Claudio Imbrenda, qemu-s390x, Cornelia Huck, Richard Henderson,
Janosch Frank
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 */
}
--
2.27.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-03 13:28 [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit Thomas Huth
@ 2021-03-03 18:22 ` Richard Henderson
2021-03-03 19:39 ` David Hildenbrand
2021-03-11 14:03 ` David Hildenbrand
2 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2021-03-03 18:22 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, David Hildenbrand
Cc: Claudio Imbrenda, qemu-s390x, Cornelia Huck, Janosch Frank
On 3/3/21 5:28 AM, 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
Works for me. I was considering adding yet another cputlb interface to help
with this, but this is sufficiently clean that I don't think it's required.
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-03 13:28 [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit Thomas Huth
2021-03-03 18:22 ` Richard Henderson
@ 2021-03-03 19:39 ` David Hildenbrand
2021-03-03 21:05 ` Richard Henderson
2021-03-11 14:03 ` David Hildenbrand
2 siblings, 1 reply; 16+ messages in thread
From: David Hildenbrand @ 2021-03-03 19:39 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Claudio Imbrenda, qemu-s390x, Cornelia Huck, Richard Henderson,
Janosch Frank
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;
>
Should we start wrapping that stuff into #ifdef CONFIG_TCG ?
> + 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;
> +
Just what I had in mind.
> 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;
^ I recall PAGE_WRITE_INV handling where we immediately set
TLB_INVALID_MASK again on write access (to handle low-address protection
cleanly). I suspect that TLB_INVALID_MASK will be set in that case (I
could be wrong, though).
What certainly would work is checking for "haddr != NULL".
/* Don't rely on TLB_INVALID_MASK - see PAGE_WRITE_INV handling. */
if (unlikely(!haddr1)) {
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);
> + }
> +
[...]
> /* 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 */
> }
>
Apart from that, looks good to me.
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-03 19:39 ` David Hildenbrand
@ 2021-03-03 21:05 ` Richard Henderson
2021-03-03 21:11 ` David Hildenbrand
0 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2021-03-03 21:05 UTC (permalink / raw)
To: David Hildenbrand, Thomas Huth, qemu-devel
Cc: Claudio Imbrenda, qemu-s390x, Cornelia Huck, Janosch Frank
On 3/3/21 11:39 AM, David Hildenbrand wrote:
> Should we start wrapping that stuff into #ifdef CONFIG_TCG ?
>
>> + uint64_t tlb_fill_tec; /* translation exception code during tlb_fill */
>> + int tlb_fill_exc; /* exception number seen during tlb_fill */
Eh, probably not. At least not until we elide the softmmu tlb, which is
fantastically larger.
>> + if (unlikely(flags & TLB_INVALID_MASK)) {
>> + return false;
>
> ^ I recall PAGE_WRITE_INV handling where we immediately set TLB_INVALID_MASK
> again on write access (to handle low-address protection cleanly). I suspect
> that TLB_INVALID_MASK will be set in that case (I could be wrong, though).
>
> What certainly would work is checking for "haddr != NULL".
>
> /* Don't rely on TLB_INVALID_MASK - see PAGE_WRITE_INV handling. */
> if (unlikely(!haddr1)) {
> return false;
> }
Ah, right. I consider TLB_INVALID_MASK being set in the return from
probe_access_flags for PAGE_WRITE_INV a bug. I'm not sure how to fix that
right away.
Well, !haddr1 is also false for TLB_MMIO, so you'd need to check for that as well.
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-03 21:05 ` Richard Henderson
@ 2021-03-03 21:11 ` David Hildenbrand
2021-03-03 21:19 ` Richard Henderson
0 siblings, 1 reply; 16+ messages in thread
From: David Hildenbrand @ 2021-03-03 21:11 UTC (permalink / raw)
To: Richard Henderson
Cc: Thomas Huth, Janosch Frank, David Hildenbrand, Cornelia Huck,
qemu-devel, qemu-s390x, Claudio Imbrenda
> Am 03.03.2021 um 22:05 schrieb Richard Henderson <richard.henderson@linaro.org>:
>
> On 3/3/21 11:39 AM, David Hildenbrand wrote:
>> Should we start wrapping that stuff into #ifdef CONFIG_TCG ?
>>> + uint64_t tlb_fill_tec; /* translation exception code during tlb_fill */
>>> + int tlb_fill_exc; /* exception number seen during tlb_fill */
>
> Eh, probably not. At least not until we elide the softmmu tlb, which is fantastically larger.
>
>>> + if (unlikely(flags & TLB_INVALID_MASK)) {
>>> + return false;
>> ^ I recall PAGE_WRITE_INV handling where we immediately set TLB_INVALID_MASK again on write access (to handle low-address protection cleanly). I suspect that TLB_INVALID_MASK will be set in that case (I could be wrong, though).
>> What certainly would work is checking for "haddr != NULL".
>> /* Don't rely on TLB_INVALID_MASK - see PAGE_WRITE_INV handling. */
>> if (unlikely(!haddr1)) {
>> return false;
>> }
>
> Ah, right. I consider TLB_INVALID_MASK being set in the return from probe_access_flags for PAGE_WRITE_INV a bug. I'm not sure how to fix that right away.
>
> Well, !haddr1 is also false for TLB_MMIO, so you'd need to check for that as well.
MMIO on s390x? :)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-03 21:11 ` David Hildenbrand
@ 2021-03-03 21:19 ` Richard Henderson
2021-03-03 21:22 ` David Hildenbrand
0 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2021-03-03 21:19 UTC (permalink / raw)
To: David Hildenbrand
Cc: Thomas Huth, Janosch Frank, Cornelia Huck, qemu-devel,
qemu-s390x, Claudio Imbrenda
On 3/3/21 1:11 PM, David Hildenbrand wrote:
> MMIO on s390x? :)
hw/s390x/s390-pci-bus.c, memory_region_init_io*().
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-03 21:19 ` Richard Henderson
@ 2021-03-03 21:22 ` David Hildenbrand
2021-03-03 21:36 ` Richard Henderson
0 siblings, 1 reply; 16+ messages in thread
From: David Hildenbrand @ 2021-03-03 21:22 UTC (permalink / raw)
To: Richard Henderson
Cc: Thomas Huth, Janosch Frank, David Hildenbrand, Cornelia Huck,
qemu-devel, qemu-s390x, Claudio Imbrenda
> Am 03.03.2021 um 22:19 schrieb Richard Henderson <richard.henderson@linaro.org>:
>
> On 3/3/21 1:11 PM, David Hildenbrand wrote:
>> MMIO on s390x? :)
>
> hw/s390x/s390-pci-bus.c, memory_region_init_io*().
>
... part of system address space where a CPU could stumble over it?
> r~
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-03 21:22 ` David Hildenbrand
@ 2021-03-03 21:36 ` Richard Henderson
2021-03-04 8:10 ` David Hildenbrand
0 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2021-03-03 21:36 UTC (permalink / raw)
To: David Hildenbrand
Cc: Thomas Huth, Janosch Frank, Cornelia Huck, qemu-devel,
qemu-s390x, Claudio Imbrenda
On 3/3/21 1:22 PM, David Hildenbrand wrote:
>
>> Am 03.03.2021 um 22:19 schrieb Richard Henderson <richard.henderson@linaro.org>:
>>
>> On 3/3/21 1:11 PM, David Hildenbrand wrote:
>>> MMIO on s390x? :)
>>
>> hw/s390x/s390-pci-bus.c, memory_region_init_io*().
>>
>
> ... part of system address space where a CPU could stumble over it?
Impossible to tell within 3 layers of object wrappers. :-(
I suppose I have no idea how "pci" was hacked onto s390x.
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-03 21:36 ` Richard Henderson
@ 2021-03-04 8:10 ` David Hildenbrand
2021-03-04 8:17 ` Cornelia Huck
2021-03-09 21:05 ` Thomas Huth
0 siblings, 2 replies; 16+ messages in thread
From: David Hildenbrand @ 2021-03-04 8:10 UTC (permalink / raw)
To: Richard Henderson
Cc: Thomas Huth, Janosch Frank, Cornelia Huck, qemu-devel,
qemu-s390x, Claudio Imbrenda
On 03.03.21 22:36, Richard Henderson wrote:
> On 3/3/21 1:22 PM, David Hildenbrand wrote:
>>
>>> Am 03.03.2021 um 22:19 schrieb Richard Henderson <richard.henderson@linaro.org>:
>>>
>>> On 3/3/21 1:11 PM, David Hildenbrand wrote:
>>>> MMIO on s390x? :)
>>>
>>> hw/s390x/s390-pci-bus.c, memory_region_init_io*().
>>>
>>
>> ... part of system address space where a CPU could stumble over it?
>
> Impossible to tell within 3 layers of object wrappers. :-(
> I suppose I have no idea how "pci" was hacked onto s390x.
You've used the right words to describe "pci" (!) on s390x.
IIRC, there is no MMIO: configuration space accesses etc. are performed
using special access instructions - which will "emulate" the MMIO access
performed on other archs via simple read/write instructions.
Ordinary instructions (e.g., mvpg) that operate on the system address
space should never stumble over MMIO regions - because that concept does
not exist on s390x.
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-04 8:10 ` David Hildenbrand
@ 2021-03-04 8:17 ` Cornelia Huck
2021-03-09 21:05 ` Thomas Huth
1 sibling, 0 replies; 16+ messages in thread
From: Cornelia Huck @ 2021-03-04 8:17 UTC (permalink / raw)
To: David Hildenbrand
Cc: Thomas Huth, Janosch Frank, Richard Henderson, qemu-devel,
qemu-s390x, Claudio Imbrenda
On Thu, 4 Mar 2021 09:10:44 +0100
David Hildenbrand <david@redhat.com> wrote:
> On 03.03.21 22:36, Richard Henderson wrote:
> > On 3/3/21 1:22 PM, David Hildenbrand wrote:
> >>
> >>> Am 03.03.2021 um 22:19 schrieb Richard Henderson <richard.henderson@linaro.org>:
> >>>
> >>> On 3/3/21 1:11 PM, David Hildenbrand wrote:
> >>>> MMIO on s390x? :)
> >>>
> >>> hw/s390x/s390-pci-bus.c, memory_region_init_io*().
> >>>
> >>
> >> ... part of system address space where a CPU could stumble over it?
> >
> > Impossible to tell within 3 layers of object wrappers. :-(
> > I suppose I have no idea how "pci" was hacked onto s390x.
>
> You've used the right words to describe "pci" (!) on s390x.
>
> IIRC, there is no MMIO: configuration space accesses etc. are performed
> using special access instructions - which will "emulate" the MMIO access
> performed on other archs via simple read/write instructions.
Yes, it's all specialized instructions. (QEMU emulates one set, there's
a newer one supported by the Linux kernel that is a bit more mmio-like
in semantics, but I don't know the details.)
>
> Ordinary instructions (e.g., mvpg) that operate on the system address
> space should never stumble over MMIO regions - because that concept does
> not exist on s390x.
>
That's my understanding as well.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-04 8:10 ` David Hildenbrand
2021-03-04 8:17 ` Cornelia Huck
@ 2021-03-09 21:05 ` Thomas Huth
2021-03-10 20:49 ` David Hildenbrand
1 sibling, 1 reply; 16+ messages in thread
From: Thomas Huth @ 2021-03-09 21:05 UTC (permalink / raw)
To: David Hildenbrand, Richard Henderson
Cc: Claudio Imbrenda, qemu-s390x, Cornelia Huck, Janosch Frank, qemu-devel
On 04/03/2021 09.10, David Hildenbrand wrote:
> On 03.03.21 22:36, Richard Henderson wrote:
>> On 3/3/21 1:22 PM, David Hildenbrand wrote:
>>>
>>>> Am 03.03.2021 um 22:19 schrieb Richard Henderson
>>>> <richard.henderson@linaro.org>:
>>>>
>>>> On 3/3/21 1:11 PM, David Hildenbrand wrote:
>>>>> MMIO on s390x? :)
>>>>
>>>> hw/s390x/s390-pci-bus.c, memory_region_init_io*().
>>>>
>>>
>>> ... part of system address space where a CPU could stumble over it?
>>
>> Impossible to tell within 3 layers of object wrappers. :-(
>> I suppose I have no idea how "pci" was hacked onto s390x.
>
> You've used the right words to describe "pci" (!) on s390x.
>
> IIRC, there is no MMIO: configuration space accesses etc. are performed
> using special access instructions - which will "emulate" the MMIO access
> performed on other archs via simple read/write instructions.
>
> Ordinary instructions (e.g., mvpg) that operate on the system address space
> should never stumble over MMIO regions - because that concept does not exist
> on s390x.
Sorry, guys, you've lost me here ... is there now something left to do for
this patch or is it good as it is?
Thomas
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-09 21:05 ` Thomas Huth
@ 2021-03-10 20:49 ` David Hildenbrand
2021-03-11 10:21 ` Cornelia Huck
0 siblings, 1 reply; 16+ messages in thread
From: David Hildenbrand @ 2021-03-10 20:49 UTC (permalink / raw)
To: Thomas Huth
Cc: Janosch Frank, David Hildenbrand, Cornelia Huck,
Richard Henderson, qemu-devel, qemu-s390x, Claudio Imbrenda
> Am 09.03.2021 um 22:05 schrieb Thomas Huth <thuth@redhat.com>:
>
> On 04/03/2021 09.10, David Hildenbrand wrote:
>>> On 03.03.21 22:36, Richard Henderson wrote:
>>> On 3/3/21 1:22 PM, David Hildenbrand wrote:
>>>>
>>>>> Am 03.03.2021 um 22:19 schrieb Richard Henderson <richard.henderson@linaro.org>:
>>>>>
>>>>> On 3/3/21 1:11 PM, David Hildenbrand wrote:
>>>>>> MMIO on s390x? :)
>>>>>
>>>>> hw/s390x/s390-pci-bus.c, memory_region_init_io*().
>>>>>
>>>>
>>>> ... part of system address space where a CPU could stumble over it?
>>>
>>> Impossible to tell within 3 layers of object wrappers. :-(
>>> I suppose I have no idea how "pci" was hacked onto s390x.
>> You've used the right words to describe "pci" (!) on s390x.
>> IIRC, there is no MMIO: configuration space accesses etc. are performed using special access instructions - which will "emulate" the MMIO access performed on other archs via simple read/write instructions.
>> Ordinary instructions (e.g., mvpg) that operate on the system address space should never stumble over MMIO regions - because that concept does not exist on s390x.
>
> Sorry, guys, you've lost me here ... is there now something left to do for this patch or is it good as it is?
I think that check for invalid TLB should be replaced by a check against host == NULL.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-10 20:49 ` David Hildenbrand
@ 2021-03-11 10:21 ` Cornelia Huck
0 siblings, 0 replies; 16+ messages in thread
From: Cornelia Huck @ 2021-03-11 10:21 UTC (permalink / raw)
To: David Hildenbrand
Cc: Thomas Huth, Janosch Frank, Richard Henderson, qemu-devel,
qemu-s390x, Claudio Imbrenda
On Wed, 10 Mar 2021 21:49:22 +0100
David Hildenbrand <david@redhat.com> wrote:
> > Am 09.03.2021 um 22:05 schrieb Thomas Huth <thuth@redhat.com>:
> >
> > On 04/03/2021 09.10, David Hildenbrand wrote:
> >>> On 03.03.21 22:36, Richard Henderson wrote:
> >>> On 3/3/21 1:22 PM, David Hildenbrand wrote:
> >>>>
> >>>>> Am 03.03.2021 um 22:19 schrieb Richard Henderson <richard.henderson@linaro.org>:
> >>>>>
> >>>>> On 3/3/21 1:11 PM, David Hildenbrand wrote:
> >>>>>> MMIO on s390x? :)
> >>>>>
> >>>>> hw/s390x/s390-pci-bus.c, memory_region_init_io*().
> >>>>>
> >>>>
> >>>> ... part of system address space where a CPU could stumble over it?
> >>>
> >>> Impossible to tell within 3 layers of object wrappers. :-(
> >>> I suppose I have no idea how "pci" was hacked onto s390x.
> >> You've used the right words to describe "pci" (!) on s390x.
> >> IIRC, there is no MMIO: configuration space accesses etc. are performed using special access instructions - which will "emulate" the MMIO access performed on other archs via simple read/write instructions.
> >> Ordinary instructions (e.g., mvpg) that operate on the system address space should never stumble over MMIO regions - because that concept does not exist on s390x.
> >
> > Sorry, guys, you've lost me here ... is there now something left to do for this patch or is it good as it is?
>
> I think that check for invalid TLB should be replaced by a check against host == NULL.
>
Just a reminder that softfreeze is on Tuesday next week, and I'd like
to send a pull request by Monday. So a v5 should arrive soon to make it
:)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-03 13:28 [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit Thomas Huth
2021-03-03 18:22 ` Richard Henderson
2021-03-03 19:39 ` David Hildenbrand
@ 2021-03-11 14:03 ` David Hildenbrand
2021-03-11 15:58 ` Richard Henderson
2 siblings, 1 reply; 16+ messages in thread
From: David Hildenbrand @ 2021-03-11 14:03 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Claudio Imbrenda, qemu-s390x, Cornelia Huck, Richard Henderson,
Janosch Frank
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-11 14:03 ` David Hildenbrand
@ 2021-03-11 15:58 ` Richard Henderson
2021-03-11 16:00 ` David Hildenbrand
0 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2021-03-11 15:58 UTC (permalink / raw)
To: David Hildenbrand, Thomas Huth, qemu-devel
Cc: Claudio Imbrenda, qemu-s390x, Cornelia Huck, Janosch Frank
On 3/11/21 8:03 AM, David Hildenbrand wrote:
> 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.
I think that comment is stale with the use of probe_access instead of
tlb_vaddr_to_host -- TLB_DIRTY is in fact handled now.
> + 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;
But this is pretty clean, and definitely works.
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit
2021-03-11 15:58 ` Richard Henderson
@ 2021-03-11 16:00 ` David Hildenbrand
0 siblings, 0 replies; 16+ messages in thread
From: David Hildenbrand @ 2021-03-11 16:00 UTC (permalink / raw)
To: Richard Henderson, Thomas Huth, qemu-devel
Cc: Claudio Imbrenda, qemu-s390x, Cornelia Huck, Janosch Frank
On 11.03.21 16:58, Richard Henderson wrote:
> On 3/11/21 8:03 AM, David Hildenbrand wrote:
>> 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.
>
> I think that comment is stale with the use of probe_access instead of
> tlb_vaddr_to_host -- TLB_DIRTY is in fact handled now.
Yes, it might be worth exploring in which cases we will still have
issues and updating the comment. LAP is certainly one.
>> + 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;
>
> But this is pretty clean, and definitely works.
Except that I need to special case CONFIG_USER_ONLY .... I'll send a v5
later, then we can discuss when looking at the full diff (including an
addon patch to handle r1/r2).
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-03-11 16:23 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-03 13:28 [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit Thomas Huth
2021-03-03 18:22 ` Richard Henderson
2021-03-03 19:39 ` David Hildenbrand
2021-03-03 21:05 ` Richard Henderson
2021-03-03 21:11 ` David Hildenbrand
2021-03-03 21:19 ` Richard Henderson
2021-03-03 21:22 ` David Hildenbrand
2021-03-03 21:36 ` Richard Henderson
2021-03-04 8:10 ` David Hildenbrand
2021-03-04 8:17 ` Cornelia Huck
2021-03-09 21:05 ` Thomas Huth
2021-03-10 20:49 ` David Hildenbrand
2021-03-11 10:21 ` Cornelia Huck
2021-03-11 14:03 ` David Hildenbrand
2021-03-11 15:58 ` Richard Henderson
2021-03-11 16:00 ` David Hildenbrand
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.