Add ifdef in irqchip struct for x86 only structures

PATCH Applied Jerone Young 4 participants 6 messages Nov 26, 2007 – Nov 27, 2007 2 messages not in the archive KVM

Share & tools

Jerone Young

author

2007-11-26 14:33 UTC #0 lore raw

PATCH 1/1 Add ifdef in irqchip struct for x86 only structures

# HG changeset patch
# User Jerone Young <jyoung5-r/[email protected]> # Date 1196087575 21600
# Node ID ccf9dd8a8e0a4513090d44d52c879fb9dfbb79dd
# Parent 32d8bd91d9441d2a3655593a0aaf99f6c403d70f
Add ifdef in irqchip struct for x86 only structures.

This patch fixes a small issue where sturctures:
kvm_pic_state
kvm_ioapic_state

are defined inside x86 specific code and may or may not be defined in anyway for other architectures. The problem caused is one cannot compile userspace apps (ex. libkvm) for other archs since a size cannot be determined for these structures.

Signed-off-by Jerone Young <jyoung5-r/[email protected]>

include/linux/kvm.h +2 -0

@@ -51,8 +51,10 @@ struct kvm_irqchip {
 	__u32 pad;
         union {
 		char dummy[512];  /* reserving space */
+#if defined(__i386__) || defined(__x86_64__)
 		struct kvm_pic_state pic;
 		struct kvm_ioapic_state ioapic;
+#endif
 	} chip;
 };

Avi Kivity

Applied

2007-11-26 15:19 UTC #1 lore raw

Quoted 11 lines · Jerone Young

# HG changeset patch
# User Jerone Young <jyoung5-r/[email protected]> # Date 1196087575 21600
# Node ID ccf9dd8a8e0a4513090d44d52c879fb9dfbb79dd
# Parent 32d8bd91d9441d2a3655593a0aaf99f6c403d70f
Add ifdef in irqchip struct for x86 only structures.
This patch fixes a small issue where sturctures:
kvm_pic_state
kvm_ioapic_state

Quoted 7 lines

are defined inside x86 specific code and may or may not be defined in anyway for other architectures. The problem caused is one cannot compile userspace apps (ex. libkvm) for other archs since a size cannot be determined for these structures.

Applied, but changed

Quoted 2 lines

+#if defined(__i386__) || defined(__x86_64__)

    #ifdef CONFIG_X86

Signature · 7 lines

error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

A message that is not in the archive (sent off-list or not yet received): 474AE413.2080905-atKUWr5tajBWk0Htik3J/[email protected]

Jerone Young

author

2007-11-26 16:13 UTC #3 lore raw

Hmm... so now begins the conflict with userspace and kernel space header sharing ;-)

So the issue (which can be easily fixed) is that CONFIG_X86 is not recognized in any userspace apps. While __i386__ & __x86_64__ are. Now this can easily be added to our build system. But it does show the problem.

Also there are other headers in include/linux that use the same if defined method that I used (actually checked to see if anyone had done it in there).

Quoted 21 lines · Avi Kivity

Quoted 11 lines · Jerone Young
# HG changeset patch
# User Jerone Young <jyoung5-r/[email protected]> # Date 1196087575 21600
# Node ID ccf9dd8a8e0a4513090d44d52c879fb9dfbb79dd
# Parent 32d8bd91d9441d2a3655593a0aaf99f6c403d70f
Add ifdef in irqchip struct for x86 only structures.
This patch fixes a small issue where sturctures:
kvm_pic_state
kvm_ioapic_state
Quoted 7 lines
are defined inside x86 specific code and may or may not be defined in anyway for other architectures. The problem caused is one cannot compile userspace apps (ex. libkvm) for other archs since a size cannot be determined for these structures.
Applied, but changed

Quoted 9 lines

Quoted 2 lines
+#if defined(__i386__) || defined(__x86_64__)
to
    #ifdef CONFIG_X86
.

Avi Kivity

2007-11-26 16:50 UTC #4 lore raw

Quoted 13 lines · Jerone Young

Hmm... so now begins the conflict with userspace and kernel space header sharing ;-)
So the issue (which can be easily fixed) is that CONFIG_X86 is not recognized in any userspace apps. While __i386__ & __x86_64__ are. Now this can easily be added to our build system. But it does show the problem.
Also there are other headers in include/linux that use the same if defined method that I used (actually checked to see if anyone had done it in there).

