qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2
@ 2019-08-22 13:58 Cornelia Huck
  2019-08-22 13:58 ` [Qemu-devel] [PATCH for-4.1?] compat: disable edid on virtio-gpu base device Cornelia Huck
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Cornelia Huck @ 2019-08-22 13:58 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-s390x, Cornelia Huck, qemu-devel

The following changes since commit f3b8f18ebf344ab359e8f79f6ed777e740dae77c:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-monitor-2019-08-21' into staging (2019-08-22 10:31:21 +0100)

are available in the Git repository at:

  https://github.com/cohuck/qemu tags/s390x-20190822

for you to fetch changes up to 065fe80fe03ff0f36a0cbebbd2d4b3c05110d96d:

  s390x/mmu: Factor out storage key handling (2019-08-22 14:53:49 +0200)

----------------------------------------------------------------
s390x updates:
- fix a bug in tcg vector handling
- improved skey handling

----------------------------------------------------------------

David Hildenbrand (7):
  s390x/tcg: Fix VERIM with 32/64 bit elements
  s390x/mmu: Trace the right value if setting/getting the storage key
    fails
  s390x/mmu: ASC selection in s390_cpu_get_phys_page_debug()
  s390x/tcg: Rework MMU selection for instruction fetches
  s390x/tcg: Flush the TLB of all CPUs on SSKE and RRBE
  s390x/mmu: Better storage key reference and change bit handling
  s390x/mmu: Factor out storage key handling

 target/s390x/cpu.h              |   7 ++
 target/s390x/helper.c           |   5 ++
 target/s390x/mem_helper.c       |  10 +++
 target/s390x/mmu_helper.c       | 135 ++++++++++++++++++++------------
 target/s390x/translate_vx.inc.c |   2 +-
 5 files changed, 108 insertions(+), 51 deletions(-)

-- 
2.20.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH for-4.1?] compat: disable edid on virtio-gpu base device
  2019-08-22 13:58 [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2 Cornelia Huck
@ 2019-08-22 13:58 ` Cornelia Huck
  2019-08-22 14:03   ` Cornelia Huck
  2019-08-22 14:45   ` no-reply
  2019-08-22 13:58 ` [Qemu-devel] [PULL 1/7] s390x/tcg: Fix VERIM with 32/64 bit elements Cornelia Huck
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 12+ messages in thread
From: Cornelia Huck @ 2019-08-22 13:58 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-s390x, Cornelia Huck, qemu-devel

'edid' is a property of the virtio-gpu base device, so turning
it off on virtio-gpu-pci is not enough (it misses -ccw). Turn
it off on the base device instead.

Fixes: 0a71966253c8 ("edid: flip the default to enabled")
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---

Only just noticed this... should we still shove this into 4.1?
Or do we need a compat 4.1.1 dance for this?

---
 hw/core/machine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 28a475ad97a3..32d1ca9abc5a 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -32,7 +32,7 @@ GlobalProperty hw_compat_4_0[] = {
     { "secondary-vga",  "edid", "false" },
     { "bochs-display",  "edid", "false" },
     { "virtio-vga",     "edid", "false" },
-    { "virtio-gpu-pci", "edid", "false" },
+    { "virtio-gpu",     "edid", "false" },
     { "virtio-device", "use-started", "false" },
     { "virtio-balloon-device", "qemu-4-0-config-size", "true" },
     { "pl031", "migrate-tick-offset", "false" },
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 1/7] s390x/tcg: Fix VERIM with 32/64 bit elements
  2019-08-22 13:58 [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2 Cornelia Huck
  2019-08-22 13:58 ` [Qemu-devel] [PATCH for-4.1?] compat: disable edid on virtio-gpu base device Cornelia Huck
@ 2019-08-22 13:58 ` Cornelia Huck
  2019-08-22 13:58 ` [Qemu-devel] [PULL 2/7] s390x/mmu: Trace the right value if setting/getting the storage key fails Cornelia Huck
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Cornelia Huck @ 2019-08-22 13:58 UTC (permalink / raw)
  To: Peter Maydell
  Cc: David Hildenbrand, Cornelia Huck, qemu-devel, qemu-stable,
	Stefano Brivio, qemu-s390x

From: David Hildenbrand <david@redhat.com>

