[Qemu-devel] [PATCH] hw/9pfs: fix P9_STATS_GEN handling

2013-10-27 Thread Kirill A. Shutemov 
From: Kirill A. Shutemov kir...@shutemov.name

Currently we have few issues with P9_STATS_GEN:

 - We don't try to read st_gen anything except files or directories, but
   still set P9_STATS_GEN bit in st_result_mask. It may mislead client:
   we present garbage as valid st_gen.

 - If we failed to get valid st_gen with ENOTTY, we ignore error, but
   still set P9_STATS_GEN bit in st_result_mask.

 - If we failed to get valid st_gen with any other errno, we fail
   getattr altogether. It's excessive: we block valid client use-cases,
   like chdir(2) to non-readable directory with execution bit set.

The patch fixes these issues and cleanup code a bit.

Signed-off-by: Kirill A. Shutemov kir...@shutemov.name
---
 hw/9pfs/cofile.c   |  4 
 hw/9pfs/virtio-9p-handle.c |  8 +++-
 hw/9pfs/virtio-9p-local.c  | 10 ++
 hw/9pfs/virtio-9p-proxy.c  |  3 ++-
 hw/9pfs/virtio-9p.c| 12 ++--
 5 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
index 194c1306c6..2efebf3571 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -38,10 +38,6 @@ int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t 
st_mode,
 });
 v9fs_path_unlock(s);
 }
-/* The ioctl may not be supported depending on the path */
-if (err == -ENOTTY) {
-err = 0;
-}
 return err;
 }
 
diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c
index fe8e0ed19d..17002a3d28 100644
--- a/hw/9pfs/virtio-9p-handle.c
+++ b/hw/9pfs/virtio-9p-handle.c
@@ -582,6 +582,7 @@ static int handle_unlinkat(FsContext *ctx, V9fsPath *dir,
 static int handle_ioc_getversion(FsContext *ctx, V9fsPath *path,
  mode_t st_mode, uint64_t *st_gen)
 {
+#ifdef FS_IOC_GETVERSION
 int err;
 V9fsFidOpenState fid_open;
 
@@ -590,7 +591,8 @@ static int handle_ioc_getversion(FsContext *ctx, V9fsPath 
*path,
  * We can get fd for regular files and directories only
  */
 if (!S_ISREG(st_mode)  !S_ISDIR(st_mode)) {
-return 0;
+errno = ENOTTY;
+return -1;
 }
 err = handle_open(ctx, path, O_RDONLY, fid_open);
 if (err  0) {
@@ -599,6 +601,10 @@ static int handle_ioc_getversion(FsContext *ctx, V9fsPath 
*path,
 err = ioctl(fid_open.fd, FS_IOC_GETVERSION, st_gen);
 handle_close(ctx, fid_open);
 return err;
+#else
+errno = ENOTTY;
+return -1;
+#endif
 }
 
 static int handle_init(FsContext *ctx)
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index fc93e9e6e8..df0dbffa7a 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -1068,8 +1068,8 @@ err_out:
 static int local_ioc_getversion(FsContext *ctx, V9fsPath *path,
 mode_t st_mode, uint64_t *st_gen)
 {
-int err;
 #ifdef FS_IOC_GETVERSION
+int err;
 V9fsFidOpenState fid_open;
 
 /*
@@ -1077,7 +1077,8 @@ static int local_ioc_getversion(FsContext *ctx, V9fsPath 
*path,
  * We can get fd for regular files and directories only
  */
 if (!S_ISREG(st_mode)  !S_ISDIR(st_mode)) {
-return 0;
+errno = ENOTTY;
+return -1;
 }
 err = local_open(ctx, path, O_RDONLY, fid_open);
 if (err  0) {
@@ -1085,10 +1086,11 @@ static int local_ioc_getversion(FsContext *ctx, 
V9fsPath *path,
 }
 err = ioctl(fid_open.fd, FS_IOC_GETVERSION, st_gen);
 local_close(ctx, fid_open);
+return err;
 #else
-err = -ENOTTY;
+errno = ENOTTY;
+return -1;
 #endif
-return err;
 }
 
 static int local_init(FsContext *ctx)
diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
index 5f44bb758b..b57966d9d8 100644
--- a/hw/9pfs/virtio-9p-proxy.c
+++ b/hw/9pfs/virtio-9p-proxy.c
@@ -1086,7 +1086,8 @@ static int proxy_ioc_getversion(FsContext *fs_ctx, 
V9fsPath *path,
  * we can get fd for regular files and directories only
  */
 if (!S_ISREG(st_mode)  !S_ISDIR(st_mode)) {
-return 0;
+errno = ENOTTY;
+return -1;
 }
 err = v9fs_request(fs_ctx-private, T_GETVERSION, st_gen, s, path);
 if (err  0) {
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 8cbb8ae32a..3e51fcd152 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -1080,10 +1080,18 @@ static void v9fs_getattr(void *opaque)
 /*  fill st_gen if requested and supported by underlying fs */
 if (request_mask  P9_STATS_GEN) {
 retval = v9fs_co_st_gen(pdu, fidp-path, stbuf.st_mode, v9stat_dotl);
-if (retval  0) {
+switch (retval) {
+case 0:
+/* we have valid st_gen: update result mask */
+v9stat_dotl.st_result_mask |= P9_STATS_GEN;
+break;
+case -EINTR:
+/* request cancelled */
 goto out;
+default:
+/* failed to get st_gen: not fatal, ignore */
+break;
 }
-v9stat_dotl.st_result_mask |= P9_STATS_GEN;

[Qemu-devel] [PATCH] hw/9pfs: fix P9_STATS_GEN handling

2013-10-27 Thread Kirill A. Shutemov 
Currently we have few issues with P9_STATS_GEN:

 - We don't try to read st_gen anything except files or directories, but
   still set P9_STATS_GEN bit in st_result_mask. It may mislead client:
   we present garbage as valid st_gen.

 - If we failed to get valid st_gen with ENOTTY, we ignore error, but
   still set P9_STATS_GEN bit in st_result_mask.

 - If we failed to get valid st_gen with any other errno, we fail
   getattr altogether. It's excessive: we block valid client use-cases,
   like chdir(2) to non-readable directory with execution bit set.

The patch fixes these issues and cleanup code a bit.

Signed-off-by: Kirill A. Shutemov kirill.shute...@linux.intel.com
---
 hw/9pfs/cofile.c   |  4 
 hw/9pfs/virtio-9p-handle.c |  8 +++-
 hw/9pfs/virtio-9p-local.c  | 10 ++
 hw/9pfs/virtio-9p-proxy.c  |  3 ++-
 hw/9pfs/virtio-9p.c| 12 ++--
 5 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
index 194c1306c6..2efebf3571 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -38,10 +38,6 @@ int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t 
st_mode,
 });
 v9fs_path_unlock(s);
 }
-/* The ioctl may not be supported depending on the path */
-if (err == -ENOTTY) {
-err = 0;
-}
 return err;
 }
 
diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c
index fe8e0ed19d..17002a3d28 100644
--- a/hw/9pfs/virtio-9p-handle.c
+++ b/hw/9pfs/virtio-9p-handle.c
@@ -582,6 +582,7 @@ static int handle_unlinkat(FsContext *ctx, V9fsPath *dir,
 static int handle_ioc_getversion(FsContext *ctx, V9fsPath *path,
  mode_t st_mode, uint64_t *st_gen)
 {
+#ifdef FS_IOC_GETVERSION
 int err;
 V9fsFidOpenState fid_open;
 
@@ -590,7 +591,8 @@ static int handle_ioc_getversion(FsContext *ctx, V9fsPath 
*path,
  * We can get fd for regular files and directories only
  */
 if (!S_ISREG(st_mode)  !S_ISDIR(st_mode)) {
-return 0;
+errno = ENOTTY;
+return -1;
 }
 err = handle_open(ctx, path, O_RDONLY, fid_open);
 if (err  0) {
@@ -599,6 +601,10 @@ static int handle_ioc_getversion(FsContext *ctx, V9fsPath 
*path,
 err = ioctl(fid_open.fd, FS_IOC_GETVERSION, st_gen);
 handle_close(ctx, fid_open);
 return err;
+#else
+errno = ENOTTY;
+return -1;
+#endif
 }
 
 static int handle_init(FsContext *ctx)
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index fc93e9e6e8..df0dbffa7a 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -1068,8 +1068,8 @@ err_out:
 static int local_ioc_getversion(FsContext *ctx, V9fsPath *path,
 mode_t st_mode, uint64_t *st_gen)
 {
-int err;
 #ifdef FS_IOC_GETVERSION
+int err;
 V9fsFidOpenState fid_open;
 
 /*
@@ -1077,7 +1077,8 @@ static int local_ioc_getversion(FsContext *ctx, V9fsPath 
*path,
  * We can get fd for regular files and directories only
  */
 if (!S_ISREG(st_mode)  !S_ISDIR(st_mode)) {
-return 0;
+errno = ENOTTY;
+return -1;
 }
 err = local_open(ctx, path, O_RDONLY, fid_open);
 if (err  0) {
@@ -1085,10 +1086,11 @@ static int local_ioc_getversion(FsContext *ctx, 
V9fsPath *path,
 }
 err = ioctl(fid_open.fd, FS_IOC_GETVERSION, st_gen);
 local_close(ctx, fid_open);
+return err;
 #else
-err = -ENOTTY;
+errno = ENOTTY;
+return -1;
 #endif
-return err;
 }
 
 static int local_init(FsContext *ctx)
diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
index 5f44bb758b..b57966d9d8 100644
--- a/hw/9pfs/virtio-9p-proxy.c
+++ b/hw/9pfs/virtio-9p-proxy.c
@@ -1086,7 +1086,8 @@ static int proxy_ioc_getversion(FsContext *fs_ctx, 
V9fsPath *path,
  * we can get fd for regular files and directories only
  */
 if (!S_ISREG(st_mode)  !S_ISDIR(st_mode)) {
-return 0;
+errno = ENOTTY;
+return -1;
 }
 err = v9fs_request(fs_ctx-private, T_GETVERSION, st_gen, s, path);
 if (err  0) {
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 8cbb8ae32a..3e51fcd152 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -1080,10 +1080,18 @@ static void v9fs_getattr(void *opaque)
 /*  fill st_gen if requested and supported by underlying fs */
 if (request_mask  P9_STATS_GEN) {
 retval = v9fs_co_st_gen(pdu, fidp-path, stbuf.st_mode, v9stat_dotl);
-if (retval  0) {
+switch (retval) {
+case 0:
+/* we have valid st_gen: update result mask */
+v9stat_dotl.st_result_mask |= P9_STATS_GEN;
+break;
+case -EINTR:
+/* request cancelled */
 goto out;
+default:
+/* failed to get st_gen: not fatal, ignore */
+break;
 }
-v9stat_dotl.st_result_mask |= P9_STATS_GEN;
 }
 retval = pdu_marshal(pdu,

Re: [Qemu-devel] qemu 1.6.1

2013-10-27 Thread Paolo Bonzini 
Il 26/10/2013 11:51, Stefan Weil ha scritto:
 Am 24.10.2013 23:47, schrieb Paolo Bonzini:
 Il 24/10/2013 17:37, Stefan Weil ha scritto:
 Yes, that works, too. It also fixes the problem with the assertion
 (tested with Wine).

 No, we cannot remove from_, because the same interface is also used
 for Linux and other hosts which don't have a 'current' variable.
 Or we would have to call qemu_coroutine_self() to get the current
 coroutine.
 Yes, I was thinking of using qemu_coroutine_self().

 By the way, can you post the two assembly language outputs for just

 - CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, from_);
 + CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, current);

 which AIUI works and is enough to fix the bug?

 Paolo
 
 See disassembled code below. I removed compiler option -fstack-protector-all
 to simplify the assembler code and tested that the result was not affected
 by this removal.
 
 The C and assembler code from the test is also available at
 http://qemu.weilnetz.de/test/coroutine-win32/.

Here is the code with annotations

 broken   works
  -
 push   %ebx
 sub$0x18,%espsub$0x1c,%esp   
  mov%ebx,0x14(%esp)  
  mov%esi,0x18(%esp)  

 movl   $0x6d62a8,(%esp)  movl   $0x6d62a8,(%esp) 
 mov0x24(%esp),%ebx   mov0x24(%esp),%ebx
  ebx = to;
 call   ___emutls_get_address call   ___emutls_get_address  
  eax = current;

  mov(%eax),%esi
  esi = current;

 mov%ebx,(%eax)   mov%ebx,(%eax)
  current = to;

 mov0x28(%esp),%eax   mov0x28(%esp),%eax
  eax = action
 mov%eax,0x24(%ebx)   mov%eax,0x24(%ebx)
  to-action = action
 mov0x20(%ebx),%eax   mov0x20(%ebx),%eax
  eax = to-fiber
 mov%eax,(%esp)   mov%eax,(%esp)
  push to-fiber
 call   *0x835fc0 call   *0x835fc0  
  SwitchToFiber(to-fiber)
 sub$0x4,%esp sub$0x4,%esp  
  undo PASCAL calling convention

**   mov0x20(%esp),%eax 
  eax = from
 mov0x24(%eax),%eax   mov0x24(%esi),%eax
  eax = from-action

  mov0x14(%esp),%ebx  
  mov0x18(%esp),%esi  
 add$0x18,%espadd$0x1c,%esp   
 pop%ebx  
 ret  ret 


I think the problem is that 0x20(%esp) gets somehow corrupted at the
instruction I highlighted with **.

The simplest fix then would be to add a barrier() before and after
SwitchToFiber.

Paolo

Re: [Qemu-devel] [PATCH] block: Don't copy backing file name on error

2013-10-27 Thread Amos Kong 
On Sat, Oct 26, 2013 at 9:44 PM, Max Reitz mre...@redhat.com wrote:

 bdrv_open_backing_file() tries to copy the backing file name using
 pstrcpy directly after calling bdrv_open() to open the backing file
 without checking whether that was actually successful. If it was not,
 ps-backing_hd-file will probably be NULL and qemu will crash.

 Fix this by moving pstrcpy after checking whether bdrv_open() succeeded.

Reviewed-by: Amos Kong kongjian...@gmail.com


 Signed-off-by: Max Reitz mre...@redhat.com
 ---
  block.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/block.c b/block.c
 index 4474012..61795fe 100644
 --- a/block.c
 +++ b/block.c
 @@ -1005,8 +1005,6 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
 *options, Error **errp)
  ret = bdrv_open(bs-backing_hd,
  *backing_filename ? backing_filename : NULL, options,
  back_flags, back_drv, local_err);
 -pstrcpy(bs-backing_file, sizeof(bs-backing_file),
 -bs-backing_hd-file-filename);
  if (ret  0) {
  bdrv_unref(bs-backing_hd);
  bs-backing_hd = NULL;
 @@ -1014,6 +1012,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
 *options, Error **errp)
  error_propagate(errp, local_err);
  return ret;
  }
 +pstrcpy(bs-backing_file, sizeof(bs-backing_file),
 +bs-backing_hd-file-filename);
  return 0;
  }

 --
 1.8.4.1

Re: [Qemu-devel] qemu 1.6.1

2013-10-27 Thread Stefan Weil 
Am 27.10.2013 07:54, schrieb Paolo Bonzini:
 Here is the code with annotations

  broken   works
   -
  push   %ebx
  sub$0x18,%espsub$0x1c,%esp   
   mov%ebx,0x14(%esp)  
   mov%esi,0x18(%esp)  
 
  movl   $0x6d62a8,(%esp)  movl   $0x6d62a8,(%esp) 
  mov0x24(%esp),%ebx   mov0x24(%esp),%ebx  
 ebx = to;
  call   ___emutls_get_address call   ___emutls_get_address
 eax = current;
 
   mov(%eax),%esi  
 esi = current;
 
  mov%ebx,(%eax)   mov%ebx,(%eax)  
 current = to;

  mov0x28(%esp),%eax   mov0x28(%esp),%eax  
 eax = action
  mov%eax,0x24(%ebx)   mov%eax,0x24(%ebx)  
 to-action = action
  mov0x20(%ebx),%eax   mov0x20(%ebx),%eax  
 eax = to-fiber
  mov%eax,(%esp)   mov%eax,(%esp)  
 push to-fiber
  call   *0x835fc0 call   *0x835fc0
 SwitchToFiber(to-fiber)
  sub$0x4,%esp sub$0x4,%esp
 undo PASCAL calling convention
 
 **   mov0x20(%esp),%eax   
 eax = from
  mov0x24(%eax),%eax   mov0x24(%esi),%eax  
 eax = from-action
 
   mov0x14(%esp),%ebx  
   mov0x18(%esp),%esi  
  add$0x18,%espadd$0x1c,%esp   
  pop%ebx  
  ret  ret 


 I think the problem is that 0x20(%esp) gets somehow corrupted at the
 instruction I highlighted with **.

 The simplest fix then would be to add a barrier() before and after
 SwitchToFiber.

 Paolo

I tried adding two barrier() statements around SwitchToFiber().
That change did not result in different assembler code (= unchanged
behaviour, QEMU still raises an assertion).

Stefan

Re: [Qemu-devel] Fix SMB security configuration on newer samba versions

2013-10-27 Thread Amos Kong 
On Mon, Oct 21, 2013 at 7:50 PM, Michael Büsch m...@bues.ch wrote:
 The following changes fix the samba security configuration on
 newer samba versions.

 samba version 4.0.10-Debian throws this warning:

   WARNING: Ignoring invalid value 'share' for parameter 'security'

 Which makes it fall back to security=user without guest login.

 Fix this by selecting 'user' explicitly and mapping unknown users to guest 
 logins.

 Signed-off-by: Michael Buesch m...@bues.ch

 ---
 Index: qemu-1.6.1/net/slirp.c
 ===
 --- qemu-1.6.1.orig/net/slirp.c 2013-10-09 21:20:32.0 +0200
 +++ qemu-1.6.1/net/slirp.c  2013-10-21 13:49:39.918960448 +0200
 @@ -529,7 +529,8 @@
  state directory=%s\n
  log file=%s/log.smbd\n
  smb passwd file=%s/smbpasswd\n
 -security = share\n
 +security = user\n
 +map to guest = Bad User\n

does this still work with old samba?

  [qemu]\n
  path=%s\n
  read only=no\n

Re: [Qemu-devel] [PATCH v2] net: disallow to specify multicast MAC address

2013-10-27 Thread Amos Kong 
On Mon, Oct 21, 2013 at 4:08 PM, Dmitry Krivenok
krivenok.dmi...@gmail.com wrote:
 Changes to v1:
 1) Resolved names clash in include/net/eth.h
 2) Reused is_multicast_ether_addr() from that header for MAC check.

 Signed-off-by: Dmitry V. Krivenok krivenok.dmi...@gmail.com

Reviewed-by: Amos Kong kongjian...@gmail.com

 ---
  include/net/eth.h | 6 +++---
  net/net.c | 6 ++
  2 files changed, 9 insertions(+), 3 deletions(-)

 diff --git a/include/net/eth.h b/include/net/eth.h
 index 1d48e06..b3273b8 100644
 --- a/include/net/eth.h
 +++ b/include/net/eth.h
 @@ -84,7 +84,7 @@ typedef struct ip_pseudo_header {
  } ip_pseudo_header;

  /* IPv6 address */
 -struct in6_addr {
 +struct in6_address {
  union {
  uint8_t __u6_addr8[16];
  } __in6_u;
 @@ -105,8 +105,8 @@ struct ip6_header {
  uint8_t  ip6_un3_ecn;  /* 2 bits ECN, top 6 bits payload length 
 */
  } ip6_un3;
  } ip6_ctlun;
 -struct in6_addr ip6_src; /* source address */
 -struct in6_addr ip6_dst; /* destination address */
 +struct in6_address ip6_src;/* source address */
 +struct in6_address ip6_dst;/* destination address */
  };

  struct ip6_ext_hdr {
...

Re: [Qemu-devel] Fix SMB security configuration on newer samba versions

2013-10-27 Thread Michael Büsch 
On Sun, 27 Oct 2013 18:50:18 +0800
Amos Kong kongjian...@gmail.com wrote:

  Index: qemu-1.6.1/net/slirp.c
  ===
  --- qemu-1.6.1.orig/net/slirp.c 2013-10-09 21:20:32.0 +0200
  +++ qemu-1.6.1/net/slirp.c  2013-10-21 13:49:39.918960448 +0200
  @@ -529,7 +529,8 @@
   state directory=%s\n
   log file=%s/log.smbd\n
   smb passwd file=%s/smbpasswd\n
  -security = share\n
  +security = user\n
  +map to guest = Bad User\n
 
 does this still work with old samba?


I didn't test it on anything older.

I'd appreciate if somebody on the list with an older version could try it.

-- 
Michael.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [patch 2/2] i386: pc: align gpa-hpa on 1GB boundary

2013-10-27 Thread igor Mammedov 
On Fri, 25 Oct 2013 11:34:22 -0200
Marcelo Tosatti mtosa...@redhat.com wrote:

 On Fri, Oct 25, 2013 at 11:57:18AM +0200, igor Mammedov wrote:
  On Fri, 25 Oct 2013 02:58:05 -0200
  Marcelo Tosatti mtosa...@redhat.com wrote:
  
   On Fri, Oct 25, 2013 at 12:55:36AM +0100, Paolo Bonzini wrote:
 +if (hpagesize == (130)) {
 +unsigned long holesize = 0x1ULL -
 below_4g_mem_size; +
 +memory_region_init_alias(ram_above_4g, NULL,
 ram-above-4g, ram,
 +0x1ULL,
 +above_4g_mem_size -
 holesize);
 +memory_region_add_subregion(system_memory,
 0x1ULL,
 +ram_above_4g);
 +
 +ram_above_4g_piecetwo =
 g_malloc(sizeof(*ram_above_4g_piecetwo));
 +memory_region_init_alias(ram_above_4g_piecetwo,
 NULL,
 + ram-above-4g-piecetwo,
 ram,
 + 0x1ULL -
 holesize, holesize);
 +memory_region_add_subregion(system_memory,
 +0x1ULL +
 +above_4g_mem_size -
 holesize,
 +
 ram_above_4g_piecetwo);

Why break it in two?  You can just allocate extra holesize
bytes in the ram MemoryRegion, and not map the part that
corresponds to [0x1ULL - holesize, 0x1ULL).
   
   - If the ram MemoryRegion is backed with 1GB hugepages, you
   might not want to allocate extra holesize bytes (which might
   require an entire 1GB page).
  From POV of moddeling current ram as dimm devices, aliasing
  wouldn't work nice. But breaking one block in two or more is fine
  since then blocks could be represented as several dimm devices.
  
  +3Gb backend ram it could be split in blocks like this:
  
[ 3Gb (1Gb pages backed) ]
[tail1 (below_4gb - 3Gb) (2mb pages backed) ]
[above_4gb whole X Gb pages (1Gb pages backed)]
[tail2 (2mb pages backed)]
 
 Yes, thought of that, unfortunately its cumbersome to add an interface
 for the user to supply both 2MB and 1GB hugetlbfs pages.
Could 2Mb tails be automated, meaning if host uses 1Gb hugepages and
there is/are tail/s, QEMU should be able to figure out alignment
issues and allocate with appropriate pages.

Goal is separate host part allocation aspect from guest related one,
aliasing 32-bit hole size at the end doesn't help it at all, it's quite
opposite, it's making current code more complicated and harder to fix
in the future.

[Qemu-devel] [PATCH v3 0/2] Documentation for coroutine annotations

2013-10-27 Thread Charlie Shepherd 
These patches were the first two from my GSoC series and were reasonably
straight-forward and well accepted. Gabriel and I are hoping the patches from
GSoC can be merged before I start my job in December, so I'm starting by sending
the simple parts of the overall patchset, when they are merged then I will redo
the later parts in several smaller and more manageable patchsets.

Charlie Shepherd (2):
  Add an explanation of when a function should be marked coroutine_fn
  Rename qemu_coroutine_self to qemu_coroutine_self_int and add an
annotated wrapper

 coroutine-gthread.c   |  2 +-
 coroutine-sigaltstack.c   |  2 +-
 coroutine-ucontext.c  |  2 +-
 coroutine-win32.c |  2 +-
 include/block/coroutine.h |  8 
 include/block/coroutine_int.h |  1 +
 qemu-coroutine.c  | 15 ++-
 7 files changed, 27 insertions(+), 5 deletions(-)

-- 
1.8.4.rc3

[Qemu-devel] [PATCH v3 1/2] Add an explanation of when a function should be marked coroutine_fn

2013-10-27 Thread Charlie Shepherd 
Coroutine functions that can yield directly or indirectly should be annotated
with a coroutine_fn annotation. Add an explanation to that effect in
include/block/coroutine.h.
---
 include/block/coroutine.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/include/block/coroutine.h b/include/block/coroutine.h
index 4232569..e11a587 100644
--- a/include/block/coroutine.h
+++ b/include/block/coroutine.h
@@ -38,6 +38,14 @@
  * static checker support for catching such errors.  This annotation might make
  * it possible and in the meantime it serves as documentation.
  *
+ * A function must be marked with coroutine_fn if it can yield execution, 
either
+ * directly or indirectly.
+ *
+ * Some functions dynamically determine whether to yield or not based on
+ * whether they are executing in a coroutine context or not. These functions
+ * do not need to be annotated coroutine_fn. Note that this practice is
+ * deprecated and is being phased out, new code should not do this.
+ *
  * For example:
  *
  *   static void coroutine_fn foo(void) {
-- 
1.8.4.rc3

[Qemu-devel] [PATCH v3 2/2] Rename qemu_coroutine_self to qemu_coroutine_self_int and add an annotated wrapper

2013-10-27 Thread Charlie Shepherd 
While it only really makes sense to call qemu_coroutine_self() in a coroutine
context, some coroutine internals need to call it from functions not annotated
as coroutine_fn, so add an annotated wrapper and rename the implementation
versions to qemu_coroutine_self_int.
---
 coroutine-gthread.c   |  2 +-
 coroutine-sigaltstack.c   |  2 +-
 coroutine-ucontext.c  |  2 +-
 coroutine-win32.c |  2 +-
 include/block/coroutine_int.h |  1 +
 qemu-coroutine.c  | 15 ++-
 6 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/coroutine-gthread.c b/coroutine-gthread.c
index d3e5b99..a913aeb 100644
--- a/coroutine-gthread.c
+++ b/coroutine-gthread.c
@@ -194,7 +194,7 @@ CoroutineAction qemu_coroutine_switch(Coroutine *from_,
 return from-action;
 }
 
-Coroutine *qemu_coroutine_self(void)
+Coroutine *qemu_coroutine_self_int(void)
 {
 CoroutineGThread *co = get_coroutine_key();
 if (!co) {
diff --git a/coroutine-sigaltstack.c b/coroutine-sigaltstack.c
index 3de0bb3..0556539 100644
--- a/coroutine-sigaltstack.c
+++ b/coroutine-sigaltstack.c
@@ -277,7 +277,7 @@ CoroutineAction qemu_coroutine_switch(Coroutine *from_, 
Coroutine *to_,
 return ret;
 }
 
-Coroutine *qemu_coroutine_self(void)
+Coroutine *qemu_coroutine_self_int(void)
 {
 CoroutineThreadState *s = coroutine_get_thread_state();
 
diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
index 4bf2cde..27d1b79 100644
--- a/coroutine-ucontext.c
+++ b/coroutine-ucontext.c
@@ -210,7 +210,7 @@ CoroutineAction qemu_coroutine_switch(Coroutine *from_, 
Coroutine *to_,
 return ret;
 }
 
-Coroutine *qemu_coroutine_self(void)
+Coroutine *qemu_coroutine_self_int(void)
 {
 CoroutineThreadState *s = coroutine_get_thread_state();
 
diff --git a/coroutine-win32.c b/coroutine-win32.c
index edc1f72..3f1f79b 100644
--- a/coroutine-win32.c
+++ b/coroutine-win32.c
@@ -77,7 +77,7 @@ void qemu_coroutine_delete(Coroutine *co_)
 g_free(co);
 }
 
-Coroutine *qemu_coroutine_self(void)
+Coroutine *qemu_coroutine_self_int(void)
 {
 if (!current) {
 current = leader.base;
diff --git a/include/block/coroutine_int.h b/include/block/coroutine_int.h
index f133d65..f6191ad 100644
--- a/include/block/coroutine_int.h
+++ b/include/block/coroutine_int.h
@@ -48,6 +48,7 @@ Coroutine *qemu_coroutine_new(void);
 void qemu_coroutine_delete(Coroutine *co);
 CoroutineAction qemu_coroutine_switch(Coroutine *from, Coroutine *to,
   CoroutineAction action);
+Coroutine *qemu_coroutine_self_int(void);
 void coroutine_fn qemu_co_queue_run_restart(Coroutine *co);
 
 #endif
diff --git a/qemu-coroutine.c b/qemu-coroutine.c
index 4708521..563e6ec 100644
--- a/qemu-coroutine.c
+++ b/qemu-coroutine.c
@@ -108,7 +108,7 @@ static void coroutine_swap(Coroutine *from, Coroutine *to)
 
 void qemu_coroutine_enter(Coroutine *co, void *opaque)
 {
-Coroutine *self = qemu_coroutine_self();
+Coroutine *self = qemu_coroutine_self_int();
 
 trace_qemu_coroutine_enter(self, co, opaque);
 
@@ -137,3 +137,16 @@ void coroutine_fn qemu_coroutine_yield(void)
 self-caller = NULL;
 coroutine_swap(self, to);
 }
+
+Coroutine *coroutine_fn qemu_coroutine_self(void)
+{
+/* Call the internal version of this function, which is
+ * non-coroutine_fn and can therefore be called from from
+ * non-coroutine contexts.  Internally we know it's always possible
+ * to pull a Coroutine* out of thin air (or thread-local storage).
+ * External callers shouldn't assume they can always get a
+ * Coroutine* since we may not be in coroutine context, hence the
+ * external version of this function.
+ */
+return qemu_coroutine_self_int();
+}
-- 
1.8.4.rc3

Re: [Qemu-devel] qemu 1.6.1

2013-10-27 Thread Stefan Weil 
Am 27.10.2013 07:54, schrieb Paolo Bonzini:

 I think the problem is that 0x20(%esp) gets somehow corrupted at the
 instruction I highlighted with **.

 The simplest fix then would be to add a barrier() before and after
 SwitchToFiber.

 Paolo

I added some debugging output (see code at
http://qemu.weilnetz.de/test/coroutine-win32/). The result with pointer
values replaced by function names is shown below.

There are two threads (WinMain, qemu_tcg_cpu_thread_fn) and one
coroutine (bdrv_rw_co_entry).

Stefan


qemu_coroutine_self:WinMain
qemu_in_coroutine:WinMain,null
qemu_coroutine_new:bdrv_rw_co_entry
qemu_coroutine_create,co=bdrv_rw_co_entry
qemu_coroutine_enter(bdrv_rw_co_entry,...),self=WinMain
coroutine_swap(WinMain,bdrv_rw_co_entry)
qemu_coroutine_yield,self=bdrv_rw_co_entry,to=WinMain
coroutine_swap(bdrv_rw_co_entry,WinMain)
qemu_coroutine_enter(bdrv_rw_co_entry,...),self=WinMain
coroutine_swap(WinMain,bdrv_rw_co_entry)
qemu_in_coroutine:bdrv_rw_co_entry,WinMain
# active coroutine bdrv_rw_co_entry is deleted
coroutine_delete(bdrv_rw_co_entry)
qemu_coroutine_self:qemu_tcg_cpu_thread_fn
qemu_in_coroutine:qemu_tcg_cpu_thread_fn,null
qemu_coroutine_create,co=bdrv_rw_co_entry
qemu_coroutine_enter(bdrv_rw_co_entry,...),self=qemu_tcg_cpu_thread_fn
coroutine_swap(qemu_tcg_cpu_thread_fn,bdrv_rw_co_entry)
qemu_coroutine_yield,self=bdrv_rw_co_entry,to=qemu_tcg_cpu_thread_fn
coroutine_swap(bdrv_rw_co_entry,qemu_tcg_cpu_thread_fn)
qemu_coroutine_enter(bdrv_rw_co_entry,...),self=qemu_tcg_cpu_thread_fn
coroutine_swap(qemu_tcg_cpu_thread_fn,bdrv_rw_co_entry)
qemu_in_coroutine:bdrv_rw_co_entry,qemu_tcg_cpu_thread_fn
qemu_in_coroutine:bdrv_rw_co_entry,qemu_tcg_cpu_thread_fn
# active coroutine bdrv_rw_co_entry is deleted
coroutine_delete(bdrv_rw_co_entry)
# now we are still in deleted coroutine bdrv_rw_co_entry
qemu_in_coroutine:bdrv_rw_co_entry,null
qemu_coroutine_create,co=bdrv_rw_co_entry
qemu_coroutine_enter(bdrv_rw_co_entry,...),self=bdrv_rw_co_entry
coroutine_swap(bdrv_rw_co_entry,bdrv_rw_co_entry)
qemu_coroutine_yield,self=bdrv_rw_co_entry,to=bdrv_rw_co_entry
coroutine_swap(bdrv_rw_co_entry,bdrv_rw_co_entry)
qemu_in_coroutine:bdrv_rw_co_entry,null

Re: [Qemu-devel] [PATCH v7] powerpc: add PVR mask support

2013-10-27 Thread Alexander Graf 

On 23.10.2013, at 07:57, Andreas Färber afaer...@suse.de wrote:

 Am 27.09.2013 09:05, schrieb Alexey Kardashevskiy:
 IBM POWERPC processors encode PVR as a CPU family in higher 16 bits and
 a CPU version in lower 16 bits. Since there is no significant change
 in behavior between versions, there is no point to add every single CPU
 version in QEMU's CPU list. Also, new CPU versions of already supported
 CPU won't break the existing code.
 
 This adds PVR value/mask support for KVM, i.e. for -cpu host option.
 
 As CPU family class name for POWER7 is POWER7-family, there is no need
 to touch aliases.
 
 Cc: Andreas Färber afaer...@suse.de
 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
 
 As promised to Paul, using the Hackathon timeslot to review this:
 
 Reviewed-by: Andreas Färber afaer...@suse.de

Thanks, applied to ppc-next-1.8


Alex

Re: [Qemu-devel] [PATCH] spapr: add vio-bus devices to categories

2013-10-27 Thread Alexander Graf 

On 10.10.2013, at 20:08, Alexey Kardashevskiy a...@ozlabs.ru wrote:

 In order to get devices appear in output of
 ./qemu-system-ppc64 -device ?,
 they must be assigned to one of DEVICE_CATEGORY_.
 
 This puts VIO devices classes to corresponding categories.
 
 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru

Thanks, applied to ppc-next-1.8


Alex

Re: [Qemu-devel] [RFC PATCH] spapr: add ibmveth to the supported network adapters list

2013-10-27 Thread Alexander Graf 

On 10.10.2013, at 20:09, Alexey Kardashevskiy a...@ozlabs.ru wrote:

 The problem is that -net nic,model=? does not print ibmveth in
 the list while it is actually supported.
 
 Most of the QEMU emulated network devices are PCI but ibmveth
 (a.k.a. spapr-vlan) is not. However with -net nic,model=?, QEMU prints
 only PCI devices in the list, even if it does not say that the list is
 all about PCI devices.
 
 This adds ?/help handling in spapr.c and adds ibmveth in the beginning
 of the list.
 
 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
 ---
 
 This is an RFC patch.
 
 The other solutions could be:
 1. add ibmveth into pci_nic_models[] in hw/pci/pci.c but this would not
 be correct as ibmveth is not PCI and it must appear only on pseries machine.
 
 2. implemement short version of qdev_print_category_devices() and call it
 with DEVICE_CATEGORY_NETWORK but that would print more devices than
 pci_nic_init_nofail() can handle (vmxnet3, usb-bt-dongle).
 
 3. fix qemu_check_nic_model() to specifically say that this is a list of
 PCI devices and there might be some other devices which -net nic,model+
 supports but there are not PCI but that could break compatibility (some
 management software may rely on this exact string).
 
 4. Reject the patch and just say that people must stop using -net. Ok for 
 me :)
 
 Since -net is kind of obsolete interface and does not seem to be extended 
 ever,
 the proposed patch does not look too ugly, does not it?
 ---
 hw/ppc/spapr.c | 15 +++
 1 file changed, 15 insertions(+)
 
 diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
 index c0613e4..45ed3da 100644
 --- a/hw/ppc/spapr.c
 +++ b/hw/ppc/spapr.c
 @@ -1276,6 +1276,21 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
 
 if (strcmp(nd-model, ibmveth) == 0) {
 spapr_vlan_create(spapr-vio_bus, nd);
 +} else if (is_help_option(nd-model)) {
 +static const char * const nic_models[] = {
 +ibmveth,
 +ne2k_pci,
 +i82551,
 +i82557b,
 +i82559er,
 +rtl8139,
 +e1000,
 +pcnet,
 +virtio,
 +NULL
 +};

I don't like the idea of duplicating that list. Basically the list of supported 
-net models is incorrect today even on x86 where you can say -net 
nic,model=ne2k_isa. It really is only a list of PCI devices.

I can think of a number of convoluted ways to fix this up, but I think that 
ignoring fully accuracy of the output of -net model=? is the most straight 
forward thing to do.


Alex

Re: [Qemu-devel] [PATCH -V6 2/3] target-ppc: Fix page table lookup with kvm enabled

2013-10-27 Thread Alexander Graf 

On 15.10.2013, at 01:58, Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com 
wrote:

 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 With kvm enabled, we store the hash page table information in the hypervisor.
 Use ioctl to read the htab contents. Without this we get the below error when
 trying to read the guest address
 
 (gdb) x/10 do_fork
 0xc0098660 do_fork:   Cannot access memory at address 
 0xc0098660
 (gdb)
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 ---
 Changes from V5:
 
 * Added two new patches
 * Address review comments
 
 hw/ppc/spapr_hcall.c| 47 --
 target-ppc/kvm.c| 53 ++
 target-ppc/kvm_ppc.h| 19 
 target-ppc/mmu-hash64.c | 77 -
 target-ppc/mmu-hash64.h | 23 ++-
 5 files changed, 181 insertions(+), 38 deletions(-)
 
 diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
 index f10ba8a..e04bf6c 100644
 --- a/hw/ppc/spapr_hcall.c
 +++ b/hw/ppc/spapr_hcall.c
 @@ -52,6 +52,8 @@ static target_ulong h_enter(PowerPCCPU *cpu, 
 sPAPREnvironment *spapr,
 target_ulong raddr;
 target_ulong i;
 hwaddr hpte;
 +void *token;
 +bool htab_fd;
 
 /* only handle 4k and 16M pages for now */
 if (pteh  HPTE64_V_LARGE) {
 @@ -94,25 +96,32 @@ static target_ulong h_enter(PowerPCCPU *cpu, 
 sPAPREnvironment *spapr,
 if ((pte_index * HASH_PTE_SIZE_64)  ~env-htab_mask) {
 return H_PARAMETER;
 }
 +
 +i = 0;
 +hpte = pte_index * HASH_PTE_SIZE_64;
 if (likely((flags  H_EXACT) == 0)) {
 pte_index = ~7ULL;
 -hpte = pte_index * HASH_PTE_SIZE_64;
 -for (i = 0; ; ++i) {
 +token = ppc_hash64_start_access(cpu, pte_index, htab_fd);
 +do {
 if (i == 8) {
 +ppc_hash64_stop_access(token, htab_fd);
 return H_PTEG_FULL;
 }
 -if ((ppc_hash64_load_hpte0(env, hpte)  HPTE64_V_VALID) == 0) {
 +if ((ppc_hash64_load_hpte0(env, token, i)  HPTE64_V_VALID) == 
 0) {
 break;
 }
 -hpte += HASH_PTE_SIZE_64;
 -}
 +} while (i++);
 +ppc_hash64_stop_access(token, htab_fd);
 } else {
 -i = 0;
 -hpte = pte_index * HASH_PTE_SIZE_64;
 -if (ppc_hash64_load_hpte0(env, hpte)  HPTE64_V_VALID) {
 +token = ppc_hash64_start_access(cpu, pte_index, htab_fd);
 +if (ppc_hash64_load_hpte0(env, token, 0)  HPTE64_V_VALID) {
 +ppc_hash64_stop_access(token, htab_fd);
 return H_PTEG_FULL;
 }
 +ppc_hash64_stop_access(token, htab_fd);
 }
 +hpte += i * HASH_PTE_SIZE_64;
 +
 ppc_hash64_store_hpte1(env, hpte, ptel);
 /* eieio();  FIXME: need some sort of barrier for smp? */
 ppc_hash64_store_hpte0(env, hpte, pteh | HPTE64_V_HPTE_DIRTY);
 @@ -134,16 +143,18 @@ static RemoveResult remove_hpte(CPUPPCState *env, 
 target_ulong ptex,
 target_ulong *vp, target_ulong *rp)
 {
 hwaddr hpte;
 +void *token;
 +bool htab_fd;
 target_ulong v, r, rb;
 
 if ((ptex * HASH_PTE_SIZE_64)  ~env-htab_mask) {
 return REMOVE_PARM;
 }
 
 -hpte = ptex * HASH_PTE_SIZE_64;
 -
 -v = ppc_hash64_load_hpte0(env, hpte);
 -r = ppc_hash64_load_hpte1(env, hpte);
 +token = ppc_hash64_start_access(ppc_env_get_cpu(env), ptex, htab_fd);
 +v = ppc_hash64_load_hpte0(env, token, 0);
 +r = ppc_hash64_load_hpte1(env, token, 0);
 +ppc_hash64_stop_access(token, htab_fd);
 
 if ((v  HPTE64_V_VALID) == 0 ||
 ((flags  H_AVPN)  (v  ~0x7fULL) != avpn) ||
 @@ -152,6 +163,7 @@ static RemoveResult remove_hpte(CPUPPCState *env, 
 target_ulong ptex,
 }
 *vp = v;
 *rp = r;
 +hpte = ptex * HASH_PTE_SIZE_64;
 ppc_hash64_store_hpte0(env, hpte, HPTE64_V_HPTE_DIRTY);
 rb = compute_tlbie_rb(v, r, ptex);
 ppc_tlb_invalidate_one(env, rb);
 @@ -260,16 +272,18 @@ static target_ulong h_protect(PowerPCCPU *cpu, 
 sPAPREnvironment *spapr,
 target_ulong pte_index = args[1];
 target_ulong avpn = args[2];
 hwaddr hpte;
 +void *token;
 +bool htab_fd;
 target_ulong v, r, rb;
 
 if ((pte_index * HASH_PTE_SIZE_64)  ~env-htab_mask) {
 return H_PARAMETER;
 }
 
 -hpte = pte_index * HASH_PTE_SIZE_64;
 -
 -v = ppc_hash64_load_hpte0(env, hpte);
 -r = ppc_hash64_load_hpte1(env, hpte);
 +token = ppc_hash64_start_access(cpu, pte_index, htab_fd);
 +v = ppc_hash64_load_hpte0(env, token, 0);
 +r = ppc_hash64_load_hpte1(env, token, 0);
 +ppc_hash64_stop_access(token, htab_fd);
 
 if ((v  HPTE64_V_VALID) == 0 ||
 ((flags  H_AVPN)  (v  ~0x7fULL) != avpn)) {
 @@ -282,6 +296,7 @@ static target_ulong h_protect(PowerPCCPU *cpu, 
 sPAPREnvironment *spapr,
 r |= (flags  48)  HPTE64_R_KEY_HI;
 r |= flags  (HPTE64_R_PP

Re: [Qemu-devel] [PATCH -V6 3/3] target-ppc: Fix htab_mask calculation

2013-10-27 Thread Alexander Graf 

On 15.10.2013, at 01:58, Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com 
wrote:

 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 Correctly update the htab_mask using the return value of
 KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
 on GET_SREGS for HV. So don't update htab_mask if sdr1
 is found to be zero. Fix the pte index calculation to be
 same as that found in the kernel
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 ---
 hw/ppc/spapr.c  | 3 ++-
 target-ppc/mmu-hash64.c | 2 +-
 target-ppc/mmu_helper.c | 4 +++-
 3 files changed, 6 insertions(+), 3 deletions(-)
 
 diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
 index 22f2a8a..d4f3502 100644
 --- a/hw/ppc/spapr.c
 +++ b/hw/ppc/spapr.c
 @@ -724,7 +724,8 @@ static void spapr_cpu_reset(void *opaque)
 env-external_htab = (void *)1;
 }
 env-htab_base = -1;
 -env-htab_mask = HTAB_SIZE(spapr) - 1;
 +/* 128 (2**7) bytes in each HPTEG */
 +env-htab_mask = (1ULL  ((spapr)-htab_shift - 7)) - 1;

HTAB_SIZE(spapr) / 128? The compiler should be smart enough to produce the same 
code out of that.

However, could you please explain why it's better to have the mask be on the 
PTEG rather than the offset? Is this something you missed in the previous 
patch? If so, please change the semantics on what htab_mask means before you 
break the code as that makes bisecting hard.

Furthermore, since you are changing the semantics of htab_mask, have you 
checked all other users of it? Most notably the hash32 code.


Alex

 env-spr[SPR_SDR1] = (target_ulong)(uintptr_t)spapr-htab |
 (spapr-htab_shift - 18);
 }
 diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
 index 5c797c3..ddd8440 100644
 --- a/target-ppc/mmu-hash64.c
 +++ b/target-ppc/mmu-hash64.c
 @@ -354,7 +354,7 @@ static hwaddr ppc_hash64_pteg_search(CPUPPCState *env, 
 hwaddr hash,
 target_ulong pte0, pte1;
 unsigned long pte_index;
 
 -pte_index = (hash * HPTES_PER_GROUP)  env-htab_mask;
 +pte_index = (hash  env-htab_mask) * HPTES_PER_GROUP;
 token = ppc_hash64_start_access(ppc_env_get_cpu(env), pte_index, 
 htab_fd);
 if (!token) {
 return -1;
 diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
 index 04a840b..c39cb7b 100644
 --- a/target-ppc/mmu_helper.c
 +++ b/target-ppc/mmu_helper.c
 @@ -2025,7 +2025,9 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong 
 value)
  stored in SDR1\n, htabsize);
 htabsize = 28;
 }
 -env-htab_mask = (1ULL  (htabsize + 18)) - 1;
 +if (htabsize) {
 +env-htab_mask = (1ULL  (htabsize + 18 - 7)) - 1;
 +}
 env-htab_base = value  SDR_64_HTABORG;
 } else
 #endif /* defined(TARGET_PPC64) */
 -- 
 1.8.3.2

Re: [Qemu-devel] [PATCH -V5] target-ppc: Fix page table lookup with kvm enabled

2013-10-27 Thread Alexander Graf 

On 11.10.2013, at 09:58, Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com 
wrote:

 Alexander Graf ag...@suse.de writes:
 
 On 11.10.2013, at 13:13, Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com 
 wrote:
 
 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 With kvm enabled, we store the hash page table information in the 
 hypervisor.
 Use ioctl to read the htab contents. Without this we get the below error 
 when
 trying to read the guest address
 
 (gdb) x/10 do_fork
 0xc0098660 do_fork:   Cannot access memory at address 
 0xc0098660
 (gdb)
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 ---
 Changes from V4:
 * Rewrite to avoid two code paths for doing hash lookups
 
 
 
 +
 +i = 0;
 +hpte = pte_index * HASH_PTE_SIZE_64;
if (likely((flags  H_EXACT) == 0)) {
pte_index = ~7ULL;
 -hpte = pte_index * HASH_PTE_SIZE_64;
 -for (i = 0; ; ++i) {
 +token = ppc_hash64_start_access(ppc_env_get_cpu(env), pte_index);
 +do {
 
 Why convert this into a while loop?
 
 I am moving i = 0 outside the loop. Hence found while () better than 
 for(;;++i) 

Outside of what loop? You're only moving it outside of the if().

 
 
if (i == 8) {
 +ppc_hash64_stop_access(token);
return H_PTEG_FULL;
}
 -if ((ppc_hash64_load_hpte0(env, hpte)  HPTE64_V_VALID) == 0) {
 +if ((ppc_hash64_load_hpte0(token, i)  HPTE64_V_VALID) == 0) {
break;
}
 -hpte += HASH_PTE_SIZE_64;
 -}
 +} while (i++);
 +ppc_hash64_stop_access(token);
 
 
 
 
 +
 +int kvm_ppc_hash64_start_access(PowerPCCPU *cpu, unsigned long pte_index,
 +struct ppc_hash64_hpte_token *token)
 +{
 +int htab_fd;
 +int hpte_group_size;
 +struct kvm_get_htab_fd ghf;
 +struct kvm_get_htab_buf {
 +struct kvm_get_htab_header header;
 +/*
 + * We required one extra byte for read
 + */
 +unsigned long hpte[(HPTES_PER_GROUP * 2) + 1];
 +} hpte_buf;;
 
 Double semicolon?
 
 Will fix
 
 
 +
 +ghf.flags = 0;
 +ghf.start_index = pte_index;
 +htab_fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, ghf);
 +if (htab_fd  0) {
 +goto error_out;
 +}
 +memset(hpte_buf, 0, sizeof(hpte_buf));
 
 
 
 diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
 index 67fc1b5..aeb4593 100644
 --- a/target-ppc/mmu-hash64.c
 +++ b/target-ppc/mmu-hash64.c
 @@ -302,29 +302,73 @@ static int ppc_hash64_amr_prot(CPUPPCState *env, 
 ppc_hash_pte64_t pte)
return prot;
 }
 
 -static hwaddr ppc_hash64_pteg_search(CPUPPCState *env, hwaddr pteg_off,
 +struct ppc_hash64_hpte_token *ppc_hash64_start_access(PowerPCCPU *cpu,
 +  unsigned long 
 pte_index)
 
 How about you also pass in the number of PTEs you want to access?
 Let's call it pte_num for now. Then if you only care about one PTE
 you can indicate so, otherwise it's clear that you want to access 8
 PTEs beginning from the one you're pointing at.
 
 So if we want to pass pte_num, then i can be any number, 1, 8, 10. That
 would make the code complex, because now we need to make the buffer
 passed to read() of variable size.Also i would need another allocation
 for the return buffer. I can do tricks like make the token handle the
 pointer to actual buffer skipping the header. But ppc_hash64=stop_acess then
 would have to know about kvm htab read header which i found not nice.
 We can possibly update the function name to indicate that it will always
 read hptegroup from the pte_index. Something like ppc64_start_hpteg_access() 
 ?. 

Just abort() if pte_num is not 1 or 8.

 
 
 +{
 +hwaddr pte_offset;
 +struct ppc_hash64_hpte_token *token;
 
 void *token = NULL;
 
 if (kvmppc_uses_htab_fd(cpu)) {
/* HTAB is controlled by KVM. Fetch the PTEG into a new buffer. */
 
int hpte_group_size = sizeof(unsigned long) * 2 * pte_num;
token = g_malloc(hpte_group_size);
if (kvm_ppc_hash64_read_pteg(cpu, pte_index, token)) {
 
 That is the tricky part, the read buffer need to have a header in the
 beginning. May be i can do kvm_ppc_hash64_stop_access(void *token) that
 does the pointer match gets to the head of token and free. Will try that.
 
free(token);
return NULL;
}
 } else {
/* HTAB is controlled by QEMU. Just point to the internally accessible 
 PTEG. */
hwaddr pte_offset;
 
pte_offset = pte_index * HASH_PTE_SIZE_64;
if (cpu-env.external_htab) {
token = cpu-env.external_htab + pte_offset;
} else {
token = (uint8_t *) cpu-env.htab_base + pte_offset;
}
 }
 
 return token;
 
 This way it's more obvious which path the normal code flow would be. We 
 also only clearly choose what to do depending on in-kernel HTAB or now. As a 
 big plus we don't need a struct that we need

Re: [Qemu-devel] [PATCH v3 0/2] Documentation for coroutine annotations

2013-10-27 Thread Gabriel Kerneis 
On Sun, Oct 27, 2013 at 04:23:54PM +0100, Charlie Shepherd wrote:
 These patches were the first two from my GSoC series and were reasonably
 straight-forward and well accepted. Gabriel and I are hoping the patches from
 GSoC can be merged before I start my job in December, so I'm starting by 
 sending
 the simple parts of the overall patchset, when they are merged then I will 
 redo
 the later parts in several smaller and more manageable patchsets.

The patches look good, I just reviewed them again.  They cannot be applied
because you forgot --signoff.

Also, I think it would be more consistent if you added the following patches in
the same series:
- add blocking_fn (I sent it to qemu-devel some time ago),
- protect coroutine_fn and blocking_fn definition with #ifndef (to allow
  redefining them easily on the command line with extra cflags).

Could you resend the series with these suggestions?

Thanks,
-- 
Gabriel

Re: [Qemu-devel] [PATCH v3 0/2] Documentation for coroutine annotations

2013-10-27 Thread Charlie Shepherd 

On 27/10/2013 20:37, Gabriel Kerneis wrote:

On Sun, Oct 27, 2013 at 04:23:54PM +0100, Charlie Shepherd wrote:

These patches were the first two from my GSoC series and were reasonably
straight-forward and well accepted. Gabriel and I are hoping the patches from
GSoC can be merged before I start my job in December, so I'm starting by sending
the simple parts of the overall patchset, when they are merged then I will redo
the later parts in several smaller and more manageable patchsets.

The patches look good, I just reviewed them again.  They cannot be applied
because you forgot --signoff.

Also, I think it would be more consistent if you added the following patches in
the same series:
- add blocking_fn (I sent it to qemu-devel some time ago),
- protect coroutine_fn and blocking_fn definition with #ifndef (to allow
   redefining them easily on the command line with extra cflags).

Could you resend the series with these suggestions?


Good points, thanks, resent with your suggested changes.


Charlie

[Qemu-devel] [PATCH v4 1/4] Add an explanation of when a function should be marked coroutine_fn

2013-10-27 Thread Charlie Shepherd 
Coroutine functions that can yield directly or indirectly should be annotated
with a coroutine_fn annotation. Add an explanation to that effect in
include/block/coroutine.h.

Signed-off-by: Charlie Shepherd char...@ctshepherd.com
---
 include/block/coroutine.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/include/block/coroutine.h b/include/block/coroutine.h
index 4232569..e11a587 100644
--- a/include/block/coroutine.h
+++ b/include/block/coroutine.h
@@ -38,6 +38,14 @@
  * static checker support for catching such errors.  This annotation might make
  * it possible and in the meantime it serves as documentation.
  *
+ * A function must be marked with coroutine_fn if it can yield execution, 
either
+ * directly or indirectly.
+ *
+ * Some functions dynamically determine whether to yield or not based on
+ * whether they are executing in a coroutine context or not. These functions
+ * do not need to be annotated coroutine_fn. Note that this practice is
+ * deprecated and is being phased out, new code should not do this.
+ *
  * For example:
  *
  *   static void coroutine_fn foo(void) {
-- 
1.8.4.rc3

[Qemu-devel] [PATCH v4 2/4] Rename qemu_coroutine_self to qemu_coroutine_self_int and add an annotated wrapper

2013-10-27 Thread Charlie Shepherd 
While it only really makes sense to call qemu_coroutine_self() in a coroutine
context, some coroutine internals need to call it from functions not annotated
as coroutine_fn, so add an annotated wrapper and rename the implementation
versions to qemu_coroutine_self_int.

Signed-off-by: Charlie Shepherd char...@ctshepherd.com
---
 coroutine-gthread.c   |  2 +-
 coroutine-sigaltstack.c   |  2 +-
 coroutine-ucontext.c  |  2 +-
 coroutine-win32.c |  2 +-
 include/block/coroutine_int.h |  1 +
 qemu-coroutine.c  | 15 ++-
 6 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/coroutine-gthread.c b/coroutine-gthread.c
index d3e5b99..a913aeb 100644
--- a/coroutine-gthread.c
+++ b/coroutine-gthread.c
@@ -194,7 +194,7 @@ CoroutineAction qemu_coroutine_switch(Coroutine *from_,
 return from-action;
 }
 
-Coroutine *qemu_coroutine_self(void)
+Coroutine *qemu_coroutine_self_int(void)
 {
 CoroutineGThread *co = get_coroutine_key();
 if (!co) {
diff --git a/coroutine-sigaltstack.c b/coroutine-sigaltstack.c
index 3de0bb3..0556539 100644
--- a/coroutine-sigaltstack.c
+++ b/coroutine-sigaltstack.c
@@ -277,7 +277,7 @@ CoroutineAction qemu_coroutine_switch(Coroutine *from_, 
Coroutine *to_,
 return ret;
 }
 
-Coroutine *qemu_coroutine_self(void)
+Coroutine *qemu_coroutine_self_int(void)
 {
 CoroutineThreadState *s = coroutine_get_thread_state();
 
diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
index 4bf2cde..27d1b79 100644
--- a/coroutine-ucontext.c
+++ b/coroutine-ucontext.c
@@ -210,7 +210,7 @@ CoroutineAction qemu_coroutine_switch(Coroutine *from_, 
Coroutine *to_,
 return ret;
 }
 
-Coroutine *qemu_coroutine_self(void)
+Coroutine *qemu_coroutine_self_int(void)
 {
 CoroutineThreadState *s = coroutine_get_thread_state();
 
diff --git a/coroutine-win32.c b/coroutine-win32.c
index edc1f72..3f1f79b 100644
--- a/coroutine-win32.c
+++ b/coroutine-win32.c
@@ -77,7 +77,7 @@ void qemu_coroutine_delete(Coroutine *co_)
 g_free(co);
 }
 
-Coroutine *qemu_coroutine_self(void)
+Coroutine *qemu_coroutine_self_int(void)
 {
 if (!current) {
 current = leader.base;
diff --git a/include/block/coroutine_int.h b/include/block/coroutine_int.h
index f133d65..f6191ad 100644
--- a/include/block/coroutine_int.h
+++ b/include/block/coroutine_int.h
@@ -48,6 +48,7 @@ Coroutine *qemu_coroutine_new(void);
 void qemu_coroutine_delete(Coroutine *co);
 CoroutineAction qemu_coroutine_switch(Coroutine *from, Coroutine *to,
   CoroutineAction action);
+Coroutine *qemu_coroutine_self_int(void);
 void coroutine_fn qemu_co_queue_run_restart(Coroutine *co);
 
 #endif
diff --git a/qemu-coroutine.c b/qemu-coroutine.c
index 4708521..563e6ec 100644
--- a/qemu-coroutine.c
+++ b/qemu-coroutine.c
@@ -108,7 +108,7 @@ static void coroutine_swap(Coroutine *from, Coroutine *to)
 
 void qemu_coroutine_enter(Coroutine *co, void *opaque)
 {
-Coroutine *self = qemu_coroutine_self();
+Coroutine *self = qemu_coroutine_self_int();
 
 trace_qemu_coroutine_enter(self, co, opaque);
 
@@ -137,3 +137,16 @@ void coroutine_fn qemu_coroutine_yield(void)
 self-caller = NULL;
 coroutine_swap(self, to);
 }
+
+Coroutine *coroutine_fn qemu_coroutine_self(void)
+{
+/* Call the internal version of this function, which is
+ * non-coroutine_fn and can therefore be called from from
+ * non-coroutine contexts.  Internally we know it's always possible
+ * to pull a Coroutine* out of thin air (or thread-local storage).
+ * External callers shouldn't assume they can always get a
+ * Coroutine* since we may not be in coroutine context, hence the
+ * external version of this function.
+ */
+return qemu_coroutine_self_int();
+}
-- 
1.8.4.rc3

[Qemu-devel] [PATCH v4 3/4] Introduce blocking_fn annotation

2013-10-27 Thread Charlie Shepherd 
From: Gabriel Kerneis gabr...@kerneis.info

A blocking function is a function that must not be called in coroutine
context, for example because it might block for a long amount of time.
This annotation should be used to mark normal functions that have a
coroutine_fn counterpart, to make sure that the former is not used
instead of the later in coroutine context.

Signed-off-by: Gabriel Kerneis gabr...@kerneis.info
---
 include/block/coroutine.h | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/include/block/coroutine.h b/include/block/coroutine.h
index e11a587..02ce32d 100644
--- a/include/block/coroutine.h
+++ b/include/block/coroutine.h
@@ -54,6 +54,29 @@
  */
 #define coroutine_fn
 
+/**
+ * Mark a function that executes in blocking context
+ *
+ * Functions that execute in blocking context cannot be called directly from
+ * coroutine functions.  In the future it would be nice to enable compiler or
+ * static checker support for catching such errors.  This annotation might make
+ * it possible and in the meantime it serves as documentation.
+ *
+ * Annotating a function as blocking is stronger than having a mere
+ * (unannotated) normal function. It means that it might block the main
+ * loop for a significant amount of time, and therefore must not be
+ * called in coroutine context. In general, its hints that an
+ * alternative coroutine function performing the same taks is available
+ * for use in coroutine context.
+ *
+ * For example:
+ *
+ *   static void blocking_fn foo(void) {
+ *   
+ *   }
+ */
+#define blocking_fn
+
 typedef struct Coroutine Coroutine;
 
 /**
-- 
1.8.4.rc3

[Qemu-devel] [PATCH v4 0/4] Documentation for coroutine annotations

2013-10-27 Thread Charlie Shepherd 
These patches were the first two from my GSoC series and were reasonably
straight-forward and well accepted. Gabriel and I are hoping the patches from
GSoC can be merged before I start my job in December, so I'm starting by sending
the simple parts of the overall patchset, when they are merged then I will redo
the later parts in several smaller and more manageable patchsets.

---

Changes since v3:

- Added missing sign-off.

- Added patches by Gabriel Kerneis to add blocking_fn annotation and to protect
  coroutine_fn and blocking_fn with #ifndef.

Charlie Shepherd (2):
  Add an explanation of when a function should be marked coroutine_fn
  Rename qemu_coroutine_self to qemu_coroutine_self_int and add an
annotated wrapper

Gabriel Kerneis (2):
  Introduce blocking_fn annotation
  Protect coroutine_fn and blocking_fn with #ifndef

 coroutine-gthread.c   |  2 +-
 coroutine-sigaltstack.c   |  2 +-
 coroutine-ucontext.c  |  2 +-
 coroutine-win32.c |  2 +-
 include/block/coroutine.h | 35 +++
 include/block/coroutine_int.h |  1 +
 qemu-coroutine.c  | 15 ++-
 7 files changed, 54 insertions(+), 5 deletions(-)

-- 
1.8.4.rc3

[Qemu-devel] [PATCH v4 4/4] Protect coroutine_fn and blocking_fn with #ifndef

2013-10-27 Thread Charlie Shepherd 
From: Gabriel Kerneis gabr...@kerneis.info

This patch allows defining coroutine and blocking annotations with
./configure --extra-cflags instead of modifying coroutine.h.

Signed-off-by: Gabriel Kerneis gabr...@kerneis.info
---
 include/block/coroutine.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/block/coroutine.h b/include/block/coroutine.h
index 02ce32d..311ce2b 100644
--- a/include/block/coroutine.h
+++ b/include/block/coroutine.h
@@ -52,7 +52,9 @@
  *   
  *   }
  */
+#ifndef coroutine_fn
 #define coroutine_fn
+#endif
 
 /**
  * Mark a function that executes in blocking context
@@ -75,7 +77,9 @@
  *   
  *   }
  */
+#ifndef blocking_fn
 #define blocking_fn
+#endif
 
 typedef struct Coroutine Coroutine;
 
-- 
1.8.4.rc3

[Qemu-devel] [PATCH for 1.7] exec: fix breakpoint_invalidate when pc may not be translated

2013-10-27 Thread Max Filippov 
This fixes qemu abort with the following message:

include/qemu/int128.h:22: int128_get64: Assertion `!a.hi' failed.

which happens due to attempt to invalidate breakpoint by virtual address
for which get_phys_page_debug couldn't find mapping.

For more details see
http://lists.nongnu.org/archive/html/qemu-devel/2013-09/msg04582.html

Cc: qemu-sta...@nongnu.org
Signed-off-by: Max Filippov jcmvb...@gmail.com
---
 exec.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index 2e31ffc..9150430 100644
--- a/exec.c
+++ b/exec.c
@@ -409,8 +409,10 @@ static void breakpoint_invalidate(CPUState *cpu, 
target_ulong pc)
 #else
 static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
 {
-tb_invalidate_phys_addr(cpu_get_phys_page_debug(cpu, pc) |
-(pc  ~TARGET_PAGE_MASK));
+hwaddr phys = cpu_get_phys_page_debug(cpu, pc);
+if (phys != -1) {
+tb_invalidate_phys_addr(phys | (pc  ~TARGET_PAGE_MASK));
+}
 }
 #endif
 #endif /* TARGET_HAS_ICE */
-- 
1.8.1.4

[Qemu-devel] [PATCH 0/2] make slirp drivern by glib directly

2013-10-27 Thread Liu Ping Fan 
This series make slirp drivern directly by glib, so we can clean up
the hooks for slrip in mainloop and stub 


Liu Ping Fan (2):
  slirp: introduce gsource event abstraction
  slirp: make slirp event dispatch based on slirp instance

 main-loop.c   |   6 ---
 net/slirp.c   |   3 ++
 slirp/Makefile.objs   |   2 +-
 slirp/TFX7d70.tmp |   0
 slirp/libslirp.h  |   7 ++-
 slirp/slirp.c | 133 --
 slirp/slirp.h |   1 +
 slirp/slirp_gsource.c |  94 +++
 slirp/slirp_gsource.h |  37 ++
 slirp/socket.c|   1 -
 slirp/socket.h|   2 +-
 stubs/Makefile.objs   |   1 -
 stubs/slirp.c |  11 -
 13 files changed, 181 insertions(+), 117 deletions(-)
 create mode 100644 slirp/TFX7d70.tmp
 create mode 100644 slirp/slirp_gsource.c
 create mode 100644 slirp/slirp_gsource.h
 delete mode 100644 stubs/slirp.c

-- 
1.8.1.4

[Qemu-devel] [PATCH 1/2] slirp: introduce gsource event abstraction

2013-10-27 Thread Liu Ping Fan 
Introduce struct SlirpGSource. It will ease the usage of GSource
associated with a group of files, which are dynamically allocated
and release for slirp.

Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
---
 slirp/Makefile.objs   |  2 +-
 slirp/slirp_gsource.c | 94 +++
 slirp/slirp_gsource.h | 37 
 3 files changed, 132 insertions(+), 1 deletion(-)
 create mode 100644 slirp/slirp_gsource.c
 create mode 100644 slirp/slirp_gsource.h

diff --git a/slirp/Makefile.objs b/slirp/Makefile.objs
index 2daa9dc..ee39eed 100644
--- a/slirp/Makefile.objs
+++ b/slirp/Makefile.objs
@@ -1,3 +1,3 @@
 common-obj-y = cksum.o if.o ip_icmp.o ip_input.o ip_output.o dnssearch.o
-common-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o tcp_output.o
+common-obj-y += slirp.o slirp_gsource.o mbuf.o misc.o sbuf.o socket.o 
tcp_input.o tcp_output.o
 common-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o
diff --git a/slirp/slirp_gsource.c b/slirp/slirp_gsource.c
new file mode 100644
index 000..b502697
--- /dev/null
+++ b/slirp/slirp_gsource.c
@@ -0,0 +1,94 @@
+/*
+ *  Copyright (C) 2013 IBM
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; under version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see http://www.gnu.org/licenses/.
+ */
+
+#include slirp_gsource.h
+#include qemu/bitops.h
+
+GPollFD *slirp_gsource_add_pollfd(SlirpGSource *src, int fd)
+{
+GPollFD *retfd;
+
+retfd = g_slice_alloc(sizeof(GPollFD));
+retfd-events = 0;
+retfd-fd = fd;
+src-pollfds_list = g_list_append(src-pollfds_list, retfd);
+if (fd = 0) {
+g_source_add_poll(src-source, retfd);
+}
+
+return retfd;
+}
+
+void slirp_gsource_remove_pollfd(SlirpGSource *src, GPollFD *pollfd)
+{
+g_source_remove_poll(src-source, pollfd);
+src-pollfds_list = g_list_remove(src-pollfds_list, pollfd);
+g_slice_free(GPollFD, pollfd);
+}
+
+static gboolean slirp_gsource_check(GSource *src)
+{
+SlirpGSource *nsrc = (SlirpGSource *)src;
+GList *cur;
+GPollFD *gfd;
+
+cur = nsrc-pollfds_list;
+while (cur) {
+gfd = cur-data;
+if (gfd-fd = 0  (gfd-revents  gfd-events)) {
+return true;
+}
+cur = g_list_next(cur);
+}
+
+return false;
+}
+
+static gboolean slirp_gsource_dispatch(GSource *src, GSourceFunc cb,
+gpointer data)
+{
+gboolean ret = false;
+
+if (cb) {
+ret = cb(data);
+}
+return ret;
+}
+
+SlirpGSource *slirp_gsource_new(GPrepare prepare, GSourceFunc dispatch_cb,
+void *opaque)
+{
+SlirpGSource *src;
+GSourceFuncs *gfuncs = g_new0(GSourceFuncs, 1);
+gfuncs-prepare = prepare;
+gfuncs-check = slirp_gsource_check,
+gfuncs-dispatch = slirp_gsource_dispatch,
+
+src = (SlirpGSource *)g_source_new(gfuncs, sizeof(SlirpGSource));
+src-gfuncs = gfuncs;
+src-pollfds_list = NULL;
+src-opaque = opaque;
+g_source_set_callback(src-source, dispatch_cb, src, NULL);
+
+return src;
+}
+
+void slirp_gsource_release(SlirpGSource *src)
+{
+assert(!src-pollfds_list);
+g_free(src-gfuncs);
+g_source_destroy(src-source);
+}
diff --git a/slirp/slirp_gsource.h b/slirp/slirp_gsource.h
new file mode 100644
index 000..98a9e3a
--- /dev/null
+++ b/slirp/slirp_gsource.h
@@ -0,0 +1,37 @@
+/*
+ *  Copyright (C) 2013 IBM
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; under version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see http://www.gnu.org/licenses/.
+ */
+
+#ifndef SLIRP_GSOURCE_H
+#define SLIRP_GSOURCE_H
+#include qemu-common.h
+
+typedef gboolean (*GPrepare)(GSource *source, gint *timeout_);
+
+/* multi fd drive GSource*/
+typedef struct SlirpGSource {
+GSource source;
+/* a group of GPollFD which dynamically join or leave the GSource */
+GList *pollfds_list;
+GSourceFuncs *gfuncs;
+void *opaque;
+} SlirpGSource;
+
+SlirpGSource *slirp_gsource_new(GPrepare prepare, GSourceFunc dispatch_cb,
+void *opaque);
+void

[Qemu-devel] [PATCH 2/2] slirp: make slirp event dispatch based on slirp instance

2013-10-27 Thread Liu Ping Fan 
Each slirp instance has its own GFuncs, so we can driver slirp by glib main 
loop.

Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
---
For easing the review, This patch does not obey coding guide. Will fix
it later
---
 main-loop.c |   6 ---
 net/slirp.c |   3 ++
 slirp/TFX7d70.tmp   |   0
 slirp/libslirp.h|   7 ++-
 slirp/slirp.c   | 133 
 slirp/slirp.h   |   1 +
 slirp/socket.c  |   1 -
 slirp/socket.h  |   2 +-
 stubs/Makefile.objs |   1 -
 stubs/slirp.c   |  11 -
 10 files changed, 49 insertions(+), 116 deletions(-)
 create mode 100644 slirp/TFX7d70.tmp
 delete mode 100644 stubs/slirp.c

diff --git a/main-loop.c b/main-loop.c
index c3c9c28..374d26f 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -465,9 +465,6 @@ int main_loop_wait(int nonblocking)
 /* poll any events */
 g_array_set_size(gpollfds, 0); /* reset for new iteration */
 /* XXX: separate device handlers from system ones */
-#ifdef CONFIG_SLIRP
-slirp_pollfds_fill(gpollfds, timeout);
-#endif
 qemu_iohandler_fill(gpollfds);
 
 if (timeout == UINT32_MAX) {
@@ -482,9 +479,6 @@ int main_loop_wait(int nonblocking)
 
 ret = os_host_main_loop_wait(timeout_ns);
 qemu_iohandler_poll(gpollfds, ret);
-#ifdef CONFIG_SLIRP
-slirp_pollfds_poll(gpollfds, (ret  0));
-#endif
 
 qemu_clock_run_all_timers();
 
diff --git a/net/slirp.c b/net/slirp.c
index 124e953..2a21e16 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -35,6 +35,7 @@
 #include monitor/monitor.h
 #include qemu/sockets.h
 #include slirp/libslirp.h
+#include slirp/slirp_gsource.h
 #include sysemu/char.h
 
 static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
@@ -76,6 +77,7 @@ typedef struct SlirpState {
 #ifndef _WIN32
 char smb_dir[128];
 #endif
+SlirpGSource *slirp_src;
 } SlirpState;
 
 static struct slirp_config_str *slirp_configs;
@@ -244,6 +246,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
 
 s-slirp = slirp_init(restricted, net, mask, host, vhostname,
   tftp_export, bootfile, dhcp, dns, dnssearch, s);
+s-slirp_src = slirp_gsource_new(slirp_prepare, slirp_handler, s-slirp);
 QTAILQ_INSERT_TAIL(slirp_stacks, s, entry);
 
 for (config = slirp_configs; config; config = config-next) {
diff --git a/slirp/TFX7d70.tmp b/slirp/TFX7d70.tmp
new file mode 100644
index 000..e69de29
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 5bdcbd5..bb0c537 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -16,10 +16,6 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork,
   void *opaque);
 void slirp_cleanup(Slirp *slirp);
 
-void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
-
-void slirp_pollfds_poll(GArray *pollfds, int select_error);
-
 void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len);
 
 /* you must provide the following functions: */
@@ -39,5 +35,8 @@ void slirp_socket_recv(Slirp *slirp, struct in_addr 
guest_addr,
int guest_port, const uint8_t *buf, int size);
 size_t slirp_socket_can_recv(Slirp *slirp, struct in_addr guest_addr,
  int guest_port);
+gboolean slirp_prepare(GSource *source, gint *time);
+gboolean slirp_handler(gpointer data);
+
 
 #endif
diff --git a/slirp/slirp.c b/slirp/slirp.c
index bad8dad..6d57994 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -25,6 +25,7 @@
 #include qemu/timer.h
 #include sysemu/char.h
 #include slirp.h
+#include slirp_gsource.h
 #include hw/hw.h
 
 /* host loopback address */
@@ -262,46 +263,34 @@ void slirp_cleanup(Slirp *slirp)
 #define CONN_CANFSEND(so) (((so)-so_state  
(SS_FCANTSENDMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
 #define CONN_CANFRCV(so) (((so)-so_state  (SS_FCANTRCVMORE|SS_ISFCONNECTED)) 
== SS_ISFCONNECTED)
 
-static void slirp_update_timeout(uint32_t *timeout)
+static void slirp_update_timeout(Slirp *slirp, gint *timeout)
 {
-Slirp *slirp;
-uint32_t t;
+gint t = *timeout;
 
 if (*timeout = TIMEOUT_FAST) {
 return;
 }
 
-t = MIN(1000, *timeout);
-
-/* If we have tcp timeout with slirp, then we will fill @timeout with
- * more precise value.
- */
-QTAILQ_FOREACH(slirp, slirp_instances, entry) {
-if (slirp-time_fasttimo) {
-*timeout = TIMEOUT_FAST;
-return;
-}
-if (slirp-do_slowtimo) {
-t = MIN(TIMEOUT_SLOW, t);
-}
+if (slirp-time_fasttimo) {
+*timeout = TIMEOUT_FAST;
+return;
+}
+if (slirp-do_slowtimo) {
+t = MIN(TIMEOUT_SLOW, t);
 }
 *timeout = t;
 }
 
-void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout)
+gboolean slirp_prepare(GSource *source, gint *time)
 {
-Slirp *slirp;
+SlirpGSource *slirp_src = (SlirpGSource *)source;
+Slirp *slirp = slirp_src-opaque;
 struct socket *so, *so_next;
+u_int curtime;
 
-if

Re: [Qemu-devel] [PATCH_v2 0/9] target-openrisc: Corrections and speed improvements

2013-10-27 Thread Sebastian Macke 

On 25/10/2013 5:21 PM, Jia Liu wrote:

On Fri, Oct 25, 2013 at 7:23 AM, Sebastian Macke sebast...@macke.de wrote:

On 22/10/2013 8:47 PM, Jia Liu wrote:

Hi Sebastian,

On Tue, Oct 22, 2013 at 8:12 AM, Sebastian Macke sebast...@macke.de
wrote:

This series is the first part to make the OpenRISC target more
reliable and faster.
It corrects several severe problems which prevented the OpenRISC
emulation
for being useful in the past.

The patchset was tested with
- the tests/tcg/openrisc tests
- booting Linux 3.11
- run configure + make + gcc of a simple terminal graphic demo called
cmatrix
- run benchmark tool nbench in qemu-user mode and in the softmmu mode

The speed improvement is less than 10% because the overhead is still to
high
as the openrisc target does not support translation block chaining.
This will be included in one of the future patches.

Only the patch which removes the npc and ppc variables removes a little
feature
from the OpenRISC target but which does not break the specification and
will lead to
a significant speed improvement.

For v2 0/9 - 9/9
Acked-by: Jia Liu pro...@gmail.com

I'll add some comment into the code to explain why we separate flags from
sr
and send a pull request if nobody raise a rejection.


Ok great, the next bunch of patches is already in development.

Then, I'll make one pull request when you finish all you jobs,
please let me know when you finish your last work, is it OK?


Ok, do you want me to send then all patches including the old ones 
together in one patchset? At the moment this are 19 patches.
Keep in mind that the new patches will change much more. And maybe there 
will be discussions of some decisions I made.


But I promise also a speed increase of a factor of 7-10 :)





Sebastian Macke (9):
target-openrisc: Speed up move instruction
target-openrisc: Remove unnecessary code generated by jump
  instructions
target-openrisc: Remove executable flag for every page
target-openrisc: Correct wrong epcr register in interrupt handler
openrisc-timer: Reduce overhead, Separate clock update functions
target-openrisc: Correct memory bounds checking for the tlb buffers
target-openrisc: Separate branch flag from Supervision register
target-openrisc: Complete remove of npc and ppc variables
target-openrisc: Correct carry flag check of l.addc and l.addic test
  cases

   hw/openrisc/cputimer.c |  29 --
   target-openrisc/cpu.c  |   1 +
   target-openrisc/cpu.h  |  16 ++-
   target-openrisc/gdbstub.c  |  20 +---
   target-openrisc/interrupt.c|  27 ++---
   target-openrisc/interrupt_helper.c |   3 +-
   target-openrisc/machine.c  |   3 +-
   target-openrisc/mmu.c  |   4 +-
   target-openrisc/sys_helper.c   |  74 ++
   target-openrisc/translate.c| 201
-
   tests/tcg/openrisc/test_addc.c |   8 +-
   tests/tcg/openrisc/test_addic.c|  10 +-
   12 files changed, 175 insertions(+), 221 deletions(-)

--
1.8.4.1


Regards,
Jia

[Qemu-devel] About the migration_set_speed in the qemu monitor

2013-10-27 Thread Yaodong Yang 
Hi all,

When we migrate a vm from one host to another, we set the  migrate_set_speed 
200 inside the qemu monitor. What does the 200 means? Is it the maximum 
migration speed is 200MB/s or something else?

Thanks!