On Wed, 3 Apr 2013 16:27:11 -0300 Eduardo Habkost <ehabk...@redhat.com> wrote:
> On Wed, Apr 03, 2013 at 08:59:07PM +0200, Igor Mammedov wrote: > > On Wed, 3 Apr 2013 15:10:05 -0300 > > Eduardo Habkost <ehabk...@redhat.com> wrote: > > > > > On Wed, Apr 03, 2013 at 07:58:00PM +0200, Igor Mammedov wrote: > > > <snip> > > > > > > +void do_cpu_hot_add(const int64_t id, Error **errp) > > > > > > +{ > > > > > > + pc_new_cpu(saved_cpu_model, id, errp); > > > > > > +} > > > > > > + > > > > > > > > > > Missing x86_cpu_apic_id_from_index(id)? > > > > There was(is?) opposition to using cpu_index to identify x86 CPU. > > > > > > Really? Do you have a pointer to the discussion? > > Here is what I could find in my mail box: > > http://lists.gnu.org/archive/html/qemu-devel/2012-05/msg02770.html > > Jan could correct me if I'm wrong. > > > > So, quoting Jan: > > From my POV, cpu_index could become equal to the physical APIC ID. > > As long as we can set it freely (provided it remains unique) and > > non-continuously, we don't need separate indexes." > > We can't choose APIC IDs freely, because the APIC ID is calculated based > on the CPU topology (socket + core + thread IDs). > > So, the cpu_index could be the same as the APIC ID if the cpu_index > value declared opaque, being just a "CPU identifier" that is chosen by > QEMU arbitrarily (and that happens to match the APIC ID). But we will > probably have some problems with this: > > - The CPU index are currently allocated contiguously, and probably > existing interfaces already assume that (e.g. the "-numa' option, > "info numa", "info cpus" and maybe other monitor commands) > - QEMU must be responsible for calculating the APIC ID of each CPU, > because it is based on the CPU topology. > - If QEMU is the one who calculates the APIC ID, what kind of identifier > we can use for a CPU object in the command-line (e.g. in the "-numa" > option)? using any kind of thread id is problematic since 2 treads from the same core could end-up on different nodes. Maybe placement interface could be better described as node[n]=sockets_list? > - We may need to redefine the meaning of the "maxcpus" -smp option, if > all our interfaces are now based in non-contiguous and freely-set CPU > identifiers. it's amount of CPUs available to guest, pretty clear from user's POV. > > In short, getting rid of the contiguous CPU indexes sounds very > difficult. We could introduce other kind of identifiers, but probably we > may need to keep the CPU indexes contiguous to keep existing interfaces > working. Once we have CPU unplug, we will have non-contiguous cpu_index. So it will be part of CPU unplug series to fix cpu_index allocation/usage where necessary. > > > > > > > > > > > > > So, it is expected from management to provide APIC ID instead of > > > > cpu_index. > > > > It could be useful to make hotplug to a specific NUMA node/cpu to work > > > > in > > > > future. > > > > Though interface of possible APIC IDs discovery is not part of this > > > > series. > > > > > > That's exactly the opposite of what I expect. The APIC ID is an internal > > > implementation detail, and external tools must _not_ be required to deal > > > with it and to calculate it. > > > > > > Communication with the BIOS, on the other hand, is entirely based on the > > > APIC ID, and not CPU indexes. So QEMU needs to translate the CPU indexes > > > (used to communicate with the outside world) to APIC IDs when talking to > > > the BIOS. > > cpu_index won't work nicely with hot-adding CPU to specific numa node > > though. > > Well, the "-numa node" options are already based on CPU indexes, so it > would match it the existing NUMA configuration interface. > > > with APIC ID (mgmt might treat it as opaque) we could expose something like > > > > /machine/icc-bridge/link<CPU[apic_id_n] > > ... > > > > for all possible CPUs, with empty links for non existing ones. > > > > and later add on something like this: > > > > /machine/numa_node[x]/link<CPU[apic_id_n]> > > ... > > > > Libvirt than could just pickup ready apic id from desired place and add CPU > > either using cpu-add id=xxx or device_add x86-cpu-...,apic_id=xxx > > > > +1 more cpu_index is QEMU implementation detail and we could not add to x86 > > CPU > > cpu-index property since hardware doesn't have such feature, so it won't be > > available with device_add. > > I don't mind hiding cpu_index too. I don't mind if we use a cpu_index, > QOM links, arbitrary IDs set by the user. I just have a problem with > requiring libvirt to set the APIC ID. > > If you give libvirt an easy way to convert a CPU "location" (index, numa > node, whatever) to an APIC ID that is pre-calculated by QEMU, then it > could work. But do we really need to require libvirt to deal with APIC > ID directly? If you just set the links properly to reflect the CPU > "location", the CPU could calculate its APIC ID based on its "location" > using the links. What about adding CPU to a specific node then, it would require interface for communicating to CPU to which node it should be plugged (part of APIC ID, I guess). > > -- > Eduardo > -- Regards, Igor