Re: [Qemu-devel] [patch 0/2] force -mem-path RAM allocation

2013-10-08 Thread Markus Armbruster
Marcelo Tosatti  writes:

> On Tue, Oct 08, 2013 at 10:02:26AM +0200, Paolo Bonzini wrote:
>> Il 08/10/2013 09:32, Markus Armbruster ha scritto:
>> > We have
>> > 
>> > -mem-path FILE  provide backing storage for guest RAM
>> > -mem-prealloc   preallocate guest memory (use with -mem-path)
>> > 
>> > PATCH 2/2 adds
>> > 
>> > -mem-path-force fail if unable to allocate RAM as specified by
>> > -mem-path
>> > 
>> > Looks like it's time to consolidate the options related to guest memory
>> > into a single, QemuOpts-style -memory NAME=VALUE,...  What do you guys
>> > think?
>> 
>> Yes, we can use "-numa memory" (or "-numa mem") that Wanlong Gao is
>> adding.  We can add path=, preallocate= and force= options there.
>> 
>> Paolo
>
> It would be important for the new option to be backportable 
> independently. Therefore mixing it with -numa is not an option.
>
> Also due to backportability supporting a new style of command line
> for -mem-path is problematic (management must be changed accordingly).

We've converted -FOO ARG options to QemuOpts-style -FOO
NAME=VALUE,... before.  You can use QemuOptsList member implied_opt_name
to get bare ARG accepted.  Works except for ARGs containing '=' or ','.

Management still has to detect whether -FOO is old or new.  QMP command
query-command-line-options should do.

> Can the new option format for memory be created incrementally on 
> top of -mem-path-force? (agree its a good thing, it avoids proliferation
> of new options).

If you do it on top, it won't avoid proliferation, or am I missing
something?



[Qemu-devel] [PATCH v5 5/6] qemu-iotests: update test cases for commit active

2013-10-08 Thread Fam Zheng
Factor out commit test common logic into super class, and update test
of committing the active image.

Signed-off-by: Fam Zheng 
---
 tests/qemu-iotests/040 | 73 +-
 1 file changed, 31 insertions(+), 42 deletions(-)

diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index aad535a..902df0d 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -63,6 +63,28 @@ class ImageCommitTestCase(iotests.QMPTestCase):
 i = i + 512
 file.close()
 
+def run_commit_test(self, top, base, is_active=False):
+self.assert_no_active_commit()
+result = self.vm.qmp('block-commit', device='drive0', top=top, 
base=base)
+self.assert_qmp(result, 'return', {})
+
+completed = False
+while not completed:
+for event in self.vm.get_qmp_events(wait=True):
+if event['event'] == 'BLOCK_JOB_COMPLETED':
+self.assert_qmp(event, 'data/type', 'commit')
+self.assert_qmp(event, 'data/device', 'drive0')
+self.assert_qmp(event, 'data/offset', self.image_len)
+self.assert_qmp(event, 'data/len', self.image_len)
+completed = True
+elif event['event'] == 'BLOCK_JOB_READY':
+self.assert_qmp(event, 'data/type', 'commit')
+self.assert_qmp(event, 'data/device', 'drive0')
+self.assert_qmp(event, 'data/len', self.image_len)
+self.vm.qmp('block-job-complete', device='drive0')
+
+self.assert_no_active_commit()
+self.vm.shutdown()
 
 class TestSingleDrive(ImageCommitTestCase):
 image_len = 1 * 1024 * 1024
@@ -84,23 +106,7 @@ class TestSingleDrive(ImageCommitTestCase):
 os.remove(backing_img)
 
 def test_commit(self):
-self.assert_no_active_commit()
-result = self.vm.qmp('block-commit', device='drive0', top='%s' % 
mid_img)
-self.assert_qmp(result, 'return', {})
-
-completed = False
-while not completed:
-for event in self.vm.get_qmp_events(wait=True):
-if event['event'] == 'BLOCK_JOB_COMPLETED':
-self.assert_qmp(event, 'data/type', 'commit')
-self.assert_qmp(event, 'data/device', 'drive0')
-self.assert_qmp(event, 'data/offset', self.image_len)
-self.assert_qmp(event, 'data/len', self.image_len)
-completed = True
-
-self.assert_no_active_commit()
-self.vm.shutdown()
-
+self.run_commit_test(mid_img, backing_img)
 self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', 
backing_img).find("verification failed"))
 self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', 
backing_img).find("verification failed"))
 
@@ -127,10 +133,9 @@ class TestSingleDrive(ImageCommitTestCase):
 self.assert_qmp(result, 'error/desc', 'Base \'badfile\' not found')
 
 def test_top_is_active(self):
-self.assert_no_active_commit()
-result = self.vm.qmp('block-commit', device='drive0', top='%s' % 
test_img, base='%s' % backing_img)
-self.assert_qmp(result, 'error/class', 'GenericError')
-self.assert_qmp(result, 'error/desc', 'Top image as the active layer 
is currently unsupported')
+self.run_commit_test(test_img, backing_img, True)
+self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', 
backing_img).find("verification failed"))
+self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', 
backing_img).find("verification failed"))
 
 def test_top_and_base_reversed(self):
 self.assert_no_active_commit()
@@ -191,23 +196,7 @@ class TestRelativePaths(ImageCommitTestCase):
 raise
 
 def test_commit(self):
-self.assert_no_active_commit()
-result = self.vm.qmp('block-commit', device='drive0', top='%s' % 
self.mid_img)
-self.assert_qmp(result, 'return', {})
-
-completed = False
-while not completed:
-for event in self.vm.get_qmp_events(wait=True):
-if event['event'] == 'BLOCK_JOB_COMPLETED':
-self.assert_qmp(event, 'data/type', 'commit')
-self.assert_qmp(event, 'data/device', 'drive0')
-self.assert_qmp(event, 'data/offset', self.image_len)
-self.assert_qmp(event, 'data/len', self.image_len)
-completed = True
-
-self.assert_no_active_commit()
-self.vm.shutdown()
-
+self.run_commit_test(self.mid_img, self.backing_img)
 self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', 
self.backing_img_abs).find("verification failed"))
 self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', 
self.backing_img_abs).find("verification failed"))
 
@@ -234,10 +223,9 @@ class TestRelativeP

[Qemu-devel] [PATCH v5 3/6] block: add commit_active_start()

2013-10-08 Thread Fam Zheng
commit_active_start is implemented in block/mirror.c, It will create a
job with "commit" type and designated base in block-commit command. This
will be used for committing active layer of device.

Sync mode is removed from MirrorBlockJob because there's no proper type
for commit. The used information is is_none_mode.

The common part of mirror_start and commit_active_start is moved to
mirror_start_job().

Fix the comment wording for commit_start.

Signed-off-by: Fam Zheng 
---
 block/mirror.c| 65 +++
 include/block/block_int.h | 22 +---
 2 files changed, 68 insertions(+), 19 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index cbdcc21..9be741a 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -32,7 +32,7 @@ typedef struct MirrorBlockJob {
 RateLimit limit;
 BlockDriverState *target;
 BlockDriverState *base;
-MirrorSyncMode mode;
+bool is_none_mode;
 BlockdevOnError on_source_error, on_target_error;
 bool synced;
 bool should_complete;
@@ -333,7 +333,7 @@ static void coroutine_fn mirror_run(void *opaque)
 sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
 mirror_free_init(s);
 
-if (s->mode != MIRROR_SYNC_MODE_NONE) {
+if (!s->is_none_mode) {
 /* First part, loop on the sectors and initialize the dirty bitmap.  */
 BlockDriverState *base = s->base;
 for (sector_num = 0; sector_num < end; ) {
@@ -532,15 +532,25 @@ static const BlockJobDriver mirror_job_driver = {
 .complete  = mirror_complete,
 };
 
-void mirror_start(BlockDriverState *bs, BlockDriverState *target,
-  int64_t speed, int64_t granularity, int64_t buf_size,
-  MirrorSyncMode mode, BlockdevOnError on_source_error,
-  BlockdevOnError on_target_error,
-  BlockDriverCompletionFunc *cb,
-  void *opaque, Error **errp)
+static const BlockJobDriver commit_active_job_driver = {
+.instance_size = sizeof(MirrorBlockJob),
+.job_type  = BLOCK_JOB_TYPE_COMMIT,
+.set_speed = mirror_set_speed,
+.iostatus_reset= mirror_iostatus_reset,
+.complete  = mirror_complete,
+};
+
+static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
+int64_t speed, int64_t granularity,
+int64_t buf_size,
+BlockdevOnError on_source_error,
+BlockdevOnError on_target_error,
+BlockDriverCompletionFunc *cb,
+void *opaque, Error **errp,
+const BlockJobDriver *driver,
+bool is_none_mode, BlockDriverState *base)
 {
 MirrorBlockJob *s;
-BlockDriverState *base = NULL;
 
 if (granularity == 0) {
 /* Choose the default granularity based on the target file's cluster
@@ -563,13 +573,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState 
*target,
 return;
 }
 
-if (mode == MIRROR_SYNC_MODE_TOP) {
-base = bs->backing_hd;
-} else {
-base = NULL;
-}
 
-s = block_job_create(&mirror_job_driver, bs, speed, cb, opaque, errp);
+s = block_job_create(driver, bs, speed, cb, opaque, errp);
 if (!s) {
 return;
 }
@@ -577,7 +582,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState 
*target,
 s->on_source_error = on_source_error;
 s->on_target_error = on_target_error;
 s->target = target;
-s->mode = mode;
+s->is_none_mode = is_none_mode;
 s->base = base;
 s->granularity = granularity;
 s->buf_size = MAX(buf_size, granularity);
@@ -590,3 +595,31 @@ void mirror_start(BlockDriverState *bs, BlockDriverState 
*target,
 trace_mirror_start(bs, s, s->common.co, opaque);
 qemu_coroutine_enter(s->common.co, s);
 }
+
+void mirror_start(BlockDriverState *bs, BlockDriverState *target,
+  int64_t speed, int64_t granularity, int64_t buf_size,
+  MirrorSyncMode mode, BlockdevOnError on_source_error,
+  BlockdevOnError on_target_error,
+  BlockDriverCompletionFunc *cb,
+  void *opaque, Error **errp)
+{
+bool is_none_mode;
+BlockDriverState *base;
+
+is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
+base = mode == MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL;
+mirror_start_job(bs, target, speed, granularity, buf_size,
+ on_source_error, on_target_error, cb, opaque, errp,
+ &mirror_job_driver, is_none_mode, base);
+}
+
+void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
+ int64_t speed,
+ BlockdevOnError on_error,
+ BlockDriverCompletionFunc *cb,
+ void *opaque, Error **errp)
+{
+mirror_start_job(bs, base, speed, 0, 0,
+  

[Qemu-devel] [PATCH v5 2/6] mirror: move base to MirrorBlockJob

2013-10-08 Thread Fam Zheng
This allows setting the base before entering mirror_run, commit will
make use of it.

Signed-off-by: Fam Zheng 
---
 block/mirror.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 01a795a..cbdcc21 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -31,6 +31,7 @@ typedef struct MirrorBlockJob {
 BlockJob common;
 RateLimit limit;
 BlockDriverState *target;
+BlockDriverState *base;
 MirrorSyncMode mode;
 BlockdevOnError on_source_error, on_target_error;
 bool synced;
@@ -334,8 +335,7 @@ static void coroutine_fn mirror_run(void *opaque)
 
 if (s->mode != MIRROR_SYNC_MODE_NONE) {
 /* First part, loop on the sectors and initialize the dirty bitmap.  */
-BlockDriverState *base;
-base = s->mode == MIRROR_SYNC_MODE_FULL ? NULL : bs->backing_hd;
+BlockDriverState *base = s->base;
 for (sector_num = 0; sector_num < end; ) {
 int64_t next = (sector_num | (sectors_per_chunk - 1)) + 1;
 ret = bdrv_is_allocated_above(bs, base,
@@ -540,6 +540,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState 
*target,
   void *opaque, Error **errp)
 {
 MirrorBlockJob *s;
+BlockDriverState *base = NULL;
 
 if (granularity == 0) {
 /* Choose the default granularity based on the target file's cluster
@@ -562,6 +563,12 @@ void mirror_start(BlockDriverState *bs, BlockDriverState 
*target,
 return;
 }
 
+if (mode == MIRROR_SYNC_MODE_TOP) {
+base = bs->backing_hd;
+} else {
+base = NULL;
+}
+
 s = block_job_create(&mirror_job_driver, bs, speed, cb, opaque, errp);
 if (!s) {
 return;
@@ -571,6 +578,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState 
*target,
 s->on_target_error = on_target_error;
 s->target = target;
 s->mode = mode;
+s->base = base;
 s->granularity = granularity;
 s->buf_size = MAX(buf_size, granularity);
 
-- 
1.8.3.1




[Qemu-devel] [PATCH v5 6/6] commit: remove unused check

2013-10-08 Thread Fam Zheng
We support top == active for commit now, remove the check and add an
assertion here.

Signed-off-by: Fam Zheng 
---
 block/commit.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/block/commit.c b/block/commit.c
index d4090cb..acec4ac 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -198,13 +198,7 @@ void commit_start(BlockDriverState *bs, BlockDriverState 
*base,
 return;
 }
 
-/* Once we support top == active layer, remove this check */
-if (top == bs) {
-error_setg(errp,
-   "Top image as the active layer is currently unsupported");
-return;
-}
-
+assert(top != bs);
 if (top == base) {
 error_setg(errp, "Invalid files for merge: top and base are the same");
 return;
-- 
1.8.3.1




[Qemu-devel] [PATCH v5 0/6] block: allow commit active as top

2013-10-08 Thread Fam Zheng
Previously live commit of active block device is not supported, this series
implements it and updates corresponding qemu-iotests cases.

This series is based on BlockJobType enum QAPI series.

v5: Address comments from Eric and Paolo:
Add mirror_start_job and front end wrapper. [Paolo]
Base on BlockJobType enum in QAPI. [Eric]
Drop "common" sync mode. [Eric]

v4: Rewrite to reuse block/mirror.c.
When committing the active layer, the job is internally a mirror job with
type name faked to "commit".
When the job completes, the BDSes are swapped, so the base image become
active and [top, base) dropped.


Fam Zheng (6):
  mirror: don't close target
  mirror: move base to MirrorBlockJob
  block: add commit_active_start()
  commit: support commit active layer
  qemu-iotests: update test cases for commit active
  commit: remove unused check

 block/commit.c|  8 +
 block/mirror.c| 77 +++
 blockdev.c|  9 --
 include/block/block_int.h | 22 --
 qapi-schema.json  |  5 +--
 tests/qemu-iotests/040| 73 +++-
 6 files changed, 125 insertions(+), 69 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH v5 4/6] commit: support commit active layer

2013-10-08 Thread Fam Zheng
If active is top, it will be mirrored to base, (with block/mirror.c
code), then the image is switched when user completes the block job.

QMP documentation is updated.

Signed-off-by: Fam Zheng 
---
 block/mirror.c   | 11 +++
 blockdev.c   |  9 +++--
 qapi-schema.json |  5 +++--
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 9be741a..86ffac8 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -478,6 +478,13 @@ immediate_exit:
 bdrv_reopen(s->target, bdrv_get_flags(s->common.bs), NULL);
 }
 bdrv_swap(s->target, s->common.bs);
+if (s->common.driver->job_type == BLOCK_JOB_TYPE_COMMIT) {
+/* drop the bs loop chain formed by the swap: break the loop then
+ * trigger the unref from the top one */
+BlockDriverState *p = s->base->backing_hd;
+s->base->backing_hd = NULL;
+bdrv_unref(p);
+}
 }
 bdrv_unref(s->target);
 block_job_completed(&s->common, ret);
@@ -619,6 +626,10 @@ void commit_active_start(BlockDriverState *bs, 
BlockDriverState *base,
  BlockDriverCompletionFunc *cb,
  void *opaque, Error **errp)
 {
+if (bdrv_reopen(base, bs->open_flags, errp)) {
+return;
+}
+bdrv_ref(base);
 mirror_start_job(bs, base, speed, 0, 0,
  on_error, on_error, cb, opaque, errp,
  &commit_active_job_driver, false, base);
diff --git a/blockdev.c b/blockdev.c
index 8aa66a9..77cbd1d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1710,8 +1710,13 @@ void qmp_block_commit(const char *device,
 return;
 }
 
-commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs,
-&local_err);
+if (top_bs == bs) {
+commit_active_start(bs, base_bs, speed, on_error, block_job_cb,
+bs, &local_err);
+} else {
+commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs,
+&local_err);
+}
 if (local_err != NULL) {
 error_propagate(errp, local_err);
 return;
diff --git a/qapi-schema.json b/qapi-schema.json
index 381ffbf..eb13707 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1910,9 +1910,11 @@
 #
 # @top:  The file name of the backing image within the image chain,
 #which contains the topmost data to be committed down.
-#Note, the active layer as 'top' is currently unsupported.
 #
 #If top == base, that is an error.
+#If top == active, the job will not be completed by itself,
+#user need to complete the job with block-job-complete
+#command after getting the ready event. (Since 1.7)
 #
 #
 # @speed:  #optional the maximum speed, in bytes per second
@@ -1922,7 +1924,6 @@
 #  If @device does not exist, DeviceNotFound
 #  If image commit is not supported by this device, NotSupported
 #  If @base or @top is invalid, a generic error is returned
-#  If @top is the active layer, or omitted, a generic error is returned
 #  If @speed is invalid, InvalidParameter
 #
 # Since: 1.3
-- 
1.8.3.1




[Qemu-devel] [PATCH v5 1/6] mirror: don't close target

2013-10-08 Thread Fam Zheng
Let reference count manage target and don't call bdrv_close here.

Signed-off-by: Fam Zheng 
---
 block/mirror.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index 7b95acf..01a795a 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -479,7 +479,6 @@ immediate_exit:
 }
 bdrv_swap(s->target, s->common.bs);
 }
-bdrv_close(s->target);
 bdrv_unref(s->target);
 block_job_completed(&s->common, ret);
 }
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH v3] Add interface to traverse the qmp command list by QmpCommand

2013-10-08 Thread Mark Wu
Thanks a lot for your all kind and helpful comments.  I would fix it by 
myself to save the time of maintainer :)
Then I have to beg your another review.BTW,  I would like to ask if 
there's good tools or practice to perform

an incremental review in qemu patch works.

Thanks.
Mark.


On Wed 09 Oct 2013 10:58:04 AM CST, Eric Blake wrote:

On 10/08/2013 08:39 PM, Mark Wu wrote:

In the original code, qmp_get_command_list is used to construct
a list of all commands' name. To get the information of all qga
commands, it traverses the name list and search the command info
with its name.  So it can cause O(n^2) in the number of commands.

This patch adds an interface to traverse the qmp command list by
QmpCommand to replace qmp_get_command_list. It can decrease the
complexity from O(n^2) to O(n).

Signed-off-by: Mark Wu 
---
v3:
 Add an accessor for cmd->name to avoid exposing internals of QmpCommand


As your two patches are related, I'd send them threaded under one cover
letter if you have to respin.  But maybe we don't need that...


v2:
1. Keep the signature of qmp_command_is_enabled (per Eric and Michael)
 2. Remove the unnecessary pointer castings (per Eric)

  include/qapi/qmp/dispatch.h |  6 ++--
  qapi/qmp-registry.c | 30 +-
  qga/commands.c  | 38 +--
  qga/main.c  | 75 ++---
  4 files changed, 57 insertions(+), 92 deletions(-)




-struct GuestAgentInfo *qmp_guest_info(Error **err)
+static void qmp_command_info(QmpCommand *cmd, void *opaque)
  {



-while (*cmd_list) {
-cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
-cmd_info->name = g_strdup(*cmd_list);
-cmd_info->enabled = qmp_command_is_enabled(cmd_info->name);
+cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
+cmd_info->name = g_strdup(cmd->name);


Oops, not using qmp_command_name().

Seems minor enough that if it is the only change after this much churn,
and the maintainer is willing to fix it on your behalf, I could live with:

Reviewed-by: Eric Blake 







Re: [Qemu-devel] [PATCH v5 2/5] hpet: entitle more irq pins for hpet

2013-10-08 Thread liu ping fan
On Mon, Sep 30, 2013 at 11:58 PM, Michael S. Tsirkin  wrote:
> On Mon, Sep 30, 2013 at 05:48:03PM +0200, Paolo Bonzini wrote:
>> Il 30/09/2013 11:30, Michael S. Tsirkin ha scritto:
>> > I was really only talking about q35 here.
>> > I thought it's ugly that users can control intcap
>> > directly. Can object_set_property be used after
>> > qdev_try_create?
>>
>> Yes, after that and before qdev_init.  This is how Ping Fan is doing
>> PIIX right now.
>>
>> > PIIX has another issue:
>> > the default value in hpet is really Q35 specific,
>> > that's also kind of ugly, isn't it?
>>
>> Yeah, perhaps it is.  So are you suggesting the default to be 4 (GSI2 only)?
>>
>> Paolo
>
> I suggest it fails unless caller set the property.
>
Sorry, out of office for a long time, and did not keep up with this
thread in time.
When letting the caller set the intcap, we should consider the
compatibility of q35. For pc-q35-1.7 or later, the caller should set
the property, otherwise not. But how can the caller tell that it runs
on q35-1.7?
The essential problem is that "set the property" will always overwrite
the property which is set up by compatible mechanism. So it is hard to
implement without breaking the current mechanism. Do you think so?

Thanks and regards,
Ping fan



[Qemu-devel] [PATCH v4] Add interface to traverse the qmp command list by QmpCommand

2013-10-08 Thread Mark Wu
In the original code, qmp_get_command_list is used to construct
a list of all commands' name. To get the information of all qga
commands, it traverses the name list and search the command info
with its name.  So it can cause O(n^2) in the number of commands.

This patch adds an interface to traverse the qmp command list by
QmpCommand to replace qmp_get_command_list. It can decrease the
complexity from O(n^2) to O(n).

Signed-off-by: Mark Wu 
---
v4:
Add the missing change from cmd->namd to qmp_command_name(cmd)
v3:
Add an accessor for cmd->name to avoid exposing internals of QmpCommand
v2:
1. Keep the signature of qmp_command_is_enabled (per Eric and Michael)
2. Remove the unnecessary pointer castings (per Eric)
 include/qapi/qmp/dispatch.h |  6 ++--
 qapi/qmp-registry.c | 30 +-
 qga/commands.c  | 38 +--
 qga/main.c  | 75 ++---
 4 files changed, 57 insertions(+), 92 deletions(-)

diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 1ce11f5..7d759ef 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -47,9 +47,11 @@ QmpCommand *qmp_find_command(const char *name);
 QObject *qmp_dispatch(QObject *request);
 void qmp_disable_command(const char *name);
 void qmp_enable_command(const char *name);
-bool qmp_command_is_enabled(const char *name);
-char **qmp_get_command_list(void);
+bool qmp_command_is_enabled(const QmpCommand *cmd);
+const char *qmp_command_name(const QmpCommand *cmd);
 QObject *qmp_build_error_object(Error *errp);
+typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque);
+void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque);
 
 #endif
 
diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
index 28bbbe8..5e26710 100644
--- a/qapi/qmp-registry.c
+++ b/qapi/qmp-registry.c
@@ -66,35 +66,21 @@ void qmp_enable_command(const char *name)
 qmp_toggle_command(name, true);
 }
 
-bool qmp_command_is_enabled(const char *name)
+bool qmp_command_is_enabled(const QmpCommand *cmd)
 {
-QmpCommand *cmd;
-
-QTAILQ_FOREACH(cmd, &qmp_commands, node) {
-if (strcmp(cmd->name, name) == 0) {
-return cmd->enabled;
-}
-}
+return cmd->enabled;
+}
 
-return false;
+const char *qmp_command_name(const QmpCommand *cmd)
+{
+return cmd->name;
 }
 
-char **qmp_get_command_list(void)
+void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque)
 {
 QmpCommand *cmd;
-int count = 1;
-char **list_head, **list;
-
-QTAILQ_FOREACH(cmd, &qmp_commands, node) {
-count++;
-}
-
-list_head = list = g_malloc0(count * sizeof(char *));
 
 QTAILQ_FOREACH(cmd, &qmp_commands, node) {
-*list = g_strdup(cmd->name);
-list++;
+fn(cmd, opaque);
 }
-
-return list_head;
 }
diff --git a/qga/commands.c b/qga/commands.c
index 528b082..e87cbf8 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -45,35 +45,27 @@ void qmp_guest_ping(Error **err)
 slog("guest-ping called");
 }
 
-struct GuestAgentInfo *qmp_guest_info(Error **err)
+static void qmp_command_info(QmpCommand *cmd, void *opaque)
 {
-GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
+GuestAgentInfo *info = opaque;
 GuestAgentCommandInfo *cmd_info;
 GuestAgentCommandInfoList *cmd_info_list;
-char **cmd_list_head, **cmd_list;
-
-info->version = g_strdup(QEMU_VERSION);
-
-cmd_list_head = cmd_list = qmp_get_command_list();
-if (*cmd_list_head == NULL) {
-goto out;
-}
 
-while (*cmd_list) {
-cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
-cmd_info->name = g_strdup(*cmd_list);
-cmd_info->enabled = qmp_command_is_enabled(cmd_info->name);
+cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
+cmd_info->name = g_strdup(qmp_command_name(cmd));
+cmd_info->enabled = qmp_command_is_enabled(cmd);
 
-cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
-cmd_info_list->value = cmd_info;
-cmd_info_list->next = info->supported_commands;
-info->supported_commands = cmd_info_list;
+cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
+cmd_info_list->value = cmd_info;
+cmd_info_list->next = info->supported_commands;
+info->supported_commands = cmd_info_list;
+}
 
-g_free(*cmd_list);
-cmd_list++;
-}
+struct GuestAgentInfo *qmp_guest_info(Error **err)
+{
+GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
 
-out:
-g_free(cmd_list_head);
+info->version = g_strdup(QEMU_VERSION);
+qmp_for_each_command(qmp_command_info, info);
 return info;
 }
diff --git a/qga/main.c b/qga/main.c
index 6c746c8..c58b26a 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -347,48 +347,35 @@ static gint ga_strcmp(gconstpointer str1, gconstpointer 
str2)
 }
 
 /* disable commands that aren't safe for fsfreeze */
-stat

Re: [Qemu-devel] [PATCH v3] Add interface to traverse the qmp command list by QmpCommand

2013-10-08 Thread Eric Blake
On 10/08/2013 08:39 PM, Mark Wu wrote:
> In the original code, qmp_get_command_list is used to construct
> a list of all commands' name. To get the information of all qga
> commands, it traverses the name list and search the command info
> with its name.  So it can cause O(n^2) in the number of commands.
> 
> This patch adds an interface to traverse the qmp command list by
> QmpCommand to replace qmp_get_command_list. It can decrease the
> complexity from O(n^2) to O(n).
> 
> Signed-off-by: Mark Wu 
> ---
> v3:
> Add an accessor for cmd->name to avoid exposing internals of QmpCommand

As your two patches are related, I'd send them threaded under one cover
letter if you have to respin.  But maybe we don't need that...

> v2:
>   1. Keep the signature of qmp_command_is_enabled (per Eric and Michael)
> 2. Remove the unnecessary pointer castings (per Eric)
> 
>  include/qapi/qmp/dispatch.h |  6 ++--
>  qapi/qmp-registry.c | 30 +-
>  qga/commands.c  | 38 +--
>  qga/main.c  | 75 
> ++---
>  4 files changed, 57 insertions(+), 92 deletions(-)
> 

> -struct GuestAgentInfo *qmp_guest_info(Error **err)
> +static void qmp_command_info(QmpCommand *cmd, void *opaque)
>  {

> -while (*cmd_list) {
> -cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
> -cmd_info->name = g_strdup(*cmd_list);
> -cmd_info->enabled = qmp_command_is_enabled(cmd_info->name);
> +cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
> +cmd_info->name = g_strdup(cmd->name);

Oops, not using qmp_command_name().

Seems minor enough that if it is the only change after this much churn,
and the maintainer is willing to fix it on your behalf, I could live with:

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v5] Extend qemu-ga's 'guest-info' command to expose flag 'success-response'

2013-10-08 Thread Eric Blake
On 10/08/2013 08:37 PM, Mark Wu wrote:
> Now we have several qemu-ga commands not returning response on success.
> It has been documented in qga/qapi-schema.json already. This patch exposes
> the 'success-response' flag by extending 'guest-info' command. With this
> change, the clients can handle the command response more flexibly.
> 
> Signed-off-by: Mark Wu 
> ---
> v5:
> Fix a tab indent and rebase 
> v4: 
> Add signature of qmp_has_success_response per Michael.
> v3: 
>   1. treat cmd->options as a bitmask instead of single option (per Eric) 
>   2. rebase on the patch " Add interface to traverse the qmp command list
> by QmpCommand" to avoid the O(n2) problem (per Eric and Michael)
> v2: 
> add the notation 'since 1.7' to the option 'success-response'
> (per Eric Blake's comments)
> 
>  include/qapi/qmp/dispatch.h | 1 +
>  qapi/qmp-registry.c | 5 +
>  qga/commands.c  | 1 +
>  qga/qapi-schema.json| 5 -
>  4 files changed, 11 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 1/2] Add interface to traverse the qmp command list by QmpCommand

2013-10-08 Thread Mark Wu

On Wed 09 Oct 2013 10:36:21 AM CST, Mark Wu wrote:

In the original code, qmp_get_command_list is used to construct
a list of all commands' name. To get the information of all qga
commands, it traverses the name list and search the command info
with its name.  So it can cause O(n^2) in the number of commands.

This patch adds an interface to traverse the qmp command list by
QmpCommand to replace qmp_get_command_list. It can decrease the
complexity from O(n^2) to O(n).

Signed-off-by: Mark Wu 
---
v3:
 Add an accessor for cmd->name to avoid exposing internals of QmpCommand
v2:
1. Keep the signature of qmp_command_is_enabled (per Eric and Michael)
 2. Remove the unnecessary pointer castings (per Eric)

  include/qapi/qmp/dispatch.h |  6 ++--
  qapi/qmp-registry.c | 30 +-
  qga/commands.c  | 38 +--
  qga/main.c  | 75 ++---
  4 files changed, 57 insertions(+), 92 deletions(-)

diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 1ce11f5..7d759ef 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -47,9 +47,11 @@ QmpCommand *qmp_find_command(const char *name);
  QObject *qmp_dispatch(QObject *request);
  void qmp_disable_command(const char *name);
  void qmp_enable_command(const char *name);
-bool qmp_command_is_enabled(const char *name);
-char **qmp_get_command_list(void);
+bool qmp_command_is_enabled(const QmpCommand *cmd);
+const char *qmp_command_name(const QmpCommand *cmd);
  QObject *qmp_build_error_object(Error *errp);
+typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque);
+void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque);

  #endif

diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
index 28bbbe8..5e26710 100644
--- a/qapi/qmp-registry.c
+++ b/qapi/qmp-registry.c
@@ -66,35 +66,21 @@ void qmp_enable_command(const char *name)
  qmp_toggle_command(name, true);
  }

-bool qmp_command_is_enabled(const char *name)
+bool qmp_command_is_enabled(const QmpCommand *cmd)
  {
-QmpCommand *cmd;
-
-QTAILQ_FOREACH(cmd, &qmp_commands, node) {
-if (strcmp(cmd->name, name) == 0) {
-return cmd->enabled;
-}
-}
+return cmd->enabled;
+}

-return false;
+const char *qmp_command_name(const QmpCommand *cmd)
+{
+return cmd->name;
  }

-char **qmp_get_command_list(void)
+void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque)
  {
  QmpCommand *cmd;
-int count = 1;
-char **list_head, **list;
-
-QTAILQ_FOREACH(cmd, &qmp_commands, node) {
-count++;
-}
-
-list_head = list = g_malloc0(count * sizeof(char *));

  QTAILQ_FOREACH(cmd, &qmp_commands, node) {
-*list = g_strdup(cmd->name);
-list++;
+fn(cmd, opaque);
  }
-
-return list_head;
  }
diff --git a/qga/commands.c b/qga/commands.c
index 528b082..063b22b 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -45,35 +45,27 @@ void qmp_guest_ping(Error **err)
  slog("guest-ping called");
  }