Wrong order of operands. The constant always comes last. Makes QEMU crash
reliably on specific git fetch invocations.

Reported-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20190814151242.27199-1-david@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Fixes: 5c4b0ab460ef ("s390x/tcg: Implement VECTOR ELEMENT ROTATE AND INSERT UNDER MASK")
Cc: qemu-stable@nongnu.org
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/s390x/translate_vx.inc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/translate_vx.inc.c b/target/s390x/translate_vx.inc.c
index 41d5cf869f94..0caddb3958cd 100644
--- a/target/s390x/translate_vx.inc.c
+++ b/target/s390x/translate_vx.inc.c
@@ -213,7 +213,7 @@ static void get_vec_element_ptr_i64(TCGv_ptr ptr, uint8_t reg, TCGv_i64 enr,
                        vec_full_reg_offset(v3), ptr, 16, 16, data, fn)
 #define gen_gvec_3i(v1, v2, v3, c, gen) \
     tcg_gen_gvec_3i(vec_full_reg_offset(v1), vec_full_reg_offset(v2), \
-                    vec_full_reg_offset(v3), c, 16, 16, gen)
+                    vec_full_reg_offset(v3), 16, 16, c, gen)
 #define gen_gvec_4(v1, v2, v3, v4, gen) \
     tcg_gen_gvec_4(vec_full_reg_offset(v1), vec_full_reg_offset(v2), \
                    vec_full_reg_offset(v3), vec_full_reg_offset(v4), \
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 2/7] s390x/mmu: Trace the right value if setting/getting the storage key fails
  2019-08-22 13:58 [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2 Cornelia Huck
  2019-08-22 13:58 ` [Qemu-devel] [PATCH for-4.1?] compat: disable edid on virtio-gpu base device Cornelia Huck
  2019-08-22 13:58 ` [Qemu-devel] [PULL 1/7] s390x/tcg: Fix VERIM with 32/64 bit elements Cornelia Huck
@ 2019-08-22 13:58 ` Cornelia Huck
  2019-08-22 13:58 ` [Qemu-devel] [PULL 3/7] s390x/mmu: ASC selection in s390_cpu_get_phys_page_debug() Cornelia Huck
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Cornelia Huck @ 2019-08-22 13:58 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Thomas Huth, qemu-s390x, Cornelia Huck, qemu-devel, David Hildenbrand

From: David Hildenbrand <david@redhat.com>

We want to trace the actual return value, not "0".

Fixes: 0f5f669147b5 ("s390x: Enable new s390-storage-keys device")
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20190816084708.602-2-david@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/s390x/mmu_helper.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 7a563110f016..6cf74502ef1e 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -423,7 +423,8 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
     *raddr = mmu_real2abs(env, *raddr);
 
     if (r == 0 && *raddr < ram_size) {
-        if (skeyclass->get_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key)) {
+        r = skeyclass->get_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key);
+        if (r) {
             trace_get_skeys_nonzero(r);
             return 0;
         }
@@ -436,7 +437,8 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
             key |= SK_C;
         }
 
-        if (skeyclass->set_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key)) {
+        r = skeyclass->set_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key);
+        if (r) {
             trace_set_skeys_nonzero(r);
             return 0;
         }
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 3/7] s390x/mmu: ASC selection in s390_cpu_get_phys_page_debug()
  2019-08-22 13:58 [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2 Cornelia Huck
                   ` (2 preceding siblings ...)
  2019-08-22 13:58 ` [Qemu-devel] [PULL 2/7] s390x/mmu: Trace the right value if setting/getting the storage key fails Cornelia Huck
@ 2019-08-22 13:58 ` Cornelia Huck
  2019-08-22 13:58 ` [Qemu-devel] [PULL 4/7] s390x/tcg: Rework MMU selection for instruction fetches Cornelia Huck
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Cornelia Huck @ 2019-08-22 13:58 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Thomas Huth, qemu-s390x, Cornelia Huck, qemu-devel, David Hildenbrand

From: David Hildenbrand <david@redhat.com>

Let's select the ASC before calling the function. This is a prepararion
to remove the ASC magic depending on the access mode from mmu_translate.

There is currently no way to distinguish if we have code or data access.
For now, we were using code access, because especially when debugging with
the gdbstub, we want to read and disassemble what we single-step.

Note: KVM guest can now no longer be crashed using qmp/hmp/gdbstub if they
happen to be in AR mode.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20190816084708.602-3-david@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/s390x/helper.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 1350ad319aba..948c0398d461 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -58,6 +58,11 @@ hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr vaddr)
         vaddr &= 0x7fffffff;
     }
 
