Re: [PATCH RFC 3/3] Break dependency between vcpu index in vcpus array and vcpu's apic id.

[Gleb Natapov <gleb@redhat.com>]

On Thu, May 21, 2009 at 05:29:57PM -0300, Marcelo Tosatti wrote:
> On Thu, May 21, 2009 at 11:06:42PM +0300, Gleb Natapov wrote:
> > > Why don't make a convention that vcpu_id 0 is the BSP, by default,
> > > instead of the first vcpu created? This way if userspace creates vcpu 3
> > > first, there are no problems.
> > > 
> > Because userspace no longer control vcpu_id allocation. I also want to
> > get rid of "first vcpu is bsp" restriction, but this fill require new
> > ioctl I guess.
> 
> Or hardcode "apic_id == 0 is BSP" for now, and later add an ioctl to set
> the BSP?
> 
We should not assume anything about apic_ids. For instance if the number
of ioapics+cpu cores are greater than 15 it is recommended to assign ids
0-(N-1) to ioapic (N is the number of ioapics). This is because ioapic
id is only 4 bits long, but it occupies the same id space as apic ids.

> > > That brings the question, why is the apic_id passed as the argument to
> > > vcpu_create? It seems that the argument to vcpu_create should be the
> > > kVM's internal vcpu_id, and the apic_id should come from somewhere else,
> > > apic_mmio_write maybe?
> > I think exactly opposite :) Why userspace should care what index vcpu
> > occupies in internal kernel data structure? This information is never
> > ever used by userspace again. 
> 
> At the moment its used to implicitly set the BSP (BSP is vcpu_id == 0).
> 
Correct. This is the only use of vcpu_id and the patch change it to be
"first cpu created is BSP" and that works with all knows userspace users
of the API. We should add another vcpu_create ioctl that will get
additional flags as a parameter, or have bsp_cpu_id ioctl that should
be issued before first vcpu is even created.

> > > is determined by hardware and BIOS), but in KVM's BIOS case the BSP is
> > > conventioned to be vcpu with vcpu_id == 0, no?
> > Yes (except not vcpu_id == 0, but apic_id == 0), we will have to fix this
> > bug too. Bios should get acpi_ids of active cpus from qemu, not assume
> > anything.
> > 
> > > Another thing, it would be better if the linking of the vcpu in the
> > > array could be in arch independent code as it is today?
> > Possible, I will have to define kvm_arch_vcpu_find(id) function to
> > search for duplicates in generic code, but it will not be prettier.
> > I did it this way initially, but decided to hide everything in arch
> > code.
> 
> Building on what you said, how KVM stores vcpu structures is no business
> of arch vcpu initialization code (or that should be the goal), IMO.
This is definitely the goal. I can remove vcpu_id/vcpus[] uses from x86
in a couple of key strokes, but other arches uses vcpu_id in a ways I
don't fully understand.

--
			Gleb.