All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 0/7] ppc-for-5.0 queue 20200324
@ 2020-03-24  5:14 David Gibson
  2020-03-24  5:14 ` [PULL 1/7] ppc/spapr: Set the effective address provided flag in mc error log David Gibson
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: David Gibson @ 2020-03-24  5:14 UTC (permalink / raw)
  To: peter.maydell; +Cc: aik, qemu-devel, groug, qemu-ppc, clg, David Gibson

The following changes since commit c532b954d96f96d361ca31308f75f1b95bd4df76:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200323' into staging (2020-03-23 17:41:21 +0000)

are available in the Git repository at:

  git://github.com/dgibson/qemu.git tags/ppc-for-5.0-20200324

for you to fetch changes up to 1583794b9b36911df116cc726750dadbeeac506a:

  ppc/ppc405_boards: Remove unnecessary NULL check (2020-03-24 11:56:37 +1100)

----------------------------------------------------------------
ppc patch queue for 2020-03-24

Here's a final pull request before the qemu-5.0 hard freeze.

We have an implementation of the POWER9 forms of the slbia
instruction, a small cleanup and a handful of assorted fixes.

----------------------------------------------------------------
Greg Kurz (1):
      spapr: Fix memory leak in h_client_architecture_support()

Mahesh Salgaonkar (1):
      ppc/spapr: Set the effective address provided flag in mc error log.

Nicholas Piggin (2):
      target/ppc: Fix slbia TLB invalidation gap
      target/ppc: Fix ISA v3.0 (POWER9) slbia implementation

Peter Maydell (1):
      hw/ppc: Take QEMU lock when calling ppc_dcr_read/write()

Philippe Mathieu-Daudé (1):
      ppc/ppc405_boards: Remove unnecessary NULL check

Vincent Fazio (1):
      target/ppc: don't byte swap ELFv2 signal handler

 hw/ppc/ppc405_boards.c       |  6 ++--
 hw/ppc/spapr_events.c        | 26 ++++++++++++++++
 hw/ppc/spapr_hcall.c         |  1 +
 linux-user/ppc/signal.c      |  6 ++--
 target/ppc/helper.h          |  2 +-
 target/ppc/mmu-hash64.c      | 73 +++++++++++++++++++++++++++++++++++++-------
 target/ppc/timebase_helper.c | 40 +++++++++++++++---------
 target/ppc/translate.c       |  5 ++-
 8 files changed, 125 insertions(+), 34 deletions(-)


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

* [PULL 1/7] ppc/spapr: Set the effective address provided flag in mc error log.
  2020-03-24  5:14 [PULL 0/7] ppc-for-5.0 queue 20200324 David Gibson
@ 2020-03-24  5:14 ` David Gibson
  2020-03-24  5:14 ` [PULL 2/7] target/ppc: Fix slbia TLB invalidation gap David Gibson
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2020-03-24  5:14 UTC (permalink / raw)
  To: peter.maydell
  Cc: aik, qemu-devel, Nicholas Piggin, groug, qemu-ppc, clg, David Gibson

From: Mahesh Salgaonkar <mahesh@linux.ibm.com>

Per PAPR, it is expected to set effective address provided flag in
sub_err_type member of mc extended error log (i.e
rtas_event_log_v6_mc.sub_err_type). This somehow got missed in original
fwnmi-mce patch series. The current code just updates the effective address
but does not set the flag to indicate that it is available. Hence guest
fails to extract effective address from mce rtas log. This patch fixes
that.

Without this patch guest MCE logs fails print DAR value:

[   11.933608] Disabling lock debugging due to kernel taint
[   11.933773] MCE: CPU0: machine check (Severe) Host TLB Multihit [Recovered]
[   11.933979] MCE: CPU0: NIP: [c000000000090b34] radix__flush_tlb_range_psize+0x194/0xf00
[   11.934223] MCE: CPU0: Initiator CPU
[   11.934341] MCE: CPU0: Unknown

After the change:

[   22.454149] Disabling lock debugging due to kernel taint
[   22.454316] MCE: CPU0: machine check (Severe) Host TLB Multihit DAR: deadbeefdeadbeef [Recovered]
[   22.454605] MCE: CPU0: NIP: [c0000000003e5804] kmem_cache_alloc+0x84/0x330
[   22.454820] MCE: CPU0: Initiator CPU
[   22.454944] MCE: CPU0: Unknown

Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Message-Id: <158451653844.22972.17999316676230071087.stgit@jupiter>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_events.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 323fcef4aa..a4a540f43d 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -243,6 +243,14 @@ struct rtas_event_log_v6_mc {
 #define RTAS_LOG_V6_MC_TLB_PARITY                        1
 #define RTAS_LOG_V6_MC_TLB_MULTIHIT                      2
 #define RTAS_LOG_V6_MC_TLB_INDETERMINATE                 3
+/*
+ * Per PAPR,
+ * For UE error type, set bit 1 of sub_err_type to indicate effective addr is
+ * provided. For other error types (SLB/ERAT/TLB), set bit 0 to indicate
+ * same.
+ */
+#define RTAS_LOG_V6_MC_UE_EA_ADDR_PROVIDED               0x40
+#define RTAS_LOG_V6_MC_EA_ADDR_PROVIDED                  0x80
     uint8_t reserved_1[6];
     uint64_t effective_address;
     uint64_t logical_address;
@@ -726,6 +734,22 @@ void spapr_hotplug_req_remove_by_count_indexed(SpaprDrcType drc_type,
                             RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, &drc_id);
 }
 
+static void spapr_mc_set_ea_provided_flag(struct mc_extended_log *ext_elog)
+{
+    switch (ext_elog->mc.error_type) {
+    case RTAS_LOG_V6_MC_TYPE_UE:
+        ext_elog->mc.sub_err_type |= RTAS_LOG_V6_MC_UE_EA_ADDR_PROVIDED;
+        break;
+    case RTAS_LOG_V6_MC_TYPE_SLB:
+    case RTAS_LOG_V6_MC_TYPE_ERAT:
+    case RTAS_LOG_V6_MC_TYPE_TLB:
+        ext_elog->mc.sub_err_type |= RTAS_LOG_V6_MC_EA_ADDR_PROVIDED;
+        break;
+    default:
+        break;
+    }
+}
+
 static uint32_t spapr_mce_get_elog_type(PowerPCCPU *cpu, bool recovered,
                                         struct mc_extended_log *ext_elog)
 {
@@ -751,6 +775,7 @@ static uint32_t spapr_mce_get_elog_type(PowerPCCPU *cpu, bool recovered,
             ext_elog->mc.sub_err_type = mc_derror_table[i].error_subtype;
             if (mc_derror_table[i].dar_valid) {
                 ext_elog->mc.effective_address = cpu_to_be64(env->spr[SPR_DAR]);
+                spapr_mc_set_ea_provided_flag(ext_elog);
             }
 
             summary |= mc_derror_table[i].initiator
@@ -769,6 +794,7 @@ static uint32_t spapr_mce_get_elog_type(PowerPCCPU *cpu, bool recovered,
             ext_elog->mc.sub_err_type = mc_ierror_table[i].error_subtype;
             if (mc_ierror_table[i].nip_valid) {
                 ext_elog->mc.effective_address = cpu_to_be64(env->nip);
+                spapr_mc_set_ea_provided_flag(ext_elog);
             }
 
             summary |= mc_ierror_table[i].initiator
-- 
2.25.1



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

* [PULL 2/7] target/ppc: Fix slbia TLB invalidation gap
  2020-03-24  5:14 [PULL 0/7] ppc-for-5.0 queue 20200324 David Gibson
  2020-03-24  5:14 ` [PULL 1/7] ppc/spapr: Set the effective address provided flag in mc error log David Gibson