+    /* We want to read the code (e.g., see what we are single-stepping).*/
+    if (asc != PSW_ASC_HOME) {
+        asc = PSW_ASC_PRIMARY;
+    }
+
     if (mmu_translate(env, vaddr, MMU_INST_FETCH, asc, &raddr, &prot, false)) {
         return -1;
     }
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 4/7] s390x/tcg: Rework MMU selection for instruction fetches
  2019-08-22 13:58 [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2 Cornelia Huck
                   ` (3 preceding siblings ...)
  2019-08-22 13:58 ` [Qemu-devel] [PULL 3/7] s390x/mmu: ASC selection in s390_cpu_get_phys_page_debug() Cornelia Huck
@ 2019-08-22 13:58 ` Cornelia Huck
  2019-08-22 13:58 ` [Qemu-devel] [PULL 5/7] s390x/tcg: Flush the TLB of all CPUs on SSKE and RRBE Cornelia Huck
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Cornelia Huck @ 2019-08-22 13:58 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Thomas Huth, qemu-s390x, Cornelia Huck, qemu-devel, David Hildenbrand

From: David Hildenbrand <david@redhat.com>

Instructions are always fetched from primary address space, except when
in home address mode. Perform the selection directly in cpu_mmu_index().

get_mem_index() is only used to perform data access, instructions are
fetched via cpu_lduw_code(), which translates to cpu_mmu_index(env, true).

We don't care about restricting the access permissions of the TLB
entries anymore, as we no longer enter PRIMARY entries into the
SECONDARY MMU. Cleanup related code a bit.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Message-Id: <20190816084708.602-4-david@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/s390x/cpu.h        |  7 +++++++
 target/s390x/mmu_helper.c | 38 +++++++++++++++-----------------------
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 3d9de25f7ce3..79202c098096 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -332,6 +332,13 @@ static inline int cpu_mmu_index(CPUS390XState *env, bool ifetch)
         return MMU_REAL_IDX;
     }
 
