Re: [Qemu-devel] [PATCH V3 01/11] qemu-img: remove unused parameter in collect_image_info()

2013-01-15 Thread Wenchao Xia

于 2013-1-15 15:27, Wenchao Xia 写道:

于 2013-1-15 1:08, Luiz Capitulino 写道:

On Mon, 14 Jan 2013 15:09:37 +0800
Wenchao Xia  wrote:


   Parameter *fmt was not used, so remove it.

Reviewed-by: Eric Blake 
Signed-off-by: Wenchao Xia 
---
  qemu-img.c |5 ++---
  1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 85d3740..9dab48f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1186,8 +1186,7 @@ static void dump_json_image_info(ImageInfo *info)

  static void collect_image_info(BlockDriverState *bs,
 ImageInfo *info,
-   const char *filename,
-   const char *fmt)
+   const char *filename)


collect_image_info_list() doc reads:

  @fmt: topmost image format (may be NULL to autodetect)

However, right now only fmt=NULL is supported, as collect_image_info()
ignores fmt altogether.

So, if this patch is correct we better update the comment. Otherwise,
we should improve collect_image_info() to actually obey fmt != NULL.


   @fmt was ignored in the function and I can't see a reason to have
it while *bs contains the info, will change the comments.


  Hi, *fmt was used only in collect_image_info_list() when it tries to
