All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()
@ 2013-11-13 23:09 ` Peter Maydell
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2013-11-13 23:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: patches, Richard Henderson, Andreas Tobler, Paolo Bonzini,
	Gleb Natapov, kvm, Anthony Liguori

Fix build failures with clang when KVM is not enabled by
providing a stub version of kvm_arch_get_supported_cpuid().
We retain the compile time check that this function isn't
called when CONFIG_KVM is not set by guarding the stub with
ifndef __OPTIMIZE__ (we assume that an optimizing build will
do sufficient constant folding and dead code elimination to
remove the calls before linking).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Changes v1->v2:
 * guard stub with ifndef __OPTIMIZE__
 * make stub abort() rather than return bogus value

 target-i386/kvm-stub.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target-i386/kvm-stub.c b/target-i386/kvm-stub.c
index 11429c4..2b9e801 100644
--- a/target-i386/kvm-stub.c
+++ b/target-i386/kvm-stub.c
@@ -16,3 +16,15 @@ bool kvm_allows_irq0_override(void)
 {
     return 1;
 }
+
+#ifndef __OPTIMIZE__
+/* This function is only called inside conditionals which we
+ * rely on the compiler to optimize out when CONFIG_KVM is not
+ * defined.
+ */
+uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
+                                      uint32_t index, int reg)
+{
+    abort();
+}
+#endif
-- 
1.7.11.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH v2 for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()
@ 2013-11-13 23:09 ` Peter Maydell
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2013-11-13 23:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Gleb Natapov, kvm, patches, Andreas Tobler, Anthony Liguori,
	Paolo Bonzini, Richard Henderson

Fix build failures with clang when KVM is not enabled by
providing a stub version of kvm_arch_get_supported_cpuid().
We retain the compile time check that this function isn't
called when CONFIG_KVM is not set by guarding the stub with
ifndef __OPTIMIZE__ (we assume that an optimizing build will
do sufficient constant folding and dead code elimination to
remove the calls before linking).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Changes v1->v2:
 * guard stub with ifndef __OPTIMIZE__
 * make stub abort() rather than return bogus value

 target-i386/kvm-stub.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target-i386/kvm-stub.c b/target-i386/kvm-stub.c
index 11429c4..2b9e801 100644
--- a/target-i386/kvm-stub.c
+++ b/target-i386/kvm-stub.c
@@ -16,3 +16,15 @@ bool kvm_allows_irq0_override(void)
 {
     return 1;
 }
+
+#ifndef __OPTIMIZE__
+/* This function is only called inside conditionals which we
+ * rely on the compiler to optimize out when CONFIG_KVM is not
+ * defined.
+ */
+uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
+                                      uint32_t index, int reg)
+{
+    abort();
+}
+#endif
-- 
1.7.11.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()
  2013-11-13 23:09 ` [Qemu-devel] " Peter Maydell
@ 2013-11-14  5:40   ` Stefan Weil
  -1 siblings, 0 replies; 10+ messages in thread
From: Stefan Weil @ 2013-11-14  5:40 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: Gleb Natapov, kvm, patches, Andreas Tobler, Anthony Liguori,
	Paolo Bonzini, Richard Henderson

Am 14.11.2013 00:09, schrieb Peter Maydell:
> Fix build failures with clang when KVM is not enabled by
> providing a stub version of kvm_arch_get_supported_cpuid().
> We retain the compile time check that this function isn't
> called when CONFIG_KVM is not set by guarding the stub with
> ifndef __OPTIMIZE__ (we assume that an optimizing build will
> do sufficient constant folding and dead code elimination to
> remove the calls before linking).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Changes v1->v2:
>  * guard stub with ifndef __OPTIMIZE__
>  * make stub abort() rather than return bogus value
>
>  target-i386/kvm-stub.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/target-i386/kvm-stub.c b/target-i386/kvm-stub.c
> index 11429c4..2b9e801 100644
> --- a/target-i386/kvm-stub.c
> +++ b/target-i386/kvm-stub.c
> @@ -16,3 +16,15 @@ bool kvm_allows_irq0_override(void)
>  {
>      return 1;
>  }
> +
> +#ifndef __OPTIMIZE__
> +/* This function is only called inside conditionals which we
> + * rely on the compiler to optimize out when CONFIG_KVM is not
> + * defined.
> + */
> +uint32_t q(KVMState *env, uint32_t function,
> +                                      uint32_t index, int reg)
> +{
> +    abort();
> +}
> +#endif


Reviewed-by: Stefan Weil <sw@weilnetz.de>

If you happen to write a v3 patch, g_assert_not_reached would be even
better than abort.

It's impossible to tell why QEMU terminated after an assert without a
debugger,
whereas g_assert_not_reached clearly shows the location.

Regards, Stefan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH v2 for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()
@ 2013-11-14  5:40   ` Stefan Weil
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Weil @ 2013-11-14  5:40 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: Gleb Natapov, kvm, patches, Andreas Tobler, Anthony Liguori,
	Paolo Bonzini, Richard Henderson

Am 14.11.2013 00:09, schrieb Peter Maydell:
> Fix build failures with clang when KVM is not enabled by
> providing a stub version of kvm_arch_get_supported_cpuid().
> We retain the compile time check that this function isn't
> called when CONFIG_KVM is not set by guarding the stub with
> ifndef __OPTIMIZE__ (we assume that an optimizing build will
> do sufficient constant folding and dead code elimination to
> remove the calls before linking).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Changes v1->v2:
>  * guard stub with ifndef __OPTIMIZE__
>  * make stub abort() rather than return bogus value
>
>  target-i386/kvm-stub.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/target-i386/kvm-stub.c b/target-i386/kvm-stub.c
> index 11429c4..2b9e801 100644
> --- a/target-i386/kvm-stub.c
> +++ b/target-i386/kvm-stub.c
> @@ -16,3 +16,15 @@ bool kvm_allows_irq0_override(void)
>  {
>      return 1;
>  }
> +
> +#ifndef __OPTIMIZE__
> +/* This function is only called inside conditionals which we
> + * rely on the compiler to optimize out when CONFIG_KVM is not
> + * defined.
> + */
> +uint32_t q(KVMState *env, uint32_t function,
> +                                      uint32_t index, int reg)
> +{
> +    abort();
> +}
> +#endif


Reviewed-by: Stefan Weil <sw@weilnetz.de>

If you happen to write a v3 patch, g_assert_not_reached would be even
better than abort.

It's impossible to tell why QEMU terminated after an assert without a
debugger,
whereas g_assert_not_reached clearly shows the location.

Regards, Stefan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()
  2013-11-13 23:09 ` [Qemu-devel] " Peter Maydell
@ 2013-11-14  8:53   ` Paolo Bonzini
  -1 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2013-11-14  8:53 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-devel, patches, Richard Henderson, Andreas Tobler,
	Gleb Natapov, kvm, Anthony Liguori

Il 14/11/2013 00:09, Peter Maydell ha scritto:
> +#ifndef __OPTIMIZE__
> +/* This function is only called inside conditionals which we
> + * rely on the compiler to optimize out when CONFIG_KVM is not
> + * defined.
> + */
> +uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
> +                                      uint32_t index, int reg)
> +{
> +    abort();
> +}
> +#endif

Thanks, I'll apply this to uq/master and send a pull request.

Paolo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH v2 for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()
@ 2013-11-14  8:53   ` Paolo Bonzini
  0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2013-11-14  8:53 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Gleb Natapov, kvm, patches, qemu-devel, Andreas Tobler,
	Anthony Liguori, Richard Henderson

Il 14/11/2013 00:09, Peter Maydell ha scritto:
> +#ifndef __OPTIMIZE__
> +/* This function is only called inside conditionals which we
> + * rely on the compiler to optimize out when CONFIG_KVM is not
> + * defined.
> + */
> +uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
> +                                      uint32_t index, int reg)
> +{
> +    abort();
> +}
> +#endif

Thanks, I'll apply this to uq/master and send a pull request.

Paolo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH v2 for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()
  2013-11-14  5:40   ` [Qemu-devel] " Stefan Weil
@ 2013-11-14 11:06     ` Peter Maydell
  -1 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2013-11-14 11:06 UTC (permalink / raw)
  To: Stefan Weil
  Cc: QEMU Developers, Gleb Natapov, kvm-devel, Patch Tracking,
	Andreas Tobler, Anthony Liguori, Paolo Bonzini,
	Richard Henderson