+    if (ifetch) {
+        if ((env->psw.mask & PSW_MASK_ASC) == PSW_ASC_HOME) {
+            return MMU_HOME_IDX;
+        }
+        return MMU_PRIMARY_IDX;
+    }
+
     switch (env->psw.mask & PSW_MASK_ASC) {
     case PSW_ASC_PRIMARY:
         return MMU_PRIMARY_IDX;
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 6cf74502ef1e..40b6c1fc36a9 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -350,8 +350,9 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
 {
     static S390SKeysState *ss;
     static S390SKeysClass *skeyclass;
-    int r = -1;
+    uint64_t asce;
     uint8_t key;
+    int r;
 
     if (unlikely(!ss)) {
         ss = s390_get_skeys_device();
@@ -381,36 +382,21 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
 
     if (!(env->psw.mask & PSW_MASK_DAT)) {
         *raddr = vaddr;
-        r = 0;
-        goto out;
+        goto nodat;
     }
 
     switch (asc) {
     case PSW_ASC_PRIMARY:
         PTE_DPRINTF("%s: asc=primary\n", __func__);
-        r = mmu_translate_asce(env, vaddr, asc, env->cregs[1], raddr, flags,
-                               rw, exc);
+        asce = env->cregs[1];
         break;
     case PSW_ASC_HOME:
         PTE_DPRINTF("%s: asc=home\n", __func__);
-        r = mmu_translate_asce(env, vaddr, asc, env->cregs[13], raddr, flags,
-                               rw, exc);
+        asce = env->cregs[13];
         break;
     case PSW_ASC_SECONDARY:
         PTE_DPRINTF("%s: asc=secondary\n", __func__);
-        /*
-         * Instruction: Primary
-         * Data: Secondary
-         */
-        if (rw == MMU_INST_FETCH) {
-            r = mmu_translate_asce(env, vaddr, PSW_ASC_PRIMARY, env->cregs[1],
-                                   raddr, flags, rw, exc);
-            *flags &= ~(PAGE_READ | PAGE_WRITE);
-        } else {
-            r = mmu_translate_asce(env, vaddr, PSW_ASC_SECONDARY, env->cregs[7],
-                                   raddr, flags, rw, exc);
-            *flags &= ~(PAGE_EXEC);
-        }
+        asce = env->cregs[7];
         break;
     case PSW_ASC_ACCREG:
     default:
@@ -418,11 +404,17 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
         break;
     }
 
- out:
+    /* perform the DAT translation */
+    r = mmu_translate_asce(env, vaddr, asc, asce, raddr, flags, rw, exc);
+    if (r) {
+        return r;
+    }
+
+nodat:
     /* Convert real address -> absolute address */
     *raddr = mmu_real2abs(env, *raddr);
 
-    if (r == 0 && *raddr < ram_size) {
+    if (*raddr < ram_size) {
         r = skeyclass->get_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key);
         if (r) {
             trace_get_skeys_nonzero(r);
@@ -444,7 +436,7 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
         }
     }
 
-    return r;
+    return 0;
 }
 
 /**
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 5/7] s390x/tcg: Flush the TLB of all CPUs on SSKE and RRBE
  2019-08-22 13:58 [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2 Cornelia Huck
                   ` (4 preceding siblings ...)
  2019-08-22 13:58 ` [Qemu-devel] [PULL 4/7] s390x/tcg: Rework MMU selection for instruction fetches Cornelia Huck
@ 2019-08-22 13:58 ` Cornelia Huck
  2019-08-22 13:58 ` [Qemu-devel] [PULL 6/7] s390x/mmu: Better storage key reference and change bit handling Cornelia Huck
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Cornelia Huck @ 2019-08-22 13:58 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-s390x, Cornelia Huck, Alex Bennée, qemu-devel,
	David Hildenbrand

From: David Hildenbrand <david@redhat.com>

Whenever we modify a storage key, we should flush the TLBs of all CPUs,
so the MMU fault handling code can properly consider the changed storage
key (to e.g., properly set the reference and change bit on the next
accesses).

These functions are barely used in modern Linux guests, so the performance
implications are neglectable for now.

This is a preparation for better reference and change bit handling for
TCG, which will require more MMU changes.

Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20190816084708.602-5-david@redhat.com>
Acked-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/s390x/mem_helper.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 29d9eaa5b725..91ba2e03d95c 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1815,6 +1815,11 @@ void HELPER(sske)(CPUS390XState *env, uint64_t r1, uint64_t r2)
 
     key = (uint8_t) r1;
     skeyclass->set_skeys(ss, addr / TARGET_PAGE_SIZE, 1, &key);
+   /*
+    * As we can only flush by virtual address and not all the entries
+    * that point to a physical address we have to flush the whole TLB.
+    */
+    tlb_flush_all_cpus_synced(env_cpu(env));
 }
 
 /* reset reference bit extended */
@@ -1843,6 +1848,11 @@ uint32_t HELPER(rrbe)(CPUS390XState *env, uint64_t r2)
     if (skeyclass->set_skeys(ss, r2 / TARGET_PAGE_SIZE, 1, &key)) {
         return 0;
     }
+   /*
+    * As we can only flush by virtual address and not all the entries
+    * that point to a physical address we have to flush the whole TLB.
+    */
+    tlb_flush_all_cpus_synced(env_cpu(env));
 
     /*
      * cc
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 6/7] s390x/mmu: Better storage key reference and change bit handling
  2019-08-22 13:58 [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2 Cornelia Huck
                   ` (5 preceding siblings ...)
  2019-08-22 13:58 ` [Qemu-devel] [PULL 5/7] s390x/tcg: Flush the TLB of all CPUs on SSKE and RRBE Cornelia Huck
@ 2019-08-22 13:58 ` Cornelia Huck
  2019-08-22 13:58 ` [Qemu-devel] [PULL 7/7] s390x/mmu: Factor out storage key handling Cornelia Huck
  2019-08-23 15:11 ` [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2 Peter Maydell
  8 siblings, 0 replies; 12+ messages in thread
From: Cornelia Huck @ 2019-08-22 13:58 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-s390x, Cornelia Huck, qemu-devel, David Hildenbrand

From: David Hildenbrand <david@redhat.com>

Any access sets the reference bit. In case we have a read-fault, we
should not allow writes to the TLB entry if the change bit was not
already set.

This is a preparation for proper storage-key reference/change bit handling
in TCG and a fix for KVM whereby read accesses would set the change
bit (old KVM versions without the ioctl to carry out the translation).

Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20190816084708.602-6-david@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/s390x/mmu_helper.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 40b6c1fc36a9..61654e07dec8 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -421,14 +421,28 @@ nodat:
             return 0;
         }
 
-        if (*flags & PAGE_READ) {
-            key |= SK_R;
-        }
-
-        if (*flags & PAGE_WRITE) {
+        switch (rw) {
+        case MMU_DATA_LOAD:
+        case MMU_INST_FETCH:
+            /*
+             * The TLB entry has to remain write-protected on read-faults if
+             * the storage key does not indicate a change already. Otherwise
+             * we might miss setting the change bit on write accesses.
+             */
+            if (!(key & SK_C)) {
+                *flags &= ~PAGE_WRITE;
+            }
+            break;
+        case MMU_DATA_STORE:
             key |= SK_C;
+            break;
+        default:
+            g_assert_not_reached();
         }
 
