[Qemu-devel] [PATCH v6 0/4] spapr: implement dispatch and suspend calls

[Qemu-devel] [PATCH v6 0/4] spapr: implement dispatch and suspend calls

[Nicholas Piggin]

Lot of good reviews (thanks), several more bug fixes and some good
cleanups.

Thanks,
Nick

Nicholas Piggin (4):
  spapr: Implement dispatch tracking for tcg
  spapr: Implement H_PROD
  spapr: Implement H_CONFER
  spapr: Implement H_JOIN

 hw/ppc/spapr.c                  |  54 ++++++++++++
 hw/ppc/spapr_hcall.c            | 152 ++++++++++++++++++++++++++++++--
 include/hw/ppc/spapr.h          |   7 ++
 include/hw/ppc/spapr_cpu_core.h |   1 +
 target/ppc/cpu.h                |   4 +
 target/ppc/translate_init.inc.c |  27 ++++++
 6 files changed, 240 insertions(+), 5 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH v6 1/4] spapr: Implement dispatch tracking for tcg

[Nicholas Piggin]

Implement cpu_exec_enter/exit on ppc which calls into new methods of
the same name in PPCVirtualHypervisorClass. These are used by spapr
to implement the splpar VPA dispatch counter initially.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Changes since v5:
- Move 'prod' into next patch.
- Use uint32_t type for dispatch counter.
- Add guest error message for incorrect dispatch counter.
- Conditionally compile away if CONFIG_USER_ONLY
- Small cleanups

Changes since v4:
- Store to VPA on the way out as well.
- Increment the dispatch counter directly in the VPA, which means it will
  migrate with guest memory the same as KVM.
- Prod need not be migrated, add a comment.

 hw/ppc/spapr.c                  | 52 +++++++++++++++++++++++++++++++++
 hw/ppc/spapr_hcall.c            |  5 ----
 include/hw/ppc/spapr.h          |  7 +++++
 target/ppc/cpu.h                |  4 +++
 target/ppc/translate_init.inc.c | 27 +++++++++++++++++
 5 files changed, 90 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 821f0d4a49..3e5678d467 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4302,6 +4302,54 @@ PowerPCCPU *spapr_find_cpu(int vcpu_id)
     return NULL;
 }
 
+#ifndef CONFIG_USER_ONLY
+static void spapr_cpu_exec_enter(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
+{
+    SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
+
+    /* These are only called by TCG, KVM maintains dispatch state */
+
+    if (spapr_cpu->vpa_addr) {
+        CPUState *cs = CPU(cpu);
+        uint32_t dispatch;
+
+        dispatch = ldl_be_phys(cs->as,
+                               spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER);
+        dispatch++;
+        if ((dispatch & 1) != 0) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "VPA: incorrect dispatch counter value for "
+                          "dispatched partition %u, correcting.\n", dispatch);
+            dispatch++;
+        }
+        stl_be_phys(cs->as,
+                    spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER, dispatch);
+    }
+}
+
+static void spapr_cpu_exec_exit(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
+{
+    SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
+
+    if (spapr_cpu->vpa_addr) {
+        CPUState *cs = CPU(cpu);
+        uint32_t dispatch;
+
+        dispatch = ldl_be_phys(cs->as,
+                               spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER);
+        dispatch++;
+        if ((dispatch & 1) != 1) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "VPA: incorrect dispatch counter value for "
+                          "preempted partition %u, correcting.\n", dispatch);
+            dispatch++;
+        }
+        stl_be_phys(cs->as,
+                    spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER, dispatch);
+    }
+}
+#endif
+
 static void spapr_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -4358,6 +4406,10 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     vhc->hpte_set_r = spapr_hpte_set_r;
     vhc->get_pate = spapr_get_pate;
     vhc->encode_hpt_for_kvm_pr = spapr_encode_hpt_for_kvm_pr;
+#ifndef CONFIG_USER_ONLY
+    vhc->cpu_exec_enter = spapr_cpu_exec_enter;
+    vhc->cpu_exec_exit = spapr_cpu_exec_exit;
+#endif
     xic->ics_get = spapr_ics_get;
     xic->ics_resend = spapr_ics_resend;
     xic->icp_get = spapr_icp_get;
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 6808d4cda8..e615881ac4 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -874,11 +874,6 @@ unmap_out:
 #define FLAGS_DEREGISTER_DTL       0x0000c00000000000ULL
 #define FLAGS_DEREGISTER_SLBSHADOW 0x0000e00000000000ULL
 
-#define VPA_MIN_SIZE           640
-#define VPA_SIZE_OFFSET        0x4
-#define VPA_SHARED_PROC_OFFSET 0x9
-#define VPA_SHARED_PROC_VAL    0x2
-
 static target_ulong register_vpa(PowerPCCPU *cpu, target_ulong vpa)
 {
     CPUState *cs = CPU(cpu);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 60553d32c4..5d36eec9d0 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -525,6 +525,13 @@ void spapr_register_hypercall(target_ulong opcode, spapr_hcall_fn fn);
 target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
                              target_ulong *args);
 
