[PULL 0/3] ppc queue

[PULL 0/3] ppc queue

[Cédric Le Goater]

The following changes since commit bc6ec396d471d9e4aae7e2ff8b72e11da9a97665:

  Merge tag 'pull-request-2022-04-01' of https://gitlab.com/thuth/qemu into staging (2022-04-02 09:36:07 +0100)

are available in the Git repository at:

  https://github.com/legoater/qemu/ tags/pull-ppc-20220404

for you to fetch changes up to 0798da8df9fd917515c957ae918d6d979cf5f3fb:

  linux-user/ppc: Narrow type of ccr in save_user_regs (2022-04-04 08:49:06 +0200)

----------------------------------------------------------------
ppc-7.0 queue:

* Coverity fixes
* Fix for a memory leak issue

----------------------------------------------------------------
Daniel Henrique Barboza (1):
      hw/ppc: free env->tb_env in spapr_unrealize_vcpu()

Frederic Barrat (1):
      ppc/pnv: Fix number of registers in the PCIe controller on POWER9

Richard Henderson (1):
      linux-user/ppc: Narrow type of ccr in save_user_regs

 include/hw/pci-host/pnv_phb4.h | 2 +-
 include/hw/ppc/ppc.h           | 1 +
 hw/ppc/ppc.c                   | 7 +++++++
 hw/ppc/spapr_cpu_core.c        | 3 +++
 linux-user/ppc/signal.c        | 2 +-
 5 files changed, 13 insertions(+), 2 deletions(-)

[PULL 1/3] hw/ppc: free env->tb_env in spapr_unrealize_vcpu()

[Cédric Le Goater]

From: Daniel Henrique Barboza <danielhb413@gmail.com>

The timebase is allocated during spapr_realize_vcpu() and it's not
freed. This results in memory leaks when doing vcpu unplugs:

==636935==
==636935== 144 (96 direct, 48 indirect) bytes in 1 blocks are definitely lost in loss record 6
,461 of 8,135
==636935==    at 0x4897468: calloc (vg_replace_malloc.c:760)
==636935==    by 0x5077213: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6400.4)
==636935==    by 0x507757F: g_malloc0_n (in /usr/lib64/libglib-2.0.so.0.6400.4)
==636935==    by 0x93C3FB: cpu_ppc_tb_init (ppc.c:1066)
==636935==    by 0x97BC2B: spapr_realize_vcpu (spapr_cpu_core.c:268)
==636935==    by 0x97C01F: spapr_cpu_core_realize (spapr_cpu_core.c:337)
==636935==    by 0xD4626F: device_set_realized (qdev.c:531)
==636935==    by 0xD55273: property_set_bool (object.c:2273)
==636935==    by 0xD523DF: object_property_set (object.c:1408)
==636935==    by 0xD588B7: object_property_set_qobject (qom-qobject.c:28)
==636935==    by 0xD52897: object_property_set_bool (object.c:1477)
==636935==    by 0xD4579B: qdev_realize (qdev.c:333)
==636935==

This patch adds a cpu_ppc_tb_free() helper in hw/ppc/ppc.c to allow us
to free the timebase. This leak is then solved by calling
cpu_ppc_tb_free() in spapr_unrealize_vcpu().

Fixes: 6f4b5c3ec590 ("spapr: CPU hot unplug support")
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Message-Id: <20220329124545.529145-2-danielhb413@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/ppc.h    | 1 +
 hw/ppc/ppc.c            | 7 +++++++
 hw/ppc/spapr_cpu_core.c | 3 +++
 3 files changed, 11 insertions(+)

diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
index b0ba4bd6b978..364f165b4b56 100644
--- a/include/hw/ppc/ppc.h
+++ b/include/hw/ppc/ppc.h
@@ -54,6 +54,7 @@ struct ppc_tb_t {
 
 uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk, int64_t tb_offset);
 clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, uint32_t freq);
+void cpu_ppc_tb_free(CPUPPCState *env);
 void cpu_ppc_hdecr_init(CPUPPCState *env);
 void cpu_ppc_hdecr_exit(CPUPPCState *env);
 
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index faa02d6710c9..fea70df45e69 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -1083,6 +1083,13 @@ clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, uint32_t freq)
     return &cpu_ppc_set_tb_clk;
 }
 
+void cpu_ppc_tb_free(CPUPPCState *env)
+{
+    timer_free(env->tb_env->decr_timer);
+    timer_free(env->tb_env->hdecr_timer);
+    g_free(env->tb_env);
+}
+
 /* cpu_ppc_hdecr_init may be used if the timer is not used by HDEC emulation */
 void cpu_ppc_hdecr_init(CPUPPCState *env)
 {
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index ed847139602f..8a4861f45a2a 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -189,10 +189,13 @@ static const VMStateDescription vmstate_spapr_cpu_state = {
 
 static void spapr_unrealize_vcpu(PowerPCCPU *cpu, SpaprCpuCore *sc)
 {
+    CPUPPCState *env = &cpu->env;
+
     if (!sc->pre_3_0_migration) {
         vmstate_unregister(NULL, &vmstate_spapr_cpu_state, cpu->machine_data);
     }
     spapr_irq_cpu_intc_destroy(SPAPR_MACHINE(qdev_get_machine()), cpu);
+    cpu_ppc_tb_free(env);
     qdev_unrealize(DEVICE(cpu));
 }
 
-- 
2.34.1

[PULL 2/3] ppc/pnv: Fix number of registers in the PCIe controller on POWER9

[Cédric Le Goater]

From: Frederic Barrat <fbarrat@linux.ibm.com>

The spec defines 3 registers, even though only index 0 and 2 are valid
on POWER9. The same model is used on POWER10. Register 1 is defined
there but we currently don't use it in skiboot. So we can keep
reporting an error on write.

Reported by Coverity (CID 1487176).

Fixes: 4f9924c4d4cf ("ppc/pnv: Add models for POWER9 PHB4 PCIe Host bridge")
Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20220401091925.770803-1-fbarrat@linux.ibm.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/pci-host/pnv_phb4.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
index b02ecdceaa4c..19dcbd6f8727 100644
--- a/include/hw/pci-host/pnv_phb4.h
+++ b/include/hw/pci-host/pnv_phb4.h
@@ -180,7 +180,7 @@ struct PnvPhb4PecState {
     MemoryRegion nest_regs_mr;
 
     /* PCI registers, excluding per-stack */
-#define PHB4_PEC_PCI_REGS_COUNT     0x2
+#define PHB4_PEC_PCI_REGS_COUNT     0x3
     uint64_t pci_regs[PHB4_PEC_PCI_REGS_COUNT];
     MemoryRegion pci_regs_mr;
 
-- 
2.34.1

[PULL 3/3] linux-user/ppc: Narrow type of ccr in save_user_regs

[Cédric Le Goater]

From: Richard Henderson <richard.henderson@linaro.org>

Coverity warns that we shift a 32-bit value by N, and then
accumulate it into a 64-bit type (target_ulong on ppc64).

The ccr is always 8 * 4-bit fields, and thus is always a
32-bit quantity; narrow the type to avoid the warning.

Fixes: Coverity CID 1487223
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Message-Id: <20220401191643.330393-1-richard.henderson@linaro.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 linux-user/ppc/signal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
index ec0b9c0df3da..ce5a4682cdfd 100644
--- a/linux-user/ppc/signal.c
+++ b/linux-user/ppc/signal.c
@@ -229,7 +229,7 @@ static void save_user_regs(CPUPPCState *env, struct target_mcontext *frame)
 {
     target_ulong msr = env->msr;
     int i;
-    target_ulong ccr = 0;
+    uint32_t ccr = 0;
 
     /* In general, the kernel attempts to be intelligent about what it
        needs to save for Altivec/FP/SPE registers.  We don't care that
-- 
2.34.1

Re: [PULL 0/3] ppc queue

[Peter Maydell]

On Mon, 4 Apr 2022 at 15:38, Cédric Le Goater <clg@kaod.org> wrote:
>
> The following changes since commit bc6ec396d471d9e4aae7e2ff8b72e11da9a97665:
>
>   Merge tag 'pull-request-2022-04-01' of https://gitlab.com/thuth/qemu into staging (2022-04-02 09:36:07 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/legoater/qemu/ tags/pull-ppc-20220404
>
> for you to fetch changes up to 0798da8df9fd917515c957ae918d6d979cf5f3fb:
>
>   linux-user/ppc: Narrow type of ccr in save_user_regs (2022-04-04 08:49:06 +0200)
>
> ----------------------------------------------------------------
> ppc-7.0 queue:
>
> * Coverity fixes
> * Fix for a memory leak issue
>
> ----------------------------------------------------------------


Applied, thanks.

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

-- PMM