@ 2020-03-24  5:14 ` David Gibson
  2020-03-24  5:14 ` [PULL 3/7] target/ppc: Fix ISA v3.0 (POWER9) slbia implementation David Gibson
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2020-03-24  5:14 UTC (permalink / raw)
  To: peter.maydell
  Cc: aik, qemu-devel, Nicholas Piggin, groug, qemu-ppc, clg, David Gibson

From: Nicholas Piggin <npiggin@gmail.com>

slbia must invalidate TLBs even if it does not remove a valid SLB
entry, because slbmte can overwrite valid entries without removing
their TLBs.

As the architecture says, slbia invalidates all lookaside information,
not conditionally based on if it removed valid entries.

It does not seem possible for POWER8 or earlier Linux kernels to hit
this bug because it never changes its kernel SLB translations, and it
should always have valid entries if any accesses are made to userspace
regions. However other operating systems which may modify SLB entry 0
or do more fancy things with segments might be affected.

When POWER9 slbia support is added in the next patch, this becomes a
real problem because some new slbia variants don't invalidate all
non-zero entries.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Message-Id: <20200318044135.851716-1-npiggin@gmail.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target/ppc/mmu-hash64.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 34f6009b1e..373d44de74 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -100,20 +100,29 @@ void helper_slbia(CPUPPCState *env)
     PowerPCCPU *cpu = env_archcpu(env);
     int n;
 
