On Fri, Aug 04, 2017 at 01:10:19PM +1000, David Gibson wrote: > On Thu, Aug 03, 2017 at 04:28:52PM +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 <[email protected]> > > Mostly good, but... > > [snip] > > +int spapr_vcpu_id(PowerPCCPU *cpu) > > +{ > > + return cpu->vcpu_id; > > +} > > + > > +PowerPCCPU *spapr_find_cpu(int vcpu_id) > > +{ > > + CPUState *cs; > > + > > + CPU_FOREACH(cs) { > > + PowerPCCPU *cpu = POWERPC_CPU(cs); > > + > > + if (cpu->vcpu_id == vcpu_id) { > > + return cpu; > > + } > > + } > > + > > + return NULL; > > +} > > [...] > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > > index 7ccb350c5f..2bf2727860 100644 > > --- a/target/ppc/kvm.c > > +++ b/target/ppc/kvm.c > > @@ -512,7 +512,7 @@ bool kvmppc_is_mem_backend_page_size_ok(const char > > *obj_path) > > > > unsigned long kvm_arch_vcpu_id(CPUState *cpu) > > { > > - return ppc_get_vcpu_id(POWERPC_CPU(cpu)); > > + return spapr_vcpu_id(POWERPC_CPU(cpu)); > > } > > Here you've replaced an implicit dependency on spapr details in the > generic code with an explicit dependency on spapr details. That's the > wrong direction.
Ah right, I'll flip it around. > Instead _this_ one should directly reference vcpu_id, the spapr one > should be something like: > > if (kvm) > return kvm_arch_vcpu_id(...) > else > return cpu_index; OK. > -- > 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
