[Qemu-devel] [PATCH] ppc/spapr: Add H_PROD and H_CONFER

Cédric Le Goater posted 1 patch 5 years ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190411063408.3998-1-clg@kaod.org
Maintainers: David Gibson <david@gibson.dropbear.id.au>
target/ppc/cpu.h     |  3 +++
hw/ppc/spapr_hcall.c | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
[Qemu-devel] [PATCH] ppc/spapr: Add H_PROD and H_CONFER
Posted by Cédric Le Goater 5 years ago
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

H_PROD should be fully functional, H_CEDE is modified to
ignore a proded CPU. H_CONFER is a stub to avoid returning
an error.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target/ppc/cpu.h     |  3 +++
 hw/ppc/spapr_hcall.c | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 42eadc4b7997..07cea7268c0c 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1141,6 +1141,9 @@ struct CPUPPCState {
      * sreset), so flag this here.
      */
     bool resume_as_sreset;
+
+    /* Used by SPAPR for H_CEDE/H_PROD */
+    bool prodded;
 #endif
 
     /* Those resources are used only during code translation */
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 8a736797b9bf..be8044a92f2f 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1057,6 +1057,11 @@ static target_ulong h_cede(PowerPCCPU *cpu, SpaprMachineState *spapr,
 
     env->msr |= (1ULL << MSR_EE);
     hreg_compute_hflags(env);
+    if (env->prodded) {
+        env->prodded = false;
+        return H_SUCCESS;
+    }
+
     if (!cpu_has_work(cs)) {
         cs->halted = 1;
         cs->exception_index = EXCP_HLT;
@@ -1065,6 +1070,36 @@ static target_ulong h_cede(PowerPCCPU *cpu, SpaprMachineState *spapr,
     return H_SUCCESS;
 }
 
+static void spapr_do_wakeup_on_cpu(CPUState *cs, run_on_cpu_data data)
+{
+    cs->halted = 0;
+    cs->exception_index = -1;
+}
+
+static target_ulong h_prod(PowerPCCPU *cpu, SpaprMachineState *spapr,
+                           target_ulong opcode, target_ulong *args)
+{
+    target_ulong target = args[0];
+    CPUPPCState *env;
+
+    cpu = spapr_find_cpu(target);
+    if (cpu) {
+        env = &cpu->env;
+        env->prodded = true;
+        run_on_cpu(CPU(cpu), spapr_do_wakeup_on_cpu, RUN_ON_CPU_NULL);
+    } else {
+        return H_PARAMETER;
+    }
+    return H_SUCCESS;
+}
+
+static target_ulong h_confer(PowerPCCPU *cpu, SpaprMachineState *spapr,
+                             target_ulong opcode, target_ulong *args)
+{
+    /* Do nothing (supposed to confer timeslice). */
+    return H_SUCCESS;
+}
+
 static target_ulong h_rtas(PowerPCCPU *cpu, SpaprMachineState *spapr,
                            target_ulong opcode, target_ulong *args)
 {
@@ -1860,6 +1895,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_CONFER, h_confer);
     spapr_register_hypercall(H_SIGNAL_SYS_RESET, h_signal_sys_reset);
 
     /* processor register resource access h-calls */
-- 
2.20.1


Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc/spapr: Add H_PROD and H_CONFER
Posted by Greg Kurz 5 years ago
On Thu, 11 Apr 2019 08:34:08 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> H_PROD should be fully functional, H_CEDE is modified to
> ignore a proded CPU. H_CONFER is a stub to avoid returning
> an error.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  target/ppc/cpu.h     |  3 +++
>  hw/ppc/spapr_hcall.c | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 42eadc4b7997..07cea7268c0c 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1141,6 +1141,9 @@ struct CPUPPCState {
>       * sreset), so flag this here.
>       */
>      bool resume_as_sreset;
> +
> +    /* Used by SPAPR for H_CEDE/H_PROD */
> +    bool prodded;
>  #endif
>  
>      /* Those resources are used only during code translation */
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 8a736797b9bf..be8044a92f2f 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1057,6 +1057,11 @@ static target_ulong h_cede(PowerPCCPU *cpu, SpaprMachineState *spapr,
>  
>      env->msr |= (1ULL << MSR_EE);
>      hreg_compute_hflags(env);
> +    if (env->prodded) {
> +        env->prodded = false;
> +        return H_SUCCESS;
> +    }
> +

The "H_CEDE is modified to ignore a proded CPU" sentence in the changelog
is slightly inaccurate since the "prod" bit is reset, as described in
LoPAPR v1.1 14.11.3.3 H_CEDE.

Appart from that LGTM.

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

>      if (!cpu_has_work(cs)) {
>          cs->halted = 1;
>          cs->exception_index = EXCP_HLT;
> @@ -1065,6 +1070,36 @@ static target_ulong h_cede(PowerPCCPU *cpu, SpaprMachineState *spapr,
>      return H_SUCCESS;
>  }
>  
> +static void spapr_do_wakeup_on_cpu(CPUState *cs, run_on_cpu_data data)
> +{
> +    cs->halted = 0;
> +    cs->exception_index = -1;
> +}
> +
> +static target_ulong h_prod(PowerPCCPU *cpu, SpaprMachineState *spapr,
> +                           target_ulong opcode, target_ulong *args)
> +{
> +    target_ulong target = args[0];
> +    CPUPPCState *env;
> +
> +    cpu = spapr_find_cpu(target);
> +    if (cpu) {
> +        env = &cpu->env;
> +        env->prodded = true;
> +        run_on_cpu(CPU(cpu), spapr_do_wakeup_on_cpu, RUN_ON_CPU_NULL);
> +    } else {
> +        return H_PARAMETER;
> +    }
> +    return H_SUCCESS;
> +}
> +
> +static target_ulong h_confer(PowerPCCPU *cpu, SpaprMachineState *spapr,
> +                             target_ulong opcode, target_ulong *args)
> +{
> +    /* Do nothing (supposed to confer timeslice). */
> +    return H_SUCCESS;
> +}
> +
>  static target_ulong h_rtas(PowerPCCPU *cpu, SpaprMachineState *spapr,
>                             target_ulong opcode, target_ulong *args)
>  {
> @@ -1860,6 +1895,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_CONFER, h_confer);
>      spapr_register_hypercall(H_SIGNAL_SYS_RESET, h_signal_sys_reset);
>  
>      /* processor register resource access h-calls */


Re: [Qemu-devel] [PATCH] ppc/spapr: Add H_PROD and H_CONFER
Posted by Nicholas Piggin 5 years ago
Cédric Le Goater's on April 11, 2019 4:34 pm:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> H_PROD should be fully functional, H_CEDE is modified to
> ignore a proded CPU. H_CONFER is a stub to avoid returning
> an error.

Huh, I just implemented these a couple of days ago to test pseries
suspend, also H_JOIN.

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

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 8a736797b9..47ee933333 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1065,6 +1065,81 @@ static target_ulong h_cede(PowerPCCPU *cpu, SpaprMachineState *spapr,
     return H_SUCCESS;
 }
 
+static target_ulong h_join(PowerPCCPU *cpu, SpaprMachineState *spapr,
+                           target_ulong opcode, target_ulong *args)
+{
+    CPUPPCState *env = &cpu->env;
+    CPUState *cs = CPU(cpu);
+
+    if (env->msr & (1ULL << MSR_EE))
+        return H_BAD_MODE;
+
+    /*
+     * This should check for single-threaded mode. In practice, Linux
+     * does not try to H_JOIN all CPUs.
+     */
+
+    cs->halted = 1;
+    cs->exception_index = EXCP_HALTED;
+    cs->exit_request = 1;
+
+    return H_SUCCESS;
+}
+
+static target_ulong h_confer(PowerPCCPU *cpu, SpaprMachineState *spapr,
+                           target_ulong opcode, target_ulong *args)
+{
+    target_long target = args[0];
+    CPUState *cs = CPU(cpu);
+
+    /*
+     * This does not do a targeted yield, but check the parameter anyway.
+     */
+    if (target != -1 && !CPU(spapr_find_cpu(target)))
+        return H_PARAMETER;
+
+    /*
+     * H_CONFER with target == this is not exactly the same as H_JOIN
+     * according to PAPR (e.g., MSR[EE] check and single threaded check
+     * is not done in this case), but unlikely to matter.
+     */
+    if (cpu == spapr_find_cpu(target))
+        return h_join(cpu, spapr, opcode, args);
+
+    /*
+     * This does not implement the dispatch sequence check.
+     */
+    cs->exception_index = EXCP_YIELD;
+    cpu_loop_exit(cs);
+
+    return H_SUCCESS;
+}
+
+/*
+ * H_PROD and H_CONFER are specified to only modify GPR r3, which is not
+ * achievable running under KVM, although KVM already implements H_CONFER
+ * this way.
+ */
+static target_ulong h_prod(PowerPCCPU *cpu, SpaprMachineState *spapr,
+                           target_ulong opcode, target_ulong *args)
+{
+    target_long target = args[0];
+    CPUState *cs;
+
+    /*
+     * Should set the prod flag in the VPA.
+     */
+
+    cs = CPU(spapr_find_cpu(target));
+    if (!cs)
+        return H_PARAMETER;
+
+    cs->halted = 0;
+    qemu_cpu_kick(cs);
+
+    return H_SUCCESS;
+}
+
 static target_ulong h_rtas(PowerPCCPU *cpu, SpaprMachineState *spapr,
                            target_ulong opcode, target_ulong *args)
 {
@@ -1860,6 +1935,10 @@ 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_JOIN, h_join);
+    spapr_register_hypercall(H_PROD, h_prod);
+
     spapr_register_hypercall(H_SIGNAL_SYS_RESET, h_signal_sys_reset);
 
     /* processor register resource access h-calls */
-- 
2.20.1


Re: [Qemu-devel] [PATCH] ppc/spapr: Add H_PROD and H_CONFER
Posted by Cédric Le Goater 5 years ago
On 4/12/19 5:28 AM, Nicholas Piggin wrote:
> Cédric Le Goater's on April 11, 2019 4:34 pm:
>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>
>> H_PROD should be fully functional, H_CEDE is modified to
>> ignore a proded CPU. H_CONFER is a stub to avoid returning
>> an error.
> 
> Huh, I just implemented these a couple of days ago to test pseries
> suspend, also H_JOIN.

Cool. Can you send your version instead ?

Thanks,

C.

> 
> ---
>  hw/ppc/spapr_hcall.c | 79 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 8a736797b9..47ee933333 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1065,6 +1065,81 @@ static target_ulong h_cede(PowerPCCPU *cpu, SpaprMachineState *spapr,
>      return H_SUCCESS;
>  }
>  
> +static target_ulong h_join(PowerPCCPU *cpu, SpaprMachineState *spapr,
> +                           target_ulong opcode, target_ulong *args)
> +{
> +    CPUPPCState *env = &cpu->env;
> +    CPUState *cs = CPU(cpu);
> +
> +    if (env->msr & (1ULL << MSR_EE))
> +        return H_BAD_MODE;
> +
> +    /*
> +     * This should check for single-threaded mode. In practice, Linux
> +     * does not try to H_JOIN all CPUs.
> +     */
> +
> +    cs->halted = 1;
> +    cs->exception_index = EXCP_HALTED;
> +    cs->exit_request = 1;
> +
> +    return H_SUCCESS;
> +}
> +
> +static target_ulong h_confer(PowerPCCPU *cpu, SpaprMachineState *spapr,
> +                           target_ulong opcode, target_ulong *args)
> +{
> +    target_long target = args[0];
> +    CPUState *cs = CPU(cpu);
> +
> +    /*
> +     * This does not do a targeted yield, but check the parameter anyway.
> +     */
> +    if (target != -1 && !CPU(spapr_find_cpu(target)))
> +        return H_PARAMETER;
> +
> +    /*
> +     * H_CONFER with target == this is not exactly the same as H_JOIN
> +     * according to PAPR (e.g., MSR[EE] check and single threaded check
> +     * is not done in this case), but unlikely to matter.
> +     */
> +    if (cpu == spapr_find_cpu(target))
> +        return h_join(cpu, spapr, opcode, args);
> +
> +    /*
> +     * This does not implement the dispatch sequence check.
> +     */
> +    cs->exception_index = EXCP_YIELD;
> +    cpu_loop_exit(cs);
> +
> +    return H_SUCCESS;
> +}
> +
> +/*
> + * H_PROD and H_CONFER are specified to only modify GPR r3, which is not
> + * achievable running under KVM, although KVM already implements H_CONFER
> + * this way.
> + */
> +static target_ulong h_prod(PowerPCCPU *cpu, SpaprMachineState *spapr,
> +                           target_ulong opcode, target_ulong *args)
> +{
> +    target_long target = args[0];
> +    CPUState *cs;
> +
> +    /*
> +     * Should set the prod flag in the VPA.
> +     */
> +
> +    cs = CPU(spapr_find_cpu(target));
> +    if (!cs)
> +        return H_PARAMETER;
> +
> +    cs->halted = 0;
> +    qemu_cpu_kick(cs);
> +
> +    return H_SUCCESS;
> +}
> +
>  static target_ulong h_rtas(PowerPCCPU *cpu, SpaprMachineState *spapr,
>                             target_ulong opcode, target_ulong *args)
>  {
> @@ -1860,6 +1935,10 @@ 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_JOIN, h_join);
> +    spapr_register_hypercall(H_PROD, h_prod);
> +
>      spapr_register_hypercall(H_SIGNAL_SYS_RESET, h_signal_sys_reset);
>  
>      /* processor register resource access h-calls */
>