2010-08-20 08:08:39

by Zachary Amsden

[permalink] [raw]
Subject: [KVM timekeeping 13/35] Perform hardware_enable in CPU_STARTING callback

The CPU_STARTING callback was added upstream with the intention
of being used for KVM, specifically for the hardware enablement
that must be done before we can run in hardware virt. It had
bugs on the x86_64 architecture at the time, where it was called
after CPU_ONLINE. The arches have since merged and the bug is
gone.

It might be noted other features should probably start making
use of this callback; microcode updates in particular which
might be fixing important erratums would be best applied before
beginning to run user tasks.

Signed-off-by: Zachary Amsden <[email protected]>
---
virt/kvm/kvm_main.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index b78b794..d4853a5 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1958,10 +1958,10 @@ static int kvm_cpu_hotplug(struct notifier_block *notifier, unsigned long val,
cpu);
hardware_disable(NULL);
break;
- case CPU_ONLINE:
+ case CPU_STARTING:
printk(KERN_INFO "kvm: enabling virtualization on CPU%d\n",
cpu);
- smp_call_function_single(cpu, hardware_enable, NULL, 1);
+ hardware_enable(NULL);
break;
}
return NOTIFY_OK;
@@ -2096,7 +2096,6 @@ int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,

static struct notifier_block kvm_cpu_notifier = {
.notifier_call = kvm_cpu_hotplug,
- .priority = 20, /* must be > scheduler priority */
};

static int vm_stat_get(void *_offset, u64 *val)
--
1.7.1


2010-08-27 17:15:36

by Jan Kiszka

[permalink] [raw]
Subject: Re: [KVM timekeeping 13/35] Perform hardware_enable in CPU_STARTING callback

Zachary Amsden wrote:
> The CPU_STARTING callback was added upstream with the intention
> of being used for KVM, specifically for the hardware enablement
> that must be done before we can run in hardware virt. It had
> bugs on the x86_64 architecture at the time, where it was called
> after CPU_ONLINE. The arches have since merged and the bug is
> gone.

What bugs are you referring to, or since which kernel version is
CPU_STARTING usable for KVM? I need to encode this into kvm-kmod.

Thanks,
Jan

>
> It might be noted other features should probably start making
> use of this callback; microcode updates in particular which
> might be fixing important erratums would be best applied before
> beginning to run user tasks.
>
> Signed-off-by: Zachary Amsden <[email protected]>
> ---
> virt/kvm/kvm_main.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index b78b794..d4853a5 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1958,10 +1958,10 @@ static int kvm_cpu_hotplug(struct notifier_block *notifier, unsigned long val,
> cpu);
> hardware_disable(NULL);
> break;
> - case CPU_ONLINE:
> + case CPU_STARTING:
> printk(KERN_INFO "kvm: enabling virtualization on CPU%d\n",
> cpu);
> - smp_call_function_single(cpu, hardware_enable, NULL, 1);
> + hardware_enable(NULL);
> break;
> }
> return NOTIFY_OK;
> @@ -2096,7 +2096,6 @@ int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
>
> static struct notifier_block kvm_cpu_notifier = {
> .notifier_call = kvm_cpu_hotplug,
> - .priority = 20, /* must be > scheduler priority */
> };
>
> static int vm_stat_get(void *_offset, u64 *val)

--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

2010-08-27 23:44:08

by Zachary Amsden

[permalink] [raw]
Subject: Re: [KVM timekeeping 13/35] Perform hardware_enable in CPU_STARTING callback

On 08/27/2010 06:32 AM, Jan Kiszka wrote:
> Zachary Amsden wrote:
>
>> The CPU_STARTING callback was added upstream with the intention
>> of being used for KVM, specifically for the hardware enablement
>> that must be done before we can run in hardware virt. It had
>> bugs on the x86_64 architecture at the time, where it was called
>> after CPU_ONLINE. The arches have since merged and the bug is
>> gone.
>>
> What bugs are you referring to, or since which kernel version is
> CPU_STARTING usable for KVM? I need to encode this into kvm-kmod.
>

Prior to the x86_64 / i386 merge, CPU_STARTING didn't work the same way
/ exist in the x86_64 code... most of this is historical guesswork. At
some point, the 32/64 versions of the code in smpboot.c got merged and
now it does.

Binary searching around my tree shows this timeframe:

2.6.11? - 2.6.23 : silver age ; i386 and x86_64 merge underway
|
2.6.24 : bronze age ; i386 and x86_64 deprecated
|
2.6.26 : iron age; smpboot_32.c / smpboot_64.c merge
\
2.6.28 : CPU_STARTING exists and first used

/me scratches head wondering how this affects operation on older kernels....

2010-08-30 09:10:40

by Jan Kiszka

[permalink] [raw]
Subject: Re: [KVM timekeeping 13/35] Perform hardware_enable in CPU_STARTING callback

Zachary Amsden wrote:
> On 08/27/2010 06:32 AM, Jan Kiszka wrote:
>> Zachary Amsden wrote:
>>
>>> The CPU_STARTING callback was added upstream with the intention
>>> of being used for KVM, specifically for the hardware enablement
>>> that must be done before we can run in hardware virt. It had
>>> bugs on the x86_64 architecture at the time, where it was called
>>> after CPU_ONLINE. The arches have since merged and the bug is
>>> gone.
>>>
>> What bugs are you referring to, or since which kernel version is
>> CPU_STARTING usable for KVM? I need to encode this into kvm-kmod.
>>
>
> Prior to the x86_64 / i386 merge, CPU_STARTING didn't work the same way
> / exist in the x86_64 code... most of this is historical guesswork. At
> some point, the 32/64 versions of the code in smpboot.c got merged and
> now it does.
>
> Binary searching around my tree shows this timeframe:
>
> 2.6.11? - 2.6.23 : silver age ; i386 and x86_64 merge underway
> |
> 2.6.24 : bronze age ; i386 and x86_64 deprecated
> |
> 2.6.26 : iron age; smpboot_32.c / smpboot_64.c merge
> \
> 2.6.28 : CPU_STARTING exists and first used
>
> /me scratches head wondering how this affects operation on older kernels....

I basically need to revert your patch on host kernels without
CPU_STARTING and also on those where it might be broken. So I will set
the barrier to 2.6.28 then.

Thanks,
Jan

--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux