Oops, seems we introduced the issue together. Acked-by Xiantao Zhang <xiantao.zhang@xxxxxxxxx> -----Original Message----- From: jan.kiszka@xxxxxx [mailto:jan.kiszka@xxxxxx] Sent: Tuesday, December 02, 2008 7:03 AM To: Hollis Blanchard Cc: Avi Kivity; Zhang, Xiantao; kvm@xxxxxxxxxxxxxxx; kvm-ia64@xxxxxxxxxxxxxxx Subject: Re: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch. Hollis Blanchard wrote: > On Fri, 2008-11-28 at 10:26 +0100, Jan Kiszka wrote: >> Zhang, Xiantao wrote: >>> >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). > > Guys, we already have stubs for this (although they've been turned into > dead code). Jan broke IA64 and PowerPC builds when he renamed > "kvm_arch_try_push_nmi" to "kvm_arch_push_nmi", and the obvious fix is > to update the stubs to match. That avoids all these ifdefs and > associated problems. Ouch - I'm sorry. > > Avi, could you revert a8d12f98755be9330fcde055134511f76ecaa538 please? > Here is a patch that reverts change and fixes the root of the issue. ----------- Subject: Fix non-x86 NMI hooks My previous x86-only change to the NMI push hook broke PPC and IA64. This is a proper fix plus a cleanup of the #ifdef-based approach to solve the breakage. Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> --- qemu/qemu-kvm-ia64.c | 3 +-- qemu/qemu-kvm-powerpc.c | 3 +-- qemu/qemu-kvm.c | 4 ---- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/qemu/qemu-kvm-ia64.c b/qemu/qemu-kvm-ia64.c index 8380f39..a6b17af 100644 --- a/qemu/qemu-kvm-ia64.c +++ b/qemu/qemu-kvm-ia64.c @@ -57,9 +57,8 @@ int kvm_arch_try_push_interrupts(void *opaque) return 1; } -int kvm_arch_try_push_nmi(void *opaque) +void kvm_arch_push_nmi(void *opaque) { - return 1; } void kvm_arch_update_regs_for_sipi(CPUState *env) diff --git a/qemu/qemu-kvm-powerpc.c b/qemu/qemu-kvm-powerpc.c index 19fde40..fa534ed 100644 --- a/qemu/qemu-kvm-powerpc.c +++ b/qemu/qemu-kvm-powerpc.c @@ -188,12 +188,11 @@ int kvm_arch_try_push_interrupts(void *opaque) return 0; } -int kvm_arch_try_push_nmi(void *opaque) +void kvm_arch_push_nmi(void *opaque) { /* no nmi irq, so discard that call for now and return success. * This might later get mapped to something on powerpc too if we want * to support the nmi monitor command somwhow */ - return 0; } void kvm_arch_update_regs_for_sipi(CPUState *env) diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c index b6c8288..cf0e85d 100644 --- a/qemu/qemu-kvm.c +++ b/qemu/qemu-kvm.c @@ -154,12 +154,10 @@ 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) { @@ -744,9 +742,7 @@ 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 -- 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
