Zhang, Xiantao wrote:
> Jan Kiszka wrote:
>> Zhang, Xiantao wrote:
>>> From 1b89616f99abc8e0983ef58a1f984f31a52fe828 Mon Sep 17 00:00:00
>>> 2001 From: Xiantao Zhang <xiantao.zhang@xxxxxxxxx>
>>> Date: Thu, 27 Nov 2008 17:24:51 +0800
>>> Subject: [PATCH] KVM: Qemu: push_nmi should be only used by I386
>>> Arch.
>>>
>>> Use TARGET_I386 to exclude other archs.
>>> Signed-off-by: Xiantao Zhang <xiantao.zhang@xxxxxxxxx> ---
>>> qemu/qemu-kvm.c | 4 ++++
>>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
>>> index cf0e85d..b6c8288 100644
>>> --- a/qemu/qemu-kvm.c
>>> +++ b/qemu/qemu-kvm.c
>>> @@ -154,10 +154,12 @@ static int try_push_interrupts(void *opaque)
>>> return kvm_arch_try_push_interrupts(opaque);
>>> }
>>>
>>> +#ifdef TARGET_I386
>>> static void push_nmi(void *opaque)
>>> {
>>> kvm_arch_push_nmi(opaque);
>>> }
>>> +#endif
>>>
>>> static void post_kvm_run(void *opaque, void *data) {
>>> @@ -742,7 +744,9 @@ static struct kvm_callbacks qemu_kvm_ops = {
>>> .shutdown = kvm_shutdown,
>>> .io_window = kvm_io_window,
>>> .try_push_interrupts = try_push_interrupts,
>>> +#ifdef TARGET_I386
>>> .push_nmi = push_nmi,
>>> +#endif
>>> .post_kvm_run = post_kvm_run,
>>> .pre_kvm_run = pre_kvm_run,
>>> #ifdef TARGET_I386
>> Well, doesn't push_nmi() from libkvm.c call into this hook
>> unconditionally if KVM_CAP_NMI is set (which is the case for all
>> recent kernel headers)? That should cause SEGVs, so you need to patch
>> kvm_run() as well.
>
> Since it doesn't generate compiler error, I didn't notice this issue, Thanks!
>
>> Makes me wonder if we shouldn't have better defined KVM_CAP_NMI
>> conditionally, only for arch that actually have NMIs (/wrt KVM: only
>> x86 ATM). But now it's too late...
>
> But a funny thing is that KVM_CAP_NMI is defined in kernel, but is not used in any code except userspace. We had better use
> TARGET_I386 to constrain it for x86 in userspace. Okay ? Attached the patch.
>
>>From c25fa2e4de40e500bd364c3267d5be89a9cfbb4d Mon Sep 17 00:00:00 2001
> From: Xiantao Zhang <xiantao.zhang@xxxxxxxxx>
> Date: Fri, 28 Nov 2008 09:38:46 +0800
> Subject: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
>
> Use TARGET_I386 to exclude other archs.
> Signed-off-by: Xiantao Zhang <xiantao.zhang@xxxxxxxxx>
> ---
> libkvm/libkvm.c | 4 ++--
> qemu/qemu-kvm.c | 4 ++++
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
> index 40c95ce..851a93a 100644
> --- a/libkvm/libkvm.c
> +++ b/libkvm/libkvm.c
> @@ -868,7 +868,7 @@ int kvm_run(kvm_context_t kvm, int vcpu, void *env)
> struct kvm_run *run = kvm->run[vcpu];
>
> again:
> -#ifdef KVM_CAP_NMI
> +#ifdef TARGET_I386
> push_nmi(kvm);
> #endif
> #if !defined(__s390__)
> @@ -1032,7 +1032,7 @@ int kvm_has_sync_mmu(kvm_context_t kvm)
>
> int kvm_inject_nmi(kvm_context_t kvm, int vcpu)
> {
> -#ifdef KVM_CAP_NMI
> +#ifdef TARGET_I386
> return ioctl(kvm->vcpu_fd[vcpu], KVM_NMI);
> #else
> return -ENOSYS;
> diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
> index cf0e85d..b6c8288 100644
> --- a/qemu/qemu-kvm.c
> +++ b/qemu/qemu-kvm.c
> @@ -154,10 +154,12 @@ static int try_push_interrupts(void *opaque)
> return kvm_arch_try_push_interrupts(opaque);
> }
>
> +#ifdef TARGET_I386
> static void push_nmi(void *opaque)
> {
> kvm_arch_push_nmi(opaque);
> }
> +#endif
>
> static void post_kvm_run(void *opaque, void *data)
> {
> @@ -742,7 +744,9 @@ static struct kvm_callbacks qemu_kvm_ops = {
> .shutdown = kvm_shutdown,
> .io_window = kvm_io_window,
> .try_push_interrupts = try_push_interrupts,
> +#ifdef TARGET_I386
> .push_nmi = push_nmi,
> +#endif
> .post_kvm_run = post_kvm_run,
> .pre_kvm_run = pre_kvm_run,
> #ifdef TARGET_I386
This will now break when KVM_CAP_NMI is undefined, ie. when there is no
KVM_NMI IOCTL (=> older kvm module sets).
Jan
--
Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe kvm-ia64" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
