* [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
@ 2008-11-27 9:36 ` Zhang, Xiantao
0 siblings, 0 replies; 18+ messages in thread
From: Zhang, Xiantao @ 2008-11-27 9:36 UTC (permalink / raw)
To: Avi Kivity, kvm@vger.kernel.org; +Cc: kvm-ia64@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 1180 bytes --]
>From 1b89616f99abc8e0983ef58a1f984f31a52fe828 Mon Sep 17 00:00:00 2001
From: Xiantao Zhang <xiantao.zhang@intel.com>
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@intel.com>
---
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
--
1.6.0
[-- Attachment #2: 0002-KVM-Qemu-push_nmi-should-be-only-used-by-I386-Arch.patch --]
[-- Type: application/octet-stream, Size: 1140 bytes --]
From 1b89616f99abc8e0983ef58a1f984f31a52fe828 Mon Sep 17 00:00:00 2001
From: Xiantao Zhang <xiantao.zhang@intel.com>
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@intel.com>
---
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
--
1.6.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
2008-11-27 9:36 ` Zhang, Xiantao
@ 2008-11-27 11:39 ` Jan Kiszka
-1 siblings, 0 replies; 18+ messages in thread
From: Jan Kiszka @ 2008-11-27 11:39 UTC (permalink / raw)
To: kvm-ia64
Zhang, Xiantao wrote:
> From 1b89616f99abc8e0983ef58a1f984f31a52fe828 Mon Sep 17 00:00:00 2001
> From: Xiantao Zhang <xiantao.zhang@intel.com>
> 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@intel.com>
> ---
> 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.
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...
Jan
--
Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
@ 2008-11-27 11:39 ` Jan Kiszka
0 siblings, 0 replies; 18+ messages in thread
From: Jan Kiszka @ 2008-11-27 11:39 UTC (permalink / raw)
To: Zhang, Xiantao; +Cc: Avi Kivity, kvm@vger.kernel.org, kvm-ia64@vger.kernel.org
Zhang, Xiantao wrote:
> From 1b89616f99abc8e0983ef58a1f984f31a52fe828 Mon Sep 17 00:00:00 2001
> From: Xiantao Zhang <xiantao.zhang@intel.com>
> 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@intel.com>
> ---
> 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.
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...
Jan
--
Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
2008-11-27 11:39 ` Jan Kiszka
@ 2008-11-28 1:47 ` Zhang, Xiantao
-1 siblings, 0 replies; 18+ messages in thread
From: Zhang, Xiantao @ 2008-11-28 1:47 UTC (permalink / raw)
To: kvm-ia64
[-- Attachment #1: Type: text/plain, Size: 3817 bytes --]
Jan Kiszka wrote:
> Zhang, Xiantao wrote:
>> From 1b89616f99abc8e0983ef58a1f984f31a52fe828 Mon Sep 17 00:00:00
>> 2001 From: Xiantao Zhang <xiantao.zhang@intel.com>
>> 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@intel.com> ---
>> 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@intel.com>
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@intel.com>
---
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
--
1.6.0
Xiantao
[-- Attachment #2: 0002-KVM-Qemu-push_nmi-should-be-only-used-by-I386-Arch.patch --]
[-- Type: application/octet-stream, Size: 1727 bytes --]
From c25fa2e4de40e500bd364c3267d5be89a9cfbb4d Mon Sep 17 00:00:00 2001
From: Xiantao Zhang <xiantao.zhang@intel.com>
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@intel.com>
---
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
--
1.6.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* RE: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
@ 2008-11-28 1:47 ` Zhang, Xiantao
0 siblings, 0 replies; 18+ messages in thread
From: Zhang, Xiantao @ 2008-11-28 1:47 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Avi Kivity, kvm@vger.kernel.org, kvm-ia64@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 3818 bytes --]
Jan Kiszka wrote:
> Zhang, Xiantao wrote:
>> From 1b89616f99abc8e0983ef58a1f984f31a52fe828 Mon Sep 17 00:00:00
>> 2001 From: Xiantao Zhang <xiantao.zhang@intel.com>
>> 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@intel.com> ---
>> 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@intel.com>
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@intel.com>
---
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
--
1.6.0
Xiantao
[-- Attachment #2: 0002-KVM-Qemu-push_nmi-should-be-only-used-by-I386-Arch.patch --]
[-- Type: application/octet-stream, Size: 1727 bytes --]
From c25fa2e4de40e500bd364c3267d5be89a9cfbb4d Mon Sep 17 00:00:00 2001
From: Xiantao Zhang <xiantao.zhang@intel.com>
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@intel.com>
---
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
--
1.6.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
2008-11-28 1:47 ` Zhang, Xiantao
@ 2008-11-28 9:26 ` Jan Kiszka
-1 siblings, 0 replies; 18+ messages in thread
From: Jan Kiszka @ 2008-11-28 9:26 UTC (permalink / raw)
To: kvm-ia64
Zhang, Xiantao wrote:
> Jan Kiszka wrote:
>> Zhang, Xiantao wrote:
>>> From 1b89616f99abc8e0983ef58a1f984f31a52fe828 Mon Sep 17 00:00:00
>>> 2001 From: Xiantao Zhang <xiantao.zhang@intel.com>
>>> 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@intel.com> ---
>>> 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@intel.com>
> 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@intel.com>
> ---
> 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
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
@ 2008-11-28 9:26 ` Jan Kiszka
0 siblings, 0 replies; 18+ messages in thread
From: Jan Kiszka @ 2008-11-28 9:26 UTC (permalink / raw)
To: Zhang, Xiantao; +Cc: Avi Kivity, kvm@vger.kernel.org, kvm-ia64@vger.kernel.org
Zhang, Xiantao wrote:
> Jan Kiszka wrote:
>> Zhang, Xiantao wrote:
>>> From 1b89616f99abc8e0983ef58a1f984f31a52fe828 Mon Sep 17 00:00:00
>>> 2001 From: Xiantao Zhang <xiantao.zhang@intel.com>
>>> 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@intel.com> ---
>>> 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@intel.com>
> 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@intel.com>
> ---
> 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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
2008-11-28 9:26 ` Jan Kiszka
@ 2008-12-01 16:38 ` Hollis Blanchard
-1 siblings, 0 replies; 18+ messages in thread
From: Hollis Blanchard @ 2008-12-01 16:38 UTC (permalink / raw)
To: kvm-ia64
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@intel.com>
> > 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@intel.com>
> > ---
> > 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.
Avi, could you revert a8d12f98755be9330fcde055134511f76ecaa538 please?
--
Hollis Blanchard
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
@ 2008-12-01 16:38 ` Hollis Blanchard
0 siblings, 0 replies; 18+ messages in thread
From: Hollis Blanchard @ 2008-12-01 16:38 UTC (permalink / raw)
To: Avi Kivity
Cc: Zhang, Xiantao, kvm@vger.kernel.org, kvm-ia64@vger.kernel.org,
Jan Kiszka
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@intel.com>
> > 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@intel.com>
> > ---
> > 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.
Avi, could you revert a8d12f98755be9330fcde055134511f76ecaa538 please?
--
Hollis Blanchard
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
2008-12-01 16:38 ` Hollis Blanchard
@ 2008-12-01 23:02 ` Jan Kiszka
-1 siblings, 0 replies; 18+ messages in thread
From: Jan Kiszka @ 2008-12-01 23:02 UTC (permalink / raw)
To: kvm-ia64
[-- Attachment #1: Type: text/plain, Size: 4917 bytes --]
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@intel.com>
>>> 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@intel.com>
>>> ---
>>> 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@siemens.com>
---
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
@ 2008-12-01 23:02 ` Jan Kiszka
0 siblings, 0 replies; 18+ messages in thread
From: Jan Kiszka @ 2008-12-01 23:02 UTC (permalink / raw)
To: Hollis Blanchard
Cc: Avi Kivity, Zhang, Xiantao, kvm@vger.kernel.org,
kvm-ia64@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 4917 bytes --]
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@intel.com>
>>> 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@intel.com>
>>> ---
>>> 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@siemens.com>
---
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
2008-12-01 23:02 ` Jan Kiszka
@ 2008-12-01 23:18 ` Hollis Blanchard
-1 siblings, 0 replies; 18+ messages in thread
From: Hollis Blanchard @ 2008-12-01 23:18 UTC (permalink / raw)
To: kvm-ia64
On Tue, 2008-12-02 at 00:02 +0100, Jan Kiszka wrote:
> >
> > 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.
Well, it happens, but I do wish that more people would use cscope or
even grep to find all users of a symbol.
I also wish that Avi would get his PPC box working so he could catch
build breaks like these. Cross-compilers would do as well.
I would also like a pony.
> > Avi, could you revert a8d12f98755be9330fcde055134511f76ecaa538 please?
> >
>
> Here is a patch that reverts change and fixes the root of the issue.
Acked-by: Hollis Blanchard <hollisb@us.ibm.com>
--
Hollis Blanchard
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
@ 2008-12-01 23:18 ` Hollis Blanchard
0 siblings, 0 replies; 18+ messages in thread
From: Hollis Blanchard @ 2008-12-01 23:18 UTC (permalink / raw)
To: Jan Kiszka
Cc: Avi Kivity, Zhang, Xiantao, kvm@vger.kernel.org,
kvm-ia64@vger.kernel.org
On Tue, 2008-12-02 at 00:02 +0100, Jan Kiszka wrote:
> >
> > 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.
Well, it happens, but I do wish that more people would use cscope or
even grep to find all users of a symbol.
I also wish that Avi would get his PPC box working so he could catch
build breaks like these. Cross-compilers would do as well.
I would also like a pony.
> > Avi, could you revert a8d12f98755be9330fcde055134511f76ecaa538 please?
> >
>
> Here is a patch that reverts change and fixes the root of the issue.
Acked-by: Hollis Blanchard <hollisb@us.ibm.com>
--
Hollis Blanchard
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
2008-12-01 23:02 ` Jan Kiszka
@ 2008-12-02 2:01 ` Zhang, Xiantao
-1 siblings, 0 replies; 18+ messages in thread
From: Zhang, Xiantao @ 2008-12-02 2:01 UTC (permalink / raw)
To: kvm-ia64
Oops, seems we introduced the issue together.
Acked-by Xiantao Zhang <xiantao.zhang@intel.com>
-----Original Message-----
From: jan.kiszka@web.de [mailto:jan.kiszka@web.de]
Sent: Tuesday, December 02, 2008 7:03 AM
To: Hollis Blanchard
Cc: Avi Kivity; Zhang, Xiantao; kvm@vger.kernel.org; kvm-ia64@vger.kernel.org
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@intel.com>
>>> 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@intel.com>
>>> ---
>>> 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@siemens.com>
---
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* RE: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
@ 2008-12-02 2:01 ` Zhang, Xiantao
0 siblings, 0 replies; 18+ messages in thread
From: Zhang, Xiantao @ 2008-12-02 2:01 UTC (permalink / raw)
To: jan.kiszka@web.de, Hollis Blanchard
Cc: Avi Kivity, kvm@vger.kernel.org, kvm-ia64@vger.kernel.org
Oops, seems we introduced the issue together.
Acked-by Xiantao Zhang <xiantao.zhang@intel.com>
-----Original Message-----
From: jan.kiszka@web.de [mailto:jan.kiszka@web.de]
Sent: Tuesday, December 02, 2008 7:03 AM
To: Hollis Blanchard
Cc: Avi Kivity; Zhang, Xiantao; kvm@vger.kernel.org; kvm-ia64@vger.kernel.org
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@intel.com>
>>> 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@intel.com>
>>> ---
>>> 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@siemens.com>
---
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
2008-12-01 23:18 ` Hollis Blanchard
@ 2008-12-02 9:26 ` Avi Kivity
-1 siblings, 0 replies; 18+ messages in thread
From: Avi Kivity @ 2008-12-02 9:26 UTC (permalink / raw)
To: kvm-ia64
Hollis Blanchard wrote:
> Well, it happens, but I do wish that more people would use cscope or
> even grep to find all users of a symbol.
>
>
That's reasonable.
> I also wish that Avi would get his PPC box working so he could catch
> build breaks like these. Cross-compilers would do as well.
>
>
I now have a build box somewhere. It's now cloning the source
repositories. Once I start rejecting patches as "won't build", I hope
people will be more careful.
> Acked-by: Hollis Blanchard <hollisb@us.ibm.com>
>
Applied, thanks Jan.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] KVM: Qemu: push_nmi should be only used by I386 Arch.
@ 2008-12-02 9:26 ` Avi Kivity
0 siblings, 0 replies; 18+ messages in thread
From: Avi Kivity @ 2008-12-02 9:26 UTC (permalink / raw)
To: Hollis Blanchard
Cc: Jan Kiszka, Zhang, Xiantao, kvm@vger.kernel.org,
kvm-ia64@vger.kernel.org
Hollis Blanchard wrote:
> Well, it happens, but I do wish that more people would use cscope or
> even grep to find all users of a symbol.
>
>
That's reasonable.
> I also wish that Avi would get his PPC box working so he could catch
> build breaks like these. Cross-compilers would do as well.
>
>
I now have a build box somewhere. It's now cloning the source
repositories. Once I start rejecting patches as "won't build", I hope
people will be more careful.
> Acked-by: Hollis Blanchard <hollisb@us.ibm.com>
>
Applied, thanks Jan.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 18+ messages in thread