[PATCH] Add kvm_set_boot_cpu_id() API.

[PATCH] Add kvm_set_boot_cpu_id() API.

[Gleb Natapov]

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 hw/pc.c    |    3 ++-
 qemu-kvm.c |   17 +++++++++++++++++
 qemu-kvm.h |    1 +
 3 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index cb5b4d0..9ef245d 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -901,7 +901,8 @@ static void pc_init1(ram_addr_t ram_size,
         cpu_model = "qemu32";
 #endif
     }
-    
+
+    kvm_set_boot_cpu_id(0);
     for(i = 0; i < smp_cpus; i++) {
 	env = pc_new_cpu(i, cpu_model, pci_enabled);
     }
diff --git a/qemu-kvm.c b/qemu-kvm.c
index c5cd038..c57df57 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -441,6 +441,18 @@ err:
 	return NULL;
 }
 
+static int kvm_set_boot_vcpu_id(kvm_context_t kvm, uint32_t id)
+{
+#ifdef KVM_CAP_SET_BOOT_CPU_ID
+    int r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_SET_BOOT_CPU_ID);
+    if (r > 0)
+        return ioctl(kvm->vm_fd, KVM_SET_BOOT_CPU_ID, id);
+    return -ENOSYS;
+#else
+    return -ENOSYS;
+#endif
+}
+
 int kvm_create_vm(kvm_context_t kvm)
 {
 	int fd = kvm->fd;
@@ -2923,3 +2935,8 @@ void qemu_kvm_cpu_stop(CPUState *env)
     if (kvm_enabled())
         env->kvm_cpu_state.stopped = 1;
 }
+
+int kvm_set_boot_cpu_id(uint32_t id)
+{
+	return kvm_set_boot_vcpu_id(kvm_context, id);
+}
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 9c28566..8b51737 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -80,6 +80,7 @@ int kvm_arch_try_push_interrupts(void *opaque);
 void kvm_arch_push_nmi(void *opaque);
 void kvm_arch_update_regs_for_sipi(CPUState *env);
 void kvm_arch_cpu_reset(CPUState *env);
+int kvm_set_boot_cpu_id(uint32_t id);
 
 struct kvm_guest_debug;
 struct kvm_debug_exit_arch;
-- 
1.6.2.1

Re: [PATCH] Add kvm_set_boot_cpu_id() API.

[Avi Kivity]

On 06/25/2009 03:21 PM, Gleb Natapov wrote:
> Signed-off-by: Gleb Natapov<gleb@redhat.com>
>    

Why is it needed? (good changelog material).

>       }
> -
> +
> +    kvm_set_boot_cpu_id(0);
>    

You're not testing for failure.  Doesn't matter here because it's the 
default, but then you can omit the call entirely.


-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] Add kvm_set_boot_cpu_id() API.

[Gleb Natapov]

On Sun, Jun 28, 2009 at 12:29:26PM +0300, Avi Kivity wrote:
> On 06/25/2009 03:21 PM, Gleb Natapov wrote:
>> Signed-off-by: Gleb Natapov<gleb@redhat.com>
>>    
>
> Why is it needed? (good changelog material).
>
OK.

>>       }
>> -
>> +
>> +    kvm_set_boot_cpu_id(0);
>>    
>
> You're not testing for failure.  Doesn't matter here because it's the  
> default, but then you can omit the call entirely.
>
This is a place holder. The patch adds an interface but doesn't add any
user if this line is omitted. As you said there is no need to check for
failure in this case, if we do we will not be able to run on older
kernels.

--
			Gleb.

Re: [PATCH] Add kvm_set_boot_cpu_id() API.

[Avi Kivity]

On 06/28/2009 12:33 PM, Gleb Natapov wrote:
> On Sun, Jun 28, 2009 at 12:29:26PM +0300, Avi Kivity wrote:
>    
>> On 06/25/2009 03:21 PM, Gleb Natapov wrote:
>>      
>>> Signed-off-by: Gleb Natapov<gleb@redhat.com>
>>>
>>>        
>> Why is it needed? (good changelog material).
>>
>>      
> OK.
>
>    

Well, why?

>>>        }
>>> -
>>> +
>>> +    kvm_set_boot_cpu_id(0);
>>>
>>>        
>> You're not testing for failure.  Doesn't matter here because it's the
>> default, but then you can omit the call entirely.
>>
>>      
> This is a place holder. The patch adds an interface but doesn't add any
> user if this line is omitted. As you said there is no need to check for
> failure in this case, if we do we will not be able to run on older
> kernels.
>    

It's also good to call the new API as a way of exercising it.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] Add kvm_set_boot_cpu_id() API.

[Gleb Natapov]

On Sun, Jun 28, 2009 at 01:31:28PM +0300, Avi Kivity wrote:
> On 06/28/2009 12:33 PM, Gleb Natapov wrote:
>> On Sun, Jun 28, 2009 at 12:29:26PM +0300, Avi Kivity wrote:
>>    
>>> On 06/25/2009 03:21 PM, Gleb Natapov wrote:
>>>      
>>>> Signed-off-by: Gleb Natapov<gleb@redhat.com>
>>>>
>>>>        
>>> Why is it needed? (good changelog material).
>>>
>>>      
>> OK.
>>
>>    
>
> Well, why?
>
Hmm, I thought you want me to resubmit with a new changelog. If
resubmission is not required then here is the changelog:

Old kernel assumed that apic id of a boot cpus is zero, recent kernels
have an IOCTL to configure BSP's apic id. The patch adds API function to
use this capability. The BSP apic id is set to zero (now explicitly)
since QEMU/BIOS not ready yet to take full advantage of the new API.

--
			Gleb.