there is an unifdef-y= statement in include/linux/Kbuild that takes care of that. The amended patch uses it.

Quoted 38 lines · Avi Kivity

Quoted 35 lines
Quoted 11 lines · Jerone Young
# HG changeset patch
# User Jerone Young <jyoung5-r/[email protected]> # Date 1196087575 21600
# Node ID ccf9dd8a8e0a4513090d44d52c879fb9dfbb79dd
# Parent 32d8bd91d9441d2a3655593a0aaf99f6c403d70f
Add ifdef in irqchip struct for x86 only structures.
This patch fixes a small issue where sturctures:
kvm_pic_state
kvm_ioapic_state
Quoted 8 lines
are defined inside x86 specific code and may or may not be defined in anyway for other architectures. The problem caused is one cannot compile userspace apps (ex. libkvm) for other archs since a size cannot be determined for these structures.
Applied, but changed
Quoted 3 lines
+#if defined(__i386__) || defined(__x86_64__)
to
    #ifdef CONFIG_X86
.

Signature · 7 lines

error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Anthony Liguori

2007-11-26 19:10 UTC #5 lore raw

In reply to Jerone Young

Perhaps we could just move the declaration of kvm_irqchip to an arch specific header instead of introducing #ifdef's?

Does ia64 have a i8259a?

Regards,

Anthony Liguori

Quoted 35 lines · Jerone Young

# HG changeset patch
# User Jerone Young <jyoung5-r/[email protected]> # Date 1196087575 21600
# Node ID ccf9dd8a8e0a4513090d44d52c879fb9dfbb79dd
# Parent 32d8bd91d9441d2a3655593a0aaf99f6c403d70f
Add ifdef in irqchip struct for x86 only structures.
This patch fixes a small issue where sturctures:
kvm_pic_state
kvm_ioapic_state
are defined inside x86 specific code and may or may not be defined in anyway for other architectures. The problem caused is one cannot compile userspace apps (ex. libkvm) for other archs since a size cannot be determined for these structures.
Signed-off-by Jerone Young <jyoung5-r/[email protected]>
include/linux/kvm.h +2 -1
@@ -51,8 +51,10 @@ struct kvm_irqchip {
 	__u32 pad;
         union {
 		char dummy[512];  /* reserving space */
+#if defined(__i386__) || defined(__x86_64__)
 		struct kvm_pic_state pic;
 		struct kvm_ioapic_state ioapic;
+#endif
 	} chip;
 };
 
 
-------------------------------------------------------------------------

Quoted 9 lines

This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel

A message that is not in the archive (sent off-list or not yet received): [email protected]

Zhang, Xiantao

2007-11-27 09:36 UTC #7 lore raw

Quoted 5 lines

Perhaps we could just move the declaration of kvm_irqchip to an arch specific header instead of introducing #ifdef's?
Does ia64 have a i8259a?

Hi Anthony,
IA64 doesn't need to keep i8259 for platform virtualization, but
we still need ioapic, so I also don't think we can
use #ifdef to solve it.
Xiantao

Regards,

Anthony Liguori

Quoted 25 lines · Jerone Young

# HG changeset patch
# User Jerone Young <jyoung5-r/[email protected]> # Date 1196087575 21600
# Node ID ccf9dd8a8e0a4513090d44d52c879fb9dfbb79dd
# Parent 32d8bd91d9441d2a3655593a0aaf99f6c403d70f
Add ifdef in irqchip struct for x86 only structures.
This patch fixes a small issue where sturctures:
kvm_pic_state
kvm_ioapic_state
are defined inside x86 specific code and may or may not be defined in anyway for other architectures. The problem caused is one cannot compile userspace apps (ex. libkvm) for other archs since a size cannot be determined for these structures.
Signed-off-by Jerone Young <jyoung5-r/[email protected]>

include/linux/kvm.h struct kvm_irqchip { · line 60

Quoted 10 lines

include/linux/kvm.h +2 -0

@@ -51,8 +51,10 @@ struct kvm_irqchip {
 	__u32 pad;
         union {
 		char dummy[512];  /* reserving space */
+#if defined(__i386__) || defined(__x86_64__)
 		struct kvm_pic_state pic;
 		struct kvm_ioapic_state ioapic;
+#endif
 	} chip;
 };

------------------------------------------------------------------------
-

Quoted 9 lines

This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel

------------------------------------------------------------------------
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel