On Thu, Oct 07, 2010 at 02:50:49PM +0200, Avi Kivity wrote:
> On 10/04/2010 05:56 PM, Gleb Natapov wrote:
> >Enable async PF in a guest if async PF capability is discovered.
> >
> >
> >+void __cpuinit kvm_guest_cpu_init(void)
> >+{
> >+ if (!kvm_para_available())
> >+ return;
> >+
> >+ if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF)&& kvmapf) {
> >+ u64 pa = __pa(&__get_cpu_var(apf_reason));
> >+
> >+ if (native_write_msr_safe(MSR_KVM_ASYNC_PF_EN,
> >+ pa | KVM_ASYNC_PF_ENABLED, pa>> 32))
>
> native_ versions of processor accessors shouldn't be used generally.
>
> Also, the MSR isn't documented to fail on valid input, so you can
> use a normal wrmsrl() here.
>
Kernel will oops on wrong write then. OK, why not.
> >+ return;
> >+ __get_cpu_var(apf_reason).enabled = 1;
> >+ printk(KERN_INFO"KVM setup async PF for cpu %d\n",
> >+ smp_processor_id());
> >+ }
> >+}
> >+
> >
> >+static int kvm_pv_reboot_notify(struct notifier_block *nb,
> >+ unsigned long code, void *unused)
> >+{
> >+ if (code == SYS_RESTART)
> >+ on_each_cpu(kvm_pv_disable_apf, NULL, 1);
> >+ return NOTIFY_DONE;
> >+}
> >+
> >+static struct notifier_block kvm_pv_reboot_nb = {
> >+ .notifier_call = kvm_pv_reboot_notify,
> >+};
>
> Does this handle kexec?
>
Yes.
> >+
> >+static void kvm_guest_cpu_notify(void *dummy)
> >+{
> >+ if (!dummy)
> >+ kvm_guest_cpu_init();
> >+ else
> >+ kvm_pv_disable_apf(NULL);
> >+}
>
> Why are you making decisions based on a dummy input?
>
> The whole thing looks strange. Use two functions?
>
What is so strange? Type of notification is passed as a parameter.
The code that does this is just under the function. I can rename
dummy to something else. Or make it two functions.
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
