[Qemu-devel] [PATCH] xen: Use conditional compilation for xen map cache (fixes w32 builds)

[Qemu-devel] [PATCH] xen: Use conditional compilation for xen map cache (fixes w32 builds)

[Stefan Weil]

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) {
-- 
1.7.0.4

Re: [Qemu-devel] [PATCH] xen: Use conditional compilation for xen map cache (fixes w32 builds)

[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

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH] xen: Use conditional compilation for xen map cache (fixes w32 builds)

[Stefano Stabellini]

On Wed, 18 May 2011, 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.

After a conversation on IRC with Stefan I found out that most of the
problems are gone away after my recent "xen mapcache fixes and
improvements" patch series.

To completely solve the compilation issues with MinGW32 I just need to
remove two includes from xen-mapcache.h, so I am appending an updated
version of patch #3 "xen: remove xen_map_block and xen_unmap_block" to
do just that.

I have also pushed another git branch:

git://xenbits.xen.org/people/sstabellini/qemu-dm.git mapcache_fixes_2

---


commit 8e1040ee219513d2c692f921f80e2d850aaa47c7
Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Date:   Fri May 13 11:58:59 2011 +0000

    xen: remove xen_map_block and xen_unmap_block
    
    Replace xen_map_block with qemu_map_cache with the appropriate locking
    and size parameters.
    Replace xen_unmap_block with qemu_invalidate_entry.
    
    Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/exec.c b/exec.c
index 01498d2..21f21f0 100644
--- a/exec.c
+++ b/exec.c
@@ -54,6 +54,7 @@
 #endif
 #else /* !CONFIG_USER_ONLY */
 #include "xen-mapcache.h"
+#include "trace.h"
 #endif
 
 //#define DEBUG_TB_INVALIDATE
@@ -3068,7 +3069,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
                 if (block->offset == 0) {
                     return qemu_map_cache(addr, 0, 1);
                 } else if (block->host == NULL) {
-                    block->host = xen_map_block(block->offset, block->length);
+                    block->host = qemu_map_cache(block->offset, block->length, 1);
                 }
             }
             return block->host + (addr - block->offset);
@@ -3097,7 +3098,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr)
                 if (block->offset == 0) {
                     return qemu_map_cache(addr, 0, 1);
                 } else if (block->host == NULL) {
-                    block->host = xen_map_block(block->offset, block->length);
+                    block->host = qemu_map_cache(block->offset, block->length, 1);
                 }
             }
             return block->host + (addr - block->offset);
@@ -3115,19 +3116,7 @@ void qemu_put_ram_ptr(void *addr)
     trace_qemu_put_ram_ptr(addr);
 
     if (xen_mapcache_enabled()) {
-        RAMBlock *block;
-
-        QLIST_FOREACH(block, &ram_list.blocks, next) {
-            if (addr == block->host) {
-                break;
-            }
-        }
-        if (block && block->host) {
-            xen_unmap_block(block->host, block->length);
-            block->host = NULL;
-        } else {
-            qemu_invalidate_entry(addr);
-        }
+            qemu_invalidate_entry(block->host);
     }
 }
 
diff --git a/xen-mapcache-stub.c b/xen-mapcache-stub.c
index 60f712b..8a2380a 100644
--- a/xen-mapcache-stub.c
+++ b/xen-mapcache-stub.c
@@ -34,7 +34,3 @@ void qemu_invalidate_map_cache(void)
 void qemu_invalidate_entry(uint8_t *buffer)
 {
 }
-uint8_t *xen_map_block(target_phys_addr_t phys_addr, target_phys_addr_t size)
-{
-    return NULL;
-}
diff --git a/xen-mapcache.c b/xen-mapcache.c
index 57fe24d..fac47cd 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -362,34 +362,3 @@ void qemu_invalidate_map_cache(void)
 
     mapcache_unlock();
 }