+    /*
+     * slbia must always flush all TLB (which is equivalent to ERAT in ppc
+     * architecture). Matching on SLB_ESID_V is not good enough, because slbmte
+     * can overwrite a valid SLB without flushing its lookaside information.
+     *
+     * It would be possible to keep the TLB in synch with the SLB by flushing
+     * when a valid entry is overwritten by slbmte, and therefore slbia would
+     * not have to flush unless it evicts a valid SLB entry. However it is
+     * expected that slbmte is more common than slbia, and slbia is usually
+     * going to evict valid SLB entries, so that tradeoff is unlikely to be a
+     * good one.
+     */
+
     /* XXX: Warning: slbia never invalidates the first segment */
     for (n = 1; n < cpu->hash64_opts->slb_size; n++) {
         ppc_slb_t *slb = &env->slb[n];
 
         if (slb->esid & SLB_ESID_V) {
             slb->esid &= ~SLB_ESID_V;
-            /*
-             * XXX: given the fact that segment size is 256 MB or 1TB,
-             *      and we still don't have a tlb_flush_mask(env, n, mask)
-             *      in QEMU, we just invalidate all TLBs
-             */
-            env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH;
         }
     }
+
+    env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH;
 }
 
 static void __helper_slbie(CPUPPCState *env, target_ulong addr,
-- 
2.25.1



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

* [PULL 3/7] target/ppc: Fix ISA v3.0 (POWER9) slbia implementation
  2020-03-24  5:14 [PULL 0/7] ppc-for-5.0 queue 20200324 David Gibson
  2020-03-24  5:14 ` [PULL 1/7] ppc/spapr: Set the effective address provided flag in mc error log David Gibson
  2020-03-24  5:14 ` [PULL 2/7] target/ppc: Fix slbia TLB invalidation gap David Gibson
@ 2020-03-24  5:14 ` David Gibson
  2020-03-24  5:14 ` [PULL 4/7] target/ppc: don't byte swap ELFv2 signal handler David Gibson
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2020-03-24  5:14 UTC (permalink / raw)
  To: peter.maydell
  Cc: aik, qemu-devel, Nicholas Piggin, groug, qemu-ppc, clg, David Gibson

From: Nicholas Piggin <npiggin@gmail.com>

The new ISA v3.0 slbia variants have not been implemented for TCG,
which can lead to crashing when a POWER9 machine boots Linux using
the hash MMU, for example ("disable_radix" kernel command line).

Add them.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Message-Id: <20200319064439.1020571-1-npiggin@gmail.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
[dwg: Fixed compile error for USER_ONLY builds]
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target/ppc/helper.h     |  2 +-
 target/ppc/mmu-hash64.c | 56 +++++++++++++++++++++++++++++++++++------
 target/ppc/translate.c  |  5 +++-
 3 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index cfb4c07085..a95c010391 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -614,7 +614,7 @@ DEF_HELPER_FLAGS_3(store_slb, TCG_CALL_NO_RWG, void, env, tl, tl)
 DEF_HELPER_2(load_slb_esid, tl, env, tl)
 DEF_HELPER_2(load_slb_vsid, tl, env, tl)
 DEF_HELPER_2(find_slb_vsid, tl, env, tl)
-DEF_HELPER_FLAGS_1(slbia, TCG_CALL_NO_RWG, void, env)
+DEF_HELPER_FLAGS_2(slbia, TCG_CALL_NO_RWG, void, env, i32)
 DEF_HELPER_FLAGS_2(slbie, TCG_CALL_NO_RWG, void, env, tl)
 DEF_HELPER_FLAGS_2(slbieg, TCG_CALL_NO_RWG, void, env, tl)
 #endif
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 373d44de74..e5baabf0e1 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -95,9 +95,10 @@ void dump_slb(PowerPCCPU *cpu)
     }
 }
 
