[PATCH] Bugfix: Don't use function pointers to inline functions

[PATCH] Bugfix: Don't use function pointers to inline functions

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 514 bytes --]


Hi!

There's code, which use inline functions on function pointers.
This is IMO a bug.

The issue has been found by gcc 4.1.1 on Gentoo Linux when compiling
a x86_64 hypervisor with -O0.
gcc link error messages are attached.

The xen_shadow.diff patch  fixes the issue.
Further, I had to move vmx_update_host_cr3() from 
xen/include/asm-x86/hvm/vmx/vmx.h to xen/arch/x86/hvm/vmx/vmx.c  or gcc 
complains in xen/arch/x86/hvm/vmx/io.c that
"vmx_update_host_cr3() is defined but not used" otherwise.

Please apply.


[-- Attachment #2: link_error.txt --]
[-- Type: text/plain, Size: 2373 bytes --]

/usr/src/xen-unstable.hg/xen/arch/x86/built_in.o: In function `vmx_setup_hvm_funcs':
/usr/src/xen-unstable.hg/xen/arch/x86/hvm/vmx/vmx.c:654: undefined reference to `vmx_update_host_cr3'
/usr/src/xen-unstable.hg/xen/arch/x86/hvm/vmx/vmx.c:654: relocation truncated to fit: R_X86_64_PC32 against undefined symbol `vmx_update_host_cr3'
/usr/src/xen-unstable.hg/xen/arch/x86/built_in.o: In function `sh_map_and_validate_gl4e__shadow_4_guest_4':
/usr/src/xen-unstable.hg/xen/arch/x86/mm/shadow/multi.c:2391: undefined reference to `shadow_l4_index'
/usr/src/xen-unstable.hg/xen/arch/x86/mm/shadow/multi.c:2391: relocation truncated to fit: R_X86_64_PC32 against undefined symbol `shadow_l4_index'
/usr/src/xen-unstable.hg/xen/arch/x86/built_in.o: In function `sh_map_and_validate_gl3e__shadow_4_guest_4':
/usr/src/xen-unstable.hg/xen/arch/x86/mm/shadow/multi.c:2407: undefined reference to `shadow_l3_index'
/usr/src/xen-unstable.hg/xen/arch/x86/mm/shadow/multi.c:2407: relocation truncated to fit: R_X86_64_PC32 against undefined symbol `shadow_l3_index'
/usr/src/xen-unstable.hg/xen/arch/x86/built_in.o: In function `sh_map_and_validate_gl2e__shadow_4_guest_4':
/usr/src/xen-unstable.hg/xen/arch/x86/mm/shadow/multi.c:2422: undefined reference to `shadow_l2_index'
/usr/src/xen-unstable.hg/xen/arch/x86/mm/shadow/multi.c:2422: relocation truncated to fit: R_X86_64_PC32 against undefined symbol `shadow_l2_index'
/usr/src/xen-unstable.hg/xen/arch/x86/built_in.o: In function `sh_map_and_validate_gl2e__shadow_3_guest_3':
/usr/src/xen-unstable.hg/xen/arch/x86/mm/shadow/multi.c:2422: undefined reference to `shadow_l2_index'
/usr/src/xen-unstable.hg/xen/arch/x86/mm/shadow/multi.c:2422: relocation truncated to fit: R_X86_64_PC32 against undefined symbol `shadow_l2_index'
/usr/src/xen-unstable.hg/xen/arch/x86/built_in.o: In function `sh_map_and_validate_gl2he__shadow_3_guest_3':
/usr/src/xen-unstable.hg/xen/arch/x86/mm/shadow/multi.c:2433: undefined reference to `shadow_l2_index'
/usr/src/xen-unstable.hg/xen/arch/x86/mm/shadow/multi.c:2433: relocation truncated to fit: R_X86_64_PC32 against undefined symbol `shadow_l2_index'
make[2]: *** [/usr/src/xen-unstable.hg/xen/xen-syms] Error 1
make[2]: Leaving directory `/usr/src/xen-unstable.hg/xen/arch/x86'
make[1]: *** [/usr/src/xen-unstable.hg/xen/xen] Error 2
make[1]: Leaving directory `/usr/src/xen-unstable.hg/xen'


[-- Attachment #3: xen_shadow.diff --]
[-- Type: text/x-diff, Size: 2109 bytes --]

diff -r d1b0a5adaeab xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c	Wed Nov 29 23:40:40 2006 +0000
+++ b/xen/arch/x86/hvm/vmx/vmx.c	Thu Nov 30 11:13:37 2006 +0100
@@ -628,6 +628,13 @@ static int vmx_pae_enabled(struct vcpu *
 {
     unsigned long cr4 = v->arch.hvm_vmx.cpu_shadow_cr4;
     return (vmx_paging_enabled(v) && (cr4 & X86_CR4_PAE));
+}
+
+/* Works only for vcpu == current */
+static void vmx_update_host_cr3(struct vcpu *v)
+{
+    ASSERT(v == current);
+    __vmwrite(HOST_CR3, v->arch.cr3);
 }
 
 /* Setup HVM interfaces */
diff -r d1b0a5adaeab xen/arch/x86/mm/shadow/multi.c
--- a/xen/arch/x86/mm/shadow/multi.c	Wed Nov 29 23:40:40 2006 +0000
+++ b/xen/arch/x86/mm/shadow/multi.c	Thu Nov 30 11:13:37 2006 +0100
@@ -581,7 +581,7 @@ guest_index(void *ptr)
     return (u32)((unsigned long)ptr & ~PAGE_MASK) / sizeof(guest_l1e_t);
 }
 
-static inline u32
+static u32
 shadow_l1_index(mfn_t *smfn, u32 guest_index)
 {
 #if (GUEST_PAGING_LEVELS == 2) && (SHADOW_PAGING_LEVELS != 2)
@@ -593,7 +593,7 @@ shadow_l1_index(mfn_t *smfn, u32 guest_i
 #endif
 }
 
-static inline u32
+static u32
 shadow_l2_index(mfn_t *smfn, u32 guest_index)
 {
 #if (GUEST_PAGING_LEVELS == 2) && (SHADOW_PAGING_LEVELS != 2)
@@ -613,13 +613,13 @@ shadow_l2_index(mfn_t *smfn, u32 guest_i
 
 #if GUEST_PAGING_LEVELS >= 4
 
-static inline u32
+static u32
 shadow_l3_index(mfn_t *smfn, u32 guest_index)
 {
     return guest_index;
 }
 
-static inline u32
+static u32
 shadow_l4_index(mfn_t *smfn, u32 guest_index)
 {
     return guest_index;
diff -r d1b0a5adaeab xen/include/asm-x86/hvm/vmx/vmx.h
--- a/xen/include/asm-x86/hvm/vmx/vmx.h	Wed Nov 29 23:40:40 2006 +0000
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h	Thu Nov 30 11:13:37 2006 +0100
@@ -272,13 +272,6 @@ static inline int vmx_lme_is_set(struct 
     return efer & EFER_LME;
 }
 
-/* Works only for vcpu == current */
-static inline void vmx_update_host_cr3(struct vcpu *v)
-{
-    ASSERT(v == current);
-    __vmwrite(HOST_CR3, v->arch.cr3);
-}
-
 static inline int vmx_pgbit_test(struct vcpu *v)
 {
     unsigned long cr0 = v->arch.hvm_vmx.cpu_shadow_cr0;

[-- Attachment #4: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] Bugfix: Don't use function pointers to inline functions

[Jan Beulich]

>There's code, which use inline functions on function pointers.
>This is IMO a bug.

I disagree - inline is a hint, not a requirement (other than one might say
about __attribute__((__always_inline__)), or when functions are declared/
defined extern inline). Hence I would expect the compiler to generate
out-of-line instances when addresses of the function are taken. Have
you brought this up with the gcc folks?

Jan