* [PATCH] target/s390x: Implement the MVPG condition-code-option bit
@ 2021-03-02 19:12 Thomas Huth
2021-03-02 19:25 ` David Hildenbrand
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Huth @ 2021-03-02 19:12 UTC (permalink / raw)
To: qemu-devel, David Hildenbrand
Cc: Claudio Imbrenda, qemu-s390x, Cornelia Huck, Richard Henderson,
Janosch Frank
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] target/s390x: Implement the MVPG condition-code-option bit
2021-03-02 19:12 [PATCH] target/s390x: Implement the MVPG condition-code-option bit Thomas Huth
@ 2021-03-02 19:25 ` David Hildenbrand
2021-03-02 21:46 ` Richard Henderson
0 siblings, 1 reply; 4+ messages in thread
From: David Hildenbrand @ 2021-03-02 19:25 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Claudio Imbrenda, qemu-s390x, Cornelia Huck, Richard Henderson,
Janosch Frank
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] target/s390x: Implement the MVPG condition-code-option bit
2021-03-02 19:25 ` David Hildenbrand
@ 2021-03-02 21:46 ` Richard Henderson
2021-03-02 22:09 ` David Hildenbrand
0 siblings, 1 reply; 4+ messages in thread
From: Richard Henderson @ 2021-03-02 21:46 UTC (permalink / raw)
To: David Hildenbrand, Thomas Huth, qemu-devel
Cc: Claudio Imbrenda, qemu-s390x, Cornelia Huck, Janosch Frank
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~
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] target/s390x: Implement the MVPG condition-code-option bit
2021-03-02 21:46 ` Richard Henderson
@ 2021-03-02 22:09 ` David Hildenbrand
0 siblings, 0 replies; 4+ messages in thread
From: David Hildenbrand @ 2021-03-02 22:09 UTC (permalink / raw)
To: Richard Henderson
Cc: Thomas Huth, Janosch Frank, David Hildenbrand, Cornelia Huck,
qemu-devel, qemu-s390x, Claudio Imbrenda
> 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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-02 22:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-02 19:12 [PATCH] target/s390x: Implement the MVPG condition-code-option bit Thomas Huth
2021-03-02 19:25 ` David Hildenbrand
2021-03-02 21:46 ` Richard Henderson
2021-03-02 22:09 ` 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.