-void helper_slbia(CPUPPCState *env)
+void helper_slbia(CPUPPCState *env, uint32_t ih)
 {
     PowerPCCPU *cpu = env_archcpu(env);
+    int starting_entry;
     int n;
 
     /*
@@ -111,18 +112,59 @@ void helper_slbia(CPUPPCState *env)
      * expected that slbmte is more common than slbia, and slbia is usually
      * going to evict valid SLB entries, so that tradeoff is unlikely to be a
      * good one.
+     *
+     * ISA v2.05 introduced IH field with values 0,1,2,6. These all invalidate
+     * the same SLB entries (everything but entry 0), but differ in what
+     * "lookaside information" is invalidated. TCG can ignore this and flush
+     * everything.
+     *
+     * ISA v3.0 introduced additional values 3,4,7, which change what SLBs are
+     * invalidated.
      */
 
-    /* XXX: Warning: slbia never invalidates the first segment */
-    for (n = 1; n < cpu->hash64_opts->slb_size; n++) {
-        ppc_slb_t *slb = &env->slb[n];
+    env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH;
+
+    starting_entry = 1; /* default for IH=0,1,2,6 */
+
+    if (env->mmu_model == POWERPC_MMU_3_00) {
+        switch (ih) {
+        case 0x7:
+            /* invalidate no SLBs, but all lookaside information */
+            return;
 
-        if (slb->esid & SLB_ESID_V) {
-            slb->esid &= ~SLB_ESID_V;
+        case 0x3:
+        case 0x4:
+            /* also considers SLB entry 0 */
+            starting_entry = 0;
+            break;
+
+        case 0x5:
+            /* treat undefined values as ih==0, and warn */
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "slbia undefined IH field %u.\n", ih);
+            break;
+
+        default:
+            /* 0,1,2,6 */
+            break;
         }
     }
 
-    env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH;
+    for (n = starting_entry; n < cpu->hash64_opts->slb_size; n++) {
+        ppc_slb_t *slb = &env->slb[n];
+
+        if (!(slb->esid & SLB_ESID_V)) {
+            continue;
+        }
+        if (env->mmu_model == POWERPC_MMU_3_00) {
+            if (ih == 0x3 && (slb->vsid & SLB_VSID_C) == 0) {
+                /* preserves entries with a class value of 0 */
+                continue;
+            }
+        }
+
+        slb->esid &= ~SLB_ESID_V;
+    }
 }
 
 static void __helper_slbie(CPUPPCState *env, target_ulong addr,
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 127c82a24e..b207fb5386 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4997,9 +4997,12 @@ static void gen_slbia(DisasContext *ctx)
 #if defined(CONFIG_USER_ONLY)
     GEN_PRIV;
 #else
+    uint32_t ih = (ctx->opcode >> 21) & 0x7;
+    TCGv_i32 t0 = tcg_const_i32(ih);
+
     CHK_SV;
 
-    gen_helper_slbia(cpu_env);
+    gen_helper_slbia(cpu_env, t0);
 #endif /* defined(CONFIG_USER_ONLY) */
 }
 
-- 
2.25.1



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

* [PULL 4/7] target/ppc: don't byte swap ELFv2 signal handler
  2020-03-24  5:14 [PULL 0/7] ppc-for-5.0 queue 20200324 David Gibson
                   ` (2 preceding siblings ...)
  2020-03-24  5:14 ` [PULL 3/7] target/ppc: Fix ISA v3.0 (POWER9) slbia implementation David Gibson
@ 2020-03-24  5:14 ` David Gibson
  2020-03-24  5:14 ` [PULL 5/7] spapr: Fix memory leak in h_client_architecture_support() David Gibson
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2020-03-24  5:14 UTC (permalink / raw)
  To: peter.maydell
  Cc: aik, Richard Henderson, qemu-devel, groug, qemu-ppc, clg,
	David Gibson, Vincent Fazio, Laurent Vivier

From: Vincent Fazio <vfazio@gmail.com>

Previously, the signal handler would be byte swapped if the target and
host CPU used different endianness. This would cause a SIGSEGV when
attempting to translate the opcode pointed to by the swapped address.

 Thread 1 "qemu-ppc64" received signal SIGSEGV, Segmentation fault.
 0x00000000600a9257 in ldl_he_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:351
 351        __builtin_memcpy(&r, ptr, sizeof(r));

 #0  0x00000000600a9257 in ldl_he_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:351
 #1  0x00000000600a92fe in ldl_be_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:449
 #2  0x00000000600c0790 in translator_ldl_swap at qemu/include/exec/translator.h:201
 #3  0x000000006011c1ab in ppc_tr_translate_insn at qemu/target/ppc/translate.c:7856
 #4  0x000000006005ae70 in translator_loop at qemu/accel/tcg/translator.c:102

The signal handler will be byte swapped as a result of the __get_user()
call in sigaction() if it is necessary, no additional swap is required.

Signed-off-by: Vincent Fazio <vfazio@gmail.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200319133244.8818-1-vfazio@xes-inc.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 linux-user/ppc/signal.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
index 0c4e7ba54c..ecd99736b7 100644
--- a/linux-user/ppc/signal.c
+++ b/linux-user/ppc/signal.c
@@ -567,10 +567,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
         env->nip = tswapl(handler->entry);
         env->gpr[2] = tswapl(handler->toc);
     } else {
-        /* ELFv2 PPC64 function pointers are entry points, but R12
-         * must also be set */
-        env->nip = tswapl((target_ulong) ka->_sa_handler);
-        env->gpr[12] = env->nip;
+        /* ELFv2 PPC64 function pointers are entry points. R12 must also be set. */
+        env->gpr[12] = env->nip = ka->_sa_handler;
     }
 #else
     env->nip = (target_ulong) ka->_sa_handler;
