Re: [Qemu-devel] [PATCH 05/23] hyperv: ensure VP index equal to QEMU cpu_index

[Roman Kagan]

On Wed, Jun 14, 2017 at 03:00:27PM +0200, Igor Mammedov wrote:
> On Wed, 14 Jun 2017 13:26:44 +0200
> Paolo Bonzini <address@hidden> 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?

> > 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.)

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.