On 14 November 2013 05:40, Stefan Weil <sw@weilnetz.de> wrote:
> If you happen to write a v3 patch, g_assert_not_reached would be even
> better than abort.
>
> It's impossible to tell why QEMU terminated after an assert without a
> debugger,
> whereas g_assert_not_reached clearly shows the location.

Maybe. I was mostly just following the pattern of existing
stub functions. Should all instances of abort() be replaced?
(g_assert_not_reached() is compile-time disablable, unlike
abort()).

-- PMM

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH v2 for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()
@ 2013-11-14 11:06     ` Peter Maydell
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2013-11-14 11:06 UTC (permalink / raw)
  To: Stefan Weil
  Cc: kvm-devel, Gleb Natapov, Patch Tracking, QEMU Developers,
	Andreas Tobler, Anthony Liguori, Paolo Bonzini,
	Richard Henderson

On 14 November 2013 05:40, Stefan Weil <sw@weilnetz.de> wrote:
> If you happen to write a v3 patch, g_assert_not_reached would be even
> better than abort.
>
> It's impossible to tell why QEMU terminated after an assert without a
> debugger,
> whereas g_assert_not_reached clearly shows the location.

Maybe. I was mostly just following the pattern of existing
stub functions. Should all instances of abort() be replaced?
(g_assert_not_reached() is compile-time disablable, unlike
abort()).

-- PMM

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH v2 for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()
  2013-11-14 11:06     ` Peter Maydell
@ 2013-11-14 11:17       ` Paolo Bonzini
  -1 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2013-11-14 11:17 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Stefan Weil, QEMU Developers, Gleb Natapov, kvm-devel,
	Patch Tracking, Andreas Tobler, Anthony Liguori,
	Richard Henderson

Il 14/11/2013 12:06, Peter Maydell ha scritto:
> On 14 November 2013 05:40, Stefan Weil <sw@weilnetz.de> wrote:
>> If you happen to write a v3 patch, g_assert_not_reached would be even
>> better than abort.
>>
>> It's impossible to tell why QEMU terminated after an assert without a
>> debugger,
>> whereas g_assert_not_reached clearly shows the location.
> 
> Maybe. I was mostly just following the pattern of existing
> stub functions. Should all instances of abort() be replaced?
> (g_assert_not_reached() is compile-time disablable, unlike
> abort()).

No, disabling asserts is almost always bad, but disabling unconditional
asserts is even worse.

Paolo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH v2 for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()
@ 2013-11-14 11:17       ` Paolo Bonzini
  0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2013-11-14 11:17 UTC (permalink / raw)
  To: Peter Maydell
  Cc: kvm-devel, Gleb Natapov, Patch Tracking, Stefan Weil,
	QEMU Developers, Andreas Tobler, Anthony Liguori,
	Richard Henderson

Il 14/11/2013 12:06, Peter Maydell ha scritto:
> On 14 November 2013 05:40, Stefan Weil <sw@weilnetz.de> wrote:
>> If you happen to write a v3 patch, g_assert_not_reached would be even
>> better than abort.
>>
>> It's impossible to tell why QEMU terminated after an assert without a
>> debugger,
>> whereas g_assert_not_reached clearly shows the location.
> 
> Maybe. I was mostly just following the pattern of existing
> stub functions. Should all instances of abort() be replaced?
> (g_assert_not_reached() is compile-time disablable, unlike
> abort()).

No, disabling asserts is almost always bad, but disabling unconditional
asserts is even worse.

Paolo

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-11-14 11:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-13 23:09 [PATCH v2 for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid() Peter Maydell
2013-11-13 23:09 ` [Qemu-devel] " Peter Maydell
2013-11-14  5:40 ` Stefan Weil
2013-11-14  5:40   ` [Qemu-devel] " Stefan Weil
2013-11-14 11:06   ` Peter Maydell
2013-11-14 11:06     ` Peter Maydell
2013-11-14 11:17     ` Paolo Bonzini
2013-11-14 11:17       ` Paolo Bonzini
2013-11-14  8:53 ` Paolo Bonzini
2013-11-14  8:53   ` [Qemu-devel] " Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.