-- 
2.25.1



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

* [PULL 5/7] spapr: Fix memory leak in h_client_architecture_support()
  2020-03-24  5:14 [PULL 0/7] ppc-for-5.0 queue 20200324 David Gibson
                   ` (3 preceding siblings ...)
  2020-03-24  5:14 ` [PULL 4/7] target/ppc: don't byte swap ELFv2 signal handler David Gibson
@ 2020-03-24  5:14 ` David Gibson
  2020-03-24  5:14 ` [PULL 6/7] hw/ppc: Take QEMU lock when calling ppc_dcr_read/write() David Gibson
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2020-03-24  5:14 UTC (permalink / raw)
  To: peter.maydell
  Cc: aik, qemu-devel, groug, qemu-ppc, clg,
	Philippe Mathieu-Daudé,
	David Gibson

From: Greg Kurz <groug@kaod.org>

This is the only error path that needs to free the previously allocated
ov1.

Reported-by: Coverity (CID 1421924)
Fixes: cbd0d7f36322 "spapr: Fail CAS if option vector table cannot be parsed"
Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <158481206205.336182.16106097429336044843.stgit@bahia.lan>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/ppc/spapr_hcall.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 40c86e91eb..0d50fc9117 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1726,6 +1726,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
     }
     ov5_guest = spapr_ovec_parse_vector(ov_table, 5);
     if (!ov5_guest) {
+        spapr_ovec_cleanup(ov1_guest);
         warn_report("guest didn't provide option vector 5");
         return H_PARAMETER;
     }
-- 
2.25.1



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

* [PULL 6/7] hw/ppc: Take QEMU lock when calling ppc_dcr_read/write()
  2020-03-24  5:14 [PULL 0/7] ppc-for-5.0 queue 20200324 David Gibson
                   ` (4 preceding siblings ...)
  2020-03-24  5:14 ` [PULL 5/7] spapr: Fix memory leak in h_client_architecture_support() David Gibson