-struct GuestAgentInfo *qmp_guest_info(Error **err)
+static void qmp_command_info(QmpCommand *cmd, void *opaque)
  {
-GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
+GuestAgentInfo *info = opaque;
  GuestAgentCommandInfo *cmd_info;
  GuestAgentCommandInfoList *cmd_info_list;
-char **cmd_list_head, **cmd_list;
-
-info->version = g_strdup(QEMU_VERSION);
-
-cmd_list_head = cmd_list = qmp_get_command_list();
-if (*cmd_list_head == NULL) {
-goto out;
-}

-while (*cmd_list) {
-cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
-cmd_info->name = g_strdup(*cmd_list);
-cmd_info->enabled = qmp_command_is_enabled(cmd_info->name);
+cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
+cmd_info->name = g_strdup(cmd->name);
+cmd_info->enabled = qmp_command_is_enabled(cmd);

-cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
-cmd_info_list->value = cmd_info;
-cmd_info_list->next = info->supported_commands;
-info->supported_commands = cmd_info_list;
+cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
+cmd_info_list->value = cmd_info;
+cmd_info_list->next = info->supported_commands;
+info->supported_commands = cmd_info_list;
+}

-g_free(*cmd_list);
-cmd_list++;
-}
+struct GuestAgentInfo *qmp_guest_info(Error **err)
+{
+GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));

-out:
-g_free(cmd_list_head);
+info->version = g_strdup(QEMU_VERSION);
+qmp_for_each_command(qmp_command_info, info);
  return info;
  }
diff --git a/qga/main.c b/qga/main.c
index 6c746c8..c58b26a 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -347,48 +347,35 @@ static gint ga_strcmp(gconstpointer str1, gconstpointer 
str2)
  }

  /* disable commands that aren't safe for fsfreeze */
-static void ga_disa

[Qemu-devel] [PATCH v3] Add interface to traverse the qmp command list by QmpCommand

2013-10-08 Thread Mark Wu
In the original code, qmp_get_command_list is used to construct
a list of all commands' name. To get the information of all qga
commands, it traverses the name list and search the command info
with its name.  So it can cause O(n^2) in the number of commands.

This patch adds an interface to traverse the qmp command list by
QmpCommand to replace qmp_get_command_list. It can decrease the
complexity from O(n^2) to O(n).

Signed-off-by: Mark Wu 
---
v3:
Add an accessor for cmd->name to avoid exposing internals of QmpCommand
v2:
1. Keep the signature of qmp_command_is_enabled (per Eric and Michael)
2. Remove the unnecessary pointer castings (per Eric)

 include/qapi/qmp/dispatch.h |  6 ++--
 qapi/qmp-registry.c | 30 +-
 qga/commands.c  | 38 +--
 qga/main.c  | 75 ++---
 4 files changed, 57 insertions(+), 92 deletions(-)

diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 1ce11f5..7d759ef 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -47,9 +47,11 @@ QmpCommand *qmp_find_command(const char *name);
 QObject *qmp_dispatch(QObject *request);
 void qmp_disable_command(const char *name);
 void qmp_enable_command(const char *name);
-bool qmp_command_is_enabled(const char *name);
-char **qmp_get_command_list(void);
+bool qmp_command_is_enabled(const QmpCommand *cmd);
+const char *qmp_command_name(const QmpCommand *cmd);
 QObject *qmp_build_error_object(Error *errp);
+typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque);
+void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque);
 
 #endif
 
diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
index 28bbbe8..5e26710 100644
--- a/qapi/qmp-registry.c
+++ b/qapi/qmp-registry.c
@@ -66,35 +66,21 @@ void qmp_enable_command(const char *name)
 qmp_toggle_command(name, true);
 }
 
-bool qmp_command_is_enabled(const char *name)
+bool qmp_command_is_enabled(const QmpCommand *cmd)
 {
-QmpCommand *cmd;
-
-QTAILQ_FOREACH(cmd, &qmp_commands, node) {
-if (strcmp(cmd->name, name) == 0) {
-return cmd->enabled;
-}
-}
+return cmd->enabled;
+}
 
-return false;
+const char *qmp_command_name(const QmpCommand *cmd)
+{
+return cmd->name;
 }
 
-char **qmp_get_command_list(void)
+void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque)
 {
 QmpCommand *cmd;
-int count = 1;
-char **list_head, **list;
-
-QTAILQ_FOREACH(cmd, &qmp_commands, node) {
-count++;
-}
-
-list_head = list = g_malloc0(count * sizeof(char *));
 
 QTAILQ_FOREACH(cmd, &qmp_commands, node) {
-*list = g_strdup(cmd->name);
-list++;
+fn(cmd, opaque);
 }
-
-return list_head;
 }
diff --git a/qga/commands.c b/qga/commands.c
index 528b082..063b22b 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -45,35 +45,27 @@ void qmp_guest_ping(Error **err)
 slog("guest-ping called");
 }
 
-struct GuestAgentInfo *qmp_guest_info(Error **err)
+static void qmp_command_info(QmpCommand *cmd, void *opaque)
 {
-GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
+GuestAgentInfo *info = opaque;
 GuestAgentCommandInfo *cmd_info;
 GuestAgentCommandInfoList *cmd_info_list;
-char **cmd_list_head, **cmd_list;
-
-info->version = g_strdup(QEMU_VERSION);
-
-cmd_list_head = cmd_list = qmp_get_command_list();
-if (*cmd_list_head == NULL) {
-goto out;
-}
 
-while (*cmd_list) {
-cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
-cmd_info->name = g_strdup(*cmd_list);
-cmd_info->enabled = qmp_command_is_enabled(cmd_info->name);
+cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
+cmd_info->name = g_strdup(cmd->name);
+cmd_info->enabled = qmp_command_is_enabled(cmd);
 
-cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
-cmd_info_list->value = cmd_info;
-cmd_info_list->next = info->supported_commands;
-info->supported_commands = cmd_info_list;
+cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
+cmd_info_list->value = cmd_info;
+cmd_info_list->next = info->supported_commands;
+info->supported_commands = cmd_info_list;
+}
 
-g_free(*cmd_list);
-cmd_list++;
-}
+struct GuestAgentInfo *qmp_guest_info(Error **err)
+{
+GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
 
-out:
-g_free(cmd_list_head);
+info->version = g_strdup(QEMU_VERSION);
+qmp_for_each_command(qmp_command_info, info);
 return info;
 }
diff --git a/qga/main.c b/qga/main.c
index 6c746c8..c58b26a 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -347,48 +347,35 @@ static gint ga_strcmp(gconstpointer str1, gconstpointer 
str2)
 }
 
 /* disable commands that aren't safe for fsfreeze */
-static void ga_disable_non_whitelisted(void)
+static void ga_disable_non_whitelisted(QmpCo

[Qemu-devel] [PATCH v5] Extend qemu-ga's 'guest-info' command to expose flag 'success-response'

2013-10-08 Thread Mark Wu
Now we have several qemu-ga commands not returning response on success.
It has been documented in qga/qapi-schema.json already. This patch exposes
the 'success-response' flag by extending 'guest-info' command. With this
change, the clients can handle the command response more flexibly.

Signed-off-by: Mark Wu 
---
v5:
Fix a tab indent and rebase 
v4: 
Add signature of qmp_has_success_response per Michael.
v3: 
1. treat cmd->options as a bitmask instead of single option (per Eric) 
2. rebase on the patch " Add interface to traverse the qmp command list
by QmpCommand" to avoid the O(n2) problem (per Eric and Michael)
v2: 
add the notation 'since 1.7' to the option 'success-response'
(per Eric Blake's comments)

 include/qapi/qmp/dispatch.h | 1 +
 qapi/qmp-registry.c | 5 +
 qga/commands.c  | 1 +
 qga/qapi-schema.json| 5 -
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 7d759ef..cea3818 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -49,6 +49,7 @@ void qmp_disable_command(const char *name);
 void qmp_enable_command(const char *name);
 bool qmp_command_is_enabled(const QmpCommand *cmd);
 const char *qmp_command_name(const QmpCommand *cmd);
+bool qmp_has_success_response(const QmpCommand *cmd);
 QObject *qmp_build_error_object(Error *errp);
 typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque);
 void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque);
diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
index 5e26710..3e4498a 100644
--- a/qapi/qmp-registry.c
+++ b/qapi/qmp-registry.c
@@ -76,6 +76,11 @@ const char *qmp_command_name(const QmpCommand *cmd)
 return cmd->name;
 }
 
+bool qmp_has_success_response(const QmpCommand *cmd)
+{
+return !(cmd->options & QCO_NO_SUCCESS_RESP);
+}
+
 void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque)
 {
 QmpCommand *cmd;
diff --git a/qga/commands.c b/qga/commands.c
index 063b22b..7f089ba 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -54,6 +54,7 @@ static void qmp_command_info(QmpCommand *cmd, void *opaque)
 cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
 cmd_info->name = g_strdup(cmd->name);
 cmd_info->enabled = qmp_command_is_enabled(cmd);
+cmd_info->success_response = qmp_has_success_response(cmd);
 
 cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
 cmd_info_list->value = cmd_info;
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 7155b7a..245f968 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -141,10 +141,13 @@
 #
 # @enabled: whether command is currently enabled by guest admin
 #
+# @success-response: whether command returns a response on success
+#(since 1.7)
+#
 # Since 1.1.0
 ##
 { 'type': 'GuestAgentCommandInfo',
-  'data': { 'name': 'str', 'enabled': 'bool' } }
+  'data': { 'name': 'str', 'enabled': 'bool', 'success-response': 'bool' } }
 
 ##
 # @GuestAgentInfo
-- 
1.8.3.1




[Qemu-devel] [PATCH 1/2] Add interface to traverse the qmp command list by QmpCommand

2013-10-08 Thread Mark Wu
In the original code, qmp_get_command_list is used to construct
a list of all commands' name. To get the information of all qga
commands, it traverses the name list and search the command info
with its name.  So it can cause O(n^2) in the number of commands.

This patch adds an interface to traverse the qmp command list by
QmpCommand to replace qmp_get_command_list. It can decrease the
complexity from O(n^2) to O(n).

Signed-off-by: Mark Wu 
---
v3:
Add an accessor for cmd->name to avoid exposing internals of QmpCommand
v2:
1. Keep the signature of qmp_command_is_enabled (per Eric and Michael)
2. Remove the unnecessary pointer castings (per Eric)

 include/qapi/qmp/dispatch.h |  6 ++--
 qapi/qmp-registry.c | 30 +-
 qga/commands.c  | 38 +--
 qga/main.c  | 75 ++---
 4 files changed, 57 insertions(+), 92 deletions(-)

diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 1ce11f5..7d759ef 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -47,9 +47,11 @@ QmpCommand *qmp_find_command(const char *name);
 QObject *qmp_dispatch(QObject *request);
 void qmp_disable_command(const char *name);
 void qmp_enable_command(const char *name);
-bool qmp_command_is_enabled(const char *name);
-char **qmp_get_command_list(void);
+bool qmp_command_is_enabled(const QmpCommand *cmd);
+const char *qmp_command_name(const QmpCommand *cmd);
 QObject *qmp_build_error_object(Error *errp);
+typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque);
+void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque);
 
 #endif
 
diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
index 28bbbe8..5e26710 100644
--- a/qapi/qmp-registry.c
+++ b/qapi/qmp-registry.c
@@ -66,35 +66,21 @@ void qmp_enable_command(const char *name)
 qmp_toggle_command(name, true);
 }
 
-bool qmp_command_is_enabled(const char *name)
+bool qmp_command_is_enabled(const QmpCommand *cmd)
 {
-QmpCommand *cmd;
-
-QTAILQ_FOREACH(cmd, &qmp_commands, node) {
-if (strcmp(cmd->name, name) == 0) {
-return cmd->enabled;
-}
-}
+return cmd->enabled;
+}
 
-return false;
+const char *qmp_command_name(const QmpCommand *cmd)
+{
+return cmd->name;
 }
 
-char **qmp_get_command_list(void)
+void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque)
 {
 QmpCommand *cmd;
-int count = 1;
-char **list_head, **list;
-
-QTAILQ_FOREACH(cmd, &qmp_commands, node) {
-count++;
-}
-
-list_head = list = g_malloc0(count * sizeof(char *));
 
 QTAILQ_FOREACH(cmd, &qmp_commands, node) {
-*list = g_strdup(cmd->name);
-list++;
+fn(cmd, opaque);
 }
-
-return list_head;
 }
diff --git a/qga/commands.c b/qga/commands.c
index 528b082..063b22b 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -45,35 +45,27 @@ void qmp_guest_ping(Error **err)
 slog("guest-ping called");
 }
 
-struct GuestAgentInfo *qmp_guest_info(Error **err)
+static void qmp_command_info(QmpCommand *cmd, void *opaque)
 {
-GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
+GuestAgentInfo *info = opaque;
 GuestAgentCommandInfo *cmd_info;
 GuestAgentCommandInfoList *cmd_info_list;
-char **cmd_list_head, **cmd_list;
-
-info->version = g_strdup(QEMU_VERSION);
-
-cmd_list_head = cmd_list = qmp_get_command_list();
-if (*cmd_list_head == NULL) {
-goto out;
-}
 
-while (*cmd_list) {
-cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
-cmd_info->name = g_strdup(*cmd_list);
-cmd_info->enabled = qmp_command_is_enabled(cmd_info->name);
+cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
+cmd_info->name = g_strdup(cmd->name);
+cmd_info->enabled = qmp_command_is_enabled(cmd);
 
-cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
-cmd_info_list->value = cmd_info;
-cmd_info_list->next = info->supported_commands;
-info->supported_commands = cmd_info_list;
+cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
+cmd_info_list->value = cmd_info;
+cmd_info_list->next = info->supported_commands;
+info->supported_commands = cmd_info_list;
+}
 
-g_free(*cmd_list);
-cmd_list++;
-}
+struct GuestAgentInfo *qmp_guest_info(Error **err)
+{
+GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
 
-out:
-g_free(cmd_list_head);
+info->version = g_strdup(QEMU_VERSION);
+qmp_for_each_command(qmp_command_info, info);
 return info;
 }
diff --git a/qga/main.c b/qga/main.c
index 6c746c8..c58b26a 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -347,48 +347,35 @@ static gint ga_strcmp(gconstpointer str1, gconstpointer 
str2)
 }
 
 /* disable commands that aren't safe for fsfreeze */
-static void ga_disable_non_whitelisted(void)
+static void ga_disable_non_whitelisted(QmpCo

Re: [Qemu-devel] [PATCH v2] Fix pc migration from qemu <= 1.5

2013-10-08 Thread Bandan Das
Cole Robinson  writes:

> The following commit introduced a migration incompatibility:
>
> commit 568f0690fd9aa4d39d84b04c1a5dbb53a915c3fe
> Author: David Gibson 
> Date:   Thu Jun 6 18:48:49 2013 +1000
>
> pci: Replace pci_find_domain() with more general pci_root_bus_path()
>
> The issue is that i440fx savevm idstr went from :00:00.0/I440FX to
> :00.0/I440FX. Unfortunately we are stuck with the breakage for
> 1.6 machine types.
>
> Add a compat property to maintain the busted idstr for the 1.6 machine
> types, but revert to the old style format for 1.7+, and <= 1.5.
>
> Tested with migration from qemu 1.5, qemu 1.6, and qemu.git.
>
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Cole Robinson 
> ---

Reviewed-by: Bandan Das 

> v2:
> Drop needless 1.7 compat bits
>
>  hw/pci-host/piix.c|  9 -
>  hw/pci-host/q35.c | 10 --
>  include/hw/i386/pc.h  | 16 
>  include/hw/pci-host/q35.h |  1 +
>  4 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index c041149..9dafe80 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -48,6 +48,7 @@ typedef struct I440FXState {
>  PCIHostState parent_obj;
>  PcPciInfo pci_info;
>  uint64_t pci_hole64_size;
> +uint32_t short_root_bus;
>  } I440FXState;
>  
>  #define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
> @@ -712,13 +713,19 @@ static const TypeInfo i440fx_info = {
>  static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
>  PCIBus *rootbus)
>  {
> +I440FXState *s = I440FX_PCI_HOST_BRIDGE(host_bridge);
> +
>  /* For backwards compat with old device paths */
> -return "";
> +if (s->short_root_bus) {
> +return "";
> +}
> +return ":00";
>  }
>  
>  static Property i440fx_props[] = {
>  DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, I440FXState,
>   pci_hole64_size, DEFAULT_PCI_HOLE64_SIZE),
> +DEFINE_PROP_UINT32("short_root_bus", I440FXState, short_root_bus, 0),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index ad703a4..cb3abfd 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -61,8 +61,13 @@ static void q35_host_realize(DeviceState *dev, Error 
> **errp)
>  static const char *q35_host_root_bus_path(PCIHostState *host_bridge,
>PCIBus *rootbus)
>  {
> -/* For backwards compat with old device paths */
> -return "";
> +Q35PCIHost *s = Q35_HOST_DEVICE(host_bridge);
> +
> + /* For backwards compat with old device paths */
> +if (s->mch.short_root_bus) {
> +return "";
> +}
> +return ":00";
>  }
>  
>  static void q35_host_get_pci_hole_start(Object *obj, Visitor *v,
> @@ -114,6 +119,7 @@ static Property mch_props[] = {
>  MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT),
>  DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, Q35PCIHost,
>   mch.pci_hole64_size, DEFAULT_PCI_HOLE64_SIZE),
> +DEFINE_PROP_UINT32("short_root_bus", Q35PCIHost, mch.short_root_bus, 0),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 9b2ddc4..79f6934 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -230,6 +230,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>  .driver   = "e1000",\
>  .property = "mitigation",\
>  .value= "off",\
> +},{\
> +.driver   = "i440FX-pcihost",\
> +.property = "short_root_bus",\
> +.value= stringify(1),\
> +},{\
> +.driver   = "mch",\
> +.property = "short_root_bus",\
> +.value= stringify(1),\
>  }
>  
>  #define PC_COMPAT_1_5 \
> @@ -266,6 +274,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>  .driver = TYPE_X86_CPU,\
>  .property = "pmu",\
>  .value = "on",\
> +},{\
> +.driver   = "i440FX-pcihost",\
> +.property = "short_root_bus",\
> +.value= stringify(0),\
> +},{\
> +.driver   = "mch",\
> +.property = "short_root_bus",\
> +.value= stringify(0),\
>  }
>  
>  #define PC_COMPAT_1_4 \
> diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
> index 56de92e..c8362b9 100644
> --- a/include/hw/pci-host/q35.h
> +++ b/include/hw/pci-host/q35.h
> @@ -61,6 +61,7 @@ typedef struct MCHPCIState {
>  ram_addr_t above_4g_mem_size;
>  uint64_t pci_hole64_size;
>  PcGuestInfo *guest_info;
> +uint32_t short_root_bus;
>  } MCHPCIState;
>  
>  typedef struct Q35PCIHost {



Re: [Qemu-devel] [PATCHv3 1/3] seccomp: adding blacklist support

2013-10-08 Thread Eric Blake
On 10/08/2013 06:42 PM, Eduardo Otubo wrote:
> v3: The "-netdev tap" option is checked in the vl.c file during the
> process of the command line argument list. It sets tap_enabled to true
> or false according to the configuration found. Later at the seccomp
> filter installation, this value is checked wheter to install or not this

s/wheter/whether/

> feature.
> 
> Adding a system call blacklist right before the vcpus starts. This
> filter is composed by the system calls that can't be executed after the
> guests are up. This list should be refined as whitelist is, with as much
> testing as we can do using virt-test.
> 
> Signed-off-by: Eduardo Otubo 
> ---
>  include/sysemu/seccomp.h |  6 -
>  qemu-seccomp.c   | 64 
> +++-
>  vl.c | 21 +++-
>  3 files changed, 77 insertions(+), 14 deletions(-)

No review on the actual patch, just spotting a typo.


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] Ensure PCIR is aligned to 4 bytes

2013-10-08 Thread Brad Smith

On 25/09/13 7:24 PM, Brad Smith wrote:

On 21/09/13 12:38 PM, Sebastian Herbszt wrote:

Brad Smith wrote:

On 19/09/13 12:53 PM, Sebastian Herbszt wrote:

Brad Smith wrote:

On 20/01/13 1:12 PM, David Woodhouse wrote:

The PCI Firmware Specification apparently requires that the PCI
Data Structure be DWORD-aligned. The implementation in OVMF also
requires this, so vgabios ROMs don't work there. With this fixed,
I can now initialise the VGA ROM from EFI, and EFI can display
using INT 10h services.

--- vgabios-0.6c/vgabios.c.orig2013-01-20
11:33:36.138548472 -0600 +++ vgabios-0.6c/vgabios.c
2013-01-20 11:36:26.060270163 -0600 @@ -204,6 +204,7 @@
vgabios_website: .byte0x00

#ifdef PCIBIOS
+.align 4 // DWORD alignment required by PCI Firmware
Specification vgabios_pci_data:
.ascii "PCIR"
#ifdef CIRRUS


We have had this in the OpenBSD port of QEMU for awhile now. Is it
possible to have this reviewed and commited?


This change was commited to upstream vgabios back in February [1].


But that has not resulted in it being brought into QEMU.


Gerd, Anthony, care to update QEMU's vgabios repository [1] with
changes from upstream CVS repository [2]?

[1] http://git.qemu.org/?p=vgabios.git
[2] http://cvs.savannah.gnu.org/viewvc/?root=vgabios


Any comment?


ping.


--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.




[Qemu-devel] [Bug 1236809] Re: qemu-system-x86_64 takes 100% CPU

2013-10-08 Thread chenlidong
i find out the reason.

because i used the --enable-debug option for the latest upstream.

so this is not a bug.

** Changed in: qemu
   Status: New => Invalid

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1236809

Title:
  qemu-system-x86_64 takes 100% CPU

Status in QEMU:
  Invalid

Bug description:
  I have rhel6 inside qemu VM. qemu process starts to take CPU cycles
  and OS inside VM is very slow and sluggish.

  the qemu version is the latest upstream git.

  the kernel version is 3.12.0.

  linux-0rsg:/home/chenlidong # uname -a
  Linux linux-0rsg 3.12.0-rc1-1.16-desktop+ #5 SMP PREEMPT Sun Sep 22 22:07:40 
EDT 2013 x86_64 x86_64 x86_64 GNU/Linux

  linux-0rsg:/home/chenlidong # qemu-system-x86_64 --version
  QEMU emulator version 1.6.50, Copyright (c) 2003-2008 Fabrice Bellard

  the command line of qemu is below:

  chenlidong@linux-0rsg:~/develop/qemu> ps -ef | grep qemu
  root 19030 1 14 19:00 ?00:04:24 
/usr/local/bin/qemu-system-x86_64 -name rhel6 -S -M pc-i440fx-1.6 -m 2048 -smp 
1,sockets=1,cores=1,threads=1 -uuid 1925a96a-54b9-3c4a-dda0-6b42fdd0af2c 
-no-user-config -nodefaults -chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/rhel6.monitor,server,nowait 
-mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown 
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive 
file=/var/lib/libvirt/images/rhel6.img,if=none,id=drive-ide0-0-0,format=raw,cache=directsync
 -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=2 
-drive if=none,id=drive-ide0-1-0,readonly=on,format=raw -device 
ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,bootindex=1 -netdev 
tap,fd=21,id=hostnet0 -device 
rtl8139,netdev=hostnet0,id=net0,mac=52:54:00:b3:b8:53,bus=pci.0,addr=0x3 
-chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 
-device usb-tablet,id=input0 -vnc 127.0.0.1:0 -vga cirrus -device 
intel-hda,id=sound0,bus=pci.0,addr=0x4 -device 
hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -device 
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5

  i used perf top, the result is below:
  Samples: 57K of event 'cycles', Event count (approx.): 26336443124

   
   15.38%  perf-22465.map   [.] 0x7f143b99c9c6
5.90%  qemu-system-x86_64   [.] phys_page_find
4.48%  qemu-system-x86_64   [.] address_space_translate_internal
3.30%  qemu-system-x86_64   [.] compute_all_subw
3.15%  qemu-system-x86_64   [.] check_regs
2.56%  qemu-system-x86_64   [.] tb_find_fast
2.34%  qemu-system-x86_64   [.] tb_find_slow
2.16%  qemu-system-x86_64   [.] cpu_x86_handle_mmu_fault
2.05%  qemu-system-x86_64   [.] address_space_lookup_region
1.66%  qemu-system-x86_64   [.] cpu_x86_exec
1.55%  qemu-system-x86_64   [.] address_space_translate
1.54%  qemu-system-x86_64   [.] lshift
1.29%  qemu-system-x86_64   [.] int128_make64
1.27%  qemu-system-x86_64   [.] helper_cc_compute_all
1.23%  qemu-system-x86_64   [.] memory_region_is_ram
1.23%  qemu-system-x86_64   [.] int128_sub
1.22%  qemu-system-x86_64   [.] cpu_get_tb_cpu_state
1.19%  qemu-system-x86_64   [.] lduw_p
1.17%  qemu-system-x86_64   [.] tcg_constant_folding
1.16%  qemu-system-x86_64   [.] ldq_phys_internal
1.09%  qemu-system-x86_64   [.] int128_min
1.08%  qemu-system-x86_64   [.] tlb_set_page
1.02%  qemu-system-x86_64   [.] ldq_p
0.90%  qemu-system-x86_64   [.] tb_jmp_cache_hash_func
0.79%  qemu-system-x86_64   [.] cpu_tb_exec
0.77%  qemu-system-x86_64   [.] qemu_get_ram_ptr
0.75%  qemu-system-x86_64   [.] helper_ret_lduw_mmu
0.75%  qemu-system-x86_64   [.] qemu_get_ram_block
0.67%  qemu-system-x86_64   [.] tcg_liveness_analysis
0.67%  qemu-system-x86_64   [.] int128_get64
0.66%  qemu-system-x86_64   [.] tcg_reg_alloc_op
0.66%  qemu-system-x86_64   [.] reset_all_temps
0.64%  qemu-system-x86_64   [.] int128_ge
0.61%  qemu-system-x86_64   [.] tcg_out_opc
0.59%  qemu-system-x86_64   [.] qemu_loglevel_mask
0.54%  qemu-system-x86_64   [.] int128_le
0.53%  qemu-system-x86_64   [.] tcg_out8
0.52%  qemu-system-x86_64   [.] ldq_le_p
0.48%  qemu-system-x86_64   [.] xen_enabled
0.47%  qemu-system-x86_64   [.] ldq_phys
0.46%  qemu-system-x86_64   

[Qemu-devel] [Bug 1236809] Re: qemu-system-x86_64 takes 100% CPU

2013-10-08 Thread chenlidong
i used the old version of qemu. when the vm is booting, the cpu is still 100%.
but the time of guest os boot is 2min50seconds. the latest upstream 
is7min21seconds.
so i think this is a performance problem.

the old version i used:
chenlidong@linux-0rsg:~> qemu-kvm --version
QEMU emulator version 1.3.1 (kvm-1.3.1-3.6.2), Copyright (c) 2003-2008 Fabrice 
Bellard

there are so many modification between the two version. 
i will test the other version to find out which modification cause this problem.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1236809

Title:
  qemu-system-x86_64 takes 100% CPU

Status in QEMU:
  New

Bug description:
  I have rhel6 inside qemu VM. qemu process starts to take CPU cycles
  and OS inside VM is very slow and sluggish.

  the qemu version is the latest upstream git.

  the kernel version is 3.12.0.

  linux-0rsg:/home/chenlidong # uname -a
  Linux linux-0rsg 3.12.0-rc1-1.16-desktop+ #5 SMP PREEMPT Sun Sep 22 22:07:40 
EDT 2013 x86_64 x86_64 x86_64 GNU/Linux

  linux-0rsg:/home/chenlidong # qemu-system-x86_64 --version
  QEMU emulator version 1.6.50, Copyright (c) 2003-2008 Fabrice Bellard

  the command line of qemu is below:

  chenlidong@linux-0rsg:~/develop/qemu> ps -ef | grep qemu
  root 19030 1 14 19:00 ?00:04:24 
/usr/local/bin/qemu-system-x86_64 -name rhel6 -S -M pc-i440fx-1.6 -m 2048 -smp 
1,sockets=1,cores=1,threads=1 -uuid 1925a96a-54b9-3c4a-dda0-6b42fdd0af2c 
-no-user-config -nodefaults -chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/rhel6.monitor,server,nowait 
-mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown 
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive 
file=/var/lib/libvirt/images/rhel6.img,if=none,id=drive-ide0-0-0,format=raw,cache=directsync
 -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=2 
-drive if=none,id=drive-ide0-1-0,readonly=on,format=raw -device 
ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,bootindex=1 -netdev 
tap,fd=21,id=hostnet0 -device 
rtl8139,netdev=hostnet0,id=net0,mac=52:54:00:b3:b8:53,bus=pci.0,addr=0x3 
-chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 
-device usb-tablet,id=input0 -vnc 127.0.0.1:0 -vga cirrus -device 
intel-hda,id=sound0,bus=pci.0,addr=0x4 -device 
hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -device 
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5

  i used perf top, the result is below:
  Samples: 57K of event 'cycles', Event count (approx.): 26336443124

   
   15.38%  perf-22465.map   [.] 0x7f143b99c9c6
5.90%  qemu-system-x86_64   [.] phys_page_find
4.48%  qemu-system-x86_64   [.] address_space_translate_internal
3.30%  qemu-system-x86_64   [.] compute_all_subw
3.15%  qemu-system-x86_64   [.] check_regs
2.56%  qemu-system-x86_64   [.] tb_find_fast
2.34%  qemu-system-x86_64   [.] tb_find_slow
2.16%  qemu-system-x86_64   [.] cpu_x86_handle_mmu_fault
2.05%  qemu-system-x86_64   [.] address_space_lookup_region
1.66%  qemu-system-x86_64   [.] cpu_x86_exec
1.55%  qemu-system-x86_64   [.] address_space_translate
1.54%  qemu-system-x86_64   [.] lshift
1.29%  qemu-system-x86_64   [.] int128_make64
1.27%  qemu-system-x86_64   [.] helper_cc_compute_all
1.23%  qemu-system-x86_64   [.] memory_region_is_ram
1.23%  qemu-system-x86_64   [.] int128_sub
1.22%  qemu-system-x86_64   [.] cpu_get_tb_cpu_state
1.19%  qemu-system-x86_64   [.] lduw_p
1.17%  qemu-system-x86_64   [.] tcg_constant_folding
1.16%  qemu-system-x86_64   [.] ldq_phys_internal
1.09%  qemu-system-x86_64   [.] int128_min
1.08%  qemu-system-x86_64   [.] tlb_set_page
1.02%  qemu-system-x86_64   [.] ldq_p
0.90%  qemu-system-x86_64   [.] tb_jmp_cache_hash_func
0.79%  qemu-system-x86_64   [.] cpu_tb_exec
0.77%  qemu-system-x86_64   [.] qemu_get_ram_ptr
0.75%  qemu-system-x86_64   [.] helper_ret_lduw_mmu
0.75%  qemu-system-x86_64   [.] qemu_get_ram_block
0.67%  qemu-system-x86_64   [.] tcg_liveness_analysis
0.67%  qemu-system-x86_64   [.] int128_get64
0.66%  qemu-system-x86_64   [.] tcg_reg_alloc_op
0.66%  qemu-system-x86_64   [.] reset_all_temps
0.64%  qemu-system-x86_64   [.] int128_ge
0.61%  qemu-system-x86_64   [.] tcg_out_opc
0.59%  qemu-system-x86_64   [.] qemu_logl

[Qemu-devel] [PATCHv3 0/3] seccomp: adding blacklist support with command line

2013-10-08 Thread Eduardo Otubo
v3: The "-netdev tap" option is checked in order to decide if the blacklist is
eligible to be installed or not, since it's one the most used features that is
known to use the exec() system call. It's an automatic mechanism to avoid Qemu
to break when using the blacklist feature.

v2: The blacklist works exactly the opposite as the whitelist. I set the
default behaiour to SCMP_ACT_ALLOW and the exceptions to SCMP_ACT_KILL;
remembering it inherits the behavior from the previous installed whitelist.

v1: The second whitelist is installed right before the vcpu starts, it contains 
all
the system calls the first one has except for exec() and select(), which are
big major syscalls that I could extensively test with virt-test and do not
cause any damage to the general execution.

This patch series also contain the command line support for this feature and
some minor fixes, all of them described in their own commit messages.

The environment in which the second whitelist is installed seems to need less
system calls than the first, so the procedure here will be the same: Keep
testing with virt-test and get to the smallest list as possible.

Eduardo Otubo (3):
  seccomp: adding blacklist support
  seccomp: adding command line support for blacklist
  seccomp: general fixes

 include/sysemu/seccomp.h |  6 -
 qemu-options.hx  |  8 +++---
 qemu-seccomp.c   | 66 ++--
 vl.c | 42 +++---
 4 files changed, 101 insertions(+), 21 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCHv3 3/3] seccomp: general fixes

2013-10-08 Thread Eduardo Otubo
 1) On qemu-seccomp.c:255, the variable ctx was being used
