Re: [Qemu-devel] [PATCH 22/22] add cpu-add qmp command and implement CPU hot-add for target-i386

[Eduardo Habkost]

On Tue, Apr 09, 2013 at 11:05:26PM +0200, Igor Mammedov wrote:
> On Tue, 9 Apr 2013 17:46:21 -0300
> Eduardo Habkost <address@hidden> wrote:
> 
> > On Tue, Apr 09, 2013 at 10:19:11PM +0200, Igor Mammedov wrote:
> > > On Fri, 5 Apr 2013 14:10:54 -0300
> > > Eduardo Habkost <address@hidden> wrote:
> > > 
> > > > On Fri, Apr 05, 2013 at 04:37:16PM +0200, Igor Mammedov wrote:
> > > > [...]
> > > > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > > > index db542f6..a760ed5 100644
> > > > > --- a/qapi-schema.json
> > > > > +++ b/qapi-schema.json
> > > > > @@ -1387,6 +1387,17 @@
> > > > >  { 'command': 'cpu', 'data': {'index': 'int'} }
> > > > >  
> > > > >  ##
> > > > > +# @cpu-add
> > > > > +#
> > > > > +# Adds CPU with specified id
> > > > > +#
> > > > > +# @id: cpu id of CPU to be created
> > > > 
> > > > Can we have the semantics/constraints of "id" documented here? Is it an
> > > > arbitrary ID chosen by the caller? Does it have to be the APIC ID? Does
> > > it's generic function so documenting it as APIC ID is not appropriate.
> > > 
> > > I for sure should document it on cpu-hotplug wiki page though, for x86 use
> > > case for starters. i.e. how to use QMP to get a list of available/free 
> > > IDs.
> > > and in which order to use them.
> > > 
> > > > it have to be the index of the CPU in the CPU list? How the IDs of
> > > > existing CPUs set using "-smp" are allocated?
> > > With current -smp implementation the same way as it was before,
> > > and for migration to work hot-plugged CPU has to be the next unused APIC
> > > ID in their sequence, so that target qemu could be started with "-smp 
> > > n+1".
> > 
> > The problem is that it's hard to find out what's the APIC ID for each
> > CPU mentioned in the command-line.
> > 
> > For example, if you use "-smp 18,cores=3,threads=3,maxcpus=36" thread ID
> > will use 2 bits, core ID will use 2 bits, the APIC IDs on startup will
> > be:
> > 
> > online on startup:
> > package 0, core 0: 0 1 2
> > package 0, core 1: 4 5 6
> > package 0, core 2: 8 9 10
> > package 1, core 0: 16 17 18
> > package 1, core 1: 20 21 22
> > package 1, core 2: 24 25 26
> > 
> > offline on startup:
> > package 2, core 0: 32 33 34
> > package 2, core 1: 36 37 38
> > package 2, core 2: 40 41 42
> > package 3, core 0: 48 49 50
> > package 3, core 1: 52 53 54
> > package 3, core 2: 56 57 58
> > 
> > 
> > What should the caller do to find out the correct ID for each of the 36
> > VCPUs? This should be clearly documented.
> Patch 21/22 exposes all APIC IDs (including offline) as links via QOM
> as /machine/icc-bridge/cpu[0..n]
> And since in above sequence IDs are monotonously increasing it's enough to use
> the next unused APIC ID to make -smp n+1 work. 

This can be one method to map CPU indexes to APIC IDs, yes. I find it
hard to explain and hard to use, and it imposes constraints on the way
the target-specific IDs are calculated (requiring them to be
monotonically increasing). But it may be a reasonable solution by now.

I have one additional question about the icc-bridge paths: are the link
paths on icc-bridge explicitly documented/required to be APIC IDs, or
the caller should assume they are arbitrary IDs with no specific
meaning?


> > 
> > 
> > > 
> > > But -smp along with -numa should be reworked to allow specifying guest 
> > > visible
> > > CPU IDs for arbitrary CPU hotplug to work.
> > 
> > I'm curious how you plan to make this work while keeping command-line
> > compatibility. See the question I sent on my other message, about how to
> > map the IDs used on -numa (that are "CPU indexes") to the IDs required
> > by cpu-add.
> It doesn't mean that we should stick to bad/insufficient interface forever. 
> We could add new one that does it right and keep old one for a time being for
> compatibility.

The problem is that I only see three possible kinds of CPU identifiers
that could work in the command-line:

 * Arbitrary user-defined IDs
 * CPU indexes (the current interface)
 * Topology-based identifiers/paths (e.g.
   "/machine/numa_node[0]/cpu_socket[1]/core[2]/thread[1]")

I don't think we can asily use APIC IDs on the command-line because the
caller simply doesn't know what will be the APIC ID for each VCPU.

But this is a problem we can discuss and solve later. By now, we are
stuck with the legacy CPU-index-based interfaces.

> 
> > 
> > > 
> > > when we done with QOMifying CPUs it might be possible to use -device for 
> > > them
> > > and keeping -smp for compat/shorcut purposes.
> > > 
> > > > 
> > > > I am looking at the code right now to understand how this implementation
> > > > works, but the documentation could contain or point to documentation on
> > > > how the "id" parameter is used and interpreted.
> > > I'll add pointer to wiki and describe there target-i386 use-case.
> > 
> > Thanks! Could you try to document it succintly inside qapi-schema.json
> > as well? Maybe just a pointer to other documents would be useful.
> It's very target specific, so separate document probably would make more 
> sense.

The IDs are target-specific, but do we really need to make the interface
specification/usage to be target-specific? We could have a
target-independent interface to find out what are the available/valid
IDs to use on cpu-add.

-- 
Eduardo