@ 2020-03-24  5:14 ` David Gibson
  2020-03-24  5:14 ` [PULL 7/7] ppc/ppc405_boards: Remove unnecessary NULL check David Gibson
  2020-03-24 11:07 ` [PULL 0/7] ppc-for-5.0 queue 20200324 Peter Maydell
  7 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2020-03-24  5:14 UTC (permalink / raw)
  To: peter.maydell
  Cc: aik, Amit Lazar, qemu-devel, groug, qemu-ppc, clg, David Gibson

From: Peter Maydell <peter.maydell@linaro.org>

The ppc_dcr_read() and ppc_dcr_write() functions call into callbacks
in device code, so we need to hold the QEMU iothread lock while
calling them.  This is the case already for the callsites in
kvmppc_handle_dcr_read/write(), but we must also take the lock when
calling the helpers from TCG.

This fixes a bug where attempting to initialise the PPC405EP
SDRAM will cause an assertion when sdram_map_bcr() attempts
to remap memory regions.

Reported-by: Amit Lazar <abasarlaz@hotmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20200322192258.14039-1-peter.maydell@linaro.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target/ppc/timebase_helper.c | 40 +++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c
index 703bd9ed18..d16360ab66 100644
--- a/target/ppc/timebase_helper.c
+++ b/target/ppc/timebase_helper.c
@@ -21,6 +21,7 @@
 #include "exec/helper-proto.h"
 #include "exec/exec-all.h"
 #include "qemu/log.h"
+#include "qemu/main-loop.h"
 
 /*****************************************************************************/
 /* SPR accesses */
@@ -167,13 +168,19 @@ target_ulong helper_load_dcr(CPUPPCState *env, target_ulong dcrn)
         raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
                                POWERPC_EXCP_INVAL |
                                POWERPC_EXCP_INVAL_INVAL, GETPC());
-    } else if (unlikely(ppc_dcr_read(env->dcr_env,
-                                     (uint32_t)dcrn, &val) != 0)) {
-        qemu_log_mask(LOG_GUEST_ERROR, "DCR read error %d %03x\n",
-                      (uint32_t)dcrn, (uint32_t)dcrn);
-        raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
-                               POWERPC_EXCP_INVAL |
-                               POWERPC_EXCP_PRIV_REG, GETPC());
+    } else {
+        int ret;
+
+        qemu_mutex_lock_iothread();
+        ret = ppc_dcr_read(env->dcr_env, (uint32_t)dcrn, &val);
+        qemu_mutex_unlock_iothread();
+        if (unlikely(ret != 0)) {
+            qemu_log_mask(LOG_GUEST_ERROR, "DCR read error %d %03x\n",
+                          (uint32_t)dcrn, (uint32_t)dcrn);
+            raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
+                                   POWERPC_EXCP_INVAL |
+                                   POWERPC_EXCP_PRIV_REG, GETPC());
+        }
     }
     return val;
 }
@@ -185,12 +192,17 @@ void helper_store_dcr(CPUPPCState *env, target_ulong dcrn, target_ulong val)
         raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
                                POWERPC_EXCP_INVAL |
                                POWERPC_EXCP_INVAL_INVAL, GETPC());
-    } else if (unlikely(ppc_dcr_write(env->dcr_env, (uint32_t)dcrn,
-                                      (uint32_t)val) != 0)) {
-        qemu_log_mask(LOG_GUEST_ERROR, "DCR write error %d %03x\n",
-                      (uint32_t)dcrn, (uint32_t)dcrn);
-        raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
-                               POWERPC_EXCP_INVAL |
-                               POWERPC_EXCP_PRIV_REG, GETPC());
+    } else {
+        int ret;
+        qemu_mutex_lock_iothread();
+        ret = ppc_dcr_write(env->dcr_env, (uint32_t)dcrn, (uint32_t)val);
+        qemu_mutex_unlock_iothread();
+        if (unlikely(ret != 0)) {
+            qemu_log_mask(LOG_GUEST_ERROR, "DCR write error %d %03x\n",
+                          (uint32_t)dcrn, (uint32_t)dcrn);
+            raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
+                                   POWERPC_EXCP_INVAL |
+                                   POWERPC_EXCP_PRIV_REG, GETPC());
+        }
     }
 }
-- 
2.25.1



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

* [PULL 7/7] ppc/ppc405_boards: Remove unnecessary NULL check
  2020-03-24  5:14 [PULL 0/7] ppc-for-5.0 queue 20200324 David Gibson
                   ` (5 preceding siblings ...)
  2020-03-24  5:14 ` [PULL 6/7] hw/ppc: Take QEMU lock when calling ppc_dcr_read/write() David Gibson
@ 2020-03-24  5:14 ` David Gibson
  2020-03-24 11:07 ` [PULL 0/7] ppc-for-5.0 queue 20200324 Peter Maydell
  7 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2020-03-24  5:14 UTC (permalink / raw)
  To: peter.maydell
  Cc: aik, Markus Armbruster, qemu-devel, groug, qemu-ppc, clg,
	Philippe Mathieu-Daudé,
	David Gibson

From: Philippe Mathieu-Daudé <philmd@redhat.com>

This code is inside the "if (dinfo)" condition, so testing
again here whether it is NULL is unnecessary.

Fixes: dd59bcae7 (Don't size flash memory to match backing image)
Reported-by: Coverity (CID 1421917)
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200320155740.5342-1-philmd@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/ppc405_boards.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index e6bffb9e1a..6198ec1035 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -191,7 +191,7 @@ static void ref405ep_init(MachineState *machine)
         bios_size = 8 * MiB;
         pflash_cfi02_register((uint32_t)(-bios_size),
                               "ef405ep.bios", bios_size,
-                              dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                              blk_by_legacy_dinfo(dinfo),
                               64 * KiB, 1,
                               2, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
                               1);
@@ -459,7 +459,7 @@ static void taihu_405ep_init(MachineState *machine)
         bios_size = 2 * MiB;
         pflash_cfi02_register(0xFFE00000,
                               "taihu_405ep.bios", bios_size,
-                              dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                              blk_by_legacy_dinfo(dinfo),
                               64 * KiB, 1,
                               4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
                               1);
@@ -494,7 +494,7 @@ static void taihu_405ep_init(MachineState *machine)
     if (dinfo) {
         bios_size = 32 * MiB;
         pflash_cfi02_register(0xfc000000, "taihu_405ep.flash", bios_size,
-                              dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                              blk_by_legacy_dinfo(dinfo),
                               64 * KiB, 1,
                               4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
                               1);
-- 
2.25.1



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

* Re: [PULL 0/7] ppc-for-5.0 queue 20200324
  2020-03-24  5:14 [PULL 0/7] ppc-for-5.0 queue 20200324 David Gibson
                   ` (6 preceding siblings ...)
  2020-03-24  5:14 ` [PULL 7/7] ppc/ppc405_boards: Remove unnecessary NULL check David Gibson
@ 2020-03-24 11:07 ` Peter Maydell
  7 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2020-03-24 11:07 UTC (permalink / raw)
  To: David Gibson
  Cc: QEMU Developers, Alexey Kardashevskiy, qemu-ppc, Greg Kurz,
	Cédric Le Goater