+        /* Any store/fetch sets the reference bit */
+        key |= SK_R;
+
         r = skeyclass->set_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key);
         if (r) {
             trace_set_skeys_nonzero(r);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 7/7] s390x/mmu: Factor out storage key handling
  2019-08-22 13:58 [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2 Cornelia Huck
                   ` (6 preceding siblings ...)
  2019-08-22 13:58 ` [Qemu-devel] [PULL 6/7] s390x/mmu: Better storage key reference and change bit handling Cornelia Huck
@ 2019-08-22 13:58 ` Cornelia Huck
  2019-08-23 15:11 ` [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2 Peter Maydell
  8 siblings, 0 replies; 12+ messages in thread
From: Cornelia Huck @ 2019-08-22 13:58 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Thomas Huth, qemu-s390x, Cornelia Huck, qemu-devel, David Hildenbrand

From: David Hildenbrand <david@redhat.com>

Factor it out, add a comment how it all works, and also use it in the
REAL MMU.

Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20190816084708.602-7-david@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/s390x/mmu_helper.c | 115 +++++++++++++++++++++++---------------
 1 file changed, 71 insertions(+), 44 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 61654e07dec8..7e6b0d050847 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -335,6 +335,75 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
     return r;
 }
 
+static void mmu_handle_skey(target_ulong addr, int rw, int *flags)
+{
+    static S390SKeysClass *skeyclass;
+    static S390SKeysState *ss;
+    uint8_t key;
+    int rc;
+
+    if (unlikely(addr >= ram_size)) {
+        return;
+    }
+
+    if (unlikely(!ss)) {
+        ss = s390_get_skeys_device();
+        skeyclass = S390_SKEYS_GET_CLASS(ss);
+    }
+
+    /*
+     * Whenever we create a new TLB entry, we set the storage key reference
+     * bit. In case we allow write accesses, we set the storage key change
+     * bit. Whenever the guest changes the storage key, we have to flush the
+     * TLBs of all CPUs (the whole TLB or all affected entries), so that the
+     * next reference/change will result in an MMU fault and make us properly
+     * update the storage key here.
+     *
+     * Note 1: "record of references ... is not necessarily accurate",
+     *         "change bit may be set in case no storing has occurred".
+     *         -> We can set reference/change bits even on exceptions.
+     * Note 2: certain accesses seem to ignore storage keys. For example,
+     *         DAT translation does not set reference bits for table accesses.
+     *
+     * TODO: key-controlled protection. Only CPU accesses make use of the
+     *       PSW key. CSS accesses are different - we have to pass in the key.
+     *
+     * TODO: we have races between getting and setting the key.
+     */
+    rc = skeyclass->get_skeys(ss, addr / TARGET_PAGE_SIZE, 1, &key);
+    if (rc) {
+        trace_get_skeys_nonzero(rc);
+        return;
+    }
+
+    switch (rw) {
+    case MMU_DATA_LOAD:
+    case MMU_INST_FETCH:
+        /*
+         * The TLB entry has to remain write-protected on read-faults if
+         * the storage key does not indicate a change already. Otherwise
+         * we might miss setting the change bit on write accesses.
+         */
+        if (!(key & SK_C)) {
+            *flags &= ~PAGE_WRITE;
+        }
+        break;
+    case MMU_DATA_STORE:
+        key |= SK_C;
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    /* Any store/fetch sets the reference bit */
+    key |= SK_R;
+
+    rc = skeyclass->set_skeys(ss, addr / TARGET_PAGE_SIZE, 1, &key);
+    if (rc) {
+        trace_set_skeys_nonzero(rc);
+    }
+}
+
 /**
  * Translate a virtual (logical) address into a physical (absolute) address.
  * @param vaddr  the virtual address
@@ -348,16 +417,9 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
 int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
                   target_ulong *raddr, int *flags, bool exc)
 {
-    static S390SKeysState *ss;
-    static S390SKeysClass *skeyclass;
     uint64_t asce;
-    uint8_t key;
     int r;
 
-    if (unlikely(!ss)) {
-        ss = s390_get_skeys_device();
-        skeyclass = S390_SKEYS_GET_CLASS(ss);
-    }
 
     *flags = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
     if (is_low_address(vaddr & TARGET_PAGE_MASK) && lowprot_enabled(env, asc)) {
@@ -414,42 +476,7 @@ nodat:
     /* Convert real address -> absolute address */
     *raddr = mmu_real2abs(env, *raddr);
 
-    if (*raddr < ram_size) {
-        r = skeyclass->get_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key);
-        if (r) {
-            trace_get_skeys_nonzero(r);
-            return 0;
-        }
-
-        switch (rw) {
-        case MMU_DATA_LOAD:
-        case MMU_INST_FETCH:
-            /*
-             * The TLB entry has to remain write-protected on read-faults if
-             * the storage key does not indicate a change already. Otherwise
-             * we might miss setting the change bit on write accesses.
-             */
-            if (!(key & SK_C)) {
-                *flags &= ~PAGE_WRITE;
-            }
-            break;
-        case MMU_DATA_STORE:
-            key |= SK_C;
-            break;
-        default:
-            g_assert_not_reached();
-        }
-
-        /* Any store/fetch sets the reference bit */
-        key |= SK_R;
-
-        r = skeyclass->set_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key);
-        if (r) {
-            trace_set_skeys_nonzero(r);
-            return 0;
-        }
-    }
-
+    mmu_handle_skey(*raddr, rw, flags);
     return 0;
 }
 