+/* Virtual Processor Area structure constants */
+#define VPA_MIN_SIZE           640
+#define VPA_SIZE_OFFSET        0x4
+#define VPA_SHARED_PROC_OFFSET 0x9
+#define VPA_SHARED_PROC_VAL    0x2
+#define VPA_DISPATCH_COUNTER   0x100
+
 /* ibm,set-eeh-option */
 #define RTAS_EEH_DISABLE                 0
 #define RTAS_EEH_ENABLE                  1
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index c9beba2a5c..9e8fd3c621 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1224,6 +1224,10 @@ struct PPCVirtualHypervisorClass {
     void (*hpte_set_r)(PPCVirtualHypervisor *vhyp, hwaddr ptex, uint64_t pte1);
     void (*get_pate)(PPCVirtualHypervisor *vhyp, ppc_v3_pate_t *entry);
     target_ulong (*encode_hpt_for_kvm_pr)(PPCVirtualHypervisor *vhyp);
+#ifndef CONFIG_USER_ONLY
+    void (*cpu_exec_enter)(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu);
+    void (*cpu_exec_exit)(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu);
+#endif
 };
 
 #define TYPE_PPC_VIRTUAL_HYPERVISOR "ppc-virtual-hypervisor"
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index 86fc8f2e31..bae4820503 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -10471,6 +10471,28 @@ static bool ppc_cpu_is_big_endian(CPUState *cs)
 
     return !msr_le;
 }
+
+static void ppc_cpu_exec_enter(CPUState *cs)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+
+    if (cpu->vhyp) {
+        PPCVirtualHypervisorClass *vhc =
+            PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+        vhc->cpu_exec_enter(cpu->vhyp, cpu);
+    }
+}
+
+static void ppc_cpu_exec_exit(CPUState *cs)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+
+    if (cpu->vhyp) {
+        PPCVirtualHypervisorClass *vhc =
+            PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+        vhc->cpu_exec_exit(cpu->vhyp, cpu);
+    }
+}
 #endif
 
 static void ppc_cpu_instance_init(Object *obj)
@@ -10624,6 +10646,11 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
     cc->tcg_initialize = ppc_translate_init;
     cc->tlb_fill = ppc_cpu_tlb_fill;
 #endif
+#ifndef CONFIG_USER_ONLY
+    cc->cpu_exec_enter = ppc_cpu_exec_enter;
+    cc->cpu_exec_exit = ppc_cpu_exec_exit;
+#endif
+
     cc->disas_set_info = ppc_disas_set_info;
 
     dc->fw_name = "PowerPC,UNKNOWN";
-- 
2.20.1

[Qemu-devel] [PATCH v6 2/4] spapr: Implement H_PROD

[Nicholas Piggin]

