[v5,4/4] spapr: Implement H_JOIN

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 v4:
- Style

 hw/ppc/spapr.c       |  1 +
 hw/ppc/spapr_hcall.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

On Wed, 17 Jul 2019 15:39:52 +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>
> ---
> Changes since v4:
> - Style
> 
>  hw/ppc/spapr.c       |  1 +
>  hw/ppc/spapr_hcall.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 13c423347e..59cd24f9c3 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 5e655172b2..57c1ee0fe1 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1069,6 +1069,48 @@ 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;
> +    SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> +    bool last_unjoined = true;
> +
> +    if (env->msr & (1ULL << MSR_EE)) {
> +        return H_BAD_MODE;
> +    }
> +
> +    if (spapr_cpu->prod) {
> +        spapr_cpu->prod = false;
> +        return H_SUCCESS;
> +    }
> +

PAPR says that H_JOIN "performs the equivalent of a H_CONFER (proc=self)",
unless called by the last unjoined thread, in which case H_CONTINUE
should be returned. It thus seems that the spapr_cpu->prod check should
be done after the loop below otherwise if the last active thread was
just prodded (can happen?), it won't return the expected value, and...

> +    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;
> +    }
> +
> +    cs = CPU(cpu);
> +    cs->halted = 1;
> +    cs->exception_index = EXCP_HALTED;
> +    cs->exit_request = 1;
> +
> +    return H_SUCCESS;

... then, you can maybe factor out this code to an h_confer_self()
helper to be called by h_join() and h_confer() ?

> +}
> +
>  static target_ulong h_confer(PowerPCCPU *cpu, SpaprMachineState *spapr,
>                             target_ulong opcode, target_ulong *args)
>  {
> @@ -1979,6 +2021,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 */

Greg Kurz's on July 18, 2019 3:30 am:
> On Wed, 17 Jul 2019 15:39:52 +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>
>> ---
>> Changes since v4:
>> - Style
>> 
>>  hw/ppc/spapr.c       |  1 +
>>  hw/ppc/spapr_hcall.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 46 insertions(+)
>> 
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 13c423347e..59cd24f9c3 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 5e655172b2..57c1ee0fe1 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1069,6 +1069,48 @@ 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;
>> +    SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
>> +    bool last_unjoined = true;
>> +
>> +    if (env->msr & (1ULL << MSR_EE)) {
>> +        return H_BAD_MODE;
>> +    }
>> +
>> +    if (spapr_cpu->prod) {
>> +        spapr_cpu->prod = false;
>> +        return H_SUCCESS;
>> +    }
>> +
> 
> PAPR says that H_JOIN "performs the equivalent of a H_CONFER (proc=self)",
> unless called by the last unjoined thread, in which case H_CONTINUE
> should be returned. It thus seems that the spapr_cpu->prod check should
> be done after the loop below otherwise if the last active thread was
> just prodded (can happen?), it won't return the expected value, and...

Good lawyering, I would say you are right.

>> +    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;
>> +    }
>> +
>> +    cs = CPU(cpu);
>> +    cs->halted = 1;
>> +    cs->exception_index = EXCP_HALTED;
>> +    cs->exit_request = 1;
>> +
>> +    return H_SUCCESS;
> 
> ... then, you can maybe factor out this code to an h_confer_self()
> helper to be called by h_join() and h_confer() ?

I'll take a look.

Thanks,
Nick