1. Patchew
  2. QEMU
  3. View series

[PATCH v1] s390x/tcg: fix and optimize SPX (SET PREFIX)

David Hildenbrand posted 1 patch 2 years, 9 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210805125938.74034-1-david@redhat.com
Maintainers: Cornelia Huck <cohuck@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Thomas Huth <thuth@redhat.com>, David Hildenbrand <david@redhat.com>
target/s390x/tcg/misc_helper.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
[PATCH v1] s390x/tcg: fix and optimize SPX (SET PREFIX)
Posted by David Hildenbrand 2 years, 9 months ago 
We not only invalidate the translation of the range 0x0-0x2000, we also
invalidate the translation of the new prefix range and the translation
of the old prefix range -- because real2abs would return different
results for all of these ranges when changing the prefix location.

This fixes the kvm-unit-tests "edat" test that just hangs before this
patch because we end up clearing the new prefix area instead of the old
prefix area.

While at it, let's not do anything in case the prefix doesn't change.

Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
Cc: qemu-s390x@nongnu.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/tcg/misc_helper.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
index 33e6999e15..aab9c47747 100644
--- a/target/s390x/tcg/misc_helper.c
+++ b/target/s390x/tcg/misc_helper.c
@@ -151,13 +151,26 @@ void HELPER(diag)(CPUS390XState *env, uint32_t r1, uint32_t r3, uint32_t num)
 /* Set Prefix */
 void HELPER(spx)(CPUS390XState *env, uint64_t a1)
 {
+    const uint32_t prefix = a1 & 0x7fffe000;
+    const uint32_t old_prefix = env->psa;
     CPUState *cs = env_cpu(env);
-    uint32_t prefix = a1 & 0x7fffe000;
+
+    if (prefix == old_prefix) {
+        return;
+    }
 
     env->psa = prefix;
     HELPER_LOG("prefix: %#x\n", prefix);
     tlb_flush_page(cs, 0);
     tlb_flush_page(cs, TARGET_PAGE_SIZE);
+    if (prefix != 0) {
+        tlb_flush_page(cs, prefix);
+        tlb_flush_page(cs, prefix + TARGET_PAGE_SIZE);
+    }
+    if (old_prefix != 0) {
+        tlb_flush_page(cs, old_prefix);
+        tlb_flush_page(cs, old_prefix + TARGET_PAGE_SIZE);
+    }
 }
 
 static void update_ckc_timer(CPUS390XState *env)
-- 
2.31.1
Re: [PATCH v1] s390x/tcg: fix and optimize SPX (SET PREFIX)
Posted by Thomas Huth 2 years, 9 months ago 
On 05/08/2021 14.59, David Hildenbrand wrote:
> We not only invalidate the translation of the range 0x0-0x2000, we also
> invalidate the translation of the new prefix range and the translation
> of the old prefix range -- because real2abs would return different
> results for all of these ranges when changing the prefix location.
> 
> This fixes the kvm-unit-tests "edat" test that just hangs before this
> patch because we end up clearing the new prefix area instead of the old
> prefix area.
> 
> While at it, let's not do anything in case the prefix doesn't change.
> 
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   target/s390x/tcg/misc_helper.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
> index 33e6999e15..aab9c47747 100644
> --- a/target/s390x/tcg/misc_helper.c
> +++ b/target/s390x/tcg/misc_helper.c
> @@ -151,13 +151,26 @@ void HELPER(diag)(CPUS390XState *env, uint32_t r1, uint32_t r3, uint32_t num)
>   /* Set Prefix */
>   void HELPER(spx)(CPUS390XState *env, uint64_t a1)
>   {
> +    const uint32_t prefix = a1 & 0x7fffe000;
> +    const uint32_t old_prefix = env->psa;
>       CPUState *cs = env_cpu(env);
> -    uint32_t prefix = a1 & 0x7fffe000;
> +
> +    if (prefix == old_prefix) {
> +        return;
> +    }
>   
>       env->psa = prefix;
>       HELPER_LOG("prefix: %#x\n", prefix);
>       tlb_flush_page(cs, 0);
>       tlb_flush_page(cs, TARGET_PAGE_SIZE);
> +    if (prefix != 0) {
> +        tlb_flush_page(cs, prefix);
> +        tlb_flush_page(cs, prefix + TARGET_PAGE_SIZE);
> +    }
> +    if (old_prefix != 0) {
> +        tlb_flush_page(cs, old_prefix);
> +        tlb_flush_page(cs, old_prefix + TARGET_PAGE_SIZE);
> +    }
>   }

Sounds reasonable.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Re: [PATCH v1] s390x/tcg: fix and optimize SPX (SET PREFIX)
Posted by Cornelia Huck 2 years, 9 months ago 
On Thu, Aug 05 2021, David Hildenbrand <david@redhat.com> wrote:

