On Wed, Aug 10, 2016 at 11:59:36AM +1000, Suraj Jitindar Singh wrote: > Add the lib/powerpc/smp.c file and associated header files as a place > to implement generic smp functionality for inclusion in tests. > > Add functions start_all_cpus(), start_cpu() and start_thread() to start > all stopped threads of all cpus, all stopped threads of a single cpu or a > single stopped thread of a guest at a given execution location, > respectively. > > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@xxxxxxxxx> > --- > lib/powerpc/asm/smp.h | 15 +++++++ > lib/powerpc/smp.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++ > lib/ppc64/asm/smp.h | 1 + > powerpc/Makefile.common | 1 + > 4 files changed, 132 insertions(+) > create mode 100644 lib/powerpc/asm/smp.h > create mode 100644 lib/powerpc/smp.c > create mode 100644 lib/ppc64/asm/smp.h > > diff --git a/lib/powerpc/asm/smp.h b/lib/powerpc/asm/smp.h > new file mode 100644 > index 0000000..a4f3e7f > --- /dev/null > +++ b/lib/powerpc/asm/smp.h > @@ -0,0 +1,15 @@ > +#ifndef _ASMPOWERPC_SMP_H_ > +#define _ASMPOWERPC_SMP_H_ > + > +#include <libcflat.h> > +#include <stdint.h> nit: no need to include stdint.h, libcflat.h already does, so most files neglect it. > + > +typedef void (*secondary_entry_fn)(void); > + > +extern void halt(void); > + > +bool start_thread(int cpu_id, secondary_entry_fn entry, uint32_t r3); > +bool start_cpu(int cpu_node, secondary_entry_fn entry, uint32_t r3); > +bool start_all_cpus(secondary_entry_fn entry, uint32_t r3); nit: kvm-unit-tests likes to use 'extern' on function declarations > + > +#endif /* _ASMPOWERPC_SMP_H_ */ > diff --git a/lib/powerpc/smp.c b/lib/powerpc/smp.c > new file mode 100644 > index 0000000..8968907 > --- /dev/null > +++ b/lib/powerpc/smp.c > @@ -0,0 +1,115 @@ > +/* > + * Secondary cpu support > + * > + * Copyright 2016 Suraj Jitindar Singh, IBM. > + * > + * This work is licensed under the terms of the GNU LGPL, version 2. > + */ > + > +#include <stdint.h> same nit as above > +#include <libfdt/libfdt.h> > +#include <libfdt/fdt.h> nit: only need libfdt/libfdt.h, as that includes fdt.h > +#include <devicetree.h> > +#include <libcflat.h> > +#include <string.h> > +#include <asm/rtas.h> > +#include <asm/smp.h> > + > +struct secondary_entry_data { > + secondary_entry_fn entry; > + uint64_t r3; > + bool init_failed; > +}; > + > +/* > + * Start stopped thread cpu_id at entry > + * Returns: 1 on success or cpu not in stopped state > + * 0 on failure to start stopped cpu Returns: TRUE ... FALSE ... > + * > + * Note: This function returns 1 on success in starting a stopped cpu or if the returns true > + * given cpu was not in the stopped state. Thus this can be called on a > + * list of cpus and all the stopped ones will be started while false > + * won't be returned if some cpus in that list were already running. Thus > + * the user should check that cpus passed to this function are already in > + * the stopped state if they want to guarantee that a return value of > + * true corresponds to the given cpu now executing at entry. This > + * function checks again however as calling cpu-start on a not stopped > + * cpu results in undefined behaviour. > + */ > +bool start_thread(int cpu_id, secondary_entry_fn entry, uint32_t r3) > +{ > + int query_token, start_token, outputs[1], ret; > + > + query_token = rtas_token("query-cpu-stopped-state"); > + start_token = rtas_token("start-cpu"); > + assert(query_token != RTAS_UNKNOWN_SERVICE && > + start_token != RTAS_UNKNOWN_SERVICE); > + > + ret = rtas_call(query_token, 1, 2, outputs, cpu_id); > + if (ret) { > + printf("query-cpu-stopped-state failed for cpu %d\n", cpu_id); > + return false; > + } > + > + if (!outputs[0]) { /* cpu in stopped state */ > + ret = rtas_call(start_token, 3, 1, NULL, cpu_id, entry, r3); > + if (ret) { > + printf("failed to start cpu %d\n", cpu_id); > + return false; > + } > + } > + > + return true; > +} > + > +/* > + * Start all stopped threads (vcpus) on cpu_node > + * Returns: 1 on success > + * 0 on failure TRUE/FALSE, but I'd actually change the return type to struct start_threads; struct start_threads { int nr_threads; int nr_started; }; Then... > + */ > +bool start_cpu(int cpu_node, secondary_entry_fn entry, uint32_t r3) > +{ > + const struct fdt_property *prop; > + int len, nr_cpu, cpu; > + u32 *cpus; > + bool ret = true; > + > + /* Get the id array of threads on this cpu_node */ > + prop = fdt_get_property(dt_fdt(), cpu_node, > + "ibm,ppc-interrupt-server#s", &len); > + assert(prop); > + > + nr_cpu = len >> 2; /* Divide by 4 since 4 bytes per cpu */ > + cpus = (u32 *)prop->data; /* Array of valid cpu numbers */ > + > + for (cpu = 0; cpu < nr_cpu && ret; cpu++) > + ret = start_thread(fdt32_to_cpu(cpus[cpu]), entry, r3); > + return ret; ...change this to nr_threads = len >> 2; /* Divide by 4 since 4 bytes per cpu */ cpus = (u32 *)prop->data; /* Array of valid cpu numbers */ for (cpu = 0; cpu < nr_threads; cpu++) nr_started += start_thread(fdt32_to_cpu(cpus[cpu]), entry, r3); return (struct start_threads){ nr_threads, nr_started }; and... > +} > + > +static void start_each_secondary(int fdtnode, u32 regval __unused, void *info) > +{ > + struct secondary_entry_data *datap = info; > + > + datap->init_failed |= !start_cpu(fdtnode, datap->entry, datap->r3); > +} ...change init_failed to nr_started start_threads = start_cpu(fdtnode, datap->entry, datap->r3); nr_cpus += start_threads.nr_threads; // nr_cpus is global like ARM has datap->nr_started += start_threads.nr_started; and below just check that datap->nr_started == nr_cpus. > + > +/* > + * Start all stopped cpus on the guest at entry with register 3 set to r3 > + * Returns: 1 on success > + * 0 on failure TRUE/FALSE > + */ > +bool start_all_cpus(secondary_entry_fn entry, uint32_t r3) > +{ > + struct secondary_entry_data data = { > + entry, > + r3, > + false > + }; > + int ret; > + > + ret = dt_for_each_cpu_node(&start_each_secondary, (void *) &data); assert(ret == 0) > + > + return !(ret || data.init_failed); See comment above about setting nr_cpus, and then just confirming they all started here instead. > +} > diff --git a/lib/ppc64/asm/smp.h b/lib/ppc64/asm/smp.h > new file mode 100644 > index 0000000..67ced75 > --- /dev/null > +++ b/lib/ppc64/asm/smp.h > @@ -0,0 +1 @@ > +#include "../../powerpc/asm/smp.h" > diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common > index 404194b..677030a 100644 > --- a/powerpc/Makefile.common > +++ b/powerpc/Makefile.common > @@ -37,6 +37,7 @@ cflatobjs += lib/powerpc/setup.o > cflatobjs += lib/powerpc/rtas.o > cflatobjs += lib/powerpc/processor.o > cflatobjs += lib/powerpc/handlers.o > +cflatobjs += lib/powerpc/smp.o > > FLATLIBS = $(libcflat) $(LIBFDT_archive) > %.elf: CFLAGS += $(arch_CFLAGS) > -- > 2.5.5 Thanks, drew -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html