On Thu, Jun 15, 2017 at 03:27:28PM +0200, Igor Mammedov wrote: > On Thu, 15 Jun 2017 15:41:08 +0300 > Roman Kagan <rka...@virtuozzo.com> wrote: > > On Wed, Jun 14, 2017 at 03:00:27PM +0200, Igor Mammedov wrote: > > > On Wed, 14 Jun 2017 13:26:44 +0200 > > > Paolo Bonzini <pbonz...@redhat.com> wrote: > > > > > > > On 14/06/2017 13:25, Roman Kagan wrote: > > > > >> The problem with that is that it will break as soon as we create > > > > >> VCPUs in a different order. Unsolvable on hosts that don't allow > > > > >> HV_X64_MSR_VP_INDEX to be set, however. > > > > > Right, thanks for putting together a detailed explanation. > > > > > > > > > > This was a thinko back then, not to have HV_X64_MSR_VP_INDEX > > > > > maintained > > > > > by QEMU. I'm going to post a patch to KVM fixing that. > > > > > > > > > > Meanwhile QEMU needs a way to maintain its notion of vp_index that is > > > > > 1) in sync with kernel's notion > > > > > 2) also with kernels that don't support setting the msr > > > > > 3) persistent across migrations > > > > > > > > > > cpu_index looked like a perfect candidate. > > > > > > > > > > > > > What you want is the APIC id, > > > > > > > which _is_ cpu_index but may not be in the > > > depending on topology cpu_index won't be the same as APIC ID/vcpu_id > > > /AMDs odd core count/. > > > > So vcpu_id can be sparse? > yes > > I suppose kernel even has cpu->apic_id map somewhere that you can reuse > instead of custom map hv_context.vp_index[]
Right, but this is not the problem. Whether apic_id matches the expected vp_index semantics is. > > > > future. But the APIC id is also the KVM vcpu_id, so there's no need to > > > > have VP_INDEX maintained by QEMU. > > > agreed it'd be better to reuse vcpu_id/apic id as interface between > > > qemu/kvm/guest instead of adding additional cpu_index concept in ABI > > > > Having consulted the spec, I'm not so confident any more this is the > > right move. > > > > > 7.8.1 Virtual Processor Index > > > > > > Virtual processors are identified by using an index (VP index). The > > > maximum number of virtual processors per partition supported by the > > > current implementation of the hypervisor can be obtained through CPUID > > > leaf 0x40000005. A virtual processor index must be less than the > > > maximum number of virtual processors per partition. > > > > This seems to imply that VP index should be dense. As if they use it > > directly as an index into an array whose length is equal to the max > > number of vcpus. (BTW the value we report for it is currently hardcoded > > to 0x40 which probably needs fixing, too.) > the question here is where "maximum number of virtual processors" comes from We provide it in a corresponding cpuid leaf. > if it's possible to tell guest value explicitly, > we can use pcms->apic_id_limit for it. But, depending on the apic_id encoding scheme, this can be arbitrarily bigger than the actual maximum number of vcpus. Which can result in guest confusion or inefficiency. > > I'm starting to drift back to adding SET_MSRS support to > > HV_X64_MSR_VP_INDEX in KVM to delegate the control to QEMU, and having > > QEMU use cpu_index as its value... :-/ Roman.
