The unicast case in h_signal_sys_reset() seems to be broken:
rather than selecting the target CPU, it looks like it will pick
either the first CPU or fail to find one at all.

Fix it by using the search function rather than open coding the
search.

This was found by inspection; the code appears to be unused because
the Linux kernel only uses the broadcast target.

Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
 hw/ppc/spapr_hcall.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 72ea5a8247..07b3da8dc4 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1431,11 +1431,10 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
 
     } else {
         /* Unicast */
-        CPU_FOREACH(cs) {
-            if (cpu->cpu_dt_id == target) {
-                run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
-                return H_SUCCESS;
-            }
+        cs = CPU(ppc_get_vcpu_by_dt_id(target));
+        if (cs) {
+            run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
+            return H_SUCCESS;
         }
         return H_PARAMETER;
     }
-- 
2.12.1.382.gc0f9c7058

On Thu, 3 Aug 2017 16:28:27 +1000
Sam Bobroff <sam.bobroff@au1.ibm.com> wrote:

> The unicast case in h_signal_sys_reset() seems to be broken:
> rather than selecting the target CPU, it looks like it will pick
> either the first CPU or fail to find one at all.
> 
> Fix it by using the search function rather than open coding the
> search.
> 

Heh the open coded search is using cpu where it should have been using
POWERPC_CPU(cs) => it can only succeed if a the vCPU is signalling itself.

> This was found by inspection; the code appears to be unused because
> the Linux kernel only uses the broadcast target.
> 
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>

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

> ---
>  hw/ppc/spapr_hcall.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 72ea5a8247..07b3da8dc4 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1431,11 +1431,10 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
>  
>      } else {
>          /* Unicast */
> -        CPU_FOREACH(cs) {
> -            if (cpu->cpu_dt_id == target) {
> -                run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
> -                return H_SUCCESS;
> -            }
> +        cs = CPU(ppc_get_vcpu_by_dt_id(target));
> +        if (cs) {
> +            run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
> +            return H_SUCCESS;
>          }
>          return H_PARAMETER;
>      }

On Thu, Aug 03, 2017 at 02:37:15PM +0200, Greg Kurz wrote:
> On Thu, 3 Aug 2017 16:28:27 +1000
> Sam Bobroff <sam.bobroff@au1.ibm.com> wrote:
> 
> > The unicast case in h_signal_sys_reset() seems to be broken:
> > rather than selecting the target CPU, it looks like it will pick
> > either the first CPU or fail to find one at all.
> > 
> > Fix it by using the search function rather than open coding the
> > search.
> > 
> 
> Heh the open coded search is using cpu where it should have been using
> POWERPC_CPU(cs) => it can only succeed if a the vCPU is signalling itself.
> 
> > This was found by inspection; the code appears to be unused because
> > the Linux kernel only uses the broadcast target.
> > 
> > Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-2.11.

> 
> > ---
> >  hw/ppc/spapr_hcall.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index 72ea5a8247..07b3da8dc4 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -1431,11 +1431,10 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
> >  
> >      } else {
> >          /* Unicast */
> > -        CPU_FOREACH(cs) {
> > -            if (cpu->cpu_dt_id == target) {
> > -                run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
> > -                return H_SUCCESS;
> > -            }
> > +        cs = CPU(ppc_get_vcpu_by_dt_id(target));
> > +        if (cs) {
> > +            run_on_cpu(cs, spapr_do_system_reset_on_cpu, RUN_ON_CPU_NULL);
> > +            return H_SUCCESS;
> >          }
> >          return H_PARAMETER;
> >      }
> 



-- 
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