On 04.11.2013, at 10:58, Alexey Kardashevskiy <a...@ozlabs.ru> wrote:
> On 11/04/2013 08:41 PM, Alexander Graf wrote: >> >> On 04.11.2013, at 02:10, Alexey Kardashevskiy <a...@ozlabs.ru> wrote: >> >>> Normally CPUState::cpu_index is used to pick the right CPU for various >>> operations. However default consecutive numbering does not always work >>> for POWERPC. >>> >>> For example, on POWER7 (which supports 4 threads per core), >>> "-smp 8,threads=4" should create CPUs with indexes 0,1,2,3,4,5,6,7 and >>> "-smp 8,threads=1" should create CPUs with indexes 0,4,8,12,16,20,24,28. >>> >>> These indexes are reflected in /proc/device-tree/cpus/PowerPC,POWER7@XX >>> and used to call KVM VCPU's ioctls. In order to achieve this, >>> kvmppc_fixup_cpu() was introduced. Roughly speaking, it multiplies >>> cpu_index by the number of threads per core. >>> >>> This approach has disadvantages such as: >>> 1. NUMA configuration stays broken after the fixup; >>> 2. CPU-related commands from QEMU Monitor do not work properly as >>> the accept fixed CPU indexes and the user does not really know >>> what they are after fixup as the number of threads per core changes >>> between CPU versions and via QEMU command line. >>> >>> This introduces a new @cpu_dt_id field in the CPUPPCState struct which >>> is set from @cpu_index by default but can be fixed later to the value >>> which a hypervisor can accept. This also introduces two POWERPC-arch >>> specific functions: >>> 1. int ppc_get_vcpu_dt_id(CPUState *cs) - returns a device-tree ID >>> for a CPU; >>> 2. CPUState *ppc_get_vcpu_by_dt_id(int cpu_dt_id) - finds CPUState by >>> a device-tree CPU ID. >>> >>> This uses the new functions to: >>> 1. fix emulated XICS hypercall handlers as they receive fixed CPU indexes; >>> 2. fix XICS-KVM to enable in-kernel XICS on right CPU; >>> 3. compose correct device-tree. >>> >>> This removes @cpu_index fixup as @cpu_dt_id is used instead so QEMU monitor >>> can accept command-line CPU indexes again. >> >> So this patch feels awkward. We use the dt fixup at random places in >> completely dt unrelated code paths. > > Yep. I called it smp_cpu_index in the first place :) > >> What we really have are 3 semantically separate entities: >> >> * QEMU internal cpu id >> * KVM internal cpu id >> * DT exposed cpu id >> > >> As you have noted, it's a good idea to keep the QEMU internal cpu id >> linear, thus completely separate from the others. The DT exposed cpu id >> should be 100% local to hw/ppc/spapr*.c. I don't think any code outside >> of the DT generation and anything that accesses the "Virtual Processor >> Number" in sPAPR needs to care about the DT cpu id. All that code is >> 100% KVM agnostic. >> >> The KVM internal cpu id should probably be a new field in the generic >> CPUState that gets used by kvm specific code that needs to know the KVM >> internal cpu id a vcpu was created with. The flow should be that >> kvm_arch_vcpu_id() tells kvm_init_vcpu() the kvm internal id to use >> which then stores it in CPUState. That way you can always get the KVM >> intenal cpu id from a CPU struct. All the references to this ID should >> _only_ happen from KVM only code. > > > If DT id is local to spapr*, then how do I implement kvm_arch_vcpu_id()? > Where will it get the fixed value? Do the same calculation in two different > places (for device tree and for kvm)? kvm_arch_vcpu_id() won't get called until qemu_init_vcpu() is issued from ppc_cpu_realizefn(). So if instead of calling cpu_ppc_init() you split up the function and set the kvm id property before realize from ppc_spapr_init(), that should work, no? Alex