target/s390x/cpu.h | 14 ++++++++++++++ target/s390x/excp_helper.c | 14 -------------- target/s390x/mem_helper.c | 23 ++++++++++++++++++++++- 3 files changed, 36 insertions(+), 15 deletions(-)
If the CCO bit is set, MVPG should not generate an exception
but report page translation faults via a CC code, so we have
to check the translation in this case before calling the
access_prepare() function.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
This patch is required to get Claudio's new kvm-unit-tests patches
working with TCG: https://www.spinics.net/lists/kvm/msg236784.html
target/s390x/cpu.h | 14 ++++++++++++++
target/s390x/excp_helper.c | 14 --------------
target/s390x/mem_helper.c | 23 ++++++++++++++++++++++-
3 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 60d434d5ed..731e2c6452 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -366,6 +366,20 @@ static inline int cpu_mmu_index(CPUS390XState *env, bool ifetch)
#endif
}
+static inline uint64_t cpu_mmu_idx_to_asc(int mmu_idx)
+{
+ switch (mmu_idx) {
+ case MMU_PRIMARY_IDX:
+ return PSW_ASC_PRIMARY;
+ case MMU_SECONDARY_IDX:
+ return PSW_ASC_SECONDARY;
+ case MMU_HOME_IDX:
+ return PSW_ASC_HOME;
+ default:
+ abort();
+ }
+}
+
static inline void cpu_get_tb_cpu_state(CPUS390XState* env, target_ulong *pc,
target_ulong *cs_base, uint32_t *flags)
{
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index ce16af394b..44bff27f8f 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -105,20 +105,6 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
#else /* !CONFIG_USER_ONLY */
-static inline uint64_t cpu_mmu_idx_to_asc(int mmu_idx)
-{
- switch (mmu_idx) {
- case MMU_PRIMARY_IDX:
- return PSW_ASC_PRIMARY;
- case MMU_SECONDARY_IDX:
- return PSW_ASC_SECONDARY;
- case MMU_HOME_IDX:
- return PSW_ASC_HOME;
- default:
- abort();
- }
-}
-
bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
MMUAccessType access_type, int mmu_idx,
bool probe, uintptr_t retaddr)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 25cfede806..c7037adf2c 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -855,10 +855,31 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t r2)
r1 = wrap_address(env, r1 & TARGET_PAGE_MASK);
r2 = wrap_address(env, r2 & TARGET_PAGE_MASK);
+ /*
+ * If the condition-code-option (CCO) bit is set and DAT is enabled,
+ * we have to check for page table translation faults first:
+ */
+#ifndef CONFIG_USER_ONLY
+ if (extract64(r0, 8, 1) && mmu_idx != MMU_REAL_IDX) {
+ uint64_t asc = cpu_mmu_idx_to_asc(mmu_idx);
+ uint64_t raddr, tec;
+ int flags, exc;
+
+ exc = mmu_translate(env, r2, MMU_DATA_LOAD, asc, &raddr, &flags, &tec);
+ if (exc) {
+ return 2;
+ }
+
+ exc = mmu_translate(env, r1, MMU_DATA_STORE, asc, &raddr, &flags, &tec);
+ if (exc && exc != PGM_PROTECTION) {
+ return 1;
+ }
+ }
+#endif
+
/*
* 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,
--
2.27.0
On 02.03.21 20:12, Thomas Huth wrote: > If the CCO bit is set, MVPG should not generate an exception > but report page translation faults via a CC code, so we have > to check the translation in this case before calling the > access_prepare() function. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > This patch is required to get Claudio's new kvm-unit-tests patches > working with TCG: https://www.spinics.net/lists/kvm/msg236784.html > > target/s390x/cpu.h | 14 ++++++++++++++ > target/s390x/excp_helper.c | 14 -------------- > target/s390x/mem_helper.c | 23 ++++++++++++++++++++++- > 3 files changed, 36 insertions(+), 15 deletions(-) > > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h > index 60d434d5ed..731e2c6452 100644 > --- a/target/s390x/cpu.h > +++ b/target/s390x/cpu.h > @@ -366,6 +366,20 @@ static inline int cpu_mmu_index(CPUS390XState *env, bool ifetch) > #endif > } > > +static inline uint64_t cpu_mmu_idx_to_asc(int mmu_idx) > +{ > + switch (mmu_idx) { > + case MMU_PRIMARY_IDX: > + return PSW_ASC_PRIMARY; > + case MMU_SECONDARY_IDX: > + return PSW_ASC_SECONDARY; > + case MMU_HOME_IDX: > + return PSW_ASC_HOME; > + default: > + abort(); > + } > +} > + > static inline void cpu_get_tb_cpu_state(CPUS390XState* env, target_ulong *pc, > target_ulong *cs_base, uint32_t *flags) > { > diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c > index ce16af394b..44bff27f8f 100644 > --- a/target/s390x/excp_helper.c > +++ b/target/s390x/excp_helper.c > @@ -105,20 +105,6 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size, > > #else /* !CONFIG_USER_ONLY */ > > -static inline uint64_t cpu_mmu_idx_to_asc(int mmu_idx) > -{ > - switch (mmu_idx) { > - case MMU_PRIMARY_IDX: > - return PSW_ASC_PRIMARY; > - case MMU_SECONDARY_IDX: > - return PSW_ASC_SECONDARY; > - case MMU_HOME_IDX: > - return PSW_ASC_HOME; > - default: > - abort(); > - } > -} > - > bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size, > MMUAccessType access_type, int mmu_idx, > bool probe, uintptr_t retaddr) > diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c > index 25cfede806..c7037adf2c 100644 > --- a/target/s390x/mem_helper.c > +++ b/target/s390x/mem_helper.c > @@ -855,10 +855,31 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t r2) > r1 = wrap_address(env, r1 & TARGET_PAGE_MASK); > r2 = wrap_address(env, r2 & TARGET_PAGE_MASK); > > + /* > + * If the condition-code-option (CCO) bit is set and DAT is enabled, > + * we have to check for page table translation faults first: > + */ > +#ifndef CONFIG_USER_ONLY > + if (extract64(r0, 8, 1) && mmu_idx != MMU_REAL_IDX) { > + uint64_t asc = cpu_mmu_idx_to_asc(mmu_idx); > + uint64_t raddr, tec; > + int flags, exc; > + > + exc = mmu_translate(env, r2, MMU_DATA_LOAD, asc, &raddr, &flags, &tec); > + if (exc) { > + return 2; > + } > + > + exc = mmu_translate(env, r1, MMU_DATA_STORE, asc, &raddr, &flags, &tec); > + if (exc && exc != PGM_PROTECTION) { > + return 1; > + } > + } > +#endif > + This way you always need two additional translations and don't even check if we have something in the TLB. While this works, it's quite inefficient. Using probe_access_flags() we can actually lookup the tlb/fill the tlb but get an error instead of a fault. We could e.g., extent probe_access() to allow specifying whether we want a fault or not. If we then store the last exception during tlb_fill in a fixed location, we could obtain that information. > /* > * 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, > -- Thanks, David / dhildenb
On 3/2/21 11:25 AM, David Hildenbrand wrote: > On 02.03.21 20:12, Thomas Huth wrote: >> If the CCO bit is set, MVPG should not generate an exception >> but report page translation faults via a CC code, so we have >> to check the translation in this case before calling the >> access_prepare() function. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> This patch is required to get Claudio's new kvm-unit-tests patches >> working with TCG: https://www.spinics.net/lists/kvm/msg236784.html >> >> target/s390x/cpu.h | 14 ++++++++++++++ >> target/s390x/excp_helper.c | 14 -------------- >> target/s390x/mem_helper.c | 23 ++++++++++++++++++++++- >> 3 files changed, 36 insertions(+), 15 deletions(-) >> >> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h >> index 60d434d5ed..731e2c6452 100644 >> --- a/target/s390x/cpu.h >> +++ b/target/s390x/cpu.h >> @@ -366,6 +366,20 @@ static inline int cpu_mmu_index(CPUS390XState *env, bool >> ifetch) >> #endif >> } >> +static inline uint64_t cpu_mmu_idx_to_asc(int mmu_idx) >> +{ >> + switch (mmu_idx) { >> + case MMU_PRIMARY_IDX: >> + return PSW_ASC_PRIMARY; >> + case MMU_SECONDARY_IDX: >> + return PSW_ASC_SECONDARY; >> + case MMU_HOME_IDX: >> + return PSW_ASC_HOME; >> + default: >> + abort(); >> + } >> +} >> + >> static inline void cpu_get_tb_cpu_state(CPUS390XState* env, target_ulong *pc, >> target_ulong *cs_base, uint32_t >> *flags) >> { >> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c >> index ce16af394b..44bff27f8f 100644 >> --- a/target/s390x/excp_helper.c >> +++ b/target/s390x/excp_helper.c >> @@ -105,20 +105,6 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int >> size, >> #else /* !CONFIG_USER_ONLY */ >> -static inline uint64_t cpu_mmu_idx_to_asc(int mmu_idx) >> -{ >> - switch (mmu_idx) { >> - case MMU_PRIMARY_IDX: >> - return PSW_ASC_PRIMARY; >> - case MMU_SECONDARY_IDX: >> - return PSW_ASC_SECONDARY; >> - case MMU_HOME_IDX: >> - return PSW_ASC_HOME; >> - default: >> - abort(); >> - } >> -} >> - >> bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size, >> MMUAccessType access_type, int mmu_idx, >> bool probe, uintptr_t retaddr) >> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c >> index 25cfede806..c7037adf2c 100644 >> --- a/target/s390x/mem_helper.c >> +++ b/target/s390x/mem_helper.c >> @@ -855,10 +855,31 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, >> uint64_t r1, uint64_t r2) >> r1 = wrap_address(env, r1 & TARGET_PAGE_MASK); >> r2 = wrap_address(env, r2 & TARGET_PAGE_MASK); >> + /* >> + * If the condition-code-option (CCO) bit is set and DAT is enabled, >> + * we have to check for page table translation faults first: >> + */ >> +#ifndef CONFIG_USER_ONLY >> + if (extract64(r0, 8, 1) && mmu_idx != MMU_REAL_IDX) { >> + uint64_t asc = cpu_mmu_idx_to_asc(mmu_idx); >> + uint64_t raddr, tec; >> + int flags, exc; >> + >> + exc = mmu_translate(env, r2, MMU_DATA_LOAD, asc, &raddr, &flags, &tec); >> + if (exc) { >> + return 2; >> + } >> + >> + exc = mmu_translate(env, r1, MMU_DATA_STORE, asc, &raddr, &flags, >> &tec); >> + if (exc && exc != PGM_PROTECTION) { >> + return 1; >> + } >> + } >> +#endif >> + > > This way you always need two additional translations and don't even check if we > have something in the TLB. While this works, it's quite inefficient. > > Using probe_access_flags() we can actually lookup the tlb/fill the tlb but get > an error instead of a fault. We could e.g., extent probe_access() to allow > specifying whether we want a fault or not. I think probe_access_flags() will do all that you need; no further extension to probe_access() required. I presume you meant access_prepare() is what you meant to extend? r~
> Am 02.03.2021 um 22:46 schrieb Richard Henderson <richard.henderson@linaro.org>: > > On 3/2/21 11:25 AM, David Hildenbrand wrote: >>> On 02.03.21 20:12, Thomas Huth wrote: >>> If the CCO bit is set, MVPG should not generate an exception >>> but report page translation faults via a CC code, so we have >>> to check the translation in this case before calling the >>> access_prepare() function. >>> >>> Signed-off-by: Thomas Huth <thuth@redhat.com> >>> --- >>> This patch is required to get Claudio's new kvm-unit-tests patches >>> working with TCG: https://www.spinics.net/lists/kvm/msg236784.html >>> >>> target/s390x/cpu.h | 14 ++++++++++++++ >>> target/s390x/excp_helper.c | 14 -------------- >>> target/s390x/mem_helper.c | 23 ++++++++++++++++++++++- >>> 3 files changed, 36 insertions(+), 15 deletions(-) >>> >>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h >>> index 60d434d5ed..731e2c6452 100644 >>> --- a/target/s390x/cpu.h >>> +++ b/target/s390x/cpu.h >>> @@ -366,6 +366,20 @@ static inline int cpu_mmu_index(CPUS390XState *env, bool ifetch) >>> #endif >>> } >>> +static inline uint64_t cpu_mmu_idx_to_asc(int mmu_idx) >>> +{ >>> + switch (mmu_idx) { >>> + case MMU_PRIMARY_IDX: >>> + return PSW_ASC_PRIMARY; >>> + case MMU_SECONDARY_IDX: >>> + return PSW_ASC_SECONDARY; >>> + case MMU_HOME_IDX: >>> + return PSW_ASC_HOME; >>> + default: >>> + abort(); >>> + } >>> +} >>> + >>> static inline void cpu_get_tb_cpu_state(CPUS390XState* env, target_ulong *pc, >>> target_ulong *cs_base, uint32_t *flags) >>> { >>> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c >>> index ce16af394b..44bff27f8f 100644 >>> --- a/target/s390x/excp_helper.c >>> +++ b/target/s390x/excp_helper.c >>> @@ -105,20 +105,6 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size, >>> #else /* !CONFIG_USER_ONLY */ >>> -static inline uint64_t cpu_mmu_idx_to_asc(int mmu_idx) >>> -{ >>> - switch (mmu_idx) { >>> - case MMU_PRIMARY_IDX: >>> - return PSW_ASC_PRIMARY; >>> - case MMU_SECONDARY_IDX: >>> - return PSW_ASC_SECONDARY; >>> - case MMU_HOME_IDX: >>> - return PSW_ASC_HOME; >>> - default: >>> - abort(); >>> - } >>> -} >>> - >>> bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size, >>> MMUAccessType access_type, int mmu_idx, >>> bool probe, uintptr_t retaddr) >>> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c >>> index 25cfede806..c7037adf2c 100644 >>> --- a/target/s390x/mem_helper.c >>> +++ b/target/s390x/mem_helper.c >>> @@ -855,10 +855,31 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t r2) >>> r1 = wrap_address(env, r1 & TARGET_PAGE_MASK); >>> r2 = wrap_address(env, r2 & TARGET_PAGE_MASK); >>> + /* >>> + * If the condition-code-option (CCO) bit is set and DAT is enabled, >>> + * we have to check for page table translation faults first: >>> + */ >>> +#ifndef CONFIG_USER_ONLY >>> + if (extract64(r0, 8, 1) && mmu_idx != MMU_REAL_IDX) { >>> + uint64_t asc = cpu_mmu_idx_to_asc(mmu_idx); >>> + uint64_t raddr, tec; >>> + int flags, exc; >>> + >>> + exc = mmu_translate(env, r2, MMU_DATA_LOAD, asc, &raddr, &flags, &tec); >>> + if (exc) { >>> + return 2; >>> + } >>> + >>> + exc = mmu_translate(env, r1, MMU_DATA_STORE, asc, &raddr, &flags, &tec); >>> + if (exc && exc != PGM_PROTECTION) { >>> + return 1; >>> + } >>> + } >>> +#endif >>> + >> This way you always need two additional translations and don't even check if we have something in the TLB. While this works, it's quite inefficient. >> Using probe_access_flags() we can actually lookup the tlb/fill the tlb but get an error instead of a fault. We could e.g., extent probe_access() to allow specifying whether we want a fault or not. > > I think probe_access_flags() will do all that you need; no further extension to probe_access() required. I presume you meant access_prepare() is what you meant to extend? I was worrying about watchpoint handling etc. as done in probe_access(). But I think what you mean is we can simply do two probe_access_flags() to catch these special pgm interrupt, followed by existing access_prepare(). That should work I guess.
© 2016 - 2024 Red Hat, Inc.