-
-uint8_t *xen_map_block(target_phys_addr_t phys_addr, target_phys_addr_t size)
-{
-    uint8_t *vaddr_base;
-    xen_pfn_t *pfns;
-    int *err;
-    unsigned int i;
-    target_phys_addr_t nb_pfn = size >> XC_PAGE_SHIFT;
-
-    trace_xen_map_block(phys_addr, size);
-    phys_addr >>= XC_PAGE_SHIFT;
-
-    pfns = qemu_mallocz(nb_pfn * sizeof (xen_pfn_t));
-    err = qemu_mallocz(nb_pfn * sizeof (int));
-
-    for (i = 0; i < nb_pfn; i++) {
-        pfns[i] = phys_addr + i;
-    }
-
-    vaddr_base = xc_map_foreign_bulk(xen_xc, xen_domid, PROT_READ|PROT_WRITE,
-                                     pfns, err, nb_pfn);
-    if (vaddr_base == NULL) {
-        perror("xc_map_foreign_bulk");
-        exit(-1);
-    }
-
-    qemu_free(pfns);
-    qemu_free(err);
-
-    return vaddr_base;
-}
diff --git a/xen-mapcache.h b/xen-mapcache.h
index b89b8f9..6216cc3 100644
--- a/xen-mapcache.h
+++ b/xen-mapcache.h
@@ -9,27 +9,12 @@
 #ifndef XEN_MAPCACHE_H
 #define XEN_MAPCACHE_H
 
-#include <sys/mman.h>
-#include "trace.h"
-
 void     qemu_map_cache_init(void);
 uint8_t  *qemu_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size, uint8_t lock);
 ram_addr_t qemu_ram_addr_from_mapcache(void *ptr);
 void     qemu_invalidate_entry(uint8_t *buffer);
 void     qemu_invalidate_map_cache(void);
 
-uint8_t *xen_map_block(target_phys_addr_t phys_addr, target_phys_addr_t size);
-
-static inline void xen_unmap_block(void *addr, ram_addr_t size)
-{
-    trace_xen_unmap_block(addr, size);
-
-    if (munmap(addr, size) != 0) {
-        hw_error("xen_unmap_block: %s", strerror(errno));
-    }
-}
-
-
 #define mapcache_lock()   ((void)0)
 #define mapcache_unlock() ((void)0)
 

Re: [Qemu-devel] [PATCH] xen: Use conditional compilation for xen map cache (fixes w32 builds)

[Stefan Weil]

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

Re: [Qemu-devel] [PATCH] xen: Use conditional compilation for xen map cache (fixes w32 builds)

[Jan Kiszka]

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

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [Qemu-devel] [PATCH] xen: Use conditional compilation for xen map cache (fixes w32 builds)

[Alexander Graf]

On 05/18/2011 08:27 PM, Stefano Stabellini wrote:
> On Wed, 18 May 2011, 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.
> After a conversation on IRC with Stefan I found out that most of the
> problems are gone away after my recent "xen mapcache fixes and
> improvements" patch series.
>
> To completely solve the compilation issues with MinGW32 I just need to
> remove two includes from xen-mapcache.h, so I am appending an updated
> version of patch #3 "xen: remove xen_map_block and xen_unmap_block" to
> do just that.
>
> I have also pushed another git branch:
>
> git://xenbits.xen.org/people/sstabellini/qemu-dm.git mapcache_fixes_2

So please resend v2 to the ML. From my POV the patches look ok, but I'd 
like to give others some more chance of commenting on them.


Alex

Re: [Qemu-devel] [PATCH] xen: Use conditional compilation for xen map cache (fixes w32 builds)

[Stefano Stabellini]

On Thu, 19 May 2011, Alexander Graf wrote:
> On 05/18/2011 08:27 PM, Stefano Stabellini wrote:
> > On Wed, 18 May 2011, 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.
> > After a conversation on IRC with Stefan I found out that most of the
> > problems are gone away after my recent "xen mapcache fixes and
> > improvements" patch series.
> >
> > To completely solve the compilation issues with MinGW32 I just need to
> > remove two includes from xen-mapcache.h, so I am appending an updated
> > version of patch #3 "xen: remove xen_map_block and xen_unmap_block" to
> > do just that.
> >
> > I have also pushed another git branch:
> >
> > git://xenbits.xen.org/people/sstabellini/qemu-dm.git mapcache_fixes_2
> 
> So please resend v2 to the ML. From my POV the patches look ok, but I'd 
> like to give others some more chance of commenting on them.

Ok, I'll do that.