@@ -567,6 +594,6 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
 
     *addr = mmu_real2abs(env, raddr & TARGET_PAGE_MASK);
 
-    /* TODO: storage key handling */
+    mmu_handle_skey(*addr, rw, flags);
     return 0;
 }
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-4.1?] compat: disable edid on virtio-gpu base device
  2019-08-22 13:58 ` [Qemu-devel] [PATCH for-4.1?] compat: disable edid on virtio-gpu base device Cornelia Huck
@ 2019-08-22 14:03   ` Cornelia Huck
  2019-08-22 14:45   ` no-reply
  1 sibling, 0 replies; 12+ messages in thread
From: Cornelia Huck @ 2019-08-22 14:03 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-s390x, qemu-devel

On Thu, 22 Aug 2019 15:58:32 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> 'edid' is a property of the virtio-gpu base device, so turning
> it off on virtio-gpu-pci is not enough (it misses -ccw). Turn
> it off on the base device instead.
> 
> Fixes: 0a71966253c8 ("edid: flip the default to enabled")
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> 
> Only just noticed this... should we still shove this into 4.1?
> Or do we need a compat 4.1.1 dance for this?
> 
> ---
>  hw/core/machine.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 28a475ad97a3..32d1ca9abc5a 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -32,7 +32,7 @@ GlobalProperty hw_compat_4_0[] = {
>      { "secondary-vga",  "edid", "false" },
>      { "bochs-display",  "edid", "false" },
>      { "virtio-vga",     "edid", "false" },
> -    { "virtio-gpu-pci", "edid", "false" },
> +    { "virtio-gpu",     "edid", "false" },
>      { "virtio-device", "use-started", "false" },
>      { "virtio-balloon-device", "qemu-4-0-config-size", "true" },
>      { "pl031", "migrate-tick-offset", "false" },

Eh... rm fail. Please ignore that patch :)


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-4.1?] compat: disable edid on virtio-gpu base device
  2019-08-22 13:58 ` [Qemu-devel] [PATCH for-4.1?] compat: disable edid on virtio-gpu base device Cornelia Huck
  2019-08-22 14:03   ` Cornelia Huck
@ 2019-08-22 14:45   ` no-reply
  1 sibling, 0 replies; 12+ messages in thread
