On Tue, Aug 08, 2017 at 04:24:01PM +1000, David Gibson wrote:
> On Mon, Aug 07, 2017 at 04:33:29PM +1000, Sam Bobroff wrote:
> > The concept of a VCPU ID that differs from the CPU's index
> > (cpu->cpu_index) exists only within SPAPR machines so, move the
> > functions ppc_get_vcpu_id() and ppc_get_cpu_by_vcpu_id() into spapr.c
> > and rename them appropriately.
> >
> > Signed-off-by: Sam Bobroff <sam.bobr...@au1.ibm.com>
> > ---
> > Changes in v2:
> >
> > * Re-arranged so that spapr_vcpu_id() calls kvm_arch_vcpu_id() rather than
> > the
> > other way around.
>
> Ah.. so close..
>
> [snip]
> > +int spapr_vcpu_id(PowerPCCPU *cpu)
> > +{
> > + CPUState *cs = CPU(cpu);
> > +
> > + if (kvm_enabled()) {
> > + return kvm_arch_vcpu_id(cs);
> > + } else {
> > + return cs->cpu_index;
> > + }
> > +}
> > +
> > +PowerPCCPU *spapr_find_cpu(int vcpu_id)
> > +{
> > + CPUState *cs;
> > +
> > + CPU_FOREACH(cs) {
> > + PowerPCCPU *cpu = POWERPC_CPU(cs);
> > +
> > + if (cpu->vcpu_id == vcpu_id) {
>
> This is still reaching into vcpu_id which should really be cpu
> private, not in spapr. You can use spapr_vcpu_id() which you already
> defined to avoid this.
OK I'll post another version.
> With that done, I believe it should be possible to put cpu->vcpu_id in
> an ifdef CONFIG_KVM and have things work.
That is indeed possible! ifdefs are needed around the initialization
block in ppc_cpu_realizefn(), although here in spapr_vcpu_id() it doesn't seem
necessary. (Presumably because kvm_enabled() becomes a constant 0 without
CONFIG_KVM so the compiler can treat it like an ifdef.)
We don't want to actually patch it to use CONFIG_KVM though, do we? I
was intending (as part of the VSMT work) to make the vcpu_id a SPAPR
concept, rather than a KVM one, so that machines could (in theory
anyway) be migrated between KVM and non-KVM systems.
> > + return cpu;
> > + }
> > + }
> > +
> > + return NULL;
> > +}
>
> --
> 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