open the image, and it is not useful any more in collect_image_info,
so nothing need change in comments.


  {
  uint64_t total_sectors;
  char backing_filename[1024];
@@ -1361,7 +1360,7 @@ static ImageInfoList
*collect_image_info_list(const char *filename,
  }

  info = g_new0(ImageInfo, 1);
-collect_image_info(bs, info, filename, fmt);
+collect_image_info(bs, info, filename);
  collect_snapshots(bs, info);

  elem = g_new0(ImageInfoList, 1);








--
Best Regards

Wenchao Xia




Re: [Qemu-devel] [PATCH 1/2] sheepdog: multiplex the rw FD to flush cache

2013-01-15 Thread MORITA Kazutaka
At Mon, 14 Jan 2013 14:01:02 +0800,
Liu Yuan wrote:
> 
> @@ -964,7 +971,10 @@ static int coroutine_fn 
> add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
>  
>  memset(&hdr, 0, sizeof(hdr));
>  
> -if (aiocb_type == AIOCB_READ_UDATA) {
> +if (aiocb_type == AIOCB_FLUSH_CACHE) {
> +wlen = 0;
> +hdr.opcode = SD_OP_FLUSH_VDI;
> +} else if (aiocb_type == AIOCB_READ_UDATA) {
>  wlen = 0;
>  hdr.opcode = SD_OP_READ_OBJ;
>  hdr.flags = flags;

I think it's better to use a switch statement here.

The other parts look good to me.

Thanks,

Kazutaka



Re: [Qemu-devel] [PATCH] qcow2: Fix segfault on zero-length write

2013-01-15 Thread Stefan Hajnoczi
On Mon, Jan 14, 2013 at 05:31:31PM +0100, Kevin Wolf wrote:
> One of the recent refactoring patches (commit f50f88b9) didn't take care
> to initialise l2meta properly, so with zero-length writes, which don't
> even enter the write loop, qemu just segfaulted.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/qcow2.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan



Re: [Qemu-devel] [Bug 1033727] Re: USB passthrough doesn't work anymore with qemu-kvm 1.1.1

2013-01-15 Thread Michael Tokarev

15.01.2013 00:02, Doug Goldstein wrote:

commit 72a04d0c178f01908d74539230d9de64ffc6da19
Author: Hans de Goede 
Date:   Wed Sep 12 15:08:40 2012 +0200

  uhci: Don't queue up packets after one with the SPD flag set



which I wanted to queue for 1.1-stable too, initially, but decided
it isn't really needed ;)



Can you go ahead and push this into the 1.1-stable repo then? I'd
rather see things that are accepted pushed into the various stable
repos than languish in someone's personal repo/list and then pushed
right before a new stable tarball is released.


  http://git.corpit.ru/?p=qemu.git;a=shortlog;h=refs/heads/stable-1.1-queue

I now updated the 1.1-queue tree, adding a few other things I picked
from various places, but it is still far from "complete' in any way.
(Note that I removed a few not-so-relevant changes from there, --
like "tools: initialize main loop before block layer" which isn't needed
since it fixes things broken after 1.2, so I had to rebase and force-push
it).  And I think we shuold release 1.1.3 finally.  But that's a different
topic.

Thanks,

/mjt







[Qemu-devel] [PATCH v2] sheepdog: multiplex the rw FD to flush cache

2013-01-15 Thread Liu Yuan
From: Liu Yuan 

This will reduce sockfds connected to the sheep server to one, which simply the
future hacks.

Cc: MORITA Kazutaka 
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Signed-off-by: Liu Yuan 
---
 v2: use switch case in add_aio_request

 block/sheepdog.c |   86 ++
 1 file changed, 41 insertions(+), 45 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 462c4b2..3caa872 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -266,6 +266,7 @@ typedef struct AIOReq {
 enum AIOCBState {
 AIOCB_WRITE_UDATA,
 AIOCB_READ_UDATA,
+AIOCB_FLUSH_CACHE,
 };
 
 struct SheepdogAIOCB {
@@ -299,7 +300,6 @@ typedef struct BDRVSheepdogState {
 char *addr;
 char *port;
 int fd;
-int flush_fd;
 
 CoMutex lock;
 Coroutine *co_send;
@@ -736,6 +736,13 @@ static void coroutine_fn aio_read_response(void *opaque)
 goto out;
 }
 break;
+case AIOCB_FLUSH_CACHE:
+if (rsp.result == SD_RES_INVALID_PARMS) {
+dprintf("disable cache since the server doesn't support it\n");
+s->cache_flags = SD_FLAG_CMD_DIRECT;
+rsp.result = SD_RES_SUCCESS;
+}
+break;
 }
 
 if (rsp.result != SD_RES_SUCCESS) {
@@ -964,18 +971,27 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState 
*s, AIOReq *aio_req,
 
 memset(&hdr, 0, sizeof(hdr));
 
-if (aiocb_type == AIOCB_READ_UDATA) {
+switch (aiocb_type) {
+case AIOCB_FLUSH_CACHE:
+wlen = 0;
+hdr.opcode = SD_OP_FLUSH_VDI;
+break;
+case AIOCB_READ_UDATA:
 wlen = 0;
 hdr.opcode = SD_OP_READ_OBJ;
 hdr.flags = flags;
-} else if (create) {
-wlen = datalen;
-hdr.opcode = SD_OP_CREATE_AND_WRITE_OBJ;
-hdr.flags = SD_FLAG_CMD_WRITE | flags;
-} else {
-wlen = datalen;
-hdr.opcode = SD_OP_WRITE_OBJ;
-hdr.flags = SD_FLAG_CMD_WRITE | flags;
+break;
+case AIOCB_WRITE_UDATA:
+if (create) {
+wlen = datalen;
+hdr.opcode = SD_OP_CREATE_AND_WRITE_OBJ;
+hdr.flags = SD_FLAG_CMD_WRITE | flags;
+} else {
+wlen = datalen;
+hdr.opcode = SD_OP_WRITE_OBJ;
+hdr.flags = SD_FLAG_CMD_WRITE | flags;
+}
+break;
 }
 
 if (s->cache_flags) {
@@ -1127,15 +1143,6 @@ static int sd_open(BlockDriverState *bs, const char 
*filename, int flags)
 s->cache_flags = SD_FLAG_CMD_DIRECT;
 }
 
-if (s->cache_flags == SD_FLAG_CMD_CACHE) {
-s->flush_fd = connect_to_sdog(s->addr, s->port);
-if (s->flush_fd < 0) {
-error_report("failed to connect");
-ret = s->flush_fd;
-goto out;
-}
-}
-
 if (snapid || tag[0] != '\0') {
 dprintf("%" PRIx32 " snapshot inode was open.\n", vid);
 s->is_snapshot = true;
@@ -1397,9 +1404,6 @@ static void sd_close(BlockDriverState *bs)
 
 qemu_aio_set_fd_handler(s->fd, NULL, NULL, NULL, NULL);
 closesocket(s->fd);
-if (s->cache_flags) {
-closesocket(s->flush_fd);
-}
 g_free(s->addr);
 }
 
@@ -1711,39 +1715,31 @@ static coroutine_fn int sd_co_readv(BlockDriverState 
*bs, int64_t sector_num,
 static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
 {
 BDRVSheepdogState *s = bs->opaque;
-SheepdogObjReq hdr = { 0 };
-SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr;
-SheepdogInode *inode = &s->inode;
+SheepdogAIOCB *acb;
+AIOReq *aio_req;
 int ret;
-unsigned int wlen = 0, rlen = 0;
 
 if (s->cache_flags != SD_FLAG_CMD_CACHE) {
 return 0;
 }
 
-hdr.opcode = SD_OP_FLUSH_VDI;
-hdr.oid = vid_to_vdi_oid(inode->vdi_id);
+acb = sd_aio_setup(bs, NULL, 0, 0, NULL, NULL);
+acb->aiocb_type = AIOCB_FLUSH_CACHE;
+acb->aio_done_func = sd_finish_aiocb;
 
-ret = do_req(s->flush_fd, (SheepdogReq *)&hdr, NULL, &wlen, &rlen);
-if (ret) {
-error_report("failed to send a request to the sheep");
+aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
+0, 0, 0, 0, 0);
+QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
+ret = add_aio_request(s, aio_req, NULL, 0, false, acb->aiocb_type);
+if (ret < 0) {
+error_report("add_aio_request is failed");
+free_aio_req(s, aio_req);
+qemu_aio_release(acb);
 return ret;
 }
 
-if (rsp->result == SD_RES_INVALID_PARMS) {
-dprintf("disable write cache since the server doesn't support it\n");
-
-s->cache_flags = SD_FLAG_CMD_DIRECT;
-closesocket(s->flush_fd);
-return 0;
-}
-
-if (rsp->result != SD_RES_SUCCESS) {
-error_report("%s", sd_strerror(rsp->result));
-return -EIO;
-}
-
-return 0;
+qemu_coroutine_yield();
+return acb->ret;
 }
 
 static int sd_snapshot_create(BlockDriverState *bs, QEMUSn

Re: [Qemu-devel] [PATCH] block: fix initialization in bdrv_io_limits_enable()

2013-01-15 Thread Stefan Hajnoczi
On Fri, Jan 11, 2013 at 01:29:55PM +0100, Peter Lieven wrote:
> bdrv_io_limits_enable() starts a new slice, but does not set io_base
> correctly for that slice.
> 
> Here is how io_base is used:
> 
>bytes_base  = bs->nr_bytes[is_write] - bs->io_base.bytes[is_write];
>bytes_res   = (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
> 
>if (bytes_base + bytes_res <= bytes_limit) {
>/* no wait */
>} else {
>/* operation needs to be throttled */
>}
> 
> As a result, any I/O operations that are triggered between now and
> bs->slice_end are incorrectly limited.  If 10 MB of data has been
> written since the VM was started, QEMU thinks that 10 MB of data has
> been written in this slice. This leads to a I/O lockup in the guest.
> 
> We fix this by delaying the start of a new slice to the next
> call of bdrv_exceed_io_limits().
> 
> Signed-off-by: Peter Lieven 
> ---
>  block.c |4 
>  1 file changed, 4 deletions(-)

I resolved a merge conflict due to whitespace issues with your patch.
Perhaps you copy-pasted from a terminal into an email client.  Please
try git-send-email(1) or double-check that spaces haven't changed before
sending.

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan



Re: [Qemu-devel] [PATCH v5 1/2] target-i386: define dr7 bit field

2013-01-15 Thread Andreas Färber
Am 15.01.2013 06:39, schrieb liguang:
> implictly use of dr7 bit field is a little hard
> to understand, so try to define them and use the
> defined name.
> 
> Signed-off-by: liguang 
> ---
>  target-i386/cpu.h |6 ++
>  target-i386/machine.c |5 +++--
>  target-i386/misc_helper.c |4 ++--
>  target-i386/seg_helper.c  |9 +
>  4 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 1283537..64fd7a5 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -231,6 +231,12 @@
>  #define DR7_TYPE_SHIFT  16
>  #define DR7_LEN_SHIFT   18
>  #define DR7_FIXED_1 0x0400
> +#define DR7_LOCAL_BP_MASK0x55
> +#define DR7_MAX_BP   4
> +#define DR7_TYPE_BP_INST 0x0
> +#define DR7_TYPE_DATA_WR 0x1
> +#define DR7_TYPE_IO_RW   0x2
> +#define DR7_TYPE_DATA_RW 0x3
>  
>  #define PG_PRESENT_BIT   0
>  #define PG_RW_BIT1
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index 8354572..8df6a6b 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -265,10 +265,11 @@ static int cpu_post_load(void *opaque, int version_id)
>  
>  cpu_breakpoint_remove_all(env, BP_CPU);
>  cpu_watchpoint_remove_all(env, BP_CPU);
> -for (i = 0; i < 4; i++)
> +for (i = 0; i < DR7_MAX_BP; i++) {
>  hw_breakpoint_insert(env, i);
> -
> +}
>  tlb_flush(env, 1);
> +
>  return 0;
>  }
>  
> diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
> index db3126b..9b0f7b3 100644
> --- a/target-i386/misc_helper.c
> +++ b/target-i386/misc_helper.c
> @@ -197,11 +197,11 @@ void helper_movl_drN_T0(CPUX86State *env, int reg, 
> target_ulong t0)
>  env->dr[reg] = t0;
>  hw_breakpoint_insert(env, reg);
>  } else if (reg == 7) {
> -for (i = 0; i < 4; i++) {
> +for (i = 0; i < DR7_MAX_BP; i++) {
>  hw_breakpoint_remove(env, i);
>  }
>  env->dr[7] = t0;
> -for (i = 0; i < 4; i++) {
> +for (i = 0; i < DR7_MAX_BP; i++) {
>  hw_breakpoint_insert(env, i);
>  }
>  } else {
> diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
> index c2a99ee..3247dee 100644
> --- a/target-i386/seg_helper.c
> +++ b/target-i386/seg_helper.c
> @@ -465,13 +465,14 @@ static void switch_tss(CPUX86State *env, int 
> tss_selector,
>  
>  #ifndef CONFIG_USER_ONLY
>  /* reset local breakpoints */
> -if (env->dr[7] & 0x55) {
> -for (i = 0; i < 4; i++) {
> -if (hw_breakpoint_enabled(env->dr[7], i) == 0x1) {
> +if (env->dr[7] & DR7_LOCAL_BP_MASK) {
> +for (i = 0; i < DR7_MAX_BP; i++) {
> +if (hw_local_breakpoint_enabled(env->dr[7], i) &&
> +!hw_global_breakpoint_enabled(env->dr[7], i)) {

Does not build.

>  hw_breakpoint_remove(env, i);
>  }
>  }
> -env->dr[7] &= ~0x55;
> +env->dr[7] &= ~DR7_LOCAL_BP_MASK;
>  }
>  #endif
>  }
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [PATCH v3] sheepdog: multiplex the rw FD to flush cache

2013-01-15 Thread Liu Yuan
From: Liu Yuan 

This will reduce sockfds connected to the sheep server to one, which simply the
future hacks.

Cc: MORITA Kazutaka 
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Signed-off-by: Liu Yuan 
---
 v3: fix gcc warning in add_aio_request()

 block/sheepdog.c |   82 --
 1 file changed, 37 insertions(+), 45 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 462c4b2..1cbfe53 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -266,6 +266,7 @@ typedef struct AIOReq {
 enum AIOCBState {
 AIOCB_WRITE_UDATA,
 AIOCB_READ_UDATA,
+AIOCB_FLUSH_CACHE,
 };
 
 struct SheepdogAIOCB {
@@ -299,7 +300,6 @@ typedef struct BDRVSheepdogState {
 char *addr;
 char *port;
 int fd;
-int flush_fd;
 
 CoMutex lock;
 Coroutine *co_send;
@@ -736,6 +736,13 @@ static void coroutine_fn aio_read_response(void *opaque)
 goto out;
 }
 break;
+case AIOCB_FLUSH_CACHE:
+if (rsp.result == SD_RES_INVALID_PARMS) {
+dprintf("disable cache since the server doesn't support it\n");
+s->cache_flags = SD_FLAG_CMD_DIRECT;
+rsp.result = SD_RES_SUCCESS;
+}
+break;
 }
 
 if (rsp.result != SD_RES_SUCCESS) {
@@ -950,7 +957,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState 
*s, AIOReq *aio_req,
 {
 int nr_copies = s->inode.nr_copies;
 SheepdogObjReq hdr;
-unsigned int wlen;
+unsigned int wlen = 0;
 int ret;
 uint64_t oid = aio_req->oid;
 unsigned int datalen = aio_req->data_len;
@@ -964,18 +971,23 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState 
*s, AIOReq *aio_req,
 
 memset(&hdr, 0, sizeof(hdr));
 
-if (aiocb_type == AIOCB_READ_UDATA) {
-wlen = 0;
+switch (aiocb_type) {
+case AIOCB_FLUSH_CACHE:
+hdr.opcode = SD_OP_FLUSH_VDI;
+break;
+case AIOCB_READ_UDATA:
 hdr.opcode = SD_OP_READ_OBJ;
 hdr.flags = flags;
-} else if (create) {
-wlen = datalen;
-hdr.opcode = SD_OP_CREATE_AND_WRITE_OBJ;
-hdr.flags = SD_FLAG_CMD_WRITE | flags;
-} else {
+break;
+case AIOCB_WRITE_UDATA:
+   if (create) {
+hdr.opcode = SD_OP_CREATE_AND_WRITE_OBJ;
+} else {
+hdr.opcode = SD_OP_WRITE_OBJ;
+}
 wlen = datalen;
-hdr.opcode = SD_OP_WRITE_OBJ;
 hdr.flags = SD_FLAG_CMD_WRITE | flags;
+break;
 }
 
 if (s->cache_flags) {
@@ -1127,15 +1139,6 @@ static int sd_open(BlockDriverState *bs, const char 
*filename, int flags)
 s->cache_flags = SD_FLAG_CMD_DIRECT;
 }
 
-if (s->cache_flags == SD_FLAG_CMD_CACHE) {
-s->flush_fd = connect_to_sdog(s->addr, s->port);
-if (s->flush_fd < 0) {
-error_report("failed to connect");
-ret = s->flush_fd;
-goto out;
-}
-}
-
 if (snapid || tag[0] != '\0') {
 dprintf("%" PRIx32 " snapshot inode was open.\n", vid);
 s->is_snapshot = true;
@@ -1397,9 +1400,6 @@ static void sd_close(BlockDriverState *bs)
 
 qemu_aio_set_fd_handler(s->fd, NULL, NULL, NULL, NULL);
 closesocket(s->fd);
-if (s->cache_flags) {
-closesocket(s->flush_fd);
-}
 g_free(s->addr);
 }
 
@@ -1711,39 +1711,31 @@ static coroutine_fn int sd_co_readv(BlockDriverState 
*bs, int64_t sector_num,
 static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
 {
 BDRVSheepdogState *s = bs->opaque;
-SheepdogObjReq hdr = { 0 };
-SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr;
-SheepdogInode *inode = &s->inode;
+SheepdogAIOCB *acb;
+AIOReq *aio_req;
 int ret;
-unsigned int wlen = 0, rlen = 0;
 
 if (s->cache_flags != SD_FLAG_CMD_CACHE) {
 return 0;
 }
 
-hdr.opcode = SD_OP_FLUSH_VDI;
-hdr.oid = vid_to_vdi_oid(inode->vdi_id);
+acb = sd_aio_setup(bs, NULL, 0, 0, NULL, NULL);
+acb->aiocb_type = AIOCB_FLUSH_CACHE;
+acb->aio_done_func = sd_finish_aiocb;
 
-ret = do_req(s->flush_fd, (SheepdogReq *)&hdr, NULL, &wlen, &rlen);
-if (ret) {
-error_report("failed to send a request to the sheep");
+aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
+0, 0, 0, 0, 0);
+QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
+ret = add_aio_request(s, aio_req, NULL, 0, false, acb->aiocb_type);
+if (ret < 0) {
+error_report("add_aio_request is failed");
+free_aio_req(s, aio_req);
+qemu_aio_release(acb);
 return ret;
 }
 
-if (rsp->result == SD_RES_INVALID_PARMS) {
-dprintf("disable write cache since the server doesn't support it\n");
-
-s->cache_flags = SD_FLAG_CMD_DIRECT;
-closesocket(s->flush_fd);
-return 0;
-}
-
-if (rsp->result != SD_RES_SUCCESS) {
-error_report("%s", sd_strerror(rsp->result));
-return -EIO;
-  

[Qemu-devel] [PATCH qom-cpu v6 0/4] target-i386: Clean up DR7 breakpoint handling

2013-01-15 Thread Andreas Färber
Hello Guang,

Are you okay with this version?

Regards,
Andreas

v5 -> v6:
* Fix bisectability by deferring use of hw_{local,global}_breakpoint_enabled().
* Reword the commit messages for clarity.
* Squash more constant usage into first patch.
* Reorder DATA_RW and DATA_WR to match original switch cases.
* Untangle introduction of breakpoint helper functions from goto/if 
refactorings.
* Make hw_breakpoint_enabled() return bool.
* Keep IO_RW in place for patch readability.
* Keep reg variable name for patch readability.
* Move {bp,wp}_match inside loop to clarify scope and to avoid double false 
init.
* Make check_hw_breakpoints() return bool, change force_dr6_update arg to bool.

changes v4->v5:

- fix some not well formated changes.
- split functional and non-functional changes for cherry-picking
suggested by Andreas Färber 

changes v3->v4:

- fix wrong logic of hw_{global,local}_breakpoint_enabled usage
suggested by Peter Maydell 

changes v2->v3:

- split hw_breakpoint_enabled into hw_{global,local}_breakpoint_enabled

changes v1->v2:

- add _TYPE_ to the name of dr7 bit field
- fix some coding styles
suggested by Peter Maydell 

Cc: liguang 
Cc: Eduardo Habkost 
Cc: Igor Mammedov 
Cc: Jan Kiszka 
Cc: Peter Maydell 

liguang (4):
  target-i386: Define DR7 bit field constants
  target-i386: Introduce hw_{local,global}_breakpoint_enabled()
  target-i386: Avoid goto in hw_breakpoint_insert()
  target-i386: Use switch in check_hw_breakpoints()

 target-i386/cpu.h |   23 ++--
 target-i386/helper.c  |   87 +
 target-i386/machine.c |5 +--
 target-i386/misc_helper.c |6 ++--
 target-i386/seg_helper.c  |9 ++---
 5 Dateien geändert, 88 Zeilen hinzugefügt(+), 42 Zeilen entfernt(-)

-- 
1.7.10.4




[Qemu-devel] [PATCH qom-cpu v6 3/4] target-i386: Avoid goto in hw_breakpoint_insert()

2013-01-15 Thread Andreas Färber
From: liguang 

  "Go To Statement Considered Harmful" -- E. Dijkstra

To avoid an unnecessary goto within the switch statement, move
watchpoint insertion out of the switch statement. Improves readability.

While at it, fix Coding Style issues (missing braces, indentation).

Signed-off-by: liguang 
Signed-off-by: Andreas Färber 
---
 target-i386/helper.c |   16 ++--
 1 Datei geändert, 10 Zeilen hinzugefügt(+), 6 Zeilen entfernt(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index ebdd6a5..a10b562 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -966,7 +966,7 @@ hwaddr cpu_get_phys_page_debug(CPUX86State *env, 
target_ulong addr)
 
 void hw_breakpoint_insert(CPUX86State *env, int index)
 {
-int type, err = 0;
+int type = 0, err = 0;
 
 switch (hw_breakpoint_type(env->dr[7], index)) {
 case DR7_TYPE_BP_INST:
@@ -977,20 +977,24 @@ void hw_breakpoint_insert(CPUX86State *env, int index)
 break;
 case DR7_TYPE_DATA_WR:
 type = BP_CPU | BP_MEM_WRITE;
-goto insert_wp;
+break;
 case DR7_TYPE_IO_RW:
- /* No support for I/O watchpoints yet */
+/* No support for I/O watchpoints yet */
 break;
 case DR7_TYPE_DATA_RW:
 type = BP_CPU | BP_MEM_ACCESS;
-insert_wp:
+break;
+}
+
+if (type != 0) {
 err = cpu_watchpoint_insert(env, env->dr[index],
 hw_breakpoint_len(env->dr[7], index),
 type, &env->cpu_watchpoint[index]);
-break;
 }
-if (err)
+
+if (err) {
 env->cpu_breakpoint[index] = NULL;
+}
 }
 
 void hw_breakpoint_remove(CPUX86State *env, int index)
-- 
1.7.10.4




[Qemu-devel] [PATCH qom-cpu v6 4/4] target-i386: Use switch in check_hw_breakpoints()

2013-01-15 Thread Andreas Färber
From: liguang 

Replace an if statement using magic numbers for breakpoint type with a
more explicit switch statement. This is to aid readability.

Change the return type and force_dr6_update argument type to bool.

While at it, fix Coding Style issues (missing braces).

Signed-off-by: liguang 
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.h |2 +-
 target-i386/helper.c  |   44 
 target-i386/misc_helper.c |2 +-
 3 Dateien geändert, 34 Zeilen hinzugefügt(+), 14 Zeilen entfernt(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 1e850a7..4e091cd 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1043,7 +1043,7 @@ static inline int hw_breakpoint_len(unsigned long dr7, 
int index)
 
 void hw_breakpoint_insert(CPUX86State *env, int index);
 void hw_breakpoint_remove(CPUX86State *env, int index);
-int check_hw_breakpoints(CPUX86State *env, int force_dr6_update);
+bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update);
 void breakpoint_handler(CPUX86State *env);
 
 /* will be suppressed */
diff --git a/target-i386/helper.c b/target-i386/helper.c
index a10b562..547c25e 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1017,26 +1017,45 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
 }
 }
 
-int check_hw_breakpoints(CPUX86State *env, int force_dr6_update)
+bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
 {
 target_ulong dr6;
-int reg, type;
-int hit_enabled = 0;
+int reg;
+bool hit_enabled = false;
 
 dr6 = env->dr[6] & ~0xf;
 for (reg = 0; reg < DR7_MAX_BP; reg++) {
-type = hw_breakpoint_type(env->dr[7], reg);
-if ((type == 0 && env->dr[reg] == env->eip) ||
-((type & 1) && env->cpu_watchpoint[reg] &&
- (env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT))) {
+bool bp_match = false;
+bool wp_match = false;
+
+switch (hw_breakpoint_type(env->dr[7], reg)) {
+case DR7_TYPE_BP_INST:
+if (env->dr[reg] == env->eip) {
+bp_match = true;
+}
+break;
+case DR7_TYPE_DATA_WR:
+case DR7_TYPE_DATA_RW:
+if (env->cpu_watchpoint[reg] &&
+env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT) {
+wp_match = true;
+}
+break;
+case DR7_TYPE_IO_RW:
+break;
+}
+if (bp_match || wp_match) {
 dr6 |= 1 << reg;
 if (hw_breakpoint_enabled(env->dr[7], reg)) {
-hit_enabled = 1;
+hit_enabled = true;
 }
 }
 }
-if (hit_enabled || force_dr6_update)
+
+if (hit_enabled || force_dr6_update) {
 env->dr[6] = dr6;
+}
+
 return hit_enabled;
 }
 
@@ -1047,16 +1066,17 @@ void breakpoint_handler(CPUX86State *env)
 if (env->watchpoint_hit) {
 if (env->watchpoint_hit->flags & BP_CPU) {
 env->watchpoint_hit = NULL;
-if (check_hw_breakpoints(env, 0))
+if (check_hw_breakpoints(env, false)) {
 raise_exception(env, EXCP01_DB);
-else
+} else {
 cpu_resume_from_signal(env, NULL);
+}
 }
 } else {
 QTAILQ_FOREACH(bp, &env->breakpoints, entry)
 if (bp->pc == env->eip) {
 if (bp->flags & BP_CPU) {
-check_hw_breakpoints(env, 1);
+check_hw_breakpoints(env, true);
 raise_exception(env, EXCP01_DB);
 }
 break;
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index b3f4e4f..b6d5740 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c
@@ -110,7 +110,7 @@ void helper_into(CPUX86State *env, int next_eip_addend)
 void helper_single_step(CPUX86State *env)
 {
 #ifndef CONFIG_USER_ONLY
-check_hw_breakpoints(env, 1);
+check_hw_breakpoints(env, true);
 env->dr[6] |= DR6_BS;
 #endif
 raise_exception(env, EXCP01_DB);
-- 
1.7.10.4




[Qemu-devel] IS_USER(s) in target-arm

2013-01-15 Thread Shijesta Victor
I am trying to use QEMU to print the virtual and physical addresses of all 
memory accesses for target-arm. I can see the addresses being computed, but 
they pass IS_USER(s) as an index to routines like gen_ld8s etc, Can you please 
explain what this does?
Any help is appreciated; thanks in advance!

regards,

shijesta

--- On Tue, 15/1/13, Wenchao Xia  wrote:

From: Wenchao Xia 
Subject: Re: [Qemu-devel] [PATCH V3 02/11] block: add bdrv_get_filename() 
function
To: "Luiz Capitulino" 
Cc: aligu...@us.ibm.com, phrd...@redhat.com, stefa...@gmail.com, 
qemu-devel@nongnu.org, arm...@redhat.com, pbonz...@redhat.com
Date: Tuesday, 15 January, 2013, 13:00

于 2013-1-15 1:08, Luiz Capitulino 写道:
> On Mon, 14 Jan 2013 15:09:38 +0800
> Wenchao Xia  wrote:
>
>>    This function will simply return the uri or filename used
>> to open the image.
>>
>> Reviewed-by: Eric Blake 
>> Signed-off-by: Wenchao Xia 
>> ---
>>   block.c               |    5 +
>>   include/block/block.h |    1 +
>>   2 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 4e28c55..5f95da5 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -2967,6 +2967,11 @@ BlockStatsList *qmp_query_blockstats(Error **errp)
>>       return head;
>>   }
>>
>> +const char *bdrv_get_filename(BlockDriverState *bs)
>> +{
>
> bs can be const.
>
OK.


-- 
Best Regards

Wenchao Xia




[Qemu-devel] [PATCH qom-cpu v6 1/4] target-i386: Define DR7 bit field constants

2013-01-15 Thread Andreas Färber
From: liguang 

Implicit use of dr7 bit field is a little hard to understand,
so define constants for them and use them consistently.

Signed-off-by: liguang 
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.h |6 ++
 target-i386/helper.c  |   18 +-
 target-i386/machine.c |5 +++--
 target-i386/misc_helper.c |4 ++--
 target-i386/seg_helper.c  |6 +++---
 5 Dateien geändert, 23 Zeilen hinzugefügt(+), 16 Zeilen entfernt(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index e4a7c50..6682022 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -231,6 +231,12 @@
 #define DR7_TYPE_SHIFT  16
 #define DR7_LEN_SHIFT   18
 #define DR7_FIXED_1 0x0400
+#define DR7_LOCAL_BP_MASK0x55
+#define DR7_MAX_BP   4
+#define DR7_TYPE_BP_INST 0x0
+#define DR7_TYPE_DATA_WR 0x1
+#define DR7_TYPE_IO_RW   0x2
+#define DR7_TYPE_DATA_RW 0x3
 
 #define PG_PRESENT_BIT 0
 #define PG_RW_BIT  1
diff --git a/target-i386/helper.c b/target-i386/helper.c
index fa622e1..1fceb91 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -969,18 +969,18 @@ void hw_breakpoint_insert(CPUX86State *env, int index)
 int type, err = 0;
 
 switch (hw_breakpoint_type(env->dr[7], index)) {
-case 0:
+case DR7_TYPE_BP_INST:
 if (hw_breakpoint_enabled(env->dr[7], index))
 err = cpu_breakpoint_insert(env, env->dr[index], BP_CPU,
 &env->cpu_breakpoint[index]);
 break;
-case 1:
+case DR7_TYPE_DATA_WR:
 type = BP_CPU | BP_MEM_WRITE;
 goto insert_wp;
-case 2:
+case DR7_TYPE_IO_RW:
  /* No support for I/O watchpoints yet */
 break;
-case 3:
+case DR7_TYPE_DATA_RW:
 type = BP_CPU | BP_MEM_ACCESS;
 insert_wp:
 err = cpu_watchpoint_insert(env, env->dr[index],
@@ -997,15 +997,15 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
 if (!env->cpu_breakpoint[index])
 return;
 switch (hw_breakpoint_type(env->dr[7], index)) {
-case 0:
+case DR7_TYPE_BP_INST:
 if (hw_breakpoint_enabled(env->dr[7], index))
 cpu_breakpoint_remove_by_ref(env, env->cpu_breakpoint[index]);
 break;
-case 1:
-case 3:
+case DR7_TYPE_DATA_WR:
+case DR7_TYPE_DATA_RW:
 cpu_watchpoint_remove_by_ref(env, env->cpu_watchpoint[index]);
 break;
-case 2:
+case DR7_TYPE_IO_RW:
 /* No support for I/O watchpoints yet */
 break;
 }
@@ -1018,7 +1018,7 @@ int check_hw_breakpoints(CPUX86State *env, int 
force_dr6_update)
 int hit_enabled = 0;
 
 dr6 = env->dr[6] & ~0xf;
-for (reg = 0; reg < 4; reg++) {
+for (reg = 0; reg < DR7_MAX_BP; reg++) {
 type = hw_breakpoint_type(env->dr[7], reg);
 if ((type == 0 && env->dr[reg] == env->eip) ||
 ((type & 1) && env->cpu_watchpoint[reg] &&
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 8354572..8df6a6b 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -265,10 +265,11 @@ static int cpu_post_load(void *opaque, int version_id)
 
 cpu_breakpoint_remove_all(env, BP_CPU);
 cpu_watchpoint_remove_all(env, BP_CPU);
-for (i = 0; i < 4; i++)
+for (i = 0; i < DR7_MAX_BP; i++) {
 hw_breakpoint_insert(env, i);
-
+}
 tlb_flush(env, 1);
+
 return 0;
 }
 
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index 719cacd..b3f4e4f 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c
@@ -197,11 +197,11 @@ void helper_movl_drN_T0(CPUX86State *env, int reg, 
target_ulong t0)
 env->dr[reg] = t0;
 hw_breakpoint_insert(env, reg);
 } else if (reg == 7) {
-for (i = 0; i < 4; i++) {
+for (i = 0; i < DR7_MAX_BP; i++) {
 hw_breakpoint_remove(env, i);
 }
 env->dr[7] = t0;
-for (i = 0; i < 4; i++) {
+for (i = 0; i < DR7_MAX_BP; i++) {
 hw_breakpoint_insert(env, i);
 }
 } else {
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index c2a99ee..c40bd96 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -465,13 +465,13 @@ static void switch_tss(CPUX86State *env, int tss_selector,
 
 #ifndef CONFIG_USER_ONLY
 /* reset local breakpoints */
-if (env->dr[7] & 0x55) {
-for (i = 0; i < 4; i++) {
+if (env->dr[7] & DR7_LOCAL_BP_MASK) {
+for (i = 0; i < DR7_MAX_BP; i++) {
 if (hw_breakpoint_enabled(env->dr[7], i) == 0x1) {
 hw_breakpoint_remove(env, i);
 }
 }
-env->dr[7] &= ~0x55;
+env->dr[7] &= ~DR7_LOCAL_BP_MASK;
 }
 #endif
 }
-- 
1.7.10.4




[Qemu-devel] [PATCH qom-cpu v6 2/4] target-i386: Introduce hw_{local, global}_breakpoint_enabled()

2013-01-15 Thread Andreas Färber
From: liguang 

hw_breakpoint_enabled() returned a bit field indicating whether a local
breakpoint and/or global breakpoint was enabled. Avoid this number magic
by using explicit boolean helper functions hw_local_breakpoint_enabled()
and hw_global_breakpoint_enabled(), to aid readability.

Reuse them for the hw_breakpoint_enabled() implementation and change
its return type to bool.

While at it, fix Coding Style issues (missing braces).

Signed-off-by: liguang 
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.h|   15 +--
 target-i386/helper.c |9 ++---
 target-i386/seg_helper.c |3 ++-
 3 Dateien geändert, 21 Zeilen hinzugefügt(+), 6 Zeilen entfernt(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 6682022..1e850a7 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1014,9 +1014,20 @@ int cpu_x86_handle_mmu_fault(CPUX86State *env, 
target_ulong addr,
 #define cpu_handle_mmu_fault cpu_x86_handle_mmu_fault
 void cpu_x86_set_a20(CPUX86State *env, int a20_state);
 
-static inline int hw_breakpoint_enabled(unsigned long dr7, int index)
+static inline bool hw_local_breakpoint_enabled(unsigned long dr7, int index)
 {
-return (dr7 >> (index * 2)) & 3;
+return (dr7 >> (index * 2)) & 1;
+}
+
+static inline bool hw_global_breakpoint_enabled(unsigned long dr7, int index)
+{
+return (dr7 >> (index * 2)) & 2;
+
+}
+static inline bool hw_breakpoint_enabled(unsigned long dr7, int index)
+{
+return hw_global_breakpoint_enabled(dr7, index) ||
+   hw_local_breakpoint_enabled(dr7, index);
 }
 
 static inline int hw_breakpoint_type(unsigned long dr7, int index)
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 1fceb91..ebdd6a5 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -970,9 +970,10 @@ void hw_breakpoint_insert(CPUX86State *env, int index)
 
 switch (hw_breakpoint_type(env->dr[7], index)) {
 case DR7_TYPE_BP_INST:
-if (hw_breakpoint_enabled(env->dr[7], index))
+if (hw_breakpoint_enabled(env->dr[7], index)) {
 err = cpu_breakpoint_insert(env, env->dr[index], BP_CPU,
 &env->cpu_breakpoint[index]);
+}
 break;
 case DR7_TYPE_DATA_WR:
 type = BP_CPU | BP_MEM_WRITE;
@@ -998,8 +999,9 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
 return;
 switch (hw_breakpoint_type(env->dr[7], index)) {
 case DR7_TYPE_BP_INST:
-if (hw_breakpoint_enabled(env->dr[7], index))
+if (hw_breakpoint_enabled(env->dr[7], index)) {
 cpu_breakpoint_remove_by_ref(env, env->cpu_breakpoint[index]);
+}
 break;
 case DR7_TYPE_DATA_WR:
 case DR7_TYPE_DATA_RW:
@@ -1024,8 +1026,9 @@ int check_hw_breakpoints(CPUX86State *env, int 
force_dr6_update)
 ((type & 1) && env->cpu_watchpoint[reg] &&
  (env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT))) {
 dr6 |= 1 << reg;
-if (hw_breakpoint_enabled(env->dr[7], reg))
+if (hw_breakpoint_enabled(env->dr[7], reg)) {
 hit_enabled = 1;
+}
 }
 }
 if (hit_enabled || force_dr6_update)
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index c40bd96..3247dee 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -467,7 +467,8 @@ static void switch_tss(CPUX86State *env, int tss_selector,
 /* reset local breakpoints */
 if (env->dr[7] & DR7_LOCAL_BP_MASK) {
 for (i = 0; i < DR7_MAX_BP; i++) {
-if (hw_breakpoint_enabled(env->dr[7], i) == 0x1) {
+if (hw_local_breakpoint_enabled(env->dr[7], i) &&
+!hw_global_breakpoint_enabled(env->dr[7], i)) {
 hw_breakpoint_remove(env, i);
 }
 }
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH 4/7] block: make discard asynchronous

2013-01-15 Thread Stefan Hajnoczi
On Mon, Jan 14, 2013 at 04:26:55PM +0100, Paolo Bonzini wrote:
> +static ssize_t handle_aiocb_discard(RawPosixAIOData *aiocb)
> +{
> +int ret = -EOPNOTSUPP;
> +BDRVRawState *s = aiocb->bs->opaque;
> +
> +if (s->has_discard == 0) {
> +return 0;
> +}
> +
> +if (aiocb->aio_type & QEMU_AIO_BLKDEV) {
> +#ifdef BLKDISCARD
> +do {
> +uint64_t range[2] = { aiocb->aio_offset, aiocb->aio_nbytes };
> +if (ioctl(aiocb->aio_fildes, BLKDISCARD, range) == 0) {
> +return 0;
> +}
> +} while (errno == EINTR);
> +
> +ret = -errno;
> +#endif
> +} else {
> +#ifdef CONFIG_XFS
> +if (s->is_xfs) {
> +return xfs_discard(s, aiocb->aio_offset, aiocb->aio_nbytes);
> +}
> +#endif
> +
> +#ifdef CONFIG_FALLOCATE_PUNCH_HOLE
> +do {
> +if (fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
> +  aiocb->aio_offset, aiocb->aio_nbytes) == 0) {

It is cleaner to use either aiocb->aio_fildes or s->fd consistently in
this function.  Minor issue, not worth respinning for.

Stefan



[Qemu-devel] [PATCH 2/2] pc87312: Avoid define conflict on mingw32

2013-01-15 Thread Andreas Färber
From: Blue Swirl 

Mingw32 headers define FAR, causing this warning:
/src/qemu/hw/pc87312.c:38:0: warning: "FAR" redefined [enabled by default]
In file included from 
/usr/local/lib/gcc/i686-mingw32msvc/4.7.0/../../../../i686-mingw32msvc/include/windows.h:48:0,
 from /src/qemu/include/sysemu/os-win32.h:29,
 from /src/qemu/include/qemu-common.h:46,
 from /src/qemu/include/exec/ioport.h:27,
 from /src/qemu/hw/isa.h:6,
 from /src/qemu/hw/pc87312.h:28,
 from /src/qemu/hw/pc87312.c:26:
/usr/local/lib/gcc/i686-mingw32msvc/4.7.0/../../../../i686-mingw32msvc/include/windef.h:34:0:
 note: this is the location of the previous definition

Avoid the warning by expanding the macros.

Signed-off-by: Blue Swirl 
Acked-by: Hervé Poussineau 
Signed-off-by: Andreas Färber 
---
 hw/pc87312.c |   38 +-
 1 Datei geändert, 17 Zeilen hinzugefügt(+), 21 Zeilen entfernt(-)

diff --git a/hw/pc87312.c b/hw/pc87312.c
index 97fabfa..38af4c1 100644
--- a/hw/pc87312.c
+++ b/hw/pc87312.c
@@ -34,10 +34,6 @@
 #define REG_FAR 1
 #define REG_PTR 2
 
-#define FER regs[REG_FER]
-#define FAR regs[REG_FAR]
-#define PTR regs[REG_PTR]
-
 #define FER_PARALLEL_EN   0x01
 #define FER_UART1_EN  0x02
 #define FER_UART2_EN  0x04
@@ -66,14 +62,14 @@
 
 static inline bool is_parallel_enabled(PC87312State *s)
 {
-return s->FER & FER_PARALLEL_EN;
+return s->regs[REG_FER] & FER_PARALLEL_EN;
 }
 
 static const uint32_t parallel_base[] = { 0x378, 0x3bc, 0x278, 0x00 };
 
 static inline uint32_t get_parallel_iobase(PC87312State *s)
 {
-return parallel_base[s->FAR & FAR_PARALLEL_ADDR];
+return parallel_base[s->regs[REG_FAR] & FAR_PARALLEL_ADDR];
 }
 
 static const uint32_t parallel_irq[] = { 5, 7, 5, 0 };
@@ -81,9 +77,9 @@ static const uint32_t parallel_irq[] = { 5, 7, 5, 0 };
 static inline uint32_t get_parallel_irq(PC87312State *s)
 {
 int idx;
-idx = (s->FAR & FAR_PARALLEL_ADDR);
+idx = (s->regs[REG_FAR] & FAR_PARALLEL_ADDR);
 if (idx == 0) {
-return (s->PTR & PTR_IRQ_5_7) ? 7 : 5;
+return (s->regs[REG_PTR] & PTR_IRQ_5_7) ? 7 : 5;
 } else {
 return parallel_irq[idx];
 }
@@ -91,7 +87,7 @@ static inline uint32_t get_parallel_irq(PC87312State *s)
 
 static inline bool is_parallel_epp(PC87312State *s)
 {
-return s->PTR & PTR_EPP_MODE;
+return s->regs[REG_PTR] & PTR_EPP_MODE;
 }
 
 
@@ -105,26 +101,26 @@ static const uint32_t uart_base[2][4] = {
 static inline uint32_t get_uart_iobase(PC87312State *s, int i)
 {
 int idx;
-idx = (s->FAR >> (2 * i + 2)) & 0x3;
+idx = (s->regs[REG_FAR] >> (2 * i + 2)) & 0x3;
 if (idx == 0) {
 return 0x3f8;
 } else if (idx == 1) {
 return 0x2f8;
 } else {
-return uart_base[idx & 1][(s->FAR & FAR_UART_3_4) >> 6];
+return uart_base[idx & 1][(s->regs[REG_FAR] & FAR_UART_3_4) >> 6];
 }
 }
 
 static inline uint32_t get_uart_irq(PC87312State *s, int i)
 {
 int idx;
-idx = (s->FAR >> (2 * i + 2)) & 0x3;
+idx = (s->regs[REG_FAR] >> (2 * i + 2)) & 0x3;
 return (idx & 1) ? 3 : 4;
 }
 
 static inline bool is_uart_enabled(PC87312State *s, int i)
 {
-return s->FER & (FER_UART1_EN << i);
+return s->regs[REG_FER] & (FER_UART1_EN << i);
 }
 
 
@@ -132,12 +128,12 @@ static inline bool is_uart_enabled(PC87312State *s, int i)
 
 static inline bool is_fdc_enabled(PC87312State *s)
 {
-return s->FER & FER_FDC_EN;
+return s->regs[REG_FER] & FER_FDC_EN;
 }
 
 static inline uint32_t get_fdc_iobase(PC87312State *s)
 {
-return (s->FER & FER_FDC_ADDR) ? 0x370 : 0x3f0;
+return (s->regs[REG_FER] & FER_FDC_ADDR) ? 0x370 : 0x3f0;
 }
 
 
@@ -145,19 +141,19 @@ static inline uint32_t get_fdc_iobase(PC87312State *s)
 
 static inline bool is_ide_enabled(PC87312State *s)
 {
-return s->FER & FER_IDE_EN;
+return s->regs[REG_FER] & FER_IDE_EN;
 }
 
 static inline uint32_t get_ide_iobase(PC87312State *s)
 {
-return (s->FER & FER_IDE_ADDR) ? 0x170 : 0x1f0;
+return (s->regs[REG_FER] & FER_IDE_ADDR) ? 0x170 : 0x1f0;
 }
 
 
 static void reconfigure_devices(PC87312State *s)
 {
 error_report("pc87312: unsupported device reconfiguration (%02x %02x 
%02x)",
- s->FER, s->FAR, s->PTR);
+ s->regs[REG_FER], s->regs[REG_FAR], s->regs[REG_PTR]);
 }
 
 static void pc87312_soft_reset(PC87312State *s)
@@ -184,9 +180,9 @@ static void pc87312_soft_reset(PC87312State *s)
 s->read_id_step = 0;
 s->selected_index = REG_FER;
 
-s->FER = fer_init[s->config & 0x1f];
-s->FAR = far_init[s->config & 0x1f];
-s->PTR = ptr_init[s->config & 0x1f];
+s->regs[REG_FER] = fer_init[s->config & 0x1f];
+s->regs[REG_FAR] = far_init[s->config & 0x1f];
+s->regs[REG_PTR] = ptr_init[s->config & 0x1f];
 }
 
 static void pc87312_hard_reset(PC87312State *s)
-- 
1.7.10.4




[Qemu-devel] [PATCH 1/2] pc87312: Replace register_ioport_*() with MemoryRegion

2013-01-15 Thread Andreas Färber
Prepare an instance_init function for the MemoryRegion init.

Signed-off-by: Andreas Färber 
Tested-by: Hervé Poussineau 
---
 hw/pc87312.c |   26 ++
 hw/pc87312.h |2 ++
 2 Dateien geändert, 24 Zeilen hinzugefügt(+), 4 Zeilen entfernt(-)

diff --git a/hw/pc87312.c b/hw/pc87312.c
index 6a17afd..97fabfa 100644
--- a/hw/pc87312.c
+++ b/hw/pc87312.c
@@ -194,7 +194,8 @@ static void pc87312_hard_reset(PC87312State *s)
 pc87312_soft_reset(s);
 }
 
-static void pc87312_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+static void pc87312_io_write(void *opaque, hwaddr addr, uint64_t val,
+ unsigned int size)
 {
 PC87312State *s = opaque;
 
@@ -213,7 +214,7 @@ static void pc87312_ioport_write(void *opaque, uint32_t 
addr, uint32_t val)
 }
 }
 
-static uint32_t pc87312_ioport_read(void *opaque, uint32_t addr)
+static uint64_t pc87312_io_read(void *opaque, hwaddr addr, unsigned int size)
 {
 PC87312State *s = opaque;
 uint32_t val;
@@ -241,6 +242,16 @@ static uint32_t pc87312_ioport_read(void *opaque, uint32_t 
addr)
 return val;
 }
 
+static const MemoryRegionOps pc87312_io_ops = {
+.read  = pc87312_io_read,
+.write = pc87312_io_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+.valid = {
+.min_access_size = 1,
+.max_access_size = 1,
+},
+};
+
 static int pc87312_post_load(void *opaque, int version_id)
 {
 PC87312State *s = opaque;
@@ -270,6 +281,7 @@ static int pc87312_init(ISADevice *dev)
 s = PC87312(dev);
 bus = isa_bus_from_device(dev);
 pc87312_hard_reset(s);
+isa_register_ioport(dev, &s->io, s->iobase);
 
 if (is_parallel_enabled(s)) {
 chr = parallel_hds[0];
@@ -337,11 +349,16 @@ static int pc87312_init(ISADevice *dev)
 trace_pc87312_info_ide(get_ide_iobase(s));
 }
 
-register_ioport_write(s->iobase, 2, 1, pc87312_ioport_write, s);
-register_ioport_read(s->iobase, 2, 1, pc87312_ioport_read, s);
 return 0;
 }
 
+static void pc87312_initfn(Object *obj)
+{
+PC87312State *s = PC87312(obj);
+
+memory_region_init_io(&s->io, &pc87312_io_ops, s, "pc87312", 2);
+}
+
 static const VMStateDescription vmstate_pc87312 = {
 .name = "pc87312",
 .version_id = 1,
@@ -376,6 +393,7 @@ static const TypeInfo pc87312_type_info = {
 .name  = TYPE_PC87312,
 .parent= TYPE_ISA_DEVICE,
 .instance_size = sizeof(PC87312State),
+.instance_init = pc87312_initfn,
 .class_init= pc87312_class_init,
 };
 
diff --git a/hw/pc87312.h b/hw/pc87312.h
index 7ca7912..7b9e6f6 100644
--- a/hw/pc87312.h
+++ b/hw/pc87312.h
@@ -56,6 +56,8 @@ typedef struct PC87312State {
 uint32_t base;
 } ide;
 
+MemoryRegion io;
+
 uint8_t read_id_step;
 uint8_t selected_index;
 
-- 
1.7.10.4




[Qemu-devel] [PULL] PReP patch queue 2013-01-15

2013-01-15 Thread Andreas Färber
Hello,

Please pull the PowerPC Reference Platform (PReP) queue into qemu.git master.

This contains a warning fix for mingw32 plus a minor I/O port cleanup.
(My pending prep_pci patch needs to be reposted first and misses the pull.)

The following changes since commit cf7c3f0cb5a7129f57fa9e69d410d6a05031988c:

  virtio-9p: fix compilation error. (2013-01-14 18:52:39 -0600)

are available in the git repository at:

  git://repo.or.cz/qemu/afaerber.git prep-up

for you to fetch changes up to 08bb4a7c9bb9c12746bce9b3a1f031dd4192afc1:

  pc87312: Avoid define conflict on mingw32 (2013-01-15 03:32:37 +0100)


Andreas Färber (1):
  pc87312: Replace register_ioport_*() with MemoryRegion

Blue Swirl (1):
  pc87312: Avoid define conflict on mingw32

 hw/pc87312.c |   64 +++---
 hw/pc87312.h |2 ++
 2 Dateien geändert, 41 Zeilen hinzugefügt(+), 25 Zeilen entfernt(-)



Re: [Qemu-devel] [PATCH qom-cpu 00/17] x86 CPU cleanup, part 3

2013-01-15 Thread Igor Mammedov
On Tue, 15 Jan 2013 05:16:52 +0100
Andreas Färber  wrote:

> Am 11.01.2013 03:10, schrieb Igor Mammedov:
> > Igor Mammedov (17):
> >   target-i386: move setting defaults out of cpu_x86_parse_featurestr()
> >   target-i386: cpu_x86_register() consolidate freeing resources
> >   target-i386: move kvm_check_features_against_host() check to realize
> > time
> 
> Thanks, I've applied 1-3 to qom-cpu:
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu
> 
> I really like patch 3, which moves more logic into our realizefn! When
> the QOM realize series gets applied, I have a queue prepared already to
> change today's signatures and to hook up to the infrastructure and to
> make this uniform across targets.
> 
> >   target-i386: add x86_cpu_vendor_words2str()
> 
> Patch 4 v3 looks okay too, but it mentions "next patch" and doesn't
> provide any value on its own:
> 
> >   target-i386: replace uint32_t vendor fields by vendor string in
> > x86_def_t
> 
> I raised some (non-)issues on patch 5 that I would like to give others a
> chance to review rather than taking the lonely decision of putting this
> in a PULL tonight; a new idea there would be a union { uint32_t
> words[3]; char str[12]; } to keep both options open while avoiding the
> string -> 3x uint32_t -> string mess that patch 5 rightfully attempts to
> clean up. Once consensus is reached and it is still needed, I would
> prefer to squash patch 4 as justification, being a rather trivial code
> movement.
I'll try it.

> 
> >   target-i386: remove vendor_override field from CPUX86State
> >   target-i386: prepare cpu_x86_parse_featurestr() to return a set of
> > key,value property pairs
> 
> Didn't get around to fully reviewing the remainder of the series yet.
Remainder of series is gradual conversion of currently existing properties
to foo,val pairs that set later by x86_cpu_set_props(). And following
static properties series will convert non property-fied features into
properties following the same pattern.

> 
> However I am still unclear and skeptic towards this patch using a QDict
> to set properties on the CPU. I was told this was for -feat and +feat
> backwards compatibility but the properties it is being used for are not
> features but regular properties that I would like to always set to get
> errors from each property rather than papering over, e.g.,
> "...,tsc_freq=invalid,tsc_freq=1G,..." (IIUC).
-feat and +feat is the only reason[1] to use dictionary as temporary storage
foo=val values, It could be hidden inside of cpu_x86_parse_featurestr()
and QList could be returned instead of it. That would achieve processing
of tsc_freq=invalid,tsc_freq=1G in sequence when setting properties and
failing on the first invalid tsc_freq or another similar case.

> By operating on the
> X86CPU, the intermediate x86_def_t can be spared too, as shown in my
> subclasses patch. Maybe I'm overlooking something, needs calm review
> from my side and some trial-and-error.

But main point of patch 7 is to separate parsing logic of command line
features (some of them legacy naming) from setting properties on actual
CPU instance.
Later when sub-classes are implemented and all features are converted into
static properties, x86_cpu_set_props() would be merged into
cpu_x86_parse_featurestr() and set global properties instead of operating
on CPU instance directly. After that cpu_x86_parse_featurestr() could be
called only once to set global properties for a given CPU type instead of
parsing the same featurestr for every CPU.

I could merge x86_cpu_set_props() into cpu_x86_parse_featurestr() and pass
cpu instance inside it temporally but I'd insist on first normalizing
featurestr into list of property pairs and then applying them to CPU.
It makes conversion to global properties trivial.

> 
> As a reminder, I have been pushing for converting to subclasses early
> because that is a prerequisite for doing some CPUClass-level changes
> across targets to clean up the way CPUs get created wrt cpu_init() and
> cpu_copy(). Feature properties are, as far as I have seen and heard, an
> x86-only project that could thus well be done on top of that common
> infrastructure IMO.
properties is x86-only project for now, but they make conversion to
sub-classes simpler and actually make x86 sub-classes meaningful.

We could push hard for properties and sub-classes to make in 1.4.


> 
> Regards,
> Andreas
> 
> >   target-i386: set custom 'vendor' without intermediate x86_def_t
> >   target-i386: print deprecated warning if xlevel < 0x8000
> >   target-i386: set custom 'xlevel' without intermediate x86_def_t
> >   target-i386: set custom 'level' without intermediate x86_def_t
> >   target-i386: set custom 'model-id' without intermediate x86_def_t
> >   target-i386: set custom 'stepping' without intermediate x86_def_t
> >   target-i386: set custom 'model' without intermediate x86_def_t
> >   target-i386: set custom 'family' without intermediate x86_def_t
> >   target-i386: set custom

Re: [Qemu-devel] [PATCH resend 0/7] Discard improvements

2013-01-15 Thread Stefan Hajnoczi
On Mon, Jan 14, 2013 at 04:26:51PM +0100, Paolo Bonzini wrote:
> This series builds on the patch from Kusanagi Kouichi, and also adds
> discard support for non-passthrough block devices (BLKDISCARD), and
> asynchronous discard support.
> 
> SCSI already calls bdrv_aio_discard, so it is not affected by these
> patches.
> 
> Kusanagi Kouichi (1):
>   raw-posix: support discard on more filesystems
> 
> Paolo Bonzini (6):
>   raw-posix: remember whether discard failed
>   raw: support discard on block devices
>   block: make discard asynchronous
>   ide: fix TRIM with empty range entry
>   ide: issue discard asynchronously but serialize the pieces
>   block: clear dirty bitmap when discarding
> 
>  block.c   |   8 +++-
>  block/raw-aio.h   |   5 ++-
>  block/raw-posix.c | 125 
> +-
>  configure |  19 +
>  hw/ide/core.c |  79 +++---
>  5 files changed, 179 insertions(+), 57 deletions(-)
> 
> -- 
> 1.8.1
> 
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan



Re: [Qemu-devel] [PATCH qom-cpu v6 0/4] target-i386: Clean up DR7 breakpoint handling

2013-01-15 Thread li guang
Thanks for your smooth work!

fine for me.


在 2013-01-15二的 09:29 +0100,Andreas Färber写道:
> Hello Guang,
> 
> Are you okay with this version?
> 
> Regards,
> Andreas
> 
> v5 -> v6:
> * Fix bisectability by deferring use of 
> hw_{local,global}_breakpoint_enabled().
> * Reword the commit messages for clarity.
> * Squash more constant usage into first patch.
> * Reorder DATA_RW and DATA_WR to match original switch cases.
> * Untangle introduction of breakpoint helper functions from goto/if 
> refactorings.
> * Make hw_breakpoint_enabled() return bool.
> * Keep IO_RW in place for patch readability.
> * Keep reg variable name for patch readability.
> * Move {bp,wp}_match inside loop to clarify scope and to avoid double false 
> init.
> * Make check_hw_breakpoints() return bool, change force_dr6_update arg to 
> bool.
> 
> changes v4->v5:
> 
> - fix some not well formated changes.
> - split functional and non-functional changes for cherry-picking
> suggested by Andreas Färber 
> 
> changes v3->v4:
> 
> - fix wrong logic of hw_{global,local}_breakpoint_enabled usage
> suggested by Peter Maydell 
> 
> changes v2->v3:
> 
> - split hw_breakpoint_enabled into hw_{global,local}_breakpoint_enabled
> 
> changes v1->v2:
> 
> - add _TYPE_ to the name of dr7 bit field
> - fix some coding styles
> suggested by Peter Maydell 
> 
> Cc: liguang 
> Cc: Eduardo Habkost 
> Cc: Igor Mammedov 
> Cc: Jan Kiszka 
> Cc: Peter Maydell 
> 
> liguang (4):
>   target-i386: Define DR7 bit field constants
>   target-i386: Introduce hw_{local,global}_breakpoint_enabled()
>   target-i386: Avoid goto in hw_breakpoint_insert()
>   target-i386: Use switch in check_hw_breakpoints()
> 
>  target-i386/cpu.h |   23 ++--
>  target-i386/helper.c  |   87 
> +
>  target-i386/machine.c |5 +--
>  target-i386/misc_helper.c |6 ++--
>  target-i386/seg_helper.c  |9 ++---
>  5 Dateien geändert, 88 Zeilen hinzugefügt(+), 42 Zeilen entfernt(-)
> 

-- 
regards!
li guang




[Qemu-devel] [PATCH] build: remove *.lo, *.a, *.la files from all subdirectories on make clean

2013-01-15 Thread Paolo Bonzini
.lo files in stubs/, util/ and libcacard/ were not cleaned.
Fix this.

Cc: Blue Swirl 
Reported-by: Stefan Hajnoczi 
Signed-off-by: Paolo Bonzini 
---
 Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index cfa2fa6..6cee692 100644
--- a/Makefile
+++ b/Makefile
@@ -214,9 +214,9 @@ clean:
 # avoid old build problems by removing potentially incorrect old files
rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h 
gen-op-arm.h
rm -f qemu-options.def
-   find . -name '*.[od]' -type f -exec rm -f {} +
-   rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~
-   rm -f *.la
+   find . -name '*.[oda]' -type f -exec rm -f {} +
+   find . -name '*.l[oa]' -type f -exec rm -f {} +
+   rm -f $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~
rm -Rf .libs
rm -f qemu-img-cmds.h
@# May not be present in GENERATED_HEADERS
-- 
1.8.1




[Qemu-devel] [PATCH] tests: adjust gcov variables for directory movement

2013-01-15 Thread Paolo Bonzini
I had missed the introduction of the gcov-files-* variables.

Cc: Blue Swirl 
Signed-off-by: Paolo Bonzini 
---
Blue, can you look at introducing a common variable for the coroutine
backend?  Like

coroutine-backend-y = gthread
coroutine-backend-$(CONFIG_SIGALTSTACK_COROUTINE) = sigaltstack
coroutine-backend-$(CONFIG_UCONTEXT_COROUTINE) = ucontext
coroutine-backend-$(CONFIG_WIN32) = win32

and using it in both Makefile.objs and tests/Makefile.
Another alternative is to use $(filter) to pick the one file that
is actually part of $(block-obj-y).  Thanks!

 tests/Makefile | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index d97a571..90ad126 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -1,17 +1,17 @@
 export SRC_PATH
 
 check-unit-y = tests/check-qdict$(EXESUF)
-gcov-files-check-qdict-y = qdict.c
+gcov-files-check-qdict-y = qobject/qdict.c
 check-unit-y += tests/check-qfloat$(EXESUF)
-gcov-files-check-qfloat-y = qfloat.c
+gcov-files-check-qfloat-y = qobject/qfloat.c
 check-unit-y += tests/check-qint$(EXESUF)
-gcov-files-check-qint-y = qint.c
+gcov-files-check-qint-y = qobject/qint.c
 check-unit-y += tests/check-qstring$(EXESUF)
-gcov-files-check-qstring-y = qstring.c
+gcov-files-check-qstring-y = qobject/qstring.c
 check-unit-y += tests/check-qlist$(EXESUF)
-gcov-files-check-qlist-y = qlist.c
+gcov-files-check-qlist-y = qobject/qlist.c
 check-unit-y += tests/check-qjson$(EXESUF)
-gcov-files-check-qjson-y = qjson.c
+gcov-files-check-qjson-y = qobject/qjson.c
 check-unit-y += tests/test-qmp-output-visitor$(EXESUF)
 gcov-files-test-qmp-output-visitor-y = qapi/qmp-output-visitor.c
 check-unit-y += tests/test-qmp-input-visitor$(EXESUF)
@@ -39,7 +39,7 @@ endif
 endif
 check-unit-y += tests/test-visitor-serialization$(EXESUF)
 check-unit-y += tests/test-iov$(EXESUF)
-gcov-files-test-iov-y = iov.c
+gcov-files-test-iov-y = util/iov.c
 check-unit-y += tests/test-aio$(EXESUF)
 gcov-files-test-aio-$(CONFIG_WIN32) = aio-win32.c
 gcov-files-test-aio-$(CONFIG_POSIX) = aio-posix.c
-- 
1.8.1





Re: [Qemu-devel] [PATCH qom-cpu v6 0/4] target-i386: Clean up DR7 breakpoint handling

2013-01-15 Thread Andreas Färber
Am 15.01.2013 09:49, schrieb li guang:
> Thanks for your smooth work!
> 
> fine for me.

Thanks, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

> 在 2013-01-15二的 09:29 +0100,Andreas Färber写道:
>> Hello Guang,
>>
>> Are you okay with this version?
>>
>> Regards,
>> Andreas
>>
>> v5 -> v6:
>> * Fix bisectability by deferring use of 
>> hw_{local,global}_breakpoint_enabled().
>> * Reword the commit messages for clarity.
>> * Squash more constant usage into first patch.
>> * Reorder DATA_RW and DATA_WR to match original switch cases.
>> * Untangle introduction of breakpoint helper functions from goto/if 
>> refactorings.
>> * Make hw_breakpoint_enabled() return bool.
>> * Keep IO_RW in place for patch readability.
>> * Keep reg variable name for patch readability.
>> * Move {bp,wp}_match inside loop to clarify scope and to avoid double false 
>> init.
>> * Make check_hw_breakpoints() return bool, change force_dr6_update arg to 
>> bool.
>>
>> changes v4->v5:
>>
>> - fix some not well formated changes.
>> - split functional and non-functional changes for cherry-picking
>> suggested by Andreas Färber 
>>
>> changes v3->v4:
>>
>> - fix wrong logic of hw_{global,local}_breakpoint_enabled usage
>> suggested by Peter Maydell 
>>
>> changes v2->v3:
>>
>> - split hw_breakpoint_enabled into hw_{global,local}_breakpoint_enabled
>>
>> changes v1->v2:
>>
>> - add _TYPE_ to the name of dr7 bit field
>> - fix some coding styles
>> suggested by Peter Maydell 
>>
>> Cc: liguang 
>> Cc: Eduardo Habkost 
>> Cc: Igor Mammedov 
>> Cc: Jan Kiszka 
>> Cc: Peter Maydell 
>>
>> liguang (4):
>>   target-i386: Define DR7 bit field constants
>>   target-i386: Introduce hw_{local,global}_breakpoint_enabled()
>>   target-i386: Avoid goto in hw_breakpoint_insert()
>>   target-i386: Use switch in check_hw_breakpoints()
>>
>>  target-i386/cpu.h |   23 ++--
>>  target-i386/helper.c  |   87 
>> +
>>  target-i386/machine.c |5 +--
>>  target-i386/misc_helper.c |6 ++--
>>  target-i386/seg_helper.c  |9 ++---
>>  5 Dateien geändert, 88 Zeilen hinzugefügt(+), 42 Zeilen entfernt(-)
>>
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [PATCH] build: fix Win32 clean build

2013-01-15 Thread Paolo Bonzini
The version.o file did not appear explicitly as a dependency, and
this caused clean builds to fail.  Force its build by making the
Makefile depend on version.o.

(We cannot add it to libqemuutil.a, because it doesn't export any
symbol and thus would not be pulled by the linker).

Cc: Blue Swirl 
Cc: Stefan Weil 
Signed-off-by: Paolo Bonzini 
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 7622a4c..cfa2fa6 100644
--- a/Makefile
+++ b/Makefile
@@ -153,6 +153,7 @@ version.o: $(SRC_PATH)/version.rc config-host.h
$(call quiet-command,$(WINDRES) -I. -o $@ $<,"  RC$(TARGET_DIR)$@")
 
 version-obj-$(CONFIG_WIN32) += version.o
+Makefile: $(version-obj-y)
 
 ##
 # Build libraries
-- 
1.8.1




[Qemu-devel] [PULL] QOM CPUState patch queue 2013-01-15

2013-01-15 Thread Andreas Färber
Hello,

This is my current QOM CPU patch queue. Please pull.

It includes:
* topology-related field movements to CPUState,
* better support for checking and enforcing x86 CPUID features,
* more cleanups and preparations for x86 realizefn and subclasses,
* x86 DR7 breakpoint cleanups.

Regards,
Andreas

Cc: Anthony Liguori 
Cc: Eduardo Habkost 
Cc: Igor Mammedov 
Cc: Li Guang 


The following changes since commit cf7c3f0cb5a7129f57fa9e69d410d6a05031988c:

  virtio-9p: fix compilation error. (2013-01-14 18:52:39 -0600)

are available in the git repository at:

  git://github.com/afaerber/qemu-cpu.git qom-cpu

for you to fetch changes up to e175bce587936bf479889881488821ea8d61c89c:

  target-i386: Use switch in check_hw_breakpoints() (2013-01-15 09:23:50 +0100)


Andreas Färber (7):
  cpu: Move nr_{cores,threads} fields to CPUState
  target-mips: Clean up mips_cpu_map_tc() documentation
  cpu: Move numa_node field to CPUState
  cpu: Move cpu_index field to CPUState
  kvm: Pass CPUState to kvm_init_vcpu()
  xen: Simplify halting of first CPU
  exec: Return CPUState from qemu_get_cpu()

Eduardo Habkost (6):
  kvm: Add fake KVM constants to avoid #ifdefs on KVM-specific code
  target-i386: Disable kvm_mmu by default
  target-i386/cpu: Introduce FeatureWord typedefs
  target-i386: kvm_check_features_against_host(): Use feature_word_info
  target-i386/cpu.c: Add feature name array for ext4_features
  target-i386: check/enforce: Check all feature words

Igor Mammedov (3):
  target-i386: Move setting defaults out of cpu_x86_parse_featurestr()
  target-i386: cpu_x86_register() consolidate freeing resources
  target-i386: Move kvm_check_features_against_host() check to realize time

liguang (4):
  target-i386: Define DR7 bit field constants
  target-i386: Introduce hw_{local,global}_breakpoint_enabled()
  target-i386: Avoid goto in hw_breakpoint_insert()
  target-i386: Use switch in check_hw_breakpoints()

 cpus.c  |   24 +++--
 exec.c  |   19 ++--
 gdbstub.c   |3 +-
 hw/alpha_typhoon.c  |4 +-
 hw/arm_gic.c|3 +-
 hw/arm_mptimer.c|8 +-
 hw/mips_malta.c |9 +-
 hw/openpic.c|5 +-
 hw/ppc/e500.c   |   17 +--
 hw/ppce500_spin.c   |8 +-
 hw/pxa.h|2 +-
 hw/pxa2xx.c |4 +-
 hw/pxa2xx_gpio.c|7 +-
 hw/spapr.c  |   13 ++-
 hw/spapr_hcall.c|4 +-
 hw/spapr_rtas.c |8 +-
 hw/xics.c   |   22 ++--
 include/exec/cpu-all.h  |1 -
 include/exec/cpu-defs.h |4 -
 include/exec/gdbstub.h  |3 +-
 include/qom/cpu.h   |   19 
 include/sysemu/kvm.h|   19 +++-
 kvm-all.c   |5 +-
 kvm-stub.c  |2 +-
 monitor.c   |   19 ++--
 target-alpha/translate.c|2 +-
 target-arm/cpu.c|2 +-
 target-arm/helper.c |3 +-
 target-cris/cpu.c   |2 +-
 target-i386/cpu.c   |  252 ++-
 target-i386/cpu.h   |   38 ++-
 target-i386/helper.c|  102 +++---
 target-i386/machine.c   |5 +-
 target-i386/misc_helper.c   |   11 +-
 target-i386/seg_helper.c|9 +-
 target-lm32/cpu.c   |2 +-
 target-m68k/cpu.c   |2 +-
 target-microblaze/cpu.c |2 +-
 target-mips/cpu.c   |8 ++
 target-mips/op_helper.c |   33 --
 target-mips/translate.c |   17 ++-
 target-openrisc/cpu.c   |2 +-
 target-ppc/kvm.c|   12 ++-
 target-ppc/kvm_ppc.h|4 +-
 target-ppc/translate_init.c |   10 +-
 target-s390x/cpu.c  |2 +-
 target-sh4/cpu.c|2 +-
 target-sparc/cpu.c  |2 +-
 xen-all.c   |4 +-
 49 Dateien geändert, 483 Zeilen hinzugefügt(+), 277 Zeilen entfernt(-)



[Qemu-devel] [PATCH 11/20] target-i386: kvm_check_features_against_host(): Use feature_word_info

2013-01-15 Thread Andreas Färber
From: Eduardo Habkost 

Instead of carrying the CPUID leaf/register and feature name array on
the model_features_t struct, move that information into
feature_word_info so it can be reused by other functions.

The goal is to eventually kill model_features_t entirely, but to do that
we have to either convert x86_def_t.features to an array or use
offsetof() inside FeatureWordInfo (to replace the pointers inside
model_features_t). So by now just move most of the model_features_t
fields to FeatureWordInfo except for the two pointers to local
arguments.

Signed-off-by: Eduardo Habkost 
Reviewed-by: Gleb Natapov 
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   73 +++--
 1 Datei geändert, 49 Zeilen hinzugefügt(+), 24 Zeilen entfernt(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e17709c..0e531f9 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -126,16 +126,39 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
 
 typedef struct FeatureWordInfo {
 const char **feat_names;
+uint32_t cpuid_eax; /* Input EAX for CPUID */
+int cpuid_reg;  /* R_* register constant */
 } FeatureWordInfo;
 
 static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
-[FEAT_1_EDX] = { .feat_names = feature_name },
-[FEAT_1_ECX] = { .feat_names = ext_feature_name },
-[FEAT_8000_0001_EDX] = { .feat_names = ext2_feature_name },
-[FEAT_8000_0001_ECX] = { .feat_names = ext3_feature_name },
-[FEAT_KVM]   = { .feat_names = kvm_feature_name },
-[FEAT_SVM]   = { .feat_names = svm_feature_name },
-[FEAT_7_0_EBX] = { .feat_names = cpuid_7_0_ebx_feature_name },
+[FEAT_1_EDX] = {
+.feat_names = feature_name,
+.cpuid_eax = 1, .cpuid_reg = R_EDX,
+},
+[FEAT_1_ECX] = {
+.feat_names = ext_feature_name,
+.cpuid_eax = 1, .cpuid_reg = R_ECX,
+},
+[FEAT_8000_0001_EDX] = {
+.feat_names = ext2_feature_name,
+.cpuid_eax = 0x8001, .cpuid_reg = R_EDX,
+},
+[FEAT_8000_0001_ECX] = {
+.feat_names = ext3_feature_name,
+.cpuid_eax = 0x8001, .cpuid_reg = R_ECX,
+},
+[FEAT_KVM] = {
+.feat_names = kvm_feature_name,
+.cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX,
+},
+[FEAT_SVM] = {
+.feat_names = svm_feature_name,
+.cpuid_eax = 0x800A, .cpuid_reg = R_EDX,
+},
+[FEAT_7_0_EBX] = {
+.feat_names = cpuid_7_0_ebx_feature_name,
+.cpuid_eax = 7, .cpuid_reg = R_EBX,
+},
 };
 
 const char *get_register_name_32(unsigned int reg)
@@ -162,9 +185,7 @@ const char *get_register_name_32(unsigned int reg)
 typedef struct model_features_t {
 uint32_t *guest_feat;
 uint32_t *host_feat;
-const char **flag_names;
-uint32_t cpuid;
-int reg;
+FeatureWord feat_word;
 } model_features_t;
 
 int check_cpuid = 0;
@@ -962,19 +983,19 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
 #endif /* CONFIG_KVM */
 }
 
-static int unavailable_host_feature(struct model_features_t *f, uint32_t mask)
+static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
 {
 int i;
 
 for (i = 0; i < 32; ++i)
 if (1 << i & mask) {
-const char *reg = get_register_name_32(f->reg);
+const char *reg = get_register_name_32(f->cpuid_reg);
 assert(reg);
 fprintf(stderr, "warning: host doesn't support requested feature: "
 "CPUID.%02XH:%s%s%s [bit %d]\n",
-f->cpuid, reg,
-f->flag_names[i] ? "." : "",
-f->flag_names[i] ? f->flag_names[i] : "", i);
+f->cpuid_eax, reg,
+f->feat_names[i] ? "." : "",
+f->feat_names[i] ? f->feat_names[i] : "", i);
 break;
 }
 return 0;
@@ -992,25 +1013,29 @@ static int kvm_check_features_against_host(x86_def_t 
*guest_def)
 int rv, i;
 struct model_features_t ft[] = {
 {&guest_def->features, &host_def.features,
-feature_name, 0x0001, R_EDX},
+FEAT_1_EDX },
 {&guest_def->ext_features, &host_def.ext_features,
-ext_feature_name, 0x0001, R_ECX},
+FEAT_1_ECX },
 {&guest_def->ext2_features, &host_def.ext2_features,
-ext2_feature_name, 0x8001, R_EDX},
+FEAT_8000_0001_EDX },
 {&guest_def->ext3_features, &host_def.ext3_features,
-ext3_feature_name, 0x8001, R_ECX}
+FEAT_8000_0001_ECX },
 };
 
 assert(kvm_enabled());
 
 kvm_cpu_fill_host(&host_def);
-for (rv = 0, i = 0; i < ARRAY_SIZE(ft); ++i)
-for (mask = 1; mask; mask <<= 1)
+for (rv = 0, i = 0; i < ARRAY_SIZE(ft); ++i) {
+FeatureWord w = ft[i].feat_word;
+FeatureWordInfo *wi = &feature_word_info[w];
+for (mask = 1; mask; mask <<= 1) {
 if (*ft[i].guest_feat & mask &&
 !(*ft[i].host_feat

[Qemu-devel] [PATCH 07/20] exec: Return CPUState from qemu_get_cpu()

2013-01-15 Thread Andreas Färber
Move the declaration to qemu/cpu.h and add documentation.
The implementation still depends on CPUArchState for CPU iteration.

Signed-off-by: Andreas Färber 
---
 exec.c  |6 +++---
 hw/pxa2xx_gpio.c|2 +-
 include/exec/cpu-all.h  |1 -
 include/qom/cpu.h   |   10 ++
 target-mips/op_helper.c |   11 ---
 5 Dateien geändert, 22 Zeilen hinzugefügt(+), 8 Zeilen entfernt(-)

diff --git a/exec.c b/exec.c
index e5265e6..5689613 100644
--- a/exec.c
+++ b/exec.c
@@ -247,10 +247,10 @@ static const VMStateDescription vmstate_cpu_common = {
 };
 #endif
 
-CPUArchState *qemu_get_cpu(int index)
+CPUState *qemu_get_cpu(int index)
 {
 CPUArchState *env = first_cpu;
-CPUState *cpu;
+CPUState *cpu = NULL;
 
 while (env) {
 cpu = ENV_GET_CPU(env);
@@ -260,7 +260,7 @@ CPUArchState *qemu_get_cpu(int index)
 env = env->next_cpu;
 }
 
-return env;
+return cpu;
 }
 
 void cpu_exec_init(CPUArchState *env)
diff --git a/hw/pxa2xx_gpio.c b/hw/pxa2xx_gpio.c
index c02c295..eec2ea3 100644
--- a/hw/pxa2xx_gpio.c
+++ b/hw/pxa2xx_gpio.c
@@ -277,7 +277,7 @@ static int pxa2xx_gpio_initfn(SysBusDevice *dev)
 
 s = FROM_SYSBUS(PXA2xxGPIOInfo, dev);
 
-s->cpu = arm_env_get_cpu(qemu_get_cpu(s->ncpu));
+s->cpu = ARM_CPU(qemu_get_cpu(s->ncpu));
 
 qdev_init_gpio_in(&dev->qdev, pxa2xx_gpio_set, s->lines);
 qdev_init_gpio_out(&dev->qdev, s->handler, s->lines);
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 439e88d..249e046 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -354,7 +354,6 @@ int page_check_range(target_ulong start, target_ulong len, 
int flags);
 #endif
 
 CPUArchState *cpu_copy(CPUArchState *env);
-CPUArchState *qemu_get_cpu(int cpu);
 
 #define CPU_DUMP_CODE 0x0001
 #define CPU_DUMP_FPU 0x0002 /* dump FPU register state, not just integer */
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index d5e0a40..773caf9 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -156,5 +156,15 @@ bool cpu_is_stopped(CPUState *cpu);
  */
 void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
 
+/**
+ * qemu_get_cpu:
+ * @index: The CPUState@cpu_index value of the CPU to obtain.
+ *
+ * Gets a CPU matching @index.
+ *
+ * Returns: The CPU or %NULL if there is no matching CPU.
+ */
+CPUState *qemu_get_cpu(int index);
+
 
 #endif
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 1816a0e..1bca4a1 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -585,8 +585,9 @@ static inline void mips_tc_sleep(MIPSCPU *cpu, int tc)
   walking the list of CPUMIPSStates.  */
 static CPUMIPSState *mips_cpu_map_tc(CPUMIPSState *env, int *tc)
 {
+MIPSCPU *cpu;
 CPUState *cs;
-CPUMIPSState *other;
+CPUState *other_cs;
 int vpe_idx;
 int tc_idx = *tc;
 
@@ -599,8 +600,12 @@ static CPUMIPSState *mips_cpu_map_tc(CPUMIPSState *env, 
int *tc)
 cs = CPU(mips_env_get_cpu(env));
 vpe_idx = tc_idx / cs->nr_threads;
 *tc = tc_idx % cs->nr_threads;
-other = qemu_get_cpu(vpe_idx);
-return other ? other : env;
+other_cs = qemu_get_cpu(vpe_idx);
+if (other_cs == NULL) {
+return env;
+}
+cpu = MIPS_CPU(other_cs);
+return &cpu->env;
 }
 
 /* The per VPE CP0_Status register shares some fields with the per TC
-- 
1.7.10.4




[Qemu-devel] [PATCH 16/20] target-i386: Move kvm_check_features_against_host() check to realize time

2013-01-15 Thread Andreas Färber
From: Igor Mammedov 

kvm_check_features_against_host() should be called when features can't
be changed, and when features are converted to properties it would be
possible to change them until realize time, so correct way is to call
kvm_check_features_against_host() in x86_cpu_realize().

Signed-off-by: Igor Mammedov 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   28 +++-
 1 Datei geändert, 15 Zeilen hinzugefügt(+), 13 Zeilen entfernt(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3a68470..333745b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1022,27 +1022,28 @@ static int unavailable_host_feature(FeatureWordInfo *f, 
uint32_t mask)
  *
  * This function may be called only if KVM is enabled.
  */
-static int kvm_check_features_against_host(x86_def_t *guest_def)
+static int kvm_check_features_against_host(X86CPU *cpu)
 {
+CPUX86State *env = &cpu->env;
 x86_def_t host_def;
 uint32_t mask;
 int rv, i;
 struct model_features_t ft[] = {
-{&guest_def->features, &host_def.features,
+{&env->cpuid_features, &host_def.features,
 FEAT_1_EDX },
-{&guest_def->ext_features, &host_def.ext_features,
+{&env->cpuid_ext_features, &host_def.ext_features,
 FEAT_1_ECX },
-{&guest_def->ext2_features, &host_def.ext2_features,
+{&env->cpuid_ext2_features, &host_def.ext2_features,
 FEAT_8000_0001_EDX },
-{&guest_def->ext3_features, &host_def.ext3_features,
+{&env->cpuid_ext3_features, &host_def.ext3_features,
 FEAT_8000_0001_ECX },
-{&guest_def->ext4_features, &host_def.ext4_features,
+{&env->cpuid_ext4_features, &host_def.ext4_features,
 FEAT_C000_0001_EDX },
-{&guest_def->cpuid_7_0_ebx_features, &host_def.cpuid_7_0_ebx_features,
+{&env->cpuid_7_0_ebx_features, &host_def.cpuid_7_0_ebx_features,
 FEAT_7_0_EBX },
-{&guest_def->svm_features, &host_def.svm_features,
+{&env->cpuid_svm_features, &host_def.svm_features,
 FEAT_SVM },
-{&guest_def->kvm_features, &host_def.kvm_features,
+{&env->cpuid_kvm_features, &host_def.kvm_features,
 FEAT_KVM },
 };
 
@@ -1471,10 +1472,6 @@ static int cpu_x86_parse_featurestr(x86_def_t 
*x86_cpu_def, char *features)
 x86_cpu_def->kvm_features &= ~minus_features[FEAT_KVM];
 x86_cpu_def->svm_features &= ~minus_features[FEAT_SVM];
 x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_features[FEAT_7_0_EBX];
-if (check_cpuid && kvm_enabled()) {
-if (kvm_check_features_against_host(x86_cpu_def) && enforce_cpuid)
-goto error;
-}
 return 0;
 
 error:
@@ -2177,6 +2174,11 @@ void x86_cpu_realize(Object *obj, Error **errp)
 #ifdef CONFIG_KVM
 filter_features_for_kvm(cpu);
 #endif
+if (check_cpuid && kvm_check_features_against_host(cpu)
+&& enforce_cpuid) {
+error_setg(errp, "Host's CPU doesn't support requested features");
+return;
+}
 }
 
 #ifndef CONFIG_USER_ONLY
-- 
1.7.10.4




[Qemu-devel] [PATCH 20/20] target-i386: Use switch in check_hw_breakpoints()

2013-01-15 Thread Andreas Färber
From: liguang 

Replace an if statement using magic numbers for breakpoint type with a
more explicit switch statement. This is to aid readability.

Change the return type and force_dr6_update argument type to bool.

While at it, fix Coding Style issues (missing braces).

Signed-off-by: liguang 
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.h |2 +-
 target-i386/helper.c  |   44 
 target-i386/misc_helper.c |2 +-
 3 Dateien geändert, 34 Zeilen hinzugefügt(+), 14 Zeilen entfernt(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 1e850a7..4e091cd 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1043,7 +1043,7 @@ static inline int hw_breakpoint_len(unsigned long dr7, 
int index)
 
 void hw_breakpoint_insert(CPUX86State *env, int index);
 void hw_breakpoint_remove(CPUX86State *env, int index);
-int check_hw_breakpoints(CPUX86State *env, int force_dr6_update);
+bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update);
 void breakpoint_handler(CPUX86State *env);
 
 /* will be suppressed */
diff --git a/target-i386/helper.c b/target-i386/helper.c
index a10b562..547c25e 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1017,26 +1017,45 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
 }
 }
 
-int check_hw_breakpoints(CPUX86State *env, int force_dr6_update)
+bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
 {
 target_ulong dr6;
-int reg, type;
-int hit_enabled = 0;
+int reg;
+bool hit_enabled = false;
 
 dr6 = env->dr[6] & ~0xf;
 for (reg = 0; reg < DR7_MAX_BP; reg++) {
-type = hw_breakpoint_type(env->dr[7], reg);
-if ((type == 0 && env->dr[reg] == env->eip) ||
-((type & 1) && env->cpu_watchpoint[reg] &&
- (env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT))) {
+bool bp_match = false;
+bool wp_match = false;
+
+switch (hw_breakpoint_type(env->dr[7], reg)) {
+case DR7_TYPE_BP_INST:
+if (env->dr[reg] == env->eip) {
+bp_match = true;
+}
+break;
+case DR7_TYPE_DATA_WR:
+case DR7_TYPE_DATA_RW:
+if (env->cpu_watchpoint[reg] &&
+env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT) {
+wp_match = true;
+}
+break;
+case DR7_TYPE_IO_RW:
+break;
+}
+if (bp_match || wp_match) {
 dr6 |= 1 << reg;
 if (hw_breakpoint_enabled(env->dr[7], reg)) {
-hit_enabled = 1;
+hit_enabled = true;
 }
 }
 }
-if (hit_enabled || force_dr6_update)
+
+if (hit_enabled || force_dr6_update) {
 env->dr[6] = dr6;
+}
+
 return hit_enabled;
 }
 
@@ -1047,16 +1066,17 @@ void breakpoint_handler(CPUX86State *env)
 if (env->watchpoint_hit) {
 if (env->watchpoint_hit->flags & BP_CPU) {
 env->watchpoint_hit = NULL;
-if (check_hw_breakpoints(env, 0))
+if (check_hw_breakpoints(env, false)) {
 raise_exception(env, EXCP01_DB);
-else
+} else {
 cpu_resume_from_signal(env, NULL);
+}
 }
 } else {
 QTAILQ_FOREACH(bp, &env->breakpoints, entry)
 if (bp->pc == env->eip) {
 if (bp->flags & BP_CPU) {
-check_hw_breakpoints(env, 1);
+check_hw_breakpoints(env, true);
 raise_exception(env, EXCP01_DB);
 }
 break;
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index b3f4e4f..b6d5740 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c
@@ -110,7 +110,7 @@ void helper_into(CPUX86State *env, int next_eip_addend)
 void helper_single_step(CPUX86State *env)
 {
 #ifndef CONFIG_USER_ONLY
-check_hw_breakpoints(env, 1);
+check_hw_breakpoints(env, true);
 env->dr[6] |= DR6_BS;
 #endif
 raise_exception(env, EXCP01_DB);
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH KVM v2 1/4] KVM: fix i8254 IRQ0 to be normally high

2013-01-15 Thread Matthew Ogilvie
On Fri, Jan 11, 2013 at 05:45:28PM +0200, Gleb Natapov wrote:
> On Thu, Jan 10, 2013 at 11:40:07PM -0700, Matthew Ogilvie wrote:
> > On Tue, Jan 08, 2013 at 09:31:19AM +0200, Gleb Natapov wrote:
> > > On Mon, Jan 07, 2013 at 06:17:22PM -0600, mmogi...@miniinfo.net wrote:
> > > > On Mon, 7 Jan 2013 11:39:18 +0200, Gleb Natapov  wrote:
> > > > > On Wed, Dec 26, 2012 at 10:39:53PM -0700, Matthew Ogilvie wrote:
> > > > >> Reading the spec, it is clear that most modes normally leave the IRQ
> > > > >> output line high, and only pulse it low to generate a leading edge.
> > > > >> Especially the most commonly used mode 2.
> > > > >> 
> > > > >> The KVM i8254 model does not try to emulate the duration of the 
> > > > >> pulse at
> > > > >> all, so just swap the high/low settings it to leave it high most of
> > > > >> the time.
> > > > >> 
> > > > >> This fix is a prerequisite to improving the i8259 model to handle
> > > > >> the trailing edge of an interupt request as indicated in its spec:
> > > > >> If it gets a trailing edge of an IRQ line before it starts to service
> > > > >> the interrupt, the request should be canceled.
> > > > >> 
> > > > >> See http://bochs.sourceforge.net/techspec/intel-82c54-timer.pdf.gz
> > > > >> or search the net for 23124406.pdf.
> > > > >> 
> > > > >> Risks:
> > > > >> 
> > > > >> There is a risk that migrating a running guest between versions
> > > > >> with and without this patch will lose or gain a single timer
> > > > >> interrupt during the migration process.  The only case where
> > > > > Can you elaborate on how exactly this can happen? Do not see it.
> > > > > 
> > > > 
> > > > KVM 8254: In the corrected model, when the count expires, the model
> > > > briefly pulses output low and then high again, with the low to high
> > > > transition being what triggers the interrupt.  In the old model,
> > > > when the count expires, the model expects the output line
> > > > to already be low, and briefly pulses it high (triggering the
> > > > interrupt) and then low again.  But if the line was already
> > > > high (because it migrated from the corrected model),
> > > > this won't generate a new leading edge (low to high) and won't
> > > > trigger a new interrupt (the first post-back-migration pulse turns
> > > > into a simple trailing edge instead of a pulse).
> > > > 
> > > > Unless there is something I'm missing?
> > > > 
> > > No, I missed that pic->last_irr/ioapic->irr  will be migrated as 1. But
> > > this means that next interrupt after migration from new to old will
> > > always be lost.  What about clearing pit bit from last_irr/irr before
> > > migration? Should not affect new->new migration and should fix new->old
> > > one. The only problem is that we may need to consult irq routing table
> > > to know how pit is connected to ioapic.
> > 
> > We should not clear both last_irr and irr.  That
> > cancels the interrupt early if the CPU hasn't started servicing
> > it yet:  If the guest CPU has interrupts disabled
> > and hasn't begun to service the interrupt, a new->new migration could
> > lose the interrupt much earlier in the countdown cycle than it otherwise
> > might be lost.
> > 
> I am talking about last_irr in pic and irr in ioapic. Of course we
> shouldn't clear pic->irr on migration. ioapic uses irr to detect edge.

I probably need to look into the details of how the ioapic is supposed
to work.  Sigh.

> 
> > Potentially we could clear the last_irr bit only.  This effectively
> > makes migration behave like the old unfixed code.  But if this is
> > considered acceptable, I would be more inclined to not fix IRQ0 at all,
> If we do not fix IRQ0 next timer interrupt after migration will always be
> lost.

Obviously true if the trailing edge consumption logic for the
IRQ0 line is corrected in the 8259.  But if it isn't:

I probably could have been clearer that we could leave the 8254 unchanged,
and adjust the 8259 fix to leave IRQ0 as-is (with the incorrect handling
of a trailing edge - only other IRQ lines would be fixed), and then
timer interrupts would just work exactly as they do now.
This minimal fix would would probably be the lowest risk of
breaking something that currently works, but I don't know if
people would go for a patch that intentionally leaves in known
breakage in IRQ0...  This is option 1 below.

--

If we cleared last_irr in the qemu model during migration,
we risk getting an EXTRA interrupt when
migrating mode 4 (misremembered as mode 2 in an earlier
email below) from old to new models.  (If the sequence goes: line
is asserted, guest migrates, interrupt is handled [all in less than
a 8254 clock tick (approx 1 MHz)], then the off-by-one edge
in the new model triggers another leading edge...)  In contrast,
this might not be a risk in the KVM model as currently implemented.

Maybe this objection is not important, and you like option 4
listed below.

---

So I'm still not sure what overall fix strategy the main qemu and kvm
maintainers would like t

Re: [Qemu-devel] [Qemu-trivial] [PATCH] configure: try pkg-config for curses

2013-01-15 Thread Stefan Hajnoczi
On Fri, Jan 11, 2013 at 04:51:24PM +0400, Vadim Evard wrote:
> ping
> 
> Пнд 31 Дек 2012 20:30:59, Vadim Evard писал:
> >configure: try pkg-config for curses
> >
> >Static linkikng against ncurses may require explicit -ltinfo.
> >In case -lcurses and -lncurses both didn't work give pkg-config a
> >chance.
> >
> >Signed-off-by: Vadim Evard 
> >---
> >configure | 4 +++-
> >1 file changed, 3 insertions(+), 1 deletion(-)
> >
> >diff --git a/configure b/configure
> >index b0c7e54..16280e2 100755
> >--- a/configure
> >+++ b/configure
> >@@ -2030,7 +2030,7 @@ fi
> >if test "$mingw32" = "yes" ; then
> >curses_list="-lpdcurses"
> >else
> >- curses_list="-lncurses -lcurses"
> >+ curses_list="-lncurses:-lcurses:$($pkg_config --libs ncurses)"
> >fi
> >
> >if test "$curses" != "no" ; then
> >@@ -2043,7 +2043,9 @@ int main(void) {
> >return s != 0;
> >}
> >EOF
> >+ IFS=:
> >for curses_lib in $curses_list; do
> >+ unset IFS
> >if compile_prog "" "$curses_lib" ; then
> >curses_found=yes
> >libs_softmmu="$curses_lib $libs_softmmu"

Please also unset IFS after the loop so there will never be a problem if
curses_list="".

Stefan



[Qemu-devel] [PATCH 19/20] target-i386: Avoid goto in hw_breakpoint_insert()

2013-01-15 Thread Andreas Färber
From: liguang 

  "Go To Statement Considered Harmful" -- E. Dijkstra

To avoid an unnecessary goto within the switch statement, move
watchpoint insertion out of the switch statement. Improves readability.

While at it, fix Coding Style issues (missing braces, indentation).

Signed-off-by: liguang 
Signed-off-by: Andreas Färber 
---
 target-i386/helper.c |   16 ++--
 1 Datei geändert, 10 Zeilen hinzugefügt(+), 6 Zeilen entfernt(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index ebdd6a5..a10b562 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -966,7 +966,7 @@ hwaddr cpu_get_phys_page_debug(CPUX86State *env, 
target_ulong addr)
 
 void hw_breakpoint_insert(CPUX86State *env, int index)
 {
-int type, err = 0;
+int type = 0, err = 0;
 
 switch (hw_breakpoint_type(env->dr[7], index)) {
 case DR7_TYPE_BP_INST:
@@ -977,20 +977,24 @@ void hw_breakpoint_insert(CPUX86State *env, int index)
 break;
 case DR7_TYPE_DATA_WR:
 type = BP_CPU | BP_MEM_WRITE;
-goto insert_wp;
+break;
 case DR7_TYPE_IO_RW:
- /* No support for I/O watchpoints yet */
+/* No support for I/O watchpoints yet */
 break;
 case DR7_TYPE_DATA_RW:
 type = BP_CPU | BP_MEM_ACCESS;
-insert_wp:
+break;
+}
+
+if (type != 0) {
 err = cpu_watchpoint_insert(env, env->dr[index],
 hw_breakpoint_len(env->dr[7], index),
 type, &env->cpu_watchpoint[index]);
-break;
 }
-if (err)
+
+if (err) {
 env->cpu_breakpoint[index] = NULL;
+}
 }
 
 void hw_breakpoint_remove(CPUX86State *env, int index)
-- 
1.7.10.4




Re: [Qemu-devel] [Qemu-trivial] [PATCH for-1.4 v2] qom: Make object_resolve_path_component() path argument const

2013-01-15 Thread Stefan Hajnoczi
On Tue, Jan 15, 2013 at 02:55:10AM +0100, Andreas Färber wrote:
> A usage with a hardcoded partial path such as
> 
>   object_resolve_path_component(obj, "foo")
> 
> is totally valid but currently leads to a compilation error. Fix this.
> 
> Signed-off-by: Andreas Färber 
> ---
>  include/qom/object.h |2 +-
>  qom/object.c |2 +-
>  2 Dateien geändert, 2 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan



Re: [Qemu-devel] USB2.0 device can not be formatted under windows(Guest OS) with qmeu 1.2.2

2013-01-15 Thread Michael Tokarev

14.01.2013 20:04, 曾君亮 wrote:

Hi all,
I want to test usb2.0 passthrough on xen 4.2.0 and qemu 1.2.2, and now I 
have done it successfully!  But these days, I encountered a lot of 
difficulties. I fonud that the USB2.0 device can not be formatted while it was 
passthrough to a windows GusetOS.However,it works fine under a linux GuestOS.
   The Dom0 circulating Print:"kernel: usb 2-1: reset high speed USB device using 
ehci_hcd and address 2".
Now,I have no idea how to get it to work under a windows operating system. 
Does anyone have encountered the same problem as me? Does it a bug of qemu 
1.2.2 ?


Please take a look at https://bugs.launchpad.net/bugs/1033727 ,
maybe it will be helpful.

Thanks,

/mjt



Re: [Qemu-devel] [RFC qom-cpu v2 1/2] target-i386: Convert CPU definitions into X86CPU subclasses

2013-01-15 Thread Eduardo Habkost
On Tue, Jan 15, 2013 at 09:41:04AM +0100, Igor Mammedov wrote:
> On Mon, 10 Dec 2012 23:59:31 +0100
> Andreas Färber  wrote:
> 
> > TODO: sort classes for -cpu ?, generalize X86CPUListState, more testing
> > 
> > Signed-off-by: Andreas Färber 
> > Cc: Eduardo Habkost 
> > Cc: Igor Mammedov 
[...]
> The patch is just renaming of the current builtin_x86_defs
> into a bunch of functions and polluting X86CPUClass
> with fields from the former x86_def_t.
> object_new() still creates a dummy cpu instance whose defaults
> are still manually copied from X86CPUClass instead of x86_def_t.
> 

That's a good thing, isn't it? It means the patch is easier to review.
:-)

No patch alone will do everything we want, because we want to do a lot.
We need to do it one step at a time.

(BTW, why are you looking at this RFC instead of the more recent one,
that I have sent on Jan 4? [that's very similar to this one])


> What's the point in having dummy sub-classes?
> How it can help in your CPU re-factoring?

It will help us to unify the CPU creation/realization code that's
duplicated over all the architectures.

It will give libvirt an easy mechanism to list the available CPU models
that won't require parsing help output (using "qom-list-types" QMP
command).

> 
> 
> On the other hand converting features to static properties first and
> then converting X86CPU to sub-classes, yields already initialized to
> defaults sub-class completely removing notion of x86_def_t and
> not polluting X86CPUClass with redundant fields.

I wouldn't disagree with this approach in principle, but I believe our
main problem today is lack of reviewer bandwidth. I learned the hard way
that trying to clean up everything before implementing something will
make sure the code takes forever to be reviewed.


> For example see following patches on
> https://github.com/imammedo/qemu/commits/x86-cpu-classes.Jan142013
> 
> e9fd18f qdev: extend DEFINE_GENERIC_PROP() to support default values
> c65eca9 qdev: make qdev_prop_find_bit return non const so prop default value 
> could be modified
> 0311952 allow to expolit default value static props for model, family, 
> stepping & vendor props
> 8b3080e target-i386: add helpers to change default values of static 
> properties before object is created
> ed506d3 target-i386: prepare for subclasses to have its own instance of 
> static properties definitions
> a48e252 target-i386: declare subclass for qemu64 cpu model
> 9c556c2 target-i386: move cpu_x86_init() & cpu_x86_register() into it and 
> switch to subclasses. PS: implemended only for qemu64
> f5dbfe6 CPU_CLASS_NAME(qemu64) hack
> 00e15b8 target-i386: properties list are per subclass: do not set them in 
> superclass to avoid defaults set by subclass be over-written
> 
> -- 
> Regards,
>   Igor

-- 
Eduardo



Re: [Qemu-devel] [PATCH 0/7] Discard improvements

2013-01-15 Thread Michael Tokarev

That's quite an.. interesting $Subject.  Can be read in at least two ways... ;)

Sorry can't resist ;)

/mjt



Re: [Qemu-devel] [PULL] pci,virtio

2013-01-15 Thread Andreas Färber
Am 13.01.2013 11:47, schrieb Michael S. Tsirkin:
> The following changes since commit 8e4a424b305e29dc0e454f52df3b35577f342975:
> 
>   Revert "virtio-pci: replace byte swap hack" (2013-01-06 18:30:17 +)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony
> 
> for you to fetch changes up to feb9a2ab4b0260d8d680a7ffd25063dafc7ec628:
> 
>   pci-assign: Enable MSIX on device to match guest (2013-01-09 12:11:16 +0200)
> 
> 
> pci,virtio
> 
> This further optimizes MSIX handling in virtio-pci.
> Also included is pci cleanup by Paolo, and pci device
> assignment fix by Alex.
> 
> Signed-off-by: Michael S. Tsirkin 
> 
> 
> Alex Williamson (1):
>   pci-assign: Enable MSIX on device to match guest
> 
> Michael S. Tsirkin (8):
>   virtio: don't waste irqfds on control vqs
>   msix: add api to access msix message
>   kvm: add stub for update msi route
>   virtio-pci: cache msix messages

This broke the build with PowerKVM:

  LINK  ppc64-softmmu/qemu-system-ppc64
../hw/virtio-pci.o: In function `kvm_virtio_pci_vq_vector_unmask':
/home/andreas/QEMU/qemu/hw/virtio-pci.c:622: undefined reference to
`kvm_irqchip_update_msi_route'
collect2: error: ld returned 1 exit status
make[1]: *** [qemu-system-ppc64] Fehler 1
make: *** [subdir-ppc64-softmmu] Fehler 2

Can you supply a fix? It's not obvious to me how.

Thanks,
Andreas

>   virtio: backend virtqueue notifier masking
>   virtio-net: set/clear vhost_started in reverse order
>   vhost: set started flag while start is in progress
>   vhost: backend masking support
> 
> Paolo Bonzini (5):
>   docs: move pci-ids.txt to docs/specs/
>   reorganize pci-ids.txt
>   virtio-9p: use symbolic constant, add to pci-ids.txt
>   ivshmem: use symbolic constant for PCI ID, add to pci-ids.txt
>   pci: use constants for devices under the 1B36 device ID, document them
> 
>  docs/specs/pci-ids.txt |  50 +++
>  hw/9pfs/virtio-9p-device.c |   2 +-
>  hw/ivshmem.c   |   7 +-
>  hw/kvm/pci-assign.c|  17 +++-
>  hw/pci/msix.c  |   2 +-
>  hw/pci/msix.h  |   1 +
>  hw/pci/pci.h   |   8 ++
>  hw/pci_bridge_dev.c|   8 +-
>  hw/serial-pci.c|  12 +--
>  hw/vhost.c | 112 +
>  hw/vhost.h |  10 +++
>  hw/vhost_net.c |  27 +-
>  hw/vhost_net.h |   3 +
>  hw/virtio-net.c|  22 -
>  hw/virtio-pci.c| 203 
> +++--
>  hw/virtio-pci.h|   2 +
>  hw/virtio.h|  15 +++-
>  kvm-stub.c |   5 ++
>  pci-ids.txt|  31 ---
>  19 files changed, 437 insertions(+), 100 deletions(-)
>  create mode 100644 docs/specs/pci-ids.txt
>  delete mode 100644 pci-ids.txt
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [PATCH 13/20] target-i386: check/enforce: Check all feature words

2013-01-15 Thread Andreas Färber
From: Eduardo Habkost 

This adds the following feature words to the list of flags to be checked
by kvm_check_features_against_host():

 - cpuid_7_0_ebx_features
 - ext4_features
 - kvm_features
 - svm_features

This will ensure the "enforce" flag works as it should: it won't allow
QEMU to be started unless every flag that was requested by the user or
defined in the CPU model is supported by the host.

This patch may cause existing configurations where "enforce" wasn't
preventing QEMU from being started to abort QEMU. But that's exactly the
point of this patch: if a flag was not supported by the host and QEMU
wasn't aborting, it was a bug in the "enforce" code.

Signed-off-by: Eduardo Habkost 
Reviewed-by: Gleb Natapov 
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   13 +++--
 1 Datei geändert, 11 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8ec9929..9a48e3f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1016,8 +1016,9 @@ static int unavailable_host_feature(FeatureWordInfo *f, 
uint32_t mask)
 return 0;
 }
 
-/* best effort attempt to inform user requested cpu flags aren't making
- * their way to the guest.
+/* Check if all requested cpu flags are making their way to the guest
+ *
+ * Returns 0 if all flags are supported by the host, non-zero otherwise.
  *
  * This function may be called only if KVM is enabled.
  */
@@ -1035,6 +1036,14 @@ static int kvm_check_features_against_host(x86_def_t 
*guest_def)
 FEAT_8000_0001_EDX },
 {&guest_def->ext3_features, &host_def.ext3_features,
 FEAT_8000_0001_ECX },
+{&guest_def->ext4_features, &host_def.ext4_features,
+FEAT_C000_0001_EDX },
+{&guest_def->cpuid_7_0_ebx_features, &host_def.cpuid_7_0_ebx_features,
+FEAT_7_0_EBX },
+{&guest_def->svm_features, &host_def.svm_features,
+FEAT_SVM },
+{&guest_def->kvm_features, &host_def.kvm_features,
+FEAT_KVM },
 };
 
 assert(kvm_enabled());
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH 00/12] Multiqueue virtio-net

2013-01-15 Thread Jason Wang
On 01/15/2013 03:44 AM, Anthony Liguori wrote:
> Jason Wang  writes:
>
>> Hello all:
>>
>> This seires is an update of last version of multiqueue virtio-net support.
>>
>> Recently, linux tap gets multiqueue support. This series implements basic
>> support for multiqueue tap, nic and vhost. Then use it as an infrastructure 
>> to
>> enable the multiqueue support for virtio-net.
>>
>> Both vhost and userspace multiqueue were implemented for virtio-net, but
>> userspace could be get much benefits since dataplane like parallized 
>> mechanism
>> were not implemented.
>>
>> User could start a multiqueue virtio-net card through adding a "queues"
>> parameter to tap.
>>
>> ./qemu -netdev tap,id=hn0,queues=2,vhost=on -device virtio-net-pci,netdev=hn0
>>
>> Management tools such as libvirt can pass multiple pre-created fds through
>>
>> ./qemu -netdev tap,id=hn0,queues=2,fd=X,fd=Y -device
>> virtio-net-pci,netdev=hn0
> I'm confused/frightened that this syntax works.  You shouldn't be
> allowed to have two values for the same property.  Better to have a
> syntax like fd[0]=X,fd[1]=Y or something along those lines.

Yes, but this what current a StringList type works for command line.
Some other parameters such as dnssearch, hostfwd and guestfwd have
already worked in this way. Looks like your suggestions need some
extension on QemuOps visitor, maybe we can do this on top.

Thanks
>
> Regards,
>
> Anthony Liguori
>
>> You can fetch and try the code from:
>> git://github.com/jasowang/qemu.git
>>
>> Patch 1 adds a generic method of creating multiqueue taps and implement the
>> linux part.
>> Patch 2 - 4 introduce some helpers which could be used to refactor the nic
>> emulation codes to support multiqueue.
>> Patch 5 introduces multiqueue support for qemu networking code: each peers of
>> NetClientState were abstracted as a queue. Though this, most of the codes 
>> could
>> be reusued without change.
>> Patch 6 adds basic multiqueue support for vhost which could let vhost just
>> handle a subset of all virtqueues.
>> Patch 7-8 introduce new helpers of virtio which is needed by multiqueue
>> virtio-net.
>> Patch 9-12 implement the multiqueue support of virtio-net
>>
>> Changes from RFC v2:
>> - rebase the codes to latest qemu
>> - align the multiqueue virtio-net implementation to virtio spec
>> - split the patches into more smaller patches
>> - set_link and hotplug support
>>
>> Changes from RFC V1:
>> - rebase to the latest
>> - fix memory leak in parse_netdev
>> - fix guest notifiers assignment/de-assignment
>> - changes the command lines to:
>>qemu -netdev tap,queues=2 -device virtio-net-pci,queues=2
>>
>> Reference:
>> v2: http://lists.gnu.org/archive/html/qemu-devel/2012-06/msg04108.html
>> v1: http://comments.gmane.org/gmane.comp.emulators.qemu/100481
>>
>> Perf Numbers:
>>
>> Two Intel Xeon 5620 with direct connected intel 82599EB
>> Host/Guest kernel: David net tree
>> vhost enabled
>>
>> - lots of improvents of both latency and cpu utilization in request-reponse 
>> test
>> - get regression of guest sending small packets which because TCP tends to 
>> batch
>>   less when the latency were improved
>>
>> 1q/2q/4q
>> TCP_RR
>>  size #sessions trans.rate  norm trans.rate  norm trans.rate  norm
>> 1 1 9393.26   595.64  9408.18   597.34  9375.19   584.12
>> 1 2072162.1   2214.24 129880.22 2456.13 196949.81 2298.13
>> 1 50107513.38 2653.99 139721.93 2490.58 259713.82 2873.57
>> 1 100   126734.63 2676.54 145553.5  2406.63 265252.68 2943
>> 64 19453.42   632.33  9371.37   616.13  9338.19   615.97
>> 64 20   70620.03  2093.68 125155.75 2409.15 191239.91 2253.32
>> 64 50   1069662448.29 146518.67 2514.47 242134.07 2720.91
>> 64 100  117046.35 2394.56 190153.09 2696.82 238881.29 2704.41
>> 256 1   8733.29   736.36  8701.07   680.83  8608.92   530.1
>> 256 20  69279.89  2274.45 115103.07 2299.76 144555.16 1963.53
>> 256 50  97676.02  2296.09 150719.57 2522.92 254510.5  3028.44
>> 256 100 150221.55 2949.56 197569.3  2790.92 300695.78 3494.83
>> TCP_CRR
>>  size #sessions trans.rate  norm trans.rate  norm trans.rate  norm
>> 1 1 2848.37  163.41 2230.39  130.89 2013.09  120.47
>> 1 2023434.5  562.11 31057.43 531.07 49488.28 564.41
>> 1 5028514.88 582.17 40494.23 605.92 60113.35 654.97
>> 1 100   28827.22 584.73 48813.25 661.6  61783.62 676.56
>> 64 12780.08  159.4  2201.07  127.96 2006.8   117.63
>> 64 20   23318.51 564.47 30982.44 530.24 49734.95 566.13
>> 64 50   28585.72 582.54 40576.7  610.08 60167.89 656.56
>> 64 100  28747.37 584.17 49081.87 667.87 60612.94 662
>> 256 1   2772.08  160.51 2231.84  131.05 2003.62  113.45
>> 256 20  23086.35 559.8  30929.09 528.16 48454.9  555.22
>> 256 50  28354.7  579.85 40578.31 60760261.71 657.87
>> 256 100 28844.55 585.67 48541.86 659.08 61941.07 676.72
>> TCP_STREAM guest receiving
>>  size #sessions throughput  norm throughput  norm throughput  norm
>> 1 1 16.27   1.33   16.11.12   16.13   0.99
>> 1 2 33.04   2.08   32.96   2.1

[Qemu-devel] [PATCH 08/20] kvm: Add fake KVM constants to avoid #ifdefs on KVM-specific code

2013-01-15 Thread Andreas Färber
From: Eduardo Habkost 

Any KVM-specific code that use these constants must check if
kvm_enabled() is true before using them.

Signed-off-by: Eduardo Habkost 
Reviewed-by: Gleb Natapov 
Signed-off-by: Andreas Färber 
---
 include/sysemu/kvm.h |   14 ++
 1 Datei geändert, 14 Zeilen hinzugefügt(+)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 2fe8f8a..6bdd513 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -22,6 +22,20 @@
 #ifdef CONFIG_KVM
 #include 
 #include 
+#else
+/* These constants must never be used at runtime if kvm_enabled() is false.
+ * They exist so we don't need #ifdefs around KVM-specific code that already
+ * checks kvm_enabled() properly.
+ */
+#define KVM_CPUID_SIGNATURE  0
+#define KVM_CPUID_FEATURES   0
+#define KVM_FEATURE_CLOCKSOURCE  0
+#define KVM_FEATURE_NOP_IO_DELAY 0
+#define KVM_FEATURE_MMU_OP   0
+#define KVM_FEATURE_CLOCKSOURCE2 0
+#define KVM_FEATURE_ASYNC_PF 0
+#define KVM_FEATURE_STEAL_TIME   0
+#define KVM_FEATURE_PV_EOI   0
 #endif
 
 extern int kvm_allowed;
-- 
1.7.10.4




[Qemu-devel] [PATCH 15/20] target-i386: cpu_x86_register() consolidate freeing resources

2013-01-15 Thread Andreas Färber
From: Igor Mammedov 

Freeing resources in one place would require setting 'error'
to not NULL, so add some more error reporting before jumping to
exit branch.

Signed-off-by: Igor Mammedov 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   19 ++-
 1 Datei geändert, 10 Zeilen hinzugefügt(+), 9 Zeilen entfernt(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e75b293..3a68470 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1594,20 +1594,23 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 
 model_pieces = g_strsplit(cpu_model, ",", 2);
 if (!model_pieces[0]) {
-goto error;
+error_setg(&error, "Invalid/empty CPU model name");
+goto out;
 }
 name = model_pieces[0];
 features = model_pieces[1];
 
 if (cpu_x86_find_by_name(def, name) < 0) {
-goto error;
+error_setg(&error, "Unable to find CPU definition: %s", name);
+goto out;
 }
 
 def->kvm_features |= kvm_default_features;
 def->ext_features |= CPUID_EXT_HYPERVISOR;
 
 if (cpu_x86_parse_featurestr(def, features) < 0) {
-goto error;
+error_setg(&error, "Invalid cpu_model string format: %s", cpu_model);
+goto out;
 }
 assert(def->vendor1);
 env->cpuid_vendor1 = def->vendor1;
@@ -1632,17 +1635,15 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 "tsc-frequency", &error);
 
 object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
+
+out:
+g_strfreev(model_pieces);
 if (error) {
 fprintf(stderr, "%s\n", error_get_pretty(error));
 error_free(error);
-goto error;
+return -1;
 }
-
-g_strfreev(model_pieces);
 return 0;
-error:
-g_strfreev(model_pieces);
-return -1;
 }
 
 #if !defined(CONFIG_USER_ONLY)
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH v3] sheepdog: multiplex the rw FD to flush cache

2013-01-15 Thread Stefan Hajnoczi
On Tue, Jan 15, 2013 at 04:28:55PM +0800, Liu Yuan wrote:
> From: Liu Yuan 
> 
> This will reduce sockfds connected to the sheep server to one, which simply 
> the
> future hacks.
> 
> Cc: MORITA Kazutaka 
> Cc: Kevin Wolf 
> Cc: Stefan Hajnoczi 
> Signed-off-by: Liu Yuan 
> ---
>  v3: fix gcc warning in add_aio_request()
> 
>  block/sheepdog.c |   82 
> --
>  1 file changed, 37 insertions(+), 45 deletions(-)

Please remember to run scripts/checkpatch.pl before posting patches.  I
fixed up a tab character that snuck into this patch.

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan



Re: [Qemu-devel] [PATCH] build: remove *.lo, *.a, *.la files from all subdirectories on make clean

2013-01-15 Thread Stefan Hajnoczi
On Tue, Jan 15, 2013 at 09:49:37AM +0100, Paolo Bonzini wrote:
> .lo files in stubs/, util/ and libcacard/ were not cleaned.
> Fix this.
> 
> Cc: Blue Swirl 
> Reported-by: Stefan Hajnoczi 
> Signed-off-by: Paolo Bonzini 
> ---
>  Makefile | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi 



Re: [Qemu-devel] [PATCH V3 07/11] block: export function bdrv_find_snapshot()

2013-01-15 Thread Wenchao Xia

于 2013-1-15 7:39, Eric Blake 写道:

On 01/14/2013 12:09 AM, Wenchao Xia wrote:

   This patch move it from savevm.c to block.c and export it.

Signed-off-by: Wenchao Xia 
---
  block.c   |   23 +++
  include/block/block.h |2 ++
  savevm.c  |   22 --
  3 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/block.c b/block.c
index 8192d8e..b7d2f03 100644
--- a/block.c
+++ b/block.c
@@ -3351,6 +3351,29 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
  return -ENOTSUP;
  }

+int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
+   const char *name)
+{
+QEMUSnapshotInfo *sn_tab, *sn;
+int nb_sns, i, ret;
+
+ret = -ENOENT;
+nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+if (nb_sns < 0) {
+return ret;
+}
+for (i = 0; i < nb_sns; i++) {
+sn = &sn_tab[i];
+if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) {


It is possible (albeit probably stupid) to create a qcow2 file where
snapshot names are merely numeric strings.  In fact, just to see what
would happen, I once[1] created a file where:

snapshot id '1' was named '2'
snapshot id '2' was named 'foo'

This code comparison favors ids over names; so if I request to delvm 2,
I end up removing the second snapshot, not the first.  This is okay, but
probably worth documenting, and probably worth making sure that all code
that looks up a snapshot by name or id goes through this function so
that we get the same behavior everywhere.  My experiment was done
several months ago, but my recollection was that at the time, there was
an inconsistency where 'qemu-img snapshot' picked a different snapshot
for the request of '2' than the online 'delvm' monitor command of qemu;
making it unsafe to rely on either behavior in that version of qemu
source code.

[1]https://bugzilla.redhat.com/show_bug.cgi?id=733143



how about:
/*  if id is not NULL, try find it with id, if not exist, return NULL
 *  if id is NULL and name is not NULL, try find it with name.
 */ if id and name is NULL, direct return fail.
int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
   const char *id, const char *name)

--
Best Regards

Wenchao Xia




Re: [Qemu-devel] [PATCH V3 06/11] qmp: add interface query-images.

2013-01-15 Thread Wenchao Xia




When the device doesn't have an image, this returns:

 {
 "device": "ide1-cd0",
 "info": {
 "virtual-size": 0,
 "filename": "",
 "format": ""
 }
 },

But it would be better to return an empty dict or even omit 'info'
completely. One more comment below.



  Will make info as optional, thanks.

+
+return head;
+}
+
  BlockInfo *bdrv_query_block_info(BlockDriverState *bs)
  {
  BlockInfo *info = g_malloc0(sizeof(*info));
diff --git a/qapi-schema.json b/qapi-schema.json
index 5dfa052..565737c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -245,6 +245,22 @@
 '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] 
} }

  ##
+# @DeviceImageInfo:
+#
+# Information about an image used by QEMU block device
+#
+# @device: name of the block device
+#
+# @info: info of the image used.


Info is too generic, please call it 'image'.



 OK.




Re: [Qemu-devel] [PATCH v3] sheepdog: multiplex the rw FD to flush cache

2013-01-15 Thread Liu Yuan
On 01/15/2013 06:20 PM, Stefan Hajnoczi wrote:
> Please remember to run scripts/checkpatch.pl before posting patches.  I
> fixed up a tab character that snuck into this patch.
> 

Ah, Okay, it was in a hasty.

> Thanks, applied to my block tree:
> https://github.com/stefanha/qemu/commits/block

Seems that lost Patch 2/2?

Thanks,
Yuan




[Qemu-devel] [PATCH 05/20] kvm: Pass CPUState to kvm_init_vcpu()

2013-01-15 Thread Andreas Färber
CPUArchState is no longer needed, and it thereby no longer depends on
NEED_CPU_H.

Signed-off-by: Andreas Färber 
---
 cpus.c   |2 +-
 include/sysemu/kvm.h |5 +++--
 kvm-all.c|3 +--
 kvm-stub.c   |2 +-
 4 Dateien geändert, 6 Zeilen hinzugefügt(+), 6 Zeilen entfernt(-)

diff --git a/cpus.c b/cpus.c
index bbb8961..a4390c3 100644
--- a/cpus.c
+++ b/cpus.c
@@ -742,7 +742,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
 cpu->thread_id = qemu_get_thread_id();
 cpu_single_env = env;
 
-r = kvm_init_vcpu(env);
+r = kvm_init_vcpu(cpu);
 if (r < 0) {
 fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r));
 exit(1);
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 3db19ff..2fe8f8a 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -17,6 +17,7 @@
 #include 
 #include "config-host.h"
 #include "qemu/queue.h"
+#include "qom/cpu.h"
 
 #ifdef CONFIG_KVM
 #include 
@@ -120,9 +121,9 @@ int kvm_has_many_ioeventfds(void);
 int kvm_has_gsi_routing(void);
 int kvm_has_intx_set_mask(void);
 
-#ifdef NEED_CPU_H
-int kvm_init_vcpu(CPUArchState *env);
+int kvm_init_vcpu(CPUState *cpu);
 
+#ifdef NEED_CPU_H
 int kvm_cpu_exec(CPUArchState *env);
 
 #if !defined(CONFIG_USER_ONLY)
diff --git a/kvm-all.c b/kvm-all.c
index 4ba77de..6e2164b 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -214,9 +214,8 @@ static void kvm_reset_vcpu(void *opaque)
 kvm_arch_reset_vcpu(cpu);
 }
 
-int kvm_init_vcpu(CPUArchState *env)
+int kvm_init_vcpu(CPUState *cpu)
 {
-CPUState *cpu = ENV_GET_CPU(env);
 KVMState *s = kvm_state;
 long mmap_size;
 int ret;
diff --git a/kvm-stub.c b/kvm-stub.c
index 81f8967..47f8dca 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -24,7 +24,7 @@ bool kvm_irqfds_allowed;
 bool kvm_msi_via_irqfd_allowed;
 bool kvm_gsi_routing_allowed;
 
-int kvm_init_vcpu(CPUArchState *env)
+int kvm_init_vcpu(CPUState *cpu)
 {
 return -ENOSYS;
 }
-- 
1.7.10.4




Re: [Qemu-devel] KVM call for 2013-01-15

2013-01-15 Thread Eduardo Habkost
On Mon, Jan 14, 2013 at 02:26:12PM +0100, Juan Quintela wrote:
> Hi
> 
> Please send in any agenda topics that you have.

- CPU hotplug plan/design

-- 
Eduardo



[Qemu-devel] [PATCH 10/20] target-i386/cpu: Introduce FeatureWord typedefs

2013-01-15 Thread Andreas Färber
From: Eduardo Habkost 

This introduces a FeatureWord enum, FeatureWordInfo struct (with
generation information about a feature word), and a FeatureWordArray
typedef, and changes add_flagname_to_bitmaps() code and
cpu_x86_parse_featurestr() to use the new typedefs instead of separate
variables for each feature word.

This will help us keep the code at kvm_check_features_against_host(),
cpu_x86_parse_featurestr() and add_flagname_to_bitmaps() sane while
adding new feature name arrays.

Signed-off-by: Eduardo Habkost 
Reviewed-by: Gleb Natapov 
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   97 ++---
 target-i386/cpu.h |   15 +
 2 Dateien geändert, 63 Zeilen hinzugefügt(+), 49 Zeilen entfernt(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index cb385fb..e17709c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -124,6 +124,20 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
 NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 };
 
+typedef struct FeatureWordInfo {
+const char **feat_names;
+} FeatureWordInfo;
+
+static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
+[FEAT_1_EDX] = { .feat_names = feature_name },
+[FEAT_1_ECX] = { .feat_names = ext_feature_name },
+[FEAT_8000_0001_EDX] = { .feat_names = ext2_feature_name },
+[FEAT_8000_0001_ECX] = { .feat_names = ext3_feature_name },
+[FEAT_KVM]   = { .feat_names = kvm_feature_name },
+[FEAT_SVM]   = { .feat_names = svm_feature_name },
+[FEAT_7_0_EBX] = { .feat_names = cpuid_7_0_ebx_feature_name },
+};
+
 const char *get_register_name_32(unsigned int reg)
 {
 static const char *reg_names[CPU_NB_REGS32] = {
@@ -271,23 +285,20 @@ static bool lookup_feature(uint32_t *pval, const char *s, 
const char *e,
 return found;
 }
 
-static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,
-uint32_t *ext_features,
-uint32_t *ext2_features,
-uint32_t *ext3_features,
-uint32_t *kvm_features,
-uint32_t *svm_features,
-uint32_t *cpuid_7_0_ebx_features)
+static void add_flagname_to_bitmaps(const char *flagname,
+FeatureWordArray words)
 {
-if (!lookup_feature(features, flagname, NULL, feature_name) &&
-!lookup_feature(ext_features, flagname, NULL, ext_feature_name) &&
-!lookup_feature(ext2_features, flagname, NULL, ext2_feature_name) &&
-!lookup_feature(ext3_features, flagname, NULL, ext3_feature_name) &&
-!lookup_feature(kvm_features, flagname, NULL, kvm_feature_name) &&
-!lookup_feature(svm_features, flagname, NULL, svm_feature_name) &&
-!lookup_feature(cpuid_7_0_ebx_features, flagname, NULL,
-cpuid_7_0_ebx_feature_name))
-fprintf(stderr, "CPU feature %s not found\n", flagname);
+FeatureWord w;
+for (w = 0; w < FEATURE_WORDS; w++) {
+FeatureWordInfo *wi = &feature_word_info[w];
+if (wi->feat_names &&
+lookup_feature(&words[w], flagname, NULL, wi->feat_names)) {
+break;
+}
+}
+if (w == FEATURE_WORDS) {
+fprintf(stderr, "CPU feature %s not found\n", flagname);
+}
 }
 
 typedef struct x86_def_t {
@@ -1283,35 +1294,23 @@ static int cpu_x86_parse_featurestr(x86_def_t 
*x86_cpu_def, char *features)
 unsigned int i;
 char *featurestr; /* Single 'key=value" string being parsed */
 /* Features to be added */
-uint32_t plus_features = 0, plus_ext_features = 0;
-uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
-uint32_t plus_kvm_features = kvm_default_features, plus_svm_features = 0;
-uint32_t plus_7_0_ebx_features = 0;
+FeatureWordArray plus_features = {
+[FEAT_KVM] = kvm_default_features,
+};
 /* Features to be removed */
-uint32_t minus_features = 0, minus_ext_features = 0;
-uint32_t minus_ext2_features = 0, minus_ext3_features = 0;
-uint32_t minus_kvm_features = 0, minus_svm_features = 0;
-uint32_t minus_7_0_ebx_features = 0;
+FeatureWordArray minus_features = { 0 };
 uint32_t numvalue;
 
-add_flagname_to_bitmaps("hypervisor", &plus_features,
-&plus_ext_features, &plus_ext2_features, &plus_ext3_features,
-&plus_kvm_features, &plus_svm_features,  &plus_7_0_ebx_features);
+add_flagname_to_bitmaps("hypervisor", plus_features);
 
 featurestr = features ? strtok(features, ",") : NULL;
 
 while (featurestr) {
 char *val;
 if (featurestr[0] == '+') {
-add_flagname_to_bitmaps(featurestr + 1, &plus_features,
-&plus_ext_features, &plus_ext2_features,
-&plus_ext3_features, &plus_kvm_features,
-&plus_svm_f

[Qemu-devel] [PATCH 06/20] xen: Simplify halting of first CPU

2013-01-15 Thread Andreas Färber
Use the global first_cpu variable to halt the CPU rather than using a
local first_cpu initialized from qemu_get_cpu(0).

This will allow to change qemu_get_cpu() return type to CPUState
despite use of the CPU_COMMON halted field in the reset handler.

Signed-off-by: Andreas Färber 
---
 xen-all.c |4 +---
 1 Datei geändert, 1 Zeile hinzugefügt(+), 3 Zeilen entfernt(-)

diff --git a/xen-all.c b/xen-all.c
index 19bcfd1..110f958 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -585,9 +585,7 @@ static void xen_reset_vcpu(void *opaque)
 
 void xen_vcpu_init(void)
 {
-CPUArchState *first_cpu;
-
-if ((first_cpu = qemu_get_cpu(0))) {
+if (first_cpu != NULL) {
 qemu_register_reset(xen_reset_vcpu, first_cpu);
 xen_reset_vcpu(first_cpu);
 }
-- 
1.7.10.4




[Qemu-devel] [PATCH 03/20] cpu: Move numa_node field to CPUState

2013-01-15 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 cpus.c  |4 +++-
 exec.c  |4 +---
 hw/spapr.c  |4 +++-
 include/exec/cpu-defs.h |1 -
 include/qom/cpu.h   |2 ++
 monitor.c   |4 +++-
 6 Dateien geändert, 12 Zeilen hinzugefügt(+), 7 Zeilen entfernt(-)

diff --git a/cpus.c b/cpus.c
index 6aee15e..d68231a 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1160,12 +1160,14 @@ static void tcg_exec_all(void)
 void set_numa_modes(void)
 {
 CPUArchState *env;
+CPUState *cpu;
 int i;
 
 for (env = first_cpu; env != NULL; env = env->next_cpu) {
+cpu = ENV_GET_CPU(env);
 for (i = 0; i < nb_numa_nodes; i++) {
 if (test_bit(env->cpu_index, node_cpumask[i])) {
-env->numa_node = i;
+cpu->numa_node = i;
 }
 }
 }
diff --git a/exec.c b/exec.c
index 34353f7..de5b27d 100644
--- a/exec.c
+++ b/exec.c
@@ -262,9 +262,7 @@ CPUArchState *qemu_get_cpu(int cpu)
 
 void cpu_exec_init(CPUArchState *env)
 {
-#ifndef CONFIG_USER_ONLY
 CPUState *cpu = ENV_GET_CPU(env);
-#endif
 CPUArchState **penv;
 int cpu_index;
 
@@ -279,7 +277,7 @@ void cpu_exec_init(CPUArchState *env)
 cpu_index++;
 }
 env->cpu_index = cpu_index;
-env->numa_node = 0;
+cpu->numa_node = 0;
 QTAILQ_INIT(&env->breakpoints);
 QTAILQ_INIT(&env->watchpoints);
 #ifndef CONFIG_USER_ONLY
diff --git a/hw/spapr.c b/hw/spapr.c
index b5e15b8..a61c71e 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -140,6 +140,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment 
*spapr)
 {
 int ret = 0, offset;
 CPUPPCState *env;
+CPUState *cpu;
 char cpu_model[32];
 int smt = kvmppc_smt_threads();
 uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
@@ -147,11 +148,12 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment 
*spapr)
 assert(spapr->cpu_model);
 
 for (env = first_cpu; env != NULL; env = env->next_cpu) {
+cpu = ENV_GET_CPU(env);
 uint32_t associativity[] = {cpu_to_be32(0x5),
 cpu_to_be32(0x0),
 cpu_to_be32(0x0),
 cpu_to_be32(0x0),
-cpu_to_be32(env->numa_node),
+cpu_to_be32(cpu->numa_node),
 cpu_to_be32(env->cpu_index)};
 
 if ((env->cpu_index % smt) != 0) {
diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index c02687b..ce178ea 100644
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -195,7 +195,6 @@ typedef struct CPUWatchpoint {
 CPUArchState *next_cpu; /* next CPU sharing TB cache */ \
 int cpu_index; /* CPU index (informative) */\
 uint32_t host_tid; /* host thread ID */ \
-int numa_node; /* NUMA node this cpu is belonging to  */\
 int running; /* Nonzero if cpu is currently running(usermode).  */  \
 /* user data */ \
 void *opaque;   \
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 806b01a..30d1e0c 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -59,6 +59,7 @@ struct kvm_run;
  * CPUState:
  * @nr_cores: Number of cores within this CPU package.
  * @nr_threads: Number of threads within this CPU.
+ * @numa_node: NUMA node this CPU is belonging to.
  * @created: Indicates whether the CPU thread has been successfully created.
  * @stop: Indicates a pending stop request.
  * @stopped: Indicates the CPU has been artificially stopped.
@@ -73,6 +74,7 @@ struct CPUState {
 
 int nr_cores;
 int nr_threads;
+int numa_node;
 
 struct QemuThread *thread;
 #ifdef _WIN32
diff --git a/monitor.c b/monitor.c
index b7ac3a3..016910d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1783,12 +1783,14 @@ static void do_info_numa(Monitor *mon)
 {
 int i;
 CPUArchState *env;
+CPUState *cpu;
 
 monitor_printf(mon, "%d nodes\n", nb_numa_nodes);
 for (i = 0; i < nb_numa_nodes; i++) {
 monitor_printf(mon, "node %d cpus:", i);
 for (env = first_cpu; env != NULL; env = env->next_cpu) {
-if (env->numa_node == i) {
+cpu = ENV_GET_CPU(env);
+if (cpu->numa_node == i) {
 monitor_printf(mon, " %d", env->cpu_index);
 }
 }
-- 
1.7.10.4




[Qemu-devel] [PATCH 18/20] target-i386: Introduce hw_{local, global}_breakpoint_enabled()

2013-01-15 Thread Andreas Färber
From: liguang 

hw_breakpoint_enabled() returned a bit field indicating whether a local
breakpoint and/or global breakpoint was enabled. Avoid this number magic
by using explicit boolean helper functions hw_local_breakpoint_enabled()
and hw_global_breakpoint_enabled(), to aid readability.

Reuse them for the hw_breakpoint_enabled() implementation and change
its return type to bool.

While at it, fix Coding Style issues (missing braces).

Signed-off-by: liguang 
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.h|   15 +--
 target-i386/helper.c |9 ++---
 target-i386/seg_helper.c |3 ++-
 3 Dateien geändert, 21 Zeilen hinzugefügt(+), 6 Zeilen entfernt(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 6682022..1e850a7 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1014,9 +1014,20 @@ int cpu_x86_handle_mmu_fault(CPUX86State *env, 
target_ulong addr,
 #define cpu_handle_mmu_fault cpu_x86_handle_mmu_fault
 void cpu_x86_set_a20(CPUX86State *env, int a20_state);
 
-static inline int hw_breakpoint_enabled(unsigned long dr7, int index)
+static inline bool hw_local_breakpoint_enabled(unsigned long dr7, int index)
 {
-return (dr7 >> (index * 2)) & 3;
+return (dr7 >> (index * 2)) & 1;
+}
+
+static inline bool hw_global_breakpoint_enabled(unsigned long dr7, int index)
+{
+return (dr7 >> (index * 2)) & 2;
+
+}
+static inline bool hw_breakpoint_enabled(unsigned long dr7, int index)
+{
+return hw_global_breakpoint_enabled(dr7, index) ||
+   hw_local_breakpoint_enabled(dr7, index);
 }
 
 static inline int hw_breakpoint_type(unsigned long dr7, int index)
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 1fceb91..ebdd6a5 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -970,9 +970,10 @@ void hw_breakpoint_insert(CPUX86State *env, int index)
 
 switch (hw_breakpoint_type(env->dr[7], index)) {
 case DR7_TYPE_BP_INST:
-if (hw_breakpoint_enabled(env->dr[7], index))
+if (hw_breakpoint_enabled(env->dr[7], index)) {
 err = cpu_breakpoint_insert(env, env->dr[index], BP_CPU,
 &env->cpu_breakpoint[index]);
+}
 break;
 case DR7_TYPE_DATA_WR:
 type = BP_CPU | BP_MEM_WRITE;
@@ -998,8 +999,9 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
 return;
 switch (hw_breakpoint_type(env->dr[7], index)) {
 case DR7_TYPE_BP_INST:
-if (hw_breakpoint_enabled(env->dr[7], index))
+if (hw_breakpoint_enabled(env->dr[7], index)) {
 cpu_breakpoint_remove_by_ref(env, env->cpu_breakpoint[index]);
+}
 break;
 case DR7_TYPE_DATA_WR:
 case DR7_TYPE_DATA_RW:
@@ -1024,8 +1026,9 @@ int check_hw_breakpoints(CPUX86State *env, int 
force_dr6_update)
 ((type & 1) && env->cpu_watchpoint[reg] &&
  (env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT))) {
 dr6 |= 1 << reg;
-if (hw_breakpoint_enabled(env->dr[7], reg))
+if (hw_breakpoint_enabled(env->dr[7], reg)) {
 hit_enabled = 1;
+}
 }
 }
 if (hit_enabled || force_dr6_update)
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index c40bd96..3247dee 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -467,7 +467,8 @@ static void switch_tss(CPUX86State *env, int tss_selector,
 /* reset local breakpoints */
 if (env->dr[7] & DR7_LOCAL_BP_MASK) {
 for (i = 0; i < DR7_MAX_BP; i++) {
-if (hw_breakpoint_enabled(env->dr[7], i) == 0x1) {
+if (hw_local_breakpoint_enabled(env->dr[7], i) &&
+!hw_global_breakpoint_enabled(env->dr[7], i)) {
 hw_breakpoint_remove(env, i);
 }
 }
-- 
1.7.10.4




[Qemu-devel] [PATCH 17/20] target-i386: Define DR7 bit field constants

2013-01-15 Thread Andreas Färber
From: liguang 

Implicit use of dr7 bit field is a little hard to understand,
so define constants for them and use them consistently.

Signed-off-by: liguang 
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.h |6 ++
 target-i386/helper.c  |   18 +-
 target-i386/machine.c |5 +++--
 target-i386/misc_helper.c |4 ++--
 target-i386/seg_helper.c  |6 +++---
 5 Dateien geändert, 23 Zeilen hinzugefügt(+), 16 Zeilen entfernt(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index e4a7c50..6682022 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -231,6 +231,12 @@
 #define DR7_TYPE_SHIFT  16
 #define DR7_LEN_SHIFT   18
 #define DR7_FIXED_1 0x0400
+#define DR7_LOCAL_BP_MASK0x55
+#define DR7_MAX_BP   4
+#define DR7_TYPE_BP_INST 0x0
+#define DR7_TYPE_DATA_WR 0x1
+#define DR7_TYPE_IO_RW   0x2
+#define DR7_TYPE_DATA_RW 0x3
 
 #define PG_PRESENT_BIT 0
 #define PG_RW_BIT  1
diff --git a/target-i386/helper.c b/target-i386/helper.c
index fa622e1..1fceb91 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -969,18 +969,18 @@ void hw_breakpoint_insert(CPUX86State *env, int index)
 int type, err = 0;
 
 switch (hw_breakpoint_type(env->dr[7], index)) {
-case 0:
+case DR7_TYPE_BP_INST:
 if (hw_breakpoint_enabled(env->dr[7], index))
 err = cpu_breakpoint_insert(env, env->dr[index], BP_CPU,
 &env->cpu_breakpoint[index]);
 break;
-case 1:
+case DR7_TYPE_DATA_WR:
 type = BP_CPU | BP_MEM_WRITE;
 goto insert_wp;
-case 2:
+case DR7_TYPE_IO_RW:
  /* No support for I/O watchpoints yet */
 break;
-case 3:
+case DR7_TYPE_DATA_RW:
 type = BP_CPU | BP_MEM_ACCESS;
 insert_wp:
 err = cpu_watchpoint_insert(env, env->dr[index],
@@ -997,15 +997,15 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
 if (!env->cpu_breakpoint[index])
 return;
 switch (hw_breakpoint_type(env->dr[7], index)) {
-case 0:
+case DR7_TYPE_BP_INST:
 if (hw_breakpoint_enabled(env->dr[7], index))
 cpu_breakpoint_remove_by_ref(env, env->cpu_breakpoint[index]);
 break;
-case 1:
-case 3:
+case DR7_TYPE_DATA_WR:
+case DR7_TYPE_DATA_RW:
 cpu_watchpoint_remove_by_ref(env, env->cpu_watchpoint[index]);
 break;
-case 2:
+case DR7_TYPE_IO_RW:
 /* No support for I/O watchpoints yet */
 break;
 }
@@ -1018,7 +1018,7 @@ int check_hw_breakpoints(CPUX86State *env, int 
force_dr6_update)
 int hit_enabled = 0;
 
 dr6 = env->dr[6] & ~0xf;
-for (reg = 0; reg < 4; reg++) {
+for (reg = 0; reg < DR7_MAX_BP; reg++) {
 type = hw_breakpoint_type(env->dr[7], reg);
 if ((type == 0 && env->dr[reg] == env->eip) ||
 ((type & 1) && env->cpu_watchpoint[reg] &&
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 8354572..8df6a6b 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -265,10 +265,11 @@ static int cpu_post_load(void *opaque, int version_id)
 
 cpu_breakpoint_remove_all(env, BP_CPU);
 cpu_watchpoint_remove_all(env, BP_CPU);
-for (i = 0; i < 4; i++)
+for (i = 0; i < DR7_MAX_BP; i++) {
 hw_breakpoint_insert(env, i);
-
+}
 tlb_flush(env, 1);
+
 return 0;
 }
 
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index 719cacd..b3f4e4f 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c
@@ -197,11 +197,11 @@ void helper_movl_drN_T0(CPUX86State *env, int reg, 
target_ulong t0)
 env->dr[reg] = t0;
 hw_breakpoint_insert(env, reg);
 } else if (reg == 7) {
-for (i = 0; i < 4; i++) {
+for (i = 0; i < DR7_MAX_BP; i++) {
 hw_breakpoint_remove(env, i);
 }
 env->dr[7] = t0;
-for (i = 0; i < 4; i++) {
+for (i = 0; i < DR7_MAX_BP; i++) {
 hw_breakpoint_insert(env, i);
 }
 } else {
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index c2a99ee..c40bd96 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -465,13 +465,13 @@ static void switch_tss(CPUX86State *env, int tss_selector,
 
 #ifndef CONFIG_USER_ONLY
 /* reset local breakpoints */
-if (env->dr[7] & 0x55) {
-for (i = 0; i < 4; i++) {
+if (env->dr[7] & DR7_LOCAL_BP_MASK) {
+for (i = 0; i < DR7_MAX_BP; i++) {
 if (hw_breakpoint_enabled(env->dr[7], i) == 0x1) {
 hw_breakpoint_remove(env, i);
 }
 }
-env->dr[7] &= ~0x55;
+env->dr[7] &= ~DR7_LOCAL_BP_MASK;
 }
 #endif
 }
-- 
1.7.10.4




[Qemu-devel] [PATCH 14/20] target-i386: Move setting defaults out of cpu_x86_parse_featurestr()

2013-01-15 Thread Andreas Färber
From: Igor Mammedov 

No functional change, needed for simplifying conversion to properties.

Signed-off-by: Igor Mammedov 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |9 -
 1 Datei geändert, 4 Zeilen hinzugefügt(+), 5 Zeilen entfernt(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9a48e3f..e75b293 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1343,15 +1343,11 @@ static int cpu_x86_parse_featurestr(x86_def_t 
*x86_cpu_def, char *features)
 unsigned int i;
 char *featurestr; /* Single 'key=value" string being parsed */
 /* Features to be added */
-FeatureWordArray plus_features = {
-[FEAT_KVM] = kvm_default_features,
-};
+FeatureWordArray plus_features = { 0 };
 /* Features to be removed */
 FeatureWordArray minus_features = { 0 };
 uint32_t numvalue;
 
-add_flagname_to_bitmaps("hypervisor", plus_features);
-
 featurestr = features ? strtok(features, ",") : NULL;
 
 while (featurestr) {
@@ -1607,6 +1603,9 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 goto error;
 }
 
+def->kvm_features |= kvm_default_features;
+def->ext_features |= CPUID_EXT_HYPERVISOR;
+
 if (cpu_x86_parse_featurestr(def, features) < 0) {
 goto error;
 }
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH 05/17] target-i386: replace uint32_t vendor fields by vendor string in x86_def_t

2013-01-15 Thread Eduardo Habkost
On Mon, Jan 14, 2013 at 10:52:52PM +0100, Igor Mammedov wrote:
[...]
> > > @@ -987,9 +937,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
> > >  x86_cpu_def->vendor_override = 0;
> > >  
> > >  /* Call Centaur's CPUID instruction. */
> > > -if (x86_cpu_def->vendor1 == CPUID_VENDOR_VIA_1 &&
> > > -x86_cpu_def->vendor2 == CPUID_VENDOR_VIA_2 &&
> > > -x86_cpu_def->vendor3 == CPUID_VENDOR_VIA_3) {
> > > +if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) {
> > 
> > I'd be more comfortable doing such comparisons with a length or, even
> > better, using a static inline helper doing so: In this case
> > ("CentaurHauls") it happens to work but I'm worried about setting a bad
> > example that gets copied-and-pasted.
> x86_cpu_vendor_words2str() guaranties that x86_cpu_def->vendor is nill
> terminated string, would you prefer:
> 
>  if (!strncmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA, 
> sizeof(x86_cpu_def->vendor))) 
> 
> instead?

I believe Andreas is worrying in case there are NUL characters in the
middle of the vendor info. e.g., this wouldn't work:

  #define CPUID_VENDOR_FOOBAR "foo\0bar\0baz\0".
  [...]
  if (!strncmp(x86_cpu_def->vendor, CPUID_VENDOR_FOOBAR, 
sizeof(x86_cpu_def->vendor))) 
  [...]

It's true that CPU vendors can do whatever they want and return anything
on the CPUID vendor leaf, but I believe this is never going to happen
because vendors know it would be a bad idea to do that. And if it
happens one day, we can easily add additional vendor1/vendor2/vendor3
properties to allow low-level setting of the vendor ID bytes.

[...]
> > > @@ -1616,10 +1552,8 @@ int cpu_x86_register(X86CPU *cpu, const char 
> > > *cpu_model)
> > >  error_setg(&error, "Invalid cpu_model string format: %s", 
> > > cpu_model);
> > >  goto out;
> > >  }
> > > -assert(def->vendor1);
> > > -env->cpuid_vendor1 = def->vendor1;
> > > -env->cpuid_vendor2 = def->vendor2;
> > > -env->cpuid_vendor3 = def->vendor3;
> > > +assert(def->vendor[0]);
> > 
> > Instead of asserting on an empty string here, we should set the Error*
> > inside the property setter.
> x86_cpuid_set_vendor() you've added some time ago, already checks for value
> to be 12 characters exactly. If it is not, it sets *Error
> Reason I've kept assert here is that after patch vendor in this place would
> represent built-in vendor value, so it would be easier to detect invalid
> default value by aborting here (it wouldn't be valid for cpu sub-classes
> though).
> But this check is really redundant, would you like it to be dropped? 

Well, every assert is _supposed_ to be redundant, right? I mean: we only
add assert()s to the code when we are already confident that other parts
of the code make sure the assert is never going to fail. I would like to
keep it.

-- 
Eduardo



[Qemu-devel] [PATCH 12/20] target-i386/cpu.c: Add feature name array for ext4_features

2013-01-15 Thread Andreas Färber
From: Eduardo Habkost 

Feature names were taken from the X86_FEATURE_* constants in the Linux
kernel code.

Signed-off-by: Eduardo Habkost 
Reviewed-by: Gleb Natapov 
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   17 +
 1 Datei geändert, 17 Zeilen hinzugefügt(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0e531f9..8ec9929 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -95,6 +95,17 @@ static const char *ext3_feature_name[] = {
 NULL, NULL, NULL, NULL,
 };
 
+static const char *ext4_feature_name[] = {
+NULL, NULL, "xstore", "xstore-en",
+NULL, NULL, "xcrypt", "xcrypt-en",
+"ace2", "ace2-en", "phe", "phe-en",
+"pmm", "pmm-en", NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+};
+
 static const char *kvm_feature_name[] = {
 "kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvmclock",
 "kvm_asyncpf", "kvm_steal_time", "kvm_pv_eoi", NULL,
@@ -147,6 +158,10 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 .feat_names = ext3_feature_name,
 .cpuid_eax = 0x8001, .cpuid_reg = R_ECX,
 },
+[FEAT_C000_0001_EDX] = {
+.feat_names = ext4_feature_name,
+.cpuid_eax = 0xC001, .cpuid_reg = R_EDX,
+},
 [FEAT_KVM] = {
 .feat_names = kvm_feature_name,
 .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX,
@@ -1439,6 +1454,7 @@ static int cpu_x86_parse_featurestr(x86_def_t 
*x86_cpu_def, char *features)
 x86_cpu_def->ext_features |= plus_features[FEAT_1_ECX];
 x86_cpu_def->ext2_features |= plus_features[FEAT_8000_0001_EDX];
 x86_cpu_def->ext3_features |= plus_features[FEAT_8000_0001_ECX];
+x86_cpu_def->ext4_features |= plus_features[FEAT_C000_0001_EDX];
 x86_cpu_def->kvm_features |= plus_features[FEAT_KVM];
 x86_cpu_def->svm_features |= plus_features[FEAT_SVM];
 x86_cpu_def->cpuid_7_0_ebx_features |= plus_features[FEAT_7_0_EBX];
@@ -1446,6 +1462,7 @@ static int cpu_x86_parse_featurestr(x86_def_t 
*x86_cpu_def, char *features)
 x86_cpu_def->ext_features &= ~minus_features[FEAT_1_ECX];
 x86_cpu_def->ext2_features &= ~minus_features[FEAT_8000_0001_EDX];
 x86_cpu_def->ext3_features &= ~minus_features[FEAT_8000_0001_ECX];
+x86_cpu_def->ext4_features &= ~minus_features[FEAT_C000_0001_EDX];
 x86_cpu_def->kvm_features &= ~minus_features[FEAT_KVM];
 x86_cpu_def->svm_features &= ~minus_features[FEAT_SVM];
 x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_features[FEAT_7_0_EBX];
-- 
1.7.10.4




[Qemu-devel] [PATCH 01/20] cpu: Move nr_{cores, threads} fields to CPUState

2013-01-15 Thread Andreas Färber
To facilitate the field movements, pass MIPSCPU to malta_mips_config();
avoid that for mips_cpu_map_tc() since callers only access MIPS Thread
Contexts, inside TCG helpers.

Signed-off-by: Andreas Färber 
---
 cpus.c  |4 ++--
 hw/mips_malta.c |9 ++---
 include/exec/cpu-defs.h |2 --
 include/qom/cpu.h   |5 +
 target-i386/cpu.c   |   18 +-
 target-mips/op_helper.c |8 +---
 6 Dateien geändert, 27 Zeilen hinzugefügt(+), 19 Zeilen entfernt(-)

diff --git a/cpus.c b/cpus.c
index 4a7782a..6aee15e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1041,8 +1041,8 @@ void qemu_init_vcpu(void *_env)
 CPUArchState *env = _env;
 CPUState *cpu = ENV_GET_CPU(env);
 
-env->nr_cores = smp_cores;
-env->nr_threads = smp_threads;
+cpu->nr_cores = smp_cores;
+cpu->nr_threads = smp_threads;
 cpu->stopped = true;
 if (kvm_enabled()) {
 qemu_kvm_start_vcpu(env);
diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index 2250e67..771d125 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -743,10 +743,13 @@ static int64_t load_kernel (void)
 return kernel_entry;
 }
 
-static void malta_mips_config(CPUMIPSState *env)
+static void malta_mips_config(MIPSCPU *cpu)
 {
+CPUMIPSState *env = &cpu->env;
+CPUState *cs = CPU(cpu);
+
 env->mvp->CP0_MVPConf0 |= ((smp_cpus - 1) << CP0MVPC0_PVPE) |
- ((smp_cpus * env->nr_threads - 1) << CP0MVPC0_PTC);
+ ((smp_cpus * cs->nr_threads - 1) << CP0MVPC0_PTC);
 }
 
 static void main_cpu_reset(void *opaque)
@@ -763,7 +766,7 @@ static void main_cpu_reset(void *opaque)
 env->CP0_Status &= ~((1 << CP0St_BEV) | (1 << CP0St_ERL));
 }
 
-malta_mips_config(env);
+malta_mips_config(cpu);
 }
 
 static void cpu_request_exit(void *opaque, int irq, int level)
diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index b22b4c6..c02687b 100644
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -196,8 +196,6 @@ typedef struct CPUWatchpoint {
 int cpu_index; /* CPU index (informative) */\
 uint32_t host_tid; /* host thread ID */ \
 int numa_node; /* NUMA node this cpu is belonging to  */\
-int nr_cores;  /* number of cores within this CPU package */\
-int nr_threads;/* number of threads within this CPU */  \
 int running; /* Nonzero if cpu is currently running(usermode).  */  \
 /* user data */ \
 void *opaque;   \
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index fbacb27..806b01a 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -57,6 +57,8 @@ struct kvm_run;
 
 /**
  * CPUState:
+ * @nr_cores: Number of cores within this CPU package.
+ * @nr_threads: Number of threads within this CPU.
  * @created: Indicates whether the CPU thread has been successfully created.
  * @stop: Indicates a pending stop request.
  * @stopped: Indicates the CPU has been artificially stopped.
@@ -69,6 +71,9 @@ struct CPUState {
 DeviceState parent_obj;
 /*< public >*/
 
+int nr_cores;
+int nr_threads;
+
 struct QemuThread *thread;
 #ifdef _WIN32
 HANDLE hThread;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 78bd61e..9f98a41 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1691,8 +1691,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 *ebx = (env->cpuid_apic_id << 24) | 8 << 8; /* CLFLUSH size in quad 
words, Linux wants it. */
 *ecx = env->cpuid_ext_features;
 *edx = env->cpuid_features;
-if (env->nr_cores * env->nr_threads > 1) {
-*ebx |= (env->nr_cores * env->nr_threads) << 16;
+if (cs->nr_cores * cs->nr_threads > 1) {
+*ebx |= (cs->nr_cores * cs->nr_threads) << 16;
 *edx |= 1 << 28;/* HTT bit */
 }
 break;
@@ -1705,8 +1705,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 break;
 case 4:
 /* cache info: needed for Core compatibility */
-if (env->nr_cores > 1) {
-*eax = (env->nr_cores - 1) << 26;
+if (cs->nr_cores > 1) {
+*eax = (cs->nr_cores - 1) << 26;
 } else {
 *eax = 0;
 }
@@ -1725,8 +1725,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 break;
 case 2: /* L2 cache info */
 *eax |= 0x143;
-if (env->nr_threads > 1) {
-*eax |= (env->nr_threads - 1) << 14;
+if (cs->nr_threads > 1) {
+*eax |= (cs->nr_threads - 1) << 14;
 }
 *ebx = 0x3c0003f;
 *ecx = 0xfff;
@@ -1830,7 +1830,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint

[Qemu-devel] [PATCH 02/20] target-mips: Clean up mips_cpu_map_tc() documentation

2013-01-15 Thread Andreas Färber
This function will be touched again soon, so a good understanding of env
vs. other helps. Adopt gtk-doc style.

Signed-off-by: Andreas Färber 
Reviewed-by: Eric Johnson 
---
 target-mips/op_helper.c |   14 +-
 1 Datei geändert, 9 Zeilen hinzugefügt(+), 5 Zeilen entfernt(-)

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index fb63d9e..1816a0e 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -572,11 +572,15 @@ static inline void mips_tc_sleep(MIPSCPU *cpu, int tc)
 }
 }
 
-/* tc should point to an int with the value of the global TC index.
-   This function will transform it into a local index within the
-   returned CPUMIPSState.
-
-   FIXME: This code assumes that all VPEs have the same number of TCs,
+/**
+ * mips_cpu_map_tc:
+ * @env: CPU from which mapping is performed.
+ * @tc: Should point to an int with the value of the global TC index.
+ *
+ * This function will transform @tc into a local index within the
+ * returned #CPUMIPSState.
+ */
+/* FIXME: This code assumes that all VPEs have the same number of TCs,
   which depends on runtime setup. Can probably be fixed by
   walking the list of CPUMIPSStates.  */
 static CPUMIPSState *mips_cpu_map_tc(CPUMIPSState *env, int *tc)
-- 
1.7.10.4




[Qemu-devel] [PATCH 04/20] cpu: Move cpu_index field to CPUState

2013-01-15 Thread Andreas Färber
Note that target-alpha accesses this field from TCG, now using a
negative offset. Therefore the field is placed last in CPUState.

Pass PowerPCCPU to [kvm]ppc_fixup_cpu() to facilitate this change.

Move common parts of mips cpu_state_reset() to mips_cpu_reset().

Acked-by: Richard Henderson  (for alpha)
[AF: Rebased onto ppc CPU subclasses and openpic changes]
Signed-off-by: Andreas Färber 
---
 cpus.c  |   14 +-
 exec.c  |   13 +++--
 gdbstub.c   |3 ++-
 hw/alpha_typhoon.c  |4 +++-
 hw/arm_gic.c|3 ++-
 hw/arm_mptimer.c|8 +---
 hw/openpic.c|5 -
 hw/ppc/e500.c   |   17 +++--
 hw/ppce500_spin.c   |8 +---
 hw/pxa.h|2 +-
 hw/pxa2xx.c |4 ++--
 hw/pxa2xx_gpio.c|5 +++--
 hw/spapr.c  |   11 ++-
 hw/spapr_hcall.c|4 +++-
 hw/spapr_rtas.c |8 +---
 hw/xics.c   |   22 --
 include/exec/cpu-defs.h |1 -
 include/exec/gdbstub.h  |3 ++-
 include/qom/cpu.h   |2 ++
 kvm-all.c   |2 +-
 monitor.c   |   15 ++-
 target-alpha/translate.c|2 +-
 target-arm/cpu.c|2 +-
 target-arm/helper.c |3 ++-
 target-cris/cpu.c   |2 +-
 target-i386/cpu.c   |7 ---
 target-i386/helper.c|   15 ---
 target-i386/misc_helper.c   |5 -
 target-lm32/cpu.c   |2 +-
 target-m68k/cpu.c   |2 +-
 target-microblaze/cpu.c |2 +-
 target-mips/cpu.c   |8 
 target-mips/translate.c |   17 +++--
 target-openrisc/cpu.c   |2 +-
 target-ppc/kvm.c|   12 +++-
 target-ppc/kvm_ppc.h|4 ++--
 target-ppc/translate_init.c |   10 ++
 target-s390x/cpu.c  |2 +-
 target-sh4/cpu.c|2 +-
 target-sparc/cpu.c  |2 +-
 40 Dateien geändert, 153 Zeilen hinzugefügt(+), 102 Zeilen entfernt(-)

diff --git a/cpus.c b/cpus.c
index d68231a..bbb8961 100644
--- a/cpus.c
+++ b/cpus.c
@@ -390,13 +390,15 @@ void hw_error(const char *fmt, ...)
 {
 va_list ap;
 CPUArchState *env;
+CPUState *cpu;
 
 va_start(ap, fmt);
 fprintf(stderr, "qemu: hardware error: ");
 vfprintf(stderr, fmt, ap);
 fprintf(stderr, "\n");
-for(env = first_cpu; env != NULL; env = env->next_cpu) {
-fprintf(stderr, "CPU #%d:\n", env->cpu_index);
+for (env = first_cpu; env != NULL; env = env->next_cpu) {
+cpu = ENV_GET_CPU(env);
+fprintf(stderr, "CPU #%d:\n", cpu->cpu_index);
 cpu_dump_state(env, stderr, fprintf, CPU_DUMP_FPU);
 }
 va_end(ap);
@@ -1166,7 +1168,7 @@ void set_numa_modes(void)
 for (env = first_cpu; env != NULL; env = env->next_cpu) {
 cpu = ENV_GET_CPU(env);
 for (i = 0; i < nb_numa_nodes; i++) {
-if (test_bit(env->cpu_index, node_cpumask[i])) {
+if (test_bit(cpu->cpu_index, node_cpumask[i])) {
 cpu->numa_node = i;
 }
 }
@@ -1215,7 +1217,7 @@ CpuInfoList *qmp_query_cpus(Error **errp)
 
 info = g_malloc0(sizeof(*info));
 info->value = g_malloc0(sizeof(*info->value));
-info->value->CPU = env->cpu_index;
+info->value->CPU = cpu->cpu_index;
 info->value->current = (env == first_cpu);
 info->value->halted = env->halted;
 info->value->thread_id = cpu->thread_id;
@@ -1253,6 +1255,7 @@ void qmp_memsave(int64_t addr, int64_t size, const char 
*filename,
 FILE *f;
 uint32_t l;
 CPUArchState *env;
+CPUState *cpu;
 uint8_t buf[1024];
 
 if (!has_cpu) {
@@ -1260,7 +1263,8 @@ void qmp_memsave(int64_t addr, int64_t size, const char 
*filename,
 }
 
 for (env = first_cpu; env; env = env->next_cpu) {
-if (cpu_index == env->cpu_index) {
+cpu = ENV_GET_CPU(env);
+if (cpu_index == cpu->cpu_index) {
 break;
 }
 }
diff --git a/exec.c b/exec.c
index de5b27d..e5265e6 100644
--- a/exec.c
+++ b/exec.c
@@ -247,13 +247,16 @@ static const VMStateDescription vmstate_cpu_common = {
 };
 #endif
 
-CPUArchState *qemu_get_cpu(int cpu)
+CPUArchState *qemu_get_cpu(int index)
 {
 CPUArchState *env = first_cpu;
+CPUState *cpu;
 
 while (env) {
-if (env->cpu_index == cpu)
+cpu = ENV_GET_CPU(env);
+if (cpu->cpu_index == index) {
 break;
+}
 env = env->next_cpu;
 }
 
@@ -276,7 +279,7 @@ void cpu_exec_init(CPUArchState *env)
 penv = &(*penv)->next_cpu;
 cpu_index++;
 }
-env->cpu_index = cpu_index;
+cpu->cpu_index = cpu_index;
 cpu->numa_node = 0;
 QTAILQ_INIT(&env->breakpoints);
 QTAILQ_INIT(&env->watchpoint

[Qemu-devel] [PATCH 09/20] target-i386: Disable kvm_mmu by default

2013-01-15 Thread Andreas Färber
From: Eduardo Habkost 

KVM_CAP_PV_MMU capability reporting was removed from the kernel since
v2.6.33 (see commit a68a6a7282373), and was completely removed from the
kernel since v3.3 (see commit fb92045843). It doesn't make sense to keep
it enabled by default, as it would cause unnecessary hassle when using
the "enforce" flag.

This disables kvm_mmu on all machine-types. With this fix, the possible
scenarios when migrating from QEMU <= 1.3 to QEMU 1.4 are:

+--+
 src kernel | dst kern.| Result
+--+
 >= 2.6.33  | any  | kvm_mmu was already disabled and will stay disabled
 <= 2.6.32  | >= 3.3   | correct live migration is impossible
 <= 2.6.32  | <= 3.2   | kvm_mmu will be disabled on next guest reboot *
+--+

 * If they are running kernel <= 2.6.32 and want kvm_mmu to be kept
   enabled on guest reboot, they can explicitly add +kvm_mmu to the QEMU
   command-line. Using 2.6.33 and higher, it is not possible to enable
   kvm_mmu explicitly anymore.

Signed-off-by: Eduardo Habkost 
Reviewed-by: Gleb Natapov 
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |1 -
 1 Datei geändert, 1 Zeile entfernt(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 992b614..cb385fb 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -159,7 +159,6 @@ int enforce_cpuid = 0;
 #if defined(CONFIG_KVM)
 static uint32_t kvm_default_features = (1 << KVM_FEATURE_CLOCKSOURCE) |
 (1 << KVM_FEATURE_NOP_IO_DELAY) |
-(1 << KVM_FEATURE_MMU_OP) |
 (1 << KVM_FEATURE_CLOCKSOURCE2) |
 (1 << KVM_FEATURE_ASYNC_PF) |
 (1 << KVM_FEATURE_STEAL_TIME) |
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH 06/20] xen: Simplify halting of first CPU

2013-01-15 Thread Stefano Stabellini
On Tue, 15 Jan 2013, Andreas Färber wrote:
> Use the global first_cpu variable to halt the CPU rather than using a
> local first_cpu initialized from qemu_get_cpu(0).
> 
> This will allow to change qemu_get_cpu() return type to CPUState
> despite use of the CPU_COMMON halted field in the reset handler.
> 
> Signed-off-by: Andreas Färber 

Acked-by: Stefano Stabellini 

>  xen-all.c |4 +---
>  1 Datei geändert, 1 Zeile hinzugefügt(+), 3 Zeilen entfernt(-)
> 
> diff --git a/xen-all.c b/xen-all.c
> index 19bcfd1..110f958 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -585,9 +585,7 @@ static void xen_reset_vcpu(void *opaque)
>  
>  void xen_vcpu_init(void)
>  {
> -CPUArchState *first_cpu;
> -
> -if ((first_cpu = qemu_get_cpu(0))) {
> +if (first_cpu != NULL) {
>  qemu_register_reset(xen_reset_vcpu, first_cpu);
>  xen_reset_vcpu(first_cpu);
>  }
> -- 
> 1.7.10.4
> 
> 

Re: [Qemu-devel] [PATCH V3 11/11] hmp: show snapshot on single block device

2013-01-15 Thread Luiz Capitulino
On Tue, 15 Jan 2013 10:36:22 +0800
Wenchao Xia  wrote:

>  > On Mon, 2013-01-14 at 15:09 +0800, Wenchao Xia wrote:
> >>This patch use block layer API to qmp snapshot info on a block
> >> device, then use the same code dumping vm snapshot info, to print
> >> in monitor.
> >>
> >> Signed-off-by: Wenchao Xia 
> >> ---
> >> Note:
> >>This patch need previous hmp extention patch which enable
> >> info sub command take qdict * as paramter.
> >
> >> diff --git a/savevm.c b/savevm.c
> >> index cabdcb6..cd474e9 100644
> >> --- a/savevm.c
> >> +++ b/savevm.c
> >> @@ -2354,9 +2354,50 @@ static void do_info_snapshots_vm(Monitor *mon)
> >>   return;
> >>   }
> >>
> >> +static void do_info_snapshots_blk(Monitor *mon, const char *device)
> >> +{
> >> +Error *err = NULL;
> >> +SnapshotInfoList *list;
> >> +BlockDriverState *bs;
> >> +
> >> +/* find the target bs */
> >> +bs = bdrv_find(device);
> >> +if (!bs) {
> >> +monitor_printf(mon, "Device '%s' not found.\n", device);
> >> +return ;
> >> +}
> >> +
> >> +if (!bdrv_can_snapshot(bs)) {
> >> +monitor_printf(mon, "Device '%s' can't have snapshot.\n", device);
> >> +return ;
> >> +}
> >> +
> >> +list = bdrv_query_snapshot_infolist(bs, NULL, NULL, &err);
> >> +if (error_is_set(&err)) {
> >> +hmp_handle_error(mon, &err);
> >> +return;
> >> +}
> >> +
> >> +if (list == NULL) {
> >> +monitor_printf(mon, "There is no snapshot available.\n");
> >> +return;
> >> +}
> >> +
> >> +monitor_printf(mon, "Device '%s':\n", device);
> >> +monitor_dump_snapshotinfolist(mon, list);
> >> +qapi_free_SnapshotInfoList(list);
> >> +return;
> >> +}
> >> +
> >>   void do_info_snapshots(Monitor *mon, const QDict *qdict)
> >>   {
> >> -do_info_snapshots_vm(mon);
> >> +const char *device = qdict_get_try_str(qdict, "device");
> >> +if (!device) {
> >> +do_info_snapshots_vm(mon);
> >> +} else {
> >> +do_info_snapshots_blk(mon, device);
> >> +}
> >> +return;
> >>   }
> >>
> >
> > I think that you should move these functions into hmp.c file. This also
> > applies to previous patch and according to this changes you don't have
> > to export hmp_handle_error() function which should be used only in hmp.c
> >
> > Pavel
> >
>It seems there are other "do_info_**" function in other .c files,
> so I suggest a different serial later which move those functions, if
> necessary.

The other functions are probably old hmp code. That is, code that is not
converted to make QMP calls. Generally, new hmp code goes into hmp.c unless
there's a good reason to put them in a different file.

So, please move it to hmp.c.



Re: [Qemu-devel] [PATCH V10 1/4] add def_print_str and use it in qemu_opts_print.

2013-01-15 Thread Kevin Wolf
Am 07.01.2013 06:26, schrieb Dong Xu Wang:
> qemu_opts_print has no user now, so can re-write the function safely.
> 
> qemu_opts_print will be used while using "qemu-img create", it will
> produce the same output as previous code.
> 
> The behavior of this function has changed:
> 
> 1. Print every possible option, whether a value has been set or not.
> 2. Option descriptors may provide a default value.
> 3. Print to stdout instead of stderr.
> 
> Previously the behavior was to print every option that has been set.
> Options that have not been set would be skipped.
> 
> Signed-off-by: Dong Xu Wang 

The subject line hasn't been updated when you changed the naem of
"def_print_str".

> ---
> v7->v8:
> 1) print "elements => accept any params" while opts_accepts_any() ==
> true.
> 2) since def_print_str is the default value if an option isn't set,
> so rename it to def_value_str.

It's not really, it still only influences printing, not what values you
get. The idea was that you say, for example:

{
.name = BLOCK_OPT_CLUSTER_SIZE,
.type = QEMU_OPT_SIZE,
.help = "qcow2 cluster size",
.def_value_str = stringify(DEFAULT_CLUSTER_SIZE)
},

And then this would actually be enough for the default to be applied.
That is, you would only write

cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE);

instead of passing the default a second time like you do now:

cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE,
 DEFAULT_CLUSTER_SIZE);

We can however add patches to make it a real default value on top of
this series, so it doesn't necessarily mean that we can't commit it as
it is.

> 
>  include/qemu/option.h |1 +
>  qemu-option.c |   31 ---
>  2 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/include/qemu/option.h b/include/qemu/option.h
> index ba197cd..394170a 100644
> --- a/include/qemu/option.h
> +++ b/include/qemu/option.h
> @@ -96,6 +96,7 @@ typedef struct QemuOptDesc {
>  const char *name;
>  enum QemuOptType type;
>  const char *help;
> +const char *def_value_str;
>  } QemuOptDesc;
>  
>  struct QemuOptsList {
> diff --git a/qemu-option.c b/qemu-option.c
> index f532b76..6f19fd3 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -862,15 +862,32 @@ void qemu_opts_del(QemuOpts *opts)
>  
>  int qemu_opts_print(QemuOpts *opts, void *dummy)
>  {
> -QemuOpt *opt;
> +QemuOptDesc *desc = opts->list->desc;
>  
> -fprintf(stderr, "%s: %s:", opts->list->name,
> -opts->id ? opts->id : "");
> -QTAILQ_FOREACH(opt, &opts->head, next) {
> -fprintf(stderr, " %s=\"%s\"", opt->name, opt->str);
> +if (desc[0].name == NULL) {
> +printf("no elements => accept any params");
> +return 0;
>  }

Wouldn't it make more sense to keep the old behaviour for accept-any
cases? That is, print all values that have actually been assigned?

Kevin



Re: [Qemu-devel] Qemu s390x emulation

2013-01-15 Thread Alexander Graf

On 15.01.2013, at 12:05, Suzuki K. Poulose wrote:

> Hi
> 
> I have been trying to setup a qemu session for qemu-system-s390x (on
> x86_64) using a kernel (with initramfs built-in the kernel) without a
> disk image. The kernel was built with s390 defconfig + disabled loadable
> modules (just to keep everything inside the kernel).
> 
> $ qemu-system-s390x -M s390 -kernel vmlinux -m 1024
> 
> 
> The session dies in say 2 secs, with an exit code of 0. I searched for
> some hints / success stories, couldn't find any.
> 
> Am I doing something wrong here ? Please let me know the right procedure
> for getting this up and running.

S390 boots using an "image" file. Please try -kernel /arch/s390/boot/image.


Alex




Re: [Qemu-devel] [PATCH V3 01/11] qemu-img: remove unused parameter in collect_image_info()

2013-01-15 Thread Luiz Capitulino
On Tue, 15 Jan 2013 15:58:34 +0800
Wenchao Xia  wrote:

> 于 2013-1-15 15:27, Wenchao Xia 写道:
> > 于 2013-1-15 1:08, Luiz Capitulino 写道:
> >> On Mon, 14 Jan 2013 15:09:37 +0800
> >> Wenchao Xia  wrote:
> >>
> >>>Parameter *fmt was not used, so remove it.
> >>>
> >>> Reviewed-by: Eric Blake 
> >>> Signed-off-by: Wenchao Xia 
> >>> ---
> >>>   qemu-img.c |5 ++---
> >>>   1 files changed, 2 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/qemu-img.c b/qemu-img.c
> >>> index 85d3740..9dab48f 100644
> >>> --- a/qemu-img.c
> >>> +++ b/qemu-img.c
> >>> @@ -1186,8 +1186,7 @@ static void dump_json_image_info(ImageInfo *info)
> >>>
> >>>   static void collect_image_info(BlockDriverState *bs,
> >>>  ImageInfo *info,
> >>> -   const char *filename,
> >>> -   const char *fmt)
> >>> +   const char *filename)
> >>
> >> collect_image_info_list() doc reads:
> >>
> >>   @fmt: topmost image format (may be NULL to autodetect)
> >>
> >> However, right now only fmt=NULL is supported, as collect_image_info()
> >> ignores fmt altogether.
> >>
> >> So, if this patch is correct we better update the comment. Otherwise,
> >> we should improve collect_image_info() to actually obey fmt != NULL.
> >>
> >@fmt was ignored in the function and I can't see a reason to have
> > it while *bs contains the info, will change the comments.
> >
>Hi, *fmt was used only in collect_image_info_list() when it tries to
> open the image, and it is not useful any more in collect_image_info,
> so nothing need change in comments.

This really doesn't answer my comment above. The code comment implies that
fmt may be NULL or non-NULL and they have different behavior.

If you choose to touch fmt (as this patch does) then please, do the
right thing. Otherwise it's better to let it untouched.



[Qemu-devel] [PATCH 00/14] migration queue

2013-01-15 Thread Juan Quintela
Hi

This is the intersect of the paolo & me patches for migration thread,
please consided for inclusion.

The following changes since commit cf7c3f0cb5a7129f57fa9e69d410d6a05031988c:

  virtio-9p: fix compilation error. (2013-01-14 18:52:39 -0600)

are available in the git repository at:

  git://repo.or.cz/qemu/quintela.git thread.next

for you to fetch changes up to 869342e49d89763f7590ebc52eaecd9ce9f7baa1:

  Rename buffered_ to migration_ (2013-01-15 12:14:40 +0100)


Juan Quintela (8):
  qemu-file: Only set last_error if it is not already set
  migration: move begining stage to the migration thread
  migration: Add buffered_flush error handling
  migration: move exit condition to migration thread
  migration: unfold rest of migrate_fd_put_ready() into thread
  migration: Only go to the iterate stage if there is anything to send
  migration: remove argument to qemu_savevm_state_cancel
  Rename buffered_ to migration_

Paolo Bonzini (6):
  Unlock ramlist lock also in error case
  Protect migration_bitmap_sync() with the ramlist lock
  use XFER_LIMIT_RATIO consistently
  migration: make function static
  migration: remove double call to migrate_fd_close
  migration: fix off-by-one in buffered_rate_limit

 arch_init.c   |   6 +-
 include/migration/migration.h |   3 -
 include/sysemu/sysemu.h   |   2 +-
 migration.c   | 183 --
 savevm.c  |  12 +--
 5 files changed, 98 insertions(+), 108 deletions(-)



[Qemu-devel] [PATCH 01/14] Unlock ramlist lock also in error case

2013-01-15 Thread Juan Quintela
From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: Juan Quintela 
---
 arch_init.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch_init.c b/arch_init.c
index 86f8544..8c833b6 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -642,12 +642,13 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
 i++;
 }

+qemu_mutex_unlock_ramlist();
+
 if (ret < 0) {
 bytes_transferred += total_sent;
 return ret;
 }

-qemu_mutex_unlock_ramlist();
 qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
 total_sent += 8;
 bytes_transferred += total_sent;
-- 
1.8.1




[Qemu-devel] [PATCH 03/14] use XFER_LIMIT_RATIO consistently

2013-01-15 Thread Juan Quintela
From: Paolo Bonzini 

Previous patch missed this case

Signed-off-by: Paolo Bonzini 
Signed-off-by: Juan Quintela 
---
 migration.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration.c b/migration.c
index c69e864..d6ec3e8 100644
--- a/migration.c
+++ b/migration.c
@@ -650,7 +650,7 @@ static int64_t buffered_set_rate_limit(void *opaque, 
int64_t new_rate)
 new_rate = SIZE_MAX;
 }

-s->xfer_limit = new_rate / 10;
+s->xfer_limit = new_rate / XFER_LIMIT_RATIO;

 out:
 return s->xfer_limit;
-- 
1.8.1




[Qemu-devel] [PATCH 09/14] migration: Add buffered_flush error handling

2013-01-15 Thread Juan Quintela
Now that we have error handling we can do proper handling of
buffered_flush().

Signed-off-by: Juan Quintela 
---
 migration.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration.c b/migration.c
index 7ae1d93..17eb27d 100644
--- a/migration.c
+++ b/migration.c
@@ -757,7 +757,8 @@ static void *buffered_file_thread(void *opaque)
 /* usleep expects microseconds */
 g_usleep((initial_time + BUFFER_DELAY - current_time)*1000);
 }
-if (buffered_flush(s) < 0) {
+ret = buffered_flush(s);
+if (ret < 0) {
 break;
 }

-- 
1.8.1




[Qemu-devel] [PATCH 11/14] migration: unfold rest of migrate_fd_put_ready() into thread

2013-01-15 Thread Juan Quintela
This will allow us finer control in next patches.

Signed-off-by: Juan Quintela 
---
 migration.c | 95 ++---
 1 file changed, 41 insertions(+), 54 deletions(-)

diff --git a/migration.c b/migration.c
index 651edd5..6d3aeed 100644
--- a/migration.c
+++ b/migration.c
@@ -662,54 +662,6 @@ static int64_t buffered_get_rate_limit(void *opaque)
 return s->xfer_limit;
 }

-static bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size)
-{
-int ret;
-uint64_t pending_size;
-bool last_round = false;
-
-qemu_mutex_lock_iothread();
-DPRINTF("iterate\n");
-pending_size = qemu_savevm_state_pending(s->file, max_size);
-DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
-if (pending_size >= max_size) {
-ret = qemu_savevm_state_iterate(s->file);
-if (ret < 0) {
-migrate_fd_error(s);
-}
-} else {
-int old_vm_running = runstate_is_running();
-int64_t start_time, end_time;
-
-DPRINTF("done iterating\n");
-start_time = qemu_get_clock_ms(rt_clock);
-qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
-if (old_vm_running) {
-vm_stop(RUN_STATE_FINISH_MIGRATE);
-} else {
-vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
-}
-
-if (qemu_savevm_state_complete(s->file) < 0) {
-migrate_fd_error(s);
-} else {
-migrate_fd_completed(s);
-}
-end_time = qemu_get_clock_ms(rt_clock);
-s->total_time = end_time - s->total_time;
-s->downtime = end_time - start_time;
-if (s->state != MIG_STATE_COMPLETED) {
-if (old_vm_running) {
-vm_start();
-}
-}
-last_round = true;
-}
-qemu_mutex_unlock_iothread();
-
-return last_round;
-}
-
 static void *buffered_file_thread(void *opaque)
 {
 MigrationState *s = opaque;
@@ -730,6 +682,7 @@ static void *buffered_file_thread(void *opaque)

 while (true) {
 int64_t current_time = qemu_get_clock_ms(rt_clock);
+uint64_t pending_size;

 qemu_mutex_lock_iothread();
 if (s->state != MIG_STATE_ACTIVE) {
@@ -741,6 +694,46 @@ static void *buffered_file_thread(void *opaque)
 qemu_mutex_unlock_iothread();
 break;
 }
+if (s->bytes_xfer < s->xfer_limit) {
+DPRINTF("iterate\n");
+pending_size = qemu_savevm_state_pending(s->file, max_size);
+DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
+if (pending_size >= max_size) {
+ret = qemu_savevm_state_iterate(s->file);
+if (ret < 0) {
+qemu_mutex_unlock_iothread();
+break;
+}
+} else {
+int old_vm_running = runstate_is_running();
+int64_t start_time, end_time;
+
+DPRINTF("done iterating\n");
+start_time = qemu_get_clock_ms(rt_clock);
+qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
+if (old_vm_running) {
+vm_stop(RUN_STATE_FINISH_MIGRATE);
+} else {
+vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
+}
+ret = qemu_savevm_state_complete(s->file);
+if (ret < 0) {
+qemu_mutex_unlock_iothread();
+break;
+} else {
+migrate_fd_completed(s);
+}
+end_time = qemu_get_clock_ms(rt_clock);
+s->total_time = end_time - s->total_time;
+s->downtime = end_time - start_time;
+if (s->state != MIG_STATE_COMPLETED) {
+if (old_vm_running) {
+vm_start();
+}
+}
+last_round = true;
+}
+}
 qemu_mutex_unlock_iothread();
 if (current_time >= initial_time + BUFFER_DELAY) {
 uint64_t transferred_bytes = s->bytes_xfer;
@@ -763,12 +756,6 @@ static void *buffered_file_thread(void *opaque)
 if (ret < 0) {
 break;
 }
-
-DPRINTF("file is ready\n");
-if (s->bytes_xfer < s->xfer_limit) {
-DPRINTF("notifying client\n");
-last_round = migrate_fd_put_ready(s, max_size);
-}
 }

 out:
-- 
1.8.1




[Qemu-devel] [PATCH 14/14] Rename buffered_ to migration_

2013-01-15 Thread Juan Quintela
This is consistent once that we have moved everything to migration.c

Signed-off-by: Juan Quintela 
---
 migration.c | 38 +++---
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/migration.c b/migration.c
index 77c1971..ac67525 100644
--- a/migration.c
+++ b/migration.c
@@ -522,7 +522,7 @@ int64_t migrate_xbzrle_cache_size(void)
 /* migration thread support */


-static ssize_t buffered_flush(MigrationState *s)
+static ssize_t migration_flush(MigrationState *s)
 {
 size_t offset = 0;
 ssize_t ret = 0;
@@ -552,7 +552,7 @@ static ssize_t buffered_flush(MigrationState *s)
 return offset;
 }

-static int buffered_put_buffer(void *opaque, const uint8_t *buf,
+static int migration_put_buffer(void *opaque, const uint8_t *buf,
int64_t pos, int size)
 {
 MigrationState *s = opaque;
@@ -585,7 +585,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t 
*buf,
 return size;
 }

-static int buffered_close(void *opaque)
+static int migration_close(void *opaque)
 {
 MigrationState *s = opaque;
 ssize_t ret = 0;
@@ -595,7 +595,7 @@ static int buffered_close(void *opaque)

 s->xfer_limit = INT_MAX;
 while (!qemu_file_get_error(s->file) && s->buffer_size) {
-ret = buffered_flush(s);
+ret = migration_flush(s);
 if (ret < 0) {
 break;
 }
@@ -609,7 +609,7 @@ static int buffered_close(void *opaque)
 return ret;
 }

-static int buffered_get_fd(void *opaque)
+static int migration_get_fd(void *opaque)
 {
 MigrationState *s = opaque;

@@ -622,7 +622,7 @@ static int buffered_get_fd(void *opaque)
  *   1: Time to stop
  *   negative: There has been an error
  */
-static int buffered_rate_limit(void *opaque)
+static int migration_rate_limit(void *opaque)
 {
 MigrationState *s = opaque;
 int ret;
@@ -639,7 +639,7 @@ static int buffered_rate_limit(void *opaque)
 return 0;
 }

-static int64_t buffered_set_rate_limit(void *opaque, int64_t new_rate)
+static int64_t migration_set_rate_limit(void *opaque, int64_t new_rate)
 {
 MigrationState *s = opaque;
 if (qemu_file_get_error(s->file)) {
@@ -655,14 +655,14 @@ out:
 return s->xfer_limit;
 }

-static int64_t buffered_get_rate_limit(void *opaque)
+static int64_t migration_get_rate_limit(void *opaque)
 {
 MigrationState *s = opaque;

 return s->xfer_limit;
 }

-static void *buffered_file_thread(void *opaque)
+static void *migration_file_thread(void *opaque)
 {
 MigrationState *s = opaque;
 int64_t initial_time = qemu_get_clock_ms(rt_clock);
@@ -752,7 +752,7 @@ static void *buffered_file_thread(void *opaque)
 /* usleep expects microseconds */
 g_usleep((initial_time + BUFFER_DELAY - current_time)*1000);
 }
-ret = buffered_flush(s);
+ret = migration_flush(s);
 if (ret < 0) {
 break;
 }
@@ -766,13 +766,13 @@ out:
 return NULL;
 }

-static const QEMUFileOps buffered_file_ops = {
-.get_fd = buffered_get_fd,
-.put_buffer = buffered_put_buffer,
-.close =  buffered_close,
-.rate_limit = buffered_rate_limit,
-.get_rate_limit = buffered_get_rate_limit,
-.set_rate_limit = buffered_set_rate_limit,
+static const QEMUFileOps migration_file_ops = {
+.get_fd = migration_get_fd,
+.put_buffer = migration_put_buffer,
+.close =  migration_close,
+.rate_limit = migration_rate_limit,
+.get_rate_limit = migration_get_rate_limit,
+.set_rate_limit = migration_set_rate_limit,
 };

 void migrate_fd_connect(MigrationState *s)
@@ -786,9 +786,9 @@ void migrate_fd_connect(MigrationState *s)
 s->xfer_limit = s->bandwidth_limit / XFER_LIMIT_RATIO;
 s->complete = false;

-s->file = qemu_fopen_ops(s, &buffered_file_ops);
+s->file = qemu_fopen_ops(s, &migration_file_ops);

-qemu_thread_create(&s->thread, buffered_file_thread, s,
+qemu_thread_create(&s->thread, migration_file_thread, s,
QEMU_THREAD_DETACHED);
 notifier_list_notify(&migration_state_notifiers, s);
 }
-- 
1.8.1




Re: [Qemu-devel] Qemu s390x emulation

2013-01-15 Thread Alexander Graf

On 15.01.2013, at 12:39, Suzuki K. Poulose wrote:

> On 01/15/2013 04:39 PM, Alexander Graf wrote:
>> 
>> On 15.01.2013, at 12:05, Suzuki K. Poulose wrote:
>> 
>>> Hi
>>> 
>>> I have been trying to setup a qemu session for qemu-system-s390x (on
>>> x86_64) using a kernel (with initramfs built-in the kernel) without a
>>> disk image. The kernel was built with s390 defconfig + disabled loadable
>>> modules (just to keep everything inside the kernel).
>>> 
>>> $ qemu-system-s390x -M s390 -kernel vmlinux -m 1024
>>> 
>>> 
>>> The session dies in say 2 secs, with an exit code of 0. I searched for
>>> some hints / success stories, couldn't find any.
>>> 
>>> Am I doing something wrong here ? Please let me know the right procedure
>>> for getting this up and running.
>> 
>> S390 boots using an "image" file. Please try -kernel > dir>/arch/s390/boot/image.
>> 
> Tried that even, but not any better. btw, moved to the upstream git for qemu.
> 
> 0
> $/data/src/qemu/s390x-softmmu/qemu-system-s390x  -m 1024 -kernel ./image 
> -nographic
> $echo $?
> 0
> $file ./image
> ./image: Linux S390
> 
> $ cd /data/src/qemu/ ; git log | head -n1
> commit cf7c3f0cb5a7129f57fa9e69d410d6a05031988c

Does this one work for you?

http://ftp.nl.debian.org/debian/dists/stable/main/installer-s390/current/images/generic/kernel.debian


Alex




[Qemu-devel] [PATCH 05/14] migration: remove double call to migrate_fd_close

2013-01-15 Thread Juan Quintela
From: Paolo Bonzini 

The call in buffered_close is enough, because buffered_close is called
already by migrate_fd_cleanup.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Juan Quintela 
---
 migration.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/migration.c b/migration.c
index 1f4c6ee..5513dde 100644
--- a/migration.c
+++ b/migration.c
@@ -605,7 +605,6 @@ static int buffered_close(void *opaque)
 if (ret >= 0) {
 ret = ret2;
 }
-ret = migrate_fd_close(s);
 s->complete = true;
 return ret;
 }
-- 
1.8.1




[Qemu-devel] [PATCH 07/14] qemu-file: Only set last_error if it is not already set

2013-01-15 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 savevm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/savevm.c b/savevm.c
index 4e970ca..611e997 100644
--- a/savevm.c
+++ b/savevm.c
@@ -419,7 +419,9 @@ int qemu_file_get_error(QEMUFile *f)

 static void qemu_file_set_error(QEMUFile *f, int ret)
 {
-f->last_error = ret;
+if (f->last_error == 0) {
+f->last_error = ret;
+}
 }

 /** Flushes QEMUFile buffer
-- 
1.8.1




[Qemu-devel] [PATCH 02/14] Protect migration_bitmap_sync() with the ramlist lock

2013-01-15 Thread Juan Quintela
From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: Juan Quintela 
---
 arch_init.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 8c833b6..dada6de 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -658,9 +658,8 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)

 static int ram_save_complete(QEMUFile *f, void *opaque)
 {
-migration_bitmap_sync();
-
 qemu_mutex_lock_ramlist();
+migration_bitmap_sync();

 /* try transferring iterative blocks of memory */

-- 
1.8.1




[Qemu-devel] [PATCH 06/14] migration: fix off-by-one in buffered_rate_limit

2013-01-15 Thread Juan Quintela
From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: Juan Quintela 
---
 migration.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration.c b/migration.c
index 5513dde..380f3cb 100644
--- a/migration.c
+++ b/migration.c
@@ -632,7 +632,7 @@ static int buffered_rate_limit(void *opaque)
 return ret;
 }

-if (s->bytes_xfer > s->xfer_limit) {
+if (s->bytes_xfer >= s->xfer_limit) {
 return 1;
 }

-- 
1.8.1




[Qemu-devel] [PATCH 12/14] migration: Only go to the iterate stage if there is anything to send

2013-01-15 Thread Juan Quintela
Signed-off-by: Orit Wasserman 
Signed-off-by: Juan Quintela 
---
 migration.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration.c b/migration.c
index 6d3aeed..fe1a103 100644
--- a/migration.c
+++ b/migration.c
@@ -698,7 +698,7 @@ static void *buffered_file_thread(void *opaque)
 DPRINTF("iterate\n");
 pending_size = qemu_savevm_state_pending(s->file, max_size);
 DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
-if (pending_size >= max_size) {
+if (pending_size && pending_size >= max_size) {
 ret = qemu_savevm_state_iterate(s->file);
 if (ret < 0) {
 qemu_mutex_unlock_iothread();
-- 
1.8.1




[Qemu-devel] [PATCH] target-i386: make x86_def_t.vendor[1, 2, 3] a string and use cpuid_vendor union in CPUX86State

2013-01-15 Thread Igor Mammedov
Tested on x86 host yet, I don't have bigendian host avilable right now.

Vendor property setter takes string as vendor value but cpudefs
use uint32_t vendor[123] fields to define vendor value. It makes it
difficult to unify and use property setter for values from cpudefs.

Simplify code by using vendor property setter, vendor[123] fields
are converted into vendor[13] array to keep its value. And vendor
property setter is used to access/set value on CPU.

Extra:
  - replace cpuid_vendor[1.2.3] words in CPUX86State with union
to simplify vendor property setter and pack/unpack procedures
  - add x86_cpu_vendor_words2str() to make for() cycle reusable
  - convert words in cpuid_vendor union to little-endian when
returning them to guest in cpuid instruction emulation, since
they are not packed manualy anymore

Signed-off-by: Igor Mammedov 
---
v4:
 - use union for cpuid_vendor to simplify convertions string<=>registers
v3:
 - replace x86cpu_vendor_words2str() with x86_cpu_vendor_words2str()
   due to renaming of the last in previous patch
 - rebased with "target-i386: move out CPU features initialization
   in separate func" patch dropped
v2:
  - restore deleted host_cpuid() call in kvm_cpu_fill_host()
 Spotted-By: Eduardo Habkost 
---
 target-i386/cpu.c   |  180 ++-
 target-i386/cpu.h   |   17 +++--
 target-i386/translate.c |4 +-
 3 files changed, 67 insertions(+), 134 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 333745b..882da50 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -45,6 +45,18 @@
 #include "hw/apic_internal.h"
 #endif
 
+static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
+ uint32_t vendor2, uint32_t vendor3)
+{
+int i;
+for (i = 0; i < 4; i++) {
+dst[i] = vendor1 >> (8 * i);
+dst[i + 4] = vendor2 >> (8 * i);
+dst[i + 8] = vendor3 >> (8 * i);
+}
+dst[CPUID_VENDOR_SZ] = '\0';
+}
+
 /* feature flags taken from "Intel Processor Identification and the CPUID
  * Instruction" and AMD's "CPUID Specification".  In cases of disagreement
  * between feature naming conventions, aliases may be added.
@@ -341,7 +353,7 @@ typedef struct x86_def_t {
 struct x86_def_t *next;
 const char *name;
 uint32_t level;
-uint32_t vendor1, vendor2, vendor3;
+char vendor[CPUID_VENDOR_SZ + 1];
 int family;
 int model;
 int stepping;
@@ -406,9 +418,7 @@ static x86_def_t builtin_x86_defs[] = {
 {
 .name = "qemu64",
 .level = 4,
-.vendor1 = CPUID_VENDOR_AMD_1,
-.vendor2 = CPUID_VENDOR_AMD_2,
-.vendor3 = CPUID_VENDOR_AMD_3,
+.vendor = CPUID_VENDOR_AMD,
 .family = 6,
 .model = 2,
 .stepping = 3,
@@ -425,9 +435,7 @@ static x86_def_t builtin_x86_defs[] = {
 {
 .name = "phenom",
 .level = 5,
-.vendor1 = CPUID_VENDOR_AMD_1,
-.vendor2 = CPUID_VENDOR_AMD_2,
-.vendor3 = CPUID_VENDOR_AMD_3,
+.vendor = CPUID_VENDOR_AMD,
 .family = 16,
 .model = 2,
 .stepping = 3,
@@ -453,9 +461,7 @@ static x86_def_t builtin_x86_defs[] = {
 {
 .name = "core2duo",
 .level = 10,
-.vendor1 = CPUID_VENDOR_INTEL_1,
-.vendor2 = CPUID_VENDOR_INTEL_2,
-.vendor3 = CPUID_VENDOR_INTEL_3,
+.vendor = CPUID_VENDOR_INTEL,
 .family = 6,
 .model = 15,
 .stepping = 11,
@@ -474,9 +480,7 @@ static x86_def_t builtin_x86_defs[] = {
 {
 .name = "kvm64",
 .level = 5,
-.vendor1 = CPUID_VENDOR_INTEL_1,
-.vendor2 = CPUID_VENDOR_INTEL_2,
-.vendor3 = CPUID_VENDOR_INTEL_3,
+.vendor = CPUID_VENDOR_INTEL,
 .family = 15,
 .model = 6,
 .stepping = 1,
@@ -500,9 +504,7 @@ static x86_def_t builtin_x86_defs[] = {
 {
 .name = "qemu32",
 .level = 4,
-.vendor1 = CPUID_VENDOR_INTEL_1,
-.vendor2 = CPUID_VENDOR_INTEL_2,
-.vendor3 = CPUID_VENDOR_INTEL_3,
+.vendor = CPUID_VENDOR_INTEL,
 .family = 6,
 .model = 3,
 .stepping = 3,
@@ -513,9 +515,7 @@ static x86_def_t builtin_x86_defs[] = {
 {
 .name = "kvm32",
 .level = 5,
-.vendor1 = CPUID_VENDOR_INTEL_1,
-.vendor2 = CPUID_VENDOR_INTEL_2,
-.vendor3 = CPUID_VENDOR_INTEL_3,
+.vendor = CPUID_VENDOR_INTEL,
 .family = 15,
 .model = 6,
 .stepping = 1,
@@ -530,9 +530,7 @@ static x86_def_t builtin_x86_defs[] = {
 {
 .name = "coreduo",
 .level = 10,
-.vendor1 = CPUID_VENDOR_INTEL_1,
-.vendor2 = CPUID_VENDOR_INTEL_2,
-.vendor3 = CPUID_VENDOR_INTEL_3,
+.vendor = CPUID_VENDOR_INTEL,
 .family = 6,
 .model = 14,
 .stepping = 8,
@@ -548,9 +546,7 @@ static x86_def_t builtin_x86_defs[] = {
 {
 .name = "486",
   

[Qemu-devel] [PATCH 10/14] migration: move exit condition to migration thread

2013-01-15 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 migration.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/migration.c b/migration.c
index 17eb27d..651edd5 100644
--- a/migration.c
+++ b/migration.c
@@ -669,12 +669,6 @@ static bool migrate_fd_put_ready(MigrationState *s, 
uint64_t max_size)
 bool last_round = false;

 qemu_mutex_lock_iothread();
-if (s->state != MIG_STATE_ACTIVE) {
-DPRINTF("put_ready returning because of non-active state\n");
-qemu_mutex_unlock_iothread();
-return false;
-}
-
 DPRINTF("iterate\n");
 pending_size = qemu_savevm_state_pending(s->file, max_size);
 DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
@@ -737,9 +731,17 @@ static void *buffered_file_thread(void *opaque)
 while (true) {
 int64_t current_time = qemu_get_clock_ms(rt_clock);

+qemu_mutex_lock_iothread();
+if (s->state != MIG_STATE_ACTIVE) {
+DPRINTF("put_ready returning because of non-active state\n");
+qemu_mutex_unlock_iothread();
+break;
+}
 if (s->complete) {
+qemu_mutex_unlock_iothread();
 break;
 }
+qemu_mutex_unlock_iothread();
 if (current_time >= initial_time + BUFFER_DELAY) {
 uint64_t transferred_bytes = s->bytes_xfer;
 uint64_t time_spent = current_time - initial_time;
-- 
1.8.1




[Qemu-devel] [Qemu-trivial] [PATCH v2] configure: try pkg-config for curses

2013-01-15 Thread Vadim Evard

Static linkikng against ncurses may require explicit -ltinfo.
In case -lcurses and -lncurses both didn't work give pkg-config a
chance.

Fixes #1094786 for me.

Signed-off-by: Vadim Evard 
---
 configure |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index c908f66..40473a9 100755
--- a/configure
+++ b/configure
@@ -2039,7 +2039,7 @@ fi
 if test "$mingw32" = "yes" ; then
 curses_list="-lpdcurses"
 else
-curses_list="-lncurses -lcurses"
+curses_list="-lncurses:-lcurses:$($pkg_config --libs ncurses)"
 fi

 if test "$curses" != "no" ; then
@@ -2052,13 +2052,16 @@ int main(void) {
   return s != 0;
 }
 EOF
+  IFS=:
   for curses_lib in $curses_list; do
+unset IFS
 if compile_prog "" "$curses_lib" ; then
   curses_found=yes
   libs_softmmu="$curses_lib $libs_softmmu"
   break
 fi
   done
+  unset IFS
   if test "$curses_found" = "yes" ; then
 curses=yes
   else
--
1.7.10.4



On 15.01.2013 13:49, Stefan Hajnoczi wrote:

On Fri, Jan 11, 2013 at 04:51:24PM +0400, Vadim Evard wrote:

ping

Пнд 31 Дек 2012 20:30:59, Vadim Evard писал:

configure: try pkg-config for curses

Static linkikng against ncurses may require explicit -ltinfo.
In case -lcurses and -lncurses both didn't work give pkg-config a
chance.

Signed-off-by: Vadim Evard
---
configure | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index b0c7e54..16280e2 100755
--- a/configure
+++ b/configure
@@ -2030,7 +2030,7 @@ fi
if test "$mingw32" = "yes" ; then
curses_list="-lpdcurses"
else
- curses_list="-lncurses -lcurses"
+ curses_list="-lncurses:-lcurses:$($pkg_config --libs ncurses)"
fi

if test "$curses" != "no" ; then
@@ -2043,7 +2043,9 @@ int main(void) {
return s != 0;
}
EOF
+ IFS=:
for curses_lib in $curses_list; do
+ unset IFS
if compile_prog "" "$curses_lib" ; then
curses_found=yes
libs_softmmu="$curses_lib $libs_softmmu"


Please also unset IFS after the loop so there will never be a problem if
curses_list="".

Stefan




[Qemu-devel] [PATCH 13/14] migration: remove argument to qemu_savevm_state_cancel

2013-01-15 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 include/sysemu/sysemu.h | 2 +-
 migration.c | 2 +-
 savevm.c| 8 
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index c07d4ee..d65a9f1 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -77,7 +77,7 @@ int qemu_savevm_state_begin(QEMUFile *f,
 const MigrationParams *params);
 int qemu_savevm_state_iterate(QEMUFile *f);
 int qemu_savevm_state_complete(QEMUFile *f);
-void qemu_savevm_state_cancel(QEMUFile *f);
+void qemu_savevm_state_cancel(void);
 uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size);
 int qemu_loadvm_state(QEMUFile *f);

diff --git a/migration.c b/migration.c
index fe1a103..77c1971 100644
--- a/migration.c
+++ b/migration.c
@@ -330,7 +330,7 @@ static void migrate_fd_cancel(MigrationState *s)

 s->state = MIG_STATE_CANCELLED;
 notifier_list_notify(&migration_state_notifiers, s);
-qemu_savevm_state_cancel(s->file);
+qemu_savevm_state_cancel();

 migrate_fd_cleanup(s);
 }
diff --git a/savevm.c b/savevm.c
index 611e997..913a623 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1590,13 +1590,13 @@ int qemu_savevm_state_begin(QEMUFile *f,

 ret = se->ops->save_live_setup(f, se->opaque);
 if (ret < 0) {
-qemu_savevm_state_cancel(f);
+qemu_savevm_state_cancel();
 return ret;
 }
 }
 ret = qemu_file_get_error(f);
 if (ret != 0) {
-qemu_savevm_state_cancel(f);
+qemu_savevm_state_cancel();
 }

 return ret;
@@ -1647,7 +1647,7 @@ int qemu_savevm_state_iterate(QEMUFile *f)
 }
 ret = qemu_file_get_error(f);
 if (ret != 0) {
-qemu_savevm_state_cancel(f);
+qemu_savevm_state_cancel();
 }
 return ret;
 }
@@ -1727,7 +1727,7 @@ uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t 
max_size)
 return ret;
 }

-void qemu_savevm_state_cancel(QEMUFile *f)
+void qemu_savevm_state_cancel(void)
 {
 SaveStateEntry *se;

-- 
1.8.1




[Qemu-devel] [PATCH 04/14] migration: make function static

2013-01-15 Thread Juan Quintela
From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: Juan Quintela 
---
 include/migration/migration.h | 2 --
 migration.c   | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index 2d5b630..95261c1 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -87,8 +87,6 @@ void migrate_fd_error(MigrationState *s);

 void migrate_fd_connect(MigrationState *s);

-ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
-  size_t size);
 int migrate_fd_close(MigrationState *s);

 void add_migration_state_change_notifier(Notifier *notify);
diff --git a/migration.c b/migration.c
index d6ec3e8..1f4c6ee 100644
--- a/migration.c
+++ b/migration.c
@@ -302,8 +302,8 @@ static void migrate_fd_completed(MigrationState *s)
 notifier_list_notify(&migration_state_notifiers, s);
 }

-ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
-  size_t size)
+static ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
+ size_t size)
 {
 ssize_t ret;

-- 
1.8.1




[Qemu-devel] [PATCH 08/14] migration: move begining stage to the migration thread

2013-01-15 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 include/migration/migration.h |  1 -
 migration.c   | 28 +++-
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index 95261c1..a8c9639 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -54,7 +54,6 @@ struct MigrationState
 bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
 int64_t xbzrle_cache_size;
 bool complete;
-bool first_time;
 };

 void process_incoming_migration(QEMUFile *f);
diff --git a/migration.c b/migration.c
index 380f3cb..7ae1d93 100644
--- a/migration.c
+++ b/migration.c
@@ -674,17 +674,6 @@ static bool migrate_fd_put_ready(MigrationState *s, 
uint64_t max_size)
 qemu_mutex_unlock_iothread();
 return false;
 }
-if (s->first_time) {
-s->first_time = false;
-DPRINTF("beginning savevm\n");
-ret = qemu_savevm_state_begin(s->file, &s->params);
-if (ret < 0) {
-DPRINTF("failed, %d\n", ret);
-migrate_fd_error(s);
-qemu_mutex_unlock_iothread();
-return false;
-}
-}

 DPRINTF("iterate\n");
 pending_size = qemu_savevm_state_pending(s->file, max_size);
@@ -733,6 +722,17 @@ static void *buffered_file_thread(void *opaque)
 int64_t initial_time = qemu_get_clock_ms(rt_clock);
 int64_t max_size = 0;
 bool last_round = false;
+int ret;
+
+qemu_mutex_lock_iothread();
+DPRINTF("beginning savevm\n");
+ret = qemu_savevm_state_begin(s->file, &s->params);
+if (ret < 0) {
+DPRINTF("failed, %d\n", ret);
+qemu_mutex_unlock_iothread();
+goto out;
+}
+qemu_mutex_unlock_iothread();

 while (true) {
 int64_t current_time = qemu_get_clock_ms(rt_clock);
@@ -768,6 +768,10 @@ static void *buffered_file_thread(void *opaque)
 }
 }

+out:
+if (ret < 0) {
+migrate_fd_error(s);
+}
 g_free(s->buffer);
 return NULL;
 }
@@ -789,8 +793,6 @@ void migrate_fd_connect(MigrationState *s)
 s->buffer_size = 0;
 s->buffer_capacity = 0;

-s->first_time = true;
-
 s->xfer_limit = s->bandwidth_limit / XFER_LIMIT_RATIO;
 s->complete = false;

-- 
1.8.1




Re: [Qemu-devel] [PATCH V10 3/4] Use QemuOpts support in block layer

2013-01-15 Thread Kevin Wolf
Am 07.01.2013 06:26, schrieb Dong Xu Wang:
> This patch will use QemuOpts related functions in block layer, add
> a member bdrv_create_options to BlockDriver struct, it will return
> a QemuOptsList pointer, which includes the image format's create
> options.
> 
> And create options's primary consumer is block creating related functions,
> so modify them together.
> 
> Signed-off-by: Dong Xu Wang 

> diff --git a/block/qcow.c b/block/qcow.c
> index 4276610..46aad7f 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -651,7 +651,7 @@ static void qcow_close(BlockDriverState *bs)
>  error_free(s->migration_blocker);
>  }
>  
> -static int qcow_create(const char *filename, QEMUOptionParameter *options)
> +static int qcow_create(const char *filename, QemuOpts *opts)
>  {
>  int header_size, backing_filename_len, l1_size, shift, i;
>  QCowHeader header;
> @@ -662,19 +662,16 @@ static int qcow_create(const char *filename, 
> QEMUOptionParameter *options)
>  int ret;
>  BlockDriverState *qcow_bs;
>  
> -/* Read out options */
> -while (options && options->name) {
> -if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> -total_size = options->value.n / 512;
> -} else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
> -backing_file = options->value.s;
> -} else if (!strcmp(options->name, BLOCK_OPT_ENCRYPT)) {
> -flags |= options->value.n ? BLOCK_FLAG_ENCRYPT : 0;
> +/* Read out opts */
> +if (opts) {

Can opts ever be NULL? (Same question for all other block drivers)

> +total_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512;
> +backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE);
> +if (qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, 0)) {
> +flags |= BLOCK_FLAG_ENCRYPT;
>  }
> -options++;
>  }
>  
> -ret = bdrv_create_file(filename, options);
> +ret = bdrv_create_file(filename, NULL);

Why is this change correct?

Previously you could pass options to the protocol that are not supported
by the file format. For example, you can specify a backing file for raw
over sheepdog. Interestingly you keep this correct behaviour for raw,
but you seem to break it for other image formats.

>  if (ret < 0) {
>  return ret;
>  }

Kevin



Re: [Qemu-devel] [PATCH v3] sheepdog: multiplex the rw FD to flush cache

2013-01-15 Thread Stefan Hajnoczi
On Tue, Jan 15, 2013 at 06:28:24PM +0800, Liu Yuan wrote:
> On 01/15/2013 06:20 PM, Stefan Hajnoczi wrote:
> > Please remember to run scripts/checkpatch.pl before posting patches.  I
> > fixed up a tab character that snuck into this patch.
> > 
> 
> Ah, Okay, it was in a hasty.
> 
> > Thanks, applied to my block tree:
> > https://github.com/stefanha/qemu/commits/block
> 
> Seems that lost Patch 2/2?

Thanks for pointing this out, 2/2 is now merged.

I suggest sending patch series with a cover letter in the future.
Resend the whole vN+1 series when you make a change.  This reduces the
chance that I or other maintainers miss patches.

Stefan



Re: [Qemu-devel] [PATCH V3 07/11] block: export function bdrv_find_snapshot()

2013-01-15 Thread Markus Armbruster
Eric Blake  writes:

> On 01/14/2013 12:09 AM, Wenchao Xia wrote:
>>   This patch move it from savevm.c to block.c and export it.
>> 
>> Signed-off-by: Wenchao Xia 
>> ---
>>  block.c   |   23 +++
>>  include/block/block.h |2 ++
>>  savevm.c  |   22 --
>>  3 files changed, 25 insertions(+), 22 deletions(-)
>> 
>> diff --git a/block.c b/block.c
>> index 8192d8e..b7d2f03 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3351,6 +3351,29 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
>>  return -ENOTSUP;
>>  }
>>  
>> +int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
>> +   const char *name)
>> +{
>> +QEMUSnapshotInfo *sn_tab, *sn;
>> +int nb_sns, i, ret;
>> +
>> +ret = -ENOENT;
>> +nb_sns = bdrv_snapshot_list(bs, &sn_tab);
>> +if (nb_sns < 0) {
>> +return ret;
>> +}
>> +for (i = 0; i < nb_sns; i++) {
>> +sn = &sn_tab[i];
>> +if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) {
>
> It is possible (albeit probably stupid) to create a qcow2 file where
> snapshot names are merely numeric strings.  In fact, just to see what
> would happen, I once[1] created a file where:
>
> snapshot id '1' was named '2'
> snapshot id '2' was named 'foo'
>
> This code comparison favors ids over names; so if I request to delvm 2,
> I end up removing the second snapshot, not the first.  This is okay, but
> probably worth documenting, and probably worth making sure that all code
> that looks up a snapshot by name or id goes through this function so
> that we get the same behavior everywhere.  My experiment was done
> several months ago, but my recollection was that at the time, there was
> an inconsistency where 'qemu-img snapshot' picked a different snapshot
> for the request of '2' than the online 'delvm' monitor command of qemu;
> making it unsafe to rely on either behavior in that version of qemu
> source code.
>
> [1]https://bugzilla.redhat.com/show_bug.cgi?id=733143

In QemuOpts, we restricted IDs to [[:alpha:]][[:alnum:]-._]*, see
id_wellformed().  I'd recommend the same for new interfaces.  But this
is an old one, and we shouldn't make existing snapshots inaccessible
just because their names were chosen unwisely.



Re: [Qemu-devel] getting rid of coroutine-gthread?

2013-01-15 Thread Kevin Wolf
Am 11.01.2013 19:11, schrieb Paolo Bonzini:
> Brad and Peter,
> 
> as far as I know OpenBSD and Linux/ARM were the main users of
> coroutine-gthread.  Do you think we could dump it and rely on
> coroutine-sigaltstack only?  The differences in signal handling of the
> gthread implementation always worried me.
> 
> What versions of OpenBSD would we have to drop support for?  Is that
> acceptable to you?

Changing the defaults for these platforms may be a good idea, but I
actually like the option of having coroutine-gthread because it's much
friendlier to debug - gdb supports threads, but not coroutines.

Is coroutine-gthread blocking anything or is it just that you're not
entirely confident in its correctness?

Kevin



Re: [Qemu-devel] getting rid of coroutine-gthread?

2013-01-15 Thread Paolo Bonzini
Il 15/01/2013 14:18, Kevin Wolf ha scritto:
>> > Brad and Peter,
>> > 
>> > as far as I know OpenBSD and Linux/ARM were the main users of
>> > coroutine-gthread.  Do you think we could dump it and rely on
>> > coroutine-sigaltstack only?  The differences in signal handling of the
>> > gthread implementation always worried me.
>> > 
>> > What versions of OpenBSD would we have to drop support for?  Is that
>> > acceptable to you?
> Changing the defaults for these platforms may be a good idea, but I
> actually like the option of having coroutine-gthread because it's much
> friendlier to debug - gdb supports threads, but not coroutines.
> 
> Is coroutine-gthread blocking anything or is it just that you're not
> entirely confident in its correctness?

I'm not entirely confident in its correctness, and I'd be afraid of
breaking things when converting dataplane to AIOContext.

Paolo



Re: [Qemu-devel] [PATCH 2/2] qxl: change rom so that 4096 < size < 8192

2013-01-15 Thread Alon Levy
On Sun, Dec 23, 2012 at 10:31:38PM +0200, Alon Levy wrote:
> On Thu, Dec 13, 2012 at 01:05:48PM +0100, Gerd Hoffmann wrote:
> > On 12/13/12 12:36, Alon Levy wrote:
> > > This is a simpler solution to 869981, where migration breaks since qxl's
> > > rom bar size has changed. Instead of ignoring fields in QXLRom, which is 
> > > what has
> > > actually changed, we remove some of the modes, a mechanism already
> > > accounted for by the guest. To reach exactly two pages and not one, we
> > > remove two out of four orientations, the remaining are normal and right
> > > turn (chosen arbitrarily). Leaving only normal would result in a single
> > > page which would also break migration.
> > > 
> > > Added assertions to ensure changes in spice-protocol in the future
> > > causing increase or decrease of page size will result in failure at
> > > startup (could do this compile time also but not sure how).
> > 
> > The assertions are not in the patch.
> > 
> > >  #define QXL_MODE_EX(x_res, y_res) \
> > >  QXL_MODE_16_32(x_res, y_res, 0),  \
> > > -QXL_MODE_16_32(y_res, x_res, 1),  \
> > > -QXL_MODE_16_32(x_res, y_res, 2),  \
> > > -QXL_MODE_16_32(y_res, x_res, 3)
> > > +QXL_MODE_16_32(x_res, y_res, 1)
> > 
> > Why do you leave orientation = 1 in?  Just to keep the size above 4K?
> > Shouldn't we just hardcode the rom size to 8k instead?  Then assert that
> > everything fits into 8k?  Or even better add a compile time check?
> > 
> > While being at it it might be a good idea to move the mode table to a
> > fixed, large enougth offset (say 0x4096), so it doesn't move around
> > again in case we extend QXLRom one more time.
> 
> This solution is breaking backward compatibility like Yonit pointed
> out. The fact that I can't produce a user that would break because of
> this doesn't prove there is no such user. I suggest we go back to the
> original patch I posted, breaking it into two like you requested. What
> do you say?

Ping?

> 
> > 
> > cheers,
> >   Gerd
> > 
> 



[Qemu-devel] [PATCH 2/4] scsi-disk: qemu_vfree(NULL) is fine, simplify

2013-01-15 Thread Markus Armbruster

Signed-off-by: Markus Armbruster 
---
 hw/scsi-disk.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index f8d7ef3..96db9a7 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -85,9 +85,7 @@ static void scsi_free_request(SCSIRequest *req)
 {
 SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
 
-if (r->iov.iov_base) {
-qemu_vfree(r->iov.iov_base);
-}
+qemu_vfree(r->iov.iov_base);
 }
 
 /* Helper function for command completion with sense.  */
-- 
1.7.11.7




[Qemu-devel] [PATCH 0/4] Fixes around qemu_vfree()

2013-01-15 Thread Markus Armbruster
Markus Armbruster (4):
  w32: Make qemu_vfree() accept NULL like the POSIX implementation
  scsi-disk: qemu_vfree(NULL) is fine, simplify
  win32-aio: Fix how win32_aio_process_completion() frees buffer
  block: Fix how mirror_run() frees its buffer

 block/mirror.c | 2 +-
 block/win32-aio.c  | 2 +-
 hw/scsi-disk.c | 4 +---
 util/oslib-win32.c | 4 +++-
 4 files changed, 6 insertions(+), 6 deletions(-)

-- 
1.7.11.7




Re: [Qemu-devel] getting rid of coroutine-gthread?

2013-01-15 Thread Kevin Wolf
Am 15.01.2013 14:28, schrieb Paolo Bonzini:
> Il 15/01/2013 14:18, Kevin Wolf ha scritto:
 Brad and Peter,

 as far as I know OpenBSD and Linux/ARM were the main users of
 coroutine-gthread.  Do you think we could dump it and rely on
 coroutine-sigaltstack only?  The differences in signal handling of the
 gthread implementation always worried me.

 What versions of OpenBSD would we have to drop support for?  Is that
 acceptable to you?
>> Changing the defaults for these platforms may be a good idea, but I
>> actually like the option of having coroutine-gthread because it's much
>> friendlier to debug - gdb supports threads, but not coroutines.
>>
>> Is coroutine-gthread blocking anything or is it just that you're not
>> entirely confident in its correctness?
> 
> I'm not entirely confident in its correctness, and I'd be afraid of
> breaking things when converting dataplane to AIOContext.

Dataplane is (at least for now) not the most important thing to have
when debugging a block driver. Would it help if we made dataplane and
coroutine-gthread mutually exclusive?

Kevin



Re: [Qemu-devel] [PATCH V3 07/11] block: export function bdrv_find_snapshot()

2013-01-15 Thread Pavel Hrdina

On 01/15/2013 01:01 PM, Markus Armbruster wrote:

Eric Blake  writes:


On 01/14/2013 12:09 AM, Wenchao Xia wrote:

   This patch move it from savevm.c to block.c and export it.

Signed-off-by: Wenchao Xia 
---
  block.c   |   23 +++
  include/block/block.h |2 ++
  savevm.c  |   22 --
  3 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/block.c b/block.c
index 8192d8e..b7d2f03 100644
--- a/block.c
+++ b/block.c
@@ -3351,6 +3351,29 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
  return -ENOTSUP;
  }

+int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
+   const char *name)
+{
+QEMUSnapshotInfo *sn_tab, *sn;
+int nb_sns, i, ret;
+
+ret = -ENOENT;
+nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+if (nb_sns < 0) {
+return ret;
+}
+for (i = 0; i < nb_sns; i++) {
+sn = &sn_tab[i];
+if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) {


It is possible (albeit probably stupid) to create a qcow2 file where
snapshot names are merely numeric strings.  In fact, just to see what
would happen, I once[1] created a file where:

snapshot id '1' was named '2'
snapshot id '2' was named 'foo'

This code comparison favors ids over names; so if I request to delvm 2,
I end up removing the second snapshot, not the first.  This is okay, but
probably worth documenting, and probably worth making sure that all code
that looks up a snapshot by name or id goes through this function so
that we get the same behavior everywhere.  My experiment was done
several months ago, but my recollection was that at the time, there was
an inconsistency where 'qemu-img snapshot' picked a different snapshot
for the request of '2' than the online 'delvm' monitor command of qemu;
making it unsafe to rely on either behavior in that version of qemu
source code.

[1]https://bugzilla.redhat.com/show_bug.cgi?id=733143


In QemuOpts, we restricted IDs to [[:alpha:]][[:alnum:]-._]*, see
id_wellformed().  I'd recommend the same for new interfaces.  But this
is an old one, and we shouldn't make existing snapshots inaccessible
just because their names were chosen unwisely.



There is my proposal for handling snapshots id and name.

http://lists.gnu.org/archive/html/qemu-devel/2013-01/msg01551.html

In general, we will have two options for operation with snapshots, an 
'id' option and a 'name' option. You could choose one of them or both to 
specify which snapshot you want to load/delete/create/modify.


This behaviour should be the same for online and offline snapshots even 
for vm-snapshots or block-snapshots.


With these modifications old snapshots should also works.

Pavel



Re: [Qemu-devel] [PATCH 3/4] win32-aio: Fix how win32_aio_process_completion() frees buffer

2013-01-15 Thread Kevin Wolf
Am 15.01.2013 14:23, schrieb Markus Armbruster:
> win32_aio_submit() allocates it with qemu_blockalign(), therefore it
> must be freed with qemu_vfree(), not g_free().
> 
> Signed-off-by: Markus Armbruster 
> ---
>  block/win32-aio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/win32-aio.c b/block/win32-aio.c
> index 46a5db7..0383370 100644
> --- a/block/win32-aio.c
> +++ b/block/win32-aio.c
> @@ -87,7 +87,7 @@ static void win32_aio_process_completion(QEMUWin32AIOState 
> *s,
>  memcpy(p, qiov->iov[i].iov_base, qiov->iov[i].iov_len);
>  p += qiov->iov[i].iov_len;
>  }
> -g_free(waiocb->buf);
> +qemu_vfree(waiocb->buf);
>  }
>  }

Independent bug: waiocb->buf is leaked for writes and failed reads.

Kevin



Re: [Qemu-devel] [PATCH 4/4] block: Fix how mirror_run() frees its buffer

2013-01-15 Thread Kevin Wolf
Am 15.01.2013 14:23, schrieb Markus Armbruster:
> It allocates with qemu_blockalign(), therefore it must free with
> qemu_vfree(), not g_free().
> 
> Since I'm touching it anyway, move the free to a more obviosly correct
> place.

...except that it's now leaked for all error cases but the first.

Kevin

> Signed-off-by: Markus Armbruster 
> ---
>  block/mirror.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index 8aeacbf..27a7d8c 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -223,9 +223,9 @@ static void coroutine_fn mirror_run(void *opaque)
>  break;
>  }
>  }
> +qemu_vfree(s->buf);
>  
>  immediate_exit:
> -g_free(s->buf);
>  bdrv_set_dirty_tracking(bs, false);
>  bdrv_iostatus_disable(s->target);
>  if (s->should_complete && ret == 0) {
> 





Re: [Qemu-devel] [PATCH 0/4] Fixes around qemu_vfree()

2013-01-15 Thread Kevin Wolf
Am 15.01.2013 14:23, schrieb Markus Armbruster:
> Markus Armbruster (4):
>   w32: Make qemu_vfree() accept NULL like the POSIX implementation
>   scsi-disk: qemu_vfree(NULL) is fine, simplify
>   win32-aio: Fix how win32_aio_process_completion() frees buffer
>   block: Fix how mirror_run() frees its buffer
> 
>  block/mirror.c | 2 +-
>  block/win32-aio.c  | 2 +-
>  hw/scsi-disk.c | 4 +---
>  util/oslib-win32.c | 4 +++-
>  4 files changed, 6 insertions(+), 6 deletions(-)

Patches 1-3:
Reviewed-by: Kevin Wolf 



[Qemu-devel] [PATCH 4/4] block: Fix how mirror_run() frees its buffer

2013-01-15 Thread Markus Armbruster
It allocates with qemu_blockalign(), therefore it must free with
qemu_vfree(), not g_free().

Since I'm touching it anyway, move the free to a more obviosly correct
place.

Signed-off-by: Markus Armbruster 
---
 block/mirror.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index 8aeacbf..27a7d8c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -223,9 +223,9 @@ static void coroutine_fn mirror_run(void *opaque)
 break;
 }
 }
+qemu_vfree(s->buf);
 
 immediate_exit:
-g_free(s->buf);
 bdrv_set_dirty_tracking(bs, false);
 bdrv_iostatus_disable(s->target);
 if (s->should_complete && ret == 0) {
-- 
1.7.11.7




[Qemu-devel] [PATCH 3/4] win32-aio: Fix how win32_aio_process_completion() frees buffer

2013-01-15 Thread Markus Armbruster
win32_aio_submit() allocates it with qemu_blockalign(), therefore it
must be freed with qemu_vfree(), not g_free().

Signed-off-by: Markus Armbruster 
---
 block/win32-aio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/win32-aio.c b/block/win32-aio.c
index 46a5db7..0383370 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -87,7 +87,7 @@ static void win32_aio_process_completion(QEMUWin32AIOState *s,
 memcpy(p, qiov->iov[i].iov_base, qiov->iov[i].iov_len);
 p += qiov->iov[i].iov_len;
 }
-g_free(waiocb->buf);
+qemu_vfree(waiocb->buf);
 }
 }
 
-- 
1.7.11.7




[Qemu-devel] [PATCH 1/4] w32: Make qemu_vfree() accept NULL like the POSIX implementation

2013-01-15 Thread Markus Armbruster
On POSIX, qemu_vfree() accepts NULL, because it's merely wrapper
around free().  As far as I can tell, the Windows implementation
doesn't.  Breeds bugs that bite only under Windows.

Make the Windows implementation behave like the POSIX implementation.

Signed-off-by: Markus Armbruster 
---
 util/oslib-win32.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index e7e283e..640194c 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -71,7 +71,9 @@ void *qemu_vmalloc(size_t size)
 void qemu_vfree(void *ptr)
 {
 trace_qemu_vfree(ptr);
-VirtualFree(ptr, 0, MEM_RELEASE);
+if (ptr) {
+VirtualFree(ptr, 0, MEM_RELEASE);
+}
 }
 
 /* FIXME: add proper locking */
-- 
1.7.11.7




  1   2   3   4   >