On Tue, 24 Mar 2020 at 05:15, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> The following changes since commit c532b954d96f96d361ca31308f75f1b95bd4df76:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200323' into staging (2020-03-23 17:41:21 +0000)
>
> are available in the Git repository at:
>
>   git://github.com/dgibson/qemu.git tags/ppc-for-5.0-20200324
>
> for you to fetch changes up to 1583794b9b36911df116cc726750dadbeeac506a:
>
>   ppc/ppc405_boards: Remove unnecessary NULL check (2020-03-24 11:56:37 +1100)
>
> ----------------------------------------------------------------
> ppc patch queue for 2020-03-24
>
> Here's a final pull request before the qemu-5.0 hard freeze.
>
> We have an implementation of the POWER9 forms of the slbia
> instruction, a small cleanup and a handful of assorted fixes.
>


Applied, thanks.

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

-- PMM


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

end of thread, other threads:[~2020-03-24 11:08 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-24  5:14 [PULL 0/7] ppc-for-5.0 queue 20200324 David Gibson
2020-03-24  5:14 ` [PULL 1/7] ppc/spapr: Set the effective address provided flag in mc error log David Gibson
2020-03-24  5:14 ` [PULL 2/7] target/ppc: Fix slbia TLB invalidation gap David Gibson
2020-03-24  5:14 ` [PULL 3/7] target/ppc: Fix ISA v3.0 (POWER9) slbia implementation David Gibson
2020-03-24  5:14 ` [PULL 4/7] target/ppc: don't byte swap ELFv2 signal handler David Gibson
2020-03-24  5:14 ` [PULL 5/7] spapr: Fix memory leak in h_client_architecture_support() David Gibson
2020-03-24  5:14 ` [PULL 6/7] hw/ppc: Take QEMU lock when calling ppc_dcr_read/write() David Gibson
2020-03-24  5:14 ` [PULL 7/7] ppc/ppc405_boards: Remove unnecessary NULL check David Gibson
2020-03-24 11:07 ` [PULL 0/7] ppc-for-5.0 queue 20200324 Peter Maydell

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.