uninitialized; now it's initialized with NULL and it's being checked at
the end of the function.

 2) Changed the name of the command line option from "enable" to
"sandbox" for a better understanding from user side.

Signed-off-by: Eduardo Otubo 
---
 qemu-seccomp.c | 4 ++--
 vl.c   | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 84a42bc..fdd0de3 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -258,7 +258,7 @@ seccomp_return:
 int seccomp_start(int list_type)
 {
 int rc = 0;
-scmp_filter_ctx ctx;
+scmp_filter_ctx ctx = NULL;
 
 switch (list_type) {
 case WHITELIST:
@@ -285,7 +285,7 @@ int seccomp_start(int list_type)
 
 rc = seccomp_load(ctx);
 
-  seccomp_return:
+seccomp_return:
 if (ctx)
 seccomp_release(ctx);
 return rc;
diff --git a/vl.c b/vl.c
index ffdf460..f5106e6 100644
--- a/vl.c
+++ b/vl.c
@@ -324,11 +324,11 @@ static QemuOptsList qemu_rtc_opts = {
 
 static QemuOptsList qemu_sandbox_opts = {
 .name = "sandbox",
-.implied_opt_name = "enable",
+.implied_opt_name = "sandbox",
 .head = QTAILQ_HEAD_INITIALIZER(qemu_sandbox_opts.head),
 .desc = {
 {
-.name = "enable",
+.name = "sandbox",
 .type = QEMU_OPT_BOOL,
 },{
 .name = "strict",
@@ -1037,7 +1037,7 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
 {
 const char *strict_value = NULL;
 /* FIXME: change this to true for 1.3 */
-if (qemu_opt_get_bool(opts, "enable", false)) {
+if (qemu_opt_get_bool(opts, "sandbox", false)) {
 #ifdef CONFIG_SECCOMP
 if (seccomp_start(WHITELIST) < 0) {
 qerror_report(ERROR_CLASS_GENERIC_ERROR,
-- 
1.8.3.1




[Qemu-devel] [PATCHv3 2/3] seccomp: adding command line support for blacklist

2013-10-08 Thread Eduardo Otubo
v3: The options for blacklist in the command line also checkes the
existence of "-netdev tap", leaving a warning message in a positive
case.

New command line options for the seccomp blacklist feature:

 $ qemu -sandbox on[,strict=]

The strict parameter will turn on or off the new system call blacklist

Signed-off-by: Eduardo Otubo 
---
 qemu-options.hx |  8 +---
 vl.c| 17 -
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index d15338e..05485e1 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2978,13 +2978,15 @@ Old param mode (ARM only).
 ETEXI
 
 DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
-"-sandbox   Enable seccomp mode 2 system call filter (default 
'off').\n",
+"-sandbox   Enable seccomp mode 2 system call filter (default 
'off').\n"
+"-sandbox on[,strict=]\n"
+"Enable seccomp mode 2 system call second level filter 
(default 'off').\n",
 QEMU_ARCH_ALL)
 STEXI
-@item -sandbox @var{arg}
+@item -sandbox @var{arg}[,strict=@var{value}]
 @findex -sandbox
 Enable Seccomp mode 2 system call filter. 'on' will enable syscall filtering 
and 'off' will
-disable it.  The default is 'off'.
+disable it.  The default is 'off'. 'strict=on' will enable second level filter 
(default is 'off').
 ETEXI
 
 DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig,
diff --git a/vl.c b/vl.c
index ee95674..ffdf460 100644
--- a/vl.c
+++ b/vl.c
@@ -330,6 +330,9 @@ static QemuOptsList qemu_sandbox_opts = {
 {
 .name = "enable",
 .type = QEMU_OPT_BOOL,
+},{
+.name = "strict",
+.type = QEMU_OPT_STRING,
 },
 { /* end of list */ }
 },
@@ -1032,6 +1035,7 @@ static int bt_parse(const char *opt)
 
 static int parse_sandbox(QemuOpts *opts, void *opaque)
 {
+const char *strict_value = NULL;
 /* FIXME: change this to true for 1.3 */
 if (qemu_opt_get_bool(opts, "enable", false)) {
 #ifdef CONFIG_SECCOMP
@@ -1040,6 +1044,17 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
   "failed to install seccomp syscall filter in the 
kernel");
 return -1;
 }
+
+strict_value = qemu_opt_get(opts, "strict");
+
+if (!tap_enabled)
+   if (strict_value && !strcmp(strict_value, "on")) {
+   enable_blacklist = true;
+   }
+} else {
+fprintf(stderr, "Warning: seccomp syscall second level filter 
\"-sandbox on,strict=on\" "
+"cannot work together with \"-netdev tap\". Disabling 
it.\n");
+}
 #else
 qerror_report(ERROR_CLASS_GENERIC_ERROR,
   "sandboxing request but seccomp is not compiled into 
this build");
@@ -1769,7 +1784,7 @@ void vm_state_notify(int running, RunState state)
 
 static void install_seccomp_blacklist(void)
 {
-if (enable_blacklist && !tap_enabled) {
+if (enable_blacklist) {
 if (seccomp_start(BLACKLIST) < 0) {
 qerror_report(ERROR_CLASS_GENERIC_ERROR,
   "failed to install seccomp syscall second level 
filter in the kernel");
-- 
1.8.3.1




[Qemu-devel] [PATCHv3 1/3] seccomp: adding blacklist support

2013-10-08 Thread Eduardo Otubo
v3: The "-netdev tap" option is checked in the vl.c file during the
process of the command line argument list. It sets tap_enabled to true
or false according to the configuration found. Later at the seccomp
filter installation, this value is checked wheter to install or not this
feature.

Adding a system call blacklist right before the vcpus starts. This
filter is composed by the system calls that can't be executed after the
guests are up. This list should be refined as whitelist is, with as much
testing as we can do using virt-test.

Signed-off-by: Eduardo Otubo 
---
 include/sysemu/seccomp.h |  6 -
 qemu-seccomp.c   | 64 +++-
 vl.c | 21 +++-
 3 files changed, 77 insertions(+), 14 deletions(-)

diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
index 1189fa2..9dc7e52 100644
--- a/include/sysemu/seccomp.h
+++ b/include/sysemu/seccomp.h
@@ -15,8 +15,12 @@
 #ifndef QEMU_SECCOMP_H
 #define QEMU_SECCOMP_H
 
+#define WHITELIST 0
+#define BLACKLIST 1
+
 #include 
 #include "qemu/osdep.h"
 
-int seccomp_start(void);
+int seccomp_start(int list_type);
+
 #endif
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 37d38f8..84a42bc 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -21,7 +21,7 @@ struct QemuSeccompSyscall {
 uint8_t priority;
 };
 
-static const struct QemuSeccompSyscall seccomp_whitelist[] = {
+static const struct QemuSeccompSyscall whitelist[] = {
 { SCMP_SYS(timer_settime), 255 },
 { SCMP_SYS(timer_gettime), 254 },
 { SCMP_SYS(futex), 253 },
@@ -221,32 +221,72 @@ static const struct QemuSeccompSyscall 
seccomp_whitelist[] = {
 { SCMP_SYS(arch_prctl), 240 }
 };
 
-int seccomp_start(void)
+/*
+ * The second list, called blacklist, basically reduces previously installed
+ * whitelist. All the syscalls configured by the previous whitelist are still
+ * allowed, except for the ones in the blacklist.
+ * */
+
+static const struct QemuSeccompSyscall blacklist[] = {
+{ SCMP_SYS(execve), 255 }
+};
+
+static int process_list(scmp_filter_ctx *ctx,
+const struct QemuSeccompSyscall *list,
+unsigned int list_size, uint32_t action)
 {
 int rc = 0;
 unsigned int i = 0;
-scmp_filter_ctx ctx;
 
-ctx = seccomp_init(SCMP_ACT_KILL);
-if (ctx == NULL) {
-goto seccomp_return;
-}
+for (i = 0; i < list_size; i++) {
+rc = seccomp_rule_add(ctx, action, list[i].num, 0);
+if (rc < 0) {
+goto seccomp_return;
+}
 
-for (i = 0; i < ARRAY_SIZE(seccomp_whitelist); i++) {
-rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, seccomp_whitelist[i].num, 
0);
+rc = seccomp_syscall_priority(ctx, list[i].num,
+  list[i].priority);
 if (rc < 0) {
 goto seccomp_return;
 }
-rc = seccomp_syscall_priority(ctx, seccomp_whitelist[i].num,
-  seccomp_whitelist[i].priority);
+}
+
+seccomp_return:
+return rc;
+}
+
+int seccomp_start(int list_type)
+{
+int rc = 0;
+scmp_filter_ctx ctx;
+
+switch (list_type) {
+case WHITELIST:
+ctx = seccomp_init(SCMP_ACT_KILL);
+if (ctx == NULL) {
+goto seccomp_return;
+}
+rc = process_list(ctx, whitelist, ARRAY_SIZE(whitelist), 
SCMP_ACT_ALLOW);
 if (rc < 0) {
 goto seccomp_return;
 }
+break;
+case BLACKLIST:
+ctx = seccomp_init(SCMP_ACT_ALLOW);
+if (ctx == NULL) {
+goto seccomp_return;
+}
+rc = process_list(ctx, blacklist, ARRAY_SIZE(blacklist), 
SCMP_ACT_KILL);
+break;
+default:
+rc = -1;
+goto seccomp_return;
 }
 
 rc = seccomp_load(ctx);
 
   seccomp_return:
-seccomp_release(ctx);
+if (ctx)
+seccomp_release(ctx);
 return rc;
 }
diff --git a/vl.c b/vl.c
index b4b119a..ee95674 100644
--- a/vl.c
+++ b/vl.c
@@ -179,6 +179,8 @@ int main(int argc, char **argv)
 #define MAX_VIRTIO_CONSOLES 1
 #define MAX_SCLP_CONSOLES 1
 
+static bool enable_blacklist = false;
+static bool tap_enabled = false;
 static const char *data_dir[16];
 static int data_dir_idx;
 const char *bios_name = NULL;
@@ -1033,7 +1035,7 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
 /* FIXME: change this to true for 1.3 */
 if (qemu_opt_get_bool(opts, "enable", false)) {
 #ifdef CONFIG_SECCOMP
-if (seccomp_start() < 0) {
+if (seccomp_start(WHITELIST) < 0) {
 qerror_report(ERROR_CLASS_GENERIC_ERROR,
   "failed to install seccomp syscall filter in the 
kernel");
 return -1;
@@ -1765,12 +1767,24 @@ void vm_state_notify(int running, RunState state)
 }
 }
 
+static void install_seccomp_blacklist(void)
+{
+if (enable_blacklist && !tap_enabled) {
+if (seccomp_start(BLACKLIST) < 0) {
+qerro

Re: [Qemu-devel] [PATCHv3 3/3] seccomp: general fixes

2013-10-08 Thread Eduardo Otubo



On 09/11/2013 01:56 PM, Corey Bryant wrote:



On 09/06/2013 03:21 PM, Eduardo Otubo wrote:

  1) On qemu-seccomp.c:255, the variable ctx was being used
uninitialized; now it's initialized with NULL and it's being checked at
the end of the function.

  2) Changed the name of the command line option from "enable" to
"sandbox" for a better understanding from user side.

Signed-off-by: Eduardo Otubo
---
  qemu-seccomp.c | 5 +++--
  vl.c   | 6 +++---
  2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 5e85eb5..f39d636 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -252,7 +252,7 @@ seccomp_return:
  int seccomp_start(int list_type)
  {
  int rc = 0;
-scmp_filter_ctx ctx;
+scmp_filter_ctx ctx = NULL;

  switch (list_type) {
  case WHITELIST:
@@ -280,6 +280,7 @@ int seccomp_start(int list_type)
  rc = seccomp_load(ctx);

  seccomp_return:
-seccomp_release(ctx);
+if (!ctx)


You need to remove the ! from this check.


+seccomp_release(ctx);
  return rc;
  }
diff --git a/vl.c b/vl.c
index 909f685..129919d 100644
--- a/vl.c
+++ b/vl.c
@@ -323,11 +323,11 @@ static QemuOptsList qemu_rtc_opts = {

  static QemuOptsList qemu_sandbox_opts = {
  .name = "sandbox",
-.implied_opt_name = "enable",
+.implied_opt_name = "sandbox",


So does this technically make it -sandbox,sandbox=on?If I understand


No. Qemu command line options is a little tricky and I had to spent some 
time to understand it. It actually make "-sandbox on,strict=on"



correctly, I don't think the implied option is ever seen or used by the
user anyway so it probably doesn't matter.  But I don't know if it's
worth changing.


I changed the name so I can remember how it works in the future, since 
it's not that trivial.





  .head = QTAILQ_HEAD_INITIALIZER(qemu_sandbox_opts.head),
  .desc = {
  {
-.name = "enable",
+.name = "sandbox",
  .type = QEMU_OPT_BOOL,
  },{
  .name = "strict",
@@ -1036,7 +1036,7 @@ static int parse_sandbox(QemuOpts *opts, void
*opaque)
  {
  const char * strict_value = NULL;
  /* FIXME: change this to true for 1.3 */
-if (qemu_opt_get_bool(opts, "enable", false)) {
+if (qemu_opt_get_bool(opts, "sandbox", false)) {
  #ifdef CONFIG_SECCOMP
  if (seccomp_start(WHITELIST) < 0) {
  qerror_report(ERROR_CLASS_GENERIC_ERROR,
-- 1.8.3.1





--
Eduardo Otubo
IBM Linux Technology Center




Re: [Qemu-devel] [patch 0/2] force -mem-path RAM allocation

2013-10-08 Thread Marcelo Tosatti
On Tue, Oct 08, 2013 at 10:02:26AM +0200, Paolo Bonzini wrote:
> Il 08/10/2013 09:32, Markus Armbruster ha scritto:
> > We have
> > 
> > -mem-path FILE  provide backing storage for guest RAM
> > -mem-prealloc   preallocate guest memory (use with -mem-path)
> > 
> > PATCH 2/2 adds
> > 
> > -mem-path-forcefail if unable to allocate RAM as specified by 
> > -mem-path
> > 
> > Looks like it's time to consolidate the options related to guest memory
> > into a single, QemuOpts-style -memory NAME=VALUE,...  What do you guys
> > think?
> 
> Yes, we can use "-numa memory" (or "-numa mem") that Wanlong Gao is
> adding.  We can add path=, preallocate= and force= options there.
> 
> Paolo

It would be important for the new option to be backportable 
independently. Therefore mixing it with -numa is not an option.

Also due to backportability supporting a new style of command line
for -mem-path is problematic (management must be changed accordingly).

Can the new option format for memory be created incrementally on 
top of -mem-path-force? (agree its a good thing, it avoids proliferation
of new options).





Re: [Qemu-devel] [patch 1/2] qemu: mempath: prefault pages manually

2013-10-08 Thread Marcelo Tosatti
On Tue, Oct 08, 2013 at 10:03:48AM +0200, Paolo Bonzini wrote:
> Il 08/10/2013 02:41, Marcelo Tosatti ha scritto:
> > +/* unblock SIGBUS */
> > +pthread_sigmask(SIG_BLOCK, NULL, &oldset);
> > +sigemptyset(&set);
> > +sigaddset(&set, SIGBUS);
> > +pthread_sigmask(SIG_UNBLOCK, &set, NULL);
> 
> Please instead modify qemu-thread-posix.c to unblock all per-thread
> signals (SIGBUS, SIGSEGV, SIGILL, SIGFPE and SIGSYS).  There is no need
> to keep those blocked.
> 
> Paolo

main-loop.c handles SIGBUS via signalfd to emulate MCEs (associated
commits). Therefore it must be blocked.

Note that what this patch does it to maintain the signal handling state
(it saves the previous state, modifies state, restores previous state) so 
that its unchanged.




Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events

2013-10-08 Thread Alex Bligh

On 8 Oct 2013, at 21:01, Hans de Goede wrote:

>>> +/* When not non-blocking always allow io-threads to acquire the lock */
>>> +if (timeout != 0 && timeout_ns == 0) {
>>> +timeout_ns = 1;
>>> +}
>>> +
>>> ret = os_host_main_loop_wait(timeout_ns);
>>> qemu_iohandler_poll(gpollfds, ret);
>>> #ifdef CONFIG_SLIRP
>> 
>> I /think/ you might mean "if (!blocking && timeout_ns == 0)"
>> as timeout can be zero on a blocking call at this stage (i.e.
>> when there is a timer which has already expired.
> 
> timeout does not get modified, except by the slirp-stuff, which
> won't ever set it to 0 I think.

Ignoring the slirp stuff (which is hopefully irrelevant):

For your patch to have any effect, timeout_ns must be zero
or your timeout_ns=1 would not execute.

timeout_ns should not be zero frequently for the reason
I described.

If the call to main_loop_wait is blocking (i.e. nonblocking==0),
timeout_ns should start at -1 - this is the case you described.
If the call is non-blocking (as I originally thought)
timeout_ns should start at zero.

Then this line happens:

timeout_ns = qemu_soonest_timeout(timeout_ns,
  timerlistgroup_deadline_ns(
  &main_loop_tlg));

If timeout_ns is zero (i.e. non-blocking, caused by timeout=0)
this will do nothing (lack of optimisation and opacity here is
down to slirp stuff).

If timeout_ns is -1 (i.e. blocking, timeout=UINT32_MAX), then
this will use the return from
timerlistgroup_deadline_ns(). You said this was returning
zero, which would cause os_host_main_loop_wait() to
be called with a zero parameter.

This would explain the symptom, and explain why the fix
worked. But the question is why is timerlistgroup_deadline_ns
returning zero.


>  But your right writing
> 
>   if (!nonblocking && timeout_ns == 0)
> 
> would be much clearer, so I'll do that in v2 of the patch.


-- 
Alex Bligh







Re: [Qemu-devel] [PATCH 0/2] [RFC] qemu-ga: add support for guest command execution

2013-10-08 Thread Michael Roth
Quoting Michael Roth (2013-10-08 16:12:52)
> Quoting srinath reddy (2013-10-07 09:06:04)
> > Hi,
> >
> > Can someone help me in finding the status of this RFC here
> > http://lists.gnu.org/archive/html/qemu-devel/2011-12/msg00722.html
> 
> I picked it up again a few months back and re-worked the interface
> to address some of the initial review comments by using glib's
> exec() wrappers. I also added support for windows along with the
> guest-file-* interfaces, but then proceeded to blow everything away
> in an rm -rf ~ incident. And, of course, I was too cool for backups.
> 
> I think I can still piece most of it back together from some vim
> backup/temp files that managed to survive, I'm scared to confirm.
> But either way I won't have time to take a look at it again until
> the QEMU 1.7 development cycle.

1.8 I mean, Nov/Dec timeframe

> 
> >
> > I need a similar functionality.
> > 
> > Thanks,
> > Srinath.
> > 
> > --
> > good day




Re: [Qemu-devel] [PATCH 0/2] [RFC] qemu-ga: add support for guest command execution

2013-10-08 Thread Michael Roth
Quoting srinath reddy (2013-10-07 09:06:04)
> Hi,
>
> Can someone help me in finding the status of this RFC here
> http://lists.gnu.org/archive/html/qemu-devel/2011-12/msg00722.html

I picked it up again a few months back and re-worked the interface
to address some of the initial review comments by using glib's
exec() wrappers. I also added support for windows along with the
guest-file-* interfaces, but then proceeded to blow everything away
in an rm -rf ~ incident. And, of course, I was too cool for backups.

I think I can still piece most of it back together from some vim
backup/temp files that managed to survive, I'm scared to confirm.
But either way I won't have time to take a look at it again until
the QEMU 1.7 development cycle.

>
> I need a similar functionality.
> 
> Thanks,
> Srinath.
> 
> --
> good day




Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events

2013-10-08 Thread Hans de Goede

Hi,

On 10/08/2013 09:48 PM, Alex Bligh wrote:


On 8 Oct 2013, at 20:10, Hans de Goede wrote:


I noticed today that current qemu master would hang as soon as Xorg starts in
the guest when using qxl + a Linux guest. This message would be printed:
main-loop: WARNING: I/O thread spun for 1000 iterations

And from then on the guest hangs and qemu consumes 100% cpu, bisecting pointed
out commit 7b595f35d89d73bc69c35bf3980a89c420e8a44b:
"aio / timers: Convert mainloop to use timeout"

After looking at that commit I had a hunch the problem might be blocking
main_loop_wait calls being turned into non-blocking ones (and thus never
releasing the io-lock), a debug printf confirmed this was happening at
the moment of the hang, so I wrote this patch which fixes the hang for me
and seems like a good idea in general.

Signed-off-by: Hans de Goede 
---
main-loop.c | 5 +
1 file changed, 5 insertions(+)

diff --git a/main-loop.c b/main-loop.c
index c3c9c28..921c939 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -480,6 +480,11 @@ int main_loop_wait(int nonblocking)
   timerlistgroup_deadline_ns(
   &main_loop_tlg));

+/* When not non-blocking always allow io-threads to acquire the lock */
+if (timeout != 0 && timeout_ns == 0) {
+timeout_ns = 1;
+}
+
 ret = os_host_main_loop_wait(timeout_ns);
 qemu_iohandler_poll(gpollfds, ret);
#ifdef CONFIG_SLIRP


I /think/ you might mean "if (!blocking && timeout_ns == 0)"
as timeout can be zero on a blocking call at this stage (i.e.
when there is a timer which has already expired.


timeout does not get modified, except by the slirp-stuff, which
won't ever set it to 0 I think. But your right writing

   if (!nonblocking && timeout_ns == 0)

would be much clearer, so I'll do that in v2 of the patch.

Regards,

Hans



[Qemu-devel] [PULL 36/58] arm11mpcore: Drop unused fields

2013-10-08 Thread Andreas Färber
Reviewed-by: Peter Maydell 
Signed-off-by: Andreas Färber 
---
 hw/cpu/arm11mpcore.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/cpu/arm11mpcore.c b/hw/cpu/arm11mpcore.c
index 27cd32b..8719634 100644
--- a/hw/cpu/arm11mpcore.c
+++ b/hw/cpu/arm11mpcore.c
@@ -20,8 +20,6 @@ typedef struct ARM11MPCorePriveState {
 SysBusDevice parent_obj;
 
 uint32_t scu_control;
-int iomemtype;
-uint32_t old_timer_status[8];
 uint32_t num_cpu;
 MemoryRegion iomem;
 MemoryRegion container;
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH] scsi: Allocate SCSITargetReq r->buf dynamically

2013-10-08 Thread Michael Roth
Quoting Asias He (2013-10-08 03:43:37)
> r->buf is hardcoded to 2056 which is (256 + 1) * 8, allowing 256 luns at
> most. If more than 256 luns are specified by user, we have buffer
> overflow in scsi_target_emulate_report_luns.
> 
> To fix, we allocate the buffer dynamically.
> 
> Signed-off-by: Asias He 

Tested-by: Michael Roth 

> ---
>  hw/scsi/scsi-bus.c | 44 +---
>  include/hw/scsi/scsi.h |  2 ++
>  2 files changed, 35 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index 4d36841..d950e6f 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -11,6 +11,8 @@ static char *scsibus_get_dev_path(DeviceState *dev);
>  static char *scsibus_get_fw_dev_path(DeviceState *dev);
>  static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf);
>  static void scsi_req_dequeue(SCSIRequest *req);
> +static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len);
> +static void scsi_target_free_buf(SCSIRequest *req);
> 
>  static Property scsi_props[] = {
>  DEFINE_PROP_UINT32("channel", SCSIDevice, channel, 0),
> @@ -317,7 +319,8 @@ typedef struct SCSITargetReq SCSITargetReq;
>  struct SCSITargetReq {
>  SCSIRequest req;
>  int len;
> -uint8_t buf[2056];
> +uint8_t *buf;
> +int buf_len;
>  };
> 
>  static void store_lun(uint8_t *outbuf, int lun)
> @@ -361,14 +364,12 @@ static bool 
> scsi_target_emulate_report_luns(SCSITargetReq *r)
>  if (!found_lun0) {
>  n += 8;
>  }
> -len = MIN(n + 8, r->req.cmd.xfer & ~7);
> -if (len > sizeof(r->buf)) {
> -/* TODO: > 256 LUNs? */
> -return false;
> -}
> 
> +scsi_target_alloc_buf(&r->req, n + 8);
> +
> +len = MIN(n + 8, r->req.cmd.xfer & ~7);
>  memset(r->buf, 0, len);
> -stl_be_p(&r->buf, n);
> +stl_be_p(&r->buf[0], n);
>  i = found_lun0 ? 8 : 16;
>  QTAILQ_FOREACH(kid, &r->req.bus->qbus.children, sibling) {
>  DeviceState *qdev = kid->child;
> @@ -387,6 +388,9 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq 
> *r)
>  static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
>  {
>  assert(r->req.dev->lun != r->req.lun);
> +
> +scsi_target_alloc_buf(&r->req, SCSI_INQUIRY_LEN);
> +
>  if (r->req.cmd.buf[1] & 0x2) {
>  /* Command support data - optional, not implemented */
>  return false;
> @@ -411,7 +415,7 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
>  return false;
>  }
>  /* done with EVPD */
> -assert(r->len < sizeof(r->buf));
> +assert(r->len < r->buf_len);
>  r->len = MIN(r->req.cmd.xfer, r->len);
>  return true;
>  }
> @@ -455,8 +459,8 @@ static int32_t scsi_target_send_command(SCSIRequest *req, 
> uint8_t *buf)
>  }
>  break;
>  case REQUEST_SENSE:
> -r->len = scsi_device_get_sense(r->req.dev, r->buf,
> -   MIN(req->cmd.xfer, sizeof r->buf),
> +scsi_target_alloc_buf(&r->req, SCSI_SENSE_LEN);
> +r->len = scsi_device_get_sense(r->req.dev, r->buf, r->buf_len,
> (req->cmd.buf[1] & 1) == 0);
>  if (r->req.dev->sense_is_ua) {
>  scsi_device_unit_attention_reported(req->dev);
> @@ -501,11 +505,29 @@ static uint8_t *scsi_target_get_buf(SCSIRequest *req)
>  return r->buf;
>  }
> 
> +static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len)
> +{
> +SCSITargetReq *r = DO_UPCAST(SCSITargetReq, req, req);
> +
> +r->buf = g_malloc(len);
> +r->buf_len = len;
> +
> +return r->buf;
> +}
> +
> +static void scsi_target_free_buf(SCSIRequest *req)
> +{
> +SCSITargetReq *r = DO_UPCAST(SCSITargetReq, req, req);
> +
> +g_free(r->buf);
> +}
> +
>  static const struct SCSIReqOps reqops_target_command = {
>  .size = sizeof(SCSITargetReq),
>  .send_command = scsi_target_send_command,
>  .read_data= scsi_target_read_data,
>  .get_buf  = scsi_target_get_buf,
> +.free_req = scsi_target_free_buf,
>  };
> 
> 
> @@ -1365,7 +1387,7 @@ int scsi_build_sense(uint8_t *in_buf, int in_len,
>  buf[7] = 10;
>  buf[12] = sense.asc;
>  buf[13] = sense.ascq;
> -return MIN(len, 18);
> +return MIN(len, SCSI_SENSE_LEN);
>  } else {
>  /* Return descriptor format sense buffer */
>  buf[0] = 0x72;
> diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
> index 1b66510..76f6ac2 100644
> --- a/include/hw/scsi/scsi.h
> +++ b/include/hw/scsi/scsi.h
> @@ -9,6 +9,8 @@
>  #define MAX_SCSI_DEVS  255
> 
>  #define SCSI_CMD_BUF_SIZE 16
> +#define SCSI_SENSE_LEN  18
> +#define SCSI_INQUIRY_LEN36
> 
>  typedef struct SCSIBus SCSIBus;
>  typedef struct SCSIBusInfo SCSIBusInfo;
> -- 
> 1.8.3.1




[Qemu-devel] [PULL 55/58] pcmcia: QOM'ify PCMCIACardState and MicroDriveState

2013-10-08 Thread Andreas Färber
Turn PCMCIACardState into a device.
Move callbacks to new PCMCIACardClass.

Derive TYPE_MICRODRIVE from TYPE_PCMCIA_CARD.
Replace ide_init2_with_non_qdev_drives().

Signed-off-by: Othmar Pasteka 
Signed-off-by: Andreas Färber 
---
 hw/Makefile.objs |   1 +
 hw/ide/microdrive.c  | 170 +++
 hw/misc/Makefile.objs|   1 -
 hw/pcmcia/Makefile.objs  |   2 +
 hw/pcmcia/pcmcia.c   |  24 
 hw/{misc/pxa2xx_pcmcia.c => pcmcia/pxa2xx.c} |  50 +---
 include/hw/pcmcia.h  |  46 ++--
 7 files changed, 219 insertions(+), 75 deletions(-)
 create mode 100644 hw/pcmcia/Makefile.objs
 create mode 100644 hw/pcmcia/pcmcia.c
 rename hw/{misc/pxa2xx_pcmcia.c => pcmcia/pxa2xx.c} (81%)

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 0243d6a..d91b9cc 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -18,6 +18,7 @@ devices-dirs-$(CONFIG_SOFTMMU) += net/
 devices-dirs-$(CONFIG_SOFTMMU) += nvram/
 devices-dirs-$(CONFIG_SOFTMMU) += pci/
 devices-dirs-$(CONFIG_PCI) += pci-bridge/ pci-host/
+devices-dirs-$(CONFIG_SOFTMMU) += pcmcia/
 devices-dirs-$(CONFIG_SOFTMMU) += scsi/
 devices-dirs-$(CONFIG_SOFTMMU) += sd/
 devices-dirs-$(CONFIG_SOFTMMU) += ssi/
diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index 92c1df0..cdf0eb9 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -30,15 +30,22 @@
 
 #include 
 
+#define TYPE_MICRODRIVE "microdrive"
+#define MICRODRIVE(obj) OBJECT_CHECK(MicroDriveState, (obj), TYPE_MICRODRIVE)
+
 /***/
 /* CF-ATA Microdrive */
 
 #define METADATA_SIZE  0x20
 
 /* DSCM-1 Microdrive hard disk with CF+ II / PCMCIA interface.  */
-typedef struct {
+
+typedef struct MicroDriveState {
+/*< private >*/
+PCMCIACardState parent_obj;
+/*< public >*/
+
 IDEBus bus;
-PCMCIACardState card;
 uint32_t attr_base;
 uint32_t io_base;
 
@@ -81,10 +88,13 @@ enum md_ctrl {
 
 static inline void md_interrupt_update(MicroDriveState *s)
 {
-if (!s->card.slot)
+PCMCIACardState *card = PCMCIA_CARD(s);
+
+if (card->slot == NULL) {
 return;
+}
 
-qemu_set_irq(s->card.slot->irq,
+qemu_set_irq(card->slot->irq,
 !(s->stat & STAT_INT) &&   /* Inverted */
 !(s->ctrl & (CTRL_IEN | CTRL_SRST)) &&
 !(s->opt & OPT_SRESET));
@@ -101,8 +111,10 @@ static void md_set_irq(void *opaque, int irq, int level)
 md_interrupt_update(s);
 }
 
-static void md_reset(MicroDriveState *s)
+static void md_reset(DeviceState *dev)
 {
+MicroDriveState *s = MICRODRIVE(dev);
+
 s->opt = OPT_MODE_MMAP;
 s->stat = 0;
 s->pins = 0;
@@ -111,14 +123,17 @@ static void md_reset(MicroDriveState *s)
 ide_bus_reset(&s->bus);
 }
 
-static uint8_t md_attr_read(void *opaque, uint32_t at)
+static uint8_t md_attr_read(PCMCIACardState *card, uint32_t at)
 {
-MicroDriveState *s = opaque;
+MicroDriveState *s = MICRODRIVE(card);
+PCMCIACardClass *pcc = PCMCIA_CARD_GET_CLASS(card);
+
 if (at < s->attr_base) {
-if (at < s->card.cis_len)
-return s->card.cis[at];
-else
+if (at < pcc->cis_len) {
+return pcc->cis[at];
+} else {
 return 0x00;
+}
 }
 
 at -= s->attr_base;
@@ -144,16 +159,18 @@ static uint8_t md_attr_read(void *opaque, uint32_t at)
 return 0;
 }
 
-static void md_attr_write(void *opaque, uint32_t at, uint8_t value)
+static void md_attr_write(PCMCIACardState *card, uint32_t at, uint8_t value)
 {
-MicroDriveState *s = opaque;
+MicroDriveState *s = MICRODRIVE(card);
+
 at -= s->attr_base;
 
 switch (at) {
 case 0x00: /* Configuration Option Register */
 s->opt = value & 0xcf;
-if (value & OPT_SRESET)
-md_reset(s);
+if (value & OPT_SRESET) {
+device_reset(DEVICE(s));
+}
 md_interrupt_update(s);
 break;
 case 0x02: /* Card Configuration Status Register */
@@ -175,9 +192,9 @@ static void md_attr_write(void *opaque, uint32_t at, 
uint8_t value)
 }
 }
 
-static uint16_t md_common_read(void *opaque, uint32_t at)
+static uint16_t md_common_read(PCMCIACardState *card, uint32_t at)
 {
-MicroDriveState *s = opaque;
+MicroDriveState *s = MICRODRIVE(card);
 IDEState *ifs;
 uint16_t ret;
 at -= s->io_base;
@@ -237,9 +254,9 @@ static uint16_t md_common_read(void *opaque, uint32_t at)
 return 0;
 }
 
-static void md_common_write(void *opaque, uint32_t at, uint16_t value)
+static void md_common_write(PCMCIACardState *card, uint32_t at, uint16_t value)
 {
-MicroDriveState *s = opaque;
+MicroDriveState *s = MICRODRIVE(card);
 at -= s->io_base;
 
 switch (s->opt & OPT_MODE) {
@@ -285,8 +302,9 @@ static void md_common_write(void *opaque, uint32_t at, 
uint16_t val

[Qemu-devel] [PATCH 08/13] usb: Add max_streams attribute to endpoint info

2013-10-08 Thread Hans de Goede
Signed-off-by: Hans de Goede 
---
 hw/usb/core.c| 22 ++
 hw/usb/desc.c|  2 ++
 include/hw/usb.h |  3 +++
 3 files changed, 27 insertions(+)

diff --git a/hw/usb/core.c b/hw/usb/core.c
index cf59a1a..67ba7d6 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -623,6 +623,7 @@ void usb_ep_reset(USBDevice *dev)
 dev->ep_ctl.type = USB_ENDPOINT_XFER_CONTROL;
 dev->ep_ctl.ifnum = 0;
 dev->ep_ctl.max_packet_size = 64;
+dev->ep_ctl.max_streams = 0;
 dev->ep_ctl.dev = dev;
 dev->ep_ctl.pipeline = false;
 for (ep = 0; ep < USB_MAX_ENDPOINTS; ep++) {
@@ -636,6 +637,8 @@ void usb_ep_reset(USBDevice *dev)
 dev->ep_out[ep].ifnum = USB_INTERFACE_INVALID;
 dev->ep_in[ep].max_packet_size = 0;
 dev->ep_out[ep].max_packet_size = 0;
+dev->ep_in[ep].max_streams = 0;
+dev->ep_out[ep].max_streams = 0;
 dev->ep_in[ep].dev = dev;
 dev->ep_out[ep].dev = dev;
 dev->ep_in[ep].pipeline = false;
@@ -764,6 +767,25 @@ int usb_ep_get_max_packet_size(USBDevice *dev, int pid, 
int ep)
 return uep->max_packet_size;
 }
 
+void usb_ep_set_max_streams(USBDevice *dev, int pid, int ep, uint8_t raw)
+{
+struct USBEndpoint *uep = usb_ep_get(dev, pid, ep);
+int MaxStreams;
+
+MaxStreams = raw & 0x1f;
+if (MaxStreams) {
+uep->max_streams = 1 << MaxStreams;
+} else {
+uep->max_streams = 0;
+}
+}
+
+int usb_ep_get_max_streams(USBDevice *dev, int pid, int ep)
+{
+struct USBEndpoint *uep = usb_ep_get(dev, pid, ep);
+return uep->max_streams;
+}
+
 void usb_ep_set_pipeline(USBDevice *dev, int pid, int ep, bool enabled)
 {
 struct USBEndpoint *uep = usb_ep_get(dev, pid, ep);
diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index bf6c522..5dbe754 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -385,6 +385,8 @@ static void usb_desc_ep_init(USBDevice *dev)
 usb_ep_set_ifnum(dev, pid, ep, iface->bInterfaceNumber);
 usb_ep_set_max_packet_size(dev, pid, ep,
iface->eps[e].wMaxPacketSize);
+usb_ep_set_max_streams(dev, pid, ep,
+   iface->eps[e].bmAttributes_super);
 }
 }
 }
diff --git a/include/hw/usb.h b/include/hw/usb.h
index a7680d4..e9d96ba 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -189,6 +189,7 @@ struct USBEndpoint {
 uint8_t type;
 uint8_t ifnum;
 int max_packet_size;
+int max_streams;
 bool pipeline;
 bool halted;
 USBDevice *dev;
@@ -421,6 +422,8 @@ void usb_ep_set_ifnum(USBDevice *dev, int pid, int ep, 
uint8_t ifnum);
 void usb_ep_set_max_packet_size(USBDevice *dev, int pid, int ep,
 uint16_t raw);
 int usb_ep_get_max_packet_size(USBDevice *dev, int pid, int ep);
+void usb_ep_set_max_streams(USBDevice *dev, int pid, int ep, uint8_t raw);
+int usb_ep_get_max_streams(USBDevice *dev, int pid, int ep);
 void usb_ep_set_pipeline(USBDevice *dev, int pid, int ep, bool enabled);
 void usb_ep_set_halted(USBDevice *dev, int pid, int ep, bool halted);
 USBPacket *usb_ep_find_packet_by_id(USBDevice *dev, int pid, int ep,
-- 
1.8.3.1




[Qemu-devel] [PULL 10/58] exynos4_boards: Silence lack of -smp 2 warning for qtest

2013-10-08 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 hw/arm/exynos4_boards.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index 2929f9f..26cedec 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -22,6 +22,7 @@
  */
 
 #include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
 #include "hw/sysbus.h"
 #include "net/net.h"
 #include "hw/arm/arm.h"
@@ -96,7 +97,7 @@ static void lan9215_init(uint32_t base, qemu_irq irq)
 static Exynos4210State *exynos4_boards_init_common(QEMUMachineInitArgs *args,
Exynos4BoardType board_type)
 {
-if (smp_cpus != EXYNOS4210_NCPUS) {
+if (smp_cpus != EXYNOS4210_NCPUS && !qtest_enabled()) {
 fprintf(stderr, "%s board supports only %d CPU cores. Ignoring 
smp_cpus"
 " value.\n",
 exynos4_machines[board_type].name,
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH v4] Extend qemu-ga's 'guest-info' command to expose flag 'success-response'

2013-10-08 Thread Michael Roth
Quoting Mark Wu (2013-10-08 01:23:19)
> Now we have several qemu-ga commands not returning response on success.
> It has been documented in qga/qapi-schema.json already. This patch exposes
> the 'success-response' flag by extending 'guest-info' command. With this
> change, the clients can handle the command response more flexibly.
> 
> Signed-off-by: Mark Wu 

Reviewed-by: Michael Roth 

Will apply on top of 'Add interface to traverse ...' patch once that goes
in.

> ---
> Changes:
> v4: 
> Add signature of qmp_has_success_response per Michael.
> v3: 
> 1. treat cmd->options as a bitmask instead of single option (per 
> Eric) 
> 2. rebase on the patch " Add interface to traverse the qmp command 
> list
> by QmpCommand" to avoid the O(n2) problem (per Eric and Michael)
> v2: 
> add the notation 'since 1.7' to the option 'success-response'
> (per Eric Blake's comments)
> 
>  include/qapi/qmp/dispatch.h | 1 +
>  qapi/qmp-registry.c | 5 +
>  qga/commands.c  | 1 +
>  qga/qapi-schema.json| 5 -
>  4 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
> index b6eb49e..cebf6aa 100644
> --- a/include/qapi/qmp/dispatch.h
> +++ b/include/qapi/qmp/dispatch.h
> @@ -48,6 +48,7 @@ QObject *qmp_dispatch(QObject *request);
>  void qmp_disable_command(const char *name);
>  void qmp_enable_command(const char *name);
>  bool qmp_command_is_enabled(const QmpCommand *cmd);
> +bool qmp_has_success_response(const QmpCommand *cmd);
>  QObject *qmp_build_error_object(Error *errp);
>  typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque);
>  void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque);
> diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
> index 3fcf10e..c75c2e8 100644
> --- a/qapi/qmp-registry.c
> +++ b/qapi/qmp-registry.c
> @@ -71,6 +71,11 @@ bool qmp_command_is_enabled(const QmpCommand *cmd)
>  return cmd->enabled;
>  }
> 
> +bool qmp_has_success_response(const QmpCommand *cmd)
> +{
> +   return !(cmd->options & QCO_NO_SUCCESS_RESP);
> +}
> +
>  void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque)
>  {
>  QmpCommand *cmd;
> diff --git a/qga/commands.c b/qga/commands.c
> index 063b22b..7f089ba 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -54,6 +54,7 @@ static void qmp_command_info(QmpCommand *cmd, void *opaque)
>  cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
>  cmd_info->name = g_strdup(cmd->name);
>  cmd_info->enabled = qmp_command_is_enabled(cmd);
> +cmd_info->success_response = qmp_has_success_response(cmd);
> 
>  cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
>  cmd_info_list->value = cmd_info;
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 7155b7a..245f968 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -141,10 +141,13 @@
>  #
>  # @enabled: whether command is currently enabled by guest admin
>  #
> +# @success-response: whether command returns a response on success
> +#(since 1.7)
> +#
>  # Since 1.1.0
>  ##
>  { 'type': 'GuestAgentCommandInfo',
> -  'data': { 'name': 'str', 'enabled': 'bool' } }
> +  'data': { 'name': 'str', 'enabled': 'bool', 'success-response': 'bool' } }
> 
>  ##
>  # @GuestAgentInfo
> -- 
> 1.8.3.1




Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events

2013-10-08 Thread Paolo Bonzini
Il 08/10/2013 22:16, Hans de Goede ha scritto:
> No, it is calling main_loop_wait with nonblocking set to 0, so
> normally the lock would get released. But
> timerlistgroup_deadline_ns(&main_loop_tlg) is returning 0,
> causing timeout_ns to be 0, and this causes the lock to not get
> released.

Yes, this was my understanding of the patch as well.  Before Alex's
series, this would be capped to MIN_REARM_TIMER_NS (250 us).  This is
why I mentioned 250 us.

However, I agree with Alex that it looks a bit fishy and I'd like to
know what timer is it that is continuously set to expire in the past.

Paolo



Re: [Qemu-devel] [PATCH] qemu-ga: execute fsfreeze-freeze in reverse order of mounts

2013-10-08 Thread Michael Roth
Quoting Tomoki Sekiyama (2013-10-01 16:09:53)
> Currently, fsfreeze-freeze may cause deadlock if a guest has loopback mounts
> of image files in its disk; e.g.:
> 
> # mount | grep ^/
> /dev/vda1 / type ext4 (rw,noatime,seclabel,data=ordered)
> /tmp/disk.img on /mnt type ext4 (rw,relatime,seclabel)
> 
> To avoid the deadlock, this freeze filesystems in reverse order of mounts.
> 
> Signed-off-by: Tomoki Sekiyama 

Thanks, applied to qga tree:

https://github.com/mdroth/qemu/commits/qga

> ---
>  qga/commands-posix.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index e199738..f453132 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -566,7 +566,7 @@ typedef struct FsMount {
>  QTAILQ_ENTRY(FsMount) next;
>  } FsMount;
> 
> -typedef QTAILQ_HEAD(, FsMount) FsMountList;
> +typedef QTAILQ_HEAD(FsMountList, FsMount) FsMountList;
> 
>  static void free_fs_mount_list(FsMountList *mounts)
>  {
> @@ -728,7 +728,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err)
>  /* cannot risk guest agent blocking itself on a write in this state */
>  ga_set_frozen(ga_state);
> 
> -QTAILQ_FOREACH(mount, &mounts, next) {
> +QTAILQ_FOREACH_REVERSE(mount, &mounts, FsMountList, next) {
>  fd = qemu_open(mount->dirname, O_RDONLY);
>  if (fd == -1) {
>  error_setg_errno(err, errno, "failed to open %s", 
> mount->dirname);




[Qemu-devel] [PULL 01/58] hw/arm/boot: Make user not specifying a kernel not an error

2013-10-08 Thread Andreas Färber
From: Peter Maydell 

Typically ARM boards will have some kind of flash which might contain
a boot ROM; it's therefore a valid use case to provide only an
image for the boot ROM and not require QEMU's internal boot loader
at all. Remove the fatal error if -kernel isn't specified.

Signed-off-by: Peter Maydell 
Signed-off-by: Andreas Färber 
---
 hw/arm/boot.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 1e313af..583ec79 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -354,8 +354,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
*info)
 
 /* Load the kernel.  */
 if (!info->kernel_filename) {
-fprintf(stderr, "Kernel image must be specified\n");
-exit(1);
+/* If no kernel specified, do nothing; we will start from address 0
+ * (typically a boot ROM image) in the same way as hardware.
+ */
+return;
 }
 
 info->dtb_filename = qemu_opt_get(qemu_get_machine_opts(), "dtb");
-- 
1.8.1.4




[Qemu-devel] [PULL 53/58] qom: Add pointer to int property helpers

2013-10-08 Thread Andreas Färber
From: "Michael S. Tsirkin" 

Make it easy to add read-only helpers for simple
integer properties in memory.

Reviewed-by: Paolo Bonzini 
Reviewed-by: Gerd Hoffmann 
Tested-by: Gerd Hoffmann 
Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Igor Mammedov 
Tested-by: Igor Mammedov 
[AF: Extended documentation]
Signed-off-by: Andreas Färber 
---
 include/qom/object.h | 52 +
 qom/object.c | 60 
 2 files changed, 112 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index 6c1e7d3..a275db2 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1113,6 +1113,58 @@ void object_property_add_bool(Object *obj, const char 
*name,
   Error **errp);
 
 /**
+ * object_property_add_uint8_ptr:
+ * @obj: the object to add a property to
+ * @name: the name of the property
+ * @v: pointer to value
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Add an integer property in memory.  This function will add a
+ * property of type 'uint8'.
+ */
+void object_property_add_uint8_ptr(Object *obj, const char *name,
+   const uint8_t *v, Error **errp);
+
+/**
+ * object_property_add_uint16_ptr:
+ * @obj: the object to add a property to
+ * @name: the name of the property
+ * @v: pointer to value
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Add an integer property in memory.  This function will add a
+ * property of type 'uint16'.
+ */
+void object_property_add_uint16_ptr(Object *obj, const char *name,
+const uint16_t *v, Error **errp);
+
+/**
+ * object_property_add_uint32_ptr:
+ * @obj: the object to add a property to
+ * @name: the name of the property
+ * @v: pointer to value
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Add an integer property in memory.  This function will add a
+ * property of type 'uint32'.
+ */
+void object_property_add_uint32_ptr(Object *obj, const char *name,
+const uint32_t *v, Error **errp);
+
+/**
+ * object_property_add_uint64_ptr:
+ * @obj: the object to add a property to
+ * @name: the name of the property
+ * @v: pointer to value
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Add an integer property in memory.  This function will add a
+ * property of type 'uint64'.
+ */
+void object_property_add_uint64_ptr(Object *obj, const char *name,
+const uint64_t *v, Error **Errp);
+
+/**
  * object_child_foreach:
  * @obj: the object whose children will be navigated
  * @fn: the iterator function to be called
diff --git a/qom/object.c b/qom/object.c
index e90e382..b617f26 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1344,6 +1344,66 @@ static char *qdev_get_type(Object *obj, Error **errp)
 return g_strdup(object_get_typename(obj));
 }
 
+static void property_get_uint8_ptr(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
+{
+uint8_t value = *(uint8_t *)opaque;
+visit_type_uint8(v, &value, name, errp);
+}
+
+static void property_get_uint16_ptr(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
+{
+uint16_t value = *(uint16_t *)opaque;
+visit_type_uint16(v, &value, name, errp);
+}
+
+static void property_get_uint32_ptr(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
+{
+uint32_t value = *(uint32_t *)opaque;
+visit_type_uint32(v, &value, name, errp);
+}
+
+static void property_get_uint64_ptr(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
+{
+uint64_t value = *(uint64_t *)opaque;
+visit_type_uint64(v, &value, name, errp);
+}
+
+void object_property_add_uint8_ptr(Object *obj, const char *name,
+   const uint8_t *v, Error **errp)
+{
+object_property_add(obj, name, "uint8", property_get_uint8_ptr,
+NULL, NULL, (void *)v, errp);
+}
+
+void object_property_add_uint16_ptr(Object *obj, const char *name,
+const uint16_t *v, Error **errp)
+{
+object_property_add(obj, name, "uint16", property_get_uint16_ptr,
+NULL, NULL, (void *)v, errp);
+}
+
+void object_property_add_uint32_ptr(Object *obj, const char *name,
+const uint32_t *v, Error **errp)
+{
+object_property_add(obj, name, "uint32", property_get_uint32_ptr,
+NULL, NULL, (void *)v, errp);
+}
+
+void object_property_add_uint64_ptr(Object *obj, const char *nam

[Qemu-devel] [PULL 00/58] QOM devices patch queue 2013-10-08

2013-10-08 Thread Andreas Färber
Hello Anthony,

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

Thanks,
Andreas

Cc: Anthony Liguori 

Cc: Peter Maydell 
Cc: Mian M. Hamayun 
Cc: Paolo Bonzini 
Cc: Michael S. Tsirkin 

The following changes since commit a684f3cf9b9b9c3cb82be87aafc463de8974610c:

  Merge remote-tracking branch 'kraxel/seabios-1.7.3.2' into staging 
(2013-09-30 17:15:27 -0500)

are available in the git repository at:


  git://github.com/afaerber/qemu-cpu.git tags/qom-devices-for-anthony

for you to fetch changes up to c0a0dd4242be94386ef7968ce47b3411ebea0aec:

  pcmcia/pxa2xx: QOM'ify PXA2xxPCMCIAState (2013-10-08 19:17:49 +0200)


QOM device refactorings

* QTest coverage for all machines
* QOM realize for Milkymist UART
* QOM realize for ARM MPCore
* device_add bug fixes and cleanups
* QOM API extensions and cleanups
* QOM for PCMCIA/MicroDrive (last legacy IDE device)


Andreas Färber (49):
  mips_mipssim: Silence BIOS loading warning for qtest
  puv3: Turn puv3_load_kernel() into a no-op for qtest without -kernel
  mainstone: Don't enforce use of -pflash for qtest
  gumstix: Don't enforce use of -pflash for qtest
  z2: Don't enforce use of -pflash for qtest
  palm: Don't enforce loading ROM or kernel for qtest
  omap_sx1: Don't enforce use of kernel or flash for qtest
  exynos4_boards: Silence lack of -smp 2 warning for qtest
  armv7m: Don't enforce use of kernel for qtest
  axis_dev88: Don't enforce use of kernel for qtest
  mcf5208: Don't enforce use of kernel for qtest
  an5206: Don't enforce use of kernel for qtest
  milkymist: Suppress -kernel/-bios/-drive error for qtest
  shix: Drop debug output
  shix: Don't require firmware presence for qtest
  leon3: Don't enforce use of -bios with qtest
  qtest: Prepare QOM machine tests
  a9mpcore: Split off instance_init
  arm_gic: Extract headers hw/intc/arm_gic{,_common}.h
  a9mpcore: Embed GICState
  a9scu: QOM cleanups
  a9mpcore: Embed A9SCUState
  arm_mptimer: Convert to QOM realize
  a9mpcore: Embed ARMMPTimerState
  a9mpcore: Convert to QOM realize
  a9mpcore: Prepare for QOM embedding
  a15mpcore: Split off instance_init
  a15mpcore: Embed GICState
  a15mpcore: Convert to QOM realize
  a15mpcore: Prepare for QOM embedding
  a9scu: Build only once
  arm11mpcore: Fix typo in MemoryRegion name
  arm11mpcore: Drop unused fields
  arm11mpcore: Create container MemoryRegion in instance_init
  arm11mpcore: Split off SCU device
  arm11mpcore: Convert ARM11MPCorePriveState to QOM realize
  realview_gic: Convert to QOM realize
  realview_gic: Prepare for QOM embedding
  arm11mpcore: Convert mpcore_rirq_state to QOM realize
  arm11mpcore: Prepare for QOM embedding
  arm11mpcore: Split off RealView MPCore
  qdev-monitor: Clean up qdev_device_add() variable naming
  qdev-monitor: Avoid qdev as variable name
  qdev-monitor: Inline qdev_init() for device_add
  pxa: Fix typo "dettach"
  pcmcia: QOM'ify PCMCIACardState and MicroDriveState
  microdrive: Coding Style cleanups
  ide: Drop ide_init2_with_non_qdev_drives()
  pcmcia/pxa2xx: QOM'ify PXA2xxPCMCIAState

Antony Pavlov (1):
  milkymist-uart: Use Device::realize instead of SysBusDevice::init

Igor Mammedov (2):
  qdev-monitor: Fix crash when device_add is called with abstract driver
  qom: Include error.h directly in object.h

Michael S. Tsirkin (2):
  qom: Clean up struct Error references
  qom: Add pointer to int property helpers

Peter Maydell (2):
  hw/arm/boot: Make user not specifying a kernel not an error
  hw/arm: Tidy up conditional calls to arm_load_kernel()

Stefan Hajnoczi (2):
  qdev-monitor: Unref device when device_add fails
  qdev: Drop misleading qdev_free() function

 default-configs/arm-softmmu.mak  |   1 +
 hw/Makefile.objs |   1 +
 hw/acpi/piix4.c  |   2 +-
 hw/arm/armv7m.c  |  25 +--
 hw/arm/boot.c|   6 +-
 hw/arm/exynos4_boards.c  |   3 +-
 hw/arm/gumstix.c |  11 +-
 hw/arm/mainstone.c   |   5 +-
 hw/arm/omap_sx1.c|  13 +-
 hw/arm/palm.c|  13 +-
 hw/arm/z2.c  |  17 +-
 hw/block/tc58128.c   |  10 +-
 hw/char/milkymist-uart.c |  24 +--
 hw/core/qdev.c   |  12 +-
 hw/cpu/Makefile.objs |   1 +
 hw/cpu/a15mpcore.c   |  81 -
 hw/cpu/a9mpcore.c| 120 +++--
 hw/cpu/arm11mpcore.c   

[Qemu-devel] [PULL 08/58] palm: Don't enforce loading ROM or kernel for qtest

2013-10-08 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 hw/arm/palm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/palm.c b/hw/arm/palm.c
index 0b72bbe..fac4f69 100644
--- a/hw/arm/palm.c
+++ b/hw/arm/palm.c
@@ -19,6 +19,7 @@
 #include "hw/hw.h"
 #include "audio/audio.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
 #include "ui/console.h"
 #include "hw/arm/omap.h"
 #include "hw/boards.h"
@@ -255,7 +256,7 @@ static void palmte_init(QEMUMachineInitArgs *args)
 }
 }
 
-if (!rom_loaded && !kernel_filename) {
+if (!rom_loaded && !kernel_filename && !qtest_enabled()) {
 fprintf(stderr, "Kernel or ROM image must be specified\n");
 exit(1);
 }
-- 
1.8.1.4




[Qemu-devel] [PATCH 11/13] usb-host-libusb: Fill in endpoint max_streams when available

2013-10-08 Thread Hans de Goede
Signed-off-by: Hans de Goede 
---
 hw/usb/host-libusb.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index fd320cd..0dd60b2 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -720,6 +720,9 @@ static void usb_host_ep_update(USBHostDevice *s)
 struct libusb_config_descriptor *conf;
 const struct libusb_interface_descriptor *intf;
 const struct libusb_endpoint_descriptor *endp;
+#if LIBUSBX_API_VERSION >= 0x01000102
+struct libusb_ss_endpoint_companion_descriptor *endp_ss_comp;
+#endif
 uint8_t devep, type;
 int pid, ep;
 int rc, i, e;
@@ -765,6 +768,15 @@ static void usb_host_ep_update(USBHostDevice *s)
 usb_ep_set_type(udev, pid, ep, type);
 usb_ep_set_ifnum(udev, pid, ep, i);
 usb_ep_set_halted(udev, pid, ep, 0);
+#if LIBUSBX_API_VERSION >= 0x01000102
+if (type == LIBUSB_TRANSFER_TYPE_BULK &&
+libusb_get_ss_endpoint_companion_descriptor(ctx, endp,
+&endp_ss_comp) == LIBUSB_SUCCESS) {
+usb_ep_set_max_streams(udev, pid, ep,
+   endp_ss_comp->bmAttributes);
+libusb_free_ss_endpoint_companion_descriptor(endp_ss_comp);
+}
+#endif
 }
 }
 
-- 
1.8.3.1




[Qemu-devel] [PULL 23/58] a9mpcore: Embed GICState

2013-10-08 Thread Andreas Färber
From: Andreas Färber 

Prepares for conversion to QOM realize.

Reviewed-by: Peter Maydell 
Signed-off-by: Andreas Färber 
---
 hw/cpu/a9mpcore.c | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
index acbdab5..c57b149 100644
--- a/hw/cpu/a9mpcore.c
+++ b/hw/cpu/a9mpcore.c
@@ -9,6 +9,7 @@
  */
 
 #include "hw/sysbus.h"
+#include "hw/intc/arm_gic.h"
 
 #define TYPE_A9MPCORE_PRIV "a9mpcore_priv"
 #define A9MPCORE_PRIV(obj) \
@@ -23,15 +24,17 @@ typedef struct A9MPPrivState {
 MemoryRegion container;
 DeviceState *mptimer;
 DeviceState *wdt;
-DeviceState *gic;
 DeviceState *scu;
 uint32_t num_irq;
+
+GICState gic;
 } A9MPPrivState;
 
 static void a9mp_priv_set_irq(void *opaque, int irq, int level)
 {
 A9MPPrivState *s = (A9MPPrivState *)opaque;
-qemu_set_irq(qdev_get_gpio_in(s->gic, irq), level);
+
+qemu_set_irq(qdev_get_gpio_in(DEVICE(&s->gic), irq), level);
 }
 
 static void a9mp_priv_initfn(Object *obj)
@@ -40,19 +43,23 @@ static void a9mp_priv_initfn(Object *obj)
 
 memory_region_init(&s->container, obj, "a9mp-priv-container", 0x2000);
 sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->container);
+
+object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
+qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
 }
 
 static int a9mp_priv_init(SysBusDevice *dev)
 {
 A9MPPrivState *s = A9MPCORE_PRIV(dev);
+DeviceState *gicdev;
 SysBusDevice *timerbusdev, *wdtbusdev, *gicbusdev, *scubusdev;
 int i;
 
-s->gic = qdev_create(NULL, "arm_gic");
-qdev_prop_set_uint32(s->gic, "num-cpu", s->num_cpu);
-qdev_prop_set_uint32(s->gic, "num-irq", s->num_irq);
-qdev_init_nofail(s->gic);
-gicbusdev = SYS_BUS_DEVICE(s->gic);
+gicdev = DEVICE(&s->gic);
+qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
+qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
+qdev_init_nofail(gicdev);
+gicbusdev = SYS_BUS_DEVICE(&s->gic);
 
 /* Pass through outbound IRQ lines from the GIC */
 sysbus_pass_irq(dev, gicbusdev);
@@ -107,9 +114,9 @@ static int a9mp_priv_init(SysBusDevice *dev)
 for (i = 0; i < s->num_cpu; i++) {
 int ppibase = (s->num_irq - 32) + i * 32;
 sysbus_connect_irq(timerbusdev, i,
-   qdev_get_gpio_in(s->gic, ppibase + 29));
+   qdev_get_gpio_in(gicdev, ppibase + 29));
 sysbus_connect_irq(wdtbusdev, i,
-   qdev_get_gpio_in(s->gic, ppibase + 30));
+   qdev_get_gpio_in(gicdev, ppibase + 30));
 }
 return 0;
 }
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events

2013-10-08 Thread Alex Bligh
Hans,

On 8 Oct 2013, at 21:16, Hans de Goede wrote:

> No, it is calling main_loop_wait with nonblocking set to 0, so
> normally the lock would get released. But
> timerlistgroup_deadline_ns(&main_loop_tlg) is returning 0,
> causing timeout_ns to be 0, and this causes the lock to not get
> released.

OK so my question is *why* is timerlistgroup_deadline_ns
returning 0 every time? That should only ever happen if there is an
expired timer every time timerlistgroup_deadline_ns is called.
Given the spin warning only prints (or is only meant to print)
if this happens 1,000 times consecutively, this implies to me
that something is setting a timer wrongly, as timers should
not constantly expire.

I would agree that this changeset may have introduced the symptom
(because before we didn't generate a zero timeout this way, we
broke out of the select loop), but I'd love to know WHY there
is an expired timereach time and what that timer is. Perhaps
something is using a timeout value in milliseconds not realising it
is nanoseconds (or similar).

My worry is that setting 1ns may be just hiding a bug here.

We could relatively easily store __FILE__ and __LINE__ when
timers are created to make it easier to track this sort of thing
(perhaps protected with an ifdef).

> I'm quite sure this is what is happing because once my
> bisect pointed to the "aio / timers: Convert mainloop to use timeout"
> commit as a culprit, I read that commit very carefully multiple
> times and that seemed like the only problem it could cause,
> so I added a debug printf to test for that case, and it triggered.
> 
> What I believe is happening in my troublesome scenario is that one
> thread is calling main_loop_wait(0) repeatedly, waiting for another
> thread to do some work (*), but that other thread is not getting a
> chance to do that work because the iothread never gets unlocked.

That's fine, but that doesn't explain why timerlistgroup_deadline_ns
thinks that a timer has always expired.

> *) likely the spice-server thread which does a lot of work for
> the qxl device
> 
> 
>> 
>> The comment at line 208 suggests that "the I/O thread is very busy
>> or we are incorrectly busy waiting in the I/O thread". Do we know
>> which is happening? Perhaps rather than give up the io_thread
>> mutex on every call (which is in practice what a 1 nanosecond
>> timeout does) we should give it up if we have not released
>> it for X nanoseconds (maybe X=250us), or on every Y calls. I think
>> someone other than me should consider the effect of dropping and
>> reacquiring a mutex so frequently under heavy I/O load, but I'm not
>> sure it's a great idea.
> 
> We're only waiting so short if there are timers which want to run
> immediately, normally we would wait a lot longer.
> 
>> So on reflection you might be more right with 1 nanosecond than
>> 250us as a timeout of 250us, but I wonder whether a strategy
>> of just dropping the lock occasionally (and still using a zero
>> timeout) might be better.
> 
> Paolo probably has some better insights on this, but he seems to
> have called it a day for today, and I'm going to do the same :)
> 
> So lets continue this tomorrow.

... the trouble with reading email from top to bottom :-)

> 
> Regards,
> 
> Hans
> 
> 
> 

-- 
Alex Bligh







[Qemu-devel] [PATCH 07/13] usb-hcd-xhci: Update endpoint context dequeue pointer for streams too

2013-10-08 Thread Hans de Goede
With streams the endpoint context dequeue pointer should point to the
dequeue value for the currently active stream.

At least Linux guests expect it to point to value set by an set_ep_dequeue
upon completion of the set_ep_dequeue (before kicking the ep).

Otherwise the Linux kernel will complain (and things won't work):

xhci_hcd :00:05.0: Mismatch between completed Set TR Deq Ptr command & xHCI 
internal state.
xhci_hcd :00:05.0: ep deq seg = 8800366f0880, deq ptr = 8800366ec010

Signed-off-by: Hans de Goede 
---
 hw/usb/hcd-xhci.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 0131151..fa27299 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1187,6 +1187,7 @@ static XHCIStreamContext *xhci_find_stream(XHCIEPContext 
*epctx,
 static void xhci_set_ep_state(XHCIState *xhci, XHCIEPContext *epctx,
   XHCIStreamContext *sctx, uint32_t state)
 {
+XHCIRing *ring = NULL;
 uint32_t ctx[5];
 uint32_t ctx2[2];
 
@@ -1197,6 +1198,7 @@ static void xhci_set_ep_state(XHCIState *xhci, 
XHCIEPContext *epctx,
 /* update ring dequeue ptr */
 if (epctx->nr_pstreams) {
 if (sctx != NULL) {
+ring = &sctx->ring;
 xhci_dma_read_u32s(xhci, sctx->pctx, ctx2, sizeof(ctx2));
 ctx2[0] &= 0xe;
 ctx2[0] |= sctx->ring.dequeue | sctx->ring.ccs;
@@ -1204,8 +1206,12 @@ static void xhci_set_ep_state(XHCIState *xhci, 
XHCIEPContext *epctx,
 xhci_dma_write_u32s(xhci, sctx->pctx, ctx2, sizeof(ctx2));
 }
 } else {
-ctx[2] = epctx->ring.dequeue | epctx->ring.ccs;
-ctx[3] = (epctx->ring.dequeue >> 16) >> 16;
+ring = &epctx->ring;
+}
+if (ring) {
+ctx[2] = ring->dequeue | ring->ccs;
+ctx[3] = (ring->dequeue >> 16) >> 16;
+
 DPRINTF("xhci: set epctx: " DMA_ADDR_FMT " state=%d 
dequeue=%08x%08x\n",
 epctx->pctx, state, ctx[3], ctx[2]);
 }
-- 
1.8.3.1




[Qemu-devel] [PATCH 00/13] usb: Add support for bulk streams to usb-host-libusb

2013-10-08 Thread Hans de Goede
Hi Gerd et al,

After quite a bit of work I'm very happy to present this patch set which adds
full support for using USB-3 devices which use bulkstreams with qemu's usb
host redirection.

This has been tested under a Linux guest, with an uas usb-3 device, and
works well and stable.

There is a bunch of pre-requisites to be able to use this though (and for
it to be stable / reliable). The Linux host will need a kernel with a bunch
of xhci driver fixes as well as patches to extend the usbfs API with streams
support. I've submitted all these upstream and I hope they get merged into the
3.13 kernel.

Further more a version of libusbx with bulk stream support is necessary, since
the usbfs API is not yet in the upstream kernel (and thus not yet finalized),
I've not pushed the necessary libusbx patches upstream yet, for now you can
find them in my libusbx repo here:
https://github.com/jwrdegoede/libusbx/commits/master

I'm an upstream libusbx committer and I will push these upstream as soon as
the kernel API has been merged (which I expect to happen without any further
changes to it).

Note the last 2 patches in the set depend on the new libusbx APIs for this,
so if you're afraid the API may still change you could consider delaying
merging these, but I'm quite confident that there will be no further API
changes.

Regards,

Hans



[Qemu-devel] [PULL 51/58] qom: Include error.h directly in object.h

2013-10-08 Thread Andreas Färber
From: Igor Mammedov 

qapi/error.h is simple enough to be included in qom/object.h
directly and prepares qom/object.h to use Error typedef.

Signed-off-by: Igor Mammedov 
Signed-off-by: Michael S. Tsirkin 
Signed-off-by: Andreas Färber 
---
 include/qom/object.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 1a7b71a..d9a0063 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -18,9 +18,9 @@
 #include 
 #include 
 #include "qemu/queue.h"
+#include "qapi/error.h"
 
 struct Visitor;
-struct Error;
 
 struct TypeImpl;
 typedef struct TypeImpl *Type;
-- 
1.8.1.4




[Qemu-devel] [PULL 13/58] mcf5208: Don't enforce use of kernel for qtest

2013-10-08 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 hw/m68k/mcf5208.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index fb96fe8..6e30c0b 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -10,6 +10,7 @@
 #include "qemu/timer.h"
 #include "hw/ptimer.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
 #include "net/net.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
@@ -267,6 +268,9 @@ static void mcf5208evb_init(QEMUMachineInitArgs *args)
 
 /* Load kernel.  */
 if (!kernel_filename) {
+if (qtest_enabled()) {
+return;
+}
 fprintf(stderr, "Kernel image must be specified\n");
 exit(1);
 }
-- 
1.8.1.4




[Qemu-devel] [PULL 44/58] arm11mpcore: Split off RealView MPCore

2013-10-08 Thread Andreas Färber
Reviewed-by: Peter Maydell 
Signed-off-by: Andreas Färber 
---
 hw/cpu/Makefile.objs |   1 +
 hw/cpu/arm11mpcore.c | 121 -
 hw/cpu/realview_mpcore.c | 139 +++
 3 files changed, 140 insertions(+), 121 deletions(-)
 create mode 100644 hw/cpu/realview_mpcore.c

diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs
index df287c1..6381238 100644
--- a/hw/cpu/Makefile.objs
+++ b/hw/cpu/Makefile.objs
@@ -1,4 +1,5 @@
 obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o
+obj-$(CONFIG_REALVIEW) += realview_mpcore.o
 obj-$(CONFIG_A9MPCORE) += a9mpcore.o
 obj-$(CONFIG_A15MPCORE) += a15mpcore.o
 obj-$(CONFIG_ICC_BUS) += icc_bus.o
diff --git a/hw/cpu/arm11mpcore.c b/hw/cpu/arm11mpcore.c
index 0ec27c7..717d3e4 100644
--- a/hw/cpu/arm11mpcore.c
+++ b/hw/cpu/arm11mpcore.c
@@ -134,126 +134,6 @@ static void mpcore_priv_initfn(Object *obj)
 qdev_set_parent_bus(DEVICE(&s->wdtimer), sysbus_get_default());
 }
 
-#define TYPE_REALVIEW_MPCORE_RIRQ "realview_mpcore"
-#define REALVIEW_MPCORE_RIRQ(obj) \
-OBJECT_CHECK(mpcore_rirq_state, (obj), TYPE_REALVIEW_MPCORE_RIRQ)
-
-/* Dummy PIC to route IRQ lines.  The baseboard has 4 independent IRQ
-   controllers.  The output of these, plus some of the raw input lines
-   are fed into a single SMP-aware interrupt controller on the CPU.  */
-typedef struct {
-SysBusDevice parent_obj;
-
-qemu_irq cpuic[32];
-qemu_irq rvic[4][64];
-uint32_t num_cpu;
-
-ARM11MPCorePriveState priv;
-RealViewGICState gic[4];
-} mpcore_rirq_state;
-
-/* Map baseboard IRQs onto CPU IRQ lines.  */
-static const int mpcore_irq_map[32] = {
--1, -1, -1, -1,  1,  2, -1, -1,
--1, -1,  6, -1,  4,  5, -1, -1,
--1, 14, 15,  0,  7,  8, -1, -1,
--1, -1, -1, -1,  9,  3, -1, -1,
-};
-
-static void mpcore_rirq_set_irq(void *opaque, int irq, int level)
-{
-mpcore_rirq_state *s = (mpcore_rirq_state *)opaque;
-int i;
-
-for (i = 0; i < 4; i++) {
-qemu_set_irq(s->rvic[i][irq], level);
-}
-if (irq < 32) {
-irq = mpcore_irq_map[irq];
-if (irq >= 0) {
-qemu_set_irq(s->cpuic[irq], level);
-}
-}
-}
-
-static void realview_mpcore_realize(DeviceState *dev, Error **errp)
-{
-SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
-mpcore_rirq_state *s = REALVIEW_MPCORE_RIRQ(dev);
-DeviceState *priv = DEVICE(&s->priv);
-DeviceState *gic;
-SysBusDevice *gicbusdev;
-Error *err = NULL;
-int n;
-int i;
-
-qdev_prop_set_uint32(priv, "num-cpu", s->num_cpu);
-object_property_set_bool(OBJECT(&s->priv), true, "realized", &err);
-if (err != NULL) {
-error_propagate(errp, err);
-return;
-}
-sysbus_pass_irq(sbd, SYS_BUS_DEVICE(&s->priv));
-for (i = 0; i < 32; i++) {
-s->cpuic[i] = qdev_get_gpio_in(priv, i);
-}
-/* ??? IRQ routing is hardcoded to "normal" mode.  */
-for (n = 0; n < 4; n++) {
-object_property_set_bool(OBJECT(&s->gic[n]), true, "realized", &err);
-if (err != NULL) {
-error_propagate(errp, err);
-return;
-}
-gic = DEVICE(&s->gic[n]);
-gicbusdev = SYS_BUS_DEVICE(&s->gic[n]);
-sysbus_mmio_map(gicbusdev, 0, 0x1004 + n * 0x1);
-sysbus_connect_irq(gicbusdev, 0, s->cpuic[10 + n]);
-for (i = 0; i < 64; i++) {
-s->rvic[n][i] = qdev_get_gpio_in(gic, i);
-}
-}
-qdev_init_gpio_in(dev, mpcore_rirq_set_irq, 64);
-}
-
-static void mpcore_rirq_init(Object *obj)
-{
-SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
-mpcore_rirq_state *s = REALVIEW_MPCORE_RIRQ(obj);
-SysBusDevice *privbusdev;
-int i;
-
-object_initialize(&s->priv, sizeof(s->priv), TYPE_ARM11MPCORE_PRIV);
-qdev_set_parent_bus(DEVICE(&s->priv), sysbus_get_default());
-privbusdev = SYS_BUS_DEVICE(&s->priv);
-sysbus_init_mmio(sbd, sysbus_mmio_get_region(privbusdev, 0));
-
-for (i = 0; i < 4; i++) {
-object_initialize(&s->gic[i], sizeof(s->gic[i]), TYPE_REALVIEW_GIC);
-qdev_set_parent_bus(DEVICE(&s->gic[i]), sysbus_get_default());
-}
-}
-
-static Property mpcore_rirq_properties[] = {
-DEFINE_PROP_UINT32("num-cpu", mpcore_rirq_state, num_cpu, 1),
-DEFINE_PROP_END_OF_LIST(),
-};
-
-static void mpcore_rirq_class_init(ObjectClass *klass, void *data)
-{
-DeviceClass *dc = DEVICE_CLASS(klass);
-
-dc->realize = realview_mpcore_realize;
-dc->props = mpcore_rirq_properties;
-}
-
-static const TypeInfo mpcore_rirq_info = {
-.name  = TYPE_REALVIEW_MPCORE_RIRQ,
-.parent= TYPE_SYS_BUS_DEVICE,
-.instance_size = sizeof(mpcore_rirq_state),
-.instance_init = mpcore_rirq_init,
-.class_init= mpcore_rirq_class_init,
-};
-
 static Property mpcore_priv_properties[] = {
 DEFINE_PROP_UINT32("num-cpu", ARM11MPCorePriveState, num_cpu, 1),
 /* The ARM11 MPCORE TRM says the on-chip controller may have
@@ -286,7 +1

[Qemu-devel] [PATCH 09/13] usb: Add usb_device_alloc/free_streams

2013-10-08 Thread Hans de Goede
Signed-off-by: Hans de Goede 
---
 hw/usb/bus.c | 18 ++
 include/hw/usb.h | 12 
 2 files changed, 30 insertions(+)

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 72d5b92..bba554c 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -203,6 +203,24 @@ void usb_device_ep_stopped(USBDevice *dev, USBEndpoint *ep)
 }
 }
 
+int usb_device_alloc_streams(USBDevice *dev, USBEndpoint **eps, int nr_eps,
+ int streams)
+{
+USBDeviceClass *klass = USB_DEVICE_GET_CLASS(dev);
+if (klass->alloc_streams) {
+return klass->alloc_streams(dev, eps, nr_eps, streams);
+}
+return 0;
+}
+
+void usb_device_free_streams(USBDevice *dev, USBEndpoint **eps, int nr_eps)
+{
+USBDeviceClass *klass = USB_DEVICE_GET_CLASS(dev);
+if (klass->free_streams) {
+klass->free_streams(dev, eps, nr_eps);
+}
+}
+
 static int usb_qdev_init(DeviceState *qdev)
 {
 USBDevice *dev = USB_DEVICE(qdev);
diff --git a/include/hw/usb.h b/include/hw/usb.h
index e9d96ba..0a6ef4a 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -315,6 +315,14 @@ typedef struct USBDeviceClass {
  */
 void (*ep_stopped)(USBDevice *dev, USBEndpoint *ep);
 
+/*
+ * Called by the hcd to alloc / free streams on a bulk endpoint.
+ * Optional may be NULL.
+ */
+int (*alloc_streams)(USBDevice *dev, USBEndpoint **eps, int nr_eps,
+ int streams);
+void (*free_streams)(USBDevice *dev, USBEndpoint **eps, int nr_eps);
+
 const char *product_desc;
 const USBDesc *usb_desc;
 } USBDeviceClass;
@@ -553,6 +561,10 @@ void usb_device_flush_ep_queue(USBDevice *dev, USBEndpoint 
*ep);
 
 void usb_device_ep_stopped(USBDevice *dev, USBEndpoint *ep);
 
+int usb_device_alloc_streams(USBDevice *dev, USBEndpoint **eps, int nr_eps,
+ int streams);
+void usb_device_free_streams(USBDevice *dev, USBEndpoint **eps, int nr_eps);
+
 const char *usb_device_get_product_desc(USBDevice *dev);
 
 const USBDesc *usb_device_get_usb_desc(USBDevice *dev);
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events

2013-10-08 Thread Hans de Goede

Hi,

On 10/08/2013 10:01 PM, Alex Bligh wrote:




The purpose of the 1 ns timeout is to cause os_host_main_loop_wait
to unlock the iothread, as $subject says the problem I'm seeing seems
to be lock starvation not cpu starvation.

Note as I already indicated I'm in no way an expert in this, if you
and or Paolo suspect cpu starvation may happen too, then bumping
the timeout to 250 us is fine with me too.

If we go with 250 us that thus pose the question though if we should
always keep a minimum timeout of 250 us when not non-blocking, or only
bump it to 250 us when main_loop_tlg has already expired events and
thus is causing a timeout of 0.


I am by no means an expert in the iothread bit, so let's pool our
ignorance ... :-)

Somewhere within that patch series (7b595f35 I think) I fixed up
the spin counter bit, which made it slightly less yucky and work
with milliseconds. I hope I didn't break it but there seems
something slightly odd about the use case here.

If you are getting the spin error, this implies something is
pretty much constantly polling os_host_main_loop_wait with a
zero timeout. As you point out this is going to be main_loop_wait
and almost certainly main_loop_wait called with nonblocking
set to 1.


No, it is calling main_loop_wait with nonblocking set to 0, so
normally the lock would get released. But
timerlistgroup_deadline_ns(&main_loop_tlg) is returning 0,
causing timeout_ns to be 0, and this causes the lock to not get
released.

I'm quite sure this is what is happing because once my
bisect pointed to the "aio / timers: Convert mainloop to use timeout"
commit as a culprit, I read that commit very carefully multiple
times and that seemed like the only problem it could cause,
so I added a debug printf to test for that case, and it triggered.

What I believe is happening in my troublesome scenario is that one
thread is calling main_loop_wait(0) repeatedly, waiting for another
thread to do some work (*), but that other thread is not getting a
chance to do that work because the iothread never gets unlocked.

*) likely the spice-server thread which does a lot of work for
the qxl device




The comment at line 208 suggests that "the I/O thread is very busy
or we are incorrectly busy waiting in the I/O thread". Do we know
which is happening? Perhaps rather than give up the io_thread
mutex on every call (which is in practice what a 1 nanosecond
timeout does) we should give it up if we have not released
it for X nanoseconds (maybe X=250us), or on every Y calls. I think
someone other than me should consider the effect of dropping and
reacquiring a mutex so frequently under heavy I/O load, but I'm not
sure it's a great idea.


We're only waiting so short if there are timers which want to run
immediately, normally we would wait a lot longer.


So on reflection you might be more right with 1 nanosecond than
250us as a timeout of 250us, but I wonder whether a strategy
of just dropping the lock occasionally (and still using a zero
timeout) might be better.


Paolo probably has some better insights on this, but he seems to
have called it a day for today, and I'm going to do the same :)

So lets continue this tomorrow.

Regards,

Hans




[Qemu-devel] [PATCH 01/13] usb-host-libusb: Fix reset handling

2013-10-08 Thread Hans de Goede
The guest will issue an initial device reset when the device is attached, but
since the current usb-host-libusb code only actually does the reset when
udev->configuration != 0, and on attach the device is not yet configured,
the reset gets ignored. This means that the device gets passed to the guest
in an unknown state, which is not good.

The udev->configuration check is there because of the release / claim
interfaces done around the libusb_device_reset call, but these are not
necessary. If interfaces are claimed when libusb_device_reset gets called
libusb will release + reclaim them itself.

The usb_host_ep_update call also is not necessary. If the reset succeeds the
original config and interface alt settings will be restored.

Last if the reset fails, that means the device has either disconnected or
morphed into an another device and has been completely re-enumerated,
so it is treated by the host as a new device and our handle is invalid,
so on reset failure we need to call usb_host_nodev().

Signed-off-by: Hans de Goede 
---
 hw/usb/host-libusb.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 128955d..428c7c5 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -1256,16 +1256,14 @@ static void usb_host_flush_ep_queue(USBDevice *dev, 
USBEndpoint *ep)
 static void usb_host_handle_reset(USBDevice *udev)
 {
 USBHostDevice *s = USB_HOST_DEVICE(udev);
+int rc;
 
 trace_usb_host_reset(s->bus_num, s->addr);
 
-if (udev->configuration == 0) {
-return;
+rc = libusb_reset_device(s->dh);
+if (rc != 0) {
+usb_host_nodev(s);
 }
-usb_host_release_interfaces(s);
-libusb_reset_device(s->dh);
-usb_host_claim_interfaces(s, 0);
-usb_host_ep_update(s);
 }
 
 /*
-- 
1.8.3.1




[Qemu-devel] [PULL 18/58] leon3: Don't enforce use of -bios with qtest

2013-10-08 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 hw/sparc/leon3.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 390f3e4..c583c3d 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -26,6 +26,7 @@
 #include "hw/ptimer.h"
 #include "sysemu/char.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
 #include "elf.h"
@@ -178,7 +179,7 @@ static void leon3_generic_hw_init(QEMUMachineInitArgs *args)
 fprintf(stderr, "qemu: could not load prom '%s'\n", filename);
 exit(1);
 }
-} else if (kernel_filename == NULL) {
+} else if (kernel_filename == NULL && !qtest_enabled()) {
 fprintf(stderr, "Can't read bios image %s\n", filename);
 exit(1);
 }
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events

2013-10-08 Thread Alex Bligh

On 8 Oct 2013, at 21:01, Alex Bligh wrote:

> Somewhere within that patch series (7b595f35 I think) I fixed up
> the spin counter bit, which made it slightly less yucky and work
> with milliseconds.

"with nanoseconds rather than microseconds" - oops

-- 
Alex Bligh







Re: [Qemu-devel] [PATCH v2] Add interface to traverse the qmp command list by QmpCommand

2013-10-08 Thread Michael Roth
Quoting Mark Wu (2013-10-08 01:23:09)
> In the original code, qmp_get_command_list is used to construct
> a list of all commands' name. To get the information of all qga
> commands, it traverses the name list and search the command info
> with its name.  So it can cause O(n^2) in the number of commands.
> 
> This patch adds an interface to traverse the qmp command list by
> QmpCommand to replace qmp_get_command_list. It can decrease the
> complexity from O(n^2) to O(n).
> 
> Signed-off-by: Mark Wu 
> ---
> Changes:
> v2:
> 1. Keep the signature of qmp_command_is_enabled (per Eric and Michael)
> 2. Remove the unnecessary pointer castings (per Eric)
> 
>  include/qapi/qmp/dispatch.h |  5 ++--
>  qapi/qmp-registry.c | 27 +++---
>  qga/commands.c  | 38 ++---
>  qga/main.c  | 68 
> +
>  4 files changed, 48 insertions(+), 90 deletions(-)
> 
> diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
> index 1ce11f5..b6eb49e 100644
> --- a/include/qapi/qmp/dispatch.h
> +++ b/include/qapi/qmp/dispatch.h
> @@ -47,9 +47,10 @@ QmpCommand *qmp_find_command(const char *name);
>  QObject *qmp_dispatch(QObject *request);
>  void qmp_disable_command(const char *name);
>  void qmp_enable_command(const char *name);
> -bool qmp_command_is_enabled(const char *name);
> -char **qmp_get_command_list(void);
> +bool qmp_command_is_enabled(const QmpCommand *cmd);
>  QObject *qmp_build_error_object(Error *errp);
> +typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque);
> +void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque);
> 
>  #endif
> 
> diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
> index 28bbbe8..3fcf10e 100644
> --- a/qapi/qmp-registry.c
> +++ b/qapi/qmp-registry.c
> @@ -66,35 +66,16 @@ void qmp_enable_command(const char *name)
>  qmp_toggle_command(name, true);
>  }
> 
> -bool qmp_command_is_enabled(const char *name)
> +bool qmp_command_is_enabled(const QmpCommand *cmd)
>  {
> -QmpCommand *cmd;
> -
> -QTAILQ_FOREACH(cmd, &qmp_commands, node) {
> -if (strcmp(cmd->name, name) == 0) {
> -return cmd->enabled;
> -}
> -}
> -
> -return false;
> +return cmd->enabled;
>  }
> 
> -char **qmp_get_command_list(void)
> +void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque)
>  {
>  QmpCommand *cmd;
> -int count = 1;
> -char **list_head, **list;
> -
> -QTAILQ_FOREACH(cmd, &qmp_commands, node) {
> -count++;
> -}
> -
> -list_head = list = g_malloc0(count * sizeof(char *));
> 
>  QTAILQ_FOREACH(cmd, &qmp_commands, node) {
> -*list = g_strdup(cmd->name);
> -list++;
> +fn(cmd, opaque);
>  }
> -
> -return list_head;
>  }
> diff --git a/qga/commands.c b/qga/commands.c
> index 528b082..063b22b 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -45,35 +45,27 @@ void qmp_guest_ping(Error **err)
>  slog("guest-ping called");
>  }
> 
> -struct GuestAgentInfo *qmp_guest_info(Error **err)
> +static void qmp_command_info(QmpCommand *cmd, void *opaque)
>  {
> -GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
> +GuestAgentInfo *info = opaque;
>  GuestAgentCommandInfo *cmd_info;
>  GuestAgentCommandInfoList *cmd_info_list;
> -char **cmd_list_head, **cmd_list;
> -
> -info->version = g_strdup(QEMU_VERSION);
> -
> -cmd_list_head = cmd_list = qmp_get_command_list();
> -if (*cmd_list_head == NULL) {
> -goto out;
> -}
> 
> -while (*cmd_list) {
> -cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
> -cmd_info->name = g_strdup(*cmd_list);
> -cmd_info->enabled = qmp_command_is_enabled(cmd_info->name);
> +cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
> +cmd_info->name = g_strdup(cmd->name);
> +cmd_info->enabled = qmp_command_is_enabled(cmd);
> 
> -cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
> -cmd_info_list->value = cmd_info;
> -cmd_info_list->next = info->supported_commands;
> -info->supported_commands = cmd_info_list;
> +cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
> +cmd_info_list->value = cmd_info;
> +cmd_info_list->next = info->supported_commands;
> +info->supported_commands = cmd_info_list;
> +}
> 
> -g_free(*cmd_list);
> -cmd_list++;
> -}
> +struct GuestAgentInfo *qmp_guest_info(Error **err)
> +{
> +GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
> 
> -out:
> -g_free(cmd_list_head);
> +info->version = g_strdup(QEMU_VERSION);
> +qmp_for_each_command(qmp_command_info, info);
>  return info;
>  }
> diff --git a/qga/main.c b/qga/main.c
> index 6c746c8..ff2ee03 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -347,48 +347,34 @@ static gint ga_strcmp(gconstpointer str1, gconstpointer 
> str2)
>  }
> 
>  /* disable commands that 

[Qemu-devel] [PATCH 13/13] usb-host-libusb: Set stream id when submitting bulk-stream transfers

2013-10-08 Thread Hans de Goede
Signed-off-by: Hans de Goede 
---
 hw/usb/host-libusb.c | 20 
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 894875b..3376b96 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -1214,10 +1214,22 @@ static void usb_host_handle_data(USBDevice *udev, 
USBPacket *p)
 usb_packet_copy(p, r->buffer, size);
 }
 ep = p->ep->nr | (r->in ? USB_DIR_IN : 0);
-libusb_fill_bulk_transfer(r->xfer, s->dh, ep,
-  r->buffer, size,
-  usb_host_req_complete_data, r,
-  BULK_TIMEOUT);
+if (p->stream) {
+#if LIBUSBX_API_VERSION >= 0x01000103
+libusb_fill_bulk_stream_transfer(r->xfer, s->dh, ep, p->stream,
+ r->buffer, size,
+ usb_host_req_complete_data, r,
+ BULK_TIMEOUT);
+#else
+usb_host_req_free(r);
+return USB_RET_STALL;
+#endif
+} else {
+libusb_fill_bulk_transfer(r->xfer, s->dh, ep,
+  r->buffer, size,
+  usb_host_req_complete_data, r,
+  BULK_TIMEOUT);
+}
 break;
 case USB_ENDPOINT_XFER_INT:
 r = usb_host_req_alloc(s, p, p->pid == USB_TOKEN_IN, p->iov.size);
-- 
1.8.3.1




[Qemu-devel] [PATCH 12/13] usb-host-libusb: Add alloc / free streams ops

2013-10-08 Thread Hans de Goede
Signed-off-by: Hans de Goede 
---
 hw/usb/host-libusb.c | 50 ++
 1 file changed, 50 insertions(+)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 0dd60b2..894875b 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -1280,6 +1280,54 @@ static void usb_host_handle_reset(USBDevice *udev)
 }
 }
 
+static int usb_host_alloc_streams(USBDevice *udev, USBEndpoint **eps,
+  int nr_eps, int streams)
+{
+#if LIBUSBX_API_VERSION >= 0x01000103
+USBHostDevice *s = USB_HOST_DEVICE(udev);
+unsigned char endpoints[30];
+int i, rc;
+
+for (i = 0; i < nr_eps; i++) {
+endpoints[i] = eps[i]->nr;
+if (eps[i]->pid == USB_TOKEN_IN) {
+endpoints[i] |= 0x80;
+}
+}
+rc = libusb_alloc_streams(s->dh, streams, endpoints, nr_eps);
+if (rc < 0) {
+usb_host_libusb_error("libusb_alloc_streams", rc);
+} else if (rc != streams) {
+fprintf(stderr,
+"libusb_alloc_streams: got less streams then requested %d < %d\n",
+rc, streams);
+}
+
+return (rc == streams) ? 0 : -1;
+#else
+fprintf(stderr, "libusb_alloc_streams: error not implemented\n");
+return -1;
+#endif
+}
+
+static void usb_host_free_streams(USBDevice *udev, USBEndpoint **eps,
+  int nr_eps)
+{
+#if LIBUSBX_API_VERSION >= 0x01000103
+USBHostDevice *s = USB_HOST_DEVICE(udev);
+unsigned char endpoints[30];
+int i;
+
+for (i = 0; i < nr_eps; i++) {
+endpoints[i] = eps[i]->nr;
+if (eps[i]->pid == USB_TOKEN_IN) {
+endpoints[i] |= 0x80;
+}
+}
+libusb_free_streams(s->dh, endpoints, nr_eps);
+#endif
+}
+
 /*
  * This is *NOT* about restoring state.  We have absolutely no idea
  * what state the host device is in at the moment and whenever it is
@@ -1361,6 +1409,8 @@ static void usb_host_class_initfn(ObjectClass *klass, 
void *data)
 uc->handle_reset   = usb_host_handle_reset;
 uc->handle_destroy = usb_host_handle_destroy;
 uc->flush_ep_queue = usb_host_flush_ep_queue;
+uc->alloc_streams  = usb_host_alloc_streams;
+uc->free_streams   = usb_host_free_streams;
 dc->vmsd = &vmstate_usb_host;
 dc->props = usb_host_dev_properties;
 set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-- 
1.8.3.1




[Qemu-devel] [PATCH 10/13] xhci: Call usb_device_alloc/free_streams

2013-10-08 Thread Hans de Goede
Note this code is not as KISS as I would like, the reason for this is that
the Linux kernel interface wants streams on eps belonging to one interface
to be allocated in one call. Things will also work if we do this one ep at a
time (as long as all eps support the same amount of streams), but lets stick
to the kernel API.

Signed-off-by: Hans de Goede 
---
 hw/usb/hcd-xhci.c | 117 ++
 1 file changed, 117 insertions(+)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index fa27299..c5889a9 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1151,6 +1151,111 @@ static void xhci_free_streams(XHCIEPContext *epctx)
 epctx->nr_pstreams = 0;
 }
 
+static int xhci_epmask_to_eps_with_streams(XHCIState *xhci,
+   unsigned int slotid,
+   uint32_t epmask,
+   XHCIEPContext **epctxs,
+   USBEndpoint **eps)
+{
+XHCISlot *slot;
+XHCIEPContext *epctx;
+USBEndpoint *ep;
+int i, j;
+
+assert(slotid >= 1 && slotid <= xhci->numslots);
+
+slot = &xhci->slots[slotid - 1];
+
+for (i = 2, j = 0; i <= 31; i++) {
+if (!(epmask & (1 << i))) {
+continue;
+}
+
+epctx = slot->eps[i - 1];
+ep = xhci_epid_to_usbep(xhci, slotid, i);
+if (!epctx || !epctx->nr_pstreams || !ep) {
+continue;
+}
+
+if (epctxs) {
+epctxs[j] = epctx;
+}
+eps[j++] = ep;
+}
+return j;
+}
+
+static void xhci_free_device_streams(XHCIState *xhci, unsigned int slotid,
+ uint32_t epmask)
+{
+USBEndpoint *eps[30];
+int nr_eps;
+
+nr_eps = xhci_epmask_to_eps_with_streams(xhci, slotid, epmask, NULL, eps);
+if (nr_eps) {
+usb_device_free_streams(eps[0]->dev, eps, nr_eps);
+}
+}
+
+static TRBCCode xhci_alloc_device_streams(XHCIState *xhci, unsigned int slotid,
+  uint32_t epmask)
+{
+XHCIEPContext *epctxs[30];
+USBEndpoint *eps[30];
+int i, r, nr_eps, req_nr_streams, dev_max_streams;
+
+nr_eps = xhci_epmask_to_eps_with_streams(xhci, slotid, epmask, epctxs,
+ eps);
+if (nr_eps == 0) {
+return CC_SUCCESS;
+}
+
+req_nr_streams = epctxs[0]->nr_pstreams;
+dev_max_streams = eps[0]->max_streams;
+
+for (i = 1; i < nr_eps; i++) {
+/*
+ * HdG: I don't expect these to ever trigger, but if they do we need
+ * to come up with another solution, ie group identical endpoints
+ * together and make an usb_device_alloc_streams call per group.
+ */
+if (epctxs[i]->nr_pstreams != req_nr_streams) {
+FIXME("guest streams config not identical for all eps");
+return CC_RESOURCE_ERROR;
+}
+if (eps[i]->max_streams != dev_max_streams) {
+FIXME("device streams config not identical for all eps");
+return CC_RESOURCE_ERROR;
+}
+}
+
+/*
+ * max-streams in both the device descriptor and in the controller is a
+ * power of 2. But stream id 0 is reserved, so if a device can do up to 4
+ * streams the guest will ask for 5 rounded up to the next power of 2 which
+ * becomes 8. For emulated devices usb_device_alloc_streams is a nop.
+ *
+ * For redirected devices however this is an issue, as there we must ask
+ * the real xhci controller to alloc streams, and the host driver for the
+ * real xhci controller will likely disallow allocating more streams then
+ * the device can handle.
+ *
+ * So we limit the requested nr_streams to the maximum number the device
+ * can handle.
+ */
+if (req_nr_streams > dev_max_streams) {
+req_nr_streams = dev_max_streams;
+}
+
+r = usb_device_alloc_streams(eps[0]->dev, eps, nr_eps, req_nr_streams);
+if (r != 0) {
+fprintf(stderr, "xhci: alloc streams failed\n");
+return CC_RESOURCE_ERROR;
+}
+
+return CC_SUCCESS;
+}
+
 static XHCIStreamContext *xhci_find_stream(XHCIEPContext *epctx,
unsigned int streamid,
uint32_t *cc_error)
@@ -2314,6 +2419,8 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, 
unsigned int slotid,
 return CC_CONTEXT_STATE_ERROR;
 }
 
+xhci_free_device_streams(xhci, slotid, ictl_ctx[0] | ictl_ctx[1]);
+
 for (i = 2; i <= 31; i++) {
 if (ictl_ctx[0] & (1<

Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events

2013-10-08 Thread Alex Bligh

On 8 Oct 2013, at 20:41, Hans de Goede wrote:

>>> 
>>> Wasn't it 1 ms until the offending commit (note 250 us does
>>> sound better to me).
>> 
>> I believe you've made it 1 nanosecond not 1 millisecond;
> 
> Correct, the 1 ms I referred to was before your commit which changed
> things from ms to ns.

OK I was looking at the patch as it would apply to master now.

> The purpose of the 1 ns timeout is to cause os_host_main_loop_wait
> to unlock the iothread, as $subject says the problem I'm seeing seems
> to be lock starvation not cpu starvation.
> 
> Note as I already indicated I'm in no way an expert in this, if you
> and or Paolo suspect cpu starvation may happen too, then bumping
> the timeout to 250 us is fine with me too.
> 
> If we go with 250 us that thus pose the question though if we should
> always keep a minimum timeout of 250 us when not non-blocking, or only
> bump it to 250 us when main_loop_tlg has already expired events and
> thus is causing a timeout of 0.

I am by no means an expert in the iothread bit, so let's pool our
ignorance ... :-) 

Somewhere within that patch series (7b595f35 I think) I fixed up
the spin counter bit, which made it slightly less yucky and work
with milliseconds. I hope I didn't break it but there seems
something slightly odd about the use case here.

If you are getting the spin error, this implies something is
pretty much constantly polling os_host_main_loop_wait with a
zero timeout. As you point out this is going to be main_loop_wait
and almost certainly main_loop_wait called with nonblocking
set to 1.

The comment at line 208 suggests that "the I/O thread is very busy
or we are incorrectly busy waiting in the I/O thread". Do we know
which is happening? Perhaps rather than give up the io_thread
mutex on every call (which is in practice what a 1 nanosecond
timeout does) we should give it up if we have not released
it for X nanoseconds (maybe X=250us), or on every Y calls. I think
someone other than me should consider the effect of dropping and
reacquiring a mutex so frequently under heavy I/O load, but I'm not
sure it's a great idea.

So on reflection you might be more right with 1 nanosecond than
250us as a timeout of 250us, but I wonder whether a strategy
of just dropping the lock occasionally (and still using a zero
timeout) might be better.

-- 
Alex Bligh







[Qemu-devel] [PATCH 06/13] usb-hcd-xhci: Report completion of active transfer with CC_STOPPED on ep stop

2013-10-08 Thread Hans de Goede
As we should per the XHCI spec "4.6.9 Stop Endpoint".

Signed-off-by: Hans de Goede 
---
 hw/usb/hcd-xhci.c | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 7cf89ce..0131151 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -505,6 +505,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int 
slotid,
  unsigned int epid, unsigned int streamid);
 static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid,
 unsigned int epid);
+static void xhci_xfer_report(XHCITransfer *xfer);
 static void xhci_event(XHCIState *xhci, XHCIEvent *event, int v);
 static void xhci_write_event(XHCIState *xhci, XHCIEvent *event, int v);
 static USBEndpoint *xhci_epid_to_usbep(XHCIState *xhci,
@@ -1302,10 +1303,15 @@ static TRBCCode xhci_enable_ep(XHCIState *xhci, 
unsigned int slotid,
 return CC_SUCCESS;
 }
 
-static int xhci_ep_nuke_one_xfer(XHCITransfer *t)
+static int xhci_ep_nuke_one_xfer(XHCITransfer *t, TRBCCode report)
 {
 int killed = 0;
 
+if (report && (t->running_async || t->running_retry)) {
+t->status = report;
+xhci_xfer_report(t);
+}
+
 if (t->running_async) {
 usb_cancel_packet(&t->packet);
 t->running_async = 0;
@@ -1318,6 +1324,7 @@ static int xhci_ep_nuke_one_xfer(XHCITransfer *t)
 timer_del(epctx->kick_timer);
 }
 t->running_retry = 0;
+killed = 1;
 }
 if (t->trbs) {
 g_free(t->trbs);
@@ -1330,7 +1337,7 @@ static int xhci_ep_nuke_one_xfer(XHCITransfer *t)
 }
 
 static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid,
-   unsigned int epid)
+   unsigned int epid, TRBCCode report)
 {
 XHCISlot *slot;
 XHCIEPContext *epctx;
@@ -1351,7 +1358,10 @@ static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned 
int slotid,
 
 xferi = epctx->next_xfer;
 for (i = 0; i < TD_QUEUE; i++) {
-killed += xhci_ep_nuke_one_xfer(&epctx->transfers[xferi]);
+killed += xhci_ep_nuke_one_xfer(&epctx->transfers[xferi], report);
+if (killed) {
+report = 0; /* Only report once */
+}
 epctx->transfers[xferi].packet.ep = NULL;
 xferi = (xferi + 1) % TD_QUEUE;
 }
@@ -1381,7 +1391,7 @@ static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned 
int slotid,
 return CC_SUCCESS;
 }
 
-xhci_ep_nuke_xfers(xhci, slotid, epid);
+xhci_ep_nuke_xfers(xhci, slotid, epid, 0);
 
 epctx = slot->eps[epid-1];
 
@@ -1423,7 +1433,7 @@ static TRBCCode xhci_stop_ep(XHCIState *xhci, unsigned 
int slotid,
 return CC_EP_NOT_ENABLED_ERROR;
 }
 
-if (xhci_ep_nuke_xfers(xhci, slotid, epid) > 0) {
+if (xhci_ep_nuke_xfers(xhci, slotid, epid, CC_STOPPED) > 0) {
 fprintf(stderr, "xhci: FIXME: endpoint stopped w/ xfers running, "
 "data might be lost\n");
 }
@@ -1468,7 +1478,7 @@ static TRBCCode xhci_reset_ep(XHCIState *xhci, unsigned 
int slotid,
 return CC_CONTEXT_STATE_ERROR;
 }
 
-if (xhci_ep_nuke_xfers(xhci, slotid, epid) > 0) {
+if (xhci_ep_nuke_xfers(xhci, slotid, epid, 0) > 0) {
 fprintf(stderr, "xhci: FIXME: endpoint reset w/ xfers running, "
 "data might be lost\n");
 }
@@ -2461,7 +2471,7 @@ static void xhci_detach_slot(XHCIState *xhci, USBPort 
*uport)
 
 for (ep = 0; ep < 31; ep++) {
 if (xhci->slots[slot].eps[ep]) {
-xhci_ep_nuke_xfers(xhci, slot+1, ep+1);
+xhci_ep_nuke_xfers(xhci, slot + 1, ep + 1, 0);
 }
 }
 xhci->slots[slot].uport = NULL;
@@ -3276,7 +3286,7 @@ static void xhci_complete(USBPort *port, USBPacket 
*packet)
 XHCITransfer *xfer = container_of(packet, XHCITransfer, packet);
 
 if (packet->status == USB_RET_REMOVE_FROM_QUEUE) {
-xhci_ep_nuke_one_xfer(xfer);
+xhci_ep_nuke_one_xfer(xfer, 0);
 return;
 }
 xhci_complete_packet(xfer);
-- 
1.8.3.1




[Qemu-devel] [PATCH 03/13] usb-host-libusb: Detach kernel drivers earlier

2013-10-08 Thread Hans de Goede
If we detach the kernel drivers on the first set_config, then they will
be still attached when the device gets its initial reset. Causing the drivers
to re-initialize the device after the reset, dirtying the device state.

Signed-off-by: Hans de Goede 
---
 hw/usb/host-libusb.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 35bae55..fd320cd 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -137,6 +137,7 @@ static QTAILQ_HEAD(, USBHostDevice) hostdevs =
 static void usb_host_auto_check(void *unused);
 static void usb_host_release_interfaces(USBHostDevice *s);
 static void usb_host_nodev(USBHostDevice *s);
+static void usb_host_detach_kernel(USBHostDevice *s);
 static void usb_host_attach_kernel(USBHostDevice *s);
 
 /*  */
@@ -787,10 +788,13 @@ static int usb_host_open(USBHostDevice *s, libusb_device 
*dev)
 goto fail;
 }
 
-libusb_get_device_descriptor(dev, &s->ddesc);
 s->dev = dev;
 s->bus_num = bus_num;
 s->addr= addr;
+
+usb_host_detach_kernel(s);
+
+libusb_get_device_descriptor(dev, &s->ddesc);
 usb_host_get_port(s->dev, s->port, sizeof(s->port));
 
 usb_ep_init(udev);
@@ -1051,7 +1055,6 @@ static void usb_host_set_config(USBHostDevice *s, int 
config, USBPacket *p)
 trace_usb_host_set_config(s->bus_num, s->addr, config);
 
 usb_host_release_interfaces(s);
-usb_host_detach_kernel(s);
 rc = libusb_set_configuration(s->dh, config);
 if (rc != 0) {
 usb_host_libusb_error("libusb_set_configuration", rc);
-- 
1.8.3.1




[Qemu-devel] [PATCH 04/13] usb-hcd-xhci: Remove unused sstreamsm member from XHCIStreamContext

2013-10-08 Thread Hans de Goede
Signed-off-by: Hans de Goede 
---
 hw/usb/hcd-xhci.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 469c24d..e078c50 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -374,7 +374,6 @@ struct XHCIStreamContext {
 dma_addr_t pctx;
 unsigned int sct;
 XHCIRing ring;
-XHCIStreamContext *sstreams;
 };
 
 struct XHCIEPContext {
@@ -1133,7 +1132,6 @@ static void xhci_reset_streams(XHCIEPContext *epctx)
 
 for (i = 0; i < epctx->nr_pstreams; i++) {
 epctx->pstreams[i].sct = -1;
-g_free(epctx->pstreams[i].sstreams);
 }
 }
 
@@ -1146,15 +1144,8 @@ static void xhci_alloc_streams(XHCIEPContext *epctx, 
dma_addr_t base)
 
 static void xhci_free_streams(XHCIEPContext *epctx)
 {
-int i;
-
 assert(epctx->pstreams != NULL);
 
-if (!epctx->lsa) {
-for (i = 0; i < epctx->nr_pstreams; i++) {
-g_free(epctx->pstreams[i].sstreams);
-}
-}
 g_free(epctx->pstreams);
 epctx->pstreams = NULL;
 epctx->nr_pstreams = 0;
-- 
1.8.3.1




[Qemu-devel] [PATCH 02/13] usb-host-libusb: Configuration 0 may be a valid configuration

2013-10-08 Thread Hans de Goede
Quoting from: linux/Documentation/ABI/stable/sysfs-bus-usb:

Note that some devices, in violation of the USB spec, have a
configuration with a value equal to 0. Writing 0 to
bConfigurationValue for these devices will install that
configuration, rather then unconfigure the device.

So don't compare the configuration value against 0 to check for unconfigured
devices, instead check for a LIBUSB_ERROR_NOT_FOUND return from
libusb_get_active_config_descriptor().

Signed-off-by: Hans de Goede 
---
 hw/usb/host-libusb.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 428c7c5..35bae55 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -992,15 +992,14 @@ static int usb_host_claim_interfaces(USBHostDevice *s, 
int configuration)
 udev->ninterfaces   = 0;
 udev->configuration = 0;
 
-if (configuration == 0) {
-/* address state - ignore */
-return USB_RET_SUCCESS;
-}
-
 usb_host_detach_kernel(s);
 
 rc = libusb_get_active_config_descriptor(s->dev, &conf);
 if (rc != 0) {
+if (rc == LIBUSB_ERROR_NOT_FOUND) {
+/* address state - ignore */
+return USB_RET_SUCCESS;
+}
 return USB_RET_STALL;
 }
 
-- 
1.8.3.1




[Qemu-devel] [PATCH 05/13] usb-hcd-xhci: Remove unused cancelled member from XHCITransfer

2013-10-08 Thread Hans de Goede
Since qemu's USB model is geared towards emulated devices cancellation
is instanteneous, so no need to wait for cancellation to complete, as
such there is no wait for cancellation code, and the cancelled bool
as well as the bogus comment about it can be removed.

Signed-off-by: Hans de Goede 
---
 hw/usb/hcd-xhci.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index e078c50..7cf89ce 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -346,7 +346,6 @@ typedef struct XHCITransfer {
 QEMUSGList sgl;
 bool running_async;
 bool running_retry;
-bool cancelled;
 bool complete;
 bool int_req;
 unsigned int iso_pkts;
@@ -1310,8 +1309,6 @@ static int xhci_ep_nuke_one_xfer(XHCITransfer *t)
 if (t->running_async) {
 usb_cancel_packet(&t->packet);
 t->running_async = 0;
-t->cancelled = 1;
-DPRINTF("xhci: cancelling transfer, waiting for it to complete\n");
 killed = 1;
 }
 if (t->running_retry) {
@@ -1728,14 +1725,12 @@ static int xhci_complete_packet(XHCITransfer *xfer)
 xfer->running_async = 1;
 xfer->running_retry = 0;
 xfer->complete = 0;
-xfer->cancelled = 0;
 return 0;
 } else if (xfer->packet.status == USB_RET_NAK) {
 trace_usb_xhci_xfer_nak(xfer);
 xfer->running_async = 0;
 xfer->running_retry = 1;
 xfer->complete = 0;
-xfer->cancelled = 0;
 return 0;
 } else {
 xfer->running_async = 0;
-- 
1.8.3.1




[Qemu-devel] [PULL 14/58] an5206: Don't enforce use of kernel for qtest

2013-10-08 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 hw/m68k/an5206.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
index a8eee44..24f2068 100644
--- a/hw/m68k/an5206.c
+++ b/hw/m68k/an5206.c
@@ -12,6 +12,7 @@
 #include "hw/loader.h"
 #include "elf.h"
 #include "exec/address-spaces.h"
+#include "sysemu/qtest.h"
 
 #define KERNEL_LOAD_ADDR 0x1
 #define AN5206_MBAR_ADDR 0x1000
@@ -62,6 +63,9 @@ static void an5206_init(QEMUMachineInitArgs *args)
 
 /* Load kernel.  */
 if (!kernel_filename) {
+if (qtest_enabled()) {
+return;
+}
 fprintf(stderr, "Kernel image must be specified\n");
 exit(1);
 }
-- 
1.8.1.4




[Qemu-devel] [PULL 09/58] omap_sx1: Don't enforce use of kernel or flash for qtest

2013-10-08 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 hw/arm/omap_sx1.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/omap_sx1.c b/hw/arm/omap_sx1.c
index 03b3816..3ba263a 100644
--- a/hw/arm/omap_sx1.c
+++ b/hw/arm/omap_sx1.c
@@ -32,6 +32,7 @@
 #include "hw/arm/arm.h"
 #include "hw/block/flash.h"
 #include "sysemu/blockdev.h"
+#include "sysemu/qtest.h"
 #include "exec/address-spaces.h"
 
 /*/
@@ -188,7 +189,7 @@ static void sx1_init(QEMUMachineInitArgs *args, const int 
version)
 OMAP_CS1_BASE, &cs[1]);
 }
 
-if (!args->kernel_filename && !fl_idx) {
+if (!args->kernel_filename && !fl_idx && !qtest_enabled()) {
 fprintf(stderr, "Kernel or Flash image must be specified\n");
 exit(1);
 }
-- 
1.8.1.4




[Qemu-devel] [PULL 52/58] qom: Clean up struct Error references

2013-10-08 Thread Andreas Färber
From: "Michael S. Tsirkin" 

Now that a typedef for struct Error is available,
use it in qom/object.h to match coding style rules.

Reviewed-by: Paolo Bonzini 
Reviewed-by: Gerd Hoffmann 
Tested-by: Gerd Hoffmann 
Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Igor Mammedov 
Tested-by: Igor Mammedov 
Signed-off-by: Andreas Färber 
---
 include/qom/object.h | 50 +-
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index d9a0063..6c1e7d3 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -301,7 +301,7 @@ typedef void (ObjectPropertyAccessor)(Object *obj,
   struct Visitor *v,
   void *opaque,
   const char *name,
-  struct Error **errp);
+  Error **errp);
 
 /**
  * ObjectPropertyRelease:
@@ -790,9 +790,9 @@ void object_property_add(Object *obj, const char *name, 
const char *type,
  ObjectPropertyAccessor *get,
  ObjectPropertyAccessor *set,
  ObjectPropertyRelease *release,
- void *opaque, struct Error **errp);
+ void *opaque, Error **errp);
 
-void object_property_del(Object *obj, const char *name, struct Error **errp);
+void object_property_del(Object *obj, const char *name, Error **errp);
 
 /**
  * object_property_find:
@@ -803,7 +803,7 @@ void object_property_del(Object *obj, const char *name, 
struct Error **errp);
  * Look up a property for an object and return its #ObjectProperty if found.
  */
 ObjectProperty *object_property_find(Object *obj, const char *name,
- struct Error **errp);
+ Error **errp);
 
 void object_unparent(Object *obj);
 
@@ -818,7 +818,7 @@ void object_unparent(Object *obj);
  * Reads a property from a object.
  */
 void object_property_get(Object *obj, struct Visitor *v, const char *name,
- struct Error **errp);
+ Error **errp);
 
 /**
  * object_property_set_str:
@@ -829,7 +829,7 @@ void object_property_get(Object *obj, struct Visitor *v, 
const char *name,
  * Writes a string value to a property.
  */
 void object_property_set_str(Object *obj, const char *value,
- const char *name, struct Error **errp);
+ const char *name, Error **errp);
 
 /**
  * object_property_get_str:
@@ -842,7 +842,7 @@ void object_property_set_str(Object *obj, const char *value,
  * The caller should free the string.
  */
 char *object_property_get_str(Object *obj, const char *name,
-  struct Error **errp);
+  Error **errp);
 
 /**
  * object_property_set_link:
@@ -853,7 +853,7 @@ char *object_property_get_str(Object *obj, const char *name,
  * Writes an object's canonical path to a property.
  */
 void object_property_set_link(Object *obj, Object *value,
-  const char *name, struct Error **errp);
+  const char *name, Error **errp);
 
 /**
  * object_property_get_link:
@@ -866,7 +866,7 @@ void object_property_set_link(Object *obj, Object *value,
  * string or not a valid object path).
  */
 Object *object_property_get_link(Object *obj, const char *name,
- struct Error **errp);
+ Error **errp);
 
 /**
  * object_property_set_bool:
@@ -877,7 +877,7 @@ Object *object_property_get_link(Object *obj, const char 
*name,
  * Writes a bool value to a property.
  */
 void object_property_set_bool(Object *obj, bool value,
-  const char *name, struct Error **errp);
+  const char *name, Error **errp);
 
 /**
  * object_property_get_bool:
@@ -889,7 +889,7 @@ void object_property_set_bool(Object *obj, bool value,
  * an error occurs (including when the property value is not a bool).
  */
 bool object_property_get_bool(Object *obj, const char *name,
-  struct Error **errp);
+  Error **errp);
 
 /**
  * object_property_set_int:
@@ -900,7 +900,7 @@ bool object_property_get_bool(Object *obj, const char *name,
  * Writes an integer value to a property.
  */
 void object_property_set_int(Object *obj, int64_t value,
- const char *name, struct Error **errp);
+ const char *name, Error **errp);
 
 /**
  * object_property_get_int:
@@ -912,7 +912,7 @@ void object_property_set_int(Object *obj, int64_t value,
  * an error occurs (including when the property value is not an integer).
  */
 int64_t object_property_get_int(Object *obj, const char *name,
-struct Error **errp);
+ 

[Qemu-devel] [PULL 15/58] milkymist: Suppress -kernel/-bios/-drive error for qtest

2013-10-08 Thread Andreas Färber
Acked-by: Michael Walle 
Signed-off-by: Andreas Färber 
---
 hw/lm32/milkymist.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
index f1744ec..15053c4 100644
--- a/hw/lm32/milkymist.c
+++ b/hw/lm32/milkymist.c
@@ -21,6 +21,7 @@
 #include "hw/hw.h"
 #include "hw/block/flash.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
 #include "hw/devices.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
@@ -143,7 +144,7 @@ milkymist_init(QEMUMachineInitArgs *args)
 reset_info->bootstrap_pc = BIOS_OFFSET;
 
 /* if no kernel is given no valid bios rom is a fatal error */
-if (!kernel_filename && !dinfo && !bios_filename) {
+if (!kernel_filename && !dinfo && !bios_filename && !qtest_enabled()) {
 fprintf(stderr, "qemu: could not load Milkymist One bios '%s'\n",
 bios_name);
 exit(1);
-- 
1.8.1.4




[Qemu-devel] [PULL 16/58] shix: Drop debug output

2013-10-08 Thread Andreas Färber
Reviewed-by: Aurelien Jarno 
Signed-off-by: Andreas Färber 
---
 hw/sh4/shix.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/hw/sh4/shix.c b/hw/sh4/shix.c
index 1ff37f5..f008b98 100644
--- a/hw/sh4/shix.c
+++ b/hw/sh4/shix.c
@@ -50,7 +50,6 @@ static void shix_init(QEMUMachineInitArgs *args)
 if (!cpu_model)
 cpu_model = "any";
 
-printf("Initializing CPU\n");
 cpu = cpu_sh4_init(cpu_model);
 if (cpu == NULL) {
 fprintf(stderr, "Unable to find CPU definition\n");
@@ -58,16 +57,13 @@ static void shix_init(QEMUMachineInitArgs *args)
 }
 
 /* Allocate memory space */
-printf("Allocating ROM\n");
 memory_region_init_ram(rom, NULL, "shix.rom", 0x4000);
 vmstate_register_ram_global(rom);
 memory_region_set_readonly(rom, true);
 memory_region_add_subregion(sysmem, 0x, rom);
-printf("Allocating SDRAM 1\n");
 memory_region_init_ram(&sdram[0], NULL, "shix.sdram1", 0x0100);
 vmstate_register_ram_global(&sdram[0]);
 memory_region_add_subregion(sysmem, 0x0800, &sdram[0]);
-printf("Allocating SDRAM 2\n");
 memory_region_init_ram(&sdram[1], NULL, "shix.sdram2", 0x0100);
 vmstate_register_ram_global(&sdram[1]);
 memory_region_add_subregion(sysmem, 0x0c00, &sdram[1]);
@@ -75,10 +71,8 @@ static void shix_init(QEMUMachineInitArgs *args)
 /* Load BIOS in 0 (and access it through P2, 0xA000) */
 if (bios_name == NULL)
 bios_name = BIOS_FILENAME;
-printf("%s: load BIOS '%s'\n", __func__, bios_name);
 ret = load_image_targphys(bios_name, 0, 0x4000);
 if (ret < 0) { /* Check bios size */
-   fprintf(stderr, "ret=%d\n", ret);
fprintf(stderr, "qemu: could not load SHIX bios '%s'\n",
bios_name);
exit(1);
@@ -88,7 +82,6 @@ static void shix_init(QEMUMachineInitArgs *args)
 s = sh7750_init(cpu, sysmem);
 /* X Check success */
 tc58128_init(s, "shix_linux_nand.bin", NULL);
-fprintf(stderr, "initialization terminated\n");
 }
 
 static QEMUMachine shix_machine = {
-- 
1.8.1.4




[Qemu-devel] [PULL 39/58] arm11mpcore: Convert ARM11MPCorePriveState to QOM realize

2013-10-08 Thread Andreas Färber
Embed child devices and replace SysBus initfn with realizefn.

Reviewed-by: Peter Maydell 
Signed-off-by: Andreas Färber 
---
 hw/cpu/arm11mpcore.c | 84 ++--
 1 file changed, 56 insertions(+), 28 deletions(-)

diff --git a/hw/cpu/arm11mpcore.c b/hw/cpu/arm11mpcore.c
index 5dcc73a..f372283 100644
--- a/hw/cpu/arm11mpcore.c
+++ b/hw/cpu/arm11mpcore.c
@@ -9,6 +9,8 @@
 
 #include "hw/sysbus.h"
 #include "hw/misc/arm11scu.h"
+#include "hw/intc/arm_gic.h"
+#include "hw/timer/arm_mptimer.h"
 #include "qemu/timer.h"
 
 /* MPCore private memory region.  */
@@ -22,12 +24,12 @@ typedef struct ARM11MPCorePriveState {
 
 uint32_t num_cpu;
 MemoryRegion container;
-DeviceState *mptimer;
-DeviceState *wdtimer;
-DeviceState *gic;
 uint32_t num_irq;
 
 ARM11SCUState scu;
+GICState gic;
+ARMMPTimerState mptimer;
+ARMMPTimerState wdtimer;
 } ARM11MPCorePriveState;
 
 /* Per-CPU private memory mapped IO.  */
@@ -36,16 +38,18 @@ typedef struct ARM11MPCorePriveState {
 static void mpcore_priv_set_irq(void *opaque, int irq, int level)
 {
 ARM11MPCorePriveState *s = (ARM11MPCorePriveState *)opaque;
-qemu_set_irq(qdev_get_gpio_in(s->gic, irq), level);
+
+qemu_set_irq(qdev_get_gpio_in(DEVICE(&s->gic), irq), level);
 }
 
 static void mpcore_priv_map_setup(ARM11MPCorePriveState *s)
 {
 int i;
 SysBusDevice *scubusdev = SYS_BUS_DEVICE(&s->scu);
-SysBusDevice *gicbusdev = SYS_BUS_DEVICE(s->gic);
-SysBusDevice *timerbusdev = SYS_BUS_DEVICE(s->mptimer);
-SysBusDevice *wdtbusdev = SYS_BUS_DEVICE(s->wdtimer);
+DeviceState *gicdev = DEVICE(&s->gic);
+SysBusDevice *gicbusdev = SYS_BUS_DEVICE(&s->gic);
+SysBusDevice *timerbusdev = SYS_BUS_DEVICE(&s->mptimer);
+SysBusDevice *wdtbusdev = SYS_BUS_DEVICE(&s->wdtimer);
 
 memory_region_add_subregion(&s->container, 0,
 sysbus_mmio_get_region(scubusdev, 0));
@@ -76,44 +80,58 @@ static void mpcore_priv_map_setup(ARM11MPCorePriveState *s)
 for (i = 0; i < s->num_cpu; i++) {
 int ppibase = (s->num_irq - 32) + i * 32;
 sysbus_connect_irq(timerbusdev, i,
-   qdev_get_gpio_in(s->gic, ppibase + 29));
+   qdev_get_gpio_in(gicdev, ppibase + 29));
 sysbus_connect_irq(wdtbusdev, i,
-   qdev_get_gpio_in(s->gic, ppibase + 30));
+   qdev_get_gpio_in(gicdev, ppibase + 30));
 }
 }
 
-static int mpcore_priv_init(SysBusDevice *sbd)
+static void mpcore_priv_realize(DeviceState *dev, Error **errp)
 {
-DeviceState *dev = DEVICE(sbd);
+SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 ARM11MPCorePriveState *s = ARM11MPCORE_PRIV(dev);
 DeviceState *scudev = DEVICE(&s->scu);
+DeviceState *gicdev = DEVICE(&s->gic);
+DeviceState *mptimerdev = DEVICE(&s->mptimer);
+DeviceState *wdtimerdev = DEVICE(&s->wdtimer);
+Error *err = NULL;
 
 qdev_prop_set_uint32(scudev, "num-cpu", s->num_cpu);
-qdev_init_nofail(scudev);
+object_property_set_bool(OBJECT(&s->scu), true, "realized", &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
+}
 
-s->gic = qdev_create(NULL, "arm_gic");
-qdev_prop_set_uint32(s->gic, "num-cpu", s->num_cpu);
-qdev_prop_set_uint32(s->gic, "num-irq", s->num_irq);
-/* Request the legacy 11MPCore GIC behaviour: */
-qdev_prop_set_uint32(s->gic, "revision", 0);
-qdev_init_nofail(s->gic);
+qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
+qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
+object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
+}
 
 /* Pass through outbound IRQ lines from the GIC */
-sysbus_pass_irq(sbd, SYS_BUS_DEVICE(s->gic));
+sysbus_pass_irq(sbd, SYS_BUS_DEVICE(&s->gic));
 
 /* Pass through inbound GPIO lines to the GIC */
 qdev_init_gpio_in(dev, mpcore_priv_set_irq, s->num_irq - 32);
 
-s->mptimer = qdev_create(NULL, "arm_mptimer");
-qdev_prop_set_uint32(s->mptimer, "num-cpu", s->num_cpu);
-qdev_init_nofail(s->mptimer);
+qdev_prop_set_uint32(mptimerdev, "num-cpu", s->num_cpu);
+object_property_set_bool(OBJECT(&s->mptimer), true, "realized", &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
+}
 
-s->wdtimer = qdev_create(NULL, "arm_mptimer");
-qdev_prop_set_uint32(s->wdtimer, "num-cpu", s->num_cpu);
-qdev_init_nofail(s->wdtimer);
+qdev_prop_set_uint32(wdtimerdev, "num-cpu", s->num_cpu);
+object_property_set_bool(OBJECT(&s->wdtimer), true, "realized", &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
+}
 
 mpcore_priv_map_setup(s);
-return 0;
 }
 
 static void mpcore_priv_initfn(Object *obj)
@@ -127,6 +145,17 @@ static void mpcore_priv_initfn(Object *obj)
 
 

Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events

2013-10-08 Thread Alex Bligh

On 8 Oct 2013, at 20:10, Hans de Goede wrote:

> I noticed today that current qemu master would hang as soon as Xorg starts in
> the guest when using qxl + a Linux guest. This message would be printed:
> main-loop: WARNING: I/O thread spun for 1000 iterations
> 
> And from then on the guest hangs and qemu consumes 100% cpu, bisecting pointed
> out commit 7b595f35d89d73bc69c35bf3980a89c420e8a44b:
> "aio / timers: Convert mainloop to use timeout"
> 
> After looking at that commit I had a hunch the problem might be blocking
> main_loop_wait calls being turned into non-blocking ones (and thus never
> releasing the io-lock), a debug printf confirmed this was happening at
> the moment of the hang, so I wrote this patch which fixes the hang for me
> and seems like a good idea in general.
> 
> Signed-off-by: Hans de Goede 
> ---
> main-loop.c | 5 +
> 1 file changed, 5 insertions(+)
> 
> diff --git a/main-loop.c b/main-loop.c
> index c3c9c28..921c939 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -480,6 +480,11 @@ int main_loop_wait(int nonblocking)
>   timerlistgroup_deadline_ns(
>   &main_loop_tlg));
> 
> +/* When not non-blocking always allow io-threads to acquire the lock */
> +if (timeout != 0 && timeout_ns == 0) {
> +timeout_ns = 1;
> +}
> +
> ret = os_host_main_loop_wait(timeout_ns);
> qemu_iohandler_poll(gpollfds, ret);
> #ifdef CONFIG_SLIRP

I /think/ you might mean "if (!blocking && timeout_ns == 0)"
as timeout can be zero on a blocking call at this stage (i.e.
when there is a timer which has already expired.

I'm not entirely sure I understand the problem from your
description - I'll answer this in your subseqent message.

-- 
Alex Bligh







[Qemu-devel] [PULL 57/58] ide: Drop ide_init2_with_non_qdev_drives()

2013-10-08 Thread Andreas Färber
All its users have finally been converted.

Signed-off-by: Andreas Färber 
---
 hw/ide/core.c | 49 -
 hw/ide/internal.h |  2 --
 2 files changed, 51 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 399b1ba..e1f4c33 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2215,55 +2215,6 @@ void ide_init2(IDEBus *bus, qemu_irq irq)
 bus->dma = &ide_dma_nop;
 }
 
-/* TODO convert users to qdev and remove */
-void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
-DriveInfo *hd1, qemu_irq irq)
-{
-int i, trans;
-DriveInfo *dinfo;
-uint32_t cyls, heads, secs;
-
-for(i = 0; i < 2; i++) {
-dinfo = i == 0 ? hd0 : hd1;
-ide_init1(bus, i);
-if (dinfo) {
-cyls  = dinfo->cyls;
-heads = dinfo->heads;
-secs  = dinfo->secs;
-trans = dinfo->trans;
-if (!cyls && !heads && !secs) {
-hd_geometry_guess(dinfo->bdrv, &cyls, &heads, &secs, &trans);
-} else if (trans == BIOS_ATA_TRANSLATION_AUTO) {
-trans = hd_bios_chs_auto_trans(cyls, heads, secs);
-}
-if (cyls < 1 || cyls > 65535) {
-error_report("cyls must be between 1 and 65535");
-exit(1);
-}
-if (heads < 1 || heads > 16) {
-error_report("heads must be between 1 and 16");
-exit(1);
-}
-if (secs < 1 || secs > 255) {
-error_report("secs must be between 1 and 255");
-exit(1);
-}
-if (ide_init_drive(&bus->ifs[i], dinfo->bdrv,
-   dinfo->media_cd ? IDE_CD : IDE_HD,
-   NULL, dinfo->serial, NULL, 0,
-   cyls, heads, secs, trans) < 0) {
-error_report("Can't set up IDE drive %s", dinfo->id);
-exit(1);
-}
-bdrv_attach_dev_nofail(dinfo->bdrv, &bus->ifs[i]);
-} else {
-ide_reset(&bus->ifs[i]);
-}
-}
-bus->irq = irq;
-bus->dma = &ide_dma_nop;
-}
-
 static const MemoryRegionPortio ide_portio_list[] = {
 { 0, 8, 1, .read = ide_ioport_read, .write = ide_ioport_write },
 { 0, 2, 2, .read = ide_data_readw, .write = ide_data_writew },
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 5d1cf87..0567a52 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -553,8 +553,6 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, 
IDEDriveKind kind,
uint32_t cylinders, uint32_t heads, uint32_t secs,
int chs_trans);
 void ide_init2(IDEBus *bus, qemu_irq irq);
-void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
-DriveInfo *hd1, qemu_irq irq);
 void ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
 
 void ide_exec_cmd(IDEBus *bus, uint32_t val);
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events

2013-10-08 Thread Hans de Goede

Hi,

On 10/08/2013 09:33 PM, Alex Bligh wrote:


On 8 Oct 2013, at 20:21, Hans de Goede wrote:


Wasn't it 1 ms until the offending commit (note 250 us does
sound better to me).


I believe you've made it 1 nanosecond not 1 millisecond;


Correct, the 1 ms I referred to was before your commit which changed
things from ms to ns.


can that be right?


The purpose of the 1 ns timeout is to cause os_host_main_loop_wait
to unlock the iothread, as $subject says the problem I'm seeing seems
to be lock starvation not cpu starvation.

Note as I already indicated I'm in no way an expert in this, if you
and or Paolo suspect cpu starvation may happen too, then bumping
the timeout to 250 us is fine with me too.

If we go with 250 us that thus pose the question though if we should
always keep a minimum timeout of 250 us when not non-blocking, or only
bump it to 250 us when main_loop_tlg has already expired events and
thus is causing a timeout of 0.

Regards,

Hans



[Qemu-devel] [PULL 31/58] a15mpcore: Embed GICState

2013-10-08 Thread Andreas Färber
From: Andreas Färber 

This covers both emulated and KVM GIC.

Prepares for QOM realize.

Reviewed-by: Peter Maydell 
Signed-off-by: Andreas Färber 
---
 hw/cpu/a15mpcore.c | 39 ---
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/hw/cpu/a15mpcore.c b/hw/cpu/a15mpcore.c
index af29c35..b2614e7 100644
--- a/hw/cpu/a15mpcore.c
+++ b/hw/cpu/a15mpcore.c
@@ -20,6 +20,7 @@
 
 #include "hw/sysbus.h"
 #include "sysemu/kvm.h"
+#include "hw/intc/arm_gic.h"
 
 /* A15MP private memory region.  */
 
@@ -35,41 +36,49 @@ typedef struct A15MPPrivState {
 uint32_t num_cpu;
 uint32_t num_irq;
 MemoryRegion container;
-DeviceState *gic;
+
+GICState gic;
 } A15MPPrivState;
 
 static void a15mp_priv_set_irq(void *opaque, int irq, int level)
 {
 A15MPPrivState *s = (A15MPPrivState *)opaque;
-qemu_set_irq(qdev_get_gpio_in(s->gic, irq), level);
+
+qemu_set_irq(qdev_get_gpio_in(DEVICE(&s->gic), irq), level);
 }
 
 static void a15mp_priv_initfn(Object *obj)
 {
 SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 A15MPPrivState *s = A15MPCORE_PRIV(obj);
+DeviceState *gicdev;
+const char *gictype = "arm_gic";
+
+if (kvm_irqchip_in_kernel()) {
+gictype = "kvm-arm-gic";
+}
 
 memory_region_init(&s->container, obj, "a15mp-priv-container", 0x8000);
 sysbus_init_mmio(sbd, &s->container);
+
+object_initialize(&s->gic, sizeof(s->gic), gictype);
+gicdev = DEVICE(&s->gic);
+qdev_set_parent_bus(gicdev, sysbus_get_default());
+qdev_prop_set_uint32(gicdev, "revision", 2);
 }
 
 static int a15mp_priv_init(SysBusDevice *dev)
 {
 A15MPPrivState *s = A15MPCORE_PRIV(dev);
+DeviceState *gicdev;
 SysBusDevice *busdev;
-const char *gictype = "arm_gic";
 int i;
 
-if (kvm_irqchip_in_kernel()) {
-gictype = "kvm-arm-gic";
-}
-
-s->gic = qdev_create(NULL, gictype);
-qdev_prop_set_uint32(s->gic, "num-cpu", s->num_cpu);
-qdev_prop_set_uint32(s->gic, "num-irq", s->num_irq);
-qdev_prop_set_uint32(s->gic, "revision", 2);
-qdev_init_nofail(s->gic);
-busdev = SYS_BUS_DEVICE(s->gic);
+gicdev = DEVICE(&s->gic);
+qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
+qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
+qdev_init_nofail(gicdev);
+busdev = SYS_BUS_DEVICE(&s->gic);
 
 /* Pass through outbound IRQ lines from the GIC */
 sysbus_pass_irq(dev, busdev);
@@ -87,10 +96,10 @@ static int a15mp_priv_init(SysBusDevice *dev)
  * since a real A15 always has TrustZone but QEMU doesn't.
  */
 qdev_connect_gpio_out(cpudev, 0,
-  qdev_get_gpio_in(s->gic, ppibase + 30));
+  qdev_get_gpio_in(gicdev, ppibase + 30));
 /* virtual timer */
 qdev_connect_gpio_out(cpudev, 1,
-  qdev_get_gpio_in(s->gic, ppibase + 27));
+  qdev_get_gpio_in(gicdev, ppibase + 27));
 }
 
 /* Memory map (addresses are offsets from PERIPHBASE):
-- 
1.8.1.4




[Qemu-devel] [PULL 02/58] hw/arm: Tidy up conditional calls to arm_load_kernel()

2013-10-08 Thread Andreas Färber
From: Peter Maydell 

Now that arm_load_kernel() doesn't insist on a kernel filename
being present, we can remove some unnecessary conditionals
in board models.

Signed-off-by: Peter Maydell 
Signed-off-by: Andreas Färber 
---
 hw/arm/omap_sx1.c | 10 --
 hw/arm/palm.c | 10 --
 hw/arm/z2.c   | 12 +---
 3 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/hw/arm/omap_sx1.c b/hw/arm/omap_sx1.c
index b0f8664..03b3816 100644
--- a/hw/arm/omap_sx1.c
+++ b/hw/arm/omap_sx1.c
@@ -194,12 +194,10 @@ static void sx1_init(QEMUMachineInitArgs *args, const int 
version)
 }
 
 /* Load the kernel.  */
-if (args->kernel_filename) {
-sx1_binfo.kernel_filename = args->kernel_filename;
-sx1_binfo.kernel_cmdline = args->kernel_cmdline;
-sx1_binfo.initrd_filename = args->initrd_filename;
-arm_load_kernel(mpu->cpu, &sx1_binfo);
-}
+sx1_binfo.kernel_filename = args->kernel_filename;
+sx1_binfo.kernel_cmdline = args->kernel_cmdline;
+sx1_binfo.initrd_filename = args->initrd_filename;
+arm_load_kernel(mpu->cpu, &sx1_binfo);
 
 /* TODO: fix next line */
 //~ qemu_console_resize(ds, 640, 480);
diff --git a/hw/arm/palm.c b/hw/arm/palm.c
index 3e39044..0b72bbe 100644
--- a/hw/arm/palm.c
+++ b/hw/arm/palm.c
@@ -261,12 +261,10 @@ static void palmte_init(QEMUMachineInitArgs *args)
 }
 
 /* Load the kernel.  */
-if (kernel_filename) {
-palmte_binfo.kernel_filename = kernel_filename;
-palmte_binfo.kernel_cmdline = kernel_cmdline;
-palmte_binfo.initrd_filename = initrd_filename;
-arm_load_kernel(mpu->cpu, &palmte_binfo);
-}
+palmte_binfo.kernel_filename = kernel_filename;
+palmte_binfo.kernel_cmdline = kernel_cmdline;
+palmte_binfo.initrd_filename = initrd_filename;
+arm_load_kernel(mpu->cpu, &palmte_binfo);
 }
 
 static QEMUMachine palmte_machine = {
diff --git a/hw/arm/z2.c b/hw/arm/z2.c
index 2e0d5d4..a00fcc0 100644
--- a/hw/arm/z2.c
+++ b/hw/arm/z2.c
@@ -360,13 +360,11 @@ static void z2_init(QEMUMachineInitArgs *args)
 qdev_connect_gpio_out(mpu->gpio, Z2_GPIO_LCD_CS,
 qemu_allocate_irqs(z2_lcd_cs, z2_lcd, 1)[0]);
 
-if (kernel_filename) {
-z2_binfo.kernel_filename = kernel_filename;
-z2_binfo.kernel_cmdline = kernel_cmdline;
-z2_binfo.initrd_filename = initrd_filename;
-z2_binfo.board_id = 0x6dd;
-arm_load_kernel(mpu->cpu, &z2_binfo);
-}
+z2_binfo.kernel_filename = kernel_filename;
+z2_binfo.kernel_cmdline = kernel_cmdline;
+z2_binfo.initrd_filename = initrd_filename;
+z2_binfo.board_id = 0x6dd;
+arm_load_kernel(mpu->cpu, &z2_binfo);
 }
 
 static QEMUMachine z2_machine = {
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH 4/7] usb-hcd-xhci: Remove unused sstreamsm member from XHCIStreamContext

2013-10-08 Thread Hans de Goede

Hi,

On 09/24/2013 11:37 AM, Gerd Hoffmann wrote:

On Mo, 2013-09-23 at 20:54 +0200, Hans de Goede wrote:

Signed-off-by: Hans de Goede 


Patch doesn't apply.


Sorry, my bad, I had some other changes in my local tree
which I was not yet ready to send and this depended on them.

I'm ready to send the whole bunch of patches in one go now,
which I'll do directly after this mail.


That are bits for the (not fully implemented yet) secondary stream
arrays btw.


I know, but ...

> We might complete the implementation instead of kicking

them out.


Looking at the spec, I don't think any guest drivers will implement
secondary streams, the lsa can handle any reasonable amount of streams
just fine. The whole secondary stream thing is only interesting
if you want to do insane amount streams, or have stream id ranges
with holes in them.


I have no idea whenever there is a reasonable way to test
that though ...


I agree, and I'm not sure there ever will be. So I vote for not worrying
about secondary streams until we actually encounter a guest which uses
them (at which point we should have a way to test through that guest).

So my vote goes to just removing this cruft for now.

Regards,

Hans



[Qemu-devel] [PATCH v2] Fix pc migration from qemu <= 1.5

2013-10-08 Thread Cole Robinson
The following commit introduced a migration incompatibility:

commit 568f0690fd9aa4d39d84b04c1a5dbb53a915c3fe
Author: David Gibson 
Date:   Thu Jun 6 18:48:49 2013 +1000

pci: Replace pci_find_domain() with more general pci_root_bus_path()

The issue is that i440fx savevm idstr went from :00:00.0/I440FX to
:00.0/I440FX. Unfortunately we are stuck with the breakage for
1.6 machine types.

Add a compat property to maintain the busted idstr for the 1.6 machine
types, but revert to the old style format for 1.7+, and <= 1.5.

Tested with migration from qemu 1.5, qemu 1.6, and qemu.git.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Cole Robinson 
---

v2:
Drop needless 1.7 compat bits

 hw/pci-host/piix.c|  9 -
 hw/pci-host/q35.c | 10 --
 include/hw/i386/pc.h  | 16 
 include/hw/pci-host/q35.h |  1 +
 4 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index c041149..9dafe80 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -48,6 +48,7 @@ typedef struct I440FXState {
 PCIHostState parent_obj;
 PcPciInfo pci_info;
 uint64_t pci_hole64_size;
+uint32_t short_root_bus;
 } I440FXState;
 
 #define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
@@ -712,13 +713,19 @@ static const TypeInfo i440fx_info = {
 static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
 PCIBus *rootbus)
 {
+I440FXState *s = I440FX_PCI_HOST_BRIDGE(host_bridge);
+
 /* For backwards compat with old device paths */
-return "";
+if (s->short_root_bus) {
+return "";
+}
+return ":00";
 }
 
 static Property i440fx_props[] = {
 DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, I440FXState,
  pci_hole64_size, DEFAULT_PCI_HOLE64_SIZE),
+DEFINE_PROP_UINT32("short_root_bus", I440FXState, short_root_bus, 0),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index ad703a4..cb3abfd 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -61,8 +61,13 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
 static const char *q35_host_root_bus_path(PCIHostState *host_bridge,
   PCIBus *rootbus)
 {
-/* For backwards compat with old device paths */
-return "";
+Q35PCIHost *s = Q35_HOST_DEVICE(host_bridge);
+
+ /* For backwards compat with old device paths */
+if (s->mch.short_root_bus) {
+return "";
+}
+return ":00";
 }
 
 static void q35_host_get_pci_hole_start(Object *obj, Visitor *v,
@@ -114,6 +119,7 @@ static Property mch_props[] = {
 MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT),
 DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, Q35PCIHost,
  mch.pci_hole64_size, DEFAULT_PCI_HOLE64_SIZE),
+DEFINE_PROP_UINT32("short_root_bus", Q35PCIHost, mch.short_root_bus, 0),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 9b2ddc4..79f6934 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -230,6 +230,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 .driver   = "e1000",\
 .property = "mitigation",\
 .value= "off",\
+},{\
+.driver   = "i440FX-pcihost",\
+.property = "short_root_bus",\
+.value= stringify(1),\
+},{\
+.driver   = "mch",\
+.property = "short_root_bus",\
+.value= stringify(1),\
 }
 
 #define PC_COMPAT_1_5 \
@@ -266,6 +274,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 .driver = TYPE_X86_CPU,\
 .property = "pmu",\
 .value = "on",\
+},{\
+.driver   = "i440FX-pcihost",\
+.property = "short_root_bus",\
+.value= stringify(0),\
+},{\
+.driver   = "mch",\
+.property = "short_root_bus",\
+.value= stringify(0),\
 }
 
 #define PC_COMPAT_1_4 \
diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index 56de92e..c8362b9 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -61,6 +61,7 @@ typedef struct MCHPCIState {
 ram_addr_t above_4g_mem_size;
 uint64_t pci_hole64_size;
 PcGuestInfo *guest_info;
+uint32_t short_root_bus;
 } MCHPCIState;
 
 typedef struct Q35PCIHost {
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events

2013-10-08 Thread Alex Bligh

On 8 Oct 2013, at 20:21, Hans de Goede wrote:

> Wasn't it 1 ms until the offending commit (note 250 us does
> sound better to me).

I believe you've made it 1 nanosecond not 1 millisecond;
can that be right?

-- 
Alex Bligh







[Qemu-devel] [PULL 07/58] z2: Don't enforce use of -pflash for qtest

2013-10-08 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 hw/arm/z2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/arm/z2.c b/hw/arm/z2.c
index a00fcc0..d52c501 100644
--- a/hw/arm/z2.c
+++ b/hw/arm/z2.c
@@ -24,6 +24,7 @@
 #include "ui/console.h"
 #include "audio/audio.h"
 #include "exec/address-spaces.h"
+#include "sysemu/qtest.h"
 
 #ifdef DEBUG_Z2
 #define DPRINTF(fmt, ...) \
@@ -323,7 +324,7 @@ static void z2_init(QEMUMachineInitArgs *args)
 be = 0;
 #endif
 dinfo = drive_get(IF_PFLASH, 0, 0);
-if (!dinfo) {
+if (!dinfo && !qtest_enabled()) {
 fprintf(stderr, "Flash image must be given with the "
 "'pflash' parameter\n");
 exit(1);
@@ -331,7 +332,7 @@ static void z2_init(QEMUMachineInitArgs *args)
 
 if (!pflash_cfi01_register(Z2_FLASH_BASE,
NULL, "z2.flash0", Z2_FLASH_SIZE,
-   dinfo->bdrv, sector_len,
+   dinfo ? dinfo->bdrv : NULL, sector_len,
Z2_FLASH_SIZE / sector_len, 4, 0, 0, 0, 0,
be)) {
 fprintf(stderr, "qemu: Error registering flash memory.\n");
-- 
1.8.1.4




[Qemu-devel] [PULL 41/58] realview_gic: Prepare for QOM embedding

2013-10-08 Thread Andreas Färber
Move state struct, type constant and cast macro to a new header.

Reviewed-by: Peter Maydell 
Signed-off-by: Andreas Färber 
---
 hw/intc/realview_gic.c | 15 +--
 include/hw/intc/realview_gic.h | 28 
 2 files changed, 29 insertions(+), 14 deletions(-)
 create mode 100644 include/hw/intc/realview_gic.h

diff --git a/hw/intc/realview_gic.c b/hw/intc/realview_gic.c
index 4ff48bb..6c81296 100644
--- a/hw/intc/realview_gic.c
+++ b/hw/intc/realview_gic.c
@@ -7,20 +7,7 @@
  * This code is licensed under the GPL.
  */
 
-#include "hw/sysbus.h"
-#include "hw/intc/arm_gic.h"
-
-#define TYPE_REALVIEW_GIC "realview_gic"
-#define REALVIEW_GIC(obj) \
-OBJECT_CHECK(RealViewGICState, (obj), TYPE_REALVIEW_GIC)
-
-typedef struct RealViewGICState {
-SysBusDevice parent_obj;
-
-MemoryRegion container;
-
-GICState gic;
-} RealViewGICState;
+#include "hw/intc/realview_gic.h"
 
 static void realview_gic_set_irq(void *opaque, int irq, int level)
 {
diff --git a/include/hw/intc/realview_gic.h b/include/hw/intc/realview_gic.h
new file mode 100644
index 000..1783ea1
--- /dev/null
+++ b/include/hw/intc/realview_gic.h
@@ -0,0 +1,28 @@
+/*
+ * ARM RealView Emulation Baseboard Interrupt Controller
+ *
+ * Copyright (c) 2006-2007 CodeSourcery.
+ * Written by Paul Brook
+ *
+ * This code is licensed under the GPL.
+ */
+
+#ifndef HW_INTC_REALVIEW_GIC_H
+#define HW_INTC_REALVIEW_GIC_H
+
+#include "hw/sysbus.h"
+#include "hw/intc/arm_gic.h"
+
+#define TYPE_REALVIEW_GIC "realview_gic"
+#define REALVIEW_GIC(obj) \
+OBJECT_CHECK(RealViewGICState, (obj), TYPE_REALVIEW_GIC)
+
+typedef struct RealViewGICState {
+SysBusDevice parent_obj;
+
+MemoryRegion container;
+
+GICState gic;
+} RealViewGICState;
+
+#endif
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events

2013-10-08 Thread Hans de Goede

Hi,

On 10/08/2013 09:13 PM, Paolo Bonzini wrote:

Il 08/10/2013 21:10, Hans de Goede ha scritto:

@@ -480,6 +480,11 @@ int main_loop_wait(int nonblocking)
timerlistgroup_deadline_ns(
&main_loop_tlg));

+/* When not non-blocking always allow io-threads to acquire the lock */
+if (timeout != 0 && timeout_ns == 0) {
+timeout_ns = 1;
+}
+


This _is_ an I/O thread, so I guess this should be changed to "other
threads".


Ok.


Also, perhaps timeout_ns can be changed to a higher value
such as 250 us that were used up to the offending commit?


Wasn't it 1 ms until the offending commit (note 250 us does
sound better to me). More over I wonder how useful is this, if threads
are waiting for the lock at his point, they should all get it and do
work (and then release it) before this thread will be able to re-aquire it.

The only case which I can see where going to sleep will help is when a
thread takes the lock, does some thing, releases it, then does something
else quite quickly (so within 250 us), and then tries to re-aquire the lock
to do more work.

Note either solution (1 ns versus 250 us) is fine with me, I'm by no means
the expert on this, just let me know which one you think is better and I'll
do a v2.

Regards,

Hans



[Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events

2013-10-08 Thread Hans de Goede
I noticed today that current qemu master would hang as soon as Xorg starts in
the guest when using qxl + a Linux guest. This message would be printed:
main-loop: WARNING: I/O thread spun for 1000 iterations

And from then on the guest hangs and qemu consumes 100% cpu, bisecting pointed
out commit 7b595f35d89d73bc69c35bf3980a89c420e8a44b:
"aio / timers: Convert mainloop to use timeout"

After looking at that commit I had a hunch the problem might be blocking
main_loop_wait calls being turned into non-blocking ones (and thus never
releasing the io-lock), a debug printf confirmed this was happening at
the moment of the hang, so I wrote this patch which fixes the hang for me
and seems like a good idea in general.

Signed-off-by: Hans de Goede 
---
 main-loop.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/main-loop.c b/main-loop.c
index c3c9c28..921c939 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -480,6 +480,11 @@ int main_loop_wait(int nonblocking)
   timerlistgroup_deadline_ns(
   &main_loop_tlg));
 
+/* When not non-blocking always allow io-threads to acquire the lock */
+if (timeout != 0 && timeout_ns == 0) {
+timeout_ns = 1;
+}
+
 ret = os_host_main_loop_wait(timeout_ns);
 qemu_iohandler_poll(gpollfds, ret);
 #ifdef CONFIG_SLIRP
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events

2013-10-08 Thread Paolo Bonzini
Il 08/10/2013 21:10, Hans de Goede ha scritto:
> @@ -480,6 +480,11 @@ int main_loop_wait(int nonblocking)
>timerlistgroup_deadline_ns(
>&main_loop_tlg));
>  
> +/* When not non-blocking always allow io-threads to acquire the lock */
> +if (timeout != 0 && timeout_ns == 0) {
> +timeout_ns = 1;
> +}
> +

This _is_ an I/O thread, so I guess this should be changed to "other
threads".  Also, perhaps timeout_ns can be changed to a higher value
such as 250 us that were used up to the offending commit?

Otherwise looks good!

Paolo



[Qemu-devel] [PULL 34/58] a9scu: Build only once

2013-10-08 Thread Andreas Färber
It does not have a target or ARMCPU dependency.

Reviewed-by: Peter Maydell 
Signed-off-by: Andreas Färber 
---
 hw/misc/Makefile.objs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 2578e29..5636299 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -10,6 +10,7 @@ obj-$(CONFIG_VMPORT) += vmport.o
 
 # ARM devices
 common-obj-$(CONFIG_PL310) += arm_l2x0.o
+common-obj-$(CONFIG_A9SCU) += a9scu.o
 
 # PKUnity SoC devices
 common-obj-$(CONFIG_PUV3) += puv3_pm.o
@@ -22,7 +23,6 @@ obj-$(CONFIG_LINUX) += vfio.o
 endif
 
 obj-$(CONFIG_REALVIEW) += arm_sysctl.o
-obj-$(CONFIG_A9SCU) += a9scu.o
 obj-$(CONFIG_NSERIES) += cbus.o
 obj-$(CONFIG_ECCMEMCTL) += eccmemctl.o
 obj-$(CONFIG_EXYNOS4) += exynos4210_pmu.o
-- 
1.8.1.4




Re: [Qemu-devel] [Spice-devel] Current qemu-master hangs when used with qxl + linux guest

2013-10-08 Thread Hans de Goede

Hi,

On 10/08/2013 04:30 PM, Daniel P. Berrange wrote:

On Tue, Oct 08, 2013 at 04:27:38PM +0200, Hans de Goede wrote:

Hi All,

I'm having this weird problem with qemu master + spice/qxl using
guests. As soon as the guest starts Xorg, I get the following message
from qemu:

main-loop: WARNING: I/O thread spun for 1000 iterations

And from then on the guest hangs and qemu consumes 100% cpu. The qemu
console still works, and I can quit qemu that way.

Doing ctrl+c + a thread apply all bt in qemy shows one cpu thread waiting
for the iothread-lock, and all other threads waiting in poll.

This happens both with non kms guests (tried RHEL-6.5, older Fedoras)
as well as with kms guests (tried a fully up2date F-19).

Since I've not seen any similar reports, I assume it is something
with my setup ...

I've tried changing various things:
-removing the spice agent channel
-changing the number of virtual cpus (tried 1 and 2 virtual cpus)
-upgrading spice-server to the latest git master

But all to no avail.

This is with qemu-master build from source on a fully up2date
F-20 system, using the F-20 seabios files.

If someone has any clever ideas I'll happily try debugging this further.


Does the QEMU build in F20 work correctly ?


Ah, yes it does, good dea.


If so that'd give you a
starting point from which to 'git bisect' the problem.


Yep, note I asked for a "clever" idea, iow not the big hammer of bisecting :)

But since 1.6.0 in F-20 worked it was not such a large bisect, so I went
ahead and bisected anyways, which pointed me to commit 7b595f35d8 :
"aio / timers: Convert mainloop to use timeout"

After careful review of that commit I had a hunch what might be wrong, and
it turned out to be right. So after this mail I'm going to send a patch
fixing this. If you want to know the details see the patch, titled:
"main-loop: Don't lock starve io-threads when main_loop_tlg has pending events"

Regards,

Hans



[Qemu-devel] [PULL 25/58] a9mpcore: Embed A9SCUState

2013-10-08 Thread Andreas Färber
From: Andreas Färber 

Prepares for QOM realize.

Reviewed-by: Peter Maydell 
Signed-off-by: Andreas Färber 
---
 hw/cpu/a9mpcore.c   | 16 ++--
 hw/misc/a9scu.c | 18 +-
 include/hw/misc/a9scu.h | 31 +++
 3 files changed, 42 insertions(+), 23 deletions(-)
 create mode 100644 include/hw/misc/a9scu.h

diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
index c57b149..df92e3f 100644
--- a/hw/cpu/a9mpcore.c
+++ b/hw/cpu/a9mpcore.c
@@ -10,6 +10,7 @@
 
 #include "hw/sysbus.h"
 #include "hw/intc/arm_gic.h"
+#include "hw/misc/a9scu.h"
 
 #define TYPE_A9MPCORE_PRIV "a9mpcore_priv"
 #define A9MPCORE_PRIV(obj) \
@@ -24,10 +25,10 @@ typedef struct A9MPPrivState {
 MemoryRegion container;
 DeviceState *mptimer;
 DeviceState *wdt;
-DeviceState *scu;
 uint32_t num_irq;
 
 GICState gic;
+A9SCUState scu;
 } A9MPPrivState;
 
 static void a9mp_priv_set_irq(void *opaque, int irq, int level)
@@ -46,12 +47,15 @@ static void a9mp_priv_initfn(Object *obj)
 
 object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
 qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
+
+object_initialize(&s->scu, sizeof(s->scu), TYPE_A9_SCU);
+qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
 }
 
 static int a9mp_priv_init(SysBusDevice *dev)
 {
 A9MPPrivState *s = A9MPCORE_PRIV(dev);
-DeviceState *gicdev;
+DeviceState *gicdev, *scudev;
 SysBusDevice *timerbusdev, *wdtbusdev, *gicbusdev, *scubusdev;
 int i;
 
@@ -67,10 +71,10 @@ static int a9mp_priv_init(SysBusDevice *dev)
 /* Pass through inbound GPIO lines to the GIC */
 qdev_init_gpio_in(DEVICE(dev), a9mp_priv_set_irq, s->num_irq - 32);
 
-s->scu = qdev_create(NULL, "a9-scu");
-qdev_prop_set_uint32(s->scu, "num-cpu", s->num_cpu);
-qdev_init_nofail(s->scu);
-scubusdev = SYS_BUS_DEVICE(s->scu);
+scudev = DEVICE(&s->scu);
+qdev_prop_set_uint32(scudev, "num-cpu", s->num_cpu);
+qdev_init_nofail(scudev);
+scubusdev = SYS_BUS_DEVICE(&s->scu);
 
 s->mptimer = qdev_create(NULL, "arm_mptimer");
 qdev_prop_set_uint32(s->mptimer, "num-cpu", s->num_cpu);
diff --git a/hw/misc/a9scu.c b/hw/misc/a9scu.c
index 2661014..4434945 100644
--- a/hw/misc/a9scu.c
+++ b/hw/misc/a9scu.c
@@ -8,23 +8,7 @@
  * This code is licensed under the GPL.
  */
 
-#include "hw/sysbus.h"
-
-/* A9MP private memory region.  */
-
-typedef struct A9SCUState {
-/*< private >*/
-SysBusDevice parent_obj;
-/*< public >*/
-
-MemoryRegion iomem;
-uint32_t control;
-uint32_t status;
-uint32_t num_cpu;
-} A9SCUState;
-
-#define TYPE_A9_SCU "a9-scu"
-#define A9_SCU(obj) OBJECT_CHECK(A9SCUState, (obj), TYPE_A9_SCU)
+#include "hw/misc/a9scu.h"
 
 static uint64_t a9_scu_read(void *opaque, hwaddr offset,
 unsigned size)
diff --git a/include/hw/misc/a9scu.h b/include/hw/misc/a9scu.h
new file mode 100644
index 000..efb0c30
--- /dev/null
+++ b/include/hw/misc/a9scu.h
@@ -0,0 +1,31 @@
+/*
+ * Cortex-A9MPCore Snoop Control Unit (SCU) emulation.
+ *
+ * Copyright (c) 2009 CodeSourcery.
+ * Copyright (c) 2011 Linaro Limited.
+ * Written by Paul Brook, Peter Maydell.
+ *
+ * This code is licensed under the GPL.
+ */
+#ifndef HW_MISC_A9SCU_H
+#define HW_MISC_A9SCU_H
+
+#include "hw/sysbus.h"
+
+/* A9MP private memory region.  */
+
+typedef struct A9SCUState {
+/*< private >*/
+SysBusDevice parent_obj;
+/*< public >*/
+
+MemoryRegion iomem;
+uint32_t control;
+uint32_t status;
+uint32_t num_cpu;
+} A9SCUState;
+
+#define TYPE_A9_SCU "a9-scu"
+#define A9_SCU(obj) OBJECT_CHECK(A9SCUState, (obj), TYPE_A9_SCU)
+
+#endif
-- 
1.8.1.4




Re: [Qemu-devel] [PATCHv6] block/get_block_status: avoid redundant callouts on raw devices

2013-10-08 Thread Eric Blake
On 10/08/2013 06:43 AM, Peter Lieven wrote:
> if a raw device like an iscsi target or host device is used
> the current implementation makes a second call out to get
> the block status of bs->file.
> 
> Signed-off-by: Peter Lieven 
> ---

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PULL 27/58] a9mpcore: Embed ARMMPTimerState

2013-10-08 Thread Andreas Färber
From: Andreas Färber 

Prepares for QOM realize.

Reviewed-by: Peter Maydell 
Signed-off-by: Andreas Färber 
---
 hw/cpu/a9mpcore.c  | 29 ++-
 hw/timer/arm_mptimer.c | 35 ---
 include/hw/timer/arm_mptimer.h | 54 ++
 3 files changed, 76 insertions(+), 42 deletions(-)
 create mode 100644 include/hw/timer/arm_mptimer.h

diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
index df92e3f..db3907e 100644
--- a/hw/cpu/a9mpcore.c
+++ b/hw/cpu/a9mpcore.c
@@ -11,6 +11,7 @@
 #include "hw/sysbus.h"
 #include "hw/intc/arm_gic.h"
 #include "hw/misc/a9scu.h"
+#include "hw/timer/arm_mptimer.h"
 
 #define TYPE_A9MPCORE_PRIV "a9mpcore_priv"
 #define A9MPCORE_PRIV(obj) \
@@ -23,12 +24,12 @@ typedef struct A9MPPrivState {
 
 uint32_t num_cpu;
 MemoryRegion container;
-DeviceState *mptimer;
-DeviceState *wdt;
 uint32_t num_irq;
 
 GICState gic;
 A9SCUState scu;
+ARMMPTimerState mptimer;
+ARMMPTimerState wdt;
 } A9MPPrivState;
 
 static void a9mp_priv_set_irq(void *opaque, int irq, int level)
@@ -50,12 +51,18 @@ static void a9mp_priv_initfn(Object *obj)
 
 object_initialize(&s->scu, sizeof(s->scu), TYPE_A9_SCU);
 qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
+
+object_initialize(&s->mptimer, sizeof(s->mptimer), TYPE_ARM_MPTIMER);
+qdev_set_parent_bus(DEVICE(&s->mptimer), sysbus_get_default());
+
+object_initialize(&s->wdt, sizeof(s->wdt), TYPE_ARM_MPTIMER);
+qdev_set_parent_bus(DEVICE(&s->wdt), sysbus_get_default());
 }
 
 static int a9mp_priv_init(SysBusDevice *dev)
 {
 A9MPPrivState *s = A9MPCORE_PRIV(dev);
-DeviceState *gicdev, *scudev;
+DeviceState *gicdev, *scudev, *mptimerdev, *wdtdev;
 SysBusDevice *timerbusdev, *wdtbusdev, *gicbusdev, *scubusdev;
 int i;
 
@@ -76,15 +83,15 @@ static int a9mp_priv_init(SysBusDevice *dev)
 qdev_init_nofail(scudev);
 scubusdev = SYS_BUS_DEVICE(&s->scu);
 
-s->mptimer = qdev_create(NULL, "arm_mptimer");
-qdev_prop_set_uint32(s->mptimer, "num-cpu", s->num_cpu);
-qdev_init_nofail(s->mptimer);
-timerbusdev = SYS_BUS_DEVICE(s->mptimer);
+mptimerdev = DEVICE(&s->mptimer);
+qdev_prop_set_uint32(mptimerdev, "num-cpu", s->num_cpu);
+qdev_init_nofail(mptimerdev);
+timerbusdev = SYS_BUS_DEVICE(&s->mptimer);
 
-s->wdt = qdev_create(NULL, "arm_mptimer");
-qdev_prop_set_uint32(s->wdt, "num-cpu", s->num_cpu);
-qdev_init_nofail(s->wdt);
-wdtbusdev = SYS_BUS_DEVICE(s->wdt);
+wdtdev = DEVICE(&s->wdt);
+qdev_prop_set_uint32(wdtdev, "num-cpu", s->num_cpu);
+qdev_init_nofail(wdtdev);
+wdtbusdev = SYS_BUS_DEVICE(&s->wdt);
 
 /* Memory map (addresses are offsets from PERIPHBASE):
  *  0x-0x00ff -- Snoop Control Unit
diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c
index 2853db4..d9f9494 100644
--- a/hw/timer/arm_mptimer.c
+++ b/hw/timer/arm_mptimer.c
@@ -19,7 +19,7 @@
  * with this program; if not, see .
  */
 
-#include "hw/sysbus.h"
+#include "hw/timer/arm_mptimer.h"
 #include "qemu/timer.h"
 #include "qom/cpu.h"
 
@@ -27,34 +27,6 @@
  * which is used in both the ARM11MPCore and Cortex-A9MP.
  */
 
-#define MAX_CPUS 4
-
-/* State of a single timer or watchdog block */
-typedef struct {
-uint32_t count;
-uint32_t load;
-uint32_t control;
-uint32_t status;
-int64_t tick;
-QEMUTimer *timer;
-qemu_irq irq;
-MemoryRegion iomem;
-} TimerBlock;
-
-#define TYPE_ARM_MPTIMER "arm_mptimer"
-#define ARM_MPTIMER(obj) \
-OBJECT_CHECK(ARMMPTimerState, (obj), TYPE_ARM_MPTIMER)
-
-typedef struct {
-/*< private >*/
-SysBusDevice parent_obj;
-/*< public >*/
-
-uint32_t num_cpu;
-TimerBlock timerblock[MAX_CPUS];
-MemoryRegion iomem;
-} ARMMPTimerState;
-
 static inline int get_current_cpu(ARMMPTimerState *s)
 {
 if (current_cpu->cpu_index >= s->num_cpu) {
@@ -240,8 +212,9 @@ static void arm_mptimer_realize(DeviceState *dev, Error 
**errp)
 ARMMPTimerState *s = ARM_MPTIMER(dev);
 int i;
 
-if (s->num_cpu < 1 || s->num_cpu > MAX_CPUS) {
-hw_error("%s: num-cpu must be between 1 and %d\n", __func__, MAX_CPUS);
+if (s->num_cpu < 1 || s->num_cpu > ARM_MPTIMER_MAX_CPUS) {
+hw_error("%s: num-cpu must be between 1 and %d\n",
+ __func__, ARM_MPTIMER_MAX_CPUS);
 }
 /* We implement one timer block per CPU, and expose multiple MMIO regions:
  *  * region 0 is "timer for this core"
diff --git a/include/hw/timer/arm_mptimer.h b/include/hw/timer/arm_mptimer.h
new file mode 100644
index 000..b34cba0
--- /dev/null
+++ b/include/hw/timer/arm_mptimer.h
@@ -0,0 +1,54 @@
+/*
+ * Private peripheral timer/watchdog blocks for ARM 11MPCore and A9MP
+ *
+ * Copyright (c) 2006-2007 CodeSourcery.
+ * Copyright (c) 2011 Linaro Limited
+ * Written by Paul Brook, Peter Maydell
+ *
+ * This program is free software; you 

[Qemu-devel] [PULL 45/58] qdev-monitor: Clean up qdev_device_add() variable naming

2013-10-08 Thread Andreas Färber
Avoid confusion between object (obj) and object class (oc).
Tidy DeviceClass variable while at it (k -> dc).

Signed-off-by: Andreas Färber 
---
 qdev-monitor.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index 410cdcb..51bfec0 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -444,8 +444,8 @@ static BusState *qbus_find(const char *path)
 
 DeviceState *qdev_device_add(QemuOpts *opts)
 {
-ObjectClass *obj;
-DeviceClass *k;
+ObjectClass *oc;
+DeviceClass *dc;
 const char *driver, *path, *id;
 DeviceState *qdev;
 BusState *bus = NULL;
@@ -457,22 +457,22 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 }
 
 /* find driver */
-obj = object_class_by_name(driver);
-if (!obj) {
+oc = object_class_by_name(driver);
+if (!oc) {
 const char *typename = find_typename_by_alias(driver);
 
 if (typename) {
 driver = typename;
-obj = object_class_by_name(driver);
+oc = object_class_by_name(driver);
 }
 }
 
-if (!obj) {
+if (!oc) {
 qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver", "device type");
 return NULL;
 }
 
-k = DEVICE_CLASS(obj);
+dc = DEVICE_CLASS(oc);
 
 /* find bus */
 path = qemu_opt_get(opts, "bus");
@@ -481,16 +481,16 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 if (!bus) {
 return NULL;
 }
-if (!object_dynamic_cast(OBJECT(bus), k->bus_type)) {
+if (!object_dynamic_cast(OBJECT(bus), dc->bus_type)) {
 qerror_report(QERR_BAD_BUS_FOR_DEVICE,
   driver, object_get_typename(OBJECT(bus)));
 return NULL;
 }
-} else if (k->bus_type != NULL) {
-bus = qbus_find_recursive(sysbus_get_default(), NULL, k->bus_type);
+} else if (dc->bus_type != NULL) {
+bus = qbus_find_recursive(sysbus_get_default(), NULL, dc->bus_type);
 if (!bus) {
 qerror_report(QERR_NO_BUS_FOR_DEVICE,
-  k->bus_type, driver);
+  dc->bus_type, driver);
 return NULL;
 }
 }
-- 
1.8.1.4




[Qemu-devel] [PULL 11/58] armv7m: Don't enforce use of kernel for qtest

2013-10-08 Thread Andreas Färber
Adopt error_report().

Signed-off-by: Andreas Färber 
---
 hw/arm/armv7m.c | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 89a9015..397e8df 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -11,6 +11,8 @@
 #include "hw/arm/arm.h"
 #include "hw/loader.h"
 #include "elf.h"
+#include "sysemu/qtest.h"
+#include "qemu/error-report.h"
 
 /* Bitbanded IO.  Each word corresponds to a single bit.  */
 
@@ -232,21 +234,22 @@ qemu_irq *armv7m_init(MemoryRegion *address_space_mem,
 big_endian = 0;
 #endif
 
-if (!kernel_filename) {
+if (!kernel_filename && !qtest_enabled()) {
 fprintf(stderr, "Guest image must be specified (using -kernel)\n");
 exit(1);
 }
 
-image_size = load_elf(kernel_filename, NULL, NULL, &entry, &lowaddr,
-  NULL, big_endian, ELF_MACHINE, 1);
-if (image_size < 0) {
-image_size = load_image_targphys(kernel_filename, 0, flash_size);
-   lowaddr = 0;
-}
-if (image_size < 0) {
-fprintf(stderr, "qemu: could not load kernel '%s'\n",
-kernel_filename);
-exit(1);
+if (kernel_filename) {
+image_size = load_elf(kernel_filename, NULL, NULL, &entry, &lowaddr,
+  NULL, big_endian, ELF_MACHINE, 1);
+if (image_size < 0) {
+image_size = load_image_targphys(kernel_filename, 0, flash_size);
+lowaddr = 0;
+}
+if (image_size < 0) {
+error_report("Could not load kernel '%s'", kernel_filename);
+exit(1);
+}
 }
 
 /* Hack to map an additional page of ram at the top of the address
-- 
1.8.1.4




[Qemu-devel] [PULL 56/58] microdrive: Coding Style cleanups

2013-10-08 Thread Andreas Färber
Add missing braces.

Signed-off-by: Andreas Färber 
---
 hw/ide/microdrive.c | 62 -
 1 file changed, 38 insertions(+), 24 deletions(-)

diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index cdf0eb9..21d6495 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -103,10 +103,12 @@ static inline void md_interrupt_update(MicroDriveState *s)
 static void md_set_irq(void *opaque, int irq, int level)
 {
 MicroDriveState *s = opaque;
-if (level)
+
+if (level) {
 s->stat |= STAT_INT;
-else
+} else {
 s->stat &= ~STAT_INT;
+}
 
 md_interrupt_update(s);
 }
@@ -142,10 +144,11 @@ static uint8_t md_attr_read(PCMCIACardState *card, 
uint32_t at)
 case 0x00: /* Configuration Option Register */
 return s->opt;
 case 0x02: /* Card Configuration Status Register */
-if (s->ctrl & CTRL_IEN)
+if (s->ctrl & CTRL_IEN) {
 return s->stat & ~STAT_INT;
-else
+} else {
 return s->stat;
+}
 case 0x04: /* Pin Replacement Register */
 return (s->pins & PINS_CRDY) | 0x0c;
 case 0x06: /* Socket and Copy Register */
@@ -174,8 +177,9 @@ static void md_attr_write(PCMCIACardState *card, uint32_t 
at, uint8_t value)
 md_interrupt_update(s);
 break;
 case 0x02: /* Card Configuration Status Register */
-if ((s->stat ^ value) & STAT_PWRDWN)
+if ((s->stat ^ value) & STAT_PWRDWN) {
 s->pins |= PINS_CRDY;
+}
 s->stat &= 0x82;
 s->stat |= value & 0x74;
 md_interrupt_update(s);
@@ -201,23 +205,26 @@ static uint16_t md_common_read(PCMCIACardState *card, 
uint32_t at)
 
 switch (s->opt & OPT_MODE) {
 case OPT_MODE_MMAP:
-if ((at & ~0x3ff) == 0x400)
+if ((at & ~0x3ff) == 0x400) {
 at = 0;
+}
 break;
 case OPT_MODE_IOMAP16:
 at &= 0xf;
 break;
 case OPT_MODE_IOMAP1:
-if ((at & ~0xf) == 0x3f0)
+if ((at & ~0xf) == 0x3f0) {
 at -= 0x3e8;
-else if ((at & ~0xf) == 0x1f0)
+} else if ((at & ~0xf) == 0x1f0) {
 at -= 0x1f0;
+}
 break;
 case OPT_MODE_IOMAP2:
-if ((at & ~0xf) == 0x370)
+if ((at & ~0xf) == 0x370) {
 at -= 0x368;
-else if ((at & ~0xf) == 0x170)
+} else if ((at & ~0xf) == 0x170) {
 at -= 0x170;
+}
 }
 
 switch (at) {
@@ -226,9 +233,9 @@ static uint16_t md_common_read(PCMCIACardState *card, 
uint32_t at)
 return ide_data_readw(&s->bus, 0);
 
 /* TODO: 8-bit accesses */
-if (s->cycle)
+if (s->cycle) {
 ret = s->io >> 8;
-else {
+} else {
 s->io = ide_data_readw(&s->bus, 0);
 ret = s->io & 0xff;
 }
@@ -240,10 +247,11 @@ static uint16_t md_common_read(PCMCIACardState *card, 
uint32_t at)
 return ide_ioport_read(&s->bus, 0x1);
 case 0xe:  /* Alternate Status */
 ifs = idebus_active_if(&s->bus);
-if (ifs->bs)
+if (ifs->bs) {
 return ifs->status;
-else
+} else {
 return 0;
+}
 case 0xf:  /* Device Address */
 ifs = idebus_active_if(&s->bus);
 return 0xc2 | ((~ifs->select << 2) & 0x3c);
@@ -261,23 +269,26 @@ static void md_common_write(PCMCIACardState *card, 
uint32_t at, uint16_t value)
 
 switch (s->opt & OPT_MODE) {
 case OPT_MODE_MMAP:
-if ((at & ~0x3ff) == 0x400)
+if ((at & ~0x3ff) == 0x400) {
 at = 0;
+}
 break;
 case OPT_MODE_IOMAP16:
 at &= 0xf;
 break;
 case OPT_MODE_IOMAP1:
-if ((at & ~0xf) == 0x3f0)
+if ((at & ~0xf) == 0x3f0) {
 at -= 0x3e8;
-else if ((at & ~0xf) == 0x1f0)
+} else if ((at & ~0xf) == 0x1f0) {
 at -= 0x1f0;
+}
 break;
 case OPT_MODE_IOMAP2:
-if ((at & ~0xf) == 0x370)
+if ((at & ~0xf) == 0x370) {
 at -= 0x368;
-else if ((at & ~0xf) == 0x170)
+} else if ((at & ~0xf) == 0x170) {
 at -= 0x170;
+}
 }
 
 switch (at) {
@@ -287,10 +298,11 @@ static void md_common_write(PCMCIACardState *card, 
uint32_t at, uint16_t value)
 break;
 
 /* TODO: 8-bit accesses */
-if (s->cycle)
+if (s->cycle) {
 ide_data_writew(&s->bus, 0, s->io | (value << 8));
-else
+} else {
 s->io = value & 0xff;
+}
 s->cycle = !s->cycle;
 break;
 case 0x9:
@@ -546,8 +558,10 @@ static int dscm1_detach(PCMCIACardState *card)
 
 PCMCIACardState *dscm1_init(DriveInfo *dinfo)
 {
-MicroDriveState *md = MICRODRIVE(object_new(TYPE_DSCM1));
-PCMCIACardState *card = PCMCIA_CARD(md);
+MicroDriveState *md;
+
+md = MICRODRIVE(object_new(TYPE_DSCM1

[Qemu-devel] [PULL 48/58] qdev: Drop misleading qdev_free() function

2013-10-08 Thread Andreas Färber
From: Stefan Hajnoczi 

The qdev_free() function name is misleading since all the function does
is unlink the device from its parent.  The device is not necessarily
freed.

The device will be freed when its QObject refcount reaches zero.  It is
usual for the parent (bus) to hold the final reference but there are
cases where something else holds a reference so "free" is a misleading
name.

Call object_unparent(obj) directly instead of having a qdev wrapper
function.

Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Andreas Färber 
---
 hw/acpi/piix4.c  |  2 +-
 hw/core/qdev.c   | 12 +++-
 hw/pci/pci-hotplug-old.c |  2 +-
 hw/pci/pci_bridge.c  |  2 +-
 hw/pci/pcie.c|  2 +-
 hw/pci/shpc.c|  2 +-
 hw/s390x/virtio-ccw.c|  2 +-
 hw/scsi/scsi-bus.c   |  6 +++---
 hw/usb/bus.c |  7 ---
 hw/usb/dev-storage.c |  2 +-
 hw/usb/host-legacy.c |  2 +-
 hw/virtio/virtio-bus.c   |  4 +---
 hw/xen/xen_platform.c|  2 +-
 include/hw/qdev-core.h   |  1 -
 qdev-monitor.c   |  2 +-
 15 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index b46bd5e..7f5ab24 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -326,7 +326,7 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned 
slots)
 if (pc->no_hotplug) {
 slot_free = false;
 } else {
-qdev_free(qdev);
+object_unparent(OBJECT(qdev));
 }
 }
 }
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 533f6dd..e374a93 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -164,7 +164,7 @@ int qdev_init(DeviceState *dev)
 if (local_err != NULL) {
 qerror_report_err(local_err);
 error_free(local_err);
-qdev_free(dev);
+object_unparent(OBJECT(dev));
 return -1;
 }
 return 0;
@@ -258,7 +258,7 @@ void qbus_reset_all_fn(void *opaque)
 int qdev_simple_unplug_cb(DeviceState *dev)
 {
 /* just zap it */
-qdev_free(dev);
+object_unparent(OBJECT(dev));
 return 0;
 }
 
@@ -280,12 +280,6 @@ void qdev_init_nofail(DeviceState *dev)
 }
 }
 
-/* Unlink device from bus and free the structure.  */
-void qdev_free(DeviceState *dev)
-{
-object_unparent(OBJECT(dev));
-}
-
 void qdev_machine_creation_done(void)
 {
 /*
@@ -458,7 +452,7 @@ static void bus_unparent(Object *obj)
 
 while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
 DeviceState *dev = kid->child;
-qdev_free(dev);
+object_unparent(OBJECT(dev));
 }
 if (bus->parent) {
 QLIST_REMOVE(bus, sibling);
diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index 619fe47..8dbc3c1 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -248,7 +248,7 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
 }
 dev = pci_create(bus, devfn, "virtio-blk-pci");
 if (qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv) < 0) {
-qdev_free(&dev->qdev);
+object_unparent(OBJECT(dev));
 dev = NULL;
 break;
 }
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index e6b22b8..290abab 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -391,7 +391,7 @@ void pci_bridge_exitfn(PCIDevice *pci_dev)
 pci_bridge_region_cleanup(s, s->windows);
 memory_region_destroy(&s->address_space_mem);
 memory_region_destroy(&s->address_space_io);
-/* qbus_free() is called automatically by qdev_free() */
+/* qbus_free() is called automatically during device deletion */
 }
 
 /*
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 50af3c1..a27acf3 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -251,7 +251,7 @@ static int pcie_cap_slot_hotplug(DeviceState *qdev,
PCI_EXP_SLTSTA_PDS);
 pcie_cap_slot_event(d, PCI_EXP_HP_EV_PDC);
 } else {
-qdev_free(&pci_dev->qdev);
+object_unparent(OBJECT(pci_dev));
 pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA,
  PCI_EXP_SLTSTA_PDS);
 pcie_cap_slot_event(d, PCI_EXP_HP_EV_PDC);
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index eb092fd..29a8c36 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -254,7 +254,7 @@ static void shpc_free_devices_in_slot(SHPCDevice *shpc, int 
slot)
  ++devfn) {
 PCIDevice *affected_dev = shpc->sec_bus->devices[devfn];
 if (affected_dev) {
-qdev_free(&affected_dev->qdev);
+object_unparent(OBJECT(affected_dev));
 }
 }
 }
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index cd67db5..f93a81c 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1239,7 +1239,7 @@ static int virtio_ccw_busdev_unplug(DeviceState *dev)
 
 css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid, 1, 0);
 
-qdev_free(dev);
+object_unparent(OB

[Qemu-devel] [PULL 49/58] qdev-monitor: Avoid qdev as variable name

2013-10-08 Thread Andreas Färber
Prepares for bringing error cleanup code into canonical QOM form.

Includes a whitespace removal after curly brace by Stefan.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Igor Mammedov 
Signed-off-by: Andreas Färber 
---
 qdev-monitor.c | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index 6aa3bb5..f259e07 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -447,7 +447,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 ObjectClass *oc;
 DeviceClass *dc;
 const char *driver, *path, *id;
-DeviceState *qdev;
+DeviceState *dev;
 BusState *bus = NULL;
 
 driver = qemu_opt_get(opts, "driver");
@@ -506,38 +506,38 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 }
 
 /* create device, set properties */
-qdev = DEVICE(object_new(driver));
+dev = DEVICE(object_new(driver));
 
 if (bus) {
-qdev_set_parent_bus(qdev, bus);
+qdev_set_parent_bus(dev, bus);
 }
 
 id = qemu_opts_id(opts);
 if (id) {
-qdev->id = id;
+dev->id = id;
 }
-if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
-object_unparent(OBJECT(qdev));
-object_unref(OBJECT(qdev));
+if (qemu_opt_foreach(opts, set_property, dev, 1) != 0) {
+object_unparent(OBJECT(dev));
+object_unref(OBJECT(dev));
 return NULL;
 }
-if (qdev->id) {
-object_property_add_child(qdev_get_peripheral(), qdev->id,
-  OBJECT(qdev), NULL);
+if (dev->id) {
+object_property_add_child(qdev_get_peripheral(), dev->id,
+  OBJECT(dev), NULL);
 } else {
 static int anon_count;
 gchar *name = g_strdup_printf("device[%d]", anon_count++);
 object_property_add_child(qdev_get_peripheral_anon(), name,
-  OBJECT(qdev), NULL);
+  OBJECT(dev), NULL);
 g_free(name);
-}
-if (qdev_init(qdev) < 0) {
-object_unref(OBJECT(qdev));
+}
+if (qdev_init(dev) < 0) {
+object_unref(OBJECT(dev));
 qerror_report(QERR_DEVICE_INIT_FAILED, driver);
 return NULL;
 }
-qdev->opts = opts;
-return qdev;
+dev->opts = opts;
+return dev;
 }
 
 
-- 
1.8.1.4




[Qemu-devel] [PULL 42/58] arm11mpcore: Convert mpcore_rirq_state to QOM realize

2013-10-08 Thread Andreas Färber
Embed ARM11MPCorePriveState and RealViewGICState and replace SysBus
initfn with realizefn.

Reviewed-by: Peter Maydell 
Signed-off-by: Andreas Färber 
---
 hw/cpu/arm11mpcore.c | 58 +++-
 1 file changed, 44 insertions(+), 14 deletions(-)

diff --git a/hw/cpu/arm11mpcore.c b/hw/cpu/arm11mpcore.c
index f372283..578e3d3 100644
--- a/hw/cpu/arm11mpcore.c
+++ b/hw/cpu/arm11mpcore.c
@@ -10,6 +10,7 @@
 #include "hw/sysbus.h"
 #include "hw/misc/arm11scu.h"
 #include "hw/intc/arm_gic.h"
+#include "hw/intc/realview_gic.h"
 #include "hw/timer/arm_mptimer.h"
 #include "qemu/timer.h"
 
@@ -168,10 +169,12 @@ static void mpcore_priv_initfn(Object *obj)
 typedef struct {
 SysBusDevice parent_obj;
 
-SysBusDevice *priv;
 qemu_irq cpuic[32];
 qemu_irq rvic[4][64];
 uint32_t num_cpu;
+
+ARM11MPCorePriveState priv;
+RealViewGICState gic[4];
 } mpcore_rirq_state;
 
 /* Map baseboard IRQs onto CPU IRQ lines.  */
@@ -198,34 +201,61 @@ static void mpcore_rirq_set_irq(void *opaque, int irq, 
int level)
 }
 }
 
-static int realview_mpcore_init(SysBusDevice *sbd)
+static void realview_mpcore_realize(DeviceState *dev, Error **errp)
 {
-DeviceState *dev = DEVICE(sbd);
+SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 mpcore_rirq_state *s = REALVIEW_MPCORE_RIRQ(dev);
+DeviceState *priv = DEVICE(&s->priv);
 DeviceState *gic;
-DeviceState *priv;
+SysBusDevice *gicbusdev;
+Error *err = NULL;
 int n;
 int i;
 
-priv = qdev_create(NULL, TYPE_ARM11MPCORE_PRIV);
 qdev_prop_set_uint32(priv, "num-cpu", s->num_cpu);
-qdev_init_nofail(priv);
-s->priv = SYS_BUS_DEVICE(priv);
-sysbus_pass_irq(sbd, s->priv);
+object_property_set_bool(OBJECT(&s->priv), true, "realized", &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
+}
+sysbus_pass_irq(sbd, SYS_BUS_DEVICE(&s->priv));
 for (i = 0; i < 32; i++) {
 s->cpuic[i] = qdev_get_gpio_in(priv, i);
 }
 /* ??? IRQ routing is hardcoded to "normal" mode.  */
 for (n = 0; n < 4; n++) {
-gic = sysbus_create_simple("realview_gic", 0x1004 + n * 0x1,
-   s->cpuic[10 + n]);
+object_property_set_bool(OBJECT(&s->gic[n]), true, "realized", &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
+}
+gic = DEVICE(&s->gic[n]);
+gicbusdev = SYS_BUS_DEVICE(&s->gic[n]);
+sysbus_mmio_map(gicbusdev, 0, 0x1004 + n * 0x1);
+sysbus_connect_irq(gicbusdev, 0, s->cpuic[10 + n]);
 for (i = 0; i < 64; i++) {
 s->rvic[n][i] = qdev_get_gpio_in(gic, i);
 }
 }
 qdev_init_gpio_in(dev, mpcore_rirq_set_irq, 64);
-sysbus_init_mmio(sbd, sysbus_mmio_get_region(s->priv, 0));
-return 0;
+}
+
+static void mpcore_rirq_init(Object *obj)
+{
+SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+mpcore_rirq_state *s = REALVIEW_MPCORE_RIRQ(obj);
+SysBusDevice *privbusdev;
+int i;
+
+object_initialize(&s->priv, sizeof(s->priv), TYPE_ARM11MPCORE_PRIV);
+qdev_set_parent_bus(DEVICE(&s->priv), sysbus_get_default());
+privbusdev = SYS_BUS_DEVICE(&s->priv);
+sysbus_init_mmio(sbd, sysbus_mmio_get_region(privbusdev, 0));
+
+for (i = 0; i < 4; i++) {
+object_initialize(&s->gic[i], sizeof(s->gic[i]), TYPE_REALVIEW_GIC);
+qdev_set_parent_bus(DEVICE(&s->gic[i]), sysbus_get_default());
+}
 }
 
 static Property mpcore_rirq_properties[] = {
@@ -236,9 +266,8 @@ static Property mpcore_rirq_properties[] = {
 static void mpcore_rirq_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-k->init = realview_mpcore_init;
+dc->realize = realview_mpcore_realize;
 dc->props = mpcore_rirq_properties;
 }
 
@@ -246,6 +275,7 @@ static const TypeInfo mpcore_rirq_info = {
 .name  = TYPE_REALVIEW_MPCORE_RIRQ,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(mpcore_rirq_state),
+.instance_init = mpcore_rirq_init,
 .class_init= mpcore_rirq_class_init,
 };
 
-- 
1.8.1.4




[Qemu-devel] [PULL 19/58] qtest: Prepare QOM machine tests

2013-10-08 Thread Andreas Färber
Instantiate all [*] machines per target, so that they get a bit of test
coverage at all. This has proven helpful during QOM refactorings.

[*] ppcemb target contains some non-working non-embedded machines, and
ppc405 CPUs are not available there either.
i386 and x86_64 do not cover pc*-x.y or xenfv.

Signed-off-by: Andreas Färber 
---
 tests/Makefile   |  26 ++
 tests/qom-test.c | 253 +++
 2 files changed, 279 insertions(+)
 create mode 100644 tests/qom-test.c

diff --git a/tests/Makefile b/tests/Makefile
index 994fef1..53a5c09 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -67,25 +67,50 @@ check-qtest-i386-y += tests/boot-order-test$(EXESUF)
 check-qtest-i386-y += tests/rtc-test$(EXESUF)
 check-qtest-i386-y += tests/i440fx-test$(EXESUF)
 check-qtest-i386-y += tests/fw_cfg-test$(EXESUF)
+check-qtest-i386-y += tests/qom-test$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/mc146818rtc.c
 gcov-files-x86_64-y = $(subst 
i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
 check-qtest-mips-y = tests/endianness-test$(EXESUF)
 check-qtest-mips64-y = tests/endianness-test$(EXESUF)
 check-qtest-mips64el-y = tests/endianness-test$(EXESUF)
+check-qtest-mips-y += tests/qom-test$(EXESUF)
+check-qtest-mipsel-y += tests/qom-test$(EXESUF)
+check-qtest-mips64-y += tests/qom-test$(EXESUF)
+check-qtest-mips64el-y += tests/qom-test$(EXESUF)
 check-qtest-ppc-y = tests/endianness-test$(EXESUF)
 check-qtest-ppc64-y = tests/endianness-test$(EXESUF)
 check-qtest-sh4-y = tests/endianness-test$(EXESUF)
 check-qtest-sh4eb-y = tests/endianness-test$(EXESUF)
+check-qtest-sh4-y += tests/qom-test$(EXESUF)
+check-qtest-sh4eb-y += tests/qom-test$(EXESUF)
 check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
 #check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
 #check-qtest-sparc64-y += tests/m48t59-test$(EXESUF)
 gcov-files-sparc-y += hw/m48t59.c
 gcov-files-sparc64-y += hw/m48t59.c
+check-qtest-sparc-y += tests/qom-test$(EXESUF)
+check-qtest-sparc64-y += tests/qom-test$(EXESUF)
 check-qtest-arm-y = tests/tmp105-test$(EXESUF)
 gcov-files-arm-y += hw/tmp105.c
+check-qtest-arm-y += tests/qom-test$(EXESUF)
 check-qtest-ppc-y += tests/boot-order-test$(EXESUF)
 check-qtest-ppc64-y += tests/boot-order-test$(EXESUF)
+check-qtest-ppc-y += tests/qom-test$(EXESUF)
+check-qtest-ppc64-y += tests/qom-test$(EXESUF)
+check-qtest-ppcemb-y += tests/qom-test$(EXESUF)
+check-qtest-alpha-y += tests/qom-test$(EXESUF)
+check-qtest-cris-y += tests/qom-test$(EXESUF)
+check-qtest-lm32-y += tests/qom-test$(EXESUF)
+check-qtest-m68k-y += tests/qom-test$(EXESUF)
+check-qtest-microblaze-y += tests/qom-test$(EXESUF)
+check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
+check-qtest-moxie-y += tests/qom-test$(EXESUF)
+check-qtest-or32-y += tests/qom-test$(EXESUF)
+check-qtest-s390x-y += tests/qom-test$(EXESUF)
+check-qtest-unicore32-y += tests/qom-test$(EXESUF)
+check-qtest-xtensa-y += tests/qom-test$(EXESUF)
+check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
 
 check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
 comments.json empty.json funny-char.json indented-expr.json \
@@ -174,6 +199,7 @@ tests/boot-order-test$(EXESUF): tests/boot-order-test.o 
$(libqos-obj-y)
 tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
 tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
 tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
+tests/qom-test$(EXESUF): tests/qom-test.o
 tests/qemu-iotests/socket_scm_helper$(EXESUF): 
tests/qemu-iotests/socket_scm_helper.o
 
 # QTest rules
diff --git a/tests/qom-test.c b/tests/qom-test.c
new file mode 100644
index 000..6ed23c5
--- /dev/null
+++ b/tests/qom-test.c
@@ -0,0 +1,253 @@
+/*
+ * QTest testcase for QOM
+ *
+ * Copyright (c) 2013 SUSE LINUX Products GmbH
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "libqtest.h"
+
+#include 
+#include 
+#include "qemu/osdep.h"
+
+static void test_nop(gconstpointer data)
+{
+QTestState *s;
+const char *machine = data;
+char *args;
+
+args = g_strdup_printf("-display none -machine %s", machine);
+s = qtest_start(args);
+if (s) {
+qtest_quit(s);
+}
+g_free(args);
+}
+
+static const char *x86_machines[] = {
+"pc",
+"isapc",
+"q35",
+};
+
+static const char *alpha_machines[] = {
+"clipper",
+};
+
+static const char *arm_machines[] = {
+"integratorcp",
+"versatilepb",
+"versatileab",
+"lm3s811evb",
+"lm3s6965evb",
+"collie",
+"akita",
+"spitz",
+"borzoi",
+"terrier",
+"tosa",
+"cheetah",
+"sx1-v1",
+"sx1",
+"realview-eb",
+"realview-eb-mpcore",
+"realview-pb-a8",
+"realview-pbx-a9",
+"musicpal",
+"mainstone",
+"connex",
+"verdex",
+"z2",
+"n800",
+"n810",
+"kzm",
+"vexpress-

[Qemu-devel] [PULL 35/58] arm11mpcore: Fix typo in MemoryRegion name

2013-10-08 Thread Andreas Färber
"mpcode" -> "mpcore"

Reviewed-by: Peter Maydell 
Signed-off-by: Andreas Färber 
---
 hw/cpu/arm11mpcore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/cpu/arm11mpcore.c b/hw/cpu/arm11mpcore.c
index a786c62..27cd32b 100644
--- a/hw/cpu/arm11mpcore.c
+++ b/hw/cpu/arm11mpcore.c
@@ -93,7 +93,7 @@ static void mpcore_priv_map_setup(ARM11MPCorePriveState *s)
 SysBusDevice *timerbusdev = SYS_BUS_DEVICE(s->mptimer);
 SysBusDevice *wdtbusdev = SYS_BUS_DEVICE(s->wdtimer);
 memory_region_init(&s->container, OBJECT(s),
-   "mpcode-priv-container", 0x2000);
+   "mpcore-priv-container", 0x2000);
 memory_region_init_io(&s->iomem, OBJECT(s),
   &mpcore_scu_ops, s, "mpcore-scu", 0x100);
 memory_region_add_subregion(&s->container, 0, &s->iomem);
-- 
1.8.1.4




  1   2   3   >