Patchwork [20/21] qemu-kvm: Move kvm_set_boot_cpu_id

login
register
mail settings
Submitter Jan Kiszka
Date Feb. 2, 2010, 8:19 a.m.
Message ID <363f516310b5e3fbdb2f4461d725d7e0406a6fef.1265098708.git.jan.kiszka@siemens.com>
Download mbox | patch
Permalink /patch/44248/
State New
Headers show

Comments

Jan Kiszka - Feb. 2, 2010, 8:19 a.m.
Setting the boot CPU ID is arch-specific KVM stuff. So push it where it
belongs to.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/pc.c        |    3 ---
 qemu-kvm-x86.c |    3 ++-
 2 files changed, 2 insertions(+), 4 deletions(-)
Gleb Natapov - Feb. 2, 2010, 2:11 p.m.
On Tue, Feb 02, 2010 at 09:19:06AM +0100, Jan Kiszka wrote:
> Setting the boot CPU ID is arch-specific KVM stuff. So push it where it
> belongs to.
> 
pc_init1 is also arch-specific, no? TCG should also be able to
have BSP apic_id != 0.

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  hw/pc.c        |    3 ---
>  qemu-kvm-x86.c |    3 ++-
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index 6c15a9f..3df6195 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -803,9 +803,6 @@ static void pc_init1(ram_addr_t ram_size,
>  #endif
>      }
>  
> -    if (kvm_enabled()) {
> -        kvm_set_boot_cpu_id(0);
> -    }
>      for (i = 0; i < smp_cpus; i++) {
>          env = pc_new_cpu(cpu_model);
>      }
> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
> index 9de018e..0f34451 100644
> --- a/qemu-kvm-x86.c
> +++ b/qemu-kvm-x86.c
> @@ -695,7 +695,8 @@ int kvm_arch_qemu_create_context(void)
>      if (kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK))
>          vmstate_register(0, &vmstate_kvmclock, &kvmclock_data);
>  #endif
> -    return 0;
> +
> +    return kvm_set_boot_cpu_id(0);
>  }
>  
>  static void set_msr_entry(struct kvm_msr_entry *entry, uint32_t index,
> -- 
> 1.6.0.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
			Gleb.
Jan Kiszka - Feb. 2, 2010, 2:20 p.m.
Gleb Natapov wrote:
> On Tue, Feb 02, 2010 at 09:19:06AM +0100, Jan Kiszka wrote:
>> Setting the boot CPU ID is arch-specific KVM stuff. So push it where it
>> belongs to.
>>
> pc_init1 is also arch-specific, no? TCG should also be able to
> have BSP apic_id != 0.

But not kvm-specific.

I don't understand your second remark. Can you help me how TCG is
affected by kvm_set_boot_cpu_id?

> 
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  hw/pc.c        |    3 ---
>>  qemu-kvm-x86.c |    3 ++-
>>  2 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 6c15a9f..3df6195 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -803,9 +803,6 @@ static void pc_init1(ram_addr_t ram_size,
>>  #endif
>>      }
>>  
>> -    if (kvm_enabled()) {
>> -        kvm_set_boot_cpu_id(0);
>> -    }
>>      for (i = 0; i < smp_cpus; i++) {
>>          env = pc_new_cpu(cpu_model);
>>      }
>> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
>> index 9de018e..0f34451 100644
>> --- a/qemu-kvm-x86.c
>> +++ b/qemu-kvm-x86.c
>> @@ -695,7 +695,8 @@ int kvm_arch_qemu_create_context(void)
>>      if (kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK))
>>          vmstate_register(0, &vmstate_kvmclock, &kvmclock_data);
>>  #endif
>> -    return 0;
>> +
>> +    return kvm_set_boot_cpu_id(0);
>>  }
>>  
>>  static void set_msr_entry(struct kvm_msr_entry *entry, uint32_t index,
>> -- 
>> 1.6.0.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> 			Gleb.

Jan
Gleb Natapov - Feb. 2, 2010, 2:28 p.m.
On Tue, Feb 02, 2010 at 03:20:02PM +0100, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > On Tue, Feb 02, 2010 at 09:19:06AM +0100, Jan Kiszka wrote:
> >> Setting the boot CPU ID is arch-specific KVM stuff. So push it where it
> >> belongs to.
> >>
> > pc_init1 is also arch-specific, no? TCG should also be able to
> > have BSP apic_id != 0.
> 
> But not kvm-specific.
> 
> I don't understand your second remark. Can you help me how TCG is
> affected by kvm_set_boot_cpu_id?
> 
It is not affected right now. It assumes that apic ID of BSP cpu is 0,
but this limitation does not exists on real HW. So when QEMU will be fixed
and it will be possible to configure what CPU is BSP this will be the
pace to do it.

> > 
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>  hw/pc.c        |    3 ---
> >>  qemu-kvm-x86.c |    3 ++-
> >>  2 files changed, 2 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/hw/pc.c b/hw/pc.c
> >> index 6c15a9f..3df6195 100644
> >> --- a/hw/pc.c
> >> +++ b/hw/pc.c
> >> @@ -803,9 +803,6 @@ static void pc_init1(ram_addr_t ram_size,
> >>  #endif
> >>      }
> >>  
> >> -    if (kvm_enabled()) {
> >> -        kvm_set_boot_cpu_id(0);
> >> -    }
> >>      for (i = 0; i < smp_cpus; i++) {
> >>          env = pc_new_cpu(cpu_model);
> >>      }
> >> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
> >> index 9de018e..0f34451 100644
> >> --- a/qemu-kvm-x86.c
> >> +++ b/qemu-kvm-x86.c
> >> @@ -695,7 +695,8 @@ int kvm_arch_qemu_create_context(void)
> >>      if (kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK))
> >>          vmstate_register(0, &vmstate_kvmclock, &kvmclock_data);
> >>  #endif
> >> -    return 0;
> >> +
> >> +    return kvm_set_boot_cpu_id(0);
> >>  }
> >>  
> >>  static void set_msr_entry(struct kvm_msr_entry *entry, uint32_t index,
> >> -- 
> >> 1.6.0.2
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe kvm" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > --
> > 			Gleb.
> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux

--
			Gleb.
Jan Kiszka - Feb. 2, 2010, 2:33 p.m.
Gleb Natapov wrote:
> On Tue, Feb 02, 2010 at 03:20:02PM +0100, Jan Kiszka wrote:
>> Gleb Natapov wrote:
>>> On Tue, Feb 02, 2010 at 09:19:06AM +0100, Jan Kiszka wrote:
>>>> Setting the boot CPU ID is arch-specific KVM stuff. So push it where it
>>>> belongs to.
>>>>
>>> pc_init1 is also arch-specific, no? TCG should also be able to
>>> have BSP apic_id != 0.
>> But not kvm-specific.
>>
>> I don't understand your second remark. Can you help me how TCG is
>> affected by kvm_set_boot_cpu_id?
>>
> It is not affected right now. It assumes that apic ID of BSP cpu is 0,
> but this limitation does not exists on real HW. So when QEMU will be fixed
> and it will be possible to configure what CPU is BSP this will be the
> pace to do it.

That day pc_init1 (or whatever x86 part) will set the bsp number
somewhere in env or apicstate, and we will transfer that afterwards to kvm.

The point is that kvm_* belongs into kvm[-all].c as far as possible. And
in this case it is possible.

Jan

Patch

diff --git a/hw/pc.c b/hw/pc.c
index 6c15a9f..3df6195 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -803,9 +803,6 @@  static void pc_init1(ram_addr_t ram_size,
 #endif
     }
 
-    if (kvm_enabled()) {
-        kvm_set_boot_cpu_id(0);
-    }
     for (i = 0; i < smp_cpus; i++) {
         env = pc_new_cpu(cpu_model);
     }
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 9de018e..0f34451 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -695,7 +695,8 @@  int kvm_arch_qemu_create_context(void)
     if (kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK))
         vmstate_register(0, &vmstate_kvmclock, &kvmclock_data);
 #endif
-    return 0;
+
+    return kvm_set_boot_cpu_id(0);
 }
 
 static void set_msr_entry(struct kvm_msr_entry *entry, uint32_t index,