| Message ID | 1305740441-17485-1-git-send-email-weil@mail.berlios.de |
|---|---|
| State | Superseded |
| Headers | show |
On 2011-05-18 19:40, Stefan Weil wrote: > The current implementation used stubs for systems without XEN. > This is unusual for QEMU and adds unneeded dependencies. > > MinGW32 for example does not provide munmap(), so the XEN > code creates compiler warnings (missing prototype). > Compilations without optimisation even result in linker > errors (missing function). > > Fix this by using conditional compilation. > > Cc: Anthony PERARD <anthony.perard@citrix.com> > Cc: Alexander Graf <agraf@suse.de> > Signed-off-by: Stefan Weil <weil@mail.berlios.de> > --- > exec.c | 22 +++++++++++++++++++--- > 1 files changed, 19 insertions(+), 3 deletions(-) > > diff --git a/exec.c b/exec.c > index a6df2d6..7075e98 100644 > --- a/exec.c > +++ b/exec.c > @@ -31,6 +31,7 @@ > #include "hw/hw.h" > #include "hw/qdev.h" > #include "osdep.h" > +#include "trace.h" > #include "kvm.h" > #include "hw/xen.h" > #include "qemu-timer.h" > @@ -53,8 +54,10 @@ > #endif > #endif > #else /* !CONFIG_USER_ONLY */ > +#if defined(CONFIG_XEN_MAPCACHE) > #include "xen-mapcache.h" > #endif > +#endif > > //#define DEBUG_TB_INVALIDATE > //#define DEBUG_FLUSH > @@ -2914,12 +2917,14 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name, > new_block->host = mmap((void*)0x1000000, size, > PROT_EXEC|PROT_READ|PROT_WRITE, > MAP_SHARED | MAP_ANONYMOUS, -1, 0); > -#else > +#elif defined(CONFIG_XEN_MAPCACHE) > if (xen_mapcache_enabled()) { > xen_ram_alloc(new_block->offset, size); > } else { > new_block->host = qemu_vmalloc(size); > } > +#else > + new_block->host = qemu_vmalloc(size); > #endif > qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE); > } > @@ -2967,12 +2972,14 @@ void qemu_ram_free(ram_addr_t addr) > } else { > #if defined(TARGET_S390X) && defined(CONFIG_KVM) > munmap(block->host, block->length); > -#else > +#elif defined(CONFIG_XEN_MAPCACHE) > if (xen_mapcache_enabled()) { > qemu_invalidate_entry(block->host); > } else { > qemu_vfree(block->host); > } > +#else > + qemu_vfree(block->host); > #endif > } > qemu_free(block); > @@ -3061,6 +3068,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr) > QLIST_REMOVE(block, next); > QLIST_INSERT_HEAD(&ram_list.blocks, block, next); > } > +#if defined(CONFIG_XEN_MAPCACHE) > if (xen_mapcache_enabled()) { > /* We need to check if the requested address is in the RAM > * because we don't want to map the entire memory in QEMU. > @@ -3071,6 +3079,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr) > block->host = xen_map_block(block->offset, block->length); > } > } > +#endif > return block->host + (addr - block->offset); > } > } > @@ -3090,6 +3099,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr) > > QLIST_FOREACH(block, &ram_list.blocks, next) { > if (addr - block->offset < block->length) { > +#if defined(CONFIG_XEN_MAPCACHE) > if (xen_mapcache_enabled()) { > /* We need to check if the requested address is in the RAM > * because we don't want to map the entire memory in QEMU. > @@ -3100,6 +3110,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr) > block->host = xen_map_block(block->offset, block->length); > } > } > +#endif > return block->host + (addr - block->offset); > } > } > @@ -3113,7 +3124,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr) > void qemu_put_ram_ptr(void *addr) > { > trace_qemu_put_ram_ptr(addr); > - > +#if defined(CONFIG_XEN_MAPCACHE) > if (xen_mapcache_enabled()) { > RAMBlock *block; > > @@ -3129,6 +3140,7 @@ void qemu_put_ram_ptr(void *addr) > qemu_map_cache_unlock(addr); > } > } > +#endif > } > > int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr) > @@ -3147,10 +3159,12 @@ int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr) > } > } > > +#if defined(CONFIG_XEN_MAPCACHE) > if (xen_mapcache_enabled()) { > *ram_addr = qemu_ram_addr_from_mapcache(ptr); > return 0; > } > +#endif > > return -1; > } > @@ -4059,6 +4073,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len, > access_len -= l; > } > } > +#if defined(CONFIG_XEN_MAPCACHE) > if (xen_mapcache_enabled()) { > uint8_t *buffer1 = buffer; > uint8_t *end_buffer = buffer + len; > @@ -4068,6 +4083,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len, > buffer1 += TARGET_PAGE_SIZE; > } > } > +#endif > return; > } > if (is_write) { Doesn't that obsolete xen-mapcache-stub.c? Then please remove it as well. Jan
Am 18.05.2011 20:02, schrieb Jan Kiszka: > On 2011-05-18 19:40, Stefan Weil wrote: >> The current implementation used stubs for systems without XEN. >> This is unusual for QEMU and adds unneeded dependencies. >> >> MinGW32 for example does not provide munmap(), so the XEN >> code creates compiler warnings (missing prototype). >> Compilations without optimisation even result in linker >> errors (missing function). >> >> Fix this by using conditional compilation. >> >> Cc: Anthony PERARD <anthony.perard@citrix.com> >> Cc: Alexander Graf <agraf@suse.de> >> Signed-off-by: Stefan Weil <weil@mail.berlios.de> >> --- >> exec.c | 22 +++++++++++++++++++--- >> 1 files changed, 19 insertions(+), 3 deletions(-) >> >> diff --git a/exec.c b/exec.c >> index a6df2d6..7075e98 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -31,6 +31,7 @@ >> #include "hw/hw.h" >> #include "hw/qdev.h" >> #include "osdep.h" >> +#include "trace.h" >> #include "kvm.h" >> #include "hw/xen.h" >> #include "qemu-timer.h" >> @@ -53,8 +54,10 @@ >> #endif >> #endif >> #else /* !CONFIG_USER_ONLY */ >> +#if defined(CONFIG_XEN_MAPCACHE) >> #include "xen-mapcache.h" >> #endif >> +#endif >> >> //#define DEBUG_TB_INVALIDATE >> //#define DEBUG_FLUSH >> @@ -2914,12 +2917,14 @@ ram_addr_t >> qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name, >> new_block->host = mmap((void*)0x1000000, size, >> PROT_EXEC|PROT_READ|PROT_WRITE, >> MAP_SHARED | MAP_ANONYMOUS, -1, 0); >> -#else >> +#elif defined(CONFIG_XEN_MAPCACHE) >> if (xen_mapcache_enabled()) { >> xen_ram_alloc(new_block->offset, size); >> } else { >> new_block->host = qemu_vmalloc(size); >> } >> +#else >> + new_block->host = qemu_vmalloc(size); >> #endif >> qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE); >> } >> @@ -2967,12 +2972,14 @@ void qemu_ram_free(ram_addr_t addr) >> } else { >> #if defined(TARGET_S390X) && defined(CONFIG_KVM) >> munmap(block->host, block->length); >> -#else >> +#elif defined(CONFIG_XEN_MAPCACHE) >> if (xen_mapcache_enabled()) { >> qemu_invalidate_entry(block->host); >> } else { >> qemu_vfree(block->host); >> } >> +#else >> + qemu_vfree(block->host); >> #endif >> } >> qemu_free(block); >> @@ -3061,6 +3068,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr) >> QLIST_REMOVE(block, next); >> QLIST_INSERT_HEAD(&ram_list.blocks, block, next); >> } >> +#if defined(CONFIG_XEN_MAPCACHE) >> if (xen_mapcache_enabled()) { >> /* We need to check if the requested address is in the RAM >> * because we don't want to map the entire memory in QEMU. >> @@ -3071,6 +3079,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr) >> block->host = xen_map_block(block->offset, block->length); >> } >> } >> +#endif >> return block->host + (addr - block->offset); >> } >> } >> @@ -3090,6 +3099,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr) >> >> QLIST_FOREACH(block, &ram_list.blocks, next) { >> if (addr - block->offset < block->length) { >> +#if defined(CONFIG_XEN_MAPCACHE) >> if (xen_mapcache_enabled()) { >> /* We need to check if the requested address is in the RAM >> * because we don't want to map the entire memory in QEMU. >> @@ -3100,6 +3110,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr) >> block->host = xen_map_block(block->offset, block->length); >> } >> } >> +#endif >> return block->host + (addr - block->offset); >> } >> } >> @@ -3113,7 +3124,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr) >> void qemu_put_ram_ptr(void *addr) >> { >> trace_qemu_put_ram_ptr(addr); >> - >> +#if defined(CONFIG_XEN_MAPCACHE) >> if (xen_mapcache_enabled()) { >> RAMBlock *block; >> >> @@ -3129,6 +3140,7 @@ void qemu_put_ram_ptr(void *addr) >> qemu_map_cache_unlock(addr); >> } >> } >> +#endif >> } >> >> int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr) >> @@ -3147,10 +3159,12 @@ int qemu_ram_addr_from_host(void *ptr, >> ram_addr_t *ram_addr) >> } >> } >> >> +#if defined(CONFIG_XEN_MAPCACHE) >> if (xen_mapcache_enabled()) { >> *ram_addr = qemu_ram_addr_from_mapcache(ptr); >> return 0; >> } >> +#endif >> >> return -1; >> } >> @@ -4059,6 +4073,7 @@ void cpu_physical_memory_unmap(void *buffer, >> target_phys_addr_t len, >> access_len -= l; >> } >> } >> +#if defined(CONFIG_XEN_MAPCACHE) >> if (xen_mapcache_enabled()) { >> uint8_t *buffer1 = buffer; >> uint8_t *end_buffer = buffer + len; >> @@ -4068,6 +4083,7 @@ void cpu_physical_memory_unmap(void *buffer, >> target_phys_addr_t len, >> buffer1 += TARGET_PAGE_SIZE; >> } >> } >> +#endif >> return; >> } >> if (is_write) { > > Doesn't that obsolete xen-mapcache-stub.c? Then please remove it as well. > > Jan Yes, it does. I had already prepared a patch which removes that stub file (and the exotic declarations in Makefile.target). But the mingw build problem is also fixed by Stefano Stabellini's xen patch series, so my patch is no longer needed (unless we want to get rid of the stub code - do we?). I suggest to apply Stefano's patches first. If needed and wanted, I'll then prepare a new patch which removes xen-mapcache-stub.c. Cheers, Stefan
On 2011-05-18 20:58, Stefan Weil wrote: > Am 18.05.2011 20:02, schrieb Jan Kiszka: >> On 2011-05-18 19:40, Stefan Weil wrote: >>> The current implementation used stubs for systems without XEN. >>> This is unusual for QEMU and adds unneeded dependencies. >>> >>> MinGW32 for example does not provide munmap(), so the XEN >>> code creates compiler warnings (missing prototype). >>> Compilations without optimisation even result in linker >>> errors (missing function). >>> >>> Fix this by using conditional compilation. >>> >>> Cc: Anthony PERARD <anthony.perard@citrix.com> >>> Cc: Alexander Graf <agraf@suse.de> >>> Signed-off-by: Stefan Weil <weil@mail.berlios.de> >>> --- >>> exec.c | 22 +++++++++++++++++++--- >>> 1 files changed, 19 insertions(+), 3 deletions(-) >>> >>> diff --git a/exec.c b/exec.c >>> index a6df2d6..7075e98 100644 >>> --- a/exec.c >>> +++ b/exec.c >>> @@ -31,6 +31,7 @@ >>> #include "hw/hw.h" >>> #include "hw/qdev.h" >>> #include "osdep.h" >>> +#include "trace.h" >>> #include "kvm.h" >>> #include "hw/xen.h" >>> #include "qemu-timer.h" >>> @@ -53,8 +54,10 @@ >>> #endif >>> #endif >>> #else /* !CONFIG_USER_ONLY */ >>> +#if defined(CONFIG_XEN_MAPCACHE) >>> #include "xen-mapcache.h" >>> #endif >>> +#endif >>> >>> //#define DEBUG_TB_INVALIDATE >>> //#define DEBUG_FLUSH >>> @@ -2914,12 +2917,14 @@ ram_addr_t >>> qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name, >>> new_block->host = mmap((void*)0x1000000, size, >>> PROT_EXEC|PROT_READ|PROT_WRITE, >>> MAP_SHARED | MAP_ANONYMOUS, -1, 0); >>> -#else >>> +#elif defined(CONFIG_XEN_MAPCACHE) >>> if (xen_mapcache_enabled()) { >>> xen_ram_alloc(new_block->offset, size); >>> } else { >>> new_block->host = qemu_vmalloc(size); >>> } >>> +#else >>> + new_block->host = qemu_vmalloc(size); >>> #endif >>> qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE); >>> } >>> @@ -2967,12 +2972,14 @@ void qemu_ram_free(ram_addr_t addr) >>> } else { >>> #if defined(TARGET_S390X) && defined(CONFIG_KVM) >>> munmap(block->host, block->length); >>> -#else >>> +#elif defined(CONFIG_XEN_MAPCACHE) >>> if (xen_mapcache_enabled()) { >>> qemu_invalidate_entry(block->host); >>> } else { >>> qemu_vfree(block->host); >>> } >>> +#else >>> + qemu_vfree(block->host); >>> #endif >>> } >>> qemu_free(block); >>> @@ -3061,6 +3068,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr) >>> QLIST_REMOVE(block, next); >>> QLIST_INSERT_HEAD(&ram_list.blocks, block, next); >>> } >>> +#if defined(CONFIG_XEN_MAPCACHE) >>> if (xen_mapcache_enabled()) { >>> /* We need to check if the requested address is in the RAM >>> * because we don't want to map the entire memory in QEMU. >>> @@ -3071,6 +3079,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr) >>> block->host = xen_map_block(block->offset, block->length); >>> } >>> } >>> +#endif >>> return block->host + (addr - block->offset); >>> } >>> } >>> @@ -3090,6 +3099,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr) >>> >>> QLIST_FOREACH(block, &ram_list.blocks, next) { >>> if (addr - block->offset < block->length) { >>> +#if defined(CONFIG_XEN_MAPCACHE) >>> if (xen_mapcache_enabled()) { >>> /* We need to check if the requested address is in the RAM >>> * because we don't want to map the entire memory in QEMU. >>> @@ -3100,6 +3110,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr) >>> block->host = xen_map_block(block->offset, block->length); >>> } >>> } >>> +#endif >>> return block->host + (addr - block->offset); >>> } >>> } >>> @@ -3113,7 +3124,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr) >>> void qemu_put_ram_ptr(void *addr) >>> { >>> trace_qemu_put_ram_ptr(addr); >>> - >>> +#if defined(CONFIG_XEN_MAPCACHE) >>> if (xen_mapcache_enabled()) { >>> RAMBlock *block; >>> >>> @@ -3129,6 +3140,7 @@ void qemu_put_ram_ptr(void *addr) >>> qemu_map_cache_unlock(addr); >>> } >>> } >>> +#endif >>> } >>> >>> int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr) >>> @@ -3147,10 +3159,12 @@ int qemu_ram_addr_from_host(void *ptr, >>> ram_addr_t *ram_addr) >>> } >>> } >>> >>> +#if defined(CONFIG_XEN_MAPCACHE) >>> if (xen_mapcache_enabled()) { >>> *ram_addr = qemu_ram_addr_from_mapcache(ptr); >>> return 0; >>> } >>> +#endif >>> >>> return -1; >>> } >>> @@ -4059,6 +4073,7 @@ void cpu_physical_memory_unmap(void *buffer, >>> target_phys_addr_t len, >>> access_len -= l; >>> } >>> } >>> +#if defined(CONFIG_XEN_MAPCACHE) >>> if (xen_mapcache_enabled()) { >>> uint8_t *buffer1 = buffer; >>> uint8_t *end_buffer = buffer + len; >>> @@ -4068,6 +4083,7 @@ void cpu_physical_memory_unmap(void *buffer, >>> target_phys_addr_t len, >>> buffer1 += TARGET_PAGE_SIZE; >>> } >>> } >>> +#endif >>> return; >>> } >>> if (is_write) { >> >> Doesn't that obsolete xen-mapcache-stub.c? Then please remove it as well. >> >> Jan > > Yes, it does. I had already prepared a patch which removes that stub file > (and the exotic declarations in Makefile.target). > > But the mingw build problem is also fixed by Stefano Stabellini's > xen patch series, so my patch is no longer needed > (unless we want to get rid of the stub code - do we?). > > I suggest to apply Stefano's patches first. No concerns. > If needed and wanted, > I'll then prepare a new patch which removes xen-mapcache-stub.c. IMHO, yes. Unless I miss something, the code that is stubbed away here is never referenced by generic parts. So there is no benefit in creating another object, built by all targets for no good except a longer build time. We may still improve the #ifdefery above by providing stub services as static inlines in the corresponding headers. Jan
diff --git a/exec.c b/exec.c index a6df2d6..7075e98 100644 --- a/exec.c +++ b/exec.c @@ -31,6 +31,7 @@ #include "hw/hw.h" #include "hw/qdev.h" #include "osdep.h" +#include "trace.h" #include "kvm.h" #include "hw/xen.h" #include "qemu-timer.h" @@ -53,8 +54,10 @@ #endif #endif #else /* !CONFIG_USER_ONLY */ +#if defined(CONFIG_XEN_MAPCACHE) #include "xen-mapcache.h" #endif +#endif //#define DEBUG_TB_INVALIDATE //#define DEBUG_FLUSH @@ -2914,12 +2917,14 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name, new_block->host = mmap((void*)0x1000000, size, PROT_EXEC|PROT_READ|PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0); -#else +#elif defined(CONFIG_XEN_MAPCACHE) if (xen_mapcache_enabled()) { xen_ram_alloc(new_block->offset, size); } else { new_block->host = qemu_vmalloc(size); } +#else + new_block->host = qemu_vmalloc(size); #endif qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE); } @@ -2967,12 +2972,14 @@ void qemu_ram_free(ram_addr_t addr) } else { #if defined(TARGET_S390X) && defined(CONFIG_KVM) munmap(block->host, block->length); -#else +#elif defined(CONFIG_XEN_MAPCACHE) if (xen_mapcache_enabled()) { qemu_invalidate_entry(block->host); } else { qemu_vfree(block->host); } +#else + qemu_vfree(block->host); #endif } qemu_free(block); @@ -3061,6 +3068,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr) QLIST_REMOVE(block, next); QLIST_INSERT_HEAD(&ram_list.blocks, block, next); } +#if defined(CONFIG_XEN_MAPCACHE) if (xen_mapcache_enabled()) { /* We need to check if the requested address is in the RAM * because we don't want to map the entire memory in QEMU. @@ -3071,6 +3079,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr) block->host = xen_map_block(block->offset, block->length); } } +#endif return block->host + (addr - block->offset); } } @@ -3090,6 +3099,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr) QLIST_FOREACH(block, &ram_list.blocks, next) { if (addr - block->offset < block->length) { +#if defined(CONFIG_XEN_MAPCACHE) if (xen_mapcache_enabled()) { /* We need to check if the requested address is in the RAM * because we don't want to map the entire memory in QEMU. @@ -3100,6 +3110,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr) block->host = xen_map_block(block->offset, block->length); } } +#endif return block->host + (addr - block->offset); } } @@ -3113,7 +3124,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr) void qemu_put_ram_ptr(void *addr) { trace_qemu_put_ram_ptr(addr); - +#if defined(CONFIG_XEN_MAPCACHE) if (xen_mapcache_enabled()) { RAMBlock *block; @@ -3129,6 +3140,7 @@ void qemu_put_ram_ptr(void *addr) qemu_map_cache_unlock(addr); } } +#endif } int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr) @@ -3147,10 +3159,12 @@ int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr) } } +#if defined(CONFIG_XEN_MAPCACHE) if (xen_mapcache_enabled()) { *ram_addr = qemu_ram_addr_from_mapcache(ptr); return 0; } +#endif return -1; } @@ -4059,6 +4073,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len, access_len -= l; } } +#if defined(CONFIG_XEN_MAPCACHE) if (xen_mapcache_enabled()) { uint8_t *buffer1 = buffer; uint8_t *end_buffer = buffer + len; @@ -4068,6 +4083,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len, buffer1 += TARGET_PAGE_SIZE; } } +#endif return; } if (is_write) {
The current implementation used stubs for systems without XEN. This is unusual for QEMU and adds unneeded dependencies. MinGW32 for example does not provide munmap(), so the XEN code creates compiler warnings (missing prototype). Compilations without optimisation even result in linker errors (missing function). Fix this by using conditional compilation. Cc: Anthony PERARD <anthony.perard@citrix.com> Cc: Alexander Graf <agraf@suse.de> Signed-off-by: Stefan Weil <weil@mail.berlios.de> --- exec.c | 22 +++++++++++++++++++--- 1 files changed, 19 insertions(+), 3 deletions(-)