H_PROD is added, and H_CEDE is modified to test the prod bit
according to PAPR.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Changes since v5:
- Add the prod bit here
- Fix target CPU

 hw/ppc/spapr.c                  |  1 +
 hw/ppc/spapr_hcall.c            | 32 ++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr_cpu_core.h |  1 +
 3 files changed, 34 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3e5678d467..68341c128d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4309,6 +4309,7 @@ static void spapr_cpu_exec_enter(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
 
     /* These are only called by TCG, KVM maintains dispatch state */
 
+    spapr_cpu->prod = false;
     if (spapr_cpu->vpa_addr) {
         CPUState *cs = CPU(cpu);
         uint32_t dispatch;
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index e615881ac4..098b3dda22 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1050,14 +1050,44 @@ static target_ulong h_cede(PowerPCCPU *cpu, SpaprMachineState *spapr,
 {
     CPUPPCState *env = &cpu->env;
     CPUState *cs = CPU(cpu);
+    SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
 
     env->msr |= (1ULL << MSR_EE);
     hreg_compute_hflags(env);
+
+    if (spapr_cpu->prod) {
+        spapr_cpu->prod = false;
+        return H_SUCCESS;
+    }
+
     if (!cpu_has_work(cs)) {
         cs->halted = 1;
         cs->exception_index = EXCP_HLT;
         cs->exit_request = 1;
     }
+
+    return H_SUCCESS;
+}
+
+static target_ulong h_prod(PowerPCCPU *cpu, SpaprMachineState *spapr,
+                           target_ulong opcode, target_ulong *args)
+{
+    target_long target = args[0];
+    PowerPCCPU *tcpu;
+    CPUState *cs;
+    SpaprCpuState *spapr_cpu;
+
+    tcpu = spapr_find_cpu(target);
+    cs = CPU(tcpu);
+    if (!cs) {
+        return H_PARAMETER;
+    }
+
+    spapr_cpu = spapr_cpu_state(tcpu);
+    spapr_cpu->prod = true;
+    cs->halted = 0;
+    qemu_cpu_kick(cs);
+
     return H_SUCCESS;
 }
 
@@ -1882,6 +1912,8 @@ static void hypercall_register_types(void)
     /* hcall-splpar */
     spapr_register_hypercall(H_REGISTER_VPA, h_register_vpa);
     spapr_register_hypercall(H_CEDE, h_cede);
+    spapr_register_hypercall(H_PROD, h_prod);
+
     spapr_register_hypercall(H_SIGNAL_SYS_RESET, h_signal_sys_reset);
 
     /* processor register resource access h-calls */
diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
index f9645a7290..a40cd08ea0 100644
--- a/include/hw/ppc/spapr_cpu_core.h
+++ b/include/hw/ppc/spapr_cpu_core.h
@@ -46,6 +46,7 @@ typedef struct SpaprCpuState {
     uint64_t vpa_addr;
     uint64_t slb_shadow_addr, slb_shadow_size;
     uint64_t dtl_addr, dtl_size;
+    bool prod; /* not migrated, only used to improve dispatch latencies */
     struct ICPState *icp;
     struct XiveTCTX *tctx;
 } SpaprCpuState;
-- 
2.20.1

[Qemu-devel] [PATCH v6 3/4] spapr: Implement H_CONFER

[Nicholas Piggin]

This does not do directed yielding and is not quite as strict as PAPR
specifies in terms of precise dispatch behaviour. This generally will
mean suboptimal performance, rather than guest misbehaviour. Linux
does not rely on exact dispatch behaviour.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Changes since v5:
- Cleanups

Changes since v4:
- Style, added justification comments, spelling.
- Fixed trying to dereference spapr_cpu for a -1 target.

 hw/ppc/spapr_hcall.c | 67 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 098b3dda22..7c659dc75c 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1069,6 +1069,72 @@ static target_ulong h_cede(PowerPCCPU *cpu, SpaprMachineState *spapr,
     return H_SUCCESS;
 }
 
+static target_ulong h_confer(PowerPCCPU *cpu, SpaprMachineState *spapr,
+                           target_ulong opcode, target_ulong *args)
+{
+    target_long target = args[0];
+    uint32_t dispatch = args[1];
+    CPUState *cs = CPU(cpu);
+    SpaprCpuState *spapr_cpu;
+
+    /*
+     * -1 means confer to all other CPUs without dispatch counter check,
+     *  otherwise it's a targeted confer.
+     */
+    if (target != -1) {
+        PowerPCCPU *target_cpu = spapr_find_cpu(target);
+        uint32_t target_dispatch;
+
+        if (!target_cpu) {
+            return H_PARAMETER;
+        }
+
+        spapr_cpu = spapr_cpu_state(target_cpu);
+
+        /*
+         * target == self is a special case, we wait until prodded, without
+         * dispatch counter check.
+         */
+        if (cpu == target_cpu) {
+            if (spapr_cpu->prod) {
+                spapr_cpu->prod = false;
+
+                return H_SUCCESS;
+            }
+
+            cs->halted = 1;
+            cs->exception_index = EXCP_HALTED;
+            cs->exit_request = 1;
+
+            return H_SUCCESS;
+        }
+
+        if (!spapr_cpu->vpa_addr || ((dispatch & 1) == 0)) {
+            return H_SUCCESS;
+        }
+
+        target_dispatch = ldl_be_phys(cs->as,
+                                  spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER);
+        if (target_dispatch != dispatch) {
+            return H_SUCCESS;
+        }
+
+        /*
+         * The targeted confer does not do anything special beyond yielding
+         * the current vCPU, but even this should be better than nothing.
+         * At least for single-threaded tcg, it gives the target a chance to
+         * run before we run again. Multi-threaded tcg does not really do
+         * anything with EXCP_YIELD yet.
+         */
+    }
+
+    cs->exception_index = EXCP_YIELD;
+    cs->exit_request = 1;
+    cpu_loop_exit(cs);
+
+    return H_SUCCESS;
+}
+
 static target_ulong h_prod(PowerPCCPU *cpu, SpaprMachineState *spapr,
                            target_ulong opcode, target_ulong *args)
 {
@@ -1912,6 +1978,7 @@ static void hypercall_register_types(void)
     /* hcall-splpar */
     spapr_register_hypercall(H_REGISTER_VPA, h_register_vpa);
     spapr_register_hypercall(H_CEDE, h_cede);
+    spapr_register_hypercall(H_CONFER, h_confer);
     spapr_register_hypercall(H_PROD, h_prod);
 
     spapr_register_hypercall(H_SIGNAL_SYS_RESET, h_signal_sys_reset);
-- 
2.20.1

[Qemu-devel] [PATCH v6 4/4] spapr: Implement H_JOIN

[Nicholas Piggin]

This has been useful to modify and test the Linux pseries suspend
code but it requires modification to the guest to call it (due to
being gated by other unimplemented features). It is not otherwise
used by Linux yet, but work is slowly progressing there.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Changes since v5:
- Fix prod bit semantics.
- Factor out the h_confer_self common code

Changes since v4:
- Style

 hw/ppc/spapr.c       |  1 +
 hw/ppc/spapr_hcall.c | 74 ++++++++++++++++++++++++++++++++++++--------
 2 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 68341c128d..00f7735a31 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1066,6 +1066,7 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
     add_str(hypertas, "hcall-tce");
     add_str(hypertas, "hcall-vio");
     add_str(hypertas, "hcall-splpar");
+    add_str(hypertas, "hcall-join");
     add_str(hypertas, "hcall-bulk");
     add_str(hypertas, "hcall-set-mode");
     add_str(hypertas, "hcall-sprg0");
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 7c659dc75c..9b72ea8b68 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1069,6 +1069,62 @@ static target_ulong h_cede(PowerPCCPU *cpu, SpaprMachineState *spapr,
     return H_SUCCESS;
 }
 
+/*
+ * Confer to self, aka join. Cede could use the same pattern as well, if
+ * EXCP_HLT can be changed to ECXP_HALTED.
+ */
+static target_ulong h_confer_self(PowerPCCPU *cpu)
+{
+    CPUState *cs = CPU(cpu);
+    SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
+
+    if (spapr_cpu->prod) {
+        spapr_cpu->prod = false;
+        return H_SUCCESS;
+    }
+    cs->halted = 1;
+    cs->exception_index = EXCP_HALTED;
+    cs->exit_request = 1;
+
+    return H_SUCCESS;
+}
+
+static target_ulong h_join(PowerPCCPU *cpu, SpaprMachineState *spapr,
+                           target_ulong opcode, target_ulong *args)
+{
+    CPUPPCState *env = &cpu->env;
+    CPUState *cs;
+    bool last_unjoined = true;
+
+    if (env->msr & (1ULL << MSR_EE)) {
+        return H_BAD_MODE;
+    }
+
+    /*
+     * Must not join the last CPU running. Interestingly, no such restriction
+     * for H_CONFER-to-self, but that is probably not intended to be used
+     * when H_JOIN is available.
+     */
+    CPU_FOREACH(cs) {
+        PowerPCCPU *c = POWERPC_CPU(cs);
+        CPUPPCState *e = &c->env;
+        if (c == cpu) {
+            continue;
+        }
+
+        /* Don't have a way to indicate joined, so use halted && MSR[EE]=0 */
+        if (!cs->halted || (e->msr & (1ULL << MSR_EE))) {
+            last_unjoined = false;
+            break;
+        }
+    }
+    if (last_unjoined) {
+        return H_CONTINUE;
+    }
+
+    return h_confer_self(cpu);
+}
+
 static target_ulong h_confer(PowerPCCPU *cpu, SpaprMachineState *spapr,
                            target_ulong opcode, target_ulong *args)
 {
@@ -1089,26 +1145,15 @@ static target_ulong h_confer(PowerPCCPU *cpu, SpaprMachineState *spapr,
             return H_PARAMETER;
         }
 
-        spapr_cpu = spapr_cpu_state(target_cpu);
-
         /*
          * target == self is a special case, we wait until prodded, without
          * dispatch counter check.
          */
         if (cpu == target_cpu) {
-            if (spapr_cpu->prod) {
-                spapr_cpu->prod = false;
-
-                return H_SUCCESS;
-            }
-
-            cs->halted = 1;
-            cs->exception_index = EXCP_HALTED;
-            cs->exit_request = 1;
-
-            return H_SUCCESS;
+            return h_confer_self(cpu);
         }
 
+        spapr_cpu = spapr_cpu_state(target_cpu);
         if (!spapr_cpu->vpa_addr || ((dispatch & 1) == 0)) {
             return H_SUCCESS;
         }
@@ -1981,6 +2026,9 @@ static void hypercall_register_types(void)
     spapr_register_hypercall(H_CONFER, h_confer);
     spapr_register_hypercall(H_PROD, h_prod);
 
+    /* hcall-join */
+    spapr_register_hypercall(H_JOIN, h_join);
+
     spapr_register_hypercall(H_SIGNAL_SYS_RESET, h_signal_sys_reset);
 
     /* processor register resource access h-calls */
-- 
2.20.1

Re: [Qemu-devel] [PATCH v6 0/4] spapr: implement dispatch and suspend calls

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 380 bytes --]

On Thu, Jul 18, 2019 at 01:42:10PM +1000, Nicholas Piggin wrote:
> Lot of good reviews (thanks), several more bug fixes and some good
> cleanups.

Applied to ppc-for-4.2, thanks.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH v6 1/4] spapr: Implement dispatch tracking for tcg

[Greg Kurz]

On Thu, 18 Jul 2019 13:42:11 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:

> Implement cpu_exec_enter/exit on ppc which calls into new methods of
> the same name in PPCVirtualHypervisorClass. These are used by spapr
> to implement the splpar VPA dispatch counter initially.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> Changes since v5:
> - Move 'prod' into next patch.
> - Use uint32_t type for dispatch counter.
> - Add guest error message for incorrect dispatch counter.
> - Conditionally compile away if CONFIG_USER_ONLY
> - Small cleanups
> 
> Changes since v4:
> - Store to VPA on the way out as well.
> - Increment the dispatch counter directly in the VPA, which means it will
>   migrate with guest memory the same as KVM.
> - Prod need not be migrated, add a comment.
> 
>  hw/ppc/spapr.c                  | 52 +++++++++++++++++++++++++++++++++
>  hw/ppc/spapr_hcall.c            |  5 ----
>  include/hw/ppc/spapr.h          |  7 +++++
>  target/ppc/cpu.h                |  4 +++
>  target/ppc/translate_init.inc.c | 27 +++++++++++++++++
>  5 files changed, 90 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 821f0d4a49..3e5678d467 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4302,6 +4302,54 @@ PowerPCCPU *spapr_find_cpu(int vcpu_id)
>      return NULL;
>  }
>  
> +#ifndef CONFIG_USER_ONLY

This file is for system emulation only, no need to guard here.

This is minor and rest looks good.

Reviewed-by: Greg Kurz <groug@kaod.org>

> +static void spapr_cpu_exec_enter(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
> +{
> +    SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> +
> +    /* These are only called by TCG, KVM maintains dispatch state */
> +
> +    if (spapr_cpu->vpa_addr) {
> +        CPUState *cs = CPU(cpu);
> +        uint32_t dispatch;
> +
> +        dispatch = ldl_be_phys(cs->as,
> +                               spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER);
> +        dispatch++;
> +        if ((dispatch & 1) != 0) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "VPA: incorrect dispatch counter value for "
> +                          "dispatched partition %u, correcting.\n", dispatch);
> +            dispatch++;
> +        }
> +        stl_be_phys(cs->as,
> +                    spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER, dispatch);
> +    }
> +}
> +
> +static void spapr_cpu_exec_exit(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
> +{
> +    SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> +
> +    if (spapr_cpu->vpa_addr) {
> +        CPUState *cs = CPU(cpu);
> +        uint32_t dispatch;
> +
> +        dispatch = ldl_be_phys(cs->as,
> +                               spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER);
> +        dispatch++;
> +        if ((dispatch & 1) != 1) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "VPA: incorrect dispatch counter value for "
> +                          "preempted partition %u, correcting.\n", dispatch);
> +            dispatch++;
> +        }
> +        stl_be_phys(cs->as,
> +                    spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER, dispatch);
> +    }
> +}
> +#endif
> +
>  static void spapr_machine_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> @@ -4358,6 +4406,10 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>      vhc->hpte_set_r = spapr_hpte_set_r;
>      vhc->get_pate = spapr_get_pate;
>      vhc->encode_hpt_for_kvm_pr = spapr_encode_hpt_for_kvm_pr;
> +#ifndef CONFIG_USER_ONLY
> +    vhc->cpu_exec_enter = spapr_cpu_exec_enter;
> +    vhc->cpu_exec_exit = spapr_cpu_exec_exit;
> +#endif
>      xic->ics_get = spapr_ics_get;
>      xic->ics_resend = spapr_ics_resend;
>      xic->icp_get = spapr_icp_get;
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 6808d4cda8..e615881ac4 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -874,11 +874,6 @@ unmap_out:
>  #define FLAGS_DEREGISTER_DTL       0x0000c00000000000ULL
>  #define FLAGS_DEREGISTER_SLBSHADOW 0x0000e00000000000ULL
>  
> -#define VPA_MIN_SIZE           640
> -#define VPA_SIZE_OFFSET        0x4
> -#define VPA_SHARED_PROC_OFFSET 0x9
> -#define VPA_SHARED_PROC_VAL    0x2
> -
>  static target_ulong register_vpa(PowerPCCPU *cpu, target_ulong vpa)
>  {
>      CPUState *cs = CPU(cpu);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 60553d32c4..5d36eec9d0 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -525,6 +525,13 @@ void spapr_register_hypercall(target_ulong opcode, spapr_hcall_fn fn);
>  target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
>                               target_ulong *args);
>  
> +/* Virtual Processor Area structure constants */
> +#define VPA_MIN_SIZE           640
> +#define VPA_SIZE_OFFSET        0x4
> +#define VPA_SHARED_PROC_OFFSET 0x9
> +#define VPA_SHARED_PROC_VAL    0x2
> +#define VPA_DISPATCH_COUNTER   0x100
> +
>  /* ibm,set-eeh-option */
>  #define RTAS_EEH_DISABLE                 0
>  #define RTAS_EEH_ENABLE                  1
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index c9beba2a5c..9e8fd3c621 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1224,6 +1224,10 @@ struct PPCVirtualHypervisorClass {
>      void (*hpte_set_r)(PPCVirtualHypervisor *vhyp, hwaddr ptex, uint64_t pte1);
>      void (*get_pate)(PPCVirtualHypervisor *vhyp, ppc_v3_pate_t *entry);
>      target_ulong (*encode_hpt_for_kvm_pr)(PPCVirtualHypervisor *vhyp);
> +#ifndef CONFIG_USER_ONLY
> +    void (*cpu_exec_enter)(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu);
> +    void (*cpu_exec_exit)(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu);
> +#endif
>  };
>  
>  #define TYPE_PPC_VIRTUAL_HYPERVISOR "ppc-virtual-hypervisor"
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index 86fc8f2e31..bae4820503 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -10471,6 +10471,28 @@ static bool ppc_cpu_is_big_endian(CPUState *cs)
>  
>      return !msr_le;
>  }
> +
> +static void ppc_cpu_exec_enter(CPUState *cs)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +
> +    if (cpu->vhyp) {
> +        PPCVirtualHypervisorClass *vhc =
> +            PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> +        vhc->cpu_exec_enter(cpu->vhyp, cpu);
> +    }
> +}
> +
> +static void ppc_cpu_exec_exit(CPUState *cs)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +
> +    if (cpu->vhyp) {
> +        PPCVirtualHypervisorClass *vhc =
> +            PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> +        vhc->cpu_exec_exit(cpu->vhyp, cpu);
> +    }
> +}
>  #endif
>  
>  static void ppc_cpu_instance_init(Object *obj)
> @@ -10624,6 +10646,11 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
>      cc->tcg_initialize = ppc_translate_init;
>      cc->tlb_fill = ppc_cpu_tlb_fill;
>  #endif
> +#ifndef CONFIG_USER_ONLY
> +    cc->cpu_exec_enter = ppc_cpu_exec_enter;
> +    cc->cpu_exec_exit = ppc_cpu_exec_exit;
> +#endif
> +
>      cc->disas_set_info = ppc_disas_set_info;
>  
>      dc->fw_name = "PowerPC,UNKNOWN";

Re: [Qemu-devel] [PATCH v6 2/4] spapr: Implement H_PROD

[Greg Kurz]

On Thu, 18 Jul 2019 13:42:12 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:

> H_PROD is added, and H_CEDE is modified to test the prod bit
> according to PAPR.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

> Changes since v5:
> - Add the prod bit here
> - Fix target CPU
> 
>  hw/ppc/spapr.c                  |  1 +
>  hw/ppc/spapr_hcall.c            | 32 ++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr_cpu_core.h |  1 +
>  3 files changed, 34 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3e5678d467..68341c128d 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4309,6 +4309,7 @@ static void spapr_cpu_exec_enter(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
>  
>      /* These are only called by TCG, KVM maintains dispatch state */
>  
> +    spapr_cpu->prod = false;
>      if (spapr_cpu->vpa_addr) {
>          CPUState *cs = CPU(cpu);
>          uint32_t dispatch;
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index e615881ac4..098b3dda22 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1050,14 +1050,44 @@ static target_ulong h_cede(PowerPCCPU *cpu, SpaprMachineState *spapr,
>  {
>      CPUPPCState *env = &cpu->env;
>      CPUState *cs = CPU(cpu);
> +    SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
>  
>      env->msr |= (1ULL << MSR_EE);
>      hreg_compute_hflags(env);
> +
> +    if (spapr_cpu->prod) {
> +        spapr_cpu->prod = false;
> +        return H_SUCCESS;
> +    }
> +
>      if (!cpu_has_work(cs)) {
>          cs->halted = 1;
>          cs->exception_index = EXCP_HLT;
>          cs->exit_request = 1;
>      }
> +
> +    return H_SUCCESS;
> +}
> +
> +static target_ulong h_prod(PowerPCCPU *cpu, SpaprMachineState *spapr,
> +                           target_ulong opcode, target_ulong *args)
> +{
> +    target_long target = args[0];
> +    PowerPCCPU *tcpu;
> +    CPUState *cs;
> +    SpaprCpuState *spapr_cpu;
> +
> +    tcpu = spapr_find_cpu(target);
> +    cs = CPU(tcpu);
> +    if (!cs) {
> +        return H_PARAMETER;
> +    }
> +
> +    spapr_cpu = spapr_cpu_state(tcpu);
> +    spapr_cpu->prod = true;
> +    cs->halted = 0;
> +    qemu_cpu_kick(cs);
> +
>      return H_SUCCESS;
>  }
>  
> @@ -1882,6 +1912,8 @@ static void hypercall_register_types(void)
>      /* hcall-splpar */
>      spapr_register_hypercall(H_REGISTER_VPA, h_register_vpa);
>      spapr_register_hypercall(H_CEDE, h_cede);
> +    spapr_register_hypercall(H_PROD, h_prod);
> +
>      spapr_register_hypercall(H_SIGNAL_SYS_RESET, h_signal_sys_reset);
>  
>      /* processor register resource access h-calls */
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index f9645a7290..a40cd08ea0 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -46,6 +46,7 @@ typedef struct SpaprCpuState {
>      uint64_t vpa_addr;
>      uint64_t slb_shadow_addr, slb_shadow_size;
>      uint64_t dtl_addr, dtl_size;
> +    bool prod; /* not migrated, only used to improve dispatch latencies */
>      struct ICPState *icp;
>      struct XiveTCTX *tctx;
>  } SpaprCpuState;

Re: [Qemu-devel] [PATCH v6 3/4] spapr: Implement H_CONFER

[Greg Kurz]

On Thu, 18 Jul 2019 13:42:13 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:

> This does not do directed yielding and is not quite as strict as PAPR
> specifies in terms of precise dispatch behaviour. This generally will
> mean suboptimal performance, rather than guest misbehaviour. Linux
> does not rely on exact dispatch behaviour.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

> Changes since v5:
> - Cleanups
> 
> Changes since v4:
> - Style, added justification comments, spelling.
> - Fixed trying to dereference spapr_cpu for a -1 target.
> 
>  hw/ppc/spapr_hcall.c | 67 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 098b3dda22..7c659dc75c 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1069,6 +1069,72 @@ static target_ulong h_cede(PowerPCCPU *cpu, SpaprMachineState *spapr,
>      return H_SUCCESS;
>  }
>  
> +static target_ulong h_confer(PowerPCCPU *cpu, SpaprMachineState *spapr,
> +                           target_ulong opcode, target_ulong *args)
> +{
> +    target_long target = args[0];
> +    uint32_t dispatch = args[1];
> +    CPUState *cs = CPU(cpu);
> +    SpaprCpuState *spapr_cpu;
> +
> +    /*
> +     * -1 means confer to all other CPUs without dispatch counter check,
> +     *  otherwise it's a targeted confer.
> +     */
> +    if (target != -1) {
> +        PowerPCCPU *target_cpu = spapr_find_cpu(target);
> +        uint32_t target_dispatch;
> +
> +        if (!target_cpu) {
> +            return H_PARAMETER;
> +        }
> +
> +        spapr_cpu = spapr_cpu_state(target_cpu);
> +
> +        /*
> +         * target == self is a special case, we wait until prodded, without
> +         * dispatch counter check.
> +         */
> +        if (cpu == target_cpu) {
> +            if (spapr_cpu->prod) {
> +                spapr_cpu->prod = false;
> +
> +                return H_SUCCESS;
> +            }
> +
> +            cs->halted = 1;
> +            cs->exception_index = EXCP_HALTED;
> +            cs->exit_request = 1;
> +
> +            return H_SUCCESS;
> +        }
> +
> +        if (!spapr_cpu->vpa_addr || ((dispatch & 1) == 0)) {
> +            return H_SUCCESS;
> +        }
> +
> +        target_dispatch = ldl_be_phys(cs->as,
> +                                  spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER);
> +        if (target_dispatch != dispatch) {
> +            return H_SUCCESS;
> +        }
> +
> +        /*
> +         * The targeted confer does not do anything special beyond yielding
> +         * the current vCPU, but even this should be better than nothing.
> +         * At least for single-threaded tcg, it gives the target a chance to
> +         * run before we run again. Multi-threaded tcg does not really do
> +         * anything with EXCP_YIELD yet.
> +         */
> +    }
> +
> +    cs->exception_index = EXCP_YIELD;
> +    cs->exit_request = 1;
> +    cpu_loop_exit(cs);
> +
> +    return H_SUCCESS;
> +}
> +
>  static target_ulong h_prod(PowerPCCPU *cpu, SpaprMachineState *spapr,
>                             target_ulong opcode, target_ulong *args)
>  {
> @@ -1912,6 +1978,7 @@ static void hypercall_register_types(void)
>      /* hcall-splpar */
>      spapr_register_hypercall(H_REGISTER_VPA, h_register_vpa);
>      spapr_register_hypercall(H_CEDE, h_cede);
> +    spapr_register_hypercall(H_CONFER, h_confer);
>      spapr_register_hypercall(H_PROD, h_prod);
>  
>      spapr_register_hypercall(H_SIGNAL_SYS_RESET, h_signal_sys_reset);

Re: [Qemu-devel] [PATCH v6 4/4] spapr: Implement H_JOIN

[Greg Kurz]

On Thu, 18 Jul 2019 13:42:14 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:

> This has been useful to modify and test the Linux pseries suspend
> code but it requires modification to the guest to call it (due to
> being gated by other unimplemented features). It is not otherwise
> used by Linux yet, but work is slowly progressing there.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

> Changes since v5:
> - Fix prod bit semantics.
> - Factor out the h_confer_self common code
> 
> Changes since v4:
> - Style
> 
>  hw/ppc/spapr.c       |  1 +
>  hw/ppc/spapr_hcall.c | 74 ++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 62 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 68341c128d..00f7735a31 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1066,6 +1066,7 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
>      add_str(hypertas, "hcall-tce");
>      add_str(hypertas, "hcall-vio");
>      add_str(hypertas, "hcall-splpar");
> +    add_str(hypertas, "hcall-join");
>      add_str(hypertas, "hcall-bulk");
>      add_str(hypertas, "hcall-set-mode");
>      add_str(hypertas, "hcall-sprg0");
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 7c659dc75c..9b72ea8b68 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1069,6 +1069,62 @@ static target_ulong h_cede(PowerPCCPU *cpu, SpaprMachineState *spapr,
>      return H_SUCCESS;
>  }
>  
> +/*
> + * Confer to self, aka join. Cede could use the same pattern as well, if
> + * EXCP_HLT can be changed to ECXP_HALTED.
> + */
> +static target_ulong h_confer_self(PowerPCCPU *cpu)
> +{
> +    CPUState *cs = CPU(cpu);
> +    SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> +
> +    if (spapr_cpu->prod) {
> +        spapr_cpu->prod = false;
> +        return H_SUCCESS;
> +    }
> +    cs->halted = 1;
> +    cs->exception_index = EXCP_HALTED;
> +    cs->exit_request = 1;
> +
> +    return H_SUCCESS;
> +}
> +
> +static target_ulong h_join(PowerPCCPU *cpu, SpaprMachineState *spapr,
> +                           target_ulong opcode, target_ulong *args)
> +{
> +    CPUPPCState *env = &cpu->env;
> +    CPUState *cs;
> +    bool last_unjoined = true;
> +
> +    if (env->msr & (1ULL << MSR_EE)) {
> +        return H_BAD_MODE;
> +    }
> +
> +    /*
> +     * Must not join the last CPU running. Interestingly, no such restriction
> +     * for H_CONFER-to-self, but that is probably not intended to be used
> +     * when H_JOIN is available.
> +     */
> +    CPU_FOREACH(cs) {
> +        PowerPCCPU *c = POWERPC_CPU(cs);
> +        CPUPPCState *e = &c->env;
> +        if (c == cpu) {
> +            continue;
> +        }
> +
> +        /* Don't have a way to indicate joined, so use halted && MSR[EE]=0 */
> +        if (!cs->halted || (e->msr & (1ULL << MSR_EE))) {
> +            last_unjoined = false;
> +            break;
> +        }
> +    }
> +    if (last_unjoined) {
> +        return H_CONTINUE;
> +    }
> +
> +    return h_confer_self(cpu);
> +}
> +
>  static target_ulong h_confer(PowerPCCPU *cpu, SpaprMachineState *spapr,
>                             target_ulong opcode, target_ulong *args)
>  {
> @@ -1089,26 +1145,15 @@ static target_ulong h_confer(PowerPCCPU *cpu, SpaprMachineState *spapr,
>              return H_PARAMETER;
>          }
>  
> -        spapr_cpu = spapr_cpu_state(target_cpu);
> -
>          /*
>           * target == self is a special case, we wait until prodded, without
>           * dispatch counter check.
>           */
>          if (cpu == target_cpu) {
> -            if (spapr_cpu->prod) {
> -                spapr_cpu->prod = false;
> -
> -                return H_SUCCESS;
> -            }
> -
> -            cs->halted = 1;
> -            cs->exception_index = EXCP_HALTED;
> -            cs->exit_request = 1;
> -
> -            return H_SUCCESS;
> +            return h_confer_self(cpu);
>          }
>  
> +        spapr_cpu = spapr_cpu_state(target_cpu);
>          if (!spapr_cpu->vpa_addr || ((dispatch & 1) == 0)) {
>              return H_SUCCESS;
>          }
> @@ -1981,6 +2026,9 @@ static void hypercall_register_types(void)
>      spapr_register_hypercall(H_CONFER, h_confer);
>      spapr_register_hypercall(H_PROD, h_prod);
>  
> +    /* hcall-join */
> +    spapr_register_hypercall(H_JOIN, h_join);
> +
>      spapr_register_hypercall(H_SIGNAL_SYS_RESET, h_signal_sys_reset);
>  
>      /* processor register resource access h-calls */

Re: [Qemu-devel] [PATCH v6 1/4] spapr: Implement dispatch tracking for tcg

[Nicholas Piggin]

Greg Kurz's on July 18, 2019 8:13 pm:
> On Thu, 18 Jul 2019 13:42:11 +1000
> Nicholas Piggin <npiggin@gmail.com> wrote:
> 
>> Implement cpu_exec_enter/exit on ppc which calls into new methods of
>> the same name in PPCVirtualHypervisorClass. These are used by spapr
>> to implement the splpar VPA dispatch counter initially.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> Changes since v5:
>> - Move 'prod' into next patch.
>> - Use uint32_t type for dispatch counter.
>> - Add guest error message for incorrect dispatch counter.
>> - Conditionally compile away if CONFIG_USER_ONLY
>> - Small cleanups
>> 
>> Changes since v4:
>> - Store to VPA on the way out as well.
>> - Increment the dispatch counter directly in the VPA, which means it will
>>   migrate with guest memory the same as KVM.
>> - Prod need not be migrated, add a comment.
>> 
>>  hw/ppc/spapr.c                  | 52 +++++++++++++++++++++++++++++++++
>>  hw/ppc/spapr_hcall.c            |  5 ----
>>  include/hw/ppc/spapr.h          |  7 +++++
>>  target/ppc/cpu.h                |  4 +++
>>  target/ppc/translate_init.inc.c | 27 +++++++++++++++++
>>  5 files changed, 90 insertions(+), 5 deletions(-)
>> 
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 821f0d4a49..3e5678d467 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -4302,6 +4302,54 @@ PowerPCCPU *spapr_find_cpu(int vcpu_id)
>>      return NULL;
>>  }
>>  
>> +#ifndef CONFIG_USER_ONLY
> 
> This file is for system emulation only, no need to guard here.
> 
> This is minor and rest looks good.
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>

Thanks for all the reviews Greg (and others), they made significant 
improvements. David I'm not sure how you like to work with merging
patches, but if you can add the Reviewed-by tags and this issue, then
I won't have to resend.

Or if you prefer I can fix it up and send to you privately to reduce
list spam.

Thanks,
Nick

Re: [Qemu-devel] [PATCH v6 1/4] spapr: Implement dispatch tracking for tcg

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2405 bytes --]

On Fri, Jul 19, 2019 at 09:12:16AM +1000, Nicholas Piggin wrote:
> Greg Kurz's on July 18, 2019 8:13 pm:
> > On Thu, 18 Jul 2019 13:42:11 +1000
> > Nicholas Piggin <npiggin@gmail.com> wrote:
> > 
> >> Implement cpu_exec_enter/exit on ppc which calls into new methods of
> >> the same name in PPCVirtualHypervisorClass. These are used by spapr
> >> to implement the splpar VPA dispatch counter initially.
> >> 
> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >> ---
> >> Changes since v5:
> >> - Move 'prod' into next patch.
> >> - Use uint32_t type for dispatch counter.
> >> - Add guest error message for incorrect dispatch counter.
> >> - Conditionally compile away if CONFIG_USER_ONLY
> >> - Small cleanups
> >> 
> >> Changes since v4:
> >> - Store to VPA on the way out as well.
> >> - Increment the dispatch counter directly in the VPA, which means it will
> >>   migrate with guest memory the same as KVM.
> >> - Prod need not be migrated, add a comment.
> >> 
> >>  hw/ppc/spapr.c                  | 52 +++++++++++++++++++++++++++++++++
> >>  hw/ppc/spapr_hcall.c            |  5 ----
> >>  include/hw/ppc/spapr.h          |  7 +++++
> >>  target/ppc/cpu.h                |  4 +++
> >>  target/ppc/translate_init.inc.c | 27 +++++++++++++++++
> >>  5 files changed, 90 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index 821f0d4a49..3e5678d467 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -4302,6 +4302,54 @@ PowerPCCPU *spapr_find_cpu(int vcpu_id)
> >>      return NULL;
> >>  }
> >>  
> >> +#ifndef CONFIG_USER_ONLY
> > 
> > This file is for system emulation only, no need to guard here.
> > 
> > This is minor and rest looks good.
> > 
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> Thanks for all the reviews Greg (and others), they made significant 
> improvements. David I'm not sure how you like to work with merging
> patches, but if you can add the Reviewed-by tags and this issue, then
> I won't have to resend.

I've fixed it up in my tree.

> 
> Or if you prefer I can fix it up and send to you privately to reduce
> list spam.
> 
> Thanks,
> Nick
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]