From: no-reply @ 2019-08-22 14:45 UTC (permalink / raw)
  To: cohuck; +Cc: peter.maydell, qemu-s390x, cohuck, qemu-devel

Patchew URL: https://patchew.org/QEMU/20190822135839.32340-2-cohuck@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH for-4.1?] compat: disable edid on virtio-gpu base device
Message-id: 20190822135839.32340-2-cohuck@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for path 'capstone'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 'roms/SLOF'
Submodule 'roms/edk2' (https://git.qemu.org/git/edk2.git) registered for path 'roms/edk2'
Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 'roms/ipxe'
Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered for path 'roms/openbios'
Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) registered for path 'roms/openhackware'
Submodule 'roms/opensbi' (https://git.qemu.org/git/opensbi.git) registered for path 'roms/opensbi'
Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for path 'roms/seabios'
Submodule 'roms/seabios-hppa' (https://git.qemu.org/git/seabios-hppa.git) registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for path 'roms/sgabios'
Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for path 'roms/skiboot'
Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for path 'roms/u-boot'
Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) registered for path 'roms/u-boot-sam460ex'
Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) registered for path 'slirp'
Submodule 'tests/fp/berkeley-softfloat-3' (https://git.qemu.org/git/berkeley-softfloat-3.git) registered for path 'tests/fp/berkeley-softfloat-3'
Submodule 'tests/fp/berkeley-testfloat-3' (https://git.qemu.org/git/berkeley-testfloat-3.git) registered for path 'tests/fp/berkeley-testfloat-3'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out '22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out '90c488d5f4a407342247b9ea869df1c2d9c8e266'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out '7bfe584e321946771692711ff83ad2b5850daca7'
Cloning into 'roms/edk2'...
Submodule path 'roms/edk2': checked out '20d2e5a125e34fc8501026613a71549b2a1a3e54'
Submodule 'SoftFloat' (https://github.com/ucb-bar/berkeley-softfloat-3.git) registered for path 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'
Submodule 'CryptoPkg/Library/OpensslLib/openssl' (https://github.com/openssl/openssl) registered for path 'CryptoPkg/Library/OpensslLib/openssl'
Cloning into 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'...
Submodule path 'roms/edk2/ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'CryptoPkg/Library/OpensslLib/openssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl': checked out '50eaac9f3337667259de725451f201e784599687'
Submodule 'boringssl' (https://boringssl.googlesource.com/boringssl) registered for path 'boringssl'
Submodule 'krb5' (https://github.com/krb5/krb5) registered for path 'krb5'
Submodule 'pyca.cryptography' (https://github.com/pyca/cryptography.git) registered for path 'pyca-cryptography'
Cloning into 'boringssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/boringssl': checked out '2070f8ad9151dc8f3a73bffaa146b5e6937a583f'
Cloning into 'krb5'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/krb5': checked out 'b9ad6c49505c96a088326b62a52568e3484f2168'
Cloning into 'pyca-cryptography'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/pyca-cryptography': checked out '09403100de2f6f1cdd0d484dcb8e620f1c335c8f'
Cloning into 'roms/ipxe'...
Submodule path 'roms/ipxe': checked out 'de4565cbe76ea9f7913a01f331be3ee901bb6e17'
Cloning into 'roms/openbios'...
Submodule path 'roms/openbios': checked out 'c79e0ecb84f4f1ee3f73f521622e264edd1bf174'
Cloning into 'roms/openhackware'...
Submodule path 'roms/openhackware': checked out 'c559da7c8eec5e45ef1f67978827af6f0b9546f5'
Cloning into 'roms/opensbi'...
Submodule path 'roms/opensbi': checked out 'ce228ee0919deb9957192d723eecc8aaae2697c6'
Cloning into 'roms/qemu-palcode'...
Submodule path 'roms/qemu-palcode': checked out 'bf0e13698872450164fa7040da36a95d2d4b326f'
Cloning into 'roms/seabios'...
Submodule path 'roms/seabios': checked out 'a5cab58e9a3fb6e168aba919c5669bea406573b4'
Cloning into 'roms/seabios-hppa'...
Submodule path 'roms/seabios-hppa': checked out '0f4fe84658165e96ce35870fd19fc634e182e77b'
Cloning into 'roms/sgabios'...
Submodule path 'roms/sgabios': checked out 'cbaee52287e5f32373181cff50a00b6c4ac9015a'
Cloning into 'roms/skiboot'...
Submodule path 'roms/skiboot': checked out '261ca8e779e5138869a45f174caa49be6a274501'
Cloning into 'roms/u-boot'...
Submodule path 'roms/u-boot': checked out 'd3689267f92c5956e09cc7d1baa4700141662bff'
Cloning into 'roms/u-boot-sam460ex'...
Submodule path 'roms/u-boot-sam460ex': checked out '60b3916f33e617a815973c5a6df77055b2e3a588'
Cloning into 'slirp'...
Submodule path 'slirp': checked out '126c04acbabd7ad32c2b018fe10dfac2a3bc1210'
Cloning into 'tests/fp/berkeley-softfloat-3'...
Submodule path 'tests/fp/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'tests/fp/berkeley-testfloat-3'...
Submodule path 'tests/fp/berkeley-testfloat-3': checked out '5a59dcec19327396a011a17fd924aed4fec416b3'
Cloning into 'ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
Switched to a new branch 'test'

=== OUTPUT BEGIN ===
checkpatch.pl: no revisions returned for revlist '1'
=== OUTPUT END ===

Test command exited with code: 255


The full log is available at
http://patchew.org/logs/20190822135839.32340-2-cohuck@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2
  2019-08-22 13:58 [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2 Cornelia Huck
                   ` (7 preceding siblings ...)
  2019-08-22 13:58 ` [Qemu-devel] [PULL 7/7] s390x/mmu: Factor out storage key handling Cornelia Huck
@ 2019-08-23 15:11 ` Peter Maydell
  8 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2019-08-23 15:11 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: qemu-s390x, QEMU Developers

On Thu, 22 Aug 2019 at 14:58, Cornelia Huck <cohuck@redhat.com> wrote:
>
> The following changes since commit f3b8f18ebf344ab359e8f79f6ed777e740dae77c:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-monitor-2019-08-21' into staging (2019-08-22 10:31:21 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/cohuck/qemu tags/s390x-20190822
>
> for you to fetch changes up to 065fe80fe03ff0f36a0cbebbd2d4b3c05110d96d:
>
>   s390x/mmu: Factor out storage key handling (2019-08-22 14:53:49 +0200)
>
> ----------------------------------------------------------------
> s390x updates:
> - fix a bug in tcg vector handling
> - improved skey handling
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.2
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-08-23 15:14 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-22 13:58 [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2 Cornelia Huck
2019-08-22 13:58 ` [Qemu-devel] [PATCH for-4.1?] compat: disable edid on virtio-gpu base device Cornelia Huck
2019-08-22 14:03   ` Cornelia Huck
2019-08-22 14:45   ` no-reply
2019-08-22 13:58 ` [Qemu-devel] [PULL 1/7] s390x/tcg: Fix VERIM with 32/64 bit elements Cornelia Huck
2019-08-22 13:58 ` [Qemu-devel] [PULL 2/7] s390x/mmu: Trace the right value if setting/getting the storage key fails Cornelia Huck
2019-08-22 13:58 ` [Qemu-devel] [PULL 3/7] s390x/mmu: ASC selection in s390_cpu_get_phys_page_debug() Cornelia Huck
2019-08-22 13:58 ` [Qemu-devel] [PULL 4/7] s390x/tcg: Rework MMU selection for instruction fetches Cornelia Huck
2019-08-22 13:58 ` [Qemu-devel] [PULL 5/7] s390x/tcg: Flush the TLB of all CPUs on SSKE and RRBE Cornelia Huck
2019-08-22 13:58 ` [Qemu-devel] [PULL 6/7] s390x/mmu: Better storage key reference and change bit handling Cornelia Huck
2019-08-22 13:58 ` [Qemu-devel] [PULL 7/7] s390x/mmu: Factor out storage key handling Cornelia Huck
2019-08-23 15:11 ` [Qemu-devel] [PULL 0/7] First batch of s390x changes for 4.2 Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).