> We not only invalidate the translation of the range 0x0-0x2000, we also
> invalidate the translation of the new prefix range and the translation
> of the old prefix range -- because real2abs would return different
> results for all of these ranges when changing the prefix location.
>
> This fixes the kvm-unit-tests "edat" test that just hangs before this
> patch because we end up clearing the new prefix area instead of the old
> prefix area.
>
> While at it, let's not do anything in case the prefix doesn't change.
>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/tcg/misc_helper.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)

Is this still -rc material? Especially as we are late in the -rc cycle
already.
Re: [PATCH v1] s390x/tcg: fix and optimize SPX (SET PREFIX)
Posted by David Hildenbrand 2 years, 9 months ago 
On 06.08.21 13:17, Cornelia Huck wrote:
> On Thu, Aug 05 2021, David Hildenbrand <david@redhat.com> wrote:
> 
>> We not only invalidate the translation of the range 0x0-0x2000, we also
>> invalidate the translation of the new prefix range and the translation
>> of the old prefix range -- because real2abs would return different
>> results for all of these ranges when changing the prefix location.
>>
>> This fixes the kvm-unit-tests "edat" test that just hangs before this
>> patch because we end up clearing the new prefix area instead of the old
>> prefix area.
>>
>> While at it, let's not do anything in case the prefix doesn't change.
>>
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: Cornelia Huck <cohuck@redhat.com>
>> Cc: Thomas Huth <thuth@redhat.com>
>> Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
>> Cc: qemu-s390x@nongnu.org
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>   target/s390x/tcg/misc_helper.c | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> Is this still -rc material? Especially as we are late in the -rc cycle
> already.
> 

It's been broken forever, this can wait I guess.

-- 
Thanks,

David / dhildenb
Re: [PATCH v1] s390x/tcg: fix and optimize SPX (SET PREFIX)
Posted by Cornelia Huck 2 years, 9 months ago 
On Thu, Aug 05 2021, David Hildenbrand <david@redhat.com> wrote:

> We not only invalidate the translation of the range 0x0-0x2000, we also
> invalidate the translation of the new prefix range and the translation
> of the old prefix range -- because real2abs would return different
> results for all of these ranges when changing the prefix location.
>
> This fixes the kvm-unit-tests "edat" test that just hangs before this
> patch because we end up clearing the new prefix area instead of the old
> prefix area.
>
> While at it, let's not do anything in case the prefix doesn't change.
>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/tcg/misc_helper.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)

Thanks, applied.
Re: [PATCH v1] s390x/tcg: fix and optimize SPX (SET PREFIX)
Posted by Claudio Imbrenda 2 years, 9 months ago 
On Thu,  5 Aug 2021 14:59:38 +0200
David Hildenbrand <david@redhat.com> wrote:

> We not only invalidate the translation of the range 0x0-0x2000, we
> also invalidate the translation of the new prefix range and the
> translation of the old prefix range -- because real2abs would return
> different results for all of these ranges when changing the prefix
> location.

looks like a good idea

> This fixes the kvm-unit-tests "edat" test that just hangs before this
> patch because we end up clearing the new prefix area instead of the
> old prefix area.
> 
> While at it, let's not do anything in case the prefix doesn't change.

also looks like a good idea

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
 
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/tcg/misc_helper.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/target/s390x/tcg/misc_helper.c
> b/target/s390x/tcg/misc_helper.c index 33e6999e15..aab9c47747 100644
> --- a/target/s390x/tcg/misc_helper.c
> +++ b/target/s390x/tcg/misc_helper.c
> @@ -151,13 +151,26 @@ void HELPER(diag)(CPUS390XState *env, uint32_t
> r1, uint32_t r3, uint32_t num) /* Set Prefix */
>  void HELPER(spx)(CPUS390XState *env, uint64_t a1)
>  {
> +    const uint32_t prefix = a1 & 0x7fffe000;
> +    const uint32_t old_prefix = env->psa;
>      CPUState *cs = env_cpu(env);
> -    uint32_t prefix = a1 & 0x7fffe000;
> +
> +    if (prefix == old_prefix) {
> +        return;
> +    }
>  
>      env->psa = prefix;
>      HELPER_LOG("prefix: %#x\n", prefix);
>      tlb_flush_page(cs, 0);
>      tlb_flush_page(cs, TARGET_PAGE_SIZE);
> +    if (prefix != 0) {
> +        tlb_flush_page(cs, prefix);
> +        tlb_flush_page(cs, prefix + TARGET_PAGE_SIZE);
> +    }
> +    if (old_prefix != 0) {
> +        tlb_flush_page(cs, old_prefix);
> +        tlb_flush_page(cs, old_prefix + TARGET_PAGE_SIZE);
> +    }
>  }
>  
>  static void update_ckc_timer(